From: Mattflaschen <mflaschen@wikimedia.org>
Date: Wed, 7 Sep 2016 20:34:58 +0000 (+0000)
Subject: Revert "Revision: Simplify loadText() with nested getWithSetCallback"
X-Git-Tag: 1.31.0-rc.0~5718^2
X-Git-Url: http://git.cyclocoop.org/url?a=commitdiff_plain;h=1f022240c73f0909d8d3eb457ede12119190621a;p=lhc%2Fweb%2Fwiklou.git

Revert "Revision: Simplify loadText() with nested getWithSetCallback"

This reverts commit aa0f6ead348d0aa3f59d51df0806e95eb76babf8.

Change-Id: I285ada2e86f014b93c7e7946f10d2dc84375fcff
---

diff --git a/includes/Revision.php b/includes/Revision.php
index bc760a3609..03c3e6d99f 100644
--- a/includes/Revision.php
+++ b/includes/Revision.php
@@ -1079,14 +1079,13 @@ class Revision implements IDBAccessObject {
 	}
 
 	/**
-	 * Get original serialized data (without checking view restrictions)
+	 * Fetch original serialized data without regard for view restrictions
 	 *
 	 * @since 1.21
 	 * @return string
 	 */
 	public function getSerializedData() {
 		if ( $this->mText === null ) {
-			// Revision is immutable. Load on demand.
 			$this->mText = $this->loadText();
 		}
 
@@ -1104,14 +1103,17 @@ class Revision implements IDBAccessObject {
 	 */
 	protected function getContentInternal() {
 		if ( $this->mContent === null ) {
-			$text = $this->getSerializedData();
+			// Revision is immutable. Load on demand:
+			if ( $this->mText === null ) {
+				$this->mText = $this->loadText();
+			}
 
-			if ( $text !== null && $text !== false ) {
+			if ( $this->mText !== null && $this->mText !== false ) {
 				// Unserialize content
 				$handler = $this->getContentHandler();
 				$format = $this->getContentFormat();
 
-				$this->mContent = $handler->unserializeContent( $text, $format );
+				$this->mContent = $handler->unserializeContent( $this->mText, $format );
 			}
 		}
 
@@ -1574,38 +1576,29 @@ class Revision implements IDBAccessObject {
 	 *
 	 * @return string|bool The revision's text, or false on failure
 	 */
-	private function loadText() {
+	protected function loadText() {
 		// Caching may be beneficial for massive use of external storage
 		global $wgRevisionCacheExpiry;
 		static $processCache = null;
 
-		if ( !$wgRevisionCacheExpiry ) {
-			return $this->fetchText();
-		}
-
 		if ( !$processCache ) {
 			$processCache = new MapCacheLRU( 10 );
 		}
 
 		$cache = ObjectCache::getMainWANInstance();
-		$key = $cache->makeKey( 'revisiontext', 'textid', $this->getTextId() );
-
-		// No negative caching; negative hits on text rows may be due to corrupted replica DBs
-		return $processCache->getWithSetCallback( $key, function () use ( $cache, $key ) {
-			global $wgRevisionCacheExpiry;
-
-			return $cache->getWithSetCallback(
-				$key,
-				$wgRevisionCacheExpiry,
-				function () {
-					return $this->fetchText();
-				}
-			);
-		} );
-	}
-
-	private function fetchText() {
 		$textId = $this->getTextId();
+		$key = wfMemcKey( 'revisiontext', 'textid', $textId );
+
+		if ( $wgRevisionCacheExpiry ) {
+			if ( $processCache->has( $key ) ) {
+				return $processCache->get( $key );
+			}
+			$text = $cache->get( $key );
+			if ( is_string( $text ) ) {
+				$processCache->set( $key, $text );
+				return $text;
+			}
+		}
 
 		// If we kept data for lazy extraction, use it now...
 		if ( $this->mTextRow !== null ) {
@@ -1645,7 +1638,13 @@ class Revision implements IDBAccessObject {
 			wfDebugLog( 'Revision', "No blob for text row '$textId' (revision {$this->getId()})." );
 		}
 
-		return is_string( $text ) ? $text : false;
+		# No negative caching -- negative hits on text rows may be due to corrupted replica DB servers
+		if ( $wgRevisionCacheExpiry && $text !== false ) {
+			$processCache->set( $key, $text );
+			$cache->set( $key, $text, $wgRevisionCacheExpiry );
+		}
+
+		return $text;
 	}
 
 	/**
diff --git a/includes/libs/MapCacheLRU.php b/includes/libs/MapCacheLRU.php
index 553ef723e4..2370ed3a8a 100644
--- a/includes/libs/MapCacheLRU.php
+++ b/includes/libs/MapCacheLRU.php
@@ -102,28 +102,6 @@ class MapCacheLRU {
 		return array_keys( $this->cache );
 	}
 
-	/**
-	 * Get an item with the given key, producing and setting it if not found.
-	 *
-	 * If the callback returns false, then nothing is stored.
-	 *
-	 * @since 1.28
-	 * @param string $key
-	 * @param callable $callback Callback that will produce the value
-	 * @return mixed The cached value if found or the result of $callback otherwise
-	 */
-	public function getWithSetCallback( $key, callable $callback ) {
-		$value = $this->get( $key );
-		if ( $value === false ) {
-			$value = call_user_func( $callback );
-			if ( $value !== false ) {
-				$this->set( $key, $value );
-			}
-		}
-
-		return $value;
-	}
-
 	/**
 	 * Clear one or several cache entries, or all cache entries
 	 *