Generate news URLs using the content URL generator (see #6604)

Description ----------- same as #6597 but for news URLs. This obviously fails in all places because it requires #6596 to be merged first. Commits ------- df9b828 Generate news URLs using content URL generator af4b3ce Simplify redirect to source in news reader bf1b922 Replace PageModel::getFrontendUrl and getAbsoluteUrl 32d39b8 Return NULL to abstain c94f4e8 Correctly generate absolute URLs 94ea7e8 Drop the serp widget url_callback 8dfebdd Remove usages of News::generateNewsUrl 9a449d6 Fix the PreviewUrlConverterListenerTest 7fde1e1 Revert BC break
contao · Jan 17, 2024 · 8ea2d08 · 8ea2d08
1 parent 3357310
commit 8ea2d08
Show file tree

Hide file tree

Showing 18 changed files with 411 additions and 227 deletions.
diff --git a/news-bundle/config/services.yaml b/news-bundle/config/services.yaml
@@ -42,13 +42,15 @@ services:
         class: Contao\NewsBundle\EventListener\GeneratePageListener
         arguments:
             - '@contao.framework'
+            - '@contao.routing.content_url_generator'
         tags:
             - { name: contao.hook, hook: generatePage }
 
     contao_news.listener.insert_tags:
         class: Contao\NewsBundle\EventListener\InsertTagsListener
         arguments:
             - '@contao.framework'
+            - '@contao.routing.content_url_generator'
             - '@logger'
         tags:
             - { name: contao.hook, hook: replaceInsertTags }
@@ -58,6 +60,7 @@ services:
         arguments:
             - '@contao.framework'
             - '@contao.image.factory'
+            - '@contao.routing.content_url_generator'
             - '@contao.insert_tag.parser'
             - '%kernel.project_dir%'
             - '@contao.cache.entity_tags'
@@ -67,6 +70,7 @@ services:
         class: Contao\NewsBundle\EventListener\PreviewUrlConvertListener
         arguments:
             - '@contao.framework'
+            - '@contao.routing.content_url_generator'
         tags:
             - kernel.event_listener
 
@@ -83,6 +87,7 @@ services:
         arguments:
             - '@contao.framework'
             - '@security.helper'
+            - '@contao.routing.content_url_generator'
         tags:
             - kernel.event_listener
 
@@ -104,6 +109,11 @@ services:
         tags:
             - { name: contao.picker_provider, priority: 128 }
 
+    contao_news.routing.news_resolver:
+        class: Contao\NewsBundle\Routing\NewsResolver
+        arguments:
+            - '@contao.framework'
+
     contao_news.security.news_access_voter:
         class: Contao\NewsBundle\Security\Voter\NewsAccessVoter
         arguments:

diff --git a/news-bundle/contao/classes/News.php b/news-bundle/contao/classes/News.php
@@ -10,17 +10,14 @@
 
 namespace Contao;
 
+use Symfony\Component\Routing\Exception\ExceptionInterface;
+use Symfony\Component\Routing\Generator\UrlGeneratorInterface;
+
 /**
  * Provide methods regarding news archives.
  */
 class News extends Frontend
 {
-	/**
-	 * URL cache array
-	 * @var array
-	 */
-	private static $arrUrlCache = array();
-
 	/**
 	 * Generate a URL and return it as string
 	 *
@@ -29,86 +26,32 @@ class News extends Frontend
 	 * @param boolean   $blnAbsolute
 	 *
 	 * @return string
+	 *
+	 * @deprecated Deprecated since Contao 5.3, to be removed in Contao 6;
+	 *             use the content URL generator instead.
 	 */
 	public static function generateNewsUrl($objItem, $blnAddArchive=false, $blnAbsolute=false)
 	{
-		$strCacheKey = 'id_' . $objItem->id . ($blnAbsolute ? '_absolute' : '') . (($blnAddArchive && Input::get('month')) ? '_' . Input::get('month') : '');
-
-		// Load the URL from cache
-		if (isset(self::$arrUrlCache[$strCacheKey]))
-		{
-			return self::$arrUrlCache[$strCacheKey];
-		}
-
-		// Initialize the cache
-		self::$arrUrlCache[$strCacheKey] = null;
-
-		switch ($objItem->source)
-		{
-			// Link to an external page
-			case 'external':
-				if (str_starts_with($objItem->url, 'mailto:'))
-				{
-					self::$arrUrlCache[$strCacheKey] = StringUtil::encodeEmail($objItem->url);
-				}
-				else
-				{
-					$url = $objItem->url;
-
-					if (Validator::isRelativeUrl($url))
-					{
-						$url = Environment::get('path') . '/' . $url;
-					}
-
-					self::$arrUrlCache[$strCacheKey] = StringUtil::ampersand($url);
-				}
-				break;
+		trigger_deprecation('contao/core-bundle', '5.3', 'Using "%s()" has been deprecated and will no longer work in Contao 6. Use the content URL generator instead.', __METHOD__);
 
-			// Link to an internal page
-			case 'internal':
-				if (($objTarget = $objItem->getRelated('jumpTo')) instanceof PageModel)
-				{
-					/** @var PageModel $objTarget */
-					self::$arrUrlCache[$strCacheKey] = StringUtil::ampersand($blnAbsolute ? $objTarget->getAbsoluteUrl() : $objTarget->getFrontendUrl());
-				}
-				break;
-
-			// Link to an article
-			case 'article':
-				if (($objArticle = ArticleModel::findByPk($objItem->articleId)) instanceof ArticleModel && ($objPid = $objArticle->getRelated('pid')) instanceof PageModel)
-				{
-					$params = '/articles/' . ($objArticle->alias ?: $objArticle->id);
-
-					/** @var PageModel $objPid */
-					self::$arrUrlCache[$strCacheKey] = StringUtil::ampersand($blnAbsolute ? $objPid->getAbsoluteUrl($params) : $objPid->getFrontendUrl($params));
-				}
-				break;
-		}
-
-		// Link to the default page
-		if (self::$arrUrlCache[$strCacheKey] === null)
+		try
 		{
-			$objPage = PageModel::findByPk($objItem->getRelated('pid')->jumpTo);
-
-			if (!$objPage instanceof PageModel)
-			{
-				self::$arrUrlCache[$strCacheKey] = StringUtil::ampersand(Environment::get('requestUri'));
-			}
-			else
-			{
-				$params = '/' . ($objItem->alias ?: $objItem->id);
-
-				self::$arrUrlCache[$strCacheKey] = StringUtil::ampersand($blnAbsolute ? $objPage->getAbsoluteUrl($params) : $objPage->getFrontendUrl($params));
-			}
+			$parameters = array();
 
 			// Add the current archive parameter (news archive)
 			if ($blnAddArchive && Input::get('month'))
 			{
-				self::$arrUrlCache[$strCacheKey] .= '?month=' . Input::get('month');
+				$parameters['month'] = Input::get('month');
 			}
+
+			$url = System::getContainer()->get('contao.routing.content_url_generator')->generate($objItem, $parameters, $blnAbsolute ? UrlGeneratorInterface::ABSOLUTE_URL : UrlGeneratorInterface::ABSOLUTE_PATH);
+		}
+		catch (ExceptionInterface)
+		{
+			return StringUtil::ampersand(Environment::get('requestUri'));
 		}
 
-		return self::$arrUrlCache[$strCacheKey];
+		return $url;
 	}
 
 	/**
@@ -121,11 +64,12 @@ public static function generateNewsUrl($objItem, $blnAddArchive=false, $blnAbsol
 	public static function getSchemaOrgData(NewsModel $objArticle): array
 	{
 		$htmlDecoder = System::getContainer()->get('contao.string.html_decoder');
+		$urlGenerator = System::getContainer()->get('contao.routing.content_url_generator');
 
 		$jsonLd = array(
 			'@type' => 'NewsArticle',
 			'identifier' => '#/schema/news/' . $objArticle->id,
-			'url' => self::generateNewsUrl($objArticle),
+			'url' => $urlGenerator->generate($objArticle),
 			'headline' => $htmlDecoder->inputEncodedToPlainText($objArticle->headline),
 			'datePublished' => date('Y-m-d\TH:i:sP', $objArticle->date),
 		);

diff --git a/news-bundle/contao/dca/tl_news.php b/news-bundle/contao/dca/tl_news.php
@@ -219,7 +219,7 @@
 		(
 			'label'                   => &$GLOBALS['TL_LANG']['MSC']['serpPreview'],
 			'inputType'               => 'serpPreview',
-			'eval'                    => array('url_callback'=>array('tl_news', 'getSerpUrl'), 'title_tag_callback'=>array('tl_news', 'getTitleTag'), 'titleFields'=>array('pageTitle', 'headline'), 'descriptionFields'=>array('description', 'teaser')),
+			'eval'                    => array('title_tag_callback'=>array('tl_news', 'getTitleTag'), 'titleFields'=>array('pageTitle', 'headline'), 'descriptionFields'=>array('description', 'teaser')),
 			'sql'                     => null
 		),
 		'canonicalLink' => array
@@ -481,18 +481,6 @@ public function loadTime($value)
 		return strtotime('1970-01-01 ' . date('H:i:s', $value));
 	}
 
-	/**
-	 * Return the SERP URL
-	 *
-	 * @param NewsModel $model
-	 *
-	 * @return string
-	 */
-	public function getSerpUrl(NewsModel $model)
-	{
-		return News::generateNewsUrl($model, false, true);
-	}
-
 	/**
 	 * Return the title tag from the associated page layout
 	 *

diff --git a/news-bundle/contao/modules/ModuleNews.php b/news-bundle/contao/modules/ModuleNews.php
@@ -86,7 +86,7 @@ protected function parseArticle($objArticle, $blnAddArchive=false, $strClass='',
 		$objTemplate->hasSubHeadline = $objArticle->subheadline ? true : false;
 		$objTemplate->linkHeadline = $this->generateLink($objArticle->headline, $objArticle, $blnAddArchive);
 		$objTemplate->more = $this->generateLink($objArticle->linkText ?: $GLOBALS['TL_LANG']['MSC']['more'], $objArticle, $blnAddArchive, true);
-		$objTemplate->link = News::generateNewsUrl($objArticle, $blnAddArchive);
+		$objTemplate->link = $this->generateContentUrl($objArticle, $blnAddArchive);
 		$objTemplate->archive = $objArticle->getRelated('pid');
 		$objTemplate->count = $intCount; // see #5708
 		$objTemplate->text = '';
@@ -302,7 +302,7 @@ protected function generateLink($strLink, $objArticle, $blnAddArchive=false, $bl
 	{
 		$blnIsInternal = $objArticle->source != 'external';
 		$strReadMore = $blnIsInternal ? $GLOBALS['TL_LANG']['MSC']['readMore'] : $GLOBALS['TL_LANG']['MSC']['open'];
-		$strArticleUrl = News::generateNewsUrl($objArticle, $blnAddArchive);
+		$strArticleUrl = $this->generateContentUrl($objArticle, $blnAddArchive);
 
 		return sprintf(
 			'<a href="%s" title="%s"%s>%s%s</a>',
@@ -313,4 +313,17 @@ protected function generateLink($strLink, $objArticle, $blnAddArchive=false, $bl
 			$blnIsReadMore && $blnIsInternal ? '<span class="invisible"> ' . $objArticle->headline . '</span>' : ''
 		);
 	}
+
+	private function generateContentUrl(NewsModel $content, bool $addArchive): string
+	{
+		$parameters = array();
+
+		// Add the current archive parameter (news archive)
+		if ($addArchive && Input::get('month'))
+		{
+			$parameters['month'] = Input::get('month');
+		}
+
+		return System::getContainer()->get('contao.routing.content_url_generator')->generate($content, $parameters);
+	}
 }
diff --git a/news-bundle/contao/modules/ModuleNewsMenu.php b/news-bundle/contao/modules/ModuleNewsMenu.php
@@ -11,6 +11,7 @@
 namespace Contao;
 
 use Contao\CoreBundle\Exception\PageNotFoundException;
+use Symfony\Component\Routing\Exception\ExceptionInterface;
 
 /**
  * Front end module "news archive".
@@ -74,8 +75,13 @@ public function generate()
 
 		if (($objTarget = $this->objModel->getRelated('jumpTo')) instanceof PageModel)
 		{
-			/** @var PageModel $objTarget */
-			$this->strUrl = $objTarget->getFrontendUrl();
+			try
+			{
+				$this->strUrl = System::getContainer()->get('contao.routing.content_url_generator')->generate($objTarget);
+			}
+			catch (ExceptionInterface)
+			{
+			}
 		}
 
 		return parent::generate();

diff --git a/news-bundle/contao/modules/ModuleNewsReader.php b/news-bundle/contao/modules/ModuleNewsReader.php
@@ -15,6 +15,7 @@
 use Contao\CoreBundle\Exception\RedirectResponseException;
 use Contao\CoreBundle\Routing\ResponseContext\HtmlHeadBag\HtmlHeadBag;
 use Contao\CoreBundle\Util\UrlUtil;
+use Symfony\Component\Routing\Generator\UrlGeneratorInterface;
 
 /**
  * Front end module "newsreader".
@@ -77,9 +78,11 @@ protected function compile()
 	{
 		$this->Template->articles = '';
 
-		if ($this->overviewPage)
+		$urlGenerator = System::getContainer()->get('contao.routing.content_url_generator');
+
+		if ($this->overviewPage && ($overviewPage = PageModel::findById($this->overviewPage)))
 		{
-			$this->Template->referer = PageModel::findById($this->overviewPage)->getFrontendUrl();
+			$this->Template->referer = $urlGenerator->generate($overviewPage);
 			$this->Template->back = $this->customLabel ?: $GLOBALS['TL_LANG']['MSC']['newsOverview'];
 		}
 
@@ -96,31 +99,9 @@ protected function compile()
 		switch ($objArticle->source)
 		{
 			case 'internal':
-				if ($page = PageModel::findPublishedById($objArticle->jumpTo))
-				{
-					throw new RedirectResponseException($page->getAbsoluteUrl(), 301);
-				}
-
-				throw new InternalServerErrorException('Invalid "jumpTo" value or target page not public');
-
 			case 'article':
-				if (($article = ArticleModel::findByPk($objArticle->articleId)) && ($page = PageModel::findPublishedById($article->pid)))
-				{
-					throw new RedirectResponseException($page->getAbsoluteUrl('/articles/' . ($article->alias ?: $article->id)), 301);
-				}
-
-				throw new InternalServerErrorException('Invalid "articleId" value or target page not public');
-
 			case 'external':
-				if ($objArticle->url)
-				{
-					$url = System::getContainer()->get('contao.insert_tag.parser')->replaceInline($objArticle->url);
-					$url = UrlUtil::makeAbsolute($url, Environment::get('base'));
-
-					throw new RedirectResponseException($url, 301);
-				}
-
-				throw new InternalServerErrorException('Empty target URL');
+				throw new RedirectResponseException(System::getContainer()->get('contao.routing.content_url_generator')->generate($objArticle, array(), UrlGeneratorInterface::ABSOLUTE_URL), 301);
 		}
 
 		// Set the default template
@@ -183,7 +164,7 @@ protected function compile()
 			}
 			elseif (!$this->news_keepCanonical)
 			{
-				$htmlHeadBag->setCanonicalUri(News::generateNewsUrl($objArticle, false, true));
+				$htmlHeadBag->setCanonicalUri($urlGenerator->generate($objArticle, array(), UrlGeneratorInterface::ABSOLUTE_URL));
 			}
 		}
 

diff --git a/news-bundle/src/EventListener/GeneratePageListener.php b/news-bundle/src/EventListener/GeneratePageListener.php
@@ -13,18 +13,23 @@
 namespace Contao\NewsBundle\EventListener;
 
 use Contao\CoreBundle\Framework\ContaoFramework;
+use Contao\CoreBundle\Routing\ContentUrlGenerator;
 use Contao\LayoutModel;
 use Contao\NewsBundle\Controller\Page\NewsFeedController;
 use Contao\PageModel;
 use Contao\StringUtil;
+use Symfony\Component\Routing\Exception\ExceptionInterface;
+use Symfony\Component\Routing\Generator\UrlGeneratorInterface;
 
 /**
  * @internal
  */
 class GeneratePageListener
 {
-    public function __construct(private readonly ContaoFramework $framework)
-    {
+    public function __construct(
+        private readonly ContaoFramework $framework,
+        private readonly ContentUrlGenerator $urlGenerator,
+    ) {
     }
 
     /**
@@ -51,8 +56,11 @@ public function __invoke(PageModel $pageModel, LayoutModel $layoutModel): void
                 continue;
             }
 
-            // TODO: Use ResponseContext, once it supports appending to <head>
-            $GLOBALS['TL_HEAD'][] = $this->generateFeedTag($feed->getAbsoluteUrl(), $feed->feedFormat, $feed->title);
+            try {
+                // TODO: Use ResponseContext, once it supports appending to <head>
+                $GLOBALS['TL_HEAD'][] = $this->generateFeedTag($this->urlGenerator->generate($feed, [], UrlGeneratorInterface::ABSOLUTE_URL), $feed->feedFormat, $feed->title);
+            } catch (ExceptionInterface) {
+            }
         }
     }