From 87489f247cbbd31157ee52ccd49718516ab23498 Mon Sep 17 00:00:00 2001 From: Bartek Wajda Date: Thu, 19 Oct 2023 18:34:34 +0200 Subject: [PATCH 1/7] IBX-6504: Used `forcedLanguage` parameter in `RoutingExtension` during preview --- .../Resources/config/templating.yml | 6 +- .../MVC/Symfony/Routing/UrlAliasRouter.php | 5 +- .../Twig/Extension/RoutingExtension.php | 58 ++++++++++++++++++- 3 files changed, 66 insertions(+), 3 deletions(-) diff --git a/eZ/Bundle/EzPublishCoreBundle/Resources/config/templating.yml b/eZ/Bundle/EzPublishCoreBundle/Resources/config/templating.yml index eb9bc89c50..388042259c 100644 --- a/eZ/Bundle/EzPublishCoreBundle/Resources/config/templating.yml +++ b/eZ/Bundle/EzPublishCoreBundle/Resources/config/templating.yml @@ -122,7 +122,11 @@ services: ezpublish.templating.extension.routing: class: eZ\Publish\Core\MVC\Symfony\Templating\Twig\Extension\RoutingExtension - arguments: ["@ezpublish.route_reference.generator", "@router"] + arguments: + $routeReferenceGenerator: "@ezpublish.route_reference.generator" + $urlGenerator: "@router" + $contentPreviewHelper: "@ezpublish.content_preview_helper" + $locationService: "@ezpublish.api.service.location" tags: - {name: twig.extension} diff --git a/eZ/Publish/Core/MVC/Symfony/Routing/UrlAliasRouter.php b/eZ/Publish/Core/MVC/Symfony/Routing/UrlAliasRouter.php index edfbb52f45..590d32115e 100644 --- a/eZ/Publish/Core/MVC/Symfony/Routing/UrlAliasRouter.php +++ b/eZ/Publish/Core/MVC/Symfony/Routing/UrlAliasRouter.php @@ -314,7 +314,10 @@ public function generate(string $name, array $parameters = [], int $referenceTyp ); } - $location = isset($parameters['location']) ? $parameters['location'] : $this->locationService->loadLocation($parameters['locationId']); + $location = $parameters['location'] ?? $this->locationService->loadLocation( + $parameters['locationId'], + isset($parameters['forcedLanguage']) ? [$parameters['forcedLanguage']] : null + ); unset($parameters['location'], $parameters['locationId'], $parameters['viewType'], $parameters['layout']); return $this->generator->generate($location, $parameters, $referenceType); diff --git a/eZ/Publish/Core/MVC/Symfony/Templating/Twig/Extension/RoutingExtension.php b/eZ/Publish/Core/MVC/Symfony/Templating/Twig/Extension/RoutingExtension.php index 49409f425a..93e43f39ef 100644 --- a/eZ/Publish/Core/MVC/Symfony/Templating/Twig/Extension/RoutingExtension.php +++ b/eZ/Publish/Core/MVC/Symfony/Templating/Twig/Extension/RoutingExtension.php @@ -8,9 +8,12 @@ namespace eZ\Publish\Core\MVC\Symfony\Templating\Twig\Extension; +use eZ\Publish\API\Repository\Exceptions\NotFoundException; +use eZ\Publish\API\Repository\LocationService; use eZ\Publish\API\Repository\Values\Content\Content; use eZ\Publish\API\Repository\Values\Content\ContentInfo; use eZ\Publish\API\Repository\Values\Content\Location; +use eZ\Publish\Core\Helper\ContentPreviewHelper; use eZ\Publish\Core\MVC\Symfony\Routing\Generator\RouteReferenceGeneratorInterface; use eZ\Publish\Core\MVC\Symfony\Routing\RouteReference; use eZ\Publish\Core\MVC\Symfony\Routing\UrlAliasRouter; @@ -30,12 +33,22 @@ class RoutingExtension extends AbstractExtension /** @var \Symfony\Component\Routing\Generator\UrlGeneratorInterface */ private $urlGenerator; + /** @var \eZ\Publish\Core\Helper\ContentPreviewHelper */ + private $contentPreviewHelper; + + /** @var \eZ\Publish\API\Repository\LocationService */ + private $locationService; + public function __construct( RouteReferenceGeneratorInterface $routeReferenceGenerator, - UrlGeneratorInterface $urlGenerator + UrlGeneratorInterface $urlGenerator, + ContentPreviewHelper $contentPreviewHelper, + LocationService $locationService ) { $this->routeReferenceGenerator = $routeReferenceGenerator; $this->urlGenerator = $urlGenerator; + $this->contentPreviewHelper = $contentPreviewHelper; + $this->locationService = $locationService; } public function getFunctions(): array @@ -74,6 +87,9 @@ public function getRouteReference($resource = null, $params = []): RouteReferenc return $this->routeReferenceGenerator->generate($resource, $params); } + /** + * @throws \eZ\Publish\API\Repository\Exceptions\UnauthorizedException + */ public function getPath(object $name, array $parameters = [], bool $relative = false): string { $referenceType = $relative ? UrlGeneratorInterface::RELATIVE_PATH : UrlGeneratorInterface::ABSOLUTE_PATH; @@ -81,6 +97,9 @@ public function getPath(object $name, array $parameters = [], bool $relative = f return $this->generateUrlForObject($name, $parameters, $referenceType); } + /** + * @throws \eZ\Publish\API\Repository\Exceptions\UnauthorizedException + */ public function getUrl(object $name, array $parameters = [], bool $schemeRelative = false): string { $referenceType = $schemeRelative ? UrlGeneratorInterface::NETWORK_PATH : UrlGeneratorInterface::ABSOLUTE_URL; @@ -88,17 +107,22 @@ public function getUrl(object $name, array $parameters = [], bool $schemeRelativ return $this->generateUrlForObject($name, $parameters, $referenceType); } + /** + * @throws \eZ\Publish\API\Repository\Exceptions\UnauthorizedException + */ private function generateUrlForObject(object $object, array $parameters, int $referenceType): string { if ($object instanceof Location) { $routeName = UrlAliasRouter::URL_ALIAS_ROUTE_NAME; $parameters += [ 'locationId' => $object->id, + 'forcedLanguage' => $this->getForcedLanguageCodeBasedOnPreview(), ]; } elseif ($object instanceof Content || $object instanceof ContentInfo) { $routeName = UrlAliasRouter::URL_ALIAS_ROUTE_NAME; $parameters += [ 'contentId' => $object->id, + 'forcedLanguage' => $this->getForcedLanguageCodeBasedOnPreview(), ]; } elseif ($object instanceof RouteReference) { $routeName = $object->getRoute(); @@ -113,6 +137,38 @@ private function generateUrlForObject(object $object, array $parameters, int $re return $this->urlGenerator->generate($routeName, $parameters, $referenceType); } + /** + * @throws \eZ\Publish\API\Repository\Exceptions\UnauthorizedException + */ + private function getForcedLanguageCodeBasedOnPreview(): ?string + { + if ($this->contentPreviewHelper->isPreviewActive() === false) { + return null; + } + + $previewedContent = $this->contentPreviewHelper->getPreviewedContent(); + $versionInfo = $previewedContent->getVersionInfo(); + $contentInfo = $versionInfo->getContentInfo(); + $alwaysAvailable = $versionInfo->getContentInfo()->alwaysAvailable; + if ($alwaysAvailable) { + return null; + } + + $previewedLocation = $this->contentPreviewHelper->getPreviewedLocation(); + try { + $this->locationService->loadLocation( + $previewedLocation->id, + [$versionInfo->initialLanguageCode], + true + ); + + return null; + } catch (NotFoundException $e) { + // Use initial language as a forced language + return $contentInfo->getMainLanguageCode(); + } + } + /** * Determines at compile time whether the generated URL will be safe and thus * saving the unneeded automatic escaping for performance reasons. From 05d8a8ea6905d091220997063467297e772c74c1 Mon Sep 17 00:00:00 2001 From: Bartek Wajda Date: Thu, 19 Oct 2023 20:30:03 +0200 Subject: [PATCH 2/7] IBX-6504: Minor refactoring --- .../Core/MVC/Symfony/Routing/UrlAliasRouter.php | 2 +- .../Twig/Extension/RoutingExtensionTest.php | 6 +++++- .../Twig/Extension/RoutingExtension.php | 16 +++++++++++++--- 3 files changed, 19 insertions(+), 5 deletions(-) diff --git a/eZ/Publish/Core/MVC/Symfony/Routing/UrlAliasRouter.php b/eZ/Publish/Core/MVC/Symfony/Routing/UrlAliasRouter.php index 590d32115e..f76e3a8985 100644 --- a/eZ/Publish/Core/MVC/Symfony/Routing/UrlAliasRouter.php +++ b/eZ/Publish/Core/MVC/Symfony/Routing/UrlAliasRouter.php @@ -316,7 +316,7 @@ public function generate(string $name, array $parameters = [], int $referenceTyp $location = $parameters['location'] ?? $this->locationService->loadLocation( $parameters['locationId'], - isset($parameters['forcedLanguage']) ? [$parameters['forcedLanguage']] : null + isset($parameters['forcedLanguageCode']) ? [$parameters['forcedLanguageCode']] : null ); unset($parameters['location'], $parameters['locationId'], $parameters['viewType'], $parameters['layout']); diff --git a/eZ/Publish/Core/MVC/Symfony/Templating/Tests/Twig/Extension/RoutingExtensionTest.php b/eZ/Publish/Core/MVC/Symfony/Templating/Tests/Twig/Extension/RoutingExtensionTest.php index ef73d615ba..2829ccaccd 100644 --- a/eZ/Publish/Core/MVC/Symfony/Templating/Tests/Twig/Extension/RoutingExtensionTest.php +++ b/eZ/Publish/Core/MVC/Symfony/Templating/Tests/Twig/Extension/RoutingExtensionTest.php @@ -8,9 +8,11 @@ namespace eZ\Publish\Core\MVC\Symfony\Templating\Tests\Twig\Extension; +use eZ\Publish\API\Repository\LocationService; use eZ\Publish\API\Repository\Values\Content\Content as APIContent; use eZ\Publish\API\Repository\Values\Content\ContentInfo; use eZ\Publish\API\Repository\Values\Content\Location as APILocation; +use eZ\Publish\Core\Helper\ContentPreviewHelper; use eZ\Publish\Core\MVC\Symfony\Routing\Generator\RouteReferenceGenerator; use eZ\Publish\Core\MVC\Symfony\Routing\Generator\RouteReferenceGeneratorInterface; use eZ\Publish\Core\MVC\Symfony\Routing\RouteReference; @@ -32,7 +34,9 @@ protected function getExtensions(): array return [ new RoutingExtension( $this->getRouteReferenceGenerator(), - $this->getUrlGenerator() + $this->getUrlGenerator(), + $this->createMock(ContentPreviewHelper::class), + $this->createMock(LocationService::class), ), ]; } diff --git a/eZ/Publish/Core/MVC/Symfony/Templating/Twig/Extension/RoutingExtension.php b/eZ/Publish/Core/MVC/Symfony/Templating/Twig/Extension/RoutingExtension.php index 93e43f39ef..1e8ee136e5 100644 --- a/eZ/Publish/Core/MVC/Symfony/Templating/Twig/Extension/RoutingExtension.php +++ b/eZ/Publish/Core/MVC/Symfony/Templating/Twig/Extension/RoutingExtension.php @@ -114,15 +114,25 @@ private function generateUrlForObject(object $object, array $parameters, int $re { if ($object instanceof Location) { $routeName = UrlAliasRouter::URL_ALIAS_ROUTE_NAME; + $forcedLanguageCode = $this->getForcedLanguageCodeBasedOnPreview(); + if ($forcedLanguageCode !== null) { + $parameters += [ + 'forcedLanguageCode' => $forcedLanguageCode, + ]; + } $parameters += [ 'locationId' => $object->id, - 'forcedLanguage' => $this->getForcedLanguageCodeBasedOnPreview(), ]; } elseif ($object instanceof Content || $object instanceof ContentInfo) { $routeName = UrlAliasRouter::URL_ALIAS_ROUTE_NAME; + $forcedLanguageCode = $this->getForcedLanguageCodeBasedOnPreview(); + if ($forcedLanguageCode !== null) { + $parameters += [ + 'forcedLanguageCode' => $forcedLanguageCode, + ]; + } $parameters += [ 'contentId' => $object->id, - 'forcedLanguage' => $this->getForcedLanguageCodeBasedOnPreview(), ]; } elseif ($object instanceof RouteReference) { $routeName = $object->getRoute(); @@ -142,7 +152,7 @@ private function generateUrlForObject(object $object, array $parameters, int $re */ private function getForcedLanguageCodeBasedOnPreview(): ?string { - if ($this->contentPreviewHelper->isPreviewActive() === false) { + if ($this->contentPreviewHelper->isPreviewActive() !== true) { return null; } From 0e2988770a2124b0172772849608507f963d45a8 Mon Sep 17 00:00:00 2001 From: Bartek Wajda Date: Fri, 20 Oct 2023 12:20:51 +0200 Subject: [PATCH 3/7] IBX-6504: Added unit tests --- .../Routing/Tests/UrlAliasRouterTest.php | 69 ++++++++++++++++++- .../MVC/Symfony/Routing/UrlAliasRouter.php | 21 +++++- 2 files changed, 86 insertions(+), 4 deletions(-) diff --git a/eZ/Publish/Core/MVC/Symfony/Routing/Tests/UrlAliasRouterTest.php b/eZ/Publish/Core/MVC/Symfony/Routing/Tests/UrlAliasRouterTest.php index a0ace6b9bf..bc834cdf35 100644 --- a/eZ/Publish/Core/MVC/Symfony/Routing/Tests/UrlAliasRouterTest.php +++ b/eZ/Publish/Core/MVC/Symfony/Routing/Tests/UrlAliasRouterTest.php @@ -777,7 +777,7 @@ public function testGenerateWithContentId() public function testGenerateWithContentIdWithMissingMainLocation() { - $this->expectException(\LogicException::class); + $this->expectException(\TypeError::class); $contentId = 456; $contentInfo = new ContentInfo(['id' => $contentId, 'mainLocationId' => null]); @@ -795,4 +795,71 @@ public function testGenerateWithContentIdWithMissingMainLocation() $referenceType ); } + + public function testGenerateForLocationIdWithForcedLanguageCode(): void + { + $locationId = 22; + $languageCode = 'ger-DE'; + $location = new Location(['id' => $locationId]); + $parameters = ['forcedLanguageCode' => $languageCode]; + $referenceType = UrlGeneratorInterface::ABSOLUTE_PATH; + $generatedLink = '/foo/bar'; + + $this->locationService + ->expects(self::once()) + ->method('loadLocation') + ->with($locationId, [$languageCode]) + ->willReturn($location); + $this->urlALiasGenerator + ->expects(self::once()) + ->method('generate') + ->with($location, [], $referenceType) + ->willReturn($generatedLink); + + self::assertSame( + $generatedLink, + $this->router->generate( + UrlAliasRouter::URL_ALIAS_ROUTE_NAME, + $parameters + ['locationId' => $locationId], + $referenceType + ) + ); + } + + public function testGenerateForContentIdWithForcedLanguageCode(): void + { + $locationId = 23; + $contentId = 34; + $languageCode = 'ger-DE'; + $location = new Location(['id' => $locationId]); + $contentInfo = new ContentInfo(['id' => $contentId, 'mainLocationId' => $locationId]); + $parameters = ['forcedLanguageCode' => $languageCode]; + $referenceType = UrlGeneratorInterface::ABSOLUTE_PATH; + $generatedLink = '/foo/bar'; + + $this->contentService + ->expects(self::once()) + ->method('loadContentInfo') + ->with($contentId) + ->will(self::returnValue($contentInfo)); + $this->locationService + ->expects(self::once()) + ->method('loadLocation') + ->with($contentInfo->mainLocationId, [$languageCode]) + ->willReturn($location); + $this->urlALiasGenerator + ->expects(self::once()) + ->method('generate') + ->with($location, [], $referenceType) + ->willReturn($generatedLink); + + self::assertSame( + $generatedLink, + $this->router->generate( + UrlAliasRouter::URL_ALIAS_ROUTE_NAME, + $parameters + ['contentId' => $contentId], + $referenceType + ) + ); + } } diff --git a/eZ/Publish/Core/MVC/Symfony/Routing/UrlAliasRouter.php b/eZ/Publish/Core/MVC/Symfony/Routing/UrlAliasRouter.php index f76e3a8985..b78f044941 100644 --- a/eZ/Publish/Core/MVC/Symfony/Routing/UrlAliasRouter.php +++ b/eZ/Publish/Core/MVC/Symfony/Routing/UrlAliasRouter.php @@ -318,21 +318,36 @@ public function generate(string $name, array $parameters = [], int $referenceTyp $parameters['locationId'], isset($parameters['forcedLanguageCode']) ? [$parameters['forcedLanguageCode']] : null ); - unset($parameters['location'], $parameters['locationId'], $parameters['viewType'], $parameters['layout']); + unset( + $parameters['location'], + $parameters['locationId'], + $parameters['viewType'], + $parameters['layout'], + $parameters['forcedLanguageCode'], + ); return $this->generator->generate($location, $parameters, $referenceType); } if (isset($parameters['contentId'])) { $contentInfo = $this->contentService->loadContentInfo($parameters['contentId']); - unset($parameters['contentId'], $parameters['viewType'], $parameters['layout']); + $location = $this->locationService->loadLocation( + $contentInfo->mainLocationId, + isset($parameters['forcedLanguageCode']) ? [$parameters['forcedLanguageCode']] : null + ); + unset( + $parameters['contentId'], + $parameters['viewType'], + $parameters['layout'], + $parameters['forcedLanguageCode'], + ); if (empty($contentInfo->mainLocationId)) { throw new LogicException('Cannot generate a UrlAlias route for content without main Location.'); } return $this->generator->generate( - $this->locationService->loadLocation($contentInfo->mainLocationId), + $location, $parameters, $referenceType ); From 1de9753918493f2911f61c8e6aa5522152f1c3d9 Mon Sep 17 00:00:00 2001 From: Bartek Wajda Date: Fri, 20 Oct 2023 13:16:49 +0200 Subject: [PATCH 4/7] IBX-6504: Gracefully handled URL generation in `RoutingExtension` --- .../Resources/config/templating.yml | 9 +- .../Routing/Tests/UrlAliasRouterTest.php | 69 +------------ .../MVC/Symfony/Routing/UrlAliasRouter.php | 26 +---- .../Twig/Extension/RoutingExtensionTest.php | 6 +- .../Twig/Extension/RoutingExtension.php | 96 ++++++------------- 5 files changed, 37 insertions(+), 169 deletions(-) diff --git a/eZ/Bundle/EzPublishCoreBundle/Resources/config/templating.yml b/eZ/Bundle/EzPublishCoreBundle/Resources/config/templating.yml index 388042259c..f0aa4f0fe0 100644 --- a/eZ/Bundle/EzPublishCoreBundle/Resources/config/templating.yml +++ b/eZ/Bundle/EzPublishCoreBundle/Resources/config/templating.yml @@ -122,13 +122,10 @@ services: ezpublish.templating.extension.routing: class: eZ\Publish\Core\MVC\Symfony\Templating\Twig\Extension\RoutingExtension - arguments: - $routeReferenceGenerator: "@ezpublish.route_reference.generator" - $urlGenerator: "@router" - $contentPreviewHelper: "@ezpublish.content_preview_helper" - $locationService: "@ezpublish.api.service.location" + arguments: ["@ezpublish.route_reference.generator", "@router"] tags: - - {name: twig.extension} + - { name: twig.extension } + - { name: 'monolog.logger', channel: 'ibexa.core' } eZ\Publish\Core\MVC\Symfony\Templating\Twig\ResourceProvider: arguments: diff --git a/eZ/Publish/Core/MVC/Symfony/Routing/Tests/UrlAliasRouterTest.php b/eZ/Publish/Core/MVC/Symfony/Routing/Tests/UrlAliasRouterTest.php index bc834cdf35..a0ace6b9bf 100644 --- a/eZ/Publish/Core/MVC/Symfony/Routing/Tests/UrlAliasRouterTest.php +++ b/eZ/Publish/Core/MVC/Symfony/Routing/Tests/UrlAliasRouterTest.php @@ -777,7 +777,7 @@ public function testGenerateWithContentId() public function testGenerateWithContentIdWithMissingMainLocation() { - $this->expectException(\TypeError::class); + $this->expectException(\LogicException::class); $contentId = 456; $contentInfo = new ContentInfo(['id' => $contentId, 'mainLocationId' => null]); @@ -795,71 +795,4 @@ public function testGenerateWithContentIdWithMissingMainLocation() $referenceType ); } - - public function testGenerateForLocationIdWithForcedLanguageCode(): void - { - $locationId = 22; - $languageCode = 'ger-DE'; - $location = new Location(['id' => $locationId]); - $parameters = ['forcedLanguageCode' => $languageCode]; - $referenceType = UrlGeneratorInterface::ABSOLUTE_PATH; - $generatedLink = '/foo/bar'; - - $this->locationService - ->expects(self::once()) - ->method('loadLocation') - ->with($locationId, [$languageCode]) - ->willReturn($location); - $this->urlALiasGenerator - ->expects(self::once()) - ->method('generate') - ->with($location, [], $referenceType) - ->willReturn($generatedLink); - - self::assertSame( - $generatedLink, - $this->router->generate( - UrlAliasRouter::URL_ALIAS_ROUTE_NAME, - $parameters + ['locationId' => $locationId], - $referenceType - ) - ); - } - - public function testGenerateForContentIdWithForcedLanguageCode(): void - { - $locationId = 23; - $contentId = 34; - $languageCode = 'ger-DE'; - $location = new Location(['id' => $locationId]); - $contentInfo = new ContentInfo(['id' => $contentId, 'mainLocationId' => $locationId]); - $parameters = ['forcedLanguageCode' => $languageCode]; - $referenceType = UrlGeneratorInterface::ABSOLUTE_PATH; - $generatedLink = '/foo/bar'; - - $this->contentService - ->expects(self::once()) - ->method('loadContentInfo') - ->with($contentId) - ->will(self::returnValue($contentInfo)); - $this->locationService - ->expects(self::once()) - ->method('loadLocation') - ->with($contentInfo->mainLocationId, [$languageCode]) - ->willReturn($location); - $this->urlALiasGenerator - ->expects(self::once()) - ->method('generate') - ->with($location, [], $referenceType) - ->willReturn($generatedLink); - - self::assertSame( - $generatedLink, - $this->router->generate( - UrlAliasRouter::URL_ALIAS_ROUTE_NAME, - $parameters + ['contentId' => $contentId], - $referenceType - ) - ); - } } diff --git a/eZ/Publish/Core/MVC/Symfony/Routing/UrlAliasRouter.php b/eZ/Publish/Core/MVC/Symfony/Routing/UrlAliasRouter.php index b78f044941..edfbb52f45 100644 --- a/eZ/Publish/Core/MVC/Symfony/Routing/UrlAliasRouter.php +++ b/eZ/Publish/Core/MVC/Symfony/Routing/UrlAliasRouter.php @@ -314,40 +314,22 @@ public function generate(string $name, array $parameters = [], int $referenceTyp ); } - $location = $parameters['location'] ?? $this->locationService->loadLocation( - $parameters['locationId'], - isset($parameters['forcedLanguageCode']) ? [$parameters['forcedLanguageCode']] : null - ); - unset( - $parameters['location'], - $parameters['locationId'], - $parameters['viewType'], - $parameters['layout'], - $parameters['forcedLanguageCode'], - ); + $location = isset($parameters['location']) ? $parameters['location'] : $this->locationService->loadLocation($parameters['locationId']); + unset($parameters['location'], $parameters['locationId'], $parameters['viewType'], $parameters['layout']); return $this->generator->generate($location, $parameters, $referenceType); } if (isset($parameters['contentId'])) { $contentInfo = $this->contentService->loadContentInfo($parameters['contentId']); - $location = $this->locationService->loadLocation( - $contentInfo->mainLocationId, - isset($parameters['forcedLanguageCode']) ? [$parameters['forcedLanguageCode']] : null - ); - unset( - $parameters['contentId'], - $parameters['viewType'], - $parameters['layout'], - $parameters['forcedLanguageCode'], - ); + unset($parameters['contentId'], $parameters['viewType'], $parameters['layout']); if (empty($contentInfo->mainLocationId)) { throw new LogicException('Cannot generate a UrlAlias route for content without main Location.'); } return $this->generator->generate( - $location, + $this->locationService->loadLocation($contentInfo->mainLocationId), $parameters, $referenceType ); diff --git a/eZ/Publish/Core/MVC/Symfony/Templating/Tests/Twig/Extension/RoutingExtensionTest.php b/eZ/Publish/Core/MVC/Symfony/Templating/Tests/Twig/Extension/RoutingExtensionTest.php index 2829ccaccd..ef73d615ba 100644 --- a/eZ/Publish/Core/MVC/Symfony/Templating/Tests/Twig/Extension/RoutingExtensionTest.php +++ b/eZ/Publish/Core/MVC/Symfony/Templating/Tests/Twig/Extension/RoutingExtensionTest.php @@ -8,11 +8,9 @@ namespace eZ\Publish\Core\MVC\Symfony\Templating\Tests\Twig\Extension; -use eZ\Publish\API\Repository\LocationService; use eZ\Publish\API\Repository\Values\Content\Content as APIContent; use eZ\Publish\API\Repository\Values\Content\ContentInfo; use eZ\Publish\API\Repository\Values\Content\Location as APILocation; -use eZ\Publish\Core\Helper\ContentPreviewHelper; use eZ\Publish\Core\MVC\Symfony\Routing\Generator\RouteReferenceGenerator; use eZ\Publish\Core\MVC\Symfony\Routing\Generator\RouteReferenceGeneratorInterface; use eZ\Publish\Core\MVC\Symfony\Routing\RouteReference; @@ -34,9 +32,7 @@ protected function getExtensions(): array return [ new RoutingExtension( $this->getRouteReferenceGenerator(), - $this->getUrlGenerator(), - $this->createMock(ContentPreviewHelper::class), - $this->createMock(LocationService::class), + $this->getUrlGenerator() ), ]; } diff --git a/eZ/Publish/Core/MVC/Symfony/Templating/Twig/Extension/RoutingExtension.php b/eZ/Publish/Core/MVC/Symfony/Templating/Twig/Extension/RoutingExtension.php index 1e8ee136e5..9e30aa4d69 100644 --- a/eZ/Publish/Core/MVC/Symfony/Templating/Twig/Extension/RoutingExtension.php +++ b/eZ/Publish/Core/MVC/Symfony/Templating/Twig/Extension/RoutingExtension.php @@ -9,46 +9,43 @@ namespace eZ\Publish\Core\MVC\Symfony\Templating\Twig\Extension; use eZ\Publish\API\Repository\Exceptions\NotFoundException; -use eZ\Publish\API\Repository\LocationService; use eZ\Publish\API\Repository\Values\Content\Content; use eZ\Publish\API\Repository\Values\Content\ContentInfo; use eZ\Publish\API\Repository\Values\Content\Location; -use eZ\Publish\Core\Helper\ContentPreviewHelper; use eZ\Publish\Core\MVC\Symfony\Routing\Generator\RouteReferenceGeneratorInterface; use eZ\Publish\Core\MVC\Symfony\Routing\RouteReference; use eZ\Publish\Core\MVC\Symfony\Routing\UrlAliasRouter; +use Psr\Log\LoggerAwareInterface; +use Psr\Log\LoggerAwareTrait; +use Psr\Log\LoggerInterface; +use Psr\Log\NullLogger; use Symfony\Cmf\Component\Routing\RouteObjectInterface; use Symfony\Component\Routing\Generator\UrlGeneratorInterface; +use Throwable; use Twig\Extension\AbstractExtension; use Twig\Node\Expression\ArrayExpression; use Twig\Node\Expression\ConstantExpression; use Twig\Node\Node; use Twig\TwigFunction; -class RoutingExtension extends AbstractExtension +class RoutingExtension extends AbstractExtension implements LoggerAwareInterface { + use LoggerAwareTrait; + /** @var \eZ\Publish\Core\MVC\Symfony\Routing\Generator\RouteReferenceGeneratorInterface */ private $routeReferenceGenerator; /** @var \Symfony\Component\Routing\Generator\UrlGeneratorInterface */ private $urlGenerator; - /** @var \eZ\Publish\Core\Helper\ContentPreviewHelper */ - private $contentPreviewHelper; - - /** @var \eZ\Publish\API\Repository\LocationService */ - private $locationService; - public function __construct( RouteReferenceGeneratorInterface $routeReferenceGenerator, UrlGeneratorInterface $urlGenerator, - ContentPreviewHelper $contentPreviewHelper, - LocationService $locationService + ?LoggerInterface $logger = null ) { $this->routeReferenceGenerator = $routeReferenceGenerator; $this->urlGenerator = $urlGenerator; - $this->contentPreviewHelper = $contentPreviewHelper; - $this->locationService = $locationService; + $this->logger = $logger ?? new NullLogger(); } public function getFunctions(): array @@ -87,50 +84,45 @@ public function getRouteReference($resource = null, $params = []): RouteReferenc return $this->routeReferenceGenerator->generate($resource, $params); } - /** - * @throws \eZ\Publish\API\Repository\Exceptions\UnauthorizedException - */ public function getPath(object $name, array $parameters = [], bool $relative = false): string { $referenceType = $relative ? UrlGeneratorInterface::RELATIVE_PATH : UrlGeneratorInterface::ABSOLUTE_PATH; - return $this->generateUrlForObject($name, $parameters, $referenceType); + return $this->tryGeneratingUrlForObject($name, $parameters, $referenceType); } - /** - * @throws \eZ\Publish\API\Repository\Exceptions\UnauthorizedException - */ public function getUrl(object $name, array $parameters = [], bool $schemeRelative = false): string { $referenceType = $schemeRelative ? UrlGeneratorInterface::NETWORK_PATH : UrlGeneratorInterface::ABSOLUTE_URL; - return $this->generateUrlForObject($name, $parameters, $referenceType); + return $this->tryGeneratingUrlForObject($name, $parameters, $referenceType); + } + + private function tryGeneratingUrlForObject(object $object, array $parameters, int $referenceType): string + { + try { + return $this->generateUrlForObject($object, $parameters, $referenceType); + } catch (NotFoundException $e) { + return ''; + } catch (Throwable $e) { + $this->logger->warning( + 'Url could not be generated.', + ['exception' => $e] + ); + + return ''; + } } - /** - * @throws \eZ\Publish\API\Repository\Exceptions\UnauthorizedException - */ private function generateUrlForObject(object $object, array $parameters, int $referenceType): string { if ($object instanceof Location) { $routeName = UrlAliasRouter::URL_ALIAS_ROUTE_NAME; - $forcedLanguageCode = $this->getForcedLanguageCodeBasedOnPreview(); - if ($forcedLanguageCode !== null) { - $parameters += [ - 'forcedLanguageCode' => $forcedLanguageCode, - ]; - } $parameters += [ 'locationId' => $object->id, ]; } elseif ($object instanceof Content || $object instanceof ContentInfo) { $routeName = UrlAliasRouter::URL_ALIAS_ROUTE_NAME; - $forcedLanguageCode = $this->getForcedLanguageCodeBasedOnPreview(); - if ($forcedLanguageCode !== null) { - $parameters += [ - 'forcedLanguageCode' => $forcedLanguageCode, - ]; - } $parameters += [ 'contentId' => $object->id, ]; @@ -147,38 +139,6 @@ private function generateUrlForObject(object $object, array $parameters, int $re return $this->urlGenerator->generate($routeName, $parameters, $referenceType); } - /** - * @throws \eZ\Publish\API\Repository\Exceptions\UnauthorizedException - */ - private function getForcedLanguageCodeBasedOnPreview(): ?string - { - if ($this->contentPreviewHelper->isPreviewActive() !== true) { - return null; - } - - $previewedContent = $this->contentPreviewHelper->getPreviewedContent(); - $versionInfo = $previewedContent->getVersionInfo(); - $contentInfo = $versionInfo->getContentInfo(); - $alwaysAvailable = $versionInfo->getContentInfo()->alwaysAvailable; - if ($alwaysAvailable) { - return null; - } - - $previewedLocation = $this->contentPreviewHelper->getPreviewedLocation(); - try { - $this->locationService->loadLocation( - $previewedLocation->id, - [$versionInfo->initialLanguageCode], - true - ); - - return null; - } catch (NotFoundException $e) { - // Use initial language as a forced language - return $contentInfo->getMainLanguageCode(); - } - } - /** * Determines at compile time whether the generated URL will be safe and thus * saving the unneeded automatic escaping for performance reasons. From 791b764e227720160ca4d6309a28c8a4b0018576 Mon Sep 17 00:00:00 2001 From: Bartek Wajda Date: Mon, 23 Oct 2023 12:43:02 +0200 Subject: [PATCH 5/7] IBX-6504: Applied review remarks --- .../Resources/config/templating.yml | 3 +-- .../Twig/Extension/RoutingExtension.php | 20 ++----------------- 2 files changed, 3 insertions(+), 20 deletions(-) diff --git a/eZ/Bundle/EzPublishCoreBundle/Resources/config/templating.yml b/eZ/Bundle/EzPublishCoreBundle/Resources/config/templating.yml index f0aa4f0fe0..eb9bc89c50 100644 --- a/eZ/Bundle/EzPublishCoreBundle/Resources/config/templating.yml +++ b/eZ/Bundle/EzPublishCoreBundle/Resources/config/templating.yml @@ -124,8 +124,7 @@ services: class: eZ\Publish\Core\MVC\Symfony\Templating\Twig\Extension\RoutingExtension arguments: ["@ezpublish.route_reference.generator", "@router"] tags: - - { name: twig.extension } - - { name: 'monolog.logger', channel: 'ibexa.core' } + - {name: twig.extension} eZ\Publish\Core\MVC\Symfony\Templating\Twig\ResourceProvider: arguments: diff --git a/eZ/Publish/Core/MVC/Symfony/Templating/Twig/Extension/RoutingExtension.php b/eZ/Publish/Core/MVC/Symfony/Templating/Twig/Extension/RoutingExtension.php index 9e30aa4d69..689b9469a4 100644 --- a/eZ/Publish/Core/MVC/Symfony/Templating/Twig/Extension/RoutingExtension.php +++ b/eZ/Publish/Core/MVC/Symfony/Templating/Twig/Extension/RoutingExtension.php @@ -15,23 +15,16 @@ use eZ\Publish\Core\MVC\Symfony\Routing\Generator\RouteReferenceGeneratorInterface; use eZ\Publish\Core\MVC\Symfony\Routing\RouteReference; use eZ\Publish\Core\MVC\Symfony\Routing\UrlAliasRouter; -use Psr\Log\LoggerAwareInterface; -use Psr\Log\LoggerAwareTrait; -use Psr\Log\LoggerInterface; -use Psr\Log\NullLogger; use Symfony\Cmf\Component\Routing\RouteObjectInterface; use Symfony\Component\Routing\Generator\UrlGeneratorInterface; -use Throwable; use Twig\Extension\AbstractExtension; use Twig\Node\Expression\ArrayExpression; use Twig\Node\Expression\ConstantExpression; use Twig\Node\Node; use Twig\TwigFunction; -class RoutingExtension extends AbstractExtension implements LoggerAwareInterface +class RoutingExtension extends AbstractExtension { - use LoggerAwareTrait; - /** @var \eZ\Publish\Core\MVC\Symfony\Routing\Generator\RouteReferenceGeneratorInterface */ private $routeReferenceGenerator; @@ -40,12 +33,10 @@ class RoutingExtension extends AbstractExtension implements LoggerAwareInterface public function __construct( RouteReferenceGeneratorInterface $routeReferenceGenerator, - UrlGeneratorInterface $urlGenerator, - ?LoggerInterface $logger = null + UrlGeneratorInterface $urlGenerator ) { $this->routeReferenceGenerator = $routeReferenceGenerator; $this->urlGenerator = $urlGenerator; - $this->logger = $logger ?? new NullLogger(); } public function getFunctions(): array @@ -103,13 +94,6 @@ private function tryGeneratingUrlForObject(object $object, array $parameters, in try { return $this->generateUrlForObject($object, $parameters, $referenceType); } catch (NotFoundException $e) { - return ''; - } catch (Throwable $e) { - $this->logger->warning( - 'Url could not be generated.', - ['exception' => $e] - ); - return ''; } } From 124723a0dd881b0ab0d983f132674124f17cf6e7 Mon Sep 17 00:00:00 2001 From: Bartek Wajda Date: Tue, 24 Oct 2023 10:45:00 +0200 Subject: [PATCH 6/7] IBX-6504: Load all locations when preview is active in `LocationParamConverter` --- .../Converter/LocationParamConverter.php | 19 ++++++++++++++++--- .../Resources/config/services.yml | 3 ++- .../Converter/LocationParamConverterTest.php | 7 ++++++- 3 files changed, 24 insertions(+), 5 deletions(-) diff --git a/eZ/Bundle/EzPublishCoreBundle/Converter/LocationParamConverter.php b/eZ/Bundle/EzPublishCoreBundle/Converter/LocationParamConverter.php index 65dac4d7db..26524099fd 100644 --- a/eZ/Bundle/EzPublishCoreBundle/Converter/LocationParamConverter.php +++ b/eZ/Bundle/EzPublishCoreBundle/Converter/LocationParamConverter.php @@ -7,15 +7,22 @@ namespace eZ\Bundle\EzPublishCoreBundle\Converter; use eZ\Publish\API\Repository\LocationService; +use eZ\Publish\API\Repository\Values\Content\Language; +use eZ\Publish\API\Repository\Values\Content\Location; +use eZ\Publish\Core\Helper\ContentPreviewHelper; class LocationParamConverter extends RepositoryParamConverter { /** @var \eZ\Publish\API\Repository\LocationService */ private $locationService; - public function __construct(LocationService $locationService) + /** @var \eZ\Publish\Core\Helper\ContentPreviewHelper */ + private $contentPreviewHelper; + + public function __construct(LocationService $locationService, ContentPreviewHelper $contentPreviewHelper) { $this->locationService = $locationService; + $this->contentPreviewHelper = $contentPreviewHelper; } protected function getSupportedClass() @@ -28,8 +35,14 @@ protected function getPropertyName() return 'locationId'; } - protected function loadValueObject($id) + /** + * @throws \eZ\Publish\API\Repository\Exceptions\NotFoundException + * @throws \eZ\Publish\API\Repository\Exceptions\UnauthorizedException + */ + protected function loadValueObject($id): Location { - return $this->locationService->loadLocation($id); + $prioritizedLanguages = $this->contentPreviewHelper->isPreviewActive() ? Language::ALL : null; + + return $this->locationService->loadLocation($id, $prioritizedLanguages); } } diff --git a/eZ/Bundle/EzPublishCoreBundle/Resources/config/services.yml b/eZ/Bundle/EzPublishCoreBundle/Resources/config/services.yml index a7c34b72c5..039ac5f963 100644 --- a/eZ/Bundle/EzPublishCoreBundle/Resources/config/services.yml +++ b/eZ/Bundle/EzPublishCoreBundle/Resources/config/services.yml @@ -191,7 +191,8 @@ services: ezpublish.param_converter.location: class: eZ\Bundle\EzPublishCoreBundle\Converter\LocationParamConverter arguments: - - "@ezpublish.siteaccessaware.service.location" + $locationService: "@ezpublish.siteaccessaware.service.location" + $contentPreviewHelper: '@ezpublish.content_preview_helper' tags: - { name: request.param_converter, priority: "%ezpublish.param_converter.location.priority%", converter: ez_location_converter } diff --git a/eZ/Bundle/EzPublishCoreBundle/Tests/Converter/LocationParamConverterTest.php b/eZ/Bundle/EzPublishCoreBundle/Tests/Converter/LocationParamConverterTest.php index 09e828fa89..58bb42e6e1 100644 --- a/eZ/Bundle/EzPublishCoreBundle/Tests/Converter/LocationParamConverterTest.php +++ b/eZ/Bundle/EzPublishCoreBundle/Tests/Converter/LocationParamConverterTest.php @@ -9,6 +9,7 @@ use eZ\Bundle\EzPublishCoreBundle\Converter\LocationParamConverter; use eZ\Publish\API\Repository\LocationService; use eZ\Publish\API\Repository\Values\Content\Location; +use eZ\Publish\Core\Helper\ContentPreviewHelper; use Symfony\Component\HttpFoundation\Request; class LocationParamConverterTest extends AbstractParamConverterTest @@ -22,11 +23,15 @@ class LocationParamConverterTest extends AbstractParamConverterTest private $locationServiceMock; + /** @var \eZ\Publish\Core\Helper\ContentPreviewHelper&\PHPUnit\Framework\MockObject\MockObject */ + private $contentPreviewHelperMock; + protected function setUp(): void { $this->locationServiceMock = $this->createMock(LocationService::class); + $this->contentPreviewHelperMock = $this->createMock(ContentPreviewHelper::class); - $this->converter = new LocationParamConverter($this->locationServiceMock); + $this->converter = new LocationParamConverter($this->locationServiceMock, $this->contentPreviewHelperMock); } public function testSupports() From c2279442372e7511bebccbe35a1346d9267c9693 Mon Sep 17 00:00:00 2001 From: Bartek Wajda Date: Thu, 26 Oct 2023 12:48:02 +0200 Subject: [PATCH 7/7] IBX-6504: CS --- eZ/Bundle/EzPublishCoreBundle/Resources/config/services.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/eZ/Bundle/EzPublishCoreBundle/Resources/config/services.yml b/eZ/Bundle/EzPublishCoreBundle/Resources/config/services.yml index 039ac5f963..967c64fbb7 100644 --- a/eZ/Bundle/EzPublishCoreBundle/Resources/config/services.yml +++ b/eZ/Bundle/EzPublishCoreBundle/Resources/config/services.yml @@ -191,7 +191,7 @@ services: ezpublish.param_converter.location: class: eZ\Bundle\EzPublishCoreBundle\Converter\LocationParamConverter arguments: - $locationService: "@ezpublish.siteaccessaware.service.location" + $locationService: '@ezpublish.siteaccessaware.service.location' $contentPreviewHelper: '@ezpublish.content_preview_helper' tags: - { name: request.param_converter, priority: "%ezpublish.param_converter.location.priority%", converter: ez_location_converter }