feature #33783 [WebProfilerBundle] Try to display the most useful pan…

…el 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 #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
-------

a45dd98 [WebProfilerBundle] Try to display the most useful panel by default

src/Symfony/Bundle/WebProfilerBundle/Controller/ProfilerController.php

@@ -17,6 +17,8 @@
 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 @@ public function panelAction(Request $request, $token)
             $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 @@ public function panelAction(Request $request, $token)
             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));
         }

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);
             }

src/Symfony/Bundle/WebProfilerBundle/Tests/Controller/ProfilerControllerTest.php

@@ -15,8 +15,16 @@
 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,17 +193,107 @@ public function provideCspVariants()
         ];
     }
 
-    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();
 
         if ($withCSP) {
             $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);
     }
 }

src/Symfony/Bundle/WebProfilerBundle/Tests/Profiler/TemplateManagerTest.php

@@ -15,7 +15,8 @@
 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 @@ protected function mockTwigEnvironment()
     {
         $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);