From f20fd0bb4c48fc3b6cf242dfd55d91b5c42ac1a6 Mon Sep 17 00:00:00 2001 From: Maciej <14310995+falkenhawk@users.noreply.github.com> Date: Thu, 25 Apr 2019 12:32:00 +0200 Subject: [PATCH 01/23] Use sourceCache if isSupported check passes, otherwise proceed without cache and do not throw an exception - so that php-di does not break if there is any problem with apcu - instead of having to wrap ContainerBuilder::build() with try...catch block --- src/ContainerBuilder.php | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/ContainerBuilder.php b/src/ContainerBuilder.php index 1869b9e1e..2c268c781 100644 --- a/src/ContainerBuilder.php +++ b/src/ContainerBuilder.php @@ -150,10 +150,8 @@ public function build() // Mutable definition source $source->setMutableDefinitionSource(new DefinitionArray([], $autowiring)); - if ($this->sourceCache) { - if (!SourceCache::isSupported()) { - throw new \Exception('APCu is not enabled, PHP-DI cannot use it as a cache'); - } + // use cache if isSupported check passes, otherwise proceed without cache and do not throw an exception + if ($this->sourceCache && SourceCache::isSupported()) { // Wrap the source with the cache decorator $source = new SourceCache($source); } From f0a109b27cb3b2940f798f6c7f22780191be028a Mon Sep 17 00:00:00 2001 From: Maciej <14310995+falkenhawk@users.noreply.github.com> Date: Thu, 25 Apr 2019 15:11:15 +0200 Subject: [PATCH 02/23] DefinitionGlob to specify a pattern for definition files - there might be multiple definitions files scattered around in a modularized architecture - instead of listing all the files upfront, specify a glob pattern where to look for the files - directories would be scanned and files read lazily, when a definition is actually requested - to avoid performance hit on init --- src/ContainerBuilder.php | 3 + src/Definition/Source/DefinitionGlob.php | 76 +++++++++++++++++++ src/Definition/Source/SourceChain.php | 2 +- .../Definition/Source/DefinitionGlobTest.php | 53 +++++++++++++ 4 files changed, 133 insertions(+), 1 deletion(-) create mode 100644 src/Definition/Source/DefinitionGlob.php create mode 100644 tests/UnitTest/Definition/Source/DefinitionGlobTest.php diff --git a/src/ContainerBuilder.php b/src/ContainerBuilder.php index 1869b9e1e..587df8bd0 100644 --- a/src/ContainerBuilder.php +++ b/src/ContainerBuilder.php @@ -8,6 +8,7 @@ use DI\Definition\Source\AnnotationBasedAutowiring; use DI\Definition\Source\DefinitionArray; use DI\Definition\Source\DefinitionFile; +use DI\Definition\Source\DefinitionGlob; use DI\Definition\Source\DefinitionSource; use DI\Definition\Source\NoAutowiring; use DI\Definition\Source\ReflectionBasedAutowiring; @@ -141,6 +142,8 @@ public function build() return new DefinitionFile($definitions, $autowiring); } elseif (is_array($definitions)) { return new DefinitionArray($definitions, $autowiring); + } elseif ($definitions instanceof DefinitionGlob) { + $definitions->setAutowiring($autowiring); } return $definitions; diff --git a/src/Definition/Source/DefinitionGlob.php b/src/Definition/Source/DefinitionGlob.php new file mode 100644 index 000000000..4948e675c --- /dev/null +++ b/src/Definition/Source/DefinitionGlob.php @@ -0,0 +1,76 @@ +pattern = $pattern; + + parent::__construct([]); + } + + public function setAutowiring(Autowiring $autowiring) + { + $this->autowiring = $autowiring; + } + + public function getDefinition(string $name, int $startIndex = 0) + { + $this->initialize(); + + return parent::getDefinition($name, $startIndex); + } + + public function getDefinitions() : array + { + $this->initialize(); + + return parent::getDefinitions(); + } + + /** + * Lazy-loading of the definitions. + */ + private function initialize() + { + if ($this->initialized === true) { + return; + } + + $paths = glob($this->pattern, GLOB_BRACE); + foreach ($paths as $path) + { + $this->sources[] = new DefinitionFile($path, $this->autowiring); + } + + $this->initialized = true; + } +} diff --git a/src/Definition/Source/SourceChain.php b/src/Definition/Source/SourceChain.php index b82d618bd..964b5b957 100644 --- a/src/Definition/Source/SourceChain.php +++ b/src/Definition/Source/SourceChain.php @@ -17,7 +17,7 @@ class SourceChain implements DefinitionSource, MutableDefinitionSource /** * @var DefinitionSource[] */ - private $sources; + protected $sources; /** * @var DefinitionSource diff --git a/tests/UnitTest/Definition/Source/DefinitionGlobTest.php b/tests/UnitTest/Definition/Source/DefinitionGlobTest.php new file mode 100644 index 000000000..6f693ac54 --- /dev/null +++ b/tests/UnitTest/Definition/Source/DefinitionGlobTest.php @@ -0,0 +1,53 @@ +getProperty('sources'); + $property->setAccessible(true); + $definitions = $property->getValue($source); + // sources are not initialized (and files are not read) before getting definitions + $this->assertCount(0, $definitions); + + $definitions = $source->getDefinitions(); + $this->assertCount(2, $definitions); + + /** @var ValueDefinition $definition */ + $definition = $definitions['foo']; + $this->assertInstanceOf(ValueDefinition::class, $definition); + $this->assertEquals('bar', $definition->getValue()); + $this->assertInternalType('string', $definition->getValue()); + } + + /** + * @test + */ + public function empty_definitions_for_pattern_not_matching_any_files() + { + $pattern = __DIR__ . '/*/no-definitions-here.php'; + $source = new DefinitionGlob($pattern); + + $definitions = $source->getDefinitions(); + $this->assertCount(0, $definitions); + } +} From d494750044e4031cbb6e55bcd78413d9a67e9cf0 Mon Sep 17 00:00:00 2001 From: Maciej Holyszko <14310995+falkenhawk@users.noreply.github.com> Date: Fri, 26 Apr 2019 09:20:17 +0200 Subject: [PATCH 03/23] Prefer composition over inheritance --- src/Definition/Source/DefinitionGlob.php | 29 ++++++++++--------- src/Definition/Source/SourceChain.php | 2 +- .../Definition/Source/DefinitionGlobTest.php | 6 ++-- 3 files changed, 19 insertions(+), 18 deletions(-) diff --git a/src/Definition/Source/DefinitionGlob.php b/src/Definition/Source/DefinitionGlob.php index 4948e675c..b0bdcd3a8 100644 --- a/src/Definition/Source/DefinitionGlob.php +++ b/src/Definition/Source/DefinitionGlob.php @@ -6,9 +6,8 @@ /** * Reads DI definitions from files matching glob pattern. - * */ -class DefinitionGlob extends SourceChain +class DefinitionGlob implements DefinitionSource { /** * @var bool @@ -16,8 +15,8 @@ class DefinitionGlob extends SourceChain private $initialized = false; /** - * Glob pattern to files containing definitions - * @var string|null + * Glob pattern to files containing definitions. + * @var string */ private $pattern; @@ -26,15 +25,17 @@ class DefinitionGlob extends SourceChain */ private $autowiring; + /** + * @var SourceChain + */ + private $sourceChain; + /** * @param string $pattern Glob pattern to files containing definitions */ - public function __construct($pattern) + public function __construct(string $pattern) { - // Lazy-loading to improve performances $this->pattern = $pattern; - - parent::__construct([]); } public function setAutowiring(Autowiring $autowiring) @@ -46,14 +47,14 @@ public function getDefinition(string $name, int $startIndex = 0) { $this->initialize(); - return parent::getDefinition($name, $startIndex); + return $this->sourceChain->getDefinition($name, $startIndex); } public function getDefinitions() : array { $this->initialize(); - return parent::getDefinitions(); + return $this->sourceChain->getDefinitions(); } /** @@ -66,10 +67,10 @@ private function initialize() } $paths = glob($this->pattern, GLOB_BRACE); - foreach ($paths as $path) - { - $this->sources[] = new DefinitionFile($path, $this->autowiring); - } + $sources = array_map(function ($path) { + return new DefinitionFile($path, $this->autowiring); + }, $paths); + $this->sourceChain = new SourceChain($sources); $this->initialized = true; } diff --git a/src/Definition/Source/SourceChain.php b/src/Definition/Source/SourceChain.php index 964b5b957..b82d618bd 100644 --- a/src/Definition/Source/SourceChain.php +++ b/src/Definition/Source/SourceChain.php @@ -17,7 +17,7 @@ class SourceChain implements DefinitionSource, MutableDefinitionSource /** * @var DefinitionSource[] */ - protected $sources; + private $sources; /** * @var DefinitionSource diff --git a/tests/UnitTest/Definition/Source/DefinitionGlobTest.php b/tests/UnitTest/Definition/Source/DefinitionGlobTest.php index 6f693ac54..23d95939f 100644 --- a/tests/UnitTest/Definition/Source/DefinitionGlobTest.php +++ b/tests/UnitTest/Definition/Source/DefinitionGlobTest.php @@ -23,11 +23,11 @@ public function should_load_definitions_from_glob() $source = new DefinitionGlob($pattern); $class = new ReflectionClass(DefinitionGlob::class); - $property = $class->getProperty('sources'); + $property = $class->getProperty('sourceChain'); $property->setAccessible(true); - $definitions = $property->getValue($source); + $sourceChain = $property->getValue($source); // sources are not initialized (and files are not read) before getting definitions - $this->assertCount(0, $definitions); + $this->assertNull($sourceChain); $definitions = $source->getDefinitions(); $this->assertCount(2, $definitions); From ddf0cbd33cc814b04f9466fb18e8dabf7b79fd3d Mon Sep 17 00:00:00 2001 From: Maciej Holyszko <14310995+falkenhawk@users.noreply.github.com> Date: Sat, 27 Apr 2019 22:23:23 +0200 Subject: [PATCH 04/23] Optimize compiled string definitions - run preg_replace_callback already during compilation --- src/CompiledContainer.php | 18 ++++++++++++++++++ src/Compiler/Compiler.php | 9 ++++++--- 2 files changed, 24 insertions(+), 3 deletions(-) diff --git a/src/CompiledContainer.php b/src/CompiledContainer.php index 88438d736..02a71244f 100644 --- a/src/CompiledContainer.php +++ b/src/CompiledContainer.php @@ -16,6 +16,7 @@ use Invoker\ParameterResolver\DefaultValueResolver; use Invoker\ParameterResolver\NumericArrayResolver; use Invoker\ParameterResolver\ResolverChain; +use Psr\Container\NotFoundExceptionInterface; /** * Compiled version of the dependency injection container. @@ -120,4 +121,21 @@ protected function resolveFactory($callable, $entryName, array $extraParameters throw new InvalidDefinition("Entry \"$entryName\" cannot be resolved: " . $e->getMessage()); } } + + /** + * Resolve a placeholder in string definition + * - wrap possible NotFound exception to conform to the one from StringDefinition::resolveExpression. + */ + protected function resolveStringPlaceholder($placeholder, $entryName) + { + try { + return $this->delegateContainer->get($placeholder); + } catch (NotFoundExceptionInterface $e) { + throw new DependencyException(sprintf( + "Error while parsing string expression for entry '%s': %s", + $entryName, + $e->getMessage() + ), 0, $e); + } + } } diff --git a/src/Compiler/Compiler.php b/src/Compiler/Compiler.php index c967cbc3e..bba07bcbd 100644 --- a/src/Compiler/Compiler.php +++ b/src/Compiler/Compiler.php @@ -182,9 +182,12 @@ private function compileDefinition(string $entryName, Definition $definition) : } break; case $definition instanceof StringDefinition: - $entryName = $this->compileValue($definition->getName()); - $expression = $this->compileValue($definition->getExpression()); - $code = 'return \DI\Definition\StringDefinition::resolveExpression(' . $entryName . ', ' . $expression . ', $this->delegateContainer);'; + $expression = $definition->getExpression(); + $callback = function (array $matches) use ($definition) { + return '\'.$this->resolveStringPlaceholder(' . $this->compileValue($matches[1]) . ', ' . $this->compileValue($definition->getName()) . ').\''; + }; + $value = preg_replace_callback('#\{([^\{\}]+)\}#', $callback, $expression); + $code = 'return \'' . $value . '\';'; break; case $definition instanceof EnvironmentVariableDefinition: $variableName = $this->compileValue($definition->getVariableName()); From 4f3fc0db1dc86ab4d36f09ef94b63359db0179b0 Mon Sep 17 00:00:00 2001 From: Maciej Holyszko <14310995+falkenhawk@users.noreply.github.com> Date: Sun, 28 Apr 2019 21:05:10 +0200 Subject: [PATCH 05/23] Optimize compiled factory definitions - compile final function with injected dependencies - only for closure factories for now (as the first step) --- src/Compiler/Compiler.php | 76 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 76 insertions(+) diff --git a/src/Compiler/Compiler.php b/src/Compiler/Compiler.php index c967cbc3e..70a389d8b 100644 --- a/src/Compiler/Compiler.php +++ b/src/Compiler/Compiler.php @@ -4,6 +4,7 @@ namespace DI\Compiler; +use DI\Container; use DI\Definition\ArrayDefinition; use DI\Definition\DecoratorDefinition; use DI\Definition\Definition; @@ -19,6 +20,8 @@ use DI\Proxy\ProxyFactory; use InvalidArgumentException; use PhpParser\Node\Expr\Closure; +use ReflectionFunction; +use ReflectionFunctionAbstract; use SuperClosure\Analyzer\AstAnalyzer; use SuperClosure\Exception\ClosureAnalysisException; @@ -244,6 +247,35 @@ private function compileDefinition(string $entryName, Definition $definition) : )); } + if ($value instanceof \Closure) { + $reflection = new ReflectionFunction($value); + $requestedEntry = new RequestedEntryHolder($entryName); + $parametersByClassName = [ + 'DI\Factory\RequestedEntry' => $requestedEntry, + ]; + // default non-typehinted parameters + $defaultParameters = [new Reference(Container::class), $requestedEntry]; + + $resolvedParameters = $this->resolveFactoryParameters( + $reflection, + $definition->getParameters(), + $parametersByClassName, + $defaultParameters + ); + + $definitionParameters = array_map(function ($value) { + return $this->compileValue($value); + }, $resolvedParameters); + + $code = sprintf( + 'return (%s)(%s);', + $this->compileValue($value), + implode(', ', $definitionParameters) + ); + break; + } + + // todo optimize other (non-closure) factories $definitionParameters = ''; if (!empty($definition->getParameters())) { $definitionParameters = ', ' . $this->compileValue($definition->getParameters()); @@ -275,6 +307,11 @@ public function compileValue($value) : string throw new InvalidDefinition($errorMessage); } + // one step ahead to skip CompiledContainer->resolveFactory + if ($value instanceof RequestedEntryHolder) { + return 'new DI\Compiler\RequestedEntryHolder(\'' . $value->getName() . '\')'; + } + if ($value instanceof Definition) { // Give it an arbitrary unique name $subEntryName = uniqid('SubEntry'); @@ -333,6 +370,10 @@ private function isCompilable($value) if ($value instanceof \Closure) { return true; } + // added for skipping CompiledContainer->resolveFactory - there is a special case for this in compileValue method + if ($value instanceof RequestedEntryHolder) { + return true; + } if (is_object($value)) { return 'An object was found but objects cannot be compiled'; } @@ -376,4 +417,39 @@ private function compileClosure(\Closure $closure) : string return $code; } + + public function resolveFactoryParameters( + ReflectionFunctionAbstract $reflection, + array $definitionParameters = [], + array $parametersByClassName = [], + array $defaultParameters = [] + ) { + $resolvedParameters = []; + $parameters = $reflection->getParameters(); + + foreach ($parameters as $index => $parameter) { + $name = $parameter->getName(); + if (array_key_exists($name, $definitionParameters)) { + $resolvedParameters[$index] = $definitionParameters[$name]; + continue; + } + + $parameterClass = $parameter->getClass(); + if (!$parameterClass) { + if (array_key_exists($index, $defaultParameters)) { + // take default parameters, when no typehint + $resolvedParameters[$index] = $defaultParameters[$index]; + } + continue; + } + + if (isset($parametersByClassName[$parameterClass->name])) { + $resolvedParameters[$index] = $parametersByClassName[$parameterClass->name]; + } else { + $resolvedParameters[$index] = new Reference($parameterClass->name); + } + } + + return $resolvedParameters; + } } From 59a22168215d68cb57b32b5cb715439bec773b4f Mon Sep 17 00:00:00 2001 From: Maciej Holyszko <14310995+falkenhawk@users.noreply.github.com> Date: Fri, 26 Apr 2019 19:50:47 +0200 Subject: [PATCH 06/23] AnnotationBasedAutowiring with additional options for performance tweaks - adds possibility to disable features which are not used - useAnnotations(boolean) toggle on autowire() helper to enable/disable reading annotations on specific definitions --- src/ContainerBuilder.php | 10 ++- src/Definition/AutowireDefinition.php | 22 ++++++ .../Helper/AutowireDefinitionHelper.php | 30 ++++++++ .../Source/AnnotationBasedAutowiring.php | 76 +++++++++++++++++-- 4 files changed, 129 insertions(+), 9 deletions(-) diff --git a/src/ContainerBuilder.php b/src/ContainerBuilder.php index 1869b9e1e..8679b686c 100644 --- a/src/ContainerBuilder.php +++ b/src/ContainerBuilder.php @@ -56,6 +56,11 @@ class ContainerBuilder */ private $useAnnotations = false; + /** + * @var int + */ + private $annotationsFlags = 0; + /** * @var bool */ @@ -126,7 +131,7 @@ public function build() $sources = array_reverse($this->definitionSources); if ($this->useAnnotations) { - $autowiring = new AnnotationBasedAutowiring($this->ignorePhpDocErrors); + $autowiring = new AnnotationBasedAutowiring($this->ignorePhpDocErrors, $this->annotationsFlags); $sources[] = $autowiring; } elseif ($this->useAutowiring) { $autowiring = new ReflectionBasedAutowiring; @@ -239,11 +244,12 @@ public function useAutowiring(bool $bool) : self * * @return $this */ - public function useAnnotations(bool $bool) : self + public function useAnnotations(bool $bool, int $flags = 0) : self { $this->ensureNotLocked(); $this->useAnnotations = $bool; + $this->annotationsFlags = $flags; return $this; } diff --git a/src/Definition/AutowireDefinition.php b/src/Definition/AutowireDefinition.php index 25dc3ecbc..6bd753319 100644 --- a/src/Definition/AutowireDefinition.php +++ b/src/Definition/AutowireDefinition.php @@ -9,4 +9,26 @@ */ class AutowireDefinition extends ObjectDefinition { + /** + * @var bool|null + */ + protected $useAnnotations; + + /** + * Enable/disable reading annotations for this definition, regardless of a container configuration. + * @param bool $flag + */ + public function useAnnotations(bool $flag = true) + { + $this->useAnnotations = $flag; + } + + /** + * Returns boolean if the useAnnotation flag was explicitly set, otherwise null. + * @return bool|null + */ + public function isUsingAnnotations() + { + return $this->useAnnotations; + } } diff --git a/src/Definition/Helper/AutowireDefinitionHelper.php b/src/Definition/Helper/AutowireDefinitionHelper.php index 8dfa4e2d5..f65395862 100644 --- a/src/Definition/Helper/AutowireDefinitionHelper.php +++ b/src/Definition/Helper/AutowireDefinitionHelper.php @@ -5,6 +5,7 @@ namespace DI\Definition\Helper; use DI\Definition\AutowireDefinition; +use DI\Definition\Definition; /** * Helps defining how to create an instance of a class using autowiring. @@ -15,6 +16,8 @@ class AutowireDefinitionHelper extends CreateDefinitionHelper { const DEFINITION_CLASS = AutowireDefinition::class; + protected $useAnnotations; + /** * Defines a value for a specific argument of the constructor. * @@ -69,4 +72,31 @@ public function methodParameter(string $method, $parameter, $value) return $this; } + + /** + * Define if entry should use annotation reader for reading dependencies. + * This is turned off by default if autowire() helper is used, and turned on if entry is not defined explicitly in the di config. + * @param bool $useAnnotations + * @return $this + */ + public function useAnnotations(bool $useAnnotations = true) + { + $this->useAnnotations = $useAnnotations; + + return $this; + } + + /** + * @return AutowireDefinition + */ + public function getDefinition(string $entryName) : Definition + { + /** @var AutowireDefinition $definition */ + $definition = parent::getDefinition($entryName); + if ($this->useAnnotations !== null) { + $definition->useAnnotations($this->useAnnotations); + } + + return $definition; + } } diff --git a/src/Definition/Source/AnnotationBasedAutowiring.php b/src/Definition/Source/AnnotationBasedAutowiring.php index f33590133..1a0df9235 100644 --- a/src/Definition/Source/AnnotationBasedAutowiring.php +++ b/src/Definition/Source/AnnotationBasedAutowiring.php @@ -6,6 +6,7 @@ use DI\Annotation\Inject; use DI\Annotation\Injectable; +use DI\Definition\AutowireDefinition; use DI\Definition\Exception\InvalidAnnotation; use DI\Definition\ObjectDefinition; use DI\Definition\ObjectDefinition\MethodInjection; @@ -32,6 +33,25 @@ */ class AnnotationBasedAutowiring implements DefinitionSource, Autowiring { + // Annotations configuration flags: + // enable on implicit definitions + const IMPLICIT = 1; + // enable on all autowire definitions (which are written in DI config) by default + const EXPLICIT = 2; + // read @Injectable annotations for classes + const INJECTABLE = 4; + // read @Inject annotations for properties + const PROPERTIES = 8; + // read @Inject annotations for methods' parameters + const METHODS = 16; + // all options enabled + const ALL = 31; + + /** + * @var int + */ + private $flags; + /** * @var Reader */ @@ -47,9 +67,10 @@ class AnnotationBasedAutowiring implements DefinitionSource, Autowiring */ private $ignorePhpDocErrors; - public function __construct($ignorePhpDocErrors = false) + public function __construct($ignorePhpDocErrors = false, int $flags = 0) { $this->ignorePhpDocErrors = (bool) $ignorePhpDocErrors; + $this->flags = $flags > 0 ? $flags : self::ALL; // all flags turned on by default } public function autowire(string $name, ObjectDefinition $definition = null) @@ -61,16 +82,35 @@ public function autowire(string $name, ObjectDefinition $definition = null) } $definition = $definition ?: new ObjectDefinition($name); + $useAnnotations = $definition instanceof AutowireDefinition + ? ($definition->isUsingAnnotations() ?? ($this->flags & self::EXPLICIT)) + : ($this->flags & self::IMPLICIT); - $class = new ReflectionClass($className); + $class = null; + if ($useAnnotations && $this->flags >= self::INJECTABLE) { + $class = new ReflectionClass($className); - $this->readInjectableAnnotation($class, $definition); + if ($this->flags & self::INJECTABLE) { + $this->readInjectableAnnotation($class, $definition); + } - // Browse the class properties looking for annotated properties - $this->readProperties($class, $definition); + // Browse the class properties looking for annotated properties + if ($this->flags & self::PROPERTIES) { + $this->readProperties($class, $definition); + } - // Browse the object's methods looking for annotated methods - $this->readMethods($class, $definition); + // Browse the object's methods looking for annotated methods + if ($this->flags & self::METHODS) { + $this->readMethods($class, $definition); + } + } + + // constructor parameters should always be read, even if annotations are disabled (completely or i.a. for methods) + // so that it behaves at least as ReflectionBasedAutowiring + if (!$useAnnotations || !($this->flags & self::METHODS)) { + $class = $class ?? new ReflectionClass($className); + $this->readConstructor($class, $definition); + } return $definition; } @@ -166,6 +206,28 @@ private function readMethods(ReflectionClass $class, ObjectDefinition $objectDef } } + /** + * Browse the object's constructor parameters and inject dependencies. + */ + private function readConstructor(ReflectionClass $class, ObjectDefinition $definition) + { + if (!($constructor = $class->getConstructor()) || !$constructor->isPublic()) { + return; + } + + $parameters = []; + foreach ($constructor->getParameters() as $index => $parameter) { + $entryName = $this->getMethodParameter($index, $parameter, []); + + if ($entryName !== null) { + $parameters[$index] = new Reference($entryName); + } + } + + $constructorInjection = MethodInjection::constructor($parameters); + $definition->completeConstructorInjection($constructorInjection); + } + /** * @param ReflectionMethod $method * From 1cce07a9e1ac2e345ee6c9650b873d258af320a8 Mon Sep 17 00:00:00 2001 From: Maciej Holyszko <14310995+falkenhawk@users.noreply.github.com> Date: Mon, 29 Apr 2019 18:46:42 +0200 Subject: [PATCH 07/23] ServiceLocator - Serving "lazy" dependencies for classes using ServiceSubscriberInterface. - A ServiceSubscriber exposes its dependencies via static getSubscribedServices() method. - A ServiceLocator instance could then be injected into a class via constructor or a property - the instance would be already configured with dependences read from getSubscribedServices(), but the dependences won't be instantiated until first get - that's how "laziness" is introduced - DI\Definition\Reference checks if it's a ServiceLocator entry by comparing its name with DI\Definition\Reference::$serviceLocatorClass - Reference definitions are passed with additional parameter - $requestingName which generally points to name of the class which implements ServiceSubscriberInterface - to resolve ServiceLocator for that class - Suggested as a lightweight alternative for heavyweight proxies from ocramius/proxy-manager --- src/Compiler/Compiler.php | 12 ++ src/Definition/Reference.php | 60 +++++++++- src/Definition/ServiceLocatorDefinition.php | 97 ++++++++++++++++ .../Source/AnnotationBasedAutowiring.php | 4 +- .../Source/ReflectionBasedAutowiring.php | 6 +- src/ServiceLocator.php | 108 ++++++++++++++++++ src/ServiceLocatorRepository.php | 105 +++++++++++++++++ src/ServiceSubscriberException.php | 11 ++ src/ServiceSubscriberInterface.php | 35 ++++++ .../Source/ReflectionBasedAutowiringTest.php | 4 +- 10 files changed, 434 insertions(+), 8 deletions(-) create mode 100644 src/Definition/ServiceLocatorDefinition.php create mode 100644 src/ServiceLocator.php create mode 100644 src/ServiceLocatorRepository.php create mode 100644 src/ServiceSubscriberException.php create mode 100644 src/ServiceSubscriberInterface.php diff --git a/src/Compiler/Compiler.php b/src/Compiler/Compiler.php index c967cbc3e..8e6d04b17 100644 --- a/src/Compiler/Compiler.php +++ b/src/Compiler/Compiler.php @@ -12,6 +12,7 @@ use DI\Definition\FactoryDefinition; use DI\Definition\ObjectDefinition; use DI\Definition\Reference; +use DI\Definition\ServiceLocatorDefinition; use DI\Definition\Source\DefinitionSource; use DI\Definition\StringDefinition; use DI\Definition\ValueDefinition; @@ -174,6 +175,17 @@ private function compileDefinition(string $entryName, Definition $definition) : $code = 'return ' . $this->compileValue($value) . ';'; break; case $definition instanceof Reference: + if ($definition->isServiceLocatorEntry()) { + $requestingEntry = $definition->getRequestingName(); + $serviceLocatorDefinition = $definition->getServiceLocatorDefinition(); + // compiled ServiceLocatorDefinition::resolve + $code = '$repository = $this->delegateContainer->get(' . $this->compileValue($serviceLocatorDefinition::$serviceLocatorRepositoryClass) . '); + $services = ' . $requestingEntry . '::getSubscribedServices(); + $serviceLocator = $repository->create(' . $this->compileValue($requestingEntry) . ', $services); + return $serviceLocator;'; + break; + } + $targetEntryName = $definition->getTargetEntryName(); $code = 'return $this->delegateContainer->get(' . $this->compileValue($targetEntryName) . ');'; // If this method is not yet compiled we store it for compilation diff --git a/src/Definition/Reference.php b/src/Definition/Reference.php index 5b4597a82..c5e17599e 100644 --- a/src/Definition/Reference.php +++ b/src/Definition/Reference.php @@ -4,6 +4,8 @@ namespace DI\Definition; +use DI\Definition\Exception\InvalidDefinition; +use DI\ServiceLocator; use Psr\Container\ContainerInterface; /** @@ -13,6 +15,8 @@ */ class Reference implements Definition, SelfResolvingDefinition { + public static $serviceLocatorClass = ServiceLocator::class; + /** * Entry name. * @var string @@ -25,12 +29,30 @@ class Reference implements Definition, SelfResolvingDefinition */ private $targetEntryName; + /** + * @var string + */ + private $requestingName; + + /** + * @var bool + */ + private $isServiceLocatorEntry; + + /** + * @var ServiceLocatorDefinition + */ + private $serviceLocatorDefinition; + /** * @param string $targetEntryName Name of the target entry + * @param string $requestingName name of an entry - holder of a definition requesting this entry */ - public function __construct(string $targetEntryName) + public function __construct(string $targetEntryName, $requestingName = null) { $this->targetEntryName = $targetEntryName; + $this->requestingName = $requestingName; + $this->isServiceLocatorEntry = $targetEntryName === self::$serviceLocatorClass; } public function getName() : string @@ -48,13 +70,49 @@ public function getTargetEntryName() : string return $this->targetEntryName; } + // added + + /** + * Returns the name of the entity requesting this entry. + * @return string + */ + public function getRequestingName() : string + { + return $this->requestingName; + } + + public function isServiceLocatorEntry() : bool + { + return $this->isServiceLocatorEntry; + } + + public function getServiceLocatorDefinition() : ServiceLocatorDefinition + { + if (!$this->isServiceLocatorEntry) { + throw new InvalidDefinition('Invalid service locator definition'); + } + if (!$this->serviceLocatorDefinition) { + $this->serviceLocatorDefinition = new ServiceLocatorDefinition($this->getTargetEntryName(), $this->requestingName); + } + + return $this->serviceLocatorDefinition; + } + public function resolve(ContainerInterface $container) { + if ($this->isServiceLocatorEntry) { + return $this->getServiceLocatorDefinition()->resolve($container); + } + return $container->get($this->getTargetEntryName()); } public function isResolvable(ContainerInterface $container) : bool { + if ($this->isServiceLocatorEntry) { + return $this->getServiceLocatorDefinition()->isResolvable($container); + } + return $container->has($this->getTargetEntryName()); } diff --git a/src/Definition/ServiceLocatorDefinition.php b/src/Definition/ServiceLocatorDefinition.php new file mode 100644 index 000000000..a5849638e --- /dev/null +++ b/src/Definition/ServiceLocatorDefinition.php @@ -0,0 +1,97 @@ +name = $name; + $this->requestingName = $requestingName; + } + + /** + * Returns the name of the entry in the container. + */ + public function getName() : string + { + return $this->name; + } + + public function setName(string $name) + { + $this->name = $name; + } + + /** + * Returns the name of the holder of the definition requesting service locator. + * @return string + */ + public function getRequestingName() : string + { + return $this->requestingName; + } + + /** + * Resolve the definition and return the resulting value. + * + * @param ContainerInterface $container + * @return mixed + */ + public function resolve(ContainerInterface $container) + { + /** @var ServiceLocatorRepository $repository */ + $repository = $container->get(self::$serviceLocatorRepositoryClass); + $services = $this->requestingName::getSubscribedServices(); + $serviceLocator = $repository->create($this->requestingName, $services); + + return $serviceLocator; + } + + /** + * Check if a definition can be resolved. + * @param ContainerInterface $container + * @return bool + */ + public function isResolvable(ContainerInterface $container) : bool + { + return true; + } + + public function replaceNestedDefinitions(callable $replacer) + { + // no nested definitions + } + + /** + * Definitions can be cast to string for debugging information. + */ + public function __toString() + { + return sprintf( + 'get(%s)', + $this->name + ); + } +} diff --git a/src/Definition/Source/AnnotationBasedAutowiring.php b/src/Definition/Source/AnnotationBasedAutowiring.php index 1a0df9235..0bc9514ac 100644 --- a/src/Definition/Source/AnnotationBasedAutowiring.php +++ b/src/Definition/Source/AnnotationBasedAutowiring.php @@ -177,7 +177,7 @@ private function readProperty(ReflectionProperty $property, ObjectDefinition $de } $definition->addPropertyInjection( - new PropertyInjection($property->getName(), new Reference($entryName), $classname) + new PropertyInjection($property->getName(), new Reference($entryName, $classname), $classname) ); } @@ -220,7 +220,7 @@ private function readConstructor(ReflectionClass $class, ObjectDefinition $defin $entryName = $this->getMethodParameter($index, $parameter, []); if ($entryName !== null) { - $parameters[$index] = new Reference($entryName); + $parameters[$index] = new Reference($entryName, $class->getName()); } } diff --git a/src/Definition/Source/ReflectionBasedAutowiring.php b/src/Definition/Source/ReflectionBasedAutowiring.php index 04a3e41ba..7b05d3068 100644 --- a/src/Definition/Source/ReflectionBasedAutowiring.php +++ b/src/Definition/Source/ReflectionBasedAutowiring.php @@ -29,7 +29,7 @@ public function autowire(string $name, ObjectDefinition $definition = null) $class = new \ReflectionClass($className); $constructor = $class->getConstructor(); if ($constructor && $constructor->isPublic()) { - $constructorInjection = MethodInjection::constructor($this->getParametersDefinition($constructor)); + $constructorInjection = MethodInjection::constructor($this->getParametersDefinition($constructor, $class->getName())); $definition->completeConstructorInjection($constructorInjection); } @@ -52,7 +52,7 @@ public function getDefinitions() : array /** * Read the type-hinting from the parameters of the function. */ - private function getParametersDefinition(\ReflectionFunctionAbstract $constructor) : array + private function getParametersDefinition(\ReflectionFunctionAbstract $constructor, string $className) : array { $parameters = []; @@ -65,7 +65,7 @@ private function getParametersDefinition(\ReflectionFunctionAbstract $constructo $parameterClass = $parameter->getClass(); if ($parameterClass) { - $parameters[$index] = new Reference($parameterClass->getName()); + $parameters[$index] = new Reference($parameterClass->getName(), $className); } } diff --git a/src/ServiceLocator.php b/src/ServiceLocator.php new file mode 100644 index 000000000..86128c390 --- /dev/null +++ b/src/ServiceLocator.php @@ -0,0 +1,108 @@ +container = $container; + $this->entry = $entry; + $this->setServices($services); + } + + /** + * @param array $services + */ + protected function setServices(array $services) + { + foreach ($services as $key => $value) { + if (is_numeric($key)) { + $key = $value; + } + $this->services[$key] = $value; + } + } + + /** + * Get defined services. + * @return array + */ + public function getServices() + { + return $this->services; + } + + /** + * Finds a service by its identifier. + * + * @param string $id Identifier of the entry to look for. + * + * @throws NotFoundExceptionInterface No entry was found for **this** identifier. + * @throws ContainerExceptionInterface Error while retrieving the entry. + * + * @return mixed Entry. + */ + public function get($id) + { + if (!isset($this->services[$id])) { + throw new NotFoundException("Service '$id' is not defined."); + } + + return $this->container->get($this->services[$id]); + } + + /** + * Returns true if the container can return an entry for the given identifier. + * Returns false otherwise. + * + * `has($id)` returning true does not mean that `get($id)` will not throw an exception. + * It does however mean that `get($id)` will not throw a `NotFoundExceptionInterface`. + * + * @param string $id Identifier of the entry to look for. + * + * @return bool + */ + public function has($id) + { + if (!isset($this->services[$id])) { + return false; + } + + return $this->container->has($this->services[$id]); + } +} diff --git a/src/ServiceLocatorRepository.php b/src/ServiceLocatorRepository.php new file mode 100644 index 000000000..0b71fc68b --- /dev/null +++ b/src/ServiceLocatorRepository.php @@ -0,0 +1,105 @@ +container = $container; + } + + /** + * Create or modify service locator. + * + * @param string $entry + * @param array $services + * @param bool $overwrite if service locator for an entry already exists, should its services be overwritten? + * @return ServiceLocator + */ + public function create(string $entry, array $services = [], $overwrite = false) : ServiceLocator + { + if (isset($this->locators[$entry]) && !$overwrite) { + $services = $overwrite + ? array_merge($this->locators[$entry]->getServices(), $services) + : array_merge($services, $this->locators[$entry]->getServices()); + } + + $this->locators[$entry] = new ServiceLocator($this->container, $services, $entry); + + return $this->locators[$entry]; + } + + /** + * Inject service locator on an ServiceSubscriber instance. + * @param ServiceSubscriberInterface $instance + * @param null $entry + * @return $this + */ + public function injectOn(ServiceSubscriberInterface $instance, $entry = null) + { + $entry = $entry ?? get_class($instance); + $serviceLocator = $this->create($entry, $instance->getSubscribedServices()); + $instance->setServiceLocator($serviceLocator); + + return $this; + } + + /** + * Modify a single entry for a service locator. + * + * @param string $entry + * @param string $serviceId + * @param string|null $serviceEntry + * @return $this + */ + public function setService(string $entry, string $serviceId, string $serviceEntry = null) + { + $serviceEntry = $serviceEntry ?? $serviceId; + $this->create($entry, [$serviceId => $serviceEntry], true); + + return $this; + } + + /** + * Get a service locator for an entry. + * @param string $entry + * @return ServiceLocator + * @throws NotFoundException + */ + public function get($entry) : ServiceLocator + { + if (!isset($this->locators[$entry])) { + throw new NotFoundException("Service locator for entry '$entry' is not initialized."); + } + + return $this->locators[$entry]; + } + + /** + * @param string $entry + * @return bool + */ + public function has($entry) + { + return isset($this->locators[$entry]); + } +} diff --git a/src/ServiceSubscriberException.php b/src/ServiceSubscriberException.php new file mode 100644 index 000000000..d207d7c11 --- /dev/null +++ b/src/ServiceSubscriberException.php @@ -0,0 +1,11 @@ +>> Suggested as a lightweight alternative for heavyweight proxies from ocramius/proxy-manager. + * + * The getSubscribedServices method returns an array of service types required by such instances, + * optionally keyed by the service names used internally. + * + * The injected service locators SHOULD NOT allow access to any other services not specified by the method. + * + * It is expected that ServiceSubscriber instances consume PSR-11-based service locators internally. + * This interface does not dictate any injection method for these service locators, although constructor + * injection is recommended. + */ +interface ServiceSubscriberInterface +{ + /** + * Lazy instantiate heavy dependencies on-demand + * Returns an array of service types required by such instances, optionally keyed by the service names used internally. + * + * * ['logger' => Psr\Log\LoggerInterface::class] means the objects use the "logger" name + * internally to fetch a service which must implement Psr\Log\LoggerInterface. + * * ['Psr\Log\LoggerInterface'] is a shortcut for + * * ['Psr\Log\LoggerInterface' => 'Psr\Log\LoggerInterface'] + * + * @return array The required service types, optionally keyed by service names + */ + public static function getSubscribedServices() : array; +} diff --git a/tests/UnitTest/Definition/Source/ReflectionBasedAutowiringTest.php b/tests/UnitTest/Definition/Source/ReflectionBasedAutowiringTest.php index 9ae98b9a2..3123df644 100644 --- a/tests/UnitTest/Definition/Source/ReflectionBasedAutowiringTest.php +++ b/tests/UnitTest/Definition/Source/ReflectionBasedAutowiringTest.php @@ -35,7 +35,7 @@ public function testConstructor() $this->assertCount(1, $parameters); $param1 = $parameters[0]; - $this->assertEquals(new Reference(AutowiringFixture::class), $param1); + $this->assertEquals(new Reference(AutowiringFixture::class, AutowiringFixture::class), $param1); } public function testConstructorInParentClass() @@ -50,6 +50,6 @@ public function testConstructorInParentClass() $this->assertCount(1, $parameters); $param1 = $parameters[0]; - $this->assertEquals(new Reference(AutowiringFixture::class), $param1); + $this->assertEquals(new Reference(AutowiringFixture::class, AutowiringFixtureChild::class), $param1); } } From 99305d209d98430a05970b24a5710d65739aa9e7 Mon Sep 17 00:00:00 2001 From: Maciej Holyszko <14310995+falkenhawk@users.noreply.github.com> Date: Tue, 30 Apr 2019 09:01:57 +0200 Subject: [PATCH 08/23] Discard ServiceLocatorRepository::injectOn() method --- src/ServiceLocatorRepository.php | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/src/ServiceLocatorRepository.php b/src/ServiceLocatorRepository.php index 0b71fc68b..535201e17 100644 --- a/src/ServiceLocatorRepository.php +++ b/src/ServiceLocatorRepository.php @@ -48,21 +48,6 @@ public function create(string $entry, array $services = [], $overwrite = false) return $this->locators[$entry]; } - /** - * Inject service locator on an ServiceSubscriber instance. - * @param ServiceSubscriberInterface $instance - * @param null $entry - * @return $this - */ - public function injectOn(ServiceSubscriberInterface $instance, $entry = null) - { - $entry = $entry ?? get_class($instance); - $serviceLocator = $this->create($entry, $instance->getSubscribedServices()); - $instance->setServiceLocator($serviceLocator); - - return $this; - } - /** * Modify a single entry for a service locator. * From 7a71a1f0b84b2ec54befbda698e4d4358ba51de1 Mon Sep 17 00:00:00 2001 From: Maciej Holyszko <14310995+falkenhawk@users.noreply.github.com> Date: Thu, 30 May 2019 15:41:16 +0200 Subject: [PATCH 09/23] ServiceLocator - rename entry to subscriber, cleanup --- src/ServiceLocator.php | 29 +++++++++++++++++++---------- 1 file changed, 19 insertions(+), 10 deletions(-) diff --git a/src/ServiceLocator.php b/src/ServiceLocator.php index 86128c390..de0dd69df 100644 --- a/src/ServiceLocator.php +++ b/src/ServiceLocator.php @@ -19,29 +19,29 @@ class ServiceLocator implements ContainerInterface /** * @var ContainerInterface */ - protected $container; + private $container; /** - * Name of an entry to which this service locator instance belongs to. - * @var string + * @var array */ - protected $entry; + private $services = []; /** - * @var array + * Name of a class to which this service locator instance belongs to. + * @var string|null */ - protected $services = []; + private $subscriber; /** * Constructor. * @param ContainerInterface $container * @param array $services - * @param string|null $entry Name of an entry to which this service locator instance belongs to + * @param string|null $subscriber className of a ServiceSubscriber to which this service locator instance belongs to */ - public function __construct(ContainerInterface $container, array $services, string $entry = null) + public function __construct(ContainerInterface $container, array $services, string $subscriber = null) { $this->container = $container; - $this->entry = $entry; + $this->subscriber = $subscriber; $this->setServices($services); } @@ -62,11 +62,20 @@ protected function setServices(array $services) * Get defined services. * @return array */ - public function getServices() + public function getServices() : array { return $this->services; } + /** + * Get name of a class to which this service locator instance belongs to. + * @return string + */ + public function getSubscriber() : string + { + return $this->subscriber; + } + /** * Finds a service by its identifier. * From 3938947ed8893f1d8f8e69792f92bcdeb0681304 Mon Sep 17 00:00:00 2001 From: Maciej Holyszko <14310995+falkenhawk@users.noreply.github.com> Date: Thu, 30 May 2019 15:45:50 +0200 Subject: [PATCH 10/23] ServiceLocatorRepository - replace setService with override method - the method may be used only before a locator instance is created - to avoid ambiguous situations - create method refactored - a locator instance will be returned if already created, only if passed services match, otherwise an exception will be thrown --- src/ServiceLocatorRepository.php | 55 ++++++++++++++++++++++++-------- 1 file changed, 41 insertions(+), 14 deletions(-) diff --git a/src/ServiceLocatorRepository.php b/src/ServiceLocatorRepository.php index 535201e17..c5604d28c 100644 --- a/src/ServiceLocatorRepository.php +++ b/src/ServiceLocatorRepository.php @@ -11,12 +11,18 @@ class ServiceLocatorRepository implements ContainerInterface /** * @var ServiceLocator[] */ - protected $locators = []; + private $locators = []; + + /** + * Overrides for ServiceLocators + * @var array + */ + private $overrides = []; /** * @var ContainerInterface */ - protected $container; + private $container; /** * Constructor. @@ -32,35 +38,56 @@ public function __construct(ContainerInterface $container) * * @param string $entry * @param array $services - * @param bool $overwrite if service locator for an entry already exists, should its services be overwritten? * @return ServiceLocator */ - public function create(string $entry, array $services = [], $overwrite = false) : ServiceLocator + public function create(string $entry, array $services = []) : ServiceLocator { - if (isset($this->locators[$entry]) && !$overwrite) { - $services = $overwrite - ? array_merge($this->locators[$entry]->getServices(), $services) - : array_merge($services, $this->locators[$entry]->getServices()); + if (isset($this->overrides[$entry])) { + $services = array_merge($services, $this->overrides[$entry]); + } + if (!isset($this->locators[$entry])) { + $this->locators[$entry] = new ServiceLocator($this->container, $services, $entry); + } else { + // the service locator cannot be re-created - the existing locator may be returned only if expected services are identical + // compare passed services and those in the already created ServiceLocator + $locatorServices = $this->locators[$entry]->getServices(); + foreach ($services as $key => $value) { + if (is_numeric($key)) { + $key = $value; + } + if (!array_key_exists($key, $locatorServices) || $locatorServices[$key] !== $value) { + throw new \LogicException(sprintf( + "ServiceLocator for '%s' cannot be recreated with different services.", + $entry + )); + } + } } - - $this->locators[$entry] = new ServiceLocator($this->container, $services, $entry); return $this->locators[$entry]; } /** - * Modify a single entry for a service locator. + * Override a single service for a service locator. + * This can be only used before the service locator for the given entry is created. * * @param string $entry * @param string $serviceId * @param string|null $serviceEntry * @return $this */ - public function setService(string $entry, string $serviceId, string $serviceEntry = null) + public function override(string $entry, string $serviceId, string $serviceEntry = null) { - $serviceEntry = $serviceEntry ?? $serviceId; - $this->create($entry, [$serviceId => $serviceEntry], true); + if (isset($this->locators[$entry])) { + throw new \LogicException(sprintf( + "Service '%s' for '%s' cannot be overridden - ServiceLocator is already created.", + $serviceId, + $entry + )); + } + $serviceEntry = $serviceEntry ?? $serviceId; + $this->overrides[$entry][$serviceId] = $serviceEntry; return $this; } From 1aa18341f5e8a6d7a128b7d224cfba54adbb8926 Mon Sep 17 00:00:00 2001 From: Maciej Holyszko <14310995+falkenhawk@users.noreply.github.com> Date: Thu, 30 May 2019 15:47:04 +0200 Subject: [PATCH 11/23] Reference and ServiceLocatorDefinition cleanup - resolve and isResolvable will check if getSubscribedServices method exists on given class --- src/Definition/Reference.php | 10 ++++++---- src/Definition/ServiceLocatorDefinition.php | 19 +++++++++++++------ 2 files changed, 19 insertions(+), 10 deletions(-) diff --git a/src/Definition/Reference.php b/src/Definition/Reference.php index c5e17599e..121b8a053 100644 --- a/src/Definition/Reference.php +++ b/src/Definition/Reference.php @@ -70,8 +70,6 @@ public function getTargetEntryName() : string return $this->targetEntryName; } - // added - /** * Returns the name of the entity requesting this entry. * @return string @@ -88,8 +86,12 @@ public function isServiceLocatorEntry() : bool public function getServiceLocatorDefinition() : ServiceLocatorDefinition { - if (!$this->isServiceLocatorEntry) { - throw new InvalidDefinition('Invalid service locator definition'); + if (!$this->isServiceLocatorEntry || $this->requestingName === null) { + throw new InvalidDefinition(sprintf( + "Invalid service locator definition ('%s' for '%s')", + $this->targetEntryName, + $this->requestingName + )); } if (!$this->serviceLocatorDefinition) { $this->serviceLocatorDefinition = new ServiceLocatorDefinition($this->getTargetEntryName(), $this->requestingName); diff --git a/src/Definition/ServiceLocatorDefinition.php b/src/Definition/ServiceLocatorDefinition.php index a5849638e..1ea9b1ff9 100644 --- a/src/Definition/ServiceLocatorDefinition.php +++ b/src/Definition/ServiceLocatorDefinition.php @@ -4,7 +4,9 @@ namespace DI\Definition; +use DI\ServiceLocator; use DI\ServiceLocatorRepository; +use DI\ServiceSubscriberException; use Psr\Container\ContainerInterface; class ServiceLocatorDefinition implements Definition, SelfResolvingDefinition @@ -57,16 +59,20 @@ public function getRequestingName() : string * Resolve the definition and return the resulting value. * * @param ContainerInterface $container - * @return mixed + * @return ServiceLocator + * @throws ServiceSubscriberException */ public function resolve(ContainerInterface $container) { + if (!method_exists($this->requestingName, 'getSubscribedServices')) { + throw new ServiceSubscriberException(sprintf('The class %s does not implement ServiceSubscriberInterface.', $this->requestingName)); + } + /** @var ServiceLocatorRepository $repository */ $repository = $container->get(self::$serviceLocatorRepositoryClass); $services = $this->requestingName::getSubscribedServices(); - $serviceLocator = $repository->create($this->requestingName, $services); - return $serviceLocator; + return $repository->create($this->requestingName, $services); } /** @@ -76,7 +82,7 @@ public function resolve(ContainerInterface $container) */ public function isResolvable(ContainerInterface $container) : bool { - return true; + return method_exists($this->requestingName, 'getSubscribedServices'); } public function replaceNestedDefinitions(callable $replacer) @@ -90,8 +96,9 @@ public function replaceNestedDefinitions(callable $replacer) public function __toString() { return sprintf( - 'get(%s)', - $this->name + 'get(%s) for \'%s\'', + $this->name, + $this->requestingName ); } } From d8ea15e16d8f989e58328206b8c4f92a2a9f7354 Mon Sep 17 00:00:00 2001 From: Maciej Holyszko <14310995+falkenhawk@users.noreply.github.com> Date: Thu, 30 May 2019 15:47:56 +0200 Subject: [PATCH 12/23] compiler - extract resolveServiceLocator method to CompiledContainer + added the same method_exists check as in ServiceLocatorDefinition --- src/CompiledContainer.php | 20 ++++++++++++++++++++ src/Compiler/Compiler.php | 7 +------ 2 files changed, 21 insertions(+), 6 deletions(-) diff --git a/src/CompiledContainer.php b/src/CompiledContainer.php index 88438d736..19c17495b 100644 --- a/src/CompiledContainer.php +++ b/src/CompiledContainer.php @@ -120,4 +120,24 @@ protected function resolveFactory($callable, $entryName, array $extraParameters throw new InvalidDefinition("Entry \"$entryName\" cannot be resolved: " . $e->getMessage()); } } + + /** + * Resolve ServiceLocator for given subscriber class (based on \DI\Definition\ServiceLocatorDefinition::resolve) + * + * @param string $requestingName class name of a subscriber, implementing ServiceSubscriberInterface + * @param string $repositoryClass ServiceLocatorRepository + * @return ServiceLocator + * @throws ServiceSubscriberException + */ + protected function resolveServiceLocator($requestingName, $repositoryClass) { + if (!method_exists($requestingName, 'getSubscribedServices')) { + throw new ServiceSubscriberException(sprintf('The class %s does not implement ServiceSubscriberInterface.', $requestingName)); + } + + /** @var ServiceLocatorRepository $repository */ + $repository = $this->delegateContainer->get($repositoryClass); + $services = $requestingName::getSubscribedServices(); + + return $repository->create($requestingName, $services); + } } diff --git a/src/Compiler/Compiler.php b/src/Compiler/Compiler.php index 8e6d04b17..0b694221f 100644 --- a/src/Compiler/Compiler.php +++ b/src/Compiler/Compiler.php @@ -12,7 +12,6 @@ use DI\Definition\FactoryDefinition; use DI\Definition\ObjectDefinition; use DI\Definition\Reference; -use DI\Definition\ServiceLocatorDefinition; use DI\Definition\Source\DefinitionSource; use DI\Definition\StringDefinition; use DI\Definition\ValueDefinition; @@ -178,11 +177,7 @@ private function compileDefinition(string $entryName, Definition $definition) : if ($definition->isServiceLocatorEntry()) { $requestingEntry = $definition->getRequestingName(); $serviceLocatorDefinition = $definition->getServiceLocatorDefinition(); - // compiled ServiceLocatorDefinition::resolve - $code = '$repository = $this->delegateContainer->get(' . $this->compileValue($serviceLocatorDefinition::$serviceLocatorRepositoryClass) . '); - $services = ' . $requestingEntry . '::getSubscribedServices(); - $serviceLocator = $repository->create(' . $this->compileValue($requestingEntry) . ', $services); - return $serviceLocator;'; + $code = 'return $this->resolveServiceLocator(' . $this->compileValue($requestingEntry) . ', ' . $this->compileValue($serviceLocatorDefinition::$serviceLocatorRepositoryClass) . ');'; break; } From 0d1265d333d82dd755059913687b934f5ffd247e Mon Sep 17 00:00:00 2001 From: Maciej Holyszko <14310995+falkenhawk@users.noreply.github.com> Date: Thu, 30 May 2019 15:48:16 +0200 Subject: [PATCH 13/23] tests for service locators --- .../ServiceLocatorDefinitionTest.php | 54 +++++ tests/IntegrationTest/ServiceLocatorTest.php | 200 ++++++++++++++++++ tests/UnitTest/Definition/ReferenceTest.php | 49 +++++ .../ServiceLocatorDefinitionTest.php | 56 +++++ .../ServiceLocatorRepositoryTest.php | 109 ++++++++++ .../ServiceLocator/ServiceLocatorTest.php | 85 ++++++++ 6 files changed, 553 insertions(+) create mode 100644 tests/IntegrationTest/Definitions/ServiceLocatorDefinitionTest.php create mode 100644 tests/IntegrationTest/ServiceLocatorTest.php create mode 100644 tests/UnitTest/Definition/ServiceLocatorDefinitionTest.php create mode 100644 tests/UnitTest/ServiceLocator/ServiceLocatorRepositoryTest.php create mode 100644 tests/UnitTest/ServiceLocator/ServiceLocatorTest.php diff --git a/tests/IntegrationTest/Definitions/ServiceLocatorDefinitionTest.php b/tests/IntegrationTest/Definitions/ServiceLocatorDefinitionTest.php new file mode 100644 index 000000000..b431c210f --- /dev/null +++ b/tests/IntegrationTest/Definitions/ServiceLocatorDefinitionTest.php @@ -0,0 +1,54 @@ +addDefinitions([ + ServiceLocatorDefinitionTest\TestClass::class => autowire() + ]); + $container = $builder->build(); + + self::assertEntryIsCompiled($container, ServiceLocatorDefinitionTest\TestClass::class); + + $instance = $container->get(ServiceLocatorDefinitionTest\TestClass::class); + $this->assertInstanceOf(ServiceLocator::class, $instance->serviceLocator); + $this->assertEquals(ServiceLocatorDefinitionTest\TestClass::class, $instance->serviceLocator->getSubscriber()); + $this->assertEquals(['foo' => 'foo'], $instance->serviceLocator->getServices()); + } +} + +namespace DI\Test\IntegrationTest\Definitions\ServiceLocatorDefinitionTest; + +use DI\ServiceLocator; +use DI\ServiceSubscriberInterface; + +class TestClass implements ServiceSubscriberInterface +{ + public $serviceLocator; + + public function __construct(ServiceLocator $serviceLocator) + { + $this->serviceLocator = $serviceLocator; + } + + public static function getSubscribedServices(): array + { + return ['foo']; + } +} \ No newline at end of file diff --git a/tests/IntegrationTest/ServiceLocatorTest.php b/tests/IntegrationTest/ServiceLocatorTest.php new file mode 100644 index 000000000..6404d9b72 --- /dev/null +++ b/tests/IntegrationTest/ServiceLocatorTest.php @@ -0,0 +1,200 @@ +addDefinitions([ + 'foo' => 'value of foo', + 'baz' => 'baz', + ]); + + $container = $builder->build(); + $instance = $container->get(ServiceLocatorTest\ServiceSubscriber::class); + $this->assertEquals('value of foo', $instance->getFoo()); + $this->assertEquals('baz', $instance->getBar()); + $this->assertInstanceOf(ServiceLocatorTest\SomeService::class, $instance->getClass()); + } + + /** + * @dataProvider provideContainer + * @expectedException \DI\NotFoundException + * @expectedExceptionMessage Service 'baz' is not defined. + */ + public function testServiceLocatorThrowsForInvalidService(ContainerBuilder $builder) + { + $builder->addDefinitions([ + 'baz' => 'baz', + ]); + + $container = $builder->build(); + $instance = $container->get(ServiceLocatorTest\ServiceSubscriber::class); + $instance->getInvalid(); + } + + /** + * @dataProvider provideContainer + */ + public function testServicesLazyResolve(ContainerBuilder $builder) + { + $container = $builder->build(); + + // services should not be resolved on instantiation of a subscriber class + $instance = $container->get(ServiceLocatorTest\ServiceSubscriber::class); + $this->assertNotContains(ServiceLocatorTest\SomeService::class, $container->getKnownEntryNames()); + + // resolve on demand + $instance->getClass(); + $this->assertContains(ServiceLocatorTest\SomeService::class, $container->getKnownEntryNames()); + } + + /** + * @dataProvider provideContainer + */ + public function testOverrideService(ContainerBuilder $builder) + { + $builder->addDefinitions([ + 'foo' => 'foo', + 'baz' => 'baz', + 'anotherFoo' => 'overridden foo', + ]); + $container = $builder->build(); + $repository = $container->get(ServiceLocatorRepository::class); + $repository->override(ServiceLocatorTest\ServiceSubscriber::class, 'foo', 'anotherFoo'); + + $instance = $container->get(ServiceLocatorTest\ServiceSubscriber::class); + $this->assertEquals('overridden foo', $instance->getFoo()); + } + + /** + * @dataProvider provideContainer + */ + public function testOverrideServiceInRepositoryDefinition(ContainerBuilder $builder) + { + $builder->addDefinitions([ + ServiceLocatorRepository::class => autowire() + ->method('override', ServiceLocatorTest\ServiceSubscriber::class, 'foo', 'anotherFoo'), + 'anotherFoo' => 'overridden foo', + ]); + + $container = $builder->build(); + + $instance = $container->get(ServiceLocatorTest\ServiceSubscriber::class); + $this->assertEquals('overridden foo', $instance->getFoo()); + } + + /** + * @dataProvider provideContainer + * @expectedException \LogicException + * @expectedExceptionMessage Service 'foo' for 'DI\Test\IntegrationTest\ServiceLocatorTest\ServiceSubscriber' cannot be overridden - ServiceLocator is already created. + */ + public function testCannotOverrideServiceForAlreadyInstantiatedSubscriber(ContainerBuilder $builder) + { + $container = $builder->build(); + + $container->get(ServiceLocatorTest\ServiceSubscriber::class); + + $repository = $container->get(ServiceLocatorRepository::class); + $repository->override(ServiceLocatorTest\ServiceSubscriber::class, 'foo', 'anotherFoo'); + } + + /** + * @dataProvider provideContainer + */ + public function testMultipleSubscriberInstances(ContainerBuilder $builder) + { + $container = $builder->build(); + $instance1 = $container->make(ServiceLocatorTest\ServiceSubscriber::class); + $instance2 = $container->make(ServiceLocatorTest\ServiceSubscriber::class); + + // different instances + $this->assertNotSame($instance1, $instance2); + // but the same service locator instance + $this->assertSame($instance1->getServiceLocator(), $instance2->getServiceLocator()); + // and an instance of a service should be shared too + $this->assertSame($instance1->getClass(), $instance2->getClass()); + } + +} + +namespace DI\Test\IntegrationTest\ServiceLocatorTest; + +use DI\ServiceLocator; +use DI\ServiceSubscriberInterface; + +/** + * Fixture class for testing service locators + */ +class ServiceSubscriber implements ServiceSubscriberInterface +{ + /** + * @var ServiceLocator + */ + protected $serviceLocator; + + /** + * @param ServiceLocator $serviceLocator + */ + public function __construct(ServiceLocator $serviceLocator) + { + $this->serviceLocator = $serviceLocator; + } + + /** + * Lazy instantiate heavy dependencies on-demand + */ + public static function getSubscribedServices(): array + { + return [ + 'foo', + 'bar' => 'baz', + SomeService::class, + ]; + } + + public function getFoo() + { + return $this->serviceLocator->get('foo'); + } + + public function getBar() + { + return $this->serviceLocator->get('bar'); + } + + public function getClass() + { + return $this->serviceLocator->get(SomeService::class); + } + + /** + * @throws \DI\NotFoundException + */ + public function getInvalid() + { + return $this->serviceLocator->get('baz'); + } + + public function getServiceLocator() + { + return $this->serviceLocator; + } +} + +class SomeService +{ +} diff --git a/tests/UnitTest/Definition/ReferenceTest.php b/tests/UnitTest/Definition/ReferenceTest.php index 9b59d6960..e618ac956 100644 --- a/tests/UnitTest/Definition/ReferenceTest.php +++ b/tests/UnitTest/Definition/ReferenceTest.php @@ -72,4 +72,53 @@ public function should_cast_to_string() { $this->assertEquals('get(bar)', (string) new Reference('bar')); } + + /** + * @test + */ + public function should_have_a_requesting_name() + { + $definition = new Reference('bar', 'foo'); + $this->assertEquals('foo', $definition->getRequestingName()); + } + + /** + * @test + */ + public function should_be_a_service_locator_entry() + { + $definition = new Reference(Reference::$serviceLocatorClass, 'foo'); + $this->assertTrue($definition->isServiceLocatorEntry()); + } + + /** + * @test + */ + public function should_not_be_a_service_locator_entry() + { + $definition = new Reference('bar', 'foo'); + $this->assertFalse($definition->isServiceLocatorEntry()); + } + + /** + * @test + * @expectedException \DI\Definition\Exception\InvalidDefinition + * @expectedExceptionMessage Invalid service locator definition ('bar' for 'foo') + */ + public function should_throw_on_invalid_service_locator_entry() + { + $definition = new Reference('bar', 'foo'); + $definition->getServiceLocatorDefinition(); + } + + /** + * @test + * @expectedException \DI\Definition\Exception\InvalidDefinition + * @expectedExceptionMessage Invalid service locator definition ('DI\ServiceLocator' for '') + */ + public function should_throw_on_invalid_service_locator_entry2() + { + $definition = new Reference(Reference::$serviceLocatorClass); + $definition->getServiceLocatorDefinition(); + } } diff --git a/tests/UnitTest/Definition/ServiceLocatorDefinitionTest.php b/tests/UnitTest/Definition/ServiceLocatorDefinitionTest.php new file mode 100644 index 000000000..9c9bd22b1 --- /dev/null +++ b/tests/UnitTest/Definition/ServiceLocatorDefinitionTest.php @@ -0,0 +1,56 @@ +assertEquals('ServiceLocator', $definition->getName()); + $definition->setName('foo'); + $this->assertEquals('foo', $definition->getName()); + + $this->assertEquals('subscriber', $definition->getRequestingName()); + } + + /** + * @test + * @expectedException \DI\ServiceSubscriberException + * @expectedExceptionMessage The class DI\Test\UnitTest\Fixtures\Singleton does not implement ServiceSubscriberInterface. + */ + public function cannot_resolve_without_proper_subscriber() + { + $container = $this->easyMock(ContainerInterface::class); + $definition = new ServiceLocatorDefinition(ServiceLocator::class, Singleton::class); + + $this->assertFalse($definition->isResolvable($container)); + $definition->resolve($container); + } + + /** + * @test + */ + public function should_cast_to_string() + { + $definition = new ServiceLocatorDefinition('bar', 'subscriber'); + $this->assertEquals("get(bar) for 'subscriber'", (string) $definition); + } +} diff --git a/tests/UnitTest/ServiceLocator/ServiceLocatorRepositoryTest.php b/tests/UnitTest/ServiceLocator/ServiceLocatorRepositoryTest.php new file mode 100644 index 000000000..701e381a9 --- /dev/null +++ b/tests/UnitTest/ServiceLocator/ServiceLocatorRepositoryTest.php @@ -0,0 +1,109 @@ + 'SomeServiceClass']; + + $serviceLocator = $repository->create('test', $services); + + $this->assertEquals('test', $serviceLocator->getSubscriber()); + $this->assertEquals($expectedServices, $serviceLocator->getServices()); + } + + /** + * @expectedException \DI\NotFoundException + * @expectedExceptionMessage Service locator for entry 'something' is not initialized. + */ + public function testServiceLocatorNotCreated() + { + $container = ContainerBuilder::buildDevContainer(); + $repository = new ServiceLocatorRepository($container); + $repository->get('something'); + } + + public function testGetServiceLocator() + { + $container = ContainerBuilder::buildDevContainer(); + $repository = new ServiceLocatorRepository($container); + $repository->create('test'); + + $this->assertInstanceOf(ServiceLocator::class, $repository->get('test')); + } + + public function testHasServiceLocator() + { + $container = ContainerBuilder::buildDevContainer(); + $repository = new ServiceLocatorRepository($container); + $repository->create('test'); + + $this->assertTrue($repository->has('test')); + $this->assertFalse($repository->has('something-else')); + } + + public function testOverrideService() + { + $container = ContainerBuilder::buildDevContainer(); + $repository = new ServiceLocatorRepository($container); + $repository->override('test', 'foo'); + $repository->override('test', 'bar', 'baz'); + + $locator = $repository->create('test'); + $this->assertEquals(['foo' => 'foo', 'bar' => 'baz'], $locator->getServices()); + } + + public function testCanCreateMultipleWithSameServices() + { + $container = ContainerBuilder::buildDevContainer(); + $repository = new ServiceLocatorRepository($container); + $locator1 = $repository->create('test', ['foo']); + $locator2 = $repository->create('test', ['foo']); + + // same instance + $this->assertSame($locator1, $locator2); + + $repository->override('test2', 'bar', 'baz'); + $locator3 = $repository->create('test2'); + $locator4 = $repository->create('test2'); + $this->assertSame($locator3, $locator4); + + // still same services, because that matches the initial override + $locator5 = $repository->create('test2', ['bar' => 'baz']); + $this->assertSame($locator3, $locator5); + } + + /** + * @expectedException \LogicException + * @expectedExceptionMessage ServiceLocator for 'test' cannot be recreated with different services. + */ + public function testCannotCreateMultipleWithDifferentServices() + { + $container = ContainerBuilder::buildDevContainer(); + $repository = new ServiceLocatorRepository($container); + + $repository->create('test', ['foo']); + $repository->create('test', ['foo2']); + } +} diff --git a/tests/UnitTest/ServiceLocator/ServiceLocatorTest.php b/tests/UnitTest/ServiceLocator/ServiceLocatorTest.php new file mode 100644 index 000000000..91ce02a3c --- /dev/null +++ b/tests/UnitTest/ServiceLocator/ServiceLocatorTest.php @@ -0,0 +1,85 @@ + 'bar', + 'baz', + ]; + $serviceLocator = new ServiceLocator($container, $services, 'test'); + + $this->assertEquals([ + 'foo' => 'bar', + 'baz' => 'baz', + ], $serviceLocator->getServices()); + $this->assertEquals('test', $serviceLocator->getSubscriber()); + } + + /** + * @expectedException \DI\NotFoundException + * @expectedExceptionMessage Service 'something' is not defined. + */ + public function testServiceNotDefined() + { + $container = ContainerBuilder::buildDevContainer(); + $serviceLocator = new ServiceLocator($container, [], 'test'); + $serviceLocator->get('something'); + } + + public function testGetService() + { + $services = [ + 'stdClass', + 'service' => Singleton::class, + ]; + $services2 = [ + Singleton::class, + ]; + + $container = ContainerBuilder::buildDevContainer(); + $serviceLocator = new ServiceLocator($container, $services, 'test'); + $serviceLocator2 = new ServiceLocator($container, $services2, 'test2'); + + $this->assertInstanceOf('stdClass', $serviceLocator->get('stdClass')); + + $service1 = $serviceLocator->get('service'); + $this->assertInstanceOf(Singleton::class, $service1); + + $service2 = $serviceLocator2->get(Singleton::class); + $this->assertInstanceOf(Singleton::class, $service2); + + // it should be the same instances shared from the container + $this->assertSame($service1, $service2); + } + + public function testHasService() + { + $services = [ + 'service' => Singleton::class, + ]; + + $container = ContainerBuilder::buildDevContainer(); + $serviceLocator = new ServiceLocator($container, $services, 'test'); + + $this->assertTrue($serviceLocator->has('service')); + $this->assertFalse($serviceLocator->has(Singleton::class)); + } +} From 2a101327481bf5cc765317c650105f0eba5b2c7a Mon Sep 17 00:00:00 2001 From: Maciej Holyszko <14310995+falkenhawk@users.noreply.github.com> Date: Thu, 30 May 2019 15:57:26 +0200 Subject: [PATCH 14/23] code formatting --- src/CompiledContainer.php | 5 +++-- src/ServiceLocatorRepository.php | 3 ++- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/CompiledContainer.php b/src/CompiledContainer.php index 19c17495b..2eb8037e8 100644 --- a/src/CompiledContainer.php +++ b/src/CompiledContainer.php @@ -122,14 +122,15 @@ protected function resolveFactory($callable, $entryName, array $extraParameters } /** - * Resolve ServiceLocator for given subscriber class (based on \DI\Definition\ServiceLocatorDefinition::resolve) + * Resolve ServiceLocator for given subscriber class (based on \DI\Definition\ServiceLocatorDefinition::resolve). * * @param string $requestingName class name of a subscriber, implementing ServiceSubscriberInterface * @param string $repositoryClass ServiceLocatorRepository * @return ServiceLocator * @throws ServiceSubscriberException */ - protected function resolveServiceLocator($requestingName, $repositoryClass) { + protected function resolveServiceLocator($requestingName, $repositoryClass) + { if (!method_exists($requestingName, 'getSubscribedServices')) { throw new ServiceSubscriberException(sprintf('The class %s does not implement ServiceSubscriberInterface.', $requestingName)); } diff --git a/src/ServiceLocatorRepository.php b/src/ServiceLocatorRepository.php index c5604d28c..663871f67 100644 --- a/src/ServiceLocatorRepository.php +++ b/src/ServiceLocatorRepository.php @@ -14,7 +14,7 @@ class ServiceLocatorRepository implements ContainerInterface private $locators = []; /** - * Overrides for ServiceLocators + * Overrides for ServiceLocators. * @var array */ private $overrides = []; @@ -88,6 +88,7 @@ public function override(string $entry, string $serviceId, string $serviceEntry $serviceEntry = $serviceEntry ?? $serviceId; $this->overrides[$entry][$serviceId] = $serviceEntry; + return $this; } From 3fef38a2b70ed252546fc331b7eb5f1e64289521 Mon Sep 17 00:00:00 2001 From: Maciej Holyszko <14310995+falkenhawk@users.noreply.github.com> Date: Thu, 30 May 2019 17:37:19 +0200 Subject: [PATCH 15/23] change package name in composer.json --- composer.json | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/composer.json b/composer.json index f9b531c87..534a82fbc 100644 --- a/composer.json +++ b/composer.json @@ -1,5 +1,8 @@ { - "name": "php-di/php-di", + "name": "ovos/php-di", + "replace": { + "php-di/php-di": "^6.0.8" + }, "type": "library", "description": "The dependency injection container for humans", "keywords": ["di", "dependency injection", "container", "ioc", "psr-11", "psr11", "container-interop"], @@ -46,5 +49,10 @@ "suggest": { "doctrine/annotations": "Install it if you want to use annotations (version ~1.2)", "ocramius/proxy-manager": "Install it if you want to use lazy injection (version ~2.0)" + }, + "extra": { + "branch-alias": { + "dev-mod": "6.x-dev" + } } } From 7b4540a1e6b47db5f756b4c69575a823180ed3ce Mon Sep 17 00:00:00 2001 From: Michal Kruczek Date: Wed, 20 Jan 2021 21:21:07 +0100 Subject: [PATCH 16/23] fix: bump phpunit assertions --- tests/UnitTest/Definition/Source/DefinitionGlobTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/UnitTest/Definition/Source/DefinitionGlobTest.php b/tests/UnitTest/Definition/Source/DefinitionGlobTest.php index 23d95939f..1d1385e99 100644 --- a/tests/UnitTest/Definition/Source/DefinitionGlobTest.php +++ b/tests/UnitTest/Definition/Source/DefinitionGlobTest.php @@ -36,7 +36,7 @@ public function should_load_definitions_from_glob() $definition = $definitions['foo']; $this->assertInstanceOf(ValueDefinition::class, $definition); $this->assertEquals('bar', $definition->getValue()); - $this->assertInternalType('string', $definition->getValue()); + $this->assertIsString($definition->getValue()); } /** From 4a570b71198482fc538c4300fe3e1461715a75b9 Mon Sep 17 00:00:00 2001 From: Michal Kruczek Date: Wed, 20 Jan 2021 22:38:15 +0100 Subject: [PATCH 17/23] adding leading backslash to all native constants https://github.com/FriendsOfPHP/PHP-CS-Fixer/pull/5426 --- src/Definition/ArrayDefinition.php | 6 +++--- src/Definition/Dumper/ObjectDefinitionDumper.php | 12 ++++++------ src/Definition/EnvironmentVariableDefinition.php | 8 ++++---- src/Definition/Exception/InvalidDefinition.php | 2 +- 4 files changed, 14 insertions(+), 14 deletions(-) diff --git a/src/Definition/ArrayDefinition.php b/src/Definition/ArrayDefinition.php index 8b06fed6d..ef77100ca 100644 --- a/src/Definition/ArrayDefinition.php +++ b/src/Definition/ArrayDefinition.php @@ -50,7 +50,7 @@ public function replaceNestedDefinitions(callable $replacer) public function __toString() { - $str = '[' . PHP_EOL; + $str = '[' . \PHP_EOL; foreach ($this->values as $key => $value) { if (is_string($key)) { @@ -60,12 +60,12 @@ public function __toString() $str .= ' ' . $key . ' => '; if ($value instanceof Definition) { - $str .= str_replace(PHP_EOL, PHP_EOL . ' ', (string) $value); + $str .= str_replace(\PHP_EOL, \PHP_EOL . ' ', (string) $value); } else { $str .= var_export($value, true); } - $str .= ',' . PHP_EOL; + $str .= ',' . \PHP_EOL; } return $str . ']'; diff --git a/src/Definition/Dumper/ObjectDefinitionDumper.php b/src/Definition/Dumper/ObjectDefinitionDumper.php index 7e1999961..b99695ff2 100644 --- a/src/Definition/Dumper/ObjectDefinitionDumper.php +++ b/src/Definition/Dumper/ObjectDefinitionDumper.php @@ -35,7 +35,7 @@ public function dump(ObjectDefinition $definition) : string $str = sprintf(' class = %s%s', $warning, $className); // Lazy - $str .= PHP_EOL . ' lazy = ' . var_export($definition->isLazy(), true); + $str .= \PHP_EOL . ' lazy = ' . var_export($definition->isLazy(), true); if ($classExist) { // Constructor @@ -48,7 +48,7 @@ public function dump(ObjectDefinition $definition) : string $str .= $this->dumpMethods($className, $definition); } - return sprintf('Object (' . PHP_EOL . '%s' . PHP_EOL . ')', $str); + return sprintf('Object (' . \PHP_EOL . '%s' . \PHP_EOL . ')', $str); } private function dumpConstructor(string $className, ObjectDefinition $definition) : string @@ -60,7 +60,7 @@ private function dumpConstructor(string $className, ObjectDefinition $definition if ($constructorInjection !== null) { $parameters = $this->dumpMethodParameters($className, $constructorInjection); - $str .= sprintf(PHP_EOL . ' __construct(' . PHP_EOL . ' %s' . PHP_EOL . ' )', $parameters); + $str .= sprintf(\PHP_EOL . ' __construct(' . \PHP_EOL . ' %s' . \PHP_EOL . ' )', $parameters); } return $str; @@ -74,7 +74,7 @@ private function dumpProperties(ObjectDefinition $definition) : string $value = $propertyInjection->getValue(); $valueStr = $value instanceof Definition ? (string) $value : var_export($value, true); - $str .= sprintf(PHP_EOL . ' $%s = %s', $propertyInjection->getPropertyName(), $valueStr); + $str .= sprintf(\PHP_EOL . ' $%s = %s', $propertyInjection->getPropertyName(), $valueStr); } return $str; @@ -87,7 +87,7 @@ private function dumpMethods(string $className, ObjectDefinition $definition) : foreach ($definition->getMethodInjections() as $methodInjection) { $parameters = $this->dumpMethodParameters($className, $methodInjection); - $str .= sprintf(PHP_EOL . ' %s(' . PHP_EOL . ' %s' . PHP_EOL . ' )', $methodInjection->getMethodName(), $parameters); + $str .= sprintf(\PHP_EOL . ' %s(' . \PHP_EOL . ' %s' . \PHP_EOL . ' )', $methodInjection->getMethodName(), $parameters); } return $str; @@ -130,6 +130,6 @@ private function dumpMethodParameters(string $className, MethodInjection $method $args[] = sprintf('$%s = #UNDEFINED#', $parameter->getName()); } - return implode(PHP_EOL . ' ', $args); + return implode(\PHP_EOL . ' ', $args); } } diff --git a/src/Definition/EnvironmentVariableDefinition.php b/src/Definition/EnvironmentVariableDefinition.php index 73e0e2979..b2a80c3e4 100644 --- a/src/Definition/EnvironmentVariableDefinition.php +++ b/src/Definition/EnvironmentVariableDefinition.php @@ -93,20 +93,20 @@ public function replaceNestedDefinitions(callable $replacer) public function __toString() { - $str = ' variable = ' . $this->variableName . PHP_EOL + $str = ' variable = ' . $this->variableName . \PHP_EOL . ' optional = ' . ($this->isOptional ? 'yes' : 'no'); if ($this->isOptional) { if ($this->defaultValue instanceof Definition) { $nestedDefinition = (string) $this->defaultValue; - $defaultValueStr = str_replace(PHP_EOL, PHP_EOL . ' ', $nestedDefinition); + $defaultValueStr = str_replace(\PHP_EOL, \PHP_EOL . ' ', $nestedDefinition); } else { $defaultValueStr = var_export($this->defaultValue, true); } - $str .= PHP_EOL . ' default = ' . $defaultValueStr; + $str .= \PHP_EOL . ' default = ' . $defaultValueStr; } - return sprintf('Environment variable (' . PHP_EOL . '%s' . PHP_EOL . ')', $str); + return sprintf('Environment variable (' . \PHP_EOL . '%s' . \PHP_EOL . ')', $str); } } diff --git a/src/Definition/Exception/InvalidDefinition.php b/src/Definition/Exception/InvalidDefinition.php index 54e602b74..682f1e78c 100644 --- a/src/Definition/Exception/InvalidDefinition.php +++ b/src/Definition/Exception/InvalidDefinition.php @@ -16,7 +16,7 @@ class InvalidDefinition extends \Exception public static function create(Definition $definition, string $message, \Exception $previous = null) : self { return new self(sprintf( - '%s' . PHP_EOL . 'Full definition:' . PHP_EOL . '%s', + '%s' . \PHP_EOL . 'Full definition:' . \PHP_EOL . '%s', $message, (string) $definition ), 0, $previous); From 87b53f12c674c7cb3a09933c555dcfcb93e1a5cd Mon Sep 17 00:00:00 2001 From: Michal Kruczek Date: Thu, 21 Jan 2021 11:55:22 +0100 Subject: [PATCH 18/23] add psr container exception to the ignored errors https://github.com/phpstan/phpstan/issues/1066 https://github.com/php-fig/container/issues/21 --- phpstan.neon | 1 + 1 file changed, 1 insertion(+) diff --git a/phpstan.neon b/phpstan.neon index ec549c629..23d107525 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -10,5 +10,6 @@ parameters: ignoreErrors: - '#Access to undefined constant DI\\CompiledContainer::METHOD_MAPPING.#' - '#Function apcu_.* not found.#' + - '#PHPDoc tag @throws with type Psr\\Container\\ContainerExceptionInterface is not subtype of Throwable#' reportUnmatchedIgnoredErrors: false inferPrivatePropertyTypeFromConstructor: true From d47439554f9f109375e3bdc2733c36a9dd11fbd8 Mon Sep 17 00:00:00 2001 From: Michal Kruczek Date: Thu, 21 Jan 2021 12:04:43 +0100 Subject: [PATCH 19/23] csfix --- src/Definition/Source/DefinitionGlob.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Definition/Source/DefinitionGlob.php b/src/Definition/Source/DefinitionGlob.php index b0bdcd3a8..7c3f46698 100644 --- a/src/Definition/Source/DefinitionGlob.php +++ b/src/Definition/Source/DefinitionGlob.php @@ -66,7 +66,7 @@ private function initialize() return; } - $paths = glob($this->pattern, GLOB_BRACE); + $paths = glob($this->pattern, \GLOB_BRACE); $sources = array_map(function ($path) { return new DefinitionFile($path, $this->autowiring); }, $paths); From 239a39fccccfbda0a627c50a08803c25896eb80c Mon Sep 17 00:00:00 2001 From: Michal Kruczek Date: Thu, 21 Jan 2021 11:25:18 +0100 Subject: [PATCH 20/23] get rid of using deprecated expectException annotations --- tests/IntegrationTest/ServiceLocatorTest.php | 10 ++++++---- tests/UnitTest/Definition/ReferenceTest.php | 10 ++++++---- .../Definition/ServiceLocatorDefinitionTest.php | 5 +++-- .../ServiceLocatorRepositoryTest.php | 14 ++++++-------- .../UnitTest/ServiceLocator/ServiceLocatorTest.php | 7 +++---- 5 files changed, 24 insertions(+), 22 deletions(-) diff --git a/tests/IntegrationTest/ServiceLocatorTest.php b/tests/IntegrationTest/ServiceLocatorTest.php index 6404d9b72..d4d271fb8 100644 --- a/tests/IntegrationTest/ServiceLocatorTest.php +++ b/tests/IntegrationTest/ServiceLocatorTest.php @@ -32,11 +32,12 @@ public function testServiceLocator(ContainerBuilder $builder) /** * @dataProvider provideContainer - * @expectedException \DI\NotFoundException - * @expectedExceptionMessage Service 'baz' is not defined. */ public function testServiceLocatorThrowsForInvalidService(ContainerBuilder $builder) { + $this->expectException(\DI\NotFoundException::class); + $this->expectExceptionMessage('Service \'baz\' is not defined.'); + $builder->addDefinitions([ 'baz' => 'baz', ]); @@ -99,11 +100,12 @@ public function testOverrideServiceInRepositoryDefinition(ContainerBuilder $buil /** * @dataProvider provideContainer - * @expectedException \LogicException - * @expectedExceptionMessage Service 'foo' for 'DI\Test\IntegrationTest\ServiceLocatorTest\ServiceSubscriber' cannot be overridden - ServiceLocator is already created. */ public function testCannotOverrideServiceForAlreadyInstantiatedSubscriber(ContainerBuilder $builder) { + $this->expectException(\LogicException::class); + $this->expectExceptionMessage('Service \'foo\' for \'DI\Test\IntegrationTest\ServiceLocatorTest\ServiceSubscriber\' cannot be overridden - ServiceLocator is already created.'); + $container = $builder->build(); $container->get(ServiceLocatorTest\ServiceSubscriber::class); diff --git a/tests/UnitTest/Definition/ReferenceTest.php b/tests/UnitTest/Definition/ReferenceTest.php index e618ac956..72b3484f7 100644 --- a/tests/UnitTest/Definition/ReferenceTest.php +++ b/tests/UnitTest/Definition/ReferenceTest.php @@ -102,22 +102,24 @@ public function should_not_be_a_service_locator_entry() /** * @test - * @expectedException \DI\Definition\Exception\InvalidDefinition - * @expectedExceptionMessage Invalid service locator definition ('bar' for 'foo') */ public function should_throw_on_invalid_service_locator_entry() { + $this->expectException(\DI\Definition\Exception\InvalidDefinition::class); + $this->expectExceptionMessage('Invalid service locator definition (\'bar\' for \'foo\')'); + $definition = new Reference('bar', 'foo'); $definition->getServiceLocatorDefinition(); } /** * @test - * @expectedException \DI\Definition\Exception\InvalidDefinition - * @expectedExceptionMessage Invalid service locator definition ('DI\ServiceLocator' for '') */ public function should_throw_on_invalid_service_locator_entry2() { + $this->expectException(\DI\Definition\Exception\InvalidDefinition::class); + $this->expectExceptionMessage('Invalid service locator definition (\'DI\ServiceLocator\' for \'\')'); + $definition = new Reference(Reference::$serviceLocatorClass); $definition->getServiceLocatorDefinition(); } diff --git a/tests/UnitTest/Definition/ServiceLocatorDefinitionTest.php b/tests/UnitTest/Definition/ServiceLocatorDefinitionTest.php index 9c9bd22b1..a984fd65e 100644 --- a/tests/UnitTest/Definition/ServiceLocatorDefinitionTest.php +++ b/tests/UnitTest/Definition/ServiceLocatorDefinitionTest.php @@ -33,11 +33,12 @@ public function should_have_a_name_and_requesting_name() /** * @test - * @expectedException \DI\ServiceSubscriberException - * @expectedExceptionMessage The class DI\Test\UnitTest\Fixtures\Singleton does not implement ServiceSubscriberInterface. */ public function cannot_resolve_without_proper_subscriber() { + $this->expectException(\DI\ServiceSubscriberException::class); + $this->expectExceptionMessage('The class DI\Test\UnitTest\Fixtures\Singleton does not implement ServiceSubscriberInterface.'); + $container = $this->easyMock(ContainerInterface::class); $definition = new ServiceLocatorDefinition(ServiceLocator::class, Singleton::class); diff --git a/tests/UnitTest/ServiceLocator/ServiceLocatorRepositoryTest.php b/tests/UnitTest/ServiceLocator/ServiceLocatorRepositoryTest.php index 701e381a9..34fd8432f 100644 --- a/tests/UnitTest/ServiceLocator/ServiceLocatorRepositoryTest.php +++ b/tests/UnitTest/ServiceLocator/ServiceLocatorRepositoryTest.php @@ -33,12 +33,11 @@ public function testCreateServiceLocator() $this->assertEquals($expectedServices, $serviceLocator->getServices()); } - /** - * @expectedException \DI\NotFoundException - * @expectedExceptionMessage Service locator for entry 'something' is not initialized. - */ public function testServiceLocatorNotCreated() { + $this->expectException(\DI\NotFoundException::class); + $this->expectExceptionMessage('Service locator for entry \'something\' is not initialized.'); + $container = ContainerBuilder::buildDevContainer(); $repository = new ServiceLocatorRepository($container); $repository->get('something'); @@ -94,12 +93,11 @@ public function testCanCreateMultipleWithSameServices() $this->assertSame($locator3, $locator5); } - /** - * @expectedException \LogicException - * @expectedExceptionMessage ServiceLocator for 'test' cannot be recreated with different services. - */ public function testCannotCreateMultipleWithDifferentServices() { + $this->expectException(\LogicException::class); + $this->expectExceptionMessage('ServiceLocator for \'test\' cannot be recreated with different services.'); + $container = ContainerBuilder::buildDevContainer(); $repository = new ServiceLocatorRepository($container); diff --git a/tests/UnitTest/ServiceLocator/ServiceLocatorTest.php b/tests/UnitTest/ServiceLocator/ServiceLocatorTest.php index 91ce02a3c..af7e748c5 100644 --- a/tests/UnitTest/ServiceLocator/ServiceLocatorTest.php +++ b/tests/UnitTest/ServiceLocator/ServiceLocatorTest.php @@ -33,12 +33,11 @@ public function testInstantiation() $this->assertEquals('test', $serviceLocator->getSubscriber()); } - /** - * @expectedException \DI\NotFoundException - * @expectedExceptionMessage Service 'something' is not defined. - */ public function testServiceNotDefined() { + $this->expectException(\DI\NotFoundException::class); + $this->expectExceptionMessage('Service \'something\' is not defined.'); + $container = ContainerBuilder::buildDevContainer(); $serviceLocator = new ServiceLocator($container, [], 'test'); $serviceLocator->get('something'); From 80d5ae390a9093e44949de588af5c566fc5190da Mon Sep 17 00:00:00 2001 From: Michal Kruczek Date: Thu, 21 Jan 2021 11:45:03 +0100 Subject: [PATCH 21/23] fix a missing default type for Reference:: --- src/Definition/Reference.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Definition/Reference.php b/src/Definition/Reference.php index 121b8a053..3ac7df824 100644 --- a/src/Definition/Reference.php +++ b/src/Definition/Reference.php @@ -40,7 +40,7 @@ class Reference implements Definition, SelfResolvingDefinition private $isServiceLocatorEntry; /** - * @var ServiceLocatorDefinition + * @var ServiceLocatorDefinition|null */ private $serviceLocatorDefinition; From 33059cf86708afb1b0e3e39d96e9d92995268905 Mon Sep 17 00:00:00 2001 From: Michal Kruczek Date: Thu, 21 Jan 2021 12:01:39 +0100 Subject: [PATCH 22/23] csfix --- src/Definition/Reference.php | 1 - src/Definition/ServiceLocatorDefinition.php | 4 ---- src/ServiceLocator.php | 7 ------- src/ServiceLocatorRepository.php | 9 --------- 4 files changed, 21 deletions(-) diff --git a/src/Definition/Reference.php b/src/Definition/Reference.php index 3ac7df824..13bc62583 100644 --- a/src/Definition/Reference.php +++ b/src/Definition/Reference.php @@ -72,7 +72,6 @@ public function getTargetEntryName() : string /** * Returns the name of the entity requesting this entry. - * @return string */ public function getRequestingName() : string { diff --git a/src/Definition/ServiceLocatorDefinition.php b/src/Definition/ServiceLocatorDefinition.php index 1ea9b1ff9..ebaf2c453 100644 --- a/src/Definition/ServiceLocatorDefinition.php +++ b/src/Definition/ServiceLocatorDefinition.php @@ -48,7 +48,6 @@ public function setName(string $name) /** * Returns the name of the holder of the definition requesting service locator. - * @return string */ public function getRequestingName() : string { @@ -58,7 +57,6 @@ public function getRequestingName() : string /** * Resolve the definition and return the resulting value. * - * @param ContainerInterface $container * @return ServiceLocator * @throws ServiceSubscriberException */ @@ -77,8 +75,6 @@ public function resolve(ContainerInterface $container) /** * Check if a definition can be resolved. - * @param ContainerInterface $container - * @return bool */ public function isResolvable(ContainerInterface $container) : bool { diff --git a/src/ServiceLocator.php b/src/ServiceLocator.php index de0dd69df..727f4352e 100644 --- a/src/ServiceLocator.php +++ b/src/ServiceLocator.php @@ -34,8 +34,6 @@ class ServiceLocator implements ContainerInterface /** * Constructor. - * @param ContainerInterface $container - * @param array $services * @param string|null $subscriber className of a ServiceSubscriber to which this service locator instance belongs to */ public function __construct(ContainerInterface $container, array $services, string $subscriber = null) @@ -45,9 +43,6 @@ public function __construct(ContainerInterface $container, array $services, stri $this->setServices($services); } - /** - * @param array $services - */ protected function setServices(array $services) { foreach ($services as $key => $value) { @@ -60,7 +55,6 @@ protected function setServices(array $services) /** * Get defined services. - * @return array */ public function getServices() : array { @@ -69,7 +63,6 @@ public function getServices() : array /** * Get name of a class to which this service locator instance belongs to. - * @return string */ public function getSubscriber() : string { diff --git a/src/ServiceLocatorRepository.php b/src/ServiceLocatorRepository.php index 663871f67..6c04d6a17 100644 --- a/src/ServiceLocatorRepository.php +++ b/src/ServiceLocatorRepository.php @@ -26,7 +26,6 @@ class ServiceLocatorRepository implements ContainerInterface /** * Constructor. - * @param ContainerInterface $container */ public function __construct(ContainerInterface $container) { @@ -35,10 +34,6 @@ public function __construct(ContainerInterface $container) /** * Create or modify service locator. - * - * @param string $entry - * @param array $services - * @return ServiceLocator */ public function create(string $entry, array $services = []) : ServiceLocator { @@ -71,9 +66,6 @@ public function create(string $entry, array $services = []) : ServiceLocator * Override a single service for a service locator. * This can be only used before the service locator for the given entry is created. * - * @param string $entry - * @param string $serviceId - * @param string|null $serviceEntry * @return $this */ public function override(string $entry, string $serviceId, string $serviceEntry = null) @@ -95,7 +87,6 @@ public function override(string $entry, string $serviceId, string $serviceEntry /** * Get a service locator for an entry. * @param string $entry - * @return ServiceLocator * @throws NotFoundException */ public function get($entry) : ServiceLocator From 2f44a4cb0d85ca86a7d92231a11af96a65984737 Mon Sep 17 00:00:00 2001 From: Michal Kruczek Date: Thu, 21 Jan 2021 12:42:32 +0100 Subject: [PATCH 23/23] csfix --- src/Definition/AutowireDefinition.php | 1 - src/Definition/Helper/AutowireDefinitionHelper.php | 1 - 2 files changed, 2 deletions(-) diff --git a/src/Definition/AutowireDefinition.php b/src/Definition/AutowireDefinition.php index 6bd753319..4d7728612 100644 --- a/src/Definition/AutowireDefinition.php +++ b/src/Definition/AutowireDefinition.php @@ -16,7 +16,6 @@ class AutowireDefinition extends ObjectDefinition /** * Enable/disable reading annotations for this definition, regardless of a container configuration. - * @param bool $flag */ public function useAnnotations(bool $flag = true) { diff --git a/src/Definition/Helper/AutowireDefinitionHelper.php b/src/Definition/Helper/AutowireDefinitionHelper.php index f65395862..9b9a278ee 100644 --- a/src/Definition/Helper/AutowireDefinitionHelper.php +++ b/src/Definition/Helper/AutowireDefinitionHelper.php @@ -76,7 +76,6 @@ public function methodParameter(string $method, $parameter, $value) /** * Define if entry should use annotation reader for reading dependencies. * This is turned off by default if autowire() helper is used, and turned on if entry is not defined explicitly in the di config. - * @param bool $useAnnotations * @return $this */ public function useAnnotations(bool $useAnnotations = true)