Skip to content

Commit f27372a

Browse files
authored
We can't extract Associated Objects from methods ending in ? or =. (#42)
Our `method_missing` based proxying is technically eager to hopefully allow us to support more of Active Record's surface area cheaply. It's been working well for over 3 years, but there's some issues. Notably, a `record.respond_to?` check is not enough and there's certain methods we don't want to proxy. Stuff like Active Record's class-level configuration API via `abtract_class?`, `descends_from_active_record?`, `abstract_class=` and `primary_abstract_class=` to name a few. So ideally opting out of proxying methods that end in `?` and `=` should be safe. I'm not including `!` to allow `find_by!` to work. Ref: rubyevents/rubyevents#873 Ref: #40
1 parent 6f73293 commit f27372a

File tree

2 files changed

+21
-5
lines changed

2 files changed

+21
-5
lines changed

lib/active_record/associated_object.rb

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -25,16 +25,16 @@ def extension(&block)
2525
record.class_eval(&block)
2626
end
2727

28-
def method_missing(method, ...)
29-
if !record.respond_to?(method) then super else
30-
record.public_send(method, ...).then do |value|
28+
def method_missing(meth, ...)
29+
if !record.respond_to?(meth) || meth.end_with?("?", "=") then super else
30+
record.public_send(meth, ...).then do |value|
3131
value.respond_to?(:each) ? value.map(&attribute_name) : value&.public_send(attribute_name)
3232
end
3333
end
3434
end
3535

36-
def respond_to_missing?(name, ...)
37-
(name != :abstract_class? && record.respond_to?(name, ...)) || super
36+
def respond_to_missing?(meth, ...)
37+
(record.respond_to?(meth, ...) && !meth.end_with?("?", "=")) || super
3838
end
3939
end
4040

test/active_record/associated_object_test.rb

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,9 @@ class ActiveRecord::AssociatedObjectTest < ActiveSupport::TestCase
3131
assert_equal @publisher, Post::Publisher.find_by(id: 1)
3232
assert_equal @publisher, Post::Publisher.find_by(title: Post.first.title)
3333
assert_equal @publisher, Post::Publisher.find_by(author: Author.first)
34+
assert_equal @publisher, Post::Publisher.find_by!(id: 1)
35+
assert_equal @publisher, Post::Publisher.find_by!(title: Post.first.title)
36+
assert_equal @publisher, Post::Publisher.find_by!(author: Author.first)
3437
assert_equal [ @publisher ], Post::Publisher.where(id: 1)
3538

3639
assert_equal @rating, Post::Comment::Rating.first
@@ -39,11 +42,24 @@ class ActiveRecord::AssociatedObjectTest < ActiveSupport::TestCase
3942
assert_equal @rating, Post::Comment::Rating.find_by(Post::Comment::Rating.primary_key => [[@post.id, @author.id]])
4043
assert_equal @rating, Post::Comment::Rating.find_by(body: "First!!!!")
4144
assert_equal @rating, Post::Comment::Rating.find_by(author: Author.first)
45+
assert_equal @rating, Post::Comment::Rating.find_by!(Post::Comment::Rating.primary_key => [[@post.id, @author.id]])
46+
assert_equal @rating, Post::Comment::Rating.find_by!(body: "First!!!!")
47+
assert_equal @rating, Post::Comment::Rating.find_by!(author: Author.first)
4248
assert_equal [ @rating ], Post::Comment::Rating.where(Post::Comment::Rating.primary_key => [[@post.id, @author.id]])
4349
end
4450

4551
test "associated object method missing extraction omittances" do
4652
refute_respond_to Post::Publisher, :abstract_class?
53+
refute_respond_to Post::Publisher, :descends_from_active_record?
54+
55+
refute_respond_to Post::Publisher, :abstract_class=
56+
refute_respond_to Post::Publisher, :primary_abstract_class=
57+
58+
assert_raise(NoMethodError) { Post::Publisher.abstract_class? }
59+
assert_raise(NoMethodError) { Post::Publisher.descends_from_active_record? }
60+
61+
assert_raise(NoMethodError) { Post::Publisher.abstract_class = true }
62+
assert_raise(NoMethodError) { Post::Publisher.primary_abstract_class = true }
4763
end
4864

4965
test "introspection" do

0 commit comments

Comments
 (0)