Drop module manager and add support for config aggregator #56

Xerkus · 2020-06-09T15:05:49Z

This PR introduces changes necessary to cut out module manager from MVC application.

Usage originally envisioned for module manager is no longer relevant:

Package specific autoloader handling is superceded by Composer
Shared application installations are a thing of the past. Applications are no longer expected to change feature sets at runtime.
A need for a flexible programmatic application assembly is superseded by dependency injection containers
Components configuration is immediately available thanks to how composer autoloading works. There is no need to load modules anymore before configuration providers can be gathered and config merged.
Frontend assets management or package dependencies resolution are no longer needed. Etc.
-Most used remaining features are a config provider and onBootstrap() convenience listener.

As proven by Mezzio, much simpler laminas-config-aggregator coupled with Container is more than sufficient.

Changes:

Module Manager removed along with its dependnecies
Laminas\Mvc\Application::init() is removed in favor of Laminas\Mvc\ConfigProvider for laminas-config-aggregator
Laminas\Mvc\Service\ConfigFactory removed as config can now be merged and provided to Container before Container is created
List of extra Application listeners moved from separate application config into main config under the $config[\Laminas\Mvc\Application::class]['listeners'] key
Configuration for the various PluginManagers moved from removed Laminas\ModuleManager\Listener\ServiceListener into each respective plugin manager factory
ServiceManagerConfig removed and replaced by ConfigProvider with the goal to eventually remove hard dependency on laminas-servicemanager as a main container.
ModuleManager's onBootstrap() convenience listener is not replaced by any alternative at this time.

Documentation tracking issue: #58

Xerkus · 2020-06-12T17:13:03Z

This PR does not concern itself with code quality and intentionally avoids any changes that are not a bare necessity

Signed-off-by: Aleksei Khudiakov <[email protected]>

weierophinney

Overall, I think it's a nice step!

I think the one thing missing for me is a write-up in the patch description detailing:

your proposal for where the config and/or container are built
- AND what each of those files look like
what the public/index.php looks like

As somebody familiar with Mezzio (clearly!) those details seem obvious, but for somebody not familiar with them, having that information documented here will help them understand how we are evolving the MVC, and what it will look like once complete.

weierophinney · 2020-06-17T20:38:35Z

src/Application.php

-    public function bootstrap(array $listeners = [])
+    public function bootstrap()
    {
        $serviceManager = $this->serviceManager;
        $events         = $this->events;

-        // Setup default listeners
-        $listeners = array_unique(array_merge($this->defaultListeners, $listeners));
-
-        foreach ($listeners as $listener) {
-            $serviceManager->get($listener)->attach($events);
-        }


Why are you removing the ability to pass additional listeners to bootstrap()?

To be clear, I'm fine with it (I don't think we document it anywhere), but you'll need to detail it as a feature removal in the changelog as well.

Extra listeners are moved to the application factory since Application::init() is going to be dropped.

real change here is that extra listeners to register would migrate from the application.config.php to the merged config

weierophinney · 2020-06-17T20:41:36Z

src/Application.php

-    /**
-     * Static method for quick and easy initialization of the Application.
-     *
-     * If you use this init() method, you cannot specify a service with the
-     * name of 'ApplicationConfig' in your service manager config. This name is
-     * reserved to hold the array from application.config.php.
-     *
-     * The following services can only be overridden from application.config.php:
-     *
-     * - ModuleManager
-     * - SharedEventManager
-     * - EventManager & Laminas\EventManager\EventManagerInterface
-     *
-     * All other services are configured after module loading, thus can be
-     * overridden by modules.
-     *
-     * @param array $configuration
-     * @return Application
-     */
-    public static function init($configuration = [])
-    {
-        // Prepare the service manager
-        $smConfig = isset($configuration['service_manager']) ? $configuration['service_manager'] : [];
-        $smConfig = new Service\ServiceManagerConfig($smConfig);
-
-        $serviceManager = new ServiceManager();
-        $smConfig->configureServiceManager($serviceManager);
-        $serviceManager->setService('ApplicationConfig', $configuration);
-
-        // Load modules
-        $serviceManager->get('ModuleManager')->loadModules();
-
-        // Prepare list of listeners to bootstrap
-        $listenersFromAppConfig     = isset($configuration['listeners']) ? $configuration['listeners'] : [];
-        $config                     = $serviceManager->get('config');
-        $listenersFromConfigService = isset($config['listeners']) ? $config['listeners'] : [];
-
-        $listeners = array_unique(array_merge($listenersFromConfigService, $listenersFromAppConfig));
-
-        return $serviceManager->get('Application')->bootstrap($listeners);
-    }
-


