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.

Sign up for GitHub

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

CacheItemPoolDecorator Psalm Issues #84

Merged
merged 2 commits into from
Apr 3, 2021
Merged

CacheItemPoolDecorator Psalm Issues #84

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f584214
qa: remove unused test asset
boesing Apr 3, 2021
a11a3eb
qa: fix psalm issues
boesing Apr 3, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 3 additions & 20 deletions psalm-baseline.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -284,19 +284,18 @@
<code>validateKeys</code>
<code>validateStorage</code>
</MissingReturnType>
<MixedAssignment occurrences="4">
<MixedAssignment occurrences="3">
<code>$cleared</code>
<code>$key</code>
<code>$value</code>
<code>$value</code>
</MixedAssignment>
<MixedInferredReturnType occurrences="6">
<code>array&lt;string,CacheItem&gt;</code>
<code>clear</code>
<code>deleteItems</code>
<code>getItem</code>
<code>getItems</code>
<code>hasItem</code>
<code>save</code>
</MixedInferredReturnType>
<RedundantConditionGivenDocblockType occurrences="1">
<code>null !== $this-&gt;storage-&gt;removeItems($keys)</code>
Expand Down Expand Up @@ -1145,8 +1144,7 @@
<MixedArgumentTypeCoercion occurrences="1">
<code>$capabilities ?? $this-&gt;defaultCapabilities</code>
</MixedArgumentTypeCoercion>
<MixedAssignment occurrences="4">
<code>$counter</code>
<MixedAssignment occurrences="3">
<code>$item</code>
<code>$item</code>
<code>$item</code>
Expand All @@ -1167,9 +1165,6 @@
<code>isHit</code>
<code>set</code>
</MixedMethodCall>
<MixedOperand occurrences="1">
<code>$counter</code>
</MixedOperand>
<PossiblyInvalidArgument occurrences="1">
<code>$items</code>
</PossiblyInvalidArgument>
Expand All @@ -1192,18 +1187,6 @@
<code>[]</code>
</InvalidArgument>
</file>
<file src="test/Psr/CacheItemPool/TestAsset/StorageAdapter.php">
<InvalidReturnType occurrences="5">
<code>clearByNamespace</code>
<code>flush</code>
<code>internalGetItem</code>
<code>internalRemoveItem</code>
<code>internalSetItem</code>
</InvalidReturnType>
<PropertyNotSetInConstructor occurrences="1">
<code>StorageAdapter</code>
</PropertyNotSetInConstructor>
</file>
<file src="test/Psr/SimpleCache/SimpleCacheDecoratorTest.php">
<MissingReturnType occurrences="9">
<code>testHasProxiesToStorage</code>
Expand Down
53 changes: 29 additions & 24 deletions src/Psr/CacheItemPool/CacheItemPoolDecorator.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,15 @@
use function array_diff_key;
use function array_flip;
use function array_keys;
use function array_map;
use function array_merge;
use function array_unique;
use function array_walk;
use function array_values;
use function assert;
use function current;
use function get_class;
use function gettype;
use function in_array;
use function is_bool;
use function is_string;
use function preg_match;
use function sprintf;
Expand All @@ -46,7 +47,7 @@ class CacheItemPoolDecorator implements CacheItemPoolInterface
/** @var StorageInterface */
private $storage;

/** @var CacheItem[] */
/** @var array<string,CacheItem> */
private $deferred = [];

