Lighthouse has a new layout. Prefer the old one? Return to the old layout, and switch back any time from the link at the top of each page.

This project is archived and is in readonly mode.

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

#1083

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.

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

State: resolved
Milestone: none
Assigned to: Pratik Pratik
Importance: none

Activity

  1. Tarmo Tänav
    Tarmo Tänav
    
    private_methods.include?(method.to_s)
    

    is likely to fail on ruby 1.9 due to method name arrays containing symbols there.

    September 21st, 2008 @ 05:52 AM

  2. Adam Milligan
    Adam Milligan

    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.

    September 26th, 2008 @ 07:11 AM

  3. Michael Koziarski
    Michael Koziarski
    • 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.

    September 29th, 2008 @ 04:45 PM

  4. Adam Milligan
  5. Michael Koziarski
    Michael Koziarski

    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?

    October 7th, 2008 @ 09:22 PM

  6. Pratik
    Pratik

    Please review this new patch. Will apply tomorrow if no one objects by then.

    @Koz quote_value was triggering method_missing, hence that change.

    October 8th, 2008 @ 12:07 AM

  7. Repository
    Repository

    (from [691aa20280456c332bfaaf69b58adc86fd86a2b8]) Ensure methods called on association proxies respect access control. [#1083 Calls to private methods via association proxies should act consistently with... state:resolved] [Adam Milligan, Pratik] http://github.com/rails/rails/co...

    October 13th, 2008 @ 06:04 PM

  8. Ian White
    Ian White

    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)

    October 14th, 2008 @ 12:01 AM

  9. Pratik
    Pratik
    • 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.

    October 14th, 2008 @ 08:54 AM

  10. Ian White
    Ian White

    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

    October 16th, 2008 @ 01:43 AM

  11. Ian White
    Ian White

    I just realised that a smarter fix would be to make AssociationCollection#proxy_respond_to? do the work, and leave send untouched.

    October 16th, 2008 @ 01:53 AM

  12. Ian White
    Ian White

    Here's a better patch, with the same test, but the fix outlined in the above comment.

    October 16th, 2008 @ 02:03 AM

  13. Repository
    Repository

    (from [517bc500ed95a84fd2aadff34fdc14cb7965bc6b]) Allow class methods to be sent (via #send) to association proxy (fix for bug introduced by 691aa20) [#1083 Calls to private methods via association proxies should act consistently with...]

    Signed-off-by: Pratik Naik pratiknaik@gmail.com http://github.com/rails/rails/co...

    October 16th, 2008 @ 09:43 AM

  14. Pratik
    Pratik
    • State changed from new to resolved

    October 16th, 2008 @ 10:26 AM

  15. Repository
    Repository

    (from [95c609357e78106e9673931d3f60d8ff3cc0a0cd]) Ensure association proxy responds to private class methods defined in associated class. [#1083 Calls to private methods via association proxies should act consistently with...] http://github.com/rails/rails/co...

    October 16th, 2008 @ 10:24 PM

  16. Joseph Palermo
    Joseph Palermo

    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.

    December 22nd, 2008 @ 07:06 PM

  17. Christopher J. Bottaro
    Christopher J. Bottaro

    I am running into the same problem as Joseph. I can't access any of my method_missing functionality through the proxy.

    December 28th, 2008 @ 10:05 PM

  18. Michael Koziarski
    Michael Koziarski

    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.

    December 28th, 2008 @ 10:14 PM

  19. Adam Milligan
    Adam Milligan

    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.

    December 28th, 2008 @ 11:02 PM

  20. Joseph Palermo
    Joseph Palermo

    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.

    December 30th, 2008 @ 07:24 AM

  21. Pete P.
    Pete P.

    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.

    January 24th, 2009 @ 11:50 PM

  22. Michael Koziarski
    Michael Koziarski

    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 :)

    January 25th, 2009 @ 02:28 AM