PHP-based routing configuration also needs to be patched.

@nicolas-grekas we did not add the controller feature on the PHP loader, so I guessed this was on purpose. I'm not really sure it's a lot valuable to do so.. what do you think?

I'm talking about https://github.com/symfony/symfony/blob/master/src/Symfony/Component/Routing/Loader/Configurator/Traits/RouteTrait.php#L121

Right, so you mean we should add a redirect method in the RouteTrait?

As stated in #24640 (comment) and below, I would do it differently: instead of setting a controller, the redirect_to entry could just set a new entry in "defaults", named eg "_redirect_to" instead of "_controller". Then it'd be up to high level layers to act upon it.

@nicolas-grekas definitely agree, see my proposed options here.

The real question now is: who is going to handle the _redirect (or two _redirects) attributes? HttpKernel or FrameworkBundle? I'd argue for HttpKernel so it can benefit other libraries only using the kernel. If it's HttpKernel, we need to move the logic from FrameworkBundle's RedirectController to the HttpKernel component.

Adding one more question we need to answer:
what about the "permanent" flag, and the "ignoreAttributes" setting?

what about the "permanent" flag, and the "ignoreAttributes" setting?

I'd say redirect_permanent and redirect_ignore_attributes configurations (and their XML equivalents with -s)

@stof @nicolas-grekas I've updated the PR to use _redirect_to and _redirect_permanent attributes populated by the loaders in Routing. A new listener in FrameworkBundle adds the _controller on kernel.request when it sees the _redirect_* attributes in the Request.

@sroze what would be the new way of doing this configuration in yaml? Can we do without a listener (yet another one for this looks like overhead to me...)

what would be the new way of doing this configuration in yaml?

The one in the PR description :)

Can we do without a listener (yet another one for this looks like overhead to me...)

Well... we could. Do you want me to merge with the existing one or doing something else? 🤔

AppVeyor's failure is not related. It's a network issue. Can somebody restart it?

ping @Tobion :)

Now that I know the router a bit better, I'd suggest leveraging RedirectableUrlMatcherInterface::redirect() instead. This also means I'd suggest to remove the permanent/non-permanent flag. Let's make this very simple, and make it a core feature on the Routing component only, since is has everything needed to deal with that standalone.

I'd suggest leveraging RedirectableUrlMatcherInterface::redirect() instead.

Can you guide me through what you have in mind a little bit more? Maybe one way would be to roughly explain how you'd change this pull-request?

when a _redirect entry is found in the matched route, a redirectable matcher could internally
return $this->redirect($ret['_redirect'], $ret['_route']);

I've been thinking about this proposal ... and I propose to make a minor change. Instead of using redirect_to for redirecting both URLs and routes, we could be more explicit and define redirect_to and redirect_to_route (this is also consistent with other Symfony parts, where "redirect" is for URLs and routes have a special redirect method). In practice:

# redirect_to for absolute or relative URLs
blog_show:
    path: /blog/{slug}
    redirect_to: /some-relative-url
    permanent: true

# redirect_to_route without route arguments
blog_show:
    path: /blog/{slug}
    redirect_to_route: 'some-other-route'
    # this also works, to be consistent with the next case:
    # redirect_to_route: ['some-other-route']

# redirect_to_route with route arguments
blog_show:
    path: /blog/{slug}
    redirect_to_route: ['some-other-route', { arg1: 'value1', arg2: 'value2' }]

@sroze I know you are very busy and you also have a much more important PR related to the new Message component ... but it'd be great if you could finish this feature before the Symfony 4.1 "feature freeze" starts in 12 days. Thanks!

@javiereguiluz yep, agree with you, it would be nice. I'll update the PR this week :)

I've explored the option of moving this logic directly within the router, especially within the PhpMatcherDumper and it worked for the redirect idea. It did not work at all for the redirect_to_route idea because it would require a UrlGeneratorInterface while the dumper matcher is... just a matcher. One way to tackle this would be to have a sort of mini-generator while dumping the matcher but this 1) would duplicate some logic and 2) won't support things like route parameters. This is therefore not a satisfying approach.

The problem with the current implementation is that is it specific to a few details about the routing and is sort of between the Routing component and the FrameworkBundle bundle. Redirection configuration (while allowed by the Routing component) wouldn't work without the FrameworkBundle, which is not what we'd like.

