Skip to content

Commit

Permalink
Merge pull request #183 from snapshotpl/missing-adapter-key-support
Browse files Browse the repository at this point in the history
Revert changes made for `adapter` name from `StoragePluginFactoryInterface` and apply to `StorageAdapterFactoryInterface` instead
  • Loading branch information
boesing authored Nov 18, 2021
2 parents d021354 + 6746130 commit c687116
Show file tree
Hide file tree
Showing 6 changed files with 42 additions and 35 deletions.
3 changes: 2 additions & 1 deletion src/Service/StorageAdapterFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,8 @@ public function __construct(PluginManagerInterface $adapters, StoragePluginFacto

public function createFromArrayConfiguration(array $configuration): StorageInterface
{
$adapterName = $configuration['name'];
$adapterName = $configuration['adapter'] ?? $configuration['name'] ?? null;
Assert::stringNotEmpty($adapterName, 'Configuration must contain a "adapter" key.');
$adapterOptions = $configuration['options'] ?? [];
$plugins = $configuration['plugins'] ?? [];

Expand Down
4 changes: 4 additions & 0 deletions src/Service/StorageAdapterFactoryInterface.php
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,10 @@
* name:non-empty-string,
* options?:array<string,mixed>,
* plugins?: list<PluginArrayConfigurationWithPriorityType>
* }|array{
* adapter:non-empty-string,
* options?:array<string,mixed>,
* plugins?: list<PluginArrayConfigurationWithPriorityType>
* }
*/
interface StorageAdapterFactoryInterface
Expand Down
14 changes: 3 additions & 11 deletions src/Service/StoragePluginFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,7 @@ public function __construct(PluginManagerInterface $plugins)

public function createFromArrayConfiguration(array $configuration): PluginInterface
{
$name = $configuration['adapter'] ?? $configuration['name'] ?? null;
Assert::stringNotEmpty($name, 'Configuration must contain a "adapter" key.');
$name = $configuration['name'];
$options = $configuration['options'] ?? [];

return $this->create($name, $options);
Expand All @@ -42,15 +41,8 @@ public function assertValidConfigurationStructure(array $configuration): void
{
try {
Assert::isNonEmptyMap($configuration, 'Configuration must be a non-empty array.');
if (! isset($configuration['name']) && ! isset($configuration['adapter'])) {
throw new PhpInvalidArgumentException('Configuration must contain a "adapter" key.');
}

Assert::stringNotEmpty(
$configuration['adapter'] ?? $configuration['name'],
'Plugin "adapter" has to be a non-empty string.'
);

Assert::keyExists($configuration, 'name', 'Configuration must contain a "name" key.');
Assert::stringNotEmpty($configuration['name'], 'Plugin "name" has to be a non-empty string.');
Assert::nullOrIsMap(
$configuration['options'] ?? null,
'Plugin "options" must be an array with string keys.'
Expand Down
2 changes: 1 addition & 1 deletion src/Service/StoragePluginFactoryInterface.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
use Laminas\Cache\Storage\Plugin\PluginInterface;

/**
* @psalm-type PluginArrayConfigurationType = array{name:non-empty-string,options?:array<string,mixed>}|array{adapter:non-empty-string,options?:array<string,mixed>}
* @psalm-type PluginArrayConfigurationType = array{name:non-empty-string,options?:array<string,mixed>}
*/
interface StoragePluginFactoryInterface
{
Expand Down
24 changes: 24 additions & 0 deletions test/Service/StorageAdapterFactoryTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,30 @@ public function testWillCreateStorageFromArrayConfiguration(
self::assertSame($adapterMock, $adapter);
}

/**
* @psalm-param non-empty-string $adapterName
* @param array<string,mixed> $adapterConfiguration
* @dataProvider storageConfigurations
*/
public function testWillCreateStorageFromArrayConfigurationAndAdapterKey(
string $adapterName,
array $adapterConfiguration
): void {
$adapterMock = $this->createMock(AbstractAdapter::class);
$this->adapters
->expects(self::once())
->method('build')
->with($adapterName, $adapterConfiguration)
->willReturn($adapterMock);

$adapter = $this->factory->createFromArrayConfiguration([
'adapter' => $adapterName,
'options' => $adapterConfiguration,
]);

self::assertSame($adapterMock, $adapter);
}

/**
* @psalm-param list<PluginArrayConfigurationWithPriorityType> $plugins
* @dataProvider pluginConfigurations
Expand Down
30 changes: 8 additions & 22 deletions test/Service/StoragePluginFactoryTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -37,18 +37,18 @@ public function invalidConfigurations(): Generator
'Configuration must be a non-empty array',
];

yield 'missing adapter' => [
yield 'missing name' => [
['options' => []],
'Configuration must contain a "adapter" key',
'Configuration must contain a "name" key',
];

yield 'empty adapter name' => [
['adapter' => ''],
'Plugin "adapter" has to be a non-empty string',
yield 'empty name' => [
['name' => ''],
'Plugin "name" has to be a non-empty string',
];

yield 'invalid options' => [
['adapter' => 'foo', 'options' => 'bar'],
['name' => 'foo', 'options' => 'bar'],
'Plugin "options" must be an array with string keys',
];
}
Expand All @@ -57,20 +57,6 @@ public function testWillCreatePluginFromArrayConfiguration(): void
{
$plugin = $this->createMock(PluginInterface::class);

$this->plugins
->expects(self::once())
->method('build')
->with('foo')
->willReturn($plugin);

$createdPlugin = $this->factory->createFromArrayConfiguration(['adapter' => 'foo']);
self::assertSame($plugin, $createdPlugin);
}

public function testWillCreatePluginFromDeprecatedArrayConfiguration(): void
{
$plugin = $this->createMock(PluginInterface::class);

$this->plugins
->expects(self::once())
->method('build')
Expand All @@ -92,7 +78,7 @@ public function testWillCreatePluginFromArrayConfigurationWithOptions(): void
->willReturn($plugin);

$createdPlugin = $this->factory->createFromArrayConfiguration(
['adapter' => 'foo', 'options' => ['bar' => 'baz']]
['name' => 'foo', 'options' => ['bar' => 'baz']]
);

self::assertSame($plugin, $createdPlugin);
Expand Down Expand Up @@ -145,7 +131,7 @@ public function testWillAssertProperConfiguration(): void
{
$this->expectNotToPerformAssertions();
$this->factory->assertValidConfigurationStructure([
'adapter' => 'foo',
'name' => 'foo',
'options' => ['bar' => 'baz'],
]);
}
Expand Down

0 comments on commit c687116

Please sign in to comment.