Repository: hyperledger/firefly (go)

Issue:
Token pools are not removed from cache when deleted
This seems to be a result of #1261 and #1291 getting merged very close together. Token pools are not cleared from the cache when they are deleted, so cached calls may continue to return old pool details.

Pull request:
Remove token pools from the cache upon deletion
Fixes #1355 

Patch:
diff --git a/go.mod b/go.mod
index 9916f7608..97120ca08 100644
--- a/go.mod
+++ b/go.mod
@@ -17,7 +17,6 @@ require (
 	github.com/hyperledger/firefly-common v1.2.17
 	github.com/hyperledger/firefly-signer v1.1.8
 	github.com/jarcoal/httpmock v1.2.0
-	github.com/karlseguin/ccache v2.0.3+incompatible
 	github.com/lib/pq v1.10.7
 	github.com/mattn/go-sqlite3 v1.14.16
 	github.com/prometheus/client_golang v1.14.0
@@ -49,6 +48,7 @@ require (
 	github.com/inconshreveable/mousetrap v1.1.0 // indirect
 	github.com/invopop/yaml v0.2.0 // indirect
 	github.com/josharian/intern v1.0.0 // indirect
+	github.com/karlseguin/ccache v2.0.3+incompatible // indirect
 	github.com/lann/builder v0.0.0-20180802200727-47ae307949d0 // indirect
 	github.com/lann/ps v0.0.0-20150810152359-62de8c46ede0 // indirect
 	github.com/magiconair/properties v1.8.7 // indirect
diff --git a/internal/assets/token_pool.go b/internal/assets/token_pool.go
index f7bb1726a..8939bb621 100644
--- a/internal/assets/token_pool.go
+++ b/internal/assets/token_pool.go
@@ -211,6 +211,15 @@ func (am *assetManager) GetTokenPoolByNameOrID(ctx context.Context, poolNameOrID
 	return pool, nil
 }
 
+func (am *assetManager) removeTokenPoolFromCache(ctx context.Context, pool *core.TokenPool) {
+	cacheKeyName := fmt.Sprintf("ns=%s,poolnameorid=%s", am.namespace, pool.Name)
+	cacheKeyID := fmt.Sprintf("ns=%s,poolnameorid=%s", am.namespace, pool.ID)
+	cacheKeyLocator := fmt.Sprintf("ns=%s,connector=%s,poollocator=%s", am.namespace, pool.Connector, pool.Locator)
+	am.cache.Delete(cacheKeyName)
+	am.cache.Delete(cacheKeyID)
+	am.cache.Delete(cacheKeyLocator)
+}
+
 func (am *assetManager) GetTokenPoolByID(ctx context.Context, poolID *fftypes.UUID) (*core.TokenPool, error) {
 	return am.database.GetTokenPoolByID(ctx, am.namespace, poolID)
 }
@@ -240,6 +249,7 @@ func (am *assetManager) DeleteTokenPool(ctx context.Context, poolNameOrID string
 		if err != nil {
 			return err
 		}
+		am.removeTokenPoolFromCache(ctx, pool)
 		if err = am.database.DeleteTokenPool(ctx, am.namespace, pool.ID); err != nil {
 			return err
 		}
diff --git a/internal/cache/cache.go b/internal/cache/cache.go
index a32b8062d..ec676fddc 100644
--- a/internal/cache/cache.go
+++ b/internal/cache/cache.go
@@ -19,12 +19,11 @@ package cache
 import (
 	"context"
 	"strings"
-	"sync"
 	"time"
 
+	"github.com/hyperledger/firefly-common/pkg/cache"
 	"github.com/hyperledger/firefly-common/pkg/config"
 	"github.com/hyperledger/firefly-common/pkg/i18n"
-	"github.com/karlseguin/ccache"
 
 	"github.com/hyperledger/firefly/internal/coreconfig"
 	"github.com/hyperledger/firefly/internal/coremsgs"
@@ -55,7 +54,7 @@ func (cc *CConfig) UniqueName() (string, error) {
 	if err != nil {
 		return "", err
 	}
-	return cc.namespace + "::" + category, nil
+	return category, nil
 }
 
 func (cc *CConfig) Category() (string, error) {
@@ -101,90 +100,20 @@ func (cc *CConfig) TTL() time.Duration {
 type Manager interface {
 	GetCache(cc *CConfig) (CInterface, error)
 	ResetCachesForNamespace(ns string)
-	ListKeys() []string
+	ListCacheNames(namespace string) []string
 }
 
-type CInterface interface {
-	Get(key string) interface{}
-	Set(key string, val interface{})
-
-	GetString(key string) string
-	SetString(key string, val string)
-
-	GetInt(key string) int
-	SetInt(key string, val int)
-}
-
-type CCache struct {
-	enabled   bool
-	ctx       context.Context
-	namespace string
-	name      string
-	cache     *ccache.Cache
-	cacheTTL  time.Duration
-}
-
-func (c *CCache) Set(key string, val interface{}) {
-	if !c.enabled {
-		return
-	}
-	c.cache.Set(c.name+":"+key, val, c.cacheTTL)
-}
-func (c *CCache) Get(key string) interface{} {
-	if !c.enabled {
-		return nil
-	}
-	if cached := c.cache.Get(c.name + ":" + key); cached != nil {
-		cached.Extend(c.cacheTTL)
-		return cached.Value()
-	}
-	return nil
-}
-
-func (c *CCache) SetString(key string, val string) {
-	c.Set(key, val)
-}
-
-func (c *CCache) GetString(key string) string {
-	val := c.Get(key)
-	if val != nil {
-		return c.Get(key).(string)
-	}
-	return ""
-}
-
-func (c *CCache) SetInt(key string, val int) {
-	c.Set(key, val)
-}
-
-func (c *CCache) GetInt(key string) int {
-	val := c.Get(key)
-	if val != nil {
-		return c.Get(key).(int)
-	}
-	return 0
-}
+type CInterface cache.CInterface
 
 type cacheManager struct {
-	ctx     context.Context
-	enabled bool
-	m       sync.Mutex
-	// maintain a list of named configured CCache, the name are unique configuration category id
-	// e.g. cache.batch
-	configuredCaches map[string]*CCache
+	ffcache cache.Manager
 }
 
 func (cm *cacheManager) ResetCachesForNamespace(ns string) {
-	cm.m.Lock()
-	defer cm.m.Unlock()
-	for k, c := range cm.configuredCaches {
-		if c.namespace == ns {
-			// Clear the cache to free the memory immediately
-			c.cache.Clear()
-			// Remove it from the map, so the next call will generate a new one
-			delete(cm.configuredCaches, k)
-		}
-	}
+	cm.ffcache.ResetCaches(ns)
+}
+func (cm *cacheManager) ListCacheNames(namespace string) []string {
+	return cm.ffcache.ListCacheNames(namespace)
 }
 
 func (cm *cacheManager) GetCache(cc *CConfig) (CInterface, error) {
@@ -196,47 +125,24 @@ func (cm *cacheManager) GetCache(cc *CConfig) (CInterface, error) {
 	if err != nil {
 		return nil, err
 	}
-	cm.m.Lock()
-	cache, exists := cm.configuredCaches[cacheName]
-	if !exists {
-		cache = &CCache{
-			ctx:       cc.ctx,
-			namespace: cc.namespace,
-			name:      cacheName,
-			cache:     ccache.New(ccache.Configure().MaxSize(maxSize)),
-			cacheTTL:  cc.TTL(),
-			enabled:   cm.enabled,
-		}
-		cm.configuredCaches[cacheName] = cache
-	}
-	cm.m.Unlock()
-	return cache, nil
-}
 
-func (cm *cacheManager) ListKeys() []string {
-	keys := make([]string, 0, len(cm.configuredCaches))
-	for k := range cm.configuredCaches {
-		keys = append(keys, k)
-	}
-	return keys
+	return cm.ffcache.GetCache(
+		cc.ctx,
+		cc.namespace,
+		cacheName,
+		maxSize,
+		cc.TTL(),
+		cm.ffcache.IsEnabled(),
+	)
 }
-
 func NewCacheManager(ctx context.Context) Manager {
 	cm := &cacheManager{
-		ctx:              ctx,
-		enabled:          config.GetBool(coreconfig.CacheEnabled),
-		configuredCaches: map[string]*CCache{},
+		ffcache: cache.NewCacheManager(ctx, config.GetBool(coreconfig.CacheEnabled)),
 	}
 	return cm
 }
 
 // should only be used for testing purpose
 func NewUmanagedCache(ctx context.Context, sizeLimit int64, ttl time.Duration) CInterface {
-	return &CCache{
-		ctx:      ctx,
-		name:     "cache.unmanaged",
-		cache:    ccache.New(ccache.Configure().MaxSize(sizeLimit)),
-		cacheTTL: ttl,
-		enabled:  true,
-	}
+	return cache.NewUmanagedCache(ctx, sizeLimit, ttl)
 }
diff --git a/mocks/cachemocks/manager.go b/mocks/cachemocks/manager.go
index bdb0f8f2c..84592f054 100644
--- a/mocks/cachemocks/manager.go
+++ b/mocks/cachemocks/manager.go
@@ -38,13 +38,13 @@ func (_m *Manager) GetCache(cc *cache.CConfig) (cache.CInterface, error) {
 	return r0, r1
 }
 
-// ListKeys provides a mock function with given fields:
-func (_m *Manager) ListKeys() []string {
-	ret := _m.Called()
+// ListCacheNames provides a mock function with given fields: namespace
+func (_m *Manager) ListCacheNames(namespace string) []string {
+	ret := _m.Called(namespace)
 
 	var r0 []string
-	if rf, ok := ret.Get(0).(func() []string); ok {
-		r0 = rf()
+	if rf, ok := ret.Get(0).(func(string) []string); ok {
+		r0 = rf(namespace)
 	} else {
 		if ret.Get(0) != nil {
 			r0 = ret.Get(0).([]string)


Test patch:
diff --git a/internal/cache/cache_test.go b/internal/cache/cache_test.go
index b3a8000a1..a5b129d90 100644
--- a/internal/cache/cache_test.go
+++ b/internal/cache/cache_test.go
@@ -48,11 +48,12 @@ func TestGetCacheReturnsSameCacheForSameConfig(t *testing.T) {
 	cache1, _ := cacheManager.GetCache(NewCacheConfig(ctx, "cache.batch.limit", "cache.batch.ttl", "testnamespace"))
 
 	assert.Equal(t, cache0, cache1)
-	assert.Equal(t, []string{"testnamespace::cache.batch"}, cacheManager.ListKeys())
+	assert.Equal(t, []string{"testnamespace:cache.batch"}, cacheManager.ListCacheNames("testnamespace"))
 
 	cache2, _ := cacheManager.GetCache(NewCacheConfig(ctx, "cache.batch.limit", "cache.batch.ttl", ""))
 	assert.NotEqual(t, cache0, cache2)
-	assert.Equal(t, 2, len(cacheManager.ListKeys()))
+	assert.Equal(t, 1, len(cacheManager.ListCacheNames("testnamespace")))
+	assert.Equal(t, 1, len(cacheManager.ListCacheNames("global")))
 }
 
 func TestTwoSeparateCacheWorksIndependently(t *testing.T) {