IBX-6312: Fixed ParentContentType View Matcher for not available pare…

…nt (#438) For more details see https://issues.ibexa.co/browse/IBX-6312 and #438 Key changes: * Changed View matcher ParentContentType not to throw an exception if parent is not available * Refactored previewAction controller to improve error response * [PHPStan] Added false negative Logger issue to PHPStan config --------- Co-Authored-By: Paweł Niedzielski <[email protected]> Co-Authored-By: Konrad Oboza <[email protected]>
ibexa · Nov 28, 2024 · 312f7ea · 312f7ea
1 parent dad432e
commit 312f7ea
Show file tree

Hide file tree

Showing 4 changed files with 110 additions and 13 deletions.
diff --git a/phpstan.neon.dist b/phpstan.neon.dist
@@ -13,6 +13,8 @@ parameters:
             paths:
                 - src/*
                 - tests/*
+        -
+            message: "#^Cannot call method warning\\(\\) on Psr\\\\Log\\\\LoggerInterface\\|null\\.$#"
     paths:
         - src
         - tests

diff --git a/src/bundle/Core/Resources/config/services.yml b/src/bundle/Core/Resources/config/services.yml
@@ -110,6 +110,8 @@ services:
             $authorizationChecker: "@security.authorization_checker"
             $locationProvider: '@Ibexa\Core\Helper\PreviewLocationProvider'
             $controllerChecker: '@Ibexa\Core\MVC\Symfony\View\CustomLocationControllerChecker'
+            $debugMode: '%kernel.debug%'
+            $logger: '@logger'
         tags:
             - { name: controller.service_arguments }
 

diff --git a/src/lib/MVC/Symfony/Controller/Content/PreviewController.php b/src/lib/MVC/Symfony/Controller/Content/PreviewController.php
@@ -8,11 +8,13 @@
 
 use Exception;
 use Ibexa\Contracts\Core\Repository\ContentService;
+use Ibexa\Contracts\Core\Repository\Exceptions\NotFoundException as APINotFoundException;
 use Ibexa\Contracts\Core\Repository\Exceptions\NotImplementedException;
 use Ibexa\Contracts\Core\Repository\Exceptions\UnauthorizedException;
 use Ibexa\Contracts\Core\Repository\LocationService;
 use Ibexa\Contracts\Core\Repository\Values\Content\Content;
 use Ibexa\Contracts\Core\Repository\Values\Content\Location;
+use Ibexa\Core\Base\Exceptions\BadStateException;
 use Ibexa\Core\Helper\ContentPreviewHelper;
 use Ibexa\Core\Helper\PreviewLocationProvider;
 use Ibexa\Core\MVC\Symfony\Routing\Generator\UrlAliasGenerator;
@@ -21,6 +23,9 @@
 use Ibexa\Core\MVC\Symfony\SiteAccess;
 use Ibexa\Core\MVC\Symfony\View\CustomLocationControllerChecker;
 use Ibexa\Core\MVC\Symfony\View\ViewManagerInterface;
+use Psr\Log\LoggerAwareTrait;
+use Psr\Log\LoggerInterface;
+use Psr\Log\NullLogger;
 use Symfony\Component\HttpFoundation\Request;
 use Symfony\Component\HttpFoundation\Response;
 use Symfony\Component\HttpKernel\HttpKernelInterface;
@@ -29,6 +34,8 @@
 
 class PreviewController
 {
+    use LoggerAwareTrait;
+
     public const PREVIEW_PARAMETER_NAME = 'isPreview';
     public const CONTENT_VIEW_ROUTE = 'ibexa.content.view';
 
@@ -53,14 +60,18 @@ class PreviewController
     /** @var \Ibexa\Core\MVC\Symfony\View\CustomLocationControllerChecker */
     private $controllerChecker;
 
+    private bool $debugMode;
+
     public function __construct(
         ContentService $contentService,
         LocationService $locationService,
         HttpKernelInterface $kernel,
         ContentPreviewHelper $previewHelper,
         AuthorizationCheckerInterface $authorizationChecker,
         PreviewLocationProvider $locationProvider,
-        CustomLocationControllerChecker $controllerChecker
+        CustomLocationControllerChecker $controllerChecker,
+        bool $debugMode = false,
+        ?LoggerInterface $logger = null
     ) {
         $this->contentService = $contentService;
         $this->locationService = $locationService;
@@ -69,6 +80,8 @@ public function __construct(
         $this->authorizationChecker = $authorizationChecker;
         $this->locationProvider = $locationProvider;
         $this->controllerChecker = $controllerChecker;
+        $this->debugMode = $debugMode;
+        $this->logger = $logger ?? new NullLogger();
     }
 
     /**
@@ -119,18 +132,19 @@ public function previewContentAction(
                 HttpKernelInterface::SUB_REQUEST,
                 false
             );
-        } catch (\Exception $e) {
-            if ($location->isDraft() && $this->controllerChecker->usesCustomController($content, $location)) {
-                // @todo This should probably be an exception that embeds the original one
-                $message = <<<EOF
-<p>The view that rendered this location draft uses a custom controller, and resulted in a fatal error.</p>
-<p>Location View is deprecated, as it causes issues with preview, such as an empty location id when previewing the first version of a content.</p>
-EOF;
-
-                throw new Exception($message, 0, $e);
-            } else {
-                throw $e;
+        } catch (APINotFoundException $e) {
+            $message = sprintf('Location (%s) not found or not available in requested language (%s)', $location->id, $language);
+            $this->logger->warning(
+                sprintf('%s %s', $message, 'when loading the preview page'),
+                ['exception' => $e]
+            );
+            if ($this->debugMode) {
+                throw new BadStateException('Preview page', $message, $e);
             }
+
+            return new Response($message);
+        } catch (Exception $e) {
+            return $this->buildResponseForGenericPreviewError($location, $content, $e);
         }
         $response->headers->addCacheControlDirective('no-cache', true);
         $response->setPrivate();
@@ -187,6 +201,41 @@ protected function getForwardRequest(
             $forwardRequestParameters
         );
     }
+
+    /**
+     * @throws \Ibexa\Contracts\Core\Repository\Exceptions\BadStateException
+     */
+    private function buildResponseForGenericPreviewError(Location $location, Content $content, Exception $e): Response
+    {
+        $message = '';
+        try {
+            if ($location->isDraft() && $this->controllerChecker->usesCustomController($content, $location)) {
+                $message = <<<EOF
+                    <p>The view that rendered this location draft uses a custom controller, and resulted in a fatal error.</p>
+                    <p>Location View is deprecated, as it causes issues with preview, such as an empty location id when previewing the first version of a content.</p>
+                    EOF;
+            }
+        } catch (Exception $innerException) {
+            $message = 'An exception occurred when handling page preview exception';
+            $this->logger->warning(
+                'Unable to check if location uses a custom controller when loading the preview page',
+                ['exception' => $innerException]
+            );
+        }
+
+        $this->logger->warning('Unable to load the preview page', ['exception' => $e]);
+
+        $message .= <<<EOF
+<p>Unable to load the preview page</p>
+<p>See logs for more information</p>
+EOF;
+
+        if ($this->debugMode) {
+            throw new BadStateException('Preview page', $message, $e);
+        }
+
+        return new Response($message);
+    }
 }
 
 class_alias(PreviewController::class, 'eZ\Publish\Core\MVC\Symfony\Controller\Content\PreviewController');
diff --git a/tests/lib/MVC/Symfony/Controller/Controller/Content/PreviewControllerTest.php b/tests/lib/MVC/Symfony/Controller/Controller/Content/PreviewControllerTest.php
@@ -12,13 +12,15 @@
 use Ibexa\Contracts\Core\Repository\LocationService;
 use Ibexa\Contracts\Core\Repository\Values\Content\Content;
 use Ibexa\Contracts\Core\Repository\Values\Content\Location;
+use Ibexa\Core\Base\Exceptions\NotFoundException;
 use Ibexa\Core\Base\Exceptions\UnauthorizedException;
 use Ibexa\Core\Helper\ContentPreviewHelper;
 use Ibexa\Core\Helper\PreviewLocationProvider;
 use Ibexa\Core\MVC\Symfony\Controller\Content\PreviewController;
 use Ibexa\Core\MVC\Symfony\SiteAccess;
 use Ibexa\Core\MVC\Symfony\View\CustomLocationControllerChecker;
 use PHPUnit\Framework\TestCase;
+use Psr\Log\LoggerInterface;
 use Symfony\Component\HttpFoundation\Request;
 use Symfony\Component\HttpFoundation\Response;
 use Symfony\Component\HttpKernel\HttpKernelInterface;
@@ -51,6 +53,9 @@ final class PreviewControllerTest extends TestCase
     /** @var \Ibexa\Core\MVC\Symfony\View\CustomLocationControllerChecker&\PHPUnit\Framework\MockObject\MockObject */
     protected CustomLocationControllerChecker $controllerChecker;
 
+    /** @var \Psr\Log\LoggerInterface&\PHPUnit\Framework\MockObject\MockObject */
+    private LoggerInterface $logger;
+
     protected function setUp(): void
     {
         parent::setUp();
@@ -62,6 +67,7 @@ protected function setUp(): void
         $this->authorizationChecker = $this->createMock(AuthorizationCheckerInterface::class);
         $this->locationProvider = $this->createMock(PreviewLocationProvider::class);
         $this->controllerChecker = $this->createMock(CustomLocationControllerChecker::class);
+        $this->logger = $this->createMock(LoggerInterface::class);
     }
 
     protected function getPreviewController(): PreviewController
@@ -73,7 +79,9 @@ protected function getPreviewController(): PreviewController
             $this->previewHelper,
             $this->authorizationChecker,
             $this->locationProvider,
-            $this->controllerChecker
+            $this->controllerChecker,
+            false,
+            $this->logger
         );
     }
 
@@ -128,6 +136,42 @@ public function testPreviewCanUserFail(): void
         $controller->previewContentAction(new Request(), $contentId, $versionNo, $lang, 'test');
     }
 
+    public function testPreviewWithLogMessage(): void
+    {
+        $controller = $this->getPreviewController();
+        $contentId = 123;
+        $lang = 'eng-GB';
+        $versionNo = 3;
+        $content = $this->createMock(Content::class);
+
+        $location = $this->createMock(Location::class);
+        $location->method('__get')->with('id')->willReturn('42');
+
+        $siteAccess = $this->createMock(SiteAccess::class);
+        $this->locationProvider
+            ->method('loadMainLocationByContent')
+            ->with($content)
+            ->willReturn($location)
+        ;
+        $this->contentService
+            ->method('loadContent')
+            ->with($contentId, [$lang], $versionNo)
+            ->willReturn($content)
+        ;
+
+        $this->authorizationChecker->method('isGranted')->willReturn(true);
+        $siteAccess->name = 'test';
+        $this->previewHelper->method('getOriginalSiteAccess')->willReturn($siteAccess);
+        $this->httpKernel->method('handle')->willThrowException(new NotFoundException('Foo Property', 'foobar'));
+
+        $this->logger
+            ->expects(self::once())
+            ->method('warning')
+            ->with('Location (42) not found or not available in requested language (eng-GB) when loading the preview page');
+
+        $controller->previewContentAction(new Request(), $contentId, $versionNo, $lang, 'test');
+    }
+
     /**
      * @return iterable<string, array{SiteAccess|null, int, string, int, int|null, string|null}>
      */
-Original file line number
+Diff line change
@@ Expand Up / @@ -13,6 +13,8 @@ parameters: @@
                 paths:
                     - src/*
                     - tests/*
+            -
+                message: "#^Cannot call method warning\\(\\) on Psr\\\\Log\\\\LoggerInterface\\|null\\.$#"
         paths:
             - src
             - tests
@@ Expand Down @@