Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

replaced decorate ThemeFilesystemLoader with more efficient Twig\ChainLoader #71

Open
wants to merge 1 commit into
base: 1.5
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions src/Resources/config/services/integrations/twig.xml
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,14 @@
<services>
<defaults public="true" />

<service id="sylius.theme.twig.loader" class="Sylius\Bundle\ThemeBundle\Twig\ThemeFilesystemLoader" decorates="twig.loader" decoration-priority="256" public="false">
<argument type="service" id="sylius.theme.twig.loader.inner" />
<service id="sylius.theme.twig.loader" class="Twig\Loader\ChainLoader" decorates="twig.loader" decoration-priority="256" public="false">
<argument type="collection">
<argument type="service" id="sylius.theme.twig.theme_loader" />
<argument type="service" id="sylius.theme.twig.loader.inner" />
</argument>
</service>

<service id="sylius.theme.twig.theme_loader" class="Sylius\Bundle\ThemeBundle\Twig\ThemeFilesystemLoader" public="false">
<argument type="service" id="templating.locator" />
<argument type="service" id="templating.name_parser" />
</service>
Expand Down
67 changes: 34 additions & 33 deletions src/Twig/ThemeFilesystemLoader.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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;
}
Expand All @@ -49,40 +48,31 @@ 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);
}

/**
* @param string|TemplateReference $name
*/
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;
}

/**
Expand All @@ -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
{
Expand All @@ -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());
}
}
}
6 changes: 3 additions & 3 deletions tests/Functional/AssetTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand All @@ -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 {
Expand All @@ -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);
}
Expand Down
8 changes: 4 additions & 4 deletions tests/Translation/TranslatorTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
];
}

Expand All @@ -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
];
}

Expand Down