From 2ccf7d2ab883c6118c8287420ada1aea80cfb973 Mon Sep 17 00:00:00 2001 From: Daniel Wong Date: Sat, 4 Oct 2025 11:48:53 -0600 Subject: [PATCH 1/9] fix: remove uuid from TOML files and unnecessary container table in components TOML files --- .../apps/authoring/backup_restore/toml.py | 31 ++++++------------- .../apps/authoring/backup_restore/zipper.py | 4 +-- 2 files changed, 12 insertions(+), 23 deletions(-) diff --git a/openedx_learning/apps/authoring/backup_restore/toml.py b/openedx_learning/apps/authoring/backup_restore/toml.py index 01951a53f..6742ab7f9 100644 --- a/openedx_learning/apps/authoring/backup_restore/toml.py +++ b/openedx_learning/apps/authoring/backup_restore/toml.py @@ -72,7 +72,6 @@ def _get_toml_publishable_entity_table( The resulting content looks like: [entity] - uuid = "f8ea9bae-b4ed-4a84-ab4f-2b9850b59cd6" can_stand_alone = true key = "xblock.v1:problem:my_published_example" @@ -88,7 +87,6 @@ def _get_toml_publishable_entity_table( a string-like TOML fragment rather than a complete TOML document. """ entity_table = tomlkit.table() - entity_table.add("uuid", str(entity.uuid)) entity_table.add("can_stand_alone", entity.can_stand_alone) # Add key since the toml filename doesn't show the real key entity_table.add("key", entity.key) @@ -133,7 +131,6 @@ def toml_publishable_entity( The resulting content looks like: [entity] - uuid = "f8ea9bae-b4ed-4a84-ab4f-2b9850b59cd6" can_stand_alone = true key = "xblock.v1:problem:my_published_example" @@ -143,17 +140,16 @@ def toml_publishable_entity( [entity.published] version_num = 1 + [entity.container.section] (if applicable) + # ### Versions [[version]] title = "My published problem" - uuid = "2e07511f-daa7-428a-9032-17fe12a77d06" version_num = 1 - [version.container] + [version.container] (if applicable) children = [] - - [version.container.unit] """ # Create the TOML representation for the entity itself entity_table = _get_toml_publishable_entity_table(entity, draft_version, published_version) @@ -179,32 +175,25 @@ def toml_publishable_entity_version(version: PublishableEntityVersion) -> tomlki The resulting content looks like: [[version]] title = "My published problem" - uuid = "2e07511f-daa7-428a-9032-17fe12a77d06" version_num = 1 - [version.container] + [version.container] (if applicable) children = [] - [version.container.unit] - graded = true - - Note: This function returns a tomlkit.items.Table, which represents + Note: This function returns a tomlkit.items.Table, which represents a string-like TOML fragment rather than a complete TOML document. """ version_table = tomlkit.table() version_table.add("title", version.title) - version_table.add("uuid", str(version.uuid)) version_table.add("version_num", version.version_num) - container_table = tomlkit.table() - - children = [] if hasattr(version, 'containerversion'): + # If the version has a container version, add its children + container_table = tomlkit.table() children = publishing_api.get_container_children_entities_keys(version.containerversion) - container_table.add("children", children) - - version_table.add("container", container_table) - return version_table # For use in AoT + container_table.add("children", children) + version_table.add("container", container_table) + return version_table def toml_collection(collection: Collection, entity_keys: list[str]) -> str: diff --git a/openedx_learning/apps/authoring/backup_restore/zipper.py b/openedx_learning/apps/authoring/backup_restore/zipper.py index 1f38c0fbd..a8c1536b3 100644 --- a/openedx_learning/apps/authoring/backup_restore/zipper.py +++ b/openedx_learning/apps/authoring/backup_restore/zipper.py @@ -402,8 +402,8 @@ class LearningPackageUnzipper: - Ensure atomicity of the restore process. Usage: - unzipper = LearningPackageUnzipper() - summary = unzipper.load("/path/to/backup.zip") + unzipper = LearningPackageUnzipper(zip_file) + result = unzipper.load() """ def __init__(self, zipf: zipfile.ZipFile) -> None: From 2c485e1da84576053c7811bd7e8994cce89349e3 Mon Sep 17 00:00:00 2001 From: Daniel Wong Date: Sat, 4 Oct 2025 11:51:06 -0600 Subject: [PATCH 2/9] test: remove uuid verification from backup test --- .../apps/authoring/backup_restore/test_backup.py | 2 -- 1 file changed, 2 deletions(-) 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 c257a1e60..8a9cafa46 100644 --- a/tests/openedx_learning/apps/authoring/backup_restore/test_backup.py +++ b/tests/openedx_learning/apps/authoring/backup_restore/test_backup.py @@ -250,7 +250,6 @@ def test_lp_dump_command(self): expected_files = { "entities/xblock.v1/problem/my_published_example_386dce.toml": [ '[entity]', - f'uuid = "{self.published_component.uuid}"', 'can_stand_alone = true', '[entity.draft]', 'version_num = 2', @@ -259,7 +258,6 @@ def test_lp_dump_command(self): ], "entities/xblock.v1/html/my_draft_example.toml": [ '[entity]', - f'uuid = "{self.draft_component.uuid}"', 'can_stand_alone = true', '[entity.draft]', 'version_num = 2', From 9932939e2507850919d2dc621e6cc091046cb510 Mon Sep 17 00:00:00 2001 From: Daniel Wong Date: Sat, 4 Oct 2025 15:39:11 -0600 Subject: [PATCH 3/9] feat: add success and error responses to the restore process --- .../apps/authoring/backup_restore/zipper.py | 120 ++++++++++++------ .../authoring/backup_restore/test_restore.py | 61 +++++++++ 2 files changed, 139 insertions(+), 42 deletions(-) diff --git a/openedx_learning/apps/authoring/backup_restore/zipper.py b/openedx_learning/apps/authoring/backup_restore/zipper.py index a8c1536b3..fb89f6ea1 100644 --- a/openedx_learning/apps/authoring/backup_restore/zipper.py +++ b/openedx_learning/apps/authoring/backup_restore/zipper.py @@ -6,6 +6,7 @@ import zipfile from collections import defaultdict from datetime import datetime, timezone +from io import StringIO from pathlib import Path from typing import Any, List, Optional, Tuple, TypedDict @@ -425,12 +426,20 @@ def __init__(self, zipf: zipfile.ZipFile) -> None: @transaction.atomic def load(self) -> dict[str, Any]: """Extracts and restores all objects from the ZIP archive in an atomic transaction.""" - organized_files = self._get_organized_file_list(self.zipf.namelist()) - - if not organized_files["learning_package"]: - raise FileNotFoundError(f"Missing required {TOML_PACKAGE_NAME} in archive.") - learning_package = self._load_learning_package(organized_files["learning_package"]) + # Step 1: Validate presence of mandatory files + _, organized_files = self.preliminary_check() + if self.errors: + # Early return if preliminary checks fail since mandatory files are missing + return { + "status": "error", + "log_file_error": self._write_errors(), # return a StringIO with the errors + "general_info": None + } + + # Step 2: Extract and validate learning package, entities and collections + # Errors are collected and reported at the end + # No saving to DB happens until all validations pass components_validated = self._extract_entities( organized_files["components"], ComponentSerializer, ComponentVersionSerializer ) @@ -442,23 +451,53 @@ def load(self) -> dict[str, Any]: organized_files["collections"] ) - self._write_errors() - if not self.errors: - self._save( - learning_package, - components_validated, - containers_validated, - collections_validated, - component_static_files=organized_files["component_static_files"] - ) + # Step 3.1: If there are validation errors, return them without saving anything + if self.errors: + return { + "status": "error", + "log_file_error": self._write_errors(), # return a StringIO with the errors + "general_info": None + } + + # Step 3.2: Save everything to the DB + # All validations passed, we can proceed to save everything + # Save the learning package first to get its ID + learning_package = self._load_learning_package(organized_files["learning_package"]) + self._save( + learning_package, + components_validated, + containers_validated, + collections_validated, + component_static_files=organized_files["component_static_files"] + ) + num_containers = sum( + len(containers_validated.get(container_type, [])) + for container_type in ["section", "subsection", "unit"] + ) return { - "learning_package": learning_package.key, - "containers": len(organized_files["containers"]), - "components": len(organized_files["components"]), - "collections": len(organized_files["collections"]), + "status": "success", + "log_file_error": None, + "general_info": { + "learning_package_key": learning_package.key, + "learning_package_title": learning_package.title, + "backed_up_at": learning_package.created, + "containers": num_containers, + "components": len(components_validated["components"]), + "collections": len(collections_validated["collections"]), + "metadata": {}, + } } + def preliminary_check(self) -> Tuple[list[dict[str, Any]], dict[str, Any]]: + """Performs a preliminary check of the zip file structure and mandatory files.""" + organized_files = self._get_organized_file_list(self.zipf.namelist()) + + if not organized_files["learning_package"]: + self.errors.append({"file": TOML_PACKAGE_NAME, "errors": "Missing learning package file."}) + + return self.errors, organized_files + # -------------------------- # Extract + Validate # -------------------------- @@ -681,31 +720,21 @@ def _save_draft_versions(self, components, containers, component_static_files): # Utilities # -------------------------- - def _write_errors(self) -> str | None: - """ - Writes restore errors to a timestamped log file and prints them to console. + def _format_errors(self) -> str: + """Return formatted error content as a string.""" + if not self.errors: + return "" + lines = [f"{err['file']}: {err['errors']}" for err in self.errors] + return "Errors encountered during restore:\n" + "\n".join(lines) + "\n" - Args: - errors (list[dict]): List of {"file": ..., "errors": ...} dicts. - log_dir (str): Directory to save the log file (default current dir). + def _write_errors(self) -> StringIO | None: """ - errors = self.errors - if not errors: + Write errors to a StringIO buffer. + """ + content = self._format_errors() + if not content: return None - - # Create timestamped log filename - timestamp = datetime.now().strftime("%Y-%m-%d_%H-%M-%S") - log_filename = f"restore_{timestamp}.log" - - # Format each error on a separate line - lines = [f"{err['file']}: {err['errors']}" for err in errors] - content = "Errors encountered during restore:\n" + "\n".join(lines) + "\n" - - # Write to file - with open(log_filename, "w", encoding="utf-8") as f: - f.write(content) - - return log_filename + return StringIO(content) def _resolve_static_files( self, @@ -784,15 +813,21 @@ def _get_organized_file_list(self, file_paths: list[str]) -> dict[str, Any]: for path in file_paths: if path.endswith("/"): + # Skip directories continue if path == TOML_PACKAGE_NAME: organized["learning_package"] = path - elif path.startswith("entities/") and str(Path(path).parent) == "entities": + elif path.startswith("entities/") and str(Path(path).parent) == "entities" and path.endswith(".toml"): + # Top-level entity TOML files are considered containers organized["containers"].append(path) elif path.startswith("entities/"): if path.endswith(".toml"): + # Component entity TOML files organized["components"].append(path) 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'] num_version = Path(path).parts[5] if len(Path(path).parts) > 5 else "v1" # e.g., 'v1' if len(component_key) == 3: @@ -801,7 +836,8 @@ def _get_organized_file_list(self, file_paths: list[str]) -> dict[str, Any]: organized["component_static_files"][component_identifier].append(path) else: self.errors.append({"file": path, "errors": "Invalid component static file path structure."}) - elif path.startswith("collections/"): + elif path.startswith("collections/") and path.endswith(".toml"): + # Collection TOML files organized["collections"].append(path) return organized diff --git a/tests/openedx_learning/apps/authoring/backup_restore/test_restore.py b/tests/openedx_learning/apps/authoring/backup_restore/test_restore.py index 52ffcf470..ffc72c178 100644 --- a/tests/openedx_learning/apps/authoring/backup_restore/test_restore.py +++ b/tests/openedx_learning/apps/authoring/backup_restore/test_restore.py @@ -140,3 +140,64 @@ def verify_collections(self, lp): ] entity_keys = [entity.key for entity in collection.entities.all()] assert set(entity_keys) == set(expected_entity_keys) + + +class RestoreLearningPackageTest(TestCase): + """Tests for restoring learning packages without using the management command.""" + + def test_successful_restore_with_no_command_line(self): + """Test restoring a learning package without using the management command.""" + zip_file = folder_to_inmemory_zip(os.path.join(os.path.dirname(__file__), "fixtures/library_backup")) + result = LearningPackageUnzipper(zip_file).load() + + expected = { + "status": "success", + "log_file_error": None, + "general_info": { + "learning_package_key": "lib:WGU:LIB_C001", + "learning_package_title": "Library test", + "backed_up_at": None, + "containers": 3, + "components": 6, + "collections": 1, + "metadata": {}, + }, + } + + assert result["status"] == expected["status"] + assert result["log_file_error"] == expected["log_file_error"] + assert ( + result["general_info"]["learning_package_key"] == expected["general_info"]["learning_package_key"] + ) + assert ( + result["general_info"]["learning_package_title"] == expected["general_info"]["learning_package_title"] + ) + assert result["general_info"]["containers"] == expected["general_info"]["containers"] + assert result["general_info"]["components"] == expected["general_info"]["components"] + assert result["general_info"]["collections"] == expected["general_info"]["collections"] + assert result["general_info"]["metadata"] == expected["general_info"]["metadata"] + + lp = publishing_api.LearningPackage.objects.filter(key="lib:WGU:LIB_C001").first() + assert lp is not None, "Learning package was not restored." + + def test_restore_with_missing_learning_package_file(self): + """Test restoring a learning package with a missing learning_package.toml file.""" + zip_file = folder_to_inmemory_zip(os.path.join(os.path.dirname(__file__), "fixtures/missing_lp_file")) + result = LearningPackageUnzipper(zip_file).load() + + assert result["status"] == "error" + assert result["general_info"] is None + assert result["log_file_error"] is not None + log_content = result["log_file_error"].getvalue() + assert "Missing learning package file." in log_content + assert "Missing required learning_package.toml in archive." not in log_content + + def test_error_preliminary_check(self): + """Test that preliminary check catches missing learning_package.toml.""" + zip_file = folder_to_inmemory_zip(os.path.join(os.path.dirname(__file__), "fixtures/missing_lp_file")) + unzipper = LearningPackageUnzipper(zip_file) + errors, _ = unzipper.preliminary_check() + + assert len(errors) == 1 + assert errors[0]["file"] == "package.toml" + assert errors[0]["errors"] == "Missing learning package file." From 1f74f97d623a8e80715350e6818cbe3e57ee4694 Mon Sep 17 00:00:00 2001 From: Daniel Wong Date: Sun, 5 Oct 2025 12:52:27 -0600 Subject: [PATCH 4/9] feat: improve validation for restore process inputs --- .../apps/authoring/backup_restore/api.py | 4 +- .../management/commands/lp_load.py | 10 ++- .../authoring/backup_restore/serializers.py | 20 +++++ .../apps/authoring/backup_restore/toml.py | 24 +---- .../apps/authoring/backup_restore/zipper.py | 78 +++++++++++------ .../entities/section1-8ca126.toml | 2 - .../entities/subsection1-48afa3.toml | 2 - .../library_backup/entities/unit1-b7eafb.toml | 2 - .../4d1b2fac-8b30-42fb-872d-6b10ab580b27.toml | 2 - .../e32d5479-9492-41f6-9222-550a7346bc37.toml | 3 - .../1ee38208-a585-4455-a27e-4930aa541f53.toml | 2 - .../256739e8-c2df-4ced-bd10-8156f6cfa90b.toml | 2 - .../6681da3f-b056-4c6e-a8f9-040967907471.toml | 2 - .../22601ebd-9da8-430b-9778-cfe059a98568.toml | 2 - .../fixtures/library_backup/package.toml | 6 +- .../authoring/backup_restore/test_restore.py | 87 ++++++++++++++++--- 16 files changed, 163 insertions(+), 85 deletions(-) diff --git a/openedx_learning/apps/authoring/backup_restore/api.py b/openedx_learning/apps/authoring/backup_restore/api.py index ba9ddd65b..df13c2072 100644 --- a/openedx_learning/apps/authoring/backup_restore/api.py +++ b/openedx_learning/apps/authoring/backup_restore/api.py @@ -17,9 +17,9 @@ def create_zip_file(lp_key: str, path: str) -> None: LearningPackageZipper(learning_package).create_zip(path) -def load_dump_zip_file(path: str) -> None: +def load_dump_zip_file(path: str) -> dict: """ Loads a zip file derived from create_zip_file """ with zipfile.ZipFile(path, "r") as zipf: - LearningPackageUnzipper(zipf).load() + return LearningPackageUnzipper(zipf).load() diff --git a/openedx_learning/apps/authoring/backup_restore/management/commands/lp_load.py b/openedx_learning/apps/authoring/backup_restore/management/commands/lp_load.py index 9699413d2..e6b0b0e71 100644 --- a/openedx_learning/apps/authoring/backup_restore/management/commands/lp_load.py +++ b/openedx_learning/apps/authoring/backup_restore/management/commands/lp_load.py @@ -18,14 +18,20 @@ class Command(BaseCommand): help = 'Load a learning package from a zip file.' def add_arguments(self, parser): - parser.add_argument('file_name', type=str, help='The name of the input zip file to load.') + parser.add_argument('file_name', type=str, help='The path of the input zip file to load.') def handle(self, *args, **options): file_name = options['file_name'] if not file_name.lower().endswith(".zip"): raise CommandError("Input file name must end with .zip") try: - load_dump_zip_file(file_name) + response = load_dump_zip_file(file_name) + if response["status"] == "error": + message = "Errors encountered during restore:\n" + log_buffer = response.get("log_file_error") + if log_buffer: + message += log_buffer.getvalue() + raise CommandError(message) message = f'{file_name} loaded successfully' self.stdout.write(self.style.SUCCESS(message)) except FileNotFoundError as exc: diff --git a/openedx_learning/apps/authoring/backup_restore/serializers.py b/openedx_learning/apps/authoring/backup_restore/serializers.py index 02fdc3201..fa2997a2f 100644 --- a/openedx_learning/apps/authoring/backup_restore/serializers.py +++ b/openedx_learning/apps/authoring/backup_restore/serializers.py @@ -8,6 +8,26 @@ from openedx_learning.apps.authoring.components import api as components_api +class LearningPackageSerializer(serializers.Serializer): # pylint: disable=abstract-method + """ + Serializer for learning packages. + """ + title = serializers.CharField(required=True) + key = serializers.CharField(required=True) + description = serializers.CharField(required=True, allow_blank=True) + created = serializers.DateTimeField(required=True, default_timezone=timezone.utc) + + +class LearningPackageMetadataSerializer(serializers.Serializer): # pylint: disable=abstract-method + """ + Serializer for learning package metadata. + """ + format_version = serializers.IntegerField(required=True) + created_by = serializers.CharField(required=False, allow_null=True) + created_at = serializers.DateTimeField(required=True, default_timezone=timezone.utc) + origin_server = serializers.CharField(required=False, allow_null=True) + + class EntitySerializer(serializers.Serializer): # pylint: disable=abstract-method """ Serializer for publishable entities. diff --git a/openedx_learning/apps/authoring/backup_restore/toml.py b/openedx_learning/apps/authoring/backup_restore/toml.py index 6742ab7f9..fc5c8240f 100644 --- a/openedx_learning/apps/authoring/backup_restore/toml.py +++ b/openedx_learning/apps/authoring/backup_restore/toml.py @@ -234,29 +234,15 @@ def parse_learning_package_toml(content: str) -> dict: Parse the learning package TOML content and return a dict of its fields. """ lp_data: Dict[str, Any] = tomlkit.parse(content) + return lp_data - # Validate the minimum required fields - if "learning_package" not in lp_data: - raise ValueError("Invalid learning package TOML: missing 'learning_package' section") - if "title" not in lp_data["learning_package"]: - raise ValueError("Invalid learning package TOML: missing 'title' in 'learning_package' section") - if "key" not in lp_data["learning_package"]: - raise ValueError("Invalid learning package TOML: missing 'key' in 'learning_package' section") - return lp_data["learning_package"] - -def parse_publishable_entity_toml(content: str) -> tuple[Dict[str, Any], list]: +def parse_publishable_entity_toml(content: str) -> dict: """ Parse the publishable entity TOML file and return a dict of its fields. """ pe_data: Dict[str, Any] = tomlkit.parse(content) - - # Validate the minimum required fields - if "entity" not in pe_data: - raise ValueError("Invalid publishable entity TOML: missing 'entity' section") - if "version" not in pe_data: - raise ValueError("Invalid publishable entity TOML: missing 'version' section") - return pe_data["entity"], pe_data.get("version", []) + return pe_data def parse_collection_toml(content: str) -> dict: @@ -264,6 +250,4 @@ def parse_collection_toml(content: str) -> dict: Parse the collection TOML content and return a dict of its fields. """ collection_data: Dict[str, Any] = tomlkit.parse(content) - if "collection" not in collection_data: - raise ValueError("Invalid collection TOML: missing 'collection' section") - return collection_data["collection"] + return collection_data diff --git a/openedx_learning/apps/authoring/backup_restore/zipper.py b/openedx_learning/apps/authoring/backup_restore/zipper.py index fb89f6ea1..343d699f5 100644 --- a/openedx_learning/apps/authoring/backup_restore/zipper.py +++ b/openedx_learning/apps/authoring/backup_restore/zipper.py @@ -31,6 +31,8 @@ ComponentVersionSerializer, ContainerSerializer, ContainerVersionSerializer, + LearningPackageMetadataSerializer, + LearningPackageSerializer, ) from openedx_learning.apps.authoring.backup_restore.toml import ( parse_collection_toml, @@ -440,6 +442,9 @@ def load(self) -> dict[str, Any]: # Step 2: Extract and validate learning package, entities and collections # Errors are collected and reported at the end # No saving to DB happens until all validations pass + learning_package_validated = self._extract_learning_package(organized_files["learning_package"]) + lp_metadata = learning_package_validated.pop("metadata", {}) + components_validated = self._extract_entities( organized_files["components"], ComponentSerializer, ComponentVersionSerializer ) @@ -462,9 +467,8 @@ def load(self) -> dict[str, Any]: # Step 3.2: Save everything to the DB # All validations passed, we can proceed to save everything # Save the learning package first to get its ID - learning_package = self._load_learning_package(organized_files["learning_package"]) - self._save( - learning_package, + learning_package = self._save( + learning_package_validated, components_validated, containers_validated, collections_validated, @@ -481,11 +485,10 @@ def load(self) -> dict[str, Any]: "general_info": { "learning_package_key": learning_package.key, "learning_package_title": learning_package.title, - "backed_up_at": learning_package.created, "containers": num_containers, "components": len(components_validated["components"]), "collections": len(collections_validated["collections"]), - "metadata": {}, + "metadata": lp_metadata, } } @@ -502,6 +505,28 @@ def preliminary_check(self) -> Tuple[list[dict[str, Any]], dict[str, Any]]: # Extract + Validate # -------------------------- + def _extract_learning_package(self, package_file: str) -> dict[str, Any]: + """Extract and validate the learning package TOML file.""" + toml_content_text = self._read_file_from_zip(package_file) + toml_content_dict = parse_learning_package_toml(toml_content_text) + lp = toml_content_dict.get("learning_package") + lp_metadata = toml_content_dict.get("meta") + + # Validate learning package data + lp_serializer = LearningPackageSerializer(data=lp) + if not lp_serializer.is_valid(): + self.errors.append({"file": f"{package_file} learning package section", "errors": lp_serializer.errors}) + + # Validate metadata if present + lp_metadata_serializer = LearningPackageMetadataSerializer(data=lp_metadata) + if not lp_metadata_serializer.is_valid(): + self.errors.append({"file": f"{package_file} meta section", "errors": lp_metadata_serializer.errors}) + + lp_validated = lp_serializer.validated_data if lp_serializer.is_valid() else {} + lp_metadata = lp_metadata_serializer.validated_data if lp_metadata_serializer.is_valid() else {} + lp_validated["metadata"] = lp_metadata + return lp_validated + def _extract_entities( self, entity_files: list[str], @@ -557,9 +582,10 @@ def _extract_collections( continue toml_content = self._read_file_from_zip(file) collection_data = parse_collection_toml(toml_content) + collection_data = collection_data.get("collection", {}) serializer = CollectionSerializer(data={"created_by": None, **collection_data}) if not serializer.is_valid(): - self.errors.append({"file": file, "errors": serializer.errors}) + self.errors.append({"file": f"{file} collection section", "errors": serializer.errors}) continue collection_validated = serializer.validated_data entities_list = collection_validated["entities"] @@ -579,26 +605,30 @@ def _extract_collections( def _save( self, - learning_package: LearningPackage, + learning_package: dict[str, Any], components: dict[str, Any], containers: dict[str, Any], collections: dict[str, Any], *, component_static_files: dict[str, List[str]] - ) -> None: + ) -> LearningPackage: """Persist all validated entities in two phases: published then drafts.""" - with publishing_api.bulk_draft_changes_for(learning_package.id): - self._save_components(learning_package, components, component_static_files) - self._save_units(learning_package, containers) - self._save_subsections(learning_package, containers) - self._save_sections(learning_package, containers) - self._save_collections(learning_package, collections) - publishing_api.publish_all_drafts(learning_package.id) + learning_package_obj = publishing_api.create_learning_package(**learning_package) + + with publishing_api.bulk_draft_changes_for(learning_package_obj.id): + self._save_components(learning_package_obj, components, component_static_files) + self._save_units(learning_package_obj, containers) + self._save_subsections(learning_package_obj, containers) + self._save_sections(learning_package_obj, containers) + self._save_collections(learning_package_obj, collections) + publishing_api.publish_all_drafts(learning_package_obj.id) - with publishing_api.bulk_draft_changes_for(learning_package.id): + with publishing_api.bulk_draft_changes_for(learning_package_obj.id): self._save_draft_versions(components, containers, component_static_files) + return learning_package_obj + def _save_collections(self, learning_package, collections): """Save collections and their entities.""" for valid_collection in collections.get("collections", []): @@ -758,22 +788,14 @@ def _resolve_children(self, entity_data: dict[str, Any], lookup_map: dict[str, A children_keys = entity_data.pop("children", []) return [lookup_map[key] for key in children_keys if key in lookup_map] - def _load_learning_package(self, package_file: str) -> LearningPackage: - """Load and persist the learning package TOML file.""" - toml_content = self._read_file_from_zip(package_file) - data = parse_learning_package_toml(toml_content) - return publishing_api.create_learning_package( - key=data["key"], - title=data["title"], - description=data["description"], - ) - def _load_entity_data( self, entity_file: str ) -> tuple[dict[str, Any], dict[str, Any] | None, dict[str, Any] | None]: """Load entity data and its versions from TOML.""" - content = self._read_file_from_zip(entity_file) - entity_data, version_data = parse_publishable_entity_toml(content) + entity_toml_txt = self._read_file_from_zip(entity_file) + entity_toml_dict = parse_publishable_entity_toml(entity_toml_txt) + entity_data = entity_toml_dict.get("entity", {}) + version_data = entity_toml_dict.get("version", []) return entity_data, *self._get_versions_to_write(version_data, entity_data) def _validate_versions(self, entity_data, draft, published, serializer_cls, *, file) -> dict[str, Any]: diff --git a/tests/openedx_learning/apps/authoring/backup_restore/fixtures/library_backup/entities/section1-8ca126.toml b/tests/openedx_learning/apps/authoring/backup_restore/fixtures/library_backup/entities/section1-8ca126.toml index 719e66595..00c34d430 100644 --- a/tests/openedx_learning/apps/authoring/backup_restore/fixtures/library_backup/entities/section1-8ca126.toml +++ b/tests/openedx_learning/apps/authoring/backup_restore/fixtures/library_backup/entities/section1-8ca126.toml @@ -1,5 +1,4 @@ [entity] -uuid = "0df28dac-aa13-473c-8c41-2f27e10362da" can_stand_alone = true key = "section1-8ca126" created = 2025-09-04T22:51:40.919872Z @@ -16,7 +15,6 @@ version_num = 2 [[version]] title = "Section1" -uuid = "60756ba1-231a-4014-9602-1fba48678674" version_num = 2 [version.container] diff --git a/tests/openedx_learning/apps/authoring/backup_restore/fixtures/library_backup/entities/subsection1-48afa3.toml b/tests/openedx_learning/apps/authoring/backup_restore/fixtures/library_backup/entities/subsection1-48afa3.toml index d94a59912..b90154307 100644 --- a/tests/openedx_learning/apps/authoring/backup_restore/fixtures/library_backup/entities/subsection1-48afa3.toml +++ b/tests/openedx_learning/apps/authoring/backup_restore/fixtures/library_backup/entities/subsection1-48afa3.toml @@ -1,5 +1,4 @@ [entity] -uuid = "0b52444b-0604-4510-be71-960251e7343d" can_stand_alone = true key = "subsection1-48afa3" created = 2025-09-04T22:51:52.824941Z @@ -16,7 +15,6 @@ version_num = 2 [[version]] title = "Subsection1" -uuid = "46642bce-0861-4e22-8b37-3cd897c99239" version_num = 2 [version.container] diff --git a/tests/openedx_learning/apps/authoring/backup_restore/fixtures/library_backup/entities/unit1-b7eafb.toml b/tests/openedx_learning/apps/authoring/backup_restore/fixtures/library_backup/entities/unit1-b7eafb.toml index c99aac310..80a932364 100644 --- a/tests/openedx_learning/apps/authoring/backup_restore/fixtures/library_backup/entities/unit1-b7eafb.toml +++ b/tests/openedx_learning/apps/authoring/backup_restore/fixtures/library_backup/entities/unit1-b7eafb.toml @@ -1,5 +1,4 @@ [entity] -uuid = "c1a69936-ac68-4e1d-843a-0708fa7c755b" can_stand_alone = true key = "unit1-b7eafb" created = 2025-09-04T22:51:59.271334Z @@ -16,7 +15,6 @@ version_num = 2 [[version]] title = "Unit1" -uuid = "f8aa4b20-8271-4838-bbe6-e0164d616c13" version_num = 2 [version.container] diff --git a/tests/openedx_learning/apps/authoring/backup_restore/fixtures/library_backup/entities/xblock.v1/drag-and-drop-v2/4d1b2fac-8b30-42fb-872d-6b10ab580b27.toml b/tests/openedx_learning/apps/authoring/backup_restore/fixtures/library_backup/entities/xblock.v1/drag-and-drop-v2/4d1b2fac-8b30-42fb-872d-6b10ab580b27.toml index 0612fbf36..5f6656052 100644 --- a/tests/openedx_learning/apps/authoring/backup_restore/fixtures/library_backup/entities/xblock.v1/drag-and-drop-v2/4d1b2fac-8b30-42fb-872d-6b10ab580b27.toml +++ b/tests/openedx_learning/apps/authoring/backup_restore/fixtures/library_backup/entities/xblock.v1/drag-and-drop-v2/4d1b2fac-8b30-42fb-872d-6b10ab580b27.toml @@ -1,5 +1,4 @@ [entity] -uuid = "a919094d-1fd0-40bb-9fe2-66284a80effb" can_stand_alone = true key = "xblock.v1:drag-and-drop-v2:4d1b2fac-8b30-42fb-872d-6b10ab580b27" created = 2025-09-04T22:39:37.001432Z @@ -14,5 +13,4 @@ version_num = 2 [[version]] title = "Drag and Drop" -uuid = "136f72d0-a793-4993-b0b1-8515e0c21e64" version_num = 2 diff --git a/tests/openedx_learning/apps/authoring/backup_restore/fixtures/library_backup/entities/xblock.v1/html/e32d5479-9492-41f6-9222-550a7346bc37.toml b/tests/openedx_learning/apps/authoring/backup_restore/fixtures/library_backup/entities/xblock.v1/html/e32d5479-9492-41f6-9222-550a7346bc37.toml index 733cc6471..9cc0044e2 100644 --- a/tests/openedx_learning/apps/authoring/backup_restore/fixtures/library_backup/entities/xblock.v1/html/e32d5479-9492-41f6-9222-550a7346bc37.toml +++ b/tests/openedx_learning/apps/authoring/backup_restore/fixtures/library_backup/entities/xblock.v1/html/e32d5479-9492-41f6-9222-550a7346bc37.toml @@ -1,5 +1,4 @@ [entity] -uuid = "1e5e93ef-c34b-4061-aae2-fbca91db6bcc" can_stand_alone = true key = "xblock.v1:html:e32d5479-9492-41f6-9222-550a7346bc37" created = 2025-08-19T04:25:43.685529Z @@ -14,10 +13,8 @@ version_num = 4 [[version]] title = "Text" -uuid = "23dec9b0-0b2c-4706-9288-6ef4f3c888ea" version_num = 5 [[version]] title = "Text" -uuid = "f6ccd316-d603-45ef-ab96-86cf9d3645ae" version_num = 4 diff --git a/tests/openedx_learning/apps/authoring/backup_restore/fixtures/library_backup/entities/xblock.v1/openassessment/1ee38208-a585-4455-a27e-4930aa541f53.toml b/tests/openedx_learning/apps/authoring/backup_restore/fixtures/library_backup/entities/xblock.v1/openassessment/1ee38208-a585-4455-a27e-4930aa541f53.toml index a480f3eb0..134fb82f5 100644 --- a/tests/openedx_learning/apps/authoring/backup_restore/fixtures/library_backup/entities/xblock.v1/openassessment/1ee38208-a585-4455-a27e-4930aa541f53.toml +++ b/tests/openedx_learning/apps/authoring/backup_restore/fixtures/library_backup/entities/xblock.v1/openassessment/1ee38208-a585-4455-a27e-4930aa541f53.toml @@ -1,5 +1,4 @@ [entity] -uuid = "50dfd1e2-dac7-46b7-94b3-8a36c39cd8a1" can_stand_alone = true key = "xblock.v1:openassessment:1ee38208-a585-4455-a27e-4930aa541f53" created = 2025-09-04T22:38:05.382684Z @@ -14,5 +13,4 @@ version_num = 2 [[version]] title = "Open Response Assessment" -uuid = "8f3c5e83-c1bf-49ba-96dc-5ee8e8b79b83" version_num = 2 diff --git a/tests/openedx_learning/apps/authoring/backup_restore/fixtures/library_backup/entities/xblock.v1/problem/256739e8-c2df-4ced-bd10-8156f6cfa90b.toml b/tests/openedx_learning/apps/authoring/backup_restore/fixtures/library_backup/entities/xblock.v1/problem/256739e8-c2df-4ced-bd10-8156f6cfa90b.toml index ced2b0679..db590484a 100644 --- a/tests/openedx_learning/apps/authoring/backup_restore/fixtures/library_backup/entities/xblock.v1/problem/256739e8-c2df-4ced-bd10-8156f6cfa90b.toml +++ b/tests/openedx_learning/apps/authoring/backup_restore/fixtures/library_backup/entities/xblock.v1/problem/256739e8-c2df-4ced-bd10-8156f6cfa90b.toml @@ -1,5 +1,4 @@ [entity] -uuid = "c5240e01-0810-4904-bd8e-5ac0d3d8370c" can_stand_alone = true key = "xblock.v1:problem:256739e8-c2df-4ced-bd10-8156f6cfa90b" created = 2025-09-04T22:37:24.780718Z @@ -14,5 +13,4 @@ version_num = 2 [[version]] title = "Single select" -uuid = "a5bcdf80-f69b-4ee5-a980-e1a93c61ad03" version_num = 2 diff --git a/tests/openedx_learning/apps/authoring/backup_restore/fixtures/library_backup/entities/xblock.v1/survey/6681da3f-b056-4c6e-a8f9-040967907471.toml b/tests/openedx_learning/apps/authoring/backup_restore/fixtures/library_backup/entities/xblock.v1/survey/6681da3f-b056-4c6e-a8f9-040967907471.toml index 24dd6f777..cf25e2de8 100644 --- a/tests/openedx_learning/apps/authoring/backup_restore/fixtures/library_backup/entities/xblock.v1/survey/6681da3f-b056-4c6e-a8f9-040967907471.toml +++ b/tests/openedx_learning/apps/authoring/backup_restore/fixtures/library_backup/entities/xblock.v1/survey/6681da3f-b056-4c6e-a8f9-040967907471.toml @@ -1,5 +1,4 @@ [entity] -uuid = "3c41aa22-998e-4161-bec8-8c28ccf4f006" can_stand_alone = true key = "xblock.v1:survey:6681da3f-b056-4c6e-a8f9-040967907471" created = 2025-09-04T22:41:02.788509Z @@ -14,5 +13,4 @@ version_num = 1 [[version]] title = "Survey" -uuid = "0bdcaf29-ca6b-47f2-8b16-86a900d1c837" version_num = 1 diff --git a/tests/openedx_learning/apps/authoring/backup_restore/fixtures/library_backup/entities/xblock.v1/video/22601ebd-9da8-430b-9778-cfe059a98568.toml b/tests/openedx_learning/apps/authoring/backup_restore/fixtures/library_backup/entities/xblock.v1/video/22601ebd-9da8-430b-9778-cfe059a98568.toml index 6258619df..5bec87789 100644 --- a/tests/openedx_learning/apps/authoring/backup_restore/fixtures/library_backup/entities/xblock.v1/video/22601ebd-9da8-430b-9778-cfe059a98568.toml +++ b/tests/openedx_learning/apps/authoring/backup_restore/fixtures/library_backup/entities/xblock.v1/video/22601ebd-9da8-430b-9778-cfe059a98568.toml @@ -1,5 +1,4 @@ [entity] -uuid = "5e8d8ec3-5adf-4a09-a81e-b4a86f78cbe6" can_stand_alone = true key = "xblock.v1:video:22601ebd-9da8-430b-9778-cfe059a98568" created = 2025-09-04T22:40:23.526816Z @@ -14,5 +13,4 @@ version_num = 3 [[version]] title = "Video" -uuid = "fcd566b9-5cb4-4228-85b6-ce2b1a0bafff" version_num = 3 diff --git a/tests/openedx_learning/apps/authoring/backup_restore/fixtures/library_backup/package.toml b/tests/openedx_learning/apps/authoring/backup_restore/fixtures/library_backup/package.toml index baffe18fc..7ddae749b 100644 --- a/tests/openedx_learning/apps/authoring/backup_restore/fixtures/library_backup/package.toml +++ b/tests/openedx_learning/apps/authoring/backup_restore/fixtures/library_backup/package.toml @@ -1,4 +1,8 @@ -# Datetime of the export: 2025-09-24 18:00:38.451670+00:00 +[meta] +format_version = 1 +created_by = "dormsbee" +created_at = 2025-10-05T18:23:45.180535Z +origin_server = "cms.test" [learning_package] title = "Library test" diff --git a/tests/openedx_learning/apps/authoring/backup_restore/test_restore.py b/tests/openedx_learning/apps/authoring/backup_restore/test_restore.py index ffc72c178..9d0a71682 100644 --- a/tests/openedx_learning/apps/authoring/backup_restore/test_restore.py +++ b/tests/openedx_learning/apps/authoring/backup_restore/test_restore.py @@ -1,5 +1,6 @@ """Tests for the lp_load management command.""" import os +from datetime import datetime, timezone from io import StringIO from unittest.mock import patch @@ -156,26 +157,29 @@ def test_successful_restore_with_no_command_line(self): "general_info": { "learning_package_key": "lib:WGU:LIB_C001", "learning_package_title": "Library test", - "backed_up_at": None, "containers": 3, "components": 6, "collections": 1, - "metadata": {}, + "metadata": { + "format_version": 1, + "created_by": "dormsbee", + "created_at": datetime(2025, 10, 5, 18, 23, 45, 180535, tzinfo=timezone.utc), + "origin_server": "cms.test", + }, }, } + # Compare dicts except for dynamic fields assert result["status"] == expected["status"] - assert result["log_file_error"] == expected["log_file_error"] - assert ( - result["general_info"]["learning_package_key"] == expected["general_info"]["learning_package_key"] - ) - assert ( - result["general_info"]["learning_package_title"] == expected["general_info"]["learning_package_title"] - ) - assert result["general_info"]["containers"] == expected["general_info"]["containers"] - assert result["general_info"]["components"] == expected["general_info"]["components"] - assert result["general_info"]["collections"] == expected["general_info"]["collections"] - assert result["general_info"]["metadata"] == expected["general_info"]["metadata"] + assert result["log_file_error"] is None + + general_info = result["general_info"] + expected_info = expected["general_info"] + metadata_general_info = general_info.pop("metadata", None) + metadata_expected_info = expected_info.pop("metadata", None) + + assert general_info == expected_info, f"General info does not match. Got {general_info}" + assert metadata_general_info == metadata_expected_info, f"Meta info does not match. Got {metadata_general_info}" lp = publishing_api.LearningPackage.objects.filter(key="lib:WGU:LIB_C001").first() assert lp is not None, "Learning package was not restored." @@ -201,3 +205,60 @@ def test_error_preliminary_check(self): assert len(errors) == 1 assert errors[0]["file"] == "package.toml" assert errors[0]["errors"] == "Missing learning package file." + + def test_error_learning_package_missing_key(self): + """Test restoring a learning package with a learning_package.toml missing the 'key' field.""" + zip_file = folder_to_inmemory_zip(os.path.join(os.path.dirname(__file__), "fixtures/library_backup")) + + # Mock parse_learning_package_toml to return a dict without 'key' + with patch( + "openedx_learning.apps.authoring.backup_restore.zipper.parse_learning_package_toml", + return_value={ + "learning_package": { + "title": "Library test", + "description": "", + "created": "2025-09-03T17:50:59.536190Z", + "updated": "2025-09-03T17:50:59.536190Z", + }, + "meta": { + "format_version": 1, + "created_by": "dormsbee", + "created_at": "2025-09-03T17:50:59.536190Z", + "origin_server": "cms.test", + }, + }, + ): + result = LearningPackageUnzipper(zip_file).load() + + assert result["status"] == "error" + assert result["general_info"] is None + assert result["log_file_error"] is not None + log_content = result["log_file_error"].getvalue() + expected_error = "Errors encountered during restore:\npackage.toml learning package section: {'key':" + assert expected_error in log_content + + def test_error_no_metadata_section(self): + """Test restoring a learning package with a learning_package.toml missing the 'meta' section.""" + zip_file = folder_to_inmemory_zip(os.path.join(os.path.dirname(__file__), "fixtures/library_backup")) + + # Mock parse_learning_package_toml to return a dict without 'meta' + with patch( + "openedx_learning.apps.authoring.backup_restore.zipper.parse_learning_package_toml", + return_value={ + "learning_package": { + "title": "Library test", + "key": "lib:WGU:LIB_C001", + "description": "", + "created": "2025-09-03T17:50:59.536190Z", + "updated": "2025-09-03T17:50:59.536190Z", + } + }, + ): + result = LearningPackageUnzipper(zip_file).load() + + assert result["status"] == "error" + assert result["general_info"] is None + assert result["log_file_error"] is not None + log_content = result["log_file_error"].getvalue() + expected_error = "Errors encountered during restore:\npackage.toml meta section: {'non_field_errors': [Er" + assert expected_error in log_content From 77cf6beaedbfabe9269d1e015724681c5f09ae45 Mon Sep 17 00:00:00 2001 From: Daniel Wong Date: Mon, 6 Oct 2025 17:50:40 -0600 Subject: [PATCH 5/9] fix: handle case when draft and published versions are the same --- .../management/commands/lp_dump.py | 5 +- .../management/commands/lp_load.py | 5 +- .../apps/authoring/backup_restore/zipper.py | 63 ++++++++++++++++++- .../c22b9f97-f1e9-4e8f-87f0-d5a3c26083e2.toml | 16 +++++ .../component_versions/v2/block.xml | 1 + .../authoring/backup_restore/test_restore.py | 8 ++- 6 files changed, 92 insertions(+), 6 deletions(-) create mode 100644 tests/openedx_learning/apps/authoring/backup_restore/fixtures/library_backup/entities/xblock.v1/html/c22b9f97-f1e9-4e8f-87f0-d5a3c26083e2.toml create mode 100644 tests/openedx_learning/apps/authoring/backup_restore/fixtures/library_backup/entities/xblock.v1/html/c22b9f97-f1e9-4e8f-87f0-d5a3c26083e2/component_versions/v2/block.xml diff --git a/openedx_learning/apps/authoring/backup_restore/management/commands/lp_dump.py b/openedx_learning/apps/authoring/backup_restore/management/commands/lp_dump.py index aed626b56..94b16be62 100644 --- a/openedx_learning/apps/authoring/backup_restore/management/commands/lp_dump.py +++ b/openedx_learning/apps/authoring/backup_restore/management/commands/lp_dump.py @@ -2,6 +2,7 @@ Django management commands to handle backup learning packages (WIP) """ import logging +import time from django.core.management import CommandError from django.core.management.base import BaseCommand @@ -28,8 +29,10 @@ def handle(self, *args, **options): if not file_name.lower().endswith(".zip"): raise CommandError("Output file name must end with .zip") try: + start_time = time.time() create_zip_file(lp_key, file_name) - message = f'{lp_key} written to {file_name}' + elapsed = time.time() - start_time + message = f'{lp_key} written to {file_name} (create_zip_file: {elapsed:.2f} seconds)' self.stdout.write(self.style.SUCCESS(message)) except LearningPackage.DoesNotExist as exc: message = f"Learning package with key {lp_key} not found" diff --git a/openedx_learning/apps/authoring/backup_restore/management/commands/lp_load.py b/openedx_learning/apps/authoring/backup_restore/management/commands/lp_load.py index e6b0b0e71..60b0632f0 100644 --- a/openedx_learning/apps/authoring/backup_restore/management/commands/lp_load.py +++ b/openedx_learning/apps/authoring/backup_restore/management/commands/lp_load.py @@ -2,6 +2,7 @@ Django management commands to handle restore learning packages (WIP) """ import logging +import time from django.core.management import CommandError from django.core.management.base import BaseCommand @@ -25,14 +26,16 @@ def handle(self, *args, **options): if not file_name.lower().endswith(".zip"): raise CommandError("Input file name must end with .zip") try: + start_time = time.time() response = load_dump_zip_file(file_name) + duration = time.time() - start_time if response["status"] == "error": message = "Errors encountered during restore:\n" log_buffer = response.get("log_file_error") if log_buffer: message += log_buffer.getvalue() raise CommandError(message) - message = f'{file_name} loaded successfully' + message = f'{file_name} loaded successfully (duration: {duration:.2f} seconds)' self.stdout.write(self.style.SUCCESS(message)) except FileNotFoundError as exc: message = f"Learning package file {file_name} not found: {exc}" diff --git a/openedx_learning/apps/authoring/backup_restore/zipper.py b/openedx_learning/apps/authoring/backup_restore/zipper.py index 343d699f5..9f9f3371a 100644 --- a/openedx_learning/apps/authoring/backup_restore/zipper.py +++ b/openedx_learning/apps/authoring/backup_restore/zipper.py @@ -420,6 +420,7 @@ def __init__(self, zipf: zipfile.ZipFile) -> None: self.subsections_map_by_key: dict[str, Any] = {} self.sections_map_by_key: dict[str, Any] = {} self.all_publishable_entities_keys: set[str] = set() + self.all_published_entities_versions: set[str] = set() # To track published entity versions # -------------------------- # Public API @@ -651,6 +652,9 @@ def _save_components(self, learning_package, components, component_static_files) entity_key = valid_published.pop("entity_key") version_num = valid_published["version_num"] # Should exist, validated earlier content_to_replace = self._resolve_static_files(version_num, entity_key, component_static_files) + self.all_published_entities_versions.add( + f"{entity_key}__v{version_num}" + ) # Track published version components_api.create_next_component_version( self.components_map_by_key[entity_key].publishable_entity.id, content_to_replace=content_to_replace, @@ -668,8 +672,14 @@ def _save_units(self, learning_package, containers): for valid_published in containers.get("unit_published", []): entity_key = valid_published.pop("entity_key") children = self._resolve_children(valid_published, self.components_map_by_key) + self.all_published_entities_versions.add( + f"{entity_key}__v{valid_published.get('version_num')}" + ) # Track published version units_api.create_next_unit_version( - self.units_map_by_key[entity_key], components=children, **valid_published + self.units_map_by_key[entity_key], + force_version_num=valid_published.pop("version_num", None), + components=children, + **valid_published ) def _save_subsections(self, learning_package, containers): @@ -682,8 +692,14 @@ def _save_subsections(self, learning_package, containers): for valid_published in containers.get("subsection_published", []): entity_key = valid_published.pop("entity_key") children = self._resolve_children(valid_published, self.units_map_by_key) + self.all_published_entities_versions.add( + f"{entity_key}__v{valid_published.get('version_num')}" + ) # Track published version subsections_api.create_next_subsection_version( - self.subsections_map_by_key[entity_key], units=children, **valid_published + self.subsections_map_by_key[entity_key], + units=children, + force_version_num=valid_published.pop("version_num", None), + **valid_published ) def _save_sections(self, learning_package, containers): @@ -696,8 +712,14 @@ def _save_sections(self, learning_package, containers): for valid_published in containers.get("section_published", []): entity_key = valid_published.pop("entity_key") children = self._resolve_children(valid_published, self.subsections_map_by_key) + self.all_published_entities_versions.add( + f"{entity_key}__v{valid_published.get('version_num')}" + ) # Track published version sections_api.create_next_section_version( - self.sections_map_by_key[entity_key], subsections=children, **valid_published + self.sections_map_by_key[entity_key], + subsections=children, + force_version_num=valid_published.pop("version_num", None), + **valid_published ) def _save_draft_versions(self, components, containers, component_static_files): @@ -705,6 +727,14 @@ def _save_draft_versions(self, components, containers, component_static_files): for valid_draft in components.get("components_drafts", []): entity_key = valid_draft.pop("entity_key") version_num = valid_draft["version_num"] # Should exist, validated earlier + entity_version_identifier = f"{entity_key}__v{version_num}" + if entity_version_identifier in self.all_published_entities_versions: + # Skip creating draft if this version is already published + # Why? Because the version itself is already created and + # we don't want to create duplicate versions. + # Otherwise, we will raise an IntegrityError on PublishableEntityVersion + # due to unique constraints between publishable_entity and version_num. + continue content_to_replace = self._resolve_static_files(version_num, entity_key, component_static_files) components_api.create_next_component_version( self.components_map_by_key[entity_key].publishable_entity.id, @@ -718,6 +748,15 @@ def _save_draft_versions(self, components, containers, component_static_files): for valid_draft in containers.get("unit_drafts", []): entity_key = valid_draft.pop("entity_key") + version_num = valid_draft["version_num"] # Should exist, validated earlier + entity_version_identifier = f"{entity_key}__v{version_num}" + if entity_version_identifier in self.all_published_entities_versions: + # Skip creating draft if this version is already published + # Why? Because the version itself is already created and + # we don't want to create duplicate versions. + # Otherwise, we will raise an IntegrityError on PublishableEntityVersion + # due to unique constraints between publishable_entity and version_num. + continue children = self._resolve_children(valid_draft, self.components_map_by_key) units_api.create_next_unit_version( self.units_map_by_key[entity_key], @@ -728,6 +767,15 @@ def _save_draft_versions(self, components, containers, component_static_files): for valid_draft in containers.get("subsection_drafts", []): entity_key = valid_draft.pop("entity_key") + version_num = valid_draft["version_num"] # Should exist, validated earlier + entity_version_identifier = f"{entity_key}__v{version_num}" + if entity_version_identifier in self.all_published_entities_versions: + # Skip creating draft if this version is already published + # Why? Because the version itself is already created and + # we don't want to create duplicate versions. + # Otherwise, we will raise an IntegrityError on PublishableEntityVersion + # due to unique constraints between publishable_entity and version_num. + continue children = self._resolve_children(valid_draft, self.units_map_by_key) subsections_api.create_next_subsection_version( self.subsections_map_by_key[entity_key], @@ -738,6 +786,15 @@ def _save_draft_versions(self, components, containers, component_static_files): for valid_draft in containers.get("section_drafts", []): entity_key = valid_draft.pop("entity_key") + version_num = valid_draft["version_num"] # Should exist, validated earlier + entity_version_identifier = f"{entity_key}__v{version_num}" + if entity_version_identifier in self.all_published_entities_versions: + # Skip creating draft if this version is already published + # Why? Because the version itself is already created and + # we don't want to create duplicate versions. + # Otherwise, we will raise an IntegrityError on PublishableEntityVersion + # due to unique constraints between publishable_entity and version_num. + continue children = self._resolve_children(valid_draft, self.subsections_map_by_key) sections_api.create_next_section_version( self.sections_map_by_key[entity_key], diff --git a/tests/openedx_learning/apps/authoring/backup_restore/fixtures/library_backup/entities/xblock.v1/html/c22b9f97-f1e9-4e8f-87f0-d5a3c26083e2.toml b/tests/openedx_learning/apps/authoring/backup_restore/fixtures/library_backup/entities/xblock.v1/html/c22b9f97-f1e9-4e8f-87f0-d5a3c26083e2.toml new file mode 100644 index 000000000..ca759834e --- /dev/null +++ b/tests/openedx_learning/apps/authoring/backup_restore/fixtures/library_backup/entities/xblock.v1/html/c22b9f97-f1e9-4e8f-87f0-d5a3c26083e2.toml @@ -0,0 +1,16 @@ +[entity] +can_stand_alone = true +key = "xblock.v1:html:c22b9f97-f1e9-4e8f-87f0-d5a3c26083e2" +created = 2025-10-06T16:59:34.160314Z + +[entity.draft] +version_num = 2 + +[entity.published] +version_num = 2 + +# ### Versions + +[[version]] +title = "Text" +version_num = 2 diff --git a/tests/openedx_learning/apps/authoring/backup_restore/fixtures/library_backup/entities/xblock.v1/html/c22b9f97-f1e9-4e8f-87f0-d5a3c26083e2/component_versions/v2/block.xml b/tests/openedx_learning/apps/authoring/backup_restore/fixtures/library_backup/entities/xblock.v1/html/c22b9f97-f1e9-4e8f-87f0-d5a3c26083e2/component_versions/v2/block.xml new file mode 100644 index 000000000..8c419ad0b --- /dev/null +++ b/tests/openedx_learning/apps/authoring/backup_restore/fixtures/library_backup/entities/xblock.v1/html/c22b9f97-f1e9-4e8f-87f0-d5a3c26083e2/component_versions/v2/block.xml @@ -0,0 +1 @@ +Hi

]]> diff --git a/tests/openedx_learning/apps/authoring/backup_restore/test_restore.py b/tests/openedx_learning/apps/authoring/backup_restore/test_restore.py index 9d0a71682..1a0a09cb5 100644 --- a/tests/openedx_learning/apps/authoring/backup_restore/test_restore.py +++ b/tests/openedx_learning/apps/authoring/backup_restore/test_restore.py @@ -82,6 +82,7 @@ def verify_components(self, lp): "xblock.v1:problem:256739e8-c2df-4ced-bd10-8156f6cfa90b", "xblock.v1:survey:6681da3f-b056-4c6e-a8f9-040967907471", "xblock.v1:video:22601ebd-9da8-430b-9778-cfe059a98568", + "xblock.v1:html:c22b9f97-f1e9-4e8f-87f0-d5a3c26083e2" ] for component in component_qs: assert component.key in expected_component_keys @@ -124,6 +125,11 @@ def verify_components(self, lp): assert draft_version is not None assert draft_version.version_num == 3 assert published_version is None + elif component.key == "xblock.v1:html:c22b9f97-f1e9-4e8f-87f0-d5a3c26083e2": + assert draft_version is not None + assert draft_version.version_num == 2 + assert published_version is not None + assert published_version.version_num == 2 else: assert False, f"Unexpected component key: {component.key}" @@ -158,7 +164,7 @@ def test_successful_restore_with_no_command_line(self): "learning_package_key": "lib:WGU:LIB_C001", "learning_package_title": "Library test", "containers": 3, - "components": 6, + "components": 7, "collections": 1, "metadata": { "format_version": 1, From 09311300c2d555efaa598daf6e8d5e3bec7c40a7 Mon Sep 17 00:00:00 2001 From: Daniel Wong Date: Fri, 10 Oct 2025 10:59:22 -0600 Subject: [PATCH 6/9] feat: introduce dataclass for load result and refactor version check - Added dataclass to represent load process result - Improved docstrings - Simplified _is_version_already_exists logic --- .../apps/authoring/backup_restore/api.py | 7 +- .../management/commands/lp_load.py | 8 +- .../authoring/backup_restore/serializers.py | 8 + .../apps/authoring/backup_restore/zipper.py | 166 +++++++++++------- .../authoring/backup_restore/test_restore.py | 49 +++--- 5 files changed, 142 insertions(+), 96 deletions(-) diff --git a/openedx_learning/apps/authoring/backup_restore/api.py b/openedx_learning/apps/authoring/backup_restore/api.py index df13c2072..e6f478a33 100644 --- a/openedx_learning/apps/authoring/backup_restore/api.py +++ b/openedx_learning/apps/authoring/backup_restore/api.py @@ -10,6 +10,7 @@ def create_zip_file(lp_key: str, path: str) -> None: """ Creates a dump zip file for the given learning package key at the given path. + The zip file contains a TOML representation of the learning package and its contents. Can throw a NotFoundError at get_learning_package_by_key """ @@ -17,9 +18,11 @@ def create_zip_file(lp_key: str, path: str) -> None: LearningPackageZipper(learning_package).create_zip(path) -def load_dump_zip_file(path: str) -> dict: +def load_library_from_zip(path: str) -> dict: """ - Loads a zip file derived from create_zip_file + Loads a learning package from a zip file at the given path. + Restores the learning package and its contents to the database. + Returns a dictionary with the status of the operation and any errors encountered. """ with zipfile.ZipFile(path, "r") as zipf: return LearningPackageUnzipper(zipf).load() diff --git a/openedx_learning/apps/authoring/backup_restore/management/commands/lp_load.py b/openedx_learning/apps/authoring/backup_restore/management/commands/lp_load.py index 60b0632f0..a1eaecb58 100644 --- a/openedx_learning/apps/authoring/backup_restore/management/commands/lp_load.py +++ b/openedx_learning/apps/authoring/backup_restore/management/commands/lp_load.py @@ -7,7 +7,7 @@ from django.core.management import CommandError from django.core.management.base import BaseCommand -from openedx_learning.apps.authoring.backup_restore.api import load_dump_zip_file +from openedx_learning.apps.authoring.backup_restore.api import load_library_from_zip logger = logging.getLogger(__name__) @@ -27,11 +27,11 @@ def handle(self, *args, **options): raise CommandError("Input file name must end with .zip") try: start_time = time.time() - response = load_dump_zip_file(file_name) + result = load_library_from_zip(file_name) duration = time.time() - start_time - if response["status"] == "error": + if result["status"] == "error": message = "Errors encountered during restore:\n" - log_buffer = response.get("log_file_error") + log_buffer = result.get("log_file_error") if log_buffer: message += log_buffer.getvalue() raise CommandError(message) diff --git a/openedx_learning/apps/authoring/backup_restore/serializers.py b/openedx_learning/apps/authoring/backup_restore/serializers.py index fa2997a2f..eb9ca736b 100644 --- a/openedx_learning/apps/authoring/backup_restore/serializers.py +++ b/openedx_learning/apps/authoring/backup_restore/serializers.py @@ -11,6 +11,10 @@ class LearningPackageSerializer(serializers.Serializer): # pylint: disable=abstract-method """ Serializer for learning packages. + + Note: + The `key` field is serialized, but it is generally not trustworthy for restoration. + During restore, a new key may be generated or overridden. """ title = serializers.CharField(required=True) key = serializers.CharField(required=True) @@ -21,6 +25,10 @@ class LearningPackageSerializer(serializers.Serializer): # pylint: disable=abst class LearningPackageMetadataSerializer(serializers.Serializer): # pylint: disable=abstract-method """ Serializer for learning package metadata. + + Note: + This serializer handles data exported to an archive (e.g., during backup), + but the metadata is not restored to the database and is meant solely for inspection. """ format_version = serializers.IntegerField(required=True) created_by = serializers.CharField(required=False, allow_null=True) diff --git a/openedx_learning/apps/authoring/backup_restore/zipper.py b/openedx_learning/apps/authoring/backup_restore/zipper.py index 9f9f3371a..76102e8eb 100644 --- a/openedx_learning/apps/authoring/backup_restore/zipper.py +++ b/openedx_learning/apps/authoring/backup_restore/zipper.py @@ -5,10 +5,11 @@ import hashlib import zipfile from collections import defaultdict +from dataclasses import asdict, dataclass from datetime import datetime, timezone from io import StringIO from pathlib import Path -from typing import Any, List, Optional, Tuple, TypedDict +from typing import Any, List, Literal, Optional, Tuple from django.db import transaction from django.db.models import Prefetch, QuerySet @@ -52,12 +53,6 @@ TOML_PACKAGE_NAME = "package.toml" -class ComponentDefaults(TypedDict): - content_to_replace: dict[str, int | bytes | None] - created: datetime - created_by: Optional[int] - - def slugify_hashed_filename(identifier: str) -> str: """ Generate a filesystem-safe filename from an identifier. @@ -395,6 +390,41 @@ def create_zip(self, path: str) -> None: ) +@dataclass +class RestoreLearningPackageData: + """ + Data about the restored learning package. + """ + key: str + key_from_zip: str + title: str + num_containers: int + num_components: int + num_collections: int + + +@dataclass +class BackupMetadata: + """ + Metadata about the backup operation. + """ + format_version: int + created_at: str + created_by: str | None = None + original_server: str | None = None + + +@dataclass +class RestoreResult: + """ + Result of the restore operation. + """ + status: Literal["success", "error"] + log_file_error: StringIO | None = None + lp_restored_data: RestoreLearningPackageData | None = None + backup_metadata: BackupMetadata | None = None + + class LearningPackageUnzipper: """ Handles extraction and restoration of learning package data from a zip archive. @@ -420,7 +450,7 @@ def __init__(self, zipf: zipfile.ZipFile) -> None: self.subsections_map_by_key: dict[str, Any] = {} self.sections_map_by_key: dict[str, Any] = {} self.all_publishable_entities_keys: set[str] = set() - self.all_published_entities_versions: set[str] = set() # To track published entity versions + self.all_published_entities_versions: set[tuple[str, int]] = set() # To track published entity versions # -------------------------- # Public API @@ -430,15 +460,17 @@ def __init__(self, zipf: zipfile.ZipFile) -> None: def load(self) -> dict[str, Any]: """Extracts and restores all objects from the ZIP archive in an atomic transaction.""" - # Step 1: Validate presence of mandatory files - _, organized_files = self.preliminary_check() + # Step 1: Validate presence of package.toml and basic structure + _, organized_files = self.check_mandatory_files() if self.errors: # Early return if preliminary checks fail since mandatory files are missing - return { - "status": "error", - "log_file_error": self._write_errors(), # return a StringIO with the errors - "general_info": None - } + result = RestoreResult( + status="error", + log_file_error=self._write_errors(), # return a StringIO with the errors + lp_restored_data=None, + backup_metadata=None, + ) + return asdict(result) # Step 2: Extract and validate learning package, entities and collections # Errors are collected and reported at the end @@ -459,11 +491,13 @@ def load(self) -> dict[str, Any]: # Step 3.1: If there are validation errors, return them without saving anything if self.errors: - return { - "status": "error", - "log_file_error": self._write_errors(), # return a StringIO with the errors - "general_info": None - } + result = RestoreResult( + status="error", + log_file_error=self._write_errors(), # return a StringIO with the errors + lp_restored_data=None, + backup_metadata=None, + ) + return asdict(result) # Step 3.2: Save everything to the DB # All validations passed, we can proceed to save everything @@ -480,21 +514,31 @@ def load(self) -> dict[str, Any]: len(containers_validated.get(container_type, [])) for container_type in ["section", "subsection", "unit"] ) - return { - "status": "success", - "log_file_error": None, - "general_info": { - "learning_package_key": learning_package.key, - "learning_package_title": learning_package.title, - "containers": num_containers, - "components": len(components_validated["components"]), - "collections": len(collections_validated["collections"]), - "metadata": lp_metadata, - } - } - def preliminary_check(self) -> Tuple[list[dict[str, Any]], dict[str, Any]]: - """Performs a preliminary check of the zip file structure and mandatory files.""" + result = RestoreResult( + status="success", + log_file_error=None, + lp_restored_data=RestoreLearningPackageData( + key=learning_package.key, + key_from_zip=learning_package_validated["key"], + title=learning_package.title, + num_containers=num_containers, + num_components=len(components_validated["components"]), + num_collections=len(collections_validated["collections"]), + ), + backup_metadata=BackupMetadata( + format_version=lp_metadata.get("format_version", 1), + created_by=lp_metadata.get("created_by"), + created_at=lp_metadata.get("created_at"), + ) if lp_metadata else None, + ) + return asdict(result) + + def check_mandatory_files(self) -> Tuple[list[dict[str, Any]], dict[str, Any]]: + """ + Check for the presence of mandatory files in the zip archive. + So far, the only mandatory file is package.toml. + """ organized_files = self._get_organized_file_list(self.zipf.namelist()) if not organized_files["learning_package"]: @@ -653,7 +697,7 @@ def _save_components(self, learning_package, components, component_static_files) version_num = valid_published["version_num"] # Should exist, validated earlier content_to_replace = self._resolve_static_files(version_num, entity_key, component_static_files) self.all_published_entities_versions.add( - f"{entity_key}__v{version_num}" + (entity_key, version_num) ) # Track published version components_api.create_next_component_version( self.components_map_by_key[entity_key].publishable_entity.id, @@ -673,7 +717,7 @@ def _save_units(self, learning_package, containers): entity_key = valid_published.pop("entity_key") children = self._resolve_children(valid_published, self.components_map_by_key) self.all_published_entities_versions.add( - f"{entity_key}__v{valid_published.get('version_num')}" + (entity_key, valid_published.get('version_num')) ) # Track published version units_api.create_next_unit_version( self.units_map_by_key[entity_key], @@ -693,7 +737,7 @@ def _save_subsections(self, learning_package, containers): entity_key = valid_published.pop("entity_key") children = self._resolve_children(valid_published, self.units_map_by_key) self.all_published_entities_versions.add( - f"{entity_key}__v{valid_published.get('version_num')}" + (entity_key, valid_published.get('version_num')) ) # Track published version subsections_api.create_next_subsection_version( self.subsections_map_by_key[entity_key], @@ -713,7 +757,7 @@ def _save_sections(self, learning_package, containers): entity_key = valid_published.pop("entity_key") children = self._resolve_children(valid_published, self.subsections_map_by_key) self.all_published_entities_versions.add( - f"{entity_key}__v{valid_published.get('version_num')}" + (entity_key, valid_published.get('version_num')) ) # Track published version sections_api.create_next_section_version( self.sections_map_by_key[entity_key], @@ -727,13 +771,7 @@ def _save_draft_versions(self, components, containers, component_static_files): for valid_draft in components.get("components_drafts", []): entity_key = valid_draft.pop("entity_key") version_num = valid_draft["version_num"] # Should exist, validated earlier - entity_version_identifier = f"{entity_key}__v{version_num}" - if entity_version_identifier in self.all_published_entities_versions: - # Skip creating draft if this version is already published - # Why? Because the version itself is already created and - # we don't want to create duplicate versions. - # Otherwise, we will raise an IntegrityError on PublishableEntityVersion - # due to unique constraints between publishable_entity and version_num. + if self._is_version_already_exists(entity_key, version_num): continue content_to_replace = self._resolve_static_files(version_num, entity_key, component_static_files) components_api.create_next_component_version( @@ -749,13 +787,7 @@ def _save_draft_versions(self, components, containers, component_static_files): for valid_draft in containers.get("unit_drafts", []): entity_key = valid_draft.pop("entity_key") version_num = valid_draft["version_num"] # Should exist, validated earlier - entity_version_identifier = f"{entity_key}__v{version_num}" - if entity_version_identifier in self.all_published_entities_versions: - # Skip creating draft if this version is already published - # Why? Because the version itself is already created and - # we don't want to create duplicate versions. - # Otherwise, we will raise an IntegrityError on PublishableEntityVersion - # due to unique constraints between publishable_entity and version_num. + if self._is_version_already_exists(entity_key, version_num): continue children = self._resolve_children(valid_draft, self.components_map_by_key) units_api.create_next_unit_version( @@ -768,13 +800,7 @@ def _save_draft_versions(self, components, containers, component_static_files): for valid_draft in containers.get("subsection_drafts", []): entity_key = valid_draft.pop("entity_key") version_num = valid_draft["version_num"] # Should exist, validated earlier - entity_version_identifier = f"{entity_key}__v{version_num}" - if entity_version_identifier in self.all_published_entities_versions: - # Skip creating draft if this version is already published - # Why? Because the version itself is already created and - # we don't want to create duplicate versions. - # Otherwise, we will raise an IntegrityError on PublishableEntityVersion - # due to unique constraints between publishable_entity and version_num. + if self._is_version_already_exists(entity_key, version_num): continue children = self._resolve_children(valid_draft, self.units_map_by_key) subsections_api.create_next_subsection_version( @@ -787,13 +813,7 @@ def _save_draft_versions(self, components, containers, component_static_files): for valid_draft in containers.get("section_drafts", []): entity_key = valid_draft.pop("entity_key") version_num = valid_draft["version_num"] # Should exist, validated earlier - entity_version_identifier = f"{entity_key}__v{version_num}" - if entity_version_identifier in self.all_published_entities_versions: - # Skip creating draft if this version is already published - # Why? Because the version itself is already created and - # we don't want to create duplicate versions. - # Otherwise, we will raise an IntegrityError on PublishableEntityVersion - # due to unique constraints between publishable_entity and version_num. + if self._is_version_already_exists(entity_key, version_num): continue children = self._resolve_children(valid_draft, self.subsections_map_by_key) sections_api.create_next_section_version( @@ -823,6 +843,20 @@ def _write_errors(self) -> StringIO | None: return None return StringIO(content) + def _is_version_already_exists(self, entity_key: str, version_num: int) -> bool: + """ + Check if a version already exists for a given entity key and version number. + + Note: + Skip creating draft if this version is already published + Why? Because the version itself is already created and + we don't want to create duplicate versions. + Otherwise, we will raise an IntegrityError on PublishableEntityVersion + due to unique constraints between publishable_entity and version_num. + """ + identifier = (entity_key, version_num) + return identifier in self.all_published_entities_versions + def _resolve_static_files( self, num_version: int, diff --git a/tests/openedx_learning/apps/authoring/backup_restore/test_restore.py b/tests/openedx_learning/apps/authoring/backup_restore/test_restore.py index 1a0a09cb5..f34451b2e 100644 --- a/tests/openedx_learning/apps/authoring/backup_restore/test_restore.py +++ b/tests/openedx_learning/apps/authoring/backup_restore/test_restore.py @@ -23,13 +23,13 @@ def setUp(self): self.zip_file = folder_to_inmemory_zip(self.fixtures_folder) self.lp_key = "lib:WGU:LIB_C001" - @patch("openedx_learning.apps.authoring.backup_restore.management.commands.lp_load.load_dump_zip_file") - def test_restore_command(self, mock_load_dump_zip_file): - # Mock load_dump_zip_file to return our in-memory zip file - mock_load_dump_zip_file.return_value = LearningPackageUnzipper(self.zip_file).load() + @patch("openedx_learning.apps.authoring.backup_restore.management.commands.lp_load.load_library_from_zip") + def test_restore_command(self, mock_load_library_from_zip): + # Mock load_library_from_zip to return our in-memory zip file + mock_load_library_from_zip.return_value = LearningPackageUnzipper(self.zip_file).load() out = StringIO() - # You can pass any dummy path, since load_dump_zip_file is mocked + # You can pass any dummy path, since load_library_from_zip is mocked call_command("lp_load", "dummy.zip", stdout=out) lp = self.verify_lp() @@ -160,18 +160,19 @@ def test_successful_restore_with_no_command_line(self): expected = { "status": "success", "log_file_error": None, - "general_info": { - "learning_package_key": "lib:WGU:LIB_C001", - "learning_package_title": "Library test", - "containers": 3, - "components": 7, - "collections": 1, - "metadata": { - "format_version": 1, - "created_by": "dormsbee", - "created_at": datetime(2025, 10, 5, 18, 23, 45, 180535, tzinfo=timezone.utc), - "origin_server": "cms.test", - }, + "lp_restored_data": { + "key": "lib:WGU:LIB_C001", + "key_from_zip": "lib:WGU:LIB_C001", + "title": "Library test", + "num_containers": 3, + "num_components": 7, + "num_collections": 1, + }, + "backup_metadata": { + "format_version": 1, + "created_by": "dormsbee", + "created_at": datetime(2025, 10, 5, 18, 23, 45, 180535, tzinfo=timezone.utc), + "origin_server": "cms.test", }, } @@ -179,10 +180,10 @@ def test_successful_restore_with_no_command_line(self): assert result["status"] == expected["status"] assert result["log_file_error"] is None - general_info = result["general_info"] - expected_info = expected["general_info"] - metadata_general_info = general_info.pop("metadata", None) - metadata_expected_info = expected_info.pop("metadata", None) + general_info = result["lp_restored_data"] + expected_info = expected["lp_restored_data"] + metadata_general_info = general_info.pop("backup_metadata", None) + metadata_expected_info = expected_info.pop("backup_metadata", None) assert general_info == expected_info, f"General info does not match. Got {general_info}" assert metadata_general_info == metadata_expected_info, f"Meta info does not match. Got {metadata_general_info}" @@ -196,7 +197,7 @@ def test_restore_with_missing_learning_package_file(self): result = LearningPackageUnzipper(zip_file).load() assert result["status"] == "error" - assert result["general_info"] is None + assert result["lp_restored_data"] is None assert result["log_file_error"] is not None log_content = result["log_file_error"].getvalue() assert "Missing learning package file." in log_content @@ -237,7 +238,7 @@ def test_error_learning_package_missing_key(self): result = LearningPackageUnzipper(zip_file).load() assert result["status"] == "error" - assert result["general_info"] is None + assert result["lp_restored_data"] is None assert result["log_file_error"] is not None log_content = result["log_file_error"].getvalue() expected_error = "Errors encountered during restore:\npackage.toml learning package section: {'key':" @@ -263,7 +264,7 @@ def test_error_no_metadata_section(self): result = LearningPackageUnzipper(zip_file).load() assert result["status"] == "error" - assert result["general_info"] is None + assert result["lp_restored_data"] is None assert result["log_file_error"] is not None log_content = result["log_file_error"].getvalue() expected_error = "Errors encountered during restore:\npackage.toml meta section: {'non_field_errors': [Er" From 04c514ca09b46321b08cee69e92c9ec9897ff8dc Mon Sep 17 00:00:00 2001 From: Daniel Wong Date: Fri, 10 Oct 2025 11:56:14 -0600 Subject: [PATCH 7/9] test: fix test_error_preliminary_check test --- .../apps/authoring/backup_restore/test_restore.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/openedx_learning/apps/authoring/backup_restore/test_restore.py b/tests/openedx_learning/apps/authoring/backup_restore/test_restore.py index f34451b2e..288f7863d 100644 --- a/tests/openedx_learning/apps/authoring/backup_restore/test_restore.py +++ b/tests/openedx_learning/apps/authoring/backup_restore/test_restore.py @@ -207,7 +207,7 @@ def test_error_preliminary_check(self): """Test that preliminary check catches missing learning_package.toml.""" zip_file = folder_to_inmemory_zip(os.path.join(os.path.dirname(__file__), "fixtures/missing_lp_file")) unzipper = LearningPackageUnzipper(zip_file) - errors, _ = unzipper.preliminary_check() + errors, _ = unzipper.check_mandatory_files() assert len(errors) == 1 assert errors[0]["file"] == "package.toml" From 2d328a38c8d8487f38a4637e1abe1e4cfa395610 Mon Sep 17 00:00:00 2001 From: Daniel Wong Date: Fri, 10 Oct 2025 18:14:12 -0600 Subject: [PATCH 8/9] feat: include staged key generation in the load process --- .../apps/authoring/backup_restore/api.py | 6 ++- .../apps/authoring/backup_restore/zipper.py | 54 +++++++++++++++++-- .../authoring/backup_restore/test_restore.py | 44 ++++++++++++++- 3 files changed, 95 insertions(+), 9 deletions(-) diff --git a/openedx_learning/apps/authoring/backup_restore/api.py b/openedx_learning/apps/authoring/backup_restore/api.py index e6f478a33..1bae0b198 100644 --- a/openedx_learning/apps/authoring/backup_restore/api.py +++ b/openedx_learning/apps/authoring/backup_restore/api.py @@ -3,6 +3,8 @@ """ import zipfile +from django.contrib.auth.models import User as UserType # pylint: disable=imported-auth-user + from openedx_learning.apps.authoring.backup_restore.zipper import LearningPackageUnzipper, LearningPackageZipper from openedx_learning.apps.authoring.publishing.api import get_learning_package_by_key @@ -18,11 +20,11 @@ def create_zip_file(lp_key: str, path: str) -> None: LearningPackageZipper(learning_package).create_zip(path) -def load_library_from_zip(path: str) -> dict: +def load_library_from_zip(path: str, user: UserType | None = None, use_staged_lp_key: bool = False) -> dict: """ Loads a learning package from a zip file at the given path. Restores the learning package and its contents to the database. Returns a dictionary with the status of the operation and any errors encountered. """ with zipfile.ZipFile(path, "r") as zipf: - return LearningPackageUnzipper(zipf).load() + return LearningPackageUnzipper(zipf, user, use_staged_lp_key).load() diff --git a/openedx_learning/apps/authoring/backup_restore/zipper.py b/openedx_learning/apps/authoring/backup_restore/zipper.py index 76102e8eb..a04d1eb1d 100644 --- a/openedx_learning/apps/authoring/backup_restore/zipper.py +++ b/openedx_learning/apps/authoring/backup_restore/zipper.py @@ -3,6 +3,7 @@ including a TOML representation of the learning package and its entities. """ import hashlib +import time import zipfile from collections import defaultdict from dataclasses import asdict, dataclass @@ -11,6 +12,7 @@ from pathlib import Path from typing import Any, List, Literal, Optional, Tuple +from django.contrib.auth.models import User as UserType # pylint: disable=imported-auth-user from django.db import transaction from django.db.models import Prefetch, QuerySet from django.utils.text import slugify @@ -51,6 +53,7 @@ from openedx_learning.apps.authoring.units import api as units_api TOML_PACKAGE_NAME = "package.toml" +DEFAULT_USERNAME = "command" def slugify_hashed_filename(identifier: str) -> str: @@ -395,8 +398,8 @@ class RestoreLearningPackageData: """ Data about the restored learning package. """ - key: str - key_from_zip: str + key: str # The key of the restored learning package (may be different if staged) + original_key: str title: str num_containers: int num_components: int @@ -425,10 +428,42 @@ class RestoreResult: backup_metadata: BackupMetadata | None = None +def generate_staged_lp_key(lp_key: str, user: UserType | None) -> str: + """ + Generate a staged learning package key based on the given base key. + + Arguments: + lp_key (str): The base key of the learning package. + user (UserType | None): The user performing the restore operation. + + Example: + Input: "lib:WGU:LIB_C001" + Output: "lib-restore:dave:WGU:LIB_C001:1728575321" + + The timestamp at the end ensures the key is unique. + """ + username = user.username if user else DEFAULT_USERNAME + parts = lp_key.split(":") + if len(parts) < 3: + raise ValueError(f"Invalid learning package key: {lp_key}") + + _, org_slug, lp_slug = parts[:3] + timestamp = int(time.time() * 1000) # Current time in milliseconds + return f"lib-restore:{username}:{org_slug}:{lp_slug}:{timestamp}" + + class LearningPackageUnzipper: """ Handles extraction and restoration of learning package data from a zip archive. + Args: + zipf (zipfile.ZipFile): The zip file containing the learning package data. + user (UserType | None): The user performing the restore operation. Not necessarily the creator. + generate_new_key (bool): Whether to generate a new key for the restored learning package. + + Returns: + dict[str, Any]: The result of the restore operation, including any errors encountered. + Responsibilities: - Parse and organize files from the zip structure. - Restore learning package, containers, components, and collections to the database. @@ -439,8 +474,10 @@ class LearningPackageUnzipper: result = unzipper.load() """ - def __init__(self, zipf: zipfile.ZipFile) -> None: + def __init__(self, zipf: zipfile.ZipFile, user: UserType | None = None, use_staged_lp_key: bool = False): self.zipf = zipf + self.user = user + self.use_staged_lp_key = use_staged_lp_key self.utc_now: datetime = datetime.now(timezone.utc) self.component_types_cache: dict[tuple[str, str], ComponentType] = {} self.errors: list[dict[str, Any]] = [] @@ -502,6 +539,7 @@ def load(self) -> dict[str, Any]: # Step 3.2: Save everything to the DB # All validations passed, we can proceed to save everything # Save the learning package first to get its ID + original_lp_key = learning_package_validated["key"] learning_package = self._save( learning_package_validated, components_validated, @@ -519,8 +557,8 @@ def load(self) -> dict[str, Any]: status="success", log_file_error=None, lp_restored_data=RestoreLearningPackageData( - key=learning_package.key, - key_from_zip=learning_package_validated["key"], + key=learning_package.key, # May be different if staged + original_key=original_lp_key, # The original key from the backup archive title=learning_package.title, num_containers=num_containers, num_components=len(components_validated["components"]), @@ -659,6 +697,12 @@ def _save( ) -> LearningPackage: """Persist all validated entities in two phases: published then drafts.""" + if self.use_staged_lp_key: + # Generate a tmp key for the staged learning package + learning_package["key"] = generate_staged_lp_key( + lp_key=learning_package["key"], + user=self.user + ) learning_package_obj = publishing_api.create_learning_package(**learning_package) with publishing_api.bulk_draft_changes_for(learning_package_obj.id): diff --git a/tests/openedx_learning/apps/authoring/backup_restore/test_restore.py b/tests/openedx_learning/apps/authoring/backup_restore/test_restore.py index 288f7863d..f149d9851 100644 --- a/tests/openedx_learning/apps/authoring/backup_restore/test_restore.py +++ b/tests/openedx_learning/apps/authoring/backup_restore/test_restore.py @@ -6,7 +6,7 @@ from django.core.management import call_command -from openedx_learning.apps.authoring.backup_restore.zipper import LearningPackageUnzipper +from openedx_learning.apps.authoring.backup_restore.zipper import LearningPackageUnzipper, generate_staged_lp_key from openedx_learning.apps.authoring.collections import api as collections_api from openedx_learning.apps.authoring.components import api as components_api from openedx_learning.apps.authoring.publishing import api as publishing_api @@ -162,7 +162,7 @@ def test_successful_restore_with_no_command_line(self): "log_file_error": None, "lp_restored_data": { "key": "lib:WGU:LIB_C001", - "key_from_zip": "lib:WGU:LIB_C001", + "original_key": "lib:WGU:LIB_C001", "title": "Library test", "num_containers": 3, "num_components": 7, @@ -191,6 +191,21 @@ def test_successful_restore_with_no_command_line(self): lp = publishing_api.LearningPackage.objects.filter(key="lib:WGU:LIB_C001").first() assert lp is not None, "Learning package was not restored." + def test_successful_restore_with_staged_key(self): + """Test restoring a learning package with a staged key.""" + zip_file = folder_to_inmemory_zip(os.path.join(os.path.dirname(__file__), "fixtures/library_backup")) + result = LearningPackageUnzipper(zip_file, use_staged_lp_key=True).load() + + assert result["status"] == "success" + assert result["lp_restored_data"] is not None + restored_key = result["lp_restored_data"]["key"] + original_key = result["lp_restored_data"]["original_key"] + assert original_key == "lib:WGU:LIB_C001" + assert restored_key.startswith("lib-restore:command:WGU:LIB_C001:") + + lp = publishing_api.LearningPackage.objects.filter(key=restored_key).first() + assert lp is not None, "Learning package with staged key was not restored." + def test_restore_with_missing_learning_package_file(self): """Test restoring a learning package with a missing learning_package.toml file.""" zip_file = folder_to_inmemory_zip(os.path.join(os.path.dirname(__file__), "fixtures/missing_lp_file")) @@ -269,3 +284,28 @@ def test_error_no_metadata_section(self): log_content = result["log_file_error"].getvalue() expected_error = "Errors encountered during restore:\npackage.toml meta section: {'non_field_errors': [Er" assert expected_error in log_content + + +class RestoreUtilitiesTest(TestCase): + """Tests for utility functions used in the restore process.""" + + def test_generate_staged_lp_key(self): + """Test generating a staged learning package key.""" + + user_mock = type("User", (), {"username": "dan"}) + lp_key = "lib:WGU:LIB_C001" + staged_key = generate_staged_lp_key(lp_key, user_mock) + + assert staged_key.startswith("lib-restore:dan:WGU:LIB_C001:") + parts = staged_key.split(":") + assert len(parts) == 5 + timestamp_part = parts[-1] + assert timestamp_part.isdigit() + + def test_error_generate_staged_lp_key_invalid_lp_key(self): + """Test that generating a staged key with an invalid lp_key raises ValueError.""" + user_mock = type("User", (), {"username": "dan"}) + invalid_lp_key = "invalid-key-format" + with self.assertRaises(ValueError) as context: + generate_staged_lp_key(invalid_lp_key, user_mock) + assert "Invalid learning package key" in str(context.exception) From 43e5159365b6c49af24aba77061c38be556cd462 Mon Sep 17 00:00:00 2001 From: Daniel Wong Date: Wed, 15 Oct 2025 23:15:57 -0600 Subject: [PATCH 9/9] feat: extend load process with updated input and output fields --- .../apps/authoring/backup_restore/api.py | 4 +- .../management/commands/lp_load.py | 10 ++- .../apps/authoring/backup_restore/zipper.py | 63 +++++++++++++------ .../authoring/backup_restore/test_restore.py | 46 ++++++++------ 4 files changed, 83 insertions(+), 40 deletions(-) diff --git a/openedx_learning/apps/authoring/backup_restore/api.py b/openedx_learning/apps/authoring/backup_restore/api.py index 1bae0b198..339eae934 100644 --- a/openedx_learning/apps/authoring/backup_restore/api.py +++ b/openedx_learning/apps/authoring/backup_restore/api.py @@ -20,11 +20,11 @@ def create_zip_file(lp_key: str, path: str) -> None: LearningPackageZipper(learning_package).create_zip(path) -def load_library_from_zip(path: str, user: UserType | None = None, use_staged_lp_key: bool = False) -> dict: +def load_learning_package(path: str, key: str | None = None, user: UserType | None = None) -> dict: """ Loads a learning package from a zip file at the given path. Restores the learning package and its contents to the database. Returns a dictionary with the status of the operation and any errors encountered. """ with zipfile.ZipFile(path, "r") as zipf: - return LearningPackageUnzipper(zipf, user, use_staged_lp_key).load() + return LearningPackageUnzipper(zipf, key, user).load() diff --git a/openedx_learning/apps/authoring/backup_restore/management/commands/lp_load.py b/openedx_learning/apps/authoring/backup_restore/management/commands/lp_load.py index a1eaecb58..12a3f55d3 100644 --- a/openedx_learning/apps/authoring/backup_restore/management/commands/lp_load.py +++ b/openedx_learning/apps/authoring/backup_restore/management/commands/lp_load.py @@ -4,10 +4,11 @@ import logging import time +from django.contrib.auth.models import User as UserType # pylint: disable=imported-auth-user from django.core.management import CommandError from django.core.management.base import BaseCommand -from openedx_learning.apps.authoring.backup_restore.api import load_library_from_zip +from openedx_learning.apps.authoring.backup_restore.api import load_learning_package logger = logging.getLogger(__name__) @@ -20,14 +21,19 @@ class Command(BaseCommand): def add_arguments(self, parser): parser.add_argument('file_name', type=str, help='The path of the input zip file to load.') + parser.add_argument('username', type=str, help='The username of the user performing the load operation.') def handle(self, *args, **options): file_name = options['file_name'] + username = options['username'] if not file_name.lower().endswith(".zip"): raise CommandError("Input file name must end with .zip") try: start_time = time.time() - result = load_library_from_zip(file_name) + # Create a tmp user to pass to the load function + user = UserType.objects.get(username=username) + + result = load_learning_package(file_name, user=user) duration = time.time() - start_time if result["status"] == "error": message = "Errors encountered during restore:\n" diff --git a/openedx_learning/apps/authoring/backup_restore/zipper.py b/openedx_learning/apps/authoring/backup_restore/zipper.py index a04d1eb1d..ed55a99dd 100644 --- a/openedx_learning/apps/authoring/backup_restore/zipper.py +++ b/openedx_learning/apps/authoring/backup_restore/zipper.py @@ -398,10 +398,16 @@ class RestoreLearningPackageData: """ Data about the restored learning package. """ + id: int # The ID of the restored learning package key: str # The key of the restored learning package (may be different if staged) - original_key: str + archive_lp_key: str # The original key from the archive + archive_org_key: str # The original organization key from the archive + archive_slug: str # The original slug from the archive title: str num_containers: int + num_sections: int + num_subsections: int + num_units: int num_components: int num_collections: int @@ -428,28 +434,35 @@ class RestoreResult: backup_metadata: BackupMetadata | None = None -def generate_staged_lp_key(lp_key: str, user: UserType | None) -> str: +def unpack_lp_key(lp_key: str) -> tuple[str, str]: + """ + Unpack a learning package key into its components. + """ + parts = lp_key.split(":") + if len(parts) < 3: + raise ValueError(f"Invalid learning package key: {lp_key}") + _, org_key, lp_slug = parts[:3] + return org_key, lp_slug + + +def generate_staged_lp_key(archive_lp_key: str, user: UserType) -> str: """ Generate a staged learning package key based on the given base key. Arguments: - lp_key (str): The base key of the learning package. + archive_lp_key (str): The original learning package key from the archive. user (UserType | None): The user performing the restore operation. Example: Input: "lib:WGU:LIB_C001" - Output: "lib-restore:dave:WGU:LIB_C001:1728575321" + Output: "lp-restore:dave:WGU:LIB_C001:1728575321" The timestamp at the end ensures the key is unique. """ - username = user.username if user else DEFAULT_USERNAME - parts = lp_key.split(":") - if len(parts) < 3: - raise ValueError(f"Invalid learning package key: {lp_key}") - - _, org_slug, lp_slug = parts[:3] + username = user.username + org_key, lp_slug = unpack_lp_key(archive_lp_key) timestamp = int(time.time() * 1000) # Current time in milliseconds - return f"lib-restore:{username}:{org_slug}:{lp_slug}:{timestamp}" + return f"lp-restore:{username}:{org_key}:{lp_slug}:{timestamp}" class LearningPackageUnzipper: @@ -474,10 +487,10 @@ class LearningPackageUnzipper: result = unzipper.load() """ - def __init__(self, zipf: zipfile.ZipFile, user: UserType | None = None, use_staged_lp_key: bool = False): + def __init__(self, zipf: zipfile.ZipFile, key: str | None = None, user: UserType | None = None): self.zipf = zipf self.user = user - self.use_staged_lp_key = use_staged_lp_key + self.lp_key = key # If provided, use this key for the restored learning package self.utc_now: datetime = datetime.now(timezone.utc) self.component_types_cache: dict[tuple[str, str], ComponentType] = {} self.errors: list[dict[str, Any]] = [] @@ -539,7 +552,7 @@ def load(self) -> dict[str, Any]: # Step 3.2: Save everything to the DB # All validations passed, we can proceed to save everything # Save the learning package first to get its ID - original_lp_key = learning_package_validated["key"] + archive_lp_key = learning_package_validated["key"] learning_package = self._save( learning_package_validated, components_validated, @@ -553,14 +566,21 @@ def load(self) -> dict[str, Any]: for container_type in ["section", "subsection", "unit"] ) + org_key, lp_slug = unpack_lp_key(archive_lp_key) result = RestoreResult( status="success", log_file_error=None, lp_restored_data=RestoreLearningPackageData( - key=learning_package.key, # May be different if staged - original_key=original_lp_key, # The original key from the backup archive + id=learning_package.id, + key=learning_package.key, + archive_lp_key=archive_lp_key, # The original key from the backup archive + archive_org_key=org_key, # The original organization key from the backup archive + archive_slug=lp_slug, # The original slug from the backup archive title=learning_package.title, num_containers=num_containers, + num_sections=len(containers_validated.get("section", [])), + num_subsections=len(containers_validated.get("subsection", [])), + num_units=len(containers_validated.get("unit", [])), num_components=len(components_validated["components"]), num_collections=len(collections_validated["collections"]), ), @@ -697,12 +717,19 @@ def _save( ) -> LearningPackage: """Persist all validated entities in two phases: published then drafts.""" - if self.use_staged_lp_key: + # Important: If not using a specific LP key, generate a temporary one + # We cannot use the original key because it may generate security issues + if not self.lp_key: # Generate a tmp key for the staged learning package + if not self.user: + raise ValueError("User is required to create lp_key") learning_package["key"] = generate_staged_lp_key( - lp_key=learning_package["key"], + archive_lp_key=learning_package["key"], user=self.user ) + else: + learning_package["key"] = self.lp_key + learning_package_obj = publishing_api.create_learning_package(**learning_package) with publishing_api.bulk_draft_changes_for(learning_package_obj.id): diff --git a/tests/openedx_learning/apps/authoring/backup_restore/test_restore.py b/tests/openedx_learning/apps/authoring/backup_restore/test_restore.py index f149d9851..b1a8db71b 100644 --- a/tests/openedx_learning/apps/authoring/backup_restore/test_restore.py +++ b/tests/openedx_learning/apps/authoring/backup_restore/test_restore.py @@ -4,6 +4,7 @@ from io import StringIO from unittest.mock import patch +from django.contrib.auth.models import User as UserType # pylint: disable=imported-auth-user from django.core.management import call_command from openedx_learning.apps.authoring.backup_restore.zipper import LearningPackageUnzipper, generate_staged_lp_key @@ -22,24 +23,26 @@ def setUp(self): self.fixtures_folder = os.path.join(os.path.dirname(__file__), "fixtures/library_backup") self.zip_file = folder_to_inmemory_zip(self.fixtures_folder) self.lp_key = "lib:WGU:LIB_C001" + self.user = UserType.objects.create_user(username='lp_user', password='12345') - @patch("openedx_learning.apps.authoring.backup_restore.management.commands.lp_load.load_library_from_zip") - def test_restore_command(self, mock_load_library_from_zip): - # Mock load_library_from_zip to return our in-memory zip file - mock_load_library_from_zip.return_value = LearningPackageUnzipper(self.zip_file).load() + @patch("openedx_learning.apps.authoring.backup_restore.api.load_learning_package") + def test_restore_command(self, mock_load_learning_package): + # Mock load_learning_package to return our in-memory zip file + restore_result = LearningPackageUnzipper(self.zip_file, self.user).load() + mock_load_learning_package.return_value = restore_result out = StringIO() - # You can pass any dummy path, since load_library_from_zip is mocked - call_command("lp_load", "dummy.zip", stdout=out) + # You can pass any dummy path, since load_learning_package is mocked + call_command("lp_load", "dummy.zip", "lp_user", stdout=out) - lp = self.verify_lp() + lp = self.verify_lp(restore_result["lp_restored_data"]["key"]) self.verify_containers(lp) self.verify_components(lp) self.verify_collections(lp) - def verify_lp(self): + def verify_lp(self, key): """Verify the learning package was restored correctly.""" - lp = publishing_api.LearningPackage.objects.filter(key=self.lp_key).first() + lp = publishing_api.LearningPackage.objects.filter(key=key).first() assert lp is not None, "Learning package was not restored." assert lp.title == "Library test" assert lp.description == "" @@ -155,18 +158,24 @@ class RestoreLearningPackageTest(TestCase): def test_successful_restore_with_no_command_line(self): """Test restoring a learning package without using the management command.""" zip_file = folder_to_inmemory_zip(os.path.join(os.path.dirname(__file__), "fixtures/library_backup")) - result = LearningPackageUnzipper(zip_file).load() + result = LearningPackageUnzipper(zip_file, key="lib-xx:WGU:LIB_C001").load() expected = { "status": "success", "log_file_error": None, "lp_restored_data": { - "key": "lib:WGU:LIB_C001", - "original_key": "lib:WGU:LIB_C001", + "id": result["lp_restored_data"]["id"], # Dynamic field + "key": "lib-xx:WGU:LIB_C001", + "archive_lp_key": "lib:WGU:LIB_C001", + "archive_org_key": "WGU", + "archive_slug": "LIB_C001", "title": "Library test", "num_containers": 3, "num_components": 7, "num_collections": 1, + "num_sections": 1, + "num_subsections": 1, + "num_units": 1, }, "backup_metadata": { "format_version": 1, @@ -188,20 +197,21 @@ def test_successful_restore_with_no_command_line(self): assert general_info == expected_info, f"General info does not match. Got {general_info}" assert metadata_general_info == metadata_expected_info, f"Meta info does not match. Got {metadata_general_info}" - lp = publishing_api.LearningPackage.objects.filter(key="lib:WGU:LIB_C001").first() + lp = publishing_api.LearningPackage.objects.filter(key="lib-xx:WGU:LIB_C001").first() assert lp is not None, "Learning package was not restored." def test_successful_restore_with_staged_key(self): """Test restoring a learning package with a staged key.""" + user = UserType.objects.create_user(username='lp_user', password='12345') zip_file = folder_to_inmemory_zip(os.path.join(os.path.dirname(__file__), "fixtures/library_backup")) - result = LearningPackageUnzipper(zip_file, use_staged_lp_key=True).load() + result = LearningPackageUnzipper(zip_file, user=user).load() assert result["status"] == "success" assert result["lp_restored_data"] is not None restored_key = result["lp_restored_data"]["key"] - original_key = result["lp_restored_data"]["original_key"] - assert original_key == "lib:WGU:LIB_C001" - assert restored_key.startswith("lib-restore:command:WGU:LIB_C001:") + archive_key = result["lp_restored_data"]["archive_lp_key"] + assert archive_key == "lib:WGU:LIB_C001" + assert restored_key.startswith("lp-restore:lp_user:WGU:LIB_C001:") lp = publishing_api.LearningPackage.objects.filter(key=restored_key).first() assert lp is not None, "Learning package with staged key was not restored." @@ -296,7 +306,7 @@ def test_generate_staged_lp_key(self): lp_key = "lib:WGU:LIB_C001" staged_key = generate_staged_lp_key(lp_key, user_mock) - assert staged_key.startswith("lib-restore:dan:WGU:LIB_C001:") + assert staged_key.startswith("lp-restore:dan:WGU:LIB_C001:") parts = staged_key.split(":") assert len(parts) == 5 timestamp_part = parts[-1]