Use Return::MultiLevel so forward & redirect can immediately return from current hook/route code. #485

Closed
wants to merge 8 commits into
from

Projects

None yet

4 participants

Owner

Implements the "specification" for forward from #432; calling forward ceases execution of the current dispatch and returns the response from dispatching a modified request.

redirect uses the same mechanism to immediately return from hook/route code to avoid halting the response, resolving #432.

Adds tests for after hook being called (once and once only) from a route, forward and redirect.

@ambs - apologies again for the largish pull request!

veryrusty added some commits Sep 27, 2013
@veryrusty veryrusty Refactor Dispatcher->dispatch (no functional change)
Move the code for calling before hooks and the route handler to its own (private)
method. This will get wrapped by Return::MultiLevel to clean up forward/redirect
DSL methods.
ee58580
@veryrusty veryrusty Dispatcher - no need to add the request into the context for every app 0c6e92d
@veryrusty veryrusty Use Return::MultiLevel to wrap route dispatch.
As suggested in #432, use Return::MultiLevel to wrap route dispatch.
Cache the multilevel return coderef in the context for forward/redirect
to use.

Add Return::MultiLevel to prereqs and Scope::Upper to recommends.
a375426
@veryrusty veryrusty Test that after hook is called once after redirect and forward 86f6eba
@veryrusty veryrusty Do not halt the response on redirect.
Use the with_return handler to return immediatly to the dispatcher after the
redirect details have been added to the response.

Resolves #432.
205a13f
@veryrusty veryrusty Use with_return handler to return control back to the dispatcher afte…
…r forward.

This is the implementation of "... after a forward is executed, any remaining
code (route and hooks) from the current dispatch will never be run." from #432.
ee62c6a
@veryrusty veryrusty Update documentation for forward and redirect
Include the description of forward and update that forward immediately
returns form the current route/hook. Update redirect docs to reflect
the immediate return from route/hook.
d2679cb
Owner
sukria commented Oct 1, 2013

Looks very nice. Thanks for the PR.

Any other core dev willing to upvote?

👍

Owner
xsawyerx commented Oct 1, 2013

I like this. 👍 @veryrusty!

One note is that it optionally uses Scope::Upper, so we need to indicate that for speed, one should have that installed too, but not to require it (and make sure it's not required), because it's XS and that would break one of our assumptions: Dancer 2 is fatpackable by default.

Owner

@xsawyerx - Should the documentation of all the recommended XS modules "for speed" be raised a a separate issue? Currently only Dancer2::Core::Request notes the two XS modules it uses (from a quick grep of the source), but there are several others listed as Prereqs/Recommends in dist.ini. (There is also no mention of fatpacking either.. I'll stop before I volunteer myself ;) )

Note that @shadowcat-mst commented in #432:

Note that Return::MultiLevel is optional-XS only so this wouldn't compromise
deployability - I've written code with it before now that needed to fatpack.

I've verified the test suite passes without Scope::Upper installed; though I'd take mst's experience over my quick check any day!

Owner
racke commented Oct 5, 2013

On 10/05/2013 10:19 AM, Russell Jenkins wrote:

@xsawyerx - Should the documentation of all the recommended XS modules "for speed" be raised a a separate issue?
Currently only Dancer2::Core::Request notes the two XS modules it uses (from a quick grep of the source), but there are several others listed as Prereqs/Recommends in dist.ini. (There is also no mention of fatpacking either.. I'll stop before I volunteer myself ;) )

This should definitely be documented, both the XS recommends and the fact that Dancer2 must be fatpackable.

Regards
Racke

Perl and Dancer Development

Visit our Open Source conference on E-commerce:

http://www.ecommerce-innovation.com/

Owner
xsawyerx commented Oct 7, 2013

+1 to what @racke said.

@veryrusty veryrusty added a commit to veryrusty/Dancer2 that referenced this pull request Oct 17, 2013
@veryrusty veryrusty Rework halt to use with_return from Return::MultiLevel.
Once #485 is merged, use the with_return handler to immediately stop processing
a before hook or route handler. Includes tests for use in a reoute handler and a
before hook.

Resolves #472.
fec5329
Owner
sukria commented Nov 4, 2013

Thanks a lot @veryrusty, patch merged. That's good stuff ;)

@sukria sukria closed this Nov 4, 2013
@veryrusty veryrusty added a commit to veryrusty/Dancer2 that referenced this pull request Nov 5, 2013
@veryrusty veryrusty Rework halt to use with_return from Return::MultiLevel.
Once #485 is merged, use the with_return handler to immediately stop processing
a before hook or route handler. Includes tests for use in a reoute handler and a
before hook.

Resolves #472.
19e0b92
@xsawyerx xsawyerx added a commit that referenced this pull request Dec 7, 2013
@veryrusty @xsawyerx veryrusty + xsawyerx Rework halt to use with_return from Return::MultiLevel.
Once #485 is merged, use the with_return handler to immediately stop processing
a before hook or route handler. Includes tests for use in a reoute handler and a
before hook.

Resolves #472.
fa28401
@veryrusty veryrusty deleted the veryrusty:Pr/return_multilevel branch Aug 15, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment