Skip to content

Commit

Permalink
Merge pull request #286 from InvisibleSmiley/test-284
Browse files Browse the repository at this point in the history
Infinite loop when fetching some cache-related services via container in projects without registered `config` service
  • Loading branch information
boesing authored Jan 19, 2024
2 parents 66cf9a2 + 734d78e commit bf8bc7f
Show file tree
Hide file tree
Showing 6 changed files with 132 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,11 @@ return [
];
```
The factory `Laminas\Cache\Service\StorageCacheAbstractServiceFactory` uses the configuration, searches for the configuration key `caches` and creates the storage adapters using the discovered configuration.
The factory `Laminas\Cache\Service\StorageCacheAbstractServiceFactory` uses the configuration, searches for the configuration key `caches` and creates the storage adapters using the discovered configuration.
WARNING: **Cache Named `config` Is Not Possible**
A cache named `config` is not possible due to internal service conflicts with MVC configuration.
The service named `config` is reserved for project configuration and thus cannot be used with the `caches` configuration.
## Create Controller
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,21 @@ final class DeprecatedStorageFactoryConfigurationCheckCommandFactory
{
public function __invoke(ContainerInterface $container): DeprecatedStorageFactoryConfigurationCheckCommand
{
$config = $this->detectConfigFromContainer($container);

$schemaDetector = new DeprecatedSchemaDetector();
return new DeprecatedStorageFactoryConfigurationCheckCommand(
$config,
$schemaDetector
);
}

private function detectConfigFromContainer(ContainerInterface $container): ArrayAccess
{
if (! $container->has('config')) {
return new ArrayObject([]);
}

$config = $container->get('config');
if (is_array($config)) {
$config = new ArrayObject($config);
Expand All @@ -28,10 +43,6 @@ public function __invoke(ContainerInterface $container): DeprecatedStorageFactor
throw new RuntimeException('Configuration from container must be either `ArrayAccess` or an array.');
}

$schemaDetector = new DeprecatedSchemaDetector();
return new DeprecatedStorageFactoryConfigurationCheckCommand(
$config,
$schemaDetector
);
return $config;
}
}
6 changes: 5 additions & 1 deletion src/ConfigProvider.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,14 @@
use Laminas\Cache\Service\StoragePluginFactory;
use Laminas\Cache\Service\StoragePluginFactoryFactory;
use Laminas\Cache\Service\StoragePluginFactoryInterface;
use Laminas\ServiceManager\ServiceManager;
use Symfony\Component\Console\Command\Command;

use function class_exists;

/**
* @psalm-import-type ServiceManagerConfiguration from ServiceManager
*/
class ConfigProvider
{
public const ADAPTER_PLUGIN_MANAGER_CONFIGURATION_KEY = 'storage_adapters';
Expand All @@ -34,7 +38,7 @@ public function __invoke()
/**
* Return default service mappings for laminas-cache.
*
* @return array
* @return ServiceManagerConfiguration
*/
public function getDependencyConfig()
{
Expand Down
7 changes: 6 additions & 1 deletion src/Service/StorageCacheAbstractServiceFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@
*/
class StorageCacheAbstractServiceFactory implements AbstractFactoryInterface
{
public const CACHES_CONFIGURATION_KEY = 'caches';
public const CACHES_CONFIGURATION_KEY = 'caches';
private const RESERVED_CONFIG_SERVICE_NAME = 'config';

/** @var array<string,mixed>|null */
protected $config;
Expand All @@ -32,6 +33,10 @@ class StorageCacheAbstractServiceFactory implements AbstractFactoryInterface
*/
public function canCreate(ContainerInterface $container, $requestedName)
{
if ($requestedName === self::RESERVED_CONFIG_SERVICE_NAME) {
return false;
}

$config = $this->getConfig($container);
if (empty($config)) {
return false;
Expand Down
92 changes: 92 additions & 0 deletions test/Service/ConfigProviderIntegrationTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
<?php

declare(strict_types=1);

namespace LaminasTest\Cache\Service;

use Generator;
use InvalidArgumentException;
use Laminas\Cache\ConfigProvider;
use Laminas\ServiceManager\ServiceManager;
use PHPUnit\Framework\TestCase;
use Psr\Container\ContainerInterface;

use function array_keys;
use function array_merge;
use function array_unique;
use function is_array;
use function is_string;

final class ConfigProviderIntegrationTest extends TestCase
{
private ContainerInterface $container;

protected function setUp(): void
{
parent::setUp();
$this->container = $this->createContainer();
}

private function createContainer(): ContainerInterface
{
return new ServiceManager((new ConfigProvider())->getDependencyConfig());
}

/**
* @dataProvider servicesProvidedByConfigProvider
*/
public function testContainerCanProvideRegisteredServices(string $serviceName): void
{
$instance = $this->container->get($serviceName);
self::assertIsObject($instance);
}

/**
* @return Generator<string, array{string}>
*/
public function servicesProvidedByConfigProvider(): Generator
{
$provider = new ConfigProvider();
$dependencies = $provider->getDependencyConfig();

$factories = $dependencies['factories'] ?? [];
self::assertArrayIsMappedWithStrings($factories);
$invokables = $dependencies['invokables'] ?? [];
self::assertArrayIsMappedWithStrings($invokables);
$services = $dependencies['services'] ?? [];
self::assertArrayIsMappedWithStrings($services);
$aliases = $dependencies['aliases'] ?? [];
self::assertArrayIsMappedWithStrings($aliases);

$serviceNames = array_unique(
array_merge(
array_keys($factories),
array_keys($invokables),
array_keys($services),
array_keys($aliases),
),
);

foreach ($serviceNames as $serviceName) {
yield $serviceName => [$serviceName];
}
}

/**
* @psalm-assert array<string,mixed> $array
*/
private static function assertArrayIsMappedWithStrings(mixed $array): void
{
if (! is_array($array)) {
throw new InvalidArgumentException('Expecting value to be an array.');
}

foreach (array_keys($array) as $value) {
if (is_string($value)) {
continue;
}

throw new InvalidArgumentException('Expecting all values to are mapped with a string.');
}
}
}
8 changes: 8 additions & 0 deletions test/Service/StorageCacheAbstractServiceFactoryTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,14 @@ public function testWillPassInvalidArgumentExceptionFromConfigurationValidityAss
($this->factory)($this->container, 'Foo');
}

public function testNeverCallsContainerWhenConfigServiceIsCheckedForCreation(): void
{
$container = $this->createMock(ContainerInterface::class);
$container->expects(self::never())->method(self::anything());

self::assertFalse($this->factory->canCreate($container, 'config'));
}

public function testInvalidCacheServiceNameWillBeIgnored(): void
{
self::assertFalse(
Expand Down

0 comments on commit bf8bc7f

Please sign in to comment.