Make canonical record URL consistent across routes

src/Canonical.php

@@ -159,15 +159,10 @@ public function setPath(?string $route = null, array $params = []): void
             $route = $this->request->attributes->get('_route');
         }
 
-        try {
-            $this->path = $this->generateLink($route, $params, false);
-        } catch (InvalidParameterException | MissingMandatoryParametersException $e) {
-            // Just use the current URL /shrug
-            $this->path = $this->request->getUri();
-        }
+        $this->path = $this->generateLink($route, $params, false);
     }
 
-    public function generateLink(string $route, array $params, $canonical = false): ?string
+    public function generateLink(?string $route, ?array $params, $canonical = false): ?string
     {
         if (isset($params['_locale']) && $params['_locale'] === $this->defaultLocale) {
             unset($params['_locale']);
@@ -181,10 +176,15 @@ public function generateLink(string $route, array $params, $canonical = false):
             }
         }
 
-        return $this->urlGenerator->generate(
-            $route,
-            $params,
-            $canonical ? UrlGeneratorInterface::ABSOLUTE_URL : UrlGeneratorInterface::ABSOLUTE_PATH
-        );
+        try {
+            return $this->urlGenerator->generate(
+                $route,
+                $params,
+                $canonical ? UrlGeneratorInterface::ABSOLUTE_URL : UrlGeneratorInterface::ABSOLUTE_PATH
+            );
+        } catch (InvalidParameterException | MissingMandatoryParametersException | RouteNotFoundException $e) {
+            // Just use the current URL /shrug
+            return$this->request->getUri();
+        }
     }
 }

src/Controller/Frontend/DetailController.php

@@ -10,6 +10,7 @@
 use Bolt\Enum\Statuses;
 use Bolt\Repository\ContentRepository;
 use Bolt\TemplateChooser;
+use Bolt\Utils\ContentHelper;
 use Symfony\Component\HttpFoundation\Response;
 use Symfony\Component\HttpKernel\Exception\NotFoundHttpException;
 use Symfony\Component\Routing\Annotation\Route;
@@ -22,10 +23,14 @@ class DetailController extends TwigAwareController implements FrontendZoneInterf
     /** @var ContentRepository */
     private $contentRepository;
 
-    public function __construct(TemplateChooser $templateChooser, ContentRepository $contentRepository)
+    /** @var ContentHelper */
+    private $contentHelper;
+
+    public function __construct(TemplateChooser $templateChooser, ContentRepository $contentRepository, ContentHelper $contentHelper)
     {
         $this->templateChooser = $templateChooser;
         $this->contentRepository = $contentRepository;
+        $this->contentHelper = $contentHelper;
     }
 
     /**
@@ -52,12 +57,7 @@ public function record($slugOrId, ?string $contentTypeSlug = null, bool $require
             $record = $this->contentRepository->findOneBySlug($slugOrId, $contentType);
         }
 
-        // Update the canonical, with the correct path
-        $this->canonical->setPath(null, [
-            'contentTypeSlug' => $record ? $record->getContentTypeSingularSlug() : null,
-            'slugOrId' => $record ? $record->getSlug() : null,
-            '_locale' => $this->request->getLocale(),
-        ]);
+        $this->contentHelper->setCanonicalPath($record);
 
         return $this->renderSingle($record, $requirePublished);
     }
@@ -67,11 +67,7 @@ public function contentByFieldValue(string $contentTypeSlug, string $field, stri
         $contentType = ContentType::factory($contentTypeSlug, $this->config->get('contenttypes'));
         $record = $this->contentRepository->findOneByFieldValue($field, $value, $contentType);
 
-        // Update the canonical, with the correct path
-        $this->canonical->setPath(null, [
-            'field' => $field,
-            'value' => $value,
-        ]);
+        $this->contentHelper->setCanonicalPath($record);
 
         return $this->renderSingle($record);
     }

src/Twig/ContentExtension.php

@@ -72,6 +72,9 @@ class ContentExtension extends AbstractExtension
     /** @var Canonical */
     private $canonical;
 
