diff --git a/openedx_learning/apps/authoring/backup_restore/zipper.py b/openedx_learning/apps/authoring/backup_restore/zipper.py index 5386e9aff..1443b63f3 100644 --- a/openedx_learning/apps/authoring/backup_restore/zipper.py +++ b/openedx_learning/apps/authoring/backup_restore/zipper.py @@ -2,11 +2,13 @@ This module provides functionality to create a zip file containing the learning package data, including a TOML representation of the learning package and its entities. """ +import hashlib import zipfile from pathlib import Path from typing import Optional from django.db.models import QuerySet +from django.utils.text import slugify from openedx_learning.apps.authoring.backup_restore.toml import toml_learning_package, toml_publishable_entity from openedx_learning.apps.authoring.publishing import api as publishing_api @@ -19,6 +21,32 @@ TOML_PACKAGE_NAME = "package.toml" +def slugify_hashed_filename(identifier: str) -> str: + """ + Generate a filesystem-safe filename from an identifier. + + Why: + Identifiers may contain characters that are invalid or ambiguous + in filesystems (e.g., slashes, colons, case differences). + Additionally, two different identifiers might normalize to the same + slug after cleaning. To avoid collisions and ensure uniqueness, + we append a short blake2b hash. + + What: + - Slugify the identifier (preserves most characters, only strips + filesystem-invalid ones). + - Append a short hash for uniqueness. + - Result: human-readable but still unique and filesystem-safe filename. + """ + slug = slugify(identifier, allow_unicode=True) + # Short digest ensures uniqueness without overly long filenames + short_hash = hashlib.blake2b( + identifier.encode("utf-8"), + digest_size=3, + ).hexdigest() + return f"{slug}_{short_hash}" + + class LearningPackageZipper: """ A class to handle the zipping of learning content for backup and restore. @@ -73,7 +101,8 @@ def create_zip(self, path: str) -> None: entity_toml_content: str = toml_publishable_entity(entity) if hasattr(entity, 'container'): - entity_toml_filename = f"{entity.key}.toml" + entity_slugify_hash = slugify_hashed_filename(entity.key) + entity_toml_filename = f"{entity_slugify_hash}.toml" entity_toml_path = entities_folder / entity_toml_filename zipf.writestr(str(entity_toml_path), entity_toml_content) @@ -87,7 +116,7 @@ def create_zip(self, path: str) -> None: # component_versions/ # v1/ # static/ - + entity_slugify_hash = slugify_hashed_filename(entity.component.local_key) component_namespace_folder = entities_folder / entity.component.component_type.namespace # Example of component namespace is: "xblock.v1" self.create_folder(component_namespace_folder, zipf) @@ -96,12 +125,12 @@ def create_zip(self, path: str) -> None: # Example of component type is: "html" self.create_folder(component_type_folder, zipf) - component_id_folder = component_type_folder / entity.component.local_key # entity.key + component_id_folder = component_type_folder / entity_slugify_hash # Example of component id is: "i-dont-like-the-sidebar-aa1645ade4a7" self.create_folder(component_id_folder, zipf) # Add the entity TOML file inside the component type folder as well - component_entity_toml_path = component_type_folder / f"{entity.component.local_key}.toml" + component_entity_toml_path = component_type_folder / f"{entity_slugify_hash}.toml" zipf.writestr(str(component_entity_toml_path), entity_toml_content) # Add component version folder into the component id folder diff --git a/tests/openedx_learning/apps/authoring/backup_restore/test_backup.py b/tests/openedx_learning/apps/authoring/backup_restore/test_backup.py index a40bc3c99..b51b3e12a 100644 --- a/tests/openedx_learning/apps/authoring/backup_restore/test_backup.py +++ b/tests/openedx_learning/apps/authoring/backup_restore/test_backup.py @@ -117,17 +117,17 @@ def check_zip_file_structure(self, zip_path: Path): "collections/", "entities/xblock.v1/", "entities/xblock.v1/html/", - "entities/xblock.v1/html/my_draft_example.toml", - "entities/xblock.v1/html/my_draft_example/", - "entities/xblock.v1/html/my_draft_example/component_versions/", - "entities/xblock.v1/html/my_draft_example/component_versions/v2/", - "entities/xblock.v1/html/my_draft_example/component_versions/v2/static/", + "entities/xblock.v1/html/my_draft_example_af06e1.toml", + "entities/xblock.v1/html/my_draft_example_af06e1/", + "entities/xblock.v1/html/my_draft_example_af06e1/component_versions/", + "entities/xblock.v1/html/my_draft_example_af06e1/component_versions/v2/", + "entities/xblock.v1/html/my_draft_example_af06e1/component_versions/v2/static/", "entities/xblock.v1/problem/", - "entities/xblock.v1/problem/my_published_example/", - "entities/xblock.v1/problem/my_published_example.toml", - "entities/xblock.v1/problem/my_published_example/component_versions/", - "entities/xblock.v1/problem/my_published_example/component_versions/v1/", - "entities/xblock.v1/problem/my_published_example/component_versions/v1/static/", + "entities/xblock.v1/problem/my_published_example_386dce/", + "entities/xblock.v1/problem/my_published_example_386dce.toml", + "entities/xblock.v1/problem/my_published_example_386dce/component_versions/", + "entities/xblock.v1/problem/my_published_example_386dce/component_versions/v1/", + "entities/xblock.v1/problem/my_published_example_386dce/component_versions/v1/static/", ] # Check that all expected paths are present @@ -166,7 +166,7 @@ def test_lp_dump_command(self): # Check the content of the entity TOML files expected_files = { - "entities/xblock.v1/problem/my_published_example.toml": [ + "entities/xblock.v1/problem/my_published_example_386dce.toml": [ '[entity]', f'uuid = "{self.published_component.uuid}"', 'can_stand_alone = true', @@ -175,7 +175,7 @@ def test_lp_dump_command(self): '[entity.published]', 'version_num = 1', ], - "entities/xblock.v1/html/my_draft_example.toml": [ + "entities/xblock.v1/html/my_draft_example_af06e1.toml": [ '[entity]', f'uuid = "{self.draft_component.uuid}"', 'can_stand_alone = true', diff --git a/tests/openedx_learning/apps/authoring/backup_restore/test_slug_hash.py b/tests/openedx_learning/apps/authoring/backup_restore/test_slug_hash.py new file mode 100644 index 000000000..292d08af1 --- /dev/null +++ b/tests/openedx_learning/apps/authoring/backup_restore/test_slug_hash.py @@ -0,0 +1,76 @@ +""" +Test cases for the slugify_hashed_filename function. + +These tests ensure consistent and predictable behavior when +generating slugified, hash-based filenames. +""" + +from openedx_learning.apps.authoring.backup_restore.zipper import slugify_hashed_filename +from openedx_learning.lib.test_utils import TestCase + + +class SlugHashTestCase(TestCase): + """ + Test the slugify_hashed_filename function. + """ + + def test_slugify_hashed_filename(self): + # Test the slugify_hashed_filename function + self.assertEqual(slugify_hashed_filename("my_example"), "my_example_880989") + + def test_slugify_hashed_filename_special_chars(self): + # Test the slugify_hashed_filename function with special characters + self.assertEqual(slugify_hashed_filename("my@ex#ample!"), "myexample_3366b5") + + def test_slugify_hashed_filename_invalid_characters_common_filesystems(self): + # Test the slugify_hashed_filename function with invalid characters for common filesystems + self.assertEqual( + slugify_hashed_filename("xblock.v1:problem:my_component"), "xblockv1problemmy_component_d346b1" + ) + self.assertEqual( + slugify_hashed_filename("xblock.v1/problem/my_component"), "xblockv1problemmy_component_2648c6" + ) + self.assertEqual( + slugify_hashed_filename("xblock.v1?problem?my_component"), "xblockv1problemmy_component_12a86d" + ) + self.assertEqual( + slugify_hashed_filename("xblock.v1\\problem_[my_component]"), "xblockv1problem_my_component_a59bb3" + ) + self.assertEqual( + slugify_hashed_filename("xblock.v1>problem_>my_component"), "xblockv1problem_my_component_8497eb" + ) + self.assertEqual( + slugify_hashed_filename("xblock.v1*problem*EndsWith|"), "xblockv1problemendswith_b88aab" + ) + self.assertEqual( + slugify_hashed_filename("xblock.v1*problem*EndsWith."), "xblockv1problemendswith_7fa0cf" + ) + + def test_slugify_hashed_filename_unicode(self): + # Test the slugify_hashed_filename function with unicode characters + + # Example with accents + self.assertEqual(slugify_hashed_filename("café"), "café_07f4df") + self.assertEqual(slugify_hashed_filename("naïve"), "naïve_308c7e") + + # Example with non-latin (e.g., Japanese) + identifier = "テスト用" + result = slugify_hashed_filename(identifier) + self.assertEqual(result, "テスト用_be48ab") + + # Example with mixed characters + identifier = "café_テスト用" + result = slugify_hashed_filename(identifier) + self.assertEqual(result, "café_テスト用_3cf9ef") + + def test_slugify_hashed_filename_long_string(self): + # Test the slugify_hashed_filename function with a long string + long_string = "a" * 100 + self.assertEqual(slugify_hashed_filename(long_string), f"{long_string}_4e84b3") + + def test_slugify_hashed_filename_case_insensitivity(self): + # Test the slugify_hashed_filename function for case insensitivity + upper_case_value = slugify_hashed_filename("MY_EXAMPLE") + lower_case_value = slugify_hashed_filename("my_example") + # The values should be different even though they are the same but with different cases + self.assertNotEqual(upper_case_value, lower_case_value)