WCM-285: write channels into database field using converter and read …

…them back into dav

core/src/zeit/connector/converter.py

@@ -51,3 +51,32 @@ def serialize(self, value):
 
     def deserialize(self, value):
         return zeit.cms.content.dav.DatetimeProperty._fromProperty(value)
+
+
+class ChannelsConverter(DefaultConverter):
+    grok.context(JSONB)
+    grok.name('channels')
+
+    def serialize(self, value):
+        if not value:
+            return ''
+        elements = []
+        for channel, subchannels in value.items():
+            if subchannels:
+                elements.append(f"{channel} {' '.join(subchannels)}")
+            else:
+                elements.append(channel)
+        return ';'.join(elements)
+
+    def deserialize(self, value):
+        channels = {}
+        if value:
+            elements = [i.split() for i in value.split(';') if i.strip()]
+            for element in elements:
+                channel = element[0]
+                subchannels = element[1:] if len(element) > 1 else []
+                if channel in channels:
+                    channels[channel].extend(subchannels)
+                else:
+                    channels[channel] = subchannels
+        return channels

core/src/zeit/connector/interfaces.py

@@ -356,6 +356,7 @@ class IResourceInvalidatedEvent(zope.interface.Interface):
 
 class IConverter(zope.interface.Interface):
     """Converts webdav values to and from the postgresql database."""
+
     def serialize(value):
         pass
 

core/src/zeit/connector/models.py

@@ -16,6 +16,7 @@
 from sqlalchemy.orm import declared_attr, mapped_column, relationship
 import pytz
 import sqlalchemy
+import zope.component
 
 from zeit.cms.content.sources import FEATURE_TOGGLES
 from zeit.connector.interfaces import INTERNAL_PROPERTY, DeleteProperty, LockStatus
@@ -35,6 +36,13 @@ class CommonMetadata:
     def table_args(tablename):
         return (Index(f'ix_{tablename}_channels', 'channels', postgresql_using='gin'),)
 
+    # converter, use name to lookup IConverter instead of type
+    channels = mapped_column(
+        JSONB,
+        nullable=True,
+        info={'namespace': 'document', 'name': 'channels', 'converter': 'channels'},
+    )
+
 
 class DevelopmentCommonMetadata:
     access = mapped_column(Unicode, index=True, info={'namespace': 'document', 'name': 'access'})
@@ -126,6 +134,17 @@ def lock_status(self):
 
     NS = 'http://namespaces.zeit.de/CMS/'
 
+    @staticmethod
+    def converter(column):
+        if 'converter' in column.info:
+            return zope.component.queryAdapter(
+                column.type,
+                zeit.connector.interfaces.IConverter,
+                column.info['converter'],
+            )
+        else:
+            return zeit.connector.interfaces.IConverter(column)
+
     def to_webdav(self):
         if self.unsorted is None:
             return {}
@@ -143,7 +162,9 @@ def to_webdav(self):
         if FEATURE_TOGGLES.find('read_metadata_columns'):
             for column in self._columns_with_name():
                 namespace, name = column.info['namespace'], column.info['name']
-                props[(name, self.NS + namespace)] = getattr(self, column.name)
+                value = getattr(self, column.name)
+                converter = self.converter(column)
+                props[(name, self.NS + namespace)] = converter.serialize(value)
 
         if self.lock:
             props[('lock_principal', INTERNAL_PROPERTY)] = self.lock.principal
@@ -171,7 +192,8 @@ def from_webdav(self, props):
                 namespace, name = column.info['namespace'], column.info['name']
                 value = props.get((name, self.NS + namespace), self)
                 if value is not self:
-                    setattr(self, column.name, value)
+                    converter = self.converter(column)
+                    setattr(self, column.name, converter.deserialize(value))
 
         unsorted = collections.defaultdict(dict)
         for (k, ns), v in props.items():

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

@@ -10,9 +10,10 @@
 import transaction
 
 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 INTERNAL_PROPERTY
-from zeit.connector.models import Content, Lock
+from zeit.connector.models import Lock
 from zeit.connector.postgresql import _unlock_overdue_locks
 from zeit.connector.resource import Resource, WriteableCachedResource
 from zeit.connector.search import SearchVar
@@ -334,15 +335,15 @@ class PropertiesColumnTest(zeit.connector.testing.SQLTest):
     def test_properties_can_be_stored_in_separate_columns(self):
         FEATURE_TOGGLES.set('write_metadata_columns', True)
         FEATURE_TOGGLES.set('read_metadata_columns', True)
-        res = self.add_resource('foo', properties={('access', Content.NS + 'document'): 'foo'})
-        self.assertEqual('foo', res.properties[('access', Content.NS + 'document')])
+        res = self.add_resource('foo', properties={('access', DOCUMENT_SCHEMA_NS): 'foo'})
+        self.assertEqual('foo', res.properties[('access', DOCUMENT_SCHEMA_NS)])
         content = self.connector._get_content(res.id)
         self.assertEqual('foo', content.access)
 
     def test_search_looks_in_columns_or_unsorted_depending_on_toggle(self):
         FEATURE_TOGGLES.set('write_metadata_columns', True)
 
