From 3ab6f2a08283041d52a05db3e26817c66024da88 Mon Sep 17 00:00:00 2001
From: Mathias Brodala <mbrodala@pagemachine.de>
Date: Fri, 11 Jul 2014 13:51:47 +0200
Subject: [PATCH] [!!!][BUGFIX] Skip cache hash for URIs to non-cacheable
 actions

When building an URI for a non-cacheable action, while the current
request also is uncached, we can skip the cache hash for the target URI
to avoid unnecessary page cache entries.

Since this is a change in behavior during link generation, which other
code may rely upon, this is marked as breaking change.

Resolves: #60272
Releases: master
Change-Id: I448c33d23b790de1064eff95d0a940878b0299ac
Reviewed-on: http://review.typo3.org/31594
Reviewed-by: Markus Klein <klein.t3@reelworx.at>
Tested-by: Markus Klein <klein.t3@reelworx.at>
Reviewed-by: Stefan Neufeind <typo3.neufeind@speedpartner.de>
Reviewed-by: Anja Leichsenring <aleichsenring@ab-softlab.de>
Tested-by: Anja Leichsenring <aleichsenring@ab-softlab.de>
Reviewed-by: Helmut Hummel <helmut.hummel@typo3.org>
Tested-by: Helmut Hummel <helmut.hummel@typo3.org>
---
 ...pCacheHashForUrisToNonCacheableActions.rst | 22 +++++++++++
 .../Classes/Mvc/Web/Routing/UriBuilder.php    | 22 +++++++++++
 .../Unit/Mvc/Web/Routing/UriBuilderTest.php   | 39 +++++++++++++++++--
 3 files changed, 80 insertions(+), 3 deletions(-)
 create mode 100644 typo3/sysext/core/Documentation/Changelog/master/Breaking-60272-SkipCacheHashForUrisToNonCacheableActions.rst

diff --git a/typo3/sysext/core/Documentation/Changelog/master/Breaking-60272-SkipCacheHashForUrisToNonCacheableActions.rst b/typo3/sysext/core/Documentation/Changelog/master/Breaking-60272-SkipCacheHashForUrisToNonCacheableActions.rst
new file mode 100644
index 000000000000..f78286be7c8f
--- /dev/null
+++ b/typo3/sysext/core/Documentation/Changelog/master/Breaking-60272-SkipCacheHashForUrisToNonCacheableActions.rst
@@ -0,0 +1,22 @@
+====================================================================
+Breaking: #60272 - Skip cache hash for URIs to non-cacheable actions
+====================================================================
+
+Description
+===========
+
+The cache hash (cHash) parameter is not added to action URIs if the current
+request is not cached and the target action is not cacheable.
+
+Impact
+======
+
+Less cache entries are generated per page and not every action URI will have
+a cHash argument any more. It might be necessary to clear caches of extensions
+generating human readable URLs like RealURL.
+
+Affected installations
+======================
+
+Extbase extensions that generate links from uncached actions/pages to not
+cacheable actions.
diff --git a/typo3/sysext/extbase/Classes/Mvc/Web/Routing/UriBuilder.php b/typo3/sysext/extbase/Classes/Mvc/Web/Routing/UriBuilder.php
index aecdecdb3055..c67bc983b6b9 100644
--- a/typo3/sysext/extbase/Classes/Mvc/Web/Routing/UriBuilder.php
+++ b/typo3/sysext/extbase/Classes/Mvc/Web/Routing/UriBuilder.php
@@ -510,6 +510,9 @@ class UriBuilder {
 		if ($pluginName === NULL) {
 			$pluginName = $this->request->getPluginName();
 		}
+
+		$this->disableCacheHashForNonCacheableAction($controllerArguments);
+
 		if ($this->environmentService->isEnvironmentInFrontendMode() && $this->configurationManager->isFeatureEnabled('skipDefaultArguments')) {
 			$controllerArguments = $this->removeDefaultControllerAndAction($controllerArguments, $extensionName, $pluginName);
 		}
@@ -529,6 +532,25 @@ class UriBuilder {
 		return $this->build();
 	}
 
+	/**
+	 * Disable cache hash if the action is not cacheable
+	 * and pointed to from an already uncached request
+	 *
+	 * @param array $controllerArguments the current controller arguments
+	 * @return void
+	 */
+	protected function disableCacheHashForNonCacheableAction(array $controllerArguments) {
+		if (isset($controllerArguments['action']) && $this->getUseCacheHash()) {
+			$actionIsCacheable = $this->extensionService->isActionCacheable(
+				NULL,
+				NULL,
+				$controllerArguments['controller'],
+				$controllerArguments['action']
+			);
+			$this->setUseCacheHash($this->request->isCached() || $actionIsCacheable);
+		}
+	}
+
 	/**
 	 * This removes controller and/or action arguments from given controllerArguments
 	 * if they are equal to the default controller/action of the target plugin.
diff --git a/typo3/sysext/extbase/Tests/Unit/Mvc/Web/Routing/UriBuilderTest.php b/typo3/sysext/extbase/Tests/Unit/Mvc/Web/Routing/UriBuilderTest.php
index d3fa9aaf63fd..5fd52211ad31 100644
--- a/typo3/sysext/extbase/Tests/Unit/Mvc/Web/Routing/UriBuilderTest.php
+++ b/typo3/sysext/extbase/Tests/Unit/Mvc/Web/Routing/UriBuilderTest.php
@@ -151,12 +151,45 @@ class UriBuilderTest extends \TYPO3\CMS\Core\Tests\UnitTestCase {
 	}
 
 	/**
+	 * @param bool $isActionCacheable
+	 * @param bool $requestIsCached
+	 * @param bool $expectedUseCacheHash
 	 * @test
+	 * @dataProvider uriForDisablesCacheHashIfPossibleDataProvider
 	 */
-	public function uriForDoesNotDisableCacheHashForNonCacheableActions() {
-		$this->mockExtensionService->expects($this->any())->method('isActionCacheable')->will($this->returnValue(FALSE));
+	public function uriForDisablesCacheHashIfPossible($isActionCacheable, $requestIsCached, $expectedUseCacheHash) {
+		$this->mockExtensionService->expects($this->once())->method('isActionCacheable')->will($this->returnValue($isActionCacheable));
+		$this->mockRequest->expects($this->once())->method('isCached')->will($this->returnValue($requestIsCached));
 		$this->uriBuilder->uriFor('someNonCacheableAction', array(), 'SomeController', 'SomeExtension');
-		$this->assertTrue($this->uriBuilder->getUseCacheHash());
+		$this->assertEquals($expectedUseCacheHash, $this->uriBuilder->getUseCacheHash());
+	}
+
+	/**
+	 * @return array
+	 */
+	public function uriForDisablesCacheHashIfPossibleDataProvider() {
+		return array(
+			'request cached, action cacheable' => array(
+				TRUE,
+				TRUE,
+				TRUE,
+			),
+			'request cached, action not cacheable' => array(
+				TRUE,
+				FALSE,
+				TRUE,
+			),
+			'request not cached, action cacheable' => array(
+				FALSE,
+				TRUE,
+				TRUE,
+			),
+			'request not cached, action not cacheable' => array(
+				FALSE,
+				FALSE,
+				FALSE,
+			),
+		);
 	}
 
 	/**
-- 
GitLab