Feedback code fixes

Author: acrobat

src/CachePlugin.php

@@ -34,31 +34,52 @@ final class CachePlugin implements Plugin
      */
     private $config;
 
+    /**
+     * Cache directives indicating if a response can be cached
+     *
+     * @var array
+     */
+    private $noCacheFlags = ['no-cache', 'private'];
+
     /**
      * @param CacheItemPoolInterface $pool
      * @param StreamFactory          $streamFactory
      * @param array                  $config        {
      *
-     *     @var bool $respect_cache_headers Whether to look at the cache directives or ignore them
+     *     @var bool $respect_cache_headers Whether to look at the cache directives or ignore them. This option is deprecated, use `respect_response_cache_directives` instead
      *     @var int $default_ttl (seconds) If we do not respect cache headers or can't calculate a good ttl, use this
      *              value
      *     @var string $hash_algo The hashing algorithm to use when generating cache keys
      *     @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 $respect_response_cache_directives list of cache directives this plugin will respect while caching responses.
      * }
      */
     public function __construct(CacheItemPoolInterface $pool, StreamFactory $streamFactory, array $config = [])
     {
         $this->pool = $pool;
         $this->streamFactory = $streamFactory;
 
+        if (isset($config['respect_cache_headers']) && $config['respect_response_cache_directives']) {
+            throw new \InvalidArgumentException('You can\'t provide config option "respect_cache_headers" and "respect_response_cache_directives". Use "respect_response_cache_directives" instead.');
+        }
+
         $optionsResolver = new OptionsResolver();
         $this->configureOptions($optionsResolver);
         $this->config = $optionsResolver->resolve($config);
     }
 
+    /**
+     * This method will setup the cachePlugin in client cache mode. When using the client cache mode the plugin will cache responses with `private` cache directive
+     *
+     * @param CacheItemPoolInterface $pool
+     * @param StreamFactory          $streamFactory
+     * @param array                  $config
+     *
+     * @return CachePlugin
+     */
     public static function clientCache(CacheItemPoolInterface $pool, StreamFactory $streamFactory, array $config = [])
     {
         // Allow caching of private requests
@@ -76,6 +97,15 @@ public static function clientCache(CacheItemPoolInterface $pool, StreamFactory $
         return new self($pool, $streamFactory, $config);
     }
 
+    /**
+     * This method will setup the cachePlugin in server cache mode. This is the default caching behavior (refuses to cache responses with the `private`or `no-cache` directives
+     *
+     * @param CacheItemPoolInterface $pool
+     * @param StreamFactory          $streamFactory
+     * @param array                  $config
+     *
+     * @return CachePlugin
+     */
     public static function serverCache(CacheItemPoolInterface $pool, StreamFactory $streamFactory, array $config = [])
     {
         return new self($pool, $streamFactory, $config);
@@ -206,12 +236,10 @@ protected function isCacheable(ResponseInterface $response)
         if (!in_array($response->getStatusCode(), [200, 203, 300, 301, 302, 404, 410])) {
             return false;
         }
-        if (!$this->config['respect_cache_headers']) {
-            return true;
-        }
 
-        foreach ($this->config['respect_response_cache_directives'] as $cacheDirective) {
-            if ($this->getCacheControlDirective($response, $cacheDirective)) {
+        $cacheableDirectives = $this->extractDirectives($this->config['respect_response_cache_directives'], $this->noCacheFlags);
+        foreach ($cacheableDirectives as $cacheableDirective) {
+            if ($this->getCacheControlDirective($response, $cacheableDirective)) {
                 return false;
             }
         }
@@ -268,7 +296,7 @@ private function createCacheKey(RequestInterface $request)
      */
     private function getMaxAge(ResponseInterface $response)
     {
-        if (!$this->config['respect_cache_headers'] || !in_array('max-age', $this->config['respect_response_cache_directives'], true)) {
+        if (!in_array('max-age', $this->config['respect_response_cache_directives'], true)) {
             return $this->config['default_ttl'];
         }
 
@@ -306,7 +334,7 @@ private function configureOptions(OptionsResolver $resolver)
             'respect_cache_headers' => true,
             'hash_algo' => 'sha1',
             'methods' => ['GET', 'HEAD'],
-            'respect_response_cache_directives' => ['no-cache', 'private', 'max-age'],
+            'respect_response_cache_directives' => ['no-cache', 'private', 'max-age', 'no-store'],
         ]);
 
         $resolver->setAllowedTypes('cache_lifetime', ['int', 'null']);
@@ -328,6 +356,14 @@ private function configureOptions(OptionsResolver $resolver)
 
             return $value;
         });
+
+        $resolver->setNormalizer('respect_response_cache_directives', function (Options $options, $value) {
+            if (false === $options['respect_cache_headers']) {
+                return [];
+            }
+
+            return $value;
+        });
     }
 
     /**
@@ -392,4 +428,9 @@ private function getETag(CacheItemInterface $cacheItem)
             }
         }
     }
+
+    private function extractDirectives($directives, $extractDirectivesList)
+    {
+        return array_intersect($directives, $extractDirectivesList);
+    }
 }