ZO-4267: deletion of collection with locked children should throw exc…

…eption if we do not own the lock

Co-authored-by:  Louisa v. Hülsen <[email protected]>

core/src/zeit/connector/mock.py

@@ -14,7 +14,14 @@
 import zope.event
 
 from zeit.connector.connector import CannonicalId
-from zeit.connector.interfaces import ID_NAMESPACE, UUID_PROPERTY, CopyError, MoveError
+from zeit.connector.interfaces import (
+    ID_NAMESPACE,
+    UUID_PROPERTY,
+    CopyError,
+    LockedByOtherSystemError,
+    LockingError,
+    MoveError,
+)
 import zeit.connector.cache
 import zeit.connector.dav.interfaces
 import zeit.connector.filesystem
@@ -167,7 +174,10 @@ def __setitem__(self, id, object):
     def __delitem__(self, id):
         id = self._get_cannonical_id(id)
         self[id]  # may raise KeyError
-        for _name, uid in self.listCollection(id):
+        list_collection = self.listCollection(id)
+        for _name, uid in list_collection:
+            if uid in self._locked:
+                raise LockedByOtherSystemError(uid, '')
             del self[uid]
         self._deleted.add(id)
         self._data.pop(id, None)

core/src/zeit/connector/postgresql.py

@@ -258,20 +258,30 @@ def __delitem__(self, uniqueid):
         content = self._get_content(uniqueid)
         if content is None:
             raise KeyError(f'The resource {uniqueid} does not exist.')
-        props = self._get_properties(uniqueid)
-        if props is None:
-            raise KeyError(uniqueid)
-        if not props.is_collection and props.binary_body:
-            blob = self.bucket.blob(props.id)
+        if content.is_collection and self._foreign_child_lock_exists(uniqueid):
+            raise LockedByOtherSystemError(
+                uniqueid, f'Could not delete {uniqueid}, because it is locked.'
+            )
         if not content.is_collection and content.binary_body:
             blob = self.bucket.blob(content.id)
             with zeit.cms.tracing.use_span(
-                __name__ + '.tracing', 'gcs', attributes={'db.operation': 'delete', 'id': content.id}
+                __name__ + '.tracing',
+                'gcs',
+                attributes={'db.operation': 'delete', 'id': content.id},
             ):
                 try:
                     blob.delete()
                 except google.api_core.exceptions.NotFound:
                     log.info('Ignored NotFound while deleting GCS blob %s', uniqueid)
+
+        self.unlock(uniqueid)
+        if content.is_collection:
+            (parent, _) = self._pathkey(uniqueid)
+            stmt = delete(Path).where(Path.parent_path.startswith(parent))
+            self.session.execute(stmt)
+        else:
+            self.session.delete(content)
+
         self.property_cache.pop(uniqueid, None)
         self.body_cache.pop(uniqueid, None)
 

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

@@ -329,6 +329,29 @@ def test_move_collection_with_locked_child_raises(self):
             self.connector.move(collection_id, 'http://xml.zeit.de/testing/target')
         self.connector._unlock(file_id, token)
 
+    def test_delitem_collection_raises_error_if_children_are_locked(self):
+        self.assertEqual([], self.listCollection('http://xml.zeit.de/testing'))
+        collection = Resource(None, None, 'image', BytesIO(b''), None, 'httpd/unix-directory')
+        self.connector['http://xml.zeit.de/testing/folder'] = collection
+        self.add_resource('folder/file')
+        self.add_resource('folder/one')
+        self.add_resource('two')
+        token_1 = self.connector.lock(
+            'http://xml.zeit.de/testing/folder/one',
+            'external',
+            datetime.now(pytz.UTC) + timedelta(hours=2),
+        )
+        self.connector.lock(
+            'http://xml.zeit.de/testing/two',
+            'zope.user',
+            datetime.now(pytz.UTC) + timedelta(hours=2),
+        )
+        transaction.commit()
+        with self.assertRaises(LockedByOtherSystemError):
+            del self.connector['http://xml.zeit.de/testing']
+        self.connector._unlock('http://xml.zeit.de/testing/folder/one', token_1)
+        self.connector.unlock('http://xml.zeit.de/testing/two')
+
 
 class ContractSearch:
     def test_search_unknown_metadata(self):