Merge pull request laravel#1690 from kdocki/master

taylorotwell · taylorotwell · commit 2e8b5575e5a1 · 2013-02-25T20:11:33.000-08:00
IoC container not resolving classes with optional parameters (i.e. models that use Eloquent)
diff --git a/laravel/ioc.php b/laravel/ioc.php
@@ -172,7 +172,7 @@ protected static function build($type, $parameters = array())
 			return new $type;
 		}
 
-		$dependencies = static::dependencies($constructor->getParameters());
+		$dependencies = static::dependencies($constructor->getParameters(), $parameters);
 
 		return $reflector->newInstanceArgs($dependencies);
 	}
@@ -181,28 +181,54 @@ protected static function build($type, $parameters = array())
 	 * Resolve all of the dependencies from the ReflectionParameters.
 	 *
 	 * @param  array  $parameters
+	 * @param  array  $arguments that might have been passed into our resolve
 	 * @return array
 	 */
-	protected static function dependencies($parameters)
+	protected static function dependencies($parameters, $arguments)
 	{
 		$dependencies = array();
 
 		foreach ($parameters as $parameter)
 		{
 			$dependency = $parameter->getClass();
 
-			// If the class is null, it means the dependency is a string or some other
-			// primitive type, which we can not resolve since it is not a class and
-			// we'll just bomb out with an error since we have nowhere to go.
-			if (is_null($dependency))
+			// If the person passed in some parameters to the class
+			// then we should probably use those instead of trying 
+			// to resolve a new instance of the class
+			if (count($arguments) > 0)
 			{
-				throw new \Exception("Unresolvable dependency resolving [$parameter].");
+				$dependencies[] = array_shift($arguments);
+			}
+			else if (is_null($dependency))
+			{
+				$dependency[] = static::resolveNonClass($parameter);
+			}
+			else
+			{
+				$dependencies[] = static::resolve($dependency->name);				
 			}
-
-			$dependencies[] = static::resolve($dependency->name);
 		}
 
 		return (array) $dependencies;
 	}
 
+	/**
+	 * Resolves optional parameters for our dependency injection
+	 * pretty much took backport straight from L4's Illuminate\Container
+	 *
+	 * @param ReflectionParameter
+	 * @return default value
+	 */
+	protected static function resolveNonClass($parameter)
+	{
+		if ($parameter->isDefaultValueAvailable())
+		{
+			return $parameter->getDefaultValue();
+		}
+		else
+		{
+			throw new \Exception("Unresolvable dependency resolving [$parameter].");
+		}
+	}	
+
 }
diff --git a/laravel/tests/cases/ioc.test.php b/laravel/tests/cases/ioc.test.php
@@ -1,5 +1,34 @@
 <?php
 
+/**
+ * Testing Optional Parameters in classes' Dependency Injection
+ */
+class TestOptionalParamClassForIoC
+{
+	public function __construct($optional_param = 42) {}
+}
+
+/**
+ * Testing Dependency Injection with this class
+ */
+class TestClassOneForIoC
+{
+	public $_variable;
+}
+
+/**
+ * Testing Dependency Injection of ClassOne
+ */
+class TestClassTwoForIoC
+{
+	public $class_one;
+	public function __construct(TestClassOneForIoC $class_one)
+	{
+		$this->class_one = $class_one;
+	}
+}
+
+
 class IoCTest extends PHPUnit_Framework_TestCase {
 
 	/**
@@ -71,4 +100,54 @@ public function testControllerMethodRegistersAController()
 		$this->assertTrue(IoC::registered('controller: ioc.test'));
 	}
 
+	/**
+	 * Test that classes with optional parameters can resolve
+	 */
+	public function testOptionalParamClassResolves()
+	{
+		$test = IoC::resolve('TestOptionalParamClassForIoC');
+		$this->assertInstanceOf('TestOptionalParamClassForIoC', $test);
+	}
+
+	/**
+	 * Test that we can resolve TestClassOneForIoC using IoC
+	 */
+	public function testClassOneForIoCResolves()
+	{
+		$test = IoC::resolve('TestClassOneForIoC');
+		$this->assertInstanceOf('TestClassOneForIoC', $test);
+	}
+
+	/**
+	 * Test that we can resolve TestClassTwoForIoC
+	 */
+	public function testClassTwoForIoCResolves()
+	{
+		$test = IoC::resolve('TestClassTwoForIoC');
+		$this->assertInstanceOf('TestClassTwoForIoC', $test);
+	}
+
+	/**
+	 * Test that when we resolve TestClassTwoForIoC we auto resolve
+	 * the dependency for TestClassOneForIoC
+	 */
+	public function testClassTwoResolvesClassOneDependency()
+	{
+		$test = IoC::resolve('TestClassTwoForIoC');
+		$this->assertInstanceOf('TestClassOneForIoC', $test->TestClassOneForIoC);
+	}
+
+	/**
+	 * Test that when we resolve TestClassTwoForIoC with a parameter
+	 * that it actually uses that instead of a blank class TestClassOneForIoC
+	 */
+	public function testClassTwoResolvesClassOneWithArgument()
+	{
+		$class_one = IoC::resolve('TestClassOneForIoC');
+		$class_one->test_variable = 42;
+
+		$class_two = IoC::resolve('TestClassTwoForIoC', [$class_one]);
+		$this->assertEquals(42, $class_two->class_one->test_variable);
+	}
+
 }
