Add Cache-control: immutable

Cache generated CSS forever!
Also cache combined JS forever
Fix tests

Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>

4 files changed, 38 insertions, 33 deletions

diff --git a/core/Controller/CssController.php b/core/Controller/CssController.php
index 43a4f453b0e..c8458eab29c 100644
--- a/core/Controller/CssController.php
+++ b/core/Controller/CssController.php
@@ -1,4 +1,5 @@
 <?php
+declare(strict_types=1);
 /**
  * @copyright Copyright (c) 2016, John Molakvoæ (skjnldsv@protonmail.com)
  *
@@ -31,11 +32,13 @@ use OCP\AppFramework\Controller;
 use OCP\AppFramework\Http;
 use OCP\AppFramework\Http\NotFoundResponse;
 use OCP\AppFramework\Http\FileDisplayResponse;
+use OCP\AppFramework\Http\Response;
 use OCP\AppFramework\Utility\ITimeFactory;
 use OCP\Files\IAppData;
 use OCP\Files\NotFoundException;
 use OCP\Files\SimpleFS\ISimpleFile;
 use OCP\Files\SimpleFS\ISimpleFolder;
+use OCP\IConfig;
 use OCP\IRequest;
 
 class CssController extends Controller {
@@ -46,13 +49,10 @@ class CssController extends Controller {
 	/** @var ITimeFactory */
 	protected $timeFactory;
 
