Skip to content

Commit 0492dff

Browse files
philipobenitophilipobenito
committed
Enhance container functionality: add getDelegate method, improve definition resolution, and update documentation
1 parent 01a0afa commit 0492dff

8 files changed

Lines changed: 216 additions & 15 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,11 @@ All Notable changes to `League\Container` will be documented in this file
1414
- `EventDispatcher::hasListenersFor()` to check whether listeners exist for a given event type
1515
- `DefinitionInterface::getTags()` for retrieving tags from definitions
1616
- Docs: [https://container.thephpleague.com/unstable/events/](https://container.thephpleague.com/unstable/events/)
17+
- `Container::getDelegate(string $class)` to retrieve a registered delegate container by type
18+
19+
### Fixed
20+
- Interface-to-concrete definitions now correctly resolve through the concrete's own registered definition instead of bypassing it via direct reflection (#275, #278)
21+
- `Definition::resolveClass()` now throws `ContainerException` with actionable guidance when a class has unsatisfied constructor dependencies, instead of a raw `ArgumentCountError`
1722

1823
### Deprecated
1924
- `Container::inflector()` - use `Container::afterResolve()` or the event system instead. Will be removed in v6.0.

‎docs/unstable/auto-wiring.md‎

Lines changed: 37 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ var_dump($foo->baz instanceof Acme\Baz); // true
7777
var_dump($foo->bar->bam instanceof Acme\Bam); // true
7878
~~~
7979

80-
**Note:** The reflection container, by default, will resolve what you are requesting every time you request it.
80+
**Note:** The reflection container, by default, will resolve what you are requesting every time you request it. Auto-wiring only applies to classes that have **not** been registered as explicit definitions. If you register a class with `add()` or `addShared()`, you must provide its constructor arguments explicitly using `addArgument()` or a callable.
8181

8282
If you would like the reflection container to cache resolutions and pull from that cache if available, you can enable it to do so as below.
8383

@@ -158,4 +158,40 @@ $container->add(DatabaseLogger::class);
158158
$service = $container->get(AdvancedService::class);
159159
~~~
160160

161+
## Passing Runtime Arguments
162+
163+
When a class has constructor parameters that cannot be auto-wired (such as scalar values), you can pass them directly to the `ReflectionContainer`. Arguments are matched by parameter name.
164+
165+
~~~ php
166+
<?php
167+
168+
namespace Acme;
169+
170+
class ApiClient
171+
{
172+
public function __construct(
173+
public readonly HttpClient $http,
174+
public readonly string $apiKey,
175+
public readonly int $timeout
176+
) {}
177+
}
178+
179+
$container = new League\Container\Container();
180+
181+
$container->delegate(
182+
new League\Container\ReflectionContainer()
183+
);
184+
185+
// Retrieve the ReflectionContainer delegate and pass runtime arguments
186+
$reflection = $container->getDelegate(League\Container\ReflectionContainer::class);
187+
$client = $reflection->get(Acme\ApiClient::class, [
188+
'apiKey' => 'sk-123',
189+
'timeout' => 30,
190+
]);
191+
192+
// HttpClient is auto-wired, apiKey and timeout are provided
193+
~~~
194+
195+
Arguments must use the parameter name as the array key. Auto-wirable dependencies (type-hinted objects) are resolved automatically; only non-auto-wirable parameters need to be provided.
196+
161197
**Note:** The reflection container, by default, will resolve what you are requesting every time you request it.

‎docs/unstable/delegate-containers.md‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,4 +39,20 @@ $container->delegate($delegate);
3939

4040
Now that the delegate has been registered, if a service cannot be resolved via the primary container, it will resort to the `has` and `get` methods of the delegates to resolve the requested service.
4141

42+
### Retrieving a Delegate
4243

44+
If you need to access a registered delegate later, use the `getDelegate` method with the delegate's class name.
45+
46+
~~~ php
47+
<?php
48+
49+
$container = new League\Container\Container();
50+
51+
$container->delegate(
52+
new League\Container\ReflectionContainer()
53+
);
54+
55+
$reflection = $container->getDelegate(League\Container\ReflectionContainer::class);
56+
~~~
57+
58+
This is useful when you need to call delegate-specific methods that are not part of the PSR-11 interface, such as passing runtime arguments to `ReflectionContainer::get()` (see [Auto Wiring](/unstable/auto-wiring/)).

‎src/Container.php‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -189,6 +189,20 @@ public function delegate(ContainerInterface $container): self
189189
return $this;
190190
}
191191

