Add new middleware chain methods for insertion into the middle of the list#595
Add new middleware chain methods for insertion into the middle of the list#595mperham merged 5 commits intosidekiq:masterfrom
Conversation
… the middle of the Middleware chains.
|
Would you make sure the API corresponds to rails' middleware API? I think it might use insert_after, etc. |
|
Done. On Dec 19, 2012, at 7:35 AM, Mike Perham notifications@github.com wrote:
|
|
Yep, looks good. http://guides.rubyonrails.org/rails_on_rack.html#configuring-middleware-stack Currently your methods do nothing if the middleware already exists. Should it instead move the existing middleware? |
|
Moving existing middleware would be a logical approach -- I'll work on that. Just noticed something, though, and I'm wondering if Rails middleware has the same behavior as Sidekiq: Sidekiq stops you from adding the same middleware with two different sets of arguments. I'm not sure that I can imagine why you might want to do that, but it does seem like something someone might want to do (for example, do something before before the logging middleware, then clean it up after the logging middleware. Admittedly, this is a little far-fetched and could be accomplished with two different middle wares, but just wondering …) On Dec 19, 2012, at 8:46 AM, Mike Perham notifications@github.com wrote:
|
…ion rather than ignoring existing middle wares.
|
Done. My methods now move existing middleware to the requested position (delete and insert). It keeps the arguments originally associated with the middleware, so you can move a middleware without knowing it's arguments (e.g. On Dec 19, 2012, at 8:46 AM, Mike Perham notifications@github.com wrote:
|
|
Nice work. |
|
Looks like travis failed on your test. Could you investigate? |
Use #delete_if instead of #delete
|
Looks like I was using Array#delete when I should have been using Array#delete_if. I've corrected and pushed to my repo. Do I need to anything else? On Dec 19, 2012, at 9:29 AM, Mike Perham notifications@github.com wrote:
|
Add new middleware chain methods for insertion into the middle of the list
|
Thanks! |
2 participants
Added the methods
#add_before(oldklass, newklass, *args)and#add_after(oldklass, newklass, *args)to facilitate insertion immediately before and after (respectively) an existing middleware class. If the class does not exist, the insertion takes place at the top and bottom (respectively) of the chain. To insert at the top of the chain, you could writechain.add_before(nil, MyServerHook).Mike, please note that I cannot get the tests to run on my machine. Nevertheless, I believe they should work (as does the code, which I've tested in my application).