4823: Add tests for Collection.clone and Collection.merge

[arvados.git] / sdk / python / arvados / collection.py

diff --git a/sdk/python/arvados/collection.py b/sdk/python/arvados/collection.py
index dd4946400161be7c87397d5ca9e2543aedf7fe32..1f2974b3488ba77772c963e93c5702502a2e7988 100644 (file)
--- a/sdk/python/arvados/collection.py
+++ b/sdk/python/arvados/collection.py
@@ -12,6 +12,7 @@ from .arvfile import ArvadosFileBase, split, ArvadosFile, ArvadosFileWriter, Arv
 from keep import *
 from .stream import StreamReader, normalize_stream, locator_block_size
 from .ranges import Range, LocatorAndRange
+from .safeapi import SafeApi
 import config
 import errors
 import util
@@ -695,7 +696,7 @@ class SynchronizedCollectionBase(CollectionBase):
         if p[0] == '.':
             del p[0]
 
-        if len(p) > 0:
+        if p and p[0]:
             item = self._items.get(p[0])
             if len(p) == 1:
                 # item must be a file
@@ -857,18 +858,31 @@ class SynchronizedCollectionBase(CollectionBase):
             raise IOError((errno.ENOENT, "File not found"))
 
     def _cloneinto(self, target):
-        for k,v in self._items:
-            target._items[k] = v.clone(new_parent=target)
+        for k,v in self._items.items():
+            target._items[k] = v.clone(target)
 
     def clone(self):
         raise NotImplementedError()
 
     @_must_be_writable
     @_synchronized
-    def copyto(self, target_path, source_path, source_collection=None, overwrite=False):
-        """
-        copyto('/foo', '/bar') will overwrite 'foo' if it exists.
-        copyto('/foo/', '/bar') will place 'bar' in subcollection 'foo'
+    def copy(self, source_path, target_path, source_collection=None, overwrite=False):
+        """Copy a file or subcollection to a new path in this collection.
+
+        :source_path:
+          Source file or subcollection
+
+        :target_path:
+          Destination file or path.  If the target path already exists and is a
+          subcollection, the item will be placed inside the subcollection.  If
+          the target path already exists and is a file, this will raise an error
+          unless you specify `overwrite=True`.
+
+        :source_collection:
+          Collection to copy `source_path` from (default `self`)
+
+        :overwrite:
+          Whether to overwrite target file if it already exists.
         """
         if source_collection is None:
             source_collection = self
@@ -881,13 +895,17 @@ class SynchronizedCollectionBase(CollectionBase):
 
         # Find parent collection the target path
         tp = target_path.split("/")
-        target_dir = self.find(tp[0:-1].join("/"), create=True, create_collection=True)
+        target_dir = self.find("/".join(tp[0:-1]), create=True, create_collection=True)
 
         # Determine the name to use.
         target_name = tp[-1] if tp[-1] else sp[-1]
 
-        if target_name in target_dir and not overwrite:
-            raise IOError((errno.EEXIST, "File already exists"))
+        if target_name in target_dir:
+            if isinstance(target_dir[target_name], SynchronizedCollectionBase):
+                target_dir = target_dir[target_name]
+                target_name = sp[-1]
+            elif not overwrite:
+                raise IOError((errno.EEXIST, "File already exists"))
 
         # Actually make the copy.
         dup = source_obj.clone(target_dir)
@@ -929,12 +947,12 @@ class SynchronizedCollectionBase(CollectionBase):
                     self[k].merge(other[k])
                 else:
                     if self[k] != other[k]:
-                        name = "%s~conflict-%s~" % (k, time.strftime("%Y-%m-%d~%H:%M%:%S",
+                        name = "%s~conflict-%s~" % (k, time.strftime("%Y-%m-%d_%H:%M:%S",
                                                                      time.gmtime()))
-                        self[name] = other[k].clone(self)
+                        self._items[name] = other[k].clone(self)
                         self.notify(self, name, ADD, self[name])
             else:
-                self[k] = other[k].clone(self)
+                self._items[k] = other[k].clone(self)
                 self.notify(self, k, ADD, self[k])
 
     def portable_data_hash(self):
@@ -1041,7 +1059,7 @@ class Collection(SynchronizedCollectionBase):
     @_synchronized
     def _my_api(self):
         if self._api_client is None:
-            self._api_client = arvados.api.SafeApi(self._config)
+            self._api_client = arvados.SafeApi(self._config)
             self._keep_client = self._api_client.keep
         return self._api_client
 
@@ -1138,7 +1156,7 @@ class Collection(SynchronizedCollectionBase):
     @_synchronized
     def clone(self, new_parent=None, new_sync=SYNC_READONLY, new_config=None):
         if new_config is None:
-            new_config = self.config
+            new_config = self._config
         c = Collection(parent=new_parent, config=new_config, sync=new_sync)
         if new_sync == SYNC_READONLY:
             c.lock = NoopLock()