elie29 / zend-di-config

PSR-11 PHP-DI container configurator for Laminas, Mezzio, ZF, Expressive applications or any framework that needs a PSR-11 container
MIT License
19 stars 4 forks source link

Circular dependency detected #38

Closed yethee closed 5 years ago

yethee commented 5 years ago

Occurs error while resolving lazy injection:

Circular dependency detected while trying to resolve entry 'config'

Seems, in this case PHP-DI tries resolve each entry of config, because it is ArrayDefinition.

Simple code to reproduce:

use DI\ContainerBuilder;
use Elie\PHPDI\Config\Config;
use Psr\Container\ContainerInterface;

include __DIR__ . '/vendor/autoload.php';

class Foo
{
    public function __construct(string $key)
    {
    }
}

class Bar
{
    public function __construct(Foo $foo)
    {
    }

    public function compute(): void
    {
    }
}

$config = new Config([
    'key' => 'null',
    'dependencies' => [
        'factories' => [
            Foo::class => function (ContainerInterface $c) {
                return new Foo($c->get('config')['key']);
            },
        ]
    ],
]);

$containerBuilder = new ContainerBuilder();
$config->configureContainer($containerBuilder);

$containerBuilder->addDefinitions([
    Bar::class => DI\create()
        ->constructor(DI\get(Foo::class))
        ->lazy(),
]);

$container = $containerBuilder->build();

$bar = $container->get(Bar::class);
$bar->compute();
php-di/php-di:^6.0
elie29/zend-phpdi-config:^4.0
ocramius/proxy-manager:^2.1

If delete config.dependencies from the definitions this solves the issue. But, I do not know whether this can lead to issues in other cases?

    private function setDependencies(): void
    {
        $this->dependencies = $this->definitions[$this::CONFIG]['dependencies'] ?? [];
+       unset($this->definitions[$this::CONFIG]['dependencies']);
    }
elie29 commented 5 years ago

@yethee thx for the report. I will check it tomorrow and give you my feedback.

elie29 commented 5 years ago

@yethee well you are right. The problem is related to the dependencies ArrayDefinition and factory callable function:

Foo::class => function (ContainerInterface $c) {
    return new Foo($c->get('config')['key']);
}

When we have a callable through an ArrayDefintion, the callable is resolved automatically which in this case, leads to a circular dependency because the callable requires the config key from the container.

This issue could be resolved by using a factory class as follow:

class FooFactory
{
    public function __invoke(ContainerInterface $c)
    {
        return new Foo($c->get('config')['key']);
    }
}

$config = new Config([
    'key' => 'null',
    'dependencies' => [
        'factories' => [
            Foo::class => function (ContainerInterface $c) {
                return new Foo($c->get('config')['key']);
            },
        ],
    ],
]);

However, I prefer also removes the dependencies key from the definitions as they are already resolved and not needed any more:

private function addDefinitions(ContainerBuilder $builder): void
{
    $this->addServices();
    $this->addFactories();
    $this->addInvokables();
    $this->addAutowires();
    $this->addAliases();
    $this->addDelegators();

    /**
      * PHPDI ArrayDefinition would resolve all keys
      * or dependencies are already resolved
      */
    unset($this->definitions[$this::CONFIG]['dependencies']);

     $builder->addDefinitions($this->definitions);
}
yethee commented 5 years ago

@elie29 thanks for fix and a new release!