Add RouterGenerateEvent to provide option to manipulate routes

[benglass]

Q	A
Bug fix?	no
New feature?	yes
BC breaks?	no
Deprecations?	no
Tests pass?	???
License	MIT
Doc PR	https://github.com/symfony-cmf/symfony-cmf-docs/pull/672

DynamicRouter dispatches RouterMatch and RouterMatchRequest events when match and matchRequest are called but does not dispatch an event when generate is called.

The specific use case that we might use something like this for in our application is supporting using some kind of special syntax for loading PHPCR pages based on a unique immutable non-client editable slug field. For example a page with a slug field value of 'privacy-policy' that is not editable by clients. We would be able to do something like this in twig

{{ path('@privacy-policy') }}

And instead of having to define a custom router router chain we could simply respond to this pre-generate event to change the route name from '@privacy-policy' to the real uuid or path to the PHPCR document.

I know there is already some work in the resource bundles by @dantleech regarding having different ways to load documents based on something other than phpcr id or uuid. It feels like if there are match/match request events that a generate event makes sense and there are probably other uses I am not thinking of.

This relates to https://github.com/symfony-cmf/RoutingBundle/issues/178 in a broad sense since it adds an extension point that might be usable for this purpose.

@dbu @dantleech thoughts?

[dantleech]

The Resource component might provide a way to do this, but infact probably not the best way.

An event could make sense. Another option, and something that I miss, is the possiblity of adding only a URL generator to the routing chain instead of having to implement the whole Router interface.

Would having a dedicated UrlGenerator for this be a cleaner solution?

e.g.

class AliasUrlGenerator implement UrlGeneratorInterface
{
    public function supports($name, $parameters)
    {
        return substr($name, 0, 1) === '@';
    }

    public function generate($name, $parameters)
    {
        $this->something->getRouteByAlias($name);

        return ...;
    }
}

[dbu]

i agree both with the idea and how you implemented the change. makes sense to have both. i commented on a few details. for more complex scenarios, the Resource things will make more sense, but at least the event in itself i have no doubt that it makes sense.

can you please open a documentation PR as well, for the event?

regarding a custom generator: we already inject the UrlGenerator to the DynamicRouter. but in the bundle, we should allow to configure what generator is used, rather than hardcode cmf_routing.generator. anything more seems like overkill - you can always just do a RouterInterface with an empty match() method if you need additional generators.

[dbu]

hm, and it looks like composer on travis is hitting a memory limit reproducably, restarted the two failing ones already :-( that is not related to this change, i guess we need to investigate separately.

[dantleech]

But this use case sounds like it could be best solved by a UrlGenerator.

We pass @something to the UrlGenerator, and it should return the URL, do we need events?

[dbu]

the event is rewriting (or injecting) parameters or the route name, not generating the url. to me this makes sense. there is of course the confusion point that this event does not trigger for standard symfony routes, but this is the same with the match.

[benglass]

@dbu @dantleech i believe I addressed your comments

I dont disagree that a url generator is another way to handle this but this solution might be simpler/less code for that use case. A url generator would have to wrap the real url generator as not to duplicate the actual url generation logic. Its not actually the url generation that is the problem with the other generator its the ability to translate the provided route name '@slug' into a Route object so I think its more of a provider issue. In ur current implementation without this change we actually wrap the Phpcr provider in a SlugAwareProvider that looks for the @slug format.

[dbu]

coming good. i noticed some details.

can you please also start with the doc pull request on symfony-cmf-docs?

[benglass]

@dbu @dantleech waiting on travis but I believe I have handled all of the questions/comments and documentation PR is open.

[benglass]

Regarding the fact that the use case I mentioned is a little strange to use this feature for, I think it would make more sense for code that takes @slug and transforms it to /cms/main/slug/path to live in the Provider but overriding the provider to do this feels like overkill. Perhaps we could have a way to hook into the logic the dynamic provider uses to load a route based on name? Defining an entirely new router for this seems like overkill since url generation doesnt change at all its just loading of the route (which happens in the provider). Some thoughts around that...

• Event in route provider that lets you modify the route (like what we are doing here.. seems unnecessary with this code here but its a little more direct)
• Chain route provider
• Route provider has transformers for route that are applied before the route is loaded

This starts to get into the area that resource bundle is trying to solve. This code in this PR I feel should still be merged because there are already events for match so not having one for generate seems like a missing feature.

One thing I think we are finding is that in userland that declaring a new router makes sense if you have a whole new source for routes. For example we have a 'url redirect router' that stores redirects created by the client and comes last in the stack and can both generate and match urls. But for a case where you want a shortcut or alternate way of linking to a resource defining a whole new router doesnt feel right and often providing a completely new url generator feels like overkill because its not the url generation logic that is different it is the location of the route.

Just a bunch of my unorganized thoughts after a day of snowboarding :)

[dantleech]

Maybe yet another option would be to have a 100% event based DynamicRouter which only has a dependency on the EventDispatcher. Then the URL generator and RouteProvider would just be listeners/subscribers.

[dbu]

yeah, the 100% event based router might be the thing to go for 2.0. right now we try to reach all that flexibility by building more and more complex nested things. the route provider actually further delegates to a candidates strategy, which again could handle the job of resolving alias to path...

but i will merge this event, it makes sense to me. currently adding some tests, cleaning up absolute to referenceType (as symfony does not call it absolute anymore, there are 4 different types now).

[dbu]

continued in #132