[RFC]: Provide an ApplicationProgrammaticConfigDelegator that pairs with ApplicationConfigInjectionDelegator

acelaya commented 2 years ago

RFC

Q	A
Proposed Version(s)	3.x
BC Break?	No

Goal

Standardize how routes and middleware are piped into the application, by providing a, let's call it ApplicationProgrammaticConfigDelegator, that wraps the logic of loading the config/routes.php and config/pipeline.php files.

Then, users would need to decorate their Application service with either ApplicationProgrammaticConfigDelegator or ApplicationConfigInjectionDelegator (or none, if they want to follow a completely different approach).

The skeleton application would use this new delegator by default.

Background

I find myself in the next situation:

I have a mezzio-based project where I have been historically defining routes and middlewares via configuration, so I had my Application service decorated with ApplicationConfigInjectionDelegator.

When the mezzio-swoole package was created (still zend-expressive-swoole back then), I started experimenting with it, in order to support serving this application with swoole.

On its first versions, the command to start a mezzio app with swoole was more opinionated, and always tried to load the config/routes.php and config/pipeline.php files, which did not exist in my case, making the start-up fail.

I provided a PR to make the loading optional, and I have been successfully using it since.

Recently I have also added support for RoadRunner, so now the project can be bootstrapped/run in three different ways (plus the CLI).

Also recently, due to a couple of new features, the loading of routes and middleware via config has become more inconvenient, and using the programmatic approach mkes more sense now.

The problem is that I now have two different scopes (php-fp and roadrunner) where I have to manually require the routes.php and pipeline.php files, as they have different entry points.

I have considered writting a common script which is then used by both entry points, but I already have too many scripts "calling" each other and it's becoming harder to follow.

I have also considered writting this delegator myself, but this would conflict with mezzio-swoole's start command, since it would try to load them again. I would need to conditionally register this delegator based on the execution context.

Considerations

On mezzio, none that I can think of. Users would need to add the new delegator and remove the manually requirement of the files from their entry points, but as long as they don't do it, everything should continue working.

On mezzio-swoole, the logic that right now loads those files if they exist, would need to somehow check if they have been already included, and avoid it in that case. That would keep backwards compatibility. Including this in a major version and just removing the logic from the start command would be another option.

Proposal(s)

As already more or less explained, my proposal would be to be less opinionated on mezzio-swoole, and also avoid forcing entry points to require the two mentioned files, in case the project has multiple entry points.

Instead, mezzio would provide two ways to load the config, which would require decorating the Application service one way or another (the docs should explain both options).

Then, different entry points would be able to pull the Application service from the container and it would already have all the config no matter the entry point.

acelaya commented 2 years ago

Just as a comment. As a short term solution I have placed my pipeline and routes files inside a different folder (config/programmatic), that way I can have my own delegator factory that can be always safely used (even with swoole).

This would be an option to make this change in a fully backwards-compatible way.

Ocramius commented 2 years ago

Just removing the convention and moving the service to a factory would enable this 👍

boesing commented 1 year ago

The skeleton application would use this new delegator by default.

if that is the case, shouldn't we put this into the skeleton instead? I'd rather get rid of the whole application delegator stuff and instead recommend to have properly targeted delegators instead.

When it comes to mezzio, laminas-cli uses the config/container.php which does not instantiate any application:

https://github.com/laminas/laminas-cli/blob/97930db3fd2550da0374a3ccbdc7f9b9d6177319/src/ContainerResolver.php#L55-L60

Now there are also CLI commands which do want to provide mezzio route tables: https://github.com/mezzio/mezzio-tooling/pull/27

So this CLI command would preferably do something like:

$registeredRoutes = $container->get(RouteCollectorInterface::class)->getRoutes();
foreach ($registeredRoutes as $route) {
    // create table
}

https://github.com/mezzio/mezzio-router/blob/182d47967b955f20adbb072526bac50300fa7dcb/src/RouteCollectorInterface.php#L89

But due to these application delegators, it actually has to do stuff like:

$container->get(Application::class);
$registeredRoutes = $container->get(RouteCollectorInterface::class)->getRoutes();
foreach ($registeredRoutes as $route) {
    // create table
}

That is why I proposed to get rid of the existing decorator in #147 and to provide new decorators for both RouteCollectorInterface and MiddlewarePipeInterface which are replacing the existing decorator in #154.

WDYT?

acelaya commented 1 year ago

Hey!

I had to read my original issue again, as I did not remember the context 😅

The thing is that, everything I described there, that motivated me to open this issue in the first place, was eventually rolled back, because it ended up introducing other issues (I won't describe them here, as it's unrelated).

But in essence, yeah, I think your proposal makes sense. Having everything related to routes bound to the router and not the application, is probably more intuitive.

I'll close this and watch the other issue for updates.

mezzio / mezzio