-
Notifications
You must be signed in to change notification settings - Fork 9
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
Middleware to parse Rails style url parameters #5
base: master
Are you sure you want to change the base?
Middleware to parse Rails style url parameters #5
Conversation
|
||
def matched_params(env) | ||
request_path(env).match(regexp).tap do |matched| | ||
raise Goliath::Validation::BadRequestError, "Request path does not match expected pattern: #{request_path(env)}" if matched.nil? |
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.
Is there a reason why this has to be exclusive? As in, currently if you include this middleware then you can't use any other type of route.
Hi Ilya, I'm not 100% sure what you mean by type of route - do you mean if you are serving multiple routes from a single goliath instance? We are only serving a single route per Goliath instance (as suggested in the google group thread on removing routing for 1.0). I guess it would be possible to make this middleware work for a number of different routes but I think if you are serving multiple routes from a single instance this functionality would belong better in the code that's doing the routing, unless I've misunderstood you? |
Well, the general recommendation is to have logically separated services, instead of a kitchen-sink controller. Having said that, a single web service can respond to multiple URL patterns. Ex: /api/param1/param2 There is no reason why the above shouldn't be served by the same API. We shouldn't have code that forces a single URL, as that will also break support for optional parameters amongst other things. It seems like the simplest approach is to remove the raise: if pattern matches, great, if not, just pass it through. Perhaps, as a config flag, you can make it raise. |
OK, that makes sense, ta. I'll remove the raise. I'd be reluctant to make it a config option as it would only work if you were using one, not multiple instances of the middleware, and it seems a bit off to have a config option that only works in some circumstances. The only other option I can think of is if a single instance of the middleware takes all possible paths as a parameter and then raises if none of them match, perhaps in a block something like this:
where But then it feels like I'm starting to implement more and more of the functionality of a router so maybe that's a bad idea? |
Yeah, it's a slippery slope. Personally, I like the multiple |
Just stumbled on this, would love to see this get merged in as this would be a perfect contrib library for those migrating from 0.9.4 to 1.0.0 ( eg routerless ) 👍 |
Hi, I've created a piece of middleware to parse Rails style URL params. It accepts a URL pattern in the same format as a rails routes file and parses out the params, adding them to the
env['params']
hash.So for example with this in your API:
a request path of
'/kittens/jeremy/mauve'
would lead to a params hash containing: