20300: Update symbols to strings in relations.

In Rails 6.1 the column names are converted to strings internally, so
checking for "foreign_key == :owner_uuid" no longer works.

Using symbols continues to work when declaring relations, but they are
updated to strings anyway for clarity, and to match
canonical/documented usage.

Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom@curii.com>

diff --git a/services/api/app/models/arvados_model.rb b/services/api/app/models/arvados_model.rb
index c909e47cef7ff10e4e4b01cc5221713c7d78a542..81cdbfcd1c108340b4fc72c92dbd4fe8ecde2e00 100644 (file)
--- a/services/api/app/models/arvados_model.rb
+++ b/services/api/app/models/arvados_model.rb
@@ -37,9 +37,9 @@ class ArvadosModel < ApplicationRecord
   # user.uuid==object.owner_uuid.
   has_many(:permissions,
            ->{where(link_class: 'permission')},
-           foreign_key: :head_uuid,
+           foreign_key: 'head_uuid',
            class_name: 'Link',
-           primary_key: :uuid)
+           primary_key: 'uuid')
 
   # If async is true at create or update, permission graph
   # update is deferred allowing making multiple calls without the performance

diff --git a/services/api/app/models/authorized_key.rb b/services/api/app/models/authorized_key.rb
index 0da9f7f8c506d758584fbdf01e2070994ca24f97..cf4a1d55de796fd57417b8994d4c77214f8b12d8 100644 (file)
--- a/services/api/app/models/authorized_key.rb
+++ b/services/api/app/models/authorized_key.rb
@@ -10,9 +10,9 @@ class AuthorizedKey < ArvadosModel
   before_update :permission_to_set_authorized_user_uuid
 
   belongs_to :authorized_user, {
-               foreign_key: :authorized_user_uuid,
+               foreign_key: 'authorized_user_uuid',
                class_name: 'User',
-               primary_key: :uuid,
+               primary_key: 'uuid',
                optional: true,
              }
 

diff --git a/services/api/app/models/container.rb b/services/api/app/models/container.rb
index 9104a21107d09f5b141ad7edc2ccdc453a99bf09..97f8dfa279f23214e446361470721034f757e391 100644 (file)
--- a/services/api/app/models/container.rb
+++ b/services/api/app/models/container.rb
@@ -51,11 +51,15 @@ class Container < ArvadosModel
   after_save :update_cr_logs
   after_save :handle_completed
 
-  has_many :container_requests, :foreign_key => :container_uuid, :class_name => 'ContainerRequest', :primary_key => :uuid
+  has_many :container_requests, {
+             class_name: 'ContainerRequest',
+             foreign_key: 'container_uuid',
+             primary_key: 'uuid',
+           }
   belongs_to :auth, {
                class_name: 'ApiClientAuthorization',
-               foreign_key: :auth_uuid,
-               primary_key: :uuid,
+               foreign_key: 'auth_uuid',
+               primary_key: 'uuid',
                optional: true,
              }
 

diff --git a/services/api/app/models/container_request.rb b/services/api/app/models/container_request.rb
index b499e37e7d1cf1145ad61fad2b6247e42545ad51..374859f2c157fb557613ca885bf3212f0e4c54ef 100644 (file)
--- a/services/api/app/models/container_request.rb
+++ b/services/api/app/models/container_request.rb
@@ -13,14 +13,14 @@ class ContainerRequest < ArvadosModel
   include WhitelistUpdate
 
   belongs_to :container, {
-               foreign_key: :container_uuid,
-               primary_key: :uuid,
+               foreign_key: 'container_uuid',
+               primary_key: 'uuid',
                optional: true,
              }
   belongs_to :requesting_container, {
                class_name: 'Container',
-               foreign_key: :requesting_container_uuid,
-               primary_key: :uuid,
+               foreign_key: 'requesting_container_uuid',
+               primary_key: 'uuid',
                optional: true,
              }
 

diff --git a/services/api/app/models/job.rb b/services/api/app/models/job.rb
index f792c04842116f781da644e614b9280acb6b7711..029a3132856ea608497ae54330fba74f4ce5f682 100644 (file)
--- a/services/api/app/models/job.rb
+++ b/services/api/app/models/job.rb
@@ -50,7 +50,7 @@ class Job < ArvadosModel
   before_create :create_disabled
   before_update :update_disabled
 
-  has_many(:nodes, foreign_key: :job_uuid, primary_key: :uuid)
+  has_many(:nodes, foreign_key: 'job_uuid', primary_key: 'uuid')
 
   class SubmitIdReused < RequestError
   end

diff --git a/services/api/app/models/node.rb b/services/api/app/models/node.rb
index 2770dd5ccc048935d137bcbcf67eb383ad4b362e..9e250acfb11f794e78452ab5e08746ad8c973eec 100644 (file)
--- a/services/api/app/models/node.rb
+++ b/services/api/app/models/node.rb
@@ -21,8 +21,8 @@ class Node < ArvadosModel
   # have access to the associated Job.  They're expected to set
   # job_readable=true if the Job UUID can be included in the API response.
   belongs_to :job, {
-               foreign_key: :job_uuid,
-               primary_key: :uuid,
+               foreign_key: 'job_uuid',
+               primary_key: 'uuid',
                optional: true,
              }
   attr_accessor :job_readable

