Skip to content

Commit

Permalink
feature: allow integer keys in iterables as per PSR integration tests
Browse files Browse the repository at this point in the history 
In the (non-official) PSR integration tests are checks for iterables with a non-empty-string cache key `0`. Internal PHP type juggling converts integerish strings to `int` which leads to a type conflict when passing these values to our internal assertion.
Therefore, this patch provides compatibility for integer keys in methods consuming iterable keys either in key-value or key-only combination.

Signed-off-by: Maximilian Bösing <2189546+boesing@users.noreply.github.com>
  • Loading branch information
@boesing
boesing committed Jun 15, 2024
1 parent ba823c8 commit cab2853
Show file tree
Hide file tree
Showing 4 changed files with 56 additions and 30 deletions.
27 changes: 20 additions & 7 deletions src/Storage/Adapter/AbstractAdapter.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
use function array_values;
use function func_num_args;
use function is_array;
use function is_int;
use function is_string;
use function preg_match;
use function sprintf;
Expand Down Expand Up @@ -625,7 +626,7 @@ public function setItems(array $keyValuePairs): array
}

Assert::isList($result);
Assert::allStringNotEmpty($result);
$this->assertValidKeys(array_keys($result));
return $result;
}

Expand Down Expand Up @@ -849,7 +850,7 @@ public function replaceItems(array $keyValuePairs): array
}

Assert::isList($result);
Assert::allStringNotEmpty($result);
$this->assertValidKeys(array_keys($result));
return $result;
}

Expand Down Expand Up @@ -1013,12 +1014,12 @@ public function touchItems(array $keys): array

$result = $this->triggerPost(__FUNCTION__, $args, $result);
Assert::isList($result);
Assert::allStringNotEmpty($result);
self::assertValidKeys(array_keys($result));
return $result;
} catch (Throwable $throwable) {
$result = $this->triggerThrowable(__FUNCTION__, $args, $keys, $throwable);
Assert::isList($result);
Assert::allStringNotEmpty($result);
self::assertValidKeys(array_keys($result));
return $result;
}
}
Expand Down Expand Up @@ -1235,13 +1236,13 @@ protected function normalizeKeyValuePairs(array $keyValuePairs): void
}

/**
* @psalm-assert non-empty-string $key
* @psalm-assert non-empty-string|int $key
*/
protected function assertValidKey(mixed $key): void
{
if (! is_string($key)) {
if (! is_int($key) && ! is_string($key)) {
throw new Exception\InvalidArgumentException(
"Key has to be string"
"Key has to be either string or int"
);
}

Expand All @@ -1251,6 +1252,8 @@ protected function assertValidKey(mixed $key): void
);
}

$key = (string) $key;

$pattern = $this->getOptions()->getKeyPattern();
if ($pattern !== '' && ! preg_match($pattern, $key)) {
throw new Exception\InvalidArgumentException(
Expand Down Expand Up @@ -1282,4 +1285,14 @@ protected function assertValidKeyValuePairs(mixed $keyValuePairs): void
$this->assertValidKey($key);
}
}

/**
* @psalm-assert list<non-empty-string|int> $keys
*/
private function assertValidKeys(array $keys): void
{
foreach ($keys as $key) {
$this->assertValidKey($key);
}
}
}
43 changes: 21 additions & 22 deletions src/Storage/StorageInterface.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,22 +4,20 @@

use Laminas\Cache\Exception\ExceptionInterface;

