From: Tom Clegg Date: Fri, 16 May 2014 13:56:28 +0000 (-0400) Subject: 2760: Assign a generic name if link_class=name and no name is given. X-Git-Url: https://git.arvados.org/arvados.git/commitdiff_plain/8e4fc832108d9cae376d3eb1b9e62f9d6b468501?ds=sidebyside 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 61375b858c..c79d082946 100644 --- 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 af3918551e..9f4f51016c 100644 --- 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 10f2b5eca5..c04addd10b 100644 --- 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