[ARVADOS] created: 15f512c3d8bc8bf090972653b3e53742197316e8

git at public.curoverse.com git at public.curoverse.com
Fri Sep 5 16:48:35 EDT 2014


        at  15f512c3d8bc8bf090972653b3e53742197316e8 (commit)


commit 15f512c3d8bc8bf090972653b3e53742197316e8
Author: Peter Amstutz <peter.amstutz at curoverse.com>
Date:   Fri Sep 5 16:48:25 2014 -0400

    3822: Updated Gemfile

diff --git a/services/api/Gemfile b/services/api/Gemfile
index 592d00d..32a2fbd 100644
--- a/services/api/Gemfile
+++ b/services/api/Gemfile
@@ -71,7 +71,7 @@ gem 'database_cleaner'
 
 gem 'themes_for_rails'
 
-gem 'arvados-cli', '>= 0.1.20140827170424'
+gem 'arvados-cli', '>= 0.1.20140905164626'
 
 # pg_power lets us use partial indexes in schema.rb in Rails 3
 gem 'pg_power'
diff --git a/services/api/Gemfile.lock b/services/api/Gemfile.lock
index 6167d7a..3c711fc 100644
--- a/services/api/Gemfile.lock
+++ b/services/api/Gemfile.lock
@@ -41,7 +41,7 @@ GEM
       google-api-client (~> 0.6.3)
       json (>= 1.7.7)
       jwt (>= 0.1.5, < 1.0.0)
-    arvados-cli (0.1.20140827170424)
+    arvados-cli (0.1.20140905164626)
       activesupport (~> 3.2, >= 3.2.13)
       andand (~> 1.3, >= 1.3.3)
       arvados (~> 0.1.0)
@@ -223,7 +223,7 @@ PLATFORMS
 DEPENDENCIES
   acts_as_api
   andand
-  arvados-cli (>= 0.1.20140827170424)
+  arvados-cli (>= 0.1.20140905164626)
   coffee-rails (~> 3.2.0)
   database_cleaner
   factory_girl_rails

commit 1c679ba62c8d7b0d962532bdb5fa2de4c685d286
Author: Peter Amstutz <peter.amstutz at curoverse.com>
Date:   Fri Sep 5 16:46:26 2014 -0400

    3822: Fixed use of string instead of symbol in hash, added missing list separator.

diff --git a/sdk/cli/bin/arv-run-pipeline-instance b/sdk/cli/bin/arv-run-pipeline-instance
index 5c36a1a..7ce1fa9 100755
--- a/sdk/cli/bin/arv-run-pipeline-instance
+++ b/sdk/cli/bin/arv-run-pipeline-instance
@@ -580,11 +580,11 @@ class WhRunPipelineInstance
                   name = c[:output_name] || "Output #{portable_data_hash[0..7]} of #{cname} of #{pipeline_name}"
 
                   # check if there is a name collision.
