xyncro / freya

Freya Web Stack - Meta-Package
https://freya.io
Other
329 stars 30 forks source link

Question: Route and Query #168

Open mexx opened 8 years ago

mexx commented 8 years ago

I have the following use case: I want to have some handlers bound to an URI, no matter whether it contains a query string or not.

The only way I've managed to achieve this is:

freyaRouter {
    route All "/" route1
    route All "/?{q*}" route1 }

Is there any more elegant way to do this?

mexx commented 8 years ago

IMO it shouldn't be restrictive like it is now. When implementing a web service one should follow the Postel's law.

freyaRouter { route All "/" route1 } should be invoked for any URI, because the query string wasn't specified in the template. I would go even further and say, even when a query was specified, then some extra values in the query string shouldn't stop the route from being hit. Otherwise I would always have to define a catch-all rule for the unexpected query string values.

mexx commented 8 years ago

Yet another thought on routing. Currently the route have to match completely, this means one can't assign a handler for a subtree easily.

kolektiv commented 8 years ago

I think in your particular case the answer should be "/{?q*}" (see http://tools.ietf.org/html/rfc6570#section-3.2.8) - which would be matched for 0 or more query string parameters. However... The matching support in Arachne.Uri.Template isn't quite complete, and Query and Query Continuation matching doesn't currently work. It will shortly!

In terms of matching a path then any tree - that again is possible with URI Template syntax - so for example "/root/more{/remaining}{?q*}" would capture any paths below/root/more` (as a list of strings) and also any optional query params (see http://tools.ietf.org/html/rfc6570#section-3.2.6).

URI Templates are pretty powerful, and they need more documentation and examples in Freya - plus also the support completed in Arachne, which is near the top of my list...

kolektiv commented 8 years ago

In terms of the catch all rule for query string values - hmmm. It's difficult to avoid because URI Templates are very specific. It might be something which we could add implicitly, but I would then worry about edge cases when people want a different handler for a route with a query, and a route without (not that uncommon I think - collection vs. search for example).

mexx commented 8 years ago

Currently at least I would have to include the catch all rule to all the templates, as I want to be liberate about what the client of my web api can use as URI, it means I have to allow any query string key I don't use in my application.

I think it's more about how we do the matching to the template. When the matching logic ignores all the query string keys that are not defined in the template explicitly, one wouldn't need the catch all rule. The template for the same path but with more query string keys defined should have more precedence, or the definition order should define the precedence, just like for match cases.

kolektiv commented 8 years ago

Yeah, fair enough, slightly awkward as it stands perhaps. To some extent that's "just one of those things" when basing a router on URI Template specifications (bound by the RFC to some extent). The thing with templates is that they just don't really ignore things - they're a relatively strict standard. I am definitely open to potentially coming up with some options/modifiers in terms of matching strictness though, as I can see some uses.

In terms of precedence, the current precedence is "first template that matches" in the order defined in the router. I'm not sure going much more complex than that will leave it as understandable - at least before we have some nice tooling around it.

I think getting the URI Template support fully working and then seeing how it goes - and where the pain points are once we've got all of URI Templates to play with - is probably the best plan. I'll take a look at that support over the next couple of days. I'm going to push the Arachne 3.0 release back a week to try and get that in, and add it to the milestone.