FEATURE: Separate RouteConfiguration from Router

[sorenmalling]

This separates the Routes configuration from the router by introducing a RoutesProviderInterface which will be used by the router implementation together with a ConfigurationRoutesProvider that implements the current configuration from Routes.yaml.

Switching out the internal implementation of the RoutesProviderInterface can be done via Objects.yaml to add custom behaviour. But be aware that this is not covered by our api promises. All Implementations should include the routes provided by the ConfigurationRoutesProvider.

This change also makes sure, that the RouteCommandController uses the current RoutesProviderInterface implementation, instead of hard coded Flow router. That ensures that all Routes available to the router are now also visible to route cli-commands.

Fixes: #2948

Upgrade instructions

This change removes the methods getRoutes and addRoute from the Router that previously were mainly used in functional-tests as they were never part of the Router Interface.

To adjust for that the existing utility FunctionalTestCase->registerRoute method has to be used instead of FunctionalTestCase->router->addRoute.

The method Router::setRoutesConfiguration, which was also previously used for internal testing has been removed without official replacement. You could technically inject a custom routes provider to do so but be aware that this is internal behaviour.

Review instructions

Run the ./flow routing:list command - you will see the list as expected

Checklist

• Code follows the PSR-2 coding style
• Tests have been created, run and adjusted as needed
• Reviewer - PR Title is brief but complete and starts with FEATURE|TASK|BUGFIX
• Reviewer - The first section explains the change briefly for change-logs
• Reviewer - Breaking Changes are marked wit !!! and have upgrade-instructions

[bwaidelich]

I still like the direction, but I stumbled upon a major issue: This change removes the lazy loading & parsing of routes

(see #2970 (comment))

[bwaidelich]

@sorenmalling here's my suggestion briefly scratched out:

interface RoutesProvider
{
    public function getRoutes(): Routes;
}

final class ConfigurationRoutesProvider implements RoutesProvider
{
    private ?Routes $routes = null;

    public function __construct(
        private readonly ConfigurationManager $configurationManager
    ) {}

    public function getRoutes(): Routes
    {
        return $this->routes ??= Routes::fromConfiguration($this->configurationManager->getConfiguration(ConfigurationManager::CONFIGURATION_TYPE_ROUTES));
    }
}

final class Router implements RouterInterface
{
    public function __construct(
        private readonly RoutesProviderInterface $routesProvider,
    ) {}

    // ...
}

The extension point would be a custom implementation of the RoutesProviderInterface, e.g.:

final class MyRoutesProvider implements RoutesProvider
{
    private ?Routes $routes = null;

    public function __construct(
        private readonly ConfigurationRoutesProvider $configurationRoutesProvider,
    ) {}

    public function getRoutes(): Routes
    {
        if ($this->routes === null) {
            $this->routes = $this->appendCustomRoutes($this->configurationRoutesProvider->getRoutes());
        }
        return $this->>routes;
    }

    private function appendCustomRoutes(Routes $routes): Routes
    {
         // load routes from annotations etc.
    }
}

[sorenmalling]

The extension point would be a custom implementation of the RoutesProviderInterface, e.g.:

How about we put the extension point into the RoutesLoader class?

Routes is a configuration, the Routes have been loaded, now extend that - we avoid having a new Provider concept, that requires overriding (which will lead to dropping the Yaml configurated routes, right?)

[sorenmalling]

@bwaidelich @mhsdesign Tried my best to implement suggestion and to address the injection issue. Will you re-review that concept of fromConfiguration implemented now?

[mhsdesign]

wow thank you so much @mficzel ;)

[mhsdesign]

I solved all open discussions and code suggestions, introduced a TestingRoutesProvider and targeted 9.0 this is now nearly finished.

I cannot really self approve partly my own work but count this as a +0.5

[mhsdesign]

As this was a joined effort and i committed as well count me as half of a review :)

Thank you so much @sorenmalling for the initial improvement and @mficzel for making it work after the long pause. (And of course @bwaidelich for the discussions ^^)

Lets merge it to 9.0

[mficzel]

The testing route provider could be moved to tests but other than that.

[mhsdesign]

as discussed with @mficzel this class resides here correctly.

Neos\Flow\Tests has no proxy building and the TestingPrivilegeManager also lives in Classes.

Changes applied

[mhsdesign]

Fyi phpstan 3 catched a bug in routing:show, but that will be fixed here:

#3261