diff --git a/change-log.md b/change-log.md index 87e7d8059..7dfdc973b 100644 --- a/change-log.md +++ b/change-log.md @@ -1,5 +1,10 @@ # Change log +## 3.4 + +* You can now define arrays of values (in YAML, PHP, …) [#106](https://github.com/mnapoli/PHP-DI/pull/106/) +* FIXED [#100](https://github.com/mnapoli/PHP-DI/issues/100): bug for lazy injection in constructors + ## 3.3 Read the [news entry](news/03-php-di-3-3.md). diff --git a/composer.json b/composer.json index 55c883770..f21123dec 100644 --- a/composer.json +++ b/composer.json @@ -18,6 +18,6 @@ "doctrine/cache": "1.*", "myclabs/php-enum": "1.*", "symfony/yaml": "2.*", - "ocramius/proxy-manager": "0.3.*" + "ocramius/proxy-manager": "~0.3" } } diff --git a/doc/definition.md b/doc/definition.md index 8653aa077..5feb6dd86 100644 --- a/doc/definition.md +++ b/doc/definition.md @@ -166,6 +166,12 @@ $container = new Container(); $container->set('db.host', 'localhost'); $container->set('db.port', 5000); +// Indexed non-empty array as value +$container->set('report.recipients', array( + 'bob@acme.example.com', + 'alice@acme.example.com' +)); + // Direct mapping (not needed if you didn't disable Reflection) $container->set('SomeClass'); @@ -230,8 +236,14 @@ return [ 'db.host' => 'localhost', 'db.port' => 5000, + // Indexed non-empty array as value + 'report.recipients' => [ + 'bob@acme.example.com', + 'alice@acme.example.com' + ], + // Direct mapping (not needed if you didn't disable Reflection) - 'SomeClass' => array(), + 'SomeClass' => [], // This is not recommended: will instantiate the class even when not used, prevents caching 'SomeOtherClass' => new SomeOtherClass(1, "hello"), @@ -298,6 +310,11 @@ Example of a `config/di.yml` file: db.host: localhost db.port: 5000 +# Indexed non-empty array as value +report.recipients: + - bob@acme.example.com + - alice@acme.example.com + # Direct mapping (not needed if you didn't disable Reflection) SomeClass: @@ -330,3 +347,4 @@ My\Interface: myNamedInstance: class: My\Class ``` + diff --git a/doc/getting-started.md b/doc/getting-started.md index 050c24024..7a7b39ca3 100644 --- a/doc/getting-started.md +++ b/doc/getting-started.md @@ -12,7 +12,7 @@ Create a file named `composer.json` in your project root: ```json { "require": { - "mnapoli/php-di": "3.3.*" + "mnapoli/php-di": "~3.4" } } ``` diff --git a/doc/performances.md b/doc/performances.md index 010a307cd..e95902022 100644 --- a/doc/performances.md +++ b/doc/performances.md @@ -12,11 +12,13 @@ PHP-DI offers an easy and complete solution to put those data into a cache based ### Setup ```php -$container->setDefinitionCache(new Doctrine\Common\Cache\ArrayCache()); +$container->setDefinitionCache(new Doctrine\Common\Cache\ApcCache()); ``` -Heads up: It is recommended not to use a cache in a development environment, else changes you make to the definitions (annotations, configuration files, etc.) may not be taken into account. - +Heads up: do not use a cache in a development environment, else changes you make to the definitions +(annotations, configuration files, etc.) may not be taken into account. +The only cache you should use in development is the `ArrayCache` because it doesn't persist data between requests. +Of course, do not use this one in production. ### Cache types @@ -35,3 +37,16 @@ The cache implementation is provided by Doctrine (because it works very well) an Read the [Doctrine documentation](http://docs.doctrine-project.org/projects/doctrine-common/en/latest/reference/caching.html) for more details. + +### Cache prefixes + +If you run the same application twice on the same machine, both installs will use the same cache, and there might be conflicts. + +To avoid this situation, you should use a cache "prefix": each installation of your app has a unique ID, and this ID is used to prefix cache keys +to avoid collisions. + +```php +$cache = new Doctrine\Common\Cache\ApcCache(); +$cache->setNamespace('MyApplication'); +$container->setDefinitionCache($cache); +``` diff --git a/src/DI/ContainerBuilder.php b/src/DI/ContainerBuilder.php index 0d63b5047..d21c2f3e0 100644 --- a/src/DI/ContainerBuilder.php +++ b/src/DI/ContainerBuilder.php @@ -18,7 +18,7 @@ use ProxyManager\GeneratorStrategy\EvaluatingGeneratorStrategy; /** - * Helper to create a Container + * Fluent helper to create a Container * * @since 3.2 * @author Matthieu Napoli @@ -102,21 +102,25 @@ public function build() * * By default, enabled * @param boolean $bool + * @return ContainerBuilder */ public function useReflection($bool) { $this->useReflection = $bool; + return $this; } /** * Enable or disable the use of annotations * * By default, enabled - * @param boolean $bool + * @param $bool + * @return ContainerBuilder */ public function useAnnotations($bool) { $this->useAnnotations = $bool; + return $this; } /** @@ -124,30 +128,36 @@ public function useAnnotations($bool) * * By default, disabled * @param bool $bool + * @return ContainerBuilder */ public function setDefinitionsValidation($bool) { $this->definitionsValidation = $bool; + return $this; } /** * Enables the use of a cache for the definitions * * @param Cache $cache Cache backend to use + * @return ContainerBuilder */ public function setDefinitionCache(Cache $cache) { $this->cache = $cache; + return $this; } /** * Add definitions contained in a file * * @param DefinitionFileLoader $definitionFileLoader + * @return ContainerBuilder */ public function addDefinitionsFromFile(DefinitionFileLoader $definitionFileLoader) { $this->fileLoaders[] = $definitionFileLoader; + return $this; } /** @@ -158,6 +168,9 @@ public function addDefinitionsFromFile(DefinitionFileLoader $definitionFileLoade * * @param boolean $writeToFile If true, write the proxies to disk to improve performances * @param string|null $proxyDirectory Directory where to write the proxies + * @return ContainerBuilder + * + * @throws InvalidArgumentException when writeToFile is set to true and the proxy directory is null */ public function writeProxiesToFile($writeToFile, $proxyDirectory = null) { @@ -167,6 +180,8 @@ public function writeProxiesToFile($writeToFile, $proxyDirectory = null) throw new InvalidArgumentException("The proxy directory must be specified if you want to write proxies on disk"); } $this->proxyDirectory = $proxyDirectory; + + return $this; } } diff --git a/src/DI/Definition/FileLoader/DefinitionFileLoader.php b/src/DI/Definition/FileLoader/DefinitionFileLoader.php index c29e233ec..d8b545ee2 100644 --- a/src/DI/Definition/FileLoader/DefinitionFileLoader.php +++ b/src/DI/Definition/FileLoader/DefinitionFileLoader.php @@ -33,11 +33,12 @@ abstract class DefinitionFileLoader */ public function __construct($fileName) { - if (!file_exists($fileName)) { + if (!is_file($fileName)) { throw new FileNotFoundException("The definition file '$fileName' has not been found"); } elseif (!is_readable($fileName)) { throw new ParseException("The definition file '$fileName' is not readable"); } + $this->definitionFile = $fileName; } diff --git a/src/DI/Definition/FileLoader/XmlDefinitionFileLoader.php b/src/DI/Definition/FileLoader/XmlDefinitionFileLoader.php index 3aaa29d46..42d01f79d 100644 --- a/src/DI/Definition/FileLoader/XmlDefinitionFileLoader.php +++ b/src/DI/Definition/FileLoader/XmlDefinitionFileLoader.php @@ -15,6 +15,8 @@ * XmlDefinitionFileLoader loads XML files definitions. * * @author Domenic Muskulus + * + * @deprecated XML definitions are not maintained and documented, will be removed */ class XmlDefinitionFileLoader extends DefinitionFileLoader { diff --git a/src/DI/Definition/Helper/ClassDefinitionHelper.php b/src/DI/Definition/Helper/ClassDefinitionHelper.php index 7e9ecebc6..5760d77f3 100644 --- a/src/DI/Definition/Helper/ClassDefinitionHelper.php +++ b/src/DI/Definition/Helper/ClassDefinitionHelper.php @@ -78,11 +78,7 @@ public function withProperty($propertyName, $entryToInject, $lazy = false) */ public function withConstructor(array $params) { - $paramInjections = array(); - foreach ($params as $key => $param) { - $paramInjections[] = new ParameterInjection($key, $param); - } - $this->classDefinition->setConstructorInjection(new MethodInjection('__construct', $paramInjections)); + $this->classDefinition->setConstructorInjection($this->createMethodInjection('__construct', $params)); return $this; } @@ -92,6 +88,24 @@ public function withConstructor(array $params) * @return $this */ public function withMethod($methodName, array $params) + { + $this->classDefinition->addMethodInjection($this->createMethodInjection($methodName, $params)); + return $this; + } + + /** + * @return ClassDefinition + */ + public function getDefinition() + { + return $this->classDefinition; + } + + /** + * @param string[] $params Parameters for the method: array of container entries names + * @return MethodInjection + */ + private function createMethodInjection($methodName, array $params) { $paramInjections = array(); @@ -108,16 +122,7 @@ public function withMethod($methodName, array $params) $paramInjections[] = $parameterInjection; } - $this->classDefinition->addMethodInjection(new MethodInjection($methodName, $paramInjections)); - return $this; - } - - /** - * @return ClassDefinition - */ - public function getDefinition() - { - return $this->classDefinition; + return new MethodInjection($methodName, $paramInjections); } } diff --git a/src/DI/Definition/Source/ArrayDefinitionSource.php b/src/DI/Definition/Source/ArrayDefinitionSource.php index 03a20a74b..3828cb406 100644 --- a/src/DI/Definition/Source/ArrayDefinitionSource.php +++ b/src/DI/Definition/Source/ArrayDefinitionSource.php @@ -54,7 +54,7 @@ public function getDefinition($name) } // Value definition - if (!is_array($arrayDefinition)) { + if (!is_array($arrayDefinition) || ($arrayDefinition && array_values($arrayDefinition) === $arrayDefinition)) { return new ValueDefinition($name, $arrayDefinition); } @@ -281,5 +281,4 @@ private function mergeWithParents($name, ClassDefinition $definition) } } } - } diff --git a/src/DI/Factory.php b/src/DI/Factory.php index 5c3e17ec3..92b89ec72 100644 --- a/src/DI/Factory.php +++ b/src/DI/Factory.php @@ -128,8 +128,11 @@ private function injectConstructor(ReflectionClass $classReflection, MethodInjec . "' of the constructor of '{$classReflection->name}' has no type defined or guessable"); } - // TODO handle lazy injections! - $args[] = $this->container->get($entryName); + if ($parameterInjection->isLazy()) { + $args[] = $this->container->get($entryName, true); + } else { + $args[] = $this->container->get($entryName); + } } return $classReflection->newInstanceArgs($args); diff --git a/tests/IntegrationTests/DI/Fixtures/Class1.php b/tests/IntegrationTests/DI/Fixtures/Class1.php index 8c6acbce0..3ea823e99 100644 --- a/tests/IntegrationTests/DI/Fixtures/Class1.php +++ b/tests/IntegrationTests/DI/Fixtures/Class1.php @@ -49,6 +49,7 @@ class Class1 public $constructorParam1; public $constructorParam2; + public $constructorParam3; public $method1Param1; @@ -60,14 +61,17 @@ class Class1 public $method4Param1; /** - * @param Class2 $param1 - * @param Interface1 $param2 + * @Inject({"param3" = {"lazy" = true}}) + * @param Class2 $param1 + * @param Interface1 $param2 + * @param LazyDependency $param3 * @throws \Exception */ - public function __construct(Class2 $param1, Interface1 $param2, $optional = true) + public function __construct(Class2 $param1, Interface1 $param2, LazyDependency $param3, $optional = true) { $this->constructorParam1 = $param1; $this->constructorParam2 = $param2; + $this->constructorParam3 = $param3; if ($optional !== true) { throw new \Exception("Expected optional parameter to not be defined"); @@ -109,7 +113,7 @@ public function method3($param1, $param2) /** * @Inject({"param1" = {"lazy" = true}}) - * @param string $param1 + * @param LazyDependency $param1 */ public function method4(LazyDependency $param1) { diff --git a/tests/IntegrationTests/DI/Fixtures/definitions.php b/tests/IntegrationTests/DI/Fixtures/definitions.php index e0fab04e1..32529fa56 100644 --- a/tests/IntegrationTests/DI/Fixtures/definitions.php +++ b/tests/IntegrationTests/DI/Fixtures/definitions.php @@ -19,6 +19,10 @@ 'constructor' => array( 'param1' => 'IntegrationTests\DI\Fixtures\Class2', 'param2' => 'IntegrationTests\DI\Fixtures\Interface1', + 'param3' => array( + 'name' => 'IntegrationTests\DI\Fixtures\LazyDependency', + 'lazy' => true, + ), ), 'methods' => array( 'method1' => 'IntegrationTests\DI\Fixtures\Class2', diff --git a/tests/IntegrationTests/DI/Fixtures/definitions.yml b/tests/IntegrationTests/DI/Fixtures/definitions.yml index f1df1e512..29f9db20b 100644 --- a/tests/IntegrationTests/DI/Fixtures/definitions.yml +++ b/tests/IntegrationTests/DI/Fixtures/definitions.yml @@ -15,6 +15,9 @@ IntegrationTests\DI\Fixtures\Class1: constructor: param1: IntegrationTests\DI\Fixtures\Class2 param2: IntegrationTests\DI\Fixtures\Interface1 + param3: + name: IntegrationTests\DI\Fixtures\LazyDependency + lazy: true methods: method1: IntegrationTests\DI\Fixtures\Class2 method2: [IntegrationTests\DI\Fixtures\Interface1] diff --git a/tests/IntegrationTests/DI/InheritanceTest.php b/tests/IntegrationTests/DI/InheritanceTest.php index b90036931..65a90e490 100644 --- a/tests/IntegrationTests/DI/InheritanceTest.php +++ b/tests/IntegrationTests/DI/InheritanceTest.php @@ -10,6 +10,7 @@ namespace IntegrationTests\DI; use DI\Container; +use DI\ContainerBuilder; use IntegrationTests\DI\Fixtures\InheritanceTest\SubClass; /** @@ -58,16 +59,18 @@ public function testInjectionBaseClass(Container $container) public static function containerProvider() { // Test with a container using annotations - $containerAnnotations = new Container(); - $containerAnnotations->useReflection(true); - $containerAnnotations->useAnnotations(true); + $builder = new ContainerBuilder(); + $builder->useReflection(true); + $builder->useAnnotations(true); + $containerAnnotations = $builder->build(); $containerAnnotations->set('IntegrationTests\DI\Fixtures\InheritanceTest\BaseClass') ->bindTo('IntegrationTests\DI\Fixtures\InheritanceTest\SubClass'); // Test with a container using array configuration - $containerFullArrayDefinitions = new Container(); - $containerFullArrayDefinitions->useReflection(false); - $containerFullArrayDefinitions->useAnnotations(false); + $builder = new ContainerBuilder(); + $builder->useReflection(false); + $builder->useAnnotations(false); + $containerFullArrayDefinitions = $builder->build(); $containerFullArrayDefinitions->addDefinitions( array( 'IntegrationTests\DI\Fixtures\InheritanceTest\BaseClass' => array( @@ -99,9 +102,10 @@ public static function containerProvider() ); // Test with a container using array configuration - $containerInheritanceDefinitions = new Container(); - $containerInheritanceDefinitions->useReflection(false); - $containerInheritanceDefinitions->useAnnotations(false); + $builder = new ContainerBuilder(); + $builder->useReflection(false); + $builder->useAnnotations(false); + $containerInheritanceDefinitions = $builder->build(); $containerInheritanceDefinitions->addDefinitions( array( 'IntegrationTests\DI\Fixtures\InheritanceTest\BaseClass' => array( diff --git a/tests/IntegrationTests/DI/InjectionTest.php b/tests/IntegrationTests/DI/InjectionTest.php index 4130eda68..84d9ae732 100644 --- a/tests/IntegrationTests/DI/InjectionTest.php +++ b/tests/IntegrationTests/DI/InjectionTest.php @@ -97,6 +97,10 @@ public static function containerProvider() array( 'param1' => 'IntegrationTests\DI\Fixtures\Class2', 'param2' => 'IntegrationTests\DI\Fixtures\Interface1', + 'param3' => array( + 'name' => 'IntegrationTests\DI\Fixtures\LazyDependency', + 'lazy' => true, + ) ) ) ->withMethod('method1', array('IntegrationTests\DI\Fixtures\Class2')) @@ -149,6 +153,18 @@ public function testConstructorInjection($type, Container $container) $this->assertInstanceOf('IntegrationTests\DI\Fixtures\Class2', $class1->constructorParam1); $this->assertInstanceOf('IntegrationTests\DI\Fixtures\Implementation1', $class1->constructorParam2); + + // Test lazy injection (not possible using reflection) + if ($type != self::DEFINITION_REFLECTION) { + $this->assertInstanceOf('IntegrationTests\DI\Fixtures\LazyDependency', $class1->constructorParam3); + $this->assertInstanceOf('ProxyManager\Proxy\LazyLoadingInterface', $class1->constructorParam3); + /** @var LazyDependency|\ProxyManager\Proxy\LazyLoadingInterface $proxy */ + $proxy = $class1->constructorParam3; + $this->assertFalse($proxy->isProxyInitialized()); + // Correct proxy resolution + $this->assertTrue($proxy->getValue()); + $this->assertTrue($proxy->isProxyInitialized()); + } } /** @@ -188,7 +204,7 @@ public function testPropertyInjectionExistingObject($type, Container $container) return; } /** @var $class1 Class1 */ - $class1 = new Class1(new Class2(), new Implementation1()); + $class1 = new Class1(new Class2(), new Implementation1(), new LazyDependency()); $container->injectOn($class1); $this->assertInstanceOf('IntegrationTests\DI\Fixtures\Class2', $class1->property1); @@ -243,7 +259,7 @@ public function testMethodInjectionExistingObject($type, Container $container) return; } /** @var $class1 Class1 */ - $class1 = new Class1(new Class2(), new Implementation1()); + $class1 = new Class1(new Class2(), new Implementation1(), new LazyDependency()); $container->injectOn($class1); $this->assertInstanceOf('IntegrationTests\DI\Fixtures\Class2', $class1->method1Param1); diff --git a/tests/UnitTests/DI/ContainerBuilderTest.php b/tests/UnitTests/DI/ContainerBuilderTest.php index c9bf5f2c3..90e9b7b1b 100644 --- a/tests/UnitTests/DI/ContainerBuilderTest.php +++ b/tests/UnitTests/DI/ContainerBuilderTest.php @@ -48,4 +48,41 @@ public function testSetDefinitionsValidation() $this->assertTrue($container->getDefinitionManager()->getDefinitionsValidation()); } + public function testFluentInterface() + { + $builder = new ContainerBuilder(); + + $result = $builder->useAnnotations(false); + $this->assertSame($builder, $result); + + $result = $builder->useAnnotations(true); + $this->assertSame($builder, $result); + + $result = $builder->useReflection(false); + $this->assertSame($builder, $result); + + $result = $builder->useReflection(true); + $this->assertSame($builder, $result); + + $result = $builder->writeProxiesToFile(true, 'somedir'); + $this->assertSame($builder, $result); + + $result = $builder->writeProxiesToFile(false); + $this->assertSame($builder, $result); + + $result = $builder->setDefinitionsValidation(true); + $this->assertSame($builder, $result); + + $result = $builder->setDefinitionsValidation(false); + $this->assertSame($builder, $result); + + $mockCache = $this->getMockForAbstractClass('Doctrine\Common\Cache\Cache'); + $result = $builder->setDefinitionCache($mockCache); + $this->assertSame($builder, $result); + + $mockLoader = $this->getMock('DI\Definition\FileLoader\DefinitionFileLoader', array(), array(), '', false); + $result = $builder->addDefinitionsFromFile($mockLoader); + $this->assertSame($builder, $result); + } + } diff --git a/tests/UnitTests/DI/Definition/FileLoader/DefinitionFileLoaderTest.php b/tests/UnitTests/DI/Definition/FileLoader/DefinitionFileLoaderTest.php index b2a660cb0..6601f4254 100644 --- a/tests/UnitTests/DI/Definition/FileLoader/DefinitionFileLoaderTest.php +++ b/tests/UnitTests/DI/Definition/FileLoader/DefinitionFileLoaderTest.php @@ -21,4 +21,12 @@ public function testFileExists() { $this->getMockForAbstractClass('DI\Definition\FileLoader\DefinitionFileLoader', array('abcFile.php')); } + + /** + * @expectedException \DI\Definition\FileLoader\Exception\FileNotFoundException + */ + public function testFileIsNotADir() + { + $this->getMockForAbstractClass('DI\Definition\FileLoader\DefinitionFileLoader', array(__DIR__)); + } } diff --git a/tests/UnitTests/DI/Definition/FileLoader/Fixtures/definitions.json b/tests/UnitTests/DI/Definition/FileLoader/Fixtures/definitions.json index f95be5d4b..c74373f09 100644 --- a/tests/UnitTests/DI/Definition/FileLoader/Fixtures/definitions.json +++ b/tests/UnitTests/DI/Definition/FileLoader/Fixtures/definitions.json @@ -1,4 +1,5 @@ { + "value4": ["bob@acme.example.com", "alice@acme.example.com"], "value3": true, "value2": 123, "value1": "abc", diff --git a/tests/UnitTests/DI/Definition/FileLoader/Fixtures/definitions.php b/tests/UnitTests/DI/Definition/FileLoader/Fixtures/definitions.php index 2f04692c0..887767bc0 100644 --- a/tests/UnitTests/DI/Definition/FileLoader/Fixtures/definitions.php +++ b/tests/UnitTests/DI/Definition/FileLoader/Fixtures/definitions.php @@ -6,6 +6,7 @@ 'value1' => 'abc', 'value2' => 123, 'value3' => true, + 'value4' => array('bob@acme.example.com', 'alice@acme.example.com'), 'namespace\class1' => array( 'scope' => 'singleton', 'lazy' => true, diff --git a/tests/UnitTests/DI/Definition/FileLoader/Fixtures/definitions.yaml b/tests/UnitTests/DI/Definition/FileLoader/Fixtures/definitions.yaml index 9f37e4427..9160b477d 100644 --- a/tests/UnitTests/DI/Definition/FileLoader/Fixtures/definitions.yaml +++ b/tests/UnitTests/DI/Definition/FileLoader/Fixtures/definitions.yaml @@ -1,6 +1,9 @@ value1: abc value2: 123 value3: true +value4: + - bob@acme.example.com + - alice@acme.example.com namespace\class1: scope: singleton lazy: true diff --git a/tests/UnitTests/DI/Definition/Source/ArrayDefinitionSourceTest.php b/tests/UnitTests/DI/Definition/Source/ArrayDefinitionSourceTest.php index bc08c167a..c9459d9f3 100644 --- a/tests/UnitTests/DI/Definition/Source/ArrayDefinitionSourceTest.php +++ b/tests/UnitTests/DI/Definition/Source/ArrayDefinitionSourceTest.php @@ -304,6 +304,19 @@ public function testKeysValidation() $source->getDefinition('foo'); } + public function testIndexedNonEmptyArrayAsValidValue() + { + $source = new ArrayDefinitionSource(); + $source->addDefinitions( + array( + 'foo' => array('a', 'b', 'c') + ) + ); + $definition = $source->getDefinition('foo'); + $this->assertInstanceOf('\\DI\\Definition\\ValueDefinition', $definition); + $this->assertEquals(array('a', 'b', 'c'), $definition->getValue()); + } + public function testClosureDefinition() { $source = new ArrayDefinitionSource();