fix(common): perf issues with etag behoind apache with deflate mod (#…

…1084)
ems-project · Nov 29, 2024 · 473c43f · 473c43f
1 parent 98dc5c0
commit 473c43f
Show file tree

Hide file tree

Showing 10 changed files with 141 additions and 29 deletions.
diff --git a/EMS/common-bundle/src/Controller/FileController.php b/EMS/common-bundle/src/Controller/FileController.php
@@ -14,6 +14,8 @@
 use Symfony\Component\HttpFoundation\Response;
 use Symfony\Component\HttpFoundation\ResponseHeaderBag;
 use Symfony\Component\HttpFoundation\StreamedResponse;
+use Symfony\Component\HttpKernel\Exception\HttpException;
+use Symfony\Component\HttpKernel\Exception\NotFoundHttpException;
 
 class FileController extends AbstractController
 {
@@ -88,11 +90,30 @@ public function generateLocalImage(Request $request, string $filename, string $c
         return $response;
     }
 
-    public function assetInArchive(Request $request, string $hash, string $path, int $maxAge = 604800, bool $extract = true, string $indexResource = null): Response
+    public function assetInArchive(Request $request, string $hash, string $path, int $maxAge = 604800, bool $extract = true, string $indexResource = null, string $notFoundTemplate = null): Response
     {
         $this->closeSession($request);
 
-        return $this->processor->getResponseFromArchive($request, $hash, $path, $maxAge, $extract, $indexResource);
+        try {
+            return $this->processor->getResponseFromArchive($request, $hash, $path, $maxAge, $extract, $indexResource);
+        } catch (NotFoundHttpException $e) {
+            if (null === $notFoundTemplate) {
+                throw $e;
+            }
+        }
+
+        try {
+            return $this->render($notFoundTemplate, [
+                'error' => $e,
+                'hash' => $hash,
+                'path' => $path,
+                'maxAge' => $maxAge,
+                'extract' => $extract,
+                'indexResource' => $indexResource,
+            ]);
+        } catch (\Throwable $e) {
+            throw $e->getPrevious() instanceof HttpException ? $e->getPrevious() : $e;
+        }
     }
 
     private function getFile(Request $request, string $hash, string $disposition): Response

diff --git a/EMS/common-bundle/src/Helper/Cache.php b/EMS/common-bundle/src/Helper/Cache.php
@@ -2,6 +2,8 @@
 
 namespace EMS\CommonBundle\Helper;
 
+use EMS\Helpers\Html\Headers;
+use Symfony\Component\HttpFoundation\Request;
 use Symfony\Component\HttpFoundation\Response;
 
 class Cache
@@ -20,12 +22,17 @@ public function generateEtag(Response $response): ?string
         return \hash($this->hashAlgo, $content);
     }
 
