Rails Ruby on Rails

Comments and changes to this ticket

• 

  Pratik July 11th, 2008 @ 10:57 PM

  • State changed from “new” to “wontfix”
  • Tag set to “activesupport, alias_method_chain, patch”

  alias_method_chain is just supposed to be a shortcut for two alias_method statements. Usecase like yours, deserves a solution of it's own.

  Thanks.

• You flagged this item as spam.
  

  Tammo Freese July 11th, 2008 @ 11:40 PM

  In your opinion, is it OK that you cannot extend/override any of

  rails's ...with... methods?

• You flagged this item as spam.
  

  Steve Purcell July 29th, 2008 @ 02:11 PM

  This seems to be a useful patch that addresses a real problem/limitation, so I'm +1. Plus, it's small and tested; Pratik, maybe you can reconsider?

• 

  Pratik July 29th, 2008 @ 02:15 PM

  • Assigned user set to “josh”
• 

  DHH September 4th, 2008 @ 04:13 PM

  • State changed from “wontfix” to “open”

  target_method_exists can use instance_method_defined?(false) instead.

• 

  josh September 7th, 2008 @ 04:12 PM

  • Assigned user changed from “josh” to “DHH”

  -1 I don't like the idea of replacing the alias with a delegate method call.

  @David I think that only cleans up the "target_method_exists = (instance_methods + private_instance_methods).include?(RUBY_VERSION < '1.9' ? with_method : with_method.to_sym)" busy.

• 

  DHH September 7th, 2008 @ 07:22 PM

  The reason for this is to allow someone to overwrite part of the chain to inject their own behavior. Which I think is very reasonable. If you have another way of achieving the same result, let's discuss that.

  Tammo, perhaps you can post the full example somewhere so Josh can see the use case? I didn't really care for this much until I saw the use case either.

• You flagged this item as spam.
  

  Tammo Freese September 7th, 2008 @ 09:50 PM

  Using alias_method instead of a delegate method call makes overwriting/ extending with methods a major pain.

  Let's say you would like to disable benchmarking, but keep the logger for the rest of the application (or you would like to implement your own benchmarking).

  The code

  class ActionController::Base def perform_action_with_benchmark(*args, &block)

    perform_action_without_benchmark(*args, &block)
  
  

  end end

  looks right, but does not work, since overwriting with methods has no effect at all.

  It is possible to workaround that, but the resulting code

  a) looks like an implementation error, and b) depends on the exact order of the chaining in rails:

  class ActionController::Base def perform_action_without_rescue(*args, &block)

    perform_action_without_benchmark(*args, &block)
  
  

  end end

• 

  josh September 7th, 2008 @ 11:14 PM

  The use case seems reasonable. I wonder if aliasing it back works?

  
  alias_method :perform_action, :perform_action_without_benchmark
  

  I also think those (ActionController) mixins should be done without alias_method_chain. Another ticket for another day.

• You flagged this item as spam.
  

  Tammo Freese September 8th, 2008 @ 08:06 AM

  Hello Joshua,

  aliasing back will not work correctly.

  alias_method :perform_action, :perform_action_without_benchmark

  will disable benchmarking, but it will disable a lot more than that, since it throws out every aspect added to perform_action after benchmarking.

  In Rails 2.1, this would mean that - rescue_from would not work anymore, - action caching would not work anymore, - every plugin extension to perform_action would be lost.

• 

  josh September 8th, 2008 @ 03:49 PM

  Is swapping out ActionController benchmark your real goal, or was it just an example? If it is, I'd like to see how we can do this w/o alias_method_chain.

• You flagged this item as spam.
  

  Tammo Freese September 9th, 2008 @ 11:15 AM

  We found the problem with …with… methods when implementing a custom benchmarking.

  However, in my opinion, every …with… method in rails is a potential extension point for an application or a plugin, and should therefore be extensible.

