Issues

ZF2-509: Zend\Di\DefinitionList with ClassDefinition collecting supertypes problem

Description

If DefinitionList has multiple classDefinition, Zend\Di\DefinitionList::getClassSupertypes or Zend\Di\DefinitionList::getInstantiator call each classDefinition. A classDefinition instance is per-Class definition. And ClassDefinition::getClassSupertypes($class) returns simply its supertypes . In this flow, the class-name-parameter is meaningless.

https://github.com/zendframework/zf2/…

DefinitionInterface expects getClassSupertypes to return array filtered with the classname parameter.

public function getClassSupertypes($class)
{
    if ($this->class !== $class) return array();
    return $this->supertypes;
}

Comments

I made up pull request about this. https://github.com/zendframework/zf2/… And this is reproductive codes.


<?php 

use Zend\Di\Definition\ClassDefinition;
use Zend\Di\DefinitionList;

$definitionClassA = new ClassDefinition("A");
$definitionClassA->setSupertypes(array("superA"));
$definitionClassB = new ClassDefinition("B");
$definitionClassB->setSupertypes(array("superB"));
$definitionList = new DefinitionList(array($definitionClassA, $definitionClassB));

$resultA = $definitionList->getClassSuperTypes("A");
var_dump($resultA);
$resultB = $definitionList->getClassSuperTypes("B");
var_dump($resultB);

Expected:

A: array("superA") B: array("superB")

Actual: A: array(2) { [0]=> string(6) "superA" [1]=> string(6) "superB" } B: array(2) { [0]=> string(6) "superA" [1]=> string(6) "superB" }

And these cause wrong injection to new-instance. Zend\Di\Di::newInstance


        foreach ($definitions->getClassSupertypes($class) as $supertype) {
            $injectionMethods[$supertype] = $definitions->getMethods($supertype);
        }
......snip....
        $this->handleInjectDependencies($instance, $injectionMethods, $params, $class, $alias, $name);

it collects wrong injectionMethods from wrong supertypes. and so pass to handleInjectDependencies. In Zend\Di\Di::handleInjectDependencies, It uses wrong injection methods and wrong parameters.

I think it should be discussed whether the ClassDefinition in its current implementation should implement DefinitionInterface. As i understand the documentation of DefinitionInterface, it describes/is a collection of class definitions, whereas ClassDefinition is a single class definition...

So, in my opinion, the above solution solves a bug that was caused by a deeper, conceptually problem.

Shouldn't Array-, Builder-, Compiler- and RuntimeDefinitions hold a map of ClassDefinition instances? More abstract: Map DefinitionInterface

pros: - operate with ClassDefinitionInterface instances within DefinitionInterfaces rather then with "plain" arrays - more intuitive API (avoid "if($this->class === $class) ..." - absurdity for a specific class definition)

cons: - BC break

I agree with you about DefinitionInterface concepts.

ArrayDefinition is "Class definitions based on a given array", so it is a Definition for multiple class. ClassDefinition's description is that "Class definitions for a single class". I feel it should be abstractive signature representing the class structure. For example, Zend\Di\Config::configure makes an ArrayDefinition that it has a collection of ClassDefinition instances.

On the other hand, we already have a Definition's collection ,the DefinitionList. I think the current implementation is not bad. A DefinitionInterface usually has multiple class, but specially it has only a single class. And so single is specially, it should have codes "if($this->class === $class) ..."

[~az] I think there's nothing wrong in having a ClassDefinition implement the DefinitionInterface. It is just a "map" with a single element in it. What is weird is that $definition->getClassSupertypes($class) seems to retrieve wrong data.

As told on the mailing list, PR #2265 seems fine to me, but should also add checks on the rest of the ClassDefinition

I closed the pull-requests #2265,#2277 .And I remade a commit squashed. https://github.com/zendframework/zf2/pull/2295 Please review it.

Thanks.

I'm sorry. #2295 had several problems. I made an amendment in it.

fixed in 2.0.1 Thanks.