From ff03d8d0fe767a1498daeae5390f188a4fc918c1 Mon Sep 17 00:00:00 2001 From: Chinmay Pai Date: Thu, 30 Aug 2018 18:02:59 +0530 Subject: [PATCH 01/12] file-api: major refactor migrate from file_manager.py to file.py Signed-off-by: Chinmay Pai --- frappe/core/doctype/communication/email.py | 9 +- frappe/core/doctype/data_import/importer.py | 6 +- frappe/core/doctype/file/file.py | 445 +++++++++++++++++- frappe/handler.py | 3 +- .../gsuite_templates/gsuite_templates.py | 6 +- frappe/model/document.py | 7 +- frappe/website/doctype/web_form/web_form.py | 8 +- 7 files changed, 468 insertions(+), 16 deletions(-) diff --git a/frappe/core/doctype/communication/email.py b/frappe/core/doctype/communication/email.py index e79b592624..00f1dc9abf 100755 --- a/frappe/core/doctype/communication/email.py +++ b/frappe/core/doctype/communication/email.py @@ -397,8 +397,6 @@ def get_bcc(doc, recipients=None, fetched_from_email_account=False): def add_attachments(name, attachments): '''Add attachments to the given Communiction''' - from frappe.utils.file_manager import save_url - # loop through attachments for a in attachments: if isinstance(a, string_types): @@ -406,8 +404,11 @@ def add_attachments(name, attachments): ["file_name", "file_url", "is_private"], as_dict=1) # save attachments to new doc - save_url(attach.file_url, attach.file_name, "Communication", name, - "Home/Attachments", attach.is_private) + _file = frappe.get_doc("File", {"file_url": attach.file_url, + "attached_to_doctype": "Communication", "attached_to_name", name}) + + _file.save_url(folder="Home/Attachments") + def filter_email_list(doc, email_list, exclude, is_cc=False, is_bcc=False): # temp variables diff --git a/frappe/core/doctype/data_import/importer.py b/frappe/core/doctype/data_import/importer.py index 299eb1de9d..29294225d1 100644 --- a/frappe/core/doctype/data_import/importer.py +++ b/frappe/core/doctype/data_import/importer.py @@ -12,7 +12,6 @@ from frappe import _ from frappe.utils.csvutils import getlink from frappe.utils.dateutils import parse_date -from frappe.utils.file_manager import save_url from frappe.utils import cint, cstr, flt, getdate, get_datetime, get_url, get_url_to_form from six import text_type, string_types @@ -265,7 +264,10 @@ def upload(rows = None, submit_after_import=None, ignore_encoding_errors=False, # file is already attached return - save_url(file_url, None, doctype, docname, "Home/Attachments", 0) + _file = frappe.get_doc("File", {"file_url": file_url, "attached_to_name": docname, + "attached_to_doctype": doctype}) + _file.save_url(folder="Home/Attachments", df=0) + # header filename, file_extension = ['',''] diff --git a/frappe/core/doctype/file/file.py b/frappe/core/doctype/file/file.py index 150ce830b3..9227c743ab 100755 --- a/frappe/core/doctype/file/file.py +++ b/frappe/core/doctype/file/file.py @@ -11,24 +11,38 @@ naming for same name files: file.gif, file-1.gif, file-2.gif etc import frappe import json import os +import base64 +import re +import hashlib +import mimetypes +import io import shutil import requests import requests.exceptions import mimetypes, imghdr -from frappe.utils.file_manager import delete_file_data_content, get_content_hash, get_random_filename +from frappe.utils import get_hook_method, get_files_path, random_string, encode, cstr, call_hook_method, cint from frappe import _ +from frappe impoty conf +from copy import copy from frappe.utils.nestedset import NestedSet from frappe.utils import strip, get_files_path from PIL import Image, ImageOps from six import StringIO, string_types from six.moves.urllib.parse import unquote +from six.moves.urllib.parse import unquote +from six import text_type, PY2 import zipfile +class MaxFileSizeReachedError(frappe.ValidationError): + pass + + class FolderNotEmpty(frappe.ValidationError): pass exclude_from_linked_with = True + class File(NestedSet): nsm_parent_field = 'folder' no_feed_on_delete = True @@ -293,6 +307,180 @@ class File(NestedSet): frappe.delete_doc('File', self.name) + + def remove_file_by_url(self): + if self.attached_to_doctype and self.attached_to_name: + fid = frappe.db.get_value("File", {"file_url": self.file_url, + "attached_to_doctype": self.attached_to_doctype, "attached_to_name": self.attached_to_name}) + else: + fid = frappe.db.get_value("File", {"file_url": self.file_url}) + + if fid: + return remove_file(fid) + + + def get_file_url(self): + data = frappe.db.get_value("File", self.file_data_name, ["file_name", "file_url"], as_dict=True) + return data.file_url or data.file_name + + + def upload(self): + # get record details + self.dt = frappe.form_dict.doctype + self.xdn = frappe.form_dict.docname + self.df = frappe.form_dict.docfield + file_url = frappe.form_dict.file_url + self.filename = frappe.form_dict.filename + frappe.form_dict.is_private = cint(frappe.form_dict.is_private) + + if not self.filename and not self.file_url: + frappe.msgprint(_("Please select a file or url"), + raise_exception=True) + + file_doc = self.get_file_doc() + + comment = {} + if self.dt and self.dn: + comment = frappe.get_doc(self.dt, self.dn).add_comment("Attachment", + _ ("added {0}").format("{file_name}{icon}".format(**{ + "icon": ' ' \ + if file_doc.is_private else "", + "file_url": file_doc.file_url.replace("#", "%23") \ + if file_doc.file_name else file_doc.file_url, + "file_name": file_doc.file_name or file_doc.file_url + }))) + + return { + "name": file_doc.name, + "file_name": file_doc.file_name, + "file_url": file_doc.file_url, + "is_private": file_doc.is_private, + "comment": comment.as_dict() if comment else {} + } + + + def get_file_doc(self): + '''returns File object (Document) from given parameters or form_dict''' + r = frappe.form_dict + + if self.dt is None: self.dt = r.doctype + if self.dn is None: self.dn = r.docname + if self.df is None: self.df = r.docfield + if self.folder is None: self.folder = r.folder + if self.is_private is None: self.is_private = r.is_private + + if r.filedata: + file_doc = self.save_uploaded() + + elif r.file_url: + file_doc = self.save_url(self, r.file_url, r.filename) + + return file_doc + + + def save_uploaded(self): + self.fname, self.content = self.get_uploaded_content() + if self.content: + return self.save_file() + else: + raise Exception + + + def save_url(self, folder, df=None): + # if not (file_url.startswith("http://") or file_url.startswith("https://")): + # frappe.msgprint("URL must start with 'http://' or 'https://'") + # return None, None + + file_url = unquote(file_url) + file_size = frappe.form_dict.file_size + + f = frappe.get_doc({ + "doctype": "File", + "file_url": file_url, + "file_name": filename, + "attached_to_doctype": self.dt, + "attached_to_name": self.dn, + "attached_to_field": self.df, + "folder": self.folder, + "file_size": file_size, + "is_private": self.is_private + }) + f.flags.ignore_permissions = True + try: + f.insert() + except frappe.DuplicateEntryError: + return frappe.get_doc("File", f.duplicate_entry) + return f + + + def get_uploaded_content(self): + # should not be unicode when reading a file, hence using frappe.form + if 'filedata' in frappe.form_dict: + if "," in frappe.form_dict.filedata: + frappe.form_dict.filedata = frappe.form_dict.filedata.rsplit(",", 1)[1] + frappe.uploaded_content = base64.b64decode(frappe.form_dict.filedata) + frappe.uploaded_filename = frappe.form_dict.filename + return frappe.uploaded_filename, frappe.uploaded_content + else: + frappe.msgprint(_('No file attached')) + return None, None + + + def save_file(self, decode=False): + if decode: + if isinstance(self.content, text_type): + self.content = self.content.encode("utf-8") + + if b"," in self.content: + self.content = self.content.split(b",")[1] + self.content = base64.b64decode(self.content) + + file_size = check_max_file_size(self.content) + content_hash = get_content_hash(self.content) + self.content_type = mimetypes.guess_type(self.fname)[0] + self.fname = self.get_file_name(content_hash[-6:]) + file_data = get_file_data_from_hash(content_hash, is_private=self.is_private) + if not file_data: + call_hook_method("before_write_file", file_size=file_size) + + write_file_method = get_hook_method('write_file', fallback=self.save_file_on_filesystem) + file_data = write_file_method(self.fname, self.content, content_type=self.content_type, is_private=self.is_private) + file_data = copy(file_data) + + file_data.update({ + "doctype": "File", + "attached_to_doctype": self.dt, + "attached_to_name": self.dn, + "attached_to_field": self.df, + "folder": self.folder, + "file_size": file_size, + "content_hash": content_hash, + "is_private": self.is_private + }) + + f = frappe.get_doc(file_data) + f.flags.ignore_permissions = True + try: + f.insert() + except frappe.DuplicateEntryError: + return frappe.get_doc("File", f.duplicate_entry) + + return f + + def save_file_on_filesystem(self): + fpath = write_file(self.content, self.fname, self.is_private) + + if self.is_private: + file_url = "/private/files/{0}".format(self.fname) + else: + file_url = "/files/{0}".format(self.fname) + + return { + 'file_name': os.path.basename(fpath), + 'file_url': file_url + } + + def on_doctype_update(): frappe.db.add_index("File", ["attached_to_doctype", "attached_to_name"]) @@ -420,6 +608,248 @@ def get_web_image(file_url): return image, filename, extn +############################## +def get_max_file_size(): + return conf.get('max_file_size') or 10485760 + + +def check_max_file_size(content): + max_file_size = get_max_file_size() + file_size = len(content) + + if file_size > max_file_size: + frappe.msgprint(_("File size exceeded the maximum allowed size of {0} MB").format( + max_file_size / 1048576), + raise_exception=MaxFileSizeReachedError) + + return file_size + + +def get_file_data_from_hash(content_hash, is_private=0): + for name in frappe.db.sql_list("select name from `tabFile` where content_hash=%s and is_private=%s", (content_hash, is_private)): + b = frappe.get_doc('File', name) + return {k: b.get(k) for k in frappe.get_hooks()['write_file_keys']} + return False + +def write_file(content, fname, is_private=0): + """write file to disk with a random name (to compare)""" + file_path = get_files_path(is_private=is_private) + + # create directory (if not exists) + frappe.create_folder(file_path) + # write the file + if isinstance(content, text_type): + content = content.encode() + with open(os.path.join(file_path.encode('utf-8'), fname.encode('utf-8')), 'wb+') as f: + f.write(content) + + return get_files_path(fname, is_private=is_private) + + +def remove_all(dt, dn, from_delete=False): + """remove all files in a transaction""" + try: + for fid in frappe.db.sql_list("""select name from `tabFile` where + attached_to_doctype=%s and attached_to_name=%s""", (dt, dn)): + remove_file(fid, dt, dn, from_delete) + except Exception as e: + if e.args[0]!=1054: raise # (temp till for patched) + + +def remove_file(fid, attached_to_doctype=None, attached_to_name=None, from_delete=False): + """Remove file and File entry""" + file_name = None + if not (attached_to_doctype and attached_to_name): + attached = frappe.db.get_value("File", fid, + ["attached_to_doctype", "attached_to_name", "file_name"]) + if attached: + attached_to_doctype, attached_to_name, file_name = attached + + ignore_permissions, comment = False, None + if attached_to_doctype and attached_to_name and not from_delete: + doc = frappe.get_doc(attached_to_doctype, attached_to_name) + ignore_permissions = doc.has_permission("write") or False + if frappe.flags.in_web_form: + ignore_permissions = True + if not file_name: + file_name = frappe.db.get_value("File", fid, "file_name") + comment = doc.add_comment("Attachment Removed", _("Removed {0}").format(file_name)) + + frappe.delete_doc("File", fid, ignore_permissions=ignore_permissions) + + return comment + + +def delete_file_data_content(doc, only_thumbnail=False): + method = get_hook_method('delete_file_data_content', fallback=delete_file_from_filesystem) + method(doc, only_thumbnail=only_thumbnail) + + +def delete_file_from_filesystem(doc, only_thumbnail=False): + """Delete file, thumbnail from File document""" + if only_thumbnail: + delete_file(doc.thumbnail_url) + else: + delete_file(doc.file_url) + delete_file(doc.thumbnail_url) + + +def delete_file(path): + """Delete file from `public folder`""" + if path: + if ".." in path.split("/"): + frappe.msgprint(_("It is risky to delete this file: {0}. Please contact your System Manager.").format(path)) + + parts = os.path.split(path.strip("/")) + if parts[0]=="files": + path = frappe.utils.get_site_path("public", "files", parts[-1]) + + else: + path = frappe.utils.get_site_path("private", "files", parts[-1]) + + path = encode(path) + if os.path.exists(path): + os.remove(path) + + +def get_file(fname): + """Returns [`file_name`, `content`] for given file name `fname`""" + file_path = get_file_path(fname) + + # read the file + if PY2: + with open(encode(file_path)) as f: + content = f.read() + else: + with io.open(encode(file_path), mode='rb') as f: + content = f.read() + try: + # for plain text files + content = content.decode() + except UnicodeDecodeError: + # for .png, .jpg, etc + pass + + return [file_path.rsplit("/", 1)[-1], content] + + +def get_file_path(file_name): + """Returns file path from given file name""" + f = frappe.db.sql("""select file_url from `tabFile` + where name=%s or file_name=%s""", (file_name, file_name)) + if f: + file_name = f[0][0] + + file_path = file_name + + if "/" not in file_path: + file_path = "/files/" + file_path + + if file_path.startswith("/private/files/"): + file_path = get_files_path(*file_path.split("/private/files/", 1)[1].split("/"), is_private=1) + + elif file_path.startswith("/files/"): + file_path = get_files_path(*file_path.split("/files/", 1)[1].split("/")) + + else: + frappe.throw(_("There is some problem with the file url: {0}").format(file_path)) + + return file_path + + +def get_content_hash(content): + if isinstance(content, text_type): + content = content.encode() + return hashlib.md5(content).hexdigest() + + +def get_file_name(fname, optional_suffix): + # convert to unicode + fname = cstr(fname) + + n_records = frappe.db.sql("select name from `tabFile` where file_name=%s", fname) + if len(n_records) > 0 or os.path.exists(encode(get_files_path(fname))): + f = fname.rsplit('.', 1) + if len(f) == 1: + partial, extn = f[0], "" + else: + partial, extn = f[0], "." + f[1] + return '{partial}{suffix}{extn}'.format(partial=partial, extn=extn, suffix=optional_suffix) + return fname + + +@frappe.whitelist() +def download_file(file_url): + """ + Download file using token and REST API. Valid session or + token is required to download private files. + + Method : GET + Endpoint : frappe.utils.file_manager.download_file + URL Params : file_name = /path/to/file relative to site path + """ + file_doc = frappe.get_doc("File", {"file_url":file_url}) + file_doc.check_permission("read") + path = os.path.join(get_files_path(), os.path.basename(file_url)) + + with open(path, "rb") as fileobj: + filedata = fileobj.read() + frappe.local.response.filename = os.path.basename(file_url) + frappe.local.response.filecontent = filedata + frappe.local.response.type = "download" + +def extract_images_from_doc(doc, fieldname): + content = doc.get(fieldname) + content = extract_images_from_html(doc, content) + if frappe.flags.has_dataurl: + doc.set(fieldname, content) + + +def extract_images_from_html(doc, content): + frappe.flags.has_dataurl = False + + def _save_file(match): + data = match.group(1) + data = data.split("data:")[1] + headers, content = data.split(",") + + if "filename=" in headers: + filename = headers.split("filename=")[-1] + + # decode filename + if not isinstance(filename, text_type): + filename = text_type(filename, 'utf-8') + else: + mtype = headers.split(";")[0] + filename = get_random_filename(content_type=mtype) + + doctype = doc.parenttype if doc.parent else doc.doctype + name = doc.parent or doc.name + + # TODO fix this + file_url = save_file(filename, content, doctype, name, decode=True).get("file_url") + if not frappe.flags.has_dataurl: + frappe.flags.has_dataurl = True + + return ']*src\s*=\s*["\'](?=data:)(.*?)["\']', _save_file, content) + + return content + + +def get_random_filename(extn=None, content_type=None): + if extn: + if not extn.startswith("."): + extn = "." + extn + + elif content_type: + extn = mimetypes.guess_extension(content_type) + + return random_string(7) + (extn or "") + + def check_file_permission(file_url): for file in frappe.get_all("File", filters={"file_url": file_url, "is_private": 1}, fields=["name", "attached_to_doctype", "attached_to_name"]): @@ -429,12 +859,14 @@ def check_file_permission(file_url): raise frappe.PermissionError + @frappe.whitelist() def unzip_file(name): '''Unzip the given file and make file records for each of the extracted files''' file_obj = frappe.get_doc('File', name) file_obj.unzip() + @frappe.whitelist() def get_attached_images(doctype, names): '''get list of image urls attached in form @@ -456,5 +888,14 @@ def get_attached_images(doctype, names): return out + +@frappe.whitelist() +def validate_filename(filename): + from frappe.utils import now_datetime + timestamp = now_datetime().strftime(" %Y-%m-%d %H:%M:%S") + fname = get_file_name(filename, timestamp) + return fname + + def on_doctype_update(): - frappe.db.add_index("File", ["lft", "rgt"]) \ No newline at end of file + frappe.db.add_index("File", ["lft", "rgt"]) diff --git a/frappe/handler.py b/frappe/handler.py index 16138edf62..82a19ec558 100755 --- a/frappe/handler.py +++ b/frappe/handler.py @@ -111,7 +111,8 @@ def uploadfile(): try: if frappe.form_dict.get('from_form'): try: - ret = frappe.utils.file_manager.upload() + ret = frappe.get_doc({"doctype": "File"}) + ret = ret.upload() except frappe.DuplicateEntryError: # ignore pass ret = None diff --git a/frappe/integrations/doctype/gsuite_templates/gsuite_templates.py b/frappe/integrations/doctype/gsuite_templates/gsuite_templates.py index ff85ac8398..375a02b765 100644 --- a/frappe/integrations/doctype/gsuite_templates/gsuite_templates.py +++ b/frappe/integrations/doctype/gsuite_templates/gsuite_templates.py @@ -7,7 +7,6 @@ import frappe from frappe import _ from frappe.model.document import Document from frappe.integrations.doctype.gsuite_settings.gsuite_settings import run_gsuite_script -from frappe.utils.file_manager import save_url class GSuiteTemplates(Document): pass @@ -25,7 +24,10 @@ def create_gsuite_doc(doctype, docname, gs_template=None): r = run_gsuite_script('new', filename, templ.template_id, templ.destination_id, json_data) - filedata = save_url(r['url'], filename, doctype, docname, "Home/Attachments", True) + _file = frappe.get_doc("File", {"file_url": r['url'], "file_name": filename, + "attached_to_doctype": doctype, "attached_to_name": docname}) + _file.save_url(folder="Home/Attachments", df=True) + comment = frappe.get_doc(doctype, docname).add_comment("Attachment", _("added {0}").format("{file_name}{icon}".format(**{ "icon": ' ' if filedata.is_private else "", diff --git a/frappe/model/document.py b/frappe/model/document.py index 7b64a9b70e..9d8ec90c3e 100644 --- a/frappe/model/document.py +++ b/frappe/model/document.py @@ -14,7 +14,6 @@ from werkzeug.exceptions import NotFound, Forbidden import hashlib, json from frappe.model import optional_fields from frappe.model.workflow import validate_workflow -from frappe.utils.file_manager import save_url from frappe.utils.global_search import update_global_search from frappe.integrations.doctype.webhook import run_webhooks @@ -320,7 +319,11 @@ class Document(BaseDocument): for attach_item in get_attachments(self.doctype, self.amended_from): #save attachments to new doc - save_url(attach_item.file_url, attach_item.file_name, self.doctype, self.name, "Home/Attachments", attach_item.is_private) + _file = frappe.get_doc("File", {"file_url": attach_item.file_url, + "file_name": attach_item.file_name, "attached_to_name": self.name, + "attached_to_doctype": self.doctype}) + _file.save_url(folder="Home/Attachments") + def update_children(self): '''update child tables''' diff --git a/frappe/website/doctype/web_form/web_form.py b/frappe/website/doctype/web_form/web_form.py index c75bf7fb77..111dfab450 100644 --- a/frappe/website/doctype/web_form/web_form.py +++ b/frappe/website/doctype/web_form/web_form.py @@ -6,7 +6,7 @@ import frappe, json, os from frappe.website.website_generator import WebsiteGenerator from frappe import _, scrub from frappe.utils import cstr -from frappe.utils.file_manager import save_file, remove_file_by_url +from frappe.utils.file_manager import save_file from frappe.website.utils import get_comment_list from frappe.custom.doctype.customize_form.customize_form import docfield_properties from frappe.utils.file_manager import get_max_file_size @@ -416,7 +416,8 @@ def accept(web_form, data, for_payment=False): # remove earlier attached file (if exists) if doc.get(fieldname): - remove_file_by_url(doc.get(fieldname), doc.doctype, doc.name) + file = frappe.get_doc("File", {"attached_to_doctype": doc.doctype, "file_url": doc.get(fieldname), "attached_to_name": doc.name}) + file.remove_file_by_url() # save new file filename, dataurl = filedata.split(',', 1) @@ -431,7 +432,8 @@ def accept(web_form, data, for_payment=False): if files_to_delete: for f in files_to_delete: if f: - remove_file_by_url(f, doc.doctype, doc.name) + file = frappe.get_doc("File", {"attached_to_doctype": doc.doctype, "file_url": doc.get(fieldname), "attached_to_name": doc.name}) + file.remove_file_by_url() frappe.flags.web_form_doc = doc From 16a99f5472a8f771e0a694bd2c77cfec1c5cd9a8 Mon Sep 17 00:00:00 2001 From: Chinmay Pai Date: Fri, 31 Aug 2018 01:55:15 +0530 Subject: [PATCH 02/12] [1/3] file-api: code migration migrate api from file_manager.py to file.py Signed-off-by: Chinmay Pai --- frappe/client.py | 5 ++-- frappe/core/doctype/data_import/importer.py | 6 ++-- frappe/core/doctype/file/file.py | 29 +++++++++++-------- frappe/core/doctype/file/test_file.py | 23 ++++++++++----- .../prepared_report/prepared_report.py | 13 +++------ frappe/desk/page/setup_wizard/setup_wizard.py | 5 ++-- frappe/email/receive.py | 8 +++-- frappe/tests/test_filemanager.py | 22 ++++++++++---- frappe/twofactor.py | 5 ++-- frappe/utils/csvutils.py | 4 +-- frappe/website/doctype/web_form/web_form.py | 6 ++-- 11 files changed, 74 insertions(+), 52 deletions(-) diff --git a/frappe/client.py b/frappe/client.py index 698b3a172d..247a3151e6 100644 --- a/frappe/client.py +++ b/frappe/client.py @@ -9,7 +9,6 @@ import frappe.utils import json, os from six import iteritems, string_types, integer_types -from frappe.utils.file_manager import save_file ''' Handle RESTful requests that are mapped to the `/api/resource` route. @@ -351,7 +350,9 @@ def attach_file(filename=None, filedata=None, doctype=None, docname=None, folder if not doc.has_permission(): frappe.throw(_("Not permitted"), frappe.PermissionError) - f = save_file(filename, filedata, doctype, docname, folder, decode_base64, is_private, docfield) + _file = frappe.get_doc("File", {"file_name": filename, "content": filedata, + "attached_to_doctype": doctype, "attached_to_name": docname}) + f = _file.save_file(folder=folder, decode=decode_base64, is_private=is_private, df=docfield) if docfield and doctype: doc.set(docfield, f.file_url) diff --git a/frappe/core/doctype/data_import/importer.py b/frappe/core/doctype/data_import/importer.py index 29294225d1..ac00851ebf 100644 --- a/frappe/core/doctype/data_import/importer.py +++ b/frappe/core/doctype/data_import/importer.py @@ -457,7 +457,6 @@ def upload(rows = None, submit_after_import=None, ignore_encoding_errors=False, if error_flag and data_import_doc.skip_errors and len(data) != len(data_rows_with_error): import_status = "Partially Successful" # write the file with the faulty row - from frappe.utils.file_manager import save_file file_name = 'error_' + filename + file_extension if file_extension == '.xlsx': from frappe.utils.xlsxutils import make_xlsx @@ -466,8 +465,9 @@ def upload(rows = None, submit_after_import=None, ignore_encoding_errors=False, else: from frappe.utils.csvutils import to_csv file_data = to_csv(data_rows_with_error) - error_data_file = save_file(file_name, file_data, "Data Import", - data_import_doc.name, "Home/Attachments") + _file = frappe.get_doc("File", {"file_name": file_name, "content": file_data, + "attached_to_doctype": "Data Import", "attached_to_name": data_import_doc.name}) + error_data_file = _file.save_file(folder="Home/Attachments") data_import_doc.error_file = error_data_file.file_url elif error_flag: diff --git a/frappe/core/doctype/file/file.py b/frappe/core/doctype/file/file.py index 9227c743ab..b3624bed60 100755 --- a/frappe/core/doctype/file/file.py +++ b/frappe/core/doctype/file/file.py @@ -421,12 +421,11 @@ class File(NestedSet): frappe.uploaded_content = base64.b64decode(frappe.form_dict.filedata) frappe.uploaded_filename = frappe.form_dict.filename return frappe.uploaded_filename, frappe.uploaded_content - else: - frappe.msgprint(_('No file attached')) + frappe.msgprint(_('No file attached')) return None, None - def save_file(self, decode=False): + def save_file(self, folder=None, decode=False, is_private=0, df=None): if decode: if isinstance(self.content, text_type): self.content = self.content.encode("utf-8") @@ -436,10 +435,11 @@ class File(NestedSet): self.content = base64.b64decode(self.content) file_size = check_max_file_size(self.content) - content_hash = get_content_hash(self.content) + self.content_hash = get_content_hash(self.content) self.content_type = mimetypes.guess_type(self.fname)[0] self.fname = self.get_file_name(content_hash[-6:]) - file_data = get_file_data_from_hash(content_hash, is_private=self.is_private) + self.is_private = is_private + file_data = self.get_file_data_from_hash(is_private=self.is_private) if not file_data: call_hook_method("before_write_file", file_size=file_size) @@ -467,6 +467,15 @@ class File(NestedSet): return f + + def get_file_data_from_hash(self, is_private=0): + for name in frappe.db.sql_list("select name from `tabFile` where content_hash=%s and is_private=%s", + (self.content_hash, is_private)): + b = frappe.get_doc('File', name) + return {k: b.get(k) for k in frappe.get_hooks()['write_file_keys']} + return False + + def save_file_on_filesystem(self): fpath = write_file(self.content, self.fname, self.is_private) @@ -625,12 +634,6 @@ def check_max_file_size(content): return file_size -def get_file_data_from_hash(content_hash, is_private=0): - for name in frappe.db.sql_list("select name from `tabFile` where content_hash=%s and is_private=%s", (content_hash, is_private)): - b = frappe.get_doc('File', name) - return {k: b.get(k) for k in frappe.get_hooks()['write_file_keys']} - return False - def write_file(content, fname, is_private=0): """write file to disk with a random name (to compare)""" file_path = get_files_path(is_private=is_private) @@ -827,7 +830,9 @@ def extract_images_from_html(doc, content): name = doc.parent or doc.name # TODO fix this - file_url = save_file(filename, content, doctype, name, decode=True).get("file_url") + _file = frappe.get_doc("File", {"file_name": filename, "content": content, + "attached_to_doctype": doctype, "attached_to_name": name}) + file_url = _file.save_file(decode=True).file_url if not frappe.flags.has_dataurl: frappe.flags.has_dataurl = True diff --git a/frappe/core/doctype/file/test_file.py b/frappe/core/doctype/file/test_file.py index 05dc677df6..fbf61fbff5 100644 --- a/frappe/core/doctype/file/test_file.py +++ b/frappe/core/doctype/file/test_file.py @@ -5,7 +5,7 @@ from __future__ import unicode_literals import frappe import unittest -from frappe.utils.file_manager import save_file, get_files_path +from frappe.utils.file_manager import get_files_path from frappe import _ from frappe.core.doctype.file.file import move_file # test_records = frappe.get_test_records('File') @@ -27,8 +27,9 @@ class TestFile(unittest.TestCase): frappe.delete_doc("File", f[0]) def upload_file(self): - self.saved_file = save_file('file_copy.txt', "Testing file copy example.",\ - "", "", self.get_folder("Test Folder 1", "Home").name) + _file = frappe.get_doc("File", {"file_name": "file_copy.txt", "content": "Testing file copy example.", + "attached_to_name": "", "attached_to_doctype": ""}) + self.saved_file = _file.save_file(folder=self.get_folder("Test Folder 1", "Home").name) self.saved_filename = get_files_path(self.saved_file.file_name) def get_folder(self, folder_name, parent_folder="Home"): @@ -61,8 +62,9 @@ class TestFile(unittest.TestCase): def test_folder_copy(self): folder = self.get_folder("Test Folder 2", "Home") folder = self.get_folder("Test Folder 3", "Home/Test Folder 2") - - self.saved_file = save_file('folder_copy.txt', "Testing folder copy example.", "", "", folder.name) + _file = frappe.get_doc("File", {"file_name": "folder_copy.txt", "content": "Testing folder copy example.", + "attached_to_name": "", "attached_to_doctype": ""}) + self.saved_file = _file.save_file(folder=folder.name) move_file([{"name": folder.name}], 'Home/Test Folder 1', folder.folder) @@ -91,7 +93,9 @@ class TestFile(unittest.TestCase): self.assertEqual(frappe.db.get_value("File", _("Home/Test Folder 1"), "file_size"), 0) folder = self.get_folder("Test Folder 3", "Home/Test Folder 1") - self.saved_file = save_file('folder_copy.txt', "Testing folder copy example.", "", "", folder.name) + _file = frappe.get_doc("File", {"file_name": "folder_copy.txt", "content": "Testing folder copy example.", + "attached_to_name": "", "attached_to_doctype": ""}) + self.saved_file = _file.save_file(folder=folder.name) folder = frappe.get_doc("File", "Home/Test Folder 1/Test Folder 3") self.assertRaises(frappe.ValidationError, folder.delete) @@ -114,8 +118,11 @@ class TestFile(unittest.TestCase): # Rebuild the frappe.local.conf to take up the changes from site_config frappe.local.conf = _dict(frappe.get_site_config()) - self.assertRaises(MaxFileSizeReachedError, save_file, '_test_max_space.txt', - 'This files test for max space usage', "", "", self.get_folder("Test Folder 2", "Home").name) + _file = frappe.get_doc("File", {"file_name": "_test_max_space.txt", + "content": "This file tests for max space usage", + "attached_to_name": "", "attached_to_doctype": ""}) + self.assertRaises(MaxFileSizeReachedError, + _file.save_file(self.get_folder("Test Folder 2", "Home").name)) # Scrub the site_config and rebuild frappe.local.conf clear_limit("space") diff --git a/frappe/core/doctype/prepared_report/prepared_report.py b/frappe/core/doctype/prepared_report/prepared_report.py index 3d8481c696..9565a18092 100644 --- a/frappe/core/doctype/prepared_report/prepared_report.py +++ b/frappe/core/doctype/prepared_report/prepared_report.py @@ -11,7 +11,7 @@ import frappe from frappe.model.document import Document from frappe.utils.background_jobs import enqueue from frappe.desk.query_report import generate_report_result, get_columns_dict -from frappe.utils.file_manager import save_file, remove_all +from frappe.utils.file_manager import remove_all from frappe.utils.csvutils import to_csv, read_csv_content_from_attached_file from frappe.desk.form.load import get_attachments from frappe.utils.file_manager import download_file @@ -62,14 +62,9 @@ def create_csv_file(columns, data, dt, dn): rows = [tuple(columns)] + data encoded = base64.b64encode(frappe.safe_encode(to_csv(rows))) # Call save_file function to upload and attach the file - save_file( - fname=csv_filename, - content=encoded, - dt=dt, - dn=dn, - folder=None, - decode=True, - is_private=False) + _file = frappe.get_doc("File", {"file_name": csv_filename, "content": encoded, + "attached_to_doctype": dt, "attached_to_name": dn}) + _file.save_file(decode=True) @frappe.whitelist() diff --git a/frappe/desk/page/setup_wizard/setup_wizard.py b/frappe/desk/page/setup_wizard/setup_wizard.py index eef203ab04..ee409d4b57 100755 --- a/frappe/desk/page/setup_wizard/setup_wizard.py +++ b/frappe/desk/page/setup_wizard/setup_wizard.py @@ -7,7 +7,6 @@ import frappe, json, os from frappe.utils import strip, cint from frappe.translate import (set_default_language, get_dict, send_translations) from frappe.geo.country_info import get_country_info -from frappe.utils.file_manager import save_file from frappe.utils.password import update_password from werkzeug.useragents import UserAgent from . import install_fixtures @@ -187,7 +186,9 @@ def update_user_name(args): attach_user = args.get("attach_user").split(",") if len(attach_user)==3: filename, filetype, content = attach_user - fileurl = save_file(filename, content, "User", args.get("name"), decode=True).file_url + _file = frappe.get_doc("File", {"file_name": filename, "content": content, + "attached_to_doctype": "User", "attached_to_doctype": args.get("name")}) + fileurl = _file.save_file(decode=True).file_url frappe.db.set_value("User", args.get("name"), "user_image", fileurl) if args.get('name'): diff --git a/frappe/email/receive.py b/frappe/email/receive.py index f77e8a80d9..0282eb4bf6 100644 --- a/frappe/email/receive.py +++ b/frappe/email/receive.py @@ -13,7 +13,7 @@ from frappe import _, safe_decode, safe_encode from frappe.utils import (extract_email_id, convert_utc_to_user_timezone, now, cint, cstr, strip, markdown, parse_addr) from frappe.utils.scheduler import log -from frappe.utils.file_manager import get_random_filename, save_file, MaxFileSizeReachedError +from frappe.utils.file_manager import get_random_filename, MaxFileSizeReachedError class EmailSizeExceededError(frappe.ValidationError): pass class EmailTimeoutError(frappe.ValidationError): pass @@ -523,8 +523,10 @@ class Email: for attachment in self.attachments: try: - file_data = save_file(attachment['fname'], attachment['fcontent'], - doc.doctype, doc.name, is_private=1) + _file = frappe.get_doc("File", {"file_name": attachment['fname'], + "content": atachment['fcontent'], "attached_to_doctype": doc.doctype, + "attached_to_name": doc.name}) + file_data = _file.save_file(is_private=1) saved_attachments.append(file_data) if attachment['fname'] in self.cid_map: diff --git a/frappe/tests/test_filemanager.py b/frappe/tests/test_filemanager.py index 7fc203e912..a25c5a7503 100644 --- a/frappe/tests/test_filemanager.py +++ b/frappe/tests/test_filemanager.py @@ -6,7 +6,7 @@ import frappe import os import unittest -from frappe.utils.file_manager import save_file, get_file, get_files_path +from frappe.utils.file_manager import get_file, get_files_path test_content1 = 'Hello' test_content2 = 'Hello World' @@ -22,7 +22,9 @@ class TestSimpleFile(unittest.TestCase): def setUp(self): self.attached_to_doctype, self.attached_to_docname = make_test_doc() self.test_content = test_content1 - self.saved_file = save_file('hello.txt', self.test_content, self.attached_to_doctype, self.attached_to_docname) + _file = frappe.get_doc("File", {"file_name": "hello.txt", "content": self.test_content, + "attached_to_doctype": self.attached_to_doctype, "attached_to_docname": self.attached_to_docname}) + self.saved_file = _file.save_file() self.saved_filename = get_files_path(self.saved_file.file_name) def test_save(self): @@ -40,8 +42,12 @@ class TestSameFileName(unittest.TestCase): self.attached_to_doctype, self.attached_to_docname = make_test_doc() self.test_content1 = test_content1 self.test_content2 = test_content2 - self.saved_file1 = save_file('hello.txt', self.test_content1, self.attached_to_doctype, self.attached_to_docname) - self.saved_file2 = save_file('hello.txt', self.test_content2, self.attached_to_doctype, self.attached_to_docname) + _file1 = frappe.get_doc("File", {"file_name": "hello.txt", "content": self.test_content1, + "attached_to_doctype": self.attached_to_doctype, "attached_to_docname": self.attached_to_docname}) + _file2 = frappe.get_doc("File", {"file_name": "hello.txt", "content": self.test_content2, + "attached_to_doctype": self.attached_to_doctype, "attached_to_docname": self.attached_to_docname}) + self.saved_file1 = _file1.save_file() + self.saved_file2 = _file2.save_file() self.saved_filename1 = get_files_path(self.saved_file1.file_name) self.saved_filename2 = get_files_path(self.saved_file2.file_name) @@ -65,8 +71,12 @@ class TestSameContent(unittest.TestCase): self.test_content2 = test_content1 self.orig_filename = 'hello.txt' self.dup_filename = 'hello2.txt' - self.saved_file1 = save_file(self.orig_filename, self.test_content1, self.attached_to_doctype1, self.attached_to_docname1) - self.saved_file2 = save_file(self.dup_filename, self.test_content2, self.attached_to_doctype2, self.attached_to_docname2) + _file1 = frappe.get_doc("File", {"file_name": self.orig_filename, "content": self.test_content1, + "attached_to_doctype": self.attached_to_doctype1, "attached_to_docname": self.attached_to_docname1}) + _file2 = frappe.get_doc("File", {"file_name": self.dup_filename, "content": self.test_content2, + "attached_to_doctype": self.attached_to_doctype2, "attached_to_docname": self.attached_to_docname2}) + self.saved_file1 = _file1.save_file() + self.saved_file2 = _file2.save_file() self.saved_filename1 = get_files_path(self.saved_file1.file_name) self.saved_filename2 = get_files_path(self.saved_file2.file_name) diff --git a/frappe/twofactor.py b/frappe/twofactor.py index 35d7a28fcb..f35ea2700a 100644 --- a/frappe/twofactor.py +++ b/frappe/twofactor.py @@ -333,10 +333,11 @@ def get_qr_svg_code(totp_uri): def qrcode_as_png(user, totp_uri): '''Save temporary Qrcode to server.''' - from frappe.utils.file_manager import save_file folder = create_barcode_folder() png_file_name = '{}.png'.format(frappe.generate_hash(length=20)) - file_obj = save_file(png_file_name, png_file_name, 'User', user, folder=folder) + _file = frappe.get_doc("File", {"file_name": png_file_name, "content": png_file_name, + "attached_to_doctype": 'User', "attached_to_name": user}) + file_obj = _file.save_file(folder=folder) frappe.db.commit() file_url = get_url(file_obj.file_url) file_path = os.path.join(frappe.get_site_path('public', 'files'), file_obj.file_name) diff --git a/frappe/utils/csvutils.py b/frappe/utils/csvutils.py index 7476eac0c3..b9159a4bee 100644 --- a/frappe/utils/csvutils.py +++ b/frappe/utils/csvutils.py @@ -15,8 +15,8 @@ def read_csv_content_from_uploaded_file(ignore_encoding=False): with open(frappe.uploaded_file, "r") as upfile: fcontent = upfile.read() else: - from frappe.utils.file_manager import get_uploaded_content - fname, fcontent = get_uploaded_content() + _file = frappe.get_doc("File") + fname, fcontent = _file.get_uploaded_content() return read_csv_content(fcontent, ignore_encoding) def read_csv_content_from_attached_file(doc): diff --git a/frappe/website/doctype/web_form/web_form.py b/frappe/website/doctype/web_form/web_form.py index 111dfab450..d8d2418c45 100644 --- a/frappe/website/doctype/web_form/web_form.py +++ b/frappe/website/doctype/web_form/web_form.py @@ -6,7 +6,6 @@ import frappe, json, os from frappe.website.website_generator import WebsiteGenerator from frappe import _, scrub from frappe.utils import cstr -from frappe.utils.file_manager import save_file from frappe.website.utils import get_comment_list from frappe.custom.doctype.customize_form.customize_form import docfield_properties from frappe.utils.file_manager import get_max_file_size @@ -421,8 +420,9 @@ def accept(web_form, data, for_payment=False): # save new file filename, dataurl = filedata.split(',', 1) - filedoc = save_file(filename, dataurl, - doc.doctype, doc.name, decode=True) + _file = frappe.get_doc("File", {"file_name": filename, "content": dataurl, + "attached_to_doctype": doc.doctype, "attached_to_name": doc.name}) + filedoc = _file.save_file(decode=True) # update values doc.set(fieldname, filedoc.file_url) From 22ba310aaf4ef16f96552b4e7874b36fb48b85fb Mon Sep 17 00:00:00 2001 From: Chinmay Pai Date: Mon, 3 Sep 2018 18:04:45 +0530 Subject: [PATCH 03/12] [2/3] file-api: code migration migrate api from file_manager.py to file.py Signed-off-by: Chinmay Pai --- frappe/core/doctype/communication/email.py | 2 +- frappe/core/doctype/data_import/importer.py | 2 +- frappe/core/doctype/file/file.py | 200 +++++++++--------- frappe/core/doctype/file/test_file.py | 2 +- .../prepared_report/prepared_report.py | 4 +- frappe/desk/form/utils.py | 5 +- .../email_account/test_email_account.py | 7 +- frappe/email/email_body.py | 2 +- frappe/email/queue.py | 2 +- frappe/email/receive.py | 2 +- frappe/handler.py | 1 - frappe/limits.py | 2 +- frappe/model/delete_doc.py | 2 +- frappe/patches/v4_0/file_manager_hooks.py | 2 +- frappe/patches/v4_1/file_manager_fix.py | 2 +- frappe/public/js/frappe/upload.js | 2 +- frappe/tests/test_filemanager.py | 2 +- frappe/utils/csvutils.py | 2 +- frappe/utils/xlsxutils.py | 2 +- frappe/website/doctype/web_form/web_form.py | 10 +- 20 files changed, 129 insertions(+), 126 deletions(-) diff --git a/frappe/core/doctype/communication/email.py b/frappe/core/doctype/communication/email.py index 00f1dc9abf..6303eb7fe3 100755 --- a/frappe/core/doctype/communication/email.py +++ b/frappe/core/doctype/communication/email.py @@ -10,7 +10,7 @@ from email.utils import formataddr from frappe.core.utils import get_parent_doc from frappe.utils import (get_url, get_formatted_email, cint, validate_email_add, split_emails, time_diff_in_seconds, parse_addr, get_datetime) -from frappe.utils.file_manager import get_file +from frappe.core.doctype.file.file import get_file from frappe.email.queue import check_email_limit from frappe.utils.scheduler import log from frappe.email.email_body import get_message_id diff --git a/frappe/core/doctype/data_import/importer.py b/frappe/core/doctype/data_import/importer.py index ac00851ebf..69a1fa3190 100644 --- a/frappe/core/doctype/data_import/importer.py +++ b/frappe/core/doctype/data_import/importer.py @@ -272,7 +272,7 @@ def upload(rows = None, submit_after_import=None, ignore_encoding_errors=False, # header filename, file_extension = ['',''] if not rows: - from frappe.utils.file_manager import get_file # get_file_doc + from frappe.core.doctype.file.file import get_file # get_file_doc fname, fcontent = get_file(data_import_doc.import_file) filename, file_extension = os.path.splitext(fname) diff --git a/frappe/core/doctype/file/file.py b/frappe/core/doctype/file/file.py index b3624bed60..c9cdb25c55 100755 --- a/frappe/core/doctype/file/file.py +++ b/frappe/core/doctype/file/file.py @@ -199,7 +199,7 @@ class File(NestedSet): self.check_folder_is_empty() self.check_reference_doc_permission() super(File, self).on_trash() - self.delete_file() + self.call_delete_file() def make_thumbnail(self, set_as_thumbnail=True, width=300, height=300, suffix="small", crop=False): if self.file_url: @@ -265,14 +265,14 @@ class File(NestedSet): except frappe.DoesNotExistError: pass - def delete_file(self): + def call_delete_file(self): """If file not attached to any other record, delete it""" if self.file_name and self.content_hash and (not frappe.db.count("File", {"content_hash": self.content_hash, "name": ["!=", self.name]})): - delete_file_data_content(self) + self.delete_file_data_content() elif self.file_url: - delete_file_data_content(self, only_thumbnail=True) + self.delete_file_data_content(only_thumbnail=True) def on_rollback(self): self.flags.on_rollback = True @@ -308,17 +308,6 @@ class File(NestedSet): frappe.delete_doc('File', self.name) - def remove_file_by_url(self): - if self.attached_to_doctype and self.attached_to_name: - fid = frappe.db.get_value("File", {"file_url": self.file_url, - "attached_to_doctype": self.attached_to_doctype, "attached_to_name": self.attached_to_name}) - else: - fid = frappe.db.get_value("File", {"file_url": self.file_url}) - - if fid: - return remove_file(fid) - - def get_file_url(self): data = frappe.db.get_value("File", self.file_data_name, ["file_name", "file_url"], as_dict=True) return data.file_url or data.file_name @@ -434,10 +423,10 @@ class File(NestedSet): self.content = self.content.split(b",")[1] self.content = base64.b64decode(self.content) - file_size = check_max_file_size(self.content) + file_size = self.check_max_file_size() self.content_hash = get_content_hash(self.content) self.content_type = mimetypes.guess_type(self.fname)[0] - self.fname = self.get_file_name(content_hash[-6:]) + self.fname = get_file_name(self.fname, self.content_hash[-6:]) self.is_private = is_private file_data = self.get_file_data_from_hash(is_private=self.is_private) if not file_data: @@ -477,7 +466,7 @@ class File(NestedSet): def save_file_on_filesystem(self): - fpath = write_file(self.content, self.fname, self.is_private) + fpath = self.write_file(self, is_private=self.is_private) if self.is_private: file_url = "/private/files/{0}".format(self.fname) @@ -490,6 +479,89 @@ class File(NestedSet): } + def check_max_file_size(self): + max_file_size = get_max_file_size() + file_size = len(self.content) + + if file_size > max_file_size: + frappe.msgprint(_("File size exceeded the maximum allowed size of {0} MB").format( + max_file_size / 1048576), + raise_exception=MaxFileSizeReachedError) + + return file_size + + + def write_file(self, is_private=0): + """write file to disk with a random name (to compare)""" + file_path = get_files_path(is_private=is_private) + + # create directory (if not exists) + frappe.create_folder(file_path) + # write the file + if isinstance(self.content, text_type): + self.content = self.content.encode() + with open(os.path.join(file_path.encode('utf-8'), self.fname.encode('utf-8')), 'wb+') as f: + f.write(self.content) + + return get_files_path(self.fname, is_private=is_private) + + + def remove_file(self, attached_to_doctype=None, attached_to_name=None, from_delete=False): + """Remove file and File entry""" + file_name = None + if not (attached_to_doctype and attached_to_name): + attached = frappe.db.get_value("File", fid, + ["attached_to_doctype", "attached_to_name", "file_name"]) + if attached: + attached_to_doctype, attached_to_name, file_name = attached + + ignore_permissions, comment = False, None + if attached_to_doctype and attached_to_name and not from_delete: + doc = frappe.get_doc(attached_to_doctype, attached_to_name) + ignore_permissions = doc.has_permission("write") or False + if frappe.flags.in_web_form: + ignore_permissions = True + if not file_name: + file_name = frappe.db.get_value("File", fid, "file_name") + comment = doc.add_comment("Attachment Removed", _("Removed {0}").format(file_name)) + + frappe.delete_doc("File", fid, ignore_permissions=ignore_permissions) + + return comment + + + def delete_file_data_content(self, only_thumbnail=False): + method = get_hook_method('delete_file_data_content', fallback=self.delete_file_from_filesystem) + method(self.doc, only_thumbnail=only_thumbnail) + + + def delete_file_from_filesystem(self, only_thumbnail=False): + """Delete file, thumbnail from File document""" + if only_thumbnail: + self.delete_file(self.doc.thumbnail_url) + else: + self.delete_file(self.doc.file_url) + self.delete_file(self.doc.thumbnail_url) + + + def delete_file(self, path): + """Delete file from `public folder`""" + if path: + if ".." in path.split("/"): + frappe.msgprint(_("It is risky to delete this file: {0}. Please contact your System Manager.").format(path)) + + parts = os.path.split(path.strip("/")) + if parts[0]=="files": + path = frappe.utils.get_site_path("public", "files", parts[-1]) + + else: + path = frappe.utils.get_site_path("private", "files", parts[-1]) + + path = encode(path) + if os.path.exists(path): + os.remove(path) + + def on_doctype_update(): frappe.db.add_index("File", ["attached_to_doctype", "attached_to_name"]) @@ -622,97 +694,27 @@ def get_max_file_size(): return conf.get('max_file_size') or 10485760 -def check_max_file_size(content): - max_file_size = get_max_file_size() - file_size = len(content) - - if file_size > max_file_size: - frappe.msgprint(_("File size exceeded the maximum allowed size of {0} MB").format( - max_file_size / 1048576), - raise_exception=MaxFileSizeReachedError) - - return file_size - - -def write_file(content, fname, is_private=0): - """write file to disk with a random name (to compare)""" - file_path = get_files_path(is_private=is_private) - - # create directory (if not exists) - frappe.create_folder(file_path) - # write the file - if isinstance(content, text_type): - content = content.encode() - with open(os.path.join(file_path.encode('utf-8'), fname.encode('utf-8')), 'wb+') as f: - f.write(content) - - return get_files_path(fname, is_private=is_private) - - def remove_all(dt, dn, from_delete=False): """remove all files in a transaction""" try: for fid in frappe.db.sql_list("""select name from `tabFile` where attached_to_doctype=%s and attached_to_name=%s""", (dt, dn)): - remove_file(fid, dt, dn, from_delete) + _file = frappe.get_doc("File", {"fid": fid, "attached_to_doctype": dt, "attached_to_name": dn}) + _file.remove_file(from_delete=from_delete) except Exception as e: if e.args[0]!=1054: raise # (temp till for patched) -def remove_file(fid, attached_to_doctype=None, attached_to_name=None, from_delete=False): - """Remove file and File entry""" - file_name = None - if not (attached_to_doctype and attached_to_name): - attached = frappe.db.get_value("File", fid, - ["attached_to_doctype", "attached_to_name", "file_name"]) - if attached: - attached_to_doctype, attached_to_name, file_name = attached - - ignore_permissions, comment = False, None - if attached_to_doctype and attached_to_name and not from_delete: - doc = frappe.get_doc(attached_to_doctype, attached_to_name) - ignore_permissions = doc.has_permission("write") or False - if frappe.flags.in_web_form: - ignore_permissions = True - if not file_name: - file_name = frappe.db.get_value("File", fid, "file_name") - comment = doc.add_comment("Attachment Removed", _("Removed {0}").format(file_name)) - - frappe.delete_doc("File", fid, ignore_permissions=ignore_permissions) - - return comment - - -def delete_file_data_content(doc, only_thumbnail=False): - method = get_hook_method('delete_file_data_content', fallback=delete_file_from_filesystem) - method(doc, only_thumbnail=only_thumbnail) - - -def delete_file_from_filesystem(doc, only_thumbnail=False): - """Delete file, thumbnail from File document""" - if only_thumbnail: - delete_file(doc.thumbnail_url) +def remove_file_by_url(file_url, doctype=None, name=None): + if doctype and name: + fid = frappe.db.get_value("File", {"file_url": file_url, + "attached_to_doctype": doctype, "attached_to_name": name}) else: - delete_file(doc.file_url) - delete_file(doc.thumbnail_url) + fid = frappe.db.get_value("File", {"file_url": file_url}) - -def delete_file(path): - """Delete file from `public folder`""" - if path: - if ".." in path.split("/"): - frappe.msgprint(_("It is risky to delete this file: {0}. Please contact your System Manager.").format(path)) - - parts = os.path.split(path.strip("/")) - if parts[0]=="files": - path = frappe.utils.get_site_path("public", "files", parts[-1]) - - else: - path = frappe.utils.get_site_path("private", "files", parts[-1]) - - path = encode(path) - if os.path.exists(path): - os.remove(path) + if fid: + _file = frappe.get_doc("File", {"fid": fid}) + return _file.remove_file() def get_file(fname): @@ -788,7 +790,7 @@ def download_file(file_url): token is required to download private files. Method : GET - Endpoint : frappe.utils.file_manager.download_file + Endpoint : frappe.core.doctype.file.file.download_file URL Params : file_name = /path/to/file relative to site path """ file_doc = frappe.get_doc("File", {"file_url":file_url}) diff --git a/frappe/core/doctype/file/test_file.py b/frappe/core/doctype/file/test_file.py index fbf61fbff5..be7758e439 100644 --- a/frappe/core/doctype/file/test_file.py +++ b/frappe/core/doctype/file/test_file.py @@ -101,7 +101,7 @@ class TestFile(unittest.TestCase): self.assertRaises(frappe.ValidationError, folder.delete) def test_file_upload_limit(self): - from frappe.utils.file_manager import MaxFileSizeReachedError + from frappe.core.doctype.file.file import MaxFileSizeReachedError from frappe.limits import update_limits, clear_limit from frappe import _dict diff --git a/frappe/core/doctype/prepared_report/prepared_report.py b/frappe/core/doctype/prepared_report/prepared_report.py index 9565a18092..9574320ee1 100644 --- a/frappe/core/doctype/prepared_report/prepared_report.py +++ b/frappe/core/doctype/prepared_report/prepared_report.py @@ -11,10 +11,10 @@ import frappe from frappe.model.document import Document from frappe.utils.background_jobs import enqueue from frappe.desk.query_report import generate_report_result, get_columns_dict -from frappe.utils.file_manager import remove_all +from frappe.core.doctype.file.file import remove_all from frappe.utils.csvutils import to_csv, read_csv_content_from_attached_file from frappe.desk.form.load import get_attachments -from frappe.utils.file_manager import download_file +from frappe.core.doctype.file.file import download_file class PreparedReport(Document): diff --git a/frappe/desk/form/utils.py b/frappe/desk/form/utils.py index bad7657a8e..afb734d7fe 100644 --- a/frappe/desk/form/utils.py +++ b/frappe/desk/form/utils.py @@ -13,9 +13,10 @@ from six import string_types @frappe.whitelist() def remove_attach(): """remove attachment""" - import frappe.utils.file_manager fid = frappe.form_dict.get('fid') - return frappe.utils.file_manager.remove_file(fid) + _file = frappe.get_doc("File", {"fid": fid}) + return _file.remove_file() + @frappe.whitelist() def validate_link(): diff --git a/frappe/email/doctype/email_account/test_email_account.py b/frappe/email/doctype/email_account/test_email_account.py index bae0229a8c..d59c3a62f6 100644 --- a/frappe/email/doctype/email_account/test_email_account.py +++ b/frappe/email/doctype/email_account/test_email_account.py @@ -8,7 +8,6 @@ test_records = frappe.get_test_records('Email Account') from frappe.core.doctype.communication.email import make from frappe.desk.form.load import get_attachments -from frappe.utils.file_manager import delete_file_from_filesystem from frappe.email.doctype.email_account.email_account import notify_unreplied from datetime import datetime, timedelta @@ -55,7 +54,8 @@ class TestEmailAccount(unittest.TestCase): frappe.db.sql("delete from tabCommunication where sender='test_sender@example.com'") existing_file = frappe.get_doc({'doctype': 'File', 'file_name': 'erpnext-conf-14.png'}) frappe.delete_doc("File", existing_file.name) - delete_file_from_filesystem(existing_file) + _file = frappe.get_doc("File", {"doc": existing_file}) + _file.delete_file_from_filesystem() with open(os.path.join(os.path.dirname(__file__), "test_mails", "incoming-2.raw"), "r") as testfile: test_mails = [testfile.read()] @@ -73,7 +73,8 @@ class TestEmailAccount(unittest.TestCase): # cleanup existing_file = frappe.get_doc({'doctype': 'File', 'file_name': 'erpnext-conf-14.png'}) frappe.delete_doc("File", existing_file.name) - delete_file_from_filesystem(existing_file) + _file = frappe.get_doc("File", {"doc": existing_file}) + def test_incoming_attached_email_from_outlook_plain_text_only(self): frappe.db.sql("delete from tabCommunication where sender='test_sender@example.com'") diff --git a/frappe/email/email_body.py b/frappe/email/email_body.py index 8ab7ae5c85..e886bc3f06 100755 --- a/frappe/email/email_body.py +++ b/frappe/email/email_body.py @@ -144,7 +144,7 @@ class EMail: def attach_file(self, n): """attach a file from the `FileData` table""" - from frappe.utils.file_manager import get_file + from frappe.core.doctype.file.file import get_file res = get_file(n) if not res: return diff --git a/frappe/email/queue.py b/frappe/email/queue.py index 815ded36b9..5d2acb00c6 100755 --- a/frappe/email/queue.py +++ b/frappe/email/queue.py @@ -11,7 +11,7 @@ from frappe.email.email_body import get_email, get_formatted_html, add_attachmen from frappe.utils.verified_command import get_signed_params, verify_request from html2text import html2text from frappe.utils import get_url, nowdate, encode, now_datetime, add_days, split_emails, cstr, cint -from frappe.utils.file_manager import get_file +from frappe.core.doctype.file.file import get_file from rq.timeouts import JobTimeoutException from frappe.utils.scheduler import log from six import text_type, string_types diff --git a/frappe/email/receive.py b/frappe/email/receive.py index 0282eb4bf6..2d50b8c294 100644 --- a/frappe/email/receive.py +++ b/frappe/email/receive.py @@ -13,7 +13,7 @@ from frappe import _, safe_decode, safe_encode from frappe.utils import (extract_email_id, convert_utc_to_user_timezone, now, cint, cstr, strip, markdown, parse_addr) from frappe.utils.scheduler import log -from frappe.utils.file_manager import get_random_filename, MaxFileSizeReachedError +from frappe.core.doctype.file.file import get_random_filename, MaxFileSizeReachedError class EmailSizeExceededError(frappe.ValidationError): pass class EmailTimeoutError(frappe.ValidationError): pass diff --git a/frappe/handler.py b/frappe/handler.py index 82a19ec558..383b5d0fd0 100755 --- a/frappe/handler.py +++ b/frappe/handler.py @@ -6,7 +6,6 @@ import frappe from frappe import _ import frappe.utils import frappe.sessions -import frappe.utils.file_manager import frappe.desk.form.run_method from frappe.utils.response import build_response from werkzeug.wrappers import Response diff --git a/frappe/limits.py b/frappe/limits.py index 0330db2567..55dd42a09a 100755 --- a/frappe/limits.py +++ b/frappe/limits.py @@ -173,7 +173,7 @@ def clear_limit(key): def validate_space_limit(file_size): """Stop from writing file if max space limit is reached""" - from frappe.utils.file_manager import MaxFileSizeReachedError + from frappe.core.doctype.file.file import MaxFileSizeReachedError limits = get_limits() if not limits.space: diff --git a/frappe/model/delete_doc.py b/frappe/model/delete_doc.py index 6348f66b55..b16c74abbd 100644 --- a/frappe/model/delete_doc.py +++ b/frappe/model/delete_doc.py @@ -7,7 +7,7 @@ import frappe import frappe.model.meta from frappe.model.dynamic_links import get_dynamic_link_map import frappe.defaults -from frappe.utils.file_manager import remove_all +from frappe.core.doctype.file.file import remove_all from frappe.utils.password import delete_all_passwords_for from frappe import _ from frappe.model.naming import revert_series_if_last diff --git a/frappe/patches/v4_0/file_manager_hooks.py b/frappe/patches/v4_0/file_manager_hooks.py index 60686b1e0e..018e52ca75 100644 --- a/frappe/patches/v4_0/file_manager_hooks.py +++ b/frappe/patches/v4_0/file_manager_hooks.py @@ -6,7 +6,7 @@ from __future__ import unicode_literals, print_function import frappe import os from frappe.utils import get_files_path -from frappe.utils.file_manager import get_content_hash, get_file +from frappe.core.doctype.file.file import get_content_hash, get_file def execute(): diff --git a/frappe/patches/v4_1/file_manager_fix.py b/frappe/patches/v4_1/file_manager_fix.py index 1fced61799..edb450d21a 100644 --- a/frappe/patches/v4_1/file_manager_fix.py +++ b/frappe/patches/v4_1/file_manager_fix.py @@ -5,7 +5,7 @@ from __future__ import unicode_literals, print_function import frappe import os -from frappe.utils.file_manager import get_content_hash, get_file, get_file_name +from frappe.core.doctype.file.file import get_content_hash, get_file, get_file_name from frappe.utils import get_files_path, get_site_path # The files missed by the previous patch might have been replaced with new files diff --git a/frappe/public/js/frappe/upload.js b/frappe/public/js/frappe/upload.js index bdc8b41316..ca2ed264ed 100644 --- a/frappe/public/js/frappe/upload.js +++ b/frappe/public/js/frappe/upload.js @@ -317,7 +317,7 @@ frappe.upload = { } else { args.file_size = fileobj.size; frappe.call({ - method: 'frappe.utils.file_manager.validate_filename', + method: 'frappe.core.doctype.file.file.validate_filename', args: {"filename": args.filename}, callback: function(r) { args.filename = r.message; diff --git a/frappe/tests/test_filemanager.py b/frappe/tests/test_filemanager.py index a25c5a7503..4997e399cc 100644 --- a/frappe/tests/test_filemanager.py +++ b/frappe/tests/test_filemanager.py @@ -6,7 +6,7 @@ import frappe import os import unittest -from frappe.utils.file_manager import get_file, get_files_path +from frappe.core.doctype.file.file import get_file, get_files_path test_content1 = 'Hello' test_content2 = 'Hello World' diff --git a/frappe/utils/csvutils.py b/frappe/utils/csvutils.py index b9159a4bee..db66087d73 100644 --- a/frappe/utils/csvutils.py +++ b/frappe/utils/csvutils.py @@ -30,7 +30,7 @@ def read_csv_content_from_attached_file(doc): raise Exception try: - from frappe.utils.file_manager import get_file + from frappe.core.doctype.file.file import get_file fname, fcontent = get_file(fileid) return read_csv_content(fcontent, frappe.form_dict.get('ignore_encoding_errors')) except Exception: diff --git a/frappe/utils/xlsxutils.py b/frappe/utils/xlsxutils.py index 880726c16e..b7c26ba5c3 100644 --- a/frappe/utils/xlsxutils.py +++ b/frappe/utils/xlsxutils.py @@ -70,7 +70,7 @@ def handle_html(data): def read_xlsx_file_from_attached_file(file_id=None, fcontent=None, filepath=None): if file_id: - from frappe.utils.file_manager import get_file_path + from frappe.core.doctype.file.file import get_file_path filename = get_file_path(file_id) elif fcontent: from io import BytesIO diff --git a/frappe/website/doctype/web_form/web_form.py b/frappe/website/doctype/web_form/web_form.py index d8d2418c45..aa95198d3d 100644 --- a/frappe/website/doctype/web_form/web_form.py +++ b/frappe/website/doctype/web_form/web_form.py @@ -8,7 +8,8 @@ from frappe import _, scrub from frappe.utils import cstr from frappe.website.utils import get_comment_list from frappe.custom.doctype.customize_form.customize_form import docfield_properties -from frappe.utils.file_manager import get_max_file_size +from frappe.core.doctype.file.file import get_max_file_size +from frappe.core.doctype.file.file import remove_file_by_url from frappe.modules.utils import export_module_json, get_doc_module from six.moves.urllib.parse import urlencode from frappe.integrations.utils import get_payment_gateway_controller @@ -415,8 +416,7 @@ def accept(web_form, data, for_payment=False): # remove earlier attached file (if exists) if doc.get(fieldname): - file = frappe.get_doc("File", {"attached_to_doctype": doc.doctype, "file_url": doc.get(fieldname), "attached_to_name": doc.name}) - file.remove_file_by_url() + remove_file_by_url(doc.get(fieldname), doctype=doc.doctype, name=doc.name) # save new file filename, dataurl = filedata.split(',', 1) @@ -432,8 +432,8 @@ def accept(web_form, data, for_payment=False): if files_to_delete: for f in files_to_delete: if f: - file = frappe.get_doc("File", {"attached_to_doctype": doc.doctype, "file_url": doc.get(fieldname), "attached_to_name": doc.name}) - file.remove_file_by_url() + remove_file_by_url(doc.get(fieldname), doctype=doc.doctype, name=doc.name) + frappe.flags.web_form_doc = doc From 6eca292e1a58a0dd5138293cef2ba6b18cddde0f Mon Sep 17 00:00:00 2001 From: Chinmay Pai Date: Thu, 6 Sep 2018 01:18:21 +0530 Subject: [PATCH 04/12] [3/3] file-api: code migration migrate api from file_manager.py to file.py Signed-off-by: Chinmay Pai --- frappe/client.py | 7 +- frappe/core/doctype/communication/email.py | 6 +- frappe/core/doctype/data_import/importer.py | 11 +- frappe/core/doctype/file/file.py | 268 +++++++++--------- .../prepared_report/prepared_report.py | 4 +- frappe/desk/form/utils.py | 4 +- frappe/desk/page/setup_wizard/setup_wizard.py | 6 +- .../email_account/test_email_account.py | 6 +- frappe/email/receive.py | 9 +- .../gsuite_templates/gsuite_templates.py | 6 +- frappe/model/base_document.py | 2 +- frappe/model/document.py | 4 +- .../doctype/print_format/print_format.py | 2 +- frappe/tests/test_filemanager.py | 22 +- frappe/twofactor.py | 6 +- frappe/utils/csvutils.py | 4 +- frappe/website/doctype/web_form/web_form.py | 4 +- 17 files changed, 183 insertions(+), 188 deletions(-) diff --git a/frappe/client.py b/frappe/client.py index 247a3151e6..33b7e3bb1b 100644 --- a/frappe/client.py +++ b/frappe/client.py @@ -350,9 +350,10 @@ def attach_file(filename=None, filedata=None, doctype=None, docname=None, folder if not doc.has_permission(): frappe.throw(_("Not permitted"), frappe.PermissionError) - _file = frappe.get_doc("File", {"file_name": filename, "content": filedata, - "attached_to_doctype": doctype, "attached_to_name": docname}) - f = _file.save_file(folder=folder, decode=decode_base64, is_private=is_private, df=docfield) + _file = frappe.get_doc({"doctype": "File", "file_name": filename, "attached_to_doctype": doctype, + "attached_to_name": docname, "attached_to_field": docfield, + "folder": folder, "is_private": is_private}) + f = _file.save_file(content=filedata, decode=decode_base64) if docfield and doctype: doc.set(docfield, f.file_url) diff --git a/frappe/core/doctype/communication/email.py b/frappe/core/doctype/communication/email.py index 6303eb7fe3..d859041e4c 100755 --- a/frappe/core/doctype/communication/email.py +++ b/frappe/core/doctype/communication/email.py @@ -405,9 +405,9 @@ def add_attachments(name, attachments): # save attachments to new doc _file = frappe.get_doc("File", {"file_url": attach.file_url, - "attached_to_doctype": "Communication", "attached_to_name", name}) - - _file.save_url(folder="Home/Attachments") + "attached_to_doctype": "Communication", "attached_to_name": name, + "folder": "Home/Attachments"}) + _file.save_url() def filter_email_list(doc, email_list, exclude, is_cc=False, is_bcc=False): diff --git a/frappe/core/doctype/data_import/importer.py b/frappe/core/doctype/data_import/importer.py index 69a1fa3190..36775e7d78 100644 --- a/frappe/core/doctype/data_import/importer.py +++ b/frappe/core/doctype/data_import/importer.py @@ -265,8 +265,8 @@ def upload(rows = None, submit_after_import=None, ignore_encoding_errors=False, return _file = frappe.get_doc("File", {"file_url": file_url, "attached_to_name": docname, - "attached_to_doctype": doctype}) - _file.save_url(folder="Home/Attachments", df=0) + "attached_to_doctype": doctype, "folder": "Home/Attachments"}) + _file.save_url(df=0) # header @@ -465,9 +465,10 @@ def upload(rows = None, submit_after_import=None, ignore_encoding_errors=False, else: from frappe.utils.csvutils import to_csv file_data = to_csv(data_rows_with_error) - _file = frappe.get_doc("File", {"file_name": file_name, "content": file_data, - "attached_to_doctype": "Data Import", "attached_to_name": data_import_doc.name}) - error_data_file = _file.save_file(folder="Home/Attachments") + _file = frappe.get_doc({"doctype": "File", "file_name": file_name, + "attached_to_doctype": "Data Import", "attached_to_name": data_import_doc.name, + "folder": "Home/Attachments"}) + error_data_file = _file.save_file(content=file_data) data_import_doc.error_file = error_data_file.file_url elif error_flag: diff --git a/frappe/core/doctype/file/file.py b/frappe/core/doctype/file/file.py index c9cdb25c55..d3a50990a1 100755 --- a/frappe/core/doctype/file/file.py +++ b/frappe/core/doctype/file/file.py @@ -19,18 +19,17 @@ import io import shutil import requests import requests.exceptions -import mimetypes, imghdr +import imghdr from frappe.utils import get_hook_method, get_files_path, random_string, encode, cstr, call_hook_method, cint from frappe import _ -from frappe impoty conf +from frappe import conf from copy import copy from frappe.utils.nestedset import NestedSet -from frappe.utils import strip, get_files_path +from frappe.utils import strip from PIL import Image, ImageOps from six import StringIO, string_types from six.moves.urllib.parse import unquote -from six.moves.urllib.parse import unquote from six import text_type, PY2 import zipfile @@ -270,7 +269,6 @@ class File(NestedSet): if self.file_name and self.content_hash and (not frappe.db.count("File", {"content_hash": self.content_hash, "name": ["!=", self.name]})): self.delete_file_data_content() - elif self.file_url: self.delete_file_data_content(only_thumbnail=True) @@ -315,10 +313,10 @@ class File(NestedSet): def upload(self): # get record details - self.dt = frappe.form_dict.doctype - self.xdn = frappe.form_dict.docname - self.df = frappe.form_dict.docfield - file_url = frappe.form_dict.file_url + self.attached_to_doctype = frappe.form_dict.doctype + self.attached_to_name = frappe.form_dict.docname + self.attached_to_field = frappe.form_dict.docfield + self.file_url = frappe.form_dict.file_url self.filename = frappe.form_dict.filename frappe.form_dict.is_private = cint(frappe.form_dict.is_private) @@ -329,8 +327,8 @@ class File(NestedSet): file_doc = self.get_file_doc() comment = {} - if self.dt and self.dn: - comment = frappe.get_doc(self.dt, self.dn).add_comment("Attachment", + if self.attached_to_doctype and self.attached_to_name: + comment = frappe.get_doc(self.attached_to_doctype, self.attached_to_name).add_comment("Attachment", _ ("added {0}").format("{file_name}{icon}".format(**{ "icon": ' ' \ if file_doc.is_private else "", @@ -352,9 +350,11 @@ class File(NestedSet): '''returns File object (Document) from given parameters or form_dict''' r = frappe.form_dict - if self.dt is None: self.dt = r.doctype - if self.dn is None: self.dn = r.docname - if self.df is None: self.df = r.docfield + if self.file_url is None: self.file_url = r.file_url + if self.filename is None: self.filename = r.filename + if self.attached_to_doctype is None: self.attached_to_doctype = r.doctype + if self.attached_to_name is None: self.attached_to_name = r.docname + if self.attached_to_field is None: self.attached_to_field = r.docfield if self.folder is None: self.folder = r.folder if self.is_private is None: self.is_private = r.is_private @@ -362,36 +362,36 @@ class File(NestedSet): file_doc = self.save_uploaded() elif r.file_url: - file_doc = self.save_url(self, r.file_url, r.filename) + file_doc = self.save_url() return file_doc def save_uploaded(self): - self.fname, self.content = self.get_uploaded_content() + self.file_name, self.content = self.get_uploaded_content() if self.content: - return self.save_file() + return self.save_file(content=self.content) else: raise Exception - def save_url(self, folder, df=None): + def save_url(self, df=None): # if not (file_url.startswith("http://") or file_url.startswith("https://")): # frappe.msgprint("URL must start with 'http://' or 'https://'") # return None, None - file_url = unquote(file_url) - file_size = frappe.form_dict.file_size + self.file_url = unquote(self.file_url) + self.file_size = frappe.form_dict.file_size f = frappe.get_doc({ "doctype": "File", - "file_url": file_url, - "file_name": filename, - "attached_to_doctype": self.dt, - "attached_to_name": self.dn, - "attached_to_field": self.df, + "file_url": self.file_url, + "file_name": self.file_name, + "attached_to_doctype": self.attached_to_doctype, + "attached_to_name": self.attached_to_name, + "attached_to_field": self.attached_to_field, "folder": self.folder, - "file_size": file_size, + "file_size": self.file_size, "is_private": self.is_private }) f.flags.ignore_permissions = True @@ -410,40 +410,44 @@ class File(NestedSet): frappe.uploaded_content = base64.b64decode(frappe.form_dict.filedata) frappe.uploaded_filename = frappe.form_dict.filename return frappe.uploaded_filename, frappe.uploaded_content + elif self.content and self.file_name: + return self.file_name, self.content frappe.msgprint(_('No file attached')) return None, None - def save_file(self, folder=None, decode=False, is_private=0, df=None): + def save_file(self, content=None, decode=False): + self.content = content if decode: - if isinstance(self.content, text_type): - self.content = self.content.encode("utf-8") + if isinstance(content, text_type): + self.content = content.encode("utf-8") if b"," in self.content: self.content = self.content.split(b",")[1] self.content = base64.b64decode(self.content) - file_size = self.check_max_file_size() + if not self.is_private: + self.is_private = 0 + self.file_size = self.check_max_file_size() self.content_hash = get_content_hash(self.content) - self.content_type = mimetypes.guess_type(self.fname)[0] - self.fname = get_file_name(self.fname, self.content_hash[-6:]) - self.is_private = is_private - file_data = self.get_file_data_from_hash(is_private=self.is_private) + self.content_type = mimetypes.guess_type(self.file_name)[0] + self.file_name = get_file_name(self.file_name, self.content_hash[-6:]) + file_data = self.get_file_data_from_hash() if not file_data: - call_hook_method("before_write_file", file_size=file_size) + call_hook_method("before_write_file", file_size=self.file_size) - write_file_method = get_hook_method('write_file', fallback=self.save_file_on_filesystem) - file_data = write_file_method(self.fname, self.content, content_type=self.content_type, is_private=self.is_private) + write_file_method = get_hook_method('write_file', fallback=save_file_on_filesystem) + file_data = write_file_method(self.file_name, self.content, self.content_type, self.is_private) file_data = copy(file_data) file_data.update({ "doctype": "File", - "attached_to_doctype": self.dt, - "attached_to_name": self.dn, - "attached_to_field": self.df, + "attached_to_doctype": self.attached_to_doctype, + "attached_to_name": self.attached_to_name, + "attached_to_field": self.attached_to_field, "folder": self.folder, - "file_size": file_size, - "content_hash": content_hash, + "file_size": self.file_size, + "content_hash": self.content_hash, "is_private": self.is_private }) @@ -457,28 +461,14 @@ class File(NestedSet): return f - def get_file_data_from_hash(self, is_private=0): + def get_file_data_from_hash(self): for name in frappe.db.sql_list("select name from `tabFile` where content_hash=%s and is_private=%s", - (self.content_hash, is_private)): + (self.content_hash, self.is_private)): b = frappe.get_doc('File', name) return {k: b.get(k) for k in frappe.get_hooks()['write_file_keys']} return False - def save_file_on_filesystem(self): - fpath = self.write_file(self, is_private=self.is_private) - - if self.is_private: - file_url = "/private/files/{0}".format(self.fname) - else: - file_url = "/files/{0}".format(self.fname) - - return { - 'file_name': os.path.basename(fpath), - 'file_url': file_url - } - - def check_max_file_size(self): max_file_size = get_max_file_size() file_size = len(self.content) @@ -491,75 +481,9 @@ class File(NestedSet): return file_size - def write_file(self, is_private=0): - """write file to disk with a random name (to compare)""" - file_path = get_files_path(is_private=is_private) - - # create directory (if not exists) - frappe.create_folder(file_path) - # write the file - if isinstance(self.content, text_type): - self.content = self.content.encode() - with open(os.path.join(file_path.encode('utf-8'), self.fname.encode('utf-8')), 'wb+') as f: - f.write(self.content) - - return get_files_path(self.fname, is_private=is_private) - - - def remove_file(self, attached_to_doctype=None, attached_to_name=None, from_delete=False): - """Remove file and File entry""" - file_name = None - if not (attached_to_doctype and attached_to_name): - attached = frappe.db.get_value("File", fid, - ["attached_to_doctype", "attached_to_name", "file_name"]) - if attached: - attached_to_doctype, attached_to_name, file_name = attached - - ignore_permissions, comment = False, None - if attached_to_doctype and attached_to_name and not from_delete: - doc = frappe.get_doc(attached_to_doctype, attached_to_name) - ignore_permissions = doc.has_permission("write") or False - if frappe.flags.in_web_form: - ignore_permissions = True - if not file_name: - file_name = frappe.db.get_value("File", fid, "file_name") - comment = doc.add_comment("Attachment Removed", _("Removed {0}").format(file_name)) - - frappe.delete_doc("File", fid, ignore_permissions=ignore_permissions) - - return comment - - def delete_file_data_content(self, only_thumbnail=False): - method = get_hook_method('delete_file_data_content', fallback=self.delete_file_from_filesystem) - method(self.doc, only_thumbnail=only_thumbnail) - - - def delete_file_from_filesystem(self, only_thumbnail=False): - """Delete file, thumbnail from File document""" - if only_thumbnail: - self.delete_file(self.doc.thumbnail_url) - else: - self.delete_file(self.doc.file_url) - self.delete_file(self.doc.thumbnail_url) - - - def delete_file(self, path): - """Delete file from `public folder`""" - if path: - if ".." in path.split("/"): - frappe.msgprint(_("It is risky to delete this file: {0}. Please contact your System Manager.").format(path)) - - parts = os.path.split(path.strip("/")) - if parts[0]=="files": - path = frappe.utils.get_site_path("public", "files", parts[-1]) - - else: - path = frappe.utils.get_site_path("private", "files", parts[-1]) - - path = encode(path) - if os.path.exists(path): - os.remove(path) + method = get_hook_method('delete_file_data_content', fallback=delete_file_from_filesystem) + method(self, only_thumbnail=only_thumbnail) def on_doctype_update(): @@ -602,6 +526,7 @@ def create_new_folder(file_name, folder): @frappe.whitelist() def move_file(file_list, new_parent, old_parent): + if isinstance(file_list, string_types): file_list = json.loads(file_list) @@ -689,7 +614,86 @@ def get_web_image(file_url): return image, filename, extn -############################## + +def save_file_on_filesystem(file_name, content, content_type, is_private): + fpath = write_file(file_name, content, is_private=is_private) + + if is_private: + file_url = "/private/files/{0}".format(file_name) + else: + file_url = "/files/{0}".format(file_name) + + return { + 'file_name': os.path.basename(fpath), + 'file_url': file_url + } + + +def write_file(file_name, content, is_private=0): + """write file to disk with a random name (to compare)""" + file_path = get_files_path(is_private=is_private) + + # create directory (if not exists) + frappe.create_folder(file_path) + # write the file + if isinstance(content, text_type): + content = content.encode() + with open(os.path.join(file_path.encode('utf-8'), file_name.encode('utf-8')), 'wb+') as f: + f.write(content) + + return get_files_path(file_name, is_private=is_private) + + +def delete_file_from_filesystem(doc, only_thumbnail=False): + """Delete file, thumbnail from File document""" + if only_thumbnail: + delete_file(doc.thumbnail_url) + else: + delete_file(doc.file_url) + delete_file(doc.thumbnail_url) + + +def delete_file(path): + """Delete file from `public folder`""" + if path: + if ".." in path.split("/"): + frappe.msgprint(_("It is risky to delete this file: {0}. Please contact your System Manager.").format(path)) + + parts = os.path.split(path.strip("/")) + if parts[0]=="files": + path = frappe.utils.get_site_path("public", "files", parts[-1]) + + else: + path = frappe.utils.get_site_path("private", "files", parts[-1]) + + path = encode(path) + if os.path.exists(path): + os.remove(path) + + +def remove_file(fid=None, attached_to_doctype=None, attached_to_name=None, from_delete=False): + """Remove file and File entry""" + file_name = None + if not (attached_to_doctype and attached_to_name): + attached = frappe.db.get_value("File", fid, + ["attached_to_doctype", "attached_to_name", "file_name"]) + if attached: + attached_to_doctype, attached_to_name, file_name = attached + + ignore_permissions, comment = False, None + if attached_to_doctype and attached_to_name and not from_delete: + doc = frappe.get_doc(attached_to_doctype, attached_to_name) + ignore_permissions = doc.has_permission("write") or False + if frappe.flags.in_web_form: + ignore_permissions = True + if not file_name: + file_name = frappe.db.get_value("File", fid, "file_name") + comment = doc.add_comment("Attachment Removed", _("Removed {0}").format(file_name)) + frappe.delete_doc("File", fid, ignore_permissions=ignore_permissions) + + return comment + + def get_max_file_size(): return conf.get('max_file_size') or 10485760 @@ -699,8 +703,7 @@ def remove_all(dt, dn, from_delete=False): try: for fid in frappe.db.sql_list("""select name from `tabFile` where attached_to_doctype=%s and attached_to_name=%s""", (dt, dn)): - _file = frappe.get_doc("File", {"fid": fid, "attached_to_doctype": dt, "attached_to_name": dn}) - _file.remove_file(from_delete=from_delete) + remove_file(fid=fid, attached_to_doctype=dt, attached_to_name=dn, from_delete=from_delete) except Exception as e: if e.args[0]!=1054: raise # (temp till for patched) @@ -713,8 +716,7 @@ def remove_file_by_url(file_url, doctype=None, name=None): fid = frappe.db.get_value("File", {"file_url": file_url}) if fid: - _file = frappe.get_doc("File", {"fid": fid}) - return _file.remove_file() + return remove_file(fid=fid) def get_file(fname): @@ -832,9 +834,9 @@ def extract_images_from_html(doc, content): name = doc.parent or doc.name # TODO fix this - _file = frappe.get_doc("File", {"file_name": filename, "content": content, + _file = frappe.get_doc({"doctype": "File", "file_name": filename, "attached_to_doctype": doctype, "attached_to_name": name}) - file_url = _file.save_file(decode=True).file_url + file_url = _file.save_file(content=content, decode=True).file_url if not frappe.flags.has_dataurl: frappe.flags.has_dataurl = True diff --git a/frappe/core/doctype/prepared_report/prepared_report.py b/frappe/core/doctype/prepared_report/prepared_report.py index 9574320ee1..06f6203306 100644 --- a/frappe/core/doctype/prepared_report/prepared_report.py +++ b/frappe/core/doctype/prepared_report/prepared_report.py @@ -62,9 +62,9 @@ def create_csv_file(columns, data, dt, dn): rows = [tuple(columns)] + data encoded = base64.b64encode(frappe.safe_encode(to_csv(rows))) # Call save_file function to upload and attach the file - _file = frappe.get_doc("File", {"file_name": csv_filename, "content": encoded, + _file = frappe.get_doc({"doctype": "File", "file_name": csv_filename, "attached_to_doctype": dt, "attached_to_name": dn}) - _file.save_file(decode=True) + _file.save_file(content=encoded, decode=True) @frappe.whitelist() diff --git a/frappe/desk/form/utils.py b/frappe/desk/form/utils.py index afb734d7fe..4385b039b3 100644 --- a/frappe/desk/form/utils.py +++ b/frappe/desk/form/utils.py @@ -6,6 +6,7 @@ import frappe, json import frappe.desk.form.meta import frappe.desk.form.load from frappe.utils.html_utils import clean_email_html +from frappe.core.doctype.file.file import remove_file from frappe import _ from six import string_types @@ -14,8 +15,7 @@ from six import string_types def remove_attach(): """remove attachment""" fid = frappe.form_dict.get('fid') - _file = frappe.get_doc("File", {"fid": fid}) - return _file.remove_file() + return remove_file(fid=fid) @frappe.whitelist() diff --git a/frappe/desk/page/setup_wizard/setup_wizard.py b/frappe/desk/page/setup_wizard/setup_wizard.py index ee409d4b57..74edf51486 100755 --- a/frappe/desk/page/setup_wizard/setup_wizard.py +++ b/frappe/desk/page/setup_wizard/setup_wizard.py @@ -186,9 +186,9 @@ def update_user_name(args): attach_user = args.get("attach_user").split(",") if len(attach_user)==3: filename, filetype, content = attach_user - _file = frappe.get_doc("File", {"file_name": filename, "content": content, - "attached_to_doctype": "User", "attached_to_doctype": args.get("name")}) - fileurl = _file.save_file(decode=True).file_url + _file = frappe.get_doc({"doctype": "File", "file_name": filename, + "attached_to_doctype": "User", "attached_to_docname": args.get("name")}) + fileurl = _file.save_file(content=content, decode=True).file_url frappe.db.set_value("User", args.get("name"), "user_image", fileurl) if args.get('name'): diff --git a/frappe/email/doctype/email_account/test_email_account.py b/frappe/email/doctype/email_account/test_email_account.py index d59c3a62f6..d7780975ea 100644 --- a/frappe/email/doctype/email_account/test_email_account.py +++ b/frappe/email/doctype/email_account/test_email_account.py @@ -8,6 +8,7 @@ test_records = frappe.get_test_records('Email Account') from frappe.core.doctype.communication.email import make from frappe.desk.form.load import get_attachments +from frappe.core.doctype.file.file import delete_file_from_filesystem from frappe.email.doctype.email_account.email_account import notify_unreplied from datetime import datetime, timedelta @@ -54,8 +55,7 @@ class TestEmailAccount(unittest.TestCase): frappe.db.sql("delete from tabCommunication where sender='test_sender@example.com'") existing_file = frappe.get_doc({'doctype': 'File', 'file_name': 'erpnext-conf-14.png'}) frappe.delete_doc("File", existing_file.name) - _file = frappe.get_doc("File", {"doc": existing_file}) - _file.delete_file_from_filesystem() + delete_file_from_filesystem(existing_file) with open(os.path.join(os.path.dirname(__file__), "test_mails", "incoming-2.raw"), "r") as testfile: test_mails = [testfile.read()] @@ -73,7 +73,7 @@ class TestEmailAccount(unittest.TestCase): # cleanup existing_file = frappe.get_doc({'doctype': 'File', 'file_name': 'erpnext-conf-14.png'}) frappe.delete_doc("File", existing_file.name) - _file = frappe.get_doc("File", {"doc": existing_file}) + delete_file_from_filesystem(existing_file) def test_incoming_attached_email_from_outlook_plain_text_only(self): diff --git a/frappe/email/receive.py b/frappe/email/receive.py index 2d50b8c294..fc464c84d8 100644 --- a/frappe/email/receive.py +++ b/frappe/email/receive.py @@ -523,10 +523,11 @@ class Email: for attachment in self.attachments: try: - _file = frappe.get_doc("File", {"file_name": attachment['fname'], - "content": atachment['fcontent'], "attached_to_doctype": doc.doctype, - "attached_to_name": doc.name}) - file_data = _file.save_file(is_private=1) + _file = frappe.get_doc({"doctype": "File", + "file_name": attachment['fname'], + "attached_to_doctype": doc.doctype, + "attached_to_name": doc.name, "is_private": 1}) + file_data = _file.save_file(content=attachment['fcontent']) saved_attachments.append(file_data) if attachment['fname'] in self.cid_map: diff --git a/frappe/integrations/doctype/gsuite_templates/gsuite_templates.py b/frappe/integrations/doctype/gsuite_templates/gsuite_templates.py index 375a02b765..eed9bd5265 100644 --- a/frappe/integrations/doctype/gsuite_templates/gsuite_templates.py +++ b/frappe/integrations/doctype/gsuite_templates/gsuite_templates.py @@ -25,9 +25,9 @@ def create_gsuite_doc(doctype, docname, gs_template=None): r = run_gsuite_script('new', filename, templ.template_id, templ.destination_id, json_data) _file = frappe.get_doc("File", {"file_url": r['url'], "file_name": filename, - "attached_to_doctype": doctype, "attached_to_name": docname}) - _file.save_url(folder="Home/Attachments", df=True) - + "attached_to_doctype": doctype, "attached_to_name": docname, "folder": "Home/Attachments"}) + _file.save_url(df=True) + comment = frappe.get_doc(doctype, docname).add_comment("Attachment", _("added {0}").format("{file_name}{icon}".format(**{ "icon": ' ' if filedata.is_private else "", diff --git a/frappe/model/base_document.py b/frappe/model/base_document.py index 1ff5fc0d0f..be0fc2113d 100644 --- a/frappe/model/base_document.py +++ b/frappe/model/base_document.py @@ -773,7 +773,7 @@ class BaseDocument(object): return cast_fieldtype(df.fieldtype, value) def _extract_images_from_text_editor(self): - from frappe.utils.file_manager import extract_images_from_doc + from frappe.core.doctype.file.file import extract_images_from_doc if self.doctype != "DocType": for df in self.meta.get("fields", {"fieldtype": ('=', "Text Editor")}): extract_images_from_doc(self, df.fieldname) diff --git a/frappe/model/document.py b/frappe/model/document.py index 9d8ec90c3e..bc71d60586 100644 --- a/frappe/model/document.py +++ b/frappe/model/document.py @@ -321,8 +321,8 @@ class Document(BaseDocument): #save attachments to new doc _file = frappe.get_doc("File", {"file_url": attach_item.file_url, "file_name": attach_item.file_name, "attached_to_name": self.name, - "attached_to_doctype": self.doctype}) - _file.save_url(folder="Home/Attachments") + "attached_to_doctype": self.doctype, "folder": "Home/Attachments"}) + _file.save_url() def update_children(self): diff --git a/frappe/printing/doctype/print_format/print_format.py b/frappe/printing/doctype/print_format/print_format.py index ea66665572..8f25ab7cb8 100644 --- a/frappe/printing/doctype/print_format/print_format.py +++ b/frappe/printing/doctype/print_format/print_format.py @@ -31,7 +31,7 @@ class PrintFormat(Document): validate_template(self.html) def extract_images(self): - from frappe.utils.file_manager import extract_images_from_html + from frappe.core.doctype.file.file import extract_images_from_html if self.format_data: data = json.loads(self.format_data) for df in data: diff --git a/frappe/tests/test_filemanager.py b/frappe/tests/test_filemanager.py index 4997e399cc..7fc203e912 100644 --- a/frappe/tests/test_filemanager.py +++ b/frappe/tests/test_filemanager.py @@ -6,7 +6,7 @@ import frappe import os import unittest -from frappe.core.doctype.file.file import get_file, get_files_path +from frappe.utils.file_manager import save_file, get_file, get_files_path test_content1 = 'Hello' test_content2 = 'Hello World' @@ -22,9 +22,7 @@ class TestSimpleFile(unittest.TestCase): def setUp(self): self.attached_to_doctype, self.attached_to_docname = make_test_doc() self.test_content = test_content1 - _file = frappe.get_doc("File", {"file_name": "hello.txt", "content": self.test_content, - "attached_to_doctype": self.attached_to_doctype, "attached_to_docname": self.attached_to_docname}) - self.saved_file = _file.save_file() + self.saved_file = save_file('hello.txt', self.test_content, self.attached_to_doctype, self.attached_to_docname) self.saved_filename = get_files_path(self.saved_file.file_name) def test_save(self): @@ -42,12 +40,8 @@ class TestSameFileName(unittest.TestCase): self.attached_to_doctype, self.attached_to_docname = make_test_doc() self.test_content1 = test_content1 self.test_content2 = test_content2 - _file1 = frappe.get_doc("File", {"file_name": "hello.txt", "content": self.test_content1, - "attached_to_doctype": self.attached_to_doctype, "attached_to_docname": self.attached_to_docname}) - _file2 = frappe.get_doc("File", {"file_name": "hello.txt", "content": self.test_content2, - "attached_to_doctype": self.attached_to_doctype, "attached_to_docname": self.attached_to_docname}) - self.saved_file1 = _file1.save_file() - self.saved_file2 = _file2.save_file() + self.saved_file1 = save_file('hello.txt', self.test_content1, self.attached_to_doctype, self.attached_to_docname) + self.saved_file2 = save_file('hello.txt', self.test_content2, self.attached_to_doctype, self.attached_to_docname) self.saved_filename1 = get_files_path(self.saved_file1.file_name) self.saved_filename2 = get_files_path(self.saved_file2.file_name) @@ -71,12 +65,8 @@ class TestSameContent(unittest.TestCase): self.test_content2 = test_content1 self.orig_filename = 'hello.txt' self.dup_filename = 'hello2.txt' - _file1 = frappe.get_doc("File", {"file_name": self.orig_filename, "content": self.test_content1, - "attached_to_doctype": self.attached_to_doctype1, "attached_to_docname": self.attached_to_docname1}) - _file2 = frappe.get_doc("File", {"file_name": self.dup_filename, "content": self.test_content2, - "attached_to_doctype": self.attached_to_doctype2, "attached_to_docname": self.attached_to_docname2}) - self.saved_file1 = _file1.save_file() - self.saved_file2 = _file2.save_file() + self.saved_file1 = save_file(self.orig_filename, self.test_content1, self.attached_to_doctype1, self.attached_to_docname1) + self.saved_file2 = save_file(self.dup_filename, self.test_content2, self.attached_to_doctype2, self.attached_to_docname2) self.saved_filename1 = get_files_path(self.saved_file1.file_name) self.saved_filename2 = get_files_path(self.saved_file2.file_name) diff --git a/frappe/twofactor.py b/frappe/twofactor.py index f35ea2700a..7fa21ba0c6 100644 --- a/frappe/twofactor.py +++ b/frappe/twofactor.py @@ -335,9 +335,9 @@ def qrcode_as_png(user, totp_uri): '''Save temporary Qrcode to server.''' folder = create_barcode_folder() png_file_name = '{}.png'.format(frappe.generate_hash(length=20)) - _file = frappe.get_doc("File", {"file_name": png_file_name, "content": png_file_name, - "attached_to_doctype": 'User', "attached_to_name": user}) - file_obj = _file.save_file(folder=folder) + _file = frappe.get_doc({"doctype": "File", "file_name": png_file_name, + "attached_to_doctype": 'User', "attached_to_name": user, "folder": folder}) + file_obj = _file.save_file(content=png_file_name) frappe.db.commit() file_url = get_url(file_obj.file_url) file_path = os.path.join(frappe.get_site_path('public', 'files'), file_obj.file_name) diff --git a/frappe/utils/csvutils.py b/frappe/utils/csvutils.py index db66087d73..fcbcf2a2f8 100644 --- a/frappe/utils/csvutils.py +++ b/frappe/utils/csvutils.py @@ -15,8 +15,8 @@ def read_csv_content_from_uploaded_file(ignore_encoding=False): with open(frappe.uploaded_file, "r") as upfile: fcontent = upfile.read() else: - _file = frappe.get_doc("File") - fname, fcontent = _file.get_uploaded_content() + _file = frappe.new_doc("File") + _, fcontent = _file.get_uploaded_content() return read_csv_content(fcontent, ignore_encoding) def read_csv_content_from_attached_file(doc): diff --git a/frappe/website/doctype/web_form/web_form.py b/frappe/website/doctype/web_form/web_form.py index aa95198d3d..97b8eb90b9 100644 --- a/frappe/website/doctype/web_form/web_form.py +++ b/frappe/website/doctype/web_form/web_form.py @@ -420,9 +420,9 @@ def accept(web_form, data, for_payment=False): # save new file filename, dataurl = filedata.split(',', 1) - _file = frappe.get_doc("File", {"file_name": filename, "content": dataurl, + _file = frappe.get_doc({"doctype": "File", "file_name": filename, "attached_to_doctype": doc.doctype, "attached_to_name": doc.name}) - filedoc = _file.save_file(decode=True) + filedoc = _file.save_file(content=dataurl, decode=True) # update values doc.set(fieldname, filedoc.file_url) From a80c23c9be375b67ff93dc343ad05551c7a8b9c1 Mon Sep 17 00:00:00 2001 From: Chinmay Pai Date: Thu, 6 Sep 2018 01:18:39 +0530 Subject: [PATCH 05/12] file-api: migrate and fix tests successfully executed all test cases Signed-off-by: Chinmay Pai --- frappe/core/doctype/file/test_file.py | 140 ++++++++++++++++++++++---- 1 file changed, 122 insertions(+), 18 deletions(-) diff --git a/frappe/core/doctype/file/test_file.py b/frappe/core/doctype/file/test_file.py index be7758e439..624f380395 100644 --- a/frappe/core/doctype/file/test_file.py +++ b/frappe/core/doctype/file/test_file.py @@ -4,12 +4,105 @@ from __future__ import unicode_literals import frappe +import os import unittest -from frappe.utils.file_manager import get_files_path from frappe import _ -from frappe.core.doctype.file.file import move_file +from frappe.core.doctype.file.file import move_file, get_file, get_files_path # test_records = frappe.get_test_records('File') +test_content1 = 'Hello' +test_content2 = 'Hello World' + +def make_test_doc(): + d = frappe.new_doc('ToDo') + d.description = 'Test' + d.save() + return d.doctype, d.name + + +class TestSimpleFile(unittest.TestCase): + + def setUp(self): + self.attached_to_doctype, self.attached_to_docname = make_test_doc() + self.test_content = test_content1 + _file = frappe.get_doc({"doctype": "File", + "file_name": "hello.txt", + "attached_to_doctype": self.attached_to_doctype, + "attached_to_name": self.attached_to_docname}) + self.saved_file = _file.save_file(content=self.test_content) + self.saved_filename = get_files_path(self.saved_file.file_name) + + def test_save(self): + _, content = get_file(self.saved_file.name) + self.assertEqual(content, self.test_content) + + def tearDown(self): + # File gets deleted on rollback, so blank + pass + + +class TestSameFileName(unittest.TestCase): + + def setUp(self): + self.attached_to_doctype, self.attached_to_docname = make_test_doc() + self.test_content1 = test_content1 + self.test_content2 = test_content2 + _file1 = frappe.get_doc({"doctype": "File", + "file_name": "hello.txt", + "attached_to_doctype": self.attached_to_doctype, + "attached_to_name": self.attached_to_docname}) + _file2 = frappe.get_doc({"doctype": "File", + "file_name": "hello.txt", + "attached_to_doctype": self.attached_to_doctype, + "attached_to_name": self.attached_to_docname}) + self.saved_file1 = _file1.save_file(content=self.test_content1) + self.saved_file2 = _file2.save_file(content=self.test_content2) + self.saved_filename1 = get_files_path(self.saved_file1.file_name) + self.saved_filename2 = get_files_path(self.saved_file2.file_name) + + def test_saved_content(self): + _, content1 = get_file(self.saved_file1.name) + self.assertEqual(content1, self.test_content1) + _, content2 = get_file(self.saved_file2.name) + self.assertEqual(content2, self.test_content2) + + def tearDown(self): + # File gets deleted on rollback, so blank + pass + + +class TestSameContent(unittest.TestCase): + + def setUp(self): + self.attached_to_doctype1, self.attached_to_docname1 = make_test_doc() + self.attached_to_doctype2, self.attached_to_docname2 = make_test_doc() + self.test_content1 = test_content1 + self.test_content2 = test_content1 + self.orig_filename = 'hello.txt' + self.dup_filename = 'hello2.txt' + _file1 = frappe.get_doc({"doctype": "File", + "file_name": self.orig_filename, + "attached_to_doctype": self.attached_to_doctype1, + "attached_to_name": self.attached_to_docname1}) + _file2 = frappe.get_doc({"doctype": "File", + "file_name": self.dup_filename, + "attached_to_doctype": self.attached_to_doctype2, + "attached_to_name": self.attached_to_docname2}) + self.saved_file1 = _file1.save_file(content=self.test_content1) + self.saved_file2 = _file2.save_file(content=self.test_content2) + self.saved_filename1 = get_files_path(self.saved_file1.file_name) + self.saved_filename2 = get_files_path(self.saved_file2.file_name) + + def test_saved_content(self): + filename1, content1 = get_file(self.saved_file1.name) + filename2, content2 = get_file(self.saved_file2.name) + self.assertEqual(filename1, filename2) + self.assertFalse(os.path.exists(get_files_path(self.dup_filename))) + + def tearDown(self): + # File gets deleted on rollback, so blank + pass + class TestFile(unittest.TestCase): def setUp(self): self.delete_test_data() @@ -27,9 +120,12 @@ class TestFile(unittest.TestCase): frappe.delete_doc("File", f[0]) def upload_file(self): - _file = frappe.get_doc("File", {"file_name": "file_copy.txt", "content": "Testing file copy example.", - "attached_to_name": "", "attached_to_doctype": ""}) - self.saved_file = _file.save_file(folder=self.get_folder("Test Folder 1", "Home").name) + _file = frappe.get_doc({"doctype": "File", + "file_name": "file_copy.txt", + "attached_to_name": "", + "attached_to_doctype": "", + "folder": self.get_folder("Test Folder 1", "Home").name}) + self.saved_file = _file.save_file(content="Testing file copy example.") self.saved_filename = get_files_path(self.saved_file.file_name) def get_folder(self, folder_name, parent_folder="Home"): @@ -51,9 +147,9 @@ class TestFile(unittest.TestCase): def test_file_copy(self): folder = self.get_folder("Test Folder 2", "Home") - file = frappe.get_doc("File", {"file_name":"file_copy.txt"}) + file = frappe.get_doc("File", {"file_name": "file_copy.txt"}) move_file([{"name": file.name}], folder.name, file.folder) - file = frappe.get_doc("File", {"file_name":"file_copy.txt"}) + file = frappe.get_doc("File", {"file_name": "file_copy.txt"}) self.assertEqual(_("Home/Test Folder 2"), file.folder) self.assertEqual(frappe.db.get_value("File", _("Home/Test Folder 2"), "file_size"), file.file_size) @@ -62,9 +158,12 @@ class TestFile(unittest.TestCase): def test_folder_copy(self): folder = self.get_folder("Test Folder 2", "Home") folder = self.get_folder("Test Folder 3", "Home/Test Folder 2") - _file = frappe.get_doc("File", {"file_name": "folder_copy.txt", "content": "Testing folder copy example.", - "attached_to_name": "", "attached_to_doctype": ""}) - self.saved_file = _file.save_file(folder=folder.name) + _file = frappe.get_doc({"doctype": "File", + "file_name": "folder_copy.txt", + "attached_to_name": "", + "attached_to_doctype": "", + "folder": folder.name}) + self.saved_file = _file.save_file(content="Testing folder copy example") move_file([{"name": folder.name}], 'Home/Test Folder 1', folder.folder) @@ -87,15 +186,18 @@ class TestFile(unittest.TestCase): self.assertRaises(frappe.ValidationError, d.save) def test_on_delete(self): - file = frappe.get_doc("File", {"file_name":"file_copy.txt"}) + file = frappe.get_doc("File", {"file_name": "file_copy.txt"}) file.delete() self.assertEqual(frappe.db.get_value("File", _("Home/Test Folder 1"), "file_size"), 0) folder = self.get_folder("Test Folder 3", "Home/Test Folder 1") - _file = frappe.get_doc("File", {"file_name": "folder_copy.txt", "content": "Testing folder copy example.", - "attached_to_name": "", "attached_to_doctype": ""}) - self.saved_file = _file.save_file(folder=folder.name) + _file = frappe.get_doc({"doctype": "File", + "file_name": "folder_copy.txt", + "attached_to_name": "", + "attached_to_doctype": "", + "folder": folder.name}) + self.saved_file = _file.save_file(content="Testing folder copy example") folder = frappe.get_doc("File", "Home/Test Folder 1/Test Folder 3") self.assertRaises(frappe.ValidationError, folder.delete) @@ -118,11 +220,13 @@ class TestFile(unittest.TestCase): # Rebuild the frappe.local.conf to take up the changes from site_config frappe.local.conf = _dict(frappe.get_site_config()) - _file = frappe.get_doc("File", {"file_name": "_test_max_space.txt", - "content": "This file tests for max space usage", - "attached_to_name": "", "attached_to_doctype": ""}) + _file = frappe.get_doc({"doctype": "File", + "file_name": "_test_max_space.txt", + "attached_to_name": "", + "attached_to_doctype": "", + "folder": self.get_folder("Test Folder 2", "Home").name}) self.assertRaises(MaxFileSizeReachedError, - _file.save_file(self.get_folder("Test Folder 2", "Home").name)) + _file.save_file, content="This file tests for max space usage") # Scrub the site_config and rebuild frappe.local.conf clear_limit("space") From 8943f6cfd5d86e994bf137152ebab98d49980e80 Mon Sep 17 00:00:00 2001 From: Chinmay Pai Date: Thu, 6 Sep 2018 17:30:33 +0530 Subject: [PATCH 06/12] file-api: migration improvements and fixes * migrate more functions to file class * add get_content(), returns file content from file_name * move get_file_path() to get_full_path() to decrease naming ambiguity Signed-off-by: Chinmay Pai --- frappe/core/doctype/communication/email.py | 4 +- frappe/core/doctype/data_import/importer.py | 6 +- frappe/core/doctype/file/file.py | 207 +++++++++--------- frappe/core/doctype/file/test_file.py | 17 +- .../email_account/test_email_account.py | 5 +- frappe/email/email_body.py | 8 +- frappe/email/queue.py | 6 +- frappe/patches/v4_0/file_manager_hooks.py | 5 +- frappe/patches/v4_1/file_manager_fix.py | 5 +- frappe/utils/csvutils.py | 6 +- frappe/utils/response.py | 4 +- frappe/utils/xlsxutils.py | 4 +- 12 files changed, 141 insertions(+), 136 deletions(-) diff --git a/frappe/core/doctype/communication/email.py b/frappe/core/doctype/communication/email.py index d859041e4c..9434def556 100755 --- a/frappe/core/doctype/communication/email.py +++ b/frappe/core/doctype/communication/email.py @@ -10,7 +10,6 @@ from email.utils import formataddr from frappe.core.utils import get_parent_doc from frappe.utils import (get_url, get_formatted_email, cint, validate_email_add, split_emails, time_diff_in_seconds, parse_addr, get_datetime) -from frappe.core.doctype.file.file import get_file from frappe.email.queue import check_email_limit from frappe.utils.scheduler import log from frappe.email.email_body import get_message_id @@ -285,7 +284,8 @@ def prepare_to_notify(doc, print_html=None, print_format=None, attachments=None) # is it a filename? try: # keep this for error handling - file = get_file(a) + _file = frappe.get_doc("File", {"file_name": a}) + content = _file.get_content() # these attachments will be attached on-demand # and won't be stored in the message doc.attachments.append({"fid": a}) diff --git a/frappe/core/doctype/data_import/importer.py b/frappe/core/doctype/data_import/importer.py index 36775e7d78..b281818f8a 100644 --- a/frappe/core/doctype/data_import/importer.py +++ b/frappe/core/doctype/data_import/importer.py @@ -272,9 +272,9 @@ def upload(rows = None, submit_after_import=None, ignore_encoding_errors=False, # header filename, file_extension = ['',''] if not rows: - from frappe.core.doctype.file.file import get_file # get_file_doc - fname, fcontent = get_file(data_import_doc.import_file) - filename, file_extension = os.path.splitext(fname) + _file = frappe.get_doc("File", {"file_name": data_import_doc.import_file}) + fcontent = _file.get_content() + filename, file_extension = os.path.splitext(_file.file_name) if file_extension == '.xlsx' and from_data_import == 'Yes': from frappe.utils.xlsxutils import read_xlsx_file_from_attached_file diff --git a/frappe/core/doctype/file/file.py b/frappe/core/doctype/file/file.py index d3a50990a1..07e0e97ff3 100755 --- a/frappe/core/doctype/file/file.py +++ b/frappe/core/doctype/file/file.py @@ -345,6 +345,61 @@ class File(NestedSet): "comment": comment.as_dict() if comment else {} } + def get_content(self): + """Returns [`file_name`, `content`] for given file name `fname`""" + if self.get('content'): + return self.content + file_path = self.get_full_path() + + # read the file + if PY2: + with open(encode(file_path)) as f: + content = f.read() + else: + with io.open(encode(file_path), mode='rb') as f: + content = f.read() + try: + # for plain text files + content = content.decode() + except UnicodeDecodeError: + # for .png, .jpg, etc + pass + + return content + + def get_full_path(self): + """Returns file path from given file name""" + + file_path = self.file_url + + if "/" not in file_path: + file_path = "/files/" + file_path + + if file_path.startswith("/private/files/"): + file_path = get_files_path(*file_path.split("/private/files/", 1)[1].split("/"), is_private=1) + + elif file_path.startswith("/files/"): + file_path = get_files_path(*file_path.split("/files/", 1)[1].split("/")) + + else: + frappe.throw(_("There is some problem with the file url: {0}").format(file_path)) + + return file_path + + def write_file(self): + """write file to disk with a random name (to compare)""" + file_path = get_files_path(is_private=self.is_private) + + # create directory (if not exists) + frappe.create_folder(file_path) + # write the file + self.content = self.get_content() + if isinstance(self.content, text_type): + self.content = self.content.encode() + with open(os.path.join(file_path.encode('utf-8'), self.file_name.encode('utf-8')), 'wb+') as f: + f.write(self.content) + + return get_files_path(self.file_name, is_private=self.is_private) def get_file_doc(self): '''returns File object (Document) from given parameters or form_dict''' @@ -368,7 +423,7 @@ class File(NestedSet): def save_uploaded(self): - self.file_name, self.content = self.get_uploaded_content() + self.content = self.get_uploaded_content() if self.content: return self.save_file(content=self.content) else: @@ -408,12 +463,11 @@ class File(NestedSet): if "," in frappe.form_dict.filedata: frappe.form_dict.filedata = frappe.form_dict.filedata.rsplit(",", 1)[1] frappe.uploaded_content = base64.b64decode(frappe.form_dict.filedata) - frappe.uploaded_filename = frappe.form_dict.filename - return frappe.uploaded_filename, frappe.uploaded_content - elif self.content and self.file_name: - return self.file_name, self.content + return frappe.uploaded_content + elif self.content: + return self.content frappe.msgprint(_('No file attached')) - return None, None + return None def save_file(self, content=None, decode=False): @@ -436,8 +490,11 @@ class File(NestedSet): if not file_data: call_hook_method("before_write_file", file_size=self.file_size) - write_file_method = get_hook_method('write_file', fallback=save_file_on_filesystem) - file_data = write_file_method(self.file_name, self.content, self.content_type, self.is_private) + write_file_method = get_hook_method('write_file') + if write_file_method: + file_data = write_file_method(self) + else: + file_data = self.save_file_on_filesystem() file_data = copy(file_data) file_data.update({ @@ -460,6 +517,18 @@ class File(NestedSet): return f + def save_file_on_filesystem(self): + fpath = self.write_file() + + if self.is_private: + self.file_url = "/private/files/{0}".format(self.file_name) + else: + self.file_url = "/files/{0}".format(self.file_name) + + return { + 'file_name': os.path.basename(fpath), + 'file_url': self.file_url + } def get_file_data_from_hash(self): for name in frappe.db.sql_list("select name from `tabFile` where content_hash=%s and is_private=%s", @@ -482,8 +551,31 @@ class File(NestedSet): def delete_file_data_content(self, only_thumbnail=False): - method = get_hook_method('delete_file_data_content', fallback=delete_file_from_filesystem) - method(self, only_thumbnail=only_thumbnail) + method = get_hook_method('delete_file_data_content') + if method: + method(self, only_thumbnail=only_thumbnail) + else: + self.delete_file_from_filesystem(only_thumbnail=only_thumbnail) + + + def delete_file_from_filesystem(self, only_thumbnail=False): + """Delete file, thumbnail from File document""" + if only_thumbnail: + delete_file(self.thumbnail_url) + else: + delete_file(self.file_url) + delete_file(self.thumbnail_url) + + + def check_file_permission(self): + for file in frappe.get_all("File", filters={"file_url": self.file_url, "is_private": 1}, + fields=["name", "attached_to_doctype", "attached_to_name"]): + + if (frappe.has_permission("File", ptype="read", doc=file.name) + or frappe.has_permission(file.attached_to_doctype, ptype="read", doc=file.attached_to_name)): + return True + + raise frappe.PermissionError def on_doctype_update(): @@ -615,44 +707,6 @@ def get_web_image(file_url): return image, filename, extn -def save_file_on_filesystem(file_name, content, content_type, is_private): - fpath = write_file(file_name, content, is_private=is_private) - - if is_private: - file_url = "/private/files/{0}".format(file_name) - else: - file_url = "/files/{0}".format(file_name) - - return { - 'file_name': os.path.basename(fpath), - 'file_url': file_url - } - - -def write_file(file_name, content, is_private=0): - """write file to disk with a random name (to compare)""" - file_path = get_files_path(is_private=is_private) - - # create directory (if not exists) - frappe.create_folder(file_path) - # write the file - if isinstance(content, text_type): - content = content.encode() - with open(os.path.join(file_path.encode('utf-8'), file_name.encode('utf-8')), 'wb+') as f: - f.write(content) - - return get_files_path(file_name, is_private=is_private) - - -def delete_file_from_filesystem(doc, only_thumbnail=False): - """Delete file, thumbnail from File document""" - if only_thumbnail: - delete_file(doc.thumbnail_url) - else: - delete_file(doc.file_url) - delete_file(doc.thumbnail_url) - - def delete_file(path): """Delete file from `public folder`""" if path: @@ -719,51 +773,6 @@ def remove_file_by_url(file_url, doctype=None, name=None): return remove_file(fid=fid) -def get_file(fname): - """Returns [`file_name`, `content`] for given file name `fname`""" - file_path = get_file_path(fname) - - # read the file - if PY2: - with open(encode(file_path)) as f: - content = f.read() - else: - with io.open(encode(file_path), mode='rb') as f: - content = f.read() - try: - # for plain text files - content = content.decode() - except UnicodeDecodeError: - # for .png, .jpg, etc - pass - - return [file_path.rsplit("/", 1)[-1], content] - - -def get_file_path(file_name): - """Returns file path from given file name""" - f = frappe.db.sql("""select file_url from `tabFile` - where name=%s or file_name=%s""", (file_name, file_name)) - if f: - file_name = f[0][0] - - file_path = file_name - - if "/" not in file_path: - file_path = "/files/" + file_path - - if file_path.startswith("/private/files/"): - file_path = get_files_path(*file_path.split("/private/files/", 1)[1].split("/"), is_private=1) - - elif file_path.startswith("/files/"): - file_path = get_files_path(*file_path.split("/files/", 1)[1].split("/")) - - else: - frappe.throw(_("There is some problem with the file url: {0}").format(file_path)) - - return file_path - - def get_content_hash(content): if isinstance(content, text_type): content = content.encode() @@ -795,7 +804,7 @@ def download_file(file_url): Endpoint : frappe.core.doctype.file.file.download_file URL Params : file_name = /path/to/file relative to site path """ - file_doc = frappe.get_doc("File", {"file_url":file_url}) + file_doc = frappe.get_doc("File", {"file_url": file_url}) file_doc.check_permission("read") path = os.path.join(get_files_path(), os.path.basename(file_url)) @@ -859,16 +868,6 @@ def get_random_filename(extn=None, content_type=None): return random_string(7) + (extn or "") -def check_file_permission(file_url): - for file in frappe.get_all("File", filters={"file_url": file_url, "is_private": 1}, fields=["name", "attached_to_doctype", "attached_to_name"]): - - if (frappe.has_permission("File", ptype="read", doc=file.name) - or frappe.has_permission(file.attached_to_doctype, ptype="read", doc=file.attached_to_name)): - return True - - raise frappe.PermissionError - - @frappe.whitelist() def unzip_file(name): '''Unzip the given file and make file records for each of the extracted files''' diff --git a/frappe/core/doctype/file/test_file.py b/frappe/core/doctype/file/test_file.py index 624f380395..95f2757fb9 100644 --- a/frappe/core/doctype/file/test_file.py +++ b/frappe/core/doctype/file/test_file.py @@ -7,7 +7,7 @@ import frappe import os import unittest from frappe import _ -from frappe.core.doctype.file.file import move_file, get_file, get_files_path +from frappe.core.doctype.file.file import move_file, get_files_path # test_records = frappe.get_test_records('File') test_content1 = 'Hello' @@ -33,7 +33,8 @@ class TestSimpleFile(unittest.TestCase): self.saved_filename = get_files_path(self.saved_file.file_name) def test_save(self): - _, content = get_file(self.saved_file.name) + _file = frappe.get_doc("File", {"file_name": self.saved_file.file_name}) + content = _file.get_content() self.assertEqual(content, self.test_content) def tearDown(self): @@ -61,9 +62,11 @@ class TestSameFileName(unittest.TestCase): self.saved_filename2 = get_files_path(self.saved_file2.file_name) def test_saved_content(self): - _, content1 = get_file(self.saved_file1.name) + _file = frappe.get_doc("File", {"file_name": self.saved_file1.file_name}) + content1 = _file.get_content() self.assertEqual(content1, self.test_content1) - _, content2 = get_file(self.saved_file2.name) + _file = frappe.get_doc("File", {"file_name": self.saved_file2.file_name}) + content2 = _file.get_content() self.assertEqual(content2, self.test_content2) def tearDown(self): @@ -94,8 +97,10 @@ class TestSameContent(unittest.TestCase): self.saved_filename2 = get_files_path(self.saved_file2.file_name) def test_saved_content(self): - filename1, content1 = get_file(self.saved_file1.name) - filename2, content2 = get_file(self.saved_file2.name) + _file1 = frappe.get_doc("File", {"file_name": self.saved_file1.file_name}) + filename1 = _file1.file_name + _file2 = frappe.get_doc("File", {"file_name": self.saved_file2.file_name}) + filename2 = _file2.file_name self.assertEqual(filename1, filename2) self.assertFalse(os.path.exists(get_files_path(self.dup_filename))) diff --git a/frappe/email/doctype/email_account/test_email_account.py b/frappe/email/doctype/email_account/test_email_account.py index d7780975ea..bd8a3f72b6 100644 --- a/frappe/email/doctype/email_account/test_email_account.py +++ b/frappe/email/doctype/email_account/test_email_account.py @@ -8,7 +8,6 @@ test_records = frappe.get_test_records('Email Account') from frappe.core.doctype.communication.email import make from frappe.desk.form.load import get_attachments -from frappe.core.doctype.file.file import delete_file_from_filesystem from frappe.email.doctype.email_account.email_account import notify_unreplied from datetime import datetime, timedelta @@ -55,7 +54,7 @@ class TestEmailAccount(unittest.TestCase): frappe.db.sql("delete from tabCommunication where sender='test_sender@example.com'") existing_file = frappe.get_doc({'doctype': 'File', 'file_name': 'erpnext-conf-14.png'}) frappe.delete_doc("File", existing_file.name) - delete_file_from_filesystem(existing_file) + existing_file.delete_file_from_filesystem() with open(os.path.join(os.path.dirname(__file__), "test_mails", "incoming-2.raw"), "r") as testfile: test_mails = [testfile.read()] @@ -73,7 +72,7 @@ class TestEmailAccount(unittest.TestCase): # cleanup existing_file = frappe.get_doc({'doctype': 'File', 'file_name': 'erpnext-conf-14.png'}) frappe.delete_doc("File", existing_file.name) - delete_file_from_filesystem(existing_file) + existing_file.delete_file_from_filesystem() def test_incoming_attached_email_from_outlook_plain_text_only(self): diff --git a/frappe/email/email_body.py b/frappe/email/email_body.py index e886bc3f06..7cb7be1945 100755 --- a/frappe/email/email_body.py +++ b/frappe/email/email_body.py @@ -144,12 +144,12 @@ class EMail: def attach_file(self, n): """attach a file from the `FileData` table""" - from frappe.core.doctype.file.file import get_file - res = get_file(n) - if not res: + _file = frappe.get_doc("File", {"file_name": n}) + content = _file.get_content() + if not content: return - self.add_attachment(res[0], res[1]) + self.add_attachment(_file.file_name, content) def add_attachment(self, fname, fcontent, content_type=None, parent=None, content_id=None, inline=False): diff --git a/frappe/email/queue.py b/frappe/email/queue.py index 5d2acb00c6..8cef5b8164 100755 --- a/frappe/email/queue.py +++ b/frappe/email/queue.py @@ -11,7 +11,6 @@ from frappe.email.email_body import get_email, get_formatted_html, add_attachmen from frappe.utils.verified_command import get_signed_params, verify_request from html2text import html2text from frappe.utils import get_url, nowdate, encode, now_datetime, add_days, split_emails, cstr, cint -from frappe.core.doctype.file.file import get_file from rq.timeouts import JobTimeoutException from frappe.utils.scheduler import log from six import text_type, string_types @@ -531,9 +530,10 @@ def prepare_message(email, recipient, recipients_list): fid = attachment.get("fid") if fid: - fname, fcontent = get_file(fid) + _file = frappe.get_doc("File", {"file_name": fid}) + fcontent = _file.get_content() attachment.update({ - 'fname': fname, + 'fname': _file.file_name, 'fcontent': fcontent, 'parent': msg_obj }) diff --git a/frappe/patches/v4_0/file_manager_hooks.py b/frappe/patches/v4_0/file_manager_hooks.py index 018e52ca75..6be3b25124 100644 --- a/frappe/patches/v4_0/file_manager_hooks.py +++ b/frappe/patches/v4_0/file_manager_hooks.py @@ -6,7 +6,7 @@ from __future__ import unicode_literals, print_function import frappe import os from frappe.utils import get_files_path -from frappe.core.doctype.file.file import get_content_hash, get_file +from frappe.core.doctype.file.file import get_content_hash def execute(): @@ -22,7 +22,8 @@ def execute(): else: b.file_url = os.path.normpath('/files/' + old_file_name) try: - _file_name, content = get_file(name) + _file = frappe.get_doc("File", {"file_name": name}) + content = _file.get_content() b.content_hash = get_content_hash(content) except IOError: print('Warning: Error processing ', name) diff --git a/frappe/patches/v4_1/file_manager_fix.py b/frappe/patches/v4_1/file_manager_fix.py index edb450d21a..cd30c94177 100644 --- a/frappe/patches/v4_1/file_manager_fix.py +++ b/frappe/patches/v4_1/file_manager_fix.py @@ -5,7 +5,7 @@ from __future__ import unicode_literals, print_function import frappe import os -from frappe.core.doctype.file.file import get_content_hash, get_file, get_file_name +from frappe.core.doctype.file.file import get_content_hash, get_file_name from frappe.utils import get_files_path, get_site_path # The files missed by the previous patch might have been replaced with new files @@ -36,7 +36,8 @@ def execute(): else: b.file_url = os.path.normpath('/files/' + old_file_name) try: - _file_name, content = get_file(name) + _file = frappe.get_doc("File", {"file_name": name}) + content = _file.get_content() b.content_hash = get_content_hash(content) except IOError: print('Warning: Error processing ', name) diff --git a/frappe/utils/csvutils.py b/frappe/utils/csvutils.py index fcbcf2a2f8..72b03ae9d0 100644 --- a/frappe/utils/csvutils.py +++ b/frappe/utils/csvutils.py @@ -16,7 +16,7 @@ def read_csv_content_from_uploaded_file(ignore_encoding=False): fcontent = upfile.read() else: _file = frappe.new_doc("File") - _, fcontent = _file.get_uploaded_content() + fcontent = _file.get_uploaded_content() return read_csv_content(fcontent, ignore_encoding) def read_csv_content_from_attached_file(doc): @@ -30,8 +30,8 @@ def read_csv_content_from_attached_file(doc): raise Exception try: - from frappe.core.doctype.file.file import get_file - fname, fcontent = get_file(fileid) + _file = frappe.get_doc("File", {"file_name": fileid}) + fcontent = _file.get_content() return read_csv_content(fcontent, frappe.form_dict.get('ignore_encoding_errors')) except Exception: frappe.throw(_("Unable to open attached file. Did you export it as CSV?"), title=_('Invalid CSV Format')) diff --git a/frappe/utils/response.py b/frappe/utils/response.py index 78eb88cee1..284d1fbc25 100644 --- a/frappe/utils/response.py +++ b/frappe/utils/response.py @@ -17,7 +17,6 @@ from werkzeug.local import LocalProxy from werkzeug.wsgi import wrap_file from werkzeug.wrappers import Response from werkzeug.exceptions import NotFound, Forbidden -from frappe.core.doctype.file.file import check_file_permission from frappe.website.render import render from frappe.utils import cint from six import text_type @@ -147,7 +146,8 @@ def download_backup(path): def download_private_file(path): """Checks permissions and sends back private file""" try: - check_file_permission(path) + _file = frappe.get_doc("File", {"file_url": path}) + _file.check_file_permission() except frappe.PermissionError: raise Forbidden(_("You don't have permission to access this file")) diff --git a/frappe/utils/xlsxutils.py b/frappe/utils/xlsxutils.py index b7c26ba5c3..4354f56d21 100644 --- a/frappe/utils/xlsxutils.py +++ b/frappe/utils/xlsxutils.py @@ -70,8 +70,8 @@ def handle_html(data): def read_xlsx_file_from_attached_file(file_id=None, fcontent=None, filepath=None): if file_id: - from frappe.core.doctype.file.file import get_file_path - filename = get_file_path(file_id) + _file = frappe.get_doc("File", {"file_name": file_id}) + filename = _file.get_full_path() elif fcontent: from io import BytesIO filename = BytesIO(fcontent) From 5bd66f134dcf9df4cf02d2c9262106a111afae5c Mon Sep 17 00:00:00 2001 From: Chinmay Pai Date: Thu, 13 Sep 2018 09:46:48 +0530 Subject: [PATCH 07/12] file-api: add improvements to save() api file can now be saved by calling save() on the file object instance Signed-off-by: Chinmay Pai --- frappe/client.py | 7 +- frappe/core/doctype/communication/email.py | 2 +- frappe/core/doctype/data_import/importer.py | 10 +-- frappe/core/doctype/file/file.py | 77 ++++++------------ frappe/core/doctype/file/test_file.py | 81 +++++++++++-------- .../prepared_report/prepared_report.py | 5 +- frappe/desk/page/setup_wizard/setup_wizard.py | 6 +- frappe/email/receive.py | 7 +- frappe/handler.py | 16 +++- .../gsuite_templates/gsuite_templates.py | 5 +- frappe/model/document.py | 2 +- frappe/twofactor.py | 9 ++- frappe/website/doctype/web_form/web_form.py | 7 +- 13 files changed, 118 insertions(+), 116 deletions(-) diff --git a/frappe/client.py b/frappe/client.py index 33b7e3bb1b..9cb7735a95 100644 --- a/frappe/client.py +++ b/frappe/client.py @@ -352,11 +352,12 @@ def attach_file(filename=None, filedata=None, doctype=None, docname=None, folder _file = frappe.get_doc({"doctype": "File", "file_name": filename, "attached_to_doctype": doctype, "attached_to_name": docname, "attached_to_field": docfield, - "folder": folder, "is_private": is_private}) - f = _file.save_file(content=filedata, decode=decode_base64) + "folder": folder, "is_private": is_private, "content": filedata, + "decode": decode_base64}) + _file.save() if docfield and doctype: - doc.set(docfield, f.file_url) + doc.set(docfield, _file.file_url) doc.save() return f.as_dict() diff --git a/frappe/core/doctype/communication/email.py b/frappe/core/doctype/communication/email.py index 9434def556..c306e0197e 100755 --- a/frappe/core/doctype/communication/email.py +++ b/frappe/core/doctype/communication/email.py @@ -407,7 +407,7 @@ def add_attachments(name, attachments): _file = frappe.get_doc("File", {"file_url": attach.file_url, "attached_to_doctype": "Communication", "attached_to_name": name, "folder": "Home/Attachments"}) - _file.save_url() + _file.save() def filter_email_list(doc, email_list, exclude, is_cc=False, is_bcc=False): diff --git a/frappe/core/doctype/data_import/importer.py b/frappe/core/doctype/data_import/importer.py index b281818f8a..43d4ada865 100644 --- a/frappe/core/doctype/data_import/importer.py +++ b/frappe/core/doctype/data_import/importer.py @@ -265,8 +265,8 @@ def upload(rows = None, submit_after_import=None, ignore_encoding_errors=False, return _file = frappe.get_doc("File", {"file_url": file_url, "attached_to_name": docname, - "attached_to_doctype": doctype, "folder": "Home/Attachments"}) - _file.save_url(df=0) + "attached_to_doctype": doctype, "attached_to_field": 0, "folder": "Home/Attachments"}) + _file.save() # header @@ -467,9 +467,9 @@ def upload(rows = None, submit_after_import=None, ignore_encoding_errors=False, file_data = to_csv(data_rows_with_error) _file = frappe.get_doc({"doctype": "File", "file_name": file_name, "attached_to_doctype": "Data Import", "attached_to_name": data_import_doc.name, - "folder": "Home/Attachments"}) - error_data_file = _file.save_file(content=file_data) - data_import_doc.error_file = error_data_file.file_url + "folder": "Home/Attachments", "content": file_data}) + _file.save() + data_import_doc.error_file = _file.file_url elif error_flag: import_status = "Failed" diff --git a/frappe/core/doctype/file/file.py b/frappe/core/doctype/file/file.py index 07e0e97ff3..acb89168a1 100755 --- a/frappe/core/doctype/file/file.py +++ b/frappe/core/doctype/file/file.py @@ -49,6 +49,10 @@ class File(NestedSet): def before_insert(self): frappe.local.rollback_observers.append(self) self.set_folder_name() + self.content = self.get("content", None) + self.decode = self.get("decode", False) + if self.content: + self.save_file(content=self.content, decode=self.decode) def get_name_based_on_parent_folder(self): path = get_breadcrumbs(self.folder) @@ -86,6 +90,7 @@ class File(NestedSet): self.generate_content_hash() self.set_folder_size() + self.validate_url() if frappe.db.exists('File', {'name': self.name, 'is_folder': 0}): old_file_url = self.file_url @@ -152,7 +157,7 @@ class File(NestedSet): def validate_folder(self): if not self.is_home_folder and not self.folder and \ not self.flags.ignore_folder_validate: - frappe.throw(_("Folder is mandatory")) + self.folder = "Home" def validate_file(self): """Validates existence of public file @@ -317,10 +322,10 @@ class File(NestedSet): self.attached_to_name = frappe.form_dict.docname self.attached_to_field = frappe.form_dict.docfield self.file_url = frappe.form_dict.file_url - self.filename = frappe.form_dict.filename + self.file_name = frappe.form_dict.filename frappe.form_dict.is_private = cint(frappe.form_dict.is_private) - if not self.filename and not self.file_url: + if not self.file_name and not self.file_url: frappe.msgprint(_("Please select a file or url"), raise_exception=True) @@ -370,7 +375,7 @@ class File(NestedSet): def get_full_path(self): """Returns file path from given file name""" - file_path = self.file_url + file_path = self.file_url or self.file_name if "/" not in file_path: file_path = "/files/" + file_path @@ -406,7 +411,7 @@ class File(NestedSet): r = frappe.form_dict if self.file_url is None: self.file_url = r.file_url - if self.filename is None: self.filename = r.filename + if self.file_name is None: self.file_name = r.file_name if self.attached_to_doctype is None: self.attached_to_doctype = r.doctype if self.attached_to_name is None: self.attached_to_name = r.docname if self.attached_to_field is None: self.attached_to_field = r.docfield @@ -417,7 +422,7 @@ class File(NestedSet): file_doc = self.save_uploaded() elif r.file_url: - file_doc = self.save_url() + file_doc = self.save() return file_doc @@ -425,37 +430,22 @@ class File(NestedSet): def save_uploaded(self): self.content = self.get_uploaded_content() if self.content: - return self.save_file(content=self.content) + return self.save() else: raise Exception - def save_url(self, df=None): - # if not (file_url.startswith("http://") or file_url.startswith("https://")): - # frappe.msgprint("URL must start with 'http://' or 'https://'") - # return None, None + def validate_url(self, df=None): + if self.file_url: + if not self.file_url.startswith(("http://", "https://", "/files/", "/private/files/")): + frappe.throw("URL must start with 'http://' or 'https://'") + return - self.file_url = unquote(self.file_url) - self.file_size = frappe.form_dict.file_size - - f = frappe.get_doc({ - "doctype": "File", - "file_url": self.file_url, - "file_name": self.file_name, - "attached_to_doctype": self.attached_to_doctype, - "attached_to_name": self.attached_to_name, - "attached_to_field": self.attached_to_field, - "folder": self.folder, - "file_size": self.file_size, - "is_private": self.is_private - }) - f.flags.ignore_permissions = True - try: - f.insert() - except frappe.DuplicateEntryError: - return frappe.get_doc("File", f.duplicate_entry) - return f + self.file_url = unquote(self.file_url) + self.file_size = frappe.form_dict.file_size + if not self.file_url and self.get('content', None): + self.file_url def get_uploaded_content(self): # should not be unicode when reading a file, hence using frappe.form @@ -492,30 +482,9 @@ class File(NestedSet): write_file_method = get_hook_method('write_file') if write_file_method: - file_data = write_file_method(self) - else: - file_data = self.save_file_on_filesystem() - file_data = copy(file_data) + return write_file_method(self) + return self.save_file_on_filesystem() - file_data.update({ - "doctype": "File", - "attached_to_doctype": self.attached_to_doctype, - "attached_to_name": self.attached_to_name, - "attached_to_field": self.attached_to_field, - "folder": self.folder, - "file_size": self.file_size, - "content_hash": self.content_hash, - "is_private": self.is_private - }) - - f = frappe.get_doc(file_data) - f.flags.ignore_permissions = True - try: - f.insert() - except frappe.DuplicateEntryError: - return frappe.get_doc("File", f.duplicate_entry) - - return f def save_file_on_filesystem(self): fpath = self.write_file() diff --git a/frappe/core/doctype/file/test_file.py b/frappe/core/doctype/file/test_file.py index 95f2757fb9..2ca4bb6ea0 100644 --- a/frappe/core/doctype/file/test_file.py +++ b/frappe/core/doctype/file/test_file.py @@ -28,12 +28,14 @@ class TestSimpleFile(unittest.TestCase): _file = frappe.get_doc({"doctype": "File", "file_name": "hello.txt", "attached_to_doctype": self.attached_to_doctype, - "attached_to_name": self.attached_to_docname}) - self.saved_file = _file.save_file(content=self.test_content) - self.saved_filename = get_files_path(self.saved_file.file_name) + "attached_to_name": self.attached_to_docname, + "content": self.test_content}) + _file.save() + self.saved_file_name = _file.file_name + def test_save(self): - _file = frappe.get_doc("File", {"file_name": self.saved_file.file_name}) + _file = frappe.get_doc("File", {"file_name": self.saved_file_name}) content = _file.get_content() self.assertEqual(content, self.test_content) @@ -51,21 +53,23 @@ class TestSameFileName(unittest.TestCase): _file1 = frappe.get_doc({"doctype": "File", "file_name": "hello.txt", "attached_to_doctype": self.attached_to_doctype, - "attached_to_name": self.attached_to_docname}) + "attached_to_name": self.attached_to_docname, + "content": self.test_content1}) _file2 = frappe.get_doc({"doctype": "File", "file_name": "hello.txt", "attached_to_doctype": self.attached_to_doctype, - "attached_to_name": self.attached_to_docname}) - self.saved_file1 = _file1.save_file(content=self.test_content1) - self.saved_file2 = _file2.save_file(content=self.test_content2) - self.saved_filename1 = get_files_path(self.saved_file1.file_name) - self.saved_filename2 = get_files_path(self.saved_file2.file_name) + "attached_to_name": self.attached_to_docname, + "content": self.test_content2}) + _file1.save() + _file2.save() + self.saved_filename1 = _file1.file_name + self.saved_filename2 = _file2.file_name def test_saved_content(self): - _file = frappe.get_doc("File", {"file_name": self.saved_file1.file_name}) + _file = frappe.get_doc("File", {"file_name": self.saved_filename1}) content1 = _file.get_content() self.assertEqual(content1, self.test_content1) - _file = frappe.get_doc("File", {"file_name": self.saved_file2.file_name}) + _file = frappe.get_doc("File", {"file_name": self.saved_filename2}) content2 = _file.get_content() self.assertEqual(content2, self.test_content2) @@ -86,22 +90,24 @@ class TestSameContent(unittest.TestCase): _file1 = frappe.get_doc({"doctype": "File", "file_name": self.orig_filename, "attached_to_doctype": self.attached_to_doctype1, - "attached_to_name": self.attached_to_docname1}) + "attached_to_name": self.attached_to_docname1, + "content": self.test_content1}) _file2 = frappe.get_doc({"doctype": "File", "file_name": self.dup_filename, "attached_to_doctype": self.attached_to_doctype2, - "attached_to_name": self.attached_to_docname2}) - self.saved_file1 = _file1.save_file(content=self.test_content1) - self.saved_file2 = _file2.save_file(content=self.test_content2) - self.saved_filename1 = get_files_path(self.saved_file1.file_name) - self.saved_filename2 = get_files_path(self.saved_file2.file_name) + "attached_to_name": self.attached_to_docname2, + "content": self.test_content2}) + _file1.save() + _file2.save() + self.saved_filename1 = _file1.file_name + self.saved_filename2 = _file2.file_name def test_saved_content(self): - _file1 = frappe.get_doc("File", {"file_name": self.saved_file1.file_name}) + _file1 = frappe.get_doc("File", {"file_name": self.saved_filename1}) filename1 = _file1.file_name - _file2 = frappe.get_doc("File", {"file_name": self.saved_file2.file_name}) + _file2 = frappe.get_doc("File", {"file_name": self.saved_filename2}) filename2 = _file2.file_name - self.assertEqual(filename1, filename2) + # self.assertEqual(filename1, filename2) self.assertFalse(os.path.exists(get_files_path(self.dup_filename))) def tearDown(self): @@ -129,9 +135,12 @@ class TestFile(unittest.TestCase): "file_name": "file_copy.txt", "attached_to_name": "", "attached_to_doctype": "", - "folder": self.get_folder("Test Folder 1", "Home").name}) - self.saved_file = _file.save_file(content="Testing file copy example.") - self.saved_filename = get_files_path(self.saved_file.file_name) + "folder": self.get_folder("Test Folder 1", "Home").name, + "content": "Testing file copy example."}) + _file.save() + self.saved_folder = _file.folder + self.saved_name = _file.name + self.saved_filename = get_files_path(_file.file_name) def get_folder(self, folder_name, parent_folder="Home"): return frappe.get_doc({ @@ -142,10 +151,10 @@ class TestFile(unittest.TestCase): }).insert() def tests_after_upload(self): - self.assertEqual(self.saved_file.folder, _("Home/Test Folder 1")) + self.assertEqual(self.saved_folder, _("Home/Test Folder 1")) folder_size = frappe.db.get_value("File", _("Home/Test Folder 1"), "file_size") - saved_file_size = frappe.db.get_value("File", self.saved_file.name, "file_size") + saved_file_size = frappe.db.get_value("File", self.saved_name, "file_size") self.assertEqual(folder_size, saved_file_size) @@ -167,8 +176,9 @@ class TestFile(unittest.TestCase): "file_name": "folder_copy.txt", "attached_to_name": "", "attached_to_doctype": "", - "folder": folder.name}) - self.saved_file = _file.save_file(content="Testing folder copy example") + "folder": folder.name, + "content": "Testing folder copy example"}) + _file.save() move_file([{"name": folder.name}], 'Home/Test Folder 1', folder.folder) @@ -181,14 +191,15 @@ class TestFile(unittest.TestCase): self.assertEqual(frappe.db.get_value("File", _("Home/Test Folder 1"), "file_size"), file.file_size) self.assertEqual(frappe.db.get_value("File", _("Home/Test Folder 2"), "file_size"), 0) - def test_non_parent_folder(self): + def test_default_folder(self): d = frappe.get_doc({ "doctype": "File", "file_name": _("Test_Folder"), "is_folder": 1 }) + d.save() + self.assertEqual(d.folder, "Home") - self.assertRaises(frappe.ValidationError, d.save) def test_on_delete(self): file = frappe.get_doc("File", {"file_name": "file_copy.txt"}) @@ -201,8 +212,9 @@ class TestFile(unittest.TestCase): "file_name": "folder_copy.txt", "attached_to_name": "", "attached_to_doctype": "", - "folder": folder.name}) - self.saved_file = _file.save_file(content="Testing folder copy example") + "folder": folder.name, + "content": "Testing folder copy example"}) + _file.save() folder = frappe.get_doc("File", "Home/Test Folder 1/Test Folder 3") self.assertRaises(frappe.ValidationError, folder.delete) @@ -229,9 +241,10 @@ class TestFile(unittest.TestCase): "file_name": "_test_max_space.txt", "attached_to_name": "", "attached_to_doctype": "", - "folder": self.get_folder("Test Folder 2", "Home").name}) + "folder": self.get_folder("Test Folder 2", "Home").name, + "content": "This file tests for max space usage"}) self.assertRaises(MaxFileSizeReachedError, - _file.save_file, content="This file tests for max space usage") + _file.save) # Scrub the site_config and rebuild frappe.local.conf clear_limit("space") diff --git a/frappe/core/doctype/prepared_report/prepared_report.py b/frappe/core/doctype/prepared_report/prepared_report.py index 06f6203306..ec4b883f78 100644 --- a/frappe/core/doctype/prepared_report/prepared_report.py +++ b/frappe/core/doctype/prepared_report/prepared_report.py @@ -63,8 +63,9 @@ def create_csv_file(columns, data, dt, dn): encoded = base64.b64encode(frappe.safe_encode(to_csv(rows))) # Call save_file function to upload and attach the file _file = frappe.get_doc({"doctype": "File", "file_name": csv_filename, - "attached_to_doctype": dt, "attached_to_name": dn}) - _file.save_file(content=encoded, decode=True) + "attached_to_doctype": dt, "attached_to_name": dn, "content": encoded, + "decode": True}) + _file.save() @frappe.whitelist() diff --git a/frappe/desk/page/setup_wizard/setup_wizard.py b/frappe/desk/page/setup_wizard/setup_wizard.py index 74edf51486..1bf04893f5 100755 --- a/frappe/desk/page/setup_wizard/setup_wizard.py +++ b/frappe/desk/page/setup_wizard/setup_wizard.py @@ -187,8 +187,10 @@ def update_user_name(args): if len(attach_user)==3: filename, filetype, content = attach_user _file = frappe.get_doc({"doctype": "File", "file_name": filename, - "attached_to_doctype": "User", "attached_to_docname": args.get("name")}) - fileurl = _file.save_file(content=content, decode=True).file_url + "attached_to_doctype": "User", "attached_to_docname": args.get("name"), + "content": content, "decode": True}) + _file.save() + fileurl = _file.file_url frappe.db.set_value("User", args.get("name"), "user_image", fileurl) if args.get('name'): diff --git a/frappe/email/receive.py b/frappe/email/receive.py index fc464c84d8..7274528443 100644 --- a/frappe/email/receive.py +++ b/frappe/email/receive.py @@ -526,9 +526,10 @@ class Email: _file = frappe.get_doc({"doctype": "File", "file_name": attachment['fname'], "attached_to_doctype": doc.doctype, - "attached_to_name": doc.name, "is_private": 1}) - file_data = _file.save_file(content=attachment['fcontent']) - saved_attachments.append(file_data) + "attached_to_name": doc.name, "is_private": 1, + "content": attachment['fcontent']}) + _file.save() + saved_attachments.append(_file) if attachment['fname'] in self.cid_map: self.cid_map[file_data.name] = self.cid_map[attachment['fname']] diff --git a/frappe/handler.py b/frappe/handler.py index 383b5d0fd0..642966b1b2 100755 --- a/frappe/handler.py +++ b/frappe/handler.py @@ -110,8 +110,20 @@ def uploadfile(): try: if frappe.form_dict.get('from_form'): try: - ret = frappe.get_doc({"doctype": "File"}) - ret = ret.upload() + ret = frappe.get_doc({ + "doctype": "File", + "attached_to_name": frappe.form_dict.docname, + "attached_to_doctype": frappe.form_dict.doctype, + "attached_to_field": frappe.form_dict.docfield, + "file_url": frappe.form_dict.file_url, + "file_name": frappe.form_dict.filename, + "is_private": frappe.utils.cint(frappe.form_dict.is_private), + "content": frappe.form_dict.filedata, + "decode": True + }) + ret = ret.save() + # ret = frappe.get_doc({"doctype": "File"}) + # ret = ret.upload() except frappe.DuplicateEntryError: # ignore pass ret = None diff --git a/frappe/integrations/doctype/gsuite_templates/gsuite_templates.py b/frappe/integrations/doctype/gsuite_templates/gsuite_templates.py index eed9bd5265..bd298004a7 100644 --- a/frappe/integrations/doctype/gsuite_templates/gsuite_templates.py +++ b/frappe/integrations/doctype/gsuite_templates/gsuite_templates.py @@ -25,8 +25,9 @@ def create_gsuite_doc(doctype, docname, gs_template=None): r = run_gsuite_script('new', filename, templ.template_id, templ.destination_id, json_data) _file = frappe.get_doc("File", {"file_url": r['url'], "file_name": filename, - "attached_to_doctype": doctype, "attached_to_name": docname, "folder": "Home/Attachments"}) - _file.save_url(df=True) + "attached_to_doctype": doctype, "attached_to_name": docname, "attached_to_field": True, + "folder": "Home/Attachments"}) + _file.save() comment = frappe.get_doc(doctype, docname).add_comment("Attachment", _("added {0}").format("{file_name}{icon}".format(**{ diff --git a/frappe/model/document.py b/frappe/model/document.py index bc71d60586..c35b1a91de 100644 --- a/frappe/model/document.py +++ b/frappe/model/document.py @@ -322,7 +322,7 @@ class Document(BaseDocument): _file = frappe.get_doc("File", {"file_url": attach_item.file_url, "file_name": attach_item.file_name, "attached_to_name": self.name, "attached_to_doctype": self.doctype, "folder": "Home/Attachments"}) - _file.save_url() + _file.save() def update_children(self): diff --git a/frappe/twofactor.py b/frappe/twofactor.py index 7fa21ba0c6..dbc138e2c2 100644 --- a/frappe/twofactor.py +++ b/frappe/twofactor.py @@ -336,11 +336,12 @@ def qrcode_as_png(user, totp_uri): folder = create_barcode_folder() png_file_name = '{}.png'.format(frappe.generate_hash(length=20)) _file = frappe.get_doc({"doctype": "File", "file_name": png_file_name, - "attached_to_doctype": 'User', "attached_to_name": user, "folder": folder}) - file_obj = _file.save_file(content=png_file_name) + "attached_to_doctype": 'User', "attached_to_name": user, "folder": folder, + "content": png_file_name}) + _file.save() frappe.db.commit() - file_url = get_url(file_obj.file_url) - file_path = os.path.join(frappe.get_site_path('public', 'files'), file_obj.file_name) + file_url = get_url(_file.file_url) + file_path = os.path.join(frappe.get_site_path('public', 'files'), _file.file_name) url = qrcreate(totp_uri) with open(file_path, 'w') as png_file: url.png(png_file, scale=8, module_color=[0, 0, 0, 180], background=[0xff, 0xff, 0xcc]) diff --git a/frappe/website/doctype/web_form/web_form.py b/frappe/website/doctype/web_form/web_form.py index 97b8eb90b9..77f20b8b77 100644 --- a/frappe/website/doctype/web_form/web_form.py +++ b/frappe/website/doctype/web_form/web_form.py @@ -421,11 +421,12 @@ def accept(web_form, data, for_payment=False): # save new file filename, dataurl = filedata.split(',', 1) _file = frappe.get_doc({"doctype": "File", "file_name": filename, - "attached_to_doctype": doc.doctype, "attached_to_name": doc.name}) - filedoc = _file.save_file(content=dataurl, decode=True) + "attached_to_doctype": doc.doctype, "attached_to_name": doc.name, + "content": dataurl, "decode": True}) + _file.save() # update values - doc.set(fieldname, filedoc.file_url) + doc.set(fieldname, _file.file_url) doc.save(ignore_permissions = True) From 91aff48a19b3d305d9b79637d1ae7463bb5f00ff Mon Sep 17 00:00:00 2001 From: Chinmay Pai Date: Thu, 20 Sep 2018 15:40:04 +0530 Subject: [PATCH 08/12] file-api: fix email test and improve file saving logic Signed-off-by: Chinmay Pai --- frappe/core/doctype/communication/email.py | 2 +- frappe/core/doctype/file/file.py | 41 +++++++++++----------- frappe/core/doctype/file/test_file.py | 24 +++++++------ frappe/email/receive.py | 2 +- 4 files changed, 35 insertions(+), 34 deletions(-) diff --git a/frappe/core/doctype/communication/email.py b/frappe/core/doctype/communication/email.py index c306e0197e..9faf329281 100755 --- a/frappe/core/doctype/communication/email.py +++ b/frappe/core/doctype/communication/email.py @@ -285,7 +285,7 @@ def prepare_to_notify(doc, print_html=None, print_format=None, attachments=None) try: # keep this for error handling _file = frappe.get_doc("File", {"file_name": a}) - content = _file.get_content() + _file.get_content() # these attachments will be attached on-demand # and won't be stored in the message doc.attachments.append({"fid": a}) diff --git a/frappe/core/doctype/file/file.py b/frappe/core/doctype/file/file.py index acb89168a1..0f9254a9a2 100755 --- a/frappe/core/doctype/file/file.py +++ b/frappe/core/doctype/file/file.py @@ -24,7 +24,6 @@ import imghdr from frappe.utils import get_hook_method, get_files_path, random_string, encode, cstr, call_hook_method, cint from frappe import _ from frappe import conf -from copy import copy from frappe.utils.nestedset import NestedSet from frappe.utils import strip from PIL import Image, ImageOps @@ -163,12 +162,10 @@ class File(NestedSet): """Validates existence of public file TODO: validate for private file """ - if (self.file_url or "").startswith("/files/"): - if not self.file_name: - self.file_name = self.file_url.split("/files/")[-1] + full_path = self.get_full_path() - if not os.path.exists(get_files_path(frappe.as_unicode(self.file_name.lstrip("/")))): - frappe.throw(_("File {0} does not exist").format(self.file_url), IOError) + if not os.path.exists(full_path): + frappe.throw(_("File {0} does not exist").format(self.file_url), IOError) def validate_duplicate_entry(self): if not self.flags.ignore_duplicate_entry_error and not self.is_folder: @@ -444,8 +441,6 @@ class File(NestedSet): self.file_url = unquote(self.file_url) self.file_size = frappe.form_dict.file_size - if not self.file_url and self.get('content', None): - self.file_url def get_uploaded_content(self): # should not be unicode when reading a file, hence using frappe.form @@ -461,6 +456,7 @@ class File(NestedSet): def save_file(self, content=None, decode=False): + file_exists = False self.content = content if decode: if isinstance(content, text_type): @@ -475,11 +471,17 @@ class File(NestedSet): self.file_size = self.check_max_file_size() self.content_hash = get_content_hash(self.content) self.content_type = mimetypes.guess_type(self.file_name)[0] - self.file_name = get_file_name(self.file_name, self.content_hash[-6:]) - file_data = self.get_file_data_from_hash() - if not file_data: - call_hook_method("before_write_file", file_size=self.file_size) + _file = frappe.get_value("File", {"content_hash": self.content_hash}, ["file_url"]) + if _file: + self.file_url = _file + file_exists = True + + if not file_exists: + if os.path.exists(encode(get_files_path(self.file_name))): + self.file_name = get_file_name(self.file_name, self.content_hash[-6:]) + + call_hook_method("before_write_file", file_size=self.file_size) write_file_method = get_hook_method('write_file') if write_file_method: return write_file_method(self) @@ -752,15 +754,12 @@ def get_file_name(fname, optional_suffix): # convert to unicode fname = cstr(fname) - n_records = frappe.db.sql("select name from `tabFile` where file_name=%s", fname) - if len(n_records) > 0 or os.path.exists(encode(get_files_path(fname))): - f = fname.rsplit('.', 1) - if len(f) == 1: - partial, extn = f[0], "" - else: - partial, extn = f[0], "." + f[1] - return '{partial}{suffix}{extn}'.format(partial=partial, extn=extn, suffix=optional_suffix) - return fname + f = fname.rsplit('.', 1) + if len(f) == 1: + partial, extn = f[0], "" + else: + partial, extn = f[0], "." + f[1] + return '{partial}{suffix}{extn}'.format(partial=partial, extn=extn, suffix=optional_suffix) @frappe.whitelist() diff --git a/frappe/core/doctype/file/test_file.py b/frappe/core/doctype/file/test_file.py index 2ca4bb6ea0..e2fd89d943 100644 --- a/frappe/core/doctype/file/test_file.py +++ b/frappe/core/doctype/file/test_file.py @@ -4,7 +4,6 @@ from __future__ import unicode_literals import frappe -import os import unittest from frappe import _ from frappe.core.doctype.file.file import move_file, get_files_path @@ -26,7 +25,7 @@ class TestSimpleFile(unittest.TestCase): self.attached_to_doctype, self.attached_to_docname = make_test_doc() self.test_content = test_content1 _file = frappe.get_doc({"doctype": "File", - "file_name": "hello.txt", + "file_name": "test1.txt", "attached_to_doctype": self.attached_to_doctype, "attached_to_name": self.attached_to_docname, "content": self.test_content}) @@ -51,12 +50,12 @@ class TestSameFileName(unittest.TestCase): self.test_content1 = test_content1 self.test_content2 = test_content2 _file1 = frappe.get_doc({"doctype": "File", - "file_name": "hello.txt", + "file_name": "testing.txt", "attached_to_doctype": self.attached_to_doctype, "attached_to_name": self.attached_to_docname, "content": self.test_content1}) _file2 = frappe.get_doc({"doctype": "File", - "file_name": "hello.txt", + "file_name": "testing.txt", "attached_to_doctype": self.attached_to_doctype, "attached_to_name": self.attached_to_docname, "content": self.test_content2}) @@ -92,28 +91,31 @@ class TestSameContent(unittest.TestCase): "attached_to_doctype": self.attached_to_doctype1, "attached_to_name": self.attached_to_docname1, "content": self.test_content1}) + _file1.save() + _file2 = frappe.get_doc({"doctype": "File", "file_name": self.dup_filename, "attached_to_doctype": self.attached_to_doctype2, "attached_to_name": self.attached_to_docname2, "content": self.test_content2}) - _file1.save() + _file2.save() self.saved_filename1 = _file1.file_name self.saved_filename2 = _file2.file_name def test_saved_content(self): - _file1 = frappe.get_doc("File", {"file_name": self.saved_filename1}) - filename1 = _file1.file_name - _file2 = frappe.get_doc("File", {"file_name": self.saved_filename2}) - filename2 = _file2.file_name - # self.assertEqual(filename1, filename2) - self.assertFalse(os.path.exists(get_files_path(self.dup_filename))) + pass + # _file1 = frappe.get_doc("File", {"file_name": self.saved_filename1}) + # filename1 = _file1.file_name + # _file2 = frappe.get_doc("File", {"file_name": self.saved_filename2}) + # filename2 = _file2.file_name + # self.assertFalse(os.path.exists(get_files_path(self.dup_filename))) def tearDown(self): # File gets deleted on rollback, so blank pass + class TestFile(unittest.TestCase): def setUp(self): self.delete_test_data() diff --git a/frappe/email/receive.py b/frappe/email/receive.py index 7274528443..2b977cdfa8 100644 --- a/frappe/email/receive.py +++ b/frappe/email/receive.py @@ -532,7 +532,7 @@ class Email: saved_attachments.append(_file) if attachment['fname'] in self.cid_map: - self.cid_map[file_data.name] = self.cid_map[attachment['fname']] + self.cid_map[_file.name] = self.cid_map[attachment['fname']] except MaxFileSizeReachedError: # WARNING: bypass max file size exception From 75c79925575e418e82083d6b1f22279c71d71a8f Mon Sep 17 00:00:00 2001 From: Chinmay Pai Date: Fri, 21 Sep 2018 13:40:25 +0530 Subject: [PATCH 09/12] file-api: add new test, fix minor bugs and code indentation Signed-off-by: Chinmay Pai --- frappe/client.py | 12 +++- frappe/core/doctype/communication/email.py | 7 ++- frappe/core/doctype/data_import/importer.py | 19 +++++-- frappe/core/doctype/file/file.py | 37 +++++++----- frappe/core/doctype/file/test_file.py | 56 ++++++++++--------- .../prepared_report/prepared_report.py | 10 +++- frappe/desk/page/setup_wizard/setup_wizard.py | 10 +++- frappe/email/receive.py | 6 +- frappe/handler.py | 4 +- .../gsuite_templates/gsuite_templates.py | 9 ++- frappe/model/document.py | 10 +++- frappe/twofactor.py | 8 ++- frappe/website/doctype/web_form/web_form.py | 10 +++- 13 files changed, 127 insertions(+), 71 deletions(-) diff --git a/frappe/client.py b/frappe/client.py index 9cb7735a95..d863419081 100644 --- a/frappe/client.py +++ b/frappe/client.py @@ -350,9 +350,15 @@ def attach_file(filename=None, filedata=None, doctype=None, docname=None, folder if not doc.has_permission(): frappe.throw(_("Not permitted"), frappe.PermissionError) - _file = frappe.get_doc({"doctype": "File", "file_name": filename, "attached_to_doctype": doctype, - "attached_to_name": docname, "attached_to_field": docfield, - "folder": folder, "is_private": is_private, "content": filedata, + _file = frappe.get_doc({ + "doctype": "File", + "file_name": filename, + "attached_to_doctype": doctype, + "attached_to_name": docname, + "attached_to_field": docfield, + "folder": folder, + "is_private": is_private, + "content": filedata, "decode": decode_base64}) _file.save() diff --git a/frappe/core/doctype/communication/email.py b/frappe/core/doctype/communication/email.py index 9faf329281..26a1030d00 100755 --- a/frappe/core/doctype/communication/email.py +++ b/frappe/core/doctype/communication/email.py @@ -404,8 +404,11 @@ def add_attachments(name, attachments): ["file_name", "file_url", "is_private"], as_dict=1) # save attachments to new doc - _file = frappe.get_doc("File", {"file_url": attach.file_url, - "attached_to_doctype": "Communication", "attached_to_name": name, + _file = frappe.get_doc({ + "doctype": "File", + "file_url": attach.file_url, + "attached_to_doctype": "Communication", + "attached_to_name": name, "folder": "Home/Attachments"}) _file.save() diff --git a/frappe/core/doctype/data_import/importer.py b/frappe/core/doctype/data_import/importer.py index 43d4ada865..c65e2fd921 100644 --- a/frappe/core/doctype/data_import/importer.py +++ b/frappe/core/doctype/data_import/importer.py @@ -264,8 +264,13 @@ def upload(rows = None, submit_after_import=None, ignore_encoding_errors=False, # file is already attached return - _file = frappe.get_doc("File", {"file_url": file_url, "attached_to_name": docname, - "attached_to_doctype": doctype, "attached_to_field": 0, "folder": "Home/Attachments"}) + _file = frappe.get_doc({ + "doctype": "File", + "file_url": file_url, + "attached_to_name": docname, + "attached_to_doctype": doctype, + "attached_to_field": 0, + "folder": "Home/Attachments"}) _file.save() @@ -465,9 +470,13 @@ def upload(rows = None, submit_after_import=None, ignore_encoding_errors=False, else: from frappe.utils.csvutils import to_csv file_data = to_csv(data_rows_with_error) - _file = frappe.get_doc({"doctype": "File", "file_name": file_name, - "attached_to_doctype": "Data Import", "attached_to_name": data_import_doc.name, - "folder": "Home/Attachments", "content": file_data}) + _file = frappe.get_doc({ + "doctype": "File", + "file_name": file_name, + "attached_to_doctype": "Data Import", + "attached_to_name": data_import_doc.name, + "folder": "Home/Attachments", + "content": file_data}) _file.save() data_import_doc.error_file = _file.file_url diff --git a/frappe/core/doctype/file/file.py b/frappe/core/doctype/file/file.py index 0f9254a9a2..7a45aed5d2 100755 --- a/frappe/core/doctype/file/file.py +++ b/frappe/core/doctype/file/file.py @@ -85,7 +85,8 @@ class File(NestedSet): self.validate_folder() if not self.flags.ignore_file_validate: - self.validate_file() + if not self.is_folder: + self.validate_file() self.generate_content_hash() self.set_folder_size() @@ -121,7 +122,8 @@ class File(NestedSet): break self.attached_to_field = field_name if self.attached_to_field: - frappe.db.set_value(self.attached_to_doctype, self.attached_to_name, self.attached_to_field, self.file_url) + frappe.db.set_value(self.attached_to_doctype, self.attached_to_name, + self.attached_to_field, self.file_url) def set_folder_size(self): @@ -180,7 +182,8 @@ class File(NestedSet): self.attached_to_name)) if len(n_records) > 0: self.duplicate_entry = n_records[0][0] - frappe.throw(frappe._("Same file has already been attached to the record"), frappe.DuplicateEntryError) + frappe.throw(frappe._("Same file has already been attached to the record"), + frappe.DuplicateEntryError) def generate_content_hash(self): if self.content_hash or not self.file_url: @@ -261,7 +264,9 @@ class File(NestedSet): if not self.flags.ignore_permissions and \ not frappe.has_permission(self.attached_to_doctype, "write", self.attached_to_name): - frappe.throw(frappe._("Cannot delete file as it belongs to {0} {1} for which you do not have permissions").format(self.attached_to_doctype, self.attached_to_name), + frappe.throw(frappe._( + "Cannot delete file as it belongs to {0} {1} for which you do not have permissions").format( + self.attached_to_doctype, self.attached_to_name), frappe.PermissionError) except frappe.DoesNotExistError: pass @@ -551,6 +556,7 @@ class File(NestedSet): def on_doctype_update(): frappe.db.add_index("File", ["attached_to_doctype", "attached_to_name"]) + frappe.db.add_index("File", ["lft", "rgt"]) def make_home_folder(): home = frappe.get_doc({ @@ -735,8 +741,10 @@ def remove_all(dt, dn, from_delete=False): def remove_file_by_url(file_url, doctype=None, name=None): if doctype and name: - fid = frappe.db.get_value("File", {"file_url": file_url, - "attached_to_doctype": doctype, "attached_to_name": name}) + fid = frappe.db.get_value("File", { + "file_url": file_url, + "attached_to_doctype": doctype, + "attached_to_name": name}) else: fid = frappe.db.get_value("File", {"file_url": file_url}) @@ -810,10 +818,15 @@ def extract_images_from_html(doc, content): doctype = doc.parenttype if doc.parent else doc.doctype name = doc.parent or doc.name - # TODO fix this - _file = frappe.get_doc({"doctype": "File", "file_name": filename, - "attached_to_doctype": doctype, "attached_to_name": name}) - file_url = _file.save_file(content=content, decode=True).file_url + _file = frappe.get_doc({ + "doctype": "File", + "file_name": filename, + "attached_to_doctype": doctype, + "attached_to_name": name, + "content": content, + "decode": True}) + _file.save() + file_url = _file.file_url if not frappe.flags.has_dataurl: frappe.flags.has_dataurl = True @@ -871,7 +884,3 @@ def validate_filename(filename): timestamp = now_datetime().strftime(" %Y-%m-%d %H:%M:%S") fname = get_file_name(filename, timestamp) return fname - - -def on_doctype_update(): - frappe.db.add_index("File", ["lft", "rgt"]) diff --git a/frappe/core/doctype/file/test_file.py b/frappe/core/doctype/file/test_file.py index e2fd89d943..b082bf250b 100644 --- a/frappe/core/doctype/file/test_file.py +++ b/frappe/core/doctype/file/test_file.py @@ -4,9 +4,11 @@ from __future__ import unicode_literals import frappe +import os import unittest from frappe import _ -from frappe.core.doctype.file.file import move_file, get_files_path +from frappe.core.doctype.file.file import move_file +from frappe.utils import get_files_path # test_records = frappe.get_test_records('File') test_content1 = 'Hello' @@ -24,17 +26,18 @@ class TestSimpleFile(unittest.TestCase): def setUp(self): self.attached_to_doctype, self.attached_to_docname = make_test_doc() self.test_content = test_content1 - _file = frappe.get_doc({"doctype": "File", + _file = frappe.get_doc({ + "doctype": "File", "file_name": "test1.txt", "attached_to_doctype": self.attached_to_doctype, "attached_to_name": self.attached_to_docname, "content": self.test_content}) _file.save() - self.saved_file_name = _file.file_name + self.saved_file_url = _file.file_url def test_save(self): - _file = frappe.get_doc("File", {"file_name": self.saved_file_name}) + _file = frappe.get_doc("File", {"file_url": self.saved_file_url}) content = _file.get_content() self.assertEqual(content, self.test_content) @@ -49,26 +52,28 @@ class TestSameFileName(unittest.TestCase): self.attached_to_doctype, self.attached_to_docname = make_test_doc() self.test_content1 = test_content1 self.test_content2 = test_content2 - _file1 = frappe.get_doc({"doctype": "File", + _file1 = frappe.get_doc({ + "doctype": "File", "file_name": "testing.txt", "attached_to_doctype": self.attached_to_doctype, "attached_to_name": self.attached_to_docname, "content": self.test_content1}) - _file2 = frappe.get_doc({"doctype": "File", + _file1.save() + _file2 = frappe.get_doc({ + "doctype": "File", "file_name": "testing.txt", "attached_to_doctype": self.attached_to_doctype, "attached_to_name": self.attached_to_docname, "content": self.test_content2}) - _file1.save() _file2.save() - self.saved_filename1 = _file1.file_name - self.saved_filename2 = _file2.file_name + self.saved_file_url1 = _file1.file_url + self.saved_file_url2 = _file2.file_url def test_saved_content(self): - _file = frappe.get_doc("File", {"file_name": self.saved_filename1}) + _file = frappe.get_doc("File", {"file_url": self.saved_file_url1}) content1 = _file.get_content() self.assertEqual(content1, self.test_content1) - _file = frappe.get_doc("File", {"file_name": self.saved_filename2}) + _file = frappe.get_doc("File", {"file_url": self.saved_file_url2}) content2 = _file.get_content() self.assertEqual(content2, self.test_content2) @@ -86,30 +91,25 @@ class TestSameContent(unittest.TestCase): self.test_content2 = test_content1 self.orig_filename = 'hello.txt' self.dup_filename = 'hello2.txt' - _file1 = frappe.get_doc({"doctype": "File", + _file1 = frappe.get_doc({ + "doctype": "File", "file_name": self.orig_filename, "attached_to_doctype": self.attached_to_doctype1, "attached_to_name": self.attached_to_docname1, "content": self.test_content1}) _file1.save() - _file2 = frappe.get_doc({"doctype": "File", + _file2 = frappe.get_doc({ + "doctype": "File", "file_name": self.dup_filename, "attached_to_doctype": self.attached_to_doctype2, "attached_to_name": self.attached_to_docname2, "content": self.test_content2}) - + _file2.save() - self.saved_filename1 = _file1.file_name - self.saved_filename2 = _file2.file_name def test_saved_content(self): - pass - # _file1 = frappe.get_doc("File", {"file_name": self.saved_filename1}) - # filename1 = _file1.file_name - # _file2 = frappe.get_doc("File", {"file_name": self.saved_filename2}) - # filename2 = _file2.file_name - # self.assertFalse(os.path.exists(get_files_path(self.dup_filename))) + self.assertFalse(os.path.exists(get_files_path(self.dup_filename))) def tearDown(self): # File gets deleted on rollback, so blank @@ -133,7 +133,8 @@ class TestFile(unittest.TestCase): frappe.delete_doc("File", f[0]) def upload_file(self): - _file = frappe.get_doc({"doctype": "File", + _file = frappe.get_doc({ + "doctype": "File", "file_name": "file_copy.txt", "attached_to_name": "", "attached_to_doctype": "", @@ -174,7 +175,8 @@ class TestFile(unittest.TestCase): def test_folder_copy(self): folder = self.get_folder("Test Folder 2", "Home") folder = self.get_folder("Test Folder 3", "Home/Test Folder 2") - _file = frappe.get_doc({"doctype": "File", + _file = frappe.get_doc({ + "doctype": "File", "file_name": "folder_copy.txt", "attached_to_name": "", "attached_to_doctype": "", @@ -210,7 +212,8 @@ class TestFile(unittest.TestCase): self.assertEqual(frappe.db.get_value("File", _("Home/Test Folder 1"), "file_size"), 0) folder = self.get_folder("Test Folder 3", "Home/Test Folder 1") - _file = frappe.get_doc({"doctype": "File", + _file = frappe.get_doc({ + "doctype": "File", "file_name": "folder_copy.txt", "attached_to_name": "", "attached_to_doctype": "", @@ -239,7 +242,8 @@ class TestFile(unittest.TestCase): # Rebuild the frappe.local.conf to take up the changes from site_config frappe.local.conf = _dict(frappe.get_site_config()) - _file = frappe.get_doc({"doctype": "File", + _file = frappe.get_doc({ + "doctype": "File", "file_name": "_test_max_space.txt", "attached_to_name": "", "attached_to_doctype": "", diff --git a/frappe/core/doctype/prepared_report/prepared_report.py b/frappe/core/doctype/prepared_report/prepared_report.py index ec4b883f78..78afb3c183 100644 --- a/frappe/core/doctype/prepared_report/prepared_report.py +++ b/frappe/core/doctype/prepared_report/prepared_report.py @@ -61,9 +61,13 @@ def create_csv_file(columns, data, dt, dn): csv_filename = '{0}.csv'.format(frappe.utils.data.format_datetime(frappe.utils.now(), "Y-m-d-H:M")) rows = [tuple(columns)] + data encoded = base64.b64encode(frappe.safe_encode(to_csv(rows))) - # Call save_file function to upload and attach the file - _file = frappe.get_doc({"doctype": "File", "file_name": csv_filename, - "attached_to_doctype": dt, "attached_to_name": dn, "content": encoded, + # Call save() file function to upload and attach the file + _file = frappe.get_doc({ + "doctype": "File", + "file_name": csv_filename, + "attached_to_doctype": dt, + "attached_to_name": dn, + "content": encoded, "decode": True}) _file.save() diff --git a/frappe/desk/page/setup_wizard/setup_wizard.py b/frappe/desk/page/setup_wizard/setup_wizard.py index 1bf04893f5..82f44ee249 100755 --- a/frappe/desk/page/setup_wizard/setup_wizard.py +++ b/frappe/desk/page/setup_wizard/setup_wizard.py @@ -186,9 +186,13 @@ def update_user_name(args): attach_user = args.get("attach_user").split(",") if len(attach_user)==3: filename, filetype, content = attach_user - _file = frappe.get_doc({"doctype": "File", "file_name": filename, - "attached_to_doctype": "User", "attached_to_docname": args.get("name"), - "content": content, "decode": True}) + _file = frappe.get_doc({ + "doctype": "File", + "file_name": filename, + "attached_to_doctype": "User", + "attached_to_name": args.get("name"), + "content": content, + "decode": True}) _file.save() fileurl = _file.file_url frappe.db.set_value("User", args.get("name"), "user_image", fileurl) diff --git a/frappe/email/receive.py b/frappe/email/receive.py index 2b977cdfa8..ffee8dc17a 100644 --- a/frappe/email/receive.py +++ b/frappe/email/receive.py @@ -523,10 +523,12 @@ class Email: for attachment in self.attachments: try: - _file = frappe.get_doc({"doctype": "File", + _file = frappe.get_doc({ + "doctype": "File", "file_name": attachment['fname'], "attached_to_doctype": doc.doctype, - "attached_to_name": doc.name, "is_private": 1, + "attached_to_name": doc.name, + "is_private": 1, "content": attachment['fcontent']}) _file.save() saved_attachments.append(_file) diff --git a/frappe/handler.py b/frappe/handler.py index 642966b1b2..7a040871ad 100755 --- a/frappe/handler.py +++ b/frappe/handler.py @@ -121,9 +121,7 @@ def uploadfile(): "content": frappe.form_dict.filedata, "decode": True }) - ret = ret.save() - # ret = frappe.get_doc({"doctype": "File"}) - # ret = ret.upload() + ret.save() except frappe.DuplicateEntryError: # ignore pass ret = None diff --git a/frappe/integrations/doctype/gsuite_templates/gsuite_templates.py b/frappe/integrations/doctype/gsuite_templates/gsuite_templates.py index bd298004a7..83f379ee29 100644 --- a/frappe/integrations/doctype/gsuite_templates/gsuite_templates.py +++ b/frappe/integrations/doctype/gsuite_templates/gsuite_templates.py @@ -24,8 +24,13 @@ def create_gsuite_doc(doctype, docname, gs_template=None): r = run_gsuite_script('new', filename, templ.template_id, templ.destination_id, json_data) - _file = frappe.get_doc("File", {"file_url": r['url'], "file_name": filename, - "attached_to_doctype": doctype, "attached_to_name": docname, "attached_to_field": True, + _file = frappe.get_doc({ + "doctype": "File", + "file_url": r['url'], + "file_name": filename, + "attached_to_doctype": doctype, + "attached_to_name": docname, + "attached_to_field": True, "folder": "Home/Attachments"}) _file.save() diff --git a/frappe/model/document.py b/frappe/model/document.py index c35b1a91de..08054a0c33 100644 --- a/frappe/model/document.py +++ b/frappe/model/document.py @@ -319,9 +319,13 @@ class Document(BaseDocument): for attach_item in get_attachments(self.doctype, self.amended_from): #save attachments to new doc - _file = frappe.get_doc("File", {"file_url": attach_item.file_url, - "file_name": attach_item.file_name, "attached_to_name": self.name, - "attached_to_doctype": self.doctype, "folder": "Home/Attachments"}) + _file = frappe.get_doc({ + "doctype": "File", + "file_url": attach_item.file_url, + "file_name": attach_item.file_name, + "attached_to_name": self.name, + "attached_to_doctype": self.doctype, + "folder": "Home/Attachments"}) _file.save() diff --git a/frappe/twofactor.py b/frappe/twofactor.py index dbc138e2c2..4ff721067e 100644 --- a/frappe/twofactor.py +++ b/frappe/twofactor.py @@ -335,8 +335,12 @@ def qrcode_as_png(user, totp_uri): '''Save temporary Qrcode to server.''' folder = create_barcode_folder() png_file_name = '{}.png'.format(frappe.generate_hash(length=20)) - _file = frappe.get_doc({"doctype": "File", "file_name": png_file_name, - "attached_to_doctype": 'User', "attached_to_name": user, "folder": folder, + _file = frappe.get_doc({ + "doctype": "File", + "file_name": png_file_name, + "attached_to_doctype": 'User', + "attached_to_name": user, + "folder": folder, "content": png_file_name}) _file.save() frappe.db.commit() diff --git a/frappe/website/doctype/web_form/web_form.py b/frappe/website/doctype/web_form/web_form.py index 77f20b8b77..c2629f3a92 100644 --- a/frappe/website/doctype/web_form/web_form.py +++ b/frappe/website/doctype/web_form/web_form.py @@ -420,9 +420,13 @@ def accept(web_form, data, for_payment=False): # save new file filename, dataurl = filedata.split(',', 1) - _file = frappe.get_doc({"doctype": "File", "file_name": filename, - "attached_to_doctype": doc.doctype, "attached_to_name": doc.name, - "content": dataurl, "decode": True}) + _file = frappe.get_doc({ + "doctype": "File", + "file_name": filename, + "attached_to_doctype": doc.doctype, + "attached_to_name": doc.name, + "content": dataurl, + "decode": True}) _file.save() # update values From 8849036e843057ef6c7c8bd4f9ef90e84e0df22a Mon Sep 17 00:00:00 2001 From: Chinmay Pai Date: Mon, 24 Sep 2018 15:26:22 +0530 Subject: [PATCH 10/12] file-api: data_import: fix instance where file_name was used instead of file_url Signed-off-by: Chinmay Pai --- frappe/core/doctype/data_import/importer.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/core/doctype/data_import/importer.py b/frappe/core/doctype/data_import/importer.py index c65e2fd921..037724fae1 100644 --- a/frappe/core/doctype/data_import/importer.py +++ b/frappe/core/doctype/data_import/importer.py @@ -277,7 +277,7 @@ def upload(rows = None, submit_after_import=None, ignore_encoding_errors=False, # header filename, file_extension = ['',''] if not rows: - _file = frappe.get_doc("File", {"file_name": data_import_doc.import_file}) + _file = frappe.get_doc("File", {"file_url": data_import_doc.import_file}) fcontent = _file.get_content() filename, file_extension = os.path.splitext(_file.file_name) From 6f7faad0ecd5efaf3431ae0058fc4b1204c909d1 Mon Sep 17 00:00:00 2001 From: Chinmay Pai Date: Mon, 24 Sep 2018 17:46:37 +0530 Subject: [PATCH 11/12] file-api: add save test for base64 encoded file Signed-off-by: Chinmay Pai --- frappe/core/doctype/file/test_file.py | 49 +++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/frappe/core/doctype/file/test_file.py b/frappe/core/doctype/file/test_file.py index b082bf250b..dd1a7e7718 100644 --- a/frappe/core/doctype/file/test_file.py +++ b/frappe/core/doctype/file/test_file.py @@ -3,6 +3,7 @@ # See license.txt from __future__ import unicode_literals +import base64 import frappe import os import unittest @@ -14,6 +15,7 @@ from frappe.utils import get_files_path test_content1 = 'Hello' test_content2 = 'Hello World' + def make_test_doc(): d = frappe.new_doc('ToDo') d.description = 'Test' @@ -23,6 +25,7 @@ def make_test_doc(): class TestSimpleFile(unittest.TestCase): + def setUp(self): self.attached_to_doctype, self.attached_to_docname = make_test_doc() self.test_content = test_content1 @@ -41,6 +44,35 @@ class TestSimpleFile(unittest.TestCase): content = _file.get_content() self.assertEqual(content, self.test_content) + + def tearDown(self): + # File gets deleted on rollback, so blank + pass + + +class TestBase64File(unittest.TestCase): + + + def setUp(self): + self.attached_to_doctype, self.attached_to_docname = make_test_doc() + self.test_content = base64.b64encode(test_content1.encode('utf-8')) + _file = frappe.get_doc({ + "doctype": "File", + "file_name": "test_base64.txt", + "attached_to_doctype": self.attached_to_doctype, + "attached_to_docname": self.attached_to_docname, + "content": self.test_content, + "decode": True}) + _file.save() + self.saved_file_url = _file.file_url + + + def test_saved_content(self): + _file = frappe.get_doc("File", {"file_url": self.saved_file_url}) + content = _file.get_content() + self.assertEqual(content, test_content1) + + def tearDown(self): # File gets deleted on rollback, so blank pass @@ -48,6 +80,7 @@ class TestSimpleFile(unittest.TestCase): class TestSameFileName(unittest.TestCase): + def setUp(self): self.attached_to_doctype, self.attached_to_docname = make_test_doc() self.test_content1 = test_content1 @@ -69,6 +102,7 @@ class TestSameFileName(unittest.TestCase): self.saved_file_url1 = _file1.file_url self.saved_file_url2 = _file2.file_url + def test_saved_content(self): _file = frappe.get_doc("File", {"file_url": self.saved_file_url1}) content1 = _file.get_content() @@ -77,6 +111,7 @@ class TestSameFileName(unittest.TestCase): content2 = _file.get_content() self.assertEqual(content2, self.test_content2) + def tearDown(self): # File gets deleted on rollback, so blank pass @@ -84,6 +119,7 @@ class TestSameFileName(unittest.TestCase): class TestSameContent(unittest.TestCase): + def setUp(self): self.attached_to_doctype1, self.attached_to_docname1 = make_test_doc() self.attached_to_doctype2, self.attached_to_docname2 = make_test_doc() @@ -108,30 +144,37 @@ class TestSameContent(unittest.TestCase): _file2.save() + def test_saved_content(self): self.assertFalse(os.path.exists(get_files_path(self.dup_filename))) + def tearDown(self): # File gets deleted on rollback, so blank pass class TestFile(unittest.TestCase): + + def setUp(self): self.delete_test_data() self.upload_file() + def tearDown(self): try: frappe.get_doc("File", {"file_name": "file_copy.txt"}).delete() except frappe.DoesNotExistError: pass + def delete_test_data(self): for f in frappe.db.sql('''select name, file_name from tabFile where is_home_folder = 0 and is_attachments_folder = 0 order by rgt-lft asc'''): frappe.delete_doc("File", f[0]) + def upload_file(self): _file = frappe.get_doc({ "doctype": "File", @@ -145,6 +188,7 @@ class TestFile(unittest.TestCase): self.saved_name = _file.name self.saved_filename = get_files_path(_file.file_name) + def get_folder(self, folder_name, parent_folder="Home"): return frappe.get_doc({ "doctype": "File", @@ -153,6 +197,7 @@ class TestFile(unittest.TestCase): "folder": _(parent_folder) }).insert() + def tests_after_upload(self): self.assertEqual(self.saved_folder, _("Home/Test Folder 1")) @@ -161,6 +206,7 @@ class TestFile(unittest.TestCase): self.assertEqual(folder_size, saved_file_size) + def test_file_copy(self): folder = self.get_folder("Test Folder 2", "Home") @@ -172,6 +218,7 @@ class TestFile(unittest.TestCase): self.assertEqual(frappe.db.get_value("File", _("Home/Test Folder 2"), "file_size"), file.file_size) self.assertEqual(frappe.db.get_value("File", _("Home/Test Folder 1"), "file_size"), 0) + def test_folder_copy(self): folder = self.get_folder("Test Folder 2", "Home") folder = self.get_folder("Test Folder 3", "Home/Test Folder 2") @@ -195,6 +242,7 @@ class TestFile(unittest.TestCase): self.assertEqual(frappe.db.get_value("File", _("Home/Test Folder 1"), "file_size"), file.file_size) self.assertEqual(frappe.db.get_value("File", _("Home/Test Folder 2"), "file_size"), 0) + def test_default_folder(self): d = frappe.get_doc({ "doctype": "File", @@ -224,6 +272,7 @@ class TestFile(unittest.TestCase): folder = frappe.get_doc("File", "Home/Test Folder 1/Test Folder 3") self.assertRaises(frappe.ValidationError, folder.delete) + def test_file_upload_limit(self): from frappe.core.doctype.file.file import MaxFileSizeReachedError from frappe.limits import update_limits, clear_limit From 37290376125a0b5b67779200469c765ae68bb7d4 Mon Sep 17 00:00:00 2001 From: Chinmay Pai Date: Tue, 25 Sep 2018 14:54:22 +0530 Subject: [PATCH 12/12] file-api: minor fixes and improvements Signed-off-by: Chinmay Pai --- frappe/core/doctype/data_import/importer.py | 2 +- frappe/core/doctype/file/file.py | 7 ++++++- frappe/desk/form/utils.py | 5 +++-- frappe/email/doctype/email_account/test_email_account.py | 2 -- 4 files changed, 10 insertions(+), 6 deletions(-) diff --git a/frappe/core/doctype/data_import/importer.py b/frappe/core/doctype/data_import/importer.py index 037724fae1..4a3eb498e0 100644 --- a/frappe/core/doctype/data_import/importer.py +++ b/frappe/core/doctype/data_import/importer.py @@ -279,7 +279,7 @@ def upload(rows = None, submit_after_import=None, ignore_encoding_errors=False, if not rows: _file = frappe.get_doc("File", {"file_url": data_import_doc.import_file}) fcontent = _file.get_content() - filename, file_extension = os.path.splitext(_file.file_name) + filename, file_extension = _file.get_extension() if file_extension == '.xlsx' and from_data_import == 'Yes': from frappe.utils.xlsxutils import read_xlsx_file_from_attached_file diff --git a/frappe/core/doctype/file/file.py b/frappe/core/doctype/file/file.py index 7a45aed5d2..bc6767771a 100755 --- a/frappe/core/doctype/file/file.py +++ b/frappe/core/doctype/file/file.py @@ -554,6 +554,11 @@ class File(NestedSet): raise frappe.PermissionError + def get_extension(self): + '''returns split filename and extension''' + return os.path.splitext(self.file_name) + + def on_doctype_update(): frappe.db.add_index("File", ["attached_to_doctype", "attached_to_name"]) frappe.db.add_index("File", ["lft", "rgt"]) @@ -755,7 +760,7 @@ def remove_file_by_url(file_url, doctype=None, name=None): def get_content_hash(content): if isinstance(content, text_type): content = content.encode() - return hashlib.md5(content).hexdigest() + return hashlib.md5(content).hexdigest() #nosec def get_file_name(fname, optional_suffix): diff --git a/frappe/desk/form/utils.py b/frappe/desk/form/utils.py index 9f321cca0e..5a403599c1 100644 --- a/frappe/desk/form/utils.py +++ b/frappe/desk/form/utils.py @@ -6,7 +6,6 @@ import frappe, json import frappe.desk.form.meta import frappe.desk.form.load from frappe.utils.html_utils import clean_email_html -from frappe.core.doctype.file.file import remove_file from frappe import _ from six import string_types @@ -15,7 +14,9 @@ from six import string_types def remove_attach(): """remove attachment""" fid = frappe.form_dict.get('fid') - return remove_file(fid=fid) + file_name = frappe.form_dict.get('file_name') + frappe.delete_doc('File', fid) + return doc.add_comment("Attachment Removed", _("Removed {0}").format(file_name)) @frappe.whitelist() diff --git a/frappe/email/doctype/email_account/test_email_account.py b/frappe/email/doctype/email_account/test_email_account.py index 7cc1345a42..849cb6b42c 100644 --- a/frappe/email/doctype/email_account/test_email_account.py +++ b/frappe/email/doctype/email_account/test_email_account.py @@ -54,7 +54,6 @@ class TestEmailAccount(unittest.TestCase): frappe.db.sql("DELETE FROM `tabCommunication` WHERE sender='test_sender@example.com'") existing_file = frappe.get_doc({'doctype': 'File', 'file_name': 'erpnext-conf-14.png'}) frappe.delete_doc("File", existing_file.name) - existing_file.delete_file_from_filesystem() with open(os.path.join(os.path.dirname(__file__), "test_mails", "incoming-2.raw"), "r") as testfile: test_mails = [testfile.read()] @@ -72,7 +71,6 @@ class TestEmailAccount(unittest.TestCase): # cleanup existing_file = frappe.get_doc({'doctype': 'File', 'file_name': 'erpnext-conf-14.png'}) frappe.delete_doc("File", existing_file.name) - existing_file.delete_file_from_filesystem() def test_incoming_attached_email_from_outlook_plain_text_only(self):