-	/**
-	 * @param string $appName
-	 * @param IRequest $request
-	 * @param Factory $appDataFactory
-	 * @param ITimeFactory $timeFactory
-	 */
-	public function __construct($appName, IRequest $request, Factory $appDataFactory, ITimeFactory $timeFactory) {
+	public function __construct(string $appName,
+								IRequest $request,
+								Factory $appDataFactory,
+								ITimeFactory $timeFactory) {
 		parent::__construct($appName, $request);
 
 		$this->appData = $appDataFactory->get('css');
@@ -67,7 +67,7 @@ class CssController extends Controller {
 	 * @param string $appName css folder name
 	 * @return FileDisplayResponse|NotFoundResponse
 	 */
-	public function getCss($fileName, $appName) {
+	public function getCss(string $fileName, string $appName): Response {
 		try {
 			$folder = $this->appData->getFolder($appName);
 			$gzip = false;
@@ -80,10 +80,13 @@ class CssController extends Controller {
 		if ($gzip) {
 			$response->addHeader('Content-Encoding', 'gzip');
 		}
-		$response->cacheFor(86400);
+
+		$ttl = 31536000;
+		$response->addHeader('Cache-Control', 'max-age='.$ttl.', immutable');
+
 		$expires = new \DateTime();
 		$expires->setTimestamp($this->timeFactory->getTime());
-		$expires->add(new \DateInterval('PT24H'));
+		$expires->add(new \DateInterval('PT'.$ttl.'S'));
 		$response->addHeader('Expires', $expires->format(\DateTime::RFC1123));
 		$response->addHeader('Pragma', 'cache');
 		return $response;
@@ -94,8 +97,9 @@ class CssController extends Controller {
 	 * @param string $fileName
 	 * @param bool $gzip is set to true if we use the gzip file
 	 * @return ISimpleFile
+	 * @throws NotFoundException
 	 */
-	private function getFile(ISimpleFolder $folder, $fileName, &$gzip) {
+	private function getFile(ISimpleFolder $folder, string $fileName, bool &$gzip): ISimpleFile {
 		$encoding = $this->request->getHeader('Accept-Encoding');
 
 		if (strpos($encoding, 'gzip') !== false) {
diff --git a/core/Controller/JsController.php b/core/Controller/JsController.php
index 670ca997257..3fc48e9a380 100644
--- a/core/Controller/JsController.php
+++ b/core/Controller/JsController.php
@@ -1,4 +1,5 @@
 <?php
+declare(strict_types=1);
 /**
  * @copyright 2017, Roeland Jago Douma <roeland@famdouma.nl>
  *
@@ -29,6 +30,7 @@ use OCP\AppFramework\Controller;
 use OCP\AppFramework\Http;
 use OCP\AppFramework\Http\NotFoundResponse;
 use OCP\AppFramework\Http\FileDisplayResponse;
+use OCP\AppFramework\Http\Response;
 use OCP\AppFramework\Utility\ITimeFactory;
 use OCP\Files\IAppData;
 use OCP\Files\NotFoundException;
@@ -44,12 +46,6 @@ class JsController extends Controller {
 	/** @var ITimeFactory */
 	protected $timeFactory;
 
-	/**
-	 * @param string $appName
-	 * @param IRequest $request
-	 * @param Factory $appDataFactory
-	 * @param ITimeFactory $timeFactory
-	 */
 	public function __construct($appName, IRequest $request, Factory $appDataFactory, ITimeFactory $timeFactory) {
 		parent::__construct($appName, $request);
 
@@ -65,7 +61,7 @@ class JsController extends Controller {
 	 * @param string $appName css folder name
 	 * @return FileDisplayResponse|NotFoundResponse
 	 */
-	public function getJs($fileName, $appName) {
+	public function getJs(string $fileName, string $appName): Response {
 		try {
 			$folder = $this->appData->getFolder($appName);
 			$gzip = false;
@@ -78,10 +74,13 @@ class JsController extends Controller {
 		if ($gzip) {
 			$response->addHeader('Content-Encoding', 'gzip');
 		}
-		$response->cacheFor(86400);
+
+		$ttl = 31536000;
+		$response->addHeader('Cache-Control', 'max-age='.$ttl.', immutable');
+
 		$expires = new \DateTime();
 		$expires->setTimestamp($this->timeFactory->getTime());
-		$expires->add(new \DateInterval('PT24H'));
+		$expires->add(new \DateInterval('PT'.$ttl.'S'));
 		$response->addHeader('Expires', $expires->format(\DateTime::RFC1123));
 		$response->addHeader('Pragma', 'cache');
 		return $response;
@@ -92,8 +91,10 @@ class JsController extends Controller {
 	 * @param string $fileName
 	 * @param bool $gzip is set to true if we use the gzip file
 	 * @return ISimpleFile
+	 *
+	 * @throws NotFoundException
 	 */
-	private function getFile(ISimpleFolder $folder, $fileName, &$gzip) {
+	private function getFile(ISimpleFolder $folder, string $fileName, bool &$gzip): ISimpleFile {
 		$encoding = $this->request->getHeader('Accept-Encoding');
 
 		if (strpos($encoding, 'gzip') !== false) {
diff --git a/tests/Core/Controller/CssControllerTest.php b/tests/Core/Controller/CssControllerTest.php
index c78233d8917..8ec15f449a0 100644
--- a/tests/Core/Controller/CssControllerTest.php
+++ b/tests/Core/Controller/CssControllerTest.php
@@ -109,10 +109,10 @@ class CssControllerTest extends TestCase {
 			->willReturn($file);
 
 		$expected = new FileDisplayResponse($file, Http::STATUS_OK, ['Content-Type' => 'text/css']);
-		$expected->cacheFor(86400);
+		$expected->addHeader('Cache-Control', 'max-age=31536000, immutable');
 		$expires = new \DateTime();
 		$expires->setTimestamp(1337);
-		$expires->add(new \DateInterval('PT24H'));
+		$expires->add(new \DateInterval('PT31536000S'));
 		$expected->addHeader('Expires', $expires->format(\DateTime::RFC1123));
 		$expected->addHeader('Pragma', 'cache');
 
@@ -137,10 +137,10 @@ class CssControllerTest extends TestCase {
 
 		$expected = new FileDisplayResponse($gzipFile, Http::STATUS_OK, ['Content-Type' => 'text/css']);
 		$expected->addHeader('Content-Encoding', 'gzip');
-		$expected->cacheFor(86400);
+		$expected->addHeader('Cache-Control', 'max-age=31536000, immutable');
 		$expires = new \DateTime();
 		$expires->setTimestamp(1337);
-		$expires->add(new \DateInterval('PT24H'));
+		$expires->add(new \DateInterval('PT31536000S'));
 		$expected->addHeader('Expires', $expires->format(\DateTime::RFC1123));
 		$expected->addHeader('Pragma', 'cache');
 
@@ -170,10 +170,10 @@ class CssControllerTest extends TestCase {
 			->willReturn('gzip, deflate');
 
 		$expected = new FileDisplayResponse($file, Http::STATUS_OK, ['Content-Type' => 'text/css']);
-		$expected->cacheFor(86400);
+		$expected->addHeader('Cache-Control', 'max-age=31536000, immutable');
 		$expires = new \DateTime();
 		$expires->setTimestamp(1337);
-		$expires->add(new \DateInterval('PT24H'));
+		$expires->add(new \DateInterval('PT31536000S'));
 		$expected->addHeader('Expires', $expires->format(\DateTime::RFC1123));
 		$expected->addHeader('Pragma', 'cache');
 
diff --git a/tests/Core/Controller/JsControllerTest.php b/tests/Core/Controller/JsControllerTest.php
index 571318c89d0..3910045276b 100644
--- a/tests/Core/Controller/JsControllerTest.php
+++ b/tests/Core/Controller/JsControllerTest.php
@@ -109,10 +109,10 @@ class JsControllerTest extends TestCase {
 			->willReturn($file);
 
 		$expected = new FileDisplayResponse($file, Http::STATUS_OK, ['Content-Type' => 'application/javascript']);
-		$expected->cacheFor(86400);
+		$expected->addHeader('Cache-Control', 'max-age=31536000, immutable');
 		$expires = new \DateTime();
 		$expires->setTimestamp(1337);
-		$expires->add(new \DateInterval('PT24H'));
+		$expires->add(new \DateInterval('PT31536000S'));
 		$expected->addHeader('Expires', $expires->format(\DateTime::RFC1123));
 		$expected->addHeader('Pragma', 'cache');
 
@@ -137,10 +137,10 @@ class JsControllerTest extends TestCase {
 
 		$expected = new FileDisplayResponse($gzipFile, Http::STATUS_OK, ['Content-Type' => 'application/javascript']);
 		$expected->addHeader('Content-Encoding', 'gzip');
-		$expected->cacheFor(86400);
+		$expected->addHeader('Cache-Control', 'max-age=31536000, immutable');
 		$expires = new \DateTime();
 		$expires->setTimestamp(1337);
-		$expires->add(new \DateInterval('PT24H'));
+		$expires->add(new \DateInterval('PT31536000S'));
 		$expected->addHeader('Expires', $expires->format(\DateTime::RFC1123));
 		$expected->addHeader('Pragma', 'cache');
 
@@ -170,10 +170,10 @@ class JsControllerTest extends TestCase {
 			->willReturn('gzip, deflate');
 
 		$expected = new FileDisplayResponse($file, Http::STATUS_OK, ['Content-Type' => 'application/javascript']);
-		$expected->cacheFor(86400);
+		$expected->addHeader('Cache-Control', 'max-age=31536000, immutable');
 		$expires = new \DateTime();
 		$expires->setTimestamp(1337);
-		$expires->add(new \DateInterval('PT24H'));
+		$expires->add(new \DateInterval('PT31536000S'));
 		$expected->addHeader('Expires', $expires->format(\DateTime::RFC1123));
 		$expected->addHeader('Pragma', 'cache');