diff --git a/core/src/zeit/connector/interfaces.py b/core/src/zeit/connector/interfaces.py index 589a046c60..242ed2af3c 100644 --- a/core/src/zeit/connector/interfaces.py +++ b/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): diff --git a/core/src/zeit/connector/postgresql.py b/core/src/zeit/connector/postgresql.py index 5d36e89a57..a58dc4323d 100644 --- a/core/src/zeit/connector/postgresql.py +++ b/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) diff --git a/core/src/zeit/connector/tests/test_contract.py b/core/src/zeit/connector/tests/test_contract.py index f6745243e2..60786f5378 100644 --- a/core/src/zeit/connector/tests/test_contract.py +++ b/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: