From b52fbcbe8c79cf7f5e3a7ea25850e9f41f3661f3 Mon Sep 17 00:00:00 2001
From: Matthew Wild <mwild1@gmail.com>
Date: Tue, 22 Dec 2015 20:10:07 +0000
Subject: [PATCH] util.cache (and tests): Call on_evict after insertion of the
 new key, so inside on_evict we can be more certain about the current state of
 the cache (i.e. full, new item added, old item removed)

---
 tests/test_util_cache.lua | 55 +++++++++++++++++++++++++++++++++++++++
 util/cache.lua            | 11 ++++----
 2 files changed, 61 insertions(+), 5 deletions(-)

diff --git a/tests/test_util_cache.lua b/tests/test_util_cache.lua
index a174eea7..72cb5a85 100644
--- a/tests/test_util_cache.lua
+++ b/tests/test_util_cache.lua
@@ -170,5 +170,60 @@ function new(new)
 	end
 	assert_equal(i, 4);
 	
+	local evicted_key, evicted_value;
+	local c = new(3, function (_key, _value)
+		evicted_key, evicted_value = _key, _value;
+	end);
+	local function set(k, v, should_evict_key, should_evict_value)
+		evicted_key, evicted_value = nil, nil;
+		c:set(k, v);
+		assert_equal(evicted_key, should_evict_key);
+		assert_equal(evicted_value, should_evict_value);
+	end
+	set("a", 1)
+	set("a", 1)
+	set("a", 1)
+	set("a", 1)
+	set("a", 1)
+
+	set("b", 2)
+	set("c", 3)
+	set("b", 2)
+	set("d", 4, "a", 1)
+	set("e", 5, "c", 3)
+	
+
+	local evicted_key, evicted_value;
+	local c3 = new(1, function (_key, _value, c3)
+		evicted_key, evicted_value = _key, _value;
+		if _key == "a" then
+			-- Put it back in...
+			-- Check that the newest key/value was set before on_evict was called
+			assert_equal(c3:get("b"), 2);
+			-- Sanity check for what we're evicting
+			assert_equal(_key, "a");
+			assert_equal(_value, 1);
+			-- Re-insert the evicted key (causes this evict function to run again with "b",2)
+			c3:set(_key, _value)
+			assert_equal(c3:get(_key), _value)
+		end
+	end);
+	local function set(k, v, should_evict_key, should_evict_value)
+		evicted_key, evicted_value = nil, nil;
+		c3:set(k, v);
+		assert_equal(evicted_key, should_evict_key);
+		assert_equal(evicted_value, should_evict_value);
+	end
+	set("a", 1)
+	set("a", 1)
+	set("a", 1)
+	set("a", 1)
+	set("a", 1)
 
+	-- The evict handler re-inserts "a"->1, so "b" gets evicted:
+	set("b", 2, "b", 2)
+	-- Check the final state is what we expect
+	assert_equal(c3:get("a"), 1);
+	assert_equal(c3:get("b"), nil);
+	assert_equal(c3:count(), 1);
 end
diff --git a/util/cache.lua b/util/cache.lua
index 72b74351..d3639b3f 100644
--- a/util/cache.lua
+++ b/util/cache.lua
@@ -51,19 +51,20 @@ function cache_methods:set(k, v)
 		return true;
 	end
 	-- Check whether we need to remove oldest k/v
+	local on_evict, evicted_key, evicted_value;
 	if self._count == self.size then
 		local tail = self._tail;
-		local on_evict = self._on_evict;
-		if on_evict then
-			on_evict(tail.key, tail.value);
-		end
+		on_evict, evicted_key, evicted_value = self._on_evict, tail.key, tail.value;
 		_remove(self, tail);
-		self._data[tail.key] = nil;
+		self._data[evicted_key] = nil;
 	end
 
 	m = { key = k, value = v, prev = nil, next = nil };
 	self._data[k] = m;
 	_insert(self, m);
+	if on_evict and evicted_key then
+		on_evict(evicted_key, evicted_value, self);
+	end
 	return true;
 end
 
-- 
2.30.2