From 4ec550812bdc15cd5622f5099af39fb05c1c46a2 Mon Sep 17 00:00:00 2001
From: Timo Tijhof <krinklemail@gmail.com>
Date: Tue, 3 Nov 2015 05:59:07 +0000
Subject: [PATCH] objectcache: Refresh key in HashBagOStuff::set() for maxKeys
 eviction

* Match behaviour of MapCacheLRU and ProcessCacheLRU.
* Add missing unit tests for TTL and maxCacheKeys eviction behaviour.

Change-Id: I559eae1cd336274b21728e86775cfbad7e2f2c6d
---
 includes/libs/objectcache/HashBagOStuff.php   |  9 +++--
 .../libs/objectcache/HashBagOStuffTest.php    | 36 +++++++++++++++++++
 2 files changed, 42 insertions(+), 3 deletions(-)

diff --git a/includes/libs/objectcache/HashBagOStuff.php b/includes/libs/objectcache/HashBagOStuff.php
index bdcf18017f..a058ecff26 100644
--- a/includes/libs/objectcache/HashBagOStuff.php
+++ b/includes/libs/objectcache/HashBagOStuff.php
@@ -1,6 +1,6 @@
 <?php
 /**
- * Object caching using PHP arrays.
+ * Per-process memory cache for storing items.
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
@@ -23,8 +23,9 @@
 use Wikimedia\Assert\Assert;
 
 /**
- * This is a test of the interface, mainly. It stores things in an associative
- * array, which is not going to persist between program runs.
+ * Simple store for keeping values in an associative array for the current process.
+ *
+ * Data will not persist and is not shared with other processes.
  *
  * @ingroup Cache
  */
@@ -72,6 +73,8 @@ class HashBagOStuff extends BagOStuff {
 	}
 
 	public function set( $key, $value, $exptime = 0, $flags = 0 ) {
+		// Refresh key position for maxCacheKeys eviction
+		unset( $this->bag[$key] );
 		$this->bag[$key] = array(
 			self::KEY_VAL => $value,
 			self::KEY_EXP => $this->convertExpiry( $exptime )
diff --git a/tests/phpunit/includes/libs/objectcache/HashBagOStuffTest.php b/tests/phpunit/includes/libs/objectcache/HashBagOStuffTest.php
index 5344e2d51c..4f8a3cb56b 100644
--- a/tests/phpunit/includes/libs/objectcache/HashBagOStuffTest.php
+++ b/tests/phpunit/includes/libs/objectcache/HashBagOStuffTest.php
@@ -17,4 +17,40 @@ class HashBagOStuffTest extends PHPUnit_Framework_TestCase {
 			$this->assertEquals( false, $cache->get( "key" . $i - 10 ) );
 		}
 	}
+
+	public function testExpire() {
+		$cache = new HashBagOStuff();
+		$cacheInternal = TestingAccessWrapper::newFromObject( $cache );
+		$cache->set( 'foo', 1 );
+		$cache->set( 'bar', 1, 10 );
+		$cache->set( 'baz', 1, -10 );
+
+		$this->assertEquals( 0, $cacheInternal->bag['foo'][$cache::KEY_EXP], 'Indefinite' );
+		// 2 seconds tolerance
+		$this->assertEquals( time() + 10, $cacheInternal->bag['bar'][$cache::KEY_EXP], 'Future', 2 );
+		$this->assertEquals( time() - 10, $cacheInternal->bag['baz'][$cache::KEY_EXP], 'Past', 2 );
+
+		$this->assertEquals( 1, $cache->get( 'bar' ), 'Key not expired' );
+		$this->assertEquals( false, $cache->get( 'baz' ), 'Key expired' );
+	}
+
+	public function testKeyOrder() {
+		$cache = new HashBagOStuff( array( 'maxKeys' => 3 ) );
+
+		foreach ( array( 'foo', 'bar', 'baz' ) as $key ) {
+			$cache->set( $key, 1 );
+		}
+
+		// Set existing key
+		$cache->set( 'foo', 1 );
+
+		// Add a 4th key (beyond the allowed maximum)
+		$cache->set( 'quux', 1 );
+
+		// Foo's life should have been extended over Bar
+		foreach ( array( 'foo', 'baz', 'quux' ) as $key ) {
+			$this->assertEquals( 1, $cache->get( $key ), "Kept $key" );
+		}
+		$this->assertEquals( false, $cache->get( 'bar' ), 'Evicted bar' );
+	}
 }
-- 
2.20.1