fix delegators to allow usage in plugin managers #5175

Closed
wants to merge 11 commits into
from
@@ -532,25 +532,7 @@ public function create($name)
}
if (isset($this->delegators[$cName])) {
- $serviceManager = $this;
- $additionalDelegators = count($this->delegators[$cName]) - 1;
- $creationCallback = function () use ($serviceManager, $rName, $cName) {
- return $serviceManager->doCreate($rName, $cName);
- };
-
- for ($i = 0; $i < $additionalDelegators; $i += 1) {
- $creationCallback = $this->createDelegatorCallback(
- $this->delegators[$cName][$i],
- $rName,
- $cName,
- $creationCallback
- );
- }
-
- /* @var $delegatorFactory DelegatorFactoryInterface */
- $delegatorFactory = $this->get($this->delegators[$cName][$i]);
-
- return $delegatorFactory->createDelegatorWithName($this, $cName, $rName, $creationCallback);
+ return $this->createDelegatorFromFactory($cName, $rName);
}
return $this->doCreate($rName, $cName);
@@ -559,22 +541,21 @@ public function create($name)
/**
* Creates a callback that uses a delegator to create a service
*
- * @param string $delegatorFactoryName name of the delegator factory service
- * @param string $rName requested service name
- * @param string $cName canonical service name
- * @param callable $creationCallback callback that is responsible for instantiating the service
+ * @param DelegatorFactoryInterface|callable $delegatorFactory the delegator factory
+ * @param string $rName requested service name
+ * @param string $cName canonical service name
+ * @param callable $creationCallback callback for instantiating the real service
*
* @return callable
*/
- private function createDelegatorCallback($delegatorFactoryName, $rName, $cName, $creationCallback)
+ private function createDelegatorCallback($delegatorFactory, $rName, $cName, $creationCallback)
{
$serviceManager = $this;
- return function () use ($serviceManager, $delegatorFactoryName, $rName, $cName, $creationCallback) {
- /* @var $delegatorFactory DelegatorFactoryInterface */
- $delegatorFactory = $serviceManager->get($delegatorFactoryName);
-
- return $delegatorFactory->createDelegatorWithName($serviceManager, $cName, $rName, $creationCallback);
+ return function () use ($serviceManager, $delegatorFactory, $rName, $cName, $creationCallback) {
+ return $delegatorFactory instanceof DelegatorFactoryInterface
+ ? $delegatorFactory->createDelegatorWithName($serviceManager, $cName, $rName, $creationCallback)
+ : $delegatorFactory($serviceManager, $cName, $rName, $creationCallback);
};
}
@@ -1046,6 +1027,50 @@ protected function createFromAbstractFactory($canonicalName, $requestedName)
}
/**
+ * @param $canonicalName
+ * @param $requestedName
+ * @return mixed
+ * @throws Exception\ServiceNotCreatedException
+ */
+ protected function createDelegatorFromFactory($canonicalName, $requestedName)
@Ocramius

Ocramius Sep 26, 2013

Member

While this is indeed clean, consider that I inlined the code for performance reasons

@stefanotorresi

stefanotorresi Sep 27, 2013

Contributor

hmmm but it's 40 lines of code now, plus there are separated methods for each service type. this way it's not only cleaner, it's more consistent too.

are you sure the performance gain is worth it? maybe there is even another way to improve that aspect?

@Ocramius

Ocramius Sep 30, 2013

Member

Hrm... it's probably ok to keep the method separate then, yep.

