Potential bug with composing and composed dependencies both aliased with interfaces #265
Comments
|
Hi,
It is actually by design, to avoid situations that could result in recursion hell, currently it doesn't expand to find another alias for the concrete argument, it just resolves it.
Does that make sense? Essentially the container isn't even considering your definition with the alias using the concrete class name, it expects explicitness.
There are ways to do it, like using a closure factory for the concrete argument and resolving from the container again.
That being said, I'm now considering a way this might be able to be done, but it would be something I look at for the 5.x release I'm working on.
Sent from Proton Mail Android
…-------- Original Message --------
On 26/10/2024 02:52, Matthew Turland wrote:
This may admittedly be a misunderstanding on my part regarding the expected behavior of this library.
See [this branch](https://github.com/thephpleague/container/compare/4.x...elazar:container:nested-interface-dependencies?expand=1#diff-b4c3bee7306f9ccb50451779c77e51e11c41c60d9d70c0b9c104218f7984212f). I've tried running the test case that it adds against the 4.x branch, the 4.2.3 tag, and the branch from [#259](#259) (leading me to believe this isn't an issue with recursive dependency resolution), but all yield the same result. See the test failure output against my branch (which is based on the 4.x branch) shown below.
➜ container git:(nested-interface-dependencies) ./vendor/bin/phpunit --filter testContainerAddsAndGetsNestedInterfaceDependency tests/ContainerTest.php
PHPUnit 8.5.40 by Sebastian Bergmann and contributors.
Runtime: PHP 8.2.24 with Xdebug 3.3.1
Configuration: container/phpunit.xml
Error: This version of PHPUnit does not support code coverage on PHP 8
E 1 / 1 (100%)
Time: 106 ms, Memory: 6.00 MB
There was 1 error:
1) League\Container\Test\ContainerTest::testContainerAddsAndGetsNestedInterfaceDependency
ArgumentCountError: Too few arguments to function League\Container\Test\Asset\Bay::__construct(), 0 passed and exactly 1 expected
container/tests/Asset/Bay.php:11
container/src/Definition/Definition.php:212
container/src/Definition/Definition.php:175
container/src/Definition/Definition.php:154
container/src/Definition/DefinitionAggregate.php:79
container/src/Container.php:161
container/src/Container.php:111
container/tests/ContainerTest.php:33
If I pass the $container variable in the test to var_dump() immediately before the call to its get() method that causes the test failure, this is the output.
container/tests/ContainerTest.php:32:
class League\Container\Container#324 (5) {
protected $defaultToShared =>
bool(false)
protected $definitions =>
class League\Container\Definition\DefinitionAggregate#296 (2) {
protected $definitions =>
array(3) {
[0] =>
class League\Container\Definition\Definition#16 (8) {
protected $alias =>
string(40) "League\Container\Test\Asset\BayInterface"
protected $concrete =>
string(31) "League\Container\Test\Asset\Bay"
protected $shared =>
bool(false)
protected $tags =>
array(0) {
}
protected $arguments =>
array(0) {
}
protected $methods =>
array(0) {
}
protected $resolved =>
NULL
protected $container =>
NULL
}
[1] =>
class League\Container\Definition\Definition#17 (8) {
protected $alias =>
string(40) "League\Container\Test\Asset\BazInterface"
protected $concrete =>
string(31) "League\Container\Test\Asset\Baz"
protected $shared =>
bool(false)
protected $tags =>
array(0) {
}
protected $arguments =>
array(0) {
}
protected $methods =>
array(0) {
}
protected $resolved =>
NULL
protected $container =>
NULL
}
[2] =>
class League\Container\Definition\Definition#23 (8) {
protected $alias =>
string(31) "League\Container\Test\Asset\Bay"
protected $concrete =>
string(31) "League\Container\Test\Asset\Bay"
protected $shared =>
bool(false)
protected $tags =>
array(0) {
}
protected $arguments =>
array(1) {
[0] =>
string(40) "League\Container\Test\Asset\BazInterface"
}
protected $methods =>
array(0) {
}
protected $resolved =>
NULL
protected $container =>
NULL
}
}
protected $container =>
...
}
protected $providers =>
class League\Container\ServiceProvider\ServiceProviderAggregate#25 (3) {
protected $providers =>
array(0) {
}
protected $registered =>
array(0) {
}
protected $container =>
...
}
protected $inflectors =>
class League\Container\Inflector\InflectorAggregate#22 (2) {
protected $inflectors =>
array(0) {
}
protected $container =>
...
}
protected $delegates =>
array(0) {
}
}
This behavior appears to be specific to these circumstances.
- One concrete class that implements an interface has a required constructor parameter that implements another interface.
- The container includes aliases from each of the two interfaces to their respective concrete class implementations and the argument of the composing class constructor.
—
Reply to this email directly, [view it on GitHub](#265), or [unsubscribe](https://github.com/notifications/unsubscribe-auth/AAP3E2I2DI4O2RDAHGIA7YDZ5LYXNAVCNFSM6AAAAABQUJ5AVOVHI2DSMVQWIX3LMV43ASLTON2WKOZSGYYTKNBVGE4DQNY).
You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>
|
|
@philipobenito I'm not sure I follow here. The pull request #254 that removed support for recursive resolution had the stated intention of preventing infinite loops when provided with a nonexistent class. Before that PR, recursive resolution was fully supported and I've had to fix my projects to version 4.2.0 in order to avoid problems. The pull request that I submitted #259 restores support for recursion while also preventing infinite loops in the event that the class provided does not exist. I don't quite get the rationale you've provided here against recursion, but either way #254 on its own introduced breaking changes that I don't think should have been included in a patch release. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
This may admittedly be a misunderstanding on my part regarding the expected behavior of this library.
See this branch. I've tried running the test case that it adds against the
4.xbranch, the4.2.3tag, and the branch from #259 (leading me to believe this isn't an issue with recursive dependency resolution), but all yield the same result. See the test failure output against my branch (which is based on the4.xbranch) shown below.If I pass the
$containervariable in the test tovar_dump()immediately before the call to itsget()method that causes the test failure, this is the output.This behavior appears to be specific to these circumstances.
The text was updated successfully, but these errors were encountered: