From 881b5afcb6c6b862ad481ef8aaaa3af7603309ec Mon Sep 17 00:00:00 2001 From: mendespedro Date: Sat, 16 Dec 2023 20:16:08 -0300 Subject: [PATCH] [fixes #50368] ActiveRecord 7.1 regression --- activerecord/lib/active_record/relation.rb | 4 ++-- activerecord/test/cases/relations_test.rb | 16 ++++++++++++++++ activerecord/test/models/api_key.rb | 3 +++ activerecord/test/models/ruby_gem.rb | 14 ++++++++++++++ activerecord/test/models/version.rb | 9 +++++++++ activerecord/test/schema/schema.rb | 15 +++++++++++++++ 6 files changed, 59 insertions(+), 2 deletions(-) create mode 100644 activerecord/test/models/api_key.rb create mode 100644 activerecord/test/models/ruby_gem.rb create mode 100644 activerecord/test/models/version.rb diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb index 294afb2a01128..c491ad97cf6a6 100644 --- a/activerecord/lib/active_record/relation.rb +++ b/activerecord/lib/active_record/relation.rb @@ -213,7 +213,7 @@ def find_or_create_by!(attributes, &block) # and failed due to validation errors it won't be persisted, you get what #create returns in # such situation. def create_or_find_by(attributes, &block) - transaction(requires_new: true) { create(attributes, &block) } + create(attributes, &block) rescue ActiveRecord::RecordNotUnique if connection.transaction_open? where(attributes).lock.find_by!(attributes) @@ -226,7 +226,7 @@ def create_or_find_by(attributes, &block) # {create!}[rdoc-ref:Persistence::ClassMethods#create!] so an exception # is raised if the created record is invalid. def create_or_find_by!(attributes, &block) - transaction(requires_new: true) { create!(attributes, &block) } + create!(attributes, &block) rescue ActiveRecord::RecordNotUnique if connection.transaction_open? where(attributes).lock.find_by!(attributes) diff --git a/activerecord/test/cases/relations_test.rb b/activerecord/test/cases/relations_test.rb index 7f4f79c54a31d..2e7efccbadf70 100644 --- a/activerecord/test/cases/relations_test.rb +++ b/activerecord/test/cases/relations_test.rb @@ -28,6 +28,9 @@ require "models/edge" require "models/subscriber" require "models/cpk" +require "models/api_key" +require "models/ruby_gem" +require "models/version" class RelationTest < ActiveRecord::TestCase fixtures :authors, :author_addresses, :topics, :entrants, :developers, :people, :companies, :developers_projects, :accounts, :categories, :categorizations, :categories_posts, :posts, :comments, :tags, :taggings, :cars, :minivans, :cpk_orders @@ -1554,6 +1557,19 @@ def test_find_or_create_by_with_create_with assert_equal bird, Bird.create_with(color: "blue").find_or_create_by(name: "bob") end + def test_regression_in_model_scoping_callback + gem1 = RubyGem.create! + api_key = ApiKey.create! + Version.create!(ruby_gem: gem1, platform: "ruby", indexed: true) + Version.create!(ruby_gem: gem1, api_key: api_key, platform: "ruby", indexed: true) + + api_key.versions.create_with(indexed: true).find_or_create_by!( + ruby_gem: gem1, number: "0.1.0", platform: "ruby" + ) + + assert_equal 3, gem1.reload.versions.count + end + def test_find_or_create_by_with_block assert_nil Bird.find_by(name: "bob") diff --git a/activerecord/test/models/api_key.rb b/activerecord/test/models/api_key.rb new file mode 100644 index 0000000000000..7c1aca2ca9e8c --- /dev/null +++ b/activerecord/test/models/api_key.rb @@ -0,0 +1,3 @@ +class ApiKey < ActiveRecord::Base + has_many :versions +end diff --git a/activerecord/test/models/ruby_gem.rb b/activerecord/test/models/ruby_gem.rb new file mode 100644 index 0000000000000..016329670240a --- /dev/null +++ b/activerecord/test/models/ruby_gem.rb @@ -0,0 +1,14 @@ +class RubyGem < ActiveRecord::Base + has_many :versions + + def reorder_versions + logger.info Version.all.to_sql + + versions_of_platforms = versions + .group_by(&:platform) + + versions_of_platforms.each_value do |platforms| + Version.find(platforms.max.id).update_column(:latest, true) + end + end +end diff --git a/activerecord/test/models/version.rb b/activerecord/test/models/version.rb new file mode 100644 index 0000000000000..8c5c3179c1e62 --- /dev/null +++ b/activerecord/test/models/version.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +class Version < ActiveRecord::Base + belongs_to :ruby_gem + belongs_to :api_key + + after_save :reorder_versions, if: -> { saved_change_to_id? } + delegate :reorder_versions, to: :ruby_gem +end diff --git a/activerecord/test/schema/schema.rb b/activerecord/test/schema/schema.rb index 02aadb7586cc7..2074aea4d7bfe 100644 --- a/activerecord/test/schema/schema.rb +++ b/activerecord/test/schema/schema.rb @@ -63,6 +63,9 @@ t.timestamp :manufactured_at, default: -> { "CURRENT_TIMESTAMP" } end + create_table :api_keys, force: true do |t| + end + create_table :articles, force: true do |t| end @@ -1294,6 +1297,18 @@ t.column :label, :string end + create_table :ruby_gems, force: true do |t| + end + + create_table :versions, force: true do |t| + t.references :api_key + t.references :ruby_gem + t.string :number + t.string :platform + t.boolean :latest, default: false + t.boolean :indexed, default: false + end + create_table "warehouse-things", force: true do |t| t.integer :value end