From 924f8f6c13c06afc8a83168929b249e0e8fa7d18 Mon Sep 17 00:00:00 2001 From: Tom Clegg Date: Mon, 14 Nov 2022 15:23:20 -0500 Subject: [PATCH] 19698: Fix savepoint usage. Arvados-DCO-1.1-Signed-off-by: Tom Clegg --- services/api/app/models/arvados_model.rb | 8 ++++---- .../arvados/v1/collections_controller_test.rb | 18 ++++++++++++++++++ 2 files changed, 22 insertions(+), 4 deletions(-) diff --git a/services/api/app/models/arvados_model.rb b/services/api/app/models/arvados_model.rb index c2725506c0..9c03c03a60 100644 --- a/services/api/app/models/arvados_model.rb +++ b/services/api/app/models/arvados_model.rb @@ -478,12 +478,11 @@ class ArvadosModel < ApplicationRecord conn.exec_query 'SAVEPOINT save_with_unique_name' begin save! + conn.exec_query 'RELEASE SAVEPOINT save_with_unique_name' rescue ActiveRecord::RecordNotUnique => rn raise if max_retries == 0 max_retries -= 1 - conn.exec_query 'ROLLBACK TO SAVEPOINT save_with_unique_name' - # Dig into the error to determine if it is specifically calling out a # (owner_uuid, name) uniqueness violation. In this specific case, and # the client requested a unique name with ensure_unique_name==true, @@ -501,6 +500,8 @@ class ArvadosModel < ApplicationRecord detail = err.result.error_field(PG::Result::PG_DIAG_MESSAGE_DETAIL) raise unless /^Key \(owner_uuid, name\)=\([a-z0-9]{5}-[a-z0-9]{5}-[a-z0-9]{15}, .*?\) already exists\./.match detail + conn.exec_query 'ROLLBACK TO SAVEPOINT save_with_unique_name' + new_name = "#{name_was} (#{db_current_time.utc.iso8601(3)})" if new_name == name # If the database is fast enough to do two attempts in the @@ -518,10 +519,9 @@ class ArvadosModel < ApplicationRecord self[:current_version_uuid] = nil end end + conn.exec_query 'SAVEPOINT save_with_unique_name' retry - ensure - conn.exec_query 'RELEASE SAVEPOINT save_with_unique_name' end end end diff --git a/services/api/test/functional/arvados/v1/collections_controller_test.rb b/services/api/test/functional/arvados/v1/collections_controller_test.rb index af11715982..8a1d044d6a 100644 --- a/services/api/test/functional/arvados/v1/collections_controller_test.rb +++ b/services/api/test/functional/arvados/v1/collections_controller_test.rb @@ -374,6 +374,24 @@ EOS "Expected 'duplicate key' error in #{response_errors.first}") end + [false, true].each do |ensure_unique_name| + test "create failure with duplicate name, ensure_unique_name #{ensure_unique_name}" do + authorize_with :active + post :create, params: { + collection: { + owner_uuid: users(:active).uuid, + manifest_text: "", + name: "this...............................................................................................................................................................................................................................................................name is too long" + }, + ensure_unique_name: ensure_unique_name + } + assert_response 422 + # check the real error isn't masked by an + # ensure_unique_name-related error (#19698) + assert_match /value too long for type/, json_response['errors'][0] + end + end + [false, true].each do |unsigned| test "create with duplicate name, ensure_unique_name, unsigned=#{unsigned}" do permit_unsigned_manifests unsigned -- 2.30.2