-        res = self.add_resource('foo', properties={('access', Content.NS + 'document'): 'foo'})
+        res = self.add_resource('foo', properties={('access', DOCUMENT_SCHEMA_NS): 'foo'})
         access = SearchVar('access', 'http://namespaces.zeit.de/CMS/document')
         for toggle in [False, True]:  # XXX parametrize would be nice
             FEATURE_TOGGLES.set('read_metadata_columns', toggle)
@@ -352,3 +353,97 @@ def test_search_looks_in_columns_or_unsorted_depending_on_toggle(self):
             result = self.connector.search([access], access == 'foo')
             unique_id, uuid = next(result)
             self.assertEqual(res.id, unique_id)
+
+
+class ChannelsColumnTest(zeit.connector.testing.SQLTest):
+    layer = zeit.connector.testing.SQL_CONTENT_LAYER
+
+    def setUp(self):
+        super().setUp()
+        FEATURE_TOGGLES.set('read_metadata_columns', True)
+        FEATURE_TOGGLES.set('write_metadata_columns', True)
+
+    def _make_resource(self, channels):
+        res = self.add_resource('foo', properties={('channels', DOCUMENT_SCHEMA_NS): channels})
+        return self.connector._get_content(res.id)
+
+    def assert_channels(self, channels, expected_channels, expected_dav_channels):
+        self.assertEqual(channels, expected_channels)
+        res = self.connector['http://xml.zeit.de/testing/foo']
+        self.assertEqual(expected_dav_channels, res.properties[('channels', DOCUMENT_SCHEMA_NS)])
+
+    def test_modify_channels(self):
+        content = self._make_resource('channel1;channel2 sub1 sub2')
+        self.assertEqual(content.channels, {'channel1': [], 'channel2': ['sub1', 'sub2']})
+        self.connector.changeProperties(
+            content.uniqueid, {('channels', DOCUMENT_SCHEMA_NS): 'channel2'}
+        )
+        content = self.connector._get_content(content.uniqueid)
+        self.assertEqual(content.channels, {'channel2': []})
+
+    def test_empty_input(self):
+        channels = ''
+        content = self._make_resource(channels)
+        self.assert_channels(content.channels, {}, channels)
+
+    def test_single_channel(self):
+        channels = 'channel1'
+        content = self._make_resource(channels)
+        self.assert_channels(content.channels, {'channel1': []}, channels)
+
+    def test_multiple_channels(self):
+        channels = 'channel1;channel2;channel3'
+        content = self._make_resource(channels)
+        self.assert_channels(
+            content.channels, {'channel1': [], 'channel2': [], 'channel3': []}, channels
+        )
+
+    def test_single_channel_with_subchannels(self):
+        channels = 'channel1 sub1 sub2'
+        content = self._make_resource(channels)
+        self.assert_channels(content.channels, {'channel1': ['sub1', 'sub2']}, channels)
+
+    def test_same_channel_with_subchannels(self):
+        channels = 'channel1 sub1;channel1 sub2 sub3;channel1 sub4'
+        content = self._make_resource(channels)
+        self.assert_channels(
+            content.channels,
+            {'channel1': ['sub1', 'sub2', 'sub3', 'sub4']},
+            'channel1 sub1 sub2 sub3 sub4',
+        )
+
+    def test_multiple_channels_with_subchannels(self):
+        channels = 'channel1 sub1;channel2 sub2 sub3;channel3 sub4'
+        content = self._make_resource(channels)
+        self.assert_channels(
+            content.channels,
+            {'channel1': ['sub1'], 'channel2': ['sub2', 'sub3'], 'channel3': ['sub4']},
+            channels,
+        )
+
+    def test_whitespace_handling(self):
+        channels = '  channel1  sub1  ;  channel2  sub2  sub3  ;  channel3  sub4  '
+        content = self._make_resource(channels)
+        self.assert_channels(
+            content.channels,
+            {'channel1': ['sub1'], 'channel2': ['sub2', 'sub3'], 'channel3': ['sub4']},
+            'channel1 sub1;channel2 sub2 sub3;channel3 sub4',
+        )
+
+    def test_trailing_semicolon(self):
+        channels = 'channel1;channel2;'
+        content = self._make_resource(channels)
+        self.assert_channels(content.channels, {'channel1': [], 'channel2': []}, channels[:-1])
+
+    def test_leading_semicolon(self):
+        channels = ';channel1;channel2'
+        content = self._make_resource(channels)
+        self.assert_channels(content.channels, {'channel1': [], 'channel2': []}, channels[1:])
+
+    def test_multiple_semicolons(self):
+        content = self._make_resource('channel1;;channel2;;;channel3')
+        self.assert_channels(
+            content.channels,
+            {'channel1': [], 'channel2': [], 'channel3': []},
+            'channel1;channel2;channel3',
+        )