-                  name_collisions = $arv.collection.list(filters: [["owner_uuid", "=", owner_uuid]
+                  name_collisions = $arv.collection.list(filters: [["owner_uuid", "=", owner_uuid],
                                                                    ["name", "=", name]])[:items]
 
                   newcollection_actual = nil
-                  if name_collisions.any? and name_collisions.first["portable_data_hash"] == portable_data_hash
+                  if name_collisions.any? and name_collisions.first[:portable_data_hash] == portable_data_hash
                     # There is already a collection with the same name and the
                     # same contents, so just point to that.
                     newcollection_actual = name_collisions.first

commit 3b7e5549bf4a17907f8e3236592bcbd5cf6ad623
Author: Peter Amstutz <peter.amstutz at curoverse.com>
Date:   Fri Sep 5 16:23:38 2014 -0400

    3822: Added 'ensure_unique_name' option to #create method for API server to
    choose a unique name when there is a name collision in the database.
    arv-run-pipeline-instance checks to see if there is an output file with the
    same name and contents, and uses 'ensure_unique_name' when creating collection.

diff --git a/sdk/cli/bin/arv-run-pipeline-instance b/sdk/cli/bin/arv-run-pipeline-instance
index 101b4ee..5c36a1a 100755
--- a/sdk/cli/bin/arv-run-pipeline-instance
+++ b/sdk/cli/bin/arv-run-pipeline-instance
@@ -570,7 +570,6 @@ class WhRunPipelineInstance
                 pipeline_name = @template[:name]
               end
               if c[:output_name] != false
-                output_name = c[:output_name] || "Output of #{cname} of #{pipeline_name}"
                 # Create a collection located in the same project as the pipeline with the contents of the output.
                 portable_data_hash = c[:job][:output]
                 collections = $arv.collection.list(limit: 1,
@@ -578,14 +577,33 @@ class WhRunPipelineInstance
                                                    select: ["portable_data_hash", "manifest_text"]
                                                    )[:items]
                 if collections.any?
-                  newcollection = {
-                    owner_uuid: owner_uuid,
-                    name: "#{output_name} at #{c[:job][:finished_at]}",
-                    portable_data_hash: collections.first[:portable_data_hash],
-                    manifest_text: collections.first[:manifest_text]
-                  }
-                  debuglog "Creating collection #{newcollection}", 0
-                  newcollection_actual = $arv.collection.create collection: newcollection
+                  name = c[:output_name] || "Output #{portable_data_hash[0..7]} of #{cname} of #{pipeline_name}"
+
+                  # check if there is a name collision.
+                  name_collisions = $arv.collection.list(filters: [["owner_uuid", "=", owner_uuid]
+                                                                   ["name", "=", name]])[:items]
+
+                  newcollection_actual = nil
+                  if name_collisions.any? and name_collisions.first["portable_data_hash"] == portable_data_hash
+                    # There is already a collection with the same name and the
+                    # same contents, so just point to that.
+                    newcollection_actual = name_collisions.first
+                  end
+
+                  if newcollection_actual.nil?
+                    # Did not find a collection with the same name (or the
+                    # collection has a different portable data hash) so create
+                    # a new collection with ensure_unique_name: true.
+                    newcollection = {
+                      owner_uuid: owner_uuid,
+                      name: name,
+                      portable_data_hash: collections.first[:portable_data_hash],
+                      manifest_text: collections.first[:manifest_text]                      
+                    }
+                    debuglog "Creating collection #{newcollection}", 0
+                    newcollection_actual = $arv.collection.create collection: newcollection, ensure_unique_name: true
+                  end
+
                   c[:output_uuid] = newcollection_actual[:uuid]
                 else
                   debuglog "Could not find a collection with portable data hash #{portable_data_hash}", 0
diff --git a/services/api/app/controllers/application_controller.rb b/services/api/app/controllers/application_controller.rb
index 2400365..3465a78 100644
--- a/services/api/app/controllers/application_controller.rb
+++ b/services/api/app/controllers/application_controller.rb
@@ -70,7 +70,47 @@ class ApplicationController < ActionController::Base
 
   def create
     @object = model_class.new resource_attrs
-    @object.save!
+    retry_save = true
+
+    if @object.respond_to? :name and params[:ensure_unique_name]
+      # Record the original name.  See below.
+      name_stem = @object.name
+      counter = 1
+    end
+
+    while retry_save
+      begin
+        retry_save = false
+        @object.save!
+      rescue ActiveRecord::RecordNotUnique => rn
+        # Dig into the error to determine if it is specifically calling out a
+        # (owner_uuid, name) uniqueness violation.  In this specific case, and
+        # the client requested a unique name with ensure_unique_name==true,
+        # update the name field and try to save again.  Loop as necessary to
+        # discover a unique name.  It is necessary to handle name choosing at
+        # this level (as opposed to the client) to ensure that record creation
+        # never fails due to a race condition.
+        if rn.original_exception.is_a? PG::UniqueViolation
+          # Unfortunately ActiveRecord doesn't abstract out any of the
+          # necessary information to figure out if this the error is actually
+          # the specific case where we want to apply the ensure_unique_name
+          # behavior, so the following code is specialized to Postgres.
+          err = rn.original_exception
+          detail = err.result.error_field(PG::Result::PG_DIAG_MESSAGE_DETAIL)
+          if /^Key \(owner_uuid, name\)=\([a-z0-9]{5}-[a-z0-9]{5}-[a-z0-9]{15}, .*?\) already exists\./.match detail
+            logger.error "params[:ensure_unique_name] is #{params[:ensure_unique_name]}"
+            if params[:ensure_unique_name]
+              counter += 1
+              @object.name = "#{name_stem} (#{counter})"
+              retry_save = true
+            end
+          end
+        end
+        if not retry_save
+          raise
+        end
+      end
+    end
     show
   end
 
diff --git a/services/api/test/functional/arvados/v1/collections_controller_test.rb b/services/api/test/functional/arvados/v1/collections_controller_test.rb
index 65dfca5..49b1fae 100644
--- a/services/api/test/functional/arvados/v1/collections_controller_test.rb
+++ b/services/api/test/functional/arvados/v1/collections_controller_test.rb
@@ -195,6 +195,24 @@ EOS
     assert_response 422
   end
 
+  test "create succeeds with with duplicate name with ensure_unique_name" do
+    permit_unsigned_manifests
+    authorize_with :admin
+    manifest_text = ". d41d8cd98f00b204e9800998ecf8427e 0:0:foo.txt\n"
+    post :create, {
+      collection: {
+        owner_uuid: 'zzzzz-tpzed-000000000000000',
+        manifest_text: manifest_text,
+        portable_data_hash: "d30fe8ae534397864cb96c544f4cf102+47",
+        name: "foo_file"
+      },
+      ensure_unique_name: true
+    }
+    assert_response :success
+    resp = JSON.parse(@response.body)
+    assert_equal 'foo_file (2)', resp['name']
+  end
+
   test "create with owner_uuid set to group i can_manage" do
     permit_unsigned_manifests
     authorize_with :active

-----------------------------------------------------------------------


hooks/post-receive
-- 




More information about the arvados-commits mailing list