From fd39333b14d2431bf956adf9c4efab1534bff79c Mon Sep 17 00:00:00 2001 From: Vidar Langseid Date: Mon, 14 Aug 2023 15:10:28 +0200 Subject: [PATCH 1/6] IBX-6312: View matcher ParentContentType should not throw execption if parent is not available --- .../ContentBased/Id/ParentContentType.php | 10 +++++++--- .../Identifier/ParentContentType.php | 20 +++++++++++-------- 2 files changed, 19 insertions(+), 11 deletions(-) diff --git a/eZ/Publish/Core/MVC/Symfony/Matcher/ContentBased/Id/ParentContentType.php b/eZ/Publish/Core/MVC/Symfony/Matcher/ContentBased/Id/ParentContentType.php index baf3bbe15e..fec16046f9 100644 --- a/eZ/Publish/Core/MVC/Symfony/Matcher/ContentBased/Id/ParentContentType.php +++ b/eZ/Publish/Core/MVC/Symfony/Matcher/ContentBased/Id/ParentContentType.php @@ -56,9 +56,13 @@ public function match(View $view) if (!$view instanceof LocationValueView) { return false; } - $parent = $this->loadParentLocation( - $view->getLocation()->parentLocationId - ); + try { + $parent = $this->loadParentLocation( + $view->getLocation()->parentLocationId + ); + } catch (\eZ\Publish\API\Repository\Exceptions\NotFoundException $e) { + return false; + } return isset($this->values[$parent->getContentInfo()->contentTypeId]); } diff --git a/eZ/Publish/Core/MVC/Symfony/Matcher/ContentBased/Identifier/ParentContentType.php b/eZ/Publish/Core/MVC/Symfony/Matcher/ContentBased/Identifier/ParentContentType.php index 612f2e6616..d7a95862e7 100644 --- a/eZ/Publish/Core/MVC/Symfony/Matcher/ContentBased/Identifier/ParentContentType.php +++ b/eZ/Publish/Core/MVC/Symfony/Matcher/ContentBased/Identifier/ParentContentType.php @@ -25,15 +25,19 @@ class ParentContentType extends MultipleValued */ public function matchLocation(APILocation $location) { - $parentContentType = $this->repository->sudo( - static function (Repository $repository) use ($location) { - $parent = $repository->getLocationService()->loadLocation($location->parentLocationId); + try { + $parentContentType = $this->repository->sudo( + static function (Repository $repository) use ($location) { + $parent = $repository->getLocationService()->loadLocation($location->parentLocationId); - return $repository - ->getContentTypeService() - ->loadContentType($parent->getContentInfo()->contentTypeId); - } - ); + return $repository + ->getContentTypeService() + ->loadContentType($parent->getContentInfo()->contentTypeId); + } + ); + } catch (\eZ\Publish\API\Repository\Exceptions\NotFoundException $e) { + return false; + } return isset($this->values[$parentContentType->identifier]); } From 19a443430a62d7aeb4f275ede4685d1061b5f083 Mon Sep 17 00:00:00 2001 From: Vidar Langseid Date: Wed, 7 Feb 2024 14:28:16 +0100 Subject: [PATCH 2/6] Revert "IBX-6312: View matcher ParentContentType should not throw execption if parent is not available" This reverts commit fd39333b14d2431bf956adf9c4efab1534bff79c. --- .../ContentBased/Id/ParentContentType.php | 10 +++------- .../Identifier/ParentContentType.php | 20 ++++++++----------- 2 files changed, 11 insertions(+), 19 deletions(-) diff --git a/eZ/Publish/Core/MVC/Symfony/Matcher/ContentBased/Id/ParentContentType.php b/eZ/Publish/Core/MVC/Symfony/Matcher/ContentBased/Id/ParentContentType.php index fec16046f9..baf3bbe15e 100644 --- a/eZ/Publish/Core/MVC/Symfony/Matcher/ContentBased/Id/ParentContentType.php +++ b/eZ/Publish/Core/MVC/Symfony/Matcher/ContentBased/Id/ParentContentType.php @@ -56,13 +56,9 @@ public function match(View $view) if (!$view instanceof LocationValueView) { return false; } - try { - $parent = $this->loadParentLocation( - $view->getLocation()->parentLocationId - ); - } catch (\eZ\Publish\API\Repository\Exceptions\NotFoundException $e) { - return false; - } + $parent = $this->loadParentLocation( + $view->getLocation()->parentLocationId + ); return isset($this->values[$parent->getContentInfo()->contentTypeId]); } diff --git a/eZ/Publish/Core/MVC/Symfony/Matcher/ContentBased/Identifier/ParentContentType.php b/eZ/Publish/Core/MVC/Symfony/Matcher/ContentBased/Identifier/ParentContentType.php index d7a95862e7..612f2e6616 100644 --- a/eZ/Publish/Core/MVC/Symfony/Matcher/ContentBased/Identifier/ParentContentType.php +++ b/eZ/Publish/Core/MVC/Symfony/Matcher/ContentBased/Identifier/ParentContentType.php @@ -25,19 +25,15 @@ class ParentContentType extends MultipleValued */ public function matchLocation(APILocation $location) { - try { - $parentContentType = $this->repository->sudo( - static function (Repository $repository) use ($location) { - $parent = $repository->getLocationService()->loadLocation($location->parentLocationId); + $parentContentType = $this->repository->sudo( + static function (Repository $repository) use ($location) { + $parent = $repository->getLocationService()->loadLocation($location->parentLocationId); - return $repository - ->getContentTypeService() - ->loadContentType($parent->getContentInfo()->contentTypeId); - } - ); - } catch (\eZ\Publish\API\Repository\Exceptions\NotFoundException $e) { - return false; - } + return $repository + ->getContentTypeService() + ->loadContentType($parent->getContentInfo()->contentTypeId); + } + ); return isset($this->values[$parentContentType->identifier]); } From 5267ac50d2d6f99e35daa2231e904d51c2c6c49a Mon Sep 17 00:00:00 2001 From: Vidar Langseid Date: Wed, 7 Feb 2024 14:27:00 +0100 Subject: [PATCH 3/6] IBX-6312: View matcher ParentContentType should not throw execption if parent is not available --- .../Resources/config/services.yml | 2 + .../Controller/Content/PreviewController.php | 51 +++++++++++++++++-- 2 files changed, 48 insertions(+), 5 deletions(-) diff --git a/eZ/Bundle/EzPublishCoreBundle/Resources/config/services.yml b/eZ/Bundle/EzPublishCoreBundle/Resources/config/services.yml index 967c64fbb7..2902742338 100644 --- a/eZ/Bundle/EzPublishCoreBundle/Resources/config/services.yml +++ b/eZ/Bundle/EzPublishCoreBundle/Resources/config/services.yml @@ -110,6 +110,8 @@ services: $authorizationChecker: '@security.authorization_checker' $locationProvider: '@ezpublish.content_preview.location_provider' $controllerChecker: '@ezpublish.view.custom_location_controller_checker' + $debugMode: '%kernel.debug%' + $logger: '@logger' tags: - { name: controller.service_arguments } diff --git a/eZ/Publish/Core/MVC/Symfony/Controller/Content/PreviewController.php b/eZ/Publish/Core/MVC/Symfony/Controller/Content/PreviewController.php index 6ec36517f2..49eca62dbf 100644 --- a/eZ/Publish/Core/MVC/Symfony/Controller/Content/PreviewController.php +++ b/eZ/Publish/Core/MVC/Symfony/Controller/Content/PreviewController.php @@ -13,6 +13,7 @@ use eZ\Publish\API\Repository\LocationService; use eZ\Publish\API\Repository\Values\Content\Content; use eZ\Publish\API\Repository\Values\Content\Location; +use eZ\Publish\Core\Base\Exceptions\NotFoundException; use eZ\Publish\Core\Helper\ContentPreviewHelper; use eZ\Publish\Core\Helper\PreviewLocationProvider; use eZ\Publish\Core\MVC\Symfony\Routing\Generator\UrlAliasGenerator; @@ -21,13 +22,19 @@ use eZ\Publish\Core\MVC\Symfony\SiteAccess; use eZ\Publish\Core\MVC\Symfony\View\CustomLocationControllerChecker; use eZ\Publish\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; use Symfony\Component\Security\Core\Authorization\AuthorizationCheckerInterface; use Symfony\Component\Security\Core\Exception\AccessDeniedException; class PreviewController { + use LoggerAwareTrait; + public const PREVIEW_PARAMETER_NAME = 'isPreview'; public const CONTENT_VIEW_ROUTE = '_ez_content_view'; @@ -52,6 +59,8 @@ class PreviewController /** @var \eZ\Publish\Core\MVC\Symfony\View\CustomLocationControllerChecker */ private $controllerChecker; + private bool $debugMode; + public function __construct( ContentService $contentService, LocationService $locationService, @@ -59,7 +68,9 @@ public function __construct( ContentPreviewHelper $previewHelper, AuthorizationCheckerInterface $authorizationChecker, PreviewLocationProvider $locationProvider, - CustomLocationControllerChecker $controllerChecker + CustomLocationControllerChecker $controllerChecker, + bool $debugMode, + ?LoggerInterface $logger = null, ) { $this->contentService = $contentService; $this->locationService = $locationService; @@ -68,6 +79,8 @@ public function __construct( $this->authorizationChecker = $authorizationChecker; $this->locationProvider = $locationProvider; $this->controllerChecker = $controllerChecker; + $this->debugMode = $debugMode; + $this->logger = $logger ?? new NullLogger(); } /** @@ -118,17 +131,45 @@ public function previewContentAction( 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 = <<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); + throw new Exception($message, 0, $e); + } + } catch (\Exception $e2) { + $this->logger->warning('Unable to check if location uses a custom controller when loading the preview page', ['exception' => $e2]); + if ($this->debugMode) { + throw $e2; + } + } + $message = ''; + + if ($e instanceof NotFoundException) { + $message .= 'Location not found or not available in requested language'; + $this->logger->warning('Location not found or not available in requested language when loading the preview page', ['exception' => $e]); + if ($this->debugMode) { + throw new Exception($message, 0, $e); + } } else { + $this->logger->warning('Unable to load the preview page', ['exception' => $e]); + } + + if ($this->debugMode) { throw $e; } + + $message = <<$message

+

Unable to load the preview page

+

See logs for more information

+EOF; + + return new Response($message); } $response->setPrivate(); From 3ab0ce5e500f3e86896d6fcebd2a20d75bb9956a Mon Sep 17 00:00:00 2001 From: Vidar Langseid Date: Wed, 7 Feb 2024 14:57:34 +0100 Subject: [PATCH 4/6] fixup! IBX-6312: View matcher ParentContentType should not throw execption if parent is not available - fixed test --- .../Tests/Controller/Content/PreviewControllerTest.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/eZ/Publish/Core/MVC/Symfony/Controller/Tests/Controller/Content/PreviewControllerTest.php b/eZ/Publish/Core/MVC/Symfony/Controller/Tests/Controller/Content/PreviewControllerTest.php index 64f8f7e5d8..0e37a3456d 100644 --- a/eZ/Publish/Core/MVC/Symfony/Controller/Tests/Controller/Content/PreviewControllerTest.php +++ b/eZ/Publish/Core/MVC/Symfony/Controller/Tests/Controller/Content/PreviewControllerTest.php @@ -70,7 +70,9 @@ protected function getPreviewController(): PreviewController $this->previewHelper, $this->authorizationChecker, $this->locationProvider, - $this->controllerChecker + $this->controllerChecker, + false, + null ); } From c530f8250e9de60b4b03c4ae25c7d29bcc127dfe Mon Sep 17 00:00:00 2001 From: Vidar Langseid Date: Wed, 7 Feb 2024 15:15:43 +0100 Subject: [PATCH 5/6] fixup! fixup! IBX-6312: View matcher ParentContentType should not throw execption if parent is not available - fixed test --- .../MVC/Symfony/Controller/Content/PreviewController.php | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/eZ/Publish/Core/MVC/Symfony/Controller/Content/PreviewController.php b/eZ/Publish/Core/MVC/Symfony/Controller/Content/PreviewController.php index 49eca62dbf..afeee1edba 100644 --- a/eZ/Publish/Core/MVC/Symfony/Controller/Content/PreviewController.php +++ b/eZ/Publish/Core/MVC/Symfony/Controller/Content/PreviewController.php @@ -59,7 +59,8 @@ class PreviewController /** @var \eZ\Publish\Core\MVC\Symfony\View\CustomLocationControllerChecker */ private $controllerChecker; - private bool $debugMode; + /** @var bool */ + private $debugMode; public function __construct( ContentService $contentService, @@ -70,7 +71,7 @@ public function __construct( PreviewLocationProvider $locationProvider, CustomLocationControllerChecker $controllerChecker, bool $debugMode, - ?LoggerInterface $logger = null, + ?LoggerInterface $logger = null ) { $this->contentService = $contentService; $this->locationService = $locationService; From d60e020e20a18106441155fc5b9b49408ffda3a7 Mon Sep 17 00:00:00 2001 From: Andrew Longosz Date: Mon, 25 Mar 2024 11:40:53 +0100 Subject: [PATCH 6/6] Refactored previewAction controller to improve error response (#404) --- .../Controller/Content/PreviewController.php | 88 +++++++++++-------- 1 file changed, 49 insertions(+), 39 deletions(-) diff --git a/eZ/Publish/Core/MVC/Symfony/Controller/Content/PreviewController.php b/eZ/Publish/Core/MVC/Symfony/Controller/Content/PreviewController.php index afeee1edba..d37c06faf0 100644 --- a/eZ/Publish/Core/MVC/Symfony/Controller/Content/PreviewController.php +++ b/eZ/Publish/Core/MVC/Symfony/Controller/Content/PreviewController.php @@ -8,12 +8,13 @@ use Exception; use eZ\Publish\API\Repository\ContentService; +use eZ\Publish\API\Repository\Exceptions\NotFoundException as APINotFoundException; use eZ\Publish\API\Repository\Exceptions\NotImplementedException; use eZ\Publish\API\Repository\Exceptions\UnauthorizedException; use eZ\Publish\API\Repository\LocationService; use eZ\Publish\API\Repository\Values\Content\Content; use eZ\Publish\API\Repository\Values\Content\Location; -use eZ\Publish\Core\Base\Exceptions\NotFoundException; +use eZ\Publish\Core\Base\Exceptions\BadStateException; use eZ\Publish\Core\Helper\ContentPreviewHelper; use eZ\Publish\Core\Helper\PreviewLocationProvider; use eZ\Publish\Core\MVC\Symfony\Routing\Generator\UrlAliasGenerator; @@ -87,7 +88,7 @@ public function __construct( /** * @throws \eZ\Publish\API\Repository\Exceptions\NotImplementedException If Content is missing location as this is not supported in current version * @throws \eZ\Publish\API\Repository\Exceptions\NotFoundException - * @throws \eZ\Publish\API\Repository\Exceptions\UnauthorizedException + * @throws \eZ\Publish\API\Repository\Exceptions\BadStateException */ public function previewContentAction( Request $request, @@ -96,7 +97,7 @@ public function previewContentAction( $language, $siteAccessName = null, ?int $locationId = null - ) { + ): Response { $this->previewHelper->setPreviewActive(true); try { @@ -131,47 +132,21 @@ public function previewContentAction( HttpKernelInterface::SUB_REQUEST, false ); - } catch (\Exception $e) { - try { - 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); - } - } catch (\Exception $e2) { - $this->logger->warning('Unable to check if location uses a custom controller when loading the preview page', ['exception' => $e2]); - if ($this->debugMode) { - throw $e2; - } - } - $message = ''; - - if ($e instanceof NotFoundException) { - $message .= 'Location not found or not available in requested language'; - $this->logger->warning('Location not found or not available in requested language when loading the preview page', ['exception' => $e]); - if ($this->debugMode) { - throw new Exception($message, 0, $e); - } - } else { - $this->logger->warning('Unable to load the preview page', ['exception' => $e]); - } - + } catch (APINotFoundException $e) { + $message = 'Location not found or not available in requested language'; + $this->logger->warning( + 'Location not found or not available in requested language when loading the preview page', + ['exception' => $e] + ); if ($this->debugMode) { - throw $e; + throw new BadStateException($message, 1, $e); } - $message = <<$message

-

Unable to load the preview page

-

See logs for more information

-EOF; - return new Response($message); + } catch (Exception $e) { + return $this->buildResponseForGenericPreviewError($location, $content, $e); } + $response->setPrivate(); $this->previewHelper->restoreConfigScope(); @@ -228,4 +203,39 @@ protected function getForwardRequest(Location $location, Content $content, SiteA $forwardRequestParameters ); } + + /** + * @throws \eZ\Publish\API\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($message, 1, $e); + } + + return new Response($message); + } }