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
Posted by Tomoaki Kosugi (kosugiatkips) on 2012-08-31T00:47:43.000+0000
I made up pull request about this. https://github.com/zendframework/zf2/… And this is reproductive codes.
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" }
Posted by Tomoaki Kosugi (kosugiatkips) on 2012-08-31T10:34:39.000+0000
And these cause wrong injection to new-instance. Zend\Di\Di::newInstance
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.
Posted by Alrik Zachert (az) on 2012-08-31T12:42:25.000+0000
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
Posted by Tomoaki Kosugi (kosugiatkips) on 2012-08-31T15:18:58.000+0000
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) ..."
Posted by Marco Pivetta (ocramius) on 2012-09-05T08:30:28.000+0000
[~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.Posted by Marco Pivetta (ocramius) on 2012-09-05T08:56:00.000+0000
As told on the mailing list, PR #2265 seems fine to me, but should also add checks on the rest of the
ClassDefinitionPosted by Tomoaki Kosugi (kosugiatkips) on 2012-09-05T14:07:35.000+0000
I closed the pull-requests #2265,#2277 .And I remade a commit squashed. https://github.com/zendframework/zf2/pull/2295 Please review it.
Thanks.
Posted by Tomoaki Kosugi (kosugiatkips) on 2012-09-06T07:01:12.000+0000
I'm sorry. #2295 had several problems. I made an amendment in it.
Posted by Tomoaki Kosugi (kosugiatkips) on 2012-09-12T11:23:52.000+0000
fixed in 2.0.1 Thanks.