From 414215ccac0f3288ccb1a40a5a8e29b9a70f0e89 Mon Sep 17 00:00:00 2001
From: daniel <dkinzler@wikimedia.org>
Date: Thu, 4 Oct 2018 12:32:06 +0200
Subject: [PATCH] ParserOutput::getCacheTime should stay the same after the
 first call.

Previously, getCacheTime would default to the current time, potentially
causing the return value to change over subsequent calls. With this change,
the value is determined on the first call, and then remembered for subsequent
calls.

Bug: T205464
Change-Id: If240161c71d523ad5b0d33b9378950e0bebceb6e
---
 includes/parser/CacheTime.php                 | 25 +++++++++----
 .../Storage/DerivedPageDataUpdaterTest.php    | 12 +++++++
 .../includes/parser/ParserOutputTest.php      | 35 +++++++++++++++++++
 3 files changed, 65 insertions(+), 7 deletions(-)

diff --git a/includes/parser/CacheTime.php b/includes/parser/CacheTime.php
index 26d5bdd3f9..e9840a4240 100644
--- a/includes/parser/CacheTime.php
+++ b/includes/parser/CacheTime.php
@@ -38,13 +38,13 @@ class CacheTime {
 	public $mVersion = Parser::VERSION;
 
 	/**
-	 * @var string|int TS_MW timestamp when this object was generated, or -1 for uncacheable. Used
+	 * @var string|int TS_MW timestamp when this object was generated, or -1 for not cacheable. Used
 	 * in ParserCache.
 	 */
 	public $mCacheTime = '';
 
 	/**
-	 * @var int|null Seconds after which the object should expire, use 0 for uncacheable. Used in
+	 * @var int|null Seconds after which the object should expire, use 0 for not cacheable. Used in
 	 * ParserCache.
 	 */
 	public $mCacheExpiry = null;
@@ -58,7 +58,11 @@ class CacheTime {
 	 * @return string TS_MW timestamp
 	 */
 	public function getCacheTime() {
-		return wfTimestamp( TS_MW, $this->mCacheTime );
+		// NOTE: keep support for undocumented used of -1 to mean "not cacheable".
+		if ( $this->mCacheTime === '' ) {
+			$this->mCacheTime = MWTimestamp::now();
+		}
+		return $this->mCacheTime;
 	}
 
 	/**
@@ -68,6 +72,11 @@ class CacheTime {
 	 * @return string
 	 */
 	public function setCacheTime( $t ) {
+		// NOTE: keep support for undocumented used of -1 to mean "not cacheable".
+		if ( is_string( $t ) && $t !== '-1' ) {
+			$t = MWTimestamp::convert( TS_MW, $t );
+		}
+
 		return wfSetVar( $this->mCacheTime, $t );
 	}
 
@@ -120,9 +129,10 @@ class CacheTime {
 	public function getCacheExpiry() {
 		global $wgParserCacheExpireTime;
 
+		// NOTE: keep support for undocumented used of -1 to mean "not cacheable".
 		if ( $this->mCacheTime < 0 ) {
 			return 0;
-		} // old-style marker for "not cacheable"
+		}
 
 		$expire = $this->mCacheExpiry;
 
@@ -157,11 +167,12 @@ class CacheTime {
 	public function expired( $touched ) {
 		global $wgCacheEpoch;
 
-		return !$this->isCacheable() // parser says it's uncacheable
+		$expiry = MWTimestamp::convert( TS_MW, MWTimestamp::time() - $this->getCacheExpiry() );
+
+		return !$this->isCacheable() // parser says it's not cacheable
 			|| $this->getCacheTime() < $touched
 			|| $this->getCacheTime() <= $wgCacheEpoch
-			|| $this->getCacheTime() <
-				wfTimestamp( TS_MW, time() - $this->getCacheExpiry() ) // expiry period has passed
+			|| $this->getCacheTime() < $expiry // expiry period has passed
 			|| !isset( $this->mVersion )
 			|| version_compare( $this->mVersion, Parser::VERSION, "lt" );
 	}
diff --git a/tests/phpunit/includes/Storage/DerivedPageDataUpdaterTest.php b/tests/phpunit/includes/Storage/DerivedPageDataUpdaterTest.php
index 79312369a2..18f039d20e 100644
--- a/tests/phpunit/includes/Storage/DerivedPageDataUpdaterTest.php
+++ b/tests/phpunit/includes/Storage/DerivedPageDataUpdaterTest.php
@@ -15,6 +15,7 @@ use MediaWiki\Storage\RevisionSlotsUpdate;
 use MediaWiki\Storage\SlotRecord;
 use MediaWikiTestCase;
 use MWCallableUpdate;
+use MWTimestamp;
 use PHPUnit\Framework\MockObject\MockObject;
 use TextContent;
 use TextContentHandler;
@@ -31,6 +32,12 @@ use WikitextContent;
  */
 class DerivedPageDataUpdaterTest extends MediaWikiTestCase {
 
+	public function tearDown() {
+		MWTimestamp::setFakeTime( false );
+
+		parent::tearDown();
+	}
+
 	/**
 	 * @param string $title
 	 *
@@ -470,6 +477,11 @@ class DerivedPageDataUpdaterTest extends MediaWikiTestCase {
 	 * @covers \MediaWiki\Storage\DerivedPageDataUpdater::getPreparedEdit()
 	 */
 	public function testGetPreparedEditAfterPrepareUpdate() {
+		$clock = MWTimestamp::convert( TS_UNIX, '20100101000000' );
+		MWTimestamp::setFakeTime( function () use ( &$clock ) {
+			return $clock++;
+		} );
+
 		$page = $this->getPage( __METHOD__ );
 
 		$mainContent = new WikitextContent( 'first [[main]] ~~~' );
diff --git a/tests/phpunit/includes/parser/ParserOutputTest.php b/tests/phpunit/includes/parser/ParserOutputTest.php
index 7091d9c62b..94cbf5c915 100644
--- a/tests/phpunit/includes/parser/ParserOutputTest.php
+++ b/tests/phpunit/includes/parser/ParserOutputTest.php
@@ -32,6 +32,12 @@ class ParserOutputTest extends MediaWikiLangTestCase {
 		];
 	}
 
+	public function tearDown() {
+		MWTimestamp::setFakeTime( false );
+
+		parent::tearDown();
+	}
+
 	/**
 	 * Test to make sure ParserOutput::isLinkInternal behaves properly
 	 * @dataProvider provideIsLinkInternal
@@ -935,4 +941,33 @@ EOF
 		}
 	}
 
+	/**
+	 * @covers ParserOutput::getCacheTime
+	 * @covers ParserOutput::setCacheTime
+	 */
+	public function testGetCacheTime() {
+		$clock = MWTimestamp::convert( TS_UNIX, '20100101000000' );
+		MWTimestamp::setFakeTime( function () use ( &$clock ) {
+			return $clock++;
+		} );
+
+		$po = new ParserOutput();
+		$time = $po->getCacheTime();
+
+		// Use current (fake) time per default. Ignore the last digit.
+		// Subsequent calls must yield the exact same timestamp as the first.
+		$this->assertStringStartsWith( '2010010100000', $time );
+		$this->assertSame( $time, $po->getCacheTime() );
+
+		// After setting, the getter must return the time that was set.
+		$time = '20110606112233';
+		$po->setCacheTime( $time );
+		$this->assertSame( $time, $po->getCacheTime() );
+
+		// support -1 as a marker for "not cacheable"
+		$time = -1;
+		$po->setCacheTime( $time );
+		$this->assertSame( $time, $po->getCacheTime() );
+	}
+
 }
-- 
2.20.1