Fix cleaner to not get stuck, finish writing test code

cache.go

@@ -18,8 +18,8 @@ func timeUntil(t time.Time) time.Duration { return t.Sub(timeNow()) }
 // actions are completed in the cleaner goroutine to be concurrency safe.
 type Cache[K comparable, V any] struct {
 	items    sync.Map
-	chain    *keyed[K]                // linked list of to-expire keys
+	chain    *keyed[K] // linked list of to-expire keys
-	sched    *rescheduler.Rescheduler // TODO: add a mode to have the goroutine running only once after multiple Run calls
+	sched    *rescheduler.Rescheduler
 	close    chan struct{}
 	chainAdd chan keyed[K]
 	chainDel chan K
@@ -48,7 +48,12 @@ func (c item[T]) HasExpired() bool {
 
 // New creates a *Cache[K, V] ready to be used by the caller
 func New[K comparable, V any]() *Cache[K, V] {
-	c := &Cache[K, V]{}
+	c := &Cache[K, V]{
+		items:    sync.Map{},
+		close:    make(chan struct{}, 1),
+		chainAdd: make(chan keyed[K], 1),
+		chainDel: make(chan K, 1),
+	}
 	c.sched = rescheduler.NewRescheduler(c.cleaner)
 	return c
 }
@@ -67,14 +72,26 @@ func (c *Cache[K, V]) Close() {
 // The cleaner is stopped whenever the chain is empty due to there being no chain
 // to manage.
 func (c *Cache[K, V]) cleaner() {
-	var t *time.Timer
+	// cleaner is always called from Set or Delete methods with a value sent on chainAdd or chainDel
-	if c.chain == nil {
+	select {
-		// fake timer as this select isn't used for an empty chain
+	case node := <-c.chainAdd:
-		t = &time.Timer{C: make(chan time.Time)}
+		c.chainInsert(node)
-	} else {
+	case key := <-c.chainDel:
-		t = time.NewTimer(timeUntil(c.chain.expires))
+		c.chainSplice(key)
+	default:
+		// skip if chainAdd or chainDel isn't ready
+
+		println("Skip first select")
 	}
 
+	// at this point if the chain is empty then exit
+	if c.chain == nil {
+		return
+	}
+
+	// create a timer for the next expiry
+	t := time.NewTimer(timeUntil(c.chain.expires))
+
 	for {
 		select {
 		case <-c.close:
@@ -87,21 +104,12 @@ func (c *Cache[K, V]) cleaner() {
 			}
 			// the chain will not be empty after this insert so no check is required
 			c.chainInsert(node)
-
-			t.Reset(timeUntil(c.chain.expires))
 		case key := <-c.chainDel:
 			// stop the timer safely
 			if !t.Stop() {
 				<-t.C
 			}
 			c.chainSplice(key)
-
-			// if there is no chain then kill the expiry scheduler
-			if c.chain == nil {
-				return
-			}
-
-			t.Reset(timeUntil(c.chain.expires))
 		case <-t.C:
 			// if there is no chain then kill the expiry scheduler
 			if c.chain == nil {
@@ -109,18 +117,18 @@ func (c *Cache[K, V]) cleaner() {
 			}
 
 			// remove all expired entries
-			for c.chain.HasExpired() {
+			for c.chain != nil && c.chain.HasExpired() {
 				c.items.Delete(c.chain.data)
 				c.chain = c.chain.next
 			}
-
-			// if there is no chain then kill the expiry scheduler
-			if c.chain == nil {
-				return
-			}
-
-			t.Reset(timeUntil(c.chain.expires))
 		}
+
+		// if there is no chain then kill the expiry scheduler
+		if c.chain == nil {
+			return
+		}
+
+		t.Reset(timeUntil(c.chain.expires))
 	}
 }
 
@@ -151,6 +159,11 @@ func (c *Cache[K, V]) chainInsert(node keyed[K]) {
 }
 
 func (c *Cache[K, V]) chainSplice(key K) {
+	// quick path if chain is empty
+	if c.chain == nil {
+		return
+	}
+
 	// quick path if the first node matches
 	if c.chain.data == key {
 		node := c.chain
@@ -208,6 +221,13 @@ func (c *Cache[K, V]) Get(key K) (V, bool) {
 //
 // If an item is added with the same key then this item with be overwritten.
 func (c *Cache[K, V]) SetPermanent(key K, value V) {
+	// if the cache is closed then just return
+	select {
+	case <-c.close:
+		return
+	default:
+	}
+
 	i := &item[V]{data: value} // expires is not set here
 	c.items.Store(key, i)
 }
@@ -216,6 +236,13 @@ func (c *Cache[K, V]) SetPermanent(key K, value V) {
 //
 // If an item is added with the same key then this item with be overwritten.
 func (c *Cache[K, V]) Set(key K, value V, expires time.Time) {
+	// if the cache is closed then just return
+	select {
+	case <-c.close:
+		return
+	default:
+	}
+
 	if expires.Before(timeNow()) {
 		return
 	}
@@ -226,6 +253,20 @@ func (c *Cache[K, V]) Set(key K, value V, expires time.Time) {
 	c.sched.Run()
 }
 
+// Delete removes an item from the cache.
+func (c *Cache[K, V]) Delete(key K) {
+	// if the cache is closed then just return
+	select {
+	case <-c.close:
+		return
+	default:
+	}
+
+	c.items.Delete(key)
+	c.chainDel <- key
+	c.sched.Run()
+}
+
 // Range calls f with every key-value pair, which has not expired, currently
 // stored in the map.
 //
@@ -239,6 +280,6 @@ func (c *Cache[K, V]) Range(f func(key K, value V) bool) {
 			return true
 		}
 
-		return f(key.(K), value.(V))
+		return f(key.(K), item.data)
 	})
 }

cache_test.go

@@ -6,11 +6,129 @@ import (
 	"time"
 )
 
+var nextYear = time.Now().Year() + 1
+
 func TestItem_HasExpired(t *testing.T) {
 	n := time.Now()
+
+	// date before now is expired
 	a := item[string]{expires: n}
 	timeNow = func() time.Time { return n.Add(time.Second) }
 	assert.True(t, a.HasExpired())
+
+	// date after now is valid
 	a = item[string]{expires: n.Add(time.Second * 2)}
 	assert.False(t, a.HasExpired())
+
+	// empty date is always valid
+	a = item[string]{}
+	assert.False(t, a.HasExpired())
+}
+
+func TestCache_GetExpires(t *testing.T) {
+	c := New[string, string]()
+	c.items.Store("a", &item[string]{
+		data:    "b",
+		expires: time.Date(nextYear, time.January, 1, 0, 0, 0, 0, time.UTC),
+	})
+	v, exp, found := c.GetExpires("a")
+	assert.Equal(t, "b", v)
+	assert.Equal(t, time.Date(nextYear, time.January, 1, 0, 0, 0, 0, time.UTC), exp)
+	assert.True(t, found)
+
+	v, exp, found = c.GetExpires("b")
+	assert.Equal(t, "", v)
+	assert.Equal(t, time.Time{}, exp)
+	assert.False(t, found)
+}
+
+func TestCache_Get(t *testing.T) {
+	c := New[string, string]()
+	c.items.Store("a", &item[string]{
+		data:    "b",
+		expires: time.Date(nextYear, time.January, 1, 0, 0, 0, 0, time.UTC),
+	})
+	v, found := c.Get("a")
+	assert.Equal(t, "b", v)
+	assert.True(t, found)
+
+	v, found = c.Get("b")
+	assert.Equal(t, "", v)
+	assert.False(t, found)
+}
+
+func TestCache_SetPermanent(t *testing.T) {
+	c := New[string, string]()
+	c.SetPermanent("a", "b")
+
+	value, ok := c.items.Load("a")
+	v := value.(*item[string])
+	assert.Equal(t, item[string]{data: "b"}, *v)
+	assert.True(t, ok)
+
+	value, ok = c.items.Load("b")
+	assert.False(t, ok)
+}
+
+func TestCache_Set(t *testing.T) {
+	c := New[string, string]()
+	c.Set("a", "b", time.Date(nextYear, time.January, 1, 0, 0, 0, 0, time.UTC))
+
+	value, ok := c.items.Load("a")
+	v := value.(*item[string])
+	assert.Equal(t, item[string]{
+		data:    "b",
+		expires: time.Date(nextYear, time.January, 1, 0, 0, 0, 0, time.UTC),
+	}, *v)
+	assert.True(t, ok)
+
+	value, ok = c.items.Load("b")
+	assert.False(t, ok)
+}
+
+func TestCache_Delete(t *testing.T) {
+	c := New[string, string]()
+	c.items.Store("a", &item[string]{data: "b", expires: time.Date(nextYear, time.January, 1, 0, 0, 0, 0, time.UTC)})
+	c.chain = &keyed[string]{item: item[string]{data: "a", expires: time.Date(nextYear, time.January, 1, 0, 0, 0, 0, time.UTC)}}
+	c.Delete("a")
+
+	// scheduler should finish after deleting the item
+	c.sched.Wait()
+	assert.Nil(t, c.chain)
+}
+
+func TestCache_Range(t *testing.T) {
+	c := New[string, string]()
+	c.items.Store("a", &item[string]{data: "b"})
+	c.items.Store("b", &item[string]{data: "c"})
+	c.items.Store("c", &item[string]{data: "d"})
+	c.items.Store("d", &item[string]{data: "e"})
+	c.Range(func(key string, value string) bool {
+		assert.Equal(t, string([]byte{[]byte(key)[0] + 1}), value)
+		return true
+	})
+}
+
+func TestCache_Cleaner(t *testing.T) {
+	timeNow = func() time.Time { return time.Now() }
+
+	n := time.Now().Add(2 * time.Second)
+	c := New[string, string]()
+	c.Set("a", "b", n)
+
+	// check before expiry
+	time.Sleep(time.Second)
+	get, b := c.Get("a")
+	assert.True(t, b)
+	assert.Equal(t, "b", get)
+
+	// check after expiry
+	time.Sleep(1001 * time.Millisecond)
+	get, b = c.Get("a")
+	assert.False(t, b)
+	assert.Equal(t, "", get)
+
+	// scheduler should finish after the chain is empty
+	c.sched.Wait()
+	assert.Nil(t, c.chain)
 }

go.mod

@@ -3,7 +3,7 @@ module github.com/1f349/cache
 go 1.21.1
 
 require (
-	github.com/MrMelon54/rescheduler v0.0.1
+	github.com/MrMelon54/rescheduler v0.0.2
 	github.com/stretchr/testify v1.8.4
 )
 

go.sum

@@ -1,5 +1,5 @@
-github.com/MrMelon54/rescheduler v0.0.1 h1:gzNvL8X81M00uYN0i9clFVrXCkG1UuLNYxDcvjKyBqo=
+github.com/MrMelon54/rescheduler v0.0.2 h1:efrRwr0BYlkaXFucZDjQqRyIawZiMEAnzjea46Bs9Oc=
-github.com/MrMelon54/rescheduler v0.0.1/go.mod h1:OQDFtZHdS4/qA/r7rtJUQA22/hbpnZ9MGQCXOPjhC6w=
+github.com/MrMelon54/rescheduler v0.0.2/go.mod h1:OQDFtZHdS4/qA/r7rtJUQA22/hbpnZ9MGQCXOPjhC6w=
 github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
 github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
 github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=