diff --git a/src/openedx_content/applets/backup_restore/serializers.py b/src/openedx_content/applets/backup_restore/serializers.py index eb43528cb..105c25094 100644 --- a/src/openedx_content/applets/backup_restore/serializers.py +++ b/src/openedx_content/applets/backup_restore/serializers.py @@ -63,7 +63,9 @@ class EntityVersionSerializer(serializers.Serializer): # pylint: disable=abstra """ Serializer for publishable entity versions. """ - title = serializers.CharField(required=True) + # We allow_blank because empty unit titles are legal and common. + title = serializers.CharField(required=True, allow_blank=True) + created = serializers.DateTimeField(required=True, default_timezone=timezone.utc) version_num = serializers.IntegerField(required=True) diff --git a/src/openedx_content/applets/backup_restore/zipper.py b/src/openedx_content/applets/backup_restore/zipper.py index 3261836c3..321cef3e2 100644 --- a/src/openedx_content/applets/backup_restore/zipper.py +++ b/src/openedx_content/applets/backup_restore/zipper.py @@ -4,6 +4,7 @@ """ import hashlib import time +import tomllib import zipfile from collections import defaultdict from dataclasses import asdict, dataclass @@ -282,7 +283,7 @@ def create_zip(self, path: str) -> None: Exception: If the learning package cannot be found or if the zip creation fails. """ - with zipfile.ZipFile(path, "w", compression=zipfile.ZIP_DEFLATED) as zipf: + with zipfile.ZipFile(path, "w", compression=zipfile.ZIP_DEFLATED, compresslevel=9) as zipf: # Add the package.toml file package_toml_content: str = toml_learning_package( self.learning_package, self.utc_now, user=self.user, origin_server=self.origin_server @@ -1060,7 +1061,17 @@ def _get_organized_file_list(self, file_paths: list[str]) -> dict[str, Any]: "collections": [], } - for path in file_paths: + # This is going to map static file directory roots to the appropriate + # entity refs. + comp_paths_to_refs = {} + + # The ordering of the file processing is important because we need to + # ensure that TOML files for a given component are processed before the + # static files for that component. '.' sorts before '/', so "foo.toml" + # will sort before "foo/component_versions/v1/static/figure1.webp" or + # any other subdirectory of the "foo" component. Processing the TOML + # first allows us to map the directory to a entity ref. + for path in sorted(file_paths): if path.endswith("/"): # Skip directories continue @@ -1073,21 +1084,44 @@ def _get_organized_file_list(self, file_paths: list[str]) -> dict[str, Any]: if path.endswith(".toml"): # Component entity TOML files organized["components"].append(path) + component_toml_str = self._read_file_from_zip(path) + component_toml = tomllib.loads(component_toml_str) + entity_ref = component_toml['entity']['key'] + comp_path = path.removesuffix(".toml") + + # This maps the root path of a component, e.g."entities/xblock.v1/html/my_component_a822bb" + # to the actual ref, e.g. "xblock.v1:html:my_component". The last part of the ref will + # often correlate to the directory name, but does not have to (a hash is sometimes added). + comp_paths_to_refs[comp_path] = entity_ref + else: # Component static files # Path structure: entities////component_versions//static/... - # Example: entities/xblock.v1/html/my_component_123456/component_versions/v1/static/... - component_key = Path(path).parts[1:4] # e.g., ['xblock.v1', 'html', 'my_component_123456'] + # Example: entities/xblock.v1/html/my_component_a822bb/component_versions/v1/static/... + + # e.g. 'entities/xblock.v1/html/my_component_a822bb' + component_root_path = '/'.join(Path(path).parts[0:4]) + + try: + component_ref = comp_paths_to_refs[component_root_path] + except KeyError: + self.errors.append( + { + "file": path, + "errors": f"Missing component TOML file at {component_root_path}.toml" + } + ) + continue + num_version = Path(path).parts[5] if len(Path(path).parts) > 5 else "v1" # e.g., 'v1' - if len(component_key) == 3: - component_identifier = ":".join(component_key) - component_identifier += f":{num_version}" - organized["component_static_files"][component_identifier].append(path) - else: - self.errors.append({"file": path, "errors": "Invalid component static file path structure."}) + + component_ref += f":{num_version}" + organized["component_static_files"][component_ref].append(path) + elif path.startswith("collections/") and path.endswith(".toml"): # Collection TOML files organized["collections"].append(path) + return organized def _get_versions_to_write( diff --git a/src/openedx_core/__init__.py b/src/openedx_core/__init__.py index 005003f0a..688bcfa14 100644 --- a/src/openedx_core/__init__.py +++ b/src/openedx_core/__init__.py @@ -6,4 +6,4 @@ """ # The version for the entire repository -__version__ = "1.0.2" +__version__ = "1.0.3" diff --git a/tests/openedx_content/applets/backup_restore/fixtures/library_backup/entities/section1-8ca126.toml b/tests/openedx_content/applets/backup_restore/fixtures/library_backup/entities/section1-extra-8ca126.toml similarity index 80% rename from tests/openedx_content/applets/backup_restore/fixtures/library_backup/entities/section1-8ca126.toml rename to tests/openedx_content/applets/backup_restore/fixtures/library_backup/entities/section1-extra-8ca126.toml index 00c34d430..855902ab3 100644 --- a/tests/openedx_content/applets/backup_restore/fixtures/library_backup/entities/section1-8ca126.toml +++ b/tests/openedx_content/applets/backup_restore/fixtures/library_backup/entities/section1-extra-8ca126.toml @@ -1,6 +1,6 @@ [entity] can_stand_alone = true -key = "section1-8ca126" +key = "section1-8ca126" # This intentionally does not match the filename created = 2025-09-04T22:51:40.919872Z [entity.draft] diff --git a/tests/openedx_content/applets/backup_restore/fixtures/library_backup/entities/unit1-b7eafb.toml b/tests/openedx_content/applets/backup_restore/fixtures/library_backup/entities/unit1-b7eafb.toml index b8ab55466..4d94f45f6 100644 --- a/tests/openedx_content/applets/backup_restore/fixtures/library_backup/entities/unit1-b7eafb.toml +++ b/tests/openedx_content/applets/backup_restore/fixtures/library_backup/entities/unit1-b7eafb.toml @@ -14,7 +14,7 @@ version_num = 2 # ### Versions [[version]] -title = "Unit1" +title = "" # Intentionally blank in order to test that empty Unit names work version_num = 2 [version.container] diff --git a/tests/openedx_content/applets/backup_restore/fixtures/library_backup/entities/xblock.v1/html/c22b9f97-f1e9-4e8f-87f0-d5a3c26083e2.toml b/tests/openedx_content/applets/backup_restore/fixtures/library_backup/entities/xblock.v1/html/c22b9f97-f1e9-4e8f-87f0-d5a3c26083e2-extra.toml similarity index 64% rename from tests/openedx_content/applets/backup_restore/fixtures/library_backup/entities/xblock.v1/html/c22b9f97-f1e9-4e8f-87f0-d5a3c26083e2.toml rename to tests/openedx_content/applets/backup_restore/fixtures/library_backup/entities/xblock.v1/html/c22b9f97-f1e9-4e8f-87f0-d5a3c26083e2-extra.toml index ca759834e..bf88a740f 100644 --- a/tests/openedx_content/applets/backup_restore/fixtures/library_backup/entities/xblock.v1/html/c22b9f97-f1e9-4e8f-87f0-d5a3c26083e2.toml +++ b/tests/openedx_content/applets/backup_restore/fixtures/library_backup/entities/xblock.v1/html/c22b9f97-f1e9-4e8f-87f0-d5a3c26083e2-extra.toml @@ -1,6 +1,6 @@ [entity] can_stand_alone = true -key = "xblock.v1:html:c22b9f97-f1e9-4e8f-87f0-d5a3c26083e2" +key = "xblock.v1:html:c22b9f97-f1e9-4e8f-87f0-d5a3c26083e2" # This intentionally does not match the filename created = 2025-10-06T16:59:34.160314Z [entity.draft] diff --git a/tests/openedx_content/applets/backup_restore/fixtures/library_backup/entities/xblock.v1/html/c22b9f97-f1e9-4e8f-87f0-d5a3c26083e2/component_versions/v2/block.xml b/tests/openedx_content/applets/backup_restore/fixtures/library_backup/entities/xblock.v1/html/c22b9f97-f1e9-4e8f-87f0-d5a3c26083e2-extra/component_versions/v2/block.xml similarity index 100% rename from tests/openedx_content/applets/backup_restore/fixtures/library_backup/entities/xblock.v1/html/c22b9f97-f1e9-4e8f-87f0-d5a3c26083e2/component_versions/v2/block.xml rename to tests/openedx_content/applets/backup_restore/fixtures/library_backup/entities/xblock.v1/html/c22b9f97-f1e9-4e8f-87f0-d5a3c26083e2-extra/component_versions/v2/block.xml diff --git a/tests/openedx_content/applets/backup_restore/test_restore.py b/tests/openedx_content/applets/backup_restore/test_restore.py index ef8156a92..fcaf99acd 100644 --- a/tests/openedx_content/applets/backup_restore/test_restore.py +++ b/tests/openedx_content/applets/backup_restore/test_restore.py @@ -342,6 +342,29 @@ def test_error_no_metadata_section(self): expected_error = "Errors encountered during restore:\npackage.toml meta section: {'non_field_errors': [Er" assert expected_error in log_content + def test_restore_with_blank_unit_title(self): + """ + Restoring should succeed when a container version has a blank title. + + Blank titles are legal and common -- content imported from courses + (e.g. via the modulestore migrator) frequently has untitled units, and + such content can be backed up. Restoring that same archive must work. + + The ``library_backup`` fixture's ``unit1`` deliberately has a blank + title to exercise this path. + """ + result = LearningPackageUnzipper(self.zip_file, package_ref="lib-xx:WGU:LIB_C001").load() + + assert result["status"] == "success", f"Restore failed: {result['log_file_error']}" + assert result["log_file_error"] is None + + lp = publishing_api.LearningPackage.objects.get(package_ref="lib-xx:WGU:LIB_C001") + unit = containers_api.get_containers(learning_package_id=lp.id).get( + publishable_entity__entity_ref="unit1-b7eafb" + ) + draft_version = publishing_api.get_draft_version(unit.publishable_entity.id) + assert draft_version.title == "" + def test_success_metadata_using_user_context(self): """Test that metadata is correctly extracted from learning_package.toml.""" restore_result = LearningPackageUnzipper(self.zip_file, user=self.user).load()