merged branch wodor/profiler_rely_on_profile_3372 (PR #3373)

Commits ------- 1472283 fixed CS bc73487 renamed template to TemplateManager , moved profiler to the deps of manager 5fd6ed6 properties protected abd0eb7 generating template names moved out from controller to another class 6138e80 [Profiler] relying on config of displayed profile instead of current config. Discussion ---------- [2.2][Profiler] relying on config of displayed profile instead of current config Bug fix: yes Feature addition: no Backwards compatibility break: no Symfony2 tests pass: code of ProfilerController is not covered by any test Fixes the following tickets: #3372 Todo: ~ This fixes the exception which is raised when viewed profile has other data collectors than in config of currently run profiler. explained here https://github.com/symfony/symfony/issues/3372 --------------------------------------------------------------------------- by fabpot at 2012-02-16T06:11:00Z This should probably be done on the 2.0 branch. Also, I think we need to check if the panel is actually available in the current profiler (if not, we won't be able to display it anyway). So, both checks are important. --------------------------------------------------------------------------- by wodor at 2012-02-18T10:15:40Z defects mentioned by Stof are fixed
2012-07-02 11:05:28 +02:00 · 2012-07-02 11:05:28 +02:00 · 155320ae83
commit 155320ae83
parent 36192a922c 1472283739
5 changed files with 325 additions and 51 deletions
--- a/src/Symfony/Bundle/WebProfilerBundle/Controller/ProfilerController.php
+++ b/src/Symfony/Bundle/WebProfilerBundle/Controller/ProfilerController.php
@ -41,6 +41,7 @@ class ProfilerController extends ContainerAware
        $panel = $this->container->get('request')->query->get('panel', 'request');
        $page = $this->container->get('request')->query->get('page', 'home');

+
        if (!$profile = $profiler->loadProfile($token)) {
            return $this->container->get('templating')->renderResponse('WebProfilerBundle:Profiler:info.html.twig', array('about' => 'no_token', 'token' => $token));
        }
@ -49,13 +50,16 @@ class ProfilerController extends ContainerAware
            throw new NotFoundHttpException(sprintf('Panel "%s" is not available for token "%s".', $panel, $token));
        }

-        return $this->container->get('templating')->renderResponse($this->getTemplateName($profiler, $panel), array(
+        /** @var $templateManager \Symfony\Bundle\WebProfilerBundle\Profiler\Template */
+        $templateManager = $this->container->get('web_profiler.profiler_template');
+
+        return $this->container->get('templating')->renderResponse($templateManager->getName($profile, $panel), array(
            'token'     => $token,
            'profile'   => $profile,
            'collector' => $profile->getCollector($panel),
            'panel'     => $panel,
            'page'      => $page,
-            'templates' => $this->getTemplates($profiler),
+            'templates' => $templateManager->getTemplates($profile),
            'is_ajax'   => $request->isXmlHttpRequest(),
        ));
    }
@ -178,10 +182,13 @@ class ProfilerController extends ContainerAware
            // the profiler is not enabled
        }

+        /** @var $templateManager \Symfony\Bundle\WebProfilerBundle\Profiler\Template */
+        $templateManager = $this->container->get('web_profiler.profiler_template');
+
        return $this->container->get('templating')->renderResponse('WebProfilerBundle:Profiler:toolbar.html.twig', array(
            'position'     => $position,
            'profile'      => $profile,
-            'templates'    => $this->getTemplates($profiler),
+            'templates'    => $templateManager->getTemplates($profile),
            'profiler_url' => $url,
        ));
    }
@ -307,52 +314,4 @@ class ProfilerController extends ContainerAware

        return new Response($phpinfo);
    }
