Skip to content

Commit d7dc148

Browse files
committed
fix: Revert changes to backup restore-format
1 parent 5f3cd3c commit d7dc148

3 files changed

Lines changed: 42 additions & 122 deletions

File tree

src/openedx_content/applets/backup_restore/serializers.py

Lines changed: 42 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
from rest_framework import serializers
77

88
from ..components import api as components_api
9+
from ..components.models import ComponentType
910

1011

1112
class LearningPackageSerializer(serializers.Serializer): # pylint: disable=abstract-method
@@ -59,52 +60,55 @@ class EntityVersionSerializer(serializers.Serializer): # pylint: disable=abstra
5960
class ComponentSerializer(EntitySerializer): # pylint: disable=abstract-method
6061
"""
6162
Serializer for components.
62-
63-
Extracts component_type and component_code from the [entity.component]
64-
section if present (archives created in Verawood or later). Falls back to
65-
parsing the entity key for archives created in Ulmo.
63+
Contains logic to convert entity_key to component_type and component_code.
6664
"""
6765

68-
component = serializers.DictField(required=False)
69-
7066
def validate(self, attrs):
7167
"""
72-
Custom validation logic: resolve component_type and component_code.
73-
74-
Archives created in Verawood or later supply an [entity.component]
75-
section with ``component_type`` (e.g. "xblock.v1:problem") and
76-
``component_code`` (e.g. "my_example"). Archives created in Ulmo only
77-
have the entity ``key`` in the format
78-
``"{namespace}:{type_name}:{component_code}"``, so we fall back to
79-
parsing that for backwards compatibility.
68+
Custom validation logic:
69+
parse the entity_key into (component_type, component_code).
8070
"""
81-
component_section = attrs.pop("component", None)
82-
if component_section:
83-
# Verawood+ format: component_type and component_code are explicit.
84-
component_type_str = component_section.get("component_type", "")
85-
component_code = component_section.get("component_code", "")
86-
try:
87-
namespace, type_name = component_type_str.split(":", 1)
88-
except ValueError as exc:
89-
raise serializers.ValidationError(
90-
{"component": f"Invalid component_type format: {component_type_str!r}. "
91-
"Expected '{namespace}:{type_name}'."}
92-
) from exc
93-
component_type_obj = components_api.get_or_create_component_type(namespace, type_name)
94-
else:
95-
# Ulmo (legacy) format: parse the entity key.
96-
entity_key = attrs["key"]
97-
try:
98-
component_type_obj, component_code = (
99-
components_api.get_or_create_component_type_by_entity_key(entity_key)
100-
)
101-
except ValueError as exc:
102-
raise serializers.ValidationError({"key": str(exc)})
103-
attrs["component_type"] = component_type_obj
104-
attrs["component_code"] = component_code
71+
entity_key = attrs["key"]
72+
try:
73+
component_type_obj, component_code = _get_or_create_component_type_by_entity_key(entity_key)
74+
attrs["component_type"] = component_type_obj
75+
attrs["component_code"] = component_code
76+
except ValueError as exc:
77+
raise serializers.ValidationError({"key": str(exc)})
10578
return attrs
10679

10780

