Merge pull request #895 from ZeitOnline/WCM-462-column-toggles

WCM-462: Toggle column access in batches, to support future migrations

core/docs/changelog/WCM-462.change

@@ -0,0 +1 @@
+WCM-462: Toggle column access in batches, to support future migrations

core/src/zeit/connector/models.py

@@ -42,67 +42,89 @@ def Index(cls, *args, name=None, ops=None, **kw):
 class CommonMetadata:
     channels = mapped_column(
         JSONBTuple,
-        info={'namespace': 'document', 'name': 'channels'},
+        info={'namespace': 'document', 'name': 'channels', 'migration': 'wcm_430'},
+    )
+    access = mapped_column(
+        Unicode, info={'namespace': 'document', 'name': 'access', 'migration': 'wcm_430'}
+    )
+    product = mapped_column(
+        Unicode, info={'namespace': 'workflow', 'name': 'product-id', 'migration': 'wcm_430'}
+    )
+    ressort = mapped_column(
+        Unicode, info={'namespace': 'document', 'name': 'ressort', 'migration': 'wcm_430'}
+    )
+    sub_ressort = mapped_column(
+        Unicode, info={'namespace': 'document', 'name': 'sub_ressort', 'migration': 'wcm_430'}
+    )
+    series = mapped_column(
+        Unicode, info={'namespace': 'document', 'name': 'serie', 'migration': 'wcm_430'}
     )
-    access = mapped_column(Unicode, info={'namespace': 'document', 'name': 'access'})
-    product = mapped_column(Unicode, info={'namespace': 'workflow', 'name': 'product-id'})
-    ressort = mapped_column(Unicode, info={'namespace': 'document', 'name': 'ressort'})
-    sub_ressort = mapped_column(Unicode, info={'namespace': 'document', 'name': 'sub_ressort'})
-    series = mapped_column(Unicode, info={'namespace': 'document', 'name': 'serie'})
 
-    print_ressort = mapped_column(Unicode, info={'namespace': 'print', 'name': 'ressort'})
-    volume_year = mapped_column(Integer, info={'namespace': 'document', 'name': 'year'})
-    volume_number = mapped_column(Integer, info={'namespace': 'document', 'name': 'volume'})
+    print_ressort = mapped_column(
+        Unicode, info={'namespace': 'print', 'name': 'ressort', 'migration': 'wcm_430'}
+    )
+    volume_year = mapped_column(
+        Integer, info={'namespace': 'document', 'name': 'year', 'migration': 'wcm_430'}
+    )
+    volume_number = mapped_column(
+        Integer, info={'namespace': 'document', 'name': 'volume', 'migration': 'wcm_430'}
+    )
 
 
 class Article:
-    article_genre = mapped_column(Unicode, info={'namespace': 'document', 'name': 'genre'})
+    article_genre = mapped_column(
+        Unicode, info={'namespace': 'document', 'name': 'genre', 'migration': 'wcm_430'}
+    )
 
 
 class SemanticChange:
     date_last_modified_semantic = mapped_column(
         TIMESTAMP,
-        info={'namespace': 'document', 'name': 'last-semantic-change'},
+        info={'namespace': 'document', 'name': 'last-semantic-change', 'migration': 'wcm_430'},
     )
 
 
 class Modified:
     date_created = mapped_column(
         TIMESTAMP,
-        info={'namespace': 'document', 'name': 'date_created'},
+        info={'namespace': 'document', 'name': 'date_created', 'migration': 'wcm_430'},
     )
     date_last_checkout = mapped_column(
         TIMESTAMP,
-        info={'namespace': 'document', 'name': 'date_last_checkout'},
+        info={'namespace': 'document', 'name': 'date_last_checkout', 'migration': 'wcm_430'},
     )
     date_last_modified = mapped_column(
         TIMESTAMP,
-        info={'namespace': 'document', 'name': 'date_last_modified'},
+        info={'namespace': 'document', 'name': 'date_last_modified', 'migration': 'wcm_430'},
     )
 
 
 class PublishInfo:
     date_first_released = mapped_column(
         TIMESTAMP,
-        info={'namespace': 'document', 'name': 'date_first_released'},
+        info={'namespace': 'document', 'name': 'date_first_released', 'migration': 'wcm_430'},
     )
     date_last_published = mapped_column(
         TIMESTAMP,
-        info={'namespace': 'workflow', 'name': 'date_last_published'},
+        info={'namespace': 'workflow', 'name': 'date_last_published', 'migration': 'wcm_430'},
     )
     date_last_published_semantic = mapped_column(
         TIMESTAMP,
-        info={'namespace': 'workflow', 'name': 'date_last_published_semantic'},
+        info={
+            'namespace': 'workflow',
+            'name': 'date_last_published_semantic',
+            'migration': 'wcm_430',
+        },
     )
     date_print_published = mapped_column(
         TIMESTAMP,
-        info={'namespace': 'document', 'name': 'print-publish'},
+        info={'namespace': 'document', 'name': 'print-publish', 'migration': 'wcm_430'},
     )
     published = mapped_column(
         Boolean,
         server_default='false',
         nullable=False,
-        info={'namespace': 'workflow', 'name': 'published'},
+        info={'namespace': 'workflow', 'name': 'published', 'migration': 'wcm_430'},
     )
 
 
