Separate routing from dispatching

[danizord]

I have a middleware that needs a matched route param to perform some pre-dispatching logic, therefore this middleware should run after routing and before the "action" middleware.

Currently this can be achieved by injecting the router instance into a pre-routing middleware and calling $router->match($request) just to get the matched route params, but I think this is a very common use case and could be achieved more elegantly with a middleware registered between routing and dispatching.

It's possible to break the Application#routeMiddleware() into 2 middlewares?

[RalfEggert]

Have you looked at the RouteResultObservers?

http://zend-expressive.readthedocs.org/en/latest/router/result-observers/

[RalfEggert]

Here is a recipe that uses it:

http://zend-expressive.readthedocs.org/en/latest/cookbook/setting-locale-depending-routing-parameter/

[danizord]

@RalfEggert RouteResultObserver will not work for me because I need access to the ServerRequestInterface instance. =\

Also, RouteResultObserver does not allow us to stop middleware pipeline by returning the Response instance without calling $next.

[harikt]

I think it would be nice to have events built in the core. May be that will help ?

[weierophinney]

@harikt — The only place events are needed are between routing and dispatching, which is something we've attempted to solve with the route result observers. I have no desire to add any events at this time, as you can typically solve the same problems by layering middleware.

@danizord — Are you aware that you can add a list of middleware to dispatch when defining a route? As an example:

[
    'name' => 'guarded',
    'route' => '/api/resource[/{id:[a-f0-9]{8}}]',
    'allowed_methods' => ['GET', 'POST'],
    'middleware' => [
        ValidateIdentifierMiddleware::class,
        ResourceMiddleware::class,
    ],
]

In the above example, if the route is matched, Expressive will create a MiddlewarePipe with the various middlewares listed. As a result, your ValidateIdentifierMiddleware could verify the id provided in the route result to determine if it's valid, use the server request, etc., and return a response if something is wrong; if all is well, it would call $next(), and delegation would pass on to the ResourceMiddleware.

If that won't work, perhaps you can explain the workflow you want to achieve in more detail?

[nesl247]

I also need this functionality. While your solution works @weierophinney, the problem is now I have to register the same middleware on every route.

[weierophinney]

@nesl247 There are ways to automate that.

If you're using a factory for your middleware already, you can modify the factory to return an array with both middleware instances. As an example, instead of this:

return new SomeMiddleware($container->get(Foo::class));

you'd instead do this:

return [
    $container->get(AuthenticationMiddleware::class),
    new SomeMiddleware($container->get(Foo::class)),
];

Another approach is to use delegator factories (zend-servicemanager) or extension (Pimple). In the case of delegator factories, you'd create a delegator factory as follows:

use Zend\ServiceManager\DelegatorFactoryInterface;
use Zend\ServiceManager\ServiceLocatorInterface;

class AuthenticationMiddlewareDelegator implements DelegatorFactoryInterface
{
    public function createDelegatorForName(ServiceLocatorInterface $container, $name, $requestedName, $callback)
    {
        return [
            $container->get(AuthenticationMiddleware::class),
            $callback(),
        ];
    }
}

Then, for any service that needs the Authentication middleware, you add a delegator definition in your configuration:

