alias_method_chain limits extensibility

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

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

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?

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.

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

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.

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.

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.

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.

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.

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

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.

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

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.

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 !