21205: Now adds the final part of the uuid to make the name unique

[arvados.git] / services / api / app / models / arvados_model.rb

diff --git a/services/api/app/models/arvados_model.rb b/services/api/app/models/arvados_model.rb
index 2256fa5b3ecb1a1374f1e5aa023c44bb8a64aeab..9ee2cca410effaba81fbb4ce9d207354c5f1b3f8 100644 (file)
--- a/services/api/app/models/arvados_model.rb
+++ b/services/api/app/models/arvados_model.rb
@@ -24,6 +24,7 @@ class ArvadosModel < ApplicationRecord
   before_destroy :ensure_owner_uuid_is_permitted
   before_destroy :ensure_permission_to_destroy
   before_create :update_modified_by_fields
+  before_create :add_uuid_to_name, :if => Proc.new { @_add_uuid_to_name }
   before_update :maybe_update_modified_by_fields
   after_create :log_create
   after_update :log_update
@@ -471,9 +472,7 @@ class ArvadosModel < ApplicationRecord
   end
 
   def save_with_unique_name!
-    uuid_was = uuid
-    name_was = name
-    max_retries = 3
+    max_retries = 2
     transaction do
       conn = ActiveRecord::Base.connection
       conn.exec_query 'SAVEPOINT save_with_unique_name'
@@ -503,27 +502,20 @@ class ArvadosModel < ApplicationRecord
 
         conn.exec_query 'ROLLBACK TO SAVEPOINT save_with_unique_name'
 
-        if uuid_was.nil? && !uuid.nil?
+        if uuid_was.nil?
+          # new record, the uuid caused a name collision (very
+          # unlikely but possible), so generate new uuid
           self[:uuid] = nil
           if self.is_a? Collection
-            # Reset so that is assigned to the new UUID
+            # Also needs to be reset
             self[:current_version_uuid] = nil
           end
+          # need to adjust the name after the uuid has been generated
+          add_uuid_to_make_unique_name
+        else
+          # existing record, just update the name directly.
+          add_uuid_to_name
         end
-
-        # make sure we have a uuid
-        self.assign_uuid
-
-        # new_name used to have a timestamp added to it, but it turns
-        # out that timestamps with 1ms precision and 2 retries wasn't
-        # enough to avoid collisions (as well as being inherently
-        # limited to 1000 collection creations per second).  So to
-        # scale better, append the final part of the uuid.
-        #
-        # The name field has a limit of 256 characters, so also
-        # truncate if necessary to avoid throwing a "field too big"
-        # exception.
-        self[:name] = "#{name_was[0..236]} (#{self.uuid[-15..-1]]})"
         retry
       end
     end
@@ -584,6 +576,26 @@ class ArvadosModel < ApplicationRecord
                           *ft[:param_out])
   end
 
+  @_add_uuid_to_name = false
+  def add_uuid_to_make_unique_name
+    @_add_uuid_to_name = true
+  end
+
+  def add_uuid_to_name
+    # Incorporate the random part of the UUID into the name.  This
+    # lets us prevent name collision but the part we add to the name
+    # is still somewhat meaningful (instead of generating a second
+    # random meaningless string).
+    #
+    # Because ArvadosModel is an abstract class and assign_uuid is
+    # part of HasUuid (which is included by the other concrete
+    # classes) the assign_uuid hook gets added (and run) after this
+    # one.  So we need to call assign_uuid here to make sure we have a
+    # uuid.
+    assign_uuid
+    self.name = "#{self.name[0..236]} (#{self.uuid[-15..-1]})"
+  end
+
   protected
 
   def self.deep_sort_hash(x)