-
-    protected function getTemplateNames($profiler)
-    {
-        $templates = array();
-        foreach ($this->container->getParameter('data_collector.templates') as $arguments) {
-            if (null === $arguments) {
-                continue;
-            }
-
-            list($name, $template) = $arguments;
-            if (!$profiler->has($name)) {
-                continue;
-            }
-
-            if ('.html.twig' === substr($template, -10)) {
-                $template = substr($template, 0, -10);
-            }
-
-            if (!$this->container->get('templating')->exists($template.'.html.twig')) {
-                throw new \UnexpectedValueException(sprintf('The profiler template "%s.html.twig" for data collector "%s" does not exist.', $template, $name));
-            }
-
-            $templates[$name] = $template.'.html.twig';
-        }
-
-        return $templates;
-    }
-
-    protected function getTemplateName($profiler, $panel)
-    {
-        $templates = $this->getTemplateNames($profiler);
-
-        if (!isset($templates[$panel])) {
-            throw new NotFoundHttpException(sprintf('Panel "%s" is not registered.', $panel));
-        }
-
-        return $templates[$panel];
-    }
-
-    protected function getTemplates($profiler)
-    {
-        $templates = $this->getTemplateNames($profiler);
-        foreach ($templates as $name => $template) {
-            $templates[$name] = $this->container->get('twig')->loadTemplate($template);
-        }
-
-        return $templates;
-    }
 }
--- a/src/Symfony/Bundle/WebProfilerBundle/Profiler/TemplateManager.php
+++ b/src/Symfony/Bundle/WebProfilerBundle/Profiler/TemplateManager.php
@ -0,0 +1,126 @@
+<?php
+
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Symfony\Bundle\WebProfilerBundle\Profiler;
+
+use Symfony\Component\HttpKernel\Exception\NotFoundHttpException;
+use Symfony\Component\HttpKernel\Profiler\Profiler;
+use Symfony\Component\HttpKernel\Profiler\Profile;
+use Symfony\Bundle\TwigBundle\TwigEngine;
+
+/**
+ * Profiler Templates Manager
+ *
+ * @author Fabien Potencier <fabien@symfony.com>
+ * @author Artur Wielogórski <wodor@wodor.net>
+ */
+class TemplateManager {
+
+    /**
+     * @var \Symfony\Bundle\TwigBundle\TwigEngine
+     */
+    protected $templating;
+
+    /**
+     * @var \Twig_Environment
+     */
+    protected $twig;
+
+    /**
+     * @var array
+     */
+    protected $templates;
+
+    /**
+     * @var \Symfony\Component\HttpKernel\Profiler\Profiler
+     */
+    protected $profiler;
+
+    /**
+     * @param \Symfony\Bundle\TwigBundle\TwigEngine $templating
+     * @param \Twig_Environment $twig
+     * @param array $templates
+     */
+    public function __construct(Profiler $profiler, TwigEngine $templating, \Twig_Environment $twig, array $templates)
+    {
+        $this->profiler = $profiler;
+        $this->templating = $templating;
+        $this->twig = $twig;
+        $this->templates = $templates;
+    }
+
+    /**
+     * @param \Symfony\Component\HttpKernel\Profiler\Profile $profile
+     * @param $panel
+     * @return mixed
+     * @throws \Symfony\Component\HttpKernel\Exception\NotFoundHttpException
+     */
+    public function getName(Profile $profile, $panel)
+    {
+        $templates = $this->getNames($profile);
+
+        if (!isset($templates[$panel])) {
+            throw new NotFoundHttpException(sprintf('Panel "%s" is not registered in profiler or is not present in viewed profile.', $panel));
+        }
+
+        return $templates[$panel];
+    }
+
+    /**
+     * @param \Symfony\Component\HttpKernel\Profiler\Profile $profile
+     * @return array
+     */
+    public function getTemplates(Profile $profile)
+    {
+        $templates = $this->getNames($profile);
+        foreach ($templates as $name => $template) {
+            $templates[$name] = $this->twig->loadTemplate($template);
+        }
+
+        return $templates;
+    }
+
+    /**
+     * Gets template names of templates that are
+     * present in the viewed profile
+     * @param \Symfony\Component\HttpKernel\Profiler\Profile $profile
+     * @return array
+     * @throws \UnexpectedValueException
+     */
+    protected function getNames(Profile $profile)
+    {
+        $templates = array();
+
+        foreach ($this->templates as $arguments) {
+            if (null === $arguments) {
+                continue;
+            }
+
+            list($name, $template) = $arguments;
+
+            if (!$this->profiler->has($name) || !$profile->hasCollector($name)) {
+                continue;
+            }
+
+            if ('.html.twig' === substr($template, -10)) {
+                $template = substr($template, 0, -10);
+            }
+
+            if (!$this->templating->exists($template.'.html.twig')) {
+                throw new \UnexpectedValueException(sprintf('The profiler template "%s.html.twig" for data collector "%s" does not exist.', $template, $name));
+            }
+
+            $templates[$name] = $template.'.html.twig';
+        }
+
+        return $templates;
+    }
+}
--- a/src/Symfony/Bundle/WebProfilerBundle/Resources/config/toolbar.xml
+++ b/src/Symfony/Bundle/WebProfilerBundle/Resources/config/toolbar.xml
@ -6,6 +6,7 @@

    <parameters>
        <parameter key="web_profiler.debug_toolbar.class">Symfony\Bundle\WebProfilerBundle\EventListener\WebDebugToolbarListener</parameter>
