[ARVADOS] created: daafdb4c939f265b4604711d0fc946a830d9d54e

[Git user]

        at  daafdb4c939f265b4604711d0fc946a830d9d54e (commit)


commit daafdb4c939f265b4604711d0fc946a830d9d54e
Author: Tom Clegg <tom at curoverse.com>
Date:   Thu Jan 12 12:43:01 2017 -0500

    9831: Use a timestamp instead of a serial number to come up with a unique name after a name collision.

diff --git a/services/api/app/controllers/application_controller.rb b/services/api/app/controllers/application_controller.rb
index d58c432..37e100a 100644
--- a/services/api/app/controllers/application_controller.rb
+++ b/services/api/app/controllers/application_controller.rb
@@ -19,6 +19,7 @@ class ApplicationController < ActionController::Base
   include CurrentApiClient
   include ThemesForRails::ActionController
   include LoadParam
+  include DbCurrentTime
 
   respond_to :json
   protect_from_forgery
@@ -48,6 +49,8 @@ class ApplicationController < ActionController::Base
 
   attr_writer :resource_attrs
 
+  MAX_UNIQUE_NAME_ATTEMPTS = 10
+
   begin
     rescue_from(Exception,
                 ArvadosModel::PermissionDeniedError,
@@ -97,13 +100,16 @@ class ApplicationController < ActionController::Base
     if @object.respond_to? :name and params[:ensure_unique_name]
       # Record the original name.  See below.
       name_stem = @object.name
-      counter = 1
+      retries = MAX_UNIQUE_NAME_ATTEMPTS
+    else
+      retries = 0
     end
 
     begin
       @object.save!
     rescue ActiveRecord::RecordNotUnique => rn
-      raise unless params[:ensure_unique_name]
+      raise unless retries > 0
+      retries -= 1
 
       # Dig into the error to determine if it is specifically calling out a
       # (owner_uuid, name) uniqueness violation.  In this specific case, and
@@ -122,13 +128,10 @@ class ApplicationController < ActionController::Base
       detail = err.result.error_field(PG::Result::PG_DIAG_MESSAGE_DETAIL)
       raise unless /^Key \(owner_uuid, name\)=\([a-z0-9]{5}-[a-z0-9]{5}-[a-z0-9]{15}, .*?\) already exists\./.match detail
 
-      # OK, this exception really is just a unique name constraint
-      # violation, and we've been asked to ensure_unique_name.
-      counter += 1
       @object.uuid = nil
-      @object.name = "#{name_stem} (#{counter})"
-      redo
-    end while false
+      @object.name = "#{name_stem} (#{db_current_time.utc.iso8601(3)})"
+      retry
+    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 4b87ebd..2391fc1 100644
--- a/services/api/test/functional/arvados/v1/collections_controller_test.rb
+++ b/services/api/test/functional/arvados/v1/collections_controller_test.rb
@@ -345,7 +345,7 @@ EOS
         ensure_unique_name: true
       }
       assert_response :success
-      assert_equal 'owned_by_active (2)', json_response['name']
+      assert_match /^owned_by_active \(\d{4}-\d\d-\d\d.*?Z\)$/, json_response['name']
     end
   end
 
diff --git a/services/api/test/functional/arvados/v1/groups_controller_test.rb b/services/api/test/functional/arvados/v1/groups_controller_test.rb
index e9abf9d..646089d 100644
--- a/services/api/test/functional/arvados/v1/groups_controller_test.rb
+++ b/services/api/test/functional/arvados/v1/groups_controller_test.rb
@@ -380,9 +380,8 @@ class Arvados::V1::GroupsControllerTest < ActionController::TestCase
     assert_not_equal(new_project['uuid'],
                      groups(:aproject).uuid,
                      "create returned same uuid as existing project")
-    assert_equal(new_project['name'],
-                 'A Project (2)',
-                 "new project name '#{new_project['name']}' was expected to be 'A Project (2)'")
+    assert_match(/^A Project \(\d{4}-\d\d-\d\dT\d\d:\d\d:\d\d\.\d{3}Z\)$/,
+                 new_project['name'])
   end
 
   test "unsharing a project results in hiding it from previously shared user" do

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


hooks/post-receive
--