@@ -180,15 +202,26 @@ def __table_args__(cls):
     )
 
     @classmethod
-    def column_by_name(cls, name, namespace):
+    def column_by_name(cls, name, namespace, mode='always'):
         namespace = namespace.replace(cls.NS, '', 1)
-        for column in cls._columns_with_name():
+        for column in cls._columns_with_name(mode):
             if namespace == column.info.get('namespace') and name == column.info.get('name'):
                 return column
 
+    _COLUMN_MODES = ('always', 'read', 'write', 'strict')
+
     @classmethod
-    def _columns_with_name(cls):
-        return [x for x in sqlalchemy.orm.class_mapper(cls).columns if x.info.get('namespace')]
+    def _columns_with_name(cls, mode):
+        assert mode in cls._COLUMN_MODES
+        result = []
+        for column in sqlalchemy.orm.class_mapper(cls).columns:
+            if not column.info.get('namespace'):
+                continue
+            migration = column.info['migration']
+            if mode == 'always' or FEATURE_TOGGLES.find(f'column_{mode}_{migration}'):
+                result.append(column)
+
+        return result
 
     @property
     def binary_body(self):
@@ -221,13 +254,12 @@ def to_webdav(self):
         props[('is_collection', INTERNAL_PROPERTY)] = self.is_collection
         props[('body_checksum', INTERNAL_PROPERTY)] = self._body_checksum()
 
-        if FEATURE_TOGGLES.find('read_metadata_columns'):
-            for column in self._columns_with_name():
-                namespace, name = column.info['namespace'], column.info['name']
-                value = getattr(self, column.name)
-                if value is not None:
-                    converter = zeit.connector.interfaces.IConverter(column)
-                    props[(name, self.NS + namespace)] = converter.serialize(value)
+        for column in self._columns_with_name('read'):
+            namespace, name = column.info['namespace'], column.info['name']
+            value = getattr(self, column.name)
+            if value is not None:
+                converter = zeit.connector.interfaces.IConverter(column)
+                props[(name, self.NS + namespace)] = converter.serialize(value)
 
         if self.lock:
             props[('lock_principal', INTERNAL_PROPERTY)] = self.lock.principal
@@ -250,26 +282,24 @@ def from_webdav(self, props):
         if type:
             self.type = type
 
-        if FEATURE_TOGGLES.find('write_metadata_columns') or FEATURE_TOGGLES.find(
-            'write_metadata_columns_strict'
-        ):
-            for column in self._columns_with_name():
-                namespace, name = column.info['namespace'], column.info['name']
-                value = props.get((name, self.NS + namespace), self)
-
-                if value is self:
-                    continue
-                if value is DeleteProperty:
-                    setattr(self, column.name, None)
-                    continue
-                if not isinstance(value, str):
-                    raise ValueError('Expected str, got %r' % value)
+        for column in self._columns_with_name('write'):
+            namespace, name = column.info['namespace'], column.info['name']
+            value = props.get((name, self.NS + namespace), self)
 
-                converter = zeit.connector.interfaces.IConverter(column)
-                setattr(self, column.name, converter.deserialize(value))
+            if value is self:
+                continue
+            if value is DeleteProperty:
+                setattr(self, column.name, None)
+                continue
+            if not isinstance(value, str):
+                raise ValueError('Expected str, got %r' % value)
+
+            converter = zeit.connector.interfaces.IConverter(column)
+            setattr(self, column.name, converter.deserialize(value))
 
-                if FEATURE_TOGGLES.find('write_metadata_columns_strict'):
-                    props.pop((name, self.NS + namespace), None)
+            migration = column.info['migration']
+            if FEATURE_TOGGLES.find(f'column_strict_{migration}'):
+                props.pop((name, self.NS + namespace), None)
 
         unsorted = collections.defaultdict(dict)
         for (k, ns), v in props.items():

core/src/zeit/connector/postgresql.py

@@ -33,7 +33,6 @@
 import zope.interface
 import zope.sqlalchemy
 