+        <parameter key="web_profiler.profiler_template">Symfony\Bundle\WebProfilerBundle\Profiler\TemplateManager</parameter>
    </parameters>

    <services>
@ -16,5 +17,12 @@
            <argument>%web_profiler.debug_toolbar.mode%</argument>
            <argument>%web_profiler.debug_toolbar.position%</argument>
        </service>
+
+        <service id="web_profiler.profiler_template" class="%web_profiler.profiler_template%">
+            <argument type="service" id="profiler" />
+            <argument type="service" id="templating.engine.twig" />
+            <argument type="service" id="twig" />
+            <argument>%data_collector.templates%</argument>
+        </service>
    </services>
 </container>
--- a/src/Symfony/Bundle/WebProfilerBundle/Tests/DependencyInjection/WebProfilerExtensionTest.php
+++ b/src/Symfony/Bundle/WebProfilerBundle/Tests/DependencyInjection/WebProfilerExtensionTest.php
@ -64,6 +64,9 @@ class WebProfilerExtensionTest extends TestCase
        $this->container->setParameter('kernel.cache_dir', __DIR__);
        $this->container->setParameter('kernel.debug', false);
        $this->container->setParameter('kernel.root_dir', __DIR__);
+        $this->container->register('profiler', $this->getMockClass('Symfony\\Component\\HttpKernel\\Profiler\\Profiler'))
+            ->addArgument(new Definition($this->getMockClass('Symfony\\Component\\HttpKernel\\Profiler\\ProfilerStorageInterface')));
+        $this->container->setParameter('data_collector.templates', array());
        $this->container->set('kernel', $this->kernel);
    }