192+
public function getDelegate(string $class): ContainerInterface
193+
{
194+
foreach ($this->delegates as $delegate) {
195+
if ($delegate instanceof $class) {
196+
return $delegate;
197+
}
198+
}
199+
200+
throw new NotFoundException(sprintf(
201+
'No delegate container of type "%s" is configured',
202+
$class
203+
));
204+
}
205+
192206
/**
193207
* @throws ContainerExceptionInterface
194208
* @throws NotFoundExceptionInterface

‎src/Definition/Definition.php‎

Lines changed: 29 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -153,6 +153,12 @@ public function resolveNew(): mixed
153153
{
154154
$concrete = $this->concrete;
155155

156+
try {
157+
$container = $this->getContainer();
158+
} catch (ContainerException) {
159+
$container = null;
160+
}
161+
156162
if (is_callable($concrete)) {
157163
$concrete = $this->resolveCallable($concrete);
158164
}
@@ -166,31 +172,31 @@ public function resolveNew(): mixed
166172
$concrete = $concrete->getValue();
167173
}
168174

169-
if (is_string($concrete) && class_exists($concrete)) {
175+
if (is_string($concrete) && $concrete !== $this->getId()) {
176+
if ($container instanceof ContainerInterface && $container->has($concrete)) {
177+
$concrete = $container->get($concrete);
178+
} elseif (class_exists($concrete)) {
179+
$concrete = $this->resolveClass($concrete);
180+
}
181+
} elseif (is_string($concrete) && class_exists($concrete)) {
170182
$concrete = $this->resolveClass($concrete);
171183
}
172184

173185
if (is_object($concrete)) {
174186
$concrete = $this->invokeMethods($concrete);
175187
}
176188

177-
try {
178-
$container = $this->getContainer();
179-
} catch (ContainerException) {
180-
$container = null;
181-
}
182-
183189
if (is_string($concrete)) {
184-
if (class_exists($concrete)) {
190+
if ($concrete !== $this->getId() && $container instanceof ContainerInterface && $container->has($concrete)) {
191+
$concrete = $container->get($concrete);
192+
} elseif (class_exists($concrete)) {
185193
$concrete = $this->resolveClass($concrete);
186194
} elseif ($this->getAlias() === $concrete) {
187195
return $concrete;
188196
}
189197
}
190198

191-
// if we still have a string, try to pull it from the container
192-
// this allows for `alias -> alias -> ... -> concrete
193-
if (is_string($concrete) && $container instanceof ContainerInterface && $container->has($concrete)) {
199+
if (is_string($concrete) && $concrete !== $this->getId() && $container instanceof ContainerInterface && $container->has($concrete)) {
194200
$this->recursiveCheck[] = $concrete;
195201
$concrete = $container->get($concrete);
196202
}
@@ -219,7 +225,18 @@ protected function resolveClass(string $concrete): object
219225
{
220226
$resolved = $this->resolveArguments($this->arguments);
221227
$reflection = new ReflectionClass($concrete);
222-
return $reflection->newInstanceArgs($resolved);
228+
229+
try {
230+
return $reflection->newInstanceArgs($resolved);
231+
} catch (\ArgumentCountError $e) {
232+
throw new ContainerException(sprintf(
233+
'Class "%s" was registered as a definition but its constructor has '
234+
. 'unsatisfied dependencies. Either provide arguments using '
235+
. '->addArgument(), use a callable to construct the class, or remove '
236+
. 'the explicit registration to allow autowiring via a delegate container.',
237+
$concrete,
238+
), 0, $e);
239+
}
223240
}
224241

225242
/**

‎tests/Asset/FooWithRequiredDependency.php‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace League\Container\Test\Asset;
6+
7+
class FooWithRequiredDependency
8+
{
9+
public function __construct(public readonly Bar $bar)
10+
{
11+
}
12+
}

‎tests/ContainerTest.php‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -322,4 +322,23 @@ public function testDefaultOverwrite(): void
322322
$this->assertSame($concreteTwo, $container->get('foo'));
323323
$this->assertNotSame($concreteOne, $container->get('foo'));
324324
}
325+
326+
public function testGetDelegateReturnsMatchingDelegate(): void
327+
{
328+
$container = new Container();
329+
$delegate = new ReflectionContainer();
330+
$container->delegate($delegate);
331+
332+
$this->assertSame($delegate, $container->getDelegate(ReflectionContainer::class));
333+
}
334+
335+
public function testGetDelegateThrowsWhenNoDelegateOfTypeExists(): void
336+
{
337+
$container = new Container();
338+
339+
$this->expectException(NotFoundException::class);
340+
$this->expectExceptionMessage('No delegate container of type');
341+
342+
$container->getDelegate(ReflectionContainer::class);
343+
}
325344
}

‎tests/Definition/DefinitionTest.php‎

Lines changed: 84 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,10 @@
99
use League\Container\Container;
1010
use League\Container\Definition\Definition;
1111
use League\Container\Test\Asset\Bar;
12+
use League\Container\Test\Asset\BarInterface;
1213
use League\Container\Test\Asset\Foo;
1314
use League\Container\Test\Asset\FooCallable;
15+
use League\Container\Test\Asset\FooWithRequiredDependency;
1416
use PHPUnit\Framework\TestCase;
1517
use Psr\Container\ContainerExceptionInterface;
1618
use Psr\Container\NotFoundExceptionInterface;
@@ -85,7 +87,10 @@ public function testDefinitionResolvesClassWithMethodCalls(): void
8587
$container = $this->getMockBuilder(Container::class)->getMock();
8688
$bar = new Bar();
8789

88-
$container->expects($this->once())->method('has')->with($this->equalTo(Bar::class))->willReturn(true);
90+
$container->method('has')->willReturnMap([
91+
[Foo::class, false],
92+
[Bar::class, true],
93+
]);
8994
$container->expects($this->once())->method('get')->with($this->equalTo(Bar::class))->willReturn($bar);
9095

9196
$definition = new Definition('callable', Foo::class);
@@ -108,7 +113,10 @@ public function testDefinitionResolvesClassWithDefinedArgs(): void
108113
$container = $this->getMockBuilder(Container::class)->getMock();
109114
$bar = new Bar();
110115

111-
$container->expects($this->once())->method('has')->with($this->equalTo(Bar::class))->willReturn(true);
116+
$container->method('has')->willReturnMap([
117+
[Foo::class, false],
118+
[Bar::class, true],
119+
]);
112120
$container->expects($this->once())->method('get')->with($this->equalTo(Bar::class))->willReturn($bar);
113121

114122
$definition = new Definition('callable', Foo::class);
@@ -195,4 +203,78 @@ public function testNonExistentClassIsReturnedAsIdenticalString(): void
195203
self::assertSame($nonExistent, $definition->getAlias());
196204
self::assertSame($nonExistent, $definition->resolve());
197205
}
206+
207+
/**
208+
* @throws ReflectionException
209+
* @throws ContainerExceptionInterface
210+
* @throws NotFoundExceptionInterface
211+
*/
212+
public function testDefinitionDelegatesToContainerForDifferentConcrete(): void
213+
{
214+
$container = $this->getMockBuilder(Container::class)->getMock();
215+
$bar = new Bar();
216+
217+
$container->expects($this->once())->method('has')->with($this->equalTo(Bar::class))->willReturn(true);
218+
$container->expects($this->once())->method('get')->with($this->equalTo(Bar::class))->willReturn($bar);
219+
220+
$definition = new Definition(BarInterface::class, Bar::class);
221+
$definition->setContainer($container);
222+
223+
$actual = $definition->resolveNew();
224+
225+
$this->assertInstanceOf(Bar::class, $actual);
226+
$this->assertSame($bar, $actual);
227+
}
228+
229+
/**
230+
* @throws ReflectionException
231+
* @throws ContainerExceptionInterface
232+
* @throws NotFoundExceptionInterface
233+
*/
234+
public function testDefinitionResolvesOwnClassWhenConcreteMatchesId(): void
235+
{
236+
$container = $this->getMockBuilder(Container::class)->getMock();
237+
238+
$container->expects($this->never())->method('has');
239+
$container->expects($this->never())->method('get');
240+
241+
$definition = new Definition(Foo::class, Foo::class);
242+
$definition->setContainer($container);
243+
244+
$actual = $definition->resolveNew();
245+
246+
$this->assertInstanceOf(Foo::class, $actual);
247+
}
248+
249+
/**
250+
* @throws ReflectionException
251+
* @throws ContainerExceptionInterface
252+
* @throws NotFoundExceptionInterface
253+
*/
254+
public function testDefinitionDelegatesToContainerWhenConcreteComesFromResolvableArgument(): void
255+
{
256+
$container = $this->getMockBuilder(Container::class)->getMock();
257+
$bar = new Bar();
258+
259+
$container->expects($this->once())->method('has')->with($this->equalTo(Bar::class))->willReturn(true);
260+
$container->expects($this->once())->method('get')->with($this->equalTo(Bar::class))->willReturn($bar);
261+
262+
$definition = new Definition(BarInterface::class, new ResolvableArgument(Bar::class));
263+
$definition->setContainer($container);
264+
265+
$actual = $definition->resolveNew();
266+
267+
$this->assertInstanceOf(Bar::class, $actual);
268+
$this->assertSame($bar, $actual);
269+
}
270+
271+
public function testResolveClassThrowsContainerExceptionForUnsatisfiedDependencies(): void
272+
{
273+
$definition = new Definition(FooWithRequiredDependency::class);
274+
275+
$this->expectException(ContainerExceptionInterface::class);
276+
$this->expectExceptionMessage('unsatisfied dependencies');
277+
278+
$definition->resolveNew();
279+
}
198280
}

0 commit comments

Comments
 (0)