+ {
+ $serviceManager = $this;
+ $delegatorsCount = count($this->delegators[$canonicalName]);
+ $creationCallback = function () use ($serviceManager, $requestedName, $canonicalName) {
+ return $serviceManager->doCreate($requestedName, $canonicalName);
+ };
+
+ for ($i = 0; $i < $delegatorsCount; $i += 1) {
+
+ $delegatorFactory = $this->delegators[$canonicalName][$i];
+
+ if (is_string($delegatorFactory)) {
+ $delegatorFactory = !$this->has($delegatorFactory) && class_exists($delegatorFactory, true) ?
+ new $delegatorFactory
+ : $this->get($delegatorFactory);
+ $this->delegators[$canonicalName][$i] = $delegatorFactory;
+ }
+
+ if (!$delegatorFactory instanceof DelegatorFactoryInterface && !is_callable($delegatorFactory)) {
+ throw new Exception\ServiceNotCreatedException(sprintf(
+ 'While attempting to create %s%s an invalid factory was registered for this instance type.',
+ $canonicalName,
+ ($requestedName ? '(alias: ' . $requestedName . ')' : '')
+ ));
+ }
+
+ $creationCallback = $this->createDelegatorCallback(
+ $delegatorFactory,
+ $requestedName,
+ $canonicalName,
+ $creationCallback
+ );
+ }
+
+ return $creationCallback($serviceManager, $canonicalName, $requestedName, $creationCallback);
+ }
+
+ /**
* Checks if the object has this class as one of its parents
*
* @see https://bugs.php.net/bug.php?id=53727
@@ -17,6 +17,7 @@
use ZendTest\ServiceManager\TestAsset\FooCounterAbstractFactory;
use ZendTest\ServiceManager\TestAsset\FooPluginManager;
+use ZendTest\ServiceManager\TestAsset\MockSelfReturningDelegatorFactory;
class AbstractPluginManagerTest extends \PHPUnit_Framework_TestCase
{
@@ -117,4 +118,78 @@ public function testCallableObjectWithMutableCreationOptions()
$method->setAccessible(true);
$method->invoke($this->pluginManager, $callable, 'foo', 'bar');
}
+
+ public function testValidatePluginIsCalledWithDelegatorFactoryIfItsAService()
+ {
+ $pluginManager = $this->getMockForAbstractClass('Zend\ServiceManager\AbstractPluginManager');
+ $delegatorFactory = $this->getMock('Zend\\ServiceManager\\DelegatorFactoryInterface');
+
+ $pluginManager->setService('delegator-factory', $delegatorFactory);
+ $pluginManager->addDelegator('foo-service', 'delegator-factory');
+
+ $pluginManager->expects($this->once())
+ ->method('validatePlugin')
+ ->with($delegatorFactory);
+
+ $pluginManager->create('foo-service');
+ }
+
+ public function testSingleDelegatorUsage()
+ {
+ $delegatorFactory = $this->getMock('Zend\\ServiceManager\\DelegatorFactoryInterface');
+ $pluginManager = $this->getMockForAbstractClass('Zend\ServiceManager\AbstractPluginManager');
+ $realService = $this->getMock('stdClass', array(), array(), 'RealService');
+ $delegator = $this->getMock('stdClass', array(), array(), 'Delegator');
+
+ $delegatorFactory
+ ->expects($this->once())
+ ->method('createDelegatorWithName')
+ ->with(
+ $pluginManager,
+ 'fooservice',
+ 'foo-service',
+ $this->callback(function ($callback) use ($realService) {
+ if (!is_callable($callback)) {
+ return false;
+ }
+
+ return call_user_func($callback) === $realService;
+ })
+ )
+ ->will($this->returnValue($delegator));
+
+ $pluginManager->setFactory('foo-service', function() use ($realService) {
+ return $realService;
+ });
+ $pluginManager->addDelegator('foo-service', $delegatorFactory);
+
+ $pluginManager->expects($this->once())
+ ->method('validatePlugin')
+ ->with($delegator);
+
+ $this->assertSame($delegator, $pluginManager->get('foo-service'));
+ }
+
+ public function testMultipleDelegatorsUsage()
+ {
+ $pluginManager = $this->getMockForAbstractClass('Zend\ServiceManager\AbstractPluginManager');
+
+ $fooDelegator = new MockSelfReturningDelegatorFactory();
+ $barDelegator = new MockSelfReturningDelegatorFactory();
+
+ $pluginManager->addDelegator('foo-service', $fooDelegator);
+ $pluginManager->addDelegator('foo-service', $barDelegator);
+ $pluginManager->setInvokableClass('foo-service', 'stdClass');
+
+ $pluginManager->expects($this->once())
+ ->method('validatePlugin')
+ ->with($barDelegator);
+
+ $this->assertSame($barDelegator, $pluginManager->get('foo-service'));
+ $this->assertCount(1, $barDelegator->instances);
+ $this->assertCount(1, $fooDelegator->instances);
+ $this->assertInstanceOf('stdClass', array_shift($fooDelegator->instances));
+ $this->assertSame($fooDelegator, array_shift($barDelegator->instances));
+
+ }
}
@@ -753,6 +753,7 @@ public function testRetrieveServiceFromPeeringServiceManagerIfretrieveFromPeerin
/**
* @covers Zend\ServiceManager\ServiceManager::create
+ * @covers Zend\ServiceManager\ServiceManager::createDelegatorFromFactory
* @covers Zend\ServiceManager\ServiceManager::createDelegatorCallback
* @covers Zend\ServiceManager\ServiceManager::addDelegator
*/
@@ -783,13 +784,12 @@ public function testUsesDelegatorWhenAvailable()
)
->will($this->returnValue($delegator));
- //die(var_dump($this->serviceManager));
-
$this->assertSame($delegator, $this->serviceManager->create('foo-service'));
}
/**
* @covers Zend\ServiceManager\ServiceManager::create
+ * @covers Zend\ServiceManager\ServiceManager::createDelegatorFromFactory
* @covers Zend\ServiceManager\ServiceManager::createDelegatorCallback
* @covers Zend\ServiceManager\ServiceManager::addDelegator
*/
@@ -810,4 +810,106 @@ public function testUsesMultipleDelegates()
$this->assertInstanceOf('stdClass', array_shift($fooDelegator->instances));
$this->assertSame($fooDelegator, array_shift($barDelegator->instances));
}
+
+ /**
+ * @covers Zend\ServiceManager\ServiceManager::createDelegatorFromFactory
+ */
+ public function testDelegatorFactoryWhenNotRegisteredAsService()
+ {
+ $delegator = $this->getMock('Zend\\ServiceManager\\DelegatorFactoryInterface');
+
+ $this->serviceManager->addDelegator('foo-service', $delegator);
+ $this->serviceManager->setInvokableClass('foo-service', 'stdClass');
+
+ $delegator
+ ->expects($this->once())
+ ->method('createDelegatorWithName')
+ ->with(
+ $this->serviceManager,
+ 'fooservice',
+ 'foo-service',
+ $this->callback(function ($callback) {
+ if (!is_callable($callback)) {
+ return false;
+ }
+
+ $service = call_user_func($callback);
+
+ return $service instanceof \stdClass;
+ })
+ )
+ ->will($this->returnValue($delegator));
+
+ $this->assertSame($delegator, $this->serviceManager->create('foo-service'));
+ }
+
+ /**
+ * @covers Zend\ServiceManager\ServiceManager::create
+ * @covers Zend\ServiceManager\ServiceManager::createDelegatorFromFactory
+ * @covers Zend\ServiceManager\ServiceManager::createDelegatorCallback
+ * @covers Zend\ServiceManager\ServiceManager::addDelegator
+ */
+ public function testMultipleDelegatorFactoriesWhenNotRegisteredAsServices()
+ {
+ $fooDelegator = new MockSelfReturningDelegatorFactory();
+ $barDelegator = new MockSelfReturningDelegatorFactory();
+
+ $this->serviceManager->addDelegator('foo-service', $fooDelegator);
+ $this->serviceManager->addDelegator('foo-service', $barDelegator);
+ $this->serviceManager->setInvokableClass('foo-service', 'stdClass');
+
+ $this->assertSame($barDelegator, $this->serviceManager->create('foo-service'));
+ $this->assertCount(1, $barDelegator->instances);
+ $this->assertCount(1, $fooDelegator->instances);
+ $this->assertInstanceOf('stdClass', array_shift($fooDelegator->instances));
+ $this->assertSame($fooDelegator, array_shift($barDelegator->instances));
+ }
+
+ public function testInvalidDelegatorFactoryThrowsException()
+ {
+ $delegatorFactory = new \stdClass;
+ $this->serviceManager->addDelegator('foo-service', $delegatorFactory);
+
+ try {
+ $this->serviceManager->create('foo-service');
+ $this->fail('Expected exception was not raised');
+ }catch (Exception\ServiceNotCreatedException $expected) {
+ $this->assertRegExp('/invalid factory/', $expected->getMessage());
+ return;
+ }
+ }
+
+ public function testInvalidDelegatorFactoryAmongMultipleOnesThrowsException()
+ {
+ $this->serviceManager->addDelegator('foo-service', new MockSelfReturningDelegatorFactory());
+ $this->serviceManager->addDelegator('foo-service', new MockSelfReturningDelegatorFactory());
+ $this->serviceManager->addDelegator('foo-service', 'stdClass');
+
+ try {
+ $this->serviceManager->create('foo-service');
+ $this->fail('Expected exception was not raised');
+ }catch (Exception\ServiceNotCreatedException $expected) {
+ $this->assertRegExp('/invalid factory/', $expected->getMessage());
+ return;
+ }
+ }
+
+ public function testDelegatorFromCallback()
+ {
+ $realService = $this->getMock('stdClass', array(), array(), 'RealService');
+ $delegator = $this->getMock('stdClass', array(), array(), 'Delegator');
+
+ $delegatorFactoryCallback = function($serviceManager, $cName, $rName, $callback) use ($delegator) {
+ $delegator->real = call_user_func($callback);
+ return $delegator;
+ };
+
+ $this->serviceManager->setFactory('foo-service', function() use ($realService) { return $realService; } );
+ $this->serviceManager->addDelegator('foo-service', $delegatorFactoryCallback);
+
+ $service = $this->serviceManager->create('foo-service');
+
+ $this->assertSame($delegator, $service);
+ $this->assertSame($realService, $service->real);
+ }
}