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 description:
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) {