From e3beb49a0ea8d5e831db050b2bd28e81423bc424 Mon Sep 17 00:00:00 2001 From: Zia Fazal Date: Fri, 21 Jun 2024 21:42:18 +0500 Subject: [PATCH] fix: S3 storage exists always return false for directories (#77) These changes use listdir method instead of exists method of stoage because storage.exists expects a file path when storage is set to S3 --- ...21_201400_zia.fazal_fix_backward_compat.md | 1 + openedxscorm/scormxblock.py | 22 ++++++++++++++++--- 2 files changed, 20 insertions(+), 3 deletions(-) create mode 100644 changelog.d/20240621_201400_zia.fazal_fix_backward_compat.md diff --git a/changelog.d/20240621_201400_zia.fazal_fix_backward_compat.md b/changelog.d/20240621_201400_zia.fazal_fix_backward_compat.md new file mode 100644 index 0000000..f0a9383 --- /dev/null +++ b/changelog.d/20240621_201400_zia.fazal_fix_backward_compat.md @@ -0,0 +1 @@ +- [Bugfix] Make addition of block usage key in scorm path backward compatible. (by @ziafazal) \ No newline at end of file diff --git a/openedxscorm/scormxblock.py b/openedxscorm/scormxblock.py index c3b660e..f94a085 100644 --- a/openedxscorm/scormxblock.py +++ b/openedxscorm/scormxblock.py @@ -341,7 +341,7 @@ def popup_window(self, request, _suffix): return Response(body=rendered) def clean_storage(self): - if self.storage.exists(self.extract_folder_base_path): + if self.path_exists(self.extract_folder_base_path): logger.info( 'Removing previously unzipped "%s"', self.extract_folder_base_path ) @@ -400,7 +400,7 @@ def index_page_url(self): return "" folder = self.extract_folder_path if self.storage.exists( - os.path.join(self.extract_folder_base_path, self.index_page_path) + 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. @@ -417,6 +417,22 @@ def extract_folder_path(self): """ return os.path.join(self.extract_folder_base_path, self.package_meta["sha1"]) + def clean_path(self, path): + """ + Removes query string from a path + """ + return path.split('?')[0] if path else path + + def path_exists(self, path): + """ + Returs True if given path exists in storage otherwise returns False + """ + try: + dirs, files = self.storage.listdir(path) + return True if dirs or files else False + except FileNotFoundError: + return False + @property def extract_folder_base_path(self): """ @@ -424,7 +440,7 @@ def extract_folder_base_path(self): Compute hash of the unique block usage_id and use that as our directory name. """ # For backwards compatibility, we return the old path if the directory exists - if self.storage.exists(self.extract_old_folder_base_path): + if self.path_exists(self.extract_old_folder_base_path): return self.extract_old_folder_base_path sha1 = hashlib.sha1() sha1.update(str(self.scope_ids.usage_id).encode())