Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP]Drop module manager #15

Closed
6 of 9 tasks
weierophinney opened this issue Dec 31, 2019 · 2 comments · Fixed by #56
Closed
6 of 9 tasks

[WIP]Drop module manager #15

weierophinney opened this issue Dec 31, 2019 · 2 comments · Fixed by #56

Comments

@weierophinney
Copy link
Member

This PR removes ModuleManager from MVC, removes dependency on its ServiceListener and changes factories for various managers that depended on it to pull config from container instead. Container is now expected to be configured before application is created.

Main container is no longer required to be ServiceManager. To accommodate that change, EventManager initializer is removed. Should users need it, they should implement it in userland code but preferable approach, however, is to inject dependencies explicitly.

Update: Bootstrapper introduced by this PR was dropped as it provided little value, considering conceptual order of early Application lifecycle stages.

Introduced new ApplicationListenerProvider service which is a listener aggregate that provides default listeners and pulls list of listeners from main config: $config[Zend\Mvc\Application::class]['listeners'] = []. Entries are either a container id string or instance of listener aggregate interface. This is one of the parts to be replaced as part of eventual migration to PSR event manager.

Application::init() helper method is dropped. zend-mvc application setup is expected to resemble that of zend-expressive:

// config/config.php
$aggregator = new ConfigAggregator([
    // ...
    Zend\Mvc\ConfigProvider::class,
    Zend\Router\ConfigProvider::class,

    // Default Application module config
    Application\ConfigProvider::class,

    new PhpFileProvider(realpath(__DIR__) . '/autoload/{{,*.}global,{,*.}local}.php'),
    // ...
], $cacheConfig['config_cache_path']);

return $aggregator->getMergedConfig();
// public/index.php
use Zend\Mvc\Application;

// ...
chdir(dirname(__DIR__));
require 'vendor/autoload.php';
 
(function () {
    $container = require 'config/container.php';
    $app = $container->get(Application::class);
    $app->run();
})();

Back in the time, module manager was introduced to provide basic packaging support, module loading, autoloading, config loading and merging. Number of important things happened since then.
Composer emerged and became a standard package manager in php world. It solved packaging and autoloading needs. Module is no longer need to be registered with zend-service manager, located and loaded in order to register autoloading rules it provides.
No longer constrained by autoloading, dedicated ConfigAggregator was able to handle config loading and merging in a simple and clean way.
Since service configuration is available before container is created, Zend\ModuleManager\Listener\ServiceListener for various plugin managers is obsoleted and could be replaced by regular factories. ( This PR did exactly that, look at eg Zend\Mvc\Container\ControllerManagerFactory)

Second most used module feature is Module::onBootstrap(). Just a convenience method for providing listener for zend-mvc bootstrap event. Zend-modulemanager registered those during module loading. Now, with config available early, factories for Zend\Mvc\Application could pull from config and register a list of extra listener aggregates ($config[Zend\Mvc\Application::class]['listeners'] = []). Listener aggregates themselves could be managed by container with all the power and flexibility that gives.

Module can be closely replicated with just a config provider:

namespace MyPackage;

use Zend\EventManager\ListenerAggregateInterface;
use Zend\Mvc\MvcEvent;
use Zend\ServiceManager\Factory\InvokableFactory;
 
class ConfigProvider implements ListenerAggregateInterface
{
   /**
    * Configuration
    */
   public function __invoke() : array
   {
       return [
           Application::class => [
               'listeners' => [
                   static::class,
               ]
           ],
           'dependencies' => [
               'factories' => [
                   static::class => InvokableFactory::class,
               ]
           ]
       ];
   }
   
   public function onBootstrap(MvcEvent $event)
   {
       $event->getApplication();
       // ...
   }

   public function attach(EventManagerInterface $events, $priority = 1)
   {
       $events->attach(MvcEvent::EVENT_BOOTSTRAP, [$this, 'onBootstrap']);
   }
}

Note that this PR provides no convenience wrapper for that use case at this time since upcoming PSR-14 for Event Manager is expected to be accepted before 4.0 is ready.


Overall, dropping module manager from zend-mvc greatly simplifies whole zend-mvc application setup and brings zend-mvc more in line with newer zend-expressive and ConfigProviders usage in Zend Framework components.

TODO

  • Replace ServiceListener as a configurator for plugin managers with factories pulling config from config service
  • Drop module manager
  • Provide alternative way to configure and bootstrap application from config
  • Add ConfigProvider
  • Move all container configuration to ConfigProvider
  • Replace main ServiceManager container with PSR-11 ContainerInterface
    • Drop EventManager initializer
    • Update all factories to PSR-11
  • Document changes

Originally posted by @Xerkus at zendframework/zend-mvc#273

@weierophinney
Copy link
Member Author

Current documentation is quite dated. I think I will leave out documentation update in this PR for the major docs update when more of the significant changes for next are done and merged.


Originally posted by @Xerkus at zendframework/zend-mvc#273 (comment)

@weierophinney
Copy link
Member Author

Any news on this PR? It looks amazing!


Originally posted by @vaclavvanik at zendframework/zend-mvc#273 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants