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 bc760a3..03c3e6d 100644 (file)
--- 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 553ef72..2370ed3 100644 (file)
--- 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
         *