perf(image-cache): let the browser cache image previews

Controller/ManagerController.php

@@ -289,7 +289,11 @@ public function binaryFileResponseAction(Request $request, string $fileName): Bi
         }
         $this->dispatch(FileManagerEvents::FILE_ACCESS, ['path' => $file]);
 
-        return new BinaryFileResponse($file);
+        $response = new BinaryFileResponse($file);
+        // Last-Modified is set by default; answer revalidation requests with 304 instead of resending the file
+        $response->isNotModified($request);
+
+        return $response;
     }
 
     #[Route("/delete/", name: 'file_manager_delete')]

Resources/doc/book/1-basic-configuration.md

@@ -163,7 +163,7 @@ artgris_file_manager:
 | :---  |:--------:|:--------:|:-------------:|
 | `cachebreaker`  | `Boolean` |  False   | True |
 
-Adding a cachebreaker ?time=RANDOM_NUMBER or & &time=RANDOM_NUMBER at the end of the images (preview) url. It's removed if you have used "twig_extension" option.
+Adding a cachebreaker ?time=FILE_MODIFICATION_TIME or &time=FILE_MODIFICATION_TIME at the end of the images (preview) url. The url changes only when the file changes, so the browser can cache previews between page loads. It's removed if you have used "twig_extension" option.
 
 ## `show_file_count` to show the number of files per directory in the file browser tree
 

Service/FileTypeService.php

@@ -34,7 +34,10 @@ public function preview(FileManager $fileManager, SplFileInfo $file) {
         if ('file' === $type) {
             $size = $this::IMAGE_SIZE[$fileManager->getView()];
 
-            return $this->fileIcon($filePath, $extension, $size, true, $fileManager->getConfigurationParameter('twig_extension'), $fileManager->getConfigurationParameter('cachebreaker'));
+            // The modification time keeps the URL stable until the file changes, so the browser can cache it
+            $cachebreaker = $fileManager->getConfigurationParameter('cachebreaker') ? $file->getMTime() : null;
+
+            return $this->fileIcon($filePath, $extension, $size, true, $fileManager->getConfigurationParameter('twig_extension'), $cachebreaker);
         }
         if ('dir' === $type) {
 
@@ -68,7 +71,7 @@ public function accept($type): bool|string {
         return $accept;
     }
 
-    public function fileIcon(string $filePath,?string $extension = null, ?int $size = 75, ?bool $lazy = false, ?string $twigExtension = null, ?bool $cachebreaker = null): array {
+    public function fileIcon(string $filePath,?string $extension = null, ?int $size = 75, ?bool $lazy = false, ?string $twigExtension = null, bool|int|null $cachebreaker = null): array {
 
         $imageTemplate = null;
 
@@ -89,7 +92,8 @@ public function fileIcon(string $filePath,?string $extension = null, ?int $size
                 $fileName = $filePath;
                 if ($cachebreaker) {
                     $query = parse_url($filePath, PHP_URL_QUERY);
-                    $time = 'time='.time();
+                    // An int is used as the version (file modification time), true falls back to the current time
+                    $time = 'time='.(\is_int($cachebreaker) ? $cachebreaker : time());
                     $fileName = $query ? $filePath.'&'.$time : $filePath.'?'.$time;
                 }
 

Tests/Controller/CachebreakerTest.php

@@ -0,0 +1,32 @@
+<?php
+
+namespace Artgris\Bundle\FileManagerBundle\Tests\Controller;
+
+use Artgris\Bundle\FileManagerBundle\Tests\Fixtures\AbstractTestCase;
+
+class CachebreakerTest extends AbstractTestCase
+{
+    public function setUp(): void
+    {
+        $this->initClient(['environment' => 'cachebreaker']);
+    }
+
+    public function testImagePreviewCachebreakerIsFileModificationTime()
+    {
+        // A fixed date in the past, so a cachebreaker built from time() cannot match by accident
+        touch(__DIR__.'/../Fixtures/web/images/image.png', 1600000000);
+
+        $crawler = $this->getManagerPage();
+        $this->assertSame('/manager/file/image.png?conf=default&time=1600000000', $crawler->filter('img.lazy')->attr('data-src'));
+    }
+
+    public function testFileIsNotResentWhenNotModified()
+    {
+        $image = __DIR__.'/../Fixtures/web/images/image.png';
+        touch($image, 1600000000);
+
+        // Only the 304 path is requested: a 200 guesses the MIME type, which needs ext-fileinfo (missing on the Windows CI)
+        $this->client->request('GET', '/manager/file/image.png?conf=default', [], [], ['HTTP_IF_MODIFIED_SINCE' => gmdate('D, d M Y H:i:s', 1600000000).' GMT']);
+        $this->assertSame(304, $this->client->getResponse()->getStatusCode());
+    }
+}

Tests/Fixtures/App/config/config_cachebreaker.yml

@@ -0,0 +1,8 @@
+imports:
+    - { resource: config.yml }
+
+artgris_file_manager:
+    conf:
+        default:
+            dir: "%kernel.project_dir%/Tests/Fixtures/web/images"
+            cachebreaker: true