5614: Ruby SDK cp_r method creates streams as needed.

diff --git a/sdk/ruby/lib/arvados/collection.rb b/sdk/ruby/lib/arvados/collection.rb
index 9670f37127aef968bde7a8cd34f1171200fe64b5..07b751908f7da26b93fd5321fe8a5c192872a8d6 100644 (file)
--- a/sdk/ruby/lib/arvados/collection.rb
+++ b/sdk/ruby/lib/arvados/collection.rb
@@ -14,8 +14,13 @@ module Arv
         loc_list = LocatorList.new(locators)
         file_specs.map { |s| manifest.split_file_token(s) }.
             each do |file_start, file_len, file_path|
-          @root.file_at(normalize_path(stream_root, file_path)).
-            add_segment(loc_list.segment(file_start, file_len))
+          begin
+            @root.file_at(normalize_path(stream_root, file_path)).
+              add_segment(loc_list.segment(file_start, file_len))
+          rescue Errno::ENOTDIR, Errno::EISDIR => error
+            raise ArgumentError.new("%p is both a stream and file" %
+                                    error.to_s.partition(" - ").last)
+          end
         end
       end
     end
@@ -101,13 +106,19 @@ module Arv
       # is found and can be copied.
       source_collection = self if source_collection.nil?
       src_stream, src_tail = source_collection.find(source)
-      dst_stream, dst_tail = find(target)
+      dst_stream_path, _, dst_tail = normalize_path(target).rpartition("/")
+      if dst_stream_path.empty?
+        dst_stream, dst_tail = @root.find(dst_tail)
+        dst_tail ||= src_tail
+      else
+        dst_stream = @root.stream_at(dst_stream_path)
+        dst_tail = src_tail if dst_tail.empty?
+      end
       if (source_collection.equal?(self) and
           (src_stream.path == dst_stream.path) and (src_tail == dst_tail))
         return self
       end
       src_item = src_stream[src_tail]
-      dst_tail ||= src_tail
       check_method = "check_can_#{copy_method}".to_sym
       target_name = nil
       if opts.fetch(:descend_target, true)
@@ -307,7 +318,7 @@ module Arv
 
       def stream_at(find_path)
         key, rest = find_path.split("/", 2)
-        next_stream = get_or_new(key, CollectionStream)
+        next_stream = get_or_new(key, CollectionStream, Errno::ENOTDIR)
         if rest.nil?
           next_stream
         else
@@ -318,7 +329,7 @@ module Arv
       def file_at(find_path)
         stream_path, _, file_name = find_path.rpartition("/")
         if stream_path.empty?
-          get_or_new(file_name, CollectionFile)
+          get_or_new(file_name, CollectionFile, Errno::EISDIR)
         else
           stream_at(stream_path).file_at(file_name)
         end
@@ -401,17 +412,15 @@ module Arv
         items[key] = item
       end
 
-      def get_or_new(key, klass)
+      def get_or_new(key, klass, err_class)
         # Return the collection item at `key` and ensure that it's a `klass`.
         # If `key` does not exist, create a new `klass` there.
-        # If the value for `key` is not a `klass`, raise an ArgumentError.
+        # If the value for `key` is not a `klass`, raise an `err_class`.
         item = items[key]
         if item.nil?
           self[key] = klass.new("#{path}/#{key}")
         elsif not item.is_a?(klass)
-          raise ArgumentError.
-            new("in stream %p, %p is a %s, not a %s" %
-                [path, key, items[key].class.human_name, klass.human_name])
+          raise err_class.new(item.path)
         else
           item
         end

diff --git a/sdk/ruby/test/test_collection.rb b/sdk/ruby/test/test_collection.rb
index e96054bfbd8a0d0cd0d1cd9a342615750a920fc3..e2a39bc5edbda8705e0434fa9738c5192b325938 100644 (file)
--- a/sdk/ruby/test/test_collection.rb
+++ b/sdk/ruby/test/test_collection.rb
@@ -223,13 +223,17 @@ class CollectionTest < Minitest::Test
     assert_equal(expected.join(""), coll.manifest_text)
   end
 
-  def test_copy_stream_over_file_raises_ENOTDIR
+  def test_copy_stream_over_file_raises_ENOTDIR(source="./s1", target="./f2")
     coll = Arv::Collection.new(TWO_BY_TWO_MANIFEST_S)
     assert_raises(Errno::ENOTDIR) do
-      coll.cp_r("./s1", "./f2")
+      coll.cp_r(source, target)
     end
   end
 
+  def test_copy_file_under_file_raises_ENOTDIR
+    test_copy_stream_over_file_raises_ENOTDIR("./f1", "./f2/newfile")
+  end
+
   def test_copy_stream_over_nonempty_stream_merges_and_overwrites
     blocks = random_blocks(3, 9)
     manifest_a =
@@ -323,6 +327,20 @@ class CollectionTest < Minitest::Test
     assert_equal(expect_lines.join(""), coll.manifest_text)
   end
 
+  def test_copy_file_into_new_stream_with_implicit_filename
+    coll = Arv::Collection.new(SIMPLEST_MANIFEST)
+    coll.cp_r("./simple.txt", "./new/")
+    assert_equal(SIMPLEST_MANIFEST + SIMPLEST_MANIFEST.sub(". ", "./new "),
+                 coll.manifest_text)
+  end
+
+  def test_copy_file_into_new_stream_with_explicit_filename
+    coll = Arv::Collection.new(SIMPLEST_MANIFEST)
+    coll.cp_r("./simple.txt", "./new/newfile.txt")
+    new_line = SIMPLEST_MANIFEST.sub(". ", "./new ").sub(":simple", ":newfile")
+    assert_equal(SIMPLEST_MANIFEST + new_line, coll.manifest_text)
+  end
+
   def test_copy_stream_contents_into_root
     coll = Arv::Collection.new(TWO_BY_TWO_MANIFEST_S)
     coll.cp_r("./s1/", ".")