Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

allow saving files with no content or local file set yet. autosave up on file upload #27

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion dfp_external_storage/__init__.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@

__version__ = '1.1.1'
__version__ = '1.1.1.1'

Original file line number Diff line number Diff line change
Expand Up @@ -330,6 +330,9 @@ def dfp_is_s3_remote_file(self):
if self.dfp_external_storage_s3_key and self.dfp_external_storage_doc:
return True

def will_be_remote(self):
return self.dfp_external_storage_doc != None

def dfp_is_cacheable(self):
return not self.is_private and self.dfp_external_storage_doc.setting_cache_files_smaller_than and self.dfp_file_size != 0 and self.dfp_file_size < self.dfp_external_storage_doc.setting_cache_files_smaller_than

Expand Down Expand Up @@ -377,13 +380,18 @@ def dfp_external_storage_upload_file(self, local_file=None):

original_file_url = self.file_url

if not self.name:
self.save()

# Define S3 key
# key = f"{frappe.local.site}/{self.file_name}" # << Before 2024.03.03
base, extension = os.path.splitext(self.file_name)
key = f"{frappe.local.site}/{base}-{self.name}{extension}"

is_public = "/public" if not self.is_private else ""
if not local_file:
if not self.file_url:
frappe.throw("dfp_external_storage_upload_file: cannot upload file, file_url is not set and local_file not given")
local_file = "./" + frappe.local.site + is_public + self.file_url

try:
Expand All @@ -402,6 +410,7 @@ def dfp_external_storage_upload_file(self, local_file=None):
self.dfp_external_storage_s3_key = key
self.dfp_external_storage = self.dfp_external_storage_doc.name
self.file_url = f"/{DFP_EXTERNAL_STORAGE_URL_SEGMENT_FOR_FILE_LOAD}/{self.name}/{self.file_name}"

os.remove(local_file)
except Exception as e:
error_msg = _("Error saving file in remote folder: {}").format(str(e))
Expand Down Expand Up @@ -521,7 +530,10 @@ def download_to_local_and_remove_remote(self):
frappe.throw(error_msg)

def validate_file_on_disk(self):
return True if self.dfp_is_s3_remote_file() else super(DFPExternalStorageFile, self).validate_file_on_disk()
if self.dfp_is_s3_remote_file() or self.will_be_remote():
return True

super(DFPExternalStorageFile, self).validate_file_on_disk()

def exists_on_disk(self):
return False if self.dfp_is_s3_remote_file() else super(DFPExternalStorageFile, self).exists_on_disk()
Expand Down Expand Up @@ -594,8 +606,12 @@ def hook_file_before_save(doc, method):
previous = doc.get_doc_before_save()

if not previous:
# NEW "File": Case 1: remote selected => upload to remote and continue "File" flow
doc.dfp_external_storage_upload_file()

# NEW "File": Case 1a: remote selected and have file => upload to remote and continue "File" flow
if doc.file_url:
doc.dfp_external_storage_upload_file()

# NEW "File": Case 1b: remote selected and no local file (file_url) to upload => do nothing for now
return

# MODIFY "File"
Expand Down Expand Up @@ -685,7 +701,10 @@ def render(self):


def file(name:str, file:str):
print(f"downloading name: {name}, file: {file}")

if not name or not file:
frappe.log_error("Page not found",f"name ({name}) or file ({file}) not given")
raise frappe.PageDoesNotExistError()

cache_key = f"{DFP_EXTERNAL_STORAGE_PUBLIC_CACHE_PREFIX}{name}"
Expand All @@ -698,6 +717,8 @@ def file(name:str, file:str):
raise Exception("File not available")
except Exception:
# If no document, no read permissions, etc. For security reasons do not give any information, so just raise a 404 error
frappe.logger().exception(f"Page not found, either no File found {doc}, could not be downloaded, or file_name {doc and doc.file_name} does equal given name {file}")
frappe.log_error("Page not found",f"either no File found {doc}, could not be downloaded, or file_name {doc and doc.file_name} does equal given name {file}")
raise frappe.PageDoesNotExistError()

response_values = {}
Expand All @@ -717,9 +738,11 @@ def file(name:str, file:str):
except frappe.Redirect:
raise
except:
frappe.logger().exception(f"Error obtaining remote file content: {name}/{file}")
frappe.log_error(f"Error obtaining remote file content: {name}/{file}")

if "response" not in response_values or not response_values["response"]:
frappe.log_error("Page not found",f"no reponse defined. response_values: {response_values}")
raise frappe.PageDoesNotExistError()

if doc.dfp_mime_type_guess_by_file_name:
Expand All @@ -734,4 +757,5 @@ def file(name:str, file:str):
if "status" in response_values and response_values["status"] == 200:
return Response(**response_values)

frappe.log_error("Page not found",f"unknown reason. response so far: {response_values}")
raise frappe.PageDoesNotExistError()