Properly deprecate HTTP.cache_control

silverstripe · Jul 19, 2018 · d92e891 · d92e891
1 parent b4d8439
commit d92e891
Show file tree

Hide file tree

Showing 2 changed files with 108 additions and 1 deletion.
diff --git a/src/Control/HTTP.php b/src/Control/HTTP.php
@@ -71,6 +71,7 @@ class HTTP
     /**
      * List of names to add to the Cache-Control header.
      *
+     * @deprecated 4.2..5.0 Handled by HTTPCacheControlMiddleware instead
      * @see HTTPCacheControlMiddleware::__construct()
      * @config
      * @var array Keys are cache control names, values are boolean flags
@@ -80,7 +81,7 @@ class HTTP
     /**
      * Vary string; A comma separated list of var header names
      *
-     * @deprecated 4.2..5.0 Handled by HTTPCacheMiddleware instead
+     * @deprecated 4.2..5.0 Handled by HTTPCacheControlMiddleware instead
      * @config
      * @var string|null
      */
@@ -473,6 +474,7 @@ public static function add_cache_headers($response = null)
      * Ensure that all deprecated HTTP cache settings are respected
      *
      * @deprecated 4.2..5.0 Use HTTPCacheControlMiddleware instead
+     * @throws \LogicException
      * @param HTTPRequest $request
      * @param HTTPResponse $response
      */
@@ -509,6 +511,37 @@ public static function augmentState(HTTPRequest $request, HTTPResponse $response
             $cacheControlMiddleware->addVary($configVary);
         }
 
+        // Pass cache_control to middleware
+        $configCacheControl = $config->get('cache_control');
+        if ($configCacheControl) {
+            Deprecation::notice('5.0', 'Use HTTPCacheControlMiddleware API instead');
+
+            $supportedDirectives = ['max-age', 'no-cache', 'no-store', 'must-revalidate'];
+            if ($foundUnsupported = array_diff(array_keys($configCacheControl), $supportedDirectives)) {
+                throw new \LogicException(
+                    'Found unsupported legacy directives in HTTP.cache_control: ' .
+                    implode(', ', $foundUnsupported) .
+                    '. Please use HTTPCacheControlMiddleware API instead'
+                );
+            }
+
+            if (isset($configCacheControl['max-age'])) {
+                $cacheControlMiddleware->setMaxAge($configCacheControl['max-age']);
+            }
+
+            if (isset($configCacheControl['no-cache'])) {
+                $cacheControlMiddleware->setNoCache((bool)$configCacheControl['no-cache']);
+            }
+
+            if (isset($configCacheControl['no-store'])) {
+                $cacheControlMiddleware->setNoStore((bool)$configCacheControl['no-store']);
+            }
+
+            if (isset($configCacheControl['must-revalidate'])) {
+                $cacheControlMiddleware->setMustRevalidate((bool)$configCacheControl['must-revalidate']);
+            }
+        }
+
         // Set modification date
         if (self::$modification_date) {
             Deprecation::notice('5.0', 'Use HTTPCacheControlMiddleware::registerModificationDate() instead');

diff --git a/tests/php/Control/HTTPTest.php b/tests/php/Control/HTTPTest.php
@@ -9,6 +9,7 @@
 use SilverStripe\Control\HTTPResponse;
 use SilverStripe\Control\Middleware\HTTPCacheControlMiddleware;
 use SilverStripe\Control\Session;
+use SilverStripe\Core\Config\Config;
 use SilverStripe\Dev\FunctionalTest;
 
 /**
@@ -112,6 +113,79 @@ public function testConfigVary()
         $this->assertEmpty($v);
     }
 
+    public function testDeprecatedVaryHandling()
+    {
+        /** @var Config */
+        Config::modify()->set(
+            HTTP::class,
+            'vary',
+            'X-Foo'
+        );
+        $response = new HTTPResponse('', 200);
+        $this->addCacheHeaders($response);
+        $header = $response->getHeader('Vary');
+        $this->assertContains('X-Foo', $header);
+    }
+
+    public function testDeprecatedCacheControlHandling()
+    {
+        HTTPCacheControlMiddleware::singleton()->publicCache();
+
+        /** @var Config */
+        Config::modify()->set(
+            HTTP::class,
+            'cache_control',
+            [
+                'no-store' => true,
+                'no-cache' => true,
+            ]
+        );
+        $response = new HTTPResponse('', 200);
+        $this->addCacheHeaders($response);
+        $header = $response->getHeader('Cache-Control');
+        $this->assertContains('no-store', $header);
+        $this->assertContains('no-cache', $header);
+    }
+
+    public function testDeprecatedCacheControlHandlingOnMaxAge()
+    {
+        HTTPCacheControlMiddleware::singleton()->publicCache();
+
+        /** @var Config */
+        Config::modify()->set(
+            HTTP::class,
+            'cache_control',
+            [
+                // Needs to be separate from no-cache and no-store,
+                // since that would unset max-age
+                'max-age' => 99,
+            ]
+        );
+        $response = new HTTPResponse('', 200);
+        $this->addCacheHeaders($response);
+        $header = $response->getHeader('Cache-Control');
+        $this->assertContains('max-age=99', $header);
+    }
+
+    /**
+     * @expectedException \LogicException
+     * @expectedExceptionMessageRegExp /Found unsupported legacy directives in HTTP\.cache_control: unknown/
+     */
+    public function testDeprecatedCacheControlHandlingThrowsWithUnknownDirectives()
+    {
+        /** @var Config */
+        Config::modify()->set(
+            HTTP::class,
+            'cache_control',
+            [
+                'no-store' => true,
+                'unknown' => true,
+            ]
+        );
+        $response = new HTTPResponse('', 200);
+        $this->addCacheHeaders($response);
+    }
+
     /**
      * Tests {@link HTTP::getLinksIn()}
      */