While discussing all of that with @nicolas-grekas, we jumped back to actually... what we are trying to solve: simplifying the YAML configuration. It might be that the only issue, for now, is the Symfony\Bundle\FrameworkBundle\Controller\RedirectController::urlRedirectAction FQCN that we always have to copy/paste. Nico's idea, which might make sense is that instead of redirect-specific configuration, we simply add controller aliases. Therefore, the redirection configuration could look like that:

redirect_to_admin:
    path:     /
    defaults:
        _controller: _redirect_path
        path: /admin
        permanent: true

What do you all think?

I haven't looked at the internals ... but from the outside this looks "just" a syntactic sugar that should be "easy" to implement. So, the end-user writes this in YAML:

blog_show:
    path:        /blog/{slug}
    redirect_to: /some-relative-url
    permanent:   true

And Symfony, when parsing that YAML file, transforms it to this transparently:

blog_show:
    path:     /blog/{slug}
    defaults:
        _controller: Symfony\Bundle\FrameworkBundle\Controller\RedirectController::urlRedirectAction
        path: /some-relative-url
        permanent: true

So, this feature "just" needs to look for a redirect_to option and then, create the defaults._controller and defaults.path options automatically ... and let Symfony take care of the rest. No more changes would be needed because the verbose config already works: this feature "just" transforms concise config to verbose config internally. Would that work?

@javiereguiluz the real problem is that:

The problem with the current implementation is that is it specific to a few details about the routing and is sort of between the Routing component and the FrameworkBundle bundle. Redirection configuration (while allowed by the Routing component) wouldn't work without the FrameworkBundle, which is not what we'd like.

@sroze this is how I imagine the implementation of this feature (the code doesn't work, I just invented it to show what I'm asking for):

public function processRoutes($yamlContent)
{
    $routesConfig = Yaml::parse($yamlContent);
    foreach ($routesConfig as $routeConfig) {
+        if (isset($routeConfig['redirect_to'])) {
+           $routeConfig['_defaults']['_controller'] = 'Symfony\Bundle\FrameworkBundle\Controller\RedirectController::urlRedirectAction';
+           $routeConfig['_defaults']['path'] = $routeConfig['redirect_to'];
+           unset($routeConfig['redirect_to']);
+       }

        // ...
    }

    return $routeCollection;
}

Yes, I agree with you. But it would mean the Routing component depends on the FrameworkBundle (at least, the way it is now).

I'm now convinced the aliasing feature is all that is needed at least as a first step. This should be a feature of RouterListener in HttpKernel IMHO (could also be in Routing matchers, but I feel better with RouterListener). So down this path, we would just need a RouterListener::setControllerAliases(array $aliases) and done.

Javier has another goal, which is to move defaults out of the "defaults" nesting level.
This goal is unrelated to redirection to me. Or, if it's for redirections only, then I'm very unsure, as this would just create a new specific case that ppl will need to learn.

All-in-all, the controller aliasing looks like the right next step to me.

To better compare the alternatives, here is a summary:

Current situation

blog_show:
    path: /blog/{slug}
    defaults:
        _controller: Symfony\Bundle\FrameworkBundle\Controller\RedirectController::urlRedirectAction
        path: /some-relative-url
        permanent: true

Proposal 1

blog_show:
    path: /blog/{slug}
    redirect_to: /some-relative-url
    permanent: true

Proposal 2

blog_show:
    path: /blog/{slug}
    defaults:
        _controller: _redirect_path
        path: /some-relative-url
        permanent: true

I think it's OK to create a redirect_to option because we recently moved the defaults._controller option to just controller and people loved it.

I'm 👎 for adding redirect specific root-options because it creates a very specific case people will have to learn, whereas the proposed alternative (2) just leverages existing and new generic knowledge. The redirect-specific entries would also prevent using new or exotic redirection options, because these are specific to the actual redirector implementation.

The only counter-argument to @nicolas-grekas' PoV I can see is that a redirection is not a "very specific case" of routing.

From a user PoV, I like the redirect_to option from Javier. Looks neat to me. That does not work well as it would not allow for specific HTTP status code, ...

Let's close this one, as this direction is a dead end :)