• You flagged this item as spam.
  

  charly71 September 21st, 2008 @ 11:01 PM

  • Tag changed from “activesupport, alias_method_chain, patch” to “activesupport, alias_method_chain, patch”

  Hi,

  I started a few blog posts on http://ruby.simapse.com about the decorator pattern and alias_method_chain. You might want to have a look.

  Anyhow a suggested approach would be to move (almost?) all instance methods of ActiveRecord::Base into the module ActiveRecord::Core, and put the later on top of all decorating modules (Core makes sens since it's the one to be deCOREated).

  Their would be no aliasing, the method_without_feature is replaced by super and voila, that's it.

  I gave it a try on 'save' and all the tests (cases/base) passed.

  With/without methods are gone there's just one and only save (seems to me less messy when you have many decorators). If i want to save without transaction, i override save in :

  module AR::Transaction def save; super; end end

  Simple, easy, consistant and i don't think you loose any convinience alias_method_chain brings you, and it should break anything (or not much). I still have to dig in though.

  Attached is my patch with save.

  If you think it's worth the try i can go on and refactor it for every method exposed in AR::Base on a github fork.

  cheers

  • rails_without_alias_method_chain_.txt 5.5 KB

• You flagged this item as spam.
  

  Tammo Freese September 25th, 2008 @ 04:12 PM

  Hello charly71,

  I see three problems with this approach:

  1) To decorate one of the save methods in the modules (extending it, not overriding it) you would still need to resort to alias_method_chain or something similar.

  2) You lose the possiblity to call the with/without methods directly on the objects. (I can't tell for sure whether this is needed or not.)

  3) You can only extend inside the module itself, but not inside classes or subclasses. As a result, you cannot restrict an extension to a subtree of the inheritance hierarchy, but it's always all or nothing. As an example, you cannot easily disable benchmarking for a single subclass of ActionController::Base, but only globally.

• You flagged this item as spam.
  

  charly71 September 25th, 2008 @ 05:40 PM

  Hi Tammo,

  yes i got immediatly aware of no 3) after posting and felt stupid.

  regarding no 2) the problem is when you call save_without_transaction, you're probably calling save_with_validation and save_with_dirty, and it is a bit misleading (you're not calling the pure, immaculate save...). Even though it is probably what you want, it doesn't give you more modularity (cherrypicking) than having a chain of 'super' called.

  I do agree though that if you want to override (for just one model down the inheritance chain) your benchmarking or transaction or whatever you'll have to resolve to some magic (which still needs to be boiled in the pot) BUT..... only in your plugin :

  module MyValidation skip :save, :in=>Validation def save "blah"; super; "blah" end end

  Yet i'm speculating because i haven't resolved the magic piece ('super' is giving me some headaches, hum any ruby ninja around ?)

• You flagged this item as spam.
  

  charly71 September 26th, 2008 @ 03:43 PM

  Hi again,

  i hope i'm not polluting this ticket, but since Joshua said mixins in ActionController should be done without alias_method_chain...

  Anyway the attached code says it all and adresses a lot of issues.

  The uninclude file was taken from here :

  http://github.com/yrashk/rbmodex...>

  It is then easy to skip a module on the fly unincluding and reincluding. I'm not sure whats the drawbacks or limitations but it certainly opens up the possibilities.

  • model-decorator-overide.rb 918 Bytes

• You flagged this item as spam.
  

  sliu October 7th, 2008 @ 03:47 AM

  alias_method_chain makes easy AOP, while we need it more manageable, like "insert new feature to specified position of the chain", "rewrite/remove/move specified feature of the chain", "call pure function", etc. thus, chain should be exposed and can be operated, and features in it should be manageable. all of them are not only behaviours to be execute, but also data structures can be operated.

  big project?

  I think it makes sense. just a suggestion to core team to consider.

  I even like it be in ruby grammar(I prefer ruby includes raw event-driven support, annotation support...) -- brainstorm :)

• You flagged this item as spam.
  

  sliu November 17th, 2008 @ 07:09 AM

  finally, I find class Hook in merb's extlib fit my needs.

  much appreciate to rails team who bring and maintain the amazing rails framework.

  I have a big dream: keep open mind to merge rails and merb community together.

  rails and merb are now more like struts and webwork, they finally merged their projects and communities.

  I think whether the dream can be realized primarily depends on not tech, not history. IT IS ALL ABOUT PEOPLE WHO OWE THEM.

• 

  josh January 19th, 2009 @ 07:14 PM

  • State changed from “open” to “wontfix”

  Yehuda is out to kill alias_method_chain. Expect it to be gone in 3.0 ;)

• You flagged this item as spam.
  

  Gaspard Bucher February 6th, 2009 @ 08:57 PM

  Good to know Yehuda is on this. This method chaining has always been hard to adapt or debug.

  I am very curious to know what he comes up with !