Don't create a timed flusher goroutine or dirtyList if there is no flusher

This is an unnecessary goroutine for uses such as the apns module in
uniqush-push.

Additionally, dirtyList was never emptied if the flusher was nil, so
this was a memory leak.

Also, update README

Author: Tyson Andre

README.md

@@ -1,11 +1,15 @@
-cache
-=====
+cache2
+======
 
 A cache implementation in Go
 
 This is used in [uniqush-push]
 
+Because some installations of uniqush-push predate versioned dependencies,
+this was created as a repo separate from [uniqush/cache],
+because of the inconvenience caused by interface incompatibilities.
+
 [uniqush-push]: http://uniqush.org
+[uniqush/cache]: https://github.com/uniqush/cache
 
 License: Apache-2.0
-

cache.go

@@ -15,7 +15,7 @@
  *
  */
 
-package cache
+package cache2
 
 import (
 	"container/list"
@@ -41,6 +41,24 @@ type cacheItem struct {
 	value interface{}
 }
 
+type CacheInterface interface {
+	Len() int
+	Set(key string, value interface{})
+	Get(key string) interface{}
+	Delete(key string) interface{}
+	Flush()
+	debug()
+}
+
+type SimpleCache struct {
+	mu       sync.Mutex
+	data     map[string]*list.Element
+	list     *list.List
+	capacity int
+}
+
+var _ CacheInterface = &SimpleCache{}
+
 type Cache struct {
 	mu          sync.Mutex
 	flushPeriod time.Duration
@@ -52,16 +70,21 @@ type Cache struct {
 	maxNrDirty  int
 }
 
+var _ CacheInterface = &Cache{}
+
+func (c *SimpleCache) Len() int {
+	return len(c.data)
+}
+
 func (c *Cache) Len() int {
 	return len(c.data)
 }
 
+func (c *SimpleCache) Flush() {}
+
 func (c *Cache) Flush() {
 	c.mu.Lock()
 	defer c.mu.Unlock()
-	if c.flusher == nil {
-		return
-	}
 	for e := c.dirtyList.Front(); e != nil; e = e.Next() {
 		if de, ok := e.Value.(*dirtyElement); ok {
 			if de.removed {
@@ -74,6 +97,16 @@ func (c *Cache) Flush() {
 	c.dirtyList = list.New()
 }
 
+func (c *SimpleCache) debug() {
+	fmt.Printf("nr elems %v <= %v", c.list.Len(), c.capacity)
+	fmt.Println("-----------------elements------------")
+	for e := c.list.Front(); e != nil; e = e.Next() {
+		item := e.Value.(*cacheItem)
+		fmt.Printf("%v: %v\n", item.key, item.value)
+	}
+	fmt.Println("-------------------------------------")
+}
+
 func (c *Cache) debug() {
 	fmt.Printf("nr elems %v <= %v, nr dirty elems %v < %v\n",
 		c.list.Len(), c.capacity, c.dirtyList.Len(), c.maxNrDirty)
@@ -112,15 +145,19 @@ func (c *Cache) checkAndFlush() {
 // flushPeriod = 0 second means no periodically flush;
 // undefined in range (0, 1).
 func New(capacity int, maxNrDirty int, flushPeriod time.Duration, flusher Flusher) *Cache {
+	initialCapacity := capacity
+	if initialCapacity < 0 {
+		initialCapacity = 1024
+	}
+	if flusher == nil {
+		panic("Should use NewSimple")
+	}
+
 	cache := new(Cache)
 
 	cache.flushPeriod = flushPeriod
-	cache.capacity = capacity
-	if cache.capacity < 0 {
-		cache.data = make(map[string]*list.Element, 1024)
-	} else {
-		cache.data = make(map[string]*list.Element, cache.capacity)
-	}
+	cache.capacity = initialCapacity
+	cache.data = make(map[string]*list.Element, initialCapacity)
 	cache.list = list.New()
 	cache.dirtyList = list.New()
 	cache.flusher = flusher
@@ -137,6 +174,29 @@ func New(capacity int, maxNrDirty int, flushPeriod time.Duration, flusher Flushe
 	return cache
 }
 
+func NewSimple(capacity int) *SimpleCache {
+	initialCapacity := capacity
+	if initialCapacity < 0 {
+		initialCapacity = 1024
+	}
+	return &SimpleCache{
+		data:     make(map[string]*list.Element, initialCapacity),
+		list:     list.New(),
+		capacity: initialCapacity,
+	}
+}
+
+func (c *SimpleCache) Get(key string) interface{} {
+	c.mu.Lock()
+	defer c.mu.Unlock()
+	if elem, ok := c.data[key]; ok {
+		item := elem.Value.(*cacheItem)
+		c.list.MoveToFront(elem)
+		return item.value
+	}
+	return nil
+}
+
 func (c *Cache) Get(key string) interface{} {
 	c.mu.Lock()
 	defer c.mu.Unlock()
@@ -148,6 +208,27 @@ func (c *Cache) Get(key string) interface{} {
 	return nil
 }
 
+func (c *SimpleCache) Set(key string, value interface{}) {
+	c.mu.Lock()
+	defer c.mu.Unlock()
+
+	if e, ok := c.data[key]; ok {
+		item := e.Value.(*cacheItem)
+		item.value = value
+		c.list.MoveToFront(e)
+	} else {
+		elem := c.list.PushFront(&cacheItem{key: key, value: value})
+		c.data[key] = elem
+		if c.capacity >= 0 && len(c.data) > c.capacity {
+			last := c.list.Back()
+			item := last.Value.(*cacheItem)
+			c.list.Remove(last)
+			delete(c.data, item.key)
+		}
+	}
+	return
+}
+
 func (c *Cache) Set(key string, value interface{}) {
 	c.mu.Lock()
 	defer c.checkAndFlush()
@@ -179,6 +260,18 @@ func (c *Cache) Set(key string, value interface{}) {
 	return
 }
 
+func (c *SimpleCache) Delete(key string) interface{} {
+	c.mu.Lock()
+	defer c.mu.Unlock()
+
+	if elem, ok := c.data[key]; ok {
+		delete(c.data, key)
+		item := elem.Value.(*cacheItem)
+		return item.value
+	}
+	return nil
+}
+
 func (c *Cache) Delete(key string) interface{} {
 	c.mu.Lock()
 	defer c.mu.Unlock()

cache_test.go

@@ -15,9 +15,10 @@
  *
  */
 
-package cache
+package cache2
 
 import (
+	"sync"
 	"testing"
 	"time"
 )
@@ -29,7 +30,7 @@ func TestInsertValues(t *testing.T) {
 	kv["key3"] = "3"
 	kv["key4"] = "4"
 
-	c := New(5, 5, 0*time.Second, nil)
+	c := NewSimple(5)
 
 	for k, v := range kv {
 		c.Set(k, v)
@@ -58,7 +59,7 @@ func TestExceedCapacityValues(t *testing.T) {
 
 	keys := []string{"key1", "key2", "key3", "key4"}
 
-	c := New(3, 5, 0*time.Second, nil)
+	c := NewSimple(3)
 
 	for _, k := range keys {
 		c.Set(k, kv[k])
@@ -78,7 +79,7 @@ func TestUpdateValue(t *testing.T) {
 	kv["key3"] = "3"
 	kv["key4"] = "4"
 
-	c := New(5, 5, 0*time.Second, nil)
+	c := NewSimple(5)
 
 	for k, v := range kv {
 		c.Set(k, v)
@@ -98,7 +99,7 @@ func TestDeleteValue(t *testing.T) {
 	kv["key3"] = "3"
 	kv["key4"] = "4"
 
-	c := New(5, 5, 0*time.Second, nil)
+	c := NewSimple(5)
 
 	for k, v := range kv {
 		c.Set(k, v)
@@ -113,6 +114,7 @@ func TestDeleteValue(t *testing.T) {
 
 type memFlusher struct {
 	data map[string]interface{}
+	m    sync.Mutex
 }
 
 func newMemFlusher() *memFlusher {
@@ -122,13 +124,25 @@ func newMemFlusher() *memFlusher {
 }
 
 func (f *memFlusher) Add(key string, value interface{}) {
+	f.m.Lock()
+	defer f.m.Unlock()
 	f.data[key] = value
 }
 
 func (f *memFlusher) Remove(key string) {
+	f.m.Lock()
+	defer f.m.Unlock()
 	delete(f.data, key)
 }
 
+// threadSafeGet checks if Add() was the most recently called function for key on this flusher. Needed for `go test -race`
+func (f *memFlusher) threadSafeGet(key string) (interface{}, bool) {
+	f.m.Lock()
+	defer f.m.Unlock()
+	value, ok := f.data[key]
+	return value, ok
+}
+
 func TestFlushOnDirty(t *testing.T) {
 	kv := make(map[string]string)
 	kv["key1"] = "1"
@@ -146,7 +160,7 @@ func TestFlushOnDirty(t *testing.T) {
 	}
 
 	for _, k := range keys[:3] {
-		if _, ok := f.data[k]; !ok {
+		if _, ok := f.threadSafeGet(k); !ok {
 			t.Errorf("%v does not exist", k)
 		}
 	}
@@ -172,7 +186,7 @@ func TestFlushOnTimeOut(t *testing.T) {
 	time.Sleep(duration + 1*time.Second)
 
 	for _, k := range keys {
-		if _, ok := f.data[k]; !ok {
+		if _, ok := f.threadSafeGet(k); !ok {
 			t.Errorf("%v does not exist", k)
 		}
 	}
@@ -203,29 +217,78 @@ func TestEvictValue(t *testing.T) {
 
 }
 
+func expectCachedValueEquals(t *testing.T, c CacheInterface, k string, expectedValue string) {
+	value := c.Get(k)
+	if value != expectedValue {
+		t.Errorf("should be %v on key %v. Got %v",
+			expectedValue, k, value)
+	}
+}
+
 func TestAlwaysInMemoryCache(t *testing.T) {
 	kv := make(map[string]string)
 	kv["key1"] = "1"
 	kv["key2"] = "2"
 	kv["key3"] = "3"
 	kv["key4"] = "4"
 
-	c := New(-1, -1, 0*time.Second, nil)
+	c := NewSimple(-1)
 
 	for k, v := range kv {
 		c.Set(k, v)
 	}
 
 	for k, v := range kv {
-		value := c.Get(k).(string)
-		if value != v {
-			t.Errorf("should be %v on key %v. Got %v",
-				v, k, value)
-		}
+		expectCachedValueEquals(t, c, k, v)
 	}
 
 	value := c.Get("notexist")
 	if value != nil {
 		t.Errorf("Got nonexist")
 	}
 }
+
+func testEvictsOldValuesHelper(t *testing.T, flusher Flusher, timeout time.Duration) {
+	kv := make(map[string]string)
+	kv["key1"] = "1"
+	kv["key2"] = "2"
+	kv["key3"] = "3"
+	// keyList is used because go maps are unordered.
+	keyList := []string{"key1", "key2", "key3"}
+
+	var c CacheInterface
+	if flusher != nil {
+		c = New(3, 0, timeout, flusher)
+	} else {
+		c = NewSimple(3)
+	}
+
+	for _, k := range keyList {
+		c.Set(k, kv[k])
+	}
+
+	for _, k := range keyList {
+		expectCachedValueEquals(t, c, k, kv[k])
+	}
+
+	c.Set("key4", "4")
+	if v := c.Get("key1"); v != nil {
+		t.Errorf("Got %v for key1, expected least recently accessed value to be evicted", v)
+	}
+	expectCachedValueEquals(t, c, "key4", "4")
+	expectCachedValueEquals(t, c, "key3", "3")
+	expectCachedValueEquals(t, c, "key2", "2")
+	c.Set("key5", "5")
+	expectCachedValueEquals(t, c, "key5", "5")
+	if v := c.Get("key4"); v != nil {
+		t.Errorf("Got %v for key4, expected least recently accessed value to be evicted", v)
+	}
+}
+
+func TestSimpleCacheEvictsOldValues(t *testing.T) {
+	testEvictsOldValuesHelper(t, nil, 0*time.Second)
+}
+
+func TestFlushingCacheEvictsOldValues(t *testing.T) {
+	testEvictsOldValuesHelper(t, newMemFlusher(), 0*time.Second)
+}