IBX-6504: Gracefully handled URL generation in RoutingExtension

ezsystems · Oct 20, 2023 · 1de9753 · 1de9753
1 parent 0e29887
commit 1de9753
Show file tree

Hide file tree

Showing 5 changed files with 37 additions and 169 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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.