From 4dbef314bbc79b1581239f92fe7e41b20cb98a37 Mon Sep 17 00:00:00 2001 From: Danyal-Faheem Date: Fri, 7 Mar 2025 19:34:01 +0500 Subject: [PATCH 1/2] docs: remove redundant docstring --- openedxscorm/scormxblock.py | 37 ------------------------------------- 1 file changed, 37 deletions(-) diff --git a/openedxscorm/scormxblock.py b/openedxscorm/scormxblock.py index adb0a07..1202e8d 100644 --- a/openedxscorm/scormxblock.py +++ b/openedxscorm/scormxblock.py @@ -55,43 +55,6 @@ def _(text): @XBlock.wants("settings") @XBlock.wants("user") class ScormXBlock(XBlock, CompletableXBlockMixin): - """ - When a user uploads a Scorm package, the zip file is stored in: - - media/{org}/{course}/{block_type}/{block_id}/{sha1}{ext} - - This zip file is then extracted to the media/{scorm_location}/{block_id}. - - The scorm location is defined by the LOCATION xblock setting. If undefined, this is - "scorm". This setting can be set e.g: - - XBLOCK_SETTINGS["ScormXBlock"] = { - "LOCATION": "alternatevalue", - } - - Note that neither the folder the folder nor the package file are deleted when the - xblock is removed. - - By default, static assets are stored in the default Django storage backend. To - override this behaviour, you should define a custom storage function. This - function must take the xblock instance as its first and only argument. For instance, - you can store assets in different directories depending on the XBlock organization with:: - - def scorm_storage(xblock): - from django.conf import settings - from django.core.files.storage import FileSystemStorage - from openedx.core.djangoapps.site_configuration.models import SiteConfiguration - - subfolder = SiteConfiguration.get_value_for_org( - xblock.location.org, "SCORM_STORAGE_NAME", "default" - ) - storage_location = os.path.join(settings.MEDIA_ROOT, subfolder) - return get_storage_class(settings.DEFAULT_FILE_STORAGE)(location=storage_location) - - XBLOCK_SETTINGS["ScormXBlock"] = { - "STORAGE_FUNC": scorm_storage, - } - """ display_name = String( display_name=_("Display Name"), From e6f8fdaa03672f95d460b92afd1d7785d7f13483 Mon Sep 17 00:00:00 2001 From: Danyal-Faheem Date: Fri, 7 Mar 2025 19:44:54 +0500 Subject: [PATCH 2/2] perf: provide option to use default storage url or proxy assets through LMS This was provided as proxying assets through the LMS can be slower than directly accessing from external storage The default value will be to proxy assets through the LMS as it is more reliable --- README.rst | 13 +++++++++++++ changelog.d/20250307_193543_danyal.faheem.md | 1 + openedxscorm/scormxblock.py | 10 ++++++++-- 3 files changed, 22 insertions(+), 2 deletions(-) create mode 100644 changelog.d/20250307_193543_danyal.faheem.md diff --git a/README.rst b/README.rst index 30b59e8..9fc5a42 100644 --- a/README.rst +++ b/README.rst @@ -106,6 +106,19 @@ This should be added both to the LMS and the CMS settings. Instead of a function Note that the SCORM XBlock comes with extended S3 storage support out of the box. See the following section: +Default storage URL +~~~~~~~~~~~~~~~~~~ + +By default, scorm will proxy assets through the LMS. This is done for security and to make the backend generic enough to be used with different storage backends. However, to utilize the default storage defined url, add the following to the ScormXBlock settings:: + +.. code-block:: python + + XBLOCK_SETTINGS["ScormXBlock"] = { + "DEFAULT_STORAGE_URL": True, + } + +Scorm will now use the configured storages default `url` method instead of proxying the data through the LMS. The url method must be defined on the configured storage class for this work. + S3 storage ~~~~~~~~~~ diff --git a/changelog.d/20250307_193543_danyal.faheem.md b/changelog.d/20250307_193543_danyal.faheem.md new file mode 100644 index 0000000..fc9d18e --- /dev/null +++ b/changelog.d/20250307_193543_danyal.faheem.md @@ -0,0 +1 @@ +- [Improvement] Provide an option to use the default storage url to get scorm assets or proxy them through the LMS. (by @Danyal-Faheem) \ No newline at end of file diff --git a/openedxscorm/scormxblock.py b/openedxscorm/scormxblock.py index 1202e8d..81ee8d3 100644 --- a/openedxscorm/scormxblock.py +++ b/openedxscorm/scormxblock.py @@ -383,14 +383,20 @@ def extract_package(self, package_file): def index_page_url(self): if not self.package_meta or not self.index_page_path: return "" + + if not self.xblock_settings.get("DEFAULT_STORAGE_URL", False): + return f"{self.proxy_base_url}/{self.index_page_path}" + + folder = self.extract_folder_path if self.storage.exists( os.path.join(self.extract_folder_base_path, self.clean_path(self.index_page_path)) ): # For backward-compatibility, we must handle the case when the xblock data # is stored in the base folder. - logger.warning("Serving SCORM content from old-style path: %s", self.extract_folder_base_path) + folder = self.extract_folder_base_path + logger.warning("Serving SCORM content from old-style path: %s", folder) - return f"{self.proxy_base_url}/{self.index_page_path}" + return self.storage.url(os.path.join(folder, self.index_page_path)) @property def proxy_base_url(self):