From 98bd6c43fca91b76d528c8ed5b83c655c86ffe3c Mon Sep 17 00:00:00 2001 From: Tom Clegg Date: Thu, 10 Sep 2020 16:33:31 -0400 Subject: [PATCH] 16813: Avoid writing db when nothing is changing in a batch update. Avoids superfluous log entries and potentially costly permission graph updates. Arvados-DCO-1.1-Signed-off-by: Tom Clegg --- .../api/app/controllers/arvados/v1/users_controller.rb | 10 +++++++++- .../functional/arvados/v1/users_controller_test.rb | 9 +++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/services/api/app/controllers/arvados/v1/users_controller.rb b/services/api/app/controllers/arvados/v1/users_controller.rb index 867b9a6e6a..cd23706d08 100644 --- a/services/api/app/controllers/arvados/v1/users_controller.rb +++ b/services/api/app/controllers/arvados/v1/users_controller.rb @@ -22,7 +22,15 @@ class Arvados::V1::UsersController < ApplicationController rescue ActiveRecord::RecordNotUnique retry end - u.update_attributes!(nullify_attrs(attrs)) + needupdate = {} + nullify_attrs(attrs).each do |k,v| + if !v.nil? && u.send(k) != v + needupdate[k] = v + end + end + if needupdate.length > 0 + u.update_attributes!(needupdate) + end @objects << u end @offset = 0 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 0ce9f1137f..ea5d5b1436 100644 --- a/services/api/test/functional/arvados/v1/users_controller_test.rb +++ b/services/api/test/functional/arvados/v1/users_controller_test.rb @@ -1039,9 +1039,12 @@ class Arvados::V1::UsersControllerTest < ActionController::TestCase test "batch update" do existinguuid = 'remot-tpzed-foobarbazwazqux' newuuid = 'remot-tpzed-newnarnazwazqux' + unchanginguuid = 'remot-tpzed-nochangingattrs' act_as_system_user do User.create!(uuid: existinguuid, email: 'root@existing.example.com') + User.create!(uuid: unchanginguuid, email: 'root@unchanging.example.com', prefs: {'foo' => {'bar' => 'baz'}}) end + assert_equal(1, Log.where(object_uuid: unchanginguuid).count) authorize_with(:admin) patch(:batch_update, @@ -1059,6 +1062,10 @@ class Arvados::V1::UsersControllerTest < ActionController::TestCase 'email' => 'root@remot.example.com', 'username' => '', }, + unchanginguuid => { + 'email' => 'root@unchanging.example.com', + 'prefs' => {'foo' => {'bar' => 'baz'}}, + }, }}) assert_response(:success) @@ -1070,6 +1077,8 @@ class Arvados::V1::UsersControllerTest < ActionController::TestCase assert_equal('noot', User.find_by_uuid(newuuid).first_name) assert_equal('root@remot.example.com', User.find_by_uuid(newuuid).email) + + assert_equal(1, Log.where(object_uuid: unchanginguuid).count) end NON_ADMIN_USER_DATA = ["uuid", "kind", "is_active", "email", "first_name", -- 2.30.2