From 1fa1235d32cf2788a3b4571e22b47d79e224fcae Mon Sep 17 00:00:00 2001 From: Aaron Schulz Date: Sat, 3 Oct 2015 22:59:38 -0700 Subject: [PATCH] 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 --- includes/libs/objectcache/APCBagOStuff.php | 4 +-- includes/libs/objectcache/BagOStuff.php | 32 ++++++++++++++++--- includes/libs/objectcache/EmptyBagOStuff.php | 2 +- includes/libs/objectcache/HashBagOStuff.php | 4 +-- .../libs/objectcache/ReplicatedBagOStuff.php | 6 ++-- .../libs/objectcache/WinCacheBagOStuff.php | 8 ++++- includes/libs/objectcache/XCacheBagOStuff.php | 2 +- includes/objectcache/MemcachedBagOStuff.php | 8 ++++- .../objectcache/MemcachedPeclBagOStuff.php | 2 +- includes/objectcache/MultiWriteBagOStuff.php | 4 +-- includes/objectcache/RedisBagOStuff.php | 3 +- includes/objectcache/SqlBagOStuff.php | 8 ++++- 12 files changed, 60 insertions(+), 23 deletions(-) diff --git a/includes/libs/objectcache/APCBagOStuff.php b/includes/libs/objectcache/APCBagOStuff.php index 0dbbaba987..522c5d7196 100644 --- 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 647d938919..ecdf48fc95 100644 --- 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 55e84b05f2..bef04569b1 100644 --- 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 b685e41fc3..d4044e9f16 100644 --- 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 9e80e9fd1a..98120477d3 100644 --- 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 c480aa08d9..592565ff83 100644 --- 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 9dbff6f174..dc34f557a3 100644 --- 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 7d1274921c..412f017302 100644 --- 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 1b2c8db62e..a7b48a2a2e 100644 --- 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 b69077270e..c05ecb6d79 100644 --- 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 7d9903fe78..e6b3f9eb2f 100644 --- 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 f634df1244..c2e5bd760c 100644 --- 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]; -- 2.20.1