From c767df2674c58e9e7fb7da6aeebe40a626cce39b Mon Sep 17 00:00:00 2001 From: Evan Coury Date: Sun, 1 Jul 2012 04:19:02 -0700 Subject: [PATCH 01/10] Setup ControllerLoader consistently - Changes the config key for controllers from 'controller' to 'controllers' and the sub-key 'classes' to 'invokables'. - This commit removes support for DI, but it will be added back in a later commit. - Small cleanup to ServiceManager's circular dep checking. --- src/ServiceManager.php | 57 +++++++++++++++++++++--------------------- 1 file changed, 29 insertions(+), 28 deletions(-) diff --git a/src/ServiceManager.php b/src/ServiceManager.php index 4bf4fd68..ae1d542b 100644 --- a/src/ServiceManager.php +++ b/src/ServiceManager.php @@ -61,7 +61,7 @@ class ServiceManager implements ServiceLocatorInterface /** * Whether or not to share by default - * + * * @var bool */ protected $shareByDefault = true; @@ -105,8 +105,8 @@ public function getAllowOverride() /** * Set flag indicating whether services are shared by default - * - * @param bool $shareByDefault + * + * @param bool $shareByDefault * @return ServiceManager * @throws Exception\RuntimeException if allowOverride is false */ @@ -124,7 +124,7 @@ public function setShareByDefault($shareByDefault) /** * Are services shared by default? - * + * * @return bool */ public function shareByDefault() @@ -152,8 +152,8 @@ public function getThrowExceptionInCreate() /** * Set flag indicating whether to pull from peering manager before attempting creation - * - * @param bool $retrieveFromPeeringManagerFirst + * + * @param bool $retrieveFromPeeringManagerFirst * @return ServiceManager */ public function setRetrieveFromPeeringManagerFirst($retrieveFromPeeringManagerFirst = true) @@ -164,7 +164,7 @@ public function setRetrieveFromPeeringManagerFirst($retrieveFromPeeringManagerFi /** * Should we retrieve from the peering manager prior to attempting to create a service? - * + * * @return bool */ public function retrieveFromPeeringManagerFirst() @@ -546,9 +546,9 @@ public function createScopedServiceManager($peering = self::SCOPE_PARENT) /** * Add a peering relationship - * - * @param ServiceManager $manager - * @param string $peering + * + * @param ServiceManager $manager + * @param string $peering * @return ServiceManager Current instance */ public function addPeeringServiceManager(ServiceManager $manager, $peering = self::SCOPE_PARENT) @@ -582,21 +582,22 @@ protected function canonicalizeName($name) protected function createServiceViaCallback($callable, $cName, $rName) { static $circularDependencyResolver = array(); + $depKey = spl_object_hash($this) . '-' . $cName; - if (isset($circularDependencyResolver[spl_object_hash($this) . '-' . $cName])) { + if (isset($circularDependencyResolver[$depKey])) { $circularDependencyResolver = array(); throw new Exception\CircularDependencyFoundException('Circular dependency for LazyServiceLoader was found for instance ' . $rName); } try { - $circularDependencyResolver[spl_object_hash($this) . '-' . $cName] = true; + $circularDependencyResolver[$depKey] = true; $instance = call_user_func($callable, $this, $cName, $rName); - unset($circularDependencyResolver[spl_object_hash($this) . '-' . $cName]); + unset($circularDependencyResolver[$depKey]); } catch (Exception\ServiceNotFoundException $e) { - unset($circularDependencyResolver[spl_object_hash($this) . '-' . $cName]); + unset($circularDependencyResolver[$depKey]); throw $e; } catch (\Exception $e) { - unset($circularDependencyResolver[spl_object_hash($this) . '-' . $cName]); + unset($circularDependencyResolver[$depKey]); throw new Exception\ServiceNotCreatedException( sprintf('Abstract factory raised an exception when creating "%s"; no instance returned', $rName), $e->getCode(), @@ -627,8 +628,8 @@ public function getRegisteredServices() /** * Attempt to retrieve an instance via a peering manager - * - * @param string $name + * + * @param string $name * @return mixed */ protected function retrieveFromPeeringManager($name) @@ -651,9 +652,9 @@ protected function retrieveFromPeeringManager($name) /** * Attempt to create an instance via an invokable class - * - * @param string $canonicalName - * @param string $requestedName + * + * @param string $canonicalName + * @param string $requestedName * @return null|\stdClass * @throws Exception\ServiceNotCreatedException If resolved class does not exist */ @@ -675,9 +676,9 @@ protected function createFromInvokable($canonicalName, $requestedName) /** * Attempt to create an instance via a factory - * - * @param string $canonicalName - * @param string $requestedName + * + * @param string $canonicalName + * @param string $requestedName * @return mixed * @throws Exception\ServiceNotCreatedException If factory is not callable */ @@ -704,9 +705,9 @@ protected function createFromFactory($canonicalName, $requestedName) /** * Attempt to create an instance via an abstract factory - * - * @param string $canonicalName - * @param string $requestedName + * + * @param string $canonicalName + * @param string $requestedName * @return \stdClass|null * @throws Exception\ServiceNotCreatedException If abstract factory is not callable */ @@ -719,8 +720,8 @@ protected function createFromAbstractFactory($canonicalName, $requestedName) } if ($abstractFactory instanceof AbstractFactoryInterface) { $instance = $this->createServiceViaCallback( - array($abstractFactory, 'createServiceWithName'), - $canonicalName, + array($abstractFactory, 'createServiceWithName'), + $canonicalName, $requestedName ); } elseif (is_callable($abstractFactory)) { From d725137ae7f86c646722083c529f74b548a1a6a9 Mon Sep 17 00:00:00 2001 From: Evan Coury Date: Sun, 1 Jul 2012 09:50:52 -0700 Subject: [PATCH 02/10] Unify the remaining service managers - View Helper Manager - Controller Plugin Manager - Controller Loader --- src/AbstractPluginManager.php | 44 ++++++++++++++++++++--------------- 1 file changed, 25 insertions(+), 19 deletions(-) diff --git a/src/AbstractPluginManager.php b/src/AbstractPluginManager.php index 4367354e..2e0f6566 100644 --- a/src/AbstractPluginManager.php +++ b/src/AbstractPluginManager.php @@ -41,7 +41,7 @@ abstract class AbstractPluginManager extends ServiceManager { /** * Allow overriding by default - * + * * @var bool */ protected $allowOverride = true; @@ -63,14 +63,20 @@ abstract class AbstractPluginManager extends ServiceManager * * Add a default initializer to ensure the plugin is valid after instance * creation. - * - * @param null|ConfigurationInterface $configuration + * + * @param null|ConfigurationInterface $configuration * @return void */ public function __construct(ConfigurationInterface $configuration = null) { parent::__construct($configuration); $this->addInitializer(array($this, 'validatePlugin'), true); + $self = $this; + $this->addInitializer(function ($instance) use ($self) { + if ($instance instanceof ServiceManagerAwareInterface) { + $instance->setServiceManager($self); + } + }); } /** @@ -78,8 +84,8 @@ public function __construct(ConfigurationInterface $configuration = null) * * Checks that the filter loaded is either a valid callback or an instance * of FilterInterface. - * - * @param mixed $plugin + * + * @param mixed $plugin * @return void * @throws Exception\RuntimeException if invalid */ @@ -91,10 +97,10 @@ abstract public function validatePlugin($plugin); * Allows passing an array of options to use when creating the instance. * createFromInvokable() will use these and pass them to the instance * constructor if not null and a non-empty array. - * - * @param string $name - * @param array $options - * @param bool $usePeeringServiceManagers + * + * @param string $name + * @param array $options + * @param bool $usePeeringServiceManagers * @return object */ public function get($name, $options = array(), $usePeeringServiceManagers = true) @@ -115,7 +121,7 @@ public function get($name, $options = array(), $usePeeringServiceManagers = true * * Validates that the service object via validatePlugin() prior to * attempting to register it. - * + * * @param string $name * @param mixed $service * @param bool $shared @@ -134,11 +140,11 @@ public function setService($name, $service, $shared = true) /** * Attempt to create an instance via an invokable class * - * Overrides parent implementation by passing $creationOptions to the + * Overrides parent implementation by passing $creationOptions to the * constructor, if non-null. - * - * @param string $canonicalName - * @param string $requestedName + * + * @param string $canonicalName + * @param string $requestedName * @return null|\stdClass * @throws Exception\ServiceNotCreatedException If resolved class does not exist */ @@ -155,7 +161,7 @@ protected function createFromInvokable($canonicalName, $requestedName) )); } - if (null === $this->creationOptions + if (null === $this->creationOptions || (is_array($this->creationOptions) && empty($this->creationOptions)) ) { $instance = new $invokable(); @@ -169,11 +175,11 @@ protected function createFromInvokable($canonicalName, $requestedName) /** * Determine if a class implements a given interface * - * For PHP versions >= 5.3.7, uses is_subclass_of; otherwise, uses + * For PHP versions >= 5.3.7, uses is_subclass_of; otherwise, uses * reflection to determine the interfaces implemented. - * - * @param string $class - * @param string $type + * + * @param string $class + * @param string $type * @return bool */ protected function isSubclassOf($class, $type) From 2122c53dcba81ca23432ce50aec34a7b483524ef Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Sun, 1 Jul 2012 23:35:32 +0200 Subject: [PATCH 03/10] Simplifying logic and testing implementation of DiAbstractServiceFactory#canCreateServiceWithName --- src/Di/DiAbstractServiceFactory.php | 15 +++------ test/Di/DiAbstractServiceFactoryTest.php | 41 ++++++++++++++++++++++++ 2 files changed, 46 insertions(+), 10 deletions(-) diff --git a/src/Di/DiAbstractServiceFactory.php b/src/Di/DiAbstractServiceFactory.php index 3461c3d5..d57db25d 100644 --- a/src/Di/DiAbstractServiceFactory.php +++ b/src/Di/DiAbstractServiceFactory.php @@ -48,15 +48,10 @@ public function createServiceWithName(ServiceLocatorInterface $serviceLocator, $ */ public function canCreateServiceWithName($name) { - if (null === $this->definedServiceNames) { - $allNames = array_unique(array_merge( - $this->instanceManager->getClasses(), - array_keys($this->instanceManager->getAliases()), - $this->definitions->getClasses() - )); - $this->definedServiceNames = array_flip(array_values($allNames)); - } - - return isset($this->definedServiceNames[$name]); + return $this->instanceManager->hasSharedInstance($name) + || $this->instanceManager->hasAlias($name) + || $this->instanceManager->hasConfiguration($name) + || $this->instanceManager->hasTypePreferences($name) + || $this->definitions->hasClass($name); } } diff --git a/test/Di/DiAbstractServiceFactoryTest.php b/test/Di/DiAbstractServiceFactoryTest.php index dcd4726b..b3222106 100644 --- a/test/Di/DiAbstractServiceFactoryTest.php +++ b/test/Di/DiAbstractServiceFactoryTest.php @@ -54,4 +54,45 @@ public function testCreateServiceWithName() $foo = $this->diAbstractServiceFactory->createServiceWithName($this->mockServiceLocator, 'foo'); $this->assertEquals($this->fooInstance, $foo); } + + /** + * @covers Zend\ServiceManager\Di\DiAbstractServiceFactory::canCreateServiceWithName + */ + public function testCanCreateServiceWithName() + { + $instance = new DiAbstractServiceFactory($this->getMock('Zend\Di\Di')); + $im = $instance->instanceManager(); + + // will check shared instances + $this->assertFalse($instance->canCreateServiceWithName('a-shared-instance-alias')); + $im->addSharedInstance(new \stdClass(), 'a-shared-instance-alias'); + $this->assertTrue($instance->canCreateServiceWithName('a-shared-instance-alias')); + + // will check aliases + $this->assertFalse($instance->canCreateServiceWithName('an-alias')); + $im->addAlias('an-alias', 'stdClass'); + $this->assertTrue($instance->canCreateServiceWithName('an-alias')); + + // will check instance configurations + $this->assertFalse($instance->canCreateServiceWithName(__NAMESPACE__ . '\Non\Existing')); + $im->setConfiguration(__NAMESPACE__ . '\Non\Existing', array('parameters' => array('a' => 'b'))); + $this->assertTrue($instance->canCreateServiceWithName(__NAMESPACE__ . '\Non\Existing')); + + // will check preferences for abstract types + $this->assertFalse($instance->canCreateServiceWithName(__NAMESPACE__ . '\AbstractClass')); + $im->setTypePreference(__NAMESPACE__ . '\AbstractClass', array(__NAMESPACE__ . '\Non\Existing')); + $this->assertTrue($instance->canCreateServiceWithName(__NAMESPACE__ . '\AbstractClass')); + + // will check definitions + $def = $instance->definitions(); + $this->assertFalse($instance->canCreateServiceWithName(__NAMESPACE__ . '\Other\Non\Existing')); + $classDefinition = $this->getMock('Zend\Di\Definition\DefinitionInterface'); + $classDefinition + ->expects($this->any()) + ->method('hasClass') + ->with($this->equalTo(__NAMESPACE__ . '\Other\Non\Existing')) + ->will($this->returnValue(true)); + $def->addDefinition($classDefinition); + $this->assertTrue($instance->canCreateServiceWithName(__NAMESPACE__ . '\Other\Non\Existing')); + } } From 3154c0f68a356bcf50d53e341b742e125dc69edc Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Sun, 1 Jul 2012 23:42:26 +0200 Subject: [PATCH 04/10] Removing unused instance variable --- src/Di/DiAbstractServiceFactory.php | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/Di/DiAbstractServiceFactory.php b/src/Di/DiAbstractServiceFactory.php index d57db25d..d63557e1 100644 --- a/src/Di/DiAbstractServiceFactory.php +++ b/src/Di/DiAbstractServiceFactory.php @@ -8,11 +8,6 @@ class DiAbstractServiceFactory extends DiServiceFactory implements AbstractFactoryInterface { - /** - * @var array instances that can be created by - */ - protected $definedServiceNames; - /** * @param \Zend\Di\Di $di * @param null|string|\Zend\Di\InstanceManager $useServiceLocator From b14349893b47aa5d4441df697fdfdc8ee8866ecc Mon Sep 17 00:00:00 2001 From: Pieter Kokx Date: Mon, 2 Jul 2012 00:15:02 +0200 Subject: [PATCH 05/10] Did some refactoring on the service manager. - We now use the has() method to check on peering service managers first. - Closures are no longer allowed as abstract factories. --- src/ServiceManager.php | 103 +++++++++++++++++------------------- test/ServiceManagerTest.php | 19 +++---- 2 files changed, 60 insertions(+), 62 deletions(-) diff --git a/src/ServiceManager.php b/src/ServiceManager.php index 4bf4fd68..9a55d2bc 100644 --- a/src/ServiceManager.php +++ b/src/ServiceManager.php @@ -28,7 +28,7 @@ class ServiceManager implements ServiceLocatorInterface protected $factories = array(); /** - * @var Closure|AbstractFactoryInterface[] + * @var AbstractFactoryInterface[] */ protected $abstractFactories = array(); @@ -61,7 +61,7 @@ class ServiceManager implements ServiceLocatorInterface /** * Whether or not to share by default - * + * * @var bool */ protected $shareByDefault = true; @@ -105,8 +105,8 @@ public function getAllowOverride() /** * Set flag indicating whether services are shared by default - * - * @param bool $shareByDefault + * + * @param bool $shareByDefault * @return ServiceManager * @throws Exception\RuntimeException if allowOverride is false */ @@ -124,7 +124,7 @@ public function setShareByDefault($shareByDefault) /** * Are services shared by default? - * + * * @return bool */ public function shareByDefault() @@ -152,8 +152,8 @@ public function getThrowExceptionInCreate() /** * Set flag indicating whether to pull from peering manager before attempting creation - * - * @param bool $retrieveFromPeeringManagerFirst + * + * @param bool $retrieveFromPeeringManagerFirst * @return ServiceManager */ public function setRetrieveFromPeeringManagerFirst($retrieveFromPeeringManagerFirst = true) @@ -164,7 +164,7 @@ public function setRetrieveFromPeeringManagerFirst($retrieveFromPeeringManagerFi /** * Should we retrieve from the peering manager prior to attempting to create a service? - * + * * @return bool */ public function retrieveFromPeeringManagerFirst() @@ -221,16 +221,30 @@ public function setFactory($name, $factory, $shared = true) } /** - * @param $factory + * @param AbstractFactoryInterface|string $factory * @param bool $topOfStack + * @throws Exception\InvalidArgumentException if the abstract factory is invalid */ public function addAbstractFactory($factory, $topOfStack = true) { - if (!is_string($factory) && !$factory instanceof AbstractFactoryInterface && !is_callable($factory)) { + if (!is_string($factory) && !$factory instanceof AbstractFactoryInterface) { throw new Exception\InvalidArgumentException( 'Provided abstract factory must be the class name of an abstract factory or an instance of an AbstractFactoryInterface.' ); } + if (is_string($factory)) { + if (!class_exists($factory, true)) { + throw new Exception\InvalidArgumentException( + 'Provided abstract factory must be the class name of an abstract factory or an instance of an AbstractFactoryInterface.' + ); + } + $refl = new \ReflectionClass($factory); + if (!$refl->implementsInterface(__NAMESPACE__ . '\\AbstractFactoryInterface')) { + throw new Exception\InvalidArgumentException( + 'Provided abstract factory must be the class name of an abstract factory or an instance of an AbstractFactoryInterface.' + ); + } + } if ($topOfStack) { array_unshift($this->abstractFactories, $factory); @@ -352,11 +366,11 @@ public function get($name, $usePeeringServiceManagers = true) if (!$instance) { try { $instance = $this->create(array($cName, $rName)); - } catch (\Exception $selfException) { - if (!$selfException instanceof Exception\ServiceNotFoundException && - !$selfException instanceof Exception\ServiceNotCreatedException) { - throw $selfException; + } catch (Exception\ServiceNotFoundException $e) { + if ($usePeeringServiceManagers && !$retrieveFromPeeringManagerFirst) { + $instance = $this->retrieveFromPeeringManager($name); } + } catch (Exception\ServiceNotCreatedException $e) { if ($usePeeringServiceManagers && !$retrieveFromPeeringManagerFirst) { $instance = $this->retrieveFromPeeringManager($name); } @@ -469,11 +483,6 @@ public function has($nameOrAlias, $usePeeringServiceManagers = true) $this->abstractFactory[$index] = $abstractFactory = new $abstractFactory(); } - // Bad abstract factory; skip - if (!$abstractFactory instanceof AbstractFactoryInterface) { - continue; - } - if ($abstractFactory->canCreateServiceWithName($cName, $rName)) { return true; } @@ -546,9 +555,9 @@ public function createScopedServiceManager($peering = self::SCOPE_PARENT) /** * Add a peering relationship - * - * @param ServiceManager $manager - * @param string $peering + * + * @param ServiceManager $manager + * @param string $peering * @return ServiceManager Current instance */ public function addPeeringServiceManager(ServiceManager $manager, $peering = self::SCOPE_PARENT) @@ -627,33 +636,25 @@ public function getRegisteredServices() /** * Attempt to retrieve an instance via a peering manager - * - * @param string $name + * + * @param string $name * @return mixed */ protected function retrieveFromPeeringManager($name) { - $instance = null; foreach ($this->peeringServiceManagers as $peeringServiceManager) { - try { - $instance = $peeringServiceManager->get($name); - } catch (Exception\ServiceNotFoundException $e) { - continue; - } catch (Exception\ServiceNotCreatedException $e) { - continue; - } catch (\Exception $e) { - throw $e; + if ($peeringServiceManager->has($name)) { + return $peeringServiceManager->get($name); } - break; } - return $instance; + return null; } /** * Attempt to create an instance via an invokable class - * - * @param string $canonicalName - * @param string $requestedName + * + * @param string $canonicalName + * @param string $requestedName * @return null|\stdClass * @throws Exception\ServiceNotCreatedException If resolved class does not exist */ @@ -675,9 +676,9 @@ protected function createFromInvokable($canonicalName, $requestedName) /** * Attempt to create an instance via a factory - * - * @param string $canonicalName - * @param string $requestedName + * + * @param string $canonicalName + * @param string $requestedName * @return mixed * @throws Exception\ServiceNotCreatedException If factory is not callable */ @@ -704,9 +705,9 @@ protected function createFromFactory($canonicalName, $requestedName) /** * Attempt to create an instance via an abstract factory - * - * @param string $canonicalName - * @param string $requestedName + * + * @param string $canonicalName + * @param string $requestedName * @return \stdClass|null * @throws Exception\ServiceNotCreatedException If abstract factory is not callable */ @@ -716,15 +717,6 @@ protected function createFromAbstractFactory($canonicalName, $requestedName) // support factories as strings if (is_string($abstractFactory) && class_exists($abstractFactory, true)) { $this->abstractFactories[$index] = $abstractFactory = new $abstractFactory; - } - if ($abstractFactory instanceof AbstractFactoryInterface) { - $instance = $this->createServiceViaCallback( - array($abstractFactory, 'createServiceWithName'), - $canonicalName, - $requestedName - ); - } elseif (is_callable($abstractFactory)) { - $instance = $this->createServiceViaCallback($abstractFactory, $canonicalName, $requestedName); } else { throw new Exception\ServiceNotCreatedException(sprintf( 'While attempting to create %s%s an abstract factory could not produce a valid instance.', @@ -732,6 +724,11 @@ protected function createFromAbstractFactory($canonicalName, $requestedName) ($requestedName ? '(alias: ' . $requestedName . ')' : '') )); } + $instance = $this->createServiceViaCallback( + array($abstractFactory, 'createServiceWithName'), + $canonicalName, + $requestedName + ); if (is_object($instance)) { break; } diff --git a/test/ServiceManagerTest.php b/test/ServiceManagerTest.php index ca9c3baa..2f5afa5c 100644 --- a/test/ServiceManagerTest.php +++ b/test/ServiceManagerTest.php @@ -268,15 +268,6 @@ public function testCreateWithAbstractFactory() $this->assertInstanceOf('ZendTest\ServiceManager\TestAsset\Foo', $this->serviceManager->get('foo')); } - /** - * @covers Zend\ServiceManager\ServiceManager::create - */ - public function testCreateWithCallableAbstractFactory() - { - $this->serviceManager->addAbstractFactory(function () { return new TestAsset\Foo; }); - $this->assertInstanceOf('ZendTest\ServiceManager\TestAsset\Foo', $this->serviceManager->get('foo')); - } - public function testCreateWithInitializerObject() { $this->serviceManager->addInitializer(new TestAsset\FooInitializer(array('foo' => 'bar'))); @@ -445,4 +436,14 @@ public function testPeeringServiceFallbackOnCreateFailure() $bar = $serviceManager->get('ZendTest\ServiceManager\TestAsset\Bar'); $this->assertInstanceOf('ZendTest\ServiceManager\TestAsset\Bar', $bar); } + + public function testDiAbstractServiceFactory() + { + $di = new Di(); + $di->instanceManager()->setParameters('ZendTest\ServiceManager\TestAsset\Bar', array('foo' => array('a'))); + $this->serviceManager->addAbstractFactory(new DiAbstractServiceFactory($di)); + $this->assertTrue($this->serviceManager->has('ZendTest\ServiceManager\TestAsset\Bar', true)); + $bar = $this->serviceManager->get('ZendTest\ServiceManager\TestAsset\Bar', true); + $this->assertInstanceOf('ZendTest\ServiceManager\TestAsset\Bar', $bar); + } } From 7dfcf2897c434b5b43dab6a6ffb4a2869e040fad Mon Sep 17 00:00:00 2001 From: Pieter Kokx Date: Mon, 2 Jul 2012 00:55:55 +0200 Subject: [PATCH 06/10] Made the factory interface consistent with the service manager. --- src/AbstractFactoryInterface.php | 2 +- src/Di/DiAbstractServiceFactory.php | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/AbstractFactoryInterface.php b/src/AbstractFactoryInterface.php index 57bc5d18..6dd154c4 100644 --- a/src/AbstractFactoryInterface.php +++ b/src/AbstractFactoryInterface.php @@ -4,6 +4,6 @@ interface AbstractFactoryInterface { - public function canCreateServiceWithName($name /*, $requestedName */); + public function canCreateServiceWithName($name, $requestedName); public function createServiceWithName(ServiceLocatorInterface $serviceLocator, $name /*, $requestedName */); } diff --git a/src/Di/DiAbstractServiceFactory.php b/src/Di/DiAbstractServiceFactory.php index d63557e1..5a9e608c 100644 --- a/src/Di/DiAbstractServiceFactory.php +++ b/src/Di/DiAbstractServiceFactory.php @@ -41,12 +41,12 @@ public function createServiceWithName(ServiceLocatorInterface $serviceLocator, $ /** * {@inheritDoc} */ - public function canCreateServiceWithName($name) + public function canCreateServiceWithName($name, $requestedName) { - return $this->instanceManager->hasSharedInstance($name) - || $this->instanceManager->hasAlias($name) - || $this->instanceManager->hasConfiguration($name) - || $this->instanceManager->hasTypePreferences($name) - || $this->definitions->hasClass($name); + return $this->instanceManager->hasSharedInstance($requestedName) + || $this->instanceManager->hasAlias($requestedName) + || $this->instanceManager->hasConfiguration($requestedName) + || $this->instanceManager->hasTypePreferences($requestedName) + || $this->definitions->hasClass($requestedName); } } From fed399ace57aedfabd9fdb355dd72874ba93fb3e Mon Sep 17 00:00:00 2001 From: Pieter Kokx Date: Mon, 2 Jul 2012 01:02:03 +0200 Subject: [PATCH 07/10] Fixed the DI tests. --- test/Di/DiAbstractServiceFactoryTest.php | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/test/Di/DiAbstractServiceFactoryTest.php b/test/Di/DiAbstractServiceFactoryTest.php index b3222106..0e5a3853 100644 --- a/test/Di/DiAbstractServiceFactoryTest.php +++ b/test/Di/DiAbstractServiceFactoryTest.php @@ -64,28 +64,28 @@ public function testCanCreateServiceWithName() $im = $instance->instanceManager(); // will check shared instances - $this->assertFalse($instance->canCreateServiceWithName('a-shared-instance-alias')); + $this->assertFalse($instance->canCreateServiceWithName('a-shared-instance-alias', 'a-shared-instance-alias')); $im->addSharedInstance(new \stdClass(), 'a-shared-instance-alias'); - $this->assertTrue($instance->canCreateServiceWithName('a-shared-instance-alias')); + $this->assertTrue($instance->canCreateServiceWithName('a-shared-instance-alias', 'a-shared-instance-alias')); // will check aliases - $this->assertFalse($instance->canCreateServiceWithName('an-alias')); + $this->assertFalse($instance->canCreateServiceWithName('an-alias', 'an-alias')); $im->addAlias('an-alias', 'stdClass'); - $this->assertTrue($instance->canCreateServiceWithName('an-alias')); + $this->assertTrue($instance->canCreateServiceWithName('an-alias', 'an-alias')); // will check instance configurations - $this->assertFalse($instance->canCreateServiceWithName(__NAMESPACE__ . '\Non\Existing')); + $this->assertFalse($instance->canCreateServiceWithName(__NAMESPACE__ . '\Non\Existing', __NAMESPACE__ . '\Non\Existing')); $im->setConfiguration(__NAMESPACE__ . '\Non\Existing', array('parameters' => array('a' => 'b'))); - $this->assertTrue($instance->canCreateServiceWithName(__NAMESPACE__ . '\Non\Existing')); + $this->assertTrue($instance->canCreateServiceWithName(__NAMESPACE__ . '\Non\Existing', __NAMESPACE__ . '\Non\Existing')); // will check preferences for abstract types - $this->assertFalse($instance->canCreateServiceWithName(__NAMESPACE__ . '\AbstractClass')); + $this->assertFalse($instance->canCreateServiceWithName(__NAMESPACE__ . '\AbstractClass', __NAMESPACE__ . '\AbstractClass')); $im->setTypePreference(__NAMESPACE__ . '\AbstractClass', array(__NAMESPACE__ . '\Non\Existing')); - $this->assertTrue($instance->canCreateServiceWithName(__NAMESPACE__ . '\AbstractClass')); + $this->assertTrue($instance->canCreateServiceWithName(__NAMESPACE__ . '\AbstractClass', __NAMESPACE__ . '\AbstractClass')); // will check definitions $def = $instance->definitions(); - $this->assertFalse($instance->canCreateServiceWithName(__NAMESPACE__ . '\Other\Non\Existing')); + $this->assertFalse($instance->canCreateServiceWithName(__NAMESPACE__ . '\Other\Non\Existing', __NAMESPACE__ . '\Other\Non\Existing')); $classDefinition = $this->getMock('Zend\Di\Definition\DefinitionInterface'); $classDefinition ->expects($this->any()) @@ -93,6 +93,6 @@ public function testCanCreateServiceWithName() ->with($this->equalTo(__NAMESPACE__ . '\Other\Non\Existing')) ->will($this->returnValue(true)); $def->addDefinition($classDefinition); - $this->assertTrue($instance->canCreateServiceWithName(__NAMESPACE__ . '\Other\Non\Existing')); + $this->assertTrue($instance->canCreateServiceWithName(__NAMESPACE__ . '\Other\Non\Existing', __NAMESPACE__ . '\Other\Non\Existing')); } } From 1b4ef41dd96ec306bf8424a005e4f1afdfce9c1c Mon Sep 17 00:00:00 2001 From: Pieter Kokx Date: Mon, 2 Jul 2012 01:30:42 +0200 Subject: [PATCH 08/10] Fixed a few errors. Now the unit tests are correct. --- src/ServiceManager.php | 8 ++++---- test/ServiceManagerTest.php | 9 ++++++--- test/TestAsset/FooAbstractFactory.php | 2 +- 3 files changed, 11 insertions(+), 8 deletions(-) diff --git a/src/ServiceManager.php b/src/ServiceManager.php index 9a55d2bc..a20270ff 100644 --- a/src/ServiceManager.php +++ b/src/ServiceManager.php @@ -366,11 +366,11 @@ public function get($name, $usePeeringServiceManagers = true) if (!$instance) { try { $instance = $this->create(array($cName, $rName)); - } catch (Exception\ServiceNotFoundException $e) { + } catch (Exception\ServiceNotFoundException $selfException) { if ($usePeeringServiceManagers && !$retrieveFromPeeringManagerFirst) { $instance = $this->retrieveFromPeeringManager($name); } - } catch (Exception\ServiceNotCreatedException $e) { + } catch (Exception\ServiceNotCreatedException $selfException) { if ($usePeeringServiceManagers && !$retrieveFromPeeringManagerFirst) { $instance = $this->retrieveFromPeeringManager($name); } @@ -462,7 +462,7 @@ public function has($nameOrAlias, $usePeeringServiceManagers = true) list($cName, $rName) = $nameOrAlias; } else { $cName = $this->canonicalizeName($nameOrAlias); - $rName = $cName; + $rName = $nameOrAlias; } $has = ( @@ -717,7 +717,7 @@ protected function createFromAbstractFactory($canonicalName, $requestedName) // support factories as strings if (is_string($abstractFactory) && class_exists($abstractFactory, true)) { $this->abstractFactories[$index] = $abstractFactory = new $abstractFactory; - } else { + } else if (!$abstractFactory instanceof AbstractFactoryInterface) { throw new Exception\ServiceNotCreatedException(sprintf( 'While attempting to create %s%s an abstract factory could not produce a valid instance.', $canonicalName, diff --git a/test/ServiceManagerTest.php b/test/ServiceManagerTest.php index 2f5afa5c..e6f9ab0a 100644 --- a/test/ServiceManagerTest.php +++ b/test/ServiceManagerTest.php @@ -439,10 +439,13 @@ public function testPeeringServiceFallbackOnCreateFailure() public function testDiAbstractServiceFactory() { - $di = new Di(); - $di->instanceManager()->setParameters('ZendTest\ServiceManager\TestAsset\Bar', array('foo' => array('a'))); - $this->serviceManager->addAbstractFactory(new DiAbstractServiceFactory($di)); + $di = $this->getMock('Zend\Di\Di'); + $factory = new DiAbstractServiceFactory($di); + $factory->instanceManager()->setConfiguration('ZendTest\ServiceManager\TestAsset\Bar', array('parameters' => array('foo' => array('a')))); + $this->serviceManager->addAbstractFactory($factory); + $this->assertTrue($this->serviceManager->has('ZendTest\ServiceManager\TestAsset\Bar', true)); + $bar = $this->serviceManager->get('ZendTest\ServiceManager\TestAsset\Bar', true); $this->assertInstanceOf('ZendTest\ServiceManager\TestAsset\Bar', $bar); } diff --git a/test/TestAsset/FooAbstractFactory.php b/test/TestAsset/FooAbstractFactory.php index 544c4b6f..0970d059 100644 --- a/test/TestAsset/FooAbstractFactory.php +++ b/test/TestAsset/FooAbstractFactory.php @@ -7,7 +7,7 @@ class FooAbstractFactory implements AbstractFactoryInterface { - public function canCreateServiceWithName($name) + public function canCreateServiceWithName($name, $requestedName) { } public function createServiceWithName(ServiceLocatorInterface $serviceLocator, $name) From ecb950b51a4d5ac716b177156f1bad8ac1fe3755 Mon Sep 17 00:00:00 2001 From: Evan Coury Date: Sun, 1 Jul 2012 23:16:57 -0700 Subject: [PATCH 09/10] ServiceManager should check for a factory first (before invokables) Currently a lot of tests expect that 'basepath', 'url', and 'doctype' are available invokable helpers. In a working MVC app, we want to use factories for these to inject things like the request, router, route match, doctype setting from config, etc. However, in the tests, these things will not be availble and they're expected to be available as invokables by default. This allows both assumptions to be made at the cost of an isset() per invokable instantiated. There's a chance that 'doctype' is the only one affected by this, which is something I'll look into more later. --- src/ServiceManager.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/ServiceManager.php b/src/ServiceManager.php index ae1d542b..42e52f8f 100644 --- a/src/ServiceManager.php +++ b/src/ServiceManager.php @@ -405,12 +405,12 @@ public function create($name) $cName = $this->canonicalizeName($cName); - if (isset($this->invokableClasses[$cName])) { - $instance = $this->createFromInvokable($cName, $rName); + if (isset($this->factories[$cName])) { + $instance = $this->createFromFactory($cName, $rName); } - if (!$instance && isset($this->factories[$cName])) { - $instance = $this->createFromFactory($cName, $rName); + if (!$instance && isset($this->invokableClasses[$cName])) { + $instance = $this->createFromInvokable($cName, $rName); } if (!$instance && !empty($this->abstractFactories)) { From 7fdbc8029a5d642c0da46ef20b5c853c60c5e3c5 Mon Sep 17 00:00:00 2001 From: Pieter Kokx Date: Mon, 2 Jul 2012 09:57:58 +0200 Subject: [PATCH 10/10] Changes in the AbstractFactoryInterface. - Added the ServiceLocatorInterface as a parameter for the methods in the AbstractFactoryInterface, to allow creation of a lazy abstract factory. - Added a requestedName parameter to all the methods in the AbstractFactoryInterface, since this is what you want in some situations (most notably, the Di as an abstract factory). --- src/AbstractFactoryInterface.php | 4 ++-- src/Di/DiAbstractServiceFactory.php | 4 ++-- src/ServiceManager.php | 2 +- test/Di/DiAbstractServiceFactoryTest.php | 27 +++++++++++++----------- test/TestAsset/FooAbstractFactory.php | 4 ++-- 5 files changed, 22 insertions(+), 19 deletions(-) diff --git a/src/AbstractFactoryInterface.php b/src/AbstractFactoryInterface.php index 6dd154c4..9cbb3876 100644 --- a/src/AbstractFactoryInterface.php +++ b/src/AbstractFactoryInterface.php @@ -4,6 +4,6 @@ interface AbstractFactoryInterface { - public function canCreateServiceWithName($name, $requestedName); - public function createServiceWithName(ServiceLocatorInterface $serviceLocator, $name /*, $requestedName */); + public function canCreateServiceWithName(ServiceLocatorInterface $serviceLocator, $name, $requestedName); + public function createServiceWithName(ServiceLocatorInterface $serviceLocator, $name, $requestedName); } diff --git a/src/Di/DiAbstractServiceFactory.php b/src/Di/DiAbstractServiceFactory.php index 5a9e608c..c3c82077 100644 --- a/src/Di/DiAbstractServiceFactory.php +++ b/src/Di/DiAbstractServiceFactory.php @@ -27,7 +27,7 @@ public function __construct(Di $di, $useServiceLocator = self::USE_SL_NONE) /** * {@inheritDoc} */ - public function createServiceWithName(ServiceLocatorInterface $serviceLocator, $serviceName, $requestedName = null) + public function createServiceWithName(ServiceLocatorInterface $serviceLocator, $serviceName, $requestedName) { $this->serviceLocator = $serviceLocator; if ($requestedName) { @@ -41,7 +41,7 @@ public function createServiceWithName(ServiceLocatorInterface $serviceLocator, $ /** * {@inheritDoc} */ - public function canCreateServiceWithName($name, $requestedName) + public function canCreateServiceWithName(ServiceLocatorInterface $serviceLocator, $name, $requestedName) { return $this->instanceManager->hasSharedInstance($requestedName) || $this->instanceManager->hasAlias($requestedName) diff --git a/src/ServiceManager.php b/src/ServiceManager.php index a20270ff..94d50c1a 100644 --- a/src/ServiceManager.php +++ b/src/ServiceManager.php @@ -483,7 +483,7 @@ public function has($nameOrAlias, $usePeeringServiceManagers = true) $this->abstractFactory[$index] = $abstractFactory = new $abstractFactory(); } - if ($abstractFactory->canCreateServiceWithName($cName, $rName)) { + if ($abstractFactory->canCreateServiceWithName($this, $cName, $rName)) { return true; } } diff --git a/test/Di/DiAbstractServiceFactoryTest.php b/test/Di/DiAbstractServiceFactoryTest.php index 0e5a3853..468033f7 100644 --- a/test/Di/DiAbstractServiceFactoryTest.php +++ b/test/Di/DiAbstractServiceFactoryTest.php @@ -3,7 +3,8 @@ namespace ZendTest\ServiceManager\Di; use Zend\ServiceManager\Di\DiAbstractServiceFactory, -Zend\ServiceManager\Di\DiInstanceManagerProxy; + Zend\ServiceManager\ServiceManager, + Zend\ServiceManager\Di\DiInstanceManagerProxy; class DiAbstractServiceFactoryTest extends \PHPUnit_Framework_TestCase { @@ -51,7 +52,7 @@ public function testConstructor() */ public function testCreateServiceWithName() { - $foo = $this->diAbstractServiceFactory->createServiceWithName($this->mockServiceLocator, 'foo'); + $foo = $this->diAbstractServiceFactory->createServiceWithName($this->mockServiceLocator, 'foo', 'foo'); $this->assertEquals($this->fooInstance, $foo); } @@ -63,29 +64,31 @@ public function testCanCreateServiceWithName() $instance = new DiAbstractServiceFactory($this->getMock('Zend\Di\Di')); $im = $instance->instanceManager(); + $locator = new ServiceManager(); + // will check shared instances - $this->assertFalse($instance->canCreateServiceWithName('a-shared-instance-alias', 'a-shared-instance-alias')); + $this->assertFalse($instance->canCreateServiceWithName($locator, 'a-shared-instance-alias', 'a-shared-instance-alias')); $im->addSharedInstance(new \stdClass(), 'a-shared-instance-alias'); - $this->assertTrue($instance->canCreateServiceWithName('a-shared-instance-alias', 'a-shared-instance-alias')); + $this->assertTrue($instance->canCreateServiceWithName($locator, 'a-shared-instance-alias', 'a-shared-instance-alias')); // will check aliases - $this->assertFalse($instance->canCreateServiceWithName('an-alias', 'an-alias')); + $this->assertFalse($instance->canCreateServiceWithName($locator, 'an-alias', 'an-alias')); $im->addAlias('an-alias', 'stdClass'); - $this->assertTrue($instance->canCreateServiceWithName('an-alias', 'an-alias')); + $this->assertTrue($instance->canCreateServiceWithName($locator, 'an-alias', 'an-alias')); // will check instance configurations - $this->assertFalse($instance->canCreateServiceWithName(__NAMESPACE__ . '\Non\Existing', __NAMESPACE__ . '\Non\Existing')); + $this->assertFalse($instance->canCreateServiceWithName($locator, __NAMESPACE__ . '\Non\Existing', __NAMESPACE__ . '\Non\Existing')); $im->setConfiguration(__NAMESPACE__ . '\Non\Existing', array('parameters' => array('a' => 'b'))); - $this->assertTrue($instance->canCreateServiceWithName(__NAMESPACE__ . '\Non\Existing', __NAMESPACE__ . '\Non\Existing')); + $this->assertTrue($instance->canCreateServiceWithName($locator, __NAMESPACE__ . '\Non\Existing', __NAMESPACE__ . '\Non\Existing')); // will check preferences for abstract types - $this->assertFalse($instance->canCreateServiceWithName(__NAMESPACE__ . '\AbstractClass', __NAMESPACE__ . '\AbstractClass')); + $this->assertFalse($instance->canCreateServiceWithName($locator, __NAMESPACE__ . '\AbstractClass', __NAMESPACE__ . '\AbstractClass')); $im->setTypePreference(__NAMESPACE__ . '\AbstractClass', array(__NAMESPACE__ . '\Non\Existing')); - $this->assertTrue($instance->canCreateServiceWithName(__NAMESPACE__ . '\AbstractClass', __NAMESPACE__ . '\AbstractClass')); + $this->assertTrue($instance->canCreateServiceWithName($locator, __NAMESPACE__ . '\AbstractClass', __NAMESPACE__ . '\AbstractClass')); // will check definitions $def = $instance->definitions(); - $this->assertFalse($instance->canCreateServiceWithName(__NAMESPACE__ . '\Other\Non\Existing', __NAMESPACE__ . '\Other\Non\Existing')); + $this->assertFalse($instance->canCreateServiceWithName($locator, __NAMESPACE__ . '\Other\Non\Existing', __NAMESPACE__ . '\Other\Non\Existing')); $classDefinition = $this->getMock('Zend\Di\Definition\DefinitionInterface'); $classDefinition ->expects($this->any()) @@ -93,6 +96,6 @@ public function testCanCreateServiceWithName() ->with($this->equalTo(__NAMESPACE__ . '\Other\Non\Existing')) ->will($this->returnValue(true)); $def->addDefinition($classDefinition); - $this->assertTrue($instance->canCreateServiceWithName(__NAMESPACE__ . '\Other\Non\Existing', __NAMESPACE__ . '\Other\Non\Existing')); + $this->assertTrue($instance->canCreateServiceWithName($locator, __NAMESPACE__ . '\Other\Non\Existing', __NAMESPACE__ . '\Other\Non\Existing')); } } diff --git a/test/TestAsset/FooAbstractFactory.php b/test/TestAsset/FooAbstractFactory.php index 0970d059..d9086696 100644 --- a/test/TestAsset/FooAbstractFactory.php +++ b/test/TestAsset/FooAbstractFactory.php @@ -7,10 +7,10 @@ class FooAbstractFactory implements AbstractFactoryInterface { - public function canCreateServiceWithName($name, $requestedName) + public function canCreateServiceWithName(ServiceLocatorInterface $serviceLocator, $name, $requestedName) { } - public function createServiceWithName(ServiceLocatorInterface $serviceLocator, $name) + public function createServiceWithName(ServiceLocatorInterface $serviceLocator, $name, $requestedName) { return new Foo; }