From 5413057fa1ad4d76675be1267113107efe901fe8 Mon Sep 17 00:00:00 2001 From: fhein Date: Sun, 28 Jan 2018 02:18:57 +0100 Subject: [PATCH 01/28] Adds test that preconfiguration through members gets applied. --- test/ServiceManagerTest.php | 47 +++++++++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/test/ServiceManagerTest.php b/test/ServiceManagerTest.php index 8c559946..51b50e33 100644 --- a/test/ServiceManagerTest.php +++ b/test/ServiceManagerTest.php @@ -283,4 +283,51 @@ public function testFactoryMayBeStaticMethodDescribedByCallableString() $serviceManager = new SimpleServiceManager($config); $this->assertEquals(stdClass::class, get_class($serviceManager->get(stdClass::class))); } + + public function testMemberBasedConfigurationGetsApplied() + { + $sm = new PreconfiguredServiceManager(); + + // will be true if $aliases array is properly setup and + // recursive alias resolution works + $this->assertTrue($sm->has('alias1')); + $this->assertInstanceOf(stdClass::class, $sm->get('alias1')); + + // will be true if $aliases array is properly setup and + // simple alias resolution works + $this->assertTrue($sm->has('alias2')); + $this->assertInstanceOf(stdClass::class, $sm->get('alias2')); + + // will return true if $services array is properly setup + $this->assertTrue($sm->has('service')); + $this->assertInstanceOf(stdClass::class, $sm->get('service')); + + // will be true if factory array is properly setup + $this->assertTrue($sm->has('delegator')); + $this->assertInstanceOf(InvokableObject::class, $sm->get('delegator')); + + // will be true if initializer is present + $this->assertTrue($sm->get('delegator')->initializerPresent); + + // will be true if factory array is properly setup + $this->assertTrue($sm->has('factory')); + $this->assertInstanceOf(InvokableObject::class, $sm->get('factory')); + + // will be true if initializer is present + $this->assertTrue($sm->get('factory')->initializerPresent); + + // will succeed if invokable is properly set up + $this->assertTrue($sm->has('invokable')); + $this->assertInstanceOf(stdClass::class, $sm->get('invokable')); + + // will be true if initializer is present + $this->assertTrue($sm->get('invokable')->initializerPresent); + + // will succeed if abstract factory is available + $this->assertTrue($sm->has('foo')); + $this->assertInstanceOf(Foo::class, $sm->get('foo')); + + // will be true if initializer is present + $this->assertTrue($sm->get('foo')->initializerPresent); + } } From 3b5ddd65b1048ccdbd54bcf15b5b1ae58c03c831 Mon Sep 17 00:00:00 2001 From: fhein Date: Sun, 28 Jan 2018 02:23:24 +0100 Subject: [PATCH 02/28] Adds PreconfiguredServiceManager test asset. --- test/PreconfiguredServiceManager.php | 52 ++++++++++++++++++++++++++++ 1 file changed, 52 insertions(+) create mode 100644 test/PreconfiguredServiceManager.php diff --git a/test/PreconfiguredServiceManager.php b/test/PreconfiguredServiceManager.php new file mode 100644 index 00000000..fb88bc94 --- /dev/null +++ b/test/PreconfiguredServiceManager.php @@ -0,0 +1,52 @@ + 'alias2', + 'alias2' => 'service', + ]; + + protected $factories = [ + 'delegator' => SampleFactory::class, + 'factory' => SampleFactory::class, + ]; + + protected $delegators = [ + 'delegator' => [ + PassthroughDelegatorFactory::class, + ], + ]; + + protected $invokables = [ + 'invokable' => stdClass::class, + ]; + + protected $initializers = [ + TaggingInitializer::class, + ]; + + protected $abstractFactories = [ + AbstractFactoryFoo::class, + ]; + + public function __construct(array $config = []) + { + $this->services = [ + 'service' => new stdClass(), + ]; + parent::__construct($config); + } +} From d51cf4b956ce6988e405001dea660e7630c31895 Mon Sep 17 00:00:00 2001 From: fhein Date: Sun, 28 Jan 2018 02:28:51 +0100 Subject: [PATCH 03/28] Adds TaggingInitializer, AbstractFactoryFoo and PassthroughDelegator test assets. --- test/TestAsset/AbstractFactoryFoo.php | 27 ++++++++++++++++++ .../TestAsset/PassthroughDelegatorFactory.php | 28 +++++++++++++++++++ test/TestAsset/TaggingInitializer.php | 22 +++++++++++++++ 3 files changed, 77 insertions(+) create mode 100644 test/TestAsset/AbstractFactoryFoo.php create mode 100644 test/TestAsset/PassthroughDelegatorFactory.php create mode 100644 test/TestAsset/TaggingInitializer.php diff --git a/test/TestAsset/AbstractFactoryFoo.php b/test/TestAsset/AbstractFactoryFoo.php new file mode 100644 index 00000000..4b991515 --- /dev/null +++ b/test/TestAsset/AbstractFactoryFoo.php @@ -0,0 +1,27 @@ +initializerPresent = true; + } +} From 860d62921cb2448301de70a20c682bc4c9b69900 Mon Sep 17 00:00:00 2001 From: fhein Date: Sun, 28 Jan 2018 02:36:39 +0100 Subject: [PATCH 04/28] Adds test asset PreconfiguredServiceManager. --- .../TestAsset/PreconfiguredServiceManager.php | 52 +++++++++++++++++++ 1 file changed, 52 insertions(+) create mode 100644 test/TestAsset/PreconfiguredServiceManager.php diff --git a/test/TestAsset/PreconfiguredServiceManager.php b/test/TestAsset/PreconfiguredServiceManager.php new file mode 100644 index 00000000..fb88bc94 --- /dev/null +++ b/test/TestAsset/PreconfiguredServiceManager.php @@ -0,0 +1,52 @@ + 'alias2', + 'alias2' => 'service', + ]; + + protected $factories = [ + 'delegator' => SampleFactory::class, + 'factory' => SampleFactory::class, + ]; + + protected $delegators = [ + 'delegator' => [ + PassthroughDelegatorFactory::class, + ], + ]; + + protected $invokables = [ + 'invokable' => stdClass::class, + ]; + + protected $initializers = [ + TaggingInitializer::class, + ]; + + protected $abstractFactories = [ + AbstractFactoryFoo::class, + ]; + + public function __construct(array $config = []) + { + $this->services = [ + 'service' => new stdClass(), + ]; + parent::__construct($config); + } +} From ab26a61d9d0fe74d7a71f5dc4ad17cf9d552ae35 Mon Sep 17 00:00:00 2001 From: fhein Date: Sun, 28 Jan 2018 02:54:29 +0100 Subject: [PATCH 05/28] Adds class Foo test asset. To be generated by AbstractFactoryFoo on requested name 'foo'. --- test/TestAsset/Foo.php | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) create mode 100644 test/TestAsset/Foo.php diff --git a/test/TestAsset/Foo.php b/test/TestAsset/Foo.php new file mode 100644 index 00000000..060a0512 --- /dev/null +++ b/test/TestAsset/Foo.php @@ -0,0 +1,18 @@ +options = $options; + } +} From 1c1f0d64ec7a52cd8c23ed9a14d5659d5ef68138 Mon Sep 17 00:00:00 2001 From: fhein Date: Sun, 28 Jan 2018 03:01:08 +0100 Subject: [PATCH 06/28] Fake. --- test/TestAsset/Foo.php | 1 + 1 file changed, 1 insertion(+) diff --git a/test/TestAsset/Foo.php b/test/TestAsset/Foo.php index 060a0512..408fef7b 100644 --- a/test/TestAsset/Foo.php +++ b/test/TestAsset/Foo.php @@ -14,5 +14,6 @@ class Foo public function __construct($options = null) { $this->options = $options; + } } From f977a4bf4878ce71cb7e133741b619cfadb356dd Mon Sep 17 00:00:00 2001 From: fhein Date: Mon, 29 Jan 2018 16:09:05 +0100 Subject: [PATCH 07/28] Fix: Process preconfigured abstract factories and initializers --- src/ServiceManager.php | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/src/ServiceManager.php b/src/ServiceManager.php index 1f4272dd..3e0cad25 100644 --- a/src/ServiceManager.php +++ b/src/ServiceManager.php @@ -149,6 +149,15 @@ class ServiceManager implements ServiceLocatorInterface public function __construct(array $config = []) { $this->creationContext = $this; + + if (! empty($this->initializers)) { + $this->resolveInitializers($this->initializers); + } + + if (! empty($this->abstractFactories)) { + $this->resolveAbstractFactories($this->abstractFactories); + } + $this->configure($config); } @@ -513,8 +522,12 @@ public function setShared($name, $flag) * * @return void */ - private function resolveAbstractFactories(array $abstractFactories) + private function resolveAbstractFactories(array $abstractFactories, $constructing = false) { + if ($constructing) { + unset($this->abstractfactories); + } + foreach ($abstractFactories as $abstractFactory) { if (is_string($abstractFactory) && class_exists($abstractFactory)) { //Cached string @@ -565,8 +578,11 @@ private function resolveAbstractFactories(array $abstractFactories) * * @return void */ - private function resolveInitializers(array $initializers) + private function resolveInitializers(array $initializers, $constructing = false) { + if ($constructing) { + unset($this->initializers); + } foreach ($initializers as $initializer) { if (is_string($initializer) && class_exists($initializer)) { $initializer = new $initializer(); From bcaf2d52a2316889851866bbc85f7772daf6f611 Mon Sep 17 00:00:00 2001 From: fhein Date: Mon, 29 Jan 2018 16:47:41 +0100 Subject: [PATCH 08/28] Adds test asset TaggingDelegatorFactory. --- test/TestAsset/TaggingDelegatorFactory.php | 29 ++++++++++++++++++++++ 1 file changed, 29 insertions(+) create mode 100644 test/TestAsset/TaggingDelegatorFactory.php diff --git a/test/TestAsset/TaggingDelegatorFactory.php b/test/TestAsset/TaggingDelegatorFactory.php new file mode 100644 index 00000000..412e5d3e --- /dev/null +++ b/test/TestAsset/TaggingDelegatorFactory.php @@ -0,0 +1,29 @@ +delegatorTag = true; + return $object; + } +} From 539cfb4024b364687dbcbf5cd3c93ee3e16a3719 Mon Sep 17 00:00:00 2001 From: fhein Date: Mon, 29 Jan 2018 17:26:52 +0100 Subject: [PATCH 09/28] Completes the fix for abstract factories and initializers. Invokables still open here. --- src/ServiceManager.php | 6 +++--- test/ServiceManagerTest.php | 11 ++++++++--- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/src/ServiceManager.php b/src/ServiceManager.php index 3e0cad25..688bd111 100644 --- a/src/ServiceManager.php +++ b/src/ServiceManager.php @@ -151,11 +151,11 @@ public function __construct(array $config = []) $this->creationContext = $this; if (! empty($this->initializers)) { - $this->resolveInitializers($this->initializers); + $this->resolveInitializers($this->initializers, true); } if (! empty($this->abstractFactories)) { - $this->resolveAbstractFactories($this->abstractFactories); + $this->resolveAbstractFactories($this->abstractFactories, true); } $this->configure($config); @@ -525,7 +525,7 @@ public function setShared($name, $flag) private function resolveAbstractFactories(array $abstractFactories, $constructing = false) { if ($constructing) { - unset($this->abstractfactories); + unset($this->abstractFactories); } foreach ($abstractFactories as $abstractFactory) { diff --git a/test/ServiceManagerTest.php b/test/ServiceManagerTest.php index 51b50e33..0cf882a4 100644 --- a/test/ServiceManagerTest.php +++ b/test/ServiceManagerTest.php @@ -14,7 +14,9 @@ use Zend\ServiceManager\Factory\FactoryInterface; use Zend\ServiceManager\Factory\InvokableFactory; use Zend\ServiceManager\ServiceManager; +use ZendTest\ServiceManager\TestAsset\Foo; use ZendTest\ServiceManager\TestAsset\InvokableObject; +use ZendTest\ServiceManager\TestAsset\PreconfiguredServiceManager; use ZendTest\ServiceManager\TestAsset\SimpleServiceManager; /** @@ -316,12 +318,15 @@ public function testMemberBasedConfigurationGetsApplied() // will be true if initializer is present $this->assertTrue($sm->get('factory')->initializerPresent); + // same problem with invokables + // see next commit + // will succeed if invokable is properly set up - $this->assertTrue($sm->has('invokable')); - $this->assertInstanceOf(stdClass::class, $sm->get('invokable')); +// $this->assertTrue($sm->has('invokable')); +// $this->assertInstanceOf(stdClass::class, $sm->get('invokable')); // will be true if initializer is present - $this->assertTrue($sm->get('invokable')->initializerPresent); +// $this->assertTrue($sm->get('invokable')->initializerPresent); // will succeed if abstract factory is available $this->assertTrue($sm->has('foo')); From 3b8155eccb8731cc7b1bafbc0a4ab6dec7112136 Mon Sep 17 00:00:00 2001 From: fhein Date: Mon, 29 Jan 2018 17:37:26 +0100 Subject: [PATCH 10/28] Fixes completed for abstract factories and initializers. --- src/ServiceManager.php | 7 +++++++ test/ServiceManagerTest.php | 1 - 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/src/ServiceManager.php b/src/ServiceManager.php index 688bd111..d967181a 100644 --- a/src/ServiceManager.php +++ b/src/ServiceManager.php @@ -76,6 +76,13 @@ class ServiceManager implements ServiceLocatorInterface */ protected $factories = []; + /** + * A list of invokable classes + * + * @var string[]|callable[] + */ + protected $invokables = []; + /** * @var Initializer\InitializerInterface[]|callable[] */ diff --git a/test/ServiceManagerTest.php b/test/ServiceManagerTest.php index 0cf882a4..34ea8ebf 100644 --- a/test/ServiceManagerTest.php +++ b/test/ServiceManagerTest.php @@ -320,7 +320,6 @@ public function testMemberBasedConfigurationGetsApplied() // same problem with invokables // see next commit - // will succeed if invokable is properly set up // $this->assertTrue($sm->has('invokable')); // $this->assertInstanceOf(stdClass::class, $sm->get('invokable')); From d2650ee1252f55b45bbf06cc7f4fe5d280e1e6b2 Mon Sep 17 00:00:00 2001 From: fhein Date: Mon, 29 Jan 2018 18:08:56 +0100 Subject: [PATCH 11/28] Removed tests which explicitly tested if invokables are converted to aliases and factories. Added test to show that invokables override delegators and delegators. Added a test to show that preconfigured invokables are ignored. Fixed that problems. --- src/ServiceManager.php | 72 +++-------------- test/CommonServiceLocatorBehaviorsTrait.php | 2 +- test/ServiceManagerTest.php | 85 +++++++++++---------- test/TestAsset/Foo.php | 1 - 4 files changed, 57 insertions(+), 103 deletions(-) diff --git a/src/ServiceManager.php b/src/ServiceManager.php index d967181a..f8a0242a 100644 --- a/src/ServiceManager.php +++ b/src/ServiceManager.php @@ -20,6 +20,7 @@ use Zend\ServiceManager\Exception\InvalidArgumentException; use Zend\ServiceManager\Exception\ServiceNotCreatedException; use Zend\ServiceManager\Exception\ServiceNotFoundException; +use Zend\ServiceManager\Factory\InvokableFactory; /** * Service Manager. @@ -249,7 +250,7 @@ public function build($name, array $options = null) public function has($name) { $name = isset($this->resolvedAliases[$name]) ? $this->resolvedAliases[$name] : $name; - $found = isset($this->services[$name]) || isset($this->factories[$name]); + $found = isset($this->services[$name]) || isset($this->factories[$name]) || isset($this->invokables[$name]); if ($found) { return $found; @@ -333,19 +334,8 @@ public function configure(array $config) $this->services = $config['services'] + $this->services; } - if (isset($config['invokables']) && ! empty($config['invokables'])) { - $aliases = $this->createAliasesForInvokables($config['invokables']); - $factories = $this->createFactoriesForInvokables($config['invokables']); - - if (! empty($aliases)) { - $config['aliases'] = (isset($config['aliases'])) - ? array_merge($config['aliases'], $aliases) - : $aliases; - } - - $config['factories'] = (isset($config['factories'])) - ? array_merge($config['factories'], $factories) - : $factories; + if (! empty($config['invokables'])) { + $this->invokables = $config['invokables'] + $this->invokables; } if (isset($config['factories'])) { @@ -441,7 +431,7 @@ public function setAlias($alias, $target) */ public function setInvokableClass($name, $class = null) { - $this->configure(['invokables' => [$name => $class ?: $name]]); + $this->configure(['invokables' => [ $name => $class ?? $name]]); } /** @@ -677,7 +667,7 @@ private function resolveNewAliasesWithPreviouslyResolvedAliases(array $aliases) * @return callable * @throws ServiceNotFoundException */ - private function getFactory($name) + private function getFactory(&$name) { $factory = isset($this->factories[$name]) ? $this->factories[$name] : null; @@ -698,8 +688,12 @@ private function getFactory($name) $factory = explode('::', $factory); } return $factory; + } elseif (isset($this->invokables[$name])) { + $name = $this->invokables[$name]; + return new InvokableFactory(); } + // Check abstract factories foreach ($this->abstractFactories as $abstractFactory) { if ($abstractFactory->canCreate($this->creationContext, $name)) { @@ -853,52 +847,6 @@ private function createLazyServiceDelegatorFactory() return $this->lazyServicesDelegator; } - /** - * Create aliases for invokable classes. - * - * If an invokable service name does not match the class it maps to, this - * creates an alias to the class (which will later be mapped as an - * invokable factory). - * - * @param array $invokables - * @return array - */ - private function createAliasesForInvokables(array $invokables) - { - $aliases = []; - foreach ($invokables as $name => $class) { - if ($name === $class) { - continue; - } - $aliases[$name] = $class; - } - return $aliases; - } - - /** - * Create invokable factories for invokable classes. - * - * If an invokable service name does not match the class it maps to, this - * creates an invokable factory entry for the class name; otherwise, it - * creates an invokable factory for the entry name. - * - * @param array $invokables - * @return array - */ - private function createFactoriesForInvokables(array $invokables) - { - $factories = []; - foreach ($invokables as $name => $class) { - if ($name === $class) { - $factories[$name] = Factory\InvokableFactory::class; - continue; - } - - $factories[$class] = Factory\InvokableFactory::class; - } - return $factories; - } - /** * Determine if one or more services already exist in the container. * diff --git a/test/CommonServiceLocatorBehaviorsTrait.php b/test/CommonServiceLocatorBehaviorsTrait.php index 2350e090..540dde33 100644 --- a/test/CommonServiceLocatorBehaviorsTrait.php +++ b/test/CommonServiceLocatorBehaviorsTrait.php @@ -640,7 +640,7 @@ public function testCanInjectInvokables() $container = $this->createContainer(); $container->setInvokableClass('foo', stdClass::class); $this->assertTrue($container->has('foo')); - $this->assertTrue($container->has(stdClass::class)); +// $this->assertTrue($container->has(stdClass::class)); $foo = $container->get('foo'); $this->assertInstanceOf(stdClass::class, $foo); } diff --git a/test/ServiceManagerTest.php b/test/ServiceManagerTest.php index 34ea8ebf..bee0534a 100644 --- a/test/ServiceManagerTest.php +++ b/test/ServiceManagerTest.php @@ -18,6 +18,8 @@ use ZendTest\ServiceManager\TestAsset\InvokableObject; use ZendTest\ServiceManager\TestAsset\PreconfiguredServiceManager; use ZendTest\ServiceManager\TestAsset\SimpleServiceManager; +use ZendTest\ServiceManager\TestAsset\SampleFactory; +use ZendTest\ServiceManager\TestAsset\TaggingDelegatorFactory; /** * @covers \Zend\ServiceManager\ServiceManager @@ -153,40 +155,6 @@ public function testShareability($sharedByDefault, $serviceShared, $serviceDefin $this->assertEquals($shouldBeSameInstance, $a === $b); } - public function testMapsOneToOneInvokablesAsInvokableFactoriesInternally() - { - $config = [ - 'invokables' => [ - InvokableObject::class => InvokableObject::class, - ], - ]; - - $serviceManager = new ServiceManager($config); - $this->assertAttributeSame([ - InvokableObject::class => InvokableFactory::class, - ], 'factories', $serviceManager, 'Invokable object factory not found'); - } - - public function testMapsNonSymmetricInvokablesAsAliasPlusInvokableFactory() - { - $config = [ - 'invokables' => [ - 'Invokable' => InvokableObject::class, - ], - ]; - - $serviceManager = new ServiceManager($config); - $this->assertAttributeSame([ - 'Invokable' => InvokableObject::class, - ], 'aliases', $serviceManager, 'Alias not found for non-symmetric invokable'); - $this->assertAttributeSame([ - InvokableObject::class => InvokableFactory::class, - ], 'factories', $serviceManager, 'Factory not found for non-symmetric invokable target'); - } - - /** - * @depends testMapsNonSymmetricInvokablesAsAliasPlusInvokableFactory - */ public function testSharedServicesReferencingInvokableAliasShouldBeHonored() { $config = [ @@ -318,14 +286,12 @@ public function testMemberBasedConfigurationGetsApplied() // will be true if initializer is present $this->assertTrue($sm->get('factory')->initializerPresent); - // same problem with invokables - // see next commit // will succeed if invokable is properly set up -// $this->assertTrue($sm->has('invokable')); -// $this->assertInstanceOf(stdClass::class, $sm->get('invokable')); + $this->assertTrue($sm->has('invokable')); + $this->assertInstanceOf(stdClass::class, $sm->get('invokable')); // will be true if initializer is present -// $this->assertTrue($sm->get('invokable')->initializerPresent); + $this->assertTrue($sm->get('invokable')->initializerPresent); // will succeed if abstract factory is available $this->assertTrue($sm->has('foo')); @@ -334,4 +300,45 @@ public function testMemberBasedConfigurationGetsApplied() // will be true if initializer is present $this->assertTrue($sm->get('foo')->initializerPresent); } + + public function testInvokablesShouldNotOverrideFactoriesAndDelegators() + { + $sm = new ServiceManager([ + 'factories' => [ + // produce InvokableObject + 'factory1' => SampleFactory::class, + 'factory2' => SampleFactory::class, + ], + 'delegators' => [ + 'factory1' => [ + // produce tagged invokable object + TaggingDelegatorFactory::class, + ] + ] + ]); + + $object1 = $sm->build('factory1'); + // assert delegated object is produced by delegator factory + $this->assertTrue(isset($object1->delegatorTag)); + $this->assertInstanceOf(InvokableObject::class, $object1); + + + $object2 = $sm->build('factory2'); + // assert delegated object is produced by SampleFactory + $this->assertFalse(isset($object2->delegatorTag)); + $this->assertInstanceOf(InvokableObject::class, $object2); + + $sm->setInvokableClass('factory1', stdClass::class); + $sm->setInvokableClass('factory2', stdClass::class); + + $object1 = $sm->build('factory1'); + // assert delegated object is still produced by delegator factory + $this->assertTrue(isset($object1->delegatorTag)); + $this->assertInstanceOf(InvokableObject::class, $object1); + + $object2 = $sm->build('factory2'); + // assert delegated object is still produced by SampleFactory + $this->assertFalse(isset($object2->delegatorTag)); + $this->assertInstanceOf(InvokableObject::class, $object2); + } } diff --git a/test/TestAsset/Foo.php b/test/TestAsset/Foo.php index 408fef7b..060a0512 100644 --- a/test/TestAsset/Foo.php +++ b/test/TestAsset/Foo.php @@ -14,6 +14,5 @@ class Foo public function __construct($options = null) { $this->options = $options; - } } From c94e1cffabad41504706501d431b9fc61c9b4fd5 Mon Sep 17 00:00:00 2001 From: fhein Date: Mon, 29 Jan 2018 18:15:25 +0100 Subject: [PATCH 12/28] Parens added. --- src/ServiceManager.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ServiceManager.php b/src/ServiceManager.php index f8a0242a..abe636da 100644 --- a/src/ServiceManager.php +++ b/src/ServiceManager.php @@ -431,7 +431,7 @@ public function setAlias($alias, $target) */ public function setInvokableClass($name, $class = null) { - $this->configure(['invokables' => [ $name => $class ?? $name]]); + $this->configure(['invokables' => [ $name => ($class ?? $name)]]); } /** From 5298ab0329ed066fc2ef18d4e8c27b1a50397fee Mon Sep 17 00:00:00 2001 From: fhein Date: Mon, 29 Jan 2018 18:38:44 +0100 Subject: [PATCH 13/28] Change for PHP 5.6 compatibility. --- src/ServiceManager.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ServiceManager.php b/src/ServiceManager.php index abe636da..a43bed2e 100644 --- a/src/ServiceManager.php +++ b/src/ServiceManager.php @@ -431,7 +431,7 @@ public function setAlias($alias, $target) */ public function setInvokableClass($name, $class = null) { - $this->configure(['invokables' => [ $name => ($class ?? $name)]]); + $this->configure(['invokables' => [ $name => (isset($class) ? $class : $name)]]); } /** From 5aa73ccbb8779bde432ccebf75d16e087a81a08d Mon Sep 17 00:00:00 2001 From: fhein Date: Mon, 29 Jan 2018 19:09:11 +0100 Subject: [PATCH 14/28] Removed $constructing parameter. --- src/ServiceManager.php | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/src/ServiceManager.php b/src/ServiceManager.php index a43bed2e..2ab0dbed 100644 --- a/src/ServiceManager.php +++ b/src/ServiceManager.php @@ -159,11 +159,15 @@ public function __construct(array $config = []) $this->creationContext = $this; if (! empty($this->initializers)) { - $this->resolveInitializers($this->initializers, true); + // null indicates that $this->initializers + // should be used for configuration + $this->resolveInitializers(null); } if (! empty($this->abstractFactories)) { - $this->resolveAbstractFactories($this->abstractFactories, true); + // null indicates that $this->abstractFactories + // should be used for configuration + $this->resolveAbstractFactories(null); } $this->configure($config); @@ -519,9 +523,10 @@ public function setShared($name, $flag) * * @return void */ - private function resolveAbstractFactories(array $abstractFactories, $constructing = false) + private function resolveAbstractFactories(array $abstractFactories = null) { - if ($constructing) { + if ($abstractFactories === null) { + $abstractFactories = $this->abstractFactories; unset($this->abstractFactories); } @@ -575,9 +580,10 @@ private function resolveAbstractFactories(array $abstractFactories, $constructin * * @return void */ - private function resolveInitializers(array $initializers, $constructing = false) + private function resolveInitializers(array $initializers = null) { - if ($constructing) { + if ($initializers === null) { + $initializers = $this->initializers; unset($this->initializers); } foreach ($initializers as $initializer) { From cb3b0b1975329c1757b88f733bc1c64903f04681 Mon Sep 17 00:00:00 2001 From: fhein Date: Mon, 29 Jan 2018 23:52:49 +0100 Subject: [PATCH 15/28] Set $this->configured after call to configure() at construction time. --- src/ServiceManager.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/ServiceManager.php b/src/ServiceManager.php index 2ab0dbed..46ec897e 100644 --- a/src/ServiceManager.php +++ b/src/ServiceManager.php @@ -171,6 +171,7 @@ public function __construct(array $config = []) } $this->configure($config); + $this->configured = true; } /** @@ -381,8 +382,6 @@ public function configure(array $config) $this->resolveInitializers($config['initializers']); } - $this->configured = true; - return $this; } From 8b787f26561a690971d141ca09a405ce02c677ec Mon Sep 17 00:00:00 2001 From: fhein Date: Thu, 1 Feb 2018 00:45:43 +0100 Subject: [PATCH 16/28] Extended benchmarks as of #231 --- benchmarks/BenchAsset/AbstractFactoryFoo.php | 2 +- benchmarks/BenchAsset/DelegatorFactoryFoo.php | 23 ++++ benchmarks/BenchAsset/FactoryFoo.php | 2 +- benchmarks/BenchAsset/InitializerFoo.php | 28 +++++ benchmarks/HasBench.php | 113 ++++++++++++++++++ benchmarks/SetNewServicesBench.php | 87 ++++++++++++-- 6 files changed, 243 insertions(+), 12 deletions(-) create mode 100644 benchmarks/BenchAsset/DelegatorFactoryFoo.php create mode 100644 benchmarks/BenchAsset/InitializerFoo.php create mode 100644 benchmarks/HasBench.php diff --git a/benchmarks/BenchAsset/AbstractFactoryFoo.php b/benchmarks/BenchAsset/AbstractFactoryFoo.php index 68879f68..b64e1525 100644 --- a/benchmarks/BenchAsset/AbstractFactoryFoo.php +++ b/benchmarks/BenchAsset/AbstractFactoryFoo.php @@ -7,8 +7,8 @@ namespace ZendBench\ServiceManager\BenchAsset; -use Zend\ServiceManager\Factory\AbstractFactoryInterface; use Interop\Container\ContainerInterface; +use Zend\ServiceManager\Factory\AbstractFactoryInterface; class AbstractFactoryFoo implements AbstractFactoryInterface { diff --git a/benchmarks/BenchAsset/DelegatorFactoryFoo.php b/benchmarks/BenchAsset/DelegatorFactoryFoo.php new file mode 100644 index 00000000..0f2e5dff --- /dev/null +++ b/benchmarks/BenchAsset/DelegatorFactoryFoo.php @@ -0,0 +1,23 @@ +options = $options; + } +} diff --git a/benchmarks/HasBench.php b/benchmarks/HasBench.php new file mode 100644 index 00000000..6e696d77 --- /dev/null +++ b/benchmarks/HasBench.php @@ -0,0 +1,113 @@ +sm = new ServiceManager([ + 'factories' => [ + 'factory1' => BenchAsset\FactoryFoo::class, + ], + 'invokables' => [ + 'invokable1' => BenchAsset\Foo::class, + ], + 'services' => [ + 'service1' => new \stdClass(), + ], + 'aliases' => [ + 'alias1' => 'service1', + 'recursiveAlias1' => 'alias1', + 'recursiveAlias2' => 'recursiveAlias1', + ], + 'abstract_factories' => [ + BenchAsset\AbstractFactoryFoo::class + ] + ]); + } + + public function benchHasFactory1() + { + // @todo @link https://github.com/phpbench/phpbench/issues/304 + $sm = clone $this->sm; + + $sm->has('factory1'); + } + + public function benchHasInvokable1() + { + // @todo @link https://github.com/phpbench/phpbench/issues/304 + $sm = clone $this->sm; + + $sm->has('invokable1'); + } + + public function benchHasService1() + { + // @todo @link https://github.com/phpbench/phpbench/issues/304 + $sm = clone $this->sm; + + $sm->has('service1'); + } + + public function benchHasAlias1() + { + // @todo @link https://github.com/phpbench/phpbench/issues/304 + $sm = clone $this->sm; + + $sm->has('alias1'); + } + + public function benchHasRecursiveAlias1() + { + // @todo @link https://github.com/phpbench/phpbench/issues/304 + $sm = clone $this->sm; + + $sm->has('recursiveAlias1'); + } + + public function benchHasRecursiveAlias2() + { + // @todo @link https://github.com/phpbench/phpbench/issues/304 + $sm = clone $this->sm; + + $sm->has('recursiveAlias2'); + } + + public function benchHasAbstractFactory() + { + // @todo @link https://github.com/phpbench/phpbench/issues/304 + $sm = clone $this->sm; + + $sm->has('foo'); + } + + public function benchHasNot() + { + // @todo @link https://github.com/phpbench/phpbench/issues/304 + $sm = clone $this->sm; + + $sm->has('42'); + } +} diff --git a/benchmarks/SetNewServicesBench.php b/benchmarks/SetNewServicesBench.php index 9e351060..5cad283d 100644 --- a/benchmarks/SetNewServicesBench.php +++ b/benchmarks/SetNewServicesBench.php @@ -11,6 +11,7 @@ use PhpBench\Benchmark\Metadata\Annotations\Revs; use PhpBench\Benchmark\Metadata\Annotations\Warmup; use Zend\ServiceManager\ServiceManager; +use ZendBench\ServiceManager\BenchAsset\DelegatorFactoryFoo; /** * @Revs(1000) @@ -43,40 +44,106 @@ public function __construct() 'recursiveFactoryAlias1' => 'factoryAlias1', 'recursiveFactoryAlias2' => 'recursiveFactoryAlias1', ], - 'abstract_factories' => [ - BenchAsset\AbstractFactoryFoo::class - ], ]; for ($i = 0; $i <= self::NUM_SERVICES; $i++) { $config['factories']["factory_$i"] = BenchAsset\FactoryFoo::class; $config['aliases']["alias_$i"] = "service_$i"; + $config['abstract_factories'][] = BenchAsset\AbstractFactoryFoo::class; + $config['invokables']['invokable_$i'] = BenchAsset\Foo::class; + $config['delegators']['delegator_$i'] = [ DelegatorFactoryFoo::class ]; } + $this->initializer = new BenchAsset\InitializerFoo(); + $this->abstractFactory = new BenchAsset\AbstractFactoryFoo(); $this->sm = new ServiceManager($config); } - public function benchSetFactory() + + public function benchSetService() { - // @todo @link https://github.com/phpbench/phpbench/issues/304 $sm = clone $this->sm; + $sm->setService('service2', new \stdClass()); + } + /** + * @todo @link https://github.com/phpbench/phpbench/issues/304 + */ + public function benchSetFactory() + { + $sm = clone $this->sm; $sm->setFactory('factory2', BenchAsset\FactoryFoo::class); } + /** + * @todo @link https://github.com/phpbench/phpbench/issues/304 + */ public function benchSetAlias() { - // @todo @link https://github.com/phpbench/phpbench/issues/304 $sm = clone $this->sm; - $sm->setAlias('factoryAlias2', 'factory1'); } - public function benchSetAliasOverrided() + /** + * @todo @link https://github.com/phpbench/phpbench/issues/304 + */ + public function benchOverrideAlias() + { + $sm = clone $this->sm; + $sm->setAlias('recursiveFactoryAlias1', 'factory1'); + } + + /** + * @todo @link https://github.com/phpbench/phpbench/issues/304 + */ + public function benchSetInvokableClass() { - // @todo @link https://github.com/phpbench/phpbench/issues/304 $sm = clone $this->sm; + $sm->setInvokableClass(BenchAsset\Foo::class, BenchAsset\Foo::class); + } - $sm->setAlias('recursiveFactoryAlias1', 'factory1'); + /** + * @todo @link https://github.com/phpbench/phpbench/issues/304 + */ + public function benchAddDelegator() + { + $sm = clone $this->sm; + $sm->addDelegator(BenchAsset\Foo::class, DelegatorFactoryFoo::class); + } + + /** + * @todo @link https://github.com/phpbench/phpbench/issues/304 + */ + public function benchAddInitializerByClassName() + { + $sm = clone $this->sm; + $sm->addInitializer(BenchAsset\InitializerFoo::class); + } + + /** + * @todo @link https://github.com/phpbench/phpbench/issues/304 + */ + public function benchAddInitializerByInstance() + { + $sm = clone $this->sm; + $sm->addInitializer($this->initializer); + } + + /** + * @todo @link https://github.com/phpbench/phpbench/issues/304 + */ + public function benchAddAbstractFactoryByClassName() + { + $sm = clone $this->sm; + $sm->addAbstractFactory(BenchAsset\AbstractFactoryFoo::class); + } + + /** + * @todo @link https://github.com/phpbench/phpbench/issues/304 + */ + public function benchAddAbstractFactoryByInstance() + { + $sm = clone $this->sm; + $sm->addAbstractFactory($this->abstractFactory); } } From 13e0febe2fff4b2e881bcd5b30ecce979be6e4b1 Mon Sep 17 00:00:00 2001 From: fhein Date: Thu, 1 Feb 2018 17:26:23 +0100 Subject: [PATCH 17/28] Deleted an assert which was assuming that invokables are handled as combination of alias and factory, which has changed. --- test/CommonServiceLocatorBehaviorsTrait.php | 1 - 1 file changed, 1 deletion(-) diff --git a/test/CommonServiceLocatorBehaviorsTrait.php b/test/CommonServiceLocatorBehaviorsTrait.php index 540dde33..c63bcd81 100644 --- a/test/CommonServiceLocatorBehaviorsTrait.php +++ b/test/CommonServiceLocatorBehaviorsTrait.php @@ -640,7 +640,6 @@ public function testCanInjectInvokables() $container = $this->createContainer(); $container->setInvokableClass('foo', stdClass::class); $this->assertTrue($container->has('foo')); -// $this->assertTrue($container->has(stdClass::class)); $foo = $container->get('foo'); $this->assertInstanceOf(stdClass::class, $foo); } From 0365c4b46cfdc5f383a3f9f8f2f28e8d4e3f3797 Mon Sep 17 00:00:00 2001 From: fhein Date: Thu, 1 Feb 2018 19:05:35 +0100 Subject: [PATCH 18/28] As requested. --- benchmarks/BenchAsset/DelegatorFactoryFoo.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/benchmarks/BenchAsset/DelegatorFactoryFoo.php b/benchmarks/BenchAsset/DelegatorFactoryFoo.php index 0f2e5dff..c7335e0c 100644 --- a/benchmarks/BenchAsset/DelegatorFactoryFoo.php +++ b/benchmarks/BenchAsset/DelegatorFactoryFoo.php @@ -18,6 +18,6 @@ class DelegatorFactoryFoo implements DelegatorFactoryInterface */ public function __invoke(ContainerInterface $container, $name, callable $callback, array $options = null) { - return $callback($options); + return ($options !== null) ? $callback($options) : $callback(); } } From 63b2302bef579856b580afee7b9c945f9aabecc0 Mon Sep 17 00:00:00 2001 From: fhein Date: Thu, 1 Feb 2018 20:32:58 +0100 Subject: [PATCH 19/28] Split PreconfigurationGetsAppliedTest. Changed tag checking assertObjectHasProperty/assertObjectNotHasProperty --- test/ServiceManagerTest.php | 62 ++++++++++++++++++++++++++++--------- 1 file changed, 48 insertions(+), 14 deletions(-) diff --git a/test/ServiceManagerTest.php b/test/ServiceManagerTest.php index bee0534a..2834ef11 100644 --- a/test/ServiceManagerTest.php +++ b/test/ServiceManagerTest.php @@ -254,7 +254,17 @@ public function testFactoryMayBeStaticMethodDescribedByCallableString() $this->assertEquals(stdClass::class, get_class($serviceManager->get(stdClass::class))); } - public function testMemberBasedConfigurationGetsApplied() + public function testMemberBasedAliasConfugrationGetsApplied() + { + $sm = new PreconfiguredServiceManager(); + + // will be true if $aliases array is properly setup and + // simple alias resolution works + $this->assertTrue($sm->has('alias2')); + $this->assertInstanceOf(stdClass::class, $sm->get('alias2')); + } + + public function testMemberBasedRecursiveAliasConfugrationGetsApplied() { $sm = new PreconfiguredServiceManager(); @@ -262,43 +272,64 @@ public function testMemberBasedConfigurationGetsApplied() // recursive alias resolution works $this->assertTrue($sm->has('alias1')); $this->assertInstanceOf(stdClass::class, $sm->get('alias1')); + } - // will be true if $aliases array is properly setup and - // simple alias resolution works - $this->assertTrue($sm->has('alias2')); - $this->assertInstanceOf(stdClass::class, $sm->get('alias2')); + public function testMemberBasedServiceConfugrationGetsApplied() + { + $sm = new PreconfiguredServiceManager(); // will return true if $services array is properly setup $this->assertTrue($sm->has('service')); $this->assertInstanceOf(stdClass::class, $sm->get('service')); + } + + public function testMemberBasedDelegatorConfugrationGetsApplied() + { + $sm = new PreconfiguredServiceManager(); // will be true if factory array is properly setup $this->assertTrue($sm->has('delegator')); $this->assertInstanceOf(InvokableObject::class, $sm->get('delegator')); // will be true if initializer is present - $this->assertTrue($sm->get('delegator')->initializerPresent); + $this->assertObjectHasAttribute('initializerPresent', $sm->get('delegator')); + } + + public function testMemberBasedFactoryConfugrationGetsApplied() + { + $sm = new PreconfiguredServiceManager(); // will be true if factory array is properly setup $this->assertTrue($sm->has('factory')); $this->assertInstanceOf(InvokableObject::class, $sm->get('factory')); // will be true if initializer is present - $this->assertTrue($sm->get('factory')->initializerPresent); + $this->assertObjectHasAttribute('initializerPresent', $sm->get('delegator')); + } + + public function testMemberBasedInvokableConfigurationGetsApplied() + { + $sm = new PreconfiguredServiceManager(); // will succeed if invokable is properly set up $this->assertTrue($sm->has('invokable')); $this->assertInstanceOf(stdClass::class, $sm->get('invokable')); // will be true if initializer is present - $this->assertTrue($sm->get('invokable')->initializerPresent); + $this->assertObjectHasAttribute('initializerPresent', $sm->get('delegator')); + } + + public function testMemberBasedAbstractFactoryConfigurationGetsApplied() + { + $sm = new PreconfiguredServiceManager(); + // will succeed if abstract factory is available $this->assertTrue($sm->has('foo')); $this->assertInstanceOf(Foo::class, $sm->get('foo')); // will be true if initializer is present - $this->assertTrue($sm->get('foo')->initializerPresent); + $this->assertObjectHasAttribute('initializerPresent', $sm->get('delegator')); } public function testInvokablesShouldNotOverrideFactoriesAndDelegators() @@ -319,13 +350,13 @@ public function testInvokablesShouldNotOverrideFactoriesAndDelegators() $object1 = $sm->build('factory1'); // assert delegated object is produced by delegator factory - $this->assertTrue(isset($object1->delegatorTag)); + $this->assertObjectHasAttribute('delegatorTag', $object1); $this->assertInstanceOf(InvokableObject::class, $object1); $object2 = $sm->build('factory2'); // assert delegated object is produced by SampleFactory - $this->assertFalse(isset($object2->delegatorTag)); + $this->assertObjectNotHasAttribute('delegatorTag', $object2); $this->assertInstanceOf(InvokableObject::class, $object2); $sm->setInvokableClass('factory1', stdClass::class); @@ -333,12 +364,15 @@ public function testInvokablesShouldNotOverrideFactoriesAndDelegators() $object1 = $sm->build('factory1'); // assert delegated object is still produced by delegator factory - $this->assertTrue(isset($object1->delegatorTag)); + $this->assertObjectHasAttribute('delegatorTag', $object1); $this->assertInstanceOf(InvokableObject::class, $object1); $object2 = $sm->build('factory2'); - // assert delegated object is still produced by SampleFactory - $this->assertFalse(isset($object2->delegatorTag)); + // assert delegated object is still produced by SampleFactor + // but not by delegator + $this->assertObjectNotHasAttribute('delegatorTag', $object2); + +// $this->assertFalse(isset($object2->delegatorTag)); $this->assertInstanceOf(InvokableObject::class, $object2); } } From 0d36f509381a86584be9c6e181ff7aa11a9c6fd7 Mon Sep 17 00:00:00 2001 From: fhein Date: Thu, 1 Feb 2018 20:47:27 +0100 Subject: [PATCH 20/28] Replaced unsets with [] assignments. Moved $configured assignment back to configure(). --- src/ServiceManager.php | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/ServiceManager.php b/src/ServiceManager.php index 46ec897e..885a2d9c 100644 --- a/src/ServiceManager.php +++ b/src/ServiceManager.php @@ -171,7 +171,6 @@ public function __construct(array $config = []) } $this->configure($config); - $this->configured = true; } /** @@ -381,7 +380,7 @@ public function configure(array $config) if (isset($config['initializers'])) { $this->resolveInitializers($config['initializers']); } - + $this->configured = true; return $this; } @@ -526,7 +525,7 @@ private function resolveAbstractFactories(array $abstractFactories = null) { if ($abstractFactories === null) { $abstractFactories = $this->abstractFactories; - unset($this->abstractFactories); + $this->abstractFactories = []; } foreach ($abstractFactories as $abstractFactory) { @@ -583,7 +582,7 @@ private function resolveInitializers(array $initializers = null) { if ($initializers === null) { $initializers = $this->initializers; - unset($this->initializers); + $this->initializers = []; } foreach ($initializers as $initializer) { if (is_string($initializer) && class_exists($initializer)) { From e568c9445d5f8ebe06c1d4d5c0186f298a13de75 Mon Sep 17 00:00:00 2001 From: fhein Date: Thu, 1 Feb 2018 20:50:08 +0100 Subject: [PATCH 21/28] As requested. --- benchmarks/SetNewServicesBench.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/benchmarks/SetNewServicesBench.php b/benchmarks/SetNewServicesBench.php index 5cad283d..99a4924d 100644 --- a/benchmarks/SetNewServicesBench.php +++ b/benchmarks/SetNewServicesBench.php @@ -50,8 +50,8 @@ public function __construct() $config['factories']["factory_$i"] = BenchAsset\FactoryFoo::class; $config['aliases']["alias_$i"] = "service_$i"; $config['abstract_factories'][] = BenchAsset\AbstractFactoryFoo::class; - $config['invokables']['invokable_$i'] = BenchAsset\Foo::class; - $config['delegators']['delegator_$i'] = [ DelegatorFactoryFoo::class ]; + $config['invokables']["invokable_$i"] = BenchAsset\Foo::class; + $config['delegators']["delegator_$i"] = [ DelegatorFactoryFoo::class ]; } $this->initializer = new BenchAsset\InitializerFoo(); From a8ba1a3a93fc338c1bb93c34feceb8900c44a66a Mon Sep 17 00:00:00 2001 From: fhein Date: Thu, 1 Feb 2018 20:51:50 +0100 Subject: [PATCH 22/28] Made $invokables private to avoid bc issues. --- src/ServiceManager.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ServiceManager.php b/src/ServiceManager.php index 885a2d9c..a49d0938 100644 --- a/src/ServiceManager.php +++ b/src/ServiceManager.php @@ -82,7 +82,7 @@ class ServiceManager implements ServiceLocatorInterface * * @var string[]|callable[] */ - protected $invokables = []; + private $invokables = []; /** * @var Initializer\InitializerInterface[]|callable[] From 8e91bc6e79ca4f7743bba615aae28aad5b647c5a Mon Sep 17 00:00:00 2001 From: fhein Date: Thu, 1 Feb 2018 21:21:21 +0100 Subject: [PATCH 23/28] Had to revert access to $invokables[] to protected in order to allow preconfiguration by child class. --- src/ServiceManager.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ServiceManager.php b/src/ServiceManager.php index a49d0938..885a2d9c 100644 --- a/src/ServiceManager.php +++ b/src/ServiceManager.php @@ -82,7 +82,7 @@ class ServiceManager implements ServiceLocatorInterface * * @var string[]|callable[] */ - private $invokables = []; + protected $invokables = []; /** * @var Initializer\InitializerInterface[]|callable[] From de467bdc2c2c0535abb00ea18aa32237ccd7ca0f Mon Sep 17 00:00:00 2001 From: fhein Date: Thu, 1 Feb 2018 23:52:55 +0100 Subject: [PATCH 24/28] Moved object creation through factory one level down to avoid using a reference parameter. getFactory was replaced by createObjectThroughFactory. Adjusted delegator creation accordingly. --- src/ServiceManager.php | 34 ++++++++++++++++------------------ 1 file changed, 16 insertions(+), 18 deletions(-) diff --git a/src/ServiceManager.php b/src/ServiceManager.php index 885a2d9c..6cb75d9e 100644 --- a/src/ServiceManager.php +++ b/src/ServiceManager.php @@ -665,43 +665,44 @@ private function resolveNewAliasesWithPreviouslyResolvedAliases(array $aliases) } /** - * Get a factory for the given service name + * Get a factory for the given service name and create an object using + * that factory * * @param string $name * @return callable * @throws ServiceNotFoundException */ - private function getFactory(&$name) + private function createObjectThroughFactory($name, array $options = null) { $factory = isset($this->factories[$name]) ? $this->factories[$name] : null; - $lazyLoaded = false; if (is_string($factory) && class_exists($factory)) { $factory = new $factory(); - $lazyLoaded = true; + if (is_callable($factory)) { + $this->factories[$name] = $factory; + } + return $factory($this->creationContext, $name, $options); } - if (is_callable($factory)) { - if ($lazyLoaded) { - $this->factories[$name] = $factory; + if (! is_callable($factory)) { + if (isset($this->invokables[$name])) { + $invokable = $this->invokables[$name]; + return $options === null ? new $invokable() : new $invokable($options); } + } else { // PHP 5.6 fails on 'class::method' callables unless we explode them: if (PHP_MAJOR_VERSION < 7 && is_string($factory) && strpos($factory, '::') !== false ) { $factory = explode('::', $factory); } - return $factory; - } elseif (isset($this->invokables[$name])) { - $name = $this->invokables[$name]; - return new InvokableFactory(); + return $factory($this->creationContext, $name, $options); } - // Check abstract factories foreach ($this->abstractFactories as $abstractFactory) { if ($abstractFactory->canCreate($this->creationContext, $name)) { - return $abstractFactory; + return $abstractFactory($this->creationContext, $name, $options); } } @@ -720,8 +721,7 @@ private function createDelegatorFromName($name, array $options = null) { $creationCallback = function () use ($name, $options) { // Code is inlined for performance reason, instead of abstracting the creation - $factory = $this->getFactory($name); - return $factory($this->creationContext, $name, $options); + return $this->createObjectThroughFactory($name, $options); }; foreach ($this->delegators[$name] as $index => $delegatorFactory) { @@ -780,9 +780,7 @@ private function doCreate($resolvedName, array $options = null) { try { if (! isset($this->delegators[$resolvedName])) { - // Let's create the service by fetching the factory - $factory = $this->getFactory($resolvedName); - $object = $factory($this->creationContext, $resolvedName, $options); + $object = $this->createObjectThroughFactory($resolvedName, $options); } else { $object = $this->createDelegatorFromName($resolvedName, $options); } From b27f5d01e7c303104eb2e9d06eace648bfd4c0a3 Mon Sep 17 00:00:00 2001 From: fhein Date: Fri, 2 Feb 2018 00:16:12 +0100 Subject: [PATCH 25/28] Changed docblock. --- src/ServiceManager.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/ServiceManager.php b/src/ServiceManager.php index 6cb75d9e..378982b3 100644 --- a/src/ServiceManager.php +++ b/src/ServiceManager.php @@ -666,10 +666,10 @@ private function resolveNewAliasesWithPreviouslyResolvedAliases(array $aliases) /** * Get a factory for the given service name and create an object using - * that factory + * that factory or create invokable if service is invokable * * @param string $name - * @return callable + * @return object * @throws ServiceNotFoundException */ private function createObjectThroughFactory($name, array $options = null) From 44a7b5b488c6d9f8eca6c2e5beefec68f9fdec2a Mon Sep 17 00:00:00 2001 From: fhein Date: Fri, 2 Feb 2018 03:00:26 +0100 Subject: [PATCH 26/28] Removed commented old line of previous implementation. --- test/ServiceManagerTest.php | 2 -- 1 file changed, 2 deletions(-) diff --git a/test/ServiceManagerTest.php b/test/ServiceManagerTest.php index 2834ef11..77d82e44 100644 --- a/test/ServiceManagerTest.php +++ b/test/ServiceManagerTest.php @@ -371,8 +371,6 @@ public function testInvokablesShouldNotOverrideFactoriesAndDelegators() // assert delegated object is still produced by SampleFactor // but not by delegator $this->assertObjectNotHasAttribute('delegatorTag', $object2); - -// $this->assertFalse(isset($object2->delegatorTag)); $this->assertInstanceOf(InvokableObject::class, $object2); } } From 73eadf1648fb9b3b1b5e1c29ceb1faf327389eaf Mon Sep 17 00:00:00 2001 From: fhein Date: Fri, 2 Feb 2018 03:07:01 +0100 Subject: [PATCH 27/28] Removed @covers getFactory, because that does not exist any longer. --- test/CommonServiceLocatorBehaviorsTrait.php | 3 --- 1 file changed, 3 deletions(-) diff --git a/test/CommonServiceLocatorBehaviorsTrait.php b/test/CommonServiceLocatorBehaviorsTrait.php index c63bcd81..fb0ba06b 100644 --- a/test/CommonServiceLocatorBehaviorsTrait.php +++ b/test/CommonServiceLocatorBehaviorsTrait.php @@ -562,9 +562,6 @@ public function testPassingInvalidInitializerTypeViaConfigurationRaisesException ]); } - /** - * @covers \Zend\ServiceManager\ServiceManager::getFactory - */ public function testGetRaisesExceptionWhenNoFactoryIsResolved() { $serviceManager = $this->createContainer(); From 1c58b3ae53569114168d342f2b334aec06999512 Mon Sep 17 00:00:00 2001 From: fhein Date: Tue, 6 Feb 2018 21:02:42 +0100 Subject: [PATCH 28/28] Renamed createObjectTroughFactory to createServiceThroughFactory --- src/ServiceManager.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/ServiceManager.php b/src/ServiceManager.php index 378982b3..e92228f5 100644 --- a/src/ServiceManager.php +++ b/src/ServiceManager.php @@ -672,7 +672,7 @@ private function resolveNewAliasesWithPreviouslyResolvedAliases(array $aliases) * @return object * @throws ServiceNotFoundException */ - private function createObjectThroughFactory($name, array $options = null) + private function createServiceThroughFactory($name, array $options = null) { $factory = isset($this->factories[$name]) ? $this->factories[$name] : null; @@ -721,7 +721,7 @@ private function createDelegatorFromName($name, array $options = null) { $creationCallback = function () use ($name, $options) { // Code is inlined for performance reason, instead of abstracting the creation - return $this->createObjectThroughFactory($name, $options); + return $this->createServiceThroughFactory($name, $options); }; foreach ($this->delegators[$name] as $index => $delegatorFactory) { @@ -780,7 +780,7 @@ private function doCreate($resolvedName, array $options = null) { try { if (! isset($this->delegators[$resolvedName])) { - $object = $this->createObjectThroughFactory($resolvedName, $options); + $object = $this->createServiceThroughFactory($resolvedName, $options); } else { $object = $this->createDelegatorFromName($resolvedName, $options); }