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

diff --git a/includes/GlobalFunctions.php b/includes/GlobalFunctions.php
index 243df92..1ea2c5e 100644 (file)
--- 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 8cbd48a..fc74985 100644 (file)
--- 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 c40f696..bc16537 100644 (file)
--- 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 e39e02f..4847b82 100644 (file)
--- 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 b9fe490..466b9f5 100644 (file)
--- 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