Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Infinite loop when fetching some cache-related services via container in projects without registered config service #286

Merged
merged 11 commits into from
Jan 19, 2024
Merged
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,12 @@ 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.

> ### 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 he projects configuration and thus cannot be used with the `caches` configuration.
boesing marked this conversation as resolved.
Show resolved Hide resolved
boesing marked this conversation as resolved.
Show resolved Hide resolved
boesing marked this conversation as resolved.
Show resolved Hide resolved

## 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 PRESERVED_CONFIG_SERVICE_NAME = 'config';
boesing marked this conversation as resolved.
Show resolved Hide resolved

/** @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::PRESERVED_CONFIG_SERVICE_NAME) {
boesing marked this conversation as resolved.
Show resolved Hide resolved
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();
boesing marked this conversation as resolved.
Show resolved Hide resolved
$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::assertMappedWithStrings($factories);
$invokables = $dependencies['invokables'] ?? [];
self::assertMappedWithStrings($invokables);
$services = $dependencies['services'] ?? [];
self::assertMappedWithStrings($services);
$aliases = $dependencies['aliases'] ?? [];
self::assertMappedWithStrings($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> $iterable
*/
private static function assertMappedWithStrings(mixed $iterable): void
{
if (! is_array($iterable)) {
boesing marked this conversation as resolved.
Show resolved Hide resolved
throw new InvalidArgumentException('Expecting value to be iterable.');
}

foreach (array_keys($iterable) 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