From 9a5e530fde6ee087b8c9e386b3972112efec8d02 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Sun, 18 Nov 2012 19:25:59 +0100 Subject: [PATCH] Providing fix for ZF2-142 zendframework/zf2#2451 --- .../Di/Definition/Builder/InjectionMethod.php | 6 ++-- .../Zend/Di/Definition/BuilderDefinition.php | 6 ++++ .../Zend/Di/Definition/ClassDefinition.php | 8 +++-- .../Zend/Di/Definition/CompilerDefinition.php | 5 ++- .../Di/Definition/DefinitionInterface.php | 1 + .../Zend/Di/Definition/RuntimeDefinition.php | 3 +- library/Zend/Di/Di.php | 10 ++---- .../Di/Definition/BuilderDefinitionTest.php | 14 ++++---- .../Di/Definition/ClassDefinitionTest.php | 7 ++-- .../Di/Definition/CompilerDefinitionTest.php | 2 +- .../Di/Definition/RuntimeDefinitionTest.php | 35 +++++++++++++++++-- 11 files changed, 70 insertions(+), 27 deletions(-) diff --git a/library/Zend/Di/Definition/Builder/InjectionMethod.php b/library/Zend/Di/Definition/Builder/InjectionMethod.php index e52f9e99ea6..aaf0ba2f51a 100644 --- a/library/Zend/Di/Definition/Builder/InjectionMethod.php +++ b/library/Zend/Di/Definition/Builder/InjectionMethod.php @@ -51,14 +51,16 @@ public function getName() * @param string $name * @param string|null $class * @param mixed|null $isRequired + * @param mixed|null $default * @return InjectionMethod */ - public function addParameter($name, $class = null, $isRequired = null) + public function addParameter($name, $class = null, $isRequired = null, $default = null) { $this->parameters[] = array( $name, $class, - ($isRequired == null) ? true : false + ($isRequired == null) ? true : false, + $default, ); return $this; diff --git a/library/Zend/Di/Definition/BuilderDefinition.php b/library/Zend/Di/Definition/BuilderDefinition.php index fb9ee2d4e59..1cb354ba7b6 100644 --- a/library/Zend/Di/Definition/BuilderDefinition.php +++ b/library/Zend/Di/Definition/BuilderDefinition.php @@ -295,20 +295,26 @@ public function hasMethodParameters($class, $method) public function getMethodParameters($class, $method) { $class = $this->getClass($class); + if ($class === false) { throw new Exception\RuntimeException('Cannot find class object in this builder definition.'); } + $methods = $class->getInjectionMethods(); + /* @var $methodObj Builder\InjectionMethod */ foreach ($methods as $methodObj) { if ($methodObj->getName() === $method) { $method = $methodObj; } } + if (!$method instanceof Builder\InjectionMethod) { throw new Exception\RuntimeException('Cannot find method object for method ' . $method . ' in this builder definition.'); } + $methodParameters = array(); + /* @var $method Builder\InjectionMethod */ foreach ($method->getParameters() as $name => $info) { $methodParameters[$class->getName() . '::' . $method->getName() . ':' . $name] = $info; diff --git a/library/Zend/Di/Definition/ClassDefinition.php b/library/Zend/Di/Definition/ClassDefinition.php index a3a5f007818..45df95849c3 100644 --- a/library/Zend/Di/Definition/ClassDefinition.php +++ b/library/Zend/Di/Definition/ClassDefinition.php @@ -104,12 +104,16 @@ public function addMethodParameter($method, $parameterName, array $parameterInfo $this->methodParameters[$method] = array(); } - $type = (isset($parameterInfo['type'])) ? $parameterInfo['type'] : null; + $type = (isset($parameterInfo['type'])) ? $parameterInfo['type'] : null; $required = (isset($parameterInfo['required'])) ? (bool) $parameterInfo['required'] : false; + $default = (isset($parameterInfo['default'])) ? $parameterInfo['default'] : null; $fqName = $this->class . '::' . $method . ':' . $parameterName; $this->methodParameters[$method][$fqName] = array( - $parameterName, $type, $required + $parameterName, + $type, + $required, + $default ); return $this; diff --git a/library/Zend/Di/Definition/CompilerDefinition.php b/library/Zend/Di/Definition/CompilerDefinition.php index be08fb8c7de..a2e1339f095 100644 --- a/library/Zend/Di/Definition/CompilerDefinition.php +++ b/library/Zend/Di/Definition/CompilerDefinition.php @@ -292,15 +292,14 @@ protected function processParams(&$def, Reflection\ClassReflection $rClass, Refl /** @var $p \ReflectionParameter */ $actualParamName = $p->getName(); - $fqName = $rClass->getName() . '::' . $rMethod->getName() . ':' . $p->getPosition(); - $def['parameters'][$methodName][$fqName] = array(); // set the class name, if it exists $def['parameters'][$methodName][$fqName][] = $actualParamName; $def['parameters'][$methodName][$fqName][] = ($p->getClass() !== null) ? $p->getClass()->getName() : null; - $def['parameters'][$methodName][$fqName][] = !$p->isOptional(); + $def['parameters'][$methodName][$fqName][] = !($optional =$p->isOptional()); + $def['parameters'][$methodName][$fqName][] = $optional ? $p->getDefaultValue() : null; } } diff --git a/library/Zend/Di/Definition/DefinitionInterface.php b/library/Zend/Di/Definition/DefinitionInterface.php index a642d9d2676..04b9b13955d 100644 --- a/library/Zend/Di/Definition/DefinitionInterface.php +++ b/library/Zend/Di/Definition/DefinitionInterface.php @@ -93,6 +93,7 @@ public function hasMethodParameters($class, $method); * 0, // string|null: Type Name (if it exists) * 1, // bool: whether this param is required * 2, // string: fully qualified path to this parameter + * 3, // mixed: default value * ); * * diff --git a/library/Zend/Di/Definition/RuntimeDefinition.php b/library/Zend/Di/Definition/RuntimeDefinition.php index f9b821bb804..3ec43d5431a 100644 --- a/library/Zend/Di/Definition/RuntimeDefinition.php +++ b/library/Zend/Di/Definition/RuntimeDefinition.php @@ -346,7 +346,8 @@ protected function processParams(&$def, Reflection\ClassReflection $rClass, Refl // set the class name, if it exists $def['parameters'][$methodName][$fqName][] = $actualParamName; $def['parameters'][$methodName][$fqName][] = ($p->getClass() !== null) ? $p->getClass()->getName() : null; - $def['parameters'][$methodName][$fqName][] = !$p->isOptional(); + $def['parameters'][$methodName][$fqName][] = !($optional =$p->isOptional()); + $def['parameters'][$methodName][$fqName][] = $optional ? $p->getDefaultValue() : null; } } diff --git a/library/Zend/Di/Di.php b/library/Zend/Di/Di.php index a943e0713db..13c797a3391 100644 --- a/library/Zend/Di/Di.php +++ b/library/Zend/Di/Di.php @@ -652,20 +652,19 @@ protected function resolveMethodParameters($class, $method, array $callTimeUserP $name = $value[0]; if (isset($computedParams['value'][$fqParamPos])) { - // if there is a value supplied, use it $resolvedParams[$index] = $computedParams['value'][$fqParamPos]; - } elseif (isset($computedParams['required'][$fqParamPos])) { - // detect circular dependencies! (they can only happen in instantiators) if ($isInstantiator && in_array($computedParams['required'][$fqParamPos][1], $this->currentDependencies)) { throw new Exception\CircularDependencyException( "Circular dependency detected: $class depends on {$value[1]} and viceversa" ); } + array_push($this->currentDependencies, $class); $dConfig = $this->instanceManager->getConfig($computedParams['required'][$fqParamPos][0]); + if ($dConfig['shared'] === false) { $resolvedParams[$index] = $this->newInstance($computedParams['required'][$fqParamPos][0], $callTimeUserParams, false); } else { @@ -673,9 +672,7 @@ protected function resolveMethodParameters($class, $method, array $callTimeUserP } array_pop($this->currentDependencies); - } elseif (!array_key_exists($fqParamPos, $computedParams['optional'])) { - if ($methodIsRequired) { // if this item was not marked as optional, // plus it cannot be resolve, and no value exist, bail out @@ -686,9 +683,8 @@ protected function resolveMethodParameters($class, $method, array $callTimeUserP } else { return false; } - } else { - $resolvedParams[$index] = null; + $resolvedParams[$index] = $value[3]; } $index++; diff --git a/tests/ZendTest/Di/Definition/BuilderDefinitionTest.php b/tests/ZendTest/Di/Definition/BuilderDefinitionTest.php index 5ecc6555ce3..8ac99064350 100644 --- a/tests/ZendTest/Di/Definition/BuilderDefinitionTest.php +++ b/tests/ZendTest/Di/Definition/BuilderDefinitionTest.php @@ -45,7 +45,7 @@ public function testBuilderCanBuildClassWithMethods() $this->assertTrue($definition->hasMethod('Foo', 'injectBar')); $this->assertContains('injectBar', $definition->getMethods('Foo')); $this->assertEquals( - array('Foo::injectBar:0' => array('bar', 'Bar', true)), + array('Foo::injectBar:0' => array('bar', 'Bar', true, null)), $definition->getMethodParameters('Foo', 'injectBar') ); } @@ -86,8 +86,8 @@ public function testBuilderCanBuildFromArray() $this->assertEquals('__construct', $definition->getInstantiator('My\DbAdapter')); $this->assertEquals( array( - 'My\DbAdapter::__construct:0' => array('username', null, true), - 'My\DbAdapter::__construct:1' => array('password', null, true) + 'My\DbAdapter::__construct:0' => array('username', null, true, null), + 'My\DbAdapter::__construct:1' => array('password', null, true, null), ), $definition->getMethodParameters('My\DbAdapter', '__construct') ); @@ -95,14 +95,14 @@ public function testBuilderCanBuildFromArray() $this->assertTrue($definition->hasClass('My\Mapper')); $this->assertEquals('__construct', $definition->getInstantiator('My\Mapper')); $this->assertEquals( - array('My\Mapper::__construct:0' => array('dbAdapter', 'My\DbAdapter', true)), + array('My\Mapper::__construct:0' => array('dbAdapter', 'My\DbAdapter', true, null)), $definition->getMethodParameters('My\Mapper', '__construct') ); $this->assertTrue($definition->hasClass('My\Repository')); $this->assertEquals('__construct', $definition->getInstantiator('My\Repository')); $this->assertEquals( - array('My\Repository::__construct:0' => array('mapper', 'My\Mapper', true)), + array('My\Repository::__construct:0' => array('mapper', 'My\Mapper', true, null)), $definition->getMethodParameters('My\Repository', '__construct') ); @@ -131,11 +131,11 @@ public function testCanCreateInjectionMethodsAndPopulateFromFluentInterface() $this->assertTrue($builder->hasMethod('Foo', 'setConfig')); $this->assertEquals( - array('Foo::setBar:0' => array('bar', 'Bar', true)), + array('Foo::setBar:0' => array('bar', 'Bar', true, null)), $builder->getMethodParameters('Foo', 'setBar') ); $this->assertEquals( - array('Foo::setConfig:0' => array('config', null, true)), + array('Foo::setConfig:0' => array('config', null, true, null)), $builder->getMethodParameters('Foo', 'setConfig') ); } diff --git a/tests/ZendTest/Di/Definition/ClassDefinitionTest.php b/tests/ZendTest/Di/Definition/ClassDefinitionTest.php index 8dc2ac399c6..0051819207c 100644 --- a/tests/ZendTest/Di/Definition/ClassDefinitionTest.php +++ b/tests/ZendTest/Di/Definition/ClassDefinitionTest.php @@ -71,8 +71,11 @@ public function testHasMethodParameters() public function testGetMethodParameters() { $definition = new ClassDefinition('Foo'); - $definition->addMethodParameter("setVar", "var", array('type' => null, 'required' => true)); + $definition->addMethodParameter("setVar", "var", array('type' => null, 'required' => true, 'default' => 'test')); $this->assertNull($definition->getMethodParameters("Bar", "setVar")); - $this->assertEquals(array('Foo::setVar:var' => array("var", null, true)), $definition->getMethodParameters("Foo", "setVar")); + $this->assertEquals( + array('Foo::setVar:var' => array("var", null, true, 'test')), + $definition->getMethodParameters("Foo", "setVar") + ); } } diff --git a/tests/ZendTest/Di/Definition/CompilerDefinitionTest.php b/tests/ZendTest/Di/Definition/CompilerDefinitionTest.php index 27a18833718..9d265430566 100644 --- a/tests/ZendTest/Di/Definition/CompilerDefinitionTest.php +++ b/tests/ZendTest/Di/Definition/CompilerDefinitionTest.php @@ -47,7 +47,7 @@ public function testCompilerCompilesAgainstConstructorInjectionAssets() $this->assertTrue($definition->hasMethod('ZendTest\Di\TestAsset\CompilerClasses\C', 'setB')); $this->assertEquals( - array('ZendTest\Di\TestAsset\CompilerClasses\C::setB:0' => array('b', 'ZendTest\Di\TestAsset\CompilerClasses\B', true)), + array('ZendTest\Di\TestAsset\CompilerClasses\C::setB:0' => array('b', 'ZendTest\Di\TestAsset\CompilerClasses\B', true, null)), $definition->getMethodParameters('ZendTest\Di\TestAsset\CompilerClasses\C', 'setB') ); } diff --git a/tests/ZendTest/Di/Definition/RuntimeDefinitionTest.php b/tests/ZendTest/Di/Definition/RuntimeDefinitionTest.php index 914c1d3c8de..afd9c273993 100644 --- a/tests/ZendTest/Di/Definition/RuntimeDefinitionTest.php +++ b/tests/ZendTest/Di/Definition/RuntimeDefinitionTest.php @@ -12,10 +12,41 @@ use PHPUnit_Framework_TestCase as TestCase; +use Zend\Di\Definition\RuntimeDefinition; + class RuntimeDefinitionTest extends TestCase { - public function testStub() + public function testIncludesDefaultMethodParameters() { - $this->markTestIncomplete(); + $definition = new RuntimeDefinition(); + + $definition->forceLoadClass('ZendTest\Di\TestAsset\ConstructorInjection\OptionalParameters'); + + $this->assertSame( + array( + 'ZendTest\Di\TestAsset\ConstructorInjection\OptionalParameters::__construct:0' => array( + 'a', + null, + false, + null, + ), + 'ZendTest\Di\TestAsset\ConstructorInjection\OptionalParameters::__construct:1' => array( + 'b', + null, + false, + 'defaultConstruct', + ), + 'ZendTest\Di\TestAsset\ConstructorInjection\OptionalParameters::__construct:2' => array( + 'c', + null, + false, + array(), + ), + ), + $definition->getMethodParameters( + 'ZendTest\Di\TestAsset\ConstructorInjection\OptionalParameters', + '__construct' + ) + ); } }