Making around_dispatch hook can call next->() multiple time

[jamadam]

Hi.

I revamped Mojolicious::Plugins::emit_chain to make it possible to re-
try
next->() again and again.

https://github.com/jamadam/mojo/commit/a71fe812e76e1cee2b6eb2e7b5184411ee545a07

The following is an example for multiple call of next->().

# Wrap whole application for redirecting 404 to /
hook around_dispatch => sub {
my ($next, $self) = @_;

# pre-processing here

$next->();

# post-processing here
if ($self->tx->res->code == 404) {

# Initialize everything (maybe this part must be more careful)
$self->{stash} = {};
$self->tx->res(Mojo::Message::Response->new);

$self->req->url->path('/');
$next->(-1); # seeking last one
}
};

I wish this for re-implementing my plugin
Mojolicious::Plugin::PlackMiddleware.
I don't know anyone except me uses this plugin, though.

There may be many way of hack to achieve this.
Do you think the patch above seems to be sensible?

[Sebastian Riedel]

> I wish this for re-implementing my plugin
> Mojolicious::Plugin::PlackMiddleware.
> I don't know anyone except me uses this plugin, though.
>
> There may be many way of hack to achieve this.
> Do you think the patch above seems to be sensible?

I don't understand why you need that change, what's the real world use case? Does the api really have to be that ugly?

--
Sebastian Riedel
http://twitter.com/kraih
http://mojolicio.us

[sri]

Your timing is also pretty bad, i'm a little opposed to making rushed
changes to emit_chain just two days after it has become stable. It was
experimental for 3 months before that after all.

[sri]

> I don't understand why you need that change, what's the real world use case? Does the api really have to be that ugly?

The test case in your patch appears to be entirely artificial, nobody
would ever use something like that in a real application. Why would
anyone ever jump around the chain?

[jamadam]

Thanks for touching a notion.

It was just a casual and personal proposal
and just wished if the demand would meets someone else's.
That's why I rasied this topic instead of sending pull request.

Following is practical example. This is a adapter to wrap
the Mojolicious dispatcher with Plack middleware.

https://github.com/jamadam/Mojolicious-Plugin-PlackMiddleware/blob/prototype/lib/Mojolicious/Plugin/PlackMiddleware.pm

Plack::Middleware::ErrorDocument calls plack app twice
inside of it. On emulating this, the plugin needs to call
$next->() twice.

https://github.com/miyagawa/Plack/blob/master/lib/Plack/Middleware/ErrorDocument.pm

However, in the first place I'm suspecting the plugin's significance
of existence. Do you think the idea of the plugin OK?

[sri]

> Plack::Middleware::ErrorDocument calls plack app twice
> inside of it. On emulating this, the plugin needs to call
> $next->() twice.

As someone on IRC just said, you basically want internal redirects,
doesn't that work with $app->handler() too? I've also noticed your
plugin uses stah values with mojo. prefix, those are considered
internal and can change at any time.

--
sebastian

[jamadam]

Thanks for considering this on IRC. I check the conversation on IRC
log.
I'm hesitating to join to IRC with my poor English.

As you said on IRC, I want $next->() execution without jumping
forward inside each hook. I don't want the API to be changed if it's
escapable.

I have no idea for now whether it's possible or not to implement
my plugin with $app->handler. I should study that layer more.
Maybe I've sticked too much around hooks.

When it comes to stash usage, I undersand that I shouldn't even
refer to mojo.* name space. My plugin refers to mojo.routed
and it's unsafe maybe in the futrue.
I'll look for another implementation.

Thank you!

[sri]

> I have no idea for now whether it's possible or not to implement
> my plugin with $app->handler. I should study that layer more.
> Maybe I've sticked too much around hooks.

Regarding internal redirects, i could totally see something like this
work much better than emit_chain hacks.

hook around_dispatch = sub {
my ($next, $c) = @_;

# Continue chain
$next->();

# No new target, but you get the idea
$c->app->handler($c) if $c->stash('my.redirect');
};

There are many ways to make this stuff work, that's why real world use
cases we can all relate to are so important.

--
sebastian

[jamadam]

Thank you for the example. That was very helpful.
I'll try to re-implement with app->handler().

I'll also consider to modify emit_chain for my brain excercise.
Since the current architecture is too genius, it may be hard to touch
in, though.

[jamadam]

By the way, I searched real time use cases in Plack::Middleware::*
and found only kind of internal redirects.

Plack::Middleware::ErrorDocument
Plack::Middleware::Recursive
Plack::Middleware::Precompressed

P::M::Precompressed is little different but basically same.

[jamadam]

I refactored my plugin with app->handler->().
It needs more tests but looks good for now.

I also tried to modify emit_chain for multiple $next->() call
without API change. It's for my self-satisfaction.

https://github.com/jamadam/mojo/commit/990d84d1c0e3ae39da76a8ec1d7d14a20364ce3d

I don't ask you to merge it for now. I'll think twice about real use
case.

Thank you!

[sri]

> I don't ask you to merge it for now. I'll think twice about real use
> case.

Turns out your proposed change requires even less code than the old
version, so i've committed it for now. I'm still not happy with the
lack of real world use cases, so it stays undocumented and might even
vanish again if we don't find something better than internal
redirects.

https://github.com/kraih/mojo/commit/72e3bbce1c0c0336064fd4445ed67a1918391591

--
sebastian

[jamadam]

Oh, your patch is so compact!
It's also educational to me on how clousure can be implemented.
Thanks a lot for spending time for it.