jenkins-bot has submitted this change and it was merged.
Change subject: Add `makeKey` and `makeGlobalKey` to BagOStuff
......................................................................
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
---
M includes/GlobalFunctions.php
M includes/libs/objectcache/BagOStuff.php
M includes/objectcache/ObjectCache.php
M tests/phpunit/includes/GlobalFunctions/GlobalTest.php
M tests/phpunit/includes/objectcache/BagOStuffTest.php
5 files changed, 126 insertions(+), 82 deletions(-)
Approvals:
Aaron Schulz: Looks good to me, approved
jenkins-bot: Verified
diff --git a/includes/GlobalFunctions.php b/includes/GlobalFunctions.php
index 243df92..1ea2c5e 100644
--- a/includes/GlobalFunctions.php
+++ b/includes/GlobalFunctions.php
@@ -3554,11 +3554,10 @@
* @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 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 @@
* @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 8cbd48a..fc74985 100644
--- a/includes/libs/objectcache/BagOStuff.php
+++ b/includes/libs/objectcache/BagOStuff.php
@@ -45,8 +45,12 @@
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;
@@ -69,6 +73,10 @@
$this->setLogger( $params['logger'] );
} else {
$this->setLogger( new NullLogger() );
+ }
+
+ if ( isset( $params['keyspace'] ) ) {
+ $this->keyspace = $params['keyspace'];
}
}
@@ -602,4 +610,39 @@
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 c40f696..bc16537 100644
--- a/includes/objectcache/ObjectCache.php
+++ b/includes/objectcache/ObjectCache.php
@@ -125,6 +125,31 @@
}
/**
+ * 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.
*
* @param array $params Must have 'factory' or 'class' property.
@@ -142,6 +167,9 @@
// For backwards-compatability with custom parameters,
lets not
// 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 );
@@ -268,9 +296,9 @@
* @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 e39e02f..4847b82 100644
--- a/tests/phpunit/includes/GlobalFunctions/GlobalTest.php
+++ b/tests/phpunit/includes/GlobalFunctions/GlobalTest.php
@@ -708,81 +708,27 @@
}
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' => '',
- ) );
+ $cache = ObjectCache::getMainClusterInstance();
$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_',
- ) );
- $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' );
+ $cache = ObjectCache::getMainClusterInstance();
$this->assertEquals(
- $one,
- 'global:some:thing'
- );
-
- $this->setMwGlobals( array(
- 'wgDBname' => 'other',
- 'wgDBprefix' => 'mw_'
- ) );
- $two = wfGlobalCacheKey( 'some', 'thing' );
-
- $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 b9fe490..466b9f5 100644
--- a/tests/phpunit/includes/objectcache/BagOStuffTest.php
+++ b/tests/phpunit/includes/objectcache/BagOStuffTest.php
@@ -24,6 +24,35 @@
}
/**
+ * @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
*/
--
To view, visit https://gerrit.wikimedia.org/r/244825
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Ib7fc2f939be3decfa97f66af8c2431c51039905f
Gerrit-PatchSet: 10
Gerrit-Project: mediawiki/core
Gerrit-Branch: master
Gerrit-Owner: Ori.livneh <[email protected]>
Gerrit-Reviewer: Aaron Schulz <[email protected]>
Gerrit-Reviewer: Krinkle <[email protected]>
Gerrit-Reviewer: Ori.livneh <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
