Circular definitions

[plasid]

I am using Slim Framework 4 and adding multiple definition files, but getting Circular Errors when I try to decorate() an existing definition from another file.

/var/www/vendor/php-di/php-di/src/Container.php: line 384
if (isset($this->entriesBeingResolved[$entryName])) {
throw new DependencyException("Circular dependency detected while trying to resolve entry '$entryName'");
}

All the below files returns an array of definitions, and works fine when I remove definitions_b.php or definitions_c.php, meaning it works fine when there are not more than 2 files trying to define and/or alter the same definition.

$containerBuilder->addDefinitions(['definitions_a.php', 'definitions_b.php', 'definitions_c.php']);

file definitions_a.php

return [
'settings' => function () {
       return ['acme' => 123];
           }
];

file definitions_b.php

use function DI\decorate;
return [
'settings' => decorate(function ($previous, ContainerInterface $c) {
      $previous['settings]['acme'] = 456;
      $result = Db::getOne('app');
      $previous['settings']['color'] = $result->color;
       return $previous;
    }
];`

file definitions_c.php

use function DI\decorate;
return [
'settings' => decorate(function ($previous, ContainerInterface $c) {
       $previous['settings']['acme'] =789;
       return $previous;
    }
];

I do understand the non-related problem of order-of-adding, but would please like your advice on the above.

[SvenRtbg]

Wow, and sorry for starting with a complaint. I really enjoyed setting up a VM using Vagrant on my private and available machine in the last hour, then trying to run your code to see what would happen.

There are plenty of syntax errors in it. So instead of (after providing a reasonable environment for it) copy-and-paste your example and immediately see your problem, I am now having to fix the syntax problems first. I can't really blame you because it seems you tried to create a reasonable example on the fly, but still that's something I would put as your responsibility: Use working code to describe your problem, making it as easy as possible for anyone trying to figure out your problem what you did and experienced.

So after fixing the various errors, and especially removing the Db::getOne call because I don't have it nor the database behind it, I get to the point in my code where I just do var_dump($container->get('settings'));, and I get:

array(2) {
  ["acme"]=>
  int(123)
  ["settings"]=>
  array(1) {
    ["acme"]=>
    int(789)
  }
}

No error, but a reasonable return value based on the fact that your example code is duplicating the "settings" key, i.e. you are supposed to work on the single value that is identified by the key "settings", which is just the array "acme", and you should return any changes to that, not duplicate "settings" again.

So your current report, in my view, is invalid as is. Please go back and illustrate it with a working example. I can imagine that maybe if your code uses the DI container again to fetch more dependencies, when fetching dependencies on decorated elements that require fetching again, you'd end up with a circular exception.

[plasid]

Hi,
Sorry about the errors, I was typing examples directly via the comment editor.
I believe how I'm doing it above is correct and the same as the official docs https://php-di.org/doc/definition-overriding.html#decorators - unless what you mean is I should not be altering $previous['settings'] directly, but add an intermediary, then assign and return $previous?

Like I explained above, 2 files with 'settings' works fine, but not more than 2 files - if my way of doing it is wrong then all combinations of below will produce an error, unless its an implicit restriction to decorate the same entry more than once, which from a practical strategy point makes sense?:

This works fine:

$containerBuilder->addDefinitions(['definitions_a.php', 'definitions_b.php']);
//OR
$containerBuilder->addDefinitions(['definitions_a.php', 'definitions_c.php']);

But this produces an error:

$containerBuilder->addDefinitions(['definitions_a.php', 'definitions_b.php', 'definitions_c.php']);

Thank you.

[SvenRtbg]

Well, if you change your code to

$containerBuilder->addDefinitions('definitions_a.php', 'definitions_b.php'); // no square brackets, no array of file names!

then it does work with more than two files for me (using PHP-DI 6.3.5 during my tests after downgrading from 6.4.0, on PHP 8.0). Note that if you pass an array, this is seen as a single parameter, which has to contain definitions. The addDefinitions() allows for infinitely many parameters, but if they are arrays, they are treated as definitions. Strings are treated as files containing definitions.

Regarding your decorating code: Remember that you'll $container->get('settings') at some point, so anything the decorator is working on is the result of that undecorated operation. And the definition

'settings' => function () {
   return ['acme' => 123];
}

is returning ['acme' => 123] when asked about settings, so your decorator has to alter that acme key, or maybe add more keys, but it has NOT to return the settings key from the definition. The decorator is getting ['acme' => 123] as the $previous value. Want to change it to ['acme' => 789], then the decorator should just do that:

$previous['acme'] =789;
return $previous;

And if you want to ask the database for the value, you can do that as well. But consider avoiding unnecessary work. PHP-DI will probably decorate multiple times because if you add for example a caching layer and then another layer around an object, you'll expect both layers. So in your case where only one scalar result is returned, it is useless to ask the database if the last decorator is overwriting the result anyways. I assume your original situation is different, but still I feel this should be mentioned.

[plasid]

Thank you very much for the time spent on this, I will give it a try.

[plasid]

Did not have time to play with this again until now... I tried new clean basic implementations from the official docs etc. and the short story is:
The above still does not work, and its not because of PHP-DI or my implementation, its just a logical problem bordering on sort of a race condition.
You cannot decorate and/or call other dependencies while they are being resolved (and the criteria is VERY precise), hence my suggestion in another post that this mechanism would have work better asynchronously, meaning all dependencies are resolved before any code tries to call a value from it - this way you would even be able to call a dependency from within another dependency regardless if its been resolved and without circular errors.
IMHO.