From 61c6002d1ee35f4b96bd724ace0a7548cd16f9d5 Mon Sep 17 00:00:00 2001 From: Justus Krapp Date: Sun, 15 Nov 2020 19:58:11 +0100 Subject: [PATCH] replaced decorate ThemeFilesystemLoader with more efficient twig chain loader --- .../config/services/integrations/twig.xml | 10 ++- src/Twig/ThemeFilesystemLoader.php | 67 ++++++++++--------- tests/Functional/AssetTest.php | 6 +- tests/Translation/TranslatorTest.php | 8 +-- 4 files changed, 49 insertions(+), 42 deletions(-) diff --git a/src/Resources/config/services/integrations/twig.xml b/src/Resources/config/services/integrations/twig.xml index 96b03c78..6a3d6575 100644 --- a/src/Resources/config/services/integrations/twig.xml +++ b/src/Resources/config/services/integrations/twig.xml @@ -15,8 +15,14 @@ - - + + + + + + + + diff --git a/src/Twig/ThemeFilesystemLoader.php b/src/Twig/ThemeFilesystemLoader.php index 54614373..094d97c9 100644 --- a/src/Twig/ThemeFilesystemLoader.php +++ b/src/Twig/ThemeFilesystemLoader.php @@ -16,15 +16,13 @@ use Symfony\Component\Config\FileLocatorInterface; use Symfony\Component\Templating\TemplateNameParserInterface; use Symfony\Component\Templating\TemplateReference; +use Twig\Error\LoaderError; use Twig\Loader\ExistsLoaderInterface; use Twig\Loader\LoaderInterface; use Twig\Source; final class ThemeFilesystemLoader implements LoaderInterface, ExistsLoaderInterface { - /** @var LoaderInterface */ - private $decoratedLoader; - /** @var FileLocatorInterface */ private $templateLocator; @@ -34,12 +32,13 @@ final class ThemeFilesystemLoader implements LoaderInterface, ExistsLoaderInterf /** @var string[] */ private $cache = []; + /** @var string[] */ + private $errorCache = []; + public function __construct( - LoaderInterface $decoratedLoader, FileLocatorInterface $templateLocator, TemplateNameParserInterface $templateNameParser ) { - $this->decoratedLoader = $decoratedLoader; $this->templateLocator = $templateLocator; $this->templateNameParser = $templateNameParser; } @@ -49,14 +48,9 @@ public function __construct( */ public function getSourceContext($name): Source { - try { - $path = $this->findTemplate($name); + $path = $this->findTemplate($name); - return new Source((string) file_get_contents($path), (string) $name, $path); - } catch (\Exception $exception) { - /** @psalm-suppress ImplicitToStringCast */ - return $this->decoratedLoader->getSourceContext($name); - } + return new Source((string) file_get_contents($path), (string) $name, $path); } /** @@ -64,25 +58,21 @@ public function getSourceContext($name): Source */ public function getCacheKey($name): string { - try { - return $this->findTemplate($name); - } catch (\Exception $exception) { - /** @psalm-suppress ImplicitToStringCast */ - return $this->decoratedLoader->getCacheKey($name); - } + return $this->findTemplate($name); } /** * @param string|TemplateReference $name + * + * @throws LoaderError */ public function isFresh($name, $time): bool { - try { - return filemtime($this->findTemplate($name)) <= $time; - } catch (\Exception $exception) { - /** @psalm-suppress ImplicitToStringCast */ - return $this->decoratedLoader->isFresh($name, $time); + if (!$path = $this->findTemplate($name)) { + return false; } + + return filemtime($path) < $time; } /** @@ -92,14 +82,15 @@ public function exists($name): bool { try { return stat($this->findTemplate($name)) !== false; - } catch (\Exception $exception) { - /** @psalm-suppress ImplicitToStringCast */ - return $this->decoratedLoader->exists($name); + } catch (LoaderError $e) { + return false; } } /** * @param string|TemplateReference $logicalName + * + * @throws LoaderError */ private function findTemplate($logicalName): string { @@ -109,14 +100,24 @@ private function findTemplate($logicalName): string return $this->cache[$logicalName]; } - $template = $this->templateNameParser->parse($logicalName); + if (isset($this->errorCache[$logicalName])) { + throw new LoaderError($this->errorCache[$logicalName]); + } - /** - * @var string - * @psalm-suppress ImplicitToStringCast - */ - $file = $this->templateLocator->locate($template); + try { + $template = $this->templateNameParser->parse($logicalName); - return $this->cache[$logicalName] = $file; + /** + * @var string + * @psalm-suppress ImplicitToStringCast + */ + $file = $this->templateLocator->locate($template); + + return $this->cache[$logicalName] = $file; + } catch (\Exception $e) { + $this->errorCache[$logicalName] = $e->getMessage(); + + throw new LoaderError($e->getMessage()); + } } } diff --git a/tests/Functional/AssetTest.php b/tests/Functional/AssetTest.php index ddc814c7..e237da50 100644 --- a/tests/Functional/AssetTest.php +++ b/tests/Functional/AssetTest.php @@ -33,7 +33,7 @@ public function it_dumps_assets(int $symlinkMask): void $this->getThemeAssetsInstaller($client)->installAssets($webDirectory, $symlinkMask); $crawler = $client->request('GET', '/template/:Asset:assetsTest.txt.twig'); - $lines = explode("\n", $crawler->text()); + $lines = explode("\n", $crawler->text(null, false)); $this->assertFileContent($lines, $webDirectory); } @@ -59,7 +59,7 @@ public function it_updates_dumped_assets_if_they_are_modified(int $symlinkMask): $this->getThemeAssetsInstaller($client)->installAssets($webDirectory, $symlinkMask); $crawler = $client->request('GET', '/template/:Asset:modifiedAssetsTest.txt.twig'); - $lines = explode("\n", $crawler->text()); + $lines = explode("\n", $crawler->text(null, false)); $this->assertFileContent($lines, $webDirectory); } finally { @@ -81,7 +81,7 @@ public function it_dumps_assets_correctly_even_if_nothing_has_changed(int $symli $this->getThemeAssetsInstaller($client)->installAssets($webDirectory, $symlinkMask); $crawler = $client->request('GET', '/template/:Asset:assetsTest.txt.twig'); - $lines = explode("\n", $crawler->text()); + $lines = explode("\n", $crawler->text(null, false)); $this->assertFileContent($lines, $webDirectory); } diff --git a/tests/Translation/TranslatorTest.php b/tests/Translation/TranslatorTest.php index fe910305..91878148 100644 --- a/tests/Translation/TranslatorTest.php +++ b/tests/Translation/TranslatorTest.php @@ -310,9 +310,9 @@ public function getThemedLocalesTests() ['francais@heron'], ['FR@heron'], ['frFR@heron'], - ['fr-FR@heron'], + // ['fr-FR@heron'], // no longer simple locale as Translator::computeFallbackLocales detects a fallback now ['fr.FR@heron'], - ['fr-FR.UTF8@heron'], + // ['fr-FR.UTF8@heron'], // no longer simple locale as Translator::computeFallbackLocales detects a fallback now ]; } @@ -327,9 +327,9 @@ public function getThemelessLocalesTests() ['francais'], ['FR'], ['frFR'], - ['fr-FR'], + // ['fr-FR'], // no longer simple locale as Translator::computeFallbackLocales detects a fallback now ['fr.FR'], - ['fr-FR.UTF8'], + // ['fr-FR.UTF8'], // no longer simple locale as Translator::computeFallbackLocales detects a fallback now ]; }