Fix mvc bootrapping

[FalkHe]

Q	A
Documentation	yes
Bugfix	yes
BC Break	no
New Feature	yes
RFC	yes/no
QA	yes/no

Description

Provide option to enable MVC Application bootstrapping.

False by default, to not break any existing app.

See Issue #106 .

[steffendietz]

I like it.

Regarding the

When enabling it, make sure not to do any HTTP-Only related stuff on Module's onBootstrap method.

Is there currently a way to tell if we are in a laminas-cli run context? The old laminas-mvc-console package had a delegator factory which was used to create a ConsoleRequest instead of the HttpRequest.

Would it maybe make sense to define a namespaced const in bin/laminas? Something like

define('Laminas\Cli\RUN_CONTEXT', true);

So that a hypothetical onBootstrap method could bail out?

public function onBootstrap(EventInterface $e)
{
    // do general purpose bootstrapping
    
    if (defined('Laminas\Cli\RUN_CONTEXT')) {
        return;
    }
    
    // do http-only bootstrapping
}

I'm just spit-balling here. :sweat_smile:

[froschdesign]

@steffendietz

Would it maybe make sense to define a namespaced const in bin/laminas?

Pollution of the global namespace is not an option and must be avoided.

[steffendietz]

Pollution of the global namespace is not an option and must be avoided.

That's why I was specifically talking about a namespaced constant.

[FalkHe]

Is there currently a way to tell if we are in a laminas-cli run context?

Not AFAIK.

But: This should not be necessary. As told in the documentation you should not do any heavy stuff in onBoostrap(). Only prepare Services, Listeners, ... things that ~aren't~ shouldn't be expensive at all. i.E.: setting up a Listener on MvcEvent::EVENT_DISPATCH ist totally fine. It's not executed, because lamas-cli doesn't fire MvcEvent::EVENT_DISPATCH.

If you've got something that really should not be executed in cli context but is currently located in onBoostrap(), it's very likely not in the right place. Think about putting the code behind an Event that's only fired in http-context (that's what i do most of the time). Use either EVENT_DISPATCH / EVENT_DISPATCH_ERROR, EVENT_RENDER / EVENT_RENDER_ERROR, ...

... but that's OT and should be diskussed in the Forum.

[FalkHe]

Regardless of the patch direction, I expect tests to verify what's being done here.

May i 'expect' your help on this, @Ocramius?

I don't know how to set up an environment for the test. And I've no time to figure it out, atm. I tried some bits, without success. I'm not familiar with the used vfs and don't want to mess the tests up by introducing something totally different.

[froschdesign]

@FalkHe I'll convert your PR into a draft, because let's find the correct solution first. Then we will also know what the tests have to look like.

[steffendietz]

@FalkHe

I created a failing test case for the original issue. https://github.com/steffendietz/laminas-cli/tree/failing-testcase-for-106

@froschdesign

What are your requirements for a correct solution? Your wording seems to indicate, that the one discussed here is somehow not correct. Could you share your heuristic for this classification?

[Xerkus]

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.

[FalkHe]

@Xerkus Any discussion on different solutions / ideas should go into #106.

[Xerkus]

Indeed. My bad.