From 6916548490c08aaf18d1d7b33a0687cebb1d0933 Mon Sep 17 00:00:00 2001 From: Ori Livneh Date: Fri, 9 Oct 2015 16:35:08 -0700 Subject: [PATCH] Add `makeKey` and `makeGlobalKey` to BagOStuff * Add a string `keyspace` member to BagOStuff instances. The default implementation, meant for simple key/value stores, treats the key space as a string prefix to prepend to keys. By default, its value is `local`, but any instance created via ObjectCache::newFromParams() (or or one of its callers) will have that default to $wgCachePrefix / wfWikiID(). * Add `makeKey` and `makeGlobalKey` methods to the base BagOStuff class. These methods are not static to allow for BagOStuff types which require a configured instance to know the underlying storage engine's key semantics. * Make wfMemcKey() and wfGlobalCacheKey() delegate to these methods on the main ObjectCache instance. Change-Id: Ib7fc2f939be3decfa97f66af8c2431c51039905f --- includes/GlobalFunctions.php | 28 ++++--- includes/libs/objectcache/BagOStuff.php | 43 +++++++++++ includes/objectcache/ObjectCache.php | 34 ++++++++- .../includes/GlobalFunctions/GlobalTest.php | 74 +++---------------- .../includes/objectcache/BagOStuffTest.php | 29 ++++++++ 5 files changed, 126 insertions(+), 82 deletions(-) diff --git a/includes/GlobalFunctions.php b/includes/GlobalFunctions.php index 243df92827..1ea2c5e5af 100644 --- a/includes/GlobalFunctions.php +++ b/includes/GlobalFunctions.php @@ -3554,11 +3554,10 @@ function wfGetPrecompiledData( $name ) { * @return string */ function wfMemcKey( /*...*/ ) { - global $wgCachePrefix; - $prefix = $wgCachePrefix === false ? wfWikiID() : $wgCachePrefix; - $args = func_get_args(); - $key = $prefix . ':' . implode( ':', $args ); - return strtr( $key, ' ', '_' ); + return call_user_func_array( + array( ObjectCache::getMainClusterInstance(), 'makeKey' ), + func_get_args() + ); } /** @@ -3573,13 +3572,11 @@ function wfMemcKey( /*...*/ ) { */ function wfForeignMemcKey( $db, $prefix /*...*/ ) { $args = array_slice( func_get_args(), 2 ); - if ( $prefix ) { - // Match wfWikiID() logic - $key = "$db-$prefix:" . implode( ':', $args ); - } else { - $key = $db . ':' . implode( ':', $args ); - } - return strtr( $key, ' ', '_' ); + $keyspace = $prefix ? "$db-$prefix" : $db; + return call_user_func_array( + array( ObjectCache::getMainClusterInstance(), 'makeKeyInternal' ), + array( $keyspace, $args ) + ); } /** @@ -3594,9 +3591,10 @@ function wfForeignMemcKey( $db, $prefix /*...*/ ) { * @return string */ function wfGlobalCacheKey( /*...*/ ) { - $args = func_get_args(); - $key = 'global:' . implode( ':', $args ); - return strtr( $key, ' ', '_' ); + return call_user_func_array( + array( ObjectCache::getMainClusterInstance(), 'makeGlobalKey' ), + func_get_args() + ); } /** diff --git a/includes/libs/objectcache/BagOStuff.php b/includes/libs/objectcache/BagOStuff.php index 8cbd48a638..fc74985ed3 100644 --- a/includes/libs/objectcache/BagOStuff.php +++ b/includes/libs/objectcache/BagOStuff.php @@ -45,9 +45,13 @@ use Psr\Log\NullLogger; abstract class BagOStuff implements LoggerAwareInterface { /** @var array[] Lock tracking */ protected $locks = array(); + /** @var integer */ protected $lastError = self::ERR_NONE; + /** @var string */ + protected $keyspace = 'local'; + /** @var LoggerInterface */ protected $logger; @@ -70,6 +74,10 @@ abstract class BagOStuff implements LoggerAwareInterface { } else { $this->setLogger( new NullLogger() ); } + + if ( isset( $params['keyspace'] ) ) { + $this->keyspace = $params['keyspace']; + } } /** @@ -602,4 +610,39 @@ abstract class BagOStuff implements LoggerAwareInterface { protected function isInteger( $value ) { return ( is_int( $value ) || ctype_digit( $value ) ); } + + /** + * Construct a cache key. + * + * @since 1.27 + * @param string $keyspace + * @param array $args + * @return string + */ + public function makeKeyInternal( $keyspace, $args ) { + $key = $keyspace . ':' . implode( ':', $args ); + return strtr( $key, ' ', '_' ); + } + + /** + * Make a global cache key. + * + * @since 1.27 + * @param string $args,... + * @return string + */ + public function makeGlobalKey() { + return $this->makeKeyInternal( 'global', func_get_args() ); + } + + /** + * Make a cache key, scoped to this instance's keyspace. + * + * @since 1.27 + * @param string $args,... + * @return string + */ + public function makeKey() { + return $this->makeKeyInternal( $this->keyspace, func_get_args() ); + } } diff --git a/includes/objectcache/ObjectCache.php b/includes/objectcache/ObjectCache.php index c40f69613f..bc16537abd 100644 --- a/includes/objectcache/ObjectCache.php +++ b/includes/objectcache/ObjectCache.php @@ -124,6 +124,31 @@ class ObjectCache { return self::newFromParams( $wgObjectCaches[$id] ); } + /** + * Get the default keyspace for this wiki. + * + * This is either the value of the `CachePrefix` configuration variable, + * or (if the former is unset) the `DBname` configuration variable, with + * `DBprefix` (if defined). + * + * @return string + */ + public static function getDefaultKeyspace() { + global $wgCachePrefix, $wgDBname, $wgDBprefix; + + $keyspace = $wgCachePrefix; + if ( is_string( $keyspace ) && $keyspace !== '' ) { + return $keyspace; + } + + $keyspace = $wgDBname; + if ( is_string( $wgDBprefix ) && $wgDBprefix !== '' ) { + $keyspace .= '-' . $wgDBprefix; + } + + return $keyspace; + } + /** * Create a new cache object from parameters. * @@ -143,6 +168,9 @@ class ObjectCache { // have all logging suddenly disappear $params['logger'] = LoggerFactory::getInstance( 'objectcache' ); } + if ( !isset( $params['keyspace'] ) ) { + $params['keyspace'] = self::getDefaultKeyspace(); + } if ( isset( $params['factory'] ) ) { return call_user_func( $params['factory'], $params ); } elseif ( isset( $params['class'] ) ) { @@ -268,9 +296,9 @@ class ObjectCache { * @return BagOStuff */ public static function getMainClusterInstance() { - $config = RequestContext::getMain()->getConfig(); - $id = $config->get( 'MainCacheType' ); - return self::getInstance( $id ); + global $wgMainCacheType; + + return self::getInstance( $wgMainCacheType ); } /** diff --git a/tests/phpunit/includes/GlobalFunctions/GlobalTest.php b/tests/phpunit/includes/GlobalFunctions/GlobalTest.php index e39e02f629..4847b8213d 100644 --- a/tests/phpunit/includes/GlobalFunctions/GlobalTest.php +++ b/tests/phpunit/includes/GlobalFunctions/GlobalTest.php @@ -708,81 +708,27 @@ class GlobalTest extends MediaWikiTestCase { } public function testWfMemcKey() { - // Just assert the exact output so we can catch unintentional changes to key - // construction, which would effectively invalidate all existing cache. - - $this->setMwGlobals( array( - 'wgCachePrefix' => false, - 'wgDBname' => 'example', - 'wgDBprefix' => '', - ) ); - $this->assertEquals( - wfMemcKey( 'foo', '123', 'bar' ), - 'example:foo:123:bar' - ); - - $this->setMwGlobals( array( - 'wgCachePrefix' => false, - 'wgDBname' => 'example', - 'wgDBprefix' => 'mw_', - ) ); - $this->assertEquals( - wfMemcKey( 'foo', '123', 'bar' ), - 'example-mw_:foo:123:bar' - ); - - $this->setMwGlobals( array( - 'wgCachePrefix' => 'custom', - 'wgDBname' => 'example', - 'wgDBprefix' => 'mw_', - ) ); + $cache = ObjectCache::getMainClusterInstance(); $this->assertEquals( - wfMemcKey( 'foo', '123', 'bar' ), - 'custom:foo:123:bar' + $cache->makeKey( 'foo', 123, 'bar' ), + wfMemcKey( 'foo', 123, 'bar' ) ); } public function testWfForeignMemcKey() { - $this->setMwGlobals( array( - 'wgCachePrefix' => false, - 'wgDBname' => 'example', - 'wgDBprefix' => '', - ) ); - $local = wfMemcKey( 'foo', 'bar' ); - - $this->setMwGlobals( array( - 'wgDBname' => 'other', - 'wgDBprefix' => 'mw_', - ) ); + $cache = ObjectCache::getMainClusterInstance(); + $keyspace = $this->readAttribute( $cache, 'keyspace' ); $this->assertEquals( - wfForeignMemcKey( 'example', '', 'foo', 'bar' ), - $local, - 'Match output of wfMemcKey from local wiki' + wfForeignMemcKey( $keyspace, '', 'foo', 'bar' ), + $cache->makeKey( 'foo', 'bar' ) ); } public function testWfGlobalCacheKey() { - $this->setMwGlobals( array( - 'wgCachePrefix' => 'ignored', - 'wgDBname' => 'example', - 'wgDBprefix' => '' - ) ); - $one = wfGlobalCacheKey( 'some', 'thing' ); - $this->assertEquals( - $one, - 'global:some:thing' - ); - - $this->setMwGlobals( array( - 'wgDBname' => 'other', - 'wgDBprefix' => 'mw_' - ) ); - $two = wfGlobalCacheKey( 'some', 'thing' ); - + $cache = ObjectCache::getMainClusterInstance(); $this->assertEquals( - $one, - $two, - 'Not fragmented by wiki id' + $cache->makeGlobalKey( 'foo', 123, 'bar' ), + wfGlobalCacheKey( 'foo', 123, 'bar' ) ); } diff --git a/tests/phpunit/includes/objectcache/BagOStuffTest.php b/tests/phpunit/includes/objectcache/BagOStuffTest.php index b9fe4903c4..466b9f58f8 100644 --- a/tests/phpunit/includes/objectcache/BagOStuffTest.php +++ b/tests/phpunit/includes/objectcache/BagOStuffTest.php @@ -23,6 +23,35 @@ class BagOStuffTest extends MediaWikiTestCase { $this->cache->delete( wfMemcKey( 'test' ) ); } + /** + * @covers BagOStuff::makeGlobalKey + * @covers BagOStuff::makeKeyInternal + */ + public function testMakeKey() { + $cache = ObjectCache::newFromId( 'hash' ); + + $localKey = $cache->makeKey( 'first', 'second', 'third' ); + $globalKey = $cache->makeGlobalKey( 'first', 'second', 'third' ); + + $this->assertStringMatchesFormat( + '%Sfirst%Ssecond%Sthird%S', + $localKey, + 'Local key interpolates parameters' + ); + + $this->assertStringMatchesFormat( + 'global%Sfirst%Ssecond%Sthird%S', + $globalKey, + 'Global key interpolates parameters and contains global prefix' + ); + + $this->assertNotEquals( + $localKey, + $globalKey, + 'Local key and global key with same parameters should not be equal' + ); + } + /** * @covers BagOStuff::merge * @covers BagOStuff::mergeViaLock -- 2.20.1