Feature/module locator registration #786

Merged
merged 9 commits into from Feb 24, 2012

Projects

None yet

3 participants

@EvanDotPro
Zend Framework member
  • New module listener: you can implement Zend\Module\Consumer\LocatorRegistered in your Module class and the listener will register the module instance as a shared instance in DI.
  • In any DI configured class you can add a setter or constructor parameter now that has a typehint of the Module class you want, as long as it implements LocatorRegistered and it will be passed the active instance of that module class. This is very handy for module options.
  • Additionally, you can parameter hint Zend\Module\Manager and the module manager will be injected into your class.
  • Includes unit tests, 100% coverage.
EvanDotPro added some commits Feb 6, 2012
@EvanDotPro EvanDotPro [Zend\Module] Initial work on LocatorRegistrationListener
Currently will add Di type preferences for every single module.

TODO:
- Make it so devs can decide which modules have a type preference loaded
- Better errer checking on DI objects returned in addTypePreference()
- Unit tests and docblocks
c445718
@EvanDotPro EvanDotPro [Zend\Module] Coding standard fixes a2fad6e
@EvanDotPro EvanDotPro [Zend\Module] Update LocatorRegistrationListener to work properly for…
… multiple modules

Before we were using the MvcEvent which only holds one module at a time. Now the
listener keeps its own internal array of modules that are enabled.
cc14b42
@EvanDotPro EvanDotPro [Zend\Module] Remove extra bootstrap event listener attachement dc1ca15
@EvanDotPro EvanDotPro [Zend\Module] Add docblock comments to locator registration listener …
…stuff
35f3e61
@EvanDotPro EvanDotPro [Zend\Module] Unit tests for LocatorRegistrationListener 02f4b18
@EvanDotPro EvanDotPro [Zend\Module] Add module manager injection preference and unit tests
Now in DI-configured classes you can add a setter or constructor parameter that
is type hinted with Zend\Module\Manager and DI will automatically inject the
module manager to the parameter.
ca3cc47
@Ocramius Ocramius commented on the diff Feb 21, 2012
.../Zend/Module/Listener/LocatorRegistrationListener.php
+ * @var array
+ */
+ protected $listeners = array();
+
+ /**
+ * loadModule
+ *
+ * Check each loaded module to see if it implements LocatorRegistered. If it
+ * does, we add it to an internal array for later.
+ *
+ * @param ModuleEvent $e
+ * @return void
+ */
+ public function loadModule(ModuleEvent $e)
+ {
+ if (!$e->getModule() instanceof LocatorRegistered) {
@Ocramius
Ocramius Feb 21, 2012

Maybe the opposite check? :)

@EvanDotPro
EvanDotPro Feb 22, 2012

I was always taught to return early... I didn't realize when writing this that I was only going to have one line of code in this listener though, heh.

@weierophinney
weierophinney Feb 24, 2012

I'd be consistent, and keep the "return early", to be honest. (I.e., don't change this)

The work of the method is on line 39. Line 36-38 is saying, "I'm not interested if this is not implementing LocatorRegistered."

@Ocramius
Ocramius Feb 24, 2012

@weierophinney logic was clear to me :) I am just for -1 line :)

