Skip to content

Commit

Permalink
Merge pull request #9804 from pamil/1.3-themes-killme
Browse files Browse the repository at this point in the history
[ThemeBundle] Add support for Twig namespaced paths and "templates/" top-level directory
  • Loading branch information
Zales0123 authored Oct 11, 2018
2 parents 6292042 + e5a5042 commit 7fe2fe3
Show file tree
Hide file tree
Showing 31 changed files with 125 additions and 76 deletions.
79 changes: 34 additions & 45 deletions src/Sylius/Bundle/ThemeBundle/Locator/BundleResourceLocator.php
Original file line number Diff line number Diff line change
Expand Up @@ -19,20 +19,12 @@

final class BundleResourceLocator implements ResourceLocatorInterface
{
/**
* @var Filesystem
*/
/** @var Filesystem */
private $filesystem;

/**
* @var KernelInterface
*/
/** @var KernelInterface */
private $kernel;

/**
* @param Filesystem $filesystem
* @param KernelInterface $kernel
*/
public function __construct(Filesystem $filesystem, KernelInterface $kernel)
{
$this->filesystem = $filesystem;
Expand All @@ -41,15 +33,35 @@ public function __construct(Filesystem $filesystem, KernelInterface $kernel)

/**
* {@inheritdoc}
*
* @param string $resourcePath Eg. "@AcmeBundle/Resources/views/template.html.twig"
*/
public function locateResource(string $resourcePath, ThemeInterface $theme): string
{
$this->assertResourcePathIsValid($resourcePath);

$bundleName = $this->getBundleNameFromResourcePath($resourcePath);
$resourceName = $this->getResourceNameFromResourcePath($resourcePath);
if (false !== strpos($resourcePath, 'Bundle/Resources/views/')) {
// When using bundle notation, we get a path like @AcmeBundle/Resources/views/template.html.twig
return $this->locateResourceBasedOnBundleNotation($resourcePath, $theme);
}

// When using namespaced Twig paths, we get a path like @Acme/template.html.twig
return $this->locateResourceBasedOnTwigNamespace($resourcePath, $theme);
}

private function assertResourcePathIsValid(string $resourcePath): void
{
if (0 !== strpos($resourcePath, '@')) {
throw new \InvalidArgumentException(sprintf('Bundle resource path (given "%s") should start with an "@".', $resourcePath));
}

if (false !== strpos($resourcePath, '..')) {
throw new \InvalidArgumentException(sprintf('File name "%s" contains invalid characters (..).', $resourcePath));
}
}

private function locateResourceBasedOnBundleNotation(string $resourcePath, ThemeInterface $theme): string
{
$bundleName = substr($resourcePath, 1, strpos($resourcePath, '/') - 1);
$resourceName = substr($resourcePath, strpos($resourcePath, 'Resources/') + strlen('Resources/'));

// Symfony 4.0+ always returns a single bundle
$bundles = $this->kernel->getBundle($bundleName, false);
Expand All @@ -70,41 +82,18 @@ public function locateResource(string $resourcePath, ThemeInterface $theme): str
throw new ResourceNotFoundException($resourcePath, $theme);
}

/**
* @param string $resourcePath
*/
private function assertResourcePathIsValid(string $resourcePath): void
private function locateResourceBasedOnTwigNamespace(string $resourcePath, ThemeInterface $theme): string
{
if (0 !== strpos($resourcePath, '@')) {
throw new \InvalidArgumentException(sprintf('Bundle resource path (given "%s") should start with an "@".', $resourcePath));
}
$twigNamespace = substr($resourcePath, 1, strpos($resourcePath, '/') - 1);
$bundleName = $twigNamespace . 'Bundle';
$resourceName = substr($resourcePath, strpos($resourcePath, '/') + 1);

if (false !== strpos($resourcePath, '..')) {
throw new \InvalidArgumentException(sprintf('File name "%s" contains invalid characters (..).', $resourcePath));
}
$path = sprintf('%s/%s/views/%s', $theme->getPath(), $bundleName, $resourceName);

if (false === strpos($resourcePath, 'Resources/')) {
throw new \InvalidArgumentException(sprintf('Resource path "%s" should be in bundles\' "Resources/" directory.', $resourcePath));
if ($this->filesystem->exists($path)) {
return $path;
}
}

/**
* @param string $resourcePath
*
* @return string
*/
private function getBundleNameFromResourcePath(string $resourcePath): string
{
return substr($resourcePath, 1, strpos($resourcePath, '/') - 1);
}

/**
* @param string $resourcePath
*
* @return string
*/
private function getResourceNameFromResourcePath(string $resourcePath): string
{
return substr($resourcePath, strpos($resourcePath, 'Resources/') + strlen('Resources/'));
throw new ResourceNotFoundException($resourcePath, $theme);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,6 @@
<argument type="service" id="kernel" />
</service>

<service id="sylius.theme.templating.name_parser" class="Sylius\Bundle\ThemeBundle\Templating\TemplateNameParser" decorates="templating.name_parser" decoration-priority="256" public="false">
<argument type="service" id="sylius.theme.templating.name_parser.inner" />
<argument type="service" id="kernel" />
</service>

<service id="sylius.theme.templating.cache" class="Doctrine\Common\Cache\ArrayCache" />

<service id="sylius.theme.templating.locator" class="Sylius\Bundle\ThemeBundle\Templating\Locator\TemplateLocator" public="false">
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
@Test/Templating/twigNamespacedBothThemesTemplate.txt.twig|sylius/first-test-theme
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
@Test/Templating/twigNamespacedVanillaOverriddenThemeTemplate.txt.twig|sylius/first-test-theme
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Templating/twigNamespacedBothThemesTemplate.txt.twig|sylius/first-test-theme
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
@Test/Templating/twigNamespacedBothThemesTemplate.txt.twig|sylius/second-test-theme
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
@Test/Templating/twigNamespacedLastThemeTemplate.txt.twig|sylius/second-test-theme
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Templating/twigNamespacedBothThemesTemplate.txt.twig|sylius/second-test-theme
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Templating/twigNamespacedLastThemeTemplate.txt.twig|sylius/second-test-theme
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
@Test/Templating/twigNamespacedBothThemesTemplate.txt.twig|sylius/third-test-theme
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
@Test/Templating/twigNamespacedLastThemeTemplate.txt.twig|sylius/third-test-theme
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Templating/twigNamespacedBothThemesTemplate.txt.twig|sylius/third-test-theme
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Templating/twigNamespacedLastThemeTemplate.txt.twig|sylius/third-test-theme
14 changes: 11 additions & 3 deletions src/Sylius/Bundle/ThemeBundle/Tests/Functional/TemplatingTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,11 @@ public function getBundleTemplatesUsingNamespacedPaths()
['@Test/Templating/vanillaOverriddenThemeTemplate.txt.twig', 'TestBundle:Templating:vanillaOverriddenThemeTemplate.txt.twig|sylius/first-test-theme'],
['@Test/Templating/bothThemesTemplate.txt.twig', 'TestBundle:Templating:bothThemesTemplate.txt.twig|sylius/first-test-theme'],
['@Test/Templating/lastThemeTemplate.txt.twig', 'TestBundle:Templating:lastThemeTemplate.txt.twig|sylius/second-test-theme'],
['@Test/Templating/twigNamespacedVanillaTemplate.txt.twig', '@Test/Templating/twigNamespacedVanillaTemplate.txt.twig'],
['@Test/Templating/twigNamespacedVanillaOverriddenTemplate.txt.twig', '@Test/Templating/twigNamespacedVanillaOverriddenTemplate.txt.twig (templates overridden)'],
['@Test/Templating/twigNamespacedVanillaOverriddenThemeTemplate.txt.twig', '@Test/Templating/twigNamespacedVanillaOverriddenThemeTemplate.txt.twig|sylius/first-test-theme'],
['@Test/Templating/twigNamespacedBothThemesTemplate.txt.twig', '@Test/Templating/twigNamespacedBothThemesTemplate.txt.twig|sylius/first-test-theme'],
['@Test/Templating/twigNamespacedLastThemeTemplate.txt.twig', '@Test/Templating/twigNamespacedLastThemeTemplate.txt.twig|sylius/second-test-theme'],
];
}

Expand Down Expand Up @@ -123,9 +128,12 @@ public function it_renders_application_templates_using_namespaced_paths($templat
public function getAppTemplatesUsingNamespacedPaths()
{
return [
['/Templating/vanillaTemplate.txt.twig', ':Templating:vanillaTemplate.txt.twig'],
['/Templating/bothThemesTemplate.txt.twig', ':Templating:bothThemesTemplate.txt.twig|sylius/first-test-theme'],
['/Templating/lastThemeTemplate.txt.twig', ':Templating:lastThemeTemplate.txt.twig|sylius/second-test-theme'],
['Templating/vanillaTemplate.txt.twig', ':Templating:vanillaTemplate.txt.twig'],
['Templating/bothThemesTemplate.txt.twig', ':Templating:bothThemesTemplate.txt.twig|sylius/first-test-theme'],
['Templating/lastThemeTemplate.txt.twig', ':Templating:lastThemeTemplate.txt.twig|sylius/second-test-theme'],
['Templating/twigNamespacedVanillaTemplate.txt.twig', 'Templating/twigNamespacedVanillaTemplate.txt.twig'],
['Templating/twigNamespacedBothThemesTemplate.txt.twig', 'Templating/twigNamespacedBothThemesTemplate.txt.twig|sylius/first-test-theme'],
['Templating/twigNamespacedLastThemeTemplate.txt.twig', 'Templating/twigNamespacedLastThemeTemplate.txt.twig|sylius/second-test-theme'],
];
}
}
Original file line number Diff line number Diff line change
@@ -1 +1 @@
TestBundle:Templating:firstOverriddenTemplate.txt.twig
TestBundle:Templating:bothThemesTemplate.txt.twig
Original file line number Diff line number Diff line change
@@ -1 +1 @@
TestBundle:Templating:lastOverriddenTemplate.txt.twig
TestBundle:Templating:lastThemeTemplate.txt.twig
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
@Test/Templating/twigNamespacedBothThemesTemplate.txt.twig
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
@Test/Templating/twigNamespacedLastThemeTemplate.txt.twig
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
@Test/Templating/twigNamespacedVanillaOverriddenTemplate.txt.twig
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
@Test/Templating/twigNamespacedVanillaOverriddenThemeTemplate.txt.twig
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
@Test/Templating/twigNamespacedVanillaTemplate.txt.twig
Original file line number Diff line number Diff line change
@@ -1 +1 @@
:Templating:firstOverriddenTemplate.txt.twig
:Templating:bothThemesTemplate.txt.twig
Original file line number Diff line number Diff line change
@@ -1 +1 @@
:Templating:lastOverriddenTemplate.txt.twig
:Templating:lastThemeTemplate.txt.twig
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ framework:
test: ~

twig:
paths: ['%kernel.project_dir%/templates']
debug: "%kernel.debug%"
strict_variables: "%kernel.debug%"

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Templating/twigNamespacedBothThemesTemplate.txt.twig
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Templating/twigNamespacedLastThemeTemplate.txt.twig
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Templating/twigNamespacedVanillaTemplate.txt.twig
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
@Test/Templating/twigNamespacedVanillaOverriddenTemplate.txt.twig (templates overridden)
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
@Test/Templating/twigNamespacedVanillaOverriddenThemeTemplate.txt.twig (templates overridden)
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ function it_implements_resource_locator_interface(): void
$this->shouldImplement(ResourceLocatorInterface::class);
}

function it_locates_bundle_resource(
function it_locates_bundle_resource_using_path_derived_from_bundle_notation_and_symfony3_kernel_behaviour(
Filesystem $filesystem,
KernelInterface $kernel,
ThemeInterface $theme,
Expand All @@ -47,13 +47,30 @@ function it_locates_bundle_resource(

$theme->getPath()->willReturn('/theme/path');

$filesystem->exists('/theme/path/ChildBundle/views/index.html.twig')->shouldBeCalled()->willReturn(false);
$filesystem->exists('/theme/path/ParentBundle/views/index.html.twig')->shouldBeCalled()->willReturn(true);
$filesystem->exists('/theme/path/ChildBundle/views/Directory/index.html.twig')->shouldBeCalled()->willReturn(false);
$filesystem->exists('/theme/path/ParentBundle/views/Directory/index.html.twig')->shouldBeCalled()->willReturn(true);

$this->locateResource('@ParentBundle/Resources/views/index.html.twig', $theme)->shouldReturn('/theme/path/ParentBundle/views/index.html.twig');
$this->locateResource('@ParentBundle/Resources/views/Directory/index.html.twig', $theme)->shouldReturn('/theme/path/ParentBundle/views/Directory/index.html.twig');
}

function it_throws_an_exception_if_resource_can_not_be_located(
function it_locates_bundle_resource_using_path_derived_from_bundle_notation_and_symfony4_kernel_behaviour(
Filesystem $filesystem,
KernelInterface $kernel,
ThemeInterface $theme,
BundleInterface $justBundle
): void {
$kernel->getBundle('JustBundle', false)->willReturn($justBundle);

$justBundle->getName()->willReturn('JustBundle');

$theme->getPath()->willReturn('/theme/path');

$filesystem->exists('/theme/path/JustBundle/views/Directory/index.html.twig')->shouldBeCalled()->willReturn(true);

$this->locateResource('@JustBundle/Resources/views/Directory/index.html.twig', $theme)->shouldReturn('/theme/path/JustBundle/views/Directory/index.html.twig');
}

function it_throws_an_exception_if_resource_can_not_be_located_using_path_derived_from_bundle_notation(
Filesystem $filesystem,
KernelInterface $kernel,
ThemeInterface $theme,
Expand All @@ -68,24 +85,42 @@ function it_throws_an_exception_if_resource_can_not_be_located(
$theme->getName()->willReturn('theme/name');
$theme->getPath()->willReturn('/theme/path');

$filesystem->exists('/theme/path/ChildBundle/views/index.html.twig')->shouldBeCalled()->willReturn(false);
$filesystem->exists('/theme/path/ParentBundle/views/index.html.twig')->shouldBeCalled()->willReturn(false);
$filesystem->exists('/theme/path/ChildBundle/views/Directory/index.html.twig')->shouldBeCalled()->willReturn(false);
$filesystem->exists('/theme/path/ParentBundle/views/Directory/index.html.twig')->shouldBeCalled()->willReturn(false);

$this->shouldThrow(ResourceNotFoundException::class)->during('locateResource', ['@ParentBundle/Resources/views/index.html.twig', $theme]);
$this->shouldThrow(ResourceNotFoundException::class)->during('locateResource', ['@ParentBundle/Resources/views/Directory/index.html.twig', $theme]);
}

function it_throws_an_exception_if_resource_path_does_not_start_with_an_asperand(ThemeInterface $theme): void
{
$this->shouldThrow(\InvalidArgumentException::class)->during('locateResource', ['ParentBundle/Resources/views/index.html.twig', $theme]);
function it_locates_bundle_resource_using_path_derived_from_twig_namespaces(
Filesystem $filesystem,
ThemeInterface $theme
): void {
$theme->getPath()->willReturn('/theme/path');

$filesystem->exists('/theme/path/JustBundle/views/Directory/index.html.twig')->shouldBeCalled()->willReturn(true);

$this->locateResource('@Just/Directory/index.html.twig', $theme)->shouldReturn('/theme/path/JustBundle/views/Directory/index.html.twig');
}

function it_throws_an_exception_if_resource_path_contains_two_dots_in_a_row(ThemeInterface $theme): void
function it_throws_an_exception_if_resource_can_not_be_located_using_path_derived_from_twig_namespaces(
Filesystem $filesystem,
ThemeInterface $theme
): void {
$theme->getName()->willReturn('theme/name');
$theme->getPath()->willReturn('/theme/path');

$filesystem->exists('/theme/path/JustBundle/views/Directory/index.html.twig')->shouldBeCalled()->willReturn(false);

$this->shouldThrow(ResourceNotFoundException::class)->during('locateResource', ['@Just/Directory/index.html.twig', $theme]);
}

function it_throws_an_exception_if_resource_path_does_not_start_with_an_asperand(ThemeInterface $theme): void
{
$this->shouldThrow(\InvalidArgumentException::class)->during('locateResource', ['@ParentBundle/Resources/views/../views/index.html.twig', $theme]);
$this->shouldThrow(\InvalidArgumentException::class)->during('locateResource', ['ParentBundle/Resources/views/Directory/index.html.twig', $theme]);
}

function it_throws_an_exception_if_resource_path_does_not_contain_resources_dir(ThemeInterface $theme): void
function it_throws_an_exception_if_resource_path_contains_two_dots_in_a_row(ThemeInterface $theme): void
{
$this->shouldThrow(\InvalidArgumentException::class)->during('locateResource', ['@ParentBundle/views/Resources.index.html.twig', $theme]);
$this->shouldThrow(\InvalidArgumentException::class)->during('locateResource', ['@ParentBundle/Resources/views/../views/Directory/index.html.twig', $theme]);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,9 @@ function it_proxies_locating_template_to_resource_locator(
TemplateReferenceInterface $template,
ThemeInterface $theme
): void {
$template->getPath()->willReturn('@AcmeBundle/Resources/views/index.html.twig');
$template->getPath()->willReturn('@AcmeBundle/Resources/views/Directory/index.html.twig');

$resourceLocator->locateResource('@AcmeBundle/Resources/views/index.html.twig', $theme)->willReturn('/acme/index.html.twig');
$resourceLocator->locateResource('@AcmeBundle/Resources/views/Directory/index.html.twig', $theme)->willReturn('/acme/index.html.twig');

$this->locateTemplate($template, $theme)->shouldReturn('/acme/index.html.twig');
}
Expand All @@ -49,9 +49,9 @@ function it_does_not_catch_exceptions_thrown_while_locating_template_to_resource
TemplateReferenceInterface $template,
ThemeInterface $theme
): void {
$template->getPath()->willReturn('@AcmeBundle/Resources/views/index.html.twig');
$template->getPath()->willReturn('@AcmeBundle/Resources/views/Directory/index.html.twig');

$resourceLocator->locateResource('@AcmeBundle/Resources/views/index.html.twig', $theme)->willThrow(ResourceNotFoundException::class);
$resourceLocator->locateResource('@AcmeBundle/Resources/views/Directory/index.html.twig', $theme)->willThrow(ResourceNotFoundException::class);

$this->shouldThrow(ResourceNotFoundException::class)->during('locateTemplate', [$template, $theme]);
}
Expand Down

0 comments on commit 7fe2fe3

Please sign in to comment.