ZO-4267: change in contract: copy and move raise CopyError and MoveEr…

…ror instead of KeyError if resources are missing

Ideally we would have a exception hierachy
CopyError
   - ResourceMissingError(CopyError)
   - TargetAlreadyExistError(CopyError)
   - etc.

and then the consumer of the connector can decide what to do with the individual errors or if the catch all at once with CopyError

core/src/zeit/connector/connector.py

@@ -281,12 +281,13 @@ def copy(self, old_id, new_id):
 
     def move(self, old_id, new_id):
         """Move the resource with id `old_id` to `new_id`."""
-        self._copy_or_move(
-            'move', zeit.connector.interfaces.MoveError, old_id, new_id, resolve_conflicts=True
-        )
+        self._copy_or_move('move', zeit.connector.interfaces.MoveError, old_id, new_id)
 
-    def _copy_or_move(self, method_name, exception, old_id, new_id, resolve_conflicts=False):
-        source = self[old_id]  # Makes sure source exists.
+    def _copy_or_move(self, method_name, exception, old_id, new_id):
+        try:
+            source = self[old_id]  # Makes sure source exists.
+        except KeyError as err:
+            raise exception(old_id, f'Can not copy or move non-existant resource {old_id}') from err
         if self._is_descendant(new_id, old_id):
             raise exception(old_id, 'Could not copy or move %s to a decendant of itself.' % old_id)
 
@@ -325,7 +326,10 @@ def _copy_or_move(self, method_name, exception, old_id, new_id, resolve_conflict
                 del self[old_id]
         else:
             token = self._get_my_locktoken(old_id)
-            method(old_loc, new_loc, locktoken=token)
+            try:
+                method(old_loc, new_loc, locktoken=token)
+            except zeit.connector.dav.interfaces.DAVNotFoundError as err:
+                raise exception('The resource %s does not exist.', old_id) from err
 
         self._invalidate_cache(old_id)
         self._invalidate_cache(new_id)

core/src/zeit/connector/interfaces.py

@@ -130,17 +130,22 @@ def add(object, verify_etag=True):
     def copy(old_id, new_id):
         """Copy the resource old_id to new_id.
 
-        raises KeyError if ther is no resource `old_id`
-        raises CopyError if there was a problem with copying itself.
+        A COPY method invocation MUST NOT duplicate any write locks active on the source.
 
+        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`.
 
-        raises KeyError if ther is no resource `old_id`
-        raises MoveError if there was a problem with moving itself.
+        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 MoveError if there was a problem with moving itself.
         """
 
     def changeProperties(id, properties):

core/src/zeit/connector/mock.py

@@ -187,7 +187,10 @@ def add(self, object, verify_etag=True):
 
     def copy(self, old_id, new_id):
         self._prevent_overwrite(old_id, new_id, CopyError)
-        r = self[old_id]
+        try:
+            r = self[old_id]
+        except KeyError:
+            raise CopyError(old_id, '')
         r.id = new_id
         r.properties.pop(UUID_PROPERTY, None)
         self.add(r, verify_etag=False)
@@ -199,7 +202,12 @@ def copy(self, old_id, new_id):
     def move(self, old_id, new_id):
         self._prevent_overwrite(old_id, new_id, MoveError)
         self._ignore_uuid_checks = True
-        r = self[old_id]
+        try:
+            r = self[old_id]
+        except KeyError:
+            raise MoveError(old_id, f'The resource {old_id} does not exist.')
+        if new_id in self:
+            raise MoveError(new_id, f'The resource {new_id} already exists.')
         r.id = new_id
         try:
             self.add(r, verify_etag=False)

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

@@ -202,8 +202,14 @@ def test_move_collection_applies_to_all_children(self):
         self.assertNotIn('http://xml.zeit.de/testing/source/file', self.connector)
         self.assertIn('http://xml.zeit.de/testing/target/file', self.connector)
 
+    def test_move_nonexistent_raises(self):
+        with self.assertRaises(MoveError):
+            self.connector.move(
+                'http://xml.zeit.de/nonexistent', 'http://xml.zeit.de/testing/target'
+            )
+
     def test_copy_nonexistent_raises(self):
-        with self.assertRaises(KeyError):
+        with self.assertRaises(CopyError):
             self.connector.copy(
                 'http://xml.zeit.de/nonexistent', 'http://xml.zeit.de/testing/target'
             )