Select the channel import class from the database's shape

kolibri/core/content/test/test_channel_import.py

@@ -4,6 +4,7 @@
 import tempfile
 import unittest
 import uuid
+from contextlib import closing
 from contextlib import contextmanager
 from contextlib import ExitStack
 
@@ -50,9 +51,11 @@
 from kolibri.core.content.utils.annotation import update_content_metadata
 from kolibri.core.content.utils.channel_import import BATCH_SIZE
 from kolibri.core.content.utils.channel_import import ChannelImport
+from kolibri.core.content.utils.channel_import import FutureSchemaError
 from kolibri.core.content.utils.channel_import import import_channel_from_data
 from kolibri.core.content.utils.channel_import import import_channel_from_local_db
 from kolibri.core.content.utils.channel_import import initialize_import_manager
+from kolibri.core.content.utils.channel_import import InvalidSchemaVersionError
 from kolibri.core.content.utils.channel_import import topological_sort
 from kolibri.core.content.utils.channels import read_channel_metadata_from_db_file
 from kolibri.core.content.utils.content_db import content_db
@@ -675,6 +678,9 @@ def setUp(self):
     def load_fixture_data(self):
         return load_content_fixture_data(self.data_name)
 
+    def amend_content_db(self, db_path):
+        pass
+
     @patch("kolibri.core.content.utils.channel_import.get_content_database_file_path")
     def set_content_fixture(self, db_path_mock):
         _, self.content_db_path = tempfile.mkstemp(suffix=".sqlite3")
@@ -683,6 +689,7 @@ def set_content_fixture(self, db_path_mock):
         build_content_db_from_frozen_schema(
             self.content_db_path, self.schema_name, self.load_fixture_data()
         )
+        self.amend_content_db(self.content_db_path)
 
         import_channel_from_local_db("6199dde695db4ee4ab392222d5af1e5c")
         update_content_metadata("6199dde695db4ee4ab392222d5af1e5c")
@@ -1542,3 +1549,74 @@ def test_file_size_matches_row_path(self):
             self._reimport_from_scratch()
 
         self.assertEqual(attached, self._localfile_file_sizes())
+
+
+class SupersetSchemaImportTestCase(NaiveImportTestCase):
+    """
+    A database shaped the way Studio publishes one: the current schema's columns,
+    the legacy file_size column its declared floor promises, and a
+    min_schema_version far below either.
+    """
+
+    name = CONTENT_SCHEMA_VERSION
+
+    # Two activities at once, which no kind maps to, so a backfill from kind shows up.
+    authored_learning_activities = "UD5UGM0z,wA01urpi"
+
+    def load_fixture_data(self):
+        data = super().load_fixture_data()
+        data["content_channelmetadata"][0]["min_schema_version"] = VERSION_1
+        for row in data["content_contentnode"]:
+            if row["kind"] != "topic":
+                row["learning_activities"] = self.authored_learning_activities
+        return data
+
+    def amend_content_db(self, db_path):
+        with closing(sqlite3.connect(db_path)) as connection:
+            connection.execute(
+                "ALTER TABLE content_localfile ADD COLUMN file_size INTEGER"
+            )
+            connection.execute(
+                "UPDATE content_localfile SET file_size = file_size_bigint"
+            )
+            connection.commit()
+
+    def test_authored_learning_activities_are_not_backfilled_from_kind(self):
+        imported = set(
+            ContentNode.objects.filter(channel_id="6199dde695db4ee4ab392222d5af1e5c")
+            .exclude(kind="topic")
+            .values_list("learning_activities", flat=True)
+        )
+
+        self.assertEqual({self.authored_learning_activities}, imported)
+
+
+class SchemaVersionSelectionTestCase(FrozenSchemaDBMixin, TestCase):
+    """
+    Which schema version picks the import class, and which one only gates it.
+    """
+
+    def build_declaring_floor(self, min_schema_version):
+        data = load_content_fixture_data(CONTENT_SCHEMA_VERSION)
+        data["content_channelmetadata"][0]["min_schema_version"] = min_schema_version
+        db_path = os.path.join(self.directory, "floor.sqlite3")
+        build_content_db_from_frozen_schema(db_path, CONTENT_SCHEMA_VERSION, data)
+        return db_path
+
+    def import_manager_for(self, min_schema_version):
+        db_path = self.build_declaring_floor(min_schema_version)
+        return initialize_import_manager(
+            read_channel_metadata_from_db_file(db_path), db_path
+        )
+
+    def test_the_shape_picks_the_class_over_a_lower_floor(self):
+        with self.import_manager_for(VERSION_1) as import_manager:
+            self.assertEqual(ChannelImport, type(import_manager))
+
+    def test_a_floor_above_this_kolibri_is_refused(self):
+        with self.assertRaises(FutureSchemaError):
+            self.import_manager_for(str(int(CONTENT_SCHEMA_VERSION) + 1))
+
+    def test_an_unreadable_floor_is_refused(self):
+        with self.assertRaises(InvalidSchemaVersionError):
+            self.import_manager_for("not-a-schema-version")

