From 9ae9f5c3e80b375a213a1b0dbfb5f46cde740da1 Mon Sep 17 00:00:00 2001 From: Jens Hatlak Date: Wed, 17 Jan 2024 12:23:08 +0100 Subject: [PATCH 01/11] Add regression test for issue 284 Signed-off-by: Jens Hatlak --- test/Storage/ContainerTest.php | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) create mode 100644 test/Storage/ContainerTest.php diff --git a/test/Storage/ContainerTest.php b/test/Storage/ContainerTest.php new file mode 100644 index 00000000..21f24f51 --- /dev/null +++ b/test/Storage/ContainerTest.php @@ -0,0 +1,26 @@ +getMergedConfig(); + $container = new ServiceManager($config['dependencies']); + + // Not calling $container->setService('config', $config) here on purpose + + $result = $container->get(StorageAdapterFactoryInterface::class); + self::assertInstanceOf(StorageAdapterFactoryInterface::class, $result); + } +} From d2b0e92a38b5084e5a76ce13a8171c2f6792150f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maximilian=20B=C3=B6sing?= <2189546+boesing@users.noreply.github.com> Date: Wed, 17 Jan 2024 15:08:44 +0100 Subject: [PATCH 02/11] qa: replace `ContainerTest` with `ConfigProviderIntegrationTest` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Maximilian Bösing <2189546+boesing@users.noreply.github.com> --- src/ConfigProvider.php | 6 +- .../Service/ConfigProviderIntegrationTest.php | 65 +++++++++++++++++++ test/Storage/ContainerTest.php | 26 -------- 3 files changed, 70 insertions(+), 27 deletions(-) create mode 100644 test/Service/ConfigProviderIntegrationTest.php delete mode 100644 test/Storage/ContainerTest.php diff --git a/src/ConfigProvider.php b/src/ConfigProvider.php index 19fddb36..ad4a4a50 100644 --- a/src/ConfigProvider.php +++ b/src/ConfigProvider.php @@ -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'; @@ -34,7 +38,7 @@ public function __invoke() /** * Return default service mappings for laminas-cache. * - * @return array + * @return ServiceManagerConfiguration */ public function getDependencyConfig() { diff --git a/test/Service/ConfigProviderIntegrationTest.php b/test/Service/ConfigProviderIntegrationTest.php new file mode 100644 index 00000000..46feafbd --- /dev/null +++ b/test/Service/ConfigProviderIntegrationTest.php @@ -0,0 +1,65 @@ +container = $this->createContainer(); + } + + private function createContainer(): ContainerInterface + { + return new ServiceManager((new ConfigProvider())->getDependencyConfig()); + } + + /** + * @param non-empty-string $serviceName + * @dataProvider servicesProvidedByConfigProvider + */ + public function testContainerCanProvideRegisteredServices(string $serviceName): void + { + $instance = $this->container->get($serviceName); + self::assertIsObject($instance); + } + + /** + * @return iterable + */ + public function servicesProvidedByConfigProvider(): iterable + { + $provider = new ConfigProvider(); + $dependencies = $provider->getDependencyConfig(); + + $serviceNames = array_unique( + array_merge( + array_keys($dependencies['factories'] ?? []), + array_keys($dependencies['invokables'] ?? []), + array_keys($dependencies['services'] ?? []), + array_keys($dependencies['aliases'] ?? []), + ), + ); + + foreach ($serviceNames as $serviceName) { + assert(is_string($serviceName) && $serviceName !== ''); + yield $serviceName => [$serviceName]; + } + } +} diff --git a/test/Storage/ContainerTest.php b/test/Storage/ContainerTest.php deleted file mode 100644 index 21f24f51..00000000 --- a/test/Storage/ContainerTest.php +++ /dev/null @@ -1,26 +0,0 @@ -getMergedConfig(); - $container = new ServiceManager($config['dependencies']); - - // Not calling $container->setService('config', $config) here on purpose - - $result = $container->get(StorageAdapterFactoryInterface::class); - self::assertInstanceOf(StorageAdapterFactoryInterface::class, $result); - } -} From 77e351a71e3c615453fd506b0bfc6d8831f4d3d9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maximilian=20B=C3=B6sing?= <2189546+boesing@users.noreply.github.com> Date: Wed, 17 Jan 2024 15:28:10 +0100 Subject: [PATCH 03/11] qa: optimize config provider integration test for better type-inference MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Maximilian Bösing <2189546+boesing@users.noreply.github.com> --- .../Service/ConfigProviderIntegrationTest.php | 45 +++++++++++++++---- 1 file changed, 36 insertions(+), 9 deletions(-) diff --git a/test/Service/ConfigProviderIntegrationTest.php b/test/Service/ConfigProviderIntegrationTest.php index 46feafbd..ee97642d 100644 --- a/test/Service/ConfigProviderIntegrationTest.php +++ b/test/Service/ConfigProviderIntegrationTest.php @@ -4,6 +4,8 @@ namespace LaminasTest\Cache\Service; +use Generator; +use InvalidArgumentException; use Laminas\Cache\ConfigProvider; use Laminas\ServiceManager\ServiceManager; use PHPUnit\Framework\TestCase; @@ -12,7 +14,7 @@ use function array_keys; use function array_merge; use function array_unique; -use function assert; +use function is_array; use function is_string; final class ConfigProviderIntegrationTest extends TestCase @@ -31,7 +33,6 @@ private function createContainer(): ContainerInterface } /** - * @param non-empty-string $serviceName * @dataProvider servicesProvidedByConfigProvider */ public function testContainerCanProvideRegisteredServices(string $serviceName): void @@ -41,25 +42,51 @@ public function testContainerCanProvideRegisteredServices(string $serviceName): } /** - * @return iterable + * @return Generator */ - public function servicesProvidedByConfigProvider(): iterable + 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($dependencies['factories'] ?? []), - array_keys($dependencies['invokables'] ?? []), - array_keys($dependencies['services'] ?? []), - array_keys($dependencies['aliases'] ?? []), + array_keys($factories), + array_keys($invokables), + array_keys($services), + array_keys($aliases), ), ); foreach ($serviceNames as $serviceName) { - assert(is_string($serviceName) && $serviceName !== ''); yield $serviceName => [$serviceName]; } } + + /** + * @psalm-assert array $iterable + */ + private static function assertMappedWithStrings(mixed $iterable): void + { + if (! is_array($iterable)) { + 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.'); + } + } } From e47446394f3fbe72b4374177aff1f434cecbf22d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maximilian=20B=C3=B6sing?= <2189546+boesing@users.noreply.github.com> Date: Wed, 17 Jan 2024 15:28:58 +0100 Subject: [PATCH 04/11] bugfix: allow deprecated storage factory configuration check to operate on projects without project configuration MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Maximilian Bösing <2189546+boesing@users.noreply.github.com> --- ...actoryConfigurationCheckCommandFactory.php | 21 ++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/src/Command/DeprecatedStorageFactoryConfigurationCheckCommandFactory.php b/src/Command/DeprecatedStorageFactoryConfigurationCheckCommandFactory.php index 73bdef3e..e605eed4 100644 --- a/src/Command/DeprecatedStorageFactoryConfigurationCheckCommandFactory.php +++ b/src/Command/DeprecatedStorageFactoryConfigurationCheckCommandFactory.php @@ -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); @@ -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; } } From 766573ca911cbea628dbb49da98fa67a23a8a8c4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maximilian=20B=C3=B6sing?= <2189546+boesing@users.noreply.github.com> Date: Wed, 17 Jan 2024 15:30:10 +0100 Subject: [PATCH 05/11] bugfix: preserve `config` cache name MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This fixes an infinite loop in combination with `laminas-servicemanager` as outlined in #284 Signed-off-by: Maximilian Bösing <2189546+boesing@users.noreply.github.com> --- .../usage-in-a-laminas-mvc-application.md | 7 ++++++- src/Service/StorageCacheAbstractServiceFactory.php | 7 ++++++- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/docs/book/v3/application-integration/usage-in-a-laminas-mvc-application.md b/docs/book/v3/application-integration/usage-in-a-laminas-mvc-application.md index 68712387..b9d1a473 100644 --- a/docs/book/v3/application-integration/usage-in-a-laminas-mvc-application.md +++ b/docs/book/v3/application-integration/usage-in-a-laminas-mvc-application.md @@ -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. ## Create Controller diff --git a/src/Service/StorageCacheAbstractServiceFactory.php b/src/Service/StorageCacheAbstractServiceFactory.php index 7cbc8ef2..89aaa5fc 100644 --- a/src/Service/StorageCacheAbstractServiceFactory.php +++ b/src/Service/StorageCacheAbstractServiceFactory.php @@ -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'; /** @var array|null */ protected $config; @@ -32,6 +33,10 @@ class StorageCacheAbstractServiceFactory implements AbstractFactoryInterface */ public function canCreate(ContainerInterface $container, $requestedName) { + if ($requestedName === self::PRESERVED_CONFIG_SERVICE_NAME) { + return false; + } + $config = $this->getConfig($container); if (empty($config)) { return false; From 411151ef9e2f3cd2bbabcc37bd9801ee5a4eda7d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maximilian=20B=C3=B6sing?= <2189546+boesing@users.noreply.github.com> Date: Wed, 17 Jan 2024 15:39:15 +0100 Subject: [PATCH 06/11] qa: add unit test to verify that for the `config` service, `canCreate` method is not calling container MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Maximilian Bösing <2189546+boesing@users.noreply.github.com> --- test/Service/StorageCacheAbstractServiceFactoryTest.php | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/test/Service/StorageCacheAbstractServiceFactoryTest.php b/test/Service/StorageCacheAbstractServiceFactoryTest.php index daad0607..f0542de3 100644 --- a/test/Service/StorageCacheAbstractServiceFactoryTest.php +++ b/test/Service/StorageCacheAbstractServiceFactoryTest.php @@ -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( From 264361533c233ccb8a5b4a3a342baebc55dad6ed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maximilian=20B=C3=B6sing?= <2189546+boesing@users.noreply.github.com> Date: Wed, 17 Jan 2024 15:42:29 +0100 Subject: [PATCH 07/11] qa: rephrase exception message MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Maximilian Bösing <2189546+boesing@users.noreply.github.com> --- test/Service/ConfigProviderIntegrationTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/Service/ConfigProviderIntegrationTest.php b/test/Service/ConfigProviderIntegrationTest.php index ee97642d..3ea15227 100644 --- a/test/Service/ConfigProviderIntegrationTest.php +++ b/test/Service/ConfigProviderIntegrationTest.php @@ -78,7 +78,7 @@ public function servicesProvidedByConfigProvider(): Generator private static function assertMappedWithStrings(mixed $iterable): void { if (! is_array($iterable)) { - throw new InvalidArgumentException('Expecting value to be iterable.'); + throw new InvalidArgumentException('Expecting value to be an array.'); } foreach (array_keys($iterable) as $value) { From 0872a58a8a8491f3dfbf16233d06e3afc15010fa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maximilian=20B=C3=B6sing?= <2189546+boesing@users.noreply.github.com> Date: Wed, 17 Jan 2024 15:43:58 +0100 Subject: [PATCH 08/11] docs: fix some typo regarding project configuration MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Maximilian Bösing <2189546+boesing@users.noreply.github.com> --- .../usage-in-a-laminas-mvc-application.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/book/v3/application-integration/usage-in-a-laminas-mvc-application.md b/docs/book/v3/application-integration/usage-in-a-laminas-mvc-application.md index b9d1a473..607805e0 100644 --- a/docs/book/v3/application-integration/usage-in-a-laminas-mvc-application.md +++ b/docs/book/v3/application-integration/usage-in-a-laminas-mvc-application.md @@ -42,7 +42,7 @@ The factory `Laminas\Cache\Service\StorageCacheAbstractServiceFactory` uses the > ### 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. +> The service named `config` is reserved for project configuration and thus cannot be used with the `caches` configuration. ## Create Controller From f965101816e5c56524caad3a7065217996214aee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maximilian=20B=C3=B6sing?= <2189546+boesing@users.noreply.github.com> Date: Wed, 17 Jan 2024 15:45:26 +0100 Subject: [PATCH 09/11] qa: fix type, there is a difference between `preserved` and `reserved` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Maximilian Bösing <2189546+boesing@users.noreply.github.com> --- src/Service/StorageCacheAbstractServiceFactory.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Service/StorageCacheAbstractServiceFactory.php b/src/Service/StorageCacheAbstractServiceFactory.php index 89aaa5fc..a1b8e24a 100644 --- a/src/Service/StorageCacheAbstractServiceFactory.php +++ b/src/Service/StorageCacheAbstractServiceFactory.php @@ -14,8 +14,8 @@ */ class StorageCacheAbstractServiceFactory implements AbstractFactoryInterface { - public const CACHES_CONFIGURATION_KEY = 'caches'; - private const PRESERVED_CONFIG_SERVICE_NAME = 'config'; + public const CACHES_CONFIGURATION_KEY = 'caches'; + private const RESERVED_CONFIG_SERVICE_NAME = 'config'; /** @var array|null */ protected $config; @@ -33,7 +33,7 @@ class StorageCacheAbstractServiceFactory implements AbstractFactoryInterface */ public function canCreate(ContainerInterface $container, $requestedName) { - if ($requestedName === self::PRESERVED_CONFIG_SERVICE_NAME) { + if ($requestedName === self::RESERVED_CONFIG_SERVICE_NAME) { return false; } From bc01ee85b902c4804eef1b61a05091a3976012e1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maximilian=20B=C3=B6sing?= <2189546+boesing@users.noreply.github.com> Date: Wed, 17 Jan 2024 15:51:49 +0100 Subject: [PATCH 10/11] qa: rename assertion method and its argument to reflect actual assertion MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Maximilian Bösing <2189546+boesing@users.noreply.github.com> --- test/Service/ConfigProviderIntegrationTest.php | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/test/Service/ConfigProviderIntegrationTest.php b/test/Service/ConfigProviderIntegrationTest.php index 3ea15227..518a04c0 100644 --- a/test/Service/ConfigProviderIntegrationTest.php +++ b/test/Service/ConfigProviderIntegrationTest.php @@ -50,13 +50,13 @@ public function servicesProvidedByConfigProvider(): Generator $dependencies = $provider->getDependencyConfig(); $factories = $dependencies['factories'] ?? []; - self::assertMappedWithStrings($factories); + self::assertArrayIsMappedWithStrings($factories); $invokables = $dependencies['invokables'] ?? []; - self::assertMappedWithStrings($invokables); + self::assertArrayIsMappedWithStrings($invokables); $services = $dependencies['services'] ?? []; - self::assertMappedWithStrings($services); + self::assertArrayIsMappedWithStrings($services); $aliases = $dependencies['aliases'] ?? []; - self::assertMappedWithStrings($aliases); + self::assertArrayIsMappedWithStrings($aliases); $serviceNames = array_unique( array_merge( @@ -73,15 +73,15 @@ public function servicesProvidedByConfigProvider(): Generator } /** - * @psalm-assert array $iterable + * @psalm-assert array $array */ - private static function assertMappedWithStrings(mixed $iterable): void + private static function assertArrayIsMappedWithStrings(mixed $array): void { - if (! is_array($iterable)) { + if (! is_array($array)) { throw new InvalidArgumentException('Expecting value to be an array.'); } - foreach (array_keys($iterable) as $value) { + foreach (array_keys($array) as $value) { if (is_string($value)) { continue; } From 734d78efd4a8ff2923b62141f80d04152004f2c2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maximilian=20B=C3=B6sing?= <2189546+boesing@users.noreply.github.com> Date: Wed, 17 Jan 2024 15:53:18 +0100 Subject: [PATCH 11/11] docs: use markdown admonitions/callouts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Maximilian Bösing <2189546+boesing@users.noreply.github.com> --- .../usage-in-a-laminas-mvc-application.md | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/docs/book/v3/application-integration/usage-in-a-laminas-mvc-application.md b/docs/book/v3/application-integration/usage-in-a-laminas-mvc-application.md index 607805e0..0652f774 100644 --- a/docs/book/v3/application-integration/usage-in-a-laminas-mvc-application.md +++ b/docs/book/v3/application-integration/usage-in-a-laminas-mvc-application.md @@ -39,10 +39,9 @@ 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. -> ### 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. +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