Skip to content

Commit

Permalink
Fix stack level too deep when model does not have attributes method.
Browse files Browse the repository at this point in the history
Without that patch when using ActiveModel::AttributeMethods
in a class that does not respond to `attributes` method,
stack level too deep error will be raised on non existing
method. While documentation is clear that you need to define
`attributes` method in order to use AttributeMethods module,
`stack level too deep` is rather obscure and hard to debug,
therefore we should try to not break `method_missing` if
someone forgets about defining `attributes`.
  • Loading branch information
drogus committed Jan 15, 2012
1 parent 85629c8 commit b164e81
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 1 deletion.
2 changes: 1 addition & 1 deletion activemodel/lib/active_model/attribute_methods.rb
Original file line number Diff line number Diff line change
Expand Up @@ -394,7 +394,7 @@ def respond_to?(method, include_private_methods = false)

protected
def attribute_method?(attr_name)
attributes.include?(attr_name)
respond_to_without_attributes?(:attributes) && attributes.include?(attr_name)
end

private
Expand Down
8 changes: 8 additions & 0 deletions activemodel/test/cases/attribute_methods_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,15 @@ def attribute(name)
end
end

class ModelWithouAttributesMethod
include ActiveModel::AttributeMethods
end

class AttributeMethodsTest < ActiveModel::TestCase
test 'method missing works correctly even if attributes method is not defined' do
assert_raises(NoMethodError) { ModelWithouAttributesMethod.new.foo }
end

test 'unrelated classes should not share attribute method matchers' do
assert_not_equal ModelWithAttributes.send(:attribute_method_matchers),
ModelWithAttributes2.send(:attribute_method_matchers)
Expand Down

5 comments on commit b164e81

@matma
Copy link

@matma matma commented on b164e81 Jan 15, 2012

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't You forget the 't'? In ModelWithouAttributesMethod ...

@sikachu
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ECOMMON

@drogus
Copy link
Member Author

@drogus drogus commented on b164e81 Jan 15, 2012

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's just to troll you :trollface:

I will push a fix to other broken tests in a few minutes and will fix it as well there.

@sikachu
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

... and we definitely should request @github for a :trollface: emoji.

@drogus
Copy link
Member Author

@drogus drogus commented on b164e81 Jan 15, 2012

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FFFFFFFUUUUUUUUUU, it seems I can't even add a comment properly :P

Please sign in to comment.