Skip to content

Commit

Permalink
Remove duplicate delgators for all services
Browse files Browse the repository at this point in the history
mergeDelegators' documentation says that it merges delegators avoiding
multiple same delegators for the same service. However, from
experimenting with it, duplicates were allowed, as best as I could tell.
So, I've created this small patch to ensure that duplicates are removed,
along with a covering test for it to verify the change.

Signed-off-by: Matthew Setter <[email protected]>
  • Loading branch information
settermjd committed Dec 13, 2024
1 parent 340409a commit ed56555
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 2 deletions.
3 changes: 1 addition & 2 deletions src/ServiceManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -705,8 +705,7 @@ private function mergeDelegators(array $config): array
{
foreach ($config as $key => $delegators) {
if (! array_key_exists($key, $this->delegators)) {
$this->delegators[$key] = $delegators;
continue;
$this->delegators[$key] = [];
}

foreach ($delegators as $delegator) {
Expand Down
25 changes: 25 additions & 0 deletions test/ServiceManagerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
namespace LaminasTest\ServiceManager;

use DateTime;
use Laminas\ContainerConfigTest\TestAsset\DelegatorFactory;
use Laminas\ServiceManager\Factory\AbstractFactoryInterface;
use Laminas\ServiceManager\Factory\FactoryInterface;
use Laminas\ServiceManager\Factory\InvokableFactory;
Expand Down Expand Up @@ -372,6 +373,30 @@ public function testFactoryMayBeStaticMethodDescribedByCallableString(): void
self::assertEquals(stdClass::class, $serviceManager->get(stdClass::class)::class);
}

public function testDuplicateDelegatorsAreRemoved(): void
{
$dependencies = [
'delegators' => [
DateTime::class => [
DelegatorFactory::class,
DelegatorFactory::class,
],
],
];
$serviceManager = new ServiceManager($dependencies);
$property = new ReflectionProperty(ServiceManager::class, "delegators");
$property->setAccessible(true);

Check failure on line 388 in test/ServiceManagerTest.php

View workflow job for this annotation

GitHub Actions / ci / QA Checks (Psalm [8.1, locked], ubuntu-latest, laminas/laminas-continuous-integration-action@v1, ...

UnusedMethodCall

test/ServiceManagerTest.php:388:20: UnusedMethodCall: The call to ReflectionProperty::setAccessible is not used (see https://psalm.dev/209)
$delegators = $property->getValue($serviceManager);
self::assertSame(
[
DateTime::class => [
DelegatorFactory::class,
],
],
$delegators
);
}

public function testResolvedAliasFromAbstractFactory(): void
{
$abstractFactory = $this->createMock(AbstractFactoryInterface::class);
Expand Down

0 comments on commit ed56555

Please sign in to comment.