Skip to content

Commit

Permalink
Merge pull request rails#51409 from fatkodima/fix-destroy_async-job-f…
Browse files Browse the repository at this point in the history
…or-cpk

Fix `destroy_async` job for owners with composite primary keys
  • Loading branch information
byroot authored May 2, 2024
2 parents 7d5c1c7 + 26fccf4 commit 8ba2b7f
Show file tree
Hide file tree
Showing 7 changed files with 38 additions and 24 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,11 @@ def handle_dependency
raise ActiveRecord::Rollback unless target.destroy
when :destroy_async
if reflection.foreign_key.is_a?(Array)
primary_key_column = reflection.active_record_primary_key.map(&:to_sym)
id = reflection.foreign_key.map { |col| owner.public_send(col.to_sym) }
primary_key_column = reflection.active_record_primary_key
id = reflection.foreign_key.map { |col| owner.public_send(col) }
else
primary_key_column = reflection.active_record_primary_key.to_sym
id = owner.public_send(reflection.foreign_key.to_sym)
primary_key_column = reflection.active_record_primary_key
id = owner.public_send(reflection.foreign_key)
end

enqueue_destroy_association(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,10 @@ def handle_dependency
unless target.empty?
association_class = target.first.class
if association_class.query_constraints_list
primary_key_column = association_class.query_constraints_list.map(&:to_sym)
primary_key_column = association_class.query_constraints_list
ids = target.collect { |assoc| primary_key_column.map { |col| assoc.public_send(col) } }
else
primary_key_column = association_class.primary_key.to_sym
primary_key_column = association_class.primary_key
ids = target.collect { |assoc| assoc.public_send(primary_key_column) }
end

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,10 @@ def delete(method = options[:dependent])
throw(:abort) unless target.destroyed?
when :destroy_async
if target.class.query_constraints_list
primary_key_column = target.class.query_constraints_list.map(&:to_sym)
primary_key_column = target.class.query_constraints_list
id = primary_key_column.map { |col| target.public_send(col) }
else
primary_key_column = target.class.primary_key.to_sym
primary_key_column = target.class.primary_key
id = target.public_send(primary_key_column)
end

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ def perform(
)
association_model = association_class.constantize
owner_class = owner_model_name.constantize
owner = owner_class.find_by(owner_class.primary_key.to_sym => owner_id)
owner = owner_class.find_by(owner_class.primary_key => [owner_id])

if !owner_destroyed?(owner, ensuring_owner_was_method)
raise DestroyAssociationAsyncError, "owner record not destroyed"
Expand Down
25 changes: 10 additions & 15 deletions activerecord/test/activejob/destroy_association_async_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@
require "models/sharded/blog_post"
require "models/sharded/blog_post_tag"
require "models/sharded/blog"
require "models/cpk/book_destroy_async"
require "models/cpk/chapter_destroy_async"

class DestroyAssociationAsyncTest < ActiveRecord::TestCase
include ActiveJob::TestHelper
Expand Down Expand Up @@ -283,22 +285,16 @@ class DestroyAssociationAsyncTest < ActiveRecord::TestCase
end

test "has_many associated with composite primary key" do
blog = Sharded::Blog.create!
blog_post = Sharded::BlogPostDestroyAsync.create!(blog_id: blog.id)

comment1 = Sharded::CommentDestroyAsync.create!(body: "Great post! :clap:")
comment2 = Sharded::CommentDestroyAsync.create!(body: "Terrible post! :thumbs-down:")

blog_post.comments << [comment1, comment2]

blog_post.save!
book = Cpk::BookDestroyAsync.create!(id: [1, 1])
_chapter1 = book.chapters.create!(id: [1, 1], title: "Chapter 1")
_chapter2 = book.chapters.create!(id: [1, 2], title: "Chapter 2")

assert_enqueued_jobs 1, only: ActiveRecord::DestroyAssociationAsyncJob do
blog_post.destroy
book.destroy
end

sql = capture_sql do
assert_difference -> { Sharded::CommentDestroyAsync.count }, -2 do
assert_difference -> { Cpk::ChapterDestroyAsync.count }, -2 do
perform_enqueued_jobs only: ActiveRecord::DestroyAssociationAsyncJob
end
end
Expand All @@ -307,12 +303,11 @@ class DestroyAssociationAsyncTest < ActiveRecord::TestCase
assert_equal 2, delete_sqls.count

delete_sqls.each do |sql|
assert_match(/#{Regexp.escape(Sharded::Tag.lease_connection.quote_table_name("sharded_comments.blog_id"))} =/, sql)
assert_match(/#{Regexp.escape(Cpk::ChapterDestroyAsync.lease_connection.quote_table_name("cpk_chapters.author_id"))} =/, sql)
end
ensure
Sharded::CommentDestroyAsync.delete_all
Sharded::BlogPostDestroyAsync.delete_all
Sharded::Blog.delete_all
Cpk::ChapterDestroyAsync.delete_all
Cpk::BookDestroyAsync.delete_all
end

test "not enqueue the job if transaction is not committed" do
Expand Down
9 changes: 9 additions & 0 deletions activerecord/test/models/cpk/book_destroy_async.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
# frozen_string_literal: true

module Cpk
class BookDestroyAsync < ActiveRecord::Base
self.table_name = :cpk_books

has_many :chapters, query_constraints: [:author_id, :book_id], class_name: "Cpk::ChapterDestroyAsync", dependent: :destroy_async
end
end
10 changes: 10 additions & 0 deletions activerecord/test/models/cpk/chapter_destroy_async.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
# frozen_string_literal: true

module Cpk
class ChapterDestroyAsync < ActiveRecord::Base
self.table_name = :cpk_chapters
self.primary_key = [:author_id, :id]

belongs_to :book, query_constraints: [:author_id, :book_id], class_name: "Cpk::BookDestroyAsync"
end
end

0 comments on commit 8ba2b7f

Please sign in to comment.