Skip to content

Commit cbc344b

Browse files
authored
feat!: Component.local_key -> Component.component_code (#544)
BREAKING CHANGE: Component.local_key has been renamed to Component.component_code. BREAKING CHANGE: Component.component_code now validates against [A-Za-z0-9\-\_\.]+ and has a max_length of 255. Previously local_key used key_field() (no regex validation, max_length=500). BREAKING CHANGE: Function parameters renamed from local_key to component_code in create_component(...) and create_component_and_version(...). BREAKING CHANGE: Functions get_component_by_key(...)/component_exists_by_key(...), renamed to get_component_by_code(...)/component_exists_by_code(...), and parameters renamed from local_key to component_code. Backup-restore format is unchanged. component_type and component_code will continue to be parsed from the entity key. This may change in a future "v2" backup format. Bumps version to 0.41.0. Part of: #322
1 parent cdcf40c commit cbc344b

17 files changed

Lines changed: 211 additions & 103 deletions

File tree

olx_importer/management/commands/load_components.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,7 @@ def import_block_type(self, block_type_name, now): # , publish_log_entry):
140140

141141
for xml_file_path in block_data_path.glob("*.xml"):
142142
components_found += 1
143-
local_key = xml_file_path.stem
143+
component_code = xml_file_path.stem
144144

145145
# Do some basic parsing of the content to see if it's even well
146146
# constructed enough to add (or whether we should skip/error on it).
@@ -155,7 +155,7 @@ def import_block_type(self, block_type_name, now): # , publish_log_entry):
155155
_component, component_version = content_api.create_component_and_version(
156156
self.learning_package.id,
157157
component_type=block_type,
158-
local_key=local_key,
158+
component_code=component_code,
159159
title=display_name,
160160
created=now,
161161
created_by=None,

src/openedx_content/applets/backup_restore/serializers.py

Lines changed: 36 additions & 4 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,24 +60,55 @@ class EntityVersionSerializer(serializers.Serializer): # pylint: disable=abstra
5960
class ComponentSerializer(EntitySerializer): # pylint: disable=abstract-method
6061
"""
6162
Serializer for components.
62-
Contains logic to convert entity_key to component_type and local_key.
63+
Contains logic to convert entity_key to component_type and component_code.
6364
"""
6465

6566
def validate(self, attrs):
6667
"""
6768
Custom validation logic:
68-
parse the entity_key into (component_type, local_key).
69+
parse the entity_key into (component_type, component_code).
6970
"""
7071
entity_key = attrs["key"]
7172
try:
72-
component_type_obj, local_key = components_api.get_or_create_component_type_by_entity_key(entity_key)
73+
component_type_obj, component_code = _get_or_create_component_type_by_entity_key(entity_key)
7374
attrs["component_type"] = component_type_obj
74-
attrs["local_key"] = local_key
75+
attrs["component_code"] = component_code
7576
except ValueError as exc:
7677
raise serializers.ValidationError({"key": str(exc)})
7778
return attrs
7879

7980

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 do 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+
80112
class ComponentVersionSerializer(EntityVersionSerializer): # pylint: disable=abstract-method
81113
"""
82114
Serializer for component versions.

src/openedx_content/applets/backup_restore/zipper.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -332,7 +332,7 @@ def create_zip(self, path: str) -> None:
332332
# v1/
333333
# static/
334334

335-
entity_filename = self.get_entity_toml_filename(entity.component.local_key)
335+
entity_filename = self.get_entity_toml_filename(entity.component.component_code)
336336

337337
component_root_folder = (
338338
# Example: "entities/xblock.v1/html/"

src/openedx_content/applets/components/api.py

Lines changed: 21 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -40,10 +40,10 @@
4040
"create_next_component_version",
4141
"create_component_and_version",
4242
"get_component",
43-
"get_component_by_key",
43+
"get_component_by_code",
4444
"get_component_by_uuid",
4545
"get_component_version_by_uuid",
46-
"component_exists_by_key",
46+
"component_exists_by_code",
4747
"get_collection_components",
4848
"get_components",
4949
"create_component_version_media",
@@ -79,27 +79,27 @@ def get_or_create_component_type_by_entity_key(entity_key: str) -> tuple[Compone
7979
Get or create a ComponentType based on a full entity key string.
8080
8181
The entity key is expected to be in the format
82-
``"{namespace}:{type_name}:{local_key}"``. This function will parse out the
83-
``namespace`` and ``type_name`` parts and use those to get or create the
82+
``"{namespace}:{type_name}:{component_code}"``. This function will parse out
83+
the ``namespace`` and ``type_name`` parts and use those to get or create the
8484
ComponentType.
8585
8686
Raises ValueError if the entity_key is not in the expected format.
8787
"""
8888
try:
89-
namespace, type_name, local_key = entity_key.split(':', 2)
89+
namespace, type_name, component_code = entity_key.split(':', 2)
9090
except ValueError as exc:
9191
raise ValueError(
9292
f"Invalid entity_key format: {entity_key!r}. "
93-
"Expected format: '{namespace}:{type_name}:{local_key}'"
93+
"Expected format: '{namespace}:{type_name}:{component_code}'"
9494
) from exc
95-
return get_or_create_component_type(namespace, type_name), local_key
95+
return get_or_create_component_type(namespace, type_name), component_code
9696

9797

9898
def create_component(
9999
learning_package_id: LearningPackage.ID,
100100
/,
101101
component_type: ComponentType,
102-
local_key: str,
102+
component_code: str,
103103
created: datetime,
104104
created_by: int | None,
105105
*,
@@ -108,7 +108,7 @@ def create_component(
108108
"""
109109
Create a new Component (an entity like a Problem or Video)
110110
"""
111-
key = f"{component_type.namespace}:{component_type.name}:{local_key}"
111+
key = f"{component_type.namespace}:{component_type.name}:{component_code}"
112112
with atomic():
113113
publishable_entity = publishing_api.create_publishable_entity(
114114
learning_package_id,
@@ -121,7 +121,7 @@ def create_component(
121121
publishable_entity=publishable_entity,
122122
learning_package_id=learning_package_id,
123123
component_type=component_type,
124-
local_key=local_key,
124+
component_code=component_code,
125125
)
126126
return component
127127

@@ -293,7 +293,7 @@ def create_component_and_version( # pylint: disable=too-many-positional-argumen
293293
learning_package_id: LearningPackage.ID,
294294
/,
295295
component_type: ComponentType,
296-
local_key: str,
296+
component_code: str,
297297
title: str,
298298
created: datetime,
299299
created_by: int | None = None,
@@ -307,7 +307,7 @@ def create_component_and_version( # pylint: disable=too-many-positional-argumen
307307
component = create_component(
308308
learning_package_id,
309309
component_type,
310-
local_key,
310+
component_code,
311311
created,
312312
created_by,
313313
can_stand_alone=can_stand_alone,
@@ -331,22 +331,22 @@ def get_component(component_id: Component.ID, /) -> Component:
331331
return Component.with_publishing_relations.get(pk=component_id)
332332

333333

334-
def get_component_by_key(
334+
def get_component_by_code(
335335
learning_package_id: LearningPackage.ID,
336336
/,
337337
namespace: str,
338338
type_name: str,
339-
local_key: str,
339+
component_code: str,
340340
) -> Component:
341341
"""
342-
Get a Component by its unique (namespace, type, local_key) tuple.
342+
Get a Component by its unique (namespace, type, component_code) tuple.
343343
"""
344344
return Component.with_publishing_relations \
345345
.get(
346346
learning_package_id=learning_package_id,
347347
component_type__namespace=namespace,
348348
component_type__name=type_name,
349-
local_key=local_key,
349+
component_code=component_code,
350350
)
351351

352352

@@ -366,12 +366,12 @@ def get_component_version_by_uuid(uuid: UUID) -> ComponentVersion:
366366
)
367367

368368

369-
def component_exists_by_key(
369+
def component_exists_by_code(
370370
learning_package_id: LearningPackage.ID,
371371
/,
372372
namespace: str,
373373
type_name: str,
374-
local_key: str
374+
component_code: str
375375
) -> bool:
376376
"""
377377
Return True/False for whether a Component exists.
@@ -384,7 +384,7 @@ def component_exists_by_key(
384384
learning_package_id=learning_package_id,
385385
component_type__namespace=namespace,
386386
component_type__name=type_name,
387-
local_key=local_key,
387+
component_code=component_code,
388388
)
389389
return True
390390
except Component.DoesNotExist:
@@ -423,12 +423,12 @@ def get_components( # pylint: disable=too-many-positional-arguments
423423
if draft_title is not None:
424424
qset = qset.filter(
425425
Q(publishable_entity__draft__version__title__icontains=draft_title) |
426-
Q(local_key__icontains=draft_title)
426+
Q(component_code__icontains=draft_title)
427427
)
428428
if published_title is not None:
429429
qset = qset.filter(
430430
Q(publishable_entity__published__version__title__icontains=published_title) |
431-
Q(local_key__icontains=published_title)
431+
Q(component_code__icontains=published_title)
432432
)
433433

434434
return qset

src/openedx_content/applets/components/models.py

Lines changed: 20 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@
2222
from django.db import models
2323
from typing_extensions import deprecated
2424

25-
from openedx_django_lib.fields import case_sensitive_char_field, key_field
25+
from openedx_django_lib.fields import case_sensitive_char_field, code_field, code_field_check, key_field
2626
from openedx_django_lib.managers import WithRelationsManager
2727

2828
from ..media.models import Media
@@ -119,9 +119,10 @@ class Component(PublishableEntityMixin):
119119
State Consistency
120120
-----------------
121121
122-
The ``key`` field on Component's ``publishable_entity`` is dervied from the
123-
``component_type`` and ``local_key`` fields in this model. We don't support
124-
changing the keys yet, but if we do, those values need to be kept in sync.
122+
The ``key`` field on Component's ``publishable_entity`` is derived from the
123+
``component_type`` and ``component_code`` fields in this model. We don't
124+
support changing the keys yet, but if we do, those values need to be kept
125+
in sync.
125126
126127
How build on this model
127128
-----------------------
@@ -176,37 +177,38 @@ def pk(self):
176177
# XBlock block_type, but we want it to be more flexible in the long term.
177178
component_type = models.ForeignKey(ComponentType, on_delete=models.PROTECT)
178179

179-
# local_key is an identifier that is local to the learning_package and
180-
# component_type. The publishable.key should be calculated as a
181-
# combination of component_type and local_key.
182-
local_key = key_field()
180+
# component_code is an identifier that is local to the learning_package and
181+
# component_type. The publishable.key is derived from component_type and
182+
# component_code.
183+
component_code = code_field()
183184

184185
class Meta:
185186
constraints = [
186-
# The combination of (component_type, local_key) is unique within
187-
# a given LearningPackage. Note that this means it is possible to
188-
# have two Components in the same LearningPackage to have the same
189-
# local_key if the component_types are different. So for example,
190-
# you could have a ProblemBlock and VideoBlock that both have the
191-
# local_key "week_1".
187+
# The combination of (component_type, component_code) is unique
188+
# within a given LearningPackage. Note that this means it is
189+
# possible to have two Components in the same LearningPackage with
190+
# the same component_code if their component_types differ. For
191+
# example, a ProblemBlock and VideoBlock could both have the
192+
# component_code "week_1".
192193
models.UniqueConstraint(
193194
fields=[
194195
"learning_package",
195196
"component_type",
196-
"local_key",
197+
"component_code",
197198
],
198199
name="oel_component_uniq_lc_ct_lk",
199200
),
201+
code_field_check("component_code", name="oel_component_code_regex"),
200202
]
201203
indexes = [
202-
# Global Component-Type/Local-Key Index:
204+
# Global Component-Type/Component-Code Index:
203205
# * Search by the different Components fields across all Learning
204206
# Packages on the site. This would be a support-oriented tool
205207
# from Django Admin.
206208
models.Index(
207209
fields=[
208210
"component_type",
209-
"local_key",
211+
"component_code",
210212
],
211213
name="oel_component_idx_ct_lk",
212214
),
@@ -217,7 +219,7 @@ class Meta:
217219
verbose_name_plural = "Components"
218220

219221
def __str__(self) -> str:
220-
return f"{self.component_type.namespace}:{self.component_type.name}:{self.local_key}"
222+
return f"{self.component_type.namespace}:{self.component_type.name}:{self.component_code}"
221223

222224

223225
class ComponentVersion(PublishableEntityVersionMixin):

src/openedx_content/applets/publishing/models/publishable_entity.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -415,7 +415,7 @@ class VersioningHelper:
415415
learning_package_id=learning_package.id,
416416
namespace="xblock.v1",
417417
type="problem",
418-
local_key="monty_hall",
418+
component_code="monty_hall",
419419
title="Monty Hall Problem",
420420
created=now,
421421
created_by=None,

src/openedx_content/management/commands/add_assets_to_component.py

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99

1010
from django.core.management.base import BaseCommand
1111

12-
from ...api import create_next_component_version, get_component_by_key, get_learning_package_by_key
12+
from ...api import create_next_component_version, get_component_by_code, get_learning_package_by_key
1313

1414

1515
class Command(BaseCommand):
@@ -59,22 +59,22 @@ def handle(self, *args, **options):
5959

6060
learning_package = get_learning_package_by_key(learning_package_key)
6161
# Parse something like: "xblock.v1:problem:area_of_circle_1"
62-
namespace, type_name, local_key = component_key.split(":", 2)
63-
component = get_component_by_key(
64-
learning_package.id, namespace, type_name, local_key
62+
namespace, type_name, component_code = component_key.split(":", 2)
63+
component = get_component_by_code(
64+
learning_package.id, namespace, type_name, component_code
6565
)
6666

6767
created = datetime.now(tz=timezone.utc)
68-
local_keys_to_content_bytes = {}
68+
media_path_to_content_bytes = {}
6969

7070
for file_mapping in file_mappings:
71-
local_key, file_path = file_mapping.split(":", 1)
71+
media_path, file_path = file_mapping.split(":", 1)
7272

73-
local_keys_to_content_bytes[local_key] = pathlib.Path(file_path).read_bytes() if file_path else None
73+
media_path_to_content_bytes[media_path] = pathlib.Path(file_path).read_bytes() if file_path else None
7474

7575
next_version = create_next_component_version(
7676
component.id,
77-
media_to_replace=local_keys_to_content_bytes,
77+
media_to_replace=media_path_to_content_bytes,
7878
created=created,
7979
)
8080

0 commit comments

Comments
 (0)