-    public function makeResponseCacheable(Response $response, string $etag, ?\DateTime $lastUpdateDate, bool $immutableRoute): void
+    public function makeResponseCacheable(Request $request, Response $response, string $etag, ?\DateTime $lastUpdateDate, bool $immutableRoute, int $maxAge = 600): void
     {
+        $rewritedEtags = [];
+        foreach ($request->getETags() as $requestEtag) {
+            $rewritedEtags[] = \preg_replace('/\-gzip"$/i', '"', $requestEtag);
+        }
+        $request->headers->replace([Headers::IF_NONE_MATCH => $rewritedEtags]);
         $response->setCache([
             'etag' => $etag,
-            'max_age' => $immutableRoute ? 604800 : 600,
-            's_maxage' => $immutableRoute ? 2_678_400 : 3600,
+            'max_age' => $immutableRoute ? 604800 : $maxAge,
+            's_maxage' => $immutableRoute ? 2_678_400 : ($maxAge * 6),
             'public' => true,
             'private' => false,
             'immutable' => $immutableRoute,

diff --git a/EMS/common-bundle/src/Storage/Processor/Processor.php b/EMS/common-bundle/src/Storage/Processor/Processor.php
@@ -76,7 +76,7 @@ public function getStreamedResponse(Request $request, Config $config, string $fi
         $cacheKey = $config->getCacheKey();
 
         $cacheResponse = new Response();
-        $this->cacheHelper->makeResponseCacheable($cacheResponse, $cacheKey, $config->getLastUpdateDate(), $immutableRoute);
+        $this->cacheHelper->makeResponseCacheable($request, $cacheResponse, $cacheKey, $config->getLastUpdateDate(), $immutableRoute);
         if ($cacheResponse->isNotModified($request)) {
             return $cacheResponse;
         }
@@ -98,7 +98,7 @@ public function getStreamedResponse(Request $request, Config $config, string $fi
             ]);
         }
 
-        $this->cacheHelper->makeResponseCacheable($response, $cacheKey, $config->getLastUpdateDate(), $immutableRoute);
+        $this->cacheHelper->makeResponseCacheable($request, $response, $cacheKey, $config->getLastUpdateDate(), $immutableRoute);
 
         return $response;
     }
@@ -341,18 +341,19 @@ private function locate(string $filename): string
 
     public function getResponseFromArchive(Request $request, string $hash, string $path, int $maxAge, bool $extract, ?string $indexResource): Response
     {
+        $etag = \hash('sha1', \sprintf('Asset in archive: %s:%s', $hash, $path));
+        $cacheResponse = new Response();
+        $this->cacheHelper->makeResponseCacheable($request, $cacheResponse, $etag, null, false, $maxAge);
+        if ($cacheResponse->isNotModified($request)) {
+            return $cacheResponse;
+        }
+
         $streamWrapper = $this->storageManager->getStreamFromArchive($hash, $path, $extract, $indexResource);
         $response = $this->getResponseFromStreamInterface($streamWrapper->getStream(), $request);
-        $response->headers->add([
+        $response->headers->replace(\array_merge($cacheResponse->headers->all(), [
             Headers::CONTENT_DISPOSITION => HeaderUtils::DISPOSITION_INLINE.'; '.HeaderUtils::toString(['filename' => \basename($path)], ';'),
             Headers::CONTENT_TYPE => $streamWrapper->getMimetype(),
-        ]);
-        $response->setCache([
-            'etag' => \hash('sha1', \sprintf('Asset in archive: %s:%s', $hash, $path)),
-            'max_age' => $maxAge,
-            'public' => true,
-            'private' => false,
-        ]);
+        ]));
 
         return $response;
     }

diff --git a/EMS/common-bundle/src/Storage/Service/S3Storage.php b/EMS/common-bundle/src/Storage/Service/S3Storage.php
@@ -22,6 +22,7 @@
 class S3Storage extends AbstractUrlStorage
 {
     private ?S3Client $s3Client = null;
+    private bool $streamWrapperRegistered = false;
 
     /**
      * @param array{version?: string, credentials?: array{key: string, secret: string}, region?: string} $credentials
@@ -33,7 +34,10 @@ public function __construct(LoggerInterface $logger, private readonly Cache $cac
 
     protected function getBaseUrl(): string
     {
-        $this->getS3Client();
+        if (!$this->streamWrapperRegistered) {
+            $this->getS3Client()->registerStreamWrapper();
+            $this->streamWrapperRegistered = true;
+        }
 
         return "s3://$this->bucket";
     }
@@ -222,7 +226,6 @@ private function getS3Client(): S3Client
     {
         if (null === $this->s3Client) {
             $this->s3Client = new S3Client($this->credentials);
-            $this->s3Client->registerStreamWrapper();
         }
 
         return $this->s3Client;

diff --git a/EMS/common-bundle/src/Twig/AssetRuntime.php b/EMS/common-bundle/src/Twig/AssetRuntime.php
@@ -14,10 +14,12 @@
 use EMS\Helpers\File\TempDirectory;
 use EMS\Helpers\File\TempFile;
 use EMS\Helpers\Standard\Json;
+use EMS\Helpers\Standard\Type;
 use Psr\Log\LoggerInterface;
 use Symfony\Component\Filesystem\Filesystem;
 use Symfony\Component\Finder\Finder;
 use Symfony\Component\Finder\SplFileInfo;
+use Symfony\Component\HttpKernel\Exception\NotFoundHttpException;
 use Symfony\Component\Routing\Generator\UrlGeneratorInterface;
 
 class AssetRuntime
@@ -197,12 +199,25 @@ public function hash(string $input, ?string $hashAlgo = null, bool $binary = fal
         return $this->storageManager->computeStringHash($input, $hashAlgo, $binary);
     }
 
-    public function fileFromArchive(string $hash, string $path): string
+    /**
+     * @param mixed[] $options
+     */
+    public function fileFromArchive(string $hash, string $path, array $options = []): null|string|TempFile
     {
-        $streamWrapper = $this->storageManager->getStreamFromArchive($hash, $path);
-        $tempFile = TempFile::create();
-        $tempFile->loadFromStream($streamWrapper->getStream());
+        $extract = Type::bool($options['extract'] ?? true);
+        $asTempFile = Type::bool($options['asTempFile'] ?? false);
+        try {
+            $streamWrapper = $this->storageManager->getStreamFromArchive($hash, $path, $extract);
+            $tempFile = TempFile::create();
+            $tempFile->loadFromStream($streamWrapper->getStream());
+        } catch (NotFoundHttpException $e) {
+            if ($extract) {
+                throw $e;
+            }
+
+            return null;
+        }
 
-        return $tempFile->path;
+        return $asTempFile ? $tempFile : $tempFile->path;
     }
 }
diff --git a/EMS/common-bundle/tests/Unit/Helper/CacheAiTest.php b/EMS/common-bundle/tests/Unit/Helper/CacheAiTest.php
@@ -6,6 +6,7 @@
 
 use EMS\CommonBundle\Helper\Cache;
 use PHPUnit\Framework\TestCase;
+use Symfony\Component\HttpFoundation\Request;
 use Symfony\Component\HttpFoundation\Response;
 
 class CacheAiTest extends TestCase
@@ -37,12 +38,13 @@ public function testGenerateEtagWithNonStringContent(): void
 
     public function testMakeResponseCacheable(): void
     {
+        $request = new Request();
         $response = new Response();
         $etag = '"test-etag"';
         $lastUpdateDate = new \DateTime('2023-01-01');
         $immutableRoute = true;
 
-        $this->cache->makeResponseCacheable($response, $etag, $lastUpdateDate, $immutableRoute);
+        $this->cache->makeResponseCacheable($request, $response, $etag, $lastUpdateDate, $immutableRoute);
 
         $this->assertEquals($etag, $response->getEtag());
         $this->assertEquals(2_678_400, $response->getMaxAge());
@@ -52,11 +54,12 @@ public function testMakeResponseCacheable(): void
 
     public function testMakeResponseCacheableWithoutLastUpdateDate(): void
     {
+        $request = new Request();
         $response = new Response();
         $etag = '"test-etag"';
         $immutableRoute = false;
 
-        $this->cache->makeResponseCacheable($response, $etag, null, $immutableRoute);
+        $this->cache->makeResponseCacheable($request, $response, $etag, null, $immutableRoute);
 
         $this->assertEquals($etag, $response->getEtag());
         $this->assertEquals(3600, $response->getMaxAge());

diff --git a/EMS/common-bundle/tests/Unit/Helper/CacheTest.php b/EMS/common-bundle/tests/Unit/Helper/CacheTest.php
@@ -5,7 +5,9 @@
 namespace EMS\CommonBundle\Tests\Unit\Helper;
 
 use EMS\CommonBundle\Helper\Cache;
+use EMS\Helpers\Html\Headers;
 use PHPUnit\Framework\TestCase;
+use Symfony\Component\HttpFoundation\Request;
 use Symfony\Component\HttpFoundation\Response;
 
 class CacheTest extends TestCase
@@ -36,31 +38,56 @@ public function testGenerateEtagShouldReturnHash(): void
 
     public function testMakeResponseCacheableReturnSameEtag(): void
     {
-        $this->cache->makeResponseCacheable($this->response, 'test', null, false);
+        $request = new Request();
+        $this->cache->makeResponseCacheable($request, $this->response, 'test', null, false);
         self::assertSame('"test"', $this->response->getEtag());
     }
 
     public function testMakeResponseCacheableReturnSameMaxAgeFalse(): void
     {
-        $this->cache->makeResponseCacheable($this->response, 'test', null, false);
+        $request = new Request();
+        $this->cache->makeResponseCacheable($request, $this->response, 'test', null, false);
         self::assertSame(3600, $this->response->getMaxAge());
     }
 
     public function testMakeResponseCacheableReturnSameMaxAgeTrue(): void
     {
-        $this->cache->makeResponseCacheable($this->response, 'test', null, true);
+        $request = new Request();
+        $this->cache->makeResponseCacheable($request, $this->response, 'test', null, true);
         self::assertSame(2_678_400, $this->response->getMaxAge());
     }
 
     public function testMakeResponseCacheableReturnSameLastUpdateDateNotNull(): void
     {
-        $this->cache->makeResponseCacheable($this->response, 'test', null, false);
+        $request = new Request();
+        $this->cache->makeResponseCacheable($request, $this->response, 'test', null, false);
         self::assertSame(null, $this->response->getLastModified());
     }
 
     public function testMakeResponseCacheableReturnSameImmutableRoute(): void
     {
-        $this->cache->makeResponseCacheable($this->response, 'test', null, false);
+        $request = new Request();
+        $this->cache->makeResponseCacheable($request, $this->response, 'test', null, false);
         self::assertSame(true, !$this->response->isImmutable());
     }
+
+    public function testEtagIsNotModified(): void
+    {
+        $request = new Request();
+        $request->headers->replace([
+            Headers::IF_NONE_MATCH => '"test"',
+        ]);
+        $this->cache->makeResponseCacheable($request, $this->response, 'test', null, false);
+        self::assertTrue($this->response->isNotModified($request));
+    }
+
+    public function testEtagIsNotModifiedBehindApacheWithDeflateMod(): void
+    {
+        $request = new Request();
+        $request->headers->replace([
+            Headers::IF_NONE_MATCH => '"test-gzip"',
+        ]);
+        $this->cache->makeResponseCacheable($request, $this->response, 'test', null, false);
+        self::assertTrue($this->response->isNotModified($request));
+    }
 }
diff --git a/EMS/helpers/src/Html/Headers.php b/EMS/helpers/src/Html/Headers.php
@@ -15,4 +15,5 @@ class Headers
     public const WWW_AUTHENTICATE = 'WWW-Authenticate';
     public const SET_COOKIE = 'set-cookie';
     public const COOKIE = 'cookie';
+    public const IF_NONE_MATCH = 'if-none-match';
 }
diff --git a/docs/dev/client-helper-bundle/twig.md b/docs/dev/client-helper-bundle/twig.md
@@ -159,3 +159,23 @@ Some repetitive computes can also be cached. For that you may call the `cacheabl
 ```
 
 The `cacheType` parameter is the content type's name used to invalidate the response in cache (if something has been updated) 
+
+
+## emsch_http_error
+
+Cancel the curent rendering an return the specified HTTP return code. Useful to redirect to another URL.
+
+Return a 404:
+
+```twig
+{% do emsch_http_error(404, 'Page not found') %}
+```
+
+Redirect to Symfony:
+
+```twig
+  {% do emsch_http_error(307, "Temporary Redirect to #{url}", {
+      location: url,
+  }) %}
+  ```
+
diff --git a/docs/dev/common-bundle/twig.md b/docs/dev/common-bundle/twig.md
@@ -383,6 +383,20 @@ Returns a path to a temporary asset extracted from an archive (a zip file). Usef
     {% set path = ems_file_from_archive('253b903b1fb3ac30975ae9844a0352a65cdcfa3d', 'img/logos/full-logo.svg') %}
 ````
 
+This function also accept extra options:
+
+ * `extract`: Specify that the function can not try to re-extract the archive if the searched path is missing. If disabled the function won't throw an error but `null`.  Default value: `true`.
+ * `asTempFile`: Returns a [EMS\Helpers\File\TempFile](https://github.com/ems-project/elasticms/blob/5.x/EMS/helpers/src/File/TempFile.php) object when activated, a path to the temporary file instead. A TempFile is useful to get file's contents with the member function `getContents()`. Default value `false`
+
+Example: 
+
+```twig
+{% set tempFile = ems_file_from_archive(hash, "#{path}/index.php", {
+    extract: false,
+    asTempFile: true,
+}) %}
+```
+
 ## ems_link
 
 Return the [EMSLink](https://github.com/ems-project/elasticms/blob/HEAD/EMS/common-bundle/src/Common/EMSLink.php) object