2760: Assign a generic name if link_class=name and no name is given.

diff --git a/apps/workbench/app/controllers/application_controller.rb b/apps/workbench/app/controllers/application_controller.rb
index 61375b858c5c208a77cbcd5f11c8b8c48fabf150..c79d082946504ed75ec90411520c94e5c7482d1f 100644 (file)
--- a/apps/workbench/app/controllers/application_controller.rb
+++ b/apps/workbench/app/controllers/application_controller.rb
@@ -179,11 +179,10 @@ class ApplicationController < ActionController::Base
     @object ||= model_class.new @new_resource_attrs
     @object.save!
     if model_class != Link
-      name = params[:name] || "New #{model_class.to_s.underscore.downcase.humanize} created #{Time.now}"
       @name_link = Link.new(tail_uuid: current_user.uuid,
                             head_uuid: @object.uuid,
                             link_class: 'name',
-                            name: name)
+                            name: params[:name])
       @name_link.save!
     end
     show

diff --git a/services/api/app/models/link.rb b/services/api/app/models/link.rb
index af3918551e441a2ccaea47ea67e19e8f23a73b1f..9f4f51016c5d5f2337843309522873e537975202 100644 (file)
--- a/services/api/app/models/link.rb
+++ b/services/api/app/models/link.rb
@@ -5,6 +5,7 @@ class Link < ArvadosModel
   serialize :properties, Hash
   before_create :permission_to_attach_to_objects
   before_update :permission_to_attach_to_objects
+  before_create :assign_generic_name_if_none_given
   after_update :maybe_invalidate_permissions_cache
   after_create :maybe_invalidate_permissions_cache
   after_destroy :maybe_invalidate_permissions_cache
@@ -40,6 +41,48 @@ class Link < ArvadosModel
 
   protected
 
+  def assign_generic_name_if_none_given
+    if new_record? and link_class == 'name' and (!name or name.empty?)
+      # Creating a name link with no name means "invent a generic
+      # name, like New Foo (1)"
+
+      head_class = ArvadosModel::resource_class_for_uuid(head_uuid)
+      if !head_class
+        errors.add :name, 'cannot be automatically assigned for this uuid'
+        return
+      end
+      name_base = "New " + head_class.to_s.underscore.humanize.downcase
+      if not Link.where(link_class: link_class,
+                        tail_uuid: tail_uuid,
+                        name: name_base).any?
+        self.name = name_base
+      else
+        # Find how many digits the largest N has among "New model (N)" names
+        maxlen = ActiveRecord::Base.connection.
+          execute("SELECT max(length(name)) maxlen FROM links "\
+                  "WHERE link_class='name' "\
+                  "AND tail_uuid=#{Link.sanitize(tail_uuid)} "\
+                  "AND name~#{Link.sanitize "#{name_base} \\([0-9]+\\)"}")[0]
+        if maxlen and maxlen['maxlen']
+          # Find the largest N by sorting alphanumerically
+          maxname = ActiveRecord::Base.connection.
+            execute("SELECT max(name) maxname FROM links "\
+                    "WHERE link_class='name' "\
+                    "AND tail_uuid=#{Link.sanitize(tail_uuid)} "\
+                    "AND length(name)=#{maxlen['maxlen']} "\
+                    "AND name~#{Link.sanitize "#{name_base} \\([0-9]+\\)"}"
+                    )[0]['maxname']
+          n = maxname.match(/\(([0-9]+)\)$/)[1].to_i
+          n += 1
+        else
+          # "New foo" is taken, but "New foo (1)" isn't.
+          n = 1
+        end
+        self.name = name_base + " (#{n})"
+      end
+    end
+  end
+
   def permission_to_attach_to_objects
     # Anonymous users cannot write links
     return false if !current_user
@@ -85,8 +128,13 @@ class Link < ArvadosModel
 
   def name_link_has_valid_name
     if link_class == 'name'
-      unless name.is_a? String and !name.empty?
-        errors.add('name', 'must be a non-empty string')
+      if new_record? and (!name or name.empty?)
+        # Unique name will be assigned in before_create filter
+        true
+      else
+        unless name.is_a? String and !name.empty?
+          errors.add('name', 'must be a non-empty string')
+        end
       end
     else
       true

diff --git a/services/api/test/unit/link_test.rb b/services/api/test/unit/link_test.rb
index 10f2b5eca54adffb9f6250198386c4292f79037d..c04addd10bf73477d6972b21e3cc42b600ef596f 100644 (file)
--- a/services/api/test/unit/link_test.rb
+++ b/services/api/test/unit/link_test.rb
@@ -37,12 +37,25 @@ class LinkTest < ActiveSupport::TestCase
   end
 
   [nil, '', false].each do |name|
-    test "name links cannot have name=#{name.inspect}" do
+    test "name links cannot be renamed to name=#{name.inspect}" do
+      a = Link.create!(tail_uuid: groups(:afolder).uuid,
+                       head_uuid: specimens(:owned_by_active_user).uuid,
+                       link_class: 'name',
+                       name: 'temp')
+      a.name = name
+      assert a.invalid?, "invalid name was accepted as valid?"
+    end
+
+    test "name links cannot be created with name=#{name.inspect}" do
       a = Link.create(tail_uuid: groups(:afolder).uuid,
                       head_uuid: specimens(:owned_by_active_user).uuid,
                       link_class: 'name',
                       name: name)
-      assert a.invalid?, "invalid name was accepted as valid?"
+      if a.name and !a.name.empty?
+        assert a.valid?, "name automatically assigned, but record not valid?"
+      else
+        assert a.invalid?, "invalid name was accepted as valid?"
+      end
     end
   end
 
@@ -57,4 +70,34 @@ class LinkTest < ActiveSupport::TestCase
       ob.destroy
     end
   end
+
+  test "assign sequential generic name links" do
+    group = Group.create!(group_class: 'folder')
+    ob = Specimen.create!
+    25.times do |n|
+      link = Link.create!(link_class: 'name',
+                          tail_uuid: group.uuid, head_uuid: ob.uuid)
+      expect_name = 'New specimen' + (n==0 ? "" : " (#{n})")
+      assert_equal expect_name, link.name, "Expected sequential generic names"
+    end
+  end
+
+  test "assign sequential generic name links for a two-word model" do
+    group = Group.create!(group_class: 'folder')
+    ob = VirtualMachine.create!
+    5.times do |n|
+      link = Link.create!(link_class: 'name',
+                          tail_uuid: group.uuid, head_uuid: ob.uuid)
+      expect_name = 'New virtual machine' + (n==0 ? "" : " (#{n})")
+      assert_equal expect_name, link.name, "Expected sequential generic names"
+    end
+  end
+
+  test "cannot assign sequential generic name links for a bogus uuid type" do
+    group = Group.create!(group_class: 'folder')
+    link = Link.create(link_class: 'name',
+                       tail_uuid: group.uuid,
+                       head_uuid: 'zzzzz-abcde-123451234512345')
+    assert link.invalid?, "gave a bogus uuid, got automatic name #{link.name}"
+  end
 end