This project is archived and is in readonly mode.
alias_method_chain limits extensibility
Reported by Tammo Freese | May 31st, 2008 @ 11:22 AM
There are two problems with the current implementaton of alias_method_chain.
1) If a method defined in a superclass is aliased, replacing the method in the superclass has no effect:
def foo; "foo" end
class B < A
def foo_with_bar; foo_without_bar + " with bar" end
alias_method_chain :foo, :bar
def foo; "new foo" end
B.new.foo # => "foo with bar"
I would have expected "new foo with bar"
2) While ...without... methods can be replaced, replacing ...with... methods has no effect:
u = User.find(:first)
u.save # => true
u.save # => true
I would have expected an exception.
I have attached a patch that fixes both problems.
Comments and changes to this ticket
- Assigned user changed from Joshua Peek 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.
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).
class ActionController::Base def perform_action_with_benchmark(*args, &block)
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)
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.
- Tag changed from activesupport, alias_method_chain, patch to activesupport, alias_method_chain, patch
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.
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.
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 ?)
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 :
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.
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.
Create your profile
Help contribute to this project by taking a few moments to create your personal profile. Create your profile »
<h2 style="font-size: 14px">Tickets have moved to Github</h2>
The new ticket tracker is available at <a href="https://github.com/rails/rails/issues">https://github.com/rails/rails/issues</a>