diff --git a/contentcuration/kolibri_content/constants/schema_versions.py b/contentcuration/kolibri_content/constants/schema_versions.py index 86e930ee52..92daccb888 100644 --- a/contentcuration/kolibri_content/constants/schema_versions.py +++ b/contentcuration/kolibri_content/constants/schema_versions.py @@ -1,6 +1,8 @@ """ This is a direct copy of the constants file of the same name in kolibri.core.content https://github.com/learningequality/kolibri/blob/c7417e1d558a1e1e52ac8423927d61a0e44da576/kolibri/core/content/constants/schema_versions.py +EXPORT_SCHEMA_VERSIONS is copied from +https://github.com/learningequality/kolibri/blob/deeec22db9f9e607714bb8ea2c8b3b377f02a5ab/kolibri/core/content/constants/schema_versions.py """ V020BETA1 = "v0.2.0-beta1" @@ -48,3 +50,11 @@ # exceptionally difficult for us to backfill, such as deleting a model field that # we cannot meaningfully infer the content of from other metadata. MIN_CONTENT_SCHEMA_VERSION = VERSION_5 + +# Every version we can provide import metadata for, oldest first. +EXPORT_SCHEMA_VERSIONS = [ + str(version) + for version in range( + int(MIN_CONTENT_SCHEMA_VERSION), int(CONTENT_SCHEMA_VERSION) + 1 + ) +] diff --git a/contentcuration/kolibri_content/contentschema/columns/__init__.py b/contentcuration/kolibri_content/contentschema/columns/__init__.py new file mode 100644 index 0000000000..6e4e1ddbc2 --- /dev/null +++ b/contentcuration/kolibri_content/contentschema/columns/__init__.py @@ -0,0 +1,17 @@ +""" +This is a copy with modifications of the file: +https://github.com/learningequality/kolibri/blob/deeec22db9f9e607714bb8ea2c8b3b377f02a5ab/kolibri/core/content/contentschema/columns/__init__.py +""" +import importlib + +from kolibri_content.constants.schema_versions import EXPORT_SCHEMA_VERSIONS + + +def for_version(version): + """ + A content schema version's table names mapped to its column names, in + declaration order. + """ + if version not in EXPORT_SCHEMA_VERSIONS: + raise ValueError(f"Unknown content schema version {version}") + return importlib.import_module(".content_columns_" + version, __name__).COLUMNS diff --git a/contentcuration/kolibri_content/contentschema/columns/content_columns_5.py b/contentcuration/kolibri_content/contentschema/columns/content_columns_5.py new file mode 100644 index 0000000000..b240d2f4a7 --- /dev/null +++ b/contentcuration/kolibri_content/contentschema/columns/content_columns_5.py @@ -0,0 +1,83 @@ +""" +This is a direct copy of the file of the same name in kolibri.core.content.contentschema.columns +https://github.com/learningequality/kolibri/blob/deeec22db9f9e607714bb8ea2c8b3b377f02a5ab/kolibri/core/content/contentschema/columns/content_columns_5.py +""" + +COLUMNS = { + "content_assessmentmetadata": ( + "id", + "assessment_item_ids", + "number_of_assessments", + "mastery_model", + "randomize", + "is_manipulable", + "contentnode_id", + ), + "content_channelmetadata": ( + "id", + "name", + "description", + "tagline", + "author", + "version", + "thumbnail", + "last_updated", + "min_schema_version", + "root_id", + ), + "content_contentnode": ( + "id", + "license_name", + "license_description", + "title", + "coach_content", + "content_id", + "channel_id", + "description", + "sort_order", + "license_owner", + "author", + "kind", + "available", + "options", + "grade_levels", + "resource_types", + "learning_activities", + "accessibility_labels", + "categories", + "learner_needs", + "duration", + "lft", + "rght", + "tree_id", + "level", + "lang_id", + "parent_id", + ), + "content_contentnode_has_prerequisite": ( + "id", + "from_contentnode_id", + "to_contentnode_id", + ), + "content_contentnode_related": ("id", "from_contentnode_id", "to_contentnode_id"), + "content_contentnode_tags": ("id", "contentnode_id", "contenttag_id"), + "content_contenttag": ("id", "tag_name"), + "content_file": ( + "id", + "preset", + "supplementary", + "thumbnail", + "priority", + "contentnode_id", + "lang_id", + "local_file_id", + ), + "content_language": ( + "id", + "lang_code", + "lang_subcode", + "lang_name", + "lang_direction", + ), + "content_localfile": ("id", "extension", "available", "file_size"), +} diff --git a/contentcuration/kolibri_content/contentschema/columns/content_columns_6.py b/contentcuration/kolibri_content/contentschema/columns/content_columns_6.py new file mode 100644 index 0000000000..173bf6df62 --- /dev/null +++ b/contentcuration/kolibri_content/contentschema/columns/content_columns_6.py @@ -0,0 +1,84 @@ +""" +This is a direct copy of the file of the same name in kolibri.core.content.contentschema.columns +https://github.com/learningequality/kolibri/blob/deeec22db9f9e607714bb8ea2c8b3b377f02a5ab/kolibri/core/content/contentschema/columns/content_columns_6.py +""" + +COLUMNS = { + "content_assessmentmetadata": ( + "id", + "assessment_item_ids", + "number_of_assessments", + "mastery_model", + "randomize", + "is_manipulable", + "contentnode_id", + ), + "content_channelmetadata": ( + "id", + "name", + "description", + "tagline", + "author", + "version", + "thumbnail", + "last_updated", + "min_schema_version", + "root_id", + ), + "content_contentnode": ( + "id", + "license_name", + "license_description", + "title", + "coach_content", + "content_id", + "channel_id", + "description", + "sort_order", + "license_owner", + "author", + "kind", + "available", + "options", + "grade_levels", + "resource_types", + "learning_activities", + "accessibility_labels", + "categories", + "learner_needs", + "duration", + "lft", + "rght", + "tree_id", + "level", + "lang_id", + "parent_id", + ), + "content_contentnode_has_prerequisite": ( + "id", + "from_contentnode_id", + "to_contentnode_id", + ), + "content_contentnode_related": ("id", "from_contentnode_id", "to_contentnode_id"), + "content_contentnode_tags": ("id", "contentnode_id", "contenttag_id"), + "content_contenttag": ("id", "tag_name"), + "content_file": ( + "id", + "preset", + "supplementary", + "thumbnail", + "priority", + "included_presets", + "contentnode_id", + "lang_id", + "local_file_id", + ), + "content_language": ( + "id", + "lang_code", + "lang_subcode", + "lang_name", + "lang_direction", + ), + "content_localfile": ("id", "extension", "available", "file_size_bigint"), +} diff --git a/contentcuration/kolibri_public/import_metadata_view.py b/contentcuration/kolibri_public/import_metadata_view.py index 5bb71ab8d5..25ffc2092f 100644 --- a/contentcuration/kolibri_public/import_metadata_view.py +++ b/contentcuration/kolibri_public/import_metadata_view.py @@ -6,19 +6,20 @@ from django.db import connection from django.db.models import Q -from django.http import HttpResponseBadRequest from django.utils.decorators import method_decorator -from kolibri_content import base_models from kolibri_content import models as kolibri_content_models from kolibri_content.constants.schema_versions import ( CONTENT_SCHEMA_VERSION, ) # Use kolibri_content +from kolibri_content.constants.schema_versions import EXPORT_SCHEMA_VERSIONS from kolibri_content.constants.schema_versions import ( MIN_CONTENT_SCHEMA_VERSION, ) # Use kolibri_content +from kolibri_content.contentschema.columns import for_version from kolibri_public import models # Use kolibri_public models from kolibri_public.views import metadata_cache from rest_framework import status +from rest_framework.exceptions import ValidationError from rest_framework.generics import get_object_or_404 from rest_framework.permissions import AllowAny from rest_framework.response import Response @@ -27,20 +28,26 @@ from contentcuration.utils.pagination import ValuesViewsetCursorPagination -def _get_kc_and_base_models(model): +# Clients that send no schema_version predate kolibri#15358 and may read any supported version. +DEFAULT_COLUMNS = { + table: tuple( + dict.fromkeys( + column + for version in EXPORT_SCHEMA_VERSIONS + for column in for_version(version)[table] + ) + ) + for table in for_version(CONTENT_SCHEMA_VERSION) +} + + +def _get_kc_model(model): try: - kc_model = getattr(kolibri_content_models, model.__name__) - base_model = getattr(base_models, model.__name__) + return getattr(kolibri_content_models, model.__name__) except AttributeError: # This will happen if it's a M2M through model, which only exist on ContentNode through_model_name = model.__name__.replace("ContentNode_", "") - kc_model = getattr( - kolibri_content_models.ContentNode, through_model_name - ).through - # Through models are not defined for the abstract base models, so we just cheat and - # use these instead. - base_model = kc_model - return kc_model, base_model + return getattr(kolibri_content_models.ContentNode, through_model_name).through class ImportMetadataPagination(ValuesViewsetCursorPagination): @@ -75,7 +82,20 @@ def _error_message(self, low): ) return error - def retrieve(self, request, pk=None): # noqa: C901 + def _validate_content_schema(self, content_schema): + try: + version = int(content_schema) + except ValueError as e: + raise ValidationError( + "Schema version is not parseable by this version of Kolibri" + ) from e + if version > int(self.default_content_schema): + raise ValidationError(self._error_message(False)) + if version < int(self.min_content_schema): + raise ValidationError(self._error_message(True)) + return str(version) + + def retrieve(self, request, pk=None): """ An endpoint to retrieve all content metadata required for importing a content node all of its ancestors, and any relevant needed metadata. @@ -92,24 +112,13 @@ def retrieve(self, request, pk=None): # noqa: C901 {"error": "Invalid UUID format."}, status=status.HTTP_400_BAD_REQUEST ) - content_schema = request.query_params.get( - "schema_version", self.default_content_schema - ) - - try: - if int(content_schema) > int(self.default_content_schema): - return HttpResponseBadRequest(self._error_message(False)) - if int(content_schema) < int(self.min_content_schema): - return HttpResponseBadRequest(self._error_message(True)) - # Remove reference to SQLAlchemy schema bases - except ValueError: - return HttpResponseBadRequest( - "Schema version is not parseable by this version of Kolibri" - ) - except AttributeError: - return HttpResponseBadRequest( - "Schema version is not known by this version of Kolibri" - ) + content_schema = request.query_params.get("schema_version") + if content_schema is None: + content_schema = self.default_content_schema + columns = DEFAULT_COLUMNS + else: + content_schema = self._validate_content_schema(content_schema) + columns = for_version(content_schema) # Get the model for the target node here - we do this so that we trigger a 404 immediately if the node # does not exist. @@ -163,32 +172,17 @@ def retrieve(self, request, pk=None): # noqa: C901 related, channel_metadata, ]: - # First get the kolibri_content model and base model to which this is equivalent - kc_model, base_model = _get_kc_and_base_models(qs.model) # Map the table name from the kolibri_public table name to the equivalent Kolibri table name - table_name = kc_model._meta.db_table - # Tweak our introspection here to rely on Django model meta instead of SQLAlchemy - # Read valid field names from the combination of the base model, and the mptt tree fields - # of the kc_model - because the base model is abstract, it does not get the mptt fields applied - # to its meta fields attribute, so we need to read the actual fields from the kc_model, but filter - # them only to names valid for the base model. - field_names = {field.column for field in base_model._meta.fields} - if hasattr(base_model, "_mptt_meta"): - field_names.add(base_model._mptt_meta.parent_attr) - field_names.add(base_model._mptt_meta.tree_id_attr) - field_names.add(base_model._mptt_meta.left_attr) - field_names.add(base_model._mptt_meta.right_attr) - field_names.add(base_model._mptt_meta.level_attr) - raw_fields = [ - field.column - for field in kc_model._meta.fields - if field.column in field_names - ] + table_name = _get_kc_model(qs.model)._meta.db_table + raw_fields = columns[table_name] + value_fields = raw_fields if qs.model is models.Language: - raw_fields = [rf for rf in raw_fields if rf != "lang_name"] + [ - "native_name" + # Studio's Language keeps the name Kolibri calls lang_name in native_name + value_fields = [ + "native_name" if field == "lang_name" else field + for field in raw_fields ] - qs = qs.values(*raw_fields) + qs = qs.values(*value_fields) # Avoid using the Django queryset directly, as it will coerce the database values # via its field 'from_db_value' transformers, whereas import metadata is read # directly from the database. @@ -208,10 +202,6 @@ def retrieve(self, request, pk=None): # noqa: C901 ) for row in cursor ] - if qs.model is models.Language: - for lang in data[table_name]: - lang["lang_name"] = lang["native_name"] - del lang["native_name"] data["schema_version"] = content_schema diff --git a/contentcuration/kolibri_public/tests/test_importmetadata_api.py b/contentcuration/kolibri_public/tests/test_importmetadata_api.py index c661b6cb39..ff1faa1473 100644 --- a/contentcuration/kolibri_public/tests/test_importmetadata_api.py +++ b/contentcuration/kolibri_public/tests/test_importmetadata_api.py @@ -8,11 +8,16 @@ from kolibri_content import base_models from kolibri_content import models as content from kolibri_content.constants.schema_versions import CONTENT_SCHEMA_VERSION +from kolibri_content.constants.schema_versions import EXPORT_SCHEMA_VERSIONS +from kolibri_content.constants.schema_versions import MIN_CONTENT_SCHEMA_VERSION +from kolibri_content.constants.schema_versions import VERSION_5 +from kolibri_content.contentschema.columns import for_version from kolibri_public import models as public from kolibri_public.tests.test_content_app import ChannelBuilder from le_utils.constants import content_kinds from rest_framework.test import APITestCase +from contentcuration.models import Language from contentcuration.tests.helpers import reverse_with_query @@ -24,6 +29,15 @@ def setUpTestData(cls): public.ContentNode.objects.all().update(available=True) cls.root = public.ContentNode.objects.get(id=cls.builder.root_node["id"]) cls.node = cls.root.get_descendants().exclude(kind=content_kinds.TOPIC).first() + cls.language = Language.objects.get_or_create( + id="fr", + defaults={ + "lang_code": "fr", + "readable_name": "French", + "native_name": "Français", + }, + )[0] + public.ContentNode.objects.filter(id=cls.node.id).update(lang=cls.language) cls.all_nodes = cls.node.get_ancestors(include_self=True) cls.files = public.File.objects.filter(contentnode__in=cls.all_nodes) cls.assessmentmetadata = public.AssessmentMetaData.objects.filter( @@ -47,9 +61,7 @@ def setUpTestData(cls): cls.family_ids = list(cls.topic.get_family().values_list("id", flat=True)) def _assert_data(self, Model, ContentModel, queryset): - response = self.client.get( - reverse("publicimportmetadata-detail", kwargs={"pk": self.node.id}) - ) + response = self._get() fields = Model._meta.fields BaseModel = getattr(base_models, Model.__name__, Model) field_names = {field.column for field in BaseModel._meta.fields} @@ -125,6 +137,60 @@ def test_import_metadata_paginated_descendants(self): self.family_ids, ) + def _get(self, schema_version=None): + url = reverse("publicimportmetadata-detail", kwargs={"pk": self.node.id}) + if schema_version is not None: + url += "?schema_version={}".format(schema_version) + return self.client.get(url) + + def _get_metadata(self, schema_version=None): + response = self._get(schema_version) + self.assertEqual(response.status_code, 200) + return response.data + + def test_no_schema_version_serves_every_supported_column(self): + data = self._get_metadata() + self.assertEqual(data["schema_version"], CONTENT_SCHEMA_VERSION) + for table in for_version(CONTENT_SCHEMA_VERSION): + rows = data[table] + if not rows: + continue + with self.subTest(table=table): + self.assertEqual( + set(rows[0]), + set().union( + *(for_version(v)[table] for v in EXPORT_SCHEMA_VERSIONS) + ), + ) + + def test_language_lang_name_is_native_name(self): + for schema_version in [None] + EXPORT_SCHEMA_VERSIONS: + with self.subTest(schema_version=schema_version): + languages = self._get_metadata(schema_version)[ + content.Language._meta.db_table + ] + self.assertEqual( + [row["lang_name"] for row in languages], + [self.language.native_name], + ) + + def test_import_metadata_columns_match_frozen_map(self): + for version in EXPORT_SCHEMA_VERSIONS: + data = self._get_metadata(version) + self.assertEqual(data["schema_version"], version) + # Iterate the frozen map rather than the response, as the response + # also carries a non-table `schema_version` key. + for table, columns in for_version(version).items(): + self.assertIn(table, data) + rows = data[table] + if not rows: + continue + with self.subTest(version=version, table=table): + self.assertEqual(sorted(rows[0].keys()), sorted(columns)) + + def test_schema_version_is_normalised(self): + self.assertEqual(self._get_metadata("05"), self._get_metadata(VERSION_5)) + def test_import_metadata_nodes(self): self._assert_data(public.ContentNode, content.ContentNode, self.all_nodes) @@ -165,34 +231,41 @@ def test_import_metadata_invalid_uuid(self): self.assertEqual(response.data["error"], "Invalid UUID format.") + def _assert_schema_error(self, schema_version, message): + response = self._get(schema_version) + self.assertEqual(response.status_code, 400) + self.assertEqual(response["Content-Type"], "application/json") + self.assertEqual(response.json(), [message]) + def test_schema_version_too_low(self): - response = self.client.get( - reverse("publicimportmetadata-detail", kwargs={"pk": self.node.id}) - + "?schema_version=1" + self._assert_schema_error( + "1", + "Schema version is too low, exports only suported for versions {} to {}".format( + MIN_CONTENT_SCHEMA_VERSION, CONTENT_SCHEMA_VERSION + ), ) - self.assertEqual(response.status_code, 400) def test_schema_version_too_high(self): - response = self.client.get( - reverse("publicimportmetadata-detail", kwargs={"pk": self.node.id}) - + "?schema_version={}".format(int(CONTENT_SCHEMA_VERSION) + 1) + self._assert_schema_error( + int(CONTENT_SCHEMA_VERSION) + 1, + "Schema version is too high, exports only suported for versions {} to {}".format( + MIN_CONTENT_SCHEMA_VERSION, CONTENT_SCHEMA_VERSION + ), ) - self.assertEqual(response.status_code, 400) - def test_schema_version_just_right(self): - response = self.client.get( - reverse("publicimportmetadata-detail", kwargs={"pk": self.node.id}) - + "?schema_version={}".format(CONTENT_SCHEMA_VERSION) - ) - self.assertEqual(response.status_code, 200) + def test_schema_version_unparseable(self): + for schema_version in ("abc", ""): + with self.subTest(schema_version=schema_version): + self._assert_schema_error( + schema_version, + "Schema version is not parseable by this version of Kolibri", + ) def test_headers(self): channel = public.ChannelMetadata.objects.get() channel.last_updated = datetime.datetime.now() channel.save() - response = self.client.get( - reverse("publicimportmetadata-detail", kwargs={"pk": self.node.id}) - ) + response = self._get() self.assertEqual(response.headers["Vary"], "Accept") self.assertEqual( response.headers["Cache-Control"],