Wip on fixing no-layout issues

.github/workflows/php.yml

@@ -0,0 +1,37 @@
+name: PHP CI
+
+on: [push]
+
+jobs:
+    build-test:
+        runs-on: ${{ matrix.operating-system }}
+
+        strategy:
+            matrix:
+                operating-system: [ ubuntu-latest, windows-latest, macos-latest ]
+                php-version:
+                    - '7.3'
+                    - '7.4'
+                    - '8.0'
+        name: PHP ${{ matrix.php-version }} on ${{ matrix.operating-system }}
+        steps:
+            - uses: actions/checkout@v2
+
+            - uses: shivammathur/setup-php@v2
+              with:
+                  php-version: "${{ matrix.php-version }}"
+
+            - run: composer validate
+
+            - id: composer-cache
+              run: echo "::set-output name=dir::$(composer config cache-files-dir)"
+
+            - uses: actions/cache@v1
+              with:
+                  path: ${{ steps.composer-cache.outputs.dir }}
+                  key: ${{ runner.os }}-composer-${{ hashFiles('**/composer.lock') }}
+                  restore-keys: ${{ runner.os }}-composer-
+
+            - run: composer install
+
+            - run: vendor/bin/phpunit

EventListener/LayoutsListener.php

@@ -11,6 +11,9 @@
 
 namespace Orbitale\Bundle\CmsBundle\EventListener;
 
+use Orbitale\Bundle\CmsBundle\Controller\CategoryController;
+use Orbitale\Bundle\CmsBundle\Controller\PageController;
+use Orbitale\Bundle\CmsBundle\Controller\PostsController;
 use Symfony\Component\EventDispatcher\EventSubscriberInterface;
 use Symfony\Component\HttpKernel\Event\RequestEvent;
 use Symfony\Component\HttpKernel\KernelEvents;
@@ -82,21 +85,45 @@ public function setRequestLayout(RequestEvent $event): void
             $layouts = $this->layouts;
             do {
                 $finalLayout = array_shift($layouts);
+                if (!$finalLayout) {
+                    continue;
+                }
                 if ($finalLayout['host'] || $finalLayout['pattern']) {
                     $finalLayout = null;
                 }
             } while (null === $finalLayout && count($layouts));
         }
 
-        if (null === $finalLayout || !$this->twig->getLoader()->exists($finalLayout['resource'])) {
-            $source = new Source('', $finalLayout['resource']);
+        if (null === $finalLayout) {
+            // Means that there is no fall-back to "default layout".
+
+            $controller = $request->attributes->get('_controller');
+
+            if (
+                !is_a($controller, PageController::class, true)
+                && !is_a($controller, CategoryController::class, true)
+                && !is_a($controller, PostsController::class, true)
+            ) {
+                // Don't do anything if there's no layout and the controller isn't supposed to use it.
+                // If the user still wants to use a layout "outside" the built-in controllers,
+                // they will have to add a layout config for it anyway.
+                return;
+            }
+
+            throw new \RuntimeException(sprintf(
+                'Unable to find layout for url "%s://%s%s". Did you forget to add a layout configuration for this path?',
+                $request->getScheme(), $host, $path
+            ));
+        }
 
-            throw new LoaderError(sprintf(
+        if (!$this->twig->getLoader()->exists($finalLayout['resource'])) {
+            throw new \RuntimeException(sprintf(
                 'Unable to find template %s for layout %s. The "layout" parameter must be a valid twig view to be used as a layout.',
-                $finalLayout['resource'], $finalLayout['name']
-            ), 0, $source);
+                $finalLayout['resource'] ?? '', $finalLayout['name'] ?? ''
+            ));
         }
 
+
         $event->getRequest()->attributes->set('_orbitale_cms_layout', $finalLayout);
     }
 }

Tests/EventListener/LayoutsListenerTest.php

@@ -12,8 +12,14 @@
 namespace Orbitale\Bundle\CmsBundle\Tests\EventListener;
 
 use Doctrine\ORM\EntityManagerInterface;
