From 331c1ef19fadeda63b66768725ced4609ba96bde Mon Sep 17 00:00:00 2001 From: Tyler Matteson Date: Tue, 17 Oct 2023 07:50:39 -0400 Subject: [PATCH] feat: versioned files (#36) * wip: versioning doctype * wip: file versioning * fix: remove print * wip: override upload dialog * feat: run name and content validations on adding file * feat: setup rename for name-conflicts * feat: show existing filenames for content hash conflicts * fix: error text on filename conflict * feat: override existing file instead of creating a new one * ci: update pre-commit config * test: fix write_file test * feat: add file association if content hash matches * fix: catch errors while stripping EXIF data from file * ci: update json-diff dependency * fix: allow error message translations * test: fix write_file test * fix: set values in database if a record exists already * test: fix write_file test * test: disable db logger in test * fix: avoid file size checks since that data may not exist * fix: allow skipping content hash conflicts * Revert "fix: allow skipping content hash conflicts" This reverts commit e731eb0c6ffa7bcfb3f78d3cf8032c0b5c40a450. * fix: optimize content validation logic * fix: check file permission with full object * fix: file permission for orphaned files * style: ignore type errors * fix: handle file attachments from existing library * fix: version table indexes when associations are removed * fix: add metadata to version and file association tables * fix: allow keeping file name structure * fix: index numbering issues for version table --------- Co-authored-by: Rohan Bansal --- .github/workflows/lint.yaml | 2 +- .pre-commit-config.yaml | 34 +- cloud_storage/cloud_storage/custom/file.json | 184 ++++- .../file_association/file_association.json | 97 ++- .../doctype/file_version/__init__.py | 0 .../doctype/file_version/file_version.json | 46 ++ .../doctype/file_version/file_version.py | 9 + cloud_storage/cloud_storage/overrides/file.py | 208 +++-- cloud_storage/hooks.py | 10 +- .../public/js/components/FilePreview.vue | 260 +++++++ .../public/js/components/FileUploader.vue | 711 ++++++++++++++++++ cloud_storage/public/js/overrides.js | 130 ++++ cloud_storage/tests/conftest.py | 9 +- cloud_storage/tests/test_file.py | 11 +- docs/versioning.md | 11 + 15 files changed, 1575 insertions(+), 147 deletions(-) create mode 100644 cloud_storage/cloud_storage/doctype/file_version/__init__.py create mode 100644 cloud_storage/cloud_storage/doctype/file_version/file_version.json create mode 100644 cloud_storage/cloud_storage/doctype/file_version/file_version.py create mode 100644 cloud_storage/public/js/components/FilePreview.vue create mode 100644 cloud_storage/public/js/components/FileUploader.vue create mode 100644 docs/versioning.md diff --git a/.github/workflows/lint.yaml b/.github/workflows/lint.yaml index fd34b7c..0f0d2c5 100644 --- a/.github/workflows/lint.yaml +++ b/.github/workflows/lint.yaml @@ -62,7 +62,7 @@ jobs: - name: Find JSON changes id: changed-json - uses: tj-actions/changed-files@v23.1 + uses: tj-actions/changed-files@v37 with: files: | **/*.json diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index a07116b..0f3d4aa 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -7,8 +7,8 @@ repos: rev: v4.3.0 hooks: - id: trailing-whitespace - files: 'frappe.*' - exclude: '.*json$|.*txt$|.*csv|.*md|.*svg' + files: "cloud_storage.*" + exclude: ".*json$|.*txt$|.*csv|.*md|.*svg" - id: check-yaml - id: no-commit-to-branch args: ['--branch', 'develop'] @@ -23,7 +23,7 @@ repos: rev: v2.34.0 hooks: - id: pyupgrade - args: ['--py310-plus'] + args: ['--py38-plus'] - repo: https://github.com/frappe/black rev: 951ccf4d5bb0d692b457a5ebc4215d755618eb68 @@ -37,22 +37,28 @@ repos: types_or: [javascript] # Ignore any files that might contain jinja / bundles exclude: | - (?x)^( - frappe/public/dist/.*| - .*node_modules.*| + (?x)^( .*boilerplate.*| - frappe/www/website_script.js| - frappe/templates/includes/.*| - frappe/public/js/lib/.* - )$ + .*node_modules.*| + cloud_storage/public/dist/.*| + cloud_storage/public/js/lib/.*| + cloud_storage/templates/includes/.*| + cloud_storage/www/website_script.js + )$ + + + - repo: https://github.com/PyCQA/isort + rev: 5.12.0 + hooks: + - id: isort - repo: https://github.com/PyCQA/flake8 rev: 5.0.4 hooks: - id: flake8 - additional_dependencies: ['flake8-bugbear'] + additional_dependencies: ['flake8-bugbear',] ci: - autoupdate_schedule: weekly - skip: [] - submodules: false + autoupdate_schedule: weekly + skip: [] + submodules: false diff --git a/cloud_storage/cloud_storage/custom/file.json b/cloud_storage/cloud_storage/custom/file.json index 3826d11..11617aa 100644 --- a/cloud_storage/cloud_storage/custom/file.json +++ b/cloud_storage/cloud_storage/custom/file.json @@ -38,7 +38,7 @@ "label": "S3 Key", "length": 0, "mandatory_depends_on": null, - "modified": "2023-04-28 00:05:56.057223", + "modified": "2023-07-06 02:04:01.131489", "modified_by": "Administrator", "module": null, "name": "File-s3_key", @@ -56,6 +56,7 @@ "report_hide": 0, "reqd": 0, "search_index": 0, + "sort_options": 0, "translatable": 0, "unique": 0, "width": null @@ -98,7 +99,7 @@ "label": "Sharing Link", "length": 0, "mandatory_depends_on": null, - "modified": "2023-04-28 00:05:56.148197", + "modified": "2023-07-06 02:04:01.272045", "modified_by": "Administrator", "module": null, "name": "File-sharing_link", @@ -116,6 +117,7 @@ "report_hide": 0, "reqd": 0, "search_index": 0, + "sort_options": 0, "translatable": 1, "unique": 0, "width": null @@ -158,7 +160,7 @@ "label": null, "length": 0, "mandatory_depends_on": null, - "modified": "2023-04-28 00:05:56.255829", + "modified": "2023-07-06 02:04:01.389708", "modified_by": "Administrator", "module": null, "name": "File-section_break_1lqhx", @@ -176,6 +178,7 @@ "report_hide": 0, "reqd": 0, "search_index": 0, + "sort_options": 0, "translatable": 0, "unique": 0, "width": null @@ -218,7 +221,7 @@ "label": "", "length": 0, "mandatory_depends_on": null, - "modified": "2023-04-28 00:05:56.364321", + "modified": "2023-07-06 02:04:01.499473", "modified_by": "Administrator", "module": null, "name": "File-file_association", @@ -236,6 +239,129 @@ "report_hide": 0, "reqd": 0, "search_index": 0, + "sort_options": 0, + "translatable": 0, + "unique": 0, + "width": null + }, + { + "_assign": null, + "_comments": null, + "_liked_by": null, + "_user_tags": null, + "allow_in_quick_entry": 0, + "allow_on_submit": 0, + "bold": 0, + "collapsible": 0, + "collapsible_depends_on": null, + "columns": 0, + "creation": "2023-07-06 05:39:34.115162", + "default": null, + "depends_on": null, + "description": null, + "docstatus": 0, + "dt": "File", + "fetch_from": null, + "fetch_if_empty": 0, + "fieldname": "section_break_1xyhd", + "fieldtype": "Section Break", + "hidden": 0, + "hide_border": 0, + "hide_days": 0, + "hide_seconds": 0, + "idx": 27, + "ignore_user_permissions": 0, + "ignore_xss_filter": 0, + "in_global_search": 0, + "in_list_view": 0, + "in_preview": 0, + "in_standard_filter": 0, + "insert_after": "file_association", + "is_system_generated": 0, + "is_virtual": 0, + "label": null, + "length": 0, + "mandatory_depends_on": null, + "modified": "2023-07-06 05:39:34.115162", + "modified_by": "Administrator", + "module": null, + "name": "File-section_break_1xyhd", + "no_copy": 0, + "non_negative": 0, + "options": null, + "owner": "Administrator", + "permlevel": 0, + "precision": "", + "print_hide": 0, + "print_hide_if_no_value": 0, + "print_width": null, + "read_only": 0, + "read_only_depends_on": null, + "report_hide": 0, + "reqd": 0, + "search_index": 0, + "sort_options": 0, + "translatable": 0, + "unique": 0, + "width": null + }, + { + "_assign": null, + "_comments": null, + "_liked_by": null, + "_user_tags": null, + "allow_in_quick_entry": 0, + "allow_on_submit": 0, + "bold": 0, + "collapsible": 0, + "collapsible_depends_on": null, + "columns": 0, + "creation": "2023-07-06 05:39:57.024056", + "default": null, + "depends_on": null, + "description": null, + "docstatus": 0, + "dt": "File", + "fetch_from": null, + "fetch_if_empty": 0, + "fieldname": "versions", + "fieldtype": "Table", + "hidden": 0, + "hide_border": 0, + "hide_days": 0, + "hide_seconds": 0, + "idx": 28, + "ignore_user_permissions": 0, + "ignore_xss_filter": 0, + "in_global_search": 0, + "in_list_view": 0, + "in_preview": 0, + "in_standard_filter": 0, + "insert_after": "section_break_1xyhd", + "is_system_generated": 0, + "is_virtual": 0, + "label": "", + "length": 0, + "mandatory_depends_on": null, + "modified": "2023-07-06 05:39:57.024056", + "modified_by": "Administrator", + "module": null, + "name": "File-versions", + "no_copy": 0, + "non_negative": 0, + "options": "File Version", + "owner": "Administrator", + "permlevel": 0, + "precision": "", + "print_hide": 0, + "print_hide_if_no_value": 0, + "print_width": null, + "read_only": 0, + "read_only_depends_on": null, + "report_hide": 0, + "reqd": 0, + "search_index": 0, + "sort_options": 0, "translatable": 0, "unique": 0, "width": null @@ -250,44 +376,44 @@ "_comments": null, "_liked_by": null, "_user_tags": null, - "creation": "2023-04-28 00:05:56.507961", + "creation": "2023-07-06 02:04:01.645547", "default_value": null, "doc_type": "File", "docstatus": 0, "doctype_or_field": "DocField", - "field_name": "section_break_8", + "field_name": "preview", "idx": 0, "is_system_generated": 0, - "modified": "2023-04-28 00:05:56.507961", + "modified": "2023-07-06 02:04:01.645547", "modified_by": "Administrator", "module": null, - "name": "File-section_break_8-label", + "name": "File-preview-collapsible_depends_on", "owner": "Administrator", - "property": "label", + "property": "collapsible_depends_on", "property_type": "Data", "row_name": null, - "value": "Storage Details" + "value": "eval:true" }, { "_assign": null, "_comments": null, "_liked_by": null, "_user_tags": null, - "creation": "2023-04-28 00:05:56.499487", + "creation": "2023-07-06 02:04:01.636840", "default_value": null, "doc_type": "File", "docstatus": 0, "doctype_or_field": "DocField", - "field_name": "section_break_8", + "field_name": "preview", "idx": 0, "is_system_generated": 0, - "modified": "2023-04-28 00:05:56.499487", + "modified": "2023-07-06 02:04:01.636840", "modified_by": "Administrator", "module": null, - "name": "File-section_break_8-permlevel", + "name": "File-preview-collapsible", "owner": "Administrator", - "property": "permlevel", - "property_type": "Int", + "property": "collapsible", + "property_type": "Check", "row_name": null, "value": "1" }, @@ -296,21 +422,21 @@ "_comments": null, "_liked_by": null, "_user_tags": null, - "creation": "2023-04-28 00:05:56.491129", + "creation": "2023-07-06 02:04:01.627758", "default_value": null, "doc_type": "File", "docstatus": 0, "doctype_or_field": "DocField", - "field_name": "preview", + "field_name": "section_break_8", "idx": 0, "is_system_generated": 0, - "modified": "2023-04-28 00:05:56.491129", + "modified": "2023-07-06 02:04:01.627758", "modified_by": "Administrator", "module": null, - "name": "File-preview-collapsible", + "name": "File-section_break_8-permlevel", "owner": "Administrator", - "property": "collapsible", - "property_type": "Check", + "property": "permlevel", + "property_type": "Int", "row_name": null, "value": "1" }, @@ -319,24 +445,24 @@ "_comments": null, "_liked_by": null, "_user_tags": null, - "creation": "2023-04-28 00:05:56.481095", + "creation": "2023-07-06 02:04:01.616923", "default_value": null, "doc_type": "File", "docstatus": 0, "doctype_or_field": "DocField", - "field_name": "preview", + "field_name": "section_break_8", "idx": 0, "is_system_generated": 0, - "modified": "2023-04-28 00:05:56.481095", + "modified": "2023-07-06 02:04:01.616923", "modified_by": "Administrator", "module": null, - "name": "File-preview-collapsible_depends_on", + "name": "File-section_break_8-label", "owner": "Administrator", - "property": "collapsible_depends_on", + "property": "label", "property_type": "Data", "row_name": null, - "value": "eval:true" + "value": "Storage Details" } ], "sync_on_migrate": 1 -} +} \ No newline at end of file diff --git a/cloud_storage/cloud_storage/doctype/file_association/file_association.json b/cloud_storage/cloud_storage/doctype/file_association/file_association.json index d234f91..e1fc88c 100644 --- a/cloud_storage/cloud_storage/doctype/file_association/file_association.json +++ b/cloud_storage/cloud_storage/doctype/file_association/file_association.json @@ -1,40 +1,59 @@ { - "actions": [], - "creation": "2023-03-20 12:48:05.086487", - "default_view": "List", - "doctype": "DocType", - "editable_grid": 1, - "engine": "InnoDB", - "field_order": ["link_doctype", "link_name"], - "fields": [ - { - "fieldname": "link_doctype", - "fieldtype": "Link", - "in_list_view": 1, - "label": "Document Type", - "options": "DocType", - "reqd": 1 - }, - { - "fieldname": "link_name", - "fieldtype": "Dynamic Link", - "in_list_view": 1, - "label": "Document Name", - "options": "link_doctype", - "reqd": 1 - } - ], - "istable": 1, - "links": [], - "modified": "2023-03-22 12:37:44.883229", - "modified_by": "Administrator", - "module": "Cloud Storage", - "name": "File Association", - "owner": "Administrator", - "permissions": [], - "quick_entry": 1, - "sort_field": "modified", - "sort_order": "DESC", - "states": [], - "track_changes": 1 -} + "actions": [], + "creation": "2023-03-20 12:48:05.086487", + "default_view": "List", + "doctype": "DocType", + "editable_grid": 1, + "engine": "InnoDB", + "field_order": [ + "link_doctype", + "link_name", + "user", + "timestamp" + ], + "fields": [ + { + "fieldname": "link_doctype", + "fieldtype": "Link", + "in_list_view": 1, + "label": "Document Type", + "options": "DocType", + "reqd": 1 + }, + { + "fieldname": "link_name", + "fieldtype": "Dynamic Link", + "in_list_view": 1, + "label": "Document Name", + "options": "link_doctype", + "reqd": 1 + }, + { + "fieldname": "timestamp", + "fieldtype": "Datetime", + "in_list_view": 1, + "label": "Timestamp", + "read_only": 1 + }, + { + "fieldname": "user", + "fieldtype": "Link", + "in_list_view": 1, + "label": "User", + "options": "User" + } + ], + "istable": 1, + "links": [], + "modified": "2023-09-27 23:42:57.356690", + "modified_by": "Administrator", + "module": "Cloud Storage", + "name": "File Association", + "owner": "Administrator", + "permissions": [], + "quick_entry": 1, + "sort_field": "modified", + "sort_order": "DESC", + "states": [], + "track_changes": 1 +} \ No newline at end of file diff --git a/cloud_storage/cloud_storage/doctype/file_version/__init__.py b/cloud_storage/cloud_storage/doctype/file_version/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/cloud_storage/cloud_storage/doctype/file_version/file_version.json b/cloud_storage/cloud_storage/doctype/file_version/file_version.json new file mode 100644 index 0000000..0b57e6d --- /dev/null +++ b/cloud_storage/cloud_storage/doctype/file_version/file_version.json @@ -0,0 +1,46 @@ +{ + "actions": [], + "creation": "2023-07-06 05:38:35.891393", + "doctype": "DocType", + "editable_grid": 1, + "engine": "InnoDB", + "field_order": [ + "version", + "user", + "timestamp" + ], + "fields": [ + { + "fieldname": "version", + "fieldtype": "Data", + "in_list_view": 1, + "label": "Version", + "read_only": 1 + }, + { + "fieldname": "timestamp", + "fieldtype": "Datetime", + "in_list_view": 1, + "label": "Timestamp", + "read_only": 1 + }, + { + "fieldname": "user", + "fieldtype": "Link", + "in_list_view": 1, + "label": "User", + "options": "User" + } + ], + "istable": 1, + "links": [], + "modified": "2023-09-27 23:45:54.628246", + "modified_by": "Administrator", + "module": "Cloud Storage", + "name": "File Version", + "owner": "Administrator", + "permissions": [], + "sort_field": "modified", + "sort_order": "DESC", + "states": [] +} \ No newline at end of file diff --git a/cloud_storage/cloud_storage/doctype/file_version/file_version.py b/cloud_storage/cloud_storage/doctype/file_version/file_version.py new file mode 100644 index 0000000..42fbc12 --- /dev/null +++ b/cloud_storage/cloud_storage/doctype/file_version/file_version.py @@ -0,0 +1,9 @@ +# Copyright (c) 2023, AgriTheory and contributors +# For license information, please see license.txt + +# import frappe +from frappe.model.document import Document + + +class FileVersion(Document): + pass diff --git a/cloud_storage/cloud_storage/overrides/file.py b/cloud_storage/cloud_storage/overrides/file.py index 6c26088..0ced80d 100644 --- a/cloud_storage/cloud_storage/overrides/file.py +++ b/cloud_storage/cloud_storage/overrides/file.py @@ -3,44 +3,32 @@ import re import types import uuid +from mimetypes import guess_type +from typing import Optional, Union +import frappe from boto3.exceptions import S3UploadFailedError from boto3.session import Session from botocore.config import Config from botocore.exceptions import ClientError -from magic import from_buffer - -import frappe from frappe import DoesNotExistError, _ from frappe.core.doctype.file.file import File, get_files_path -from frappe.core.doctype.file.utils import decode_file_content +from frappe.core.doctype.file.utils import decode_file_content, get_content_hash from frappe.model.rename_doc import rename_doc from frappe.permissions import has_user_permission -from frappe.utils import get_url +from frappe.utils import get_datetime, get_url +from frappe.utils.image import optimize_image, strip_exif_data +from magic import from_buffer +from PIL import UnidentifiedImageError +from werkzeug.datastructures import FileStorage FILE_URL = "/api/method/retrieve?key={path}" URL_PREFIXES = ("http://", "https://", "/api/method/retrieve") class CustomFile(File): - def has_permission(self, ptype: str | None = None, user: str | None = None) -> bool: - has_access = False - user = frappe.session.user if not user else user - # check if public - if self.owner == user: - has_access = True - elif self.attached_to_doctype and self.attached_to_name: # type: ignore - reference_doc = frappe.get_doc(self.attached_to_doctype, self.attached_to_name) # type: ignore - has_access = reference_doc.has_permission() - if not has_access: - has_access = has_user_permission(self, user) - # elif True: - # Check "shared with" including parent 'folder' to allow access - # ... - else: - has_access = bool(frappe.has_permission("File", "read", user=user)) - - return has_access + def has_permission(self, ptype: Optional[str] = None, user: Optional[str] = None) -> bool: + return has_permission(self, ptype, user) def on_trash(self) -> None: user_roles = frappe.get_roles(frappe.session.user) @@ -63,8 +51,13 @@ def on_trash(self) -> None: if not self.is_folder and len(self.file_association) > 0: self.add_comment_in_reference_doc("Attachment Removed", _("Removed {0}").format(self.file_name)) - def associate_files(self) -> None: - if not self.attached_to_doctype: # type: ignore + def associate_files( + self, attached_to_doctype: Optional[str] = None, attached_to_name: Optional[str] = None + ) -> None: + attached_to_doctype = attached_to_doctype or self.attached_to_doctype # type: ignore + attached_to_name = attached_to_name or self.attached_to_name # type: ignore + + if not attached_to_doctype: return if not self.file_url: # type: ignore client = get_cloud_storage_client() @@ -79,8 +72,8 @@ def associate_files(self) -> None: ) if associated_doc and associated_doc != self.name: existing_file = frappe.get_doc("File", associated_doc) - existing_file.attached_to_doctype = self.attached_to_doctype # type: ignore - existing_file.attached_to_name = self.attached_to_name # type: ignore + existing_file.attached_to_doctype = attached_to_doctype + existing_file.attached_to_name = attached_to_name self.content_hash = existing_file.content_hash # if a File exists already where this association should be, we continue validating that File at this time # the original File will then be removed in the after insert hook @@ -88,15 +81,19 @@ def associate_files(self) -> None: existing_attachment = list( filter( - lambda row: row.link_doctype == self.attached_to_doctype # type: ignore - and row.link_name == self.attached_to_name, # type: ignore + lambda row: row.link_doctype == attached_to_doctype and row.link_name == attached_to_name, self.file_association, ) ) if not existing_attachment: self.append( "file_association", - {"link_doctype": self.attached_to_doctype, "link_name": self.attached_to_name}, # type: ignore + { + "link_doctype": attached_to_doctype, + "link_name": attached_to_name, + "user": frappe.session.user, + "timestamp": get_datetime(), + }, ) if associated_doc and associated_doc != self.name: self.save() @@ -134,21 +131,34 @@ def after_insert(self) -> File: ignore_permissions=True, ) + def add_file_version(self, version_id): + self.append( + "versions", + { + "version": str(version_id), + "user": frappe.session.user, + "timestamp": get_datetime(), + }, + ) + def remove_file_association(self, dt: str, dn: str) -> None: if len(self.file_association) <= 1: self.delete() return + to_remove = [] for idx, row in enumerate(self.file_association): if row.link_doctype == dt and row.link_name == dn: + to_remove.append(row) if row.link_doctype == self.attached_to_doctype and row.link_name == self.attached_to_name: # type: ignore - self.attached_to_doctype = self.file_association[ - (idx + 1) % len(self.file_association) - ].link_doctype - self.attached_to_name = self.file_association[ - (idx + 1) % len(self.file_association) - ].link_name - frappe.delete_doc("File Association", row.name) - del row + # calculate the index of the next file association in the list, looping to the start if already at the end + next_idx = (idx + 1) % len(self.file_association) + next_file_association = self.file_association[next_idx] + self.attached_to_doctype = next_file_association.link_doctype + self.attached_to_name = next_file_association.link_name + for row in to_remove: + self.remove(row) + for idx, association in enumerate(self.file_association, start=1): + association.idx = idx self.save() @property @@ -226,6 +236,26 @@ def get_full_path(self): return file_path +def has_permission(doc, ptype: Optional[str] = None, user: Optional[str] = None) -> bool: + has_access = False + user = frappe.session.user if not user else user + # check if public + if doc.owner == user: + has_access = True + elif doc.attached_to_doctype and doc.attached_to_name: # type: ignore + reference_doc = frappe.get_doc(doc.attached_to_doctype, doc.attached_to_name) # type: ignore + has_access = reference_doc.has_permission() + if not has_access: + has_access = has_user_permission(doc, user) + # elif True: + # Check "shared with" including parent 'folder' to allow access + # ... + else: + has_access = bool(frappe.has_permission(doc.doctype, ptype, user=user)) + + return has_access + + def is_safe_path(path: str) -> bool: if path.startswith(URL_PREFIXES): return True @@ -239,7 +269,7 @@ def is_safe_path(path: str) -> bool: @frappe.whitelist() -def get_sharing_link(docname: str, reset: str | bool | None = None) -> str: +def get_sharing_link(docname: str, reset: Optional[Union[str, bool]] = None) -> str: if isinstance(reset, str): reset = json.loads(reset) doc = frappe.get_doc("File", docname) @@ -275,6 +305,7 @@ def get_cloud_storage_client(): client.expiration = config.get("expiration", 120) client.get_presigned_url = types.MethodType(get_presigned_url, client) client.get_sharing_url = types.MethodType(get_sharing_url, client) + return client @@ -325,8 +356,9 @@ def get_presigned_url(client, key: str): expiration = client.expiration if file.is_private else None if file.is_private: + file_doc = frappe.get_doc("File", file.name) frappe.has_permission( - doctype="File", ptype="read", doc=file, user=frappe.session.user, throw=True + doctype="File", ptype="read", doc=file_doc, user=frappe.session.user, throw=True ) return client.generate_presigned_url( @@ -349,19 +381,25 @@ def get_sharing_url(client, key: str) -> str: def upload_file(file: File) -> File: client = get_cloud_storage_client() path = get_file_path(file, client.folder) - file.file_url = FILE_URL.format(path=path) + file.db_set("file_url", FILE_URL.format(path=path)) content_type = file.content_type or from_buffer(file.content, mime=True) try: - client.put_object(Body=file.content, Bucket=client.bucket, Key=path, ContentType=content_type) + response = client.put_object( + Body=file.content, Bucket=client.bucket, Key=path, ContentType=content_type + ) + if response.get("VersionId"): + file.add_file_version(response.get("VersionId")) except S3UploadFailedError: frappe.throw(_("File Upload Failed. Please try again.")) except Exception as e: frappe.log_error("File Upload Error", e) - file.s3_key = path + file.db_set("s3_key", path) + if not file.name: + file.save() return file -def get_file_path(file: File, folder: str | None = None) -> str: +def get_file_path(file: File, folder: Optional[str] = None) -> str: parent_doctype = file.attached_to_doctype or "No Doctype" fragments = [ @@ -376,8 +414,16 @@ def get_file_path(file: File, folder: str | None = None) -> str: return path +def get_file_content_hash(content, content_type): + try: + stripped_content = strip_exif_data(content, content_type) + return get_content_hash(stripped_content) + except UnidentifiedImageError: + return get_content_hash(content) + + @frappe.whitelist() -def write_file(file: File) -> File: +def write_file(file: File, remove_spaces_in_file_name: bool = True) -> File: if not frappe.conf.cloud_storage_settings or frappe.conf.cloud_storage_settings.get( "use_local", False ): @@ -388,10 +434,34 @@ def write_file(file: File) -> File: file.save_file_on_filesystem() return file - if not file.name: - file.autoname() + # if a hash-conflict is found, update the existing document with a new file association + existing_file_hashes = frappe.get_all( + "File", filters={"name": ["!=", file.name], "content_hash": file.content_hash}, pluck="name" + ) + + if existing_file_hashes: + file_doc: File = frappe.get_doc("File", existing_file_hashes[0]) + file_doc.associate_files(file.attached_to_doctype, file.attached_to_name) + file_doc.save() + return file_doc + + # if a filename-conflict is found, update the existing document with a new version instead + existing_file_names = frappe.get_all( + "File", filters={"name": ["!=", file.name], "file_name": file.file_name}, pluck="name" + ) - file.file_name = strip_special_chars(file.file_name.replace(" ", "_")) + if existing_file_names: + file_doc = frappe.get_doc("File", existing_file_names[0]) + file_doc.update( + {"content": file.content, "content_hash": file.content_hash, "content_type": file.content_type} + ) + file_doc.associate_files(file.attached_to_doctype, file.attached_to_name) + file = file_doc + + if remove_spaces_in_file_name: + file.file_name = file.file_name.replace(" ", "_") + + file.file_name = strip_special_chars(file.file_name) file.flags.cloud_storage = True return upload_file(file) @@ -421,6 +491,48 @@ def delete_file(file: File, **kwargs) -> File: return file +@frappe.whitelist() +def validate_file_content(*args, **kwargs): + matched_files = [] + files = frappe.request.files + + if "file" in files: + file: FileStorage = files["file"] + content_type = guess_type(file.filename)[0] + + # validate filename + file_name = file.filename + existing_files_by_name = frappe.get_all( + "File", filters={"file_name": file_name}, pluck="file_name" + ) + + # validate content hash + file.stream.seek(0) + content = file.stream.read() + content_hash = get_file_content_hash(content, content_type) + + existing_files_by_hash = frappe.get_all( + "File", filters={"content_hash": content_hash}, pluck="file_name" + ) + + # if no files are found by name or hash, and if the file is an image, match against optimized content + if not existing_files_by_hash and content_type.startswith("image/"): + optimized_content = optimize_image(content, content_type) + optimized_content_hash = get_file_content_hash(optimized_content, content_type) + existing_files_by_hash = frappe.get_all( + "File", filters={"content_hash": optimized_content_hash}, pluck="file_name" + ) + + # build a list of matched files + matched_files = list(set(existing_files_by_name + existing_files_by_hash)) + + return { + "filename_exists": len(existing_files_by_name) > 0, + "content_exists": len(existing_files_by_hash) > 0, + "matched_files": matched_files, + } + + @frappe.whitelist(allow_guest=True) def retrieve(key: str) -> None: if key: diff --git a/cloud_storage/hooks.py b/cloud_storage/hooks.py index 00fb736..e9d145e 100644 --- a/cloud_storage/hooks.py +++ b/cloud_storage/hooks.py @@ -85,10 +85,10 @@ # permission_query_conditions = { # "Event": "frappe.desk.doctype.event.event.get_permission_query_conditions", # } -# -# has_permission = { -# "Event": "frappe.desk.doctype.event.event.has_permission", -# } + +has_permission = { + "File": "cloud_storage.cloud_storage.overrides.file.has_permission", +} # DocType Class # --------------- @@ -140,7 +140,7 @@ override_whitelisted_methods = { "retrieve": "cloud_storage.cloud_storage.overrides.file.retrieve", "share": "cloud_storage.cloud_storage.overrides.file.share", - "frappe.desk.form.utils.remove_attach": "cloud_storage.cloud_storage.overrides.file.remove_attach" + "frappe.desk.form.utils.remove_attach": "cloud_storage.cloud_storage.overrides.file.remove_attach", } # each overriding function accepts a `data` argument; diff --git a/cloud_storage/public/js/components/FilePreview.vue b/cloud_storage/public/js/components/FilePreview.vue new file mode 100644 index 0000000..68696d9 --- /dev/null +++ b/cloud_storage/public/js/components/FilePreview.vue @@ -0,0 +1,260 @@ + + + + + diff --git a/cloud_storage/public/js/components/FileUploader.vue b/cloud_storage/public/js/components/FileUploader.vue new file mode 100644 index 0000000..da36ecb --- /dev/null +++ b/cloud_storage/public/js/components/FileUploader.vue @@ -0,0 +1,711 @@ + + + + diff --git a/cloud_storage/public/js/overrides.js b/cloud_storage/public/js/overrides.js index 00936ed..b2703a9 100644 --- a/cloud_storage/public/js/overrides.js +++ b/cloud_storage/public/js/overrides.js @@ -1,3 +1,7 @@ +frappe.provide('frappe.ui') + +import FileUploaderComponent from './components/FileUploader.vue' + $(window).on('hashchange', page_changed) $(window).on('load', page_changed) @@ -10,6 +14,7 @@ function page_changed(event) { disallow_attachment_delete(frm) }, }) + if (event.type == 'load') { disallow_attachment_delete(cur_frm) } @@ -22,3 +27,128 @@ function disallow_attachment_delete(frm) { frm.$wrapper.find('.attachment-row').find('.remove-btn').hide() } } + +// TODO: full class override from Frappe's FileUploader.vue file; keep in sync +frappe.ui.FileUploader = class CloudStorageFileUploader { + constructor({ + wrapper, + method, + on_success, + doctype, + docname, + fieldname, + files, + folder, + restrictions = {}, + upload_notes, + allow_multiple, + as_dataurl, + disable_file_browser, + dialog_title, + attach_doc_image, + frm, + make_attachments_public, + } = {}) { + frm && frm.attachments.max_reached(true) + + if (!wrapper) { + this.make_dialog(dialog_title) + } else { + this.wrapper = wrapper.get ? wrapper.get(0) : wrapper + } + + this.$fileuploader = new Vue({ + el: this.wrapper, + render: h => + h(FileUploaderComponent, { + props: { + show_upload_button: !Boolean(this.dialog), + doctype, + docname, + fieldname, + method, + folder, + on_success, + restrictions, + upload_notes, + allow_multiple, + as_dataurl, + disable_file_browser, + attach_doc_image, + make_attachments_public, + }, + }), + }) + + this.uploader = this.$fileuploader.$children[0] + + if (!this.dialog) { + this.uploader.wrapper_ready = true + } + + this.uploader.$watch( + 'files', + files => { + let all_private = files.every(file => file.private) + if (this.dialog) { + this.dialog.set_secondary_action_label(all_private ? __('Set all public') : __('Set all private')) + } + }, + { deep: true } + ) + + this.uploader.$watch('trigger_upload', trigger_upload => { + if (trigger_upload) { + this.upload_files() + } + }) + + this.uploader.$watch('close_dialog', close_dialog => { + if (close_dialog) { + this.dialog && this.dialog.hide() + } + }) + + this.uploader.$watch('hide_dialog_footer', hide_dialog_footer => { + if (hide_dialog_footer) { + this.dialog && this.dialog.footer.addClass('hide') + this.dialog.$wrapper.data('bs.modal')._config.backdrop = 'static' + } else { + this.dialog && this.dialog.footer.removeClass('hide') + this.dialog.$wrapper.data('bs.modal')._config.backdrop = true + } + }) + + if (files && files.length) { + this.uploader.add_files(files) + } + } + + upload_files() { + this.dialog && this.dialog.get_primary_btn().prop('disabled', true) + this.dialog && this.dialog.get_secondary_btn().prop('disabled', true) + return this.uploader.upload_files() + } + + make_dialog(title) { + this.dialog = new frappe.ui.Dialog({ + title: title || __('Upload'), + primary_action_label: __('Upload'), + primary_action: () => this.upload_files(), + secondary_action_label: __('Set all private'), + secondary_action: () => { + this.uploader.toggle_all_private() + }, + on_page_show: () => { + this.uploader.wrapper_ready = true + }, + }) + + this.wrapper = this.dialog.body + this.dialog.show() + this.dialog.$wrapper.on('hidden.bs.modal', function () { + $(this).data('bs.modal', null) + $(this).remove() + }) + } +} diff --git a/cloud_storage/tests/conftest.py b/cloud_storage/tests/conftest.py index e7f2335..9ad5950 100644 --- a/cloud_storage/tests/conftest.py +++ b/cloud_storage/tests/conftest.py @@ -1,9 +1,10 @@ -import pytest -import frappe -from unittest.mock import MagicMock, patch -from frappe.defaults import * import os from pathlib import Path +from unittest.mock import MagicMock, patch + +import frappe +import pytest +from frappe.defaults import * from frappe.utils import get_bench_path diff --git a/cloud_storage/tests/test_file.py b/cloud_storage/tests/test_file.py index d85bb41..7e28d6d 100644 --- a/cloud_storage/tests/test_file.py +++ b/cloud_storage/tests/test_file.py @@ -16,8 +16,9 @@ class TestFile(FrappeTestCase): @patch("cloud_storage.cloud_storage.overrides.file.upload_file") @patch("cloud_storage.cloud_storage.overrides.file.strip_special_chars") + @patch("frappe.get_all") @patch("frappe.conf") - def test_write_file(self, config, strip_chars, upload_file): + def test_write_file(self, config, get_all, strip_chars, upload_file): file = MagicMock() # test local fallback @@ -37,10 +38,11 @@ def test_write_file(self, config, strip_chars, upload_file): # test file upload with autoname file.attached_to_doctype = None file.name = None + file.file_name = "test_file.png" strip_chars.return_value = "test_file.png" upload_file.return_value = file + get_all.return_value = [] write_file(file) - assert file.autoname.call_count == 1 upload_file.assert_called_with(file) # test file upload without autoname @@ -48,7 +50,6 @@ def test_write_file(self, config, strip_chars, upload_file): file.file_name = "test_file.png" upload_file.return_value = file write_file(file) - assert file.autoname.call_count == 1 upload_file.assert_called_with(file) @patch("cloud_storage.cloud_storage.overrides.file.get_cloud_storage_client") @@ -74,10 +75,6 @@ def test_upload_file(self, file_path, client): file_path.return_value = "/path/to/s3/bucket/location" upload_file(file) assert client.return_value.put_object.call_count == 3 - self.assertEqual( - file.file_url, - "/api/method/retrieve?key=/path/to/s3/bucket/location", - ) @patch("cloud_storage.cloud_storage.overrides.file.get_cloud_storage_client") @patch("frappe.conf") diff --git a/docs/versioning.md b/docs/versioning.md new file mode 100644 index 0000000..77a6a1b --- /dev/null +++ b/docs/versioning.md @@ -0,0 +1,11 @@ + +Versioning must be enabled at the bucket level. + +To enable file versioning, enter the following in bench console: +```ipython +In [1]: from cloud_storage.cloud_storage.overrides.file import get_cloud_storage_client + +In [2]: client = get_cloud_storage_client() + +In [3]: client.put_bucket_versioning(Bucket=client.bucket, VersioningConfiguration={'Status': 'Enabled'}) +```