Open steffendietz opened 1 year ago
When using
laminas/laminas-cli
in the context of a MvcApplication (ContainerResolver::resolveMvcContainer
), modules don't get bootstrapped completely, which results in services potentially missing configuration.
laminas-cli do not start an application, not a laminas-mvc, Mezzio or something else. laminas-cli uses the same configuration of the service container of your laminas-mvc application but it will not launch the application or trigger any events.
Therefore, the method onBootstrap
is not called in modules.
Hey Steffen,
yes, you are right. I am not 100% aware of why we decided to avoid calling bootstrap
in CLI. AFAIR, that was outlined in the RFC, maybe @michalbundyra can remember.
As a workaround, you can create a config/container.php
file (which we do have in our MVC application as well) where you then bootstrap your application as you need it:
<?php
use Laminas\Mvc\Service\ServiceManagerConfig;
use Laminas\ServiceManager\ServiceManager;
use Psr\Container\ContainerInterface;
chdir(\dirname(__DIR__));
// Setup autoloading
require_once __DIR__ . '/../vendor/autoload.php';
return (static function (): ContainerInterface {
// Our `application.config.php` does already handle development config merge if exists
$appConfig = require __DIR__ . '/application.config.php';
$smConfig = new ServiceManagerConfig($appConfig['service_manager'] ?? []);
$serviceManager = new ServiceManager();
$smConfig->configureServiceManager($serviceManager);
$serviceManager->setService('ApplicationConfig', $appConfig);
$serviceManager->get('ModuleManager')->loadModules();
// Fetch `Application` from service manager and bootstrap with configured listeners as done in the `Application#init` method
// Restore error and exception handlers to avoid something is suppressing errors as applications are usually called via HTTP
restore_error_handler();
restore_exception_handler();
return $serviceManager;
})();
@froschdesign Yes, but I understand that someone expects that when creating a CLI command, it can interact with modules and if modules are only properly setup when calling the bootstrap event, thats actually a problem.
We have a pretty large MVC application but I never liked the bootstrap event. We do not configure anything in there but used service delegators so that services which do need runtime configuration can be configured in their factories rather than in some event listeners. The major benefit when using delegators is, that you only bootstrap stuff which is actually used at runtime. But I have no use-case where I would have a need for the "bootstrap" event and thus, I probably can't relate properly.
@froschdesign Yes, I understand that, but as @boesing mentioned that is kind of the whole point of the bug report.
Currently it means, that from a users point of view, a service acquired through the Container in a laminas-cli
context has a different level of setup than when acquired in the web context.
I find this is kind of difficult to explain.
The ContainerResolver
is even aware that we are inside of a MvcApplication context and calls the loaded config $appConfig
.
Replacing the current ContainerResolver::resolveMvcContainer
with something like
private function resolveMvcContainer(string $path): ContainerInterface
{
/**
* @psalm-suppress UnresolvableInclude
* @psalm-var array<array-key, mixed> $appConfig
*/
$appConfig = include $path;
Assert::isMap($appConfig);
$developmentConfigPath = sprintf('%s/config/development.config.php', $this->projectRoot);
if (file_exists($developmentConfigPath)) {
$devConfig = include $developmentConfigPath;
Assert::isMap($devConfig);
$appConfig = ArrayUtils::merge($appConfig, $devConfig);
Assert::isMap($appConfig);
}
$application = MvcApplication::init($appConfig);
return $application->getServiceManager();
}
would in my opinion completely solve the issue, since it lets application bootstrapping as well as module loading and container initialization remain the concern of the MvcApplication.
Edit: What would even be the intended way of executing the missed setup steps? Calling something like:
$container->get('Application')->bootstrap();
in the CommandFactory
?
@boesing regarding the use of the onBootstrap
. I know there are probably more elegant ways to do things nowadays. But as of today the bootstrap
is part of the documented application life-cycle and as such should be supported.
Also, laminas-cli
is published as a replacement for laminas-console
and laminas-mvc-console
and as such should probably support the pecularities of its ecosystem.
Bottomline is: If this is not considered a bug, then I would like to have a pointer to the "idiomatic" laminas-mvc
way of integrating laminas-cli
.
@boesing @steffendietz Please don't misunderstand me, I didn't say that it has to be this way or that I like it that way. I have only written what it is like. There have already been several questions or the expectation of this. (see also in the forum)
My expectation is quite clear that the individual modules will be loaded and booted.
The documentation doesn't say - don't use this onBootstrap method.
Some settings for the ServiceManager can be regulated in this way, and it is precisely these settings that are then missing in the ServiceManager within a CLI.
This just feels wrong and not thought through to the end. If the framework offers a modification path, it must also be observed in any case.
@nusphere
This just feels wrong and not thought through to the end. If the framework offers a modification path, it must also be observed in any case.
Every pull request is welcome. Go for it! 👍🏻
is something open or done?
@nusphere not AFAIK
To give a use case:
I usually set up EventListeners within onBoostrap()
or via the 'listeners'
config key. Both are omitted when running a command via laminas-cli. That's neither obvious nor documented and there's no clean way to attach the same EventListeners for both scenarios (mvc+cli). (correct me if i'm wrong)
yes, you are right. I am not 100% aware of why we decided to avoid calling
bootstrap
in CLI. AFAIR, that was outlined in the RFC, maybe @michalbundyra can remember.
Because request and response are in container, a lot MVC applications in the wild do http request specific initialization in onBootstrap
leading to some horrible breakages.
I gave it a shot. Let me know your thoughts.
I would rather prefer Mvc application to provide config/container.php which would init and potentially bootstrap the Mvc before it would return container for laminas-cli to consume.
Bootstrapping Mvc unconditionally is not something I am comfortable with. That is a decision that needs to be made consciously for every specific application.
Change like this applied to application would solve this without assumptions from this component.
https://github.com/Xerkus/laminas-mvc-skeleton/commit/1a242e41135d49f159a5d45cb5eec8c9639c1682
@boesing
What RFC are you refferring to? Could you please explain the reason/source for this decision?
But I have no use-case where I would have a need for the "bootstrap" event
How/When/Where do you attach your EventListeners? This needs to be done during boot. And here is no other place except onBootstrap to do this, AFAIK.
Changes now were made to MVC skeleton application to include config/container.php
which will fire MVC onBootstrap()
events. https://github.com/laminas/laminas-mvc-skeleton/pull/67
I suggest what happens here is we update documentation detailing how to apply same changes to existing MVC applications to utilize laminas-cli. Drop MVC specific container resolver in the next major.
@boesing
What RFC are you refferring to? Could you please explain the reason/source for this decision?
Maybe I had this in mind and just did not remembered correct:
https://github.com/laminas/laminas-cli/issues/92#issuecomment-1083172593
@xerkus we should deprecate listener stuff in init and remove it in v4. listeners should get registered with delegator rather than manual config extraction.
@steffendietz @nusphere
Can you check the option via configuration file for the container (container.php
) in your applications?
Please see the related changes in the skeleton application:
Can you give feedback on this? Thanks in advance! 👍🏻
@froschdesign As @Xerkus changes to the skeleton application basically replicated the workaround @boesing proposed 3 months ago, I can confirm that this indeed mitigates the problem. :+1:
Concerning this component (laminas/laminas-cli
) however, I fear the initial issue is still not resolved, since the ContainerResolver::resolveMvcContainer
code path still omits the bootstrapping life-cycle stage of modules.
Also, there is no clear answer from one of the core maintainers, how this missing life-cycle step should be triggered, although this question came up two times in this thread.
Additionally, inspired by the comment of @boesing, I looked at some open issues. At least #92 and #85 in part struggled with the same underlying problem.
At the very least, a deprecation message or better yet, a warning would need to be displayed, if the container was resolved using resolveMvcContainer
, so that the user is made aware that the modules are not bootstrapped properly.
TBH, we made bad decisions in the past - or at least it feels to me as that is the case.
Having application magic (as we do have in mezzio aswell with all these Application#pipe
, Application#get
, Application#post
methods) to configure the application is problematic and will most probably not work with laminas-cli
.
Pretty sure these methods were introduced to have syntax sugar when starting with mezzio but when introducing a CLI command, that won't have access to that index.php
initialized stuff.
So you will never have those routes or middlewares available in laminas-cli
when passed these are only configured in index.php
as in https://github.com/mezzio/mezzio-skeleton/blob/4f60efe8a5d96b0178d446268c6fc2586f88df6b/public/index.php#L26 or via Application#init
.
Thats a problem by-design and IMHO it should have never been implemented like this.
Configuring the application should be part of the config and not a procedural thing as it is right now. When fetching applications from container, it should be configured and immutable (usually same to the container, but that is not always possible I guess).
So if you are using mezzio with the latest skeleton, you will have issues with missing routes as well. Thats why I also encourage every1 who is asking me to use https://github.com/mezzio/mezzio/blob/3.17.x/src/Container/ApplicationConfigInjectionDelegator.php along with configuration-driven routes and pipeline so that these will be registered via config (even tho, the routes should be registered via the router directly but I guess thats another point).
So pretty sure, whatever solution we might find for this specific problem here, there will be other issues where other stuff is not properly working.
I guess, the only thing, why this is MVC problem is a thing, is, that we encourage projects to migrate from laminas-console which actually bootstrapped MVC applications and thus thats some kind of a problem when migrating to laminas-cli
.
I wonder if we should provide some kind of decorator to allow projects to actually have BC feeling when migrating from laminas-console
.
Thoughts @laminas/technical-steering-committee ?
@steffendietz First, thank you very much for your response! 👍🏻
Concerning this component (
laminas/laminas-cli
) however, I fear the initial issue is still not resolved, since theContainerResolver::resolveMvcContainer
code path still omits the bootstrapping life-cycle stage of modules.
It may be frustrating, but we have to deal with the problem: https://github.com/laminas/laminas-cli/issues/106#issuecomment-1462047887
Because request and response are in container, a lot MVC applications in the wild do http request specific initialization in onBootstrap leading to some horrible breakages.
I understand the desire to solve this quickly and easily in the resolveMvcContainer
method and call the init
method of the application there. Unfortunately, we create new problems because existing applications then break because it is not checked if the route match exists or not, for example. Therefore, the concern and caution here.
You may call the container.php
a workaround, I see it as a single step towards a future-proof solution. And yes, some work is still needed on Mezzio, laminas-mvc and laminas-eventmanager to simplify the entire process and add a complete solution:
(Positive side effect: a harmonisation of laminas-mvc and Mezzio.)
ContainerResolver::resolveMvcContainer
code path still omits the bootstrapping life-cycle stage of modules.
It is a part of MVC application lifecycle. It should not be used for anything else.
Legacy Application::init()
fires bootstrap event and it is way more convenient to add onBootstrap()
to module than any other approach like container delegator factories or registering listener for ModuleEvent::EVENT_LOAD_MODULES_POST
event from Module::init()
. With the frankly lacking documentation a lot of things end up in onBootstrap()
that should not be there.
Just recently a user looking for support had their services initialized in listener on MvcEvent:EVENT_ROUTE
. From my own experience I had applications breaking more often than not by initializing container for cli tooling like doctrine-migrations using Application::init()->getServiceManager()
We can not make assumption for all from a new library and decision was made not to bootstrap MVC as a proper approach. What we failed to do is to provide documentation clearly outlining that.
User application can make assumptions and make application specific decision to fire bootstrap events to setup container. It is why it is acceptable to be done in mvc skeleton but IMO not here.
Only tangentially related, but in MVC v4 module manager will be removed. When it is released assumptions made here will become invalid.
Thats why I also encourage every1 who is asking me to use https://github.com/mezzio/mezzio/blob/3.17.x/src/Container/ApplicationConfigInjectionDelegator.php along with configuration-driven routes and pipeline so that these will be registered via config (even tho, the routes should be registered via the router directly but I guess thats another point).
I actually don't recommend that, in part because the configuration is so difficult to write, difficult to debug, and because configuration merging issues are rampant. I tend to recommend using a delegator on the Application
instance or the RouteCollectorInterface
, and then register using that service. This ensures that the routes are present for your CLI commands as well, as you will get them when pulling the related services.
The idea behind config/routes.php
was reasonable for the purpose of RAD-style microframework utilization, but not great as we expanded the framework.
I tend to recommend using a delegator on the Application instance or the RouteCollectorInterface, and then register using that service. This ensures that the routes are present for your CLI commands as well, as you will get them when pulling the related services.
I would love to do so as well but sadly there is no such delegator as of now. I do not expect every1 to write such a delegator himself and thus point to the application config injection delegator.
Might be worth creating one in mezzio/mezzio
which then replaces the existing one. Gonna create a feature request for that.
I would love to do so as well but sadly there is no such delegator as of now. I do not expect every1 to write such a delegator himself and thus point to the application config injection delegator.
Correct, that cannot be the goal, that a user has to create this himself.
Hello,
the same case:
migrated to laminas-cli
and got onBootstrap
-issue.
in onBootstrap
mostly we:
i have read the whole discussing, but not clearly understood what is the suggested way to use laminas-cli
and onBootstrap
-logic?
logic here adds code inside index.php
, which AFAIU is not used by laminas-cli
(it runs via ./vendor/bin/laminas
)
what is the workaround?
where we should put our logic to make it work for web
and cli
simultaneously?
Thanks!
@lon9man
logic here adds code inside
index.php
, which AFAIU is not used bylaminas-cli
(it runs via./vendor/bin/laminas
)
The important part is not the change in the index.php
file but the new file container.php
in the config
folder. This file will be used by laminas-cli:
This calls the init
method of Laminas\Mvc\Application
:
And then the bootstrap
method:
Which includes the MvcEvent::EVENT_BOOTSTRAP
event:
@froschdesign, thanks for the fast and valuable response! it seems works.
maybe it should be some big red note 'DANGER-DANGER' in docs for this important edge case? module-manager laminas-cli
Just loading modules should be enough to have application setup for basic usage.
Logic not specific to MVC and web should not really be in onBootstrap()
. That logic however is in onBootstrap()
.
config/container.php
from skeleton is a compromise that does questionable if not wrong thing and fires bootstrap event. Because it is decided in application and not in this package it is acceptable. It allows application to easily ditch bootstrap event for cli usage.
@froschdesign, thanks for the fast and valuable response! it seems works.
maybe it should be some big red note 'DANGER-DANGER' in docs for this important edge case? module-manager laminas-cli
Why add this to the documentation? let the idiots spoil their nerves and then switch to Laravel
@lon9man
in
onBootstrap
mostly we:
- attach events
Can be done in the configuration: https://docs.laminas.dev/laminas-eventmanager/application-integration/usage-in-a-laminas-mvc-application/#register-listener
maybe it should be some big red note 'DANGER-DANGER' in docs for this important edge case?
Correct, the documentation needs to be expanded, but a warning about a danger is a bit too much.
@Xerkus
for basic usage
only hello-world
is basic) any else very often is custom.
Logic not specific to MVC and web should not really be in onBootstrap()
ok, onBootstrap
- bad place.
what is the good place or way?
class Module
{
public function getConfig(): array
{
$provider = new ConfigProvider();
return $provider->getConfig();
}
public function onBootstrap( Event $e )
{
$application = $e->getApplication();
$serviceManager = $application->getServiceManager();
$eventManager = $e->getTarget()->getEventManager();
$notificationListener = $serviceManager->get( EventListener::class );
$notificationListener->attach( $eventManager );
}
}
EventListener::class
globally tracks insert/update/delete of models into database.
others onBootstrap
looks similarly with different target functionality (events of LmcUser
and etc).
so what is the correct place (without onBootstrap
) nowadays, which will cover web
and cli
simultaneously?
Thanks!
UPD. froschdesign answered already, what is your point of view?
where we should put our logic to make it work for
web
andcli
simultaneously?
Ideally in container configuration. Factories/delegator factories.
For example, this delegator factory adds listeners to listener provider instance https://github.com/laminas/laminas.dev/blob/f872adb6eed0c0196d8026b78fea44c553f4c8fd/src/GitHub/ListenerProviderDelegatorFactory.php Whenever listener provider is pulled from container it is already configured, no special bootstrap event is needed (and in case of mezzio does not exist).
In mvc a go to method to setup anything is on bootstrap and heavier it is loaded with setting up http handling logic the less suitable it is for cli usage. onBootstrap is in a way a remnant of ZF1 init system from one and half decade ago. A different design philosophy. Sure it was convenient for migrating back then but now DI container is expected to provide ready for use instances.
onBootstrap is not really that much of a problem, if it works for you you can use it. Up to you to ensure it is compatible for cli usage.
In context of this package we can not assume what is happening in onBootstrap. Eg quite a few applications out there outright require http request to perform logic and can not be invoked outside of http request handling context.
Moving this decision onto application with the config/container.php
allows application developers to take responsibility and decide which way suits their case better.
What needs to be done here is documentation covering how to setup said file with or without calling bootstrap.
IMHO, the whole eventmanager
documentation is kinda flawed.
There are a bunch of ways to register stuff to whatever event manager (both listeners
keys are only handled by laminas-mvc
), i.e.:
return [
'listeners' => [], // whatever listener is listed here as class-string will be instantiated and attached to the `Application#getEventManager`
];
Then you can register listeners
by adding that key to application.config.php
for laminas-mvc
.
Then you can register delegators listening to EventManager
service, i.e. by using a config file like (which works for both laminas-mvc
and mezzio
):
return [
'service_manager' /* or 'dependencies' for mezzio */ => [
'delegators' => [
'EventManager' => [
ProjectSpecificEventsToEventManagerAttacher::class,
],
],
],
];
along with the delegator:
final class ProjectSpecificEventsToEventManagerAttacher
{
public function __invoke(ContainerInterface $container, $name, callable $callback): EventManagerInterface
{
$notificationListener = $serviceManager->get( EventListener::class );
$eventManager = $callback();
$notificationListener->attach($eventManager);
return $eventManager;
}
}
Imho, none of the examples within the documentation of laminas-mvc
nor mezzio
do explain on how to attach events properly and thus, most people are kinda lost and/or have added this kind of stuff to onBootstrap
as that is how it is recommended in the MVC documentation AFAIR.
This should be removed and instead replaced with the delegator approach: https://docs.laminas.dev/laminas-mvc/examples/#bootstrapping
I know there was no activity on this issue in 8 months, but I wanted to know if anyone found a solution?
I think, like most people here, that the application should be bootstraps and have all the MVC listener present when calling commands. It is so convoluted to duplicate the code so it works in commands. Using the delegator approach describe above is very complicated since any objects that receives the EventManager in a factory now create infinite loops.
Hopefully, a real working solution will be provided soon.
Thanks and keep up the good work!
@duprasf
…but I wanted to know if anyone found a solution?
Already there! Use the same file structure as in the skeleton application and you're done:
Bonjour @froschdesign ,
I copied the container.php and the line from the index.php. I pasted those lines in my CommandFactory file since this is the first thing that is executed in my code when I run a command (using the vendor/bin/laminas executable) and it seems to be working. It did output my layout so I have to use ob_start and ob_clean to prevent the output in the terminal. But this still feels like a hack and not something permanent. Unless I don't know where to put the code from the index.php. From what I have done right now, every time I create a new command I will have to put the index.php lines in that new command as well.
Anyway, it seems to be working for now, my logs shows the activity from my cronjob. Thank you very much.
Those two files in skeleton are exactly the two files in your application that you need to modify. vendor/bin/laminas
uses config/container.php
to obtain container and only in its absence it tries to load modules from MVC application.
As I stated prior, bootstrapping MVC application for container usage in other contexts such as cli is wrong. It is required for legacy reasons in older applications but decision to invoke bootstrap event can not be made by this component and must be deferred to the specific app. Application can then do anything needed to get container ready for use in conifg/container.php
Bug Report
Summary
When using
laminas/laminas-cli
in the context of a MvcApplication (ContainerResolver::resolveMvcContainer
), modules don't get bootstrapped completely, which results in services potentially missing configuration.Current behavior
We use the
onBootstrap
event of our MvcApplication to set up listeners for services or set up service state based on context. When using those services inside of commands executed bylaminas/laminas-cli
, those services are not fully set up, which results in unexpected behavior, since the application/module is missing thebootstrap
lifecycle stage.Based on the Documentation, the
onBootstrap
method should/could be used for:How to reproduce
Use a service somehow modified or configured in the
onBootstrap
method in aCommand
executed throughlaminas/laminas-cli
. The service passed to the command has not gone through the MvcApplications bootstrap lifecycle.I try to illustrate this on the following pseudo-module:
SampleCommand
returns1
.Expected behavior
From my perspective, using commands through
laminas/laminas-cli
in the context of an MvcApplication, should guarantee the same initialisation level as withApplication::init
.In the above example,
SampleCommand
should return0
, as it has access to a bootstrapped version of theSampleService
.