zendframework / zend-router

Standalone routing implementation for HTTP and console requests
BSD 3-Clause "New" or "Revised" License
32 stars 20 forks source link

filterable routes #29

Closed chris-kruining closed 7 years ago

chris-kruining commented 7 years ago

Hey,

First of all, either I'm really stupid and did a bunch of work for nothing or this might be a useful request.

I am making a rest-api like application and wanted my routes to be filterable, as in 'allowed' based on a boolean condition. I know this is possible by listening to the dispatch event but I really wanted the config to be at the routes them self. Unless I missed a clue it is not possible to set extra data in the routes config. So I implemented my own router that extends this module.

To sum it up:

Link to my extension: zf3-filterableRouter

dstockto commented 7 years ago

@chris-kruining I'd avoid putting closures in your config files as it prevents them from being serializable and they cannot be cached. If you wanted to add a callable parameter you could do it in your Module's getConfig.

Alternatively, your callable could be a class name of an invokable class and your extension could retrieve it through the service locator and execute it. Then it would be all (almost) within the config, other than the actual executable code being contained in a class.

weierophinney commented 7 years ago

Each route allows you to add "defaults". Generally, these are used to provide default values for optional captured segments of your route, but they do not need to. Those values will be merged with any matched parameters in the route and returned in the RouteResult parameters. I would suggest using these for your use case — add a named flag you want to check for in a listener.

(We do similarly in Apigility, though we usually base it on the matched controller name.)

chris-kruining commented 7 years ago

@dstockto it is true that there should be no closure's in the config, I don't do that either as it is bad practice. but all I need to get the filter is a funtion with function(void) : bool as signature.

@weierophinney okay nice, but doesn't seem weird that an option that should allow or disallow a route's execution based on a default parameter? That why I choose to implement an extension :P. Could you possibly give a code snippet to show what you mean with a named flag? I have trouble imagining it :P

weierophinney commented 7 years ago

@chris-kruining You already use one in zend-mvc: "controller" :smile: :

'defaults' => [
    "controller" => Your\Controller::class,
],

After routing occurs, the dispatcher (which is an event listener) pulls that route match parameter to determine what controller to use and dispatch.

What I'm suggesting is really no different: you could do similarly with any other listener, using any route match parameter. As an example, I could block execution for a matched route if it requires authorization (as specified by a route default), and authorization fails:

// defaults include:
'defaults' => [
    /* ... */
    '__AUTHORIZATION_REQUIRED__' => true,
],

// Listener:
function (MvcEvent $e) {
    $routeMatch = $e->getRouteMatch();
    if (! $routeMatch || ! $routeMatch->getParam('__AUTHORIZATION_REQUIRED__', false)) {
        return;
    }

    if (! authorize($e->getRequest())) {
        // return a 401 response
        return $e->getResponse()->setStatusCode(401);
    }
}

By using a double-underscore-prefix with the name, you're flagging it's a "special" value. You then only use it internally with code that's interested in that parameter.

chris-kruining commented 7 years ago

@weierophinney Aaah I see, but I was kind of hoping to prevent the route from even triggering a dispatch and instead dispatch to an configured action (I now realize my extension is limited to 1 error action :S) instead.

thank you very much for taking the time to explain!

I do think I'll keep my extension as to me it seems more clean to define a filter to a route itself and not use the defaults for any other purpose than parameters ;)

would it be worth my time to write my extension as a pull request or do you simply not want this method of configuration?

weierophinney commented 7 years ago

I was kind of hoping to prevent the route from even triggering a dispatch and instead dispatch to an configured action (I now realize my extension is limited to 1 error action :S) instead.

That can still be done; have your listener be on the route event at a lower priority than the RouteListener, and have it swap an alternate RouteMatch instance into the MvcEvent; then when you hit the DispatchListener, that alternate RouteMatch will be used to determine what to dispatch.

Feel free to implement however you wish; I'm just providing an alternate approach that will work with what we have shipped today.

Most likely, we won't accept such an extension at this time; you could, however, ship it as a standalone Composer package for others to use with zend-router.

chris-kruining commented 7 years ago

That can still be done; have your listener be on the route event at a lower priority than the RouteListener, and have it swap an alternate RouteMatch instance into the MvcEvent; then when you hit the DispatchListener, that alternate RouteMatch will be used to determine what to dispatch.

this is in combination with the 'filter' key being in the defaults array?

because that method of configuration doesn't appeal to me :S I already provided my extension via composer and will keep it that way then.

Thanks for your help!!