From c1419d2eefa967c4dc4534de8c7c7fce3fae0a14 Mon Sep 17 00:00:00 2001 From: Florent Morselli Date: Sun, 2 Jun 2024 11:38:16 +0200 Subject: [PATCH] Refactor file compiler services to use iterable (#214) The file compiler services have been refactored to use iterable instead of arrays. This includes the FaviconsCompiler, ManifestCompiler, and ServiceWorkerCompiler. Now, the getFiles() method of these services returns iterable, which improves the application's efficiency by avoiding the need to load all the data into memory at once. --- phpstan-baseline.neon | 47 +++++++++++++++++++++- phpunit.xml.dist | 1 + src/DataCollector/PwaCollector.php | 31 ++++++++++----- src/Service/FaviconsCompiler.php | 45 ++++++--------------- src/Service/FileCompilerInterface.php | 2 +- src/Service/ManifestCompiler.php | 56 ++++++++++++--------------- src/Service/ServiceWorkerCompiler.php | 56 +++++++++++++++------------ 7 files changed, 136 insertions(+), 102 deletions(-) diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index 0e60f4e..27b867b 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -20,6 +20,11 @@ parameters: count: 1 path: src/CachingStrategy/AssetCache.php + - + message: "#^Attribute class Symfony\\\\Component\\\\DependencyInjection\\\\Attribute\\\\TaggedIterator is deprecated\\: since Symfony 7\\.1, use \\{@see AutowireIterator\\} instead\\.$#" + count: 1 + path: src/CachingStrategy/BackgroundSync.php + - message: "#^Only iterables can be unpacked, mixed given in argument \\#1\\.$#" count: 1 @@ -35,6 +40,16 @@ parameters: count: 2 path: src/CachingStrategy/FontCache.php + - + message: "#^Attribute class Symfony\\\\Component\\\\DependencyInjection\\\\Attribute\\\\TaggedIterator is deprecated\\: since Symfony 7\\.1, use \\{@see AutowireIterator\\} instead\\.$#" + count: 1 + path: src/CachingStrategy/PreloadUrlsGeneratorManager.php + + - + message: "#^Attribute class Symfony\\\\Component\\\\DependencyInjection\\\\Attribute\\\\TaggedIterator is deprecated\\: since Symfony 7\\.1, use \\{@see AutowireIterator\\} instead\\.$#" + count: 1 + path: src/CachingStrategy/ResourceCaches.php + - message: "#^Only iterables can be unpacked, mixed given in argument \\#1\\.$#" count: 1 @@ -120,6 +135,16 @@ parameters: count: 1 path: src/Command/CreateScreenshotCommand.php + - + message: "#^Attribute class Symfony\\\\Component\\\\DependencyInjection\\\\Attribute\\\\TaggedIterator is deprecated\\: since Symfony 7\\.1, use \\{@see AutowireIterator\\} instead\\.$#" + count: 1 + path: src/Command/ListCacheStrategiesCommand.php + + - + message: "#^Attribute class Symfony\\\\Component\\\\DependencyInjection\\\\Attribute\\\\TaggedIterator is deprecated\\: since Symfony 7\\.1, use \\{@see AutowireIterator\\} instead\\.$#" + count: 1 + path: src/DataCollector/PwaCollector.php + - message: "#^Class SpomkyLabs\\\\PwaBundle\\\\Dto\\\\BackgroundSync has an uninitialized property \\$forceSyncFallback\\. Give it default value or assign it in the constructor\\.$#" count: 1 @@ -580,6 +605,16 @@ parameters: count: 1 path: src/Service/FaviconsCompiler.php + - + message: "#^Attribute class Symfony\\\\Component\\\\DependencyInjection\\\\Attribute\\\\TaggedIterator is deprecated\\: since Symfony 7\\.1, use \\{@see AutowireIterator\\} instead\\.$#" + count: 1 + path: src/Service/ServiceWorkerCompiler.php + + - + message: "#^Attribute class Symfony\\\\Component\\\\DependencyInjection\\\\Attribute\\\\TaggedIterator is deprecated\\: since Symfony 7\\.1, use \\{@see AutowireIterator\\} instead\\.$#" + count: 1 + path: src/ServiceWorkerRule/AppendCacheStrategies.php + - message: "#^Parameter \\#2 \\.\\.\\.\\$values of function sprintf expects bool\\|float\\|int\\|string\\|null, mixed given\\.$#" count: 1 @@ -593,4 +628,14 @@ parameters: - message: "#^Method SpomkyLabs\\\\PwaBundle\\\\SpomkyLabsPwaBundle\\:\\:loadExtension\\(\\) has parameter \\$config with no value type specified in iterable type array\\.$#" count: 1 - path: src/SpomkyLabsPwaBundle.php \ No newline at end of file + path: src/SpomkyLabsPwaBundle.php + + - + message: "#^Attribute class Symfony\\\\Component\\\\DependencyInjection\\\\Attribute\\\\TaggedIterator is deprecated\\: since Symfony 7\\.1, use \\{@see AutowireIterator\\} instead\\.$#" + count: 1 + path: src/Subscriber/FileCompileEventListener.php + + - + message: "#^Attribute class Symfony\\\\Component\\\\DependencyInjection\\\\Attribute\\\\TaggedIterator is deprecated\\: since Symfony 7\\.1, use \\{@see AutowireIterator\\} instead\\.$#" + count: 1 + path: src/Subscriber/PwaDevServerSubscriber.php \ No newline at end of file diff --git a/phpunit.xml.dist b/phpunit.xml.dist index 4772da6..e286cda 100644 --- a/phpunit.xml.dist +++ b/phpunit.xml.dist @@ -18,6 +18,7 @@ + diff --git a/src/DataCollector/PwaCollector.php b/src/DataCollector/PwaCollector.php index e53765c..0fb7862 100644 --- a/src/DataCollector/PwaCollector.php +++ b/src/DataCollector/PwaCollector.php @@ -66,29 +66,24 @@ public function collect(Request $request, Response $response, Throwable $excepti $this->data['serviceWorker'] = [ 'enabled' => $this->serviceWorker->enabled, 'data' => $this->serviceWorker, - 'files' => $swFiles, + 'files' => $this->dataToFiles($swFiles), ]; $manifestFiles = $this->manifestCompiler->getFiles(); + $manifestFiles = is_array($manifestFiles) ? $manifestFiles : iterator_to_array($manifestFiles); $this->data['manifest'] = [ 'enabled' => $this->serviceWorker->enabled, 'data' => $this->manifest, 'installable' => $this->isInstallable(), 'output' => $this->serializer->serialize($this->manifest, 'json', $jsonOptions), - 'files' => $manifestFiles, + 'files' => $this->dataToFiles($manifestFiles), ]; $faviconsFiles = $this->faviconsCompiler->getFiles(); + $faviconsFiles = is_array($faviconsFiles) ? $faviconsFiles : iterator_to_array($faviconsFiles); $this->data['favicons'] = [ 'enabled' => $this->favicons->enabled, 'data' => $this->favicons, - 'files' => array_map( - static fn (\SpomkyLabs\PwaBundle\Service\Data $data): array => [ - 'url' => $data->url, - 'headers' => $data->headers, - 'html' => $data->html, - ], - $faviconsFiles - ), + 'files' => $this->dataToFiles($faviconsFiles), ]; } @@ -157,6 +152,22 @@ public function getName(): string return 'pwa'; } + /** + * @param \SpomkyLabs\PwaBundle\Service\Data[] $data + * @return array{url: string, html: string|null, headers: array}[] + */ + private function dataToFiles(array $data): array + { + return array_map( + static fn (\SpomkyLabs\PwaBundle\Service\Data $data): array => [ + 'url' => $data->url, + 'headers' => $data->headers, + 'html' => $data->html, + ], + $data + ); + } + /** * @return array{status: bool, reasons: array} */ diff --git a/src/Service/FaviconsCompiler.php b/src/Service/FaviconsCompiler.php index 02394bf..b5c4e10 100644 --- a/src/Service/FaviconsCompiler.php +++ b/src/Service/FaviconsCompiler.php @@ -19,11 +19,6 @@ final class FaviconsCompiler implements FileCompilerInterface, CanLogInterface { - /** - * @var null|array - */ - private null|array $files = null; - private LoggerInterface $logger; public function __construct( @@ -37,25 +32,21 @@ public function __construct( } /** - * @return array + * @return iterable */ - public function getFiles(): array + public function getFiles(): iterable { - if ($this->files !== null) { - return $this->files; - } $this->logger->debug('Compiling favicons.', [ 'favicons' => $this->favicons, ]); if ($this->imageProcessor === null || $this->favicons->enabled === false) { $this->logger->debug('Favicons are disabled or no image processor is available.'); - $this->files = []; + yield from []; - return $this->files; + return; } [$asset, $hash] = $this->getFavicon(); assert($asset !== null, 'The asset does not exist.'); - $this->files = []; $sizes = [ //Always [ @@ -243,27 +234,16 @@ public function getFiles(): array ); $completeHash = hash('xxh128', $hash . $configuration); $filename = sprintf($size['url'], $size['width'], $size['height'], $completeHash); - $this->files[$filename] = $this->processIcon( - $asset, - $filename, - $configuration, - $size['mimetype'], - $size['rel'], - ); + yield $this->processIcon($asset, $filename, $configuration, $size['mimetype'], $size['rel']); } if ($this->favicons->tileColor !== null) { $this->logger->debug('Creating browserconfig.xml.'); - $this->files = [...$this->files, ...$this->processBrowserConfig($asset, $hash)]; + yield from $this->processBrowserConfig($asset, $hash); } if ($this->favicons->safariPinnedTabColor !== null && $this->favicons->useSilhouette === true) { - $safariPinnedTab = $this->generateSafariPinnedTab($asset); - $this->files[$safariPinnedTab->url] = $safariPinnedTab; + yield $this->generateSafariPinnedTab($asset, $hash); } - $this->logger->debug('Favicons created.', [ - 'files' => $this->files, - ]); - - return $this->files; + $this->logger->debug('Favicons created.'); } public function setLogger(LoggerInterface $logger): void @@ -330,7 +310,7 @@ private function processIcon( */ private function processBrowserConfig(string $asset, string $hash): array { - if ($this->favicons->useSilhouette === true) { + if ($this->favicons->useSilhouette === true && $this->debug === false) { $asset = $this->generateSilhouette($asset); } $this->logger->debug('Processing browserconfig.xml.'); @@ -456,15 +436,14 @@ private function getFavicon(): array return [$data, $hash]; } - private function generateSafariPinnedTab(string $content): Data + private function generateSafariPinnedTab(string $content, string $hash): Data { - $silhouette = $this->generateSilhouette($content); - $hash = hash('xxh128', $silhouette); + $callback = fn (): string => $this->generateSilhouette($content); $url = sprintf('/safari-pinned-tab-%s.svg', $hash); return Data::create( $url, - $silhouette, + $callback, [ 'Cache-Control' => 'public, max-age=604800, immutable', 'Content-Type' => 'image/svg+xml', diff --git a/src/Service/FileCompilerInterface.php b/src/Service/FileCompilerInterface.php index bd88c1b..a119a53 100644 --- a/src/Service/FileCompilerInterface.php +++ b/src/Service/FileCompilerInterface.php @@ -7,7 +7,7 @@ interface FileCompilerInterface { /** - * @return iterable + * @return iterable */ public function getFiles(): iterable; } diff --git a/src/Service/ManifestCompiler.php b/src/Service/ManifestCompiler.php index 07e465c..021e903 100644 --- a/src/Service/ManifestCompiler.php +++ b/src/Service/ManifestCompiler.php @@ -36,11 +36,6 @@ final class ManifestCompiler implements FileCompilerInterface, CanLogInterface private LoggerInterface $logger; - /** - * @var array - */ - private null|array $files = null; - /** * @param array $locales */ @@ -71,38 +66,30 @@ public function __construct( } /** - * @return array + * @return iterable */ - public function getFiles(): array + public function getFiles(): iterable { - if ($this->files !== null) { - return $this->files; - } $this->logger->debug('Compiling manifest files.', [ 'manifest' => $this->manifest, ]); if ($this->manifest->enabled === false) { $this->logger->debug('Manifest is disabled. No file to compile.'); - $this->files = []; + yield from []; - return $this->files; + return; } - $this->files = []; if ($this->locales === []) { $this->logger->debug('No locale defined. Compiling default manifest.'); - $this->files[$this->manifestPublicUrl] = $this->compileManifest(null); + yield $this->compileManifest(null); } foreach ($this->locales as $locale) { $this->logger->debug('Compiling manifest for locale.', [ 'locale' => $locale, ]); - $this->files[str_replace('{locale}', $locale, $this->manifestPublicUrl)] = $this->compileManifest($locale); + yield $this->compileManifest($locale); } - $this->logger->debug('Manifest files compiled.', [ - 'files' => $this->files, - ]); - - return $this->files; + $this->logger->debug('Manifest files compiled.'); } public function setLogger(LoggerInterface $logger): void @@ -116,30 +103,35 @@ private function compileManifest(null|string $locale): Data 'locale' => $locale, ]); $manifest = clone $this->manifest; - $preEvent = new PreManifestCompileEvent($manifest); - $preEvent = $this->dispatcher->dispatch($preEvent); - assert($preEvent instanceof PreManifestCompileEvent); - - $options = $this->jsonOptions; $manifestPublicUrl = $this->manifestPublicUrl; if ($locale !== null) { - $options[TranslatableNormalizer::NORMALIZATION_LOCALE_KEY] = $locale; $manifestPublicUrl = str_replace('{locale}', $locale, $this->manifestPublicUrl); } - $data = $this->serializer->serialize($preEvent->manifest, 'json', $options); - $postEvent = new PostManifestCompileEvent($manifest, $data); - $postEvent = $this->dispatcher->dispatch($postEvent); - assert($postEvent instanceof PostManifestCompileEvent); + $callback = function () use ($manifest, $locale): string { + $preEvent = new PreManifestCompileEvent($manifest); + $preEvent = $this->dispatcher->dispatch($preEvent); + assert($preEvent instanceof PreManifestCompileEvent); + + $options = $this->jsonOptions; + if ($locale !== null) { + $options[TranslatableNormalizer::NORMALIZATION_LOCALE_KEY] = $locale; + } + $data = $this->serializer->serialize($preEvent->manifest, 'json', $options); + $postEvent = new PostManifestCompileEvent($manifest, $data); + $postEvent = $this->dispatcher->dispatch($postEvent); + assert($postEvent instanceof PostManifestCompileEvent); + + return $postEvent->data; + }; return Data::create( $manifestPublicUrl, - $postEvent->data, + $callback, [ 'Cache-Control' => 'public, max-age=604800, immutable', 'Content-Type' => 'application/manifest+json', 'X-Manifest-Dev' => true, - 'Etag' => hash('xxh128', $data), ] ); } diff --git a/src/Service/ServiceWorkerCompiler.php b/src/Service/ServiceWorkerCompiler.php index ad58520..9ffacb2 100644 --- a/src/Service/ServiceWorkerCompiler.php +++ b/src/Service/ServiceWorkerCompiler.php @@ -53,11 +53,11 @@ public function __construct( } /** - * @return iterable + * @return iterable */ public function getFiles(): iterable { - yield $this->serviceWorkerPublicUrl => $this->compileSW(); + yield $this->compileSW(); yield from $this->getWorkboxFiles(); } @@ -69,30 +69,33 @@ public function setLogger(LoggerInterface $logger): void private function compileSW(): Data { $this->logger->debug('Service Worker compilation started.'); - $body = ''; - - foreach ($this->serviceworkerRules as $rule) { - $this->logger->debug('Processing service worker rule.', [ - 'rule' => $rule::class, - ]); - $ruleBody = $rule->process($this->debug); - if ($this->debug === false) { - $ruleBody = trim($ruleBody); + $callback = function (): string { + $body = ''; + + foreach ($this->serviceworkerRules as $rule) { + $this->logger->debug('Processing service worker rule.', [ + 'rule' => $rule::class, + ]); + $ruleBody = $rule->process($this->debug); + if ($this->debug === false) { + $ruleBody = trim($ruleBody); + } + $body .= $ruleBody; } - $body .= $ruleBody; - } - $body .= $this->includeRootSW(); - $this->logger->debug('Service Worker compilation completed.', [ - 'body' => $body, - ]); + $body .= $this->includeRootSW(); + $this->logger->debug('Service Worker compilation completed.', [ + 'body' => $body, + ]); + + return $body; + }; return Data::create( $this->serviceWorkerPublicUrl, - $body, + $callback, [ 'Content-Type' => 'application/javascript', 'X-SW-Dev' => true, - 'Etag' => hash('xxh128', $body), ] ); } @@ -115,7 +118,7 @@ private function includeRootSW(): string } /** - * @return iterable + * @return iterable */ private function getWorkboxFiles(): iterable { @@ -147,7 +150,7 @@ private function getWorkboxFiles(): iterable if ($data === null) { continue; } - yield $publicUrl => $data; + yield $data; } } @@ -171,16 +174,19 @@ private function getWorkboxFile(string $publicUrl): null|Data return null; } - $body = file_get_contents($resourcePath); - assert(is_string($body), 'Unable to load the file content.'); + $callback = function () use ($resourcePath): string { + $body = file_get_contents($resourcePath); + assert(is_string($body), 'Unable to load the file content.'); + + return $body; + }; return Data::create( $publicUrl, - $body, + $callback, [ 'Content-Type' => 'application/javascript', 'X-SW-Dev' => true, - 'Etag' => hash('xxh128', $body), ] ); }