+    /** @var ContentHelper */
+    private $contentHelper;
+
     public function __construct(
         UrlGeneratorInterface $urlGenerator,
         ContentRepository $contentRepository,
@@ -82,7 +85,8 @@ public function __construct(
         Query $query,
         TaxonomyRepository $taxonomyRepository,
         TranslatorInterface $translator,
-        Canonical $canonical
+        Canonical $canonical,
+        ContentHelper $contentHelper
     ) {
         $this->urlGenerator = $urlGenerator;
         $this->contentRepository = $contentRepository;
@@ -94,6 +98,7 @@ public function __construct(
         $this->taxonomyRepository = $taxonomyRepository;
         $this->translator = $translator;
         $this->canonical = $canonical;
+        $this->contentHelper = $contentHelper;
     }
 
     /**
@@ -325,27 +330,13 @@ public function allowTwig(Environment $env, Content $content): void
         $content->setTwig($env);
     }
 
-    public function getLink(Content $content, bool $canonical = false, string $locale = ''): ?string
+    public function getLink(Content $content, bool $canonical = false, ?string $locale = null): ?string
     {
         if ($content->getId() === null || $content->getDefinition()->get('viewless')) {
             return null;
         }
 
-        if (empty($locale)) {
-            $locale = $this->request->getLocale();
-        }
-
-        if ($this->isHomepage($content)) {
-            return $this->generateLink('homepage_locale', ['_locale' => $locale], $canonical);
-        }
-
-        $params = [
-            '_locale' => $locale,
-            'slugOrId' => $content->getSlug() ?: $content->getId(),
-            'contentTypeSlug' => $content->getContentTypeSingularSlug(),
-        ];
-
-        return $this->generateLink($content->getDefinition()->get('record_route'), $params, $canonical);
+        return $this->contentHelper->getLink($content, $canonical, $locale);
     }
 
     public function getEditLink(Content $content): ?string
@@ -681,45 +672,17 @@ public function getSpecialFeature(Content $record): string
 
     public function isHomepage(Content $content): bool
     {
-        return $this->isSpecialpage($content, 'homepage');
+        return $this->contentHelper->isHomepage($content);
     }
 
     public function is404(Content $content): bool
     {
-        return $this->isSpecialpage($content, 'notfound');
+        return $this->contentHelper->is404($content);
     }
 
     public function isMaintenance(Content $content): bool
     {
-        return $this->isSpecialpage($content, 'maintenance');
-    }
-
-    private function isSpecialpage(Content $content, string $type): bool
-    {
-        $configSetting = $this->config->get('general/' . $type);
-
-        if (! is_iterable($configSetting)) {
-            $configSetting = (array) $configSetting;
-        }
-
-        foreach ($configSetting as $item) {
-            $item = explode('/', $item);
-
-            // Discard candidate if contentTypes don't match
-            if ($item[0] !== $content->getContentTypeSingularSlug() && $item[0] !== $content->getContentTypeSlug()) {
-                continue;
-            }
-
-            $idOrSlug = $item[1] ?? null;
-
-            // Success if we either have no id/slug for a Singleton, or if the id/slug matches
-            if ((empty($idOrSlug) && $content->getDefinition()->get('singleton')) ||
-                ($idOrSlug === $content->getSlug() || $idOrSlug === (string) $content->getId())) {
-                return true;
-            }
-        }
-
-        return false;
+        return $this->contentHelper->isMaintenance($content);
     }
 
     public function isHomepageListing(ContentType $contentType): bool

src/Utils/ContentHelper.php

@@ -4,11 +4,97 @@
 
 namespace Bolt\Utils;
 
+use Bolt\Canonical;
+use Bolt\Configuration\Config;
 use Bolt\Entity\Content;
 use Bolt\Entity\Field\Excerptable;
+use Symfony\Component\HttpFoundation\RequestStack;
 
 class ContentHelper
 {
+    /** @var Canonical */
+    private $canonical;
+
+    /** @var \Symfony\Component\HttpFoundation\Request|null */
+    private $request;
+
+    /** @var Config */
+    private $config;
+
+    public function __construct(Canonical $canonical, RequestStack $requestStack, Config $config)
+    {
+        $this->canonical = $canonical;
+        $this->request = $requestStack->getCurrentRequest();
+        $this->config = $config;
+    }
+
+    public function setCanonicalPath($record, ?string $locale = null): void
+    {
+        if (! $record instanceof Content) {
+            return;
+        }
+
+        $route = $this->getCanonicalRouteAndParams($record, $locale);
+
+        $this->canonical->setPath($route['route'], $route['params']);
+    }
+
+    public function getLink($record, bool $canonical = false, ?string $locale = null): ?string
+    {
+        if (! $record instanceof Content) {
+            return '';
+        }
+
+        $route = $this->getCanonicalRouteAndParams($record, $locale);
+
+        // Clone the canonical, as it is a shared service.
+        // We only want to get the url for the current request
+        $canonicalObj = clone $this->canonical;
+        $canonicalObj->setPath($route['route'], $route['params']);
+
+        return $canonical ? $canonicalObj->get() : $canonicalObj->getPath();
+    }
+
+    private function getCanonicalRouteAndParams(Content $record, ?string $locale = null): array
+    {
+        if ($this->isHomepage($record)) {
+            return [
+                'route' => 'homepage_locale',
+                'params' => [
+                    '_locale' => $locale,
+                ],
+            ];
+        }
+
+        if (! $locale) {
+            $locale = $this->request->getLocale();
+        }
+
+        return [
+            'route' => $record->getDefinition()->get('record_route'),
+            'params' => [
+                'contentTypeSlug' => $record->getContentTypeSingularSlug(),
+                'slugOrId' => $record->getSlug(),
+                '_locale' => $locale,
+            ],
+        ];
+    }
+
+    public function isHomepage(Content $content): bool
+    {
+        return $this->isSpecialpage($content, 'homepage');
+    }
+
+    public function is404(Content $content): bool
+    {
+        return $this->isSpecialpage($content, 'notfound');
+    }
+
+    public function isMaintenance(Content $content): bool
+    {
+        return $this->isSpecialpage($content, 'maintenance');
+    }
+
     public static function isSuitable(Content $record, string $which = 'title_format'): bool
     {
         $definition = $record->getDefinition();
@@ -116,4 +202,32 @@ public static function getFieldBasedTitle(Content $content, string $locale = '')
 
         return implode(' ', $titleParts);
     }
+
+    private function isSpecialpage(Content $content, string $type): bool
+    {
+        $configSetting = $this->config->get('general/' . $type);
+
+        if (! is_iterable($configSetting)) {
+            $configSetting = (array) $configSetting;
+        }
+
+        foreach ($configSetting as $item) {
+            $item = explode('/', $item);
+
+            // Discard candidate if contentTypes don't match
+            if ($item[0] !== $content->getContentTypeSingularSlug() && $item[0] !== $content->getContentTypeSlug()) {
+                continue;
+            }
+
+            $idOrSlug = $item[1] ?? null;
+
+            // Success if we either have no id/slug for a Singleton, or if the id/slug matches
+            if ((empty($idOrSlug) && $content->getDefinition()->get('singleton')) ||
+                ($idOrSlug === $content->getSlug() || $idOrSlug === (string) $content->getId())) {
+                return true;
+            }
+        }
+
+        return false;
+    }
 }