#1083 Calls to private methods via association proxies should act consistently with Ruby method dispatch - Ruby on Rails

Type	To find
responsible:me	tickets assigned to you
tagged:"@high"	tickets tagged @high
milestone:next	tickets in the upcoming milestone
state:invalid	tickets with the state invalid
created:"last week"	tickets created last week
sort:number, importance, updated	tickets sorted by #, importance or updated
Combine keywords for powerful searching.
Use advanced searching »

This project is archived and is in readonly mode.

#1083 ✓resolved

Calls to private methods via association proxies should act consistently with Ruby method dispatch

Reported by Adam Milligan | September 21st, 2008 @ 01:13 AM

From core discussion thread http://groups.google.com/group/r...

The problem in a nutshell:

class Fork < ActiveRecord::Base has_many :tines

private def stab_stab_stab

...

end end

class Tine < ActiveRecord::Base belongs_to :fork end

fork.stab_stab_stab # NoMethodError tine.fork.stab_stab_stab # no exception

With this patch, calls to private methods will act consistently when called on association proxies (including when called with #send).

Tests included.

proxy_privates.diff 9 KB

Comments and changes to this ticket

Tarmo Tänav September 21st, 2008 @ 05:52 AM
```
private_methods.include?(method.to_s)
```
is likely to fail on ruby 1.9 due to method name arrays containing symbols there.
You flagged this item as spam.
Adam Milligan September 26th, 2008 @ 07:11 AM
Here is a new patch with some documentation for the somewhat non-obvious decoration of #send in AssociationProxy, and some unnecessary white space removed. I also rebased to the current master commit.
- proxy_privates.diff 14.2 KB
Michael Koziarski September 29th, 2008 @ 04:45 PM
- Assigned user set to “Michael Koziarski”
- Milestone cleared.
Can you squash this down to a single changeset please? Also, perhaps you can just define the private method in the fixture model files themselves rather than the test cases.
You flagged this item as spam.
Adam Milligan October 5th, 2008 @ 12:13 AM
Sorry for the delay on this.
- proxy_privates.diff 7.2 KB

Michael Koziarski October 7th, 2008 @ 09:22 PM

I don't quite follow why you need to use alias_method_chain there?

Also:


-            quote_value(@owner.send(@reflection.options[:primary_key]))
+            @owner.class.quote_value(@owner.send(@reflection.options[:primary_key]))

That change seems unrelated?

Pratik October 8th, 2008 @ 12:07 AM
Please review this new patch. Will apply tomorrow if no one objects by then.

@Koz quote_value was triggering method_missing, hence that change.
- private_proxy_methods.diff 5.6 KB
Repository October 13th, 2008 @ 06:04 PM
(from [691aa20280456c332bfaaf69b58adc86fd86a2b8]) Ensure methods called on association proxies respect access control. [#1083 state:resolved] [Adam Milligan, Pratik] http://github.com/rails/rails/co...
You flagged this item as spam.
Ian White October 14th, 2008 @ 12:01 AM
This has had the following unfortunate effect

(using the example in comments above)
```
  fork.tines.new  # => fine, builds a tine in fork scope

  fork.tines.send(:new)

  NoMethodError: undefined method `new' for []:Array
```
In practical terms, this means that it's now difficult to simply proxy to an association proxy (I do this in the resources_controller where resource_service is a proxy for a class, association_proxy, or something else)
Pratik October 14th, 2008 @ 08:54 AM
- Assigned user changed from “Michael Koziarski” to “Pratik”
@Ian : Ouch. Can you think of any way to fix that ? If not, I might as well revert this for 2.2.
You flagged this item as spam.
Ian White October 16th, 2008 @ 01:43 AM
The new implementation of send forwards to the @target when the proxy doesn't respond to the method.

This is a problem if the proxy itself uses method_missing to do stuff (like construct scopes and forward the method to the association class - the problem I noted above).

I've attached a fix for this, with a test for the above problem. Caveat: I haven't looked at performance, or other issues - just fixed the problem introduced in the changset. If you want to revert the entire changeset, that's fine by me.

Cheers, Ian
- Allow-singleton-methods-sent-to-association-proxy.patch 1.7 KB
You flagged this item as spam.
Ian White October 16th, 2008 @ 01:53 AM
I just realised that a smarter fix would be to make AssociationCollection#proxy_respond_to? do the work, and leave send untouched.
You flagged this item as spam.
Ian White October 16th, 2008 @ 02:03 AM
Here's a better patch, with the same test, but the fix outlined in the above comment.
- Allow-class-methods-to-be-sent-to-assoc-proxy.patch 1.7 KB
Repository October 16th, 2008 @ 09:43 AM
(from [517bc500ed95a84fd2aadff34fdc14cb7965bc6b]) Allow class methods to be sent (via #send) to association proxy (fix for bug introduced by 691aa20) [#1083]

Signed-off-by: Pratik Naik pratiknaik@gmail.com http://github.com/rails/rails/co...
Pratik October 16th, 2008 @ 10:26 AM
- State changed from “new” to “resolved”
Repository October 16th, 2008 @ 10:24 PM
(from [95c609357e78106e9673931d3f60d8ff3cc0a0cd]) Ensure association proxy responds to private class methods defined in associated class. [#1083] http://github.com/rails/rails/co...
You flagged this item as spam.
Joseph Palermo December 22nd, 2008 @ 07:06 PM
We just ran into a problem with this, since you are relying on responds_to? anything that is done using method missing on the proxy_target is no longer directly callable through the proxy.
You flagged this item as spam.
Christopher J. Bottaro December 28th, 2008 @ 10:05 PM
I am running into the same problem as Joseph. I can't access any of my method_missing functionality through the proxy.
Michael Koziarski December 28th, 2008 @ 10:14 PM
If you override method_missing in ruby, it's good practise for you to override respond_to? also. If you fix that, everything will continue working.
You flagged this item as spam.
Adam Milligan December 28th, 2008 @ 11:02 PM
I've submitted a fix for this problem in this ticket. However, upon reflection, I think I like Koz's suggestions better. The target object should respond correctly when interrogated about its capabilities.
You flagged this item as spam.
Joseph Palermo December 30th, 2008 @ 07:24 AM
I agree in the ideal world that if you override method_missing, you should override respond_to?, but that doesn't really help with the plugins that I'm using that don't do this. Now I have a bunch of "object"."association".proxy_target."method_missing_method" in my code. It's either that or I start monkey patching code.

I'm in favor of Adam's patch. It'd be nice if there were a way to use Adam's patch, but also throw a warning if method_missing implements something not found in respond_to?, but I can't really see a way of doing that without throwing false warnings when you really call a method that doesn't exist on accident.
You flagged this item as spam.
Pete P. January 24th, 2009 @ 11:50 PM
Koz, I was pulling my hair out until I came across this ticket. I definitely agree, if you implement method_missing, you should likewise implement respond_to.

However, I think there should be a strong caveat about this in the documentation.
Michael Koziarski January 25th, 2009 @ 02:28 AM
Pete,

If you'd like to have a go at a patch for the documentation, I'll take a look. someone who was bitten by this is the right person to write the warning :)