Skip to content

Commit f1e2ce5

Browse files
committed
Base: Replace regexps in method_missing and respond_to_missing
Prior to this commit, the regular expressions used in both `method_missing` and `respond_to_missing` were similar, but had slight variations. That resulted in unknown attributes being able to be written to through methods "did not respond to" (like `send("#{attr_name}=", value)`). Similarly, the `?`-suffixed predicate methods would return the same way when invoked. Without the implementation changes, the following tests introduced in this commit fail: ``` 1) Failure: BaseTest#test_respond_to_unknown_attribute_writer [test/cases/base_test.rb:967]: Expected #<Post:0x0000000120cb7958 @attributes={}, @prefix_options={}, @persisted=false> (Post) to respond to #unknown_attribute=. ``` This commit's changes replace the regular expressions with explicit [String#end_with?][]. Originally (in [4179e98][] on May 4, 2007!), the implementation was based on String comparison, but was eventually replaced with a Regular-expression based implementation in [c7629a6][] (on on Nov 9, 2009). The commit message for `c7629a6` cited <q>reliance on string access core extension</q>. In the intervening years, Ruby's standard library String implementation added `end_with?`, so the core extensions mentioned in the message are no longer necessary in modern Ruby versions. Considerations --- This change introduces subtle behavior changes. Whether or not it fixes a bug or introduces a breaking change might be open to interpretation. Conventions recommend that `respond_to?` and `method_missing` *should* remain synchronized in what they consider inside and outside of a class interface. Supporting the method invocations for `=` and `?` suffixed methods without providing indications of that support when asked by callers deviates from that convention. In order to respond to `=`-suffixed writers and `?`-suffixed predicates, the prior implementation required that the attributes being written to or queried must have already been assigned to the internal `@attributes` hash. That means that a `method_missing`-triggered write must've pre-dated the `respond_to?` invocation. Whether or not this is coincidental or intentional is a question best asked of the historical authors and creators of the gem. Ignoring intent, this commit proposes that both `respond_to?` and `method_missing` communicate support for `=`- and `?`-suffixed methods regardless of whether or not they've been written already. [String#end_with?]: https://ruby-doc.org/3.4.1/String.html#method-i-end_with-3F [4179e98]: 4179e98 [c7629a6]: c7629a6
1 parent b549319 commit f1e2ce5

2 files changed

Lines changed: 36 additions & 2 deletions

File tree

lib/active_resource/base.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1653,7 +1653,7 @@ def respond_to_missing?(method, include_priv = false)
16531653
super
16541654
elsif known_attributes.include?(method_name)
16551655
true
1656-
elsif method_name =~ /(?:=|\?)$/ && attributes.include?($`)
1656+
elsif method_name.end_with?("=", "?")
16571657
true
16581658
else
16591659
# super must be called at the end of the method, because the inherited respond_to?

test/cases/base_test.rb

Lines changed: 35 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -960,7 +960,41 @@ def test_respond_to
960960
assert_respond_to matz, :name
961961
assert_respond_to matz, :name=
962962
assert_respond_to matz, :name?
963-
assert_not matz.respond_to?(:super_scalable_stuff)
963+
assert_not_respond_to matz, :super_scalable_stuff
964+
end
965+
966+
def test_respond_to_unknown_attribute_writer
967+
assert_respond_to Post.new, :unknown_attribute=
968+
end
969+
970+
def test_reading_an_unknown_attribute_raises_NoMethodError
971+
assert_raises NoMethodError, match: "unknown_attribute" do
972+
Post.new.unknown_attribute
973+
end
974+
end
975+
976+
def test_writing_an_unknown_attribute_assigns_a_value_that_can_be_read
977+
post = Post.new
978+
979+
post.unknown_attribute = "assigned"
980+
981+
assert_respond_to post, :unknown_attribute
982+
assert_equal "assigned", post.unknown_attribute
983+
end
984+
985+
def test_writing_nil_to_an_existing_attribute_can_be_read
986+
post = Post.new unknown_attribute: "assigned"
987+
988+
post.unknown_attribute = nil
989+
990+
assert_nil post.unknown_attribute
991+
assert_respond_to post, :unknown_attribute
992+
end
993+
994+
def test_predicate_for_an_unknown_attribute_returns_nil
995+
post = Post.new
996+
997+
assert_not_predicate post, :unknown_attribute?
964998
end
965999

9661000
def test_custom_header

0 commit comments

Comments
 (0)