Skip to content

Commit

Permalink
[fixes rails#50368] ActiveRecord 7.1 regression
Browse files Browse the repository at this point in the history
  • Loading branch information
p-schlickmann committed Dec 16, 2023
1 parent 32afa64 commit 881b5af
Show file tree
Hide file tree
Showing 6 changed files with 59 additions and 2 deletions.
4 changes: 2 additions & 2 deletions activerecord/lib/active_record/relation.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
Expand Down
16 changes: 16 additions & 0 deletions activerecord/test/cases/relations_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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")

Expand Down
3 changes: 3 additions & 0 deletions activerecord/test/models/api_key.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
class ApiKey < ActiveRecord::Base
has_many :versions
end
14 changes: 14 additions & 0 deletions activerecord/test/models/ruby_gem.rb
Original file line number Diff line number Diff line change
@@ -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
9 changes: 9 additions & 0 deletions activerecord/test/models/version.rb
Original file line number Diff line number Diff line change
@@ -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
15 changes: 15 additions & 0 deletions activerecord/test/schema/schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 881b5af

Please sign in to comment.