feature #33783 [WebProfilerBundle] Try to display the most useful panel by default (fancyweb)

This PR was merged into the 4.4 branch. Discussion ---------- [WebProfilerBundle] Try to display the most useful panel by default | Q | A | ------------- | --- | Branch? | 4.4 | Bug fix? | no | New feature? | yes | Deprecations? | yes | Tickets | - | License | MIT | Doc PR | - Alternative to https://github.com/symfony/symfony/pull/32491, the goal stays the same. I think reserving a data collector name is fine, isn'it ? It's not likely that end users use this name (that's why I added an underscore) + shouldn't be hard for them to just rename it. I don't think adding a configuration option to toggle the "_best" behavior is useful, should be by default for DX IMHO. Not adding an extension point for now (for end users to set their panel as the "best"), maybe later if someone request it? Commits ------- a45dd98b73 [WebProfilerBundle] Try to display the most useful panel by default
2019-10-02 14:22:29 +02:00 · 2019-10-02 14:22:29 +02:00 · 4d6fe5ce5d
commit 4d6fe5ce5d
parent e3aed1055d a45dd98b73
4 changed files with 130 additions and 10 deletions
--- a/src/Symfony/Bundle/WebProfilerBundle/Controller/ProfilerController.php
+++ b/src/Symfony/Bundle/WebProfilerBundle/Controller/ProfilerController.php
@ -17,6 +17,8 @@ use Symfony\Component\HttpFoundation\RedirectResponse;
 use Symfony\Component\HttpFoundation\Request;
 use Symfony\Component\HttpFoundation\Response;
 use Symfony\Component\HttpFoundation\Session\Flash\AutoExpireFlashBag;
+use Symfony\Component\HttpKernel\DataCollector\DumpDataCollector;
+use Symfony\Component\HttpKernel\DataCollector\ExceptionDataCollector;
 use Symfony\Component\HttpKernel\Exception\NotFoundHttpException;
 use Symfony\Component\HttpKernel\Profiler\Profiler;
 use Symfony\Component\Routing\Generator\UrlGeneratorInterface;
@ -78,7 +80,7 @@ class ProfilerController
            $this->cspHandler->disableCsp();
        }

-        $panel = $request->query->get('panel', 'request');
+        $panel = $request->query->get('panel');
        $page = $request->query->get('page', 'home');

        if ('latest' === $token && $latest = current($this->profiler->find(null, null, 1, null, null, null))) {
@ -89,6 +91,22 @@ class ProfilerController
            return new Response($this->twig->render('@WebProfiler/Profiler/info.html.twig', ['about' => 'no_token', 'token' => $token, 'request' => $request]), 200, ['Content-Type' => 'text/html']);
        }

+        if (null === $panel) {
+            $panel = 'request';
+
+            foreach ($profile->getCollectors() as $collector) {
+                if ($collector instanceof ExceptionDataCollector && $collector->hasException()) {
+                    $panel = $collector->getName();
+
+                    break;
+                }
+
+                if ($collector instanceof DumpDataCollector && $collector->getDumpsCount() > 0) {
+                    $panel = $collector->getName();
+                }
+            }
+        }
+
        if (!$profile->hasCollector($panel)) {
            throw new NotFoundHttpException(sprintf('Panel "%s" is not available for token "%s".', $panel, $token));
        }
--- a/src/Symfony/Bundle/WebProfilerBundle/Resources/views/Profiler/base_js.html.twig
+++ b/src/Symfony/Bundle/WebProfilerBundle/Resources/views/Profiler/base_js.html.twig
@ -267,7 +267,7 @@
            if (request.profilerUrl) {
                profilerCell.textContent = '';
                var profilerLink = document.createElement('a');
-                profilerLink.setAttribute('href', request.statusCode < 400 ? request.profilerUrl : request.profilerUrl + '?panel=exception');
+                profilerLink.setAttribute('href', request.profilerUrl);
                profilerLink.textContent = request.profile;
                profilerCell.appendChild(profilerLink);
            }
--- a/src/Symfony/Bundle/WebProfilerBundle/Tests/Controller/ProfilerControllerTest.php
+++ b/src/Symfony/Bundle/WebProfilerBundle/Tests/Controller/ProfilerControllerTest.php
@ -15,8 +15,16 @@ use PHPUnit\Framework\TestCase;
 use Symfony\Bundle\WebProfilerBundle\Controller\ProfilerController;
 use Symfony\Bundle\WebProfilerBundle\Csp\ContentSecurityPolicyHandler;
 use Symfony\Component\HttpFoundation\Request;
+use Symfony\Component\HttpFoundation\Response;
+use Symfony\Component\HttpKernel\DataCollector\DumpDataCollector;
+use Symfony\Component\HttpKernel\DataCollector\ExceptionDataCollector;
+use Symfony\Component\HttpKernel\DataCollector\RequestDataCollector;
 use Symfony\Component\HttpKernel\Exception\NotFoundHttpException;
 use Symfony\Component\HttpKernel\Profiler\Profile;
+use Symfony\Component\HttpKernel\Profiler\Profiler;
+use Twig\Environment;
+use Twig\Loader\LoaderInterface;
+use Twig\Loader\SourceContextLoaderInterface;

 class ProfilerControllerTest extends TestCase
 {
@ -185,7 +193,97 @@ class ProfilerControllerTest extends TestCase
        ];
    }

