refactor: normalize component types with ComponentType (#148)

Prior to this commit, Components encoded their namespace + type info as
two columns on the Component model itself. This wasted a lot of space on
a very common index lookup–many rows of ('xblock.v1', 'video'),
('xblock.v1', 'problem'), and ('xblock.v1', 'text'). But worse, the lack
of a separate ComponentType model meant that there was no first class
entity to store per-type policy data against. For example, we might want
to say "the following types are supported by libraries, while these
other types are experimental", or "these types are enabled for these
orgs", etc.

Components are required to have a ComponentType. We're rewriting the
first migration for the components app here, since this app hasn't been
added to edx-platform yet.

olx_importer/management/commands/load_components.py

@@ -158,7 +158,7 @@ def import_block_type(self, block_type, now): # , publish_log_entry):
             _component, component_version = components_api.create_component_and_version(
                 self.learning_package.id,
                 namespace=namespace,
-                type=block_type,
+                type_name=block_type,
                 local_key=local_key,
                 title=display_name,
                 created=now,

openedx_learning/core/components/admin.py

@@ -35,16 +35,15 @@ class ComponentAdmin(ReadOnlyModelAdmin):
     """
     Django admin configuration for Component
     """
-    list_display = ("key", "uuid", "namespace", "type", "created")
+    list_display = ("key", "uuid", "component_type", "created")
     readonly_fields = [
         "learning_package",
         "uuid",
-        "namespace",
-        "type",
+        "component_type",
         "key",
         "created",
     ]
-    list_filter = ("type", "learning_package")
+    list_filter = ("component_type", "learning_package")
     search_fields = ["publishable_entity__uuid", "publishable_entity__key"]
     inlines = [ComponentVersionInline]
 

openedx_learning/core/components/api.py

@@ -18,32 +18,44 @@
 from django.db.models import Q, QuerySet
 from django.db.transaction import atomic
 
+from ...lib.cache import lru_cache
 from ..publishing import api as publishing_api
-from .models import Component, ComponentVersion, ComponentVersionRawContent
+from .models import Component, ComponentType, ComponentVersion, ComponentVersionRawContent
+
+
+@lru_cache(maxsize=128)
+def get_or_create_component_type_id(namespace: str, name: str) -> int:
+    """
+    Get the ID of a ComponentType, and create if missing.
+    """
+    component_type, _created = ComponentType.objects.get_or_create(
+        namespace=namespace,
+        name=name,
+    )
+    return component_type.id
 
 
 def create_component(
     learning_package_id: int,
     /,
     namespace: str,
-    type: str,  # pylint: disable=redefined-builtin
+    type_name: str,
     local_key: str,
     created: datetime,
     created_by: int | None,
 ) -> Component:
     """
     Create a new Component (an entity like a Problem or Video)
     """
-    key = f"{namespace}:{type}@{local_key}"
+    key = f"{namespace}:{type_name}@{local_key}"
     with atomic():
         publishable_entity = publishing_api.create_publishable_entity(
             learning_package_id, key, created, created_by
         )
         component = Component.objects.create(
             publishable_entity=publishable_entity,
             learning_package_id=learning_package_id,
-            namespace=namespace,
-            type=type,
+            component_type_id=get_or_create_component_type_id(namespace, type_name),
             local_key=local_key,
         )
     return component
@@ -163,7 +175,7 @@ def create_component_and_version(
     learning_package_id: int,
     /,
     namespace: str,
-    type: str,  # pylint: disable=redefined-builtin
+    type_name: str,
     local_key: str,
     title: str,
     created: datetime,
@@ -174,7 +186,7 @@ def create_component_and_version(
     """
     with atomic():
         component = create_component(
-            learning_package_id, namespace, type, local_key, created, created_by
+            learning_package_id, namespace, type_name, local_key, created, created_by
         )
         component_version = create_component_version(
             component.pk,
@@ -199,7 +211,7 @@ def get_component_by_key(
     learning_package_id: int,
     /,
     namespace: str,
-    type: str,  # pylint: disable=redefined-builtin
+    type_name: str,
     local_key: str,
 ) -> Component:
     """
@@ -208,8 +220,8 @@ def get_component_by_key(
     return Component.with_publishing_relations \
                     .get(
                         learning_package_id=learning_package_id,
-                        namespace=namespace,
-                        type=type,
+                        component_type__namespace=namespace,
+                        component_type__name=type_name,
                         local_key=local_key,
                     )
 
@@ -218,7 +230,7 @@ def component_exists_by_key(
     learning_package_id: int,
     /,
     namespace: str,
-    type: str,  # pylint: disable=redefined-builtin
+    type_name: str,
     local_key: str
 ) -> bool:
     """
@@ -228,10 +240,10 @@ def component_exists_by_key(
     no current Draft version for it), or if it's been unpublished.
     """
     try:
-        _component = Component.objects.only('pk').get(
+        _component = Component.objects.only('pk', 'component_type').get(
             learning_package_id=learning_package_id,
-            namespace=namespace,
-            type=type,
+            component_type__namespace=namespace,
+            component_type__name=type_name,
             local_key=local_key,
         )
         return True
@@ -245,7 +257,7 @@ def get_components(
     draft: bool | None = None,
     published: bool | None = None,
     namespace: str | None = None,
-    types: list[str] | None = None,
+    type_names: list[str] | None = None,
     draft_title: str | None = None,
     published_title: str | None = None,
 ) -> QuerySet[Component]:
@@ -265,9 +277,9 @@ def get_components(
     if published is not None:
         qset = qset.filter(publishable_entity__published__version__isnull=not published)
     if namespace is not None:
-        qset = qset.filter(namespace=namespace)
-    if types is not None:
-        qset = qset.filter(type__in=types)
+        qset = qset.filter(component_type__namespace=namespace)
+    if type_names is not None:
+        qset = qset.filter(component_type__name__in=type_names)
     if draft_title is not None:
         qset = qset.filter(
             publishable_entity__draft__version__title__icontains=draft_title

openedx_learning/core/components/migrations/0001_initial.py

@@ -1,4 +1,4 @@
-# Generated by Django 3.2.23 on 2024-01-22 00:38
+# Generated by Django 3.2.23 on 2024-01-31 05:34
 
 import uuid
 
@@ -13,25 +13,30 @@ class Migration(migrations.Migration):
     initial = True
 
     dependencies = [
-        ('oel_contents', '0001_initial'),
         ('oel_publishing', '0001_initial'),
+        ('oel_contents', '0001_initial'),
     ]
 
     operations = [
         migrations.CreateModel(
             name='Component',
             fields=[
                 ('publishable_entity', models.OneToOneField(on_delete=django.db.models.deletion.CASCADE, primary_key=True, serialize=False, to='oel_publishing.publishableentity')),
-                ('namespace', openedx_learning.lib.fields.MultiCollationCharField(db_collations={'mysql': 'utf8mb4_bin', 'sqlite': 'BINARY'}, max_length=100)),
-                ('type', openedx_learning.lib.fields.MultiCollationCharField(blank=True, db_collations={'mysql': 'utf8mb4_bin', 'sqlite': 'BINARY'}, max_length=100)),
                 ('local_key', openedx_learning.lib.fields.MultiCollationCharField(db_collations={'mysql': 'utf8mb4_bin', 'sqlite': 'BINARY'}, max_length=500)),
-                ('learning_package', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to='oel_publishing.learningpackage')),
             ],
             options={
                 'verbose_name': 'Component',
                 'verbose_name_plural': 'Components',
             },
         ),
+        migrations.CreateModel(
+            name='ComponentType',
+            fields=[
+                ('id', models.AutoField(primary_key=True, serialize=False)),
+                ('namespace', openedx_learning.lib.fields.MultiCollationCharField(db_collations={'mysql': 'utf8mb4_bin', 'sqlite': 'BINARY'}, max_length=100)),
+                ('name', openedx_learning.lib.fields.MultiCollationCharField(blank=True, db_collations={'mysql': 'utf8mb4_bin', 'sqlite': 'BINARY'}, max_length=100)),
+            ],
+        ),
         migrations.CreateModel(
             name='ComponentVersion',
             fields=[
@@ -59,6 +64,16 @@ class Migration(migrations.Migration):
             name='raw_contents',
             field=models.ManyToManyField(related_name='component_versions', through='oel_components.ComponentVersionRawContent', to='oel_contents.RawContent'),
         ),
+        migrations.AddField(
+            model_name='component',
+            name='component_type',
+            field=models.ForeignKey(on_delete=django.db.models.deletion.PROTECT, to='oel_components.componenttype'),
+        ),
+        migrations.AddField(
+            model_name='component',
+            name='learning_package',
+            field=models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to='oel_publishing.learningpackage'),
+        ),
         migrations.AddIndex(
             model_name='componentversionrawcontent',
             index=models.Index(fields=['raw_content', 'component_version'], name='oel_cvrawcontent_c_cv'),
@@ -73,10 +88,10 @@ class Migration(migrations.Migration):
         ),
         migrations.AddIndex(
             model_name='component',
-            index=models.Index(fields=['learning_package', 'namespace', 'type', 'local_key'], name='oel_component_idx_lc_ns_t_lk'),
+            index=models.Index(fields=['component_type', 'local_key'], name='oel_component_idx_ct_lk'),
         ),
         migrations.AddConstraint(
             model_name='component',
-            constraint=models.UniqueConstraint(fields=('learning_package', 'namespace', 'type', 'local_key'), name='oel_component_uniq_lc_ns_t_lk'),
+            constraint=models.UniqueConstraint(fields=('learning_package', 'component_type', 'local_key'), name='oel_component_uniq_lc_ct_lk'),
         ),
     ]

openedx_learning/core/components/models.py

@@ -28,6 +28,45 @@
 from ..publishing.models import LearningPackage
 
 
+class ComponentType(models.Model):
+    """
+    Normalized representation of a type of Component.
+
+    The only namespace being used initially will be 'xblock.v1', but we will
+    probably add a few others over time, such as a component type to represent
+    packages of files for things like Files and Uploads or python_lib.zip files.
+
+    Make a ForeignKey against this table if you have to set policy based on the
+    type of Components–e.g. marking certain types of XBlocks as approved vs.
+    experimental for use in libraries.
+    """
+    id = models.AutoField(primary_key=True)
+
+    # namespace and name work together to help figure out what Component needs
+    # to handle this data. A namespace is *required*. The namespace for XBlocks
+    # is "xblock.v1" (to match the setup.py entrypoint naming scheme).
+    namespace = case_sensitive_char_field(max_length=100, blank=False)
+
+    # name is a way to help sub-divide namespace if that's convenient. This
+    # field cannot be null, but it can be blank if it's not necessary. For an
+    # XBlock, this corresponds to tag, e.g. "video". It's also the block_type in
+    # the UsageKey.
+    name = case_sensitive_char_field(max_length=100, blank=True)
+
+    constraints = [
+        models.UniqueConstraint(
+            fields=[
+                "namespace",
+                "name",
+            ],
+            name="oel_component_type_uniq_ns_n",
+        ),
+    ]
+
+    def __str__(self):
+        return f"{self.namespace}:{self.name}"
+
+
 class Component(PublishableEntityMixin):  # type: ignore[django-manager-missing]
     """
     This represents any Component that has ever existed in a LearningPackage.
@@ -63,7 +102,7 @@ class Component(PublishableEntityMixin):  # type: ignore[django-manager-missing]
     -----------------
 
     The ``key`` field on Component's ``publishable_entity`` is dervied from the
-    ``(namespace, type, local_key)`` fields in this model. We don't support
+    ``component_type`` and ``local_key`` fields in this model. We don't support
     changing the keys yet, but if we do, those values need to be kept in sync.
 
     How build on this model
@@ -75,9 +114,12 @@ class Component(PublishableEntityMixin):  # type: ignore[django-manager-missing]
     # Tell mypy what type our objects manager has.
     # It's actually PublishableEntityMixinManager, but that has the exact same
     # interface as the base manager class.
-    objects: models.Manager[Component]
+    objects: models.Manager[Component] = WithRelationsManager(
+        'component_type'
+    )
 
-    with_publishing_relations = WithRelationsManager(
+    with_publishing_relations: models.Manager[Component] = WithRelationsManager(
+        'component_type',
         'publishable_entity',
         'publishable_entity__draft__version',
         'publishable_entity__draft__version__componentversion',
@@ -93,53 +135,43 @@ class Component(PublishableEntityMixin):  # type: ignore[django-manager-missing]
     # columns from different tables).
     learning_package = models.ForeignKey(LearningPackage, on_delete=models.CASCADE)
 
-    # namespace and type work together to help figure out what Component needs
-    # to handle this data. A namespace is *required*. The namespace for XBlocks
-    # is "xblock.v1" (to match the setup.py entrypoint naming scheme).
-    namespace = case_sensitive_char_field(max_length=100, blank=False)
+    # What kind of Component are we? This will usually represent a specific
+    # XBlock block_type, but we want it to be more flexible in the long term.
+    component_type = models.ForeignKey(ComponentType, on_delete=models.PROTECT)
 
-    # type is a way to help sub-divide namespace if that's convenient. This
-    # field cannot be null, but it can be blank if it's not necessary. For an
-    # XBlock, type corresponds to tag, e.g. "video". It's also the block_type in
-    # the UsageKey.
-    type = case_sensitive_char_field(max_length=100, blank=True)
-
-    # local_key is an identifier that is local to the (namespace, type).  The
-    # publishable.key should be calculated as a combination of (namespace, type,
-    # local_key).
+    # local_key is an identifier that is local to the learning_package and
+    # component_type.  The publishable.key should be calculated as a
+    # combination of component_type and local_key.
     local_key = key_field()
 
     class Meta:
         constraints = [
-            # The combination of (namespace, type, local_key) is unique within
+            # The combination of (component_type, local_key) is unique within
             # a given LearningPackage. Note that this means it is possible to
-            # have two Components that have the exact same local_key. An XBlock
-            # would be modeled as namespace="xblock.v1" with the type as the
-            # block_type, so the local_key would only be the block_id (the
-            # very last part of the UsageKey).
+            # have two Components in the same LearningPackage to have the same
+            # local_key if the component_types are different. So for example,
+            # you could have a ProblemBlock and VideoBlock that both have the
+            # local_key "week_1".
             models.UniqueConstraint(
                 fields=[
                     "learning_package",
-                    "namespace",
-                    "type",
+                    "component_type",
                     "local_key",
                 ],
-                name="oel_component_uniq_lc_ns_t_lk",
+                name="oel_component_uniq_lc_ct_lk",
             ),
         ]
         indexes = [
-            # Global Namespace/Type/Local-Key Index:
+            # Global Component-Type/Local-Key Index:
             #   * Search by the different Components fields across all Learning
             #     Packages on the site. This would be a support-oriented tool
             #     from Django Admin.
             models.Index(
                 fields=[
-                    "learning_package",
-                    "namespace",
-                    "type",
+                    "component_type",
                     "local_key",
                 ],
-                name="oel_component_idx_lc_ns_t_lk",
+                name="oel_component_idx_ct_lk",
             ),
         ]
 
@@ -148,7 +180,7 @@ class Meta:
         verbose_name_plural = "Components"
 
     def __str__(self):
-        return f"{self.namespace}:{self.type}:{self.local_key}"
+        return f"{self.component_type.namespace}:{self.component_type.name}:{self.local_key}"
 
 
 class ComponentVersion(PublishableEntityVersionMixin):