81+
def _get_or_create_component_type_by_entity_key(entity_key: str) -> tuple[ComponentType, str]:
82+
"""
83+
Get or create a ComponentType based on a full [entity].key string.
84+
85+
The entity key is expected to be in the format
86+
``"{namespace}:{type_name}:{component_code}"``. This function will parse out
87+
the ``namespace`` and ``type_name`` parts and use those to get or create the
88+
ComponentType.
89+
90+
Raises ValueError if the entity_key is not in the expected format.
91+
92+
Historical note: In Ulmo, this function was part of the public API. This was
93+
inappropriate because the exact format of entity_keys is just a convention
94+
rather than something API callers should count on. That said, it is safe to
95+
assume that in all "v1" archives, the components' entity keys are safe to
96+
parse into (namespace, type, code). So, we have moved this parsing logic
97+
from the public API to just this internal halper function. Future devs,
98+
please to not make new external guarantees about the format of entity keys
99+
(aka entity_refs). A future "v2" backup-restore format will drop this
100+
assumption of parse-ability..
101+
"""
102+
try:
103+
namespace, type_name, component_code = entity_key.split(':', 2)
104+
except ValueError as exc:
105+
raise ValueError(
106+
f"Invalid entity_key format: {entity_key!r}. "
107+
"Expected format: '{namespace}:{type_name}:{component_code}'"
108+
) from exc
109+
return components_api.get_or_create_component_type(namespace, type_name), component_code
110+
111+
108112
class ComponentVersionSerializer(EntityVersionSerializer): # pylint: disable=abstract-method
109113
"""
110114
Serializer for component versions.

src/openedx_content/applets/backup_restore/toml.py

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -108,15 +108,6 @@ def _get_toml_publishable_entity_table(
108108
published_table.add(tomlkit.comment("unpublished: no published_version_num"))
109109
entity_table.add("published", published_table)
110110

111-
if hasattr(entity, "component"):
112-
component = entity.component
113-
component_table = tomlkit.table()
114-
# Write component_type and component_code explicitly so that restore
115-
# (Verawood and later) does not need to parse the entity key.
116-
component_table.add("component_type", str(component.component_type))
117-
component_table.add("component_code", component.component_code)
118-
entity_table.add("component", component_table)
119-
120111
if hasattr(entity, "container"):
121112
container_table = tomlkit.table()
122113
container_types = ["section", "subsection", "unit"]

tests/openedx_content/applets/backup_restore/test_restore.py

Lines changed: 0 additions & 75 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@
88
from django.core.management import call_command
99
from django.test import TestCase
1010

11-
from openedx_content.applets.backup_restore.serializers import ComponentSerializer
1211
from openedx_content.applets.backup_restore.zipper import LearningPackageUnzipper, generate_staged_lp_key
1312
from openedx_content.applets.collections import api as collections_api
1413
from openedx_content.applets.components import api as components_api
@@ -361,80 +360,6 @@ def test_success_metadata_using_user_context(self):
361360
assert metadata == expected_metadata
362361

363362

364-
class ComponentSerializerBackcompatTest(TestCase):
365-
"""
366-
Unit tests for ComponentSerializer's back-compat handling of Ulmo archives
367-
(entity key only) vs. Verawood+ archives ([entity.component] section).
368-
"""
369-
370-
BASE_DATA = {
371-
"can_stand_alone": True,
372-
"key": "xblock.v1:problem:my_code",
373-
"created": "2025-09-04T22:51:59Z",
374-
}
375-
376-
def _serialize(self, extra=None, base=None):
377-
data = {**(base or self.BASE_DATA), **(extra or {})}
378-
s = ComponentSerializer(data=data)
379-
s.is_valid()
380-
return s
381-
382-
def test_legacy_entity_key_parsed(self):
383-
"""Ulmo archives have no [entity.component] section; fall back to parsing the entity key."""
384-
s = self._serialize()
385-
assert s.is_valid(), s.errors
386-
assert s.validated_data["component_type"].namespace == "xblock.v1"
387-
assert s.validated_data["component_type"].name == "problem"
388-
assert s.validated_data["component_code"] == "my_code"
389-
assert "component" not in s.validated_data
390-
391-
def test_new_component_section(self):
392-
"""Verawood+ archives supply an explicit [entity.component] section."""
393-
s = self._serialize({
394-
"component": {
395-
"component_type": "xblock.v1:problem",
396-
"component_code": "my_code",
397-
}
398-
})
399-
assert s.is_valid(), s.errors
400-
assert s.validated_data["component_type"].namespace == "xblock.v1"
401-
assert s.validated_data["component_type"].name == "problem"
402-
assert s.validated_data["component_code"] == "my_code"
403-
assert "component" not in s.validated_data
404-
405-
def test_component_section_overrides_key(self):
406-
"""When [entity.component] is present, it is used even if the key has different info."""
407-
s = self._serialize({
408-
"component": {
409-
"component_type": "xblock.v1:html",
410-
"component_code": "different_code",
411-
}
412-
})
413-
assert s.is_valid(), s.errors
414-
assert s.validated_data["component_type"].name == "html"
415-
assert s.validated_data["component_code"] == "different_code"
416-
417-
def test_invalid_entity_key_format(self):
418-
"""Entity key without enough colons raises a validation error."""
419-
s = self._serialize(base={
420-
"can_stand_alone": True,
421-
"key": "invalid-key",
422-
"created": "2025-09-04T22:51:59Z",
423-
})
424-
assert not s.is_valid()
425-
426-
def test_invalid_component_type_format(self):
427-
"""component_type without a colon raises a validation error."""
428-
s = self._serialize({
429-
"component": {
430-
"component_type": "no-colon-here",
431-
"component_code": "my_code",
432-
}
433-
})
434-
assert not s.is_valid()
435-
assert "component" in s.errors
436-
437-
438363
class RestoreUtilitiesTest(TestCase):
439364
"""Tests for utility functions used in the restore process."""
440365

0 commit comments

Comments
 (0)