/**
Expand Down Expand Up @@ -88,7 +89,7 @@ public function getItem($key)
// ignore
}

return new CacheItem($key, $value, $isHit);
return new CacheItem($key, $value, $isHit ?? false);
}

return clone $this->deferred[$key];
Expand Down Expand Up @@ -122,8 +123,8 @@ public function getItems(array $keys = [])
}

foreach ($cacheItems as $key => $value) {
$isHit = true;
$items[$key] = new CacheItem($key, $value, $isHit);
assert(is_string($key));
$items[$key] = new CacheItem($key, $value, true);
}

// Return empty items for any keys that where not found
Expand Down Expand Up @@ -180,6 +181,7 @@ public function clear()
$cleared = false;
}

assert(is_bool($cleared));
return $cleared;
}

Expand Down Expand Up @@ -244,16 +246,20 @@ public function saveDeferred(CacheItemInterface $item)
*/
public function commit()
{
/** @var array<string,non-empty-array<string,CacheItem>> $groupedByTtl */
$groupedByTtl = [];
foreach ($this->deferred as $cacheKey => $item) {
$itemTtl = var_export($item->getTtl(), true);
$group = $groupedByTtl[$itemTtl] ?? [];
$group[$cacheKey] = $item;
$groupedByTtl[$itemTtl] = $group;
$groupedByTtl[$itemTtl] = array_values($group);
}

$notSavedItems = [];
/**
* NOTE: we are not using the array key for the TTL in here as TTL might be `null`.
* Since we stringify the TTL by using `var_export`, the array has string and integer keys.
* Converting a string `null` back to native null-type would be a huge mess.
*/
foreach ($groupedByTtl as $keyValuePairs) {
$itemTtl = current($keyValuePairs)->getTtl();
$notSavedItems[] = $this->saveMultipleItems($keyValuePairs, $itemTtl);
Expand Down Expand Up @@ -359,33 +365,34 @@ private function validateKeys($keys)
}

/**
* @param array<int,CacheItem> $items
* @psalm-param list<CacheItem> $items
* @return array<string,CacheItem>
* @psalm-param non-empty-list<CacheItem> $items
* @psalm-return array<string,CacheItem>
*/
private function saveMultipleItems(array $items, ?int $itemTtl): array
{
$keyItemPair = [];
foreach ($items as $item) {
$keyItemPair[$item->getKey()] = $item;
}

// delete expired item
if ($itemTtl < 0) {
$this->deleteItems(array_map(static function (CacheItemInterface $item): string {
return $item->getKey();
}, $items));
array_walk($items, static function (CacheItem $cacheItem): void {
$this->deleteItems(array_keys($keyItemPair));
foreach ($keyItemPair as $cacheItem) {
$cacheItem->setIsHit(false);
});
}

return $items;
return $keyItemPair;
}

$options = $this->storage->getOptions();
$ttl = $options->getTtl();

$keyValuePair = [];
$keyItemPair = [];
foreach ($items as $item) {
$key = $item->getKey();
$key = $item->getKey();
/** @psalm-suppress MixedAssignment */
$keyValuePair[$key] = $item->get();
$keyItemPair[$key] = $item;
}

$options->setTtl($itemTtl ?? 0);
Expand All @@ -396,17 +403,15 @@ private function saveMultipleItems(array $items, ?int $itemTtl): array
} catch (Exception\InvalidArgumentException $e) {
throw new InvalidArgumentException($e->getMessage(), $e->getCode(), $e);
} catch (Exception\ExceptionInterface $e) {
foreach (array_keys($keyValuePair) as $key) {
$notSavedKeys[] = $key;
}
$notSavedKeys = array_keys($keyValuePair);
} finally {
$options->setTtl($ttl);
}

$notSavedItems = [];
foreach ($keyItemPair as $key => $item) {
if (in_array($key, $notSavedKeys, true)) {
$notSavedItems[] = $item;
$notSavedItems[$key] = $item;
continue;
}

Expand Down
10 changes: 10 additions & 0 deletions test/Psr/CacheItemPool/CacheItemPoolDecoratorTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -615,6 +615,11 @@ public function testClearDeferred(): void

$item = $adapter->getItem('foo');
$adapter->saveDeferred($item);
$this->storage
->expects(self::once())
->method('clearByNamespace')
->willReturn(true);

$adapter->clear();
self::assertFalse($adapter->hasItem('foo'));
}
Expand Down Expand Up @@ -906,6 +911,11 @@ protected function tearDown(): void
} catch (Throwable $throwable) {
/** Cleanup deferred items as {@see CacheItemPoolDecorator::__destruct} is gonna try to store them. */
} finally {
/**
* Suppress this as we are safe in tear down
*
* @psalm-suppress PossiblyNullPropertyAssignmentValue
*/
$this->adapter = null;
}
parent::tearDown();
Expand Down
58 changes: 0 additions & 58 deletions test/Psr/CacheItemPool/TestAsset/StorageAdapter.php
View file
Open in desktop

This file was deleted.