Split file handling of ArrayDefinitionSource to new PHPFileDefinition…

…Source

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;
     }
 
     /**

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 <matthieu@mnapoli.fr>
  */
@@ -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}
      */

src/DI/Definition/Source/PHPFileDefinitionSource.php

@@ -0,0 +1,76 @@
+<?php
+/**
+ * PHP-DI
+ *
+ * @link      http://php-di.org/
+ * @copyright Matthieu Napoli (http://mnapoli.fr/)
+ * @license   http://www.opensource.org/licenses/mit-license.php MIT (see the LICENSE file)
+ */
+
+namespace DI\Definition\Source;
+
+use DI\Definition\Exception\DefinitionException;
+use DI\Definition\MergeableDefinition;
+
+/**
+ * Reads DI definitions from a file returning a PHP array.
+ *
+ * @author Matthieu Napoli <matthieu@mnapoli.fr>
+ */
+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;
+    }
+}

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

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'

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

tests/UnitTests/DI/Definition/Source/PHPFileDefinitionSourceTest.php

@@ -0,0 +1,38 @@
+<?php
+/**
+ * PHP-DI
+ *
+ * @link      http://php-di.org/
+ * @copyright Matthieu Napoli (http://mnapoli.fr/)
+ * @license   http://www.opensource.org/licenses/mit-license.php MIT (see the LICENSE file)
+ */
+
+namespace UnitTests\DI\Definition\Source;
+
+use DI\Definition\ClassDefinition;
+use DI\Definition\Source\PHPFileDefinitionSource;
+
+/**
+ * Test class for PHPFileDefinitionSource
+ */
+class PHPFileDefinitionSourceTest extends \PHPUnit_Framework_TestCase
+{
+    /**
+     * @covers \DI\Definition\Source\PHPFileDefinitionSource
+     */
+    public function testLoadFromFile()
+    {
+        $source = new PHPFileDefinitionSource(__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());
+    }
+}