/**
* NOTE: when providing integrish cache keys in iterables, internal array conversion might convert these to int, even
* tho they were non-empty-string beforehand. See https://3v4l.org/GsiBl for more details.
*
* @psalm-type CacheKeyInIterableType = non-empty-string|int
*/
interface StorageInterface
{
/**
* Set options.
*/
public function setOptions(iterable|Adapter\AdapterOptions $options): self;

/**
* Get options
*/
public function getOptions(): Adapter\AdapterOptions;

/* reading */
/**
* Get an item.
*
* @param non-empty-string $key
* @param-out bool $success
* @return mixed Data on success, null on failure
Expand All @@ -30,8 +28,8 @@ public function getItem(string $key, bool|null &$success = null, mixed &$casToke
/**
* Get multiple items.
*
* @param non-empty-list<non-empty-string> $keys
* @return array<non-empty-string,mixed> Associative array of keys and values
* @param non-empty-list<CacheKeyInIterableType> $keys
* @return array<CacheKeyInIterableType,mixed> Associative array of keys and values
* @throws ExceptionInterface
*/
public function getItems(array $keys): array;
Expand All @@ -47,8 +45,8 @@ public function hasItem(string $key): bool;
/**
* Test multiple items.
*
* @param non-empty-list<non-empty-string> $keys
* @return list<non-empty-string> Array of found keys
* @param non-empty-list<CacheKeyInIterableType> $keys
* @return list<CacheKeyInIterableType> Array of found keys
* @throws ExceptionInterface
*/
public function hasItems(array $keys): array;
Expand All @@ -65,24 +63,25 @@ public function setItem(string $key, mixed $value): bool;
/**
* Store multiple items.
*
* @param non-empty-array<non-empty-string,mixed> $keyValuePairs
* @return list<non-empty-string> Array of not stored keys
* @param non-empty-array<CacheKeyInIterableType,mixed> $keyValuePairs
* @return list<CacheKeyInIterableType> Array of not stored keys
* @throws ExceptionInterface
*/
public function setItems(array $keyValuePairs): array;

/**
* Add an item.
*
* @param non-empty-string $key
* @throws ExceptionInterface
*/
public function addItem(string $key, mixed $value): bool;

/**
* Add multiple items.
*
* @param non-empty-array<non-empty-string,mixed> $keyValuePairs
* @return list<non-empty-string> Array of not stored keys
* @param non-empty-array<CacheKeyInIterableType,mixed> $keyValuePairs
* @return list<CacheKeyInIterableType> Array of not stored keys
* @throws ExceptionInterface
*/
public function addItems(array $keyValuePairs): array;
Expand All @@ -98,8 +97,8 @@ public function replaceItem(string $key, mixed $value): bool;
/**
* Replace multiple existing items.
*
* @param non-empty-array<non-empty-string,mixed> $keyValuePairs
* @return list<non-empty-string> Array of not stored keys
* @param non-empty-array<CacheKeyInIterableType,mixed> $keyValuePairs
* @return list<CacheKeyInIterableType> Array of not stored keys
* @throws ExceptionInterface
*/
public function replaceItems(array $keyValuePairs): array;
Expand Down Expand Up @@ -130,8 +129,8 @@ public function touchItem(string $key): bool;
/**
* Reset lifetime of multiple items.
*
* @param non-empty-list<non-empty-string> $keys
* @return list<non-empty-string> Array of not updated keys
* @param non-empty-list<CacheKeyInIterableType> $keys
* @return list<CacheKeyInIterableType> Array of not updated keys
* @throws ExceptionInterface
*/
public function touchItems(array $keys): array;
Expand All @@ -147,8 +146,8 @@ public function removeItem(string $key): bool;
/**
* Remove multiple items.
*
* @param non-empty-list<non-empty-string> $keys
* @return list<non-empty-string> Array of not removed keys
* @param non-empty-list<CacheKeyInIterableType> $keys
* @return list<CacheKeyInIterableType> Array of not removed keys
* @throws ExceptionInterface
*/
public function removeItems(array $keys): array;
Expand Down
15 changes: 14 additions & 1 deletion test/Storage/Adapter/AbstractAdapterTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
use ArrayObject;
use Laminas\Cache;
use Laminas\Cache\Exception;
use Laminas\Cache\Exception\InvalidArgumentException;
use Laminas\Cache\Exception\RuntimeException;
use Laminas\Cache\Storage\Adapter\AbstractAdapter;
use Laminas\Cache\Storage\Adapter\AdapterOptions;
Expand All @@ -19,6 +18,7 @@
use Laminas\EventManager\ResponseCollection;
use Laminas\Serializer\AdapterPluginManager;
use Laminas\ServiceManager\ServiceManager;
use LaminasTest\Cache\Storage\TestAsset\MockAdapter;
use LaminasTest\Cache\Storage\TestAsset\MockPlugin;
use PHPUnit\Framework\MockObject\MockObject;
use PHPUnit\Framework\TestCase;
Expand Down Expand Up @@ -801,4 +801,17 @@ public function testCanCompareOldValueWithTokenWhenUsedWithSerializerPlugin(): v

self::assertTrue($storage->checkAndSetItem('bar', 'foo', 'baz'));
}

public function testCanHandleIntegerishKeysInIterables(): void
{
$storage = new MockAdapter();
$keyValuePair = ['0' => '0'];
self::assertSame([], $storage->setItems($keyValuePair));
self::assertSame([], $storage->addItems($keyValuePair));
self::assertSame([0], $storage->replaceItems($keyValuePair));
self::assertSame([], $storage->removeItems(array_keys($keyValuePair)));
self::assertSame([], $storage->getItems(array_keys($keyValuePair)));
self::assertSame([], $storage->hasItems(array_keys($keyValuePair)));
self::assertSame([0], $storage->touchItems(array_keys($keyValuePair)));
}
}
1 change: 1 addition & 0 deletions test/Storage/TestAsset/MockAdapter.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ class MockAdapter extends AbstractAdapter
{
protected function internalGetItem(string $normalizedKey, ?bool &$success = null, mixed &$casToken = null): mixed
{
$success = false;
return null;
}

Expand Down

0 comments on commit cab2853

Please sign in to comment.