+use Orbitale\Bundle\CmsBundle\Controller\CategoryController;
+use Orbitale\Bundle\CmsBundle\Controller\PageController;
+use Orbitale\Bundle\CmsBundle\Controller\PostsController;
+use Orbitale\Bundle\CmsBundle\EventListener\LayoutsListener;
 use Orbitale\Bundle\CmsBundle\Tests\AbstractTestCase;
 use Orbitale\Bundle\CmsBundle\Tests\Fixtures\TestBundle\Entity\Page;
+use Symfony\Component\HttpFoundation\Request;
+use Symfony\Component\HttpKernel\Event\RequestEvent;
 use Twig\Environment;
 use Twig\Error\LoaderError;
 
@@ -44,11 +50,64 @@ public function testHostLayout(): void
 
     public function testLayoutWrong(): void
     {
-        $this->expectException(LoaderError::class);
-        $this->expectExceptionMessage('Unable to find template this_layout_does_not_exist.html.twig for layout front. The "layout" parameter must be a valid twig view to be used as a layout in "this_layout_does_not_exist.html.twig".');
+        $this->expectException(\RuntimeException::class);
+        $this->expectExceptionMessage('Unable to find template this_layout_does_not_exist.html.twig for layout front. The "layout" parameter must be a valid twig view to be used as a layout.');
         static::createClient(['environment' => 'layout_wrong'])->request('GET', '/page/');
     }
 
+    public function testNoLayoutsDoesNotSetRequestAttribute()
+    {
+        $kernel = static::bootKernel();
+        $request = Request::create('/');
+        $listener = new LayoutsListener([], $this->getTwig());
+        $listener->setRequestLayout(new RequestEvent($kernel, $request, $kernel::MASTER_REQUEST));
+        static::assertFalse($request->attributes->has('_orbitale_cms_layout'));
+    }
+
+    public function testNoMatchingLayoutDoesNotSetRequestAttribute()
+    {
+        $kernel = static::bootKernel();
+
+        $listener = new LayoutsListener([
+            [
+                'pattern' => '/noop',
+                'host' => '',
+                'resource' => '.',
+            ],
+        ], $this->getTwig());
+        $request = Request::create('/');
+        $listener->setRequestLayout(new RequestEvent($kernel, $request, $kernel::MASTER_REQUEST));
+
+        static::assertFalse($request->attributes->has('_orbitale_cms_layout'));
+    }
+
+    /** @dataProvider provideBundleControllerClasses */
+    public function testNoMatchingLayoutWithBundleControllerThrowsException(string $controller)
+    {
+        $kernel = static::bootKernel();
+
+        $listener = new LayoutsListener([
+            [
+                'pattern' => '/noop',
+                'host' => '',
+                'resource' => '.',
+            ],
+        ], $this->getTwig());
+        $request = Request::create('/no-way');
+        $request->attributes->set('_controller', $controller);
+
+        $this->expectException(\RuntimeException::class);
+        $this->expectExceptionMessage('Unable to find layout for url "http://localhost/no-way". Did you forget to add a layout configuration for this path?');
+        $listener->setRequestLayout(new RequestEvent($kernel, $request, $kernel::MASTER_REQUEST));
+    }
+
+    public function provideBundleControllerClasses(): \Generator
+    {
+        yield [PageController::class];
+        yield [CategoryController::class];
+        yield [PostsController::class];
+    }
+
     /**
      * {@inheritdoc}
      */
@@ -71,4 +130,9 @@ protected static function createClient(array $options = [], array $server = [])
 
         return $client;
     }
+
+    private function getTwig(): Environment
+    {
+        return $this->createMock(Environment::class);
+    }
 }

Tests/bootstrap.php

@@ -49,8 +49,8 @@
 
     $application = new Application($kernel);
     $application->setAutoExit(false);
-    $application->run(new ArrayInput(['command' => 'doctrine:database:create']));
-    $application->run(new ArrayInput(['command' => 'doctrine:schema:create']));
+    $application->run(new ArrayInput(['command' => 'doctrine:database:create']), new NullOutput());
+    $application->run(new ArrayInput(['command' => 'doctrine:schema:create']), new NullOutput());
 
     $kernel->shutdown();
 })();