config[:strip_trailing_slash] defined on nested controller doesn't have effect
Closed this issue · 5 comments
Because config[:strip_trailing_slash] handling takes place before finding a matched route it only has effect in the outer controller. I think the correct behaviour would be to allow inner controller to redefine trailing slash handing.
The :strip_trailing_slash option has three settings. You can set it to :ignore the trailing slash when matching routes, :redirect the client to the same URL but with the trailing slash removed, or if set to some other value (e.g. false), it'll do nothing special and simply treat a trailing forward slash as it would any other trailing character.
It's only the :redirect option that if set, cannot effectively be overridden by sub-controllers, as like you said, it takes effect in the outermost controller that has :strip_trailing_slash set to :redirect. This is for efficiency sake, as if you can imagine an application 3 or 4 sub-controllers deep, if you don't do the redirection as soon as possible (in the outermost controller), you're potentially doing a whole lot more processing, such as running before filters and extra middleware on those sub-controllers before doing the redirection, which is effectively just a waste of resources.
I'm curious to know what your use case is. I imagine if you had a sub-controller that had a ** capture, you may not want the trailing slash to be split, but I'm not really sure of a scenario where the presence of a trailing slash (or lack there of) in some variable would change a logical outcome.
... This is for efficiency sake, as if you can imagine an application 3 or 4 sub-controllers deep, if you don't do the redirection as soon as possible (in the outermost controller), you're potentially doing a whole lot more processing, such as running before filters and extra middleware on those sub-controllers before doing the redirection, which is effectively just a waste of resources.
Right, but this is still confusing. I suggest moving :strip_trailing_slash out of config and place as a separate controller attribute, as config being an instance of Scorched::Options has semantics for being overridable attached to it. Also maybe changing default to :ignore — I'm not sure if :redirect is the right default behaviour for trailing slash?
I'm curious to know what your use case is. I imagine if you had a sub-controller that had a ** capture, you may not want the trailing slash to be split...
I have a nested controller which should ignore trailing slash but that makes it depend on how I configured outer controller.
Btw. I've extended Scorched::Controller with a shortcut to mount other controllers on specified prefix and with a routing by URI templates (see gist https://gist.github.com/andreypopp/6069936).
...but I'm not really sure of a scenario where the presence of a trailing slash (or lack there of) in some variable would change a logical outcome.
Well for me it does change — I have a route where a parameter with ** represents a path in the git repository and I don't want to deal with empty paths.
Right, but this is still confusing. I suggest moving
:strip_trailing_slashout ofconfigand place as a separate controller attribute, asconfigbeing an instance ofScorched::Optionshas semantics for being overridable attached to it. Also maybe changing default to:ignore— I'm not sure if:redirectis the right default behaviour for trailing slash?
But it is inheritable and overridable. It's only when :redirect is in effect on an outer controller does any potential problem arise. :ignore and false behave exactly as expected. You also need to keep in mind that one may set such an option in a Base controller that itself is never invoked, but is just used for configuration inheritance and the like. In this instance, :redirect is inheritable and overridable without any side-effects.
As for the best default, :redirect was chosen as it's bad form to have identical content available at multiple URL's. You want to have a single canonical URL, not http://example.com/about and http://example.com/about/ and http://www.example.com/about, etc, all pointing to the exact same content. It's therefore the best/safest default in my opinion.
As a work-around, you may set :strip_trailing_slash to :ignore and implement your own before filter or middleware for handling trailing slash redirections which doesn't affect your sub-controller. There's many ways to skin this cat (or animal of your choosing).
Well for me it does change — I have a route where a parameter with
**represents a path in the git repository and I don't want to deal with empty paths.
I don't know of any file system or OS that doesn't treat "/some/dir" and "/some/dir/" in the same way. Are you sure the trailing slash is a requirement?
Ok, I think this issue should be closed — I extended Scorched to the extent I can't argue about this. But I still think that :strip_trailing_slash overriding behaviour should be documented at least.
Yeah, probably worth documenting. I'll make a mention of it.
Cheers,
Tom