schmittjoh / JMSDiExtraBundle

Provides Advanced Dependency Injection Features for Symfony2
http://jmsyst.com/bundles/JMSDiExtraBundle
330 stars 130 forks source link

@Service and class inheritance issue #119

Open zimmermanj42 opened 11 years ago

zimmermanj42 commented 11 years ago

Posted an issue earlier, but closed it as I found out more about it and it has changed quite a bit.

Here is my setup:

<?php

namespace FirstBundle\Services;

use JMS\DiExtraBundle\Annotation as DI;

/**
 * @DI\Service("foo", public = true)
 */
class Foo {}

<?php

namespace SecondBundle\Services;

use JMS\DiExtraBundle\Annotation as DI;
use FirstBundle\Services\Foo as BaseFoo;

/**
 * @DI\Service("foo", public = true)
 */
class Foo extends BaseFoo {}

Ultimately, I need to replace a service defined in FirstBundle with a new implementation in SecondBundle (I am using bundle inheritance to extend/replace other things in the FirstBundle).

However, the above configuration doesn't work; some kind of infinite loop is entered in Symfony and I run out of memory (just running the "container:debug" command).

I've discovered that JMSDiExtraBundle is setting the "parent" of the service definition (rather a definition decorator) for the SecondBundle's Foo class to "foo", so in some kind of compiler pass Symfony just keeps looping over the "foo" service as it's parent is itself...

It seems that JMSDiExtraBundle is determining the parent of the SecondBundle's Foo by inspecting the class data (since it extends the FirstBundle's Foo), and even if I explicitly set the "parent" attribute in the Service annotation markup for SecondBundle's Foo to "null" (not the string, but just plain null), it is still setting the parent based on this class metadata.

I don't know if this is intended or not, but if I don't use the JMSDiExtraBundle annotations and just stick with YAML config, this scheme works just fine and as intended.

zimmermanj42 commented 11 years ago

This is were the "issue" is happening (file is "/Metadata/MetadataConverter.php"):

class MetadataConverter
{

    public function convert(ClassHierarchyMetadata $metadata)
    {
        static $count = 0;
        $definitions = array();

        $previous = null;
        foreach ($metadata->classMetadata as $classMetadata) {
            if (null === $previous && null === $classMetadata->parent) {
                $definition = new Definition();
            } else {
                $definition = new DefinitionDecorator(
                    $classMetadata->parent ?: $previous->id
                );
            }

The "&&" in "if (null === $previous && null === $classMetadata->parent)" is what is causing this to happen. Even if the Service annotation is set to null for the parent, the fact that the class extends another class seems to trump that and still create a DefinitionDecorator compared to a Definition that would end up overwriting the current one (assuming the same name is used). If the "&&" is changed to "||", the fact that I set a null parent in the annotation takes affect and this then works.

Like I've said, what is there may be the intended effect, but it would seem that if you explicitly set a null parent in the annotation, then the parent should be null and a new Definition would be made.

toothman commented 9 years ago

I can confirm this bug when using standard class Inheritance in Symfony Controllers.

Example:

class AbstractController extends Controller
{
    /**
     * @DI\Inject("my_software.config")
     */
    public $config;
}
class FooController extends AbstractController
{
    public function fooAction()
    {
        return new Response('hello');
    }
}

FooController is loaded as an Instance of AbstractController. As soon as I Inject another Property in FooController as well, the problem disappears. The problem only appears when there are >1 controllers extending AbstractController... Just a quick guess but the Bug seems to be somewhere around MetadataFactory:67 (getMetadataForClass)..

jtreminio commented 7 years ago

Ran into this myself. @zimmermanj42's comment fixes the issue. Can we get dev feedback?