From d00d9dd9b5830e962458feb5212621ebeed4b293 Mon Sep 17 00:00:00 2001 From: Matthieu Napoli Date: Tue, 10 Dec 2013 23:36:12 +0100 Subject: [PATCH 01/10] Started refactoring the definition sources --- .../Exception/DefinitionException.php | 1 - .../Source/AnnotationDefinitionSource.php | 166 +++++++++--------- .../Source/ClassDefinitionSource.php | 44 +++++ src/DI/Definition/Source/DefinitionSource.php | 8 +- .../Source/ReflectionDefinitionSource.php | 61 +++---- .../ClassDefinitionResolver.php | 1 - .../Source/AnnotationDefinitionSourceTest.php | 57 +++++- .../Source/Fixtures/AnnotationFixture.php | 8 + .../Source/Fixtures/ReflectionFixture.php | 2 - .../Fixtures/ReflectionFixtureChild.php | 8 +- .../Source/ReflectionDefinitionSourceTest.php | 48 ++++- 11 files changed, 255 insertions(+), 149 deletions(-) create mode 100644 src/DI/Definition/Source/ClassDefinitionSource.php rename src/DI/Definition/ClassInjection/UndefinedInjection.php => tests/UnitTests/DI/Definition/Source/Fixtures/ReflectionFixtureChild.php (53%) diff --git a/src/DI/Definition/Exception/DefinitionException.php b/src/DI/Definition/Exception/DefinitionException.php index ea5b797f9..65786433f 100644 --- a/src/DI/Definition/Exception/DefinitionException.php +++ b/src/DI/Definition/Exception/DefinitionException.php @@ -16,5 +16,4 @@ */ class DefinitionException extends \Exception { - } diff --git a/src/DI/Definition/Source/AnnotationDefinitionSource.php b/src/DI/Definition/Source/AnnotationDefinitionSource.php index 38dd301cf..cbcdc05d9 100644 --- a/src/DI/Definition/Source/AnnotationDefinitionSource.php +++ b/src/DI/Definition/Source/AnnotationDefinitionSource.php @@ -17,7 +17,6 @@ use DI\Definition\Exception\DefinitionException; use DI\Definition\ClassInjection\MethodInjection; use DI\Definition\ClassInjection\PropertyInjection; -use DI\Definition\ClassInjection\UndefinedInjection; use Doctrine\Common\Annotations\AnnotationRegistry; use Doctrine\Common\Annotations\Reader; use Doctrine\Common\Annotations\SimpleAnnotationReader; @@ -25,16 +24,19 @@ use PhpDocReader\PhpDocReader; use ReflectionClass; use ReflectionMethod; +use ReflectionParameter; +use ReflectionProperty; use UnexpectedValueException; /** * Source of DI class definitions in annotations such as @ Inject and @ var annotations. * - * Uses Reflection, Doctrine's Annotations and regex docblock parsing. + * Uses ReflectionDefinitionSource, Doctrine's Annotations and regex docblock parsing. + * This source automatically includes the reflection source. * * @author Matthieu Napoli */ -class AnnotationDefinitionSource implements DefinitionSource +class AnnotationDefinitionSource implements DefinitionSource, ClassDefinitionSource { /** * @var Reader @@ -53,24 +55,24 @@ class AnnotationDefinitionSource implements DefinitionSource */ public function getDefinition($name) { - if (!$this->classExists($name)) { + if (!class_exists($name) && !interface_exists($name)) { return null; } - $reflectionClass = new ReflectionClass($name); + $class = new ReflectionClass($name); $classDefinition = new ClassDefinition($name); // Injectable annotation - /** @var $injectableAnnotation Injectable */ + /** @var $injectableAnnotation Injectable|null */ try { $injectableAnnotation = $this->getAnnotationReader() - ->getClassAnnotation($reflectionClass, 'DI\Annotation\Injectable'); + ->getClassAnnotation($class, 'DI\Annotation\Injectable'); } catch (UnexpectedValueException $e) { throw new DefinitionException(sprintf( 'Error while reading @Injectable on %s: %s', - $reflectionClass->getName(), + $class->getName(), $e->getMessage() - )); + ), 0, $e); } if ($injectableAnnotation) { @@ -83,10 +85,10 @@ public function getDefinition($name) } // Browse the class properties looking for annotated properties - $this->readProperties($reflectionClass, $classDefinition); + $this->readProperties($class, $classDefinition); // Browse the object's methods looking for annotated methods - $this->readMethods($reflectionClass, $classDefinition); + $this->readMethods($class, $classDefinition); return $classDefinition; } @@ -99,73 +101,60 @@ public function getDefinition($name) */ private function readProperties(ReflectionClass $reflectionClass, ClassDefinition $classDefinition) { + // This will look in all the properties, including those of the parent classes foreach ($reflectionClass->getProperties() as $property) { // Ignore static properties if ($property->isStatic()) { continue; } - // Look for @Inject annotation - $annotation = $this->getAnnotationReader()->getPropertyAnnotation($property, 'DI\Annotation\Inject'); - if ($annotation === null) { - continue; - } - - /** @var $annotation Inject */ - $entryName = $annotation->getName(); - - // Look for @var content - $entryName = $entryName ?: $this->getPhpDocReader()->getPropertyType($property); - - if ($entryName === null) { - $value = new UndefinedInjection(); - } else { - $value = new EntryReference($entryName); - } - - $classDefinition->addPropertyInjection( - new PropertyInjection($property->name, $value) - ); + $classDefinition->addPropertyInjection($this->getPropertyInjection($property)); } } /** - * Browse the object's methods looking for annotated methods. - * - * @param ReflectionClass $reflectionClass - * @param ClassDefinition $classDefinition + * {@inheritdoc} */ - private function readMethods(ReflectionClass $reflectionClass, ClassDefinition $classDefinition) + public function getPropertyInjection(ReflectionProperty $property) { - foreach ($reflectionClass->getMethods(\ReflectionMethod::IS_PUBLIC) as $method) { - // Ignore static methods - if ($method->isStatic()) { - continue; - } + // Look for @Inject annotation + /** @var $annotation Inject */ + $annotation = $this->getAnnotationReader()->getPropertyAnnotation($property, 'DI\Annotation\Inject'); + if ($annotation === null) { + return null; + } + + // @Inject("name") or look for @var content + $entryName = $annotation->getName() ?: $this->getPhpDocReader()->getPropertyType($property); - $this->readMethod($method, $classDefinition); + if ($entryName === null) { + return null; } + + return new PropertyInjection($property->getName(), new EntryReference($entryName)); } /** * Browse the object's methods looking for annotated methods. * - * @param ReflectionMethod $reflectionMethod - * @param ClassDefinition $classDefinition + * @param ReflectionClass $class + * @param ClassDefinition $classDefinition */ - private function readMethod(ReflectionMethod $reflectionMethod, ClassDefinition $classDefinition) + private function readMethods(ReflectionClass $class, ClassDefinition $classDefinition) { - // Look for @Inject annotation - /** @var $annotation Inject|null */ - $annotation = $this->getAnnotationReader()->getMethodAnnotation($reflectionMethod, 'DI\Annotation\Inject'); + // This will look in all the methods, including those of the parent classes + foreach ($class->getMethods(ReflectionMethod::IS_PUBLIC) as $method) { + if ($method->isStatic()) { + continue; + } - if ($annotation) { - // @Inject found, create MethodInjection - $parameters = $this->readMethodParameters($reflectionMethod, $annotation); + $methodInjection = $this->getMethodInjection($class, $method); - $methodInjection = new MethodInjection($reflectionMethod->name, $parameters); + if (! $methodInjection) { + continue; + } - if ($reflectionMethod->isConstructor()) { + if ($method->isConstructor()) { $classDefinition->setConstructorInjection($methodInjection); } else { $classDefinition->addMethodInjection($methodInjection); @@ -174,39 +163,57 @@ private function readMethod(ReflectionMethod $reflectionMethod, ClassDefinition } /** - * @param ReflectionMethod $method - * @param Inject $annotation - * - * @return array + * {@inheritdoc} */ - private function readMethodParameters(ReflectionMethod $method, Inject $annotation) + public function getMethodInjection(ReflectionClass $class, ReflectionMethod $method) { - $annotationParameters = $annotation->getParameters(); + // Look for @Inject annotation + /** @var $annotation Inject|null */ + $annotation = $this->getAnnotationReader()->getMethodAnnotation($method, 'DI\Annotation\Inject'); + $annotationParameters = $annotation ? $annotation->getParameters() : array(); + + // @Inject on constructor is implicit + if (! ($annotation ||$method->isConstructor())) { + return null; + } $parameters = array(); foreach ($method->getParameters() as $index => $parameter) { + $entryName = $this->getMethodParameter($index, $parameter, $annotationParameters); - $entryName = null; - - // @Inject has definition for this parameter - if (isset($annotationParameters[$index])) { - $entryName = $annotationParameters[$index]; + if ($entryName !== null) { + $parameters[$parameter->getName()] = new EntryReference($entryName); } + } - // Look for @param tag or PHP type-hinting (only case where we use reflection) - if ($entryName === null) { - $entryName = $this->getPhpDocReader()->getParameterType($parameter); - } + return new MethodInjection($method->getName(), $parameters); + } - if ($entryName === null) { - $parameters[] = new UndefinedInjection(); - continue; - } + /** + * @param int $parameterIndex + * @param ReflectionParameter $parameter + * @param array $annotationParameters + * + * @return string|null Entry name or null if not found. + */ + private function getMethodParameter($parameterIndex, ReflectionParameter $parameter, array $annotationParameters) + { + // @Inject has definition for this parameter (by index, or by name) + if (isset($annotationParameters[$parameterIndex])) { + return $annotationParameters[$parameterIndex]; + } + if (isset($annotationParameters[$parameter->getName()])) { + return $annotationParameters[$parameter->getName()]; + } - $parameters[] = new EntryReference($entryName); + // Try to use the type-hinting + $parameterClass = $parameter->getClass(); + if ($parameterClass) { + return $parameterClass->getName(); } - return $parameters; + // Last resort, look for @param tag + return $this->getPhpDocReader()->getParameterType($parameter); } /** @@ -231,15 +238,6 @@ public function setAnnotationReader(Reader $annotationReader) $this->annotationReader = $annotationReader; } - /** - * @param string $class - * @return bool - */ - private function classExists($class) - { - return class_exists($class) || interface_exists($class); - } - /** * @return PhpDocReader */ diff --git a/src/DI/Definition/Source/ClassDefinitionSource.php b/src/DI/Definition/Source/ClassDefinitionSource.php new file mode 100644 index 000000000..9bfc596a2 --- /dev/null +++ b/src/DI/Definition/Source/ClassDefinitionSource.php @@ -0,0 +1,44 @@ + + * @since 4.0 + */ +interface ClassDefinitionSource +{ + /** + * Returns the injection definition for the given property. + * + * @param ReflectionProperty $property + * + * @return PropertyInjection|null + */ + public function getPropertyInjection(ReflectionProperty $property); + + /** + * Returns the injection definition for the given method. + * + * @param ReflectionClass $class + * @param ReflectionMethod $method + * + * @return MethodInjection|null + */ + public function getMethodInjection(ReflectionClass $class, ReflectionMethod $method); +} diff --git a/src/DI/Definition/Source/DefinitionSource.php b/src/DI/Definition/Source/DefinitionSource.php index 4df0c7cdb..2eb470007 100644 --- a/src/DI/Definition/Source/DefinitionSource.php +++ b/src/DI/Definition/Source/DefinitionSource.php @@ -13,16 +13,18 @@ use DI\Definition\Exception\DefinitionException; /** - * Source of Dependency Injection definitions + * Source of definitions for entries of the container. * * @author Matthieu Napoli */ interface DefinitionSource { /** - * Returns DI definition for the entry name + * Returns the DI definition for the entry name. + * * @param string $name - * @throws DefinitionException Invalid DI definitions + * + * @throws DefinitionException An invalid definition was found. * @return Definition|null */ public function getDefinition($name); diff --git a/src/DI/Definition/Source/ReflectionDefinitionSource.php b/src/DI/Definition/Source/ReflectionDefinitionSource.php index 8a6038ece..f75c9f140 100644 --- a/src/DI/Definition/Source/ReflectionDefinitionSource.php +++ b/src/DI/Definition/Source/ReflectionDefinitionSource.php @@ -12,74 +12,63 @@ use DI\Definition\ClassDefinition; use DI\Definition\EntryReference; use DI\Definition\ClassInjection\MethodInjection; -use DI\Definition\ClassInjection\UndefinedInjection; use ReflectionClass; -use ReflectionParameter; +use ReflectionMethod; +use ReflectionProperty; /** - * Reads DI class definitions using only reflection - * - * Will guess injection only on class constructors + * Reads DI class definitions using reflection. * * @author Matthieu Napoli */ -class ReflectionDefinitionSource implements DefinitionSource +class ReflectionDefinitionSource implements DefinitionSource, ClassDefinitionSource { /** * {@inheritdoc} */ public function getDefinition($name) { - if (!$this->classExists($name)) { + if (!class_exists($name) && !interface_exists($name)) { return null; } - $reflectionClass = new ReflectionClass($name); - + $class = new ReflectionClass($name); $classDefinition = new ClassDefinition($name); // Constructor - $constructor = $reflectionClass->getConstructor(); + $constructor = $class->getConstructor(); if ($constructor && $constructor->isPublic()) { - $parameters = array(); - foreach ($constructor->getParameters() as $parameter) { - $parameterType = $this->getParameterType($parameter); - - if ($parameterType) { - $parameters[] = new EntryReference($parameterType); - } else { - $parameters[] = new UndefinedInjection(); - } - } - - $classDefinition->setConstructorInjection( - new MethodInjection($constructor->name, $parameters) - ); + $classDefinition->setConstructorInjection($this->getMethodInjection($class, $constructor)); } return $classDefinition; } /** - * @param ReflectionParameter $parameter - * @return string|null Type of the parameter + * {@inheritdoc} */ - private function getParameterType(ReflectionParameter $parameter) + public function getPropertyInjection(ReflectionProperty $property) { - $reflectionClass = $parameter->getClass(); - if ($reflectionClass === null) { - return null; - } - return $reflectionClass->name; + // Nothing to guess on properties + return null; } /** - * @param string $class - * @return bool + * {@inheritdoc} */ - private function classExists($class) + public function getMethodInjection(ReflectionClass $class, ReflectionMethod $method) { - return class_exists($class) || interface_exists($class); + $parameters = array(); + + foreach ($method->getParameters() as $parameter) { + $parameterClass = $parameter->getClass(); + + if ($parameterClass) { + $parameters[$parameter->getName()] = new EntryReference($parameterClass->getName()); + } + } + + return new MethodInjection($method->getName(), $parameters); } } diff --git a/src/DI/DefinitionResolver/ClassDefinitionResolver.php b/src/DI/DefinitionResolver/ClassDefinitionResolver.php index c4818eb65..e0921bdd1 100644 --- a/src/DI/DefinitionResolver/ClassDefinitionResolver.php +++ b/src/DI/DefinitionResolver/ClassDefinitionResolver.php @@ -16,7 +16,6 @@ use DI\Definition\Exception\DefinitionException; use DI\Definition\ClassInjection\MethodInjection; use DI\Definition\ClassInjection\PropertyInjection; -use DI\Definition\ClassInjection\UndefinedInjection; use DI\DependencyException; use DI\NotFoundException; use Exception; diff --git a/tests/UnitTests/DI/Definition/Source/AnnotationDefinitionSourceTest.php b/tests/UnitTests/DI/Definition/Source/AnnotationDefinitionSourceTest.php index e172b7ab5..d9ea3201a 100644 --- a/tests/UnitTests/DI/Definition/Source/AnnotationDefinitionSourceTest.php +++ b/tests/UnitTests/DI/Definition/Source/AnnotationDefinitionSourceTest.php @@ -17,12 +17,18 @@ */ class AnnotationDefinitionSourceTest extends \PHPUnit_Framework_TestCase { + /** + * @covers \DI\Definition\Source\AnnotationDefinitionSource::getDefinition + */ public function testUnknownClass() { $source = new AnnotationDefinitionSource(); $this->assertNull($source->getDefinition('foo')); } + /** + * @covers \DI\Definition\Source\AnnotationDefinitionSource + */ public function testProperty1() { $source = new AnnotationDefinitionSource(); @@ -37,6 +43,9 @@ public function testProperty1() $this->assertEquals(new EntryReference('foo'), $property->getValue()); } + /** + * @covers \DI\Definition\Source\AnnotationDefinitionSource + */ public function testConstructor() { $source = new AnnotationDefinitionSource(); @@ -48,10 +57,13 @@ public function testConstructor() $parameters = $constructorInjection->getParameters(); $this->assertCount(2, $parameters); - $this->assertEquals(new EntryReference('foo'), $parameters[0]); - $this->assertEquals(new EntryReference('bar'), $parameters[1]); + $this->assertEquals(new EntryReference('foo'), $parameters['param1']); + $this->assertEquals(new EntryReference('bar'), $parameters['param2']); } + /** + * @covers \DI\Definition\Source\AnnotationDefinitionSource + */ public function testMethod1() { $source = new AnnotationDefinitionSource(); @@ -65,6 +77,9 @@ public function testMethod1() $this->assertEmpty($methodInjection->getParameters()); } + /** + * @covers \DI\Definition\Source\AnnotationDefinitionSource + */ public function testMethod2() { $source = new AnnotationDefinitionSource(); @@ -77,10 +92,13 @@ public function testMethod2() $parameters = $methodInjection->getParameters(); $this->assertCount(2, $parameters); - $this->assertEquals(new EntryReference('foo'), $parameters[0]); - $this->assertEquals(new EntryReference('bar'), $parameters[1]); + $this->assertEquals(new EntryReference('foo'), $parameters['param1']); + $this->assertEquals(new EntryReference('bar'), $parameters['param2']); } + /** + * @covers \DI\Definition\Source\AnnotationDefinitionSource + */ public function testMethod3() { $source = new AnnotationDefinitionSource(); @@ -94,10 +112,14 @@ public function testMethod3() $parameters = $methodInjection->getParameters(); $this->assertCount(2, $parameters); - $this->assertEquals(new EntryReference('UnitTests\DI\Definition\Source\Fixtures\AnnotationFixture2'), $parameters[0]); - $this->assertEquals(new EntryReference('UnitTests\DI\Definition\Source\Fixtures\AnnotationFixture2'), $parameters[1]); + $reference = new EntryReference('UnitTests\DI\Definition\Source\Fixtures\AnnotationFixture2'); + $this->assertEquals($reference, $parameters['param1']); + $this->assertEquals($reference, $parameters['param2']); } + /** + * @covers \DI\Definition\Source\AnnotationDefinitionSource + */ public function testMethod4() { $source = new AnnotationDefinitionSource(); @@ -110,7 +132,26 @@ public function testMethod4() $parameters = $methodInjection->getParameters(); $this->assertCount(2, $parameters); - $this->assertEquals(new EntryReference('foo'), $parameters[0]); - $this->assertEquals(new EntryReference('bar'), $parameters[1]); + $this->assertEquals(new EntryReference('foo'), $parameters['param1']); + $this->assertEquals(new EntryReference('bar'), $parameters['param2']); + } + + /** + * @covers \DI\Definition\Source\AnnotationDefinitionSource + */ + public function testMethod5() + { + $source = new AnnotationDefinitionSource(); + $definition = $source->getDefinition('UnitTests\DI\Definition\Source\Fixtures\AnnotationFixture'); + $this->assertInstanceOf('DI\Definition\Definition', $definition); + + $methodInjections = $definition->getMethodInjections(); + $methodInjection = $methodInjections['method5']; + $this->assertInstanceOf('DI\Definition\ClassInjection\MethodInjection', $methodInjection); + + $parameters = $methodInjection->getParameters(); + $this->assertCount(1, $parameters); + + $this->assertEquals(new EntryReference('bar'), $parameters['param2']); } } diff --git a/tests/UnitTests/DI/Definition/Source/Fixtures/AnnotationFixture.php b/tests/UnitTests/DI/Definition/Source/Fixtures/AnnotationFixture.php index f143dd6f8..b4a6398e0 100644 --- a/tests/UnitTests/DI/Definition/Source/Fixtures/AnnotationFixture.php +++ b/tests/UnitTests/DI/Definition/Source/Fixtures/AnnotationFixture.php @@ -67,4 +67,12 @@ public function method3(AnnotationFixture2 $param1, $param2) public function method4($param1, $param2) { } + + /** + * Indexed by name, param1 not specified: + * @Inject({"param2" = "bar"}) + */ + public function method5($param1, $param2) + { + } } diff --git a/tests/UnitTests/DI/Definition/Source/Fixtures/ReflectionFixture.php b/tests/UnitTests/DI/Definition/Source/Fixtures/ReflectionFixture.php index 42d306539..3fd871c7b 100644 --- a/tests/UnitTests/DI/Definition/Source/Fixtures/ReflectionFixture.php +++ b/tests/UnitTests/DI/Definition/Source/Fixtures/ReflectionFixture.php @@ -14,9 +14,7 @@ */ class ReflectionFixture { - public function __construct(ReflectionFixture $param1, $param2, $param3 = null) { } - } diff --git a/src/DI/Definition/ClassInjection/UndefinedInjection.php b/tests/UnitTests/DI/Definition/Source/Fixtures/ReflectionFixtureChild.php similarity index 53% rename from src/DI/Definition/ClassInjection/UndefinedInjection.php rename to tests/UnitTests/DI/Definition/Source/Fixtures/ReflectionFixtureChild.php index 74cdbabea..80b1e5f80 100644 --- a/src/DI/Definition/ClassInjection/UndefinedInjection.php +++ b/tests/UnitTests/DI/Definition/Source/Fixtures/ReflectionFixtureChild.php @@ -7,13 +7,11 @@ * @license http://www.opensource.org/licenses/mit-license.php MIT (see the LICENSE file) */ -namespace DI\Definition\ClassInjection; +namespace UnitTests\DI\Definition\Source\Fixtures; /** - * Represents an injection not yet defined (e.g. a constructor parameter not defined). - * - * @author Matthieu Napoli + * Fixture class for the ReflectionDefinitionSource tests */ -class UndefinedInjection +class ReflectionFixtureChild extends ReflectionFixture { } diff --git a/tests/UnitTests/DI/Definition/Source/ReflectionDefinitionSourceTest.php b/tests/UnitTests/DI/Definition/Source/ReflectionDefinitionSourceTest.php index f9c406a78..1c5a277ae 100644 --- a/tests/UnitTests/DI/Definition/Source/ReflectionDefinitionSourceTest.php +++ b/tests/UnitTests/DI/Definition/Source/ReflectionDefinitionSourceTest.php @@ -11,35 +11,65 @@ use DI\Definition\EntryReference; use DI\Definition\Source\ReflectionDefinitionSource; -use DI\Definition\ClassInjection\UndefinedInjection; class ReflectionDefinitionSourceTest extends \PHPUnit_Framework_TestCase { + /** + * @covers \DI\Definition\Source\ReflectionDefinitionSource::getDefinition + */ public function testUnknownClass() { $source = new ReflectionDefinitionSource(); $this->assertNull($source->getDefinition('foo')); } - public function testFixtureClass() + /** + * @covers \DI\Definition\Source\ReflectionDefinitionSource::getDefinition + * @covers \DI\Definition\Source\ReflectionDefinitionSource::getMethodInjection + */ + public function testConstructor() { $source = new ReflectionDefinitionSource(); $definition = $source->getDefinition('UnitTests\DI\Definition\Source\Fixtures\ReflectionFixture'); - $this->assertInstanceOf('DI\Definition\Definition', $definition); + $this->assertInstanceOf('DI\Definition\ClassDefinition', $definition); $constructorInjection = $definition->getConstructorInjection(); $this->assertInstanceOf('DI\Definition\ClassInjection\MethodInjection', $constructorInjection); $parameters = $constructorInjection->getParameters(); - $this->assertCount(3, $parameters); + $this->assertCount(1, $parameters); - $param1 = $parameters[0]; + $param1 = $parameters['param1']; $this->assertEquals(new EntryReference('UnitTests\DI\Definition\Source\Fixtures\ReflectionFixture'), $param1); + } + + /** + * @covers \DI\Definition\Source\ReflectionDefinitionSource::getDefinition + */ + public function testConstructorInParentClass() + { + $source = new ReflectionDefinitionSource(); + $definition = $source->getDefinition('UnitTests\DI\Definition\Source\Fixtures\ReflectionFixtureChild'); + $this->assertInstanceOf('DI\Definition\ClassDefinition', $definition); + + $constructorInjection = $definition->getConstructorInjection(); + $this->assertInstanceOf('DI\Definition\ClassInjection\MethodInjection', $constructorInjection); - $param2 = $parameters[1]; - $this->assertEquals(new UndefinedInjection(), $param2); + $parameters = $constructorInjection->getParameters(); + $this->assertCount(1, $parameters); + + $param1 = $parameters['param1']; + $this->assertEquals(new EntryReference('UnitTests\DI\Definition\Source\Fixtures\ReflectionFixture'), $param1); + } - $param3 = $parameters[2]; - $this->assertEquals(new UndefinedInjection(), $param3); + /** + * @covers \DI\Definition\Source\ReflectionDefinitionSource::getPropertyInjection + */ + public function testGetPropertyInjection() + { + $property = $this->getMock('\ReflectionProperty', null, array(), '', false); + + $source = new ReflectionDefinitionSource(); + $this->assertNull($source->getPropertyInjection($property)); } } From d344124ac61a6df86e562c47b2cee8655f2c6af3 Mon Sep 17 00:00:00 2001 From: Matthieu Napoli Date: Tue, 10 Dec 2013 23:43:59 +0100 Subject: [PATCH 02/10] Improved lazy reading of a definition file --- src/DI/Definition/Source/ArrayDefinitionSource.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/DI/Definition/Source/ArrayDefinitionSource.php b/src/DI/Definition/Source/ArrayDefinitionSource.php index 5aced67fd..019f0bd0e 100644 --- a/src/DI/Definition/Source/ArrayDefinitionSource.php +++ b/src/DI/Definition/Source/ArrayDefinitionSource.php @@ -49,10 +49,6 @@ public function __construct($file = null) return; } - if (! is_readable($file)) { - throw new DefinitionException("File $file doesn't exist or is not readable"); - } - // If we are given a file containing an array, we lazy-load it to improve performance $this->initialized = false; $this->file = $file; @@ -112,6 +108,10 @@ private function initialize() return; } + if (! is_readable($this->file)) { + throw new DefinitionException("File {$this->file} doesn't exist or is not readable"); + } + $definitions = require $this->file; if (! is_array($definitions)) { From 322c63cc45c3931eb38f5c014b02d469d1ba12a3 Mon Sep 17 00:00:00 2001 From: Matthieu Napoli Date: Wed, 11 Dec 2013 00:36:25 +0100 Subject: [PATCH 03/10] Refactoring of the ArrayDefinitionSource, DefinitionManager, Container, ContainerBuilder --- src/DI/Container.php | 10 -- src/DI/ContainerBuilder.php | 37 ++++- src/DI/Definition/ClassDefinition.php | 18 +++ src/DI/Definition/DefinitionManager.php | 152 ++---------------- .../Source/AnnotationDefinitionSource.php | 8 +- .../Source/ArrayDefinitionSource.php | 88 +++++----- .../Source/ChainableDefinitionSource.php | 25 +++ .../Source/ClassDefinitionSource.php | 7 +- .../Source/InMemoryDefinitionSource.php | 95 +++++++++++ .../Source/ReflectionDefinitionSource.php | 6 +- .../Source/SimpleDefinitionSource.php | 47 ------ .../Source/ArrayDefinitionSourceTest.php | 34 ++++ 12 files changed, 267 insertions(+), 260 deletions(-) create mode 100644 src/DI/Definition/Source/ChainableDefinitionSource.php create mode 100644 src/DI/Definition/Source/InMemoryDefinitionSource.php delete mode 100644 src/DI/Definition/Source/SimpleDefinitionSource.php diff --git a/src/DI/Container.php b/src/DI/Container.php index 925459879..75861d4bf 100644 --- a/src/DI/Container.php +++ b/src/DI/Container.php @@ -196,16 +196,6 @@ public function set($name, $value) $this->definitionManager->addDefinition($definition); } - /** - * Add definitions from an array - * - * @param array $definitions - */ - public function addDefinitions(array $definitions) - { - $this->definitionManager->addArrayDefinitions($definitions); - } - /** * @return DefinitionManager */ diff --git a/src/DI/ContainerBuilder.php b/src/DI/ContainerBuilder.php index 99484e18b..4d989b561 100644 --- a/src/DI/ContainerBuilder.php +++ b/src/DI/ContainerBuilder.php @@ -10,7 +10,9 @@ namespace DI; use DI\Definition\DefinitionManager; -use DI\Definition\Source\DefinitionSource; +use DI\Definition\Source\AnnotationDefinitionSource; +use DI\Definition\Source\ChainableDefinitionSource; +use DI\Definition\Source\ReflectionDefinitionSource; use Doctrine\Common\Cache\Cache; use InvalidArgumentException; use ProxyManager\Configuration; @@ -68,7 +70,7 @@ class ContainerBuilder /** * Source of definitions for the container. - * @var DefinitionSource[] + * @var ChainableDefinitionSource[] */ private $definitionSources = array(); @@ -96,14 +98,33 @@ public function __construct($containerClass = 'DI\Container') */ public function build() { + // Definition sources + $source = null; + foreach ($this->definitionSources as $definitionSource) { + if ($source instanceof ChainableDefinitionSource) { + $definitionSource->chain($source); + } + $source = $definitionSource; + } + if ($this->useAnnotations) { + if ($source) { + $source->chain(new AnnotationDefinitionSource()); + } else { + $source = new AnnotationDefinitionSource(); + } + } elseif ($this->useReflection) { + if ($source) { + $source->chain(new ReflectionDefinitionSource()); + } else { + $source = new ReflectionDefinitionSource(); + } + } + // Definition manager - $definitionManager = new DefinitionManager($this->useReflection, $this->useAnnotations); + $definitionManager = new DefinitionManager($source); if ($this->cache) { $definitionManager->setCache($this->cache); } - foreach ($this->definitionSources as $definitionSource) { - $definitionManager->addDefinitionSource($definitionSource); - } // Proxy factory $proxyFactory = $this->buildProxyFactory(); @@ -197,9 +218,9 @@ public function wrapContainer(ContainerInterface $otherContainer) * Do not add ReflectionDefinitionSource or AnnotationDefinitionSource manually, they should be * handled with useReflection() and useAnnotations(). * - * @param DefinitionSource $definitionSource + * @param ChainableDefinitionSource $definitionSource */ - public function addDefinitions(DefinitionSource $definitionSource) + public function addDefinitions(ChainableDefinitionSource $definitionSource) { $this->definitionSources[] = $definitionSource; } diff --git a/src/DI/Definition/ClassDefinition.php b/src/DI/Definition/ClassDefinition.php index 6d7409bdf..6fbf717c6 100644 --- a/src/DI/Definition/ClassDefinition.php +++ b/src/DI/Definition/ClassDefinition.php @@ -130,6 +130,15 @@ public function getPropertyInjections() return $this->propertyInjections; } + /** + * @param string $propertyName + * @return PropertyInjection + */ + public function getPropertyInjection($propertyName) + { + return isset($this->propertyInjections[$propertyName]) ? $this->propertyInjections[$propertyName] : null; + } + /** * @param PropertyInjection $propertyInjection */ @@ -146,6 +155,15 @@ public function getMethodInjections() return $this->methodInjections; } + /** + * @param string $methodName + * @return MethodInjection + */ + public function getMethodInjection($methodName) + { + return isset($this->methodInjections[$methodName]) ? $this->methodInjections[$methodName] : null; + } + /** * @param MethodInjection $methodInjection */ diff --git a/src/DI/Definition/DefinitionManager.php b/src/DI/Definition/DefinitionManager.php index 218be4abc..e6998f85c 100644 --- a/src/DI/Definition/DefinitionManager.php +++ b/src/DI/Definition/DefinitionManager.php @@ -9,12 +9,8 @@ namespace DI\Definition; -use DI\Definition\Source\AnnotationDefinitionSource; -use DI\Definition\Source\ArrayDefinitionSource; -use DI\Definition\Source\CombinedDefinitionSource; use DI\Definition\Source\DefinitionSource; -use DI\Definition\Source\ReflectionDefinitionSource; -use DI\Definition\Source\SimpleDefinitionSource; +use DI\Definition\Source\InMemoryDefinitionSource; use Doctrine\Common\Cache\Cache; /** @@ -37,45 +33,17 @@ class DefinitionManager private $cache; /** - * Source merging definitions of sub-sources - * @var CombinedDefinitionSource + * @var InMemoryDefinitionSource */ - private $combinedSource; + private $source; - /** - * Keep a reference to the simple source to add definitions to it - * @var SimpleDefinitionSource - */ - private $simpleSource; - - /** - * Keep a reference to the reflection source to ensure only one is used - * @var ReflectionDefinitionSource|null - */ - private $reflectionSource; - - /** - * Keep a reference to the annotation source to ensure only one is used - * @var AnnotationDefinitionSource|null - */ - private $annotationSource; - - /** - * @var DefinitionSource[] - */ - private $otherSources = array(); - - public function __construct($useReflection = true, $useAnnotations = true) + public function __construct(DefinitionSource $source = null) { - $this->simpleSource = new SimpleDefinitionSource(); - if ($useReflection) { - $this->reflectionSource = new ReflectionDefinitionSource(); - } - if ($useAnnotations) { - $this->annotationSource = new AnnotationDefinitionSource(); - } + $this->source = new InMemoryDefinitionSource(); - $this->updateCombinedSource(); + if ($source) { + $this->source->chain($source); + } } /** @@ -85,20 +53,11 @@ public function __construct($useReflection = true, $useAnnotations = true) */ public function getDefinition($name) { - // Look in cache first + // Look in cache $definition = $this->fetchFromCache($name); if ($definition === false) { - $definition = $this->combinedSource->getDefinition($name); - - // If alias, merge definition with alias - if ($definition instanceof ClassDefinition && $definition->isAlias()) { - $implementationDefinition = $this->getDefinition($definition->getClassName()); - - if ($implementationDefinition) { - $definition->merge($implementationDefinition); - } - } + $definition = $this->source->getDefinition($name); // Save to cache if ($definition === null || ($definition && $definition->isCacheable())) { @@ -109,69 +68,6 @@ public function getDefinition($name) return $definition; } - /** - * Enable or disable the use of reflection - * - * @param boolean $bool - */ - public function useReflection($bool) - { - // Enable - if ($bool && $this->reflectionSource === null) { - $this->reflectionSource = new ReflectionDefinitionSource(); - $this->updateCombinedSource(); - // Disable - } elseif (!$bool && $this->reflectionSource !== null) { - $this->reflectionSource = null; - $this->updateCombinedSource(); - } - } - - /** - * Enable or disable the use of annotations - * - * @param boolean $bool - */ - public function useAnnotations($bool) - { - // Enable - if ($bool && $this->annotationSource === null) { - $this->annotationSource = new AnnotationDefinitionSource(); - $this->updateCombinedSource(); - // Disable - } elseif (!$bool && $this->annotationSource !== null) { - $this->annotationSource = null; - $this->updateCombinedSource(); - } - } - - /** - * Add a source of definitions - * - * @param DefinitionSource $definitionSource - */ - public function addDefinitionSource(DefinitionSource $definitionSource) - { - $this->otherSources[] = $definitionSource; - - $this->updateCombinedSource(); - } - - /** - * Add definitions from an array - * - * @param array $definitions - */ - public function addArrayDefinitions(array $definitions) - { - $arraySource = new ArrayDefinitionSource(); - $arraySource->addDefinitions($definitions); - - $this->otherSources[] = $arraySource; - - $this->updateCombinedSource(); - } - /** * Add a single definition * @@ -179,7 +75,7 @@ public function addArrayDefinitions(array $definitions) */ public function addDefinition(Definition $definition) { - $this->simpleSource->addDefinition($definition); + $this->source->addDefinition($definition); } /** @@ -234,30 +130,4 @@ private function saveToCache($name, Definition $definition = null) $cacheKey = self::CACHE_PREFIX . $name; $this->cache->save($cacheKey, $definition); } - - /** - * Builds the combined source - * - * Builds from scratch every time because the order of the sources is important. - */ - private function updateCombinedSource() - { - // Sources are added from highest priority to least priority - $this->combinedSource = new CombinedDefinitionSource(); - - $this->combinedSource->addSource($this->simpleSource); - - // Traverses the array reversed so that the latest added is first - foreach (array_reverse($this->otherSources) as $arraySource) { - $this->combinedSource->addSource($arraySource); - } - - if ($this->annotationSource) { - $this->combinedSource->addSource($this->annotationSource); - } - - if ($this->reflectionSource) { - $this->combinedSource->addSource($this->reflectionSource); - } - } } diff --git a/src/DI/Definition/Source/AnnotationDefinitionSource.php b/src/DI/Definition/Source/AnnotationDefinitionSource.php index cbcdc05d9..249898d95 100644 --- a/src/DI/Definition/Source/AnnotationDefinitionSource.php +++ b/src/DI/Definition/Source/AnnotationDefinitionSource.php @@ -108,14 +108,14 @@ private function readProperties(ReflectionClass $reflectionClass, ClassDefinitio continue; } - $classDefinition->addPropertyInjection($this->getPropertyInjection($property)); + $classDefinition->addPropertyInjection($this->getPropertyInjection($reflectionClass->getName(), $property)); } } /** * {@inheritdoc} */ - public function getPropertyInjection(ReflectionProperty $property) + public function getPropertyInjection($entryName, ReflectionProperty $property) { // Look for @Inject annotation /** @var $annotation Inject */ @@ -148,7 +148,7 @@ private function readMethods(ReflectionClass $class, ClassDefinition $classDefin continue; } - $methodInjection = $this->getMethodInjection($class, $method); + $methodInjection = $this->getMethodInjection($class->getName(), $method); if (! $methodInjection) { continue; @@ -165,7 +165,7 @@ private function readMethods(ReflectionClass $class, ClassDefinition $classDefin /** * {@inheritdoc} */ - public function getMethodInjection(ReflectionClass $class, ReflectionMethod $method) + public function getMethodInjection($entryName, ReflectionMethod $method) { // Look for @Inject annotation /** @var $annotation Inject|null */ diff --git a/src/DI/Definition/Source/ArrayDefinitionSource.php b/src/DI/Definition/Source/ArrayDefinitionSource.php index 019f0bd0e..3250aff0a 100644 --- a/src/DI/Definition/Source/ArrayDefinitionSource.php +++ b/src/DI/Definition/Source/ArrayDefinitionSource.php @@ -14,14 +14,21 @@ use DI\Definition\Exception\DefinitionException; use DI\Definition\ValueDefinition; use DI\DefinitionHelper\DefinitionHelper; +use ReflectionMethod; +use ReflectionProperty; /** * Reads DI definitions from a PHP array, or a file returning a PHP array. * * @author Matthieu Napoli */ -class ArrayDefinitionSource implements DefinitionSource +class ArrayDefinitionSource implements DefinitionSource, ChainableDefinitionSource, ClassDefinitionSource { + /** + * @var DefinitionSource + */ + private $chainedSource; + /** * @var bool */ @@ -61,11 +68,10 @@ public function getDefinition($name) { $this->initialize(); - if (!array_key_exists($name, $this->definitions)) { - if (class_exists($name)) { - $definition = new ClassDefinition($name); - $this->mergeWithParents($name, $definition); - return $definition; + if (! array_key_exists($name, $this->definitions)) { + // Not found, we use the chain or return null + if ($this->chainedSource) { + return $this->chainedSource->getDefinition($name); } return null; } @@ -80,14 +86,41 @@ public function getDefinition($name) $definition = new ValueDefinition($name, $definition); } - // If it's a class, merge definitions from parent classes and interfaces if ($definition instanceof ClassDefinition) { - $this->mergeWithParents($name, $definition); + // TODO merge properties and methods with sub-sources } return $definition; } + /** + * {@inheritdoc} + */ + public function getPropertyInjection($entryName, ReflectionProperty $property) + { + $definition = $this->getDefinition($entryName); + + if ($definition && $definition instanceof ClassDefinition) { + return $definition->getPropertyInjection($property->getName()); + } + + return null; + } + + /** + * {@inheritdoc} + */ + public function getMethodInjection($entryName, ReflectionMethod $method) + { + $definition = $this->getDefinition($entryName); + + if ($definition && $definition instanceof ClassDefinition) { + return $definition->getMethodInjection($method->getName()); + } + + return null; + } + /** * @param array $definitions DI definitions in a PHP array. */ @@ -124,43 +157,10 @@ private function initialize() } /** - * Merge a class definition which the definitions of its parent classes and its interfaces. - * - * @param string $name - * @param ClassDefinition $definition + * {@inheritdoc} */ - private function mergeWithParents($name, ClassDefinition $definition) + public function chain(DefinitionSource $source) { - $className = $definition->getClassName(); - if (!class_exists($className)) { - return; - } - - // Parent class - $parentClass = get_parent_class($className); - - // Avoids loops (if AbstractClass1 is an alias to Class1, which extends AbstractClass1) - if ($parentClass && $parentClass != $name) { - $parentDefinition = $this->getDefinition($parentClass); - if ($parentDefinition instanceof ClassDefinition) { - $definition->merge($this->getDefinition($parentClass)); - } - } - - // Interfaces - $interfaces = class_implements($className); - - if (is_array($interfaces)) { - foreach ($interfaces as $interfaceName) { - // Avoids loops (if Interface1 is an alias to Class1, which implements Interface1) - if ($interfaceName == $name) { - continue; - } - $interfaceDefinition = $this->getDefinition($interfaceName); - if ($interfaceDefinition instanceof ClassDefinition) { - $definition->merge($interfaceDefinition); - } - } - } + $this->chainedSource = $source; } } diff --git a/src/DI/Definition/Source/ChainableDefinitionSource.php b/src/DI/Definition/Source/ChainableDefinitionSource.php new file mode 100644 index 000000000..30d9ce128 --- /dev/null +++ b/src/DI/Definition/Source/ChainableDefinitionSource.php @@ -0,0 +1,25 @@ + + */ +interface ChainableDefinitionSource extends DefinitionSource +{ + /** + * Chain another definition source after this one. + * + * @param DefinitionSource $source + */ + public function chain(DefinitionSource $source); +} diff --git a/src/DI/Definition/Source/ClassDefinitionSource.php b/src/DI/Definition/Source/ClassDefinitionSource.php index 9bfc596a2..7288f225e 100644 --- a/src/DI/Definition/Source/ClassDefinitionSource.php +++ b/src/DI/Definition/Source/ClassDefinitionSource.php @@ -26,19 +26,20 @@ interface ClassDefinitionSource /** * Returns the injection definition for the given property. * + * @param string $entryName * @param ReflectionProperty $property * * @return PropertyInjection|null */ - public function getPropertyInjection(ReflectionProperty $property); + public function getPropertyInjection($entryName, ReflectionProperty $property); /** * Returns the injection definition for the given method. * - * @param ReflectionClass $class + * @param string $entryName * @param ReflectionMethod $method * * @return MethodInjection|null */ - public function getMethodInjection(ReflectionClass $class, ReflectionMethod $method); + public function getMethodInjection($entryName, ReflectionMethod $method); } diff --git a/src/DI/Definition/Source/InMemoryDefinitionSource.php b/src/DI/Definition/Source/InMemoryDefinitionSource.php new file mode 100644 index 000000000..4e9d8a0b1 --- /dev/null +++ b/src/DI/Definition/Source/InMemoryDefinitionSource.php @@ -0,0 +1,95 @@ + + */ +class InMemoryDefinitionSource implements DefinitionSource, ChainableDefinitionSource, ClassDefinitionSource +{ + /** + * @var Definition[] + */ + private $definitions = array(); + + /** + * @var DefinitionSource + */ + private $chainedSource; + + /** + * {@inheritdoc} + */ + public function getDefinition($name) + { + if (array_key_exists($name, $this->definitions)) { + return $this->definitions[$name]; + } + + if ($this->chainedSource) { + return $this->chainedSource->getDefinition($name); + } + + return null; + } + + /** + * Add a definition + * + * @param Definition $definition + */ + public function addDefinition(Definition $definition) + { + $this->definitions[$definition->getName()] = $definition; + } + + /** + * {@inheritdoc} + */ + public function chain(DefinitionSource $source) + { + $this->chainedSource = $source; + } + + /** + * {@inheritdoc} + */ + public function getPropertyInjection($entryName, ReflectionProperty $property) + { + $definition = $this->getDefinition($entryName); + + if ($definition && $definition instanceof ClassDefinition) { + return $definition->getPropertyInjection($property->getName()); + } + + return null; + } + + /** + * {@inheritdoc} + */ + public function getMethodInjection($entryName, ReflectionMethod $method) + { + $definition = $this->getDefinition($entryName); + + if ($definition && $definition instanceof ClassDefinition) { + return $definition->getMethodInjection($method->getName()); + } + + return null; + } +} diff --git a/src/DI/Definition/Source/ReflectionDefinitionSource.php b/src/DI/Definition/Source/ReflectionDefinitionSource.php index f75c9f140..0009d60e1 100644 --- a/src/DI/Definition/Source/ReflectionDefinitionSource.php +++ b/src/DI/Definition/Source/ReflectionDefinitionSource.php @@ -39,7 +39,7 @@ public function getDefinition($name) $constructor = $class->getConstructor(); if ($constructor && $constructor->isPublic()) { - $classDefinition->setConstructorInjection($this->getMethodInjection($class, $constructor)); + $classDefinition->setConstructorInjection($this->getMethodInjection($name, $constructor)); } return $classDefinition; @@ -48,7 +48,7 @@ public function getDefinition($name) /** * {@inheritdoc} */ - public function getPropertyInjection(ReflectionProperty $property) + public function getPropertyInjection($entryName, ReflectionProperty $property) { // Nothing to guess on properties return null; @@ -57,7 +57,7 @@ public function getPropertyInjection(ReflectionProperty $property) /** * {@inheritdoc} */ - public function getMethodInjection(ReflectionClass $class, ReflectionMethod $method) + public function getMethodInjection($entryName, ReflectionMethod $method) { $parameters = array(); diff --git a/src/DI/Definition/Source/SimpleDefinitionSource.php b/src/DI/Definition/Source/SimpleDefinitionSource.php deleted file mode 100644 index bbc2f4448..000000000 --- a/src/DI/Definition/Source/SimpleDefinitionSource.php +++ /dev/null @@ -1,47 +0,0 @@ - - */ -class SimpleDefinitionSource implements DefinitionSource -{ - /** - * @var Definition[] - */ - private $definitions = array(); - - /** - * {@inheritdoc} - */ - public function getDefinition($name) - { - if (array_key_exists($name, $this->definitions)) { - return $this->definitions[$name]; - } - - return null; - } - - /** - * Add a definition - * - * @param Definition $definition - */ - public function addDefinition(Definition $definition) - { - $this->definitions[$definition->getName()] = $definition; - } -} diff --git a/tests/UnitTests/DI/Definition/Source/ArrayDefinitionSourceTest.php b/tests/UnitTests/DI/Definition/Source/ArrayDefinitionSourceTest.php index 8579f453e..6b7503a1f 100644 --- a/tests/UnitTests/DI/Definition/Source/ArrayDefinitionSourceTest.php +++ b/tests/UnitTests/DI/Definition/Source/ArrayDefinitionSourceTest.php @@ -18,6 +18,9 @@ */ class ArrayDefinitionSourceTest extends \PHPUnit_Framework_TestCase { + /** + * @covers \DI\Definition\Source\ArrayDefinitionSource + */ public function testValueDefinition() { $source = new ArrayDefinitionSource(); @@ -32,6 +35,9 @@ public function testValueDefinition() $this->assertEquals('bar', $definition->getValue()); } + /** + * @covers \DI\Definition\Source\ArrayDefinitionSource + */ public function testValueTypes() { $source = new ArrayDefinitionSource(); @@ -76,6 +82,9 @@ public function testValueTypes() $this->assertInstanceOf('Closure', $definition->getValue()); } + /** + * @covers \DI\Definition\Source\ArrayDefinitionSource + */ public function testClassDefinition() { $source = new ArrayDefinitionSource(); @@ -89,6 +98,9 @@ public function testClassDefinition() $this->assertEquals('foo', $definition->getClassName()); } + /** + * @covers \DI\Definition\Source\ArrayDefinitionSource + */ public function testClosureDefinition() { $callable = function () { @@ -105,6 +117,9 @@ public function testClosureDefinition() $this->assertEquals($callable, $definition->getCallable()); } + /** + * @covers \DI\Definition\Source\ArrayDefinitionSource + */ public function testLoadFromFile() { $source = new ArrayDefinitionSource(__DIR__ . '/Fixtures/definitions.php'); @@ -120,4 +135,23 @@ public function testLoadFromFile() $this->assertEquals('bim', $definition->getName()); $this->assertEquals('bim', $definition->getClassName()); } + + /** + * @covers \DI\Definition\Source\ArrayDefinitionSource::getDefinition + * @covers \DI\Definition\Source\ArrayDefinitionSource::chain + */ + public function testChainableSource() + { + $source = new ArrayDefinitionSource(__DIR__ . '/Fixtures/definitions.php'); + + $otherSource = $this->getMockForAbstractClass('DI\Definition\Source\DefinitionSource'); + $otherSource->expects($this->once()) + ->method('getDefinition') + ->with('some unknown entry') + ->will($this->returnValue(42)); + + $source->chain($otherSource); + + $this->assertEquals(42, $source->getDefinition('some unknown entry')); + } } From 6ea5f25acaf6b126f065b288bcbe23070527fa6e Mon Sep 17 00:00:00 2001 From: Matthieu Napoli Date: Wed, 11 Dec 2013 20:24:15 +0100 Subject: [PATCH 04/10] Improved code coverage reports by using @covers annotations --- tests/UnitTests/DI/ContainerBuilderTest.php | 13 +++++ tests/UnitTests/DI/ContainerTest.php | 47 ++++++++++++------- .../DI/Definition/DefinitionManagerTest.php | 1 + tests/UnitTests/DI/FunctionsTest.php | 9 ++++ 4 files changed, 53 insertions(+), 17 deletions(-) diff --git a/tests/UnitTests/DI/ContainerBuilderTest.php b/tests/UnitTests/DI/ContainerBuilderTest.php index 2f555369e..45a8b6d14 100644 --- a/tests/UnitTests/DI/ContainerBuilderTest.php +++ b/tests/UnitTests/DI/ContainerBuilderTest.php @@ -17,6 +17,9 @@ */ class ContainerBuilderTest extends \PHPUnit_Framework_TestCase { + /** + * @covers \DI\ContainerBuilder + */ public function testDefaultConfiguration() { // Make the ContainerBuilder use our fake class to catch constructor parameters @@ -28,6 +31,9 @@ public function testDefaultConfiguration() $this->assertNull($container->definitionManager->getCache()); } + /** + * @covers \DI\ContainerBuilder + */ public function testSetCache() { $cache = $this->getMockForAbstractClass('Doctrine\Common\Cache\Cache'); @@ -43,6 +49,7 @@ public function testSetCache() /** * By default, the definition resolvers should not be overridden + * @covers \DI\ContainerBuilder */ public function testContainerNotWrapped() { @@ -53,6 +60,9 @@ public function testContainerNotWrapped() $this->assertNull($container->wrapperContainer); } + /** + * @covers \DI\ContainerBuilder + */ public function testContainerWrapped() { $otherContainer = $this->getMockForAbstractClass('DI\ContainerInterface'); @@ -66,6 +76,9 @@ public function testContainerWrapped() $this->assertSame($otherContainer, $container->wrapperContainer); } + /** + * @covers \DI\ContainerBuilder + */ public function testFluentInterface() { $builder = new ContainerBuilder(); diff --git a/tests/UnitTests/DI/ContainerTest.php b/tests/UnitTests/DI/ContainerTest.php index 51d7b10d9..777591562 100644 --- a/tests/UnitTests/DI/ContainerTest.php +++ b/tests/UnitTests/DI/ContainerTest.php @@ -17,6 +17,9 @@ */ class ContainerTest extends \PHPUnit_Framework_TestCase { + /** + * @covers \DI\Container + */ public function testSetGet() { $container = ContainerBuilder::buildDevContainer(); @@ -27,6 +30,7 @@ public function testSetGet() /** * @expectedException \DI\NotFoundException + * @covers \DI\Container */ public function testGetNotFound() { @@ -34,6 +38,9 @@ public function testGetNotFound() $container->get('key'); } + /** + * @coversNothing + */ public function testClosureIsNotResolved() { $closure = function () { @@ -44,12 +51,18 @@ public function testClosureIsNotResolved() $this->assertSame($closure, $container->get('key')); } + /** + * @covers \DI\Container + */ public function testGetWithClassName() { $container = ContainerBuilder::buildDevContainer(); $this->assertInstanceOf('stdClass', $container->get('stdClass')); } + /** + * @covers \DI\Container + */ public function testGetWithPrototypeScope() { $container = ContainerBuilder::buildDevContainer(); @@ -59,6 +72,9 @@ public function testGetWithPrototypeScope() $this->assertNotSame($instance1, $instance2); } + /** + * @covers \DI\Container + */ public function testGetWithSingletonScope() { $container = ContainerBuilder::buildDevContainer(); @@ -75,6 +91,7 @@ public function testGetWithSingletonScope() /** * @expectedException \DI\Definition\Exception\DefinitionException * @expectedExceptionMessage Error while reading @Injectable on UnitTests\DI\Fixtures\InvalidScope: Value 'foobar' is not part of the enum DI\Scope + * @coversNothing */ public function testGetWithInvalidScope() { @@ -82,25 +99,9 @@ public function testGetWithInvalidScope() $container->get('UnitTests\DI\Fixtures\InvalidScope'); } - public function testGetWithProxy() - { - $container = ContainerBuilder::buildDevContainer(); - $this->assertInstanceOf('stdClass', $container->get('stdClass', true)); - } - - /** - * Issue #58 - * @see https://github.com/mnapoli/PHP-DI/issues/58 - */ - public function testGetWithProxyWithAlias() - { - $container = ContainerBuilder::buildDevContainer(); - $container->set('foo', \DI\object('stdClass')); - $this->assertInstanceOf('stdClass', $container->get('foo', true)); - } - /** * Tests if instantiation unlock works. We should be able to create two instances of the same class. + * @covers \DI\Container */ public function testCircularDependencies() { @@ -112,6 +113,7 @@ public function testCircularDependencies() /** * @expectedException \DI\DependencyException * @expectedExceptionMessage Circular dependency detected while trying to get entry 'UnitTests\DI\Fixtures\Class1CircularDependencies' + * @covers \DI\Container */ public function testCircularDependencyException() { @@ -122,6 +124,7 @@ public function testCircularDependencyException() /** * @expectedException \DI\DependencyException * @expectedExceptionMessage Circular dependency detected while trying to get entry 'foo' + * @covers \DI\Container */ public function testCircularDependencyExceptionWithAlias() { @@ -134,6 +137,7 @@ public function testCircularDependencyExceptionWithAlias() /** * @expectedException \InvalidArgumentException * @expectedExceptionMessage The name parameter must be of type string + * @covers \DI\Container::get */ public function testGetNonStringParameter() { @@ -141,6 +145,9 @@ public function testGetNonStringParameter() $container->get(new stdClass()); } + /** + * @covers \DI\Container + */ public function testHas() { $container = ContainerBuilder::buildDevContainer(); @@ -153,6 +160,7 @@ public function testHas() /** * @expectedException \InvalidArgumentException * @expectedExceptionMessage The name parameter must be of type string + * @covers \DI\Container::has */ public function testHasNonStringParameter() { @@ -162,6 +170,7 @@ public function testHasNonStringParameter() /** * Test that injecting an existing object returns the same reference to that object + * @covers \DI\Container */ public function testInjectOnMaintainsReferentialEquality() { @@ -174,6 +183,7 @@ public function testInjectOnMaintainsReferentialEquality() /** * Test that injection on null yields null + * @covers \DI\Container */ public function testInjectNull() { @@ -186,6 +196,7 @@ public function testInjectNull() /** * We should be able to set a null value * @see https://github.com/mnapoli/PHP-DI/issues/79 + * @covers \DI\Container */ public function testSetNullValue() { @@ -197,6 +208,7 @@ public function testSetNullValue() /** * The container auto-registers itself + * @covers \DI\Container */ public function testContainerIsRegistered() { @@ -209,6 +221,7 @@ public function testContainerIsRegistered() /** * @see https://github.com/mnapoli/PHP-DI/issues/126 * @test + * @covers \DI\Container */ public function testSetGetSetGet() { diff --git a/tests/UnitTests/DI/Definition/DefinitionManagerTest.php b/tests/UnitTests/DI/Definition/DefinitionManagerTest.php index 1917c4f7d..65f364ce1 100644 --- a/tests/UnitTests/DI/Definition/DefinitionManagerTest.php +++ b/tests/UnitTests/DI/Definition/DefinitionManagerTest.php @@ -18,6 +18,7 @@ class DefinitionManagerTest extends \PHPUnit_Framework_TestCase { /** * @test + * @covers \DI\Definition\DefinitionManager */ public function shouldUseCache() { diff --git a/tests/UnitTests/DI/FunctionsTest.php b/tests/UnitTests/DI/FunctionsTest.php index e271501dd..e42bb7337 100644 --- a/tests/UnitTests/DI/FunctionsTest.php +++ b/tests/UnitTests/DI/FunctionsTest.php @@ -14,6 +14,9 @@ */ class FunctionsTest extends \PHPUnit_Framework_TestCase { + /** + * @covers ::\DI\object + */ public function testObject() { $definition = \DI\object(); @@ -27,6 +30,9 @@ public function testObject() $this->assertEquals('foo', $definition->getDefinition('entry')->getClassName()); } + /** + * @covers ::\DI\factory + */ public function testFactory() { $definition = \DI\factory(function () { @@ -38,6 +44,9 @@ public function testFactory() $this->assertEquals(42, $callable()); } + /** + * @covers ::\DI\link + */ public function testLink() { $reference = \DI\link('foo'); From b6082ca21e6f099b4fbcb5b37a05abadca6bd24e Mon Sep 17 00:00:00 2001 From: Matthieu Napoli Date: Wed, 11 Dec 2013 20:24:30 +0100 Subject: [PATCH 05/10] Fixed a bug in AnnotationDefinitionSource --- src/DI/Definition/Source/AnnotationDefinitionSource.php | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/DI/Definition/Source/AnnotationDefinitionSource.php b/src/DI/Definition/Source/AnnotationDefinitionSource.php index 249898d95..5b370c5e6 100644 --- a/src/DI/Definition/Source/AnnotationDefinitionSource.php +++ b/src/DI/Definition/Source/AnnotationDefinitionSource.php @@ -108,7 +108,11 @@ private function readProperties(ReflectionClass $reflectionClass, ClassDefinitio continue; } - $classDefinition->addPropertyInjection($this->getPropertyInjection($reflectionClass->getName(), $property)); + $propertyInjection = $this->getPropertyInjection($reflectionClass->getName(), $property); + + if ($propertyInjection) { + $classDefinition->addPropertyInjection($propertyInjection); + } } } From c96243561b246dd0d9d37c06a32b81e94be78645 Mon Sep 17 00:00:00 2001 From: Matthieu Napoli Date: Wed, 11 Dec 2013 20:24:47 +0100 Subject: [PATCH 06/10] Removed CombinedDefinitionSource --- .../Source/CombinedDefinitionSource.php | 87 ------------------- .../Source/CombinedDefinitionSourceTest.php | 53 ----------- 2 files changed, 140 deletions(-) delete mode 100644 src/DI/Definition/Source/CombinedDefinitionSource.php delete mode 100644 tests/UnitTests/DI/Definition/Source/CombinedDefinitionSourceTest.php diff --git a/src/DI/Definition/Source/CombinedDefinitionSource.php b/src/DI/Definition/Source/CombinedDefinitionSource.php deleted file mode 100644 index c58f77679..000000000 --- a/src/DI/Definition/Source/CombinedDefinitionSource.php +++ /dev/null @@ -1,87 +0,0 @@ - - */ -class CombinedDefinitionSource implements DefinitionSource -{ - /** - * Sub-sources - * @var DefinitionSource[] - */ - private $subSources = array(); - - /** - * {@inheritdoc} - */ - public function getDefinition($name) - { - /** @var $definition Definition|null */ - $definition = null; - - foreach ($this->subSources as $subSource) { - $subDefinition = $subSource->getDefinition($name); - - if (!$subDefinition) { - continue; - } - - // If the definition can't be merged with other definitions, then we stop looking further - if (! $subDefinition::isMergeable()) { - return $subDefinition; - } - - if ($definition) { - $subDefinition->merge($definition); - } - - $definition = $subDefinition; - } - - return $definition; - } - - /** - * @return DefinitionSource[] - */ - public function getSources() - { - return $this->subSources; - } - - /** - * @param DefinitionSource $source - */ - public function removeSource(DefinitionSource $source) - { - foreach ($this->subSources as $key => $subSource) { - if ($subSource === $source) { - unset($this->subSources[$key]); - } - } - } - - /** - * Add a definition source to the stack - * @param DefinitionSource $source - */ - public function addSource($source) - { - $this->subSources[] = $source; - } -} diff --git a/tests/UnitTests/DI/Definition/Source/CombinedDefinitionSourceTest.php b/tests/UnitTests/DI/Definition/Source/CombinedDefinitionSourceTest.php deleted file mode 100644 index 6a999a4bd..000000000 --- a/tests/UnitTests/DI/Definition/Source/CombinedDefinitionSourceTest.php +++ /dev/null @@ -1,53 +0,0 @@ -assertEmpty($source->getSources()); - - $source->addSource(new ArrayDefinitionSource()); - $this->assertCount(1, $source->getSources()); - - $source->addSource(new ArrayDefinitionSource()); - $this->assertCount(2, $source->getSources()); - } - - public function testSubSourcesCalled() - { - $source = new CombinedDefinitionSource(); - $this->assertEmpty($source->getSources()); - - $subSource1 = $this->getMockForAbstractClass('DI\Definition\Source\DefinitionSource'); - $source->addSource($subSource1); - - // The sub source should have its method 'getDefinition' called once - $subSource1->expects($this->once())->method('getDefinition') - ->will($this->returnValue(null)); - - $subSource2 = $this->getMockForAbstractClass('DI\Definition\Source\DefinitionSource'); - $source->addSource($subSource2); - - // The sub source should have its method 'getDefinition' called once - $subSource2->expects($this->once())->method('getDefinition') - ->will($this->returnValue(null)); - - $source->getDefinition('foo'); - } -} From f1d4f3b97e336f605595e60b1e53ac7af9a5d833 Mon Sep 17 00:00:00 2001 From: Matthieu Napoli Date: Wed, 11 Dec 2013 20:25:50 +0100 Subject: [PATCH 07/10] Fixed test --- .../DI/Definition/Source/ReflectionDefinitionSourceTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/UnitTests/DI/Definition/Source/ReflectionDefinitionSourceTest.php b/tests/UnitTests/DI/Definition/Source/ReflectionDefinitionSourceTest.php index 1c5a277ae..c6e5df643 100644 --- a/tests/UnitTests/DI/Definition/Source/ReflectionDefinitionSourceTest.php +++ b/tests/UnitTests/DI/Definition/Source/ReflectionDefinitionSourceTest.php @@ -70,6 +70,6 @@ public function testGetPropertyInjection() $property = $this->getMock('\ReflectionProperty', null, array(), '', false); $source = new ReflectionDefinitionSource(); - $this->assertNull($source->getPropertyInjection($property)); + $this->assertNull($source->getPropertyInjection('foo', $property)); } } From c58488550140c8606d06d4c7e7421b10df0e263e Mon Sep 17 00:00:00 2001 From: Matthieu Napoli Date: Wed, 11 Dec 2013 20:32:35 +0100 Subject: [PATCH 08/10] Improved exception message in container --- src/DI/Container.php | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/DI/Container.php b/src/DI/Container.php index 75861d4bf..21c2d5ed0 100644 --- a/src/DI/Container.php +++ b/src/DI/Container.php @@ -96,7 +96,10 @@ public function __construct( public function get($name) { if (! is_string($name)) { - throw new InvalidArgumentException("The name parameter must be of type string"); + throw new InvalidArgumentException(sprintf( + 'The name parameter must be of type string, %s given', + is_object($name) ? get_class($name) : gettype($name) + )); } // Try to find the entry in the map From 61f03adeeaa57880d698d05dc09cb07248d10500 Mon Sep 17 00:00:00 2001 From: Matthieu Napoli Date: Wed, 11 Dec 2013 23:51:07 +0100 Subject: [PATCH 09/10] Refactoring of DefinitionSources --- src/DI/ContainerBuilder.php | 3 + src/DI/Definition/AliasDefinition.php | 18 +--- src/DI/Definition/CallableDefinition.php | 16 ---- src/DI/Definition/ClassDefinition.php | 33 +++---- .../ClassInjection/MethodInjection.php | 17 +++- .../ClassInjection/PropertyInjection.php | 14 --- src/DI/Definition/Definition.php | 16 ---- src/DI/Definition/DefinitionManager.php | 6 +- src/DI/Definition/MergeableDefinition.php | 27 ++++++ .../Source/AnnotationDefinitionSource.php | 50 ++++++---- .../Source/ArrayDefinitionSource.php | 58 +++++------ .../Source/ClassDefinitionSource.php | 45 --------- src/DI/Definition/Source/DefinitionSource.php | 7 +- .../Source/InMemoryDefinitionSource.php | 95 ------------------- .../Source/ReflectionDefinitionSource.php | 45 +++++---- src/DI/Definition/ValueDefinition.php | 20 +--- .../ClassDefinitionResolver.php | 49 +++------- .../DI/BuggyInjectionsTest.php | 2 + tests/IntegrationTests/DI/Fixtures/Class1.php | 5 +- .../LazyInjectionClass.php | 4 +- tests/IntegrationTests/DI/InheritanceTest.php | 51 +++++----- tests/IntegrationTests/DI/InjectionTest.php | 40 ++++---- .../DI/Issues/Issue70and76Test.php | 2 + .../DI/Issues/Issue72/definitions.php | 11 +++ .../DI/Issues/Issue72Test.php | 50 ++++++---- .../DI/PropertyInjectionTest.php | 2 + .../DI/Definition/AliasDefinitionTest.php | 15 --- .../DI/Definition/CallableDefinitionTest.php | 19 ---- .../DI/Definition/ClassDefinitionTest.php | 9 +- .../Source/AnnotationDefinitionSourceTest.php | 19 ++-- .../Source/ReflectionDefinitionSourceTest.php | 28 +----- .../DI/Definition/ValueDefinitionTest.php | 15 --- 32 files changed, 277 insertions(+), 514 deletions(-) create mode 100644 src/DI/Definition/MergeableDefinition.php delete mode 100644 src/DI/Definition/Source/ClassDefinitionSource.php delete mode 100644 src/DI/Definition/Source/InMemoryDefinitionSource.php create mode 100644 tests/IntegrationTests/DI/Issues/Issue72/definitions.php diff --git a/src/DI/ContainerBuilder.php b/src/DI/ContainerBuilder.php index 4d989b561..d9bcb15c2 100644 --- a/src/DI/ContainerBuilder.php +++ b/src/DI/ContainerBuilder.php @@ -219,6 +219,8 @@ public function wrapContainer(ContainerInterface $otherContainer) * handled with useReflection() and useAnnotations(). * * @param ChainableDefinitionSource $definitionSource + * + * @todo Give file directly */ public function addDefinitions(ChainableDefinitionSource $definitionSource) { @@ -231,6 +233,7 @@ public function addDefinitions(ChainableDefinitionSource $definitionSource) private function buildProxyFactory() { $config = new Configuration(); + // TODO use non-deprecated method $config->setAutoGenerateProxies(true); if ($this->writeProxiesToFile) { diff --git a/src/DI/Definition/AliasDefinition.php b/src/DI/Definition/AliasDefinition.php index 8f5ff3e31..c94b1020e 100644 --- a/src/DI/Definition/AliasDefinition.php +++ b/src/DI/Definition/AliasDefinition.php @@ -12,7 +12,7 @@ use DI\Scope; /** - * Defines an alias from an entry to another + * Defines an alias from an entry to another. * * @author Matthieu Napoli */ @@ -64,14 +64,6 @@ public function getTargetEntryName() return $this->targetEntryName; } - /** - * {@inheritdoc} - */ - public function merge(Definition $definition) - { - throw new \BadMethodCallException("Impossible to merge an AliasDefinition with another definition"); - } - /** * {@inheritdoc} */ @@ -79,12 +71,4 @@ public function isCacheable() { return true; } - - /** - * {@inheritdoc} - */ - public static function isMergeable() - { - return false; - } } diff --git a/src/DI/Definition/CallableDefinition.php b/src/DI/Definition/CallableDefinition.php index 9e9c0e310..4ba7c9e0a 100644 --- a/src/DI/Definition/CallableDefinition.php +++ b/src/DI/Definition/CallableDefinition.php @@ -73,14 +73,6 @@ public function getCallable() return $this->callable; } - /** - * {@inheritdoc} - */ - public function merge(Definition $definition) - { - throw new \BadMethodCallException("Impossible to merge a CallableDefinition with another definition"); - } - /** * {@inheritdoc} */ @@ -88,12 +80,4 @@ public function isCacheable() { return false; } - - /** - * {@inheritdoc} - */ - public static function isMergeable() - { - return false; - } } diff --git a/src/DI/Definition/ClassDefinition.php b/src/DI/Definition/ClassDefinition.php index 6fbf717c6..ba945cb2f 100644 --- a/src/DI/Definition/ClassDefinition.php +++ b/src/DI/Definition/ClassDefinition.php @@ -19,7 +19,7 @@ * * @author Matthieu Napoli */ -class ClassDefinition implements Definition +class ClassDefinition implements MergeableDefinition { /** * Entry name (most of the time, same as $classname) @@ -157,7 +157,7 @@ public function getMethodInjections() /** * @param string $methodName - * @return MethodInjection + * @return MethodInjection|null */ public function getMethodInjection($methodName) { @@ -212,21 +212,23 @@ public function isLazy() /** * {@inheritdoc} */ - public function merge(Definition $definition) + public function merge(MergeableDefinition $definition) { if (!$definition instanceof ClassDefinition) { - throw new DefinitionException("DI definition conflict: there are 2 different definitions for '" - . $definition->getName() . "' that are incompatible, they are not of the same type"); + throw new DefinitionException( + "DI definition conflict: there are 2 different definitions for '" . $this->getName() + . "' that are incompatible, they are not of the same type" + ); } - // The latter prevails - if ($definition->className !== null) { + // The current prevails + if ($this->className === null) { $this->className = $definition->className; } - if ($definition->scope !== null) { + if ($this->scope === null) { $this->scope = $definition->scope; } - if ($definition->lazy !== null) { + if ($this->lazy === null) { $this->lazy = $definition->lazy; } @@ -243,10 +245,7 @@ public function merge(Definition $definition) // Merge property injections foreach ($definition->getPropertyInjections() as $propertyName => $propertyInjection) { - if (array_key_exists($propertyName, $this->propertyInjections)) { - // Merge - $this->propertyInjections[$propertyName]->merge($propertyInjection); - } else { + if (! array_key_exists($propertyName, $this->propertyInjections)) { // Add $this->propertyInjections[$propertyName] = $propertyInjection; } @@ -271,12 +270,4 @@ public function isCacheable() { return true; } - - /** - * {@inheritdoc} - */ - public static function isMergeable() - { - return true; - } } diff --git a/src/DI/Definition/ClassInjection/MethodInjection.php b/src/DI/Definition/ClassInjection/MethodInjection.php index a060209f0..58e7ef92c 100644 --- a/src/DI/Definition/ClassInjection/MethodInjection.php +++ b/src/DI/Definition/ClassInjection/MethodInjection.php @@ -52,15 +52,28 @@ public function getParameters() return $this->parameters; } + /** + * @param int $index Position of the parameter (starting at 0) + * @return mixed|null Value to inject, or null if no injection defined. + */ + public function getParameter($index) + { + if (! isset($this->parameters[$index])) { + return null; + } + + return $this->parameters[$index]; + } + /** * Merge another definition into the current definition. * - * In case of conflicts, the latter prevails (i.e. the other definition) + * In case of conflicts, the current definition prevails. * * @param MethodInjection $methodInjection */ public function merge(MethodInjection $methodInjection) { - $this->parameters = $methodInjection->parameters + $this->parameters; + $this->parameters = $this->parameters + $methodInjection->parameters; } } diff --git a/src/DI/Definition/ClassInjection/PropertyInjection.php b/src/DI/Definition/ClassInjection/PropertyInjection.php index 087292de9..a5ac33a30 100644 --- a/src/DI/Definition/ClassInjection/PropertyInjection.php +++ b/src/DI/Definition/ClassInjection/PropertyInjection.php @@ -53,18 +53,4 @@ public function getValue() { return $this->value; } - - /** - * Merge another definition into the current definition - * - * In case of conflicts, the latter prevails (i.e. the other definition) - * - * @param PropertyInjection $propertyInjection - */ - public function merge(PropertyInjection $propertyInjection) - { - if ($propertyInjection->value !== null) { - $this->value = $propertyInjection->value; - } - } } diff --git a/src/DI/Definition/Definition.php b/src/DI/Definition/Definition.php index 07f25eba2..2c95f80c8 100644 --- a/src/DI/Definition/Definition.php +++ b/src/DI/Definition/Definition.php @@ -32,26 +32,10 @@ public function getName(); */ public function getScope(); - /** - * Merge another definition into the current definition - * - * In case of conflicts, the latter prevails (i.e. the other definition) - * - * @param Definition $definition - */ - public function merge(Definition $definition); - /** * Returns true if the definition can be cached, false otherwise * * @return bool */ public function isCacheable(); - - /** - * Returns true if the definition is mergeable with other definitions - * - * @return bool - */ - public static function isMergeable(); } diff --git a/src/DI/Definition/DefinitionManager.php b/src/DI/Definition/DefinitionManager.php index e6998f85c..f2bdb8c15 100644 --- a/src/DI/Definition/DefinitionManager.php +++ b/src/DI/Definition/DefinitionManager.php @@ -9,8 +9,8 @@ namespace DI\Definition; +use DI\Definition\Source\ArrayDefinitionSource; use DI\Definition\Source\DefinitionSource; -use DI\Definition\Source\InMemoryDefinitionSource; use Doctrine\Common\Cache\Cache; /** @@ -33,13 +33,13 @@ class DefinitionManager private $cache; /** - * @var InMemoryDefinitionSource + * @var ArrayDefinitionSource */ private $source; public function __construct(DefinitionSource $source = null) { - $this->source = new InMemoryDefinitionSource(); + $this->source = new ArrayDefinitionSource(); if ($source) { $this->source->chain($source); diff --git a/src/DI/Definition/MergeableDefinition.php b/src/DI/Definition/MergeableDefinition.php new file mode 100644 index 000000000..96c0e56ff --- /dev/null +++ b/src/DI/Definition/MergeableDefinition.php @@ -0,0 +1,27 @@ + + */ +interface MergeableDefinition extends Definition +{ + /** + * Merge another definition into the current definition. + * + * In case of conflicts, the current definition prevails. + * + * @param MergeableDefinition $definition + */ + public function merge(MergeableDefinition $definition); +} diff --git a/src/DI/Definition/Source/AnnotationDefinitionSource.php b/src/DI/Definition/Source/AnnotationDefinitionSource.php index 5b370c5e6..d8e4a43e1 100644 --- a/src/DI/Definition/Source/AnnotationDefinitionSource.php +++ b/src/DI/Definition/Source/AnnotationDefinitionSource.php @@ -17,6 +17,7 @@ use DI\Definition\Exception\DefinitionException; use DI\Definition\ClassInjection\MethodInjection; use DI\Definition\ClassInjection\PropertyInjection; +use DI\Definition\MergeableDefinition; use Doctrine\Common\Annotations\AnnotationRegistry; use Doctrine\Common\Annotations\Reader; use Doctrine\Common\Annotations\SimpleAnnotationReader; @@ -36,7 +37,7 @@ * * @author Matthieu Napoli */ -class AnnotationDefinitionSource implements DefinitionSource, ClassDefinitionSource +class AnnotationDefinitionSource implements DefinitionSource { /** * @var Reader @@ -53,14 +54,21 @@ class AnnotationDefinitionSource implements DefinitionSource, ClassDefinitionSou * @throws AnnotationException * @throws InvalidArgumentException The class doesn't exist */ - public function getDefinition($name) + public function getDefinition($name, MergeableDefinition $parentDefinition = null) { - if (!class_exists($name) && !interface_exists($name)) { + // Only merges with class definition + if ($parentDefinition && (! $parentDefinition instanceof ClassDefinition)) { return null; } - $class = new ReflectionClass($name); - $classDefinition = new ClassDefinition($name); + $className = $parentDefinition ? $parentDefinition->getClassName() : $name; + + if (!class_exists($className) && !interface_exists($className)) { + return null; + } + + $class = new ReflectionClass($className); + $definition = new ClassDefinition($name); // Injectable annotation /** @var $injectableAnnotation Injectable|null */ @@ -77,20 +85,26 @@ public function getDefinition($name) if ($injectableAnnotation) { if ($injectableAnnotation->getScope()) { - $classDefinition->setScope($injectableAnnotation->getScope()); + $definition->setScope($injectableAnnotation->getScope()); } if ($injectableAnnotation->isLazy() !== null) { - $classDefinition->setLazy($injectableAnnotation->isLazy()); + $definition->setLazy($injectableAnnotation->isLazy()); } } // Browse the class properties looking for annotated properties - $this->readProperties($class, $classDefinition); + $this->readProperties($class, $definition); // Browse the object's methods looking for annotated methods - $this->readMethods($class, $classDefinition); + $this->readMethods($class, $definition); - return $classDefinition; + // Merge with parent + if ($parentDefinition) { + $parentDefinition->merge($definition); + $definition = $parentDefinition; + } + + return $definition; } /** @@ -108,7 +122,7 @@ private function readProperties(ReflectionClass $reflectionClass, ClassDefinitio continue; } - $propertyInjection = $this->getPropertyInjection($reflectionClass->getName(), $property); + $propertyInjection = $this->getPropertyInjection($property); if ($propertyInjection) { $classDefinition->addPropertyInjection($propertyInjection); @@ -119,7 +133,7 @@ private function readProperties(ReflectionClass $reflectionClass, ClassDefinitio /** * {@inheritdoc} */ - public function getPropertyInjection($entryName, ReflectionProperty $property) + private function getPropertyInjection(ReflectionProperty $property) { // Look for @Inject annotation /** @var $annotation Inject */ @@ -132,7 +146,11 @@ public function getPropertyInjection($entryName, ReflectionProperty $property) $entryName = $annotation->getName() ?: $this->getPhpDocReader()->getPropertyType($property); if ($entryName === null) { - return null; + throw new AnnotationException(sprintf( + '@Inject found on property %s::%s but unable to guess what to inject, use a @var annotation', + $property->getDeclaringClass()->getName(), + $property->getName() + )); } return new PropertyInjection($property->getName(), new EntryReference($entryName)); @@ -152,7 +170,7 @@ private function readMethods(ReflectionClass $class, ClassDefinition $classDefin continue; } - $methodInjection = $this->getMethodInjection($class->getName(), $method); + $methodInjection = $this->getMethodInjection($method); if (! $methodInjection) { continue; @@ -169,7 +187,7 @@ private function readMethods(ReflectionClass $class, ClassDefinition $classDefin /** * {@inheritdoc} */ - public function getMethodInjection($entryName, ReflectionMethod $method) + private function getMethodInjection(ReflectionMethod $method) { // Look for @Inject annotation /** @var $annotation Inject|null */ @@ -186,7 +204,7 @@ public function getMethodInjection($entryName, ReflectionMethod $method) $entryName = $this->getMethodParameter($index, $parameter, $annotationParameters); if ($entryName !== null) { - $parameters[$parameter->getName()] = new EntryReference($entryName); + $parameters[$index] = new EntryReference($entryName); } } diff --git a/src/DI/Definition/Source/ArrayDefinitionSource.php b/src/DI/Definition/Source/ArrayDefinitionSource.php index 3250aff0a..bfb04b15d 100644 --- a/src/DI/Definition/Source/ArrayDefinitionSource.php +++ b/src/DI/Definition/Source/ArrayDefinitionSource.php @@ -9,20 +9,18 @@ namespace DI\Definition\Source; -use DI\Definition\ClassDefinition; use DI\Definition\Definition; use DI\Definition\Exception\DefinitionException; +use DI\Definition\MergeableDefinition; use DI\Definition\ValueDefinition; use DI\DefinitionHelper\DefinitionHelper; -use ReflectionMethod; -use ReflectionProperty; /** * Reads DI definitions from a PHP array, or a file returning a PHP array. * * @author Matthieu Napoli */ -class ArrayDefinitionSource implements DefinitionSource, ChainableDefinitionSource, ClassDefinitionSource +class ArrayDefinitionSource implements ChainableDefinitionSource { /** * @var DefinitionSource @@ -64,14 +62,14 @@ public function __construct($file = null) /** * {@inheritdoc} */ - public function getDefinition($name) + public function getDefinition($name, MergeableDefinition $parentDefinition = null) { $this->initialize(); if (! array_key_exists($name, $this->definitions)) { // Not found, we use the chain or return null if ($this->chainedSource) { - return $this->chainedSource->getDefinition($name); + return $this->chainedSource->getDefinition($name, $parentDefinition); } return null; } @@ -86,43 +84,27 @@ public function getDefinition($name) $definition = new ValueDefinition($name, $definition); } - if ($definition instanceof ClassDefinition) { - // TODO merge properties and methods with sub-sources + // If the definition we have is not mergeable, and we are supposed to merge, we ignore it + if ($parentDefinition && (! $definition instanceof MergeableDefinition)) { + return $parentDefinition; } - return $definition; - } - - /** - * {@inheritdoc} - */ - public function getPropertyInjection($entryName, ReflectionProperty $property) - { - $definition = $this->getDefinition($entryName); - - if ($definition && $definition instanceof ClassDefinition) { - return $definition->getPropertyInjection($property->getName()); + // Merge with parent + if ($parentDefinition) { + $parentDefinition->merge($definition); + $definition = $parentDefinition; } - return null; - } - - /** - * {@inheritdoc} - */ - public function getMethodInjection($entryName, ReflectionMethod $method) - { - $definition = $this->getDefinition($entryName); - - if ($definition && $definition instanceof ClassDefinition) { - return $definition->getMethodInjection($method->getName()); + // Enrich definition in sub-source + if ($this->chainedSource && $definition instanceof MergeableDefinition) { + $this->chainedSource->getDefinition($name, $definition); } - return null; + return $definition; } /** - * @param array $definitions DI definitions in a PHP array. + * @param array $definitions DI definitions in a PHP array indexed by the definition name. */ public function addDefinitions(array $definitions) { @@ -131,6 +113,14 @@ public function addDefinitions(array $definitions) $this->definitions = $definitions + $this->definitions; } + /** + * @param Definition $definition + */ + public function addDefinition(Definition $definition) + { + $this->definitions[$definition->getName()] = $definition; + } + /** * Lazy-loading of the definitions. * @throws DefinitionException diff --git a/src/DI/Definition/Source/ClassDefinitionSource.php b/src/DI/Definition/Source/ClassDefinitionSource.php deleted file mode 100644 index 7288f225e..000000000 --- a/src/DI/Definition/Source/ClassDefinitionSource.php +++ /dev/null @@ -1,45 +0,0 @@ - - * @since 4.0 - */ -interface ClassDefinitionSource -{ - /** - * Returns the injection definition for the given property. - * - * @param string $entryName - * @param ReflectionProperty $property - * - * @return PropertyInjection|null - */ - public function getPropertyInjection($entryName, ReflectionProperty $property); - - /** - * Returns the injection definition for the given method. - * - * @param string $entryName - * @param ReflectionMethod $method - * - * @return MethodInjection|null - */ - public function getMethodInjection($entryName, ReflectionMethod $method); -} diff --git a/src/DI/Definition/Source/DefinitionSource.php b/src/DI/Definition/Source/DefinitionSource.php index 2eb470007..309edd88d 100644 --- a/src/DI/Definition/Source/DefinitionSource.php +++ b/src/DI/Definition/Source/DefinitionSource.php @@ -11,6 +11,7 @@ use DI\Definition\Definition; use DI\Definition\Exception\DefinitionException; +use DI\Definition\MergeableDefinition; /** * Source of definitions for entries of the container. @@ -22,10 +23,12 @@ interface DefinitionSource /** * Returns the DI definition for the entry name. * - * @param string $name + * @param string $name + * @param MergeableDefinition|null $parentDefinition Given if a definition already exists + * and we are supposed to enrich it. * * @throws DefinitionException An invalid definition was found. * @return Definition|null */ - public function getDefinition($name); + public function getDefinition($name, MergeableDefinition $parentDefinition = null); } diff --git a/src/DI/Definition/Source/InMemoryDefinitionSource.php b/src/DI/Definition/Source/InMemoryDefinitionSource.php deleted file mode 100644 index 4e9d8a0b1..000000000 --- a/src/DI/Definition/Source/InMemoryDefinitionSource.php +++ /dev/null @@ -1,95 +0,0 @@ - - */ -class InMemoryDefinitionSource implements DefinitionSource, ChainableDefinitionSource, ClassDefinitionSource -{ - /** - * @var Definition[] - */ - private $definitions = array(); - - /** - * @var DefinitionSource - */ - private $chainedSource; - - /** - * {@inheritdoc} - */ - public function getDefinition($name) - { - if (array_key_exists($name, $this->definitions)) { - return $this->definitions[$name]; - } - - if ($this->chainedSource) { - return $this->chainedSource->getDefinition($name); - } - - return null; - } - - /** - * Add a definition - * - * @param Definition $definition - */ - public function addDefinition(Definition $definition) - { - $this->definitions[$definition->getName()] = $definition; - } - - /** - * {@inheritdoc} - */ - public function chain(DefinitionSource $source) - { - $this->chainedSource = $source; - } - - /** - * {@inheritdoc} - */ - public function getPropertyInjection($entryName, ReflectionProperty $property) - { - $definition = $this->getDefinition($entryName); - - if ($definition && $definition instanceof ClassDefinition) { - return $definition->getPropertyInjection($property->getName()); - } - - return null; - } - - /** - * {@inheritdoc} - */ - public function getMethodInjection($entryName, ReflectionMethod $method) - { - $definition = $this->getDefinition($entryName); - - if ($definition && $definition instanceof ClassDefinition) { - return $definition->getMethodInjection($method->getName()); - } - - return null; - } -} diff --git a/src/DI/Definition/Source/ReflectionDefinitionSource.php b/src/DI/Definition/Source/ReflectionDefinitionSource.php index 0009d60e1..b205b5f70 100644 --- a/src/DI/Definition/Source/ReflectionDefinitionSource.php +++ b/src/DI/Definition/Source/ReflectionDefinitionSource.php @@ -12,63 +12,66 @@ use DI\Definition\ClassDefinition; use DI\Definition\EntryReference; use DI\Definition\ClassInjection\MethodInjection; +use DI\Definition\MergeableDefinition; use ReflectionClass; use ReflectionMethod; -use ReflectionProperty; /** * Reads DI class definitions using reflection. * * @author Matthieu Napoli */ -class ReflectionDefinitionSource implements DefinitionSource, ClassDefinitionSource +class ReflectionDefinitionSource implements DefinitionSource { /** * {@inheritdoc} */ - public function getDefinition($name) + public function getDefinition($name, MergeableDefinition $parentDefinition = null) { - if (!class_exists($name) && !interface_exists($name)) { + // Only merges with class definition + if ($parentDefinition && (! $parentDefinition instanceof ClassDefinition)) { return null; } - $class = new ReflectionClass($name); - $classDefinition = new ClassDefinition($name); + $className = $parentDefinition ? $parentDefinition->getClassName() : $name; + + if (!class_exists($className) && !interface_exists($className)) { + return null; + } + + $class = new ReflectionClass($className); + $definition = new ClassDefinition($name); // Constructor $constructor = $class->getConstructor(); - if ($constructor && $constructor->isPublic()) { - $classDefinition->setConstructorInjection($this->getMethodInjection($name, $constructor)); + $definition->setConstructorInjection($this->getConstructorInjection($constructor)); } - return $classDefinition; - } + // Merge with parent + if ($parentDefinition) { + $parentDefinition->merge($definition); + $definition = $parentDefinition; + } - /** - * {@inheritdoc} - */ - public function getPropertyInjection($entryName, ReflectionProperty $property) - { - // Nothing to guess on properties - return null; + return $definition; } /** * {@inheritdoc} */ - public function getMethodInjection($entryName, ReflectionMethod $method) + private function getConstructorInjection(ReflectionMethod $constructor) { $parameters = array(); - foreach ($method->getParameters() as $parameter) { + foreach ($constructor->getParameters() as $index => $parameter) { $parameterClass = $parameter->getClass(); if ($parameterClass) { - $parameters[$parameter->getName()] = new EntryReference($parameterClass->getName()); + $parameters[$index] = new EntryReference($parameterClass->getName()); } } - return new MethodInjection($method->getName(), $parameters); + return new MethodInjection($constructor->getName(), $parameters); } } diff --git a/src/DI/Definition/ValueDefinition.php b/src/DI/Definition/ValueDefinition.php index 621474f11..c655ea005 100644 --- a/src/DI/Definition/ValueDefinition.php +++ b/src/DI/Definition/ValueDefinition.php @@ -12,13 +12,12 @@ use DI\Scope; /** - * Definition of a value for dependency injection + * Definition of a value for dependency injection. * * @author Matthieu Napoli */ class ValueDefinition implements Definition { - /** * Entry name * @var string @@ -66,14 +65,6 @@ public function getValue() return $this->value; } - /** - * {@inheritdoc} - */ - public function merge(Definition $definition) - { - throw new \BadMethodCallException("Impossible to merge a ValueDefinition with another definition"); - } - /** * {@inheritdoc} */ @@ -81,13 +72,4 @@ public function isCacheable() { return false; } - - /** - * {@inheritdoc} - */ - public static function isMergeable() - { - return false; - } - } diff --git a/src/DI/DefinitionResolver/ClassDefinitionResolver.php b/src/DI/DefinitionResolver/ClassDefinitionResolver.php index e0921bdd1..2f12f3db1 100644 --- a/src/DI/DefinitionResolver/ClassDefinitionResolver.php +++ b/src/DI/DefinitionResolver/ClassDefinitionResolver.php @@ -216,41 +216,26 @@ private function prepareMethodParameters( MethodInjection $methodInjection = null, ReflectionMethod $methodReflection = null ) { - if (!$methodReflection) { + if (! $methodReflection) { return array(); } - // Check the number of parameters match - $nbRequiredParameters = $methodReflection->getNumberOfRequiredParameters(); - $parameterInjections = $methodInjection ? $methodInjection->getParameters() : array(); - if (count($parameterInjections) < $nbRequiredParameters) { - throw new DefinitionException(sprintf( - "%s::%s takes %d parameters, %d defined or guessed", - $methodReflection->getDeclaringClass()->getName(), - $methodReflection->getName(), - $nbRequiredParameters, - count($parameterInjections) - )); - } - - // No parameters - if (empty($parameterInjections)) { - return array(); - } + $args = array(); - $reflectionParameters = $methodReflection->getParameters(); + foreach ($methodReflection->getParameters() as $index => $parameter) { + $value = $methodInjection ? $methodInjection->getParameter($index) : null; - $args = array(); - foreach ($parameterInjections as $index => $value) { - if ($value instanceof UndefinedInjection) { + // Unknown injection + if ($value === null) { // If the parameter is optional and wasn't specified, we take its default value - if ($reflectionParameters[$index]->isOptional()) { - $args[] = $this->getParameterDefaultValue($reflectionParameters[$index], $methodReflection); + if ($parameter->isOptional()) { + $args[] = $this->getParameterDefaultValue($parameter, $methodReflection); continue; } + throw new DefinitionException(sprintf( "The parameter '%s' of %s::%s has no value defined or guessable", - $reflectionParameters[$index]->getName(), + $parameter->getName(), $methodReflection->getDeclaringClass()->getName(), $methodReflection->getName() )); @@ -270,7 +255,7 @@ private function prepareMethodParameters( * Inject dependencies into properties. * * @param object $object Object to inject dependencies into - * @param \DI\Definition\ClassInjection\PropertyInjection $propertyInjection Property injection definition + * @param PropertyInjection $propertyInjection Property injection definition * * @throws DependencyException * @throws DefinitionException @@ -282,14 +267,6 @@ private function injectProperty($object, PropertyInjection $propertyInjection) $value = $propertyInjection->getValue(); - if ($value instanceof UndefinedInjection) { - throw new DefinitionException(sprintf( - "The property %s::%s has no value defined or guessable", - get_class($object), - $propertyInjection->getPropertyName() - )); - } - if ($value instanceof EntryReference) { try { $value = $this->container->get($value->getName()); @@ -306,7 +283,9 @@ private function injectProperty($object, PropertyInjection $propertyInjection) } } - $property->setAccessible(true); + if (! $property->isPublic()) { + $property->setAccessible(true); + } $property->setValue($object, $value); } diff --git a/tests/IntegrationTests/DI/BuggyInjectionsTest.php b/tests/IntegrationTests/DI/BuggyInjectionsTest.php index 39e029f90..23cb6a891 100644 --- a/tests/IntegrationTests/DI/BuggyInjectionsTest.php +++ b/tests/IntegrationTests/DI/BuggyInjectionsTest.php @@ -13,6 +13,8 @@ /** * Tests buggy cases + * + * @coversNothing */ class BuggyInjectionsTest extends \PHPUnit_Framework_TestCase { diff --git a/tests/IntegrationTests/DI/Fixtures/Class1.php b/tests/IntegrationTests/DI/Fixtures/Class1.php index 60d222821..26a074bda 100644 --- a/tests/IntegrationTests/DI/Fixtures/Class1.php +++ b/tests/IntegrationTests/DI/Fixtures/Class1.php @@ -41,7 +41,7 @@ class Class1 public $property4; /** - * @Inject(lazy=true) + * @Inject * @var LazyDependency */ public $property5; @@ -60,7 +60,6 @@ class Class1 public $method4Param1; /** - * @Inject({"param3" = {"lazy" = true}}) * @param Class2 $param1 * @param Interface1 $param2 * @param LazyDependency $param3 @@ -111,7 +110,7 @@ public function method3($param1, $param2) } /** - * @Inject({"param1" = {"lazy" = true}}) + * @Inject * @param LazyDependency $param1 */ public function method4(LazyDependency $param1) diff --git a/tests/IntegrationTests/DI/Fixtures/PropertyInjectionTest/LazyInjectionClass.php b/tests/IntegrationTests/DI/Fixtures/PropertyInjectionTest/LazyInjectionClass.php index b75af515a..155a485ad 100644 --- a/tests/IntegrationTests/DI/Fixtures/PropertyInjectionTest/LazyInjectionClass.php +++ b/tests/IntegrationTests/DI/Fixtures/PropertyInjectionTest/LazyInjectionClass.php @@ -16,9 +16,8 @@ */ class LazyInjectionClass { - /** - * @Inject(lazy=true) + * @Inject * @var \IntegrationTests\DI\Fixtures\PropertyInjectionTest\Class2 */ private $class2; @@ -42,5 +41,4 @@ public function getDependencyAttribute() } return $this->class2->getBoolean(); } - } diff --git a/tests/IntegrationTests/DI/InheritanceTest.php b/tests/IntegrationTests/DI/InheritanceTest.php index db858f0a3..fad54940e 100644 --- a/tests/IntegrationTests/DI/InheritanceTest.php +++ b/tests/IntegrationTests/DI/InheritanceTest.php @@ -15,6 +15,8 @@ /** * Test class for bean injection + * + * @coversNothing */ class InheritanceTest extends \PHPUnit_Framework_TestCase { @@ -52,7 +54,8 @@ public function testInjectionBaseClass(Container $container) /** - * PHPUnit data provider: generates container configurations for running the same tests for each configuration possible + * PHPUnit data provider: generates container configurations for running the same tests + * for each configuration possible * @return array */ public static function containerProvider() @@ -62,44 +65,38 @@ public static function containerProvider() $builder->useReflection(true); $builder->useAnnotations(true); $containerAnnotations = $builder->build(); - $containerAnnotations->set('IntegrationTests\DI\Fixtures\InheritanceTest\BaseClass', \DI\object('IntegrationTests\DI\Fixtures\InheritanceTest\SubClass')); + $containerAnnotations->set( + 'IntegrationTests\DI\Fixtures\InheritanceTest\BaseClass', + \DI\object('IntegrationTests\DI\Fixtures\InheritanceTest\SubClass') + ); - // Test with a container using array configuration + // Test with a container using PHP configuration -> entries are different, + // definitions shouldn't be shared between 2 different entries se we redefine all properties and methods $builder = new ContainerBuilder(); $builder->useReflection(false); $builder->useAnnotations(false); - $containerFullArrayDefinitions = $builder->build(); - $containerFullArrayDefinitions->addDefinitions(array( - 'IntegrationTests\DI\Fixtures\InheritanceTest\BaseClass' => \DI\object('IntegrationTests\DI\Fixtures\InheritanceTest\SubClass') + $containerPHPDefinitions = $builder->build(); + $containerPHPDefinitions->set('IntegrationTests\DI\Fixtures\InheritanceTest\Dependency', \DI\object()); + $containerPHPDefinitions->set( + 'IntegrationTests\DI\Fixtures\InheritanceTest\BaseClass', + \DI\object('IntegrationTests\DI\Fixtures\InheritanceTest\SubClass') ->withProperty('property1', \DI\link('IntegrationTests\DI\Fixtures\InheritanceTest\Dependency')) ->withProperty('property4', \DI\link('IntegrationTests\DI\Fixtures\InheritanceTest\Dependency')) ->withConstructor(\DI\link('IntegrationTests\DI\Fixtures\InheritanceTest\Dependency')) - ->withMethod('setProperty2', \DI\link('IntegrationTests\DI\Fixtures\InheritanceTest\Dependency')), - 'IntegrationTests\DI\Fixtures\InheritanceTest\SubClass' => \DI\object() + ->withMethod('setProperty2', \DI\link('IntegrationTests\DI\Fixtures\InheritanceTest\Dependency')) + ); + $containerPHPDefinitions->set( + 'IntegrationTests\DI\Fixtures\InheritanceTest\SubClass', + \DI\object() ->withProperty('property1', \DI\link('IntegrationTests\DI\Fixtures\InheritanceTest\Dependency')) ->withProperty('property4', \DI\link('IntegrationTests\DI\Fixtures\InheritanceTest\Dependency')) ->withConstructor(\DI\link('IntegrationTests\DI\Fixtures\InheritanceTest\Dependency')) - ->withMethod('setProperty2', \DI\link('IntegrationTests\DI\Fixtures\InheritanceTest\Dependency')), - )); - - // Test with a container using array configuration - $builder = new ContainerBuilder(); - $builder->useReflection(false); - $builder->useAnnotations(false); - $containerInheritanceDefinitions = $builder->build(); - $containerInheritanceDefinitions->addDefinitions(array( - 'IntegrationTests\DI\Fixtures\InheritanceTest\BaseClass' => \DI\object('IntegrationTests\DI\Fixtures\InheritanceTest\SubClass') - ->withProperty('property1', \DI\link('IntegrationTests\DI\Fixtures\InheritanceTest\Dependency')) - ->withConstructor(\DI\link('IntegrationTests\DI\Fixtures\InheritanceTest\Dependency')) - ->withMethod('setProperty2', \DI\link('IntegrationTests\DI\Fixtures\InheritanceTest\Dependency')), - 'IntegrationTests\DI\Fixtures\InheritanceTest\SubClass' => \DI\object() - ->withProperty('property4', \DI\link('IntegrationTests\DI\Fixtures\InheritanceTest\Dependency')), - )); + ->withMethod('setProperty2', \DI\link('IntegrationTests\DI\Fixtures\InheritanceTest\Dependency')) + ); return array( - array($containerAnnotations), - array($containerFullArrayDefinitions), - array($containerInheritanceDefinitions), + 'annotation' => array($containerAnnotations), + 'php' => array($containerPHPDefinitions), ); } } diff --git a/tests/IntegrationTests/DI/InjectionTest.php b/tests/IntegrationTests/DI/InjectionTest.php index eb6613e4c..dbdde5456 100644 --- a/tests/IntegrationTests/DI/InjectionTest.php +++ b/tests/IntegrationTests/DI/InjectionTest.php @@ -20,6 +20,8 @@ /** * Test class for injection + * + * @coversNothing Because integration test */ class InjectionTest extends \PHPUnit_Framework_TestCase { @@ -40,25 +42,29 @@ public static function containerProvider() $builder->useReflection(true); $builder->useAnnotations(false); $containerReflection = $builder->build(); - $containerReflection->addDefinitions(array( - 'foo' => 'bar', - 'IntegrationTests\DI\Fixtures\Interface1' => \DI\object('IntegrationTests\DI\Fixtures\Implementation1'), - 'namedDependency' => \DI\object('IntegrationTests\DI\Fixtures\Class2'), - 'IntegrationTests\DI\Fixtures\LazyDependency' => \DI\object()->lazy(), - 'alias' => \DI\link('namedDependency'), - )); + // We have to define some entries for the test because reflection on itself doesn't make it possible + $containerReflection->set('foo', 'bar'); + $containerReflection->set( + 'IntegrationTests\DI\Fixtures\Interface1', + \DI\object('IntegrationTests\DI\Fixtures\Implementation1') + ); + $containerReflection->set('namedDependency', \DI\object('IntegrationTests\DI\Fixtures\Class2')); + $containerReflection->set('IntegrationTests\DI\Fixtures\LazyDependency', \DI\object()->lazy()); + $containerReflection->set('alias', \DI\link('namedDependency')); // Test with a container using annotations and reflection $builder = new ContainerBuilder(); $builder->useReflection(true); $builder->useAnnotations(true); $containerAnnotations = $builder->build(); - $containerAnnotations->addDefinitions(array( - 'foo' => 'bar', - 'IntegrationTests\DI\Fixtures\Interface1' => \DI\object('IntegrationTests\DI\Fixtures\Implementation1'), - 'namedDependency' => \DI\object('IntegrationTests\DI\Fixtures\Class2'), - 'alias' => \DI\link('namedDependency'), - )); + // We have to define some entries for the test because annotations on itself doesn't make it possible + $containerAnnotations->set('foo', 'bar'); + $containerAnnotations->set( + 'IntegrationTests\DI\Fixtures\Interface1', + \DI\object('IntegrationTests\DI\Fixtures\Implementation1') + ); + $containerAnnotations->set('namedDependency', \DI\object('IntegrationTests\DI\Fixtures\Class2')); + $containerAnnotations->set('alias', \DI\link('namedDependency')); // Test with a container using array configuration $builder = new ContainerBuilder(); @@ -104,10 +110,10 @@ public static function containerProvider() $containerPHP->set('alias', \DI\link('namedDependency')); return array( - array(self::DEFINITION_REFLECTION, $containerReflection), - array(self::DEFINITION_ANNOTATIONS, $containerAnnotations), - array(self::DEFINITION_ARRAY, $containerArray), - array(self::DEFINITION_PHP, $containerPHP), + 'reflection' => array(self::DEFINITION_REFLECTION, $containerReflection), + 'annotations' => array(self::DEFINITION_ANNOTATIONS, $containerAnnotations), + 'array' => array(self::DEFINITION_ARRAY, $containerArray), + 'php' => array(self::DEFINITION_PHP, $containerPHP), ); } diff --git a/tests/IntegrationTests/DI/Issues/Issue70and76Test.php b/tests/IntegrationTests/DI/Issues/Issue70and76Test.php index 133e07287..25a317255 100644 --- a/tests/IntegrationTests/DI/Issues/Issue70and76Test.php +++ b/tests/IntegrationTests/DI/Issues/Issue70and76Test.php @@ -14,6 +14,8 @@ /** * @see https://github.com/mnapoli/PHP-DI/issues/70 * @see https://github.com/mnapoli/PHP-DI/issues/76 + * + * @coversNothing */ class Issue70and76Test extends \PHPUnit_Framework_TestCase { diff --git a/tests/IntegrationTests/DI/Issues/Issue72/definitions.php b/tests/IntegrationTests/DI/Issues/Issue72/definitions.php new file mode 100644 index 000000000..0004396ac --- /dev/null +++ b/tests/IntegrationTests/DI/Issues/Issue72/definitions.php @@ -0,0 +1,11 @@ + \DI\factory(function () { + $value = new \stdClass(); + $value->foo = 'bar'; + return $value; + }), + 'IntegrationTests\DI\Issues\Issue72\Class1' => \DI\object() + ->withConstructor(\DI\link('service2')), +); diff --git a/tests/IntegrationTests/DI/Issues/Issue72Test.php b/tests/IntegrationTests/DI/Issues/Issue72Test.php index e7dd03544..acdc0c0f0 100644 --- a/tests/IntegrationTests/DI/Issues/Issue72Test.php +++ b/tests/IntegrationTests/DI/Issues/Issue72Test.php @@ -10,12 +10,15 @@ namespace IntegrationTests\DI\Issues; use DI\ContainerBuilder; +use DI\Definition\Source\ArrayDefinitionSource; use IntegrationTests\DI\Issues\Issue72\Class1; /** * Test that the manager prioritize correctly the different sources * * @see https://github.com/mnapoli/PHP-DI/issues/72 + * + * @coversNothing */ class Issue72Test extends \PHPUnit_Framework_TestCase { @@ -39,6 +42,26 @@ public function annotationDefinitionShouldOverrideReflectionDefinition() $this->assertEquals('bar', $class1->arg1->foo); } + /** + * @test + */ + public function arrayDefinitionShouldOverrideReflectionDefinition() + { + $builder = new ContainerBuilder(); + $builder->useReflection(true); + $builder->useAnnotations(false); + + // Override to 'service2' in the definition file + $builder->addDefinitions(new ArrayDefinitionSource(__DIR__ . '/Issue72/definitions.php')); + + $container = $builder->build(); + + /** @var Class1 $class1 */ + $class1 = $container->get('IntegrationTests\DI\Issues\Issue72\Class1'); + + $this->assertEquals('bar', $class1->arg1->foo); + } + /** * @test */ @@ -47,18 +70,11 @@ public function arrayDefinitionShouldOverrideAnnotationDefinition() $builder = new ContainerBuilder(); $builder->useReflection(false); $builder->useAnnotations(true); - $container = $builder->build(); - // Override 'service1' to 'service2' - $container->addDefinitions(array( - 'service2' => \DI\factory(function () { - $value = new \stdClass(); - $value->foo = 'bar'; - return $value; - }), - 'IntegrationTests\DI\Issues\Issue72\Class1' => \DI\object() - ->withConstructor(\DI\link('service2')), - )); + // Override 'service1' to 'service2' in the definition file + $builder->addDefinitions(new ArrayDefinitionSource(__DIR__ . '/Issue72/definitions.php')); + + $container = $builder->build(); /** @var Class1 $class1 */ $class1 = $container->get('IntegrationTests\DI\Issues\Issue72\Class1'); @@ -69,22 +85,14 @@ public function arrayDefinitionShouldOverrideAnnotationDefinition() /** * @test */ - public function simpleDefinitionShouldOverrideArrayDefinition() + public function phpDefinitionShouldOverrideArrayDefinition() { $builder = new ContainerBuilder(); $builder->useReflection(false); $builder->useAnnotations(false); + $builder->addDefinitions(new ArrayDefinitionSource(__DIR__ . '/Issue72/definitions.php')); $container = $builder->build(); - $container->addDefinitions(array( - 'service2' => \DI\factory(function () { - $value = new \stdClass(); - $value->foo = 'bar'; - return $value; - }), - 'IntegrationTests\DI\Issues\Issue72\Class1' => \DI\object() - ->withConstructor(\DI\link('service1')), - )); // Override 'service1' to 'service2' $container->set( 'IntegrationTests\DI\Issues\Issue72\Class1', diff --git a/tests/IntegrationTests/DI/PropertyInjectionTest.php b/tests/IntegrationTests/DI/PropertyInjectionTest.php index 5f31f9c40..ebcf3743a 100644 --- a/tests/IntegrationTests/DI/PropertyInjectionTest.php +++ b/tests/IntegrationTests/DI/PropertyInjectionTest.php @@ -16,6 +16,8 @@ /** * Test class for bean injection + * + * @coversNothing */ class PropertyInjectionTest extends \PHPUnit_Framework_TestCase { diff --git a/tests/UnitTests/DI/Definition/AliasDefinitionTest.php b/tests/UnitTests/DI/Definition/AliasDefinitionTest.php index 0edeaf8b4..f4c12b8f1 100644 --- a/tests/UnitTests/DI/Definition/AliasDefinitionTest.php +++ b/tests/UnitTests/DI/Definition/AliasDefinitionTest.php @@ -37,19 +37,4 @@ public function testCacheable() $definition = new AliasDefinition('foo', 'bar'); $this->assertTrue($definition->isCacheable()); } - - public function testMergeable() - { - $this->assertFalse(AliasDefinition::isMergeable()); - } - - /** - * @expectedException \BadMethodCallException - */ - public function testMerge() - { - $definition1 = new AliasDefinition('foo', 'bar'); - $definition2 = new AliasDefinition('foo', 'baz'); - $definition1->merge($definition2); - } } diff --git a/tests/UnitTests/DI/Definition/CallableDefinitionTest.php b/tests/UnitTests/DI/Definition/CallableDefinitionTest.php index 06c9d456b..5798e67d3 100644 --- a/tests/UnitTests/DI/Definition/CallableDefinitionTest.php +++ b/tests/UnitTests/DI/Definition/CallableDefinitionTest.php @@ -48,23 +48,4 @@ public function testScope() $this->assertEquals(Scope::PROTOTYPE(), $definition->getScope()); } - - public function testMergeable() - { - $this->assertFalse(CallableDefinition::isMergeable()); - } - - /** - * @expectedException \BadMethodCallException - */ - public function testMerge() - { - $definition1 = new CallableDefinition('foo', function () { - return 1; - }); - $definition2 = new CallableDefinition('foo', function () { - return 2; - }); - $definition1->merge($definition2); - } } diff --git a/tests/UnitTests/DI/Definition/ClassDefinitionTest.php b/tests/UnitTests/DI/Definition/ClassDefinitionTest.php index 099beae12..302c32775 100644 --- a/tests/UnitTests/DI/Definition/ClassDefinitionTest.php +++ b/tests/UnitTests/DI/Definition/ClassDefinitionTest.php @@ -45,19 +45,16 @@ public function testDefaultValues() $this->assertEmpty($definition->getMethodInjections()); } - public function testMergeable() - { - $this->assertTrue(ClassDefinition::isMergeable()); - } - /** * @expectedException \DI\Definition\Exception\DefinitionException * @expectedExceptionMessage DI definition conflict: there are 2 different definitions for 'foo' that are incompatible, they are not of the same type */ public function testMergeIncompatibleTypes() { + $otherDefinition = $this->getMockForAbstractClass('DI\Definition\MergeableDefinition'); + $definition = new ClassDefinition('foo', 'bar'); - $definition->merge(new ValueDefinition('foo', 1)); + $definition->merge($otherDefinition); } /** diff --git a/tests/UnitTests/DI/Definition/Source/AnnotationDefinitionSourceTest.php b/tests/UnitTests/DI/Definition/Source/AnnotationDefinitionSourceTest.php index d9ea3201a..604e4431f 100644 --- a/tests/UnitTests/DI/Definition/Source/AnnotationDefinitionSourceTest.php +++ b/tests/UnitTests/DI/Definition/Source/AnnotationDefinitionSourceTest.php @@ -57,8 +57,8 @@ public function testConstructor() $parameters = $constructorInjection->getParameters(); $this->assertCount(2, $parameters); - $this->assertEquals(new EntryReference('foo'), $parameters['param1']); - $this->assertEquals(new EntryReference('bar'), $parameters['param2']); + $this->assertEquals(new EntryReference('foo'), $parameters[0]); + $this->assertEquals(new EntryReference('bar'), $parameters[1]); } /** @@ -92,8 +92,8 @@ public function testMethod2() $parameters = $methodInjection->getParameters(); $this->assertCount(2, $parameters); - $this->assertEquals(new EntryReference('foo'), $parameters['param1']); - $this->assertEquals(new EntryReference('bar'), $parameters['param2']); + $this->assertEquals(new EntryReference('foo'), $parameters[0]); + $this->assertEquals(new EntryReference('bar'), $parameters[1]); } /** @@ -113,8 +113,8 @@ public function testMethod3() $this->assertCount(2, $parameters); $reference = new EntryReference('UnitTests\DI\Definition\Source\Fixtures\AnnotationFixture2'); - $this->assertEquals($reference, $parameters['param1']); - $this->assertEquals($reference, $parameters['param2']); + $this->assertEquals($reference, $parameters[0]); + $this->assertEquals($reference, $parameters[1]); } /** @@ -132,8 +132,8 @@ public function testMethod4() $parameters = $methodInjection->getParameters(); $this->assertCount(2, $parameters); - $this->assertEquals(new EntryReference('foo'), $parameters['param1']); - $this->assertEquals(new EntryReference('bar'), $parameters['param2']); + $this->assertEquals(new EntryReference('foo'), $parameters[0]); + $this->assertEquals(new EntryReference('bar'), $parameters[1]); } /** @@ -152,6 +152,7 @@ public function testMethod5() $parameters = $methodInjection->getParameters(); $this->assertCount(1, $parameters); - $this->assertEquals(new EntryReference('bar'), $parameters['param2']); + // Offset is 1, not 0, because parameter 0 wasn't defined + $this->assertEquals(new EntryReference('bar'), $parameters[1]); } } diff --git a/tests/UnitTests/DI/Definition/Source/ReflectionDefinitionSourceTest.php b/tests/UnitTests/DI/Definition/Source/ReflectionDefinitionSourceTest.php index c6e5df643..0a25a62a8 100644 --- a/tests/UnitTests/DI/Definition/Source/ReflectionDefinitionSourceTest.php +++ b/tests/UnitTests/DI/Definition/Source/ReflectionDefinitionSourceTest.php @@ -12,21 +12,17 @@ use DI\Definition\EntryReference; use DI\Definition\Source\ReflectionDefinitionSource; +/** + * @covers \DI\Definition\Source\ReflectionDefinitionSource + */ class ReflectionDefinitionSourceTest extends \PHPUnit_Framework_TestCase { - /** - * @covers \DI\Definition\Source\ReflectionDefinitionSource::getDefinition - */ public function testUnknownClass() { $source = new ReflectionDefinitionSource(); $this->assertNull($source->getDefinition('foo')); } - /** - * @covers \DI\Definition\Source\ReflectionDefinitionSource::getDefinition - * @covers \DI\Definition\Source\ReflectionDefinitionSource::getMethodInjection - */ public function testConstructor() { $source = new ReflectionDefinitionSource(); @@ -39,13 +35,10 @@ public function testConstructor() $parameters = $constructorInjection->getParameters(); $this->assertCount(1, $parameters); - $param1 = $parameters['param1']; + $param1 = $parameters[0]; $this->assertEquals(new EntryReference('UnitTests\DI\Definition\Source\Fixtures\ReflectionFixture'), $param1); } - /** - * @covers \DI\Definition\Source\ReflectionDefinitionSource::getDefinition - */ public function testConstructorInParentClass() { $source = new ReflectionDefinitionSource(); @@ -58,18 +51,7 @@ public function testConstructorInParentClass() $parameters = $constructorInjection->getParameters(); $this->assertCount(1, $parameters); - $param1 = $parameters['param1']; + $param1 = $parameters[0]; $this->assertEquals(new EntryReference('UnitTests\DI\Definition\Source\Fixtures\ReflectionFixture'), $param1); } - - /** - * @covers \DI\Definition\Source\ReflectionDefinitionSource::getPropertyInjection - */ - public function testGetPropertyInjection() - { - $property = $this->getMock('\ReflectionProperty', null, array(), '', false); - - $source = new ReflectionDefinitionSource(); - $this->assertNull($source->getPropertyInjection('foo', $property)); - } } diff --git a/tests/UnitTests/DI/Definition/ValueDefinitionTest.php b/tests/UnitTests/DI/Definition/ValueDefinitionTest.php index 5a60414da..309b5f92e 100644 --- a/tests/UnitTests/DI/Definition/ValueDefinitionTest.php +++ b/tests/UnitTests/DI/Definition/ValueDefinitionTest.php @@ -32,25 +32,10 @@ public function testCacheable() $this->assertFalse($definition->isCacheable()); } - public function testMergeable() - { - $this->assertFalse(ValueDefinition::isMergeable()); - } - public function testScope() { $definition = new ValueDefinition('foo', 1); $this->assertEquals(Scope::SINGLETON(), $definition->getScope()); } - - /** - * @expectedException \BadMethodCallException - */ - public function testMerge() - { - $definition1 = new ValueDefinition('foo', 1); - $definition2 = new ValueDefinition('foo', 2); - $definition1->merge($definition2); - } } From 8bd26fa6d33ea4e7df260f9459f2fe05ae0cb09e Mon Sep 17 00:00:00 2001 From: Matthieu Napoli Date: Thu, 12 Dec 2013 13:52:55 +0100 Subject: [PATCH 10/10] Split file handling of ArrayDefinitionSource to new PHPFileDefinitionSource --- src/DI/ContainerBuilder.php | 30 ++++---- .../Source/ArrayDefinitionSource.php | 56 +------------- .../Source/PHPFileDefinitionSource.php | 76 +++++++++++++++++++ tests/IntegrationTests/DI/InjectionTest.php | 2 +- .../DI/Issues/Issue72Test.php | 6 +- .../Source/ArrayDefinitionSourceTest.php | 19 ----- .../Source/PHPFileDefinitionSourceTest.php | 38 ++++++++++ 7 files changed, 133 insertions(+), 94 deletions(-) create mode 100644 src/DI/Definition/Source/PHPFileDefinitionSource.php create mode 100644 tests/UnitTests/DI/Definition/Source/PHPFileDefinitionSourceTest.php diff --git a/src/DI/ContainerBuilder.php b/src/DI/ContainerBuilder.php index d9bcb15c2..9b7a96d10 100644 --- a/src/DI/ContainerBuilder.php +++ b/src/DI/ContainerBuilder.php @@ -12,6 +12,7 @@ use DI\Definition\DefinitionManager; use DI\Definition\Source\AnnotationDefinitionSource; use DI\Definition\Source\ChainableDefinitionSource; +use DI\Definition\Source\PHPFileDefinitionSource; use DI\Definition\Source\ReflectionDefinitionSource; use Doctrine\Common\Cache\Cache; use InvalidArgumentException; @@ -69,10 +70,10 @@ class ContainerBuilder private $wrapperContainer; /** - * Source of definitions for the container. - * @var ChainableDefinitionSource[] + * Files of definitions for the container. + * @var string[] */ - private $definitionSources = array(); + private $files = array(); /** * Build a container configured for the dev environment. @@ -100,11 +101,13 @@ public function build() { // Definition sources $source = null; - foreach ($this->definitionSources as $definitionSource) { - if ($source instanceof ChainableDefinitionSource) { - $definitionSource->chain($source); + foreach ($this->files as $file) { + $newSource = new PHPFileDefinitionSource($file); + // Chain file sources + if ($source) { + $newSource->chain($source); } - $source = $definitionSource; + $source = $newSource; } if ($this->useAnnotations) { if ($source) { @@ -213,18 +216,13 @@ public function wrapContainer(ContainerInterface $otherContainer) } /** - * Add definitions to the container by adding a source of definitions. - * - * Do not add ReflectionDefinitionSource or AnnotationDefinitionSource manually, they should be - * handled with useReflection() and useAnnotations(). - * - * @param ChainableDefinitionSource $definitionSource + * Add a file containing definitions to the container. * - * @todo Give file directly + * @param string $file */ - public function addDefinitions(ChainableDefinitionSource $definitionSource) + public function addDefinitions($file) { - $this->definitionSources[] = $definitionSource; + $this->files[] = $file; } /** diff --git a/src/DI/Definition/Source/ArrayDefinitionSource.php b/src/DI/Definition/Source/ArrayDefinitionSource.php index bfb04b15d..266be0485 100644 --- a/src/DI/Definition/Source/ArrayDefinitionSource.php +++ b/src/DI/Definition/Source/ArrayDefinitionSource.php @@ -10,13 +10,12 @@ namespace DI\Definition\Source; use DI\Definition\Definition; -use DI\Definition\Exception\DefinitionException; use DI\Definition\MergeableDefinition; use DI\Definition\ValueDefinition; use DI\DefinitionHelper\DefinitionHelper; /** - * Reads DI definitions from a PHP array, or a file returning a PHP array. + * Reads DI definitions from a PHP array. * * @author Matthieu Napoli */ @@ -27,45 +26,17 @@ class ArrayDefinitionSource implements ChainableDefinitionSource */ private $chainedSource; - /** - * @var bool - */ - private $initialized; - - /** - * File containing definitions, or null if the definitions are given as a PHP array. - * @var string|null - */ - private $file; - /** * DI definitions in a PHP array * @var array */ private $definitions = array(); - /** - * @param string|null $file File in which the definitions are returned as an array. - */ - public function __construct($file = null) - { - if (! $file) { - $this->initialized = true; - return; - } - - // If we are given a file containing an array, we lazy-load it to improve performance - $this->initialized = false; - $this->file = $file; - } - /** * {@inheritdoc} */ public function getDefinition($name, MergeableDefinition $parentDefinition = null) { - $this->initialize(); - if (! array_key_exists($name, $this->definitions)) { // Not found, we use the chain or return null if ($this->chainedSource) { @@ -121,31 +92,6 @@ public function addDefinition(Definition $definition) $this->definitions[$definition->getName()] = $definition; } - /** - * Lazy-loading of the definitions. - * @throws DefinitionException - */ - private function initialize() - { - if ($this->initialized === true) { - return; - } - - if (! is_readable($this->file)) { - throw new DefinitionException("File {$this->file} doesn't exist or is not readable"); - } - - $definitions = require $this->file; - - if (! is_array($definitions)) { - throw new DefinitionException("File {$this->file} should return an array of definitions"); - } - - $this->addDefinitions($definitions); - - $this->initialized = true; - } - /** * {@inheritdoc} */ diff --git a/src/DI/Definition/Source/PHPFileDefinitionSource.php b/src/DI/Definition/Source/PHPFileDefinitionSource.php new file mode 100644 index 000000000..280e7d573 --- /dev/null +++ b/src/DI/Definition/Source/PHPFileDefinitionSource.php @@ -0,0 +1,76 @@ + + */ +class PHPFileDefinitionSource extends ArrayDefinitionSource +{ + /** + * @var bool + */ + private $initialized = false; + + /** + * File containing definitions, or null if the definitions are given as a PHP array. + * @var string|null + */ + private $file; + + /** + * @param string $file File in which the definitions are returned as an array. + */ + public function __construct($file) + { + // Lazy-loading to improve performances + $this->file = $file; + } + + /** + * {@inheritdoc} + */ + public function getDefinition($name, MergeableDefinition $parentDefinition = null) + { + $this->initialize(); + + return parent::getDefinition($name, $parentDefinition); + } + + /** + * Lazy-loading of the definitions. + * @throws DefinitionException + */ + private function initialize() + { + if ($this->initialized === true) { + return; + } + + if (! is_readable($this->file)) { + throw new DefinitionException("File {$this->file} doesn't exist or is not readable"); + } + + $definitions = require $this->file; + + if (! is_array($definitions)) { + throw new DefinitionException("File {$this->file} should return an array of definitions"); + } + + $this->addDefinitions($definitions); + + $this->initialized = true; + } +} diff --git a/tests/IntegrationTests/DI/InjectionTest.php b/tests/IntegrationTests/DI/InjectionTest.php index dbdde5456..4db154c52 100644 --- a/tests/IntegrationTests/DI/InjectionTest.php +++ b/tests/IntegrationTests/DI/InjectionTest.php @@ -70,7 +70,7 @@ public static function containerProvider() $builder = new ContainerBuilder(); $builder->useReflection(false); $builder->useAnnotations(false); - $builder->addDefinitions(new ArrayDefinitionSource(__DIR__ . '/Fixtures/definitions.php')); + $builder->addDefinitions(__DIR__ . '/Fixtures/definitions.php'); $containerArray = $builder->build(); // Test with a container using PHP configuration diff --git a/tests/IntegrationTests/DI/Issues/Issue72Test.php b/tests/IntegrationTests/DI/Issues/Issue72Test.php index acdc0c0f0..6a64274cd 100644 --- a/tests/IntegrationTests/DI/Issues/Issue72Test.php +++ b/tests/IntegrationTests/DI/Issues/Issue72Test.php @@ -52,7 +52,7 @@ public function arrayDefinitionShouldOverrideReflectionDefinition() $builder->useAnnotations(false); // Override to 'service2' in the definition file - $builder->addDefinitions(new ArrayDefinitionSource(__DIR__ . '/Issue72/definitions.php')); + $builder->addDefinitions(__DIR__ . '/Issue72/definitions.php'); $container = $builder->build(); @@ -72,7 +72,7 @@ public function arrayDefinitionShouldOverrideAnnotationDefinition() $builder->useAnnotations(true); // Override 'service1' to 'service2' in the definition file - $builder->addDefinitions(new ArrayDefinitionSource(__DIR__ . '/Issue72/definitions.php')); + $builder->addDefinitions(__DIR__ . '/Issue72/definitions.php'); $container = $builder->build(); @@ -90,7 +90,7 @@ public function phpDefinitionShouldOverrideArrayDefinition() $builder = new ContainerBuilder(); $builder->useReflection(false); $builder->useAnnotations(false); - $builder->addDefinitions(new ArrayDefinitionSource(__DIR__ . '/Issue72/definitions.php')); + $builder->addDefinitions(__DIR__ . '/Issue72/definitions.php'); $container = $builder->build(); // Override 'service1' to 'service2' diff --git a/tests/UnitTests/DI/Definition/Source/ArrayDefinitionSourceTest.php b/tests/UnitTests/DI/Definition/Source/ArrayDefinitionSourceTest.php index 6b7503a1f..d50c53330 100644 --- a/tests/UnitTests/DI/Definition/Source/ArrayDefinitionSourceTest.php +++ b/tests/UnitTests/DI/Definition/Source/ArrayDefinitionSourceTest.php @@ -117,25 +117,6 @@ public function testClosureDefinition() $this->assertEquals($callable, $definition->getCallable()); } - /** - * @covers \DI\Definition\Source\ArrayDefinitionSource - */ - public function testLoadFromFile() - { - $source = new ArrayDefinitionSource(__DIR__ . '/Fixtures/definitions.php'); - - $definition = $source->getDefinition('foo'); - $this->assertNotNull($definition); - $this->assertEquals('bar', $definition->getValue()); - $this->assertInternalType('string', $definition->getValue()); - - /** @var $definition ClassDefinition */ - $definition = $source->getDefinition('bim'); - $this->assertInstanceOf('DI\Definition\ClassDefinition', $definition); - $this->assertEquals('bim', $definition->getName()); - $this->assertEquals('bim', $definition->getClassName()); - } - /** * @covers \DI\Definition\Source\ArrayDefinitionSource::getDefinition * @covers \DI\Definition\Source\ArrayDefinitionSource::chain diff --git a/tests/UnitTests/DI/Definition/Source/PHPFileDefinitionSourceTest.php b/tests/UnitTests/DI/Definition/Source/PHPFileDefinitionSourceTest.php new file mode 100644 index 000000000..4b7e2d0b3 --- /dev/null +++ b/tests/UnitTests/DI/Definition/Source/PHPFileDefinitionSourceTest.php @@ -0,0 +1,38 @@ +getDefinition('foo'); + $this->assertNotNull($definition); + $this->assertEquals('bar', $definition->getValue()); + $this->assertInternalType('string', $definition->getValue()); + + /** @var $definition ClassDefinition */ + $definition = $source->getDefinition('bim'); + $this->assertInstanceOf('DI\Definition\ClassDefinition', $definition); + $this->assertEquals('bim', $definition->getName()); + $this->assertEquals('bim', $definition->getClassName()); + } +}