From: Timo Tijhof <krinklemail@gmail.com>
Date: Tue, 23 Jun 2015 04:52:30 +0000 (+0100)
Subject: Implement wfGlobalCacheKey() for database-agnostic keys
X-Git-Tag: 1.31.0-rc.0~11007^2
X-Git-Url: http://git.cyclocoop.org/%7B%24admin_url%7Dmes_infos.php?a=commitdiff_plain;h=0f64b61271edce060f8519dd0bb3f58f56054aaa;p=lhc%2Fweb%2Fwiklou.git

Implement wfGlobalCacheKey() for database-agnostic keys

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
---

diff --git a/includes/GlobalFunctions.php b/includes/GlobalFunctions.php
index e2e06669ca..c6184261e7 100644
--- a/includes/GlobalFunctions.php
+++ b/includes/GlobalFunctions.php
@@ -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
diff --git a/tests/phpunit/includes/GlobalFunctions/GlobalTest.php b/tests/phpunit/includes/GlobalFunctions/GlobalTest.php
index 5c28ece369..e89e36f691 100644
--- a/tests/phpunit/includes/GlobalFunctions/GlobalTest.php
+++ b/tests/phpunit/includes/GlobalFunctions/GlobalTest.php
@@ -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;