-
Notifications
You must be signed in to change notification settings - Fork 20
Add support for per route middleware stacks #65
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
jasonayre
wants to merge
20
commits into
mxenabled:master
Choose a base branch
from
jasonayre:master
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
20 commits
Select commit
Hold shift + click to select a range
b7030e3
add middleware stack for sake of allowing forking
jasonayre 2e46a33
fetch middleware
jasonayre 03ddfc7
instantiate own middleware stack
jasonayre 946b350
routes can have their own middleware which can be enhanced with addit…
jasonayre 69b18f2
allow for block when defining route so use can be used to add middlew…
jasonayre 0631d28
middleware gets passed with env
jasonayre 96c9443
call middleware pulled from route not ActionSubscriber.config
jasonayre c085e4c
fetch middleware for route
jasonayre 80ce5a8
just pass middleware klass for now to use as splatting not passing ar…
jasonayre c09babb
delegate middleware methods
jasonayre 4018f25
routes can define a stack of middleware and use that, like a pheonix …
jasonayre 29fd602
fetch middleware or fork from config as default
jasonayre 4d1e03c
add spec for middleware stack
jasonayre a71fb51
set stacks like routes for consistency
jasonayre ff56991
fix forked stack to preserve args, just deep dup array and set ivar d…
jasonayre 00d5653
add router stack spec
jasonayre f2a9032
add example of adding/applying middleware stack in routes
jasonayre 9a05360
call route specific middleware for march hare subscriber
jasonayre 5512438
add hash accessors to env to as it mimics rack/makes middleware more …
jasonayre cfe42ab
add action_subscriber_routes load hook so plugin authors can define o…
jasonayre File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,13 @@ | ||
| require 'middleware/builder' | ||
|
|
||
| module ActionSubscriber | ||
| module Middleware | ||
| class Stack < ::Middleware::Builder | ||
| def forked | ||
| forked_stack = self.class.new(:runner_class => @runner_class) | ||
| forked_stack.instance_variable_set(:@stack, @stack.dup) | ||
| forked_stack | ||
| end | ||
| end | ||
| end | ||
| end |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| describe ActionSubscriber::Middleware::Stack do | ||
| subject { ::ActionSubscriber::Middleware.initialize_stack } | ||
|
|
||
| context "#forked" do | ||
| let(:forked_stack) { subject.forked } | ||
|
|
||
| it "duplicates the stack without modifying original" do | ||
| class A; end; | ||
| forked_stack.use(A) | ||
| expect(forked_stack.instance_variable_get(:@stack).object_id).to_not eq subject.instance_variable_get(:@stack).object_id | ||
| expect(forked_stack.instance_variable_get(:@stack).map(&:first)).to include(A) | ||
| expect(subject.instance_variable_get(:@stack).map(&:first)).to_not include(A) | ||
| end | ||
| end | ||
| end |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we don't fork the middleware stack and instead use a tiered middleware where we first spin through the global middleware and then run through a route's middleware stack at the end of the global middleware, we wouldn't need to run any load hooks here. We also wouldn't need to make our own stack forking mechanism (because we wouldn't need it).
I understand that would mean you couldn't change a middleware in the global stack for a specific route, but in my mind, that's exactly how I'd expect a middleware stack to work. It's a singleton-esque pattern. The main reason I point this out is because I recently went through load hook hell while work on ActionSubscriber to get a few bugs fixed, and seeing a second load hook make me nervous.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, this was added more for the ability of extensibility, i.e. in my gem which uses a middleware stack which adds a certain behavior, I can go
etc. But the implementation could be changed if the router is removed from the stack and stacks were composable, i.e.
For example