Why are you removing this method?

Removal of this MUST be called out in both the CHANGELOG and the documentation, as our own skeleton has consistently used this method in order to initialize and bootstrap the application from aggregated configuration. If it goes away, users need to know what they need to change in their public/index.php.

This method no longer provides utility it once did.
With the the module manager dropped, responsibility to setup the application pushed completely out of the scope of this package.

It is now entirely up to skeleton application to provide the default wiring. As initially outlined in the Zend\Mvc v4 RFC the goal is to move towards current mezzio skeleton application setup. Config+container proved to be flexible enough.

At this time I am working on a new feature that would somewhat approximate laminas-httphanderrunner and would affect how public/index.php needs to be changed.

On a side note, I have no immediate intention to provide update for the laminas-modulemanager. I can easily see it retired unless there is a demand for the different set of features not available with the config aggregator.

weierophinney · 2020-06-17T20:44:08Z

src/ConfigProvider.php

+                'EventManagerAwareInitializer' => static function ($first, $second) {
+                    if ($first instanceof ContainerInterface) {
+                        $container = $first;
+                        $instance = $second;
+                    } else {
+                        $container = $second;
+                        $instance = $first;
+                    }
+
+                    if (! $instance instanceof EventManagerAwareInterface) {
+                        return;
+                    }
+
+                    $eventManager = $instance->getEventManager();
+
+                    // If the instance has an EM WITH an SEM composed, do nothing.
+                    if ($eventManager instanceof EventManagerInterface
+                        && $eventManager->getSharedManager() instanceof SharedEventManagerInterface
+                    ) {
+                        return;
+                    }
+
+                    $instance->setEventManager($container->get('EventManager'));
+                },


We can simplify this by only allowing laminas-servicemanager v3+ and laminas-eventmanager v3+. Personally, I'd do that as part of this patch to simplify this part of the ConfigProvider.

Alternately, move that code into a dedicated method, and use Closure::fromCallable() to provide it as an initializer.

This change would be outside of the scope for the module manager removal.

I do not see a reason to keep EventManager initializer and expect to drop any initializers entirely as a part of the move towards generic PSR containers. I will need to double check container integrations supported by mezzio, I am not sure if it is something supported by all them.

weierophinney · 2020-06-17T20:45:17Z

src/Service/ApplicationFactory.php

+        $listeners = $container->get('config')[Application::class]['listeners'] ?? [];
+        foreach ($listeners as $listener) {
+            $container->get($listener)->attach($em);
+        }


This answers part of my question about the changes to Application::bootstrap(). However, you need to document this in the changelog and the manual.

This is meant more of a placeholder at this point that is meant to preserve the functionality. Event system would need to have a more detailed revision and potential utilization of PSR-14 would change this part entirely.

One of the reasons why I opted for skipping documenting the changes in CHANGELOG and providing updated documentation by this PR.

weierophinney · 2020-06-17T20:48:57Z

