Merge "objectcache: merge getWithToken() into doGet() for simplicity"

This commit is contained in:
jenkins-bot 2019-03-27 20:58:25 +00:00 committed by Gerrit Code Review
commit 046b9d7158
15 changed files with 62 additions and 97 deletions

View file

@ -72,11 +72,7 @@ class APCBagOStuff extends BagOStuff {
}
}
protected function doGet( $key, $flags = 0 ) {
return $this->unserialize( apc_fetch( $key . self::KEY_SUFFIX ) );
}
protected function getWithToken( $key, &$casToken, $flags = 0 ) {
protected function doGet( $key, $flags = 0, &$casToken = null ) {
$casToken = null;
$blob = apc_fetch( $key . self::KEY_SUFFIX );

View file

@ -39,11 +39,7 @@ class APCUBagOStuff extends APCBagOStuff {
parent::__construct( $params );
}
protected function doGet( $key, $flags = 0 ) {
return $this->unserialize( apcu_fetch( $key . self::KEY_SUFFIX ) );
}
protected function getWithToken( $key, &$casToken, $flags = 0 ) {
protected function doGet( $key, $flags = 0, &$casToken = null ) {
$casToken = null;
$blob = apcu_fetch( $key . self::KEY_SUFFIX );

View file

@ -175,13 +175,9 @@ abstract class BagOStuff implements IExpiringStore, LoggerAwareInterface {
*
* @param string $key
* @param int $flags Bitfield of BagOStuff::READ_* constants [optional]
* @param int|null $oldFlags [unused]
* @return mixed Returns false on failure or if the item does not exist
*/
public function get( $key, $flags = 0, $oldFlags = null ) {
// B/C for ( $key, &$casToken = null, $flags = 0 )
$flags = is_int( $oldFlags ) ? $oldFlags : $flags;
public function get( $key, $flags = 0 ) {
$this->trackDuplicateKeys( $key );
return $this->doGet( $key, $flags );
@ -223,22 +219,10 @@ abstract class BagOStuff implements IExpiringStore, LoggerAwareInterface {
/**
* @param string $key
* @param int $flags Bitfield of BagOStuff::READ_* constants [optional]
* @param mixed|null &$casToken Token to use for check-and-set comparisons
* @return mixed Returns false on failure or if the item does not exist
*/
abstract protected function doGet( $key, $flags = 0 );
/**
* @note This method is only needed if merge() uses mergeViaCas()
*
* @param string $key
* @param mixed &$casToken
* @param int $flags Bitfield of BagOStuff::READ_* constants [optional]
* @return mixed Returns false on failure or if the item does not exist
* @throws Exception
*/
protected function getWithToken( $key, &$casToken, $flags = 0 ) {
throw new Exception( __METHOD__ . ' not implemented.' );
}
abstract protected function doGet( $key, $flags = 0, &$casToken = null );
/**
* Set an item
@ -308,7 +292,7 @@ abstract class BagOStuff implements IExpiringStore, LoggerAwareInterface {
$reportDupes = $this->reportDupes;
$this->reportDupes = false;
$casToken = null; // passed by reference
$currentValue = $this->getWithToken( $key, $casToken, self::READ_LATEST );
$currentValue = $this->doGet( $key, self::READ_LATEST, $casToken );
$this->reportDupes = $reportDupes;
if ( $this->getLastError() ) {
@ -365,7 +349,7 @@ abstract class BagOStuff implements IExpiringStore, LoggerAwareInterface {
}
$curCasToken = null; // passed by reference
$this->getWithToken( $key, $curCasToken, self::READ_LATEST );
$this->doGet( $key, self::READ_LATEST, $curCasToken );
if ( $casToken === $curCasToken ) {
$success = $this->set( $key, $value, $exptime, $flags );
} else {

View file

@ -32,6 +32,7 @@
* up going to the HashBagOStuff used for the in-memory cache).
*
* @ingroup Cache
* @TODO: Make this class use composition instead of calling super
*/
class CachedBagOStuff extends HashBagOStuff {
/** @var BagOStuff */
@ -50,10 +51,10 @@ class CachedBagOStuff extends HashBagOStuff {
$this->attrMap = $backend->attrMap;
}
protected function doGet( $key, $flags = 0 ) {
$ret = parent::doGet( $key, $flags );
public function get( $key, $flags = 0 ) {
$ret = parent::get( $key, $flags );
if ( $ret === false && !$this->hasKey( $key ) ) {
$ret = $this->backend->doGet( $key, $flags );
$ret = $this->backend->get( $key, $flags );
$this->set( $key, $ret, 0, self::WRITE_CACHE_ONLY );
}
return $ret;

View file

@ -27,7 +27,9 @@
* @ingroup Cache
*/
class EmptyBagOStuff extends BagOStuff {
protected function doGet( $key, $flags = 0 ) {
protected function doGet( $key, $flags = 0, &$casToken = null ) {
$casToken = null;
return false;
}

View file

@ -58,12 +58,10 @@ class HashBagOStuff extends BagOStuff {
}
}
protected function doGet( $key, $flags = 0 ) {
if ( !$this->hasKey( $key ) ) {
return false;
}
protected function doGet( $key, $flags = 0, &$casToken = null ) {
$casToken = null;
if ( $this->expire( $key ) ) {
if ( !$this->hasKey( $key ) || $this->expire( $key ) ) {
return false;
}
@ -72,20 +70,11 @@ class HashBagOStuff extends BagOStuff {
unset( $this->bag[$key] );
$this->bag[$key] = $temp;
$casToken = $this->bag[$key][self::KEY_CAS];
return $this->bag[$key][self::KEY_VAL];
}
protected function getWithToken( $key, &$casToken, $flags = 0 ) {
$casToken = null;
$value = $this->doGet( $key );
if ( $value !== false ) {
$casToken = $this->bag[$key][self::KEY_CAS];
}
return $value;
}
public function set( $key, $value, $exptime = 0, $flags = 0 ) {
// Refresh key position for maxCacheKeys eviction
unset( $this->bag[$key] );

View file

@ -50,13 +50,7 @@ class MemcachedBagOStuff extends BagOStuff {
];
}
protected function doGet( $key, $flags = 0 ) {
$casToken = null;
return $this->getWithToken( $key, $casToken, $flags );
}
protected function getWithToken( $key, &$casToken, $flags = 0 ) {
protected function doGet( $key, $flags = 0, &$casToken = null ) {
return $this->client->get( $this->validateKeyEncoding( $key ), $casToken );
}

View file

@ -138,7 +138,7 @@ class MemcachedPeclBagOStuff extends MemcachedBagOStuff {
return $params;
}
protected function getWithToken( $key, &$casToken, $flags = 0 ) {
protected function doGet( $key, $flags = 0, &$casToken = null ) {
$this->debugLog( "get($key)" );
if ( defined( Memcached::class . '::GET_EXTENDED' ) ) { // v3.0.0
$flags = Memcached::GET_EXTENDED;

View file

@ -103,7 +103,7 @@ class MultiWriteBagOStuff extends BagOStuff {
}
}
protected function doGet( $key, $flags = 0 ) {
public function get( $key, $flags = 0 ) {
if ( ( $flags & self::READ_LATEST ) == self::READ_LATEST ) {
// If the latest write was a delete(), we do NOT want to fallback
// to the other tiers and possibly see the old value. Also, this
@ -349,4 +349,8 @@ class MultiWriteBagOStuff extends BagOStuff {
public function makeGlobalKey( $class, $component = null ) {
return $this->caches[0]->makeGlobalKey( ...func_get_args() );
}
protected function doGet( $key, $flags = 0, &$casToken = null ) {
throw new LogicException( __METHOD__ . ': proxy class does not need this method.' );
}
}

View file

@ -84,13 +84,7 @@ class RESTBagOStuff extends BagOStuff {
$this->client->setLogger( $logger );
}
protected function doGet( $key, $flags = 0 ) {
$casToken = null;
return $this->getWithToken( $key, $casToken, $flags );
}
protected function getWithToken( $key, &$casToken, $flags = 0 ) {
protected function doGet( $key, $flags = 0, &$casToken = null ) {
$casToken = null;
$req = [

View file

@ -86,13 +86,9 @@ class RedisBagOStuff extends BagOStuff {
$this->attrMap[self::ATTR_SYNCWRITES] = self::QOS_SYNCWRITES_NONE;
}
protected function doGet( $key, $flags = 0 ) {
protected function doGet( $key, $flags = 0, &$casToken = null ) {
$casToken = null;
return $this->getWithToken( $key, $casToken, $flags );
}
protected function getWithToken( $key, &$casToken, $flags = 0 ) {
list( $server, $conn ) = $this->getConnection( $key );
if ( !$conn ) {
return false;

View file

@ -74,7 +74,7 @@ class ReplicatedBagOStuff extends BagOStuff {
$this->readStore->setDebug( $debug );
}
protected function doGet( $key, $flags = 0 ) {
public function get( $key, $flags = 0 ) {
return ( $flags & self::READ_LATEST )
? $this->writeStore->get( $key, $flags )
: $this->readStore->get( $key, $flags );
@ -160,4 +160,8 @@ class ReplicatedBagOStuff extends BagOStuff {
public function makeGlobalKey( $class, $component = null ) {
return $this->writeStore->makeGlobalKey( ...func_get_args() );
}
protected function doGet( $key, $flags = 0, &$casToken = null ) {
throw new LogicException( __METHOD__ . ': proxy class does not need this method.' );
}
}

View file

@ -28,13 +28,7 @@
* @ingroup Cache
*/
class WinCacheBagOStuff extends BagOStuff {
protected function doGet( $key, $flags = 0 ) {
$blob = wincache_ucache_get( $key );
return is_string( $blob ) ? unserialize( $blob ) : false;
}
protected function getWithToken( $key, &$casToken, $flags = 0 ) {
protected function doGet( $key, $flags = 0, &$casToken = null ) {
$casToken = null;
$blob = wincache_ucache_get( $key );
@ -43,12 +37,10 @@ class WinCacheBagOStuff extends BagOStuff {
}
$value = unserialize( $blob );
if ( $value === false ) {
return false;
if ( $value !== false ) {
$casToken = (string)$blob; // don't bother hashing this
}
$casToken = $blob; // don't bother hashing this
return $value;
}
@ -58,7 +50,7 @@ class WinCacheBagOStuff extends BagOStuff {
}
$curCasToken = null; // passed by reference
$this->getWithToken( $key, $curCasToken, self::READ_LATEST );
$this->doGet( $key, self::READ_LATEST, $curCasToken );
if ( $casToken === $curCasToken ) {
$success = $this->set( $key, $value, $exptime, $flags );
} else {

View file

@ -234,22 +234,34 @@ class SqlBagOStuff extends BagOStuff {
}
}
protected function doGet( $key, $flags = 0 ) {
protected function doGet( $key, $flags = 0, &$casToken = null ) {
$casToken = null;
return $this->getWithToken( $key, $casToken, $flags );
}
$blobs = $this->fetchBlobMulti( [ $key ] );
if ( array_key_exists( $key, $blobs ) ) {
$blob = $blobs[$key];
$value = $this->unserialize( $blob );
protected function getWithToken( $key, &$casToken, $flags = 0 ) {
$values = $this->getMulti( [ $key ] );
if ( array_key_exists( $key, $values ) ) {
$casToken = $values[$key];
return $values[$key];
$casToken = ( $value !== false ) ? $blob : null;
return $value;
}
return false;
}
public function getMulti( array $keys, $flags = 0 ) {
$values = [];
$blobs = $this->fetchBlobMulti( $keys );
foreach ( $blobs as $key => $blob ) {
$values[$key] = $this->unserialize( $blob );
}
return $values;
}
public function fetchBlobMulti( array $keys, $flags = 0 ) {
$values = []; // array of (key => value)
$keysByTable = [];
@ -298,7 +310,7 @@ class SqlBagOStuff extends BagOStuff {
if ( $this->isExpired( $db, $row->exptime ) ) { // MISS
$this->debug( "get: key has expired" );
} else { // HIT
$values[$key] = $this->unserialize( $db->decodeBlob( $row->value ) );
$values[$key] = $db->decodeBlob( $row->value );
}
} catch ( DBQueryError $e ) {
$this->handleWriteError( $e, $db, $row->serverIndex );
@ -420,7 +432,7 @@ class SqlBagOStuff extends BagOStuff {
],
[
'keyname' => $key,
'value' => $db->encodeBlob( $this->serialize( $casToken ) )
'value' => $db->encodeBlob( $casToken )
],
__METHOD__
);

View file

@ -11,7 +11,7 @@ class CachedBagOStuffTest extends PHPUnit\Framework\TestCase {
/**
* @covers CachedBagOStuff::__construct
* @covers CachedBagOStuff::doGet
* @covers CachedBagOStuff::get
*/
public function testGetFromBackend() {
$backend = new HashBagOStuff;
@ -36,6 +36,7 @@ class CachedBagOStuffTest extends PHPUnit\Framework\TestCase {
$cache->set( "key$i", 1 );
$this->assertEquals( 1, $cache->get( "key$i" ) );
$this->assertEquals( 1, $backend->get( "key$i" ) );
$cache->delete( "key$i" );
$this->assertEquals( false, $cache->get( "key$i" ) );
$this->assertEquals( false, $backend->get( "key$i" ) );
@ -67,7 +68,7 @@ class CachedBagOStuffTest extends PHPUnit\Framework\TestCase {
}
/**
* @covers CachedBagOStuff::doGet
* @covers CachedBagOStuff::get
*/
public function testCacheBackendMisses() {
$backend = new HashBagOStuff;