diff --git a/services/api/app/models/pipeline_instance.rb b/services/api/app/models/pipeline_instance.rb
index d5ed5e533d041805e18aa5ace551dda171465f86..23a08051caf75c1860992cbdeaeeb8b3d495d099 100644 (file)
--- a/services/api/app/models/pipeline_instance.rb
+++ b/services/api/app/models/pipeline_instance.rb
@@ -10,8 +10,8 @@ class PipelineInstance < ArvadosModel
   serialize :properties, Hash
   serialize :components_summary, Hash
   belongs_to :pipeline_template, {
-               foreign_key: :pipeline_template_uuid,
-               primary_key: :uuid,
+               foreign_key: 'pipeline_template_uuid',
+               primary_key: 'uuid',
                optional: true,
              }
 

diff --git a/services/api/app/models/user.rb b/services/api/app/models/user.rb
index 88314c67753558d9a567ad4003af215306e7a56d..afc2d18b8a306efc29ebecb88ce7a1e888c536f0 100644 (file)
--- a/services/api/app/models/user.rb
+++ b/services/api/app/models/user.rb
@@ -56,8 +56,8 @@ class User < ArvadosModel
   before_destroy :clear_permissions
   after_destroy :remove_self_from_permissions
 
-  has_many :authorized_keys, :foreign_key => :authorized_user_uuid, :primary_key => :uuid
-  has_many :repositories, foreign_key: :owner_uuid, primary_key: :uuid
+  has_many :authorized_keys, foreign_key: 'authorized_user_uuid', primary_key: 'uuid'
+  has_many :repositories, foreign_key: 'owner_uuid', primary_key: 'uuid'
 
   default_scope { where('redirect_to_user_uuid is null') }
 

diff --git a/services/api/app/models/virtual_machine.rb b/services/api/app/models/virtual_machine.rb
index 0b3557eef6c68a802c977436dc979c3c138fedfa..09687385cad88b29c4a388e713d805a67003de2d 100644 (file)
--- a/services/api/app/models/virtual_machine.rb
+++ b/services/api/app/models/virtual_machine.rb
@@ -9,9 +9,9 @@ class VirtualMachine < ArvadosModel
 
   has_many(:login_permissions,
            -> { where("link_class = 'permission' and name = 'can_login'") },
-           foreign_key: :head_uuid,
+           foreign_key: 'head_uuid',
            class_name: 'Link',
-           primary_key: :uuid)
+           primary_key: 'uuid')
 
   api_accessible :user, extend: :common do |t|
     t.add :hostname

diff --git a/services/api/lib/can_be_an_owner.rb b/services/api/lib/can_be_an_owner.rb
index 6f30f5ae33801afabd6c6a5e3fa6805c05e628fa..fc66f84bfc7e4cf52df8ea9f64595a4e6d9ed40f 100644 (file)
--- a/services/api/lib/can_be_an_owner.rb
+++ b/services/api/lib/can_be_an_owner.rb
@@ -22,8 +22,8 @@ module CanBeAnOwner
       klass = t.classify.constantize
       next unless klass and 'owner_uuid'.in?(klass.columns.collect(&:name))
       base.has_many(t.to_sym,
-                    foreign_key: :owner_uuid,
-                    primary_key: :uuid,
+                    foreign_key: 'owner_uuid',
+                    primary_key: 'uuid',
                     dependent: :restrict_with_exception)
     end
     # We need custom protection for changing an owner's primary
@@ -75,7 +75,7 @@ module CanBeAnOwner
 
     # Check for objects that have my old uuid listed as their owner.
     self.class.reflect_on_all_associations(:has_many).each do |assoc|
-      next unless assoc.foreign_key == :owner_uuid
+      next unless assoc.foreign_key == 'owner_uuid'
       if assoc.klass.where(owner_uuid: uuid_was).any?
         errors.add(:uuid,
                    "cannot be changed on a #{self.class} that owns objects")

diff --git a/services/api/lib/has_uuid.rb b/services/api/lib/has_uuid.rb
index 2074566941fec7c6a79903df712b82541e9a5853..217113beec34d8193bce89c7cf5c9517ada014fa 100644 (file)
--- a/services/api/lib/has_uuid.rb
+++ b/services/api/lib/has_uuid.rb
@@ -14,14 +14,14 @@ module HasUuid
     base.has_many(:links_via_head,
                   -> { where("not (link_class = 'permission')") },
                   class_name: 'Link',
-                  foreign_key: :head_uuid,
-                  primary_key: :uuid,
+                  foreign_key: 'head_uuid',
+                  primary_key: 'uuid',
                   dependent: :destroy)
     base.has_many(:links_via_tail,
                   -> { where("not (link_class = 'permission')") },
                   class_name: 'Link',
-                  foreign_key: :tail_uuid,
-                  primary_key: :uuid,
+                  foreign_key: 'tail_uuid',
+                  primary_key: 'uuid',
                   dependent: :destroy)
   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 8a1d044d6a760fca9ec969114382eef77b71d2ef..574cd366fc3869c70af30ae5e8986cd52a1cc06d 100644 (file)
--- a/services/api/test/functional/arvados/v1/collections_controller_test.rb
+++ b/services/api/test/functional/arvados/v1/collections_controller_test.rb
@@ -1222,6 +1222,20 @@ EOS
     assert_nil json_response['trash_at']
   end
 
+  test 'untrash a trashed collection by assigning nil to trash_at' do
+    authorize_with :active
+    post :update, params: {
+           id: collections(:expired_collection).uuid,
+           collection: {
+             trash_at: nil,
+           },
+           include_trash: true,
+    }
+    assert_response 200
+    assert_equal false, json_response['is_trashed']
+    assert_nil json_response['trash_at']
+  end
+
   test 'untrash error on not trashed collection' do
     authorize_with :active
     post :untrash, params: {