--- a/src/Symfony/Bundle/WebProfilerBundle/Tests/Profiler/TemplateManagerTest.php
+++ b/src/Symfony/Bundle/WebProfilerBundle/Tests/Profiler/TemplateManagerTest.php
@ -0,0 +1,178 @@
+<?php
+
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Symfony\Bundle\WebProfilerBundle\Tests\Profiler;
+
+use Symfony\Bundle\WebProfilerBundle\Tests\TestCase;
+use Symfony\Bundle\WebProfilerBundle\Profiler\TemplateManager;
+
+/**
+ * Test for TemplateManager class.
+ *
+ * @author Artur Wielogórski <wodor@wodor.net>
+ */
+class TemplateManagerTest extends TestCase
+{
+    /**
+     * @var \Symfony\Bundle\TwigBundle\TwigEngine
+     */
+    protected $twigEngine;
+
+    /**
+     * @var \Twig_Environment
+     */
+    protected $twigEnvironment;
+
+    /**
+     * @var \Symfony\Component\HttpKernel\Profiler\Profiler
+     */
+    protected $profiler;
+
+    /**
+     * @var \PHPUnit_Framework_MockObject_MockObject
+     */
+    protected $profile;
+
+    /**
+     * @var \Symfony\Bundle\WebProfilerBundle\Profiler\TemplateManager
+     */
+    protected $templateManager;
+
+    public function setUp()
+    {
+        parent::setUp();
+
+        $profiler = $this->mockProfiler();
+        $twigEngine = $this->mockTwigEngine();
+        $twigEnvironment = $this->mockTwigEnvironment();
+        $templates = array(
+            'data_collector.foo'=>array('foo','FooBundle:Collector:foo'),
+            'data_collector.bar'=>array('bar','FooBundle:Collector:bar'),
+            'data_collector.baz'=>array('baz','FooBundle:Collector:baz')
+            );
+
+        $this->templateManager = new TemplateManager($profiler, $twigEngine, $twigEnvironment, $templates);
+    }
+
+    /**
+     * @expectedException Symfony\Component\HttpKernel\Exception\NotFoundHttpException
+     */
+    public function testGetNameOfInvalidTemplate()
+    {
+        $profile = $this->mockProfile();
+        $this->templateManager->getName($profile, 'notexisitingpanel');
+    }
+
+    /**
+     * if template exists in both profile and profiler then it's name should be returned
+     */
+    public function testGetNameValidTemplate()
+    {
+        $this->profiler->expects($this->any())
+            ->method('has')
+            ->withAnyParameters()
+            ->will($this->returnCallback(array($this, 'profilerHasCallback')));
+
+        $profile = $this->mockProfile();
+        $profile->expects($this->any())
+            ->method('hasCollector')
+            ->will($this->returnCallback(array($this, 'profileHasCollectorCallback')));
+
+       $this->assertEquals('FooBundle:Collector:foo.html.twig', $this->templateManager->getName($profile, 'foo'));
+    }
+
+    /**
+     * template should be loaded for 'foo' because other collectors are
+     * missing in profile or in profiler
+     */
+    public function testGetTemplates()
+    {
+
+        $profile = $this->mockProfile();
+        $profile->expects($this->any())
+            ->method('hasCollector')
+            ->will($this->returnCallback(array($this, 'profilerHasCallback')));
+
+        $this->profiler->expects($this->any())
+            ->method('has')
+            ->withAnyParameters()
+            ->will($this->returnCallback(array($this, 'profileHasCollectorCallback')));
+
+        $result = $this->templateManager->getTemplates($profile);
+        $this->assertArrayHasKey('foo',$result);
+        $this->assertArrayNotHasKey('bar',$result);
+        $this->assertArrayNotHasKey('baz',$result);
+    }
+
+    public function profilerHasCallback($panel)
+    {
+        switch ($panel) {
+            case 'foo':
+            case 'bar':
+                return true;
+            default:
+                return false;
+        }
+    }
+
+    public function profileHasCollectorCallback($panel)
+    {
+        switch ($panel) {
+            case 'foo':
+            case 'baz':
+                return true;
+            default:
+                return false;
+        }
+    }
+
+    protected function mockProfile()
+    {
+        $this->profile = $this->getMockBuilder('Symfony\Component\HttpKernel\Profiler\Profile')
+            ->disableOriginalConstructor()
+            ->getMock();
+
+        return  $this->profile;
+    }
+
+    protected function mockTwigEnvironment()
+    {
+        $this->twigEnvironment = $this->getMockBuilder('Twig_Environment')->getMock();
+
+        $this->twigEnvironment->expects($this->any())
+            ->method('loadTemplate')
+            ->will($this->returnValue('loadedTemplate'));
+
+        return $this->twigEnvironment;
+    }
+
+    protected function mockTwigEngine()
+    {
+        $this->twigEngine = $this->getMockBuilder('Symfony\Bundle\TwigBundle\TwigEngine')
+            ->disableOriginalConstructor()
+            ->getMock();
+
+        $this->twigEngine->expects($this->any())
+            ->method('exists')
+            ->will($this->returnValue(true));
+
+        return $this->twigEngine;
+    }
+
+    protected function mockProfiler()
+    {
+        $this->profiler = $this->getMockBuilder('Symfony\Component\HttpKernel\Profiler\Profiler')
+            ->disableOriginalConstructor()
+            ->getMock();
+
+        return $this->profiler;
+    }
+}