update roles cache to use sync.cache

add tests for roles cache
add changelog
use strings in sync cache tests

changelog/unreleased/roles-cache.md

@@ -0,0 +1,7 @@
+Enhancement: use sync.cache for roles cache
+
+Tags: ocis-pkg
+
+update ocis-pkg/roles cache to use ocis-pkg/sync cache
+
+https://github.com/owncloud/ocis/pull/1367

ocis-pkg/roles/cache.go

@@ -1,71 +1,35 @@
 package roles
 
 import (
-	"sync"
 	"time"
-
+	"github.com/owncloud/ocis/ocis-pkg/sync"
 	settings "github.com/owncloud/ocis/settings/pkg/proto/v0"
 )
 
-// entry extends a bundle and adds a TTL
-type entry struct {
-	*settings.Bundle
-	inserted time.Time
-}
-
 // cache is a cache implementation for roles, keyed by roleIDs.
 type cache struct {
-	entries map[string]entry
+	sc   sync.Cache
-	size    int
+	ttl  time.Duration
-	ttl     time.Duration
-	m       sync.Mutex
 }
 
 // newCache returns a new instance of Cache.
-func newCache(size int, ttl time.Duration) cache {
+func newCache(capacity int, ttl time.Duration) cache {
 	return cache{
-		size:    size,
 		ttl:     ttl,
-		entries: map[string]entry{},
+		sc: sync.NewCache(capacity),
 	}
 }
 
 // get gets a role-bundle by a given `roleID`.
 func (c *cache) get(roleID string) *settings.Bundle {
-	c.m.Lock()
+	if ce := c.sc.Load(roleID); ce != nil {
-	defer c.m.Unlock()
+		return ce.V.(*settings.Bundle)
-
-	if _, ok := c.entries[roleID]; ok {
-		return c.entries[roleID].Bundle
 	}
+
 	return nil
 }
 
 // set sets a roleID / role-bundle.
 func (c *cache) set(roleID string, value *settings.Bundle) {
-	c.m.Lock()
+	c.sc.Store(roleID, value, time.Now().Add(c.ttl))
-	defer c.m.Unlock()
+}
-
-	if !c.fits() {
-		c.evict()
-	}
-
-	c.entries[roleID] = entry{
-		value,
-		time.Now(),
-	}
-}
-
-// evict frees memory from the cache by removing entries that exceeded the cache TTL.
-func (c *cache) evict() {
-	for i := range c.entries {
-		if c.entries[i].inserted.Add(c.ttl).Before(time.Now()) {
-			delete(c.entries, i)
-		}
-	}
-}
-
-// fits returns whether the cache fits more entries.
-func (c *cache) fits() bool {
-	return c.size > len(c.entries)
-}

ocis-pkg/roles/cache_test.go

@@ -0,0 +1,54 @@
+package roles
+
+import (
+	settings "github.com/owncloud/ocis/settings/pkg/proto/v0"
+	"github.com/stretchr/testify/assert"
+	"strconv"
+	"sync"
+	"testing"
+	"time"
+)
+
+func cacheRunner(size int, ttl time.Duration) (*cache, func(f func(v string))) {
+	c := newCache(size, ttl)
+	run := func(f func(v string)) {
+		wg := sync.WaitGroup{}
+		for i := 0; i < size; i++ {
+			wg.Add(1)
+			go func(i int) {
+				f(strconv.Itoa(i))
+				wg.Done()
+			}(i)
+		}
+		wg.Wait()
+	}
+
+	return &c, run
+}
+
+func BenchmarkCache(b *testing.B) {
+	b.ReportAllocs()
+	size := 1024
+	c, cr := cacheRunner(size, 100 * time.Millisecond)
+
+	cr(func(v string) { c.set(v, &settings.Bundle{})})
+	cr(func(v string) { c.get(v)})
+}
+
+func TestCache(t *testing.T) {
+	size := 1024
+	ttl := 100 * time.Millisecond
+	c, cr := cacheRunner(size, ttl)
+
+	cr(func(v string) {
+		c.set(v, &settings.Bundle{Id: v})
+	})
+
+	assert.Equal(t, "50", c.get("50").Id, "it returns the right bundle")
+	assert.Nil(t, c.get("unknown"), "unknown bundle ist nil")
+
+	time.Sleep(ttl + 1)
+	// roles cache has no access to evict, adding new items triggers a cleanup
+	c.set("evict", nil)
+	assert.Nil(t, c.get("50"), "old bundles get removed")
+}

ocis-pkg/sync/cache_test.go

@@ -8,16 +8,16 @@ import (
 	"time"
 )
 
-func cacheRunner(size int) (*Cache, func(f func(i int))) {
+func cacheRunner(size int) (*Cache, func(f func(v string))) {
 	c := NewCache(size)
-	run := func(f func(i int)) {
+	run := func(f func(v string)) {
 		wg := sync.WaitGroup{}
 		for i := 0; i < size; i++ {
 			wg.Add(1)
-			go func(i int) {
+			go func(v string) {
-				f(i)
+				f(v)
 				wg.Done()
-			}(i)
+			}(strconv.Itoa(i))
 		}
 		wg.Wait()
 	}
@@ -30,21 +30,21 @@ func BenchmarkCache(b *testing.B) {
 	size := 1024
 	c, cr := cacheRunner(size)
 
-	cr(func(i int) { c.Store(strconv.Itoa(i), i, time.Now().Add(100*time.Millisecond)) })
+	cr(func(v string) { c.Store(v, v, time.Now().Add(100*time.Millisecond)) })
-	cr(func(i int) { c.Delete(strconv.Itoa(i)) })
+	cr(func(v string) { c.Delete(v) })
 }
 
 func TestCache(t *testing.T) {
 	size := 1024
 	c, cr := cacheRunner(size)
 
-	cr(func(i int) { c.Store(strconv.Itoa(i), i, time.Now().Add(100*time.Millisecond)) })
+	cr(func(v string) { c.Store(v, v, time.Now().Add(100*time.Millisecond)) })
 	assert.Equal(t, size, int(c.length), "length is atomic")
 
-	cr(func(i int) { c.Delete(strconv.Itoa(i)) })
+	cr(func(v string) { c.Delete(v) })
 	assert.Equal(t, 0, int(c.length), "delete is atomic")
 
-	cr(func(i int) {
+	cr(func(v string) {
 		time.Sleep(101 * time.Millisecond)
 		c.evict()
 	})
@@ -55,50 +55,50 @@ func TestCache_Load(t *testing.T) {
 	size := 1024
 	c, cr := cacheRunner(size)
 
-	cr(func(i int) {
+	cr(func(v string) {
-		c.Store(strconv.Itoa(i), i, time.Now().Add(10*time.Second))
+		c.Store(v, v, time.Now().Add(10*time.Second))
 	})
 
-	cr(func(i int) {
+	cr(func(v string) {
-		assert.Equal(t, i, c.Load(strconv.Itoa(i)).V, "entry value is the same")
+		assert.Equal(t, v, c.Load(v).V, "entry value is the same")
 	})
 
-	cr(func(i int) {
+	cr(func(v string) {
-		assert.Nil(t, c.Load(strconv.Itoa(i+size)), "entry is nil if unknown")
+		assert.Nil(t, c.Load(v+strconv.Itoa(size)), "entry is nil if unknown")
 	})
 
-	cr(func(i int) {
+	cr(func(v string) {
 		wait := 100 * time.Millisecond
-		c.Store(strconv.Itoa(i), i, time.Now().Add(wait))
+		c.Store(v, v, time.Now().Add(wait))
 		time.Sleep(wait + 1)
-		assert.Nil(t, c.Load(strconv.Itoa(i)), "entry is nil if it's expired")
+		assert.Nil(t, c.Load(v), "entry is nil if it's expired")
 	})
 }
 
 func TestCache_Store(t *testing.T) {
 	c, cr := cacheRunner(1024)
 
-	cr(func(i int) {
+	cr(func(v string) {
-		c.Store(strconv.Itoa(i), i, time.Now().Add(100*time.Millisecond))
+		c.Store(v, v, time.Now().Add(100*time.Millisecond))
-		assert.Equal(t, i, c.Load(strconv.Itoa(i)).V, "new entries can be added")
+		assert.Equal(t, v, c.Load(v).V, "new entries can be added")
 	})
 
-	cr(func(i int) {
+	cr(func(v string) {
 		replacedExpiration := time.Now().Add(10 * time.Minute)
-		c.Store(strconv.Itoa(i), "old", time.Now().Add(10*time.Minute))
+		c.Store(v, "old", time.Now().Add(10*time.Minute))
-		c.Store(strconv.Itoa(i), "updated", replacedExpiration)
+		c.Store(v, "updated", replacedExpiration)
-		assert.Equal(t, "updated", c.Load(strconv.Itoa(i)).V, "entry values can be updated")
+		assert.Equal(t, "updated", c.Load(v).V, "entry values can be updated")
-		assert.Equal(t, replacedExpiration, c.Load(strconv.Itoa(i)).expiration, "entry expiration can be updated")
+		assert.Equal(t, replacedExpiration, c.Load(v).expiration, "entry expiration can be updated")
 	})
 }
 
 func TestCache_Delete(t *testing.T) {
 	c, cr := cacheRunner(1024)
 
-	cr(func(i int) {
+	cr(func(v string) {
-		c.Store(strconv.Itoa(i), i, time.Now().Add(100*time.Millisecond))
+		c.Store(v, v, time.Now().Add(100*time.Millisecond))
-		c.Delete(strconv.Itoa(i))
+		c.Delete(v)
-		assert.Nil(t, c.Load(strconv.Itoa(i)), "entries can be deleted")
+		assert.Nil(t, c.Load(v), "entries can be deleted")
 	})
 
 	assert.Equal(t, 0, int(c.length), "removing a entry decreases the cache size")