From 3b40453701265dc66f8efb5865d29cf508f3ca43 Mon Sep 17 00:00:00 2001 From: Tom Clegg Date: Wed, 6 Sep 2023 17:21:50 -0400 Subject: [PATCH] 20300: Change deprecated update_attributes to update. Arvados-DCO-1.1-Signed-off-by: Tom Clegg --- .../app/controllers/application_controller.rb | 4 +- .../app/controllers/collections_controller.rb | 4 +- .../container_requests_controller.rb | 2 +- .../app/controllers/projects_controller.rb | 6 +- apps/workbench/app/models/arvados_base.rb | 4 +- .../workbench/app/views/layouts/body.html.erb | 2 +- .../test/integration/ajax_errors_test.rb | 2 +- .../integration/collection_upload_test.rb | 6 +- .../collection_unit_test.rb | 2 +- ...install-workbench2-app.html.textile.liquid | 2 +- .../app/controllers/application_controller.rb | 2 +- .../arvados/v1/groups_controller.rb | 2 +- .../arvados/v1/nodes_controller.rb | 2 +- .../arvados/v1/users_controller.rb | 6 +- .../app/models/api_client_authorization.rb | 8 +- services/api/app/models/arvados_model.rb | 2 +- services/api/app/models/container.rb | 8 +- services/api/app/models/container_request.rb | 6 +- services/api/app/models/job.rb | 2 +- services/api/app/models/keep_disk.rb | 2 +- services/api/app/models/node.rb | 2 +- services/api/app/models/user.rb | 2 +- ...130118002239_rename_metadata_attributes.rb | 4 +- ...0223_set_group_class_on_anonymous_group.rb | 4 +- ...portable_data_hash_with_hinted_manifest.rb | 2 +- .../20221219165512_dedup_permission_links.rb | 4 +- .../lib/tasks/manage_long_lived_tokens.rake | 2 +- services/api/lib/trashable.rb | 6 +- .../v1/container_requests_controller_test.rb | 4 +- .../arvados/v1/groups_controller_test.rb | 2 +- .../arvados/v1/users_controller_test.rb | 2 +- services/api/test/unit/arvados_model_test.rb | 10 +- services/api/test/unit/collection_test.rb | 106 ++++---- .../api/test/unit/container_request_test.rb | 156 ++++++------ services/api/test/unit/container_test.rb | 238 +++++++++--------- .../test/unit/create_superuser_token_test.rb | 2 +- services/api/test/unit/group_test.rb | 58 ++--- services/api/test/unit/link_test.rb | 6 +- services/api/test/unit/log_test.rb | 2 +- services/api/test/unit/owner_test.rb | 8 +- services/api/test/unit/permission_test.rb | 18 +- services/api/test/unit/repository_test.rb | 2 +- services/api/test/unit/user_test.rb | 4 +- services/api/test/unit/workflow_test.rb | 18 +- 44 files changed, 368 insertions(+), 368 deletions(-) diff --git a/apps/workbench/app/controllers/application_controller.rb b/apps/workbench/app/controllers/application_controller.rb index c2636bf5d7..3e4502545b 100644 --- a/apps/workbench/app/controllers/application_controller.rb +++ b/apps/workbench/app/controllers/application_controller.rb @@ -374,7 +374,7 @@ class ApplicationController < ActionController::Base end end end - if @object.update_attributes @updates + if @object.update @updates show else self.render_error status: 422 @@ -400,7 +400,7 @@ class ApplicationController < ActionController::Base @new_resource_attrs ||= params[model_class.to_s.underscore.singularize] @new_resource_attrs ||= {} @object = @object.dup - @object.update_attributes @new_resource_attrs + @object.update @new_resource_attrs if not @new_resource_attrs[:name] and @object.respond_to? :name if @object.name and @object.name != '' @object.name = "Copy of #{@object.name}" diff --git a/apps/workbench/app/controllers/collections_controller.rb b/apps/workbench/app/controllers/collections_controller.rb index 680c324f5c..812b80b8ce 100644 --- a/apps/workbench/app/controllers/collections_controller.rb +++ b/apps/workbench/app/controllers/collections_controller.rb @@ -258,7 +258,7 @@ class CollectionsController < ApplicationController arv_coll.rm "."+p end - if @object.update_attributes manifest_text: arv_coll.manifest_text + if @object.update manifest_text: arv_coll.manifest_text show else self.render_error status: 422 @@ -289,7 +289,7 @@ class CollectionsController < ApplicationController else arv_coll.rename "./"+file_path, new_file_path - if @object.update_attributes manifest_text: arv_coll.manifest_text + if @object.update manifest_text: arv_coll.manifest_text show else self.render_error status: 422 diff --git a/apps/workbench/app/controllers/container_requests_controller.rb b/apps/workbench/app/controllers/container_requests_controller.rb index be463b022c..9fb534ec24 100644 --- a/apps/workbench/app/controllers/container_requests_controller.rb +++ b/apps/workbench/app/controllers/container_requests_controller.rb @@ -83,7 +83,7 @@ class ContainerRequestsController < ApplicationController @object.state = 'Final' end end - @object.update_attributes! priority: 0 + @object.update! priority: 0 if params[:return_to] redirect_to params[:return_to] else diff --git a/apps/workbench/app/controllers/projects_controller.rb b/apps/workbench/app/controllers/projects_controller.rb index e448e1b453..53a6d80446 100644 --- a/apps/workbench/app/controllers/projects_controller.rb +++ b/apps/workbench/app/controllers/projects_controller.rb @@ -141,7 +141,7 @@ class ProjectsController < ApplicationController # Object is owned by this project. Remove it from the project by # changing owner to the current user. begin - item.update_attributes owner_uuid: current_user.uuid + item.update owner_uuid: current_user.uuid @removed_uuids << item.uuid rescue ArvadosApiClient::ApiErrorResponseException => e if e.message.include? '_owner_uuid_' @@ -151,7 +151,7 @@ class ProjectsController < ApplicationController updates = {} updates[:name] = rename_to updates[:owner_uuid] = current_user.uuid - item.update_attributes updates + item.update updates @removed_uuids << item.uuid else raise @@ -170,7 +170,7 @@ class ProjectsController < ApplicationController end while (objects = @object.contents).any? objects.each do |object| - object.update_attributes! owner_uuid: current_user.uuid + object.update! owner_uuid: current_user.uuid end end if ArvadosBase::resource_class_for_uuid(@object.owner_uuid) == Group diff --git a/apps/workbench/app/models/arvados_base.rb b/apps/workbench/app/models/arvados_base.rb index c5e1a4ed22..cb5eb803bc 100644 --- a/apps/workbench/app/models/arvados_base.rb +++ b/apps/workbench/app/models/arvados_base.rb @@ -300,12 +300,12 @@ class ArvadosBase self.name.underscore.pluralize.downcase end - def update_attributes raw_params={} + def update raw_params={} assign_attributes(self.class.permit_attribute_params(raw_params)) save end - def update_attributes! raw_params={} + def update! raw_params={} assign_attributes(self.class.permit_attribute_params(raw_params)) save! end diff --git a/apps/workbench/app/views/layouts/body.html.erb b/apps/workbench/app/views/layouts/body.html.erb index 8b114478ba..7cb00a15c0 100644 --- a/apps/workbench/app/views/layouts/body.html.erb +++ b/apps/workbench/app/views/layouts/body.html.erb @@ -286,6 +286,6 @@ SPDX-License-Identifier: AGPL-3.0 %> <% prefs = current_user.prefs prefs[:getting_started_shown] = Time.now - current_user.update_attributes prefs: prefs.to_json + current_user.update prefs: prefs.to_json %> <% end %> diff --git a/apps/workbench/test/integration/ajax_errors_test.rb b/apps/workbench/test/integration/ajax_errors_test.rb index b3b1f1f57a..40e7f9ea8f 100644 --- a/apps/workbench/test/integration/ajax_errors_test.rb +++ b/apps/workbench/test/integration/ajax_errors_test.rb @@ -38,7 +38,7 @@ class AjaxErrorsTest < ActionDispatch::IntegrationTest # Go behind Workbench's back to expire the "active" token. token = api_fixture('api_client_authorizations')['active']['api_token'] auth = ApiClientAuthorization.find(token) - auth.update_attributes(expires_at: '1999-12-31T23:59:59Z') + auth.update(expires_at: '1999-12-31T23:59:59Z') end click_link "Subprojects" wait_for_ajax diff --git a/apps/workbench/test/integration/collection_upload_test.rb b/apps/workbench/test/integration/collection_upload_test.rb index 608cd521de..43e7a22dc2 100644 --- a/apps/workbench/test/integration/collection_upload_test.rb +++ b/apps/workbench/test/integration/collection_upload_test.rb @@ -21,7 +21,7 @@ class CollectionUploadTest < ActionDispatch::IntegrationTest teardown do use_token :admin do @keep_services.each do |ks| - KeepService.find(ks.uuid).update_attributes(ks.attributes) + KeepService.find(ks.uuid).update(ks.attributes) end end testfiles.each do |filename, _| @@ -80,7 +80,7 @@ class CollectionUploadTest < ActionDispatch::IntegrationTest need_selenium "to make file uploads work" use_token :admin do KeepService.where(service_type: 'proxy').first. - update_attributes(service_ssl_flag: false) + update(service_ssl_flag: false) end visit page_with_token 'active', sandbox_path find('.nav-tabs a', text: 'Upload').click @@ -99,7 +99,7 @@ class CollectionUploadTest < ActionDispatch::IntegrationTest # Even if port 0 is a thing, surely nx.example.net won't # respond KeepService.where(service_type: 'proxy').first. - update_attributes(service_host: 'nx.example.net', + update(service_host: 'nx.example.net', service_port: 0) end visit page_with_token 'active', sandbox_path diff --git a/apps/workbench/test/integration_performance/collection_unit_test.rb b/apps/workbench/test/integration_performance/collection_unit_test.rb index 3feef945d1..bf5dd5b478 100644 --- a/apps/workbench/test/integration_performance/collection_unit_test.rb +++ b/apps/workbench/test/integration_performance/collection_unit_test.rb @@ -58,7 +58,7 @@ class BigCollectionTest < ActiveSupport::TestCase end time_block 'update(name-only)' do manifest_text_length = c.manifest_text.length - c.update_attributes name: 'renamed during test case' + c.update name: 'renamed during test case' assert_equal c.manifest_text.length, manifest_text_length end time_block 'update' do diff --git a/doc/install/install-workbench2-app.html.textile.liquid b/doc/install/install-workbench2-app.html.textile.liquid index 6315961182..bbcbd7ef1d 100644 --- a/doc/install/install-workbench2-app.html.textile.liquid +++ b/doc/install/install-workbench2-app.html.textile.liquid @@ -99,7 +99,7 @@ At the console, enter the following commands to locate the ApiClient record for => ["https://workbench.example.com/", Sat, 19 Apr 2014 03:35:12 UTC +00:00] irb(main):002:0> include CurrentApiClient => true -irb(main):003:0> act_as_system_user do wb.update_attributes!(is_trusted: true) end +irb(main):003:0> act_as_system_user do wb.update!(is_trusted: true) end => true diff --git a/services/api/app/controllers/application_controller.rb b/services/api/app/controllers/application_controller.rb index b191550240..d62a09b0a9 100644 --- a/services/api/app/controllers/application_controller.rb +++ b/services/api/app/controllers/application_controller.rb @@ -120,7 +120,7 @@ class ApplicationController < ActionController::Base attrs_to_update = resource_attrs.reject { |k,v| [:kind, :etag, :href].index k } - @object.update_attributes! attrs_to_update + @object.update! attrs_to_update show end diff --git a/services/api/app/controllers/arvados/v1/groups_controller.rb b/services/api/app/controllers/arvados/v1/groups_controller.rb index efcc43db26..c362cf32d7 100644 --- a/services/api/app/controllers/arvados/v1/groups_controller.rb +++ b/services/api/app/controllers/arvados/v1/groups_controller.rb @@ -92,7 +92,7 @@ class Arvados::V1::GroupsController < ApplicationController attrs_to_update = resource_attrs.reject { |k, v| [:kind, :etag, :href].index k }.merge({async_permissions_update: true}) - @object.update_attributes!(attrs_to_update) + @object.update!(attrs_to_update) @object.save! render_accepted else diff --git a/services/api/app/controllers/arvados/v1/nodes_controller.rb b/services/api/app/controllers/arvados/v1/nodes_controller.rb index eb72b7096d..2510fd49fa 100644 --- a/services/api/app/controllers/arvados/v1/nodes_controller.rb +++ b/services/api/app/controllers/arvados/v1/nodes_controller.rb @@ -37,7 +37,7 @@ class Arvados::V1::NodesController < ApplicationController attrs_to_update = resource_attrs.reject { |k,v| [:kind, :etag, :href].index k } - @object.update_attributes!(attrs_to_update) + @object.update!(attrs_to_update) @object.assign_slot if params[:assign_slot] @object.save! show diff --git a/services/api/app/controllers/arvados/v1/users_controller.rb b/services/api/app/controllers/arvados/v1/users_controller.rb index ded86aa66d..b872c0bbab 100644 --- a/services/api/app/controllers/arvados/v1/users_controller.rb +++ b/services/api/app/controllers/arvados/v1/users_controller.rb @@ -30,7 +30,7 @@ class Arvados::V1::UsersController < ApplicationController end if needupdate.length > 0 begin - u.update_attributes!(needupdate) + u.update!(needupdate) rescue ActiveRecord::RecordInvalid loginCluster = Rails.configuration.Login.LoginCluster if u.uuid[0..4] == loginCluster && !needupdate[:username].nil? @@ -40,7 +40,7 @@ class Arvados::V1::UsersController < ApplicationController if local_user.andand.uuid[0..4] == loginCluster && local_user.uuid != u.uuid new_username = "#{needupdate[:username]}conflict#{rand(99999999)}" Rails.logger.warn("cached username '#{needupdate[:username]}' collision with user '#{local_user.uuid}' - renaming to '#{new_username}' before retrying") - local_user.update_attributes!({username: new_username}) + local_user.update!({username: new_username}) retry end end @@ -103,7 +103,7 @@ class Arvados::V1::UsersController < ApplicationController collect(&:head_uuid) todo_uuids = required_uuids - signed_uuids if todo_uuids.empty? - @object.update_attributes is_active: true + @object.update is_active: true logger.info "User #{@object.uuid} activated" else logger.warn "User #{@object.uuid} called users.activate " + diff --git a/services/api/app/models/api_client_authorization.rb b/services/api/app/models/api_client_authorization.rb index c149ffc329..a6ce6aa54a 100644 --- a/services/api/app/models/api_client_authorization.rb +++ b/services/api/app/models/api_client_authorization.rb @@ -413,9 +413,9 @@ class ApiClientAuthorization < ArvadosModel (remote_user_prefix == Rails.configuration.Login.LoginCluster or Rails.configuration.Users.NewUsersAreActive or Rails.configuration.RemoteClusters[remote_user_prefix].andand["ActivateUsers"]) - user.update_attributes!(is_active: true) + user.update!(is_active: true) elsif user.is_active && !remote_user['is_active'] - user.update_attributes!(is_active: false) + user.update!(is_active: false) end if remote_user_prefix == Rails.configuration.Login.LoginCluster and @@ -423,7 +423,7 @@ class ApiClientAuthorization < ArvadosModel user.is_admin != remote_user['is_admin'] # Remote cluster controls our user database, including the # admin flag. - user.update_attributes!(is_admin: remote_user['is_admin']) + user.update!(is_admin: remote_user['is_admin']) end end end @@ -459,7 +459,7 @@ class ApiClientAuthorization < ArvadosModel return nil end end - auth.update_attributes!(user: user, + auth.update!(user: user, api_token: stored_secret, api_client_id: 0, scopes: scopes, diff --git a/services/api/app/models/arvados_model.rb b/services/api/app/models/arvados_model.rb index d910320ec0..3d4d6ef31f 100644 --- a/services/api/app/models/arvados_model.rb +++ b/services/api/app/models/arvados_model.rb @@ -145,7 +145,7 @@ class ArvadosModel < ApplicationRecord super(permit_attribute_params(raw_params), *args) end - def update_attributes raw_params={}, *args + def update raw_params={}, *args super(self.class.permit_attribute_params(raw_params), *args) end diff --git a/services/api/app/models/container.rb b/services/api/app/models/container.rb index d2e76f74e3..44334ba12f 100644 --- a/services/api/app/models/container.rb +++ b/services/api/app/models/container.rb @@ -379,7 +379,7 @@ class Container < ArvadosModel if self.state != Queued raise LockFailedError.new("cannot lock when #{self.state}") end - self.update_attributes!(state: Locked) + self.update!(state: Locked) end end @@ -397,7 +397,7 @@ class Container < ArvadosModel if self.state != Locked raise InvalidStateTransitionError.new("cannot unlock when #{self.state}") end - self.update_attributes!(state: Queued) + self.update!(state: Queued) end end @@ -642,7 +642,7 @@ class Container < ArvadosModel # ensure the token doesn't validate later in the same # transaction (e.g., in a test case) by satisfying expires_at > # transaction timestamp. - self.auth.andand.update_attributes(expires_at: db_transaction_time) + self.auth.andand.update(expires_at: db_transaction_time) self.auth = nil return elsif self.auth @@ -835,7 +835,7 @@ class Container < ArvadosModel # Queued with priority 0. (OTOH, if the child is already # running, leave it alone so it can get cancelled the # usual way, get a copy of the log collection, etc.) - cr.update_attributes!(state: ContainerRequest::Final) + cr.update!(state: ContainerRequest::Final) end end end diff --git a/services/api/app/models/container_request.rb b/services/api/app/models/container_request.rb index d72f00edc8..9895ceb14e 100644 --- a/services/api/app/models/container_request.rb +++ b/services/api/app/models/container_request.rb @@ -164,7 +164,7 @@ class ContainerRequest < ArvadosModel end elsif state == Committed # Behave as if the container is cancelled - update_attributes!(state: Final) + update!(state: Final) end return true end @@ -228,7 +228,7 @@ class ContainerRequest < ArvadosModel end end end - update_attributes!(state: Final) + update!(state: Final) end def update_collections(container:, collections: ['log', 'output']) @@ -308,7 +308,7 @@ class ContainerRequest < ArvadosModel end def set_priority_zero - self.update_attributes!(priority: 0) if self.priority > 0 && self.state != Final + self.update!(priority: 0) if self.priority > 0 && self.state != Final end protected diff --git a/services/api/app/models/job.rb b/services/api/app/models/job.rb index 37e5f455df..f792c04842 100644 --- a/services/api/app/models/job.rb +++ b/services/api/app/models/job.rb @@ -107,7 +107,7 @@ class Job < ArvadosModel end def assert_finished - update_attributes(finished_at: finished_at || db_current_time, + update(finished_at: finished_at || db_current_time, success: success.nil? ? false : success, running: false) end diff --git a/services/api/app/models/keep_disk.rb b/services/api/app/models/keep_disk.rb index 5751c135d8..589936f845 100644 --- a/services/api/app/models/keep_disk.rb +++ b/services/api/app/models/keep_disk.rb @@ -40,7 +40,7 @@ class KeepDisk < ArvadosModel end @bypass_arvados_authorization = true - self.update_attributes!(o.select { |k,v| + self.update!(o.select { |k,v| [:bytes_total, :bytes_free, :is_readable, diff --git a/services/api/app/models/node.rb b/services/api/app/models/node.rb index c8a606e2b8..adc2512475 100644 --- a/services/api/app/models/node.rb +++ b/services/api/app/models/node.rb @@ -176,7 +176,7 @@ class Node < ArvadosModel # as the new node. Clear the ip_address field on the stale # nodes. Otherwise, we (via SLURM) might inadvertently connect # to the new node using the old node's hostname. - stale_node.update_attributes!(ip_address: nil) + stale_node.update!(ip_address: nil) end end if hostname_before_last_save && saved_change_to_hostname? diff --git a/services/api/app/models/user.rb b/services/api/app/models/user.rb index bbdd9c2843..88314c6775 100644 --- a/services/api/app/models/user.rb +++ b/services/api/app/models/user.rb @@ -497,7 +497,7 @@ SELECT target_uuid, perm_level end if redirect_to_new_user - update_attributes!(redirect_to_user_uuid: new_user.uuid, username: nil) + update!(redirect_to_user_uuid: new_user.uuid, username: nil) end skip_check_permissions_against_full_refresh do update_permissions self.uuid, self.uuid, CAN_MANAGE_PERM diff --git a/services/api/db/migrate/20130118002239_rename_metadata_attributes.rb b/services/api/db/migrate/20130118002239_rename_metadata_attributes.rb index 049b5e2d63..2c1b406000 100644 --- a/services/api/db/migrate/20130118002239_rename_metadata_attributes.rb +++ b/services/api/db/migrate/20130118002239_rename_metadata_attributes.rb @@ -17,7 +17,7 @@ class RenameMetadataAttributes < ActiveRecord::Migration[4.2] Metadatum.where('head like ?', 'orvos#%').each do |m| kind_uuid = m.head.match /^(orvos\#.*)\#([-0-9a-z]+)$/ if kind_uuid - m.update_attributes(head_kind: kind_uuid[1], + m.update(head_kind: kind_uuid[1], head: kind_uuid[2]) end end @@ -28,7 +28,7 @@ class RenameMetadataAttributes < ActiveRecord::Migration[4.2] def down begin Metadatum.where('head_kind is not null and head_kind <> ? and head is not null', '').each do |m| - m.update_attributes(head: m.head_kind + '#' + m.head) + m.update(head: m.head_kind + '#' + m.head) end rescue end diff --git a/services/api/db/migrate/20150203180223_set_group_class_on_anonymous_group.rb b/services/api/db/migrate/20150203180223_set_group_class_on_anonymous_group.rb index 71f769c157..0a05718fdd 100644 --- a/services/api/db/migrate/20150203180223_set_group_class_on_anonymous_group.rb +++ b/services/api/db/migrate/20150203180223_set_group_class_on_anonymous_group.rb @@ -6,13 +6,13 @@ class SetGroupClassOnAnonymousGroup < ActiveRecord::Migration[4.2] include CurrentApiClient def up act_as_system_user do - anonymous_group.update_attributes group_class: 'role', name: 'Anonymous users', description: 'Anonymous users' + anonymous_group.update group_class: 'role', name: 'Anonymous users', description: 'Anonymous users' end end def down act_as_system_user do - anonymous_group.update_attributes group_class: nil, name: 'Anonymous group', description: 'Anonymous group' + anonymous_group.update group_class: nil, name: 'Anonymous group', description: 'Anonymous group' end end end diff --git a/services/api/db/migrate/20150303210106_fix_collection_portable_data_hash_with_hinted_manifest.rb b/services/api/db/migrate/20150303210106_fix_collection_portable_data_hash_with_hinted_manifest.rb index 8814fc87d3..1d3a6ed1b4 100644 --- a/services/api/db/migrate/20150303210106_fix_collection_portable_data_hash_with_hinted_manifest.rb +++ b/services/api/db/migrate/20150303210106_fix_collection_portable_data_hash_with_hinted_manifest.rb @@ -107,7 +107,7 @@ class FixCollectionPortableDataHashWithHintedManifest < ActiveRecord::Migration[ attributes[:properties]["migrated_from"] ||= coll.uuid coll_copy = Collection.create!(attributes) Log.log_create(coll_copy) - coll.update_attributes(portable_data_hash: stripped_pdh) + coll.update(portable_data_hash: stripped_pdh) Log.log_update(coll, start_log) end end diff --git a/services/api/db/migrate/20221219165512_dedup_permission_links.rb b/services/api/db/migrate/20221219165512_dedup_permission_links.rb index 6ae04cc954..6aef343f1c 100644 --- a/services/api/db/migrate/20221219165512_dedup_permission_links.rb +++ b/services/api/db/migrate/20221219165512_dedup_permission_links.rb @@ -22,7 +22,7 @@ class DedupPermissionLinks < ActiveRecord::Migration[5.2] # This no-op update has the side effect that the update hooks # will merge the highest available permission into this one # and then delete the others. - link.update_attributes!(properties: link.properties.dup) + link.update!(properties: link.properties.dup) end rows = ActiveRecord::Base.connection.select_all("SELECT MIN(uuid) AS uuid, COUNT(uuid) AS n FROM links @@ -35,7 +35,7 @@ class DedupPermissionLinks < ActiveRecord::Migration[5.2] rows.each do |row| Rails.logger.debug "DedupPermissionLinks: consolidating #{row['n']} links into #{row['uuid']}" link = Link.find_by_uuid(row['uuid']) - link.update_attributes!(properties: link.properties.dup) + link.update!(properties: link.properties.dup) end end end diff --git a/services/api/lib/tasks/manage_long_lived_tokens.rake b/services/api/lib/tasks/manage_long_lived_tokens.rake index 7a665ff7e7..70a0f24284 100644 --- a/services/api/lib/tasks/manage_long_lived_tokens.rake +++ b/services/api/lib/tasks/manage_long_lived_tokens.rake @@ -31,7 +31,7 @@ namespace :db do end if (auth.user.uuid =~ /-tpzed-000000000000000/).nil? and (auth.user.uuid =~ /-tpzed-anonymouspublic/).nil? CurrentApiClientHelper.act_as_system_user do - auth.update_attributes!(expires_at: exp_date) + auth.update!(expires_at: exp_date) end token_count += 1 end diff --git a/services/api/lib/trashable.rb b/services/api/lib/trashable.rb index c99b08513b..50611c305d 100644 --- a/services/api/lib/trashable.rb +++ b/services/api/lib/trashable.rb @@ -93,19 +93,19 @@ end module TrashableController def destroy if !@object.is_trashed - @object.update_attributes!(trash_at: db_current_time) + @object.update!(trash_at: db_current_time) end earliest_delete = (@object.trash_at + Rails.configuration.Collections.BlobSigningTTL) if @object.delete_at > earliest_delete - @object.update_attributes!(delete_at: earliest_delete) + @object.update!(delete_at: earliest_delete) end show end def trash if !@object.is_trashed - @object.update_attributes!(trash_at: db_current_time) + @object.update!(trash_at: db_current_time) end show end diff --git a/services/api/test/functional/arvados/v1/container_requests_controller_test.rb b/services/api/test/functional/arvados/v1/container_requests_controller_test.rb index f287a11faf..87eb37cde7 100644 --- a/services/api/test/functional/arvados/v1/container_requests_controller_test.rb +++ b/services/api/test/functional/arvados/v1/container_requests_controller_test.rb @@ -103,7 +103,7 @@ class Arvados::V1::ContainerRequestsControllerTest < ActionController::TestCase test "update without deleting secret_mounts" do authorize_with :active req = container_requests(:uncommitted) - req.update_attributes!(secret_mounts: {'/foo' => {'kind' => 'json', 'content' => 'bar'}}) + req.update!(secret_mounts: {'/foo' => {'kind' => 'json', 'content' => 'bar'}}) patch :update, params: { id: req.uuid, @@ -169,7 +169,7 @@ class Arvados::V1::ContainerRequestsControllerTest < ActionController::TestCase test "filter on container subproperty runtime_status[foo] = bar" do ctr = containers(:running) act_as_system_user do - ctr.update_attributes!(runtime_status: {foo: 'bar'}) + ctr.update!(runtime_status: {foo: 'bar'}) end authorize_with :active get :index, params: { 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 a64ea76692..d8daa4bdd7 100644 --- a/services/api/test/functional/arvados/v1/groups_controller_test.rb +++ b/services/api/test/functional/arvados/v1/groups_controller_test.rb @@ -984,7 +984,7 @@ class Arvados::V1::GroupsControllerTest < ActionController::TestCase innertrash = Collection.create!(name: 'inner-trashed', owner_uuid: innerproj.uuid, trash_at: trashtime) innertrashproj = Group.create!(group_class: 'project', name: 'inner-trashed-proj', owner_uuid: innerproj.uuid, trash_at: trashtime) outertrash = Collection.create!(name: 'outer-trashed', owner_uuid: outerproj.uuid, trash_at: trashtime) - innerproj.update_attributes!(frozen_by_uuid: users(:active).uuid) + innerproj.update!(frozen_by_uuid: users(:active).uuid) get :contents, params: {id: outerproj.uuid, include_trash: true, recursive: true} assert_response :success uuids = json_response['items'].collect { |item| item['uuid'] } diff --git a/services/api/test/functional/arvados/v1/users_controller_test.rb b/services/api/test/functional/arvados/v1/users_controller_test.rb index b7d683df29..8bffac8dd1 100644 --- a/services/api/test/functional/arvados/v1/users_controller_test.rb +++ b/services/api/test/functional/arvados/v1/users_controller_test.rb @@ -889,7 +889,7 @@ The Arvados team. ['dst', :project_viewer_trustedclient]].each do |which_scoped, auth| test "refuse to merge with scoped #{which_scoped} token" do act_as_system_user do - api_client_authorizations(auth).update_attributes(scopes: ["GET /", "POST /", "PUT /"]) + api_client_authorizations(auth).update(scopes: ["GET /", "POST /", "PUT /"]) end authorize_with(:active_trustedclient) post(:merge, params: { diff --git a/services/api/test/unit/arvados_model_test.rb b/services/api/test/unit/arvados_model_test.rb index 1e2e08059e..69a2710bb9 100644 --- a/services/api/test/unit/arvados_model_test.rb +++ b/services/api/test/unit/arvados_model_test.rb @@ -217,13 +217,13 @@ class ArvadosModelTest < ActiveSupport::TestCase assert group.valid?, "group is not valid" # update 1 - group.update_attributes!(name: "test create and update name 1") + group.update!(name: "test create and update name 1") results = Group.where(uuid: group.uuid) assert_equal "test create and update name 1", results.first.name, "Expected name to be updated to 1" updated_at_1 = results.first.updated_at.to_f # update 2 - group.update_attributes!(name: "test create and update name 2") + group.update!(name: "test create and update name 2") results = Group.where(uuid: group.uuid) assert_equal "test create and update name 2", results.first.name, "Expected name to be updated to 2" updated_at_2 = results.first.updated_at.to_f @@ -237,15 +237,15 @@ class ArvadosModelTest < ActiveSupport::TestCase c = Collection.create!(properties: {}) assert_equal({}, c.properties) - c.update_attributes(properties: {'foo' => 'foo'}) + c.update(properties: {'foo' => 'foo'}) c.reload assert_equal({'foo' => 'foo'}, c.properties) - c.update_attributes(properties: nil) + c.update(properties: nil) c.reload assert_equal({}, c.properties) - c.update_attributes(properties: {foo: 'bar'}) + c.update(properties: {foo: 'bar'}) assert_equal({'foo' => 'bar'}, c.properties) c.reload assert_equal({'foo' => 'bar'}, c.properties) diff --git a/services/api/test/unit/collection_test.rb b/services/api/test/unit/collection_test.rb index e7134a5be5..f3b48dbf70 100644 --- a/services/api/test/unit/collection_test.rb +++ b/services/api/test/unit/collection_test.rb @@ -91,19 +91,19 @@ class CollectionTest < ActiveSupport::TestCase assert_equal 34, c.file_size_total # Updating the manifest should change file stats - c.update_attributes(manifest_text: ". d41d8cd98f00b204e9800998ecf8427e 0:34:foo.txt 0:34:foo2.txt\n") + c.update(manifest_text: ". d41d8cd98f00b204e9800998ecf8427e 0:34:foo.txt 0:34:foo2.txt\n") assert c.valid? assert_equal 2, c.file_count assert_equal 68, c.file_size_total # Updating file stats and the manifest should use manifest values - c.update_attributes(manifest_text: ". d41d8cd98f00b204e9800998ecf8427e 0:34:foo.txt\n", file_count:10, file_size_total: 10) + c.update(manifest_text: ". d41d8cd98f00b204e9800998ecf8427e 0:34:foo.txt\n", file_count:10, file_size_total: 10) assert c.valid? assert_equal 1, c.file_count assert_equal 34, c.file_size_total # Updating just the file stats should be ignored - c.update_attributes(file_count: 10, file_size_total: 10) + c.update(file_count: 10, file_size_total: 10) assert c.valid? assert_equal 1, c.file_count assert_equal 34, c.file_size_total @@ -166,7 +166,7 @@ class CollectionTest < ActiveSupport::TestCase assert_equal 1, c.version assert_equal false, c.preserve_version # Make a versionable update, it shouldn't create a new version yet - c.update_attributes!({'name' => 'bar'}) + c.update!({'name' => 'bar'}) c.reload assert_equal 'bar', c.name assert_equal 1, c.version @@ -175,12 +175,12 @@ class CollectionTest < ActiveSupport::TestCase c.update_column('modified_at', fifteen_min_ago) # Update without validations/callbacks c.reload assert_equal fifteen_min_ago.to_i, c.modified_at.to_i - c.update_attributes!({'name' => 'baz'}) + c.update!({'name' => 'baz'}) c.reload assert_equal 'baz', c.name assert_equal 2, c.version # Make another update, no new version should be created - c.update_attributes!({'name' => 'foobar'}) + c.update!({'name' => 'foobar'}) c.reload assert_equal 'foobar', c.name assert_equal 2, c.version @@ -197,7 +197,7 @@ class CollectionTest < ActiveSupport::TestCase assert_not_nil c.replication_confirmed_at assert_not_nil c.replication_confirmed # Make the versionable update - c.update_attributes!({'name' => 'foobarbaz'}) + c.update!({'name' => 'foobarbaz'}) c.reload assert_equal 'foobarbaz', c.name assert_equal 3, c.version @@ -214,7 +214,7 @@ class CollectionTest < ActiveSupport::TestCase assert_equal 1, c.version assert_equal false, c.preserve_version # This update shouldn't produce a new version, as the idle time is not up - c.update_attributes!({ + c.update!({ 'name' => 'bar' }) c.reload @@ -223,7 +223,7 @@ class CollectionTest < ActiveSupport::TestCase assert_equal false, c.preserve_version # This update should produce a new version, even if the idle time is not up # and also keep the preserve_version=true flag to persist it. - c.update_attributes!({ + c.update!({ 'name' => 'baz', 'preserve_version' => true }) @@ -234,7 +234,7 @@ class CollectionTest < ActiveSupport::TestCase # Make sure preserve_version is not disabled after being enabled, unless # a new version is created. # This is a non-versionable update - c.update_attributes!({ + c.update!({ 'preserve_version' => false, 'replication_desired' => 2 }) @@ -243,7 +243,7 @@ class CollectionTest < ActiveSupport::TestCase assert_equal 2, c.replication_desired assert_equal true, c.preserve_version # This is a versionable update - c.update_attributes!({ + c.update!({ 'preserve_version' => false, 'name' => 'foobar' }) @@ -252,7 +252,7 @@ class CollectionTest < ActiveSupport::TestCase assert_equal false, c.preserve_version assert_equal 'foobar', c.name # Flipping only 'preserve_version' to true doesn't create a new version - c.update_attributes!({'preserve_version' => true}) + c.update!({'preserve_version' => true}) c.reload assert_equal 3, c.version assert_equal true, c.preserve_version @@ -265,7 +265,7 @@ class CollectionTest < ActiveSupport::TestCase assert c.valid? assert_equal false, c.preserve_version modified_at = c.modified_at.to_f - c.update_attributes!({'preserve_version' => true}) + c.update!({'preserve_version' => true}) c.reload assert_equal true, c.preserve_version assert_equal modified_at, c.modified_at.to_f, @@ -285,7 +285,7 @@ class CollectionTest < ActiveSupport::TestCase assert_equal 1, c.version assert_raises(ActiveRecord::RecordInvalid) do - c.update_attributes!({ + c.update!({ name => new_value }) end @@ -302,14 +302,14 @@ class CollectionTest < ActiveSupport::TestCase assert c.valid? assert_equal 1, c.version # Make changes so that a new version is created - c.update_attributes!({'name' => 'bar'}) + c.update!({'name' => 'bar'}) c.reload assert_equal 2, c.version assert_equal 2, Collection.where(current_version_uuid: c.uuid).count new_uuid = 'zzzzz-4zz18-somefakeuuidnow' assert_empty Collection.where(uuid: new_uuid) # Update UUID on current version, check that both collections point to it - c.update_attributes!({'uuid' => new_uuid}) + c.update!({'uuid' => new_uuid}) c.reload assert_equal new_uuid, c.uuid assert_equal 2, Collection.where(current_version_uuid: new_uuid).count @@ -364,7 +364,7 @@ class CollectionTest < ActiveSupport::TestCase # Set up initial collection c = create_collection 'foo', Encoding::US_ASCII assert c.valid? - c.update_attributes!({'properties' => value_1}) + c.update!({'properties' => value_1}) c.reload assert c.changes.keys.empty? c.properties = value_2 @@ -386,7 +386,7 @@ class CollectionTest < ActiveSupport::TestCase assert c.valid? original_version_modified_at = c.modified_at.to_f # Make changes so that a new version is created - c.update_attributes!({'name' => 'bar'}) + c.update!({'name' => 'bar'}) c.reload assert_equal 2, c.version # Get the old version @@ -400,7 +400,7 @@ class CollectionTest < ActiveSupport::TestCase # Make update on current version so old version get the attribute synced; # its modified_at should not change. new_replication = 3 - c.update_attributes!({'replication_desired' => new_replication}) + c.update!({'replication_desired' => new_replication}) c.reload assert_equal new_replication, c.replication_desired c_old.reload @@ -441,7 +441,7 @@ class CollectionTest < ActiveSupport::TestCase c = create_collection 'foo', Encoding::US_ASCII assert c.valid? # Make changes so that a new version is created - c.update_attributes!({'name' => 'bar'}) + c.update!({'name' => 'bar'}) c.reload assert_equal 2, c.version # Get the old version @@ -479,7 +479,7 @@ class CollectionTest < ActiveSupport::TestCase assert_not_equal first_val, c.attributes[attr] # Make changes so that a new version is created and a synced field is # updated on both - c.update_attributes!({'name' => 'bar', attr => first_val}) + c.update!({'name' => 'bar', attr => first_val}) c.reload assert_equal 2, c.version assert_equal first_val, c.attributes[attr] @@ -487,7 +487,7 @@ class CollectionTest < ActiveSupport::TestCase assert_equal first_val, Collection.where(current_version_uuid: c.uuid, version: 1).first.attributes[attr] # Only make an update on the same synced field & check that the previously # created version also gets it. - c.update_attributes!({attr => second_val}) + c.update!({attr => second_val}) c.reload assert_equal 2, c.version assert_equal second_val, c.attributes[attr] @@ -525,7 +525,7 @@ class CollectionTest < ActiveSupport::TestCase # Update attribute and check if version number should be incremented old_value = c.attributes[attr] - c.update_attributes!({attr => val}) + c.update!({attr => val}) assert_equal new_version_expected, c.version == 2 assert_equal val, c.attributes[attr] @@ -559,11 +559,11 @@ class CollectionTest < ActiveSupport::TestCase col2 = create_collection 'bar', Encoding::US_ASCII assert col2.valid? assert_equal 1, col2.version - col2.update_attributes({name: 'baz'}) + col2.update({name: 'baz'}) assert_equal 2, col2.version # Try to make col2 a past version of col1. It shouldn't be possible - col2.update_attributes({current_version_uuid: col1.uuid}) + col2.update({current_version_uuid: col1.uuid}) assert col2.invalid? col2.reload assert_not_equal col1.uuid, col2.current_version_uuid @@ -725,10 +725,10 @@ class CollectionTest < ActiveSupport::TestCase test "storage_classes_desired cannot be empty" do act_as_user users(:active) do c = collections(:collection_owned_by_active) - c.update_attributes storage_classes_desired: ["hot"] + c.update storage_classes_desired: ["hot"] assert_equal ["hot"], c.storage_classes_desired assert_raise ArvadosModel::InvalidStateTransitionError do - c.update_attributes storage_classes_desired: [] + c.update storage_classes_desired: [] end end end @@ -736,7 +736,7 @@ class CollectionTest < ActiveSupport::TestCase test "storage classes lists should only contain non-empty strings" do c = collections(:storage_classes_desired_default_unconfirmed) act_as_user users(:admin) do - assert c.update_attributes(storage_classes_desired: ["default", "a_string"], + assert c.update(storage_classes_desired: ["default", "a_string"], storage_classes_confirmed: ["another_string"]) [ ["storage_classes_desired", ["default", 42]], @@ -745,7 +745,7 @@ class CollectionTest < ActiveSupport::TestCase ["storage_classes_confirmed", [""]], ].each do |attr, val| assert_raise ArvadosModel::InvalidStateTransitionError do - assert c.update_attributes({attr => val}) + assert c.update({attr => val}) end end end @@ -754,7 +754,7 @@ class CollectionTest < ActiveSupport::TestCase test "storage_classes_confirmed* can be set by admin user" do c = collections(:storage_classes_desired_default_unconfirmed) act_as_user users(:admin) do - assert c.update_attributes(storage_classes_confirmed: ["default"], + assert c.update(storage_classes_confirmed: ["default"], storage_classes_confirmed_at: Time.now) end end @@ -764,16 +764,16 @@ class CollectionTest < ActiveSupport::TestCase c = collections(:storage_classes_desired_default_unconfirmed) # Cannot set just one at a time. assert_raise ArvadosModel::PermissionDeniedError do - c.update_attributes storage_classes_confirmed: ["default"] + c.update storage_classes_confirmed: ["default"] end c.reload assert_raise ArvadosModel::PermissionDeniedError do - c.update_attributes storage_classes_confirmed_at: Time.now + c.update storage_classes_confirmed_at: Time.now end # Cannot set bot at once, either. c.reload assert_raise ArvadosModel::PermissionDeniedError do - assert c.update_attributes(storage_classes_confirmed: ["default"], + assert c.update(storage_classes_confirmed: ["default"], storage_classes_confirmed_at: Time.now) end end @@ -784,15 +784,15 @@ class CollectionTest < ActiveSupport::TestCase c = collections(:storage_classes_desired_default_confirmed_default) # Cannot clear just one at a time. assert_raise ArvadosModel::PermissionDeniedError do - c.update_attributes storage_classes_confirmed: [] + c.update storage_classes_confirmed: [] end c.reload assert_raise ArvadosModel::PermissionDeniedError do - c.update_attributes storage_classes_confirmed_at: nil + c.update storage_classes_confirmed_at: nil end # Can clear both at once. c.reload - assert c.update_attributes(storage_classes_confirmed: [], + assert c.update(storage_classes_confirmed: [], storage_classes_confirmed_at: nil) end end @@ -802,7 +802,7 @@ class CollectionTest < ActiveSupport::TestCase Rails.configuration.Collections.DefaultReplication = 2 act_as_user users(:active) do c = collections(:replication_undesired_unconfirmed) - c.update_attributes replication_desired: ask + c.update replication_desired: ask assert_equal ask, c.replication_desired end end @@ -811,7 +811,7 @@ class CollectionTest < ActiveSupport::TestCase test "replication_confirmed* can be set by admin user" do c = collections(:replication_desired_2_unconfirmed) act_as_user users(:admin) do - assert c.update_attributes(replication_confirmed: 2, + assert c.update(replication_confirmed: 2, replication_confirmed_at: Time.now) end end @@ -821,14 +821,14 @@ class CollectionTest < ActiveSupport::TestCase c = collections(:replication_desired_2_unconfirmed) # Cannot set just one at a time. assert_raise ArvadosModel::PermissionDeniedError do - c.update_attributes replication_confirmed: 1 + c.update replication_confirmed: 1 end assert_raise ArvadosModel::PermissionDeniedError do - c.update_attributes replication_confirmed_at: Time.now + c.update replication_confirmed_at: Time.now end # Cannot set both at once, either. assert_raise ArvadosModel::PermissionDeniedError do - c.update_attributes(replication_confirmed: 1, + c.update(replication_confirmed: 1, replication_confirmed_at: Time.now) end end @@ -839,15 +839,15 @@ class CollectionTest < ActiveSupport::TestCase c = collections(:replication_desired_2_confirmed_2) # Cannot clear just one at a time. assert_raise ArvadosModel::PermissionDeniedError do - c.update_attributes replication_confirmed: nil + c.update replication_confirmed: nil end c.reload assert_raise ArvadosModel::PermissionDeniedError do - c.update_attributes replication_confirmed_at: nil + c.update replication_confirmed_at: nil end # Can clear both at once. c.reload - assert c.update_attributes(replication_confirmed: nil, + assert c.update(replication_confirmed: nil, replication_confirmed_at: nil) end end @@ -855,7 +855,7 @@ class CollectionTest < ActiveSupport::TestCase test "clear replication_confirmed* when introducing a new block in manifest" do c = collections(:replication_desired_2_confirmed_2) act_as_user users(:active) do - assert c.update_attributes(manifest_text: collections(:user_agreement).signed_manifest_text_only_for_tests) + assert c.update(manifest_text: collections(:user_agreement).signed_manifest_text_only_for_tests) assert_nil c.replication_confirmed assert_nil c.replication_confirmed_at end @@ -865,7 +865,7 @@ class CollectionTest < ActiveSupport::TestCase c = collections(:replication_desired_2_confirmed_2) act_as_user users(:active) do new_manifest = c.signed_manifest_text_only_for_tests.sub(':bar', ':foo') - assert c.update_attributes(manifest_text: new_manifest) + assert c.update(manifest_text: new_manifest) assert_equal 2, c.replication_confirmed assert_not_nil c.replication_confirmed_at end @@ -882,7 +882,7 @@ class CollectionTest < ActiveSupport::TestCase # not, this test would pass without testing the relevant case): assert_operator new_manifest.length+40, :<, c.signed_manifest_text_only_for_tests.length - assert c.update_attributes(manifest_text: new_manifest) + assert c.update(manifest_text: new_manifest) assert_equal 2, c.replication_confirmed assert_not_nil c.replication_confirmed_at end @@ -892,7 +892,7 @@ class CollectionTest < ActiveSupport::TestCase act_as_user users(:active) do t0 = db_current_time c = Collection.create!(manifest_text: ". d41d8cd98f00b204e9800998ecf8427e+0 0:0:x\n", name: 'foo') - c.update_attributes! trash_at: (t0 + 1.hours) + c.update! trash_at: (t0 + 1.hours) c.reload sig_exp = /\+A[0-9a-f]{40}\@([0-9]+)/.match(c.signed_manifest_text_only_for_tests)[1].to_i assert_operator sig_exp.to_i, :<=, (t0 + 1.hours).to_i @@ -932,7 +932,7 @@ class CollectionTest < ActiveSupport::TestCase assert_not_empty c, 'Should be able to find live collection' # mark collection as expired - c.first.update_attributes!(trash_at: Time.new.strftime("%Y-%m-%d")) + c.first.update!(trash_at: Time.new.strftime("%Y-%m-%d")) c = Collection.readable_by(current_user).where(uuid: uuid) assert_empty c, 'Should not be able to find expired collection' @@ -947,7 +947,7 @@ class CollectionTest < ActiveSupport::TestCase act_as_user users(:active) do t0 = db_current_time c = Collection.create!(manifest_text: '', name: 'foo') - c.update_attributes! trash_at: (t0 - 2.weeks) + c.update! trash_at: (t0 - 2.weeks) c.reload assert_operator c.trash_at, :>, t0 end @@ -1002,7 +1002,7 @@ class CollectionTest < ActiveSupport::TestCase else c = collections(fixture_name) end - updates_ok = c.update_attributes(updates) + updates_ok = c.update(updates) expect_valid = expect[:state] != :invalid assert_equal expect_valid, updates_ok, c.errors.full_messages.to_s case expect[:state] @@ -1039,13 +1039,13 @@ class CollectionTest < ActiveSupport::TestCase start = db_current_time act_as_user users(:active) do c = Collection.create!(manifest_text: '', name: 'foo') - c.update_attributes!(trash_at: start + 86400.seconds) + c.update!(trash_at: start + 86400.seconds) assert_operator c.delete_at, :>=, start + (86400*22).seconds assert_operator c.delete_at, :<, start + (86400*22 + 30).seconds c.destroy c = Collection.create!(manifest_text: '', name: 'foo') - c.update_attributes!(is_trashed: true) + c.update!(is_trashed: true) assert_operator c.delete_at, :>=, start + (86400*21).seconds end end diff --git a/services/api/test/unit/container_request_test.rb b/services/api/test/unit/container_request_test.rb index a64adba6ff..98136aa53b 100644 --- a/services/api/test/unit/container_request_test.rb +++ b/services/api/test/unit/container_request_test.rb @@ -34,8 +34,8 @@ class ContainerRequestTest < ActiveSupport::TestCase def lock_and_run(ctr) act_as_system_user do - ctr.update_attributes!(state: Container::Locked) - ctr.update_attributes!(state: Container::Running) + ctr.update!(state: Container::Locked) + ctr.update!(state: Container::Running) end end @@ -129,7 +129,7 @@ class ContainerRequestTest < ActiveSupport::TestCase cr.save! assert_raises(ActiveRecord::RecordInvalid) do cr = ContainerRequest.find_by_uuid cr.uuid - cr.update_attributes!({state: "Committed", + cr.update!({state: "Committed", priority: 1}.merge(value)) end end @@ -138,7 +138,7 @@ class ContainerRequestTest < ActiveSupport::TestCase test "Update from fixture" do set_user_from_auth :active cr = ContainerRequest.find_by_uuid(container_requests(:running).uuid) - cr.update_attributes!(description: "New description") + cr.update!(description: "New description") assert_equal "New description", cr.description end @@ -147,7 +147,7 @@ class ContainerRequestTest < ActiveSupport::TestCase cr = create_minimal_req!(state: "Uncommitted", priority: 1) cr.save! cr = ContainerRequest.find_by_uuid cr.uuid - cr.update_attributes!(state: "Committed", + cr.update!(state: "Committed", runtime_constraints: {"vcpus" => 1, "ram" => 23}) assert_not_nil cr.container_uuid end @@ -217,7 +217,7 @@ class ContainerRequestTest < ActiveSupport::TestCase assert_operator c1.priority, :<, c2.priority c2priority_was = c2.priority - cr1.update_attributes!(priority: 0) + cr1.update!(priority: 0) c1.reload assert_equal 0, c1.priority @@ -233,7 +233,7 @@ class ContainerRequestTest < ActiveSupport::TestCase act_as_system_user do Container.find_by_uuid(cr.container_uuid). - update_attributes!(state: Container::Cancelled, cost: 1.25) + update!(state: Container::Cancelled, cost: 1.25) end cr.reload @@ -252,8 +252,8 @@ class ContainerRequestTest < ActiveSupport::TestCase c = act_as_system_user do c = Container.find_by_uuid(cr.container_uuid) - c.update_attributes!(state: Container::Locked) - c.update_attributes!(state: Container::Running) + c.update!(state: Container::Locked) + c.update!(state: Container::Running) c end @@ -263,7 +263,7 @@ class ContainerRequestTest < ActiveSupport::TestCase output_pdh = '1f4b0bc7583c2a7f9102c395f4ffc5e3+45' log_pdh = 'fa7aeb5140e2848d39b416daeef4ffc5+45' act_as_system_user do - c.update_attributes!(state: Container::Complete, + c.update!(state: Container::Complete, cost: 1.25, output: output_pdh, log: log_pdh) @@ -302,8 +302,8 @@ class ContainerRequestTest < ActiveSupport::TestCase c = act_as_system_user do c = Container.find_by_uuid(cr.container_uuid) - c.update_attributes!(state: Container::Locked) - c.update_attributes!(state: Container::Running, + c.update!(state: Container::Locked) + c.update!(state: Container::Running, output: output_pdh, log: log_pdh) c @@ -315,7 +315,7 @@ class ContainerRequestTest < ActiveSupport::TestCase act_as_system_user do Collection.where(portable_data_hash: output_pdh).delete_all Collection.where(portable_data_hash: log_pdh).delete_all - c.update_attributes!(state: Container::Complete) + c.update!(state: Container::Complete) end cr.reload @@ -333,8 +333,8 @@ class ContainerRequestTest < ActiveSupport::TestCase c = act_as_system_user do c = Container.find_by_uuid(cr.container_uuid) - c.update_attributes!(state: Container::Locked) - c.update_attributes!(state: Container::Running) + c.update!(state: Container::Locked) + c.update!(state: Container::Running) c end @@ -454,7 +454,7 @@ class ContainerRequestTest < ActiveSupport::TestCase # increasing priority of the most recent toplevel container should # reprioritize all of its descendants (including the shared # grandchild) above everything else. - toplevel_crs[2].update_attributes!(priority: 72) + toplevel_crs[2].update!(priority: 72) (parents + children + grandchildren + [shared_grandchild]).map(&:reload) assert_operator shared_grandchild.priority, :>, grandchildren[0].priority assert_operator shared_grandchild.priority, :>, children[0].priority @@ -471,7 +471,7 @@ class ContainerRequestTest < ActiveSupport::TestCase # cancelling the most recent toplevel container should # reprioritize all of its descendants (except the shared # grandchild) to zero - toplevel_crs[2].update_attributes!(priority: 0) + toplevel_crs[2].update!(priority: 0) (parents + children + grandchildren + [shared_grandchild]).map(&:reload) assert_operator 0, :==, parents[2].priority assert_operator 0, :==, children[2].priority @@ -480,7 +480,7 @@ class ContainerRequestTest < ActiveSupport::TestCase # cancel a child request, the parent should be > 0 but # the child and grandchild go to 0. - children_crs[1].update_attributes!(priority: 0) + children_crs[1].update!(priority: 0) (parents + children + grandchildren + [shared_grandchild]).map(&:reload) assert_operator 0, :<, parents[1].priority assert_operator parents[0].priority, :>, parents[1].priority @@ -490,7 +490,7 @@ class ContainerRequestTest < ActiveSupport::TestCase # update the parent, it should get a higher priority but the children and # grandchildren should remain at 0 - toplevel_crs[1].update_attributes!(priority: 6) + toplevel_crs[1].update!(priority: 6) (parents + children + grandchildren + [shared_grandchild]).map(&:reload) assert_operator 0, :<, parents[1].priority assert_operator parents[0].priority, :<, parents[1].priority @@ -805,7 +805,7 @@ class ContainerRequestTest < ActiveSupport::TestCase # should be assigned. # * When use_existing is false, a different container should be assigned. # * When env1 and env2 are different, a different container should be assigned. - cr2.update_attributes!({state: ContainerRequest::Committed}) + cr2.update!({state: ContainerRequest::Committed}) assert_equal (cr2.use_existing == true and (env1 == env2)), (cr1.container_uuid == cr2.container_uuid) end @@ -826,8 +826,8 @@ class ContainerRequestTest < ActiveSupport::TestCase c = act_as_system_user do c = Container.find_by_uuid(cr.container_uuid) - c.update_attributes!(state: Container::Locked) - c.update_attributes!(state: Container::Running) + c.update!(state: Container::Locked) + c.update!(state: Container::Running) c end @@ -839,8 +839,8 @@ class ContainerRequestTest < ActiveSupport::TestCase prev_container_uuid = cr.container_uuid act_as_system_user do - c.update_attributes!(cost: 0.5, subrequests_cost: 1.25) - c.update_attributes!(state: Container::Cancelled) + c.update!(cost: 0.5, subrequests_cost: 1.25) + c.update!(state: Container::Cancelled) end cr.reload @@ -852,10 +852,10 @@ class ContainerRequestTest < ActiveSupport::TestCase c = act_as_system_user do c = Container.find_by_uuid(cr.container_uuid) - c.update_attributes!(state: Container::Locked) - c.update_attributes!(state: Container::Running) - c.update_attributes!(cost: 0.125) - c.update_attributes!(state: Container::Cancelled) + c.update!(state: Container::Locked) + c.update!(state: Container::Running) + c.update!(cost: 0.125) + c.update!(state: Container::Cancelled) c end @@ -878,8 +878,8 @@ class ContainerRequestTest < ActiveSupport::TestCase c = act_as_system_user do c = Container.find_by_uuid(cr.container_uuid) assert_equal spec.token, c.runtime_token - c.update_attributes!(state: Container::Locked) - c.update_attributes!(state: Container::Running) + c.update!(state: Container::Locked) + c.update!(state: Container::Running) c end @@ -889,7 +889,7 @@ class ContainerRequestTest < ActiveSupport::TestCase prev_container_uuid = cr.container_uuid act_as_system_user do - c.update_attributes!(state: Container::Cancelled) + c.update!(state: Container::Cancelled) end cr.reload @@ -900,7 +900,7 @@ class ContainerRequestTest < ActiveSupport::TestCase c = act_as_system_user do c = Container.find_by_uuid(cr.container_uuid) assert_equal spec.token, c.runtime_token - c.update_attributes!(state: Container::Cancelled) + c.update!(state: Container::Cancelled) c end @@ -916,8 +916,8 @@ class ContainerRequestTest < ActiveSupport::TestCase c = act_as_system_user do c = Container.find_by_uuid(cr.container_uuid) - c.update_attributes!(state: Container::Locked) - c.update_attributes!(state: Container::Running) + c.update!(state: Container::Locked) + c.update!(state: Container::Running) c end @@ -932,7 +932,7 @@ class ContainerRequestTest < ActiveSupport::TestCase logc = Collection.new(manifest_text: ". 37b51d194a7513e45b56f6524f2d51f2+3 0:3:bar\n") logc.save! c = Container.find_by_uuid(cr.container_uuid) - c.update_attributes!(state: Container::Cancelled, log: logc.portable_data_hash) + c.update!(state: Container::Cancelled, log: logc.portable_data_hash) c end end @@ -955,8 +955,8 @@ class ContainerRequestTest < ActiveSupport::TestCase cr1 = create_minimal_req!(priority: 1, state: "Committed", container_count_max: 2, command: ["echo", "foo1"]) c1 = Container.find_by_uuid(cr1.container_uuid) act_as_system_user do - c1.update_attributes!(state: Container::Locked) - c1.update_attributes!(state: Container::Running) + c1.update!(state: Container::Locked) + c1.update!(state: Container::Running) end cr2 = with_container_auth(c1) do @@ -964,8 +964,8 @@ class ContainerRequestTest < ActiveSupport::TestCase end c2 = Container.find_by_uuid(cr2.container_uuid) act_as_system_user do - c2.update_attributes!(state: Container::Locked) - c2.update_attributes!(state: Container::Running) + c2.update!(state: Container::Locked) + c2.update!(state: Container::Running) end cr3 = with_container_auth(c2) do @@ -974,8 +974,8 @@ class ContainerRequestTest < ActiveSupport::TestCase c3 = Container.find_by_uuid(cr3.container_uuid) act_as_system_user do - c3.update_attributes!(state: Container::Locked) - c3.update_attributes!(state: Container::Running) + c3.update!(state: Container::Locked) + c3.update!(state: Container::Running) end # All the containers are in running state @@ -1007,8 +1007,8 @@ class ContainerRequestTest < ActiveSupport::TestCase cr1 = create_minimal_req!(priority: 1, state: "Committed", container_count_max: 2, command: ["echo", "foo1"]) c1 = Container.find_by_uuid(cr1.container_uuid) act_as_system_user do - c1.update_attributes!(state: Container::Locked) - c1.update_attributes!(state: Container::Running) + c1.update!(state: Container::Locked) + c1.update!(state: Container::Running) end cr2 = with_container_auth(c1) do @@ -1016,8 +1016,8 @@ class ContainerRequestTest < ActiveSupport::TestCase end c2 = Container.find_by_uuid(cr2.container_uuid) act_as_system_user do - c2.update_attributes!(state: Container::Locked) - c2.update_attributes!(state: Container::Running) + c2.update!(state: Container::Locked) + c2.update!(state: Container::Running) end cr3 = with_container_auth(c2) do @@ -1026,8 +1026,8 @@ class ContainerRequestTest < ActiveSupport::TestCase c3 = Container.find_by_uuid(cr3.container_uuid) act_as_system_user do - c3.update_attributes!(state: Container::Locked) - c3.update_attributes!(state: Container::Running) + c3.update!(state: Container::Locked) + c3.update!(state: Container::Running) end # All the containers are in running state @@ -1066,8 +1066,8 @@ class ContainerRequestTest < ActiveSupport::TestCase cr1 = create_minimal_req!(priority: 1, state: "Committed", container_count_max: 2, command: ["echo", "foo1"]) c1 = Container.find_by_uuid(cr1.container_uuid) act_as_system_user do - c1.update_attributes!(state: Container::Locked) - c1.update_attributes!(state: Container::Running) + c1.update!(state: Container::Locked) + c1.update!(state: Container::Running) end cr2 = with_container_auth(c1) do @@ -1075,8 +1075,8 @@ class ContainerRequestTest < ActiveSupport::TestCase end c2 = Container.find_by_uuid(cr2.container_uuid) act_as_system_user do - c2.update_attributes!(state: Container::Locked) - c2.update_attributes!(state: Container::Running) + c2.update!(state: Container::Locked) + c2.update!(state: Container::Running) end cr3 = with_container_auth(c2) do @@ -1085,8 +1085,8 @@ class ContainerRequestTest < ActiveSupport::TestCase c3 = Container.find_by_uuid(cr3.container_uuid) act_as_system_user do - c3.update_attributes!(state: Container::Locked) - c3.update_attributes!(state: Container::Running) + c3.update!(state: Container::Locked) + c3.update!(state: Container::Running) end # All the containers are in running state @@ -1185,9 +1185,9 @@ class ContainerRequestTest < ActiveSupport::TestCase logc.save! c = Container.find_by_uuid(cr.container_uuid) - c.update_attributes!(state: Container::Locked) - c.update_attributes!(state: Container::Running) - c.update_attributes!(state: final_state, + c.update!(state: Container::Locked) + c.update!(state: Container::Running) + c.update!(state: final_state, exit_code: exit_code, output: '1f4b0bc7583c2a7f9102c395f4ffc5e3+45', log: logc.portable_data_hash) @@ -1211,7 +1211,7 @@ class ContainerRequestTest < ActiveSupport::TestCase cr3 = create_minimal_req!(priority: 1, state: ContainerRequest::Uncommitted) assert_equal ContainerRequest::Uncommitted, cr3.state - cr3.update_attributes!(state: ContainerRequest::Committed) + cr3.update!(state: ContainerRequest::Committed) assert_equal cr.container_uuid, cr3.container_uuid assert_equal ContainerRequest::Final, cr3.state end @@ -1307,7 +1307,7 @@ class ContainerRequestTest < ActiveSupport::TestCase # Even though preemptible is not allowed, we should be able to # commit a CR that was created earlier when preemptible was the # default. - commit_later.update_attributes!(priority: 1, state: "Committed") + commit_later.update!(priority: 1, state: "Committed") expect[false].push commit_later end @@ -1323,7 +1323,7 @@ class ContainerRequestTest < ActiveSupport::TestCase # Cancelling the parent used to fail while updating the child # containers' priority, because the child containers' unchanged # preemptible fields caused validation to fail. - parent.update_attributes!(state: 'Cancelled') + parent.update!(state: 'Cancelled') [false, true].each do |pflag| expect[pflag].each do |cr| @@ -1450,7 +1450,7 @@ class ContainerRequestTest < ActiveSupport::TestCase when 'Final' act_as_system_user do Container.find_by_uuid(cr.container_uuid). - update_attributes!(state: Container::Cancelled) + update!(state: Container::Cancelled) end cr.reload else @@ -1458,10 +1458,10 @@ class ContainerRequestTest < ActiveSupport::TestCase end assert_equal state, cr.state if permitted - assert cr.update_attributes!(updates) + assert cr.update!(updates) else assert_raises(ActiveRecord::RecordInvalid) do - cr.update_attributes!(updates) + cr.update!(updates) end end end @@ -1493,7 +1493,7 @@ class ContainerRequestTest < ActiveSupport::TestCase assert_equal 1, c.priority prj = Group.find_by_uuid cr.owner_uuid - prj.update_attributes!(trash_at: db_current_time) + prj.update!(trash_at: db_current_time) # the cr's container now has priority of 0 c.reload @@ -1506,7 +1506,7 @@ class ContainerRequestTest < ActiveSupport::TestCase # container request to go to final state and run the finalize # function act_as_system_user do - c.update_attributes!(state: 'Cancelled', log: 'fa7aeb5140e2848d39b416daeef4ffc5+45') + c.update!(state: 'Cancelled', log: 'fa7aeb5140e2848d39b416daeef4ffc5+45') end c.reload cr.reload @@ -1615,7 +1615,7 @@ class ContainerRequestTest < ActiveSupport::TestCase sm = {'/secret/foo' => {'kind' => 'text', 'content' => secret_string}} set_user_from_auth :active cr = create_minimal_req! - assert_equal false, cr.update_attributes(state: "Committed", + assert_equal false, cr.update(state: "Committed", priority: 1, mounts: cr.mounts.merge(sm), secret_mounts: sm) @@ -1635,7 +1635,7 @@ class ContainerRequestTest < ActiveSupport::TestCase assert_not_nil ApiClientAuthorization.find_by_uuid(spec.uuid) act_as_system_user do - c.update_attributes!(state: Container::Complete, + c.update!(state: Container::Complete, exit_code: 0, output: '1f4b0bc7583c2a7f9102c395f4ffc5e3+45', log: 'fa7aeb5140e2848d39b416daeef4ffc5+45') @@ -1714,7 +1714,7 @@ class ContainerRequestTest < ActiveSupport::TestCase assert_nil cr2.container_uuid # Update cr2 to commited state, check for reuse, then run it - cr2.update_attributes!({state: ContainerRequest::Committed}) + cr2.update!({state: ContainerRequest::Committed}) assert_equal cr1.container_uuid, cr2.container_uuid cr2.reload @@ -1748,12 +1748,12 @@ class ContainerRequestTest < ActiveSupport::TestCase logc.save! c = Container.find_by_uuid(cr.container_uuid) - c.update_attributes!(state: Container::Locked) - c.update_attributes!(state: Container::Running) + c.update!(state: Container::Locked) + c.update!(state: Container::Running) - c.update_attributes!(output_properties: container_prop) + c.update!(output_properties: container_prop) - c.update_attributes!(state: Container::Complete, + c.update!(state: Container::Complete, exit_code: 0, output: '1f4b0bc7583c2a7f9102c395f4ffc5e3+45', log: logc.portable_data_hash) @@ -1773,9 +1773,9 @@ class ContainerRequestTest < ActiveSupport::TestCase cr = create_minimal_req!(priority: 5, state: "Committed", container_count_max: 3) c = Container.find_by_uuid cr.container_uuid act_as_system_user do - c.update_attributes!(state: Container::Locked) - c.update_attributes!(state: Container::Running) - c.update_attributes!(state: Container::Cancelled, cost: 3) + c.update!(state: Container::Locked) + c.update!(state: Container::Running) + c.update!(state: Container::Cancelled, cost: 3) end cr.reload assert_equal 3, cr.cumulative_cost @@ -1792,12 +1792,12 @@ class ContainerRequestTest < ActiveSupport::TestCase assert_equal c.uuid, cr2.requesting_container_uuid c2 = Container.find_by_uuid cr2.container_uuid act_as_system_user do - c2.update_attributes!(state: Container::Locked) - c2.update_attributes!(state: Container::Running) + c2.update!(state: Container::Locked) + c2.update!(state: Container::Running) logc = Collection.new(owner_uuid: system_user_uuid, manifest_text: ". ef772b2f28e2c8ca84de45466ed19ee9+7815 0:0:arv-mount.txt\n") logc.save! - c2.update_attributes!(state: Container::Complete, + c2.update!(state: Container::Complete, exit_code: 0, output: '1f4b0bc7583c2a7f9102c395f4ffc5e3+45', log: logc.portable_data_hash, @@ -1818,7 +1818,7 @@ class ContainerRequestTest < ActiveSupport::TestCase assert_equal 7, c.subrequests_cost act_as_system_user do - c.update_attributes!(state: Container::Complete, exit_code: 0, cost: 9) + c.update!(state: Container::Complete, exit_code: 0, cost: 9) end c.reload diff --git a/services/api/test/unit/container_test.rb b/services/api/test/unit/container_test.rb index cb9cd7503b..09b885b391 100644 --- a/services/api/test/unit/container_test.rb +++ b/services/api/test/unit/container_test.rb @@ -76,7 +76,7 @@ class ContainerTest < ActiveSupport::TestCase def check_illegal_updates c, bad_updates bad_updates.each do |u| - refute c.update_attributes(u), u.inspect + refute c.update(u), u.inspect refute c.valid?, u.inspect c.reload end @@ -173,15 +173,15 @@ class ContainerTest < ActiveSupport::TestCase assert_equal Container::Queued, c.state set_user_from_auth :dispatch1 - c.update_attributes! state: Container::Locked - c.update_attributes! state: Container::Running + c.update! state: Container::Locked + c.update! state: Container::Running [ 'error', 'errorDetail', 'warning', 'warningDetail', 'activity' ].each do |k| # String type is allowed string_val = 'A string is accepted' - c.update_attributes! runtime_status: {k => string_val} + c.update! runtime_status: {k => string_val} assert_equal string_val, c.runtime_status[k] # Other types aren't allowed @@ -189,7 +189,7 @@ class ContainerTest < ActiveSupport::TestCase 42, false, [], {}, nil ].each do |unallowed_val| assert_raises ActiveRecord::RecordInvalid do - c.update_attributes! runtime_status: {k => unallowed_val} + c.update! runtime_status: {k => unallowed_val} end end end @@ -209,41 +209,41 @@ class ContainerTest < ActiveSupport::TestCase assert_equal Container::Queued, c1.state assert_raises ArvadosModel::PermissionDeniedError do - c1.update_attributes! runtime_status: {'error' => 'Oops!'} + c1.update! runtime_status: {'error' => 'Oops!'} end set_user_from_auth :dispatch1 # Allow updates when state = Locked - c1.update_attributes! state: Container::Locked - c1.update_attributes! runtime_status: {'error' => 'Oops!'} + c1.update! state: Container::Locked + c1.update! runtime_status: {'error' => 'Oops!'} assert c1.runtime_status.key? 'error' # Reset when transitioning from Locked to Queued - c1.update_attributes! state: Container::Queued + c1.update! state: Container::Queued assert_equal c1.runtime_status, {} # Allow updates when state = Running - c1.update_attributes! state: Container::Locked - c1.update_attributes! state: Container::Running - c1.update_attributes! runtime_status: {'error' => 'Oops!'} + c1.update! state: Container::Locked + c1.update! state: Container::Running + c1.update! runtime_status: {'error' => 'Oops!'} assert c1.runtime_status.key? 'error' # Don't allow updates on other states - c1.update_attributes! state: Container::Complete + c1.update! state: Container::Complete assert_raises ActiveRecord::RecordInvalid do - c1.update_attributes! runtime_status: {'error' => 'Some other error'} + c1.update! runtime_status: {'error' => 'Some other error'} end set_user_from_auth :active c2, _ = minimal_new(attrs) assert_equal c2.runtime_status, {} set_user_from_auth :dispatch1 - c2.update_attributes! state: Container::Locked - c2.update_attributes! state: Container::Running - c2.update_attributes! state: Container::Cancelled + c2.update! state: Container::Locked + c2.update! state: Container::Running + c2.update! state: Container::Cancelled assert_raises ActiveRecord::RecordInvalid do - c2.update_attributes! runtime_status: {'error' => 'Oops!'} + c2.update! runtime_status: {'error' => 'Oops!'} end end @@ -294,13 +294,13 @@ class ContainerTest < ActiveSupport::TestCase assert_not_equal c_older.uuid, c_recent.uuid set_user_from_auth :dispatch1 - c_older.update_attributes!({state: Container::Locked}) - c_older.update_attributes!({state: Container::Running}) - c_older.update_attributes!(completed_attrs) + c_older.update!({state: Container::Locked}) + c_older.update!({state: Container::Running}) + c_older.update!(completed_attrs) - c_recent.update_attributes!({state: Container::Locked}) - c_recent.update_attributes!({state: Container::Running}) - c_recent.update_attributes!(completed_attrs) + c_recent.update!({state: Container::Locked}) + c_recent.update!({state: Container::Running}) + c_recent.update!(completed_attrs) reused = Container.find_reusable(common_attrs) assert_not_nil reused @@ -334,14 +334,14 @@ class ContainerTest < ActiveSupport::TestCase out1 = '1f4b0bc7583c2a7f9102c395f4ffc5e3+45' log1 = collections(:real_log_collection).portable_data_hash - c_output1.update_attributes!({state: Container::Locked}) - c_output1.update_attributes!({state: Container::Running}) - c_output1.update_attributes!(completed_attrs.merge({log: log1, output: out1})) + c_output1.update!({state: Container::Locked}) + c_output1.update!({state: Container::Running}) + c_output1.update!(completed_attrs.merge({log: log1, output: out1})) out2 = 'fa7aeb5140e2848d39b416daeef4ffc5+45' - c_output2.update_attributes!({state: Container::Locked}) - c_output2.update_attributes!({state: Container::Running}) - c_output2.update_attributes!(completed_attrs.merge({log: log1, output: out2})) + c_output2.update!({state: Container::Locked}) + c_output2.update!({state: Container::Running}) + c_output2.update!(completed_attrs.merge({log: log1, output: out2})) set_user_from_auth :active reused = Container.resolve(ContainerRequest.new(request_only(common_attrs))) @@ -357,14 +357,14 @@ class ContainerTest < ActiveSupport::TestCase # Confirm the 3 container UUIDs are different. assert_equal 3, [c_slower.uuid, c_faster_started_first.uuid, c_faster_started_second.uuid].uniq.length set_user_from_auth :dispatch1 - c_slower.update_attributes!({state: Container::Locked}) - c_slower.update_attributes!({state: Container::Running, + c_slower.update!({state: Container::Locked}) + c_slower.update!({state: Container::Running, progress: 0.1}) - c_faster_started_first.update_attributes!({state: Container::Locked}) - c_faster_started_first.update_attributes!({state: Container::Running, + c_faster_started_first.update!({state: Container::Locked}) + c_faster_started_first.update!({state: Container::Running, progress: 0.15}) - c_faster_started_second.update_attributes!({state: Container::Locked}) - c_faster_started_second.update_attributes!({state: Container::Running, + c_faster_started_second.update!({state: Container::Locked}) + c_faster_started_second.update!({state: Container::Running, progress: 0.15}) reused = Container.find_reusable(common_attrs) assert_not_nil reused @@ -381,14 +381,14 @@ class ContainerTest < ActiveSupport::TestCase # Confirm the 3 container UUIDs are different. assert_equal 3, [c_slower.uuid, c_faster_started_first.uuid, c_faster_started_second.uuid].uniq.length set_user_from_auth :dispatch1 - c_slower.update_attributes!({state: Container::Locked}) - c_slower.update_attributes!({state: Container::Running, + c_slower.update!({state: Container::Locked}) + c_slower.update!({state: Container::Running, progress: 0.1}) - c_faster_started_first.update_attributes!({state: Container::Locked}) - c_faster_started_first.update_attributes!({state: Container::Running, + c_faster_started_first.update!({state: Container::Locked}) + c_faster_started_first.update!({state: Container::Running, progress: 0.15}) - c_faster_started_second.update_attributes!({state: Container::Locked}) - c_faster_started_second.update_attributes!({state: Container::Running, + c_faster_started_second.update!({state: Container::Locked}) + c_faster_started_second.update!({state: Container::Running, progress: 0.2}) reused = Container.find_reusable(common_attrs) assert_not_nil reused @@ -405,16 +405,16 @@ class ContainerTest < ActiveSupport::TestCase # Confirm the 3 container UUIDs are different. assert_equal 3, [c_slower.uuid, c_faster_started_first.uuid, c_faster_started_second.uuid].uniq.length set_user_from_auth :dispatch1 - c_slower.update_attributes!({state: Container::Locked}) - c_slower.update_attributes!({state: Container::Running, + c_slower.update!({state: Container::Locked}) + c_slower.update!({state: Container::Running, progress: 0.1}) - c_faster_started_first.update_attributes!({state: Container::Locked}) - c_faster_started_first.update_attributes!({state: Container::Running, + c_faster_started_first.update!({state: Container::Locked}) + c_faster_started_first.update!({state: Container::Running, runtime_status: {'warning' => 'This is not an error'}, progress: 0.15}) - c_faster_started_second.update_attributes!({state: Container::Locked}) + c_faster_started_second.update!({state: Container::Locked}) assert_equal 0, Container.where("runtime_status->'error' is not null").count - c_faster_started_second.update_attributes!({state: Container::Running, + c_faster_started_second.update!({state: Container::Running, runtime_status: {'error' => 'Something bad happened'}, progress: 0.2}) assert_equal 1, Container.where("runtime_status->'error' is not null").count @@ -433,11 +433,11 @@ class ContainerTest < ActiveSupport::TestCase # Confirm the 3 container UUIDs are different. assert_equal 3, [c_low_priority.uuid, c_high_priority_older.uuid, c_high_priority_newer.uuid].uniq.length set_user_from_auth :dispatch1 - c_low_priority.update_attributes!({state: Container::Locked, + c_low_priority.update!({state: Container::Locked, priority: 1}) - c_high_priority_older.update_attributes!({state: Container::Locked, + c_high_priority_older.update!({state: Container::Locked, priority: 2}) - c_high_priority_newer.update_attributes!({state: Container::Locked, + c_high_priority_newer.update!({state: Container::Locked, priority: 2}) reused = Container.find_reusable(common_attrs) assert_not_nil reused @@ -451,14 +451,14 @@ class ContainerTest < ActiveSupport::TestCase c_running, _ = minimal_new(common_attrs.merge({use_existing: false})) assert_not_equal c_failed.uuid, c_running.uuid set_user_from_auth :dispatch1 - c_failed.update_attributes!({state: Container::Locked}) - c_failed.update_attributes!({state: Container::Running}) - c_failed.update_attributes!({state: Container::Complete, + c_failed.update!({state: Container::Locked}) + c_failed.update!({state: Container::Running}) + c_failed.update!({state: Container::Complete, exit_code: 42, log: 'ea10d51bcf88862dbcc36eb292017dfd+45', output: 'ea10d51bcf88862dbcc36eb292017dfd+45'}) - c_running.update_attributes!({state: Container::Locked}) - c_running.update_attributes!({state: Container::Running, + c_running.update!({state: Container::Locked}) + c_running.update!({state: Container::Running, progress: 0.15}) reused = Container.find_reusable(common_attrs) assert_not_nil reused @@ -472,14 +472,14 @@ class ContainerTest < ActiveSupport::TestCase c_running, _ = minimal_new(common_attrs.merge({use_existing: false})) assert_not_equal c_completed.uuid, c_running.uuid set_user_from_auth :dispatch1 - c_completed.update_attributes!({state: Container::Locked}) - c_completed.update_attributes!({state: Container::Running}) - c_completed.update_attributes!({state: Container::Complete, + c_completed.update!({state: Container::Locked}) + c_completed.update!({state: Container::Running}) + c_completed.update!({state: Container::Complete, exit_code: 0, log: 'ea10d51bcf88862dbcc36eb292017dfd+45', output: '1f4b0bc7583c2a7f9102c395f4ffc5e3+45'}) - c_running.update_attributes!({state: Container::Locked}) - c_running.update_attributes!({state: Container::Running, + c_running.update!({state: Container::Locked}) + c_running.update!({state: Container::Running, progress: 0.15}) reused = Container.find_reusable(common_attrs) assert_not_nil reused @@ -493,9 +493,9 @@ class ContainerTest < ActiveSupport::TestCase c_running, _ = minimal_new(common_attrs.merge({use_existing: false})) assert_not_equal c_running.uuid, c_locked.uuid set_user_from_auth :dispatch1 - c_locked.update_attributes!({state: Container::Locked}) - c_running.update_attributes!({state: Container::Locked}) - c_running.update_attributes!({state: Container::Running, + c_locked.update!({state: Container::Locked}) + c_running.update!({state: Container::Locked}) + c_running.update!({state: Container::Running, progress: 0.15}) reused = Container.find_reusable(common_attrs) assert_not_nil reused @@ -509,7 +509,7 @@ class ContainerTest < ActiveSupport::TestCase c_queued, _ = minimal_new(common_attrs.merge({use_existing: false})) assert_not_equal c_queued.uuid, c_locked.uuid set_user_from_auth :dispatch1 - c_locked.update_attributes!({state: Container::Locked}) + c_locked.update!({state: Container::Locked}) reused = Container.find_reusable(common_attrs) assert_not_nil reused assert_equal reused.uuid, c_locked.uuid @@ -520,9 +520,9 @@ class ContainerTest < ActiveSupport::TestCase attrs = REUSABLE_COMMON_ATTRS.merge({environment: {"var" => "failed"}}) c, _ = minimal_new(attrs) set_user_from_auth :dispatch1 - c.update_attributes!({state: Container::Locked}) - c.update_attributes!({state: Container::Running}) - c.update_attributes!({state: Container::Complete, + c.update!({state: Container::Locked}) + c.update!({state: Container::Running}) + c.update!({state: Container::Complete, exit_code: 33}) reused = Container.find_reusable(attrs) assert_nil reused @@ -543,8 +543,8 @@ class ContainerTest < ActiveSupport::TestCase c1_attrs = REUSABLE_COMMON_ATTRS.merge({environment: {"test" => name, "state" => c1_state}, scheduling_parameters: {"preemptible" => c1_preemptible}}) c1, _ = minimal_new(c1_attrs) set_user_from_auth :dispatch1 - c1.update_attributes!({state: Container::Locked}) if c1_state != Container::Queued - c1.update_attributes!({state: Container::Running, priority: c1_priority}) if c1_state == Container::Running + c1.update!({state: Container::Locked}) if c1_state != Container::Queued + c1.update!({state: Container::Running, priority: c1_priority}) if c1_state == Container::Running c2_attrs = c1_attrs.merge({scheduling_parameters: {"preemptible" => c2_preemptible}}) reused = Container.find_reusable(c2_attrs) if should_reuse && c1_priority > 0 @@ -676,7 +676,7 @@ class ContainerTest < ActiveSupport::TestCase {state: Container::Complete}] c.lock - c.update_attributes! state: Container::Running + c.update! state: Container::Running check_illegal_modify c check_bogus_states c @@ -684,7 +684,7 @@ class ContainerTest < ActiveSupport::TestCase check_illegal_updates c, [{state: Container::Queued}] c.reload - c.update_attributes! priority: 3 + c.update! priority: 3 end test "Lock and unlock" do @@ -699,11 +699,11 @@ class ContainerTest < ActiveSupport::TestCase c.lock end c.reload - assert cr.update_attributes priority: 1 + assert cr.update priority: 1 - refute c.update_attributes(state: Container::Running), "not locked" + refute c.update(state: Container::Running), "not locked" c.reload - refute c.update_attributes(state: Container::Complete), "not locked" + refute c.update(state: Container::Complete), "not locked" c.reload assert c.lock, show_errors(c) @@ -717,13 +717,13 @@ class ContainerTest < ActiveSupport::TestCase refute c.locked_by_uuid refute c.auth_uuid - refute c.update_attributes(state: Container::Running), "not locked" + refute c.update(state: Container::Running), "not locked" c.reload refute c.locked_by_uuid refute c.auth_uuid assert c.lock, show_errors(c) - assert c.update_attributes(state: Container::Running), show_errors(c) + assert c.update(state: Container::Running), show_errors(c) assert c.locked_by_uuid assert c.auth_uuid @@ -740,7 +740,7 @@ class ContainerTest < ActiveSupport::TestCase end c.reload - assert c.update_attributes(state: Container::Complete), show_errors(c) + assert c.update(state: Container::Complete), show_errors(c) refute c.locked_by_uuid refute c.auth_uuid @@ -800,7 +800,7 @@ class ContainerTest < ActiveSupport::TestCase set_user_from_auth :active c, cr = minimal_new({container_count_max: 1}) set_user_from_auth :dispatch1 - assert c.update_attributes(state: Container::Cancelled), show_errors(c) + assert c.update(state: Container::Cancelled), show_errors(c) check_no_change_from_cancelled c cr.reload assert_equal ContainerRequest::Final, cr.state @@ -823,7 +823,7 @@ class ContainerTest < ActiveSupport::TestCase c, _ = minimal_new set_user_from_auth :dispatch1 assert c.lock, show_errors(c) - assert c.update_attributes(state: Container::Cancelled), show_errors(c) + assert c.update(state: Container::Cancelled), show_errors(c) check_no_change_from_cancelled c end @@ -843,7 +843,7 @@ class ContainerTest < ActiveSupport::TestCase c, _ = minimal_new set_user_from_auth :dispatch1 assert c.lock, show_errors(c) - assert c.update_attributes( + assert c.update( state: Container::Cancelled, log: collections(:real_log_collection).portable_data_hash, ), show_errors(c) @@ -855,8 +855,8 @@ class ContainerTest < ActiveSupport::TestCase c, _ = minimal_new set_user_from_auth :dispatch1 c.lock - c.update_attributes! state: Container::Running - c.update_attributes! state: Container::Cancelled + c.update! state: Container::Running + c.update! state: Container::Cancelled check_no_change_from_cancelled c end @@ -906,16 +906,16 @@ class ContainerTest < ActiveSupport::TestCase set_user_from_auth :dispatch1 c.lock if start_state != Container::Locked - c.update_attributes! state: Container::Running + c.update! state: Container::Running if start_state != Container::Running - c.update_attributes! state: start_state + c.update! state: start_state end end end assert_equal c.state, start_state set_user_from_auth :active assert_raises(ArvadosModel::PermissionDeniedError) do - c.update_attributes! updates + c.update! updates end end end @@ -926,9 +926,9 @@ class ContainerTest < ActiveSupport::TestCase set_user_from_auth :dispatch1 c.lock check_illegal_updates c, [{exit_code: 1}] - c.update_attributes! state: Container::Running - assert c.update_attributes(exit_code: 1) - assert c.update_attributes(exit_code: 1, state: Container::Complete) + c.update! state: Container::Running + assert c.update(exit_code: 1) + assert c.update(exit_code: 1, state: Container::Complete) end test "locked_by_uuid can update log when locked/running, and output when running" do @@ -947,8 +947,8 @@ class ContainerTest < ActiveSupport::TestCase set_user_from_auth :dispatch1 c.lock assert_equal c.locked_by_uuid, Thread.current[:api_client_authorization].uuid - c.update_attributes!(log: logpdh_time1) - c.update_attributes!(state: Container::Running) + c.update!(log: logpdh_time1) + c.update!(state: Container::Running) cr1.reload cr2.reload cr1log_uuid = cr1.log_uuid @@ -959,17 +959,17 @@ class ContainerTest < ActiveSupport::TestCase assert_not_equal logcoll.uuid, cr2log_uuid assert_not_equal cr1log_uuid, cr2log_uuid - logcoll.update_attributes!(manifest_text: logcoll.manifest_text + ". acbd18db4cc2f85cedef654fccc4a4d8+3 0:3:foo.txt\n") + logcoll.update!(manifest_text: logcoll.manifest_text + ". acbd18db4cc2f85cedef654fccc4a4d8+3 0:3:foo.txt\n") logpdh_time2 = logcoll.portable_data_hash - assert c.update_attributes(output: collections(:collection_owned_by_active).portable_data_hash) - assert c.update_attributes(log: logpdh_time2) - assert c.update_attributes(state: Container::Complete, log: logcoll.portable_data_hash) + assert c.update(output: collections(:collection_owned_by_active).portable_data_hash) + assert c.update(log: logpdh_time2) + assert c.update(state: Container::Complete, log: logcoll.portable_data_hash) c.reload assert_equal collections(:collection_owned_by_active).portable_data_hash, c.output assert_equal logpdh_time2, c.log - refute c.update_attributes(output: nil) - refute c.update_attributes(log: nil) + refute c.update(output: nil) + refute c.update(log: nil) cr1.reload cr2.reload assert_equal cr1log_uuid, cr1.log_uuid @@ -992,7 +992,7 @@ class ContainerTest < ActiveSupport::TestCase end set_user_from_auth :dispatch1 c.lock - c.update_attributes! state: Container::Running + c.update! state: Container::Running if tok == "runtime_token" auth = ApiClientAuthorization.validate(token: c.runtime_token) @@ -1008,14 +1008,14 @@ class ContainerTest < ActiveSupport::TestCase Thread.current[:user] = auth.user end - assert c.update_attributes(gateway_address: "127.0.0.1:9") - assert c.update_attributes(output: collections(:collection_owned_by_active).portable_data_hash) - assert c.update_attributes(runtime_status: {'warning' => 'something happened'}) - assert c.update_attributes(progress: 0.5) - assert c.update_attributes(exit_code: 0) - refute c.update_attributes(log: collections(:real_log_collection).portable_data_hash) + assert c.update(gateway_address: "127.0.0.1:9") + assert c.update(output: collections(:collection_owned_by_active).portable_data_hash) + assert c.update(runtime_status: {'warning' => 'something happened'}) + assert c.update(progress: 0.5) + assert c.update(exit_code: 0) + refute c.update(log: collections(:real_log_collection).portable_data_hash) c.reload - assert c.update_attributes(state: Container::Complete, exit_code: 0) + assert c.update(state: Container::Complete, exit_code: 0) end end @@ -1024,13 +1024,13 @@ class ContainerTest < ActiveSupport::TestCase c, _ = minimal_new set_user_from_auth :dispatch1 c.lock - c.update_attributes! state: Container::Running + c.update! state: Container::Running Thread.current[:api_client_authorization] = ApiClientAuthorization.find_by_uuid(c.auth_uuid) Thread.current[:user] = User.find_by_id(Thread.current[:api_client_authorization].user_id) assert_raises ActiveRecord::RecordInvalid do - c.update_attributes! output: collections(:collection_not_readable_by_active).portable_data_hash + c.update! output: collections(:collection_not_readable_by_active).portable_data_hash end end @@ -1039,11 +1039,11 @@ class ContainerTest < ActiveSupport::TestCase c, _ = minimal_new set_user_from_auth :dispatch1 c.lock - c.update_attributes! state: Container::Running + c.update! state: Container::Running set_user_from_auth :running_to_be_deleted_container_auth assert_raises(ArvadosModel::PermissionDeniedError) do - c.update_attributes(output: collections(:foo_file).portable_data_hash) + c.update(output: collections(:foo_file).portable_data_hash) end end @@ -1052,13 +1052,13 @@ class ContainerTest < ActiveSupport::TestCase c, _ = minimal_new set_user_from_auth :dispatch1 c.lock - c.update_attributes! state: Container::Running + c.update! state: Container::Running output = Collection.find_by_uuid('zzzzz-4zz18-mto52zx1s7sn3jk') assert output.is_trashed - assert c.update_attributes output: output.portable_data_hash - assert c.update_attributes! state: Container::Complete + assert c.update output: output.portable_data_hash + assert c.update! state: Container::Complete end test "not allowed to set trashed output that is not readable by current user" do @@ -1066,7 +1066,7 @@ class ContainerTest < ActiveSupport::TestCase c, _ = minimal_new set_user_from_auth :dispatch1 c.lock - c.update_attributes! state: Container::Running + c.update! state: Container::Running output = Collection.find_by_uuid('zzzzz-4zz18-mto52zx1s7sn3jr') @@ -1074,7 +1074,7 @@ class ContainerTest < ActiveSupport::TestCase Thread.current[:user] = User.find_by_id(Thread.current[:api_client_authorization].user_id) assert_raises ActiveRecord::RecordInvalid do - c.update_attributes! output: output.portable_data_hash + c.update! output: output.portable_data_hash end end @@ -1097,12 +1097,12 @@ class ContainerTest < ActiveSupport::TestCase container_count_max: 1, runtime_token: api_client_authorizations(:active).token) set_user_from_auth :dispatch1 c.lock - c.update_attributes!(state: Container::Running) + c.update!(state: Container::Running) c.reload assert c.secret_mounts.has_key?('/secret') assert_equal api_client_authorizations(:active).token, c.runtime_token - c.update_attributes!(final_attrs) + c.update!(final_attrs) c.reload assert_equal({}, c.secret_mounts) assert_nil c.runtime_token @@ -1150,7 +1150,7 @@ class ContainerTest < ActiveSupport::TestCase assert_equal(1, containers.length) _, container1 = containers.shift container1.lock - container1.update_attributes!(state: Container::Cancelled) + container1.update!(state: Container::Cancelled) container1.reload request1 = requests.shift request1.reload @@ -1277,8 +1277,8 @@ class ContainerTest < ActiveSupport::TestCase end container, request = minimal_new(request_params) container.lock - container.update_attributes!(state: Container::Running) - container.update_attributes!(final_attrs) + container.update!(state: Container::Running) + container.update!(final_attrs) return container, request end diff --git a/services/api/test/unit/create_superuser_token_test.rb b/services/api/test/unit/create_superuser_token_test.rb index 3c6dcbdbbc..86ba78cb99 100644 --- a/services/api/test/unit/create_superuser_token_test.rb +++ b/services/api/test/unit/create_superuser_token_test.rb @@ -54,7 +54,7 @@ class CreateSuperUserTokenTest < ActiveSupport::TestCase apiClientAuth = ApiClientAuthorization.where(api_token: 'atesttoken').first refute_nil apiClientAuth Thread.current[:user] = users(:admin) - apiClientAuth.update_attributes expires_at: '2000-10-10' + apiClientAuth.update expires_at: '2000-10-10' token2 = create_superuser_token assert_not_nil token2 diff --git a/services/api/test/unit/group_test.rb b/services/api/test/unit/group_test.rb index a0c375a6f9..36f42006ff 100644 --- a/services/api/test/unit/group_test.rb +++ b/services/api/test/unit/group_test.rb @@ -82,7 +82,7 @@ class GroupTest < ActiveSupport::TestCase set_user_from_auth :active_trustedclient g = Group.create!(name: "foo", group_class: "role") assert_raises(ActiveRecord::RecordInvalid) do - g.update_attributes!(group_class: "project") + g.update!(group_class: "project") end end @@ -95,7 +95,7 @@ class GroupTest < ActiveSupport::TestCase c = Collection.create!(name: "bzzz124") assert_raises(ArvadosModel::PermissionDeniedError) do - c.update_attributes!(owner_uuid: role.uuid) + c.update!(owner_uuid: role.uuid) end end @@ -336,7 +336,7 @@ update links set tail_uuid='#{g5}' where uuid='#{l1.uuid}' # Cannot set frozen_by_uuid to a different user assert_raises do - proj.update_attributes!(frozen_by_uuid: users(:spectator).uuid) + proj.update!(frozen_by_uuid: users(:spectator).uuid) end proj.reload @@ -348,7 +348,7 @@ update links set tail_uuid='#{g5}' where uuid='#{l1.uuid}' # First confirm we have write permission assert Collection.create(name: 'bar', owner_uuid: proj.uuid) assert_raises(ArvadosModel::PermissionDeniedError) do - proj.update_attributes!(frozen_by_uuid: users(:spectator).uuid) + proj.update!(frozen_by_uuid: users(:spectator).uuid) end end proj.reload @@ -356,12 +356,12 @@ update links set tail_uuid='#{g5}' where uuid='#{l1.uuid}' # Cannot set frozen_by_uuid without description (if so configured) Rails.configuration.API.FreezeProjectRequiresDescription = true err = assert_raises do - proj.update_attributes!(frozen_by_uuid: users(:active).uuid) + proj.update!(frozen_by_uuid: users(:active).uuid) end assert_match /can only be set if description is non-empty/, err.inspect proj.reload err = assert_raises do - proj.update_attributes!(frozen_by_uuid: users(:active).uuid, description: '') + proj.update!(frozen_by_uuid: users(:active).uuid, description: '') end assert_match /can only be set if description is non-empty/, err.inspect proj.reload @@ -369,7 +369,7 @@ update links set tail_uuid='#{g5}' where uuid='#{l1.uuid}' # Cannot set frozen_by_uuid without properties (if so configured) Rails.configuration.API.FreezeProjectRequiresProperties['frobity'] = true err = assert_raises do - proj.update_attributes!( + proj.update!( frozen_by_uuid: users(:active).uuid, description: 'ready to freeze') end @@ -379,20 +379,20 @@ update links set tail_uuid='#{g5}' where uuid='#{l1.uuid}' # Cannot set frozen_by_uuid while project or its parent is # trashed [parent, proj].each do |trashed| - trashed.update_attributes!(trash_at: db_current_time) + trashed.update!(trash_at: db_current_time) err = assert_raises do - proj.update_attributes!( + proj.update!( frozen_by_uuid: users(:active).uuid, description: 'ready to freeze', properties: {'frobity' => 'bar baz'}) end assert_match /cannot be set on a trashed project/, err.inspect proj.reload - trashed.update_attributes!(trash_at: nil) + trashed.update!(trash_at: nil) end # Can set frozen_by_uuid if all conditions are met - ok = proj.update_attributes( + ok = proj.update( frozen_by_uuid: users(:active).uuid, description: 'ready to freeze', properties: {'frobity' => 'bar baz'}) @@ -404,7 +404,7 @@ update links set tail_uuid='#{g5}' where uuid='#{l1.uuid}' # its descendants [proj, proj_inner].each do |frozen| assert_raises do - collections(:collection_owned_by_active).update_attributes!(owner_uuid: frozen.uuid) + collections(:collection_owned_by_active).update!(owner_uuid: frozen.uuid) end assert_raises do Collection.create!(owner_uuid: frozen.uuid, name: 'inside-frozen-project') @@ -427,31 +427,31 @@ update links set tail_uuid='#{g5}' where uuid='#{l1.uuid}' # trash, or delete the project or anything beneath it [proj, proj_inner, coll].each do |frozen| assert_raises(StandardError, "should reject rename of #{frozen.uuid} (#{frozen.name}) with parent #{frozen.owner_uuid}") do - frozen.update_attributes!(name: 'foo2') + frozen.update!(name: 'foo2') end frozen.reload if frozen.is_a?(Collection) assert_raises(StandardError, "should reject manifest change of #{frozen.uuid}") do - frozen.update_attributes!(manifest_text: ". d41d8cd98f00b204e9800998ecf8427e+0 0:0:foo\n") + frozen.update!(manifest_text: ". d41d8cd98f00b204e9800998ecf8427e+0 0:0:foo\n") end else assert_raises(StandardError, "should reject moving a project into #{frozen.uuid}") do - groups(:private).update_attributes!(owner_uuid: frozen.uuid) + groups(:private).update!(owner_uuid: frozen.uuid) end end frozen.reload assert_raises(StandardError, "should reject moving #{frozen.uuid} to a different parent project") do - frozen.update_attributes!(owner_uuid: groups(:private).uuid) + frozen.update!(owner_uuid: groups(:private).uuid) end frozen.reload assert_raises(StandardError, "should reject setting trash_at of #{frozen.uuid}") do - frozen.update_attributes!(trash_at: db_current_time) + frozen.update!(trash_at: db_current_time) end frozen.reload assert_raises(StandardError, "should reject setting delete_at of #{frozen.uuid}") do - frozen.update_attributes!(delete_at: db_current_time) + frozen.update!(delete_at: db_current_time) end frozen.reload assert_raises(StandardError, "should reject delete of #{frozen.uuid}") do @@ -470,35 +470,35 @@ update links set tail_uuid='#{g5}' where uuid='#{l1.uuid}' # First confirm we have write permission on the parent project assert Collection.create(name: 'bar', owner_uuid: parent.uuid) assert_raises(ArvadosModel::PermissionDeniedError) do - proj.update_attributes!(frozen_by_uuid: nil) + proj.update!(frozen_by_uuid: nil) end end proj.reload # User with manage permission can unfreeze, then create items # inside it and its children - assert proj.update_attributes(frozen_by_uuid: nil) + assert proj.update(frozen_by_uuid: nil) assert Collection.create!(owner_uuid: proj.uuid, name: 'inside-unfrozen-project') assert Collection.create!(owner_uuid: proj_inner.uuid, name: 'inside-inner-unfrozen-project') # Re-freeze, and reconfigure so only admins can unfreeze. - assert proj.update_attributes(frozen_by_uuid: users(:active).uuid) + assert proj.update(frozen_by_uuid: users(:active).uuid) Rails.configuration.API.UnfreezeProjectRequiresAdmin = true # Owner cannot unfreeze, because not admin. err = assert_raises do - proj.update_attributes!(frozen_by_uuid: nil) + proj.update!(frozen_by_uuid: nil) end assert_match /can only be changed by an admin user, once set/, err.inspect proj.reload # Cannot trash or delete a frozen project's ancestor assert_raises(StandardError, "should not be able to set trash_at on parent of frozen project") do - parent.update_attributes!(trash_at: db_current_time) + parent.update!(trash_at: db_current_time) end parent.reload assert_raises(StandardError, "should not be able to set delete_at on parent of frozen project") do - parent.update_attributes!(delete_at: db_current_time) + parent.update!(delete_at: db_current_time) end parent.reload assert_nil parent.frozen_by_uuid @@ -506,13 +506,13 @@ update links set tail_uuid='#{g5}' where uuid='#{l1.uuid}' act_as_user users(:admin) do # Even admin cannot change frozen_by_uuid to someone else's UUID. err = assert_raises do - proj.update_attributes!(frozen_by_uuid: users(:project_viewer).uuid) + proj.update!(frozen_by_uuid: users(:project_viewer).uuid) end assert_match /can only be set to the current user's UUID/, err.inspect proj.reload # Admin can unfreeze. - assert proj.update_attributes(frozen_by_uuid: nil), proj.errors.messages + assert proj.update(frozen_by_uuid: nil), proj.errors.messages end # Cannot freeze a project if it contains container requests in @@ -521,15 +521,15 @@ update links set tail_uuid='#{g5}' where uuid='#{l1.uuid}' creq_uncommitted = ContainerRequest.create!(test_cr_attrs.merge(owner_uuid: proj_inner.uuid)) creq_committed = ContainerRequest.create!(test_cr_attrs.merge(owner_uuid: proj_inner.uuid, state: 'Committed')) err = assert_raises do - proj.update_attributes!(frozen_by_uuid: users(:active).uuid) + proj.update!(frozen_by_uuid: users(:active).uuid) end assert_match /container request zzzzz-xvhdp-.* with state = Committed/, err.inspect proj.reload # Can freeze once all container requests are in Uncommitted or # Final state - creq_committed.update_attributes!(state: ContainerRequest::Final) - assert proj.update_attributes(frozen_by_uuid: users(:active).uuid) + creq_committed.update!(state: ContainerRequest::Final) + assert proj.update(frozen_by_uuid: users(:active).uuid) end end diff --git a/services/api/test/unit/link_test.rb b/services/api/test/unit/link_test.rb index 5d36653a56..b9806486ad 100644 --- a/services/api/test/unit/link_test.rb +++ b/services/api/test/unit/link_test.rb @@ -109,7 +109,7 @@ class LinkTest < ActiveSupport::TestCase test "updating permission causes any conflicting links to be deleted" do link1, link2 = create_overlapping_permissions(['can_read', 'can_manage']) - Link.find_by_uuid(link2).update_attributes!(name: 'can_write') + Link.find_by_uuid(link2).update!(name: 'can_write') assert_empty Link.where(uuid: link1) end @@ -121,8 +121,8 @@ class LinkTest < ActiveSupport::TestCase test "updating login permission causes any conflicting links to be deleted" do link1, link2 = create_overlapping_permissions(['can_login', 'can_login'], {properties: {username: 'foo1'}}) - Link.find_by_uuid(link1).update_attributes!(properties: {'username' => 'foo2'}) - Link.find_by_uuid(link2).update_attributes!(properties: {'username' => 'foo2'}) + Link.find_by_uuid(link1).update!(properties: {'username' => 'foo2'}) + Link.find_by_uuid(link2).update!(properties: {'username' => 'foo2'}) assert_empty Link.where(uuid: link1) end diff --git a/services/api/test/unit/log_test.rb b/services/api/test/unit/log_test.rb index 66c8c8d923..d3a1b618d5 100644 --- a/services/api/test/unit/log_test.rb +++ b/services/api/test/unit/log_test.rb @@ -319,7 +319,7 @@ class LogTest < ActiveSupport::TestCase assert_logged(coll, :create) do |props| assert_equal(txt, props['new_attributes']['manifest_text']) end - coll.update_attributes!(name: "testing") + coll.update!(name: "testing") assert_logged(coll, :update) do |props| assert_equal(txt, props['old_attributes']['manifest_text']) assert_equal(txt, props['new_attributes']['manifest_text']) diff --git a/services/api/test/unit/owner_test.rb b/services/api/test/unit/owner_test.rb index aa0ac5f361..1c1bd93b81 100644 --- a/services/api/test/unit/owner_test.rb +++ b/services/api/test/unit/owner_test.rb @@ -63,7 +63,7 @@ class OwnerTest < ActiveSupport::TestCase assert(Specimen.where(uuid: i.uuid).any?, "new item should really be in DB") - assert(i.update_attributes(owner_uuid: new_o.uuid), + assert(i.update(owner_uuid: new_o.uuid), "should change owner_uuid from #{o.uuid} to #{new_o.uuid}") end end @@ -92,7 +92,7 @@ class OwnerTest < ActiveSupport::TestCase "new #{o_class} should really be in DB") old_uuid = o.uuid new_uuid = o.uuid.sub(/..........$/, rand(2**256).to_s(36)[0..9]) - assert(o.update_attributes(uuid: new_uuid), + assert(o.update(uuid: new_uuid), "should change #{o_class} uuid from #{old_uuid} to #{new_uuid}") assert_equal(false, o_class.where(uuid: old_uuid).any?, "#{old_uuid} should disappear when renamed to #{new_uuid}") @@ -118,7 +118,7 @@ class OwnerTest < ActiveSupport::TestCase assert_equal(true, Specimen.where(owner_uuid: o.uuid).any?, "need something to be owned by #{o.uuid} for this test") new_uuid = o.uuid.sub(/..........$/, rand(2**256).to_s(36)[0..9]) - assert(!o.update_attributes(uuid: new_uuid), + assert(!o.update(uuid: new_uuid), "should not change uuid of #{ofixt} that owns objects") end end @@ -126,7 +126,7 @@ class OwnerTest < ActiveSupport::TestCase test "delete User that owns self" do o = User.create! assert User.where(uuid: o.uuid).any?, "new User should really be in DB" - assert_equal(true, o.update_attributes(owner_uuid: o.uuid), + assert_equal(true, o.update(owner_uuid: o.uuid), "setting owner to self should work") skip_check_permissions_against_full_refresh do diff --git a/services/api/test/unit/permission_test.rb b/services/api/test/unit/permission_test.rb index db60b4e6e1..14c810d81a 100644 --- a/services/api/test/unit/permission_test.rb +++ b/services/api/test/unit/permission_test.rb @@ -84,7 +84,7 @@ class PermissionTest < ActiveSupport::TestCase assert users(:active).can?(write: ob) assert users(:active).can?(read: ob) - l1.update_attributes!(name: 'can_read') + l1.update!(name: 'can_read') assert !users(:active).can?(write: ob) assert users(:active).can?(read: ob) @@ -293,7 +293,7 @@ class PermissionTest < ActiveSupport::TestCase "manager saw the minion's private stuff") assert_raises(ArvadosModel::PermissionDeniedError, "manager could update minion's private stuff") do - minions_specimen.update_attributes(properties: {'x' => 'y'}) + minions_specimen.update(properties: {'x' => 'y'}) end end @@ -310,7 +310,7 @@ class PermissionTest < ActiveSupport::TestCase .where(uuid: minions_specimen.uuid), "manager could not find minion's specimen by uuid") assert_equal(true, - minions_specimen.update_attributes(properties: {'x' => 'y'}), + minions_specimen.update(properties: {'x' => 'y'}), "manager could not update minion's specimen object") end end @@ -355,17 +355,17 @@ class PermissionTest < ActiveSupport::TestCase "OTHER can see #{u.first_name} in the user list") act_as_user u do assert_raises ArvadosModel::PermissionDeniedError, "wrote without perm" do - other.update_attributes!(prefs: {'pwned' => true}) + other.update!(prefs: {'pwned' => true}) end - assert_equal(true, u.update_attributes!(prefs: {'thisisme' => true}), + assert_equal(true, u.update!(prefs: {'thisisme' => true}), "#{u.first_name} can't update its own prefs") end act_as_user other do assert_raises(ArvadosModel::PermissionDeniedError, "OTHER wrote #{u.first_name} without perm") do - u.update_attributes!(prefs: {'pwned' => true}) + u.update!(prefs: {'pwned' => true}) end - assert_equal(true, other.update_attributes!(prefs: {'thisisme' => true}), + assert_equal(true, other.update!(prefs: {'thisisme' => true}), "OTHER can't update its own prefs") end end @@ -382,7 +382,7 @@ class PermissionTest < ActiveSupport::TestCase set_user_from_auth :rominiadmin ob = Collection.create! assert_raises ArvadosModel::PermissionDeniedError, "changed owner to unwritable user" do - ob.update_attributes!(owner_uuid: users(:active).uuid) + ob.update!(owner_uuid: users(:active).uuid) end end @@ -397,7 +397,7 @@ class PermissionTest < ActiveSupport::TestCase set_user_from_auth :rominiadmin ob = Collection.create! assert_raises ArvadosModel::PermissionDeniedError, "changed owner to unwritable group" do - ob.update_attributes!(owner_uuid: groups(:aproject).uuid) + ob.update!(owner_uuid: groups(:aproject).uuid) end end diff --git a/services/api/test/unit/repository_test.rb b/services/api/test/unit/repository_test.rb index cb562ef977..674a34ffd8 100644 --- a/services/api/test/unit/repository_test.rb +++ b/services/api/test/unit/repository_test.rb @@ -263,7 +263,7 @@ class RepositoryTest < ActiveSupport::TestCase test "non-admin can rename own repo" do act_as_user users(:active) do - assert repositories(:foo).update_attributes(name: 'active/foo12345') + assert repositories(:foo).update(name: 'active/foo12345') end end diff --git a/services/api/test/unit/user_test.rb b/services/api/test/unit/user_test.rb index 7e19ad5821..35e6ec9e07 100644 --- a/services/api/test/unit/user_test.rb +++ b/services/api/test/unit/user_test.rb @@ -166,7 +166,7 @@ class UserTest < ActiveSupport::TestCase if auto_admin_first_user_config # This test requires no admin users exist (except for the system user) act_as_system_user do - users(:admin).update_attributes!(is_admin: false) + users(:admin).update!(is_admin: false) end @all_users = User.where("uuid not like '%-000000000000000'").where(:is_admin => true) assert_equal 0, @all_users.count, "No admin users should exist (except for the system user)" @@ -800,7 +800,7 @@ class UserTest < ActiveSupport::TestCase test "empty identity_url saves as null" do set_user_from_auth :admin user = users(:active) - assert user.update_attributes(identity_url: '') + assert user.update(identity_url: '') user.reload assert_nil user.identity_url end diff --git a/services/api/test/unit/workflow_test.rb b/services/api/test/unit/workflow_test.rb index 26cd7f215e..4b3e6095d9 100644 --- a/services/api/test/unit/workflow_test.rb +++ b/services/api/test/unit/workflow_test.rb @@ -60,7 +60,7 @@ class WorkflowTest < ActiveSupport::TestCase definition = "k1:\n v1: x\n v2: y" assert_raises(ActiveRecord::RecordInvalid) do - w.update_attributes!(definition: definition) + w.update!(definition: definition) end end @@ -71,7 +71,7 @@ class WorkflowTest < ActiveSupport::TestCase # when it does not already have custom values for these fields w = Workflow.find_by_uuid(workflows(:workflow_with_no_name_and_desc).uuid) definition = "name: test name 1\ndescription: test desc 1\nother: some more" - w.update_attributes!(definition: definition) + w.update!(definition: definition) w.reload assert_equal "test name 1", w.name assert_equal "test desc 1", w.description @@ -79,7 +79,7 @@ class WorkflowTest < ActiveSupport::TestCase # Workflow name and desc should be set with values from definition yaml # when it does not already have custom values for these fields definition = "name: test name 2\ndescription: test desc 2\nother: some more" - w.update_attributes!(definition: definition) + w.update!(definition: definition) w.reload assert_equal "test name 2", w.name assert_equal "test desc 2", w.description @@ -87,7 +87,7 @@ class WorkflowTest < ActiveSupport::TestCase # Workflow name and desc should be set with values from definition yaml # even if it means emptying them out definition = "more: etc" - w.update_attributes!(definition: definition) + w.update!(definition: definition) w.reload assert_nil w.name assert_nil w.description @@ -95,17 +95,17 @@ class WorkflowTest < ActiveSupport::TestCase # Workflow name and desc set using definition yaml should be cleared # if definition yaml is cleared definition = "name: test name 2\ndescription: test desc 2\nother: some more" - w.update_attributes!(definition: definition) + w.update!(definition: definition) w.reload definition = nil - w.update_attributes!(definition: definition) + w.update!(definition: definition) w.reload assert_nil w.name assert_nil w.description # Workflow name and desc should be set to provided custom values definition = "name: test name 3\ndescription: test desc 3\nother: some more" - w.update_attributes!(name: "remains", description: "remains", definition: definition) + w.update!(name: "remains", description: "remains", definition: definition) w.reload assert_equal "remains", w.name assert_equal "remains", w.description @@ -113,7 +113,7 @@ class WorkflowTest < ActiveSupport::TestCase # Workflow name and desc should retain provided custom values # and should not be overwritten by values from yaml definition = "name: test name 4\ndescription: test desc 4\nother: some more" - w.update_attributes!(definition: definition) + w.update!(definition: definition) w.reload assert_equal "remains", w.name assert_equal "remains", w.description @@ -121,7 +121,7 @@ class WorkflowTest < ActiveSupport::TestCase # Workflow name and desc should retain provided custom values # and not be affected by the clearing of the definition yaml definition = nil - w.update_attributes!(definition: definition) + w.update!(definition: definition) w.reload assert_equal "remains", w.name assert_equal "remains", w.description -- 2.30.2