src/Service/ViewHelperManagerFactory.php

    {
        // Configure URL view helper
        $urlFactory = $this->createUrlHelperFactory($services);
-        $plugins->setFactory(ViewHelper\Url::class, $urlFactory);
-        $plugins->setFactory('laminasviewhelperurl', $urlFactory);
+        $managerConfig['aliases']['laminasviewhelperurl'] = ViewHelper\Url::class;


I think we can drop these "normalized" aliases - users should only be using the FCQN or the class-name-minus-namespace when retrieving helpers.

That change would be outside of the scope of this PR. Too easy to lose track of small details like that during review.

I plan to drop all obsolete aliases in another PR.

…config Signed-off-by: Aleksei Khudiakov <[email protected]>

Signed-off-by: Aleksei Khudiakov <[email protected]>

Application listeners should be registered by the time `Application` service is retrieved from container. Previously they were registered at the time of `Application::bootstrap()` call. Signed-off-by: Aleksei <[email protected]>

Xerkus · 2022-06-03T16:46:08Z

Migration path for existing users would need to be provided via separate package providing necessary bindings between module manager and mvc.
config() and onBootstrap() are two most often used module methods. First one is covered by config provider while second... I would need to look at event changes first. I do not remember intended details anymore, but it involved PSR event listener providers.

weierophinney

This looks good.

The only feedback I have is around usage of the laminas-servicemanager FactoryInterface; I think we could drop explicitly implementing that interface, and just doing invokables, to help reduce ties to laminas-servicemanager. However, that change is likely out of scope for this PR.

weierophinney · 2022-06-03T17:52:31Z

src/Application.php

-    public function bootstrap(array $listeners = [])
+    public function bootstrap()
    {
        $serviceManager = $this->serviceManager;
        $events         = $this->events;

-        // Setup default listeners
-        $listeners = array_unique(array_merge($this->defaultListeners, $listeners));
-
-        foreach ($listeners as $listener) {
-            $serviceManager->get($listener)->attach($events);
-        }


weierophinney · 2022-06-03T17:53:30Z

src/Application.php

-    /**
-     * Static method for quick and easy initialization of the Application.
-     *
-     * If you use this init() method, you cannot specify a service with the
-     * name of 'ApplicationConfig' in your service manager config. This name is
-     * reserved to hold the array from application.config.php.
-     *
-     * The following services can only be overridden from application.config.php:
-     *
-     * - ModuleManager
-     * - SharedEventManager
-     * - EventManager & Laminas\EventManager\EventManagerInterface
-     *
-     * All other services are configured after module loading, thus can be
-     * overridden by modules.
-     *
-     * @param array $configuration
-     * @return Application
-     */
-    public static function init($configuration = [])
-    {
-        // Prepare the service manager
-        $smConfig = isset($configuration['service_manager']) ? $configuration['service_manager'] : [];
-        $smConfig = new Service\ServiceManagerConfig($smConfig);
-
-        $serviceManager = new ServiceManager();
-        $smConfig->configureServiceManager($serviceManager);
-        $serviceManager->setService('ApplicationConfig', $configuration);
-
-        // Load modules
-        $serviceManager->get('ModuleManager')->loadModules();
-
-        // Prepare list of listeners to bootstrap
-        $listenersFromAppConfig     = isset($configuration['listeners']) ? $configuration['listeners'] : [];
-        $config                     = $serviceManager->get('config');
-        $listenersFromConfigService = isset($config['listeners']) ? $config['listeners'] : [];
-
-        $listeners = array_unique(array_merge($listenersFromConfigService, $listenersFromAppConfig));
-
-        return $serviceManager->get('Application')->bootstrap($listeners);
-    }
-


weierophinney · 2022-06-03T17:54:24Z

src/ConfigProvider.php

+                'request' => 'Request',
+                'response' => 'Response',


I don't think the request and response should be services anymore in v4, but I'm unsure if that's in-scope for this patch.

They definitely do not belong to services.
I plan to drop them not only from container but also from bootstrap event. New event would need to be introduced to do request aware bootstap before routing. I think I did that work in zend-mvc but abandoned when we migrated to laminas.

We discussed it long time ago, and it should be on the old rfc: it would make sense to inject them to the run method with laminas-httphandlerrunner when mvc is migrated to PSR and something similar before that.

weierophinney · 2022-06-03T17:55:31Z

src/ConfigProvider.php

+                'EventManagerAwareInitializer' => static function ($first, $second) {
+                    if ($first instanceof ContainerInterface) {
+                        $container = $first;
+                        $instance = $second;
+                    } else {
+                        $container = $second;
+                        $instance = $first;
+                    }
+
+                    if (! $instance instanceof EventManagerAwareInterface) {
+                        return;
+                    }
+
+                    $eventManager = $instance->getEventManager();
+
+                    // If the instance has an EM WITH an SEM composed, do nothing.
+                    if ($eventManager instanceof EventManagerInterface
+                        && $eventManager->getSharedManager() instanceof SharedEventManagerInterface
+                    ) {
+                        return;
+                    }
+
+                    $instance->setEventManager($container->get('EventManager'));
+                },


weierophinney · 2022-06-03T17:56:29Z

src/Service/ApplicationFactory.php

+        $listeners = $container->get('config')[Application::class]['listeners'] ?? [];
+        foreach ($listeners as $listener) {
+            $container->get($listener)->attach($em);
+        }


weierophinney · 2022-06-03T18:00:04Z

src/Service/ViewHelperManagerFactory.php

 use Laminas\View\Helper as ViewHelper;
 use Laminas\View\HelperPluginManager;

-class ViewHelperManagerFactory extends AbstractPluginManagerFactory
+class ViewHelperManagerFactory implements FactoryInterface


Honestly, I'd drop the implements FactoryInterface part. Factories only need to be invokable,and it's another explicit binding to laminas-servicemanager instead of a generic PSR-11 implementation.

I agree.

Factories look for extra input parameters that are only supported by service manager. the $options specifically, while for the $name we do provide decorators for others containers iirc.

I will have a pass dedicated to factories. I think quite a few of them can be safely dropped and replaced by factories provided by components.

weierophinney · 2022-06-03T18:00:27Z

src/Service/ViewHelperManagerFactory.php

    {
        // Configure URL view helper
        $urlFactory = $this->createUrlHelperFactory($services);
-        $plugins->setFactory(ViewHelper\Url::class, $urlFactory);
-        $plugins->setFactory('laminasviewhelperurl', $urlFactory);
+        $managerConfig['aliases']['laminasviewhelperurl'] = ViewHelper\Url::class;


manishranjan-adobe · 2025-01-07T11:03:47Z

Hi @froschdesign When do you plan to release version 4.0.*?

Xerkus added BC Break Feature Removal Work In Progress labels Jun 9, 2020

Xerkus force-pushed the feature/drop-modulemanager-dependency branch from 02f72d5 to b53dfe8 Compare June 12, 2020 11:59

Xerkus linked an issue Jun 12, 2020 that may be closed by this pull request

[WIP]Drop module manager #15

Closed

9 tasks

Xerkus added this to the 4.0.0 milestone Jun 12, 2020

Xerkus force-pushed the feature/drop-modulemanager-dependency branch from b53dfe8 to 86bcde3 Compare June 12, 2020 17:07

Xerkus removed the Work In Progress label Jun 12, 2020

Xerkus marked this pull request as ready for review June 12, 2020 17:11

Xerkus added a commit to Xerkus/laminas-mvc that referenced this pull request Jun 12, 2020

Add basic CHANGELOG entry for laminas#56

e4e6b6c

Signed-off-by: Aleksei Khudiakov <[email protected]>

Xerkus mentioned this pull request Jun 12, 2020

Provide documentation for the accumulated changes towards 4.0 #58

Open

4 tasks

weierophinney reviewed Jun 17, 2020

View reviewed changes

geerteltink changed the base branch from next to 4.0.x September 12, 2020 09:02

Xerkus force-pushed the 4.0.x branch from 9510e9f to 7ff2bfb Compare February 22, 2022 12:56

Xerkus added 12 commits June 4, 2022 01:33

Introduce ConfigProvider and extract ServiceManagerConfig dependency …

d4fb7fe

…config Signed-off-by: Aleksei Khudiakov <[email protected]>

Extract dependency config from ServiceListenerFactory

844b305

Signed-off-by: Aleksei Khudiakov <[email protected]>

Drop ServiceListenerFactory

07c5601

Signed-off-by: Aleksei Khudiakov <[email protected]>

Drop ModuleManagerFactory

dce7e38

Signed-off-by: Aleksei Khudiakov <[email protected]>

Drop ServiceManagerConfig

b367513

Signed-off-by: Aleksei Khudiakov <[email protected]>

Drop ConfigFactory

b2dbce1

Signed-off-by: Aleksei Khudiakov <[email protected]>

Extract Application::init() listeners bits and drop the rest

a0be4db

Signed-off-by: Aleksei Khudiakov <[email protected]>

Update and drop tests for removed functionality

8fc0754

Signed-off-by: Aleksei Khudiakov <[email protected]>

Update integration tests to use ConfigProvider

a61e46a

Signed-off-by: Aleksei Khudiakov <[email protected]>

Disable bad test in DefaultRenderingStrategyTest

aa0caeb

Signed-off-by: Aleksei Khudiakov <[email protected]>

Remove unused imports

482b81f

Signed-off-by: Aleksei Khudiakov <[email protected]>

Drop laminas-modulemanager

0f36590

Signed-off-by: Aleksei Khudiakov <[email protected]>

Xerkus changed the base branch from backup-4.0.x-at-2022-02-24 to 4.0.x June 3, 2022 16:15

Xerkus force-pushed the feature/drop-modulemanager-dependency branch from e4e6b6c to 0f36590 Compare June 3, 2022 16:15

Drop test for empty initial listeners

6943764

Application listeners should be registered by the time `Application` service is retrieved from container. Previously they were registered at the time of `Application::bootstrap()` call. Signed-off-by: Aleksei <[email protected]>

weierophinney approved these changes Jun 3, 2022

View reviewed changes

Xerkus merged commit fb59173 into laminas:4.0.x Jun 3, 2022

froschdesign mentioned this pull request Jun 27, 2022

fix - loadModule reinitializes modules if the module has already been… laminas/laminas-modulemanager#34

Open

Xerkus mentioned this pull request Mar 15, 2023

Drop ApplicationInterface #59

Merged

froschdesign mentioned this pull request Jul 31, 2024

[RFC]: Drop Support for MVC in 3.0 so it can be added later without a BC break laminas/laminas-validator#336

Closed

froschdesign mentioned this pull request Dec 5, 2024

Remove abandoned dependencies (laminas-config, laminas-loader, etc.) laminas/laminas-modulemanager#55

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Drop module manager and add support for config aggregator #56

Drop module manager and add support for config aggregator #56

Xerkus commented Jun 9, 2020 •

edited

Loading

Xerkus commented Jun 12, 2020

weierophinney left a comment

weierophinney Jun 17, 2020

Xerkus Jun 17, 2020

weierophinney Jun 3, 2022

weierophinney Jun 17, 2020

Xerkus Jun 17, 2020

weierophinney Jun 3, 2022

weierophinney Jun 17, 2020

Xerkus Jun 17, 2020

weierophinney Jun 3, 2022

weierophinney Jun 17, 2020

Xerkus Jun 17, 2020

weierophinney Jun 3, 2022

weierophinney Jun 17, 2020

Xerkus Jun 17, 2020

weierophinney Jun 3, 2022

Xerkus commented Jun 3, 2022

weierophinney left a comment

weierophinney Jun 3, 2022

weierophinney Jun 3, 2022

weierophinney Jun 3, 2022

Xerkus Jun 3, 2022

weierophinney Jun 3, 2022

weierophinney Jun 3, 2022

weierophinney Jun 3, 2022

Xerkus Jun 3, 2022 •

edited

Loading

weierophinney Jun 3, 2022

manishranjan-adobe commented Jan 7, 2025

Drop module manager and add support for config aggregator #56

Drop module manager and add support for config aggregator #56

Conversation

Xerkus commented Jun 9, 2020 • edited Loading

Xerkus commented Jun 12, 2020

weierophinney left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Xerkus commented Jun 3, 2022

weierophinney left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Xerkus Jun 3, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

manishranjan-adobe commented Jan 7, 2025

Xerkus commented Jun 9, 2020 •

edited

Loading

Xerkus Jun 3, 2022 •

edited

Loading