From 99bba506e527d62edb4f419eac9a086b883acf27 Mon Sep 17 00:00:00 2001 From: David Buchmann Date: Mon, 25 Oct 2021 08:20:10 +0200 Subject: [PATCH 01/12] add phpstan to ci build --- .github/workflows/.editorconfig | 2 ++ .github/workflows/static.yml | 21 +++++++++++++++++++++ .github/workflows/tests.yml | 2 ++ phpstan.neon.dist | 4 ++++ 4 files changed, 29 insertions(+) create mode 100644 .github/workflows/.editorconfig create mode 100644 .github/workflows/static.yml create mode 100644 phpstan.neon.dist diff --git a/.github/workflows/.editorconfig b/.github/workflows/.editorconfig new file mode 100644 index 0000000..7bd3346 --- /dev/null +++ b/.github/workflows/.editorconfig @@ -0,0 +1,2 @@ +[*.yml] +indent_size = 2 diff --git a/.github/workflows/static.yml b/.github/workflows/static.yml new file mode 100644 index 0000000..4e821aa --- /dev/null +++ b/.github/workflows/static.yml @@ -0,0 +1,21 @@ +name: Static analysis + +on: + push: + branches: + - master + pull_request: + +jobs: + phpstan: + name: PHPStan + runs-on: ubuntu-latest + + steps: + - name: Checkout code + uses: actions/checkout@v2 + + - name: PHPStan + uses: docker://oskarstark/phpstan-ga + with: + args: analyze --no-progress diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index e4d0050..1bbd75a 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -2,6 +2,8 @@ name: tests on: push: + branches: + - master pull_request: jobs: diff --git a/phpstan.neon.dist b/phpstan.neon.dist new file mode 100644 index 0000000..77c3651 --- /dev/null +++ b/phpstan.neon.dist @@ -0,0 +1,4 @@ +parameters: + level: 1 + paths: + - src From 7079a0886a73e52e894afe70a061fdf4ddfa49a4 Mon Sep 17 00:00:00 2001 From: David Buchmann Date: Mon, 25 Oct 2021 08:29:26 +0200 Subject: [PATCH 02/12] fixes for phpstan level 1 --- src/CachePlugin.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/CachePlugin.php b/src/CachePlugin.php index 69fd523..38cb335 100644 --- a/src/CachePlugin.php +++ b/src/CachePlugin.php @@ -207,7 +207,7 @@ protected function doHandleRequest(RequestInterface $request, callable $next, ca $this->pool->save($cacheItem); } - return $this->handleCacheListeners($request, $response, false, isset($cacheItem) ? $cacheItem : null); + return $this->handleCacheListeners($request, $response, false, $cacheItem); }); } From 66aa9f237f2982140f09b7e0e6aa5f3af43228c7 Mon Sep 17 00:00:00 2001 From: David Buchmann Date: Mon, 25 Oct 2021 08:29:43 +0200 Subject: [PATCH 03/12] phpstan level 2 --- phpstan.neon.dist | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/phpstan.neon.dist b/phpstan.neon.dist index 77c3651..093e815 100644 --- a/phpstan.neon.dist +++ b/phpstan.neon.dist @@ -1,4 +1,4 @@ parameters: - level: 1 + level: 2 paths: - src From 297fda539522e4f844ed72155875c709ce0afcf4 Mon Sep 17 00:00:00 2001 From: David Buchmann Date: Mon, 25 Oct 2021 08:32:45 +0200 Subject: [PATCH 04/12] fix phpstan level 2 --- src/Cache/Generator/HeaderCacheKeyGenerator.php | 4 ++-- src/CachePlugin.php | 6 ++++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/Cache/Generator/HeaderCacheKeyGenerator.php b/src/Cache/Generator/HeaderCacheKeyGenerator.php index 562ed48..16587be 100644 --- a/src/Cache/Generator/HeaderCacheKeyGenerator.php +++ b/src/Cache/Generator/HeaderCacheKeyGenerator.php @@ -14,12 +14,12 @@ class HeaderCacheKeyGenerator implements CacheKeyGenerator /** * The header names we should take into account when creating the cache key. * - * @var array + * @var string[] */ private $headerNames; /** - * @param $headerNames + * @param string[] $headerNames */ public function __construct(array $headerNames) { diff --git a/src/CachePlugin.php b/src/CachePlugin.php index 38cb335..9074a99 100644 --- a/src/CachePlugin.php +++ b/src/CachePlugin.php @@ -333,7 +333,7 @@ private function getMaxAge(ResponseInterface $response) if (!is_bool($maxAge)) { $ageHeaders = $response->getHeader('Age'); foreach ($ageHeaders as $age) { - return $maxAge - ((int) $age); + return ((int) $maxAge) - ((int) $age); } return (int) $maxAge; @@ -449,7 +449,7 @@ private function getETag(CacheItemInterface $cacheItem) $data = $cacheItem->get(); // The isset() is to be removed in 2.0. if (!isset($data['etag'])) { - return; + return null; } foreach ($data['etag'] as $etag) { @@ -457,6 +457,8 @@ private function getETag(CacheItemInterface $cacheItem) return $etag; } } + + return null; } /** From 664c7fdf4a7f520a7c5a378aad032cc2269ef02b Mon Sep 17 00:00:00 2001 From: David Buchmann Date: Mon, 25 Oct 2021 08:34:30 +0200 Subject: [PATCH 05/12] phpstan level 3 --- phpstan.neon.dist | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/phpstan.neon.dist b/phpstan.neon.dist index 093e815..acd135a 100644 --- a/phpstan.neon.dist +++ b/phpstan.neon.dist @@ -1,4 +1,4 @@ parameters: - level: 2 + level: 3 paths: - src From aa82a78c80f36ba4c8619b06b1afea20a83e261f Mon Sep 17 00:00:00 2001 From: David Buchmann Date: Mon, 25 Oct 2021 08:35:41 +0200 Subject: [PATCH 06/12] fix phpstan level 3 --- src/CachePlugin.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/CachePlugin.php b/src/CachePlugin.php index 9074a99..0112261 100644 --- a/src/CachePlugin.php +++ b/src/CachePlugin.php @@ -222,7 +222,7 @@ protected function doHandleRequest(RequestInterface $request, callable $next, ca private function calculateCacheItemExpiresAfter($maxAge) { if (null === $this->config['cache_lifetime'] && null === $maxAge) { - return; + return null; } return $this->config['cache_lifetime'] + $maxAge; @@ -239,7 +239,7 @@ private function calculateCacheItemExpiresAfter($maxAge) private function calculateResponseExpiresAt($maxAge) { if (null === $maxAge) { - return; + return null; } return time() + $maxAge; @@ -430,7 +430,7 @@ private function getModifiedSinceHeaderValue(CacheItemInterface $cacheItem) $data = $cacheItem->get(); // The isset() is to be removed in 2.0. if (!isset($data['createdAt'])) { - return; + return null; } $modified = new \DateTime('@'.$data['createdAt']); From 757753af99ba6435bd7130002bd849118917979b Mon Sep 17 00:00:00 2001 From: David Buchmann Date: Mon, 25 Oct 2021 08:36:00 +0200 Subject: [PATCH 07/12] phpstan level 4 --- phpstan.neon.dist | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/phpstan.neon.dist b/phpstan.neon.dist index acd135a..c9b813b 100644 --- a/phpstan.neon.dist +++ b/phpstan.neon.dist @@ -1,4 +1,5 @@ parameters: - level: 3 + level: 4 paths: - src + treatPhpDocTypesAsCertain: false From 220a4b5458e4236400c7b8f23014777fbee93845 Mon Sep 17 00:00:00 2001 From: David Buchmann Date: Mon, 25 Oct 2021 08:42:30 +0200 Subject: [PATCH 08/12] prefer array_key_exists over isset for more robust code --- src/CachePlugin.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/CachePlugin.php b/src/CachePlugin.php index 0112261..beb4f96 100644 --- a/src/CachePlugin.php +++ b/src/CachePlugin.php @@ -78,7 +78,7 @@ public function __construct(CacheItemPoolInterface $pool, $streamFactory, array $this->pool = $pool; $this->streamFactory = $streamFactory; - if (isset($config['respect_cache_headers']) && isset($config['respect_response_cache_directives'])) { + if (\array_key_exists('respect_cache_headers', $config) && \array_key_exists('respect_response_cache_directives', $config)) { throw new \InvalidArgumentException('You can\'t provide config option "respect_cache_headers" and "respect_response_cache_directives". Use "respect_response_cache_directives" instead.'); } @@ -103,7 +103,7 @@ public function __construct(CacheItemPoolInterface $pool, $streamFactory, array public static function clientCache(CacheItemPoolInterface $pool, $streamFactory, array $config = []) { // Allow caching of private requests - if (isset($config['respect_response_cache_directives'])) { + if (\array_key_exists('respect_response_cache_directives', $config)) { $config['respect_response_cache_directives'][] = 'no-cache'; $config['respect_response_cache_directives'][] = 'max-age'; $config['respect_response_cache_directives'] = array_unique($config['respect_response_cache_directives']); From 95a502bebbf4fc3703423813bc1e51679209bc3f Mon Sep 17 00:00:00 2001 From: David Buchmann Date: Mon, 25 Oct 2021 08:42:42 +0200 Subject: [PATCH 09/12] phpstan level 7 --- phpstan.neon.dist | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/phpstan.neon.dist b/phpstan.neon.dist index c9b813b..63b0db6 100644 --- a/phpstan.neon.dist +++ b/phpstan.neon.dist @@ -1,5 +1,5 @@ parameters: - level: 4 + level: 7 paths: - src treatPhpDocTypesAsCertain: false From cb8755be388faa25a3ba6ae71b43387acf14908f Mon Sep 17 00:00:00 2001 From: David Buchmann Date: Mon, 25 Oct 2021 08:57:54 +0200 Subject: [PATCH 10/12] cleanup type hints and phpdoc --- src/CachePlugin.php | 77 ++++++++++++++++++--------------------------- 1 file changed, 30 insertions(+), 47 deletions(-) diff --git a/src/CachePlugin.php b/src/CachePlugin.php index beb4f96..8064638 100644 --- a/src/CachePlugin.php +++ b/src/CachePlugin.php @@ -9,6 +9,7 @@ use Http\Client\Common\Plugin\Cache\Listener\CacheListener; use Http\Message\StreamFactory; use Http\Promise\FulfilledPromise; +use Http\Promise\Promise; use Psr\Cache\CacheItemInterface; use Psr\Cache\CacheItemPoolInterface; use Psr\Http\Message\RequestInterface; @@ -39,20 +40,20 @@ final class CachePlugin implements Plugin private $streamFactory; /** - * @var array + * @var mixed[] */ private $config; /** * Cache directives indicating if a response can not be cached. * - * @var array + * @var string[] */ private $noCacheFlags = ['no-cache', 'private', 'no-store']; /** * @param StreamFactory|StreamFactoryInterface $streamFactory - * @param array $config { + * @param mixed[] $config { * * @var bool $respect_cache_headers Whether to look at the cache directives or ignore them * @var int $default_ttl (seconds) If we do not respect cache headers or can't calculate a good ttl, use this @@ -61,9 +62,9 @@ final class CachePlugin implements Plugin * @var int $cache_lifetime (seconds) To support serving a previous stale response when the server answers 304 * we have to store the cache for a longer time than the server originally says it is valid for. * We store a cache item for $cache_lifetime + max age of the response. - * @var array $methods list of request methods which can be cached - * @var array $blacklisted_paths list of regex for URLs explicitly not to be cached - * @var array $respect_response_cache_directives list of cache directives this plugin will respect while caching responses + * @var string[] $methods list of request methods which can be cached + * @var string[] $blacklisted_paths list of regex for URLs explicitly not to be cached + * @var string[] $respect_response_cache_directives list of cache directives this plugin will respect while caching responses * @var CacheKeyGenerator $cache_key_generator an object to generate the cache key. Defaults to a new instance of SimpleGenerator * @var CacheListener[] $cache_listeners an array of objects to act on the response based on the results of the cache check. * Defaults to an empty array @@ -96,7 +97,7 @@ public function __construct(CacheItemPoolInterface $pool, $streamFactory, array * cache responses with `private` cache directive. * * @param StreamFactory|StreamFactoryInterface $streamFactory - * @param array $config For all possible config options see the constructor docs + * @param mixed[] $config For all possible config options see the constructor docs * * @return CachePlugin */ @@ -119,7 +120,7 @@ public static function clientCache(CacheItemPoolInterface $pool, $streamFactory, * cache responses with the `private`or `no-cache` directives. * * @param StreamFactory|StreamFactoryInterface $streamFactory - * @param array $config For all possible config options see the constructor docs + * @param mixed[] $config For all possible config options see the constructor docs * * @return CachePlugin */ @@ -128,6 +129,11 @@ public static function serverCache(CacheItemPoolInterface $pool, $streamFactory, return new self($pool, $streamFactory, $config); } + /** + * {@inheritdoc} + * + * @return Promise Resolves a PSR-7 Response or fails with an Http\Client\Exception (The same as HttpAsyncClient) + */ protected function doHandleRequest(RequestInterface $request, callable $next, callable $first) { $method = strtoupper($request->getMethod()); @@ -215,11 +221,9 @@ protected function doHandleRequest(RequestInterface $request, callable $next, ca * Calculate the timestamp when this cache item should be dropped from the cache. The lowest value that can be * returned is $maxAge. * - * @param int|null $maxAge - * * @return int|null Unix system time passed to the PSR-6 cache */ - private function calculateCacheItemExpiresAfter($maxAge) + private function calculateCacheItemExpiresAfter(?int $maxAge): ?int { if (null === $this->config['cache_lifetime'] && null === $maxAge) { return null; @@ -232,11 +236,9 @@ private function calculateCacheItemExpiresAfter($maxAge) * Calculate the timestamp when a response expires. After that timestamp, we need to send a * If-Modified-Since / If-None-Match request to validate the response. * - * @param int|null $maxAge - * * @return int|null Unix system time. A null value means that the response expires when the cache item expires */ - private function calculateResponseExpiresAt($maxAge) + private function calculateResponseExpiresAt(?int $maxAge): ?int { if (null === $maxAge) { return null; @@ -268,10 +270,8 @@ protected function isCacheable(ResponseInterface $response) /** * Verify that we can cache this request. - * - * @return bool */ - private function isCacheableRequest(RequestInterface $request) + private function isCacheableRequest(RequestInterface $request): bool { $uri = $request->getUri()->__toString(); foreach ($this->config['blacklisted_paths'] as $regex) { @@ -290,7 +290,7 @@ private function isCacheableRequest(RequestInterface $request) * * @return bool|string The value of the directive, true if directive without value, false if directive not present */ - private function getCacheControlDirective(ResponseInterface $response, $name) + private function getCacheControlDirective(ResponseInterface $response, string $name) { $headers = $response->getHeader('Cache-Control'); foreach ($headers as $header) { @@ -307,10 +307,7 @@ private function getCacheControlDirective(ResponseInterface $response, $name) return false; } - /** - * @return string - */ - private function createCacheKey(RequestInterface $request) + private function createCacheKey(RequestInterface $request): string { $key = $this->config['cache_key_generator']->generate($request); @@ -318,11 +315,11 @@ private function createCacheKey(RequestInterface $request) } /** - * Get a ttl in seconds. It could return null if we do not respect cache headers and got no defaultTtl. + * Get a ttl in seconds. * - * @return int|null + * Returns null if we do not respect cache headers and got no defaultTtl. */ - private function getMaxAge(ResponseInterface $response) + private function getMaxAge(ResponseInterface $response): ?int { if (!in_array('max-age', $this->config['respect_response_cache_directives'], true)) { return $this->config['default_ttl']; @@ -351,7 +348,7 @@ private function getMaxAge(ResponseInterface $response) /** * Configure an options resolver. */ - private function configureOptions(OptionsResolver $resolver) + private function configureOptions(OptionsResolver $resolver): void { $resolver->setDefaults([ 'cache_lifetime' => 86400 * 30, // 30 days @@ -398,10 +395,7 @@ private function configureOptions(OptionsResolver $resolver) }); } - /** - * @return ResponseInterface - */ - private function createResponseFromCacheItem(CacheItemInterface $cacheItem) + private function createResponseFromCacheItem(CacheItemInterface $cacheItem): ResponseInterface { $data = $cacheItem->get(); @@ -415,17 +409,13 @@ private function createResponseFromCacheItem(CacheItemInterface $cacheItem) throw new RewindStreamException('Cannot rewind stream.', 0, $e); } - $response = $response->withBody($stream); - - return $response; + return $response->withBody($stream); } /** - * Get the value of the "If-Modified-Since" header. - * - * @return string|null + * Get the value for the "If-Modified-Since" header. */ - private function getModifiedSinceHeaderValue(CacheItemInterface $cacheItem) + private function getModifiedSinceHeaderValue(CacheItemInterface $cacheItem): ?string { $data = $cacheItem->get(); // The isset() is to be removed in 2.0. @@ -441,10 +431,8 @@ private function getModifiedSinceHeaderValue(CacheItemInterface $cacheItem) /** * Get the ETag from the cached response. - * - * @return string|null */ - private function getETag(CacheItemInterface $cacheItem) + private function getETag(CacheItemInterface $cacheItem): ?string { $data = $cacheItem->get(); // The isset() is to be removed in 2.0. @@ -462,14 +450,9 @@ private function getETag(CacheItemInterface $cacheItem) } /** - * Call the cache listeners, if they are set. - * - * @param bool $cacheHit - * @param CacheItemInterface|null $cacheItem - * - * @return ResponseInterface + * Call the registered cache listeners. */ - private function handleCacheListeners(RequestInterface $request, ResponseInterface $response, $cacheHit, $cacheItem) + private function handleCacheListeners(RequestInterface $request, ResponseInterface $response, bool $cacheHit, ?CacheItemInterface $cacheItem): ResponseInterface { foreach ($this->config['cache_listeners'] as $cacheListener) { $response = $cacheListener->onCacheResponse($request, $response, $cacheHit, $cacheItem); From a519bf995d5d4332309269756e623189bda209cb Mon Sep 17 00:00:00 2001 From: David Buchmann Date: Mon, 25 Oct 2021 08:58:45 +0200 Subject: [PATCH 11/12] phpstan level 8 --- phpstan.neon.dist | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/phpstan.neon.dist b/phpstan.neon.dist index 63b0db6..ac454e0 100644 --- a/phpstan.neon.dist +++ b/phpstan.neon.dist @@ -1,5 +1,5 @@ parameters: - level: 7 + level: 8 paths: - src treatPhpDocTypesAsCertain: false From 3f0266a302dec1564fbb9771b926c3b1745fb3e4 Mon Sep 17 00:00:00 2001 From: David Buchmann Date: Mon, 25 Oct 2021 09:07:49 +0200 Subject: [PATCH 12/12] be more defensive about the value of the cache hit --- src/CachePlugin.php | 30 ++++++++++++++++-------------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/src/CachePlugin.php b/src/CachePlugin.php index 8064638..8b60a33 100644 --- a/src/CachePlugin.php +++ b/src/CachePlugin.php @@ -152,22 +152,24 @@ protected function doHandleRequest(RequestInterface $request, callable $next, ca if ($cacheItem->isHit()) { $data = $cacheItem->get(); - // The array_key_exists() is to be removed in 2.0. - if (array_key_exists('expiresAt', $data) && (null === $data['expiresAt'] || time() < $data['expiresAt'])) { - // This item is still valid according to previous cache headers - $response = $this->createResponseFromCacheItem($cacheItem); - $response = $this->handleCacheListeners($request, $response, true, $cacheItem); - - return new FulfilledPromise($response); - } + if (is_array($data)) { + // The array_key_exists() is to be removed in 2.0. + if (array_key_exists('expiresAt', $data) && (null === $data['expiresAt'] || time() < $data['expiresAt'])) { + // This item is still valid according to previous cache headers + $response = $this->createResponseFromCacheItem($cacheItem); + $response = $this->handleCacheListeners($request, $response, true, $cacheItem); + + return new FulfilledPromise($response); + } - // Add headers to ask the server if this cache is still valid - if ($modifiedSinceValue = $this->getModifiedSinceHeaderValue($cacheItem)) { - $request = $request->withHeader('If-Modified-Since', $modifiedSinceValue); - } + // Add headers to ask the server if this cache is still valid + if ($modifiedSinceValue = $this->getModifiedSinceHeaderValue($cacheItem)) { + $request = $request->withHeader('If-Modified-Since', $modifiedSinceValue); + } - if ($etag = $this->getETag($cacheItem)) { - $request = $request->withHeader('If-None-Match', $etag); + if ($etag = $this->getETag($cacheItem)) { + $request = $request->withHeader('If-None-Match', $etag); + } } }