-    private function createController($profiler, $twig, $withCSP): ProfilerController
+    /**
+     * @dataProvider defaultPanelProvider
+     */
+    public function testDefaultPanel(string $expectedPanel, Profile $profile)
+    {
+        $profiler = $this->createMock(Profiler::class);
+        $profiler
+            ->expects($this->atLeastOnce())
+            ->method('loadProfile')
+            ->with($profile->getToken())
+            ->willReturn($profile);
+
+        $profiler
+            ->expects($this->atLeastOnce())
+            ->method('has')
+            ->with($this->logicalXor($collectorsNames = array_keys($profile->getCollectors())))
+            ->willReturn(true);
+
+        $expectedTemplate = 'expected_template.html.twig';
+
+        if (Environment::MAJOR_VERSION > 1) {
+            $loader = $this->createMock(LoaderInterface::class);
+            $loader
+                ->expects($this->atLeastOnce())
+                ->method('exists')
+                ->with($this->logicalXor($expectedTemplate, 'other_template.html.twig'))
+                ->willReturn(true);
+        } else {
+            $loader = $this->createMock(SourceContextLoaderInterface::class);
+        }
+
+        $twig = $this->createMock(Environment::class);
+        $twig
+            ->expects($this->atLeastOnce())
+            ->method('getLoader')
+            ->willReturn($loader);
+        $twig
+            ->expects($this->once())
+            ->method('render')
+            ->with($expectedTemplate);
+
+        $this
+            ->createController($profiler, $twig, false, array_map(function (string $collectorName) use ($expectedPanel, $expectedTemplate): array {
+                if ($collectorName === $expectedPanel) {
+                    return [$expectedPanel, $expectedTemplate];
+                }
+
+                return [$collectorName, 'other_template.html.twig'];
+            }, $collectorsNames))
+            ->panelAction(new Request(), $profile->getToken());
+    }
+
+    public function defaultPanelProvider(): \Generator
+    {
+        // Test default behavior
+        $profile = new Profile('xxxxxx');
+        $profile->addCollector($requestDataCollector = new RequestDataCollector());
+        yield [$requestDataCollector->getName(), $profile];
+
+        // Test exception
+        $profile = new Profile('xxxxxx');
+        $profile->addCollector($exceptionDataCollector = new ExceptionDataCollector());
+        $exceptionDataCollector->collect(new Request(), new Response(), new \DomainException());
+        yield [$exceptionDataCollector->getName(), $profile];
+
+        // Test exception priority
+        $dumpDataCollector = $this->createMock(DumpDataCollector::class);
+        $dumpDataCollector
+            ->expects($this->atLeastOnce())
+            ->method('getName')
+            ->willReturn('dump');
+        $dumpDataCollector
+            ->expects($this->atLeastOnce())
+            ->method('getDumpsCount')
+            ->willReturn(1);
+        $profile = new Profile('xxxxxx');
+        $profile->setCollectors([$exceptionDataCollector, $dumpDataCollector]);
+        yield [$exceptionDataCollector->getName(), $profile];
+
+        // Test exception priority when defined afterwards
+        $profile = new Profile('xxxxxx');
+        $profile->setCollectors([$dumpDataCollector, $exceptionDataCollector]);
+        yield [$exceptionDataCollector->getName(), $profile];
+
+        // Test dump
+        $profile = new Profile('xxxxxx');
+        $profile->addCollector($dumpDataCollector);
+        yield [$dumpDataCollector->getName(), $profile];
+    }
+
+    private function createController($profiler, $twig, $withCSP, array $templates = []): ProfilerController
    {
        $urlGenerator = $this->getMockBuilder('Symfony\Component\Routing\Generator\UrlGeneratorInterface')->getMock();

@ -193,9 +291,9 @@ class ProfilerControllerTest extends TestCase
            $nonceGenerator = $this->getMockBuilder('Symfony\Bundle\WebProfilerBundle\Csp\NonceGenerator')->getMock();
            $nonceGenerator->method('generate')->willReturn('dummy_nonce');

-            return new ProfilerController($urlGenerator, $profiler, $twig, [], new ContentSecurityPolicyHandler($nonceGenerator));
+            return new ProfilerController($urlGenerator, $profiler, $twig, $templates, new ContentSecurityPolicyHandler($nonceGenerator));
        }

-        return new ProfilerController($urlGenerator, $profiler, $twig, []);
+        return new ProfilerController($urlGenerator, $profiler, $twig, $templates);
    }
 }
--- a/src/Symfony/Bundle/WebProfilerBundle/Tests/Profiler/TemplateManagerTest.php
+++ b/src/Symfony/Bundle/WebProfilerBundle/Tests/Profiler/TemplateManagerTest.php
@ -15,7 +15,8 @@ use Symfony\Bundle\WebProfilerBundle\Profiler\TemplateManager;
 use Symfony\Bundle\WebProfilerBundle\Tests\TestCase;
 use Symfony\Component\HttpKernel\Profiler\Profile;
 use Twig\Environment;
-use Twig\Source;
+use Twig\Loader\LoaderInterface;
+use Twig\Loader\SourceContextLoaderInterface;

 /**
 * Test for TemplateManager class.
@ -104,11 +105,14 @@ class TemplateManagerTest extends TestCase
    {
        $this->twigEnvironment = $this->getMockBuilder('Twig\Environment')->disableOriginalConstructor()->getMock();

-        if (interface_exists('Twig\Loader\SourceContextLoaderInterface')) {
-            $loader = $this->getMockBuilder('Twig\Loader\SourceContextLoaderInterface')->getMock();
+        if (Environment::MAJOR_VERSION > 1) {
+            $loader = $this->createMock(LoaderInterface::class);
+            $loader
+                ->expects($this->any())
+                ->method('exists')
+                ->willReturn(true);
        } else {
-            $loader = $this->getMockBuilder('Twig\Loader\LoaderInterface')->getMock();
-            $loader->method('getSourceContext')->willReturn(new Source('source-code', 'source-name'));
+            $loader = $this->createMock(SourceContextLoaderInterface::class);
        }

        $this->twigEnvironment->expects($this->any())->method('getLoader')->willReturn($loader);