-from zeit.cms.content.sources import FEATURE_TOGGLES
 from zeit.cms.interfaces import DOCUMENT_SCHEMA_NS
 from zeit.cms.repository.interfaces import ConflictError
 from zeit.connector.interfaces import (
@@ -642,10 +641,9 @@ def _build_filter(self, expr):
             (var, value) = expr.operands
             name = var.name
             namespace = var.namespace.replace(Content.NS, '', 1)
-            if FEATURE_TOGGLES.find('read_metadata_columns'):
-                column = Content.column_by_name(name, namespace)
-                if column is not None:
-                    return column == value
+            column = Content.column_by_name(name, namespace, 'read')
+            if column is not None:
+                return column == value
             value = json.dumps(str(value))  # Apply correct quoting for jsonpath.
             return Content.unsorted.path_match(f'$."{namespace}"."{name}" == {value}')
         else:

core/src/zeit/connector/tests/test_postgresql.py

@@ -345,8 +345,8 @@ class PropertiesColumnTest(zeit.connector.testing.SQLTest):
     layer = zeit.connector.testing.SQL_CONTENT_LAYER
 
     def test_properties_are_written_simultaneously_to_separate_column_and_unsorted(self):
-        FEATURE_TOGGLES.set('write_metadata_columns')
-        FEATURE_TOGGLES.set('read_metadata_columns')
+        FEATURE_TOGGLES.set('column_write_wcm_430')
+        FEATURE_TOGGLES.set('column_read_wcm_430')
         timestamp = pendulum.datetime(1980, 1, 1)
         isoformat = timestamp.isoformat()
         res = self.add_resource('foo', properties={('date_created', f'{NS}document'): isoformat})
@@ -356,8 +356,9 @@ def test_properties_are_written_simultaneously_to_separate_column_and_unsorted(s
         self.assertEqual(timestamp, content.date_created)
 
     def test_properties_can_be_stored_in_separate_columns(self):
-        FEATURE_TOGGLES.set('write_metadata_columns_strict')
-        FEATURE_TOGGLES.set('read_metadata_columns')
+        FEATURE_TOGGLES.set('column_write_wcm_430')
+        FEATURE_TOGGLES.set('column_read_wcm_430')
+        FEATURE_TOGGLES.set('column_strict_wcm_430')
         timestamp = pendulum.datetime(1980, 1, 1)
         isoformat = timestamp.isoformat()
         res = self.add_resource('foo', properties={('date_created', f'{NS}document'): isoformat})
@@ -367,12 +368,12 @@ def test_properties_can_be_stored_in_separate_columns(self):
         self.assertEqual(timestamp, content.date_created)
 
     def test_search_looks_in_columns_or_unsorted_depending_on_toggle(self):
-        FEATURE_TOGGLES.set('write_metadata_columns')
+        FEATURE_TOGGLES.set('column_write_wcm_430')
 
         res = self.add_resource('foo', properties={('ressort', f'{NS}document'): 'Wissen'})
         var = SearchVar('ressort', f'{NS}document')
         for toggle in [False, True]:  # XXX parametrize would be nice
-            FEATURE_TOGGLES.factory.override(toggle, 'read_metadata_columns')
+            FEATURE_TOGGLES.factory.override(toggle, 'column_read_wcm_430')
             if toggle:
                 self.connector._get_content(res.id).unsorted = {}
                 transaction.commit()
@@ -381,16 +382,16 @@ def test_search_looks_in_columns_or_unsorted_depending_on_toggle(self):
             self.assertEqual(res.id, unique_id)
 
     def test_revoke_write_toggle_must_not_break_checkin(self):
-        FEATURE_TOGGLES.set('write_metadata_columns')
+        FEATURE_TOGGLES.set('column_write_wcm_430')
         self.repository['testcontent'] = ExampleContentType()
         example_date = pendulum.datetime(2024, 10, 1)
         with checked_out(self.repository['testcontent']) as co:
             IModified(co).date_created = example_date
-            FEATURE_TOGGLES.unset('write_metadata_columns')
+            FEATURE_TOGGLES.unset('column_write_wcm_430')
 
     def test_delete_property_from_column(self):
-        FEATURE_TOGGLES.set('read_metadata_columns')
-        FEATURE_TOGGLES.set('write_metadata_columns')
+        FEATURE_TOGGLES.set('column_write_wcm_430')
+        FEATURE_TOGGLES.set('column_read_wcm_430')
         id = 'http://xml.zeit.de/testcontent'
         self.repository['testcontent'] = ExampleContentType()
         example_date = pendulum.datetime(2024, 1, 1).isoformat()
@@ -406,6 +407,6 @@ def test_unsorted_properties_must_be_strings(self):
         date = pendulum.datetime(2024, 1, 1)
         with raises(ValueError):
             self.add_resource('foo', properties={('date_created', f'{NS}document'): date})
-        FEATURE_TOGGLES.set('write_metadata_columns')
+        FEATURE_TOGGLES.set('column_write_document_date_created')
         with raises(ValueError):
             self.add_resource('bar', properties={('date_created', f'{NS}document'): date})