Skip to content

Commit 8ba2b7f

Browse files
authored
Merge pull request rails#51409 from fatkodima/fix-destroy_async-job-for-cpk
Fix `destroy_async` job for owners with composite primary keys
2 parents 7d5c1c7 + 26fccf4 commit 8ba2b7f

File tree

7 files changed

+38
-24
lines changed

7 files changed

+38
-24
lines changed

activerecord/lib/active_record/associations/belongs_to_association.rb

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,11 +12,11 @@ def handle_dependency
1212
raise ActiveRecord::Rollback unless target.destroy
1313
when :destroy_async
1414
if reflection.foreign_key.is_a?(Array)
15-
primary_key_column = reflection.active_record_primary_key.map(&:to_sym)
16-
id = reflection.foreign_key.map { |col| owner.public_send(col.to_sym) }
15+
primary_key_column = reflection.active_record_primary_key
16+
id = reflection.foreign_key.map { |col| owner.public_send(col) }
1717
else
18-
primary_key_column = reflection.active_record_primary_key.to_sym
19-
id = owner.public_send(reflection.foreign_key.to_sym)
18+
primary_key_column = reflection.active_record_primary_key
19+
id = owner.public_send(reflection.foreign_key)
2020
end
2121

2222
enqueue_destroy_association(

activerecord/lib/active_record/associations/has_many_association.rb

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,10 +35,10 @@ def handle_dependency
3535
unless target.empty?
3636
association_class = target.first.class
3737
if association_class.query_constraints_list
38-
primary_key_column = association_class.query_constraints_list.map(&:to_sym)
38+
primary_key_column = association_class.query_constraints_list
3939
ids = target.collect { |assoc| primary_key_column.map { |col| assoc.public_send(col) } }
4040
else
41-
primary_key_column = association_class.primary_key.to_sym
41+
primary_key_column = association_class.primary_key
4242
ids = target.collect { |assoc| assoc.public_send(primary_key_column) }
4343
end
4444

activerecord/lib/active_record/associations/has_one_association.rb

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,10 +34,10 @@ def delete(method = options[:dependent])
3434
throw(:abort) unless target.destroyed?
3535
when :destroy_async
3636
if target.class.query_constraints_list
37-
primary_key_column = target.class.query_constraints_list.map(&:to_sym)
37+
primary_key_column = target.class.query_constraints_list
3838
id = primary_key_column.map { |col| target.public_send(col) }
3939
else
40-
primary_key_column = target.class.primary_key.to_sym
40+
primary_key_column = target.class.primary_key
4141
id = target.public_send(primary_key_column)
4242
end
4343

activerecord/lib/active_record/destroy_association_async_job.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ def perform(
1919
)
2020
association_model = association_class.constantize
2121
owner_class = owner_model_name.constantize
22-
owner = owner_class.find_by(owner_class.primary_key.to_sym => owner_id)
22+
owner = owner_class.find_by(owner_class.primary_key => [owner_id])
2323

2424
if !owner_destroyed?(owner, ensuring_owner_was_method)
2525
raise DestroyAssociationAsyncError, "owner record not destroyed"

activerecord/test/activejob/destroy_association_async_test.rb

Lines changed: 10 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,8 @@
2424
require "models/sharded/blog_post"
2525
require "models/sharded/blog_post_tag"
2626
require "models/sharded/blog"
27+
require "models/cpk/book_destroy_async"
28+
require "models/cpk/chapter_destroy_async"
2729

2830
class DestroyAssociationAsyncTest < ActiveRecord::TestCase
2931
include ActiveJob::TestHelper
@@ -283,22 +285,16 @@ class DestroyAssociationAsyncTest < ActiveRecord::TestCase
283285
end
284286

285287
test "has_many associated with composite primary key" do
286-
blog = Sharded::Blog.create!
287-
blog_post = Sharded::BlogPostDestroyAsync.create!(blog_id: blog.id)
288-
289-
comment1 = Sharded::CommentDestroyAsync.create!(body: "Great post! :clap:")
290-
comment2 = Sharded::CommentDestroyAsync.create!(body: "Terrible post! :thumbs-down:")
291-
292-
blog_post.comments << [comment1, comment2]
293-
294-
blog_post.save!
288+
book = Cpk::BookDestroyAsync.create!(id: [1, 1])
289+
_chapter1 = book.chapters.create!(id: [1, 1], title: "Chapter 1")
290+
_chapter2 = book.chapters.create!(id: [1, 2], title: "Chapter 2")
295291

296292
assert_enqueued_jobs 1, only: ActiveRecord::DestroyAssociationAsyncJob do
297-
blog_post.destroy
293+
book.destroy
298294
end
299295

300296
sql = capture_sql do
301-
assert_difference -> { Sharded::CommentDestroyAsync.count }, -2 do
297+
assert_difference -> { Cpk::ChapterDestroyAsync.count }, -2 do
302298
perform_enqueued_jobs only: ActiveRecord::DestroyAssociationAsyncJob
303299
end
304300
end
@@ -307,12 +303,11 @@ class DestroyAssociationAsyncTest < ActiveRecord::TestCase
307303
assert_equal 2, delete_sqls.count
308304

309305
delete_sqls.each do |sql|
310-
assert_match(/#{Regexp.escape(Sharded::Tag.lease_connection.quote_table_name("sharded_comments.blog_id"))} =/, sql)
306+
assert_match(/#{Regexp.escape(Cpk::ChapterDestroyAsync.lease_connection.quote_table_name("cpk_chapters.author_id"))} =/, sql)
311307
end
312308
ensure
313-
Sharded::CommentDestroyAsync.delete_all
314-
Sharded::BlogPostDestroyAsync.delete_all
315-
Sharded::Blog.delete_all
309+
Cpk::ChapterDestroyAsync.delete_all
310+
Cpk::BookDestroyAsync.delete_all
316311
end
317312

318313
test "not enqueue the job if transaction is not committed" do
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
# frozen_string_literal: true
2+
3+
module Cpk
4+
class BookDestroyAsync < ActiveRecord::Base
5+
self.table_name = :cpk_books
6+
7+
has_many :chapters, query_constraints: [:author_id, :book_id], class_name: "Cpk::ChapterDestroyAsync", dependent: :destroy_async
8+
end
9+
end
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
# frozen_string_literal: true
2+
3+
module Cpk
4+
class ChapterDestroyAsync < ActiveRecord::Base
5+
self.table_name = :cpk_chapters
6+
self.primary_key = [:author_id, :id]
7+
8+
belongs_to :book, query_constraints: [:author_id, :book_id], class_name: "Cpk::BookDestroyAsync"
9+
end
10+
end

0 commit comments

Comments
 (0)