Implement wfGlobalCacheKey() for database-agnostic keys
authorTimo Tijhof <krinklemail@gmail.com>
Tue, 23 Jun 2015 04:52:30 +0000 (05:52 +0100)
committerTimo Tijhof <krinklemail@gmail.com>
Tue, 23 Jun 2015 06:52:18 +0000 (07:52 +0100)
Some code paths abuse wfForeignMemcKey() for this purpose. This is semantically
incorrect and seems fragile. Also the empty second argument (for db-prefix) is
either misused or akwardly set to the empty string.

It also creates a namespace conflict between arbitrary application keys (from
the software) and any database names (from users). This commit reduces the
conflict surface down to a single key (namely, "global").

Also added unit tests to assert the implicit restrictions and
assumptions between these cache key functions are valid.

Change-Id: Ia0953b51005fe3de3b881dd1bd64c9d3c85e8c66

includes/GlobalFunctions.php
tests/phpunit/includes/GlobalFunctions/GlobalTest.php

index e2e0666..c618426 100644 (file)
@@ -3495,7 +3495,7 @@ function wfGetPrecompiledData( $name ) {
 }
 
 /**
- * Get a cache key
+ * Make a cache key for the local wiki.
  *
  * @param string $args,...
  * @return string
@@ -3510,7 +3510,9 @@ function wfMemcKey( /*...*/ ) {
 }
 
 /**
- * Get a cache key for a foreign DB
+ * Make a cache key for a foreign DB.
+ *
+ * Must match what wfMemcKey() would produce in context of the foreign wiki.
  *
  * @param string $db
  * @param string $prefix
@@ -3520,6 +3522,7 @@ 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 );
@@ -3527,6 +3530,24 @@ function wfForeignMemcKey( $db, $prefix /*...*/ ) {
        return str_replace( ' ', '_', $key );
 }
 
+/**
+ * Make a cache key with database-agnostic prefix.
+ *
+ * Doesn't have a wiki-specific namespace. Uses a generic 'global' prefix
+ * instead. Must have a prefix as otherwise keys that use a database name
+ * in the first segment will clash with wfMemcKey/wfForeignMemcKey.
+ *
+ * @since 1.26
+ * @param string $args,...
+ * @return string
+ */
+function wfGlobalCacheKey( /*...*/ ) {
+       $args = func_get_args();
+       $key = 'global:' . implode( ':', $args );
+       $key = str_replace( ' ', '_', $key );
+       return $key;
+}
+
 /**
  * Get an ASCII string identifying this wiki
  * This is used as a prefix in memcached keys
index 5c28ece..e89e36f 100644 (file)
@@ -687,6 +687,105 @@ class GlobalTest extends MediaWikiTestCase {
                $this->assertEquals( $expected, $actual, $description );
        }
 
+       public function wfWikiID() {
+               $this->setMwGlobals( array(
+                       'wgDBname' => 'example',
+                       'wgDBprefix' => '',
+               ) );
+               $this->assertEquals(
+                       wfWikiID(),
+                       'example'
+               );
+
+               $this->setMwGlobals( array(
+                       'wgDBname' => 'example',
+                       'wgDBprefix' => 'mw_',
+               ) );
+               $this->assertEquals(
+                       wfWikiID(),
+                       'example-mw_'
+               );
+       }
+
+       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_',
+               ) );
+               $this->assertEquals(
+                       wfMemcKey( 'foo', '123', 'bar' ),
+                       'custom: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_',
+               ) );
+               $this->assertEquals(
+                       wfForeignMemcKey( 'example', '', 'foo', 'bar' ),
+                       $local,
+                       'Match output of wfMemcKey from local wiki'
+               );
+       }
+
+       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' );
+
+               $this->assertEquals(
+                       $one,
+                       $two,
+                       'Not fragmented by wiki id'
+               );
+       }
+
        public static function provideWfShellWikiCmdList() {
                global $wgPhpCli;