From 909768a40919ec846edadeb770fddad12b40db8a Mon Sep 17 00:00:00 2001 From: David Ormsbee Date: Mon, 30 Sep 2024 12:31:44 -0400 Subject: [PATCH] temp: rename file_path/storage_path to path/os_path for clarity --- .../apps/authoring/components/admin.py | 2 +- .../apps/authoring/contents/admin.py | 15 +++++---- .../apps/authoring/contents/models.py | 32 +++++++++++++------ .../authoring/contents/test_file_storage.py | 2 +- 4 files changed, 34 insertions(+), 17 deletions(-) diff --git a/openedx_learning/apps/authoring/components/admin.py b/openedx_learning/apps/authoring/components/admin.py index 2dc6f441..edb83921 100644 --- a/openedx_learning/apps/authoring/components/admin.py +++ b/openedx_learning/apps/authoring/components/admin.py @@ -152,7 +152,7 @@ def content_preview(cvc_obj: ComponentVersionContent) -> SafeText: '
{}
', content_obj.mime_type, base64.encodebytes(data).decode('utf8'), - content_obj.storage_path(), + content_obj.os_path(), ) return format_text_for_admin_display( diff --git a/openedx_learning/apps/authoring/contents/admin.py b/openedx_learning/apps/authoring/contents/admin.py index d0ce7133..029ebfd8 100644 --- a/openedx_learning/apps/authoring/contents/admin.py +++ b/openedx_learning/apps/authoring/contents/admin.py @@ -31,21 +31,24 @@ class ContentAdmin(ReadOnlyModelAdmin): "size", "created", "has_file", - "file_path", - "storage_path", + "path", + "os_path", "text_preview", "image_preview", ] list_filter = ("media_type", "learning_package") search_fields = ("hash_digest",) - def storage_path(self, content: Content): - return content.storage_path() if content.has_file else "" + @admin.display(description="OS Path") + def os_path(self, content: Content): + return content.os_path() or "" - def file_path(self, content: Content): - return content.file_path() if content.has_file else "" + def path(self, content: Content): + return content.path if content.has_file else "" def text_preview(self, content: Content): + if not content.text: + return "" return format_html( '
\n{}\n
', content.text, diff --git a/openedx_learning/apps/authoring/contents/models.py b/openedx_learning/apps/authoring/contents/models.py index f2887c4e..f903861e 100644 --- a/openedx_learning/apps/authoring/contents/models.py +++ b/openedx_learning/apps/authoring/contents/models.py @@ -296,28 +296,43 @@ def mime_type(self) -> str: """ return str(self.media_type) - def file_path(self): + @cached_property + def path(self): """ Logical path at which this content is stored (or would be stored). This path is relative to OPENEDX_LEARNING['MEDIA'] configured storage - root. + root. This file may not exist because has_file=False, or because we + haven't written the file yet (this is the method we call when trying to + figure out where the file *should* go). """ return f"content/{self.learning_package.uuid}/{self.hash_digest}" - def storage_path(self): + def os_path(self): """ The full OS path for the underlying file for this Content. This will not be supported by all Storage class types. + + This will return ``None`` if there is no backing file (has_file=False). """ - return get_storage().path(self.file_path()) + if self.has_file: + return get_storage().path(self.path) + return None def read_file(self) -> File: """ Get a File object that has been open for reading. + + We intentionally don't expose an `open()` call where callers can open + this file in write mode. Writing a Content file should happen at most + once, and the logic is not obvious (see ``write_file``). + + At the end of the day, the caller can close the returned File and reopen + it in whatever mode they want, but we're trying to gently discourage + that kind of usage. """ - return get_storage().open(self.file_path(), 'rb') + return get_storage().open(self.path, 'rb') def write_file(self, file: File) -> None: """ @@ -328,7 +343,6 @@ def write_file(self, file: File) -> None: for a given Content row. """ storage = get_storage() - file_path = self.file_path() # There are two reasons why a file might already exist even if the the # Content row is new: @@ -345,15 +359,15 @@ def write_file(self, file: File) -> None: # 3. Similar to (2), but only part of the file was written before an # error occurred. This seems unlikely, but possible if the underlying # storage engine writes in chunks. - if storage.exists(file_path) and storage.size(file_path) == file.size: + if storage.exists(self.path) and storage.size(self.path) == file.size: return - storage.save(file_path, file) + storage.save(self.path, file) def file_url(self) -> str: """ This will sometimes be a time-limited signed URL. """ - return content_file_url(self.file_path()) + return content_file_url(self.path()) def clean(self): """ diff --git a/tests/openedx_learning/apps/authoring/contents/test_file_storage.py b/tests/openedx_learning/apps/authoring/contents/test_file_storage.py index b7eceb53..3a27bc03 100644 --- a/tests/openedx_learning/apps/authoring/contents/test_file_storage.py +++ b/tests/openedx_learning/apps/authoring/contents/test_file_storage.py @@ -54,7 +54,7 @@ def test_file_path(self): assert content.file_path() == f"content/{content.learning_package.uuid}/{content.hash_digest}" storage_root = settings.OPENEDX_LEARNING['MEDIA']['OPTIONS']['location'] - assert content.storage_path() == f"{storage_root}/{content.file_path()}" + assert content.os_path() == f"{storage_root}/{content.file_path()}" def test_read(self): """Make sure we can read the file data back."""