From 8db4c163e80efcf86f769fdceebcf459ae3ab85c Mon Sep 17 00:00:00 2001
From: Aaron Schulz <aschulz@wikimedia.org>
Date: Thu, 8 Sep 2016 11:34:39 -0700
Subject: [PATCH] objectcache: pass in the $oldValue as-of time in
 getWithSetCallback()

This lets callers use adaptive TTLs on the near-expiration
preemptive refreshes if the new and current values match,
using the as-of time as $mtime.

Change-Id: Ie541c35f890c9f789d1accf9f2a43506daaf31f0
---
 includes/libs/objectcache/WANObjectCache.php         | 12 +++++++-----
 .../includes/libs/objectcache/WANObjectCacheTest.php | 12 +++++++++++-
 2 files changed, 18 insertions(+), 6 deletions(-)

diff --git a/includes/libs/objectcache/WANObjectCache.php b/includes/libs/objectcache/WANObjectCache.php
index ed3b5cc484..9293631956 100644
--- a/includes/libs/objectcache/WANObjectCache.php
+++ b/includes/libs/objectcache/WANObjectCache.php
@@ -643,6 +643,7 @@ class WANObjectCache implements IExpiringStore, LoggerAwareInterface {
 	 *   - $oldValue : current cache value or false if not present
 	 *   - &$ttl : a reference to the TTL which can be altered
 	 *   - &$setOpts : a reference to options for set() which can be altered
+	 *   - $oldAsOf : generation UNIX timestamp of $oldValue or null if not present (since 1.28)
 	 *
 	 * It is strongly recommended to set the 'lag' and 'since' fields to avoid race conditions
 	 * that can cause stale values to get stuck at keys. Usually, callbacks ignore the current
@@ -840,7 +841,8 @@ class WANObjectCache implements IExpiringStore, LoggerAwareInterface {
 				$cur = $this->doGetWithSetCallback(
 					$key,
 					$ttl,
-					function ( $oldValue, &$ttl, &$setOpts ) use ( $callback, $version ) {
+					function ( $oldValue, &$ttl, &$setOpts, $oldAsOf )
+					use ( $callback, $version ) {
 						if ( is_array( $oldValue )
 							&& array_key_exists( self::VFLD_DATA, $oldValue )
 						) {
@@ -851,7 +853,7 @@ class WANObjectCache implements IExpiringStore, LoggerAwareInterface {
 						}
 
 						return [
-							self::VFLD_DATA => $callback( $oldData, $ttl, $setOpts ),
+							self::VFLD_DATA => $callback( $oldData, $ttl, $setOpts, $oldAsOf ),
 							self::VFLD_VERSION => $version
 						];
 					},
@@ -969,13 +971,13 @@ class WANObjectCache implements IExpiringStore, LoggerAwareInterface {
 
 		// Generate the new value from the callback...
 		$setOpts = [];
-		$value = call_user_func_array( $callback, [ $cValue, &$ttl, &$setOpts ] );
-		$asOf = microtime( true );
+		$value = call_user_func_array( $callback, [ $cValue, &$ttl, &$setOpts, $asOf ] );
 		// When delete() is called, writes are write-holed by the tombstone,
 		// so use a special INTERIM key to pass the new value around threads.
 		if ( ( $isTombstone && $lockTSE > 0 ) && $value !== false && $ttl >= 0 ) {
 			$tempTTL = max( 1, (int)$lockTSE ); // set() expects seconds
-			$wrapped = $this->wrap( $value, $tempTTL, $asOf );
+			$newAsOf = microtime( true );
+			$wrapped = $this->wrap( $value, $tempTTL, $newAsOf );
 			// Avoid using set() to avoid pointless mcrouter broadcasting
 			$this->cache->merge(
 				self::INTERIM_KEY_PREFIX . $key,
diff --git a/tests/phpunit/includes/libs/objectcache/WANObjectCacheTest.php b/tests/phpunit/includes/libs/objectcache/WANObjectCacheTest.php
index cef31dc460..99b959b3e2 100644
--- a/tests/phpunit/includes/libs/objectcache/WANObjectCacheTest.php
+++ b/tests/phpunit/includes/libs/objectcache/WANObjectCacheTest.php
@@ -161,9 +161,15 @@ class WANObjectCacheTest extends MediaWikiTestCase {
 		$cKey1 = wfRandomString();
 		$cKey2 = wfRandomString();
 
+		$priorValue = null;
+		$priorAsOf = null;
 		$wasSet = 0;
-		$func = function( $old, &$ttl ) use ( &$wasSet, $value ) {
+		$func = function( $old, &$ttl, &$opts, $asOf )
+			use ( &$wasSet, &$priorValue, &$priorAsOf, $value )
+		{
 			++$wasSet;
+			$priorValue = $old;
+			$priorAsOf = $asOf;
 			$ttl = 20; // override with another value
 			return $value;
 		};
@@ -172,6 +178,8 @@ class WANObjectCacheTest extends MediaWikiTestCase {
 		$v = $cache->getWithSetCallback( $key, 30, $func, [ 'lockTSE' => 5 ] + $extOpts );
 		$this->assertEquals( $value, $v, "Value returned" );
 		$this->assertEquals( 1, $wasSet, "Value regenerated" );
+		$this->assertFalse( $priorValue, "No prior value" );
+		$this->assertNull( $priorAsOf, "No prior value" );
 
 		$curTTL = null;
 		$cache->get( $key, $curTTL );
@@ -194,6 +202,8 @@ class WANObjectCacheTest extends MediaWikiTestCase {
 		);
 		$this->assertEquals( $value, $v, "Value returned" );
 		$this->assertEquals( 1, $wasSet, "Value regenerated due to check keys" );
+		$this->assertEquals( $value, $priorValue, "Has prior value" );
+		$this->assertType( 'float', $priorAsOf, "Has prior value" );
 		$t1 = $cache->getCheckKeyTime( $cKey1 );
 		$this->assertGreaterThanOrEqual( $priorTime, $t1, 'Check keys generated on miss' );
 		$t2 = $cache->getCheckKeyTime( $cKey2 );
-- 
2.20.1