Clean up BagOStuff::get() interface

* Callers of get() no longer have to contend with
  the annoying $casToken parameter, which is there
  but totally unusable to non-BagOStuff code.
* The default get() now delegates to doGet(),
  which callers must implement instead. They can
  ignore the overhead of generating $casToken if
  they do not implement cas(), which applies to
  callers that use the stock merge(). If cas() is
  used for merge(), then getWithToken() must be
  implemented.
* Also add BagOStuff::READ_LATEST to mergeViaCas()
  for sanity, as that missing before.
  Likewise with mergeViaLock().

Change-Id: I4efce6a9ab4b1eadd2f161dff641004a7239c516

diff --git a/includes/libs/objectcache/APCBagOStuff.php b/includes/libs/objectcache/APCBagOStuff.php
index 0dbbaba..522c5d7 100644 (file)
--- a/includes/libs/objectcache/APCBagOStuff.php
+++ b/includes/libs/objectcache/APCBagOStuff.php
@@ -34,11 +34,9 @@ class APCBagOStuff extends BagOStuff {
         **/
        const KEY_SUFFIX = ':1';
 
-       public function get( $key, &$casToken = null, $flags = 0 ) {
+       protected function doGet( $key, $flags = 0 ) {
                $val = apc_fetch( $key . self::KEY_SUFFIX );
 
-               $casToken = $val;
-
                return $val;
        }
 

diff --git a/includes/libs/objectcache/BagOStuff.php b/includes/libs/objectcache/BagOStuff.php
index 647d938..ecdf48f 100644 (file)
--- a/includes/libs/objectcache/BagOStuff.php
+++ b/includes/libs/objectcache/BagOStuff.php
@@ -124,11 +124,35 @@ abstract class BagOStuff implements LoggerAwareInterface {
         * higher tiers using standard TTLs.
         *
         * @param string $key
-        * @param mixed $casToken [optional]
         * @param integer $flags Bitfield of BagOStuff::READ_* constants [optional]
+        * @param integer $oldFlags [unused]
         * @return mixed Returns false on failure and if the item does not exist
         */
-       abstract public function get( $key, &$casToken = null, $flags = 0 );
+       public function get( $key, $flags = 0, $oldFlags = null ) {
+               // B/C for ( $key, &$casToken = null, $flags = 0 )
+               $flags = is_int( $oldFlags ) ? $oldFlags : $flags;
+
+               return $this->doGet( $key, $flags );
+       }
+
+       /**
+        * @param string $key
+        * @param integer $flags Bitfield of BagOStuff::READ_* constants [optional]
+        * @return mixed Returns false on failure and if the item does not exist
+        */
+       abstract protected function doGet( $key, $flags = 0 );
+
+       /**
+        * @note: This method is only needed if cas() is not is used for merge()
+        *
+        * @param string $key
+        * @param mixed $casToken
+        * @param integer $flags Bitfield of BagOStuff::READ_* constants [optional]
+        * @return mixed Returns false on failure and if the item does not exist
+        */
+       protected function getWithToken( $key, &$casToken, $flags = 0 ) {
+               throw new Exception( __METHOD__ . ' not implemented.' );
+       }
 
        /**
         * Set an item
@@ -182,7 +206,7 @@ abstract class BagOStuff implements LoggerAwareInterface {
                do {
                        $this->clearLastError();
                        $casToken = null; // passed by reference
-                       $currentValue = $this->get( $key, $casToken );
+                       $currentValue = $this->getWithToken( $key, $casToken, BagOStuff::READ_LATEST );
                        if ( $this->getLastError() ) {
                                return false; // don't spam retries (retry only on races)
                        }
@@ -237,7 +261,7 @@ abstract class BagOStuff implements LoggerAwareInterface {
                }
 
                $this->clearLastError();
-               $currentValue = $this->get( $key );
+               $currentValue = $this->get( $key, BagOStuff::READ_LATEST );
                if ( $this->getLastError() ) {
                        $success = false;
                } else {

diff --git a/includes/libs/objectcache/EmptyBagOStuff.php b/includes/libs/objectcache/EmptyBagOStuff.php
index 55e84b0..bef0456 100644 (file)
--- a/includes/libs/objectcache/EmptyBagOStuff.php
+++ b/includes/libs/objectcache/EmptyBagOStuff.php
@@ -27,7 +27,7 @@
  * @ingroup Cache
  */
 class EmptyBagOStuff extends BagOStuff {
-       public function get( $key, &$casToken = null, $flags = 0 ) {
+       protected function doGet( $key, $flags = 0 ) {
                return false;
        }
 

diff --git a/includes/libs/objectcache/HashBagOStuff.php b/includes/libs/objectcache/HashBagOStuff.php
index b685e41..d4044e9 100644 (file)
--- a/includes/libs/objectcache/HashBagOStuff.php
+++ b/includes/libs/objectcache/HashBagOStuff.php
@@ -48,7 +48,7 @@ class HashBagOStuff extends BagOStuff {
                return true;
        }
 
-       public function get( $key, &$casToken = null, $flags = 0 ) {
+       protected function doGet( $key, $flags = 0 ) {
                if ( !isset( $this->bag[$key] ) ) {
                        return false;
                }
@@ -57,8 +57,6 @@ class HashBagOStuff extends BagOStuff {
                        return false;
                }
 
-               $casToken = $this->bag[$key][0];
-
                return $this->bag[$key][0];
        }
 

diff --git a/includes/libs/objectcache/ReplicatedBagOStuff.php b/includes/libs/objectcache/ReplicatedBagOStuff.php
index 9e80e9f..9812047 100644 (file)
--- a/includes/libs/objectcache/ReplicatedBagOStuff.php
+++ b/includes/libs/objectcache/ReplicatedBagOStuff.php
@@ -72,10 +72,10 @@ class ReplicatedBagOStuff extends BagOStuff {
                $this->readStore->setDebug( $debug );
        }
 
-       public function get( $key, &$casToken = null, $flags = 0 ) {
+       protected function doGet( $key, $flags = 0 ) {
                return ( $flags & self::READ_LATEST )
-                       ? $this->writeStore->get( $key, $casToken, $flags )
-                       : $this->readStore->get( $key, $casToken, $flags );
+                       ? $this->writeStore->get( $key, $flags )
+                       : $this->readStore->get( $key, $flags );
        }
 
        public function getMulti( array $keys, $flags = 0 ) {

diff --git a/includes/libs/objectcache/WinCacheBagOStuff.php b/includes/libs/objectcache/WinCacheBagOStuff.php
index c480aa0..592565f 100644 (file)
--- a/includes/libs/objectcache/WinCacheBagOStuff.php
+++ b/includes/libs/objectcache/WinCacheBagOStuff.php
@@ -28,7 +28,13 @@
  * @ingroup Cache
  */
 class WinCacheBagOStuff extends BagOStuff {
-       public function get( $key, &$casToken = null, $flags = 0 ) {
+       protected function doGet( $key, $flags = 0 ) {
+               $casToken = null;
+
+               return $this->getWithToken( $key, $casToken, $flags );
+       }
+
+       protected function getWithToken( $key, &$casToken, $flags = 0 ) {
                $val = wincache_ucache_get( $key );
 
                $casToken = $val;

diff --git a/includes/libs/objectcache/XCacheBagOStuff.php b/includes/libs/objectcache/XCacheBagOStuff.php
index 9dbff6f..dc34f55 100644 (file)
--- a/includes/libs/objectcache/XCacheBagOStuff.php
+++ b/includes/libs/objectcache/XCacheBagOStuff.php
@@ -28,7 +28,7 @@
  * @ingroup Cache
  */
 class XCacheBagOStuff extends BagOStuff {
-       public function get( $key, &$casToken = null, $flags = 0 ) {
+       protected function doGet( $key, $flags = 0 ) {
                $val = xcache_get( $key );
 
                if ( is_string( $val ) ) {

diff --git a/includes/objectcache/MemcachedBagOStuff.php b/includes/objectcache/MemcachedBagOStuff.php
index 7d12749..412f017 100644 (file)
--- a/includes/objectcache/MemcachedBagOStuff.php
+++ b/includes/objectcache/MemcachedBagOStuff.php
@@ -57,7 +57,13 @@ class MemcachedBagOStuff extends BagOStuff {
                return $params;
        }
 
-       public function get( $key, &$casToken = null, $flags = 0 ) {
+       protected function doGet( $key, $flags = 0 ) {
+               $casToken = null;
+
+               return $this->getWithToken( $key, $casToken, $flags );
+       }
+
+       protected function getWithToken( $key, &$casToken, $flags = 0 ) {
                return $this->client->get( $this->encodeKey( $key ), $casToken );
        }
 

diff --git a/includes/objectcache/MemcachedPeclBagOStuff.php b/includes/objectcache/MemcachedPeclBagOStuff.php
index 1b2c8db..a7b48a2 100644 (file)
--- a/includes/objectcache/MemcachedPeclBagOStuff.php
+++ b/includes/objectcache/MemcachedPeclBagOStuff.php
@@ -115,7 +115,7 @@ class MemcachedPeclBagOStuff extends MemcachedBagOStuff {
                $this->client->addServers( $servers );
        }
 
-       public function get( $key, &$casToken = null, $flags = 0 ) {
+       protected function getWithToken( $key, &$casToken, $flags = 0 ) {
                $this->debugLog( "get($key)" );
                $result = $this->client->get( $this->encodeKey( $key ), null, $casToken );
                $result = $this->checkResult( $key, $result );

diff --git a/includes/objectcache/MultiWriteBagOStuff.php b/includes/objectcache/MultiWriteBagOStuff.php
index b690772..c05ecb6 100644 (file)
--- a/includes/objectcache/MultiWriteBagOStuff.php
+++ b/includes/objectcache/MultiWriteBagOStuff.php
@@ -87,11 +87,11 @@ class MultiWriteBagOStuff extends BagOStuff {
                $this->doWrite( self::ALL, 'setDebug', $debug );
        }
 
-       public function get( $key, &$casToken = null, $flags = 0 ) {
+       protected function doGet( $key, $flags = 0 ) {
                $misses = 0; // number backends checked
                $value = false;
                foreach ( $this->caches as $cache ) {
-                       $value = $cache->get( $key, $casToken, $flags );
+                       $value = $cache->get( $key, $flags );
                        if ( $value !== false ) {
                                break;
                        }

diff --git a/includes/objectcache/RedisBagOStuff.php b/includes/objectcache/RedisBagOStuff.php
index 7d9903f..e6b3f9e 100644 (file)
--- a/includes/objectcache/RedisBagOStuff.php
+++ b/includes/objectcache/RedisBagOStuff.php
@@ -85,14 +85,13 @@ class RedisBagOStuff extends BagOStuff {
                }
        }
 
-       public function get( $key, &$casToken = null, $flags = 0 ) {
+       protected function doGet( $key, $flags = 0 ) {
                list( $server, $conn ) = $this->getConnection( $key );
                if ( !$conn ) {
                        return false;
                }
                try {
                        $value = $conn->get( $key );
-                       $casToken = $value;
                        $result = $this->unserialize( $value );
                } catch ( RedisException $e ) {
                        $result = false;

diff --git a/includes/objectcache/SqlBagOStuff.php b/includes/objectcache/SqlBagOStuff.php
index f634df1..c2e5bd7 100644 (file)
--- a/includes/objectcache/SqlBagOStuff.php
+++ b/includes/objectcache/SqlBagOStuff.php
@@ -213,7 +213,13 @@ class SqlBagOStuff extends BagOStuff {
                }
        }
 
-       public function get( $key, &$casToken = null, $flags = 0 ) {
+       protected function doGet( $key, $flags = 0 ) {
+               $casToken = null;
+
+               return $this->getWithToken( $key, $casToken, $flags );
+       }
+
+       protected function getWithToken( $key, &$casToken, $flags = 0 ) {
                $values = $this->getMulti( array( $key ) );
                if ( array_key_exists( $key, $values ) ) {
                        $casToken = $values[$key];