kolibri/core/content/test/utils/test_source_db.py

@@ -5,6 +5,8 @@
 
 from kolibri.core.content.constants.schema_versions import CONTENT_DB_SCHEMA_VERSIONS
 from kolibri.core.content.constants.schema_versions import V020BETA1
+from kolibri.core.content.constants.schema_versions import VERSION_2
+from kolibri.core.content.constants.schema_versions import VERSION_3
 from kolibri.core.content.constants.schema_versions import VERSION_6
 from kolibri.core.content.contentschema.columns import for_version
 from kolibri.core.content.errors import SchemaNotFoundError
@@ -55,15 +57,27 @@ def test_columns_of_an_absent_table_is_empty(self):
             self.assertEqual([], source.columns("content_localfile"))
 
     def test_schema_version_is_inferred_for_every_frozen_schema(self):
+        # Version 2 is the one shape that fits a schema other than its own, and is
+        # covered by the test below instead.
         for version in CONTENT_DB_SCHEMA_VERSIONS:
+            if version == VERSION_2:
+                continue
             with self.subTest(version=version):
                 with SourceDB(self.build(version)) as source:
                     self.assertEqual(version, source.schema_version)
 
-    def test_a_shape_satisfying_two_schemas_infers_the_more_specific(self):
+    def test_a_shape_fitting_an_older_and_a_newer_schema_infers_the_newer(self):
+        # Schema 3 dropped ContentNode.stemmed_metaphone and File.available, so a
+        # version 2 file fits version 3 as well. The newest fit wins, because it is
+        # the reader that takes the most of the file — matching the most columns
+        # would answer 2 here.
+        with SourceDB(self.build(VERSION_2)) as source:
+            self.assertEqual(VERSION_3, source.schema_version)
+
+    def test_a_shape_satisfying_two_schemas_infers_the_newest(self):
         # Schema 6 renamed LocalFile.file_size to file_size_bigint, so a file carrying
-        # both satisfies 5 and 6. The more specific match is 6 — inferring 5 would
-        # drop File.included_presets on import.
+        # both satisfies 5 and 6. Inferring 5 would drop File.included_presets on
+        # import, and map file_size_bigint off a column that was never selected.
         db_path = self.build(VERSION_6)
         with sqlite3.connect(db_path) as connection:
             connection.execute("ALTER TABLE content_localfile ADD COLUMN file_size")

kolibri/core/content/utils/channel_import.py

@@ -1243,6 +1243,30 @@ class InvalidSchemaVersionError(Exception):
     pass
 
 
