From 38560f59f44a2124cd203f5ff4a8bf67ebefef10 Mon Sep 17 00:00:00 2001 From: Jaime Martinez Date: Tue, 13 Oct 2020 11:33:45 +1100 Subject: [PATCH 1/2] Delete expired items before setting Check if an item exists and it has expired before setting so that onEvicted is actually called. Fixes #48. Add test and split func Use nanoseconds instead Call onEvicted if item existed before setting Call onEvicted if item.Expired --- cache.go | 13 +++++++++++++ cache_test.go | 33 +++++++++++++++++++++++++++++++++ 2 files changed, 46 insertions(+) diff --git a/cache.go b/cache.go index db88d2f..43f4e58 100644 --- a/cache.go +++ b/cache.go @@ -57,7 +57,15 @@ func (c *cache) Set(k string, x interface{}, d time.Duration) { if d > 0 { e = time.Now().Add(d).UnixNano() } + + var item Item + var evicted bool + c.mu.Lock() + if c.onEvicted != nil { + item, evicted = c.items[k] + } + c.items[k] = Item{ Object: x, Expiration: e, @@ -65,6 +73,11 @@ func (c *cache) Set(k string, x interface{}, d time.Duration) { // TODO: Calls to mu.Unlock are currently not deferred because defer // adds ~200 ns (as of go1.) c.mu.Unlock() + + // try to call onEvicted if key existed before but it was expired before cleanup + if evicted && item.Expired() { + c.onEvicted(k, item.Object) + } } func (c *cache) set(k string, x interface{}, d time.Duration) { diff --git a/cache_test.go b/cache_test.go index de3e9d6..af7fff2 100644 --- a/cache_test.go +++ b/cache_test.go @@ -1247,6 +1247,39 @@ func TestOnEvicted(t *testing.T) { } } +func TestOnEvictedCalledBeforeSet(t *testing.T) { + tc := New(DefaultExpiration, 0) + expiry := 1 * time.Nanosecond + + works := false + tc.OnEvicted(func(k string, v interface{}) { + if k == "foo" && v.(int) == 3 { + + works = true + } + tc.Set("bar", 4, DefaultExpiration) + }) + + tc.Set("foo", 3, expiry) + if tc.onEvicted == nil { + t.Fatal("tc.onEvicted is nil") + } + + // ensure item expires + time.Sleep(expiry) + + // calling Set again should evict expired item + tc.Set("foo", 3, DefaultExpiration) + + x, _ := tc.Get("bar") + if !works { + t.Fatal("works bool not true") + } + if x.(int) != 4 { + t.Error("bar was not 4") + } +} + func TestCacheSerialization(t *testing.T) { tc := New(DefaultExpiration, 0) testFillAndSerialize(t, tc) From 9b97ae73e4c3734949a57a32b3edb1931ae8ad78 Mon Sep 17 00:00:00 2001 From: Jaime Martinez Date: Fri, 16 Oct 2020 16:55:06 +1100 Subject: [PATCH 2/2] Fix eviction check --- cache.go | 4 ++-- cache_test.go | 8 +++----- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/cache.go b/cache.go index 43f4e58..750f95e 100644 --- a/cache.go +++ b/cache.go @@ -74,8 +74,8 @@ func (c *cache) Set(k string, x interface{}, d time.Duration) { // adds ~200 ns (as of go1.) c.mu.Unlock() - // try to call onEvicted if key existed before but it was expired before cleanup - if evicted && item.Expired() { + // try to call onEvicted if key existed before but the item is different + if evicted && item.Object != x { c.onEvicted(k, item.Object) } } diff --git a/cache_test.go b/cache_test.go index af7fff2..5dc61f9 100644 --- a/cache_test.go +++ b/cache_test.go @@ -1265,11 +1265,9 @@ func TestOnEvictedCalledBeforeSet(t *testing.T) { t.Fatal("tc.onEvicted is nil") } - // ensure item expires - time.Sleep(expiry) - - // calling Set again should evict expired item - tc.Set("foo", 3, DefaultExpiration) + // calling Set again with the same key should evict + // the item if different + tc.Set("foo", 5, DefaultExpiration) x, _ := tc.Get("bar") if !works {