@Ocramius Ocramius and 1 other commented on an outdated diff Feb 21, 2012
.../Zend/Module/Listener/LocatorRegistrationListener.php
+ /**
+ * loadModulesPost
+ *
+ * Once all the modules are loaded, loop
+ *
+ * @param Event $e
+ * @return void
+ */
+ public function loadModulesPost(Event $e)
+ {
+ $events = StaticEventManager::getInstance();
+ $moduleManager = $e->getTarget();
+ $events->attach('bootstrap', 'bootstrap', function ($e) use ($moduleManager) {
+ $moduleClassName = get_class($moduleManager);
+ $im = $e->getParam('application')->getLocator()->instanceManager();
+ if (!$im->hasTypePreferences($moduleClassName)) {
@Ocramius
Ocramius Feb 21, 2012

As of #794 I'm not sure this should be type preference anymore.
@ralphschindler should this be a shared instance instead?

@EvanDotPro
EvanDotPro Feb 22, 2012

Updated to use shared instance with d0f39c8.

@Ocramius Ocramius commented on an outdated diff Feb 21, 2012
.../Zend/Module/Listener/LocatorRegistrationListener.php
+ *
+ * This is ran during the MVC bootstrap event because it requires access to
+ * the DI container.
+ *
+ * @TODO: Check the application / locator / etc a bit better to make sure
+ * the env looks how we're expecting it to?
+ * @param Event $e
+ * @return void
+ */
+ public function onBootstrap(Event $e)
+ {
+ $im = $e->getParam('application')->getLocator()->instanceManager();
+
+ foreach ($this->modules as $module) {
+ $moduleClassName = get_class($module);
+ if (!$im->hasTypePreferences($moduleClassName)) {
@Ocramius Ocramius and 1 other commented on an outdated diff Feb 21, 2012
...d/Module/Listener/LocatorRegistrationListenerTest.php
+ $test = $this;
+ $this->moduleManager->events()->attach('loadModule', function ($e) use ($test) {
+ $test->module = $e->getModule();
+ }, -1000);
+ $this->moduleManager->loadModules();
+
+ $bootstrap = new Bootstrap(new Config(array('di' => array())));
+ $application = new Application;
+ $bootstrap->bootstrap($application);
+ $locator = $application->getLocator();
+ $typePreferences1 = $locator->instanceManager()->getTypePreferences('ListenerTestModule\Module');
+ $typePreferences2 = $locator->instanceManager()->getTypePreferences('Zend\Module\Manager');
+
+ $this->assertEquals(1, count($typePreferences1));
+ $this->assertInstanceOf('ListenerTestModule\Module', $typePreferences1[0]);
+ $this->assertEquals(spl_object_hash($this->module), spl_object_hash($locator->get('Foo\Bar')->module));
@Ocramius
Ocramius Feb 21, 2012

this is just an assertSame

@EvanDotPro
EvanDotPro Feb 22, 2012

Fixed, thanks.

@Ocramius Ocramius and 1 other commented on an outdated diff Feb 21, 2012
...d/Module/Listener/LocatorRegistrationListenerTest.php
+ $this->moduleManager->loadModules();
+
+ $bootstrap = new Bootstrap(new Config(array('di' => array())));
+ $application = new Application;
+ $bootstrap->bootstrap($application);
+ $locator = $application->getLocator();
+ $typePreferences1 = $locator->instanceManager()->getTypePreferences('ListenerTestModule\Module');
+ $typePreferences2 = $locator->instanceManager()->getTypePreferences('Zend\Module\Manager');
+
+ $this->assertEquals(1, count($typePreferences1));
+ $this->assertInstanceOf('ListenerTestModule\Module', $typePreferences1[0]);
+ $this->assertEquals(spl_object_hash($this->module), spl_object_hash($locator->get('Foo\Bar')->module));
+
+ $this->assertEquals(1, count($typePreferences2));
+ $this->assertInstanceOf('Zend\Module\Manager', $typePreferences2[0]);
+ $this->assertEquals(spl_object_hash($this->moduleManager), spl_object_hash($locator->get('Foo\Bar')->moduleManager));
@Ocramius
Zend Framework member

Sorry for the late comments @EvanDotPro, will ping you later on IRC...

@Ocramius
Zend Framework member

@EvanDotPro: What about addTypePreference vs addSharedInstance?

@Ocramius
Zend Framework member

As discussed with @ralphschindler yesterday, using addSharedInstance is fine.
Shared instances should be used in Zend\Di for already initialized instances (as if it was a registry), while type preference is to aid Zend\Di in selecting what to instantiate...

@Ocramius
Zend Framework member

Nice one :) thx

@weierophinney weierophinney merged commit d0f39c8 into zendframework:master Feb 24, 2012
@Ocramius
Zend Framework member

@weierophinney, as discussed on IRC with @ralphschindler usage of shared instance is correct here, while not in #794

@Ocramius

@EvanDotPro we got a problem here!
The locator in your application is a Zend\Di\Locator, and it has no method instanceManager() defined on it.

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