From 312f7ea498f9642ea20c0c217840a73ab23f1739 Mon Sep 17 00:00:00 2001 From: Vidar Langseid Date: Thu, 28 Nov 2024 11:44:37 +0100 Subject: [PATCH] IBX-6312: Fixed ParentContentType View Matcher for not available parent (#438) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit For more details see https://issues.ibexa.co/browse/IBX-6312 and https://github.com/ibexa/core/pull/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 Co-Authored-By: Konrad Oboza --- phpstan.neon.dist | 2 + src/bundle/Core/Resources/config/services.yml | 2 + .../Controller/Content/PreviewController.php | 73 ++++++++++++++++--- .../Content/PreviewControllerTest.php | 46 +++++++++++- 4 files changed, 110 insertions(+), 13 deletions(-) diff --git a/phpstan.neon.dist b/phpstan.neon.dist index a8e4451c5c..6dcc8ee0aa 100644 --- 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 index 93db51df29..03585c9438 100644 --- 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 index 2bfec8d11f..919d0b2906 100644 --- 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,6 +60,8 @@ class PreviewController /** @var \Ibexa\Core\MVC\Symfony\View\CustomLocationControllerChecker */ private $controllerChecker; + private bool $debugMode; + public function __construct( ContentService $contentService, LocationService $locationService, @@ -60,7 +69,9 @@ public function __construct( 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 = <<The view that rendered this location draft uses a custom controller, and resulted in a fatal error.

-

Location View is deprecated, as it causes issues with preview, such as an empty location id when previewing the first version of a content.

-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 = <<The view that rendered this location draft uses a custom controller, and resulted in a fatal error.

+

Location View is deprecated, as it causes issues with preview, such as an empty location id when previewing the first version of a content.

+ 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 .= <<Unable to load the preview page

+

See logs for more information

+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 index 04eee4c2c1..7040853152 100644 --- a/tests/lib/MVC/Symfony/Controller/Controller/Content/PreviewControllerTest.php +++ b/tests/lib/MVC/Symfony/Controller/Controller/Content/PreviewControllerTest.php @@ -12,6 +12,7 @@ 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; @@ -19,6 +20,7 @@ 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 */