Skip to content

Commit

Permalink
Providing fix for ZF2-142 zendframework#2451
Browse files Browse the repository at this point in the history
  • Loading branch information
Ocramius committed Nov 18, 2012
1 parent 7746dcc commit 9a5e530
Show file tree
Hide file tree
Showing 11 changed files with 70 additions and 27 deletions.
6 changes: 4 additions & 2 deletions library/Zend/Di/Definition/Builder/InjectionMethod.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
6 changes: 6 additions & 0 deletions library/Zend/Di/Definition/BuilderDefinition.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
8 changes: 6 additions & 2 deletions library/Zend/Di/Definition/ClassDefinition.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
5 changes: 2 additions & 3 deletions library/Zend/Di/Definition/CompilerDefinition.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

}
Expand Down
1 change: 1 addition & 0 deletions library/Zend/Di/Definition/DefinitionInterface.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
* );
*
*
Expand Down
3 changes: 2 additions & 1 deletion library/Zend/Di/Definition/RuntimeDefinition.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

}
Expand Down
10 changes: 3 additions & 7 deletions library/Zend/Di/Di.php
Original file line number Diff line number Diff line change
Expand Up @@ -652,30 +652,27 @@ 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 {
$resolvedParams[$index] = $this->get($computedParams['required'][$fqParamPos][0], $callTimeUserParams);
}

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
Expand All @@ -686,9 +683,8 @@ protected function resolveMethodParameters($class, $method, array $callTimeUserP
} else {
return false;
}

} else {
$resolvedParams[$index] = null;
$resolvedParams[$index] = $value[3];
}

$index++;
Expand Down
14 changes: 7 additions & 7 deletions tests/ZendTest/Di/Definition/BuilderDefinitionTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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')
);
}
Expand Down Expand Up @@ -86,23 +86,23 @@ 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')
);

$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')
);

Expand Down Expand Up @@ -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')
);
}
Expand Down
7 changes: 5 additions & 2 deletions tests/ZendTest/Di/Definition/ClassDefinitionTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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")
);
}
}
2 changes: 1 addition & 1 deletion tests/ZendTest/Di/Definition/CompilerDefinitionTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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')
);
}
Expand Down
35 changes: 33 additions & 2 deletions tests/ZendTest/Di/Definition/RuntimeDefinitionTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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'
)
);
}
}

0 comments on commit 9a5e530

Please sign in to comment.