return [
    'dependencies' => [
        'delegator_factories' => [
            SomeMiddleware::class => [ AuthenticationMiddlewareDelegator::class ],
            OtherMiddleware::class => [ AuthenticationMiddlewareDelegator::class ],
        ],
    /* ... */
];

With Pimple, you use the extend() method:

$pimple->extend(SomeMiddleware::class, function ($instance, $container) {
    return [
        $container->get(AuthenticationMiddleware::class),
        $instance
    ];
});

Unfortunately, the above is not something we support via configuration yet; I'm considering adding some features for that in 1.1. However, it can be done in your config/container.php class, before returning the container instance.

My point is that this can be done in automated fashion. I'd even argue that using delegator factories/pimple extension is a cleaner way of doing it as it allows re-use, and makes explicit how instance creation works.

[nesl247]

I'm not quote sure I'm following there. Seems like a lot of wasted effort for something that should be easily done like all other middleware.

I'd honestly expect the pipeline to be more like pre_routing, on_route_match, post_routing, or whatever the names would be exactly. I was kinda shocked it wasn't like this already.

The annoyance is I need the route name for middleware that is executed on every single request. I actually have several of these already. For now, I'm just gonna use the easy route of registering the middleware a bunch of times.

[weierophinney]

Can you explain the use case? I'd like to understand what you're trying to accomplish, so I can determine if new features are required, or if I can provide a solution that works with the current feature set. On Jan 10, 2016 12:13 PM, "Harrison Heck" notifications@github.com wrote:

I'm not quote sure I'm following there. Seems like a lot of wasted effort for something that should be easily done like all other middleware.

I'd honestly expect the pipeline to be more like pre_routing, on_route_match, post_routing, or whatever the names would be exactly. I was kinda shocked it wasn't like this already.

The annoyance is I need the route name for middleware that is executed on every single request. I actually have several of these already. For now, I'm just gonna use the easy route of registering the middleware a bunch of times.

— Reply to this email directly or view it on GitHub https://github.com/zendframework/zend-expressive/issues/259#issuecomment-170376623 .

[nesl247]

While I'm sure everyone is going to have different use cases, and I'm sure having the route configuration for middleware is something that would be commonly used, mine is for logging, validation, and binding. I create classes based on the route name and they are automatically used by my middleware.

[weierophinney]

@danizord and/or @nesl247 — would passing the request and/or response to the route result observers solve your use cases?

In terms of short-circuiting, you can do that already from route result observers: throw an exception, and create corresponding error middleware that would return a response based on that exception type. I would consider short-circuiting from route result observers an exceptional case, which is why I suggest that approach. You could even have a special exception type that wraps the response you want to return, and the error handler middleware could simply extract and return that.

To re-iterate: would passing the request and/or response to the route result observers solve these use cases? If so, I'll work up a patch. It won't be in time for 1.0, but we can add to a 1.1 version to release very soon after the 1.0 version.

[nesl247]

In my opinion, it is not an acceptable solution. By doing that, we are breaking out of the already defined pattern of using middleware, and replacing it with observers for certain functionality. Maybe I'm being a stick in the mud here or something, but it seems odd to split this functionality out.

[weierophinney]

@nesl247 Your observer would use middleware in this case.

Alternately, we go down the road of having the routing middleware separate. This is possible in a later release as it's currently an internal implementation detail. However, there were a number of design considerations we had that led to keeping it internal to the Application instance that we'll have to resolve before doing so. As such, this has to be a 1.1 or later change if we decide to implement it.

[danizord]

Can you explain the use case? I'd like to understand what you're trying to accomplish, so I can determine if new features are required, or if I can provide a solution that works with the current feature set.

Most authorization middlewares would need data from both request attributes and route result:

function ($req, $res, $next) {
    $routeResult = $req->getAttribute(RouteResult::class);
    // Yes I can fetch route attributes from $req but should happen after routing anyway
    $documentId  = $routeResult->getMatchedParams()['documentId'];
    $document    = $this->documentService->findDocumentById($documentId);

    if ($document->getOwnerId() ==! $request->getAttribute('userId')) {
        // You're not allowed to access this document!
    }
}

@weierophinney Having middlewares piped after router middleware and before routed middleware is way better than "route result observers" IMHO, because it's much more easier to understand while it brings the advantages of middlewares (short-circuit, onion-style, request/response access).

I think if we split routing from dispatching the route result observers would become useless after all (@weierophinney can you confirm?).

Alternately, we go down the road of having the routing middleware separate

Yes I think this is the best road :+1: Tell me if I can help :)

[weierophinney]

@danizord and @nesl247 — please see #270, which attempts to address this. I still have a few things to work out (route(), for instance, injects the pipeline middleware still), but when complete, this approach will allow you to inject your own routing middleware.

I do not plan on splitting routing from dispatching at this time, as dispatching uses the results of routing; however, by making it possible for you to use your own routing middleware, you can choose your own workflow for this.

One note: right now, the routeMiddleware() relies on functionality that's internal to the Application class; I'll be separating that out to a trait, which will allow re-use in your own routing middleware.

[danizord]

@weierophinney

I do not plan on splitting routing from dispatching at this time, as dispatching uses the results of routing

Yeah, but you can fetch it from $request->getAttribute(RouteResult::class); - just like the way ZF2 works, which is great btw.

I'm not sure what's the advantage of doing routing and dispatching together as it only reduces flexibility and forces us to introduce other patterns like "route result observers" to do things that could be achieved with middlewares.

[weierophinney]

@danizord — On reflection, I like the idea of removing the route result observers. I'm going to see if I can make that work on #270, after I figure out a solution for ordering middleware.

[danizord]

@weierophinney nice! Let me know if I can help in something :D

[RalfEggert]

@weierophinney

How about priorities like we have in the event manager? Maybe that might the solve the issue when merging config files?

return [
    'middleware_pipeline' => [
        /* ... */ 
        100 =>  Helper\ServerUrlMiddleware::class,
        /* ... */ 
        200 =>  Helper\UrlHelperMiddleware::class,
        /* ... */ 
        500 => Zend\Expressive\Container\ApplicationFactory::ROUTING_MIDDLEWARE,
        /* ... */ 
        600 =>  My\Authentication\ServerMiddleware::class,
        /* ... */ 
        700 => Zend\Expressive\Container\ApplicationFactory::DISPATCHING_MIDDLEWARE,
        /* ... */ 
        800 =>  Just\Another\Logging\Middleware::class,
        /* ... */ 
    ],
]

Or will that be too simple?

[weierophinney]

@RalfEggert That's likely the route I'll be taking. :wink:

[weierophinney]

Fixed with #270.