+def _check_schema_supported(schema_version):
+    if schema_version in mappings:
+        return
+    try:
+        version_number = int(schema_version)
+    except (TypeError, ValueError):
+        raise InvalidSchemaVersionError(
+            "Tried to import invalid schema version {version}".format(
+                version=schema_version
+            )
+        )
+    if version_number > int(CONTENT_SCHEMA_VERSION):
+        raise FutureSchemaError(
+            "Tried to import schema version, {version}, which is not supported by this version of Kolibri.".format(
+                version=schema_version
+            )
+        )
+    raise InvalidSchemaVersionError(
+        "Tried to import unsupported schema version {version}".format(
+            version=schema_version
+        )
+    )
+
+
 def initialize_import_manager(
     channel_metadata,
     source,
@@ -1252,42 +1276,19 @@ def initialize_import_manager(
     version_requested=False,
     force_upgrade=False,
 ):
-    # For data-based imports the schema is the version the data was serialized at;
-    # for file-based imports use the channel's min_schema_version.
     if isinstance(source, dict):
-        min_version = source["schema_version"]
+        schema_version = source["schema_version"]
     else:
-        min_version = channel_metadata.get(
-            "min_schema_version",
-            channel_metadata.get("inferred_schema_version"),
-        )
+        # min_schema_version is a floor, not a description: Studio keeps its published
+        # databases readable by the oldest schema it still supports, and has declared 1
+        # throughout. It gates whether we can read the file; the file's own shape picks
+        # the mappings.
+        _check_schema_supported(channel_metadata.get("min_schema_version", NO_VERSION))
+        schema_version = channel_metadata["inferred_schema_version"]
 
-    try:
-        ImportClass = mappings.get(min_version)
-    except KeyError:
-        try:
-            version_number = int(min_version)
-            if version_number > int(CONTENT_SCHEMA_VERSION):
-                raise FutureSchemaError(
-                    "Tried to import schema version, {version}, which is not supported by this version of Kolibri.".format(
-                        version=min_version
-                    )
-                )
-            elif version_number < int(CONTENT_SCHEMA_VERSION):
-                # If it's a valid integer, but there is no schema for it, then we have stopped supporting this version
-                raise InvalidSchemaVersionError(
-                    "Tried to import unsupported schema version {version}".format(
-                        version=min_version
-                    )
-                )
-        except ValueError:
-            raise InvalidSchemaVersionError(
-                "Tried to import invalid schema version {version}".format(
-                    version=min_version
-                )
-            )
+    _check_schema_supported(schema_version)
 
-    return ImportClass(
+    return mappings[schema_version](
         channel_metadata["id"],
         source,
         channel_version=channel_metadata["version"],

kolibri/core/content/utils/source_db.py

@@ -8,7 +8,6 @@
 """
 
 import sqlite3
-from functools import lru_cache
 from pathlib import Path
 
 from django.utils.functional import cached_property
@@ -18,21 +17,6 @@
 from kolibri.core.content.errors import SchemaNotFoundError
 
 
-@lru_cache(maxsize=None)
-def _versions_by_specificity():
-    """
-    The schema versions ordered by declared column count, most first, so the first
-    version a file satisfies is its most specific match.
-
-    Schema 3 dropped ContentNode.stemmed_metaphone and File.available, so every version
-    2 database also satisfies version 3 — the container has to be tried first.
-    """
-    return sorted(
-        CONTENT_DB_SCHEMA_VERSIONS,
-        key=lambda version: -sum(map(len, for_version(version).values())),
-    )
-
-
 class SourceDB:
     """
     A channel database file, opened read-only.
@@ -115,12 +99,15 @@ def rows(self, table, columns=None):
     @cached_property
     def schema_version(self):
         """
-        The content schema version this file's shape corresponds to. A file may declare
-        more than its own version does, as a Studio export does, so this matches on
-        superset rather than equality.
+        The newest content schema version this file's shape satisfies.
+
+        Studio publishes a database that is a superset of the oldest schema it
+        declares itself readable by, keeping legacy columns alongside current ones,
+        so this matches on superset rather than equality and ratchets down from the
+        newest version until one fits.
         """
         declared = {table: frozenset(columns) for table, columns in self._shape.items()}
-        for version in _versions_by_specificity():
+        for version in CONTENT_DB_SCHEMA_VERSIONS:
             if all(
                 declared.get(table, frozenset()).issuperset(columns)
                 for table, columns in for_version(version).items()