ZO-4267: update documentation for interfaces and change contract for …

…non existent resources

core/src/zeit/connector/interfaces.py

@@ -130,14 +130,22 @@ def add(object, verify_etag=True):
     def copy(old_id, new_id):
         """Copy the resource old_id to new_id.
 
+        A COPY method invocation MUST NOT duplicate any write locks active on the source.
+
         raises KeyError if ther is no resource `old_id`
         raises CopyError if there was a problem with copying itself.
-
         """
 
     def move(old_id, new_id):
         """Move the resource with id `old_id` to `new_id`.
 
+        A successful MOVE request on a write locked resource MUST NOT move the
+        write lock with the resource.
+        However, the resource is subject to being added to an existing lock at the destination.
+
+        If the collection /a/b/ is write locked and the resource /c is moved to /a/b/c
+        then resource /a/b/c will be added to the write lock.
+
         raises KeyError if ther is no resource `old_id`
         raises MoveError if there was a problem with moving itself.
 
@@ -154,6 +162,17 @@ def changeProperties(id, properties):
     def lock(id, principal, until):
         """Lock resource for principal until a given datetime.
 
+        A client MUST NOT submit the same write lock request twice.
+
+        A successful request for an write lock MUST result in the generation of a unique lock token
+        associated with the requesting principal.
+
+        A write locked null resource, referred to as a lock-null resource is possible.
+
+        A write lock request is issued to a collection containing member URIs
+        identifying resources that are currently locked in a manner which conflicts
+        with the write lock, the request MUST fail with a 423 (Locked) status code.
+
         id: unique id
         until: datetime until the lock is valid.
 
@@ -166,8 +185,7 @@ def unlock(id, locktoken=None):
 
         Use `locktoken` if given. Otherwise use stored locktoken.
 
-        returns used locktoken.
-
+        returns used locktoken, if the id of the resource does not exist, return none
         """
 
     def locked(id):
@@ -179,9 +197,7 @@ def locked(id):
             my lock: True if the lock was issued by this system, False
                 otherwise.
 
-        returns None, None, None if the resource is not locked.
-        raises KeyError if the resource does not exist.
-
+        returns None, None, None if the resource is not locked or is non-existant
         """
 
     def search(attributes, search_expression):

core/src/zeit/connector/postgresql.py

@@ -346,6 +346,9 @@ def _is_own_lock(self, id, requesting_principal):
 
     def _add_lock(self, id, principal, until):
         path = self.session.get(Path, self._pathkey(id))
+        if path is None:
+            log.warning('Unable to add lock to resource %s that does not exist.', str(id))
+            return
         token = secrets.token_hex()
         self.session.add(
             Lock(
@@ -386,7 +389,8 @@ def unlock(self, id, locktoken=None):
     def locked(self, id):
         path = self.session.get(Path, self._pathkey(id))
         if path is None:
-            raise KeyError('The resource %s does not exist.' % str(id))
+            log.warning('The resource %s does not exist.', str(id))
+            return (None, None, False)
         lock = self.session.get(Lock, (path.parent_path, path.name, path.id))
         if lock is None:
             return (None, None, False)

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

@@ -311,17 +311,32 @@ def test_locking_already_locked_resource_raises(self):
                 id, 'zope.another_user', datetime.now(pytz.UTC) + timedelta(hours=2)
             )
 
-    def test_query_lock_status_for_non_existent_resource_raises(self):
-        with self.assertRaises(KeyError):
-            self.connector.locked('http://xml.zeit.de/nonexistent')
+    def test_query_lock_status_for_non_existent_resource(self):
+        self.assertEqual(
+            self.connector.locked('http://xml.zeit.de/nonexistent'), (None, None, False)
+        )
 
-    def test_unlocking_non_existent_resource_raises(self):
-        with self.assertRaises(KeyError):
-            self.connector.locked('http://xml.zeit.de/nonexistent')
+    def test_unlocking_non_existent_resource(self):
+        self.assertIsNone(self.connector.unlock('http://xml.zeit.de/nonexistent'), None)
 
-    def test_locking_non_existent_resource_raises(self):
-        with self.assertRaises(KeyError):
-            self.connector.locked('http://xml.zeit.de/nonexistent')
+    def test_locking_non_existent_resource(self):
+        token = self.connector.lock(
+            'http://xml.zeit.de/nonexistent',
+            'zope.user',
+            datetime.now(pytz.UTC) + timedelta(hours=2),
+        )
+        self.assertEqual(None, token)
+
+    def test_move_operation_keeps_lock(self):
+        id = self.add_resource('foo').id
+        self.connector.lock(id, 'zope.user', datetime.now(pytz.UTC) + timedelta(hours=2))
+        transaction.commit()
+        self.connector.move(id, 'http://xml.zeit.de/testing/bar')
+        transaction.commit()
+        user, until, mine = self.connector.locked('http://xml.zeit.de/testing/bar')
+        self.assertTrue(mine)
+        self.assertEqual('zope.user', user)
+        self.assertTrue(isinstance(until, datetime))
 
 
 class ContractSearch: