Skip to content

Conversation

rsslldnphy
Copy link

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:

use Goliath::Contrib::PathParams, '/kittens/:name/:favourite_colour'

a request path of '/kittens/jeremy/mauve' would lead to a params hash containing:

{ 'name' => 'jeremy', 'favourite_colour' => 'mauve' }

Copy link
Member

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.

@rsslldnphy
Copy link
Author

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?

@igrigorik
Copy link
Member

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
/api/private/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.

@rsslldnphy
Copy link
Author

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:

use Goliath::Contrib::PathParams do
  path '/api/:param1/:param2'
  path '/api/private/:param1/:param2'
end

where path is just a method that adds its argument to an array of url patterns to try.

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?

@igrigorik
Copy link
Member

Yeah, it's a slippery slope. Personally, I like the multiple path idea. As long as we don't make any promises about dispatch of these requests.. only parsing of the parameters, I think we're in good shape.

@karlfreeman
Copy link

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants