Skip to content

Commit

Permalink
[Templating] fix logic regarding template references and many phpdocs
Browse files Browse the repository at this point in the history
  • Loading branch information
Tobion authored and fabpot committed Oct 1, 2013
1 parent 164c1cb commit f6c12bd
Show file tree
Hide file tree
Showing 22 changed files with 142 additions and 174 deletions.
22 changes: 22 additions & 0 deletions src/Symfony/Bridge/Twig/Tests/TwigEngineTest.php
Expand Up @@ -12,6 +12,7 @@
namespace Symfony\Bridge\Twig\Tests;

use Symfony\Bridge\Twig\TwigEngine;
use Symfony\Component\Templating\TemplateReference;

class TwigEngineTest extends \PHPUnit_Framework_TestCase
{
Expand All @@ -27,20 +28,41 @@ public function testExistsWithNonExistentTemplates()
$engine = $this->getTwig();

$this->assertFalse($engine->exists('foobar'));
$this->assertFalse($engine->exists(new TemplateReference('foorbar')));
}

public function testExistsWithTemplateWithSyntaxErrors()
{
$engine = $this->getTwig();

$this->assertTrue($engine->exists('error'));
$this->assertTrue($engine->exists(new TemplateReference('error')));
}

public function testExists()
{
$engine = $this->getTwig();

$this->assertTrue($engine->exists('index'));
$this->assertTrue($engine->exists(new TemplateReference('index')));
}

public function testRender()
{
$engine = $this->getTwig();

$this->assertSame('foo', $engine->render('index'));
$this->assertSame('foo', $engine->render(new TemplateReference('index')));
}

/**
* @expectedException \Twig_Error_Syntax
*/
public function testRenderWithError()
{
$engine = $this->getTwig();

$engine->render(new TemplateReference('error'));
}

protected function getTwig()
Expand Down
41 changes: 18 additions & 23 deletions src/Symfony/Bridge/Twig/TwigEngine.php
Expand Up @@ -14,6 +14,7 @@
use Symfony\Component\Templating\EngineInterface;
use Symfony\Component\Templating\StreamingEngineInterface;
use Symfony\Component\Templating\TemplateNameParserInterface;
use Symfony\Component\Templating\TemplateReferenceInterface;

/**
* This engine knows how to render Twig templates.
Expand All @@ -38,40 +39,33 @@ public function __construct(\Twig_Environment $environment, TemplateNameParserIn
}

/**
* Renders a template.
* {@inheritdoc}
*
* @param mixed $name A template name
* @param array $parameters An array of parameters to pass to the template
* It also supports \Twig_Template as name parameter.
*
* @return string The evaluated template as a string
*
* @throws \InvalidArgumentException if the template does not exist
* @throws \RuntimeException if the template cannot be rendered
* @throws \Twig_Error if something went wrong like a thrown exception while rendering the template
*/
public function render($name, array $parameters = array())
{
return $this->load($name)->render($parameters);
}

/**
* Streams a template.
* {@inheritdoc}
*
* @param mixed $name A template name or a TemplateReferenceInterface instance
* @param array $parameters An array of parameters to pass to the template
* It also supports \Twig_Template as name parameter.
*
* @throws \RuntimeException if the template cannot be rendered
* @throws \Twig_Error if something went wrong like a thrown exception while rendering the template
*/
public function stream($name, array $parameters = array())
{
$this->load($name)->display($parameters);
}

/**
* Returns true if the template exists.
* {@inheritdoc}
*
* @param mixed $name A template name
*
* @return Boolean true if the template exists, false otherwise
* It also supports \Twig_Template as name parameter.
*/
public function exists($name)
{
Expand All @@ -82,11 +76,13 @@ public function exists($name)
$loader = $this->environment->getLoader();

if ($loader instanceof \Twig_ExistsLoaderInterface) {
return $loader->exists($name);
return $loader->exists((string) $name);
}

try {
$loader->getSource($name);
// cast possible TemplateReferenceInterface to string because the
// EngineInterface supports them but Twig_LoaderInterface does not
$loader->getSource((string) $name);
} catch (\Twig_Error_Loader $e) {
return false;
}
Expand All @@ -95,11 +91,9 @@ public function exists($name)
}

/**
* Returns true if this class is able to render the given template.
*
* @param string $name A template name
* {@inheritdoc}
*
* @return Boolean True if this class supports the given resource, false otherwise
* It also supports \Twig_Template as name parameter.
*/
public function supports($name)
{
Expand All @@ -115,7 +109,8 @@ public function supports($name)
/**
* Loads the given template.
*
* @param mixed $name A template name or an instance of Twig_Template
* @param string|TemplateReferenceInterface|\Twig_Template $name A template name or an instance of
* TemplateReferenceInterface or \Twig_Template
*
* @return \Twig_TemplateInterface A \Twig_TemplateInterface instance
*
Expand All @@ -128,7 +123,7 @@ protected function load($name)
}

try {
return $this->environment->loadTemplate($name);
return $this->environment->loadTemplate((string) $name);
} catch (\Twig_Error_Loader $e) {
throw new \InvalidArgumentException($e->getMessage(), $e->getCode(), $e);
}
Expand Down
16 changes: 8 additions & 8 deletions src/Symfony/Bridge/Twig/composer.json
Expand Up @@ -30,14 +30,14 @@
"symfony/stopwatch": "~2.2"
},
"suggest": {
"symfony/form": "",
"symfony/http-kernel": "",
"symfony/routing": "",
"symfony/templating": "",
"symfony/translation": "",
"symfony/yaml": "",
"symfony/security": "",
"symfony/stopwatch": ""
"symfony/form": "For using the FormExtension",
"symfony/http-kernel": "For using the HttpKernelExtension",
"symfony/routing": "For using the RoutingExtension",
"symfony/templating": "For using the TwigEngine",
"symfony/translation": "For using the TranslationExtension",
"symfony/yaml": "For using the YamlExtension",
"symfony/security": "For using the SecurityExtension",
"symfony/stopwatch": "For using the StopwatchExtension"
},
"autoload": {
"psr-0": { "Symfony\\Bridge\\Twig\\": "" }
Expand Down
50 changes: 21 additions & 29 deletions src/Symfony/Bundle/FrameworkBundle/Templating/DelegatingEngine.php
Expand Up @@ -40,50 +40,42 @@ public function __construct(ContainerInterface $container, array $engineIds)
/**
* {@inheritdoc}
*/
public function supports($name)
public function getEngine($name)
{
foreach ($this->engines as $i => $engine) {
if (is_string($engine)) {
$engine = $this->engines[$i] = $this->container->get($engine);
}

if ($engine->supports($name)) {
return true;
}
}
$this->resolveEngines();

return false;
return parent::getEngine($name);
}

/**
* {@inheritdoc}
*/
public function getEngine($name)
public function renderResponse($view, array $parameters = array(), Response $response = null)
{
foreach ($this->engines as $i => $engine) {
if (is_string($engine)) {
$engine = $this->engines[$i] = $this->container->get($engine);
}
$engine = $this->getEngine($view);

if ($engine->supports($name)) {
return $engine;
}
if ($engine instanceof EngineInterface) {
return $engine->renderResponse($view, $parameters, $response);
}

if (null === $response) {
$response = new Response();
}

throw new \RuntimeException(sprintf('No engine is able to work with the template "%s".', $name));
$response->setContent($engine->render($view, $parameters));

return $response;
}

/**
* Renders a view and returns a Response.
*
* @param string $view The view name
* @param array $parameters An array of parameters to pass to the view
* @param Response $response A Response instance
*
* @return Response A Response instance
* Resolved engine ids to their real engine instances from the container.
*/
public function renderResponse($view, array $parameters = array(), Response $response = null)
private function resolveEngines()
{
return $this->getEngine($view)->renderResponse($view, $parameters, $response);
foreach ($this->engines as $i => $engine) {
if (is_string($engine)) {
$this->engines[$i] = $this->container->get($engine);
}
}
}
}
Expand Up @@ -29,6 +29,8 @@ interface EngineInterface extends BaseEngineInterface
* @param Response $response A Response instance
*
* @return Response A Response instance
*
* @throws \RuntimeException if the template cannot be rendered
*/
public function renderResponse($view, array $parameters = array(), Response $response = null);
}
Expand Up @@ -36,11 +36,7 @@ public function __construct(FileLocatorInterface $locator)
}

/**
* Loads a template.
*
* @param TemplateReferenceInterface $template A template
*
* @return FileStorage|Boolean false if the template cannot be loaded, a Storage instance otherwise
* {@inheritdoc}
*/
public function load(TemplateReferenceInterface $template)
{
Expand All @@ -54,12 +50,7 @@ public function load(TemplateReferenceInterface $template)
}

/**
* Returns true if the template is still fresh.
*
* @param TemplateReferenceInterface $template The template name as an array
* @param integer $time The last modification time of the cached template (timestamp)
*
* @return Boolean
* {@inheritdoc}
*/
public function isFresh(TemplateReferenceInterface $template, $time)
{
Expand Down
Expand Up @@ -66,7 +66,7 @@ protected function getCacheKey($template)
public function locate($template, $currentPath = null, $first = true)
{
if (!$template instanceof TemplateReferenceInterface) {
throw new \InvalidArgumentException("The template must be an instance of TemplateReferenceInterface.");
throw new \InvalidArgumentException('The template must be an instance of TemplateReferenceInterface.');
}

$key = $this->getCacheKey($template);
Expand Down
10 changes: 2 additions & 8 deletions src/Symfony/Bundle/FrameworkBundle/Templating/PhpEngine.php
Expand Up @@ -46,7 +46,7 @@ public function __construct(TemplateNameParserInterface $parser, ContainerInterf
}

/**
* @throws \InvalidArgumentException When the helper is not defined
* {@inheritdoc}
*/
public function get($name)
{
Expand All @@ -71,13 +71,7 @@ public function setHelpers(array $helpers)
}

/**
* Renders a view and returns a Response.
*
* @param string $view The view name
* @param array $parameters An array of parameters to pass to the view
* @param Response $response A Response instance
*
* @return Response A Response instance
* {@inheritdoc}
*/
public function renderResponse($view, array $parameters = array(), Response $response = null)
{
Expand Down
Expand Up @@ -24,9 +24,13 @@ class TemplateFilenameParser implements TemplateNameParserInterface
/**
* {@inheritdoc}
*/
public function parse($file)
public function parse($name)
{
$parts = explode('/', strtr($file, '\\', '/'));
if ($name instanceof TemplateReferenceInterface) {
return $name;
}

$parts = explode('/', strtr($name, '\\', '/'));

$elements = explode('.', array_pop($parts));
if (3 > count($elements)) {
Expand Down
Expand Up @@ -11,8 +11,6 @@

namespace Symfony\Bundle\FrameworkBundle\Templating;

use Symfony\Bundle\FrameworkBundle\Templating\PhpEngine;
use Symfony\Bundle\FrameworkBundle\Templating\GlobalVariables;
use Symfony\Component\Templating\TemplateNameParserInterface;
use Symfony\Component\Stopwatch\Stopwatch;
use Symfony\Component\Templating\Loader\LoaderInterface;
Expand Down
Expand Up @@ -58,7 +58,7 @@ public function testGetInvalidEngine()
$delegatingEngine->getEngine('template.php', array('foo' => 'bar'));
}

public function testRenderResponse()
public function testRenderResponseWithFrameworkEngine()
{
$response = $this->getMock('Symfony\Component\HttpFoundation\Response');
$engine = $this->getFrameworkEngineMock('template.php', true);
Expand All @@ -73,6 +73,15 @@ public function testRenderResponse()
$this->assertSame($response, $delegatingEngine->renderResponse('template.php', array('foo' => 'bar')));
}

public function testRenderResponseWithTemplatingEngine()
{
$engine = $this->getEngineMock('template.php', true);
$container = $this->getContainerMock(array('engine' => $engine));
$delegatingEngine = new DelegatingEngine($container, array('engine'));

$this->assertInstanceOf('Symfony\Component\HttpFoundation\Response', $delegatingEngine->renderResponse('template.php', array('foo' => 'bar')));
}

private function getEngineMock($template, $supports)
{
$engine = $this->getMock('Symfony\Component\Templating\EngineInterface');
Expand Down

0 comments on commit f6c12bd

Please sign in to comment.