Use always strings for cache keys (#13631)

* Use always strings for cache keys

* fixing tests

* Addressing PR review comments

* Adding the base to FormatInt

* Fix typo

api4/openGraph.go

@@ -51,7 +51,7 @@ func getOpenGraphMetadata(c *Context, w http.ResponseWriter, r *http.Request) {
 
 	og := c.App.GetOpenGraphMetadata(url)
 	ogJSON, err := og.ToJSON()
-	openGraphDataCache.AddWithExpiresInSecs(props["url"], ogJSON, 3600) // Cache would expire after 1 hour
+	openGraphDataCache.AddWithExpiresInSecs(url, ogJSON, 3600) // Cache would expire after 1 hour
 	if err != nil {
 		w.Write([]byte(`{"url": ""}`))
 		return

app/post_metadata.go

@@ -9,6 +9,7 @@ import (
 	"io"
 	"net/http"
 	"net/url"
+	"strconv"
 	"strings"
 	"time"
 
@@ -434,7 +435,7 @@ func resolveMetadataURL(requestURL string, siteURL string) string {
 }
 
 func getLinkMetadataFromCache(requestURL string, timestamp int64) (*opengraph.OpenGraph, *model.PostImage, bool) {
-	cached, ok := linkCache.Get(model.GenerateLinkMetadataHash(requestURL, timestamp))
+	cached, ok := linkCache.Get(strconv.FormatInt(model.GenerateLinkMetadataHash(requestURL, timestamp), 16))
 	if !ok {
 		return nil, nil, false
 	}
@@ -497,7 +498,7 @@ func cacheLinkMetadata(requestURL string, timestamp int64, og *opengraph.OpenGra
 		val = image
 	}
 
-	linkCache.AddWithExpiresInSecs(model.GenerateLinkMetadataHash(requestURL, timestamp), val, LINK_CACHE_DURATION)
+	linkCache.AddWithExpiresInSecs(strconv.FormatInt(model.GenerateLinkMetadataHash(requestURL, timestamp), 16), val, LINK_CACHE_DURATION)
 }
 
 func (a *App) parseLinkMetadata(requestURL string, body io.Reader, contentType string) (*opengraph.OpenGraph, *model.PostImage, error) {

app/status.go

@@ -34,11 +34,9 @@ func (a *App) GetAllStatuses() map[string]*model.Status {
 	statusMap := map[string]*model.Status{}
 
 	for _, userId := range userIds {
-		if id, ok := userId.(string); ok {
-			status := a.GetStatusFromCache(id)
-			if status != nil {
-				statusMap[id] = status
-			}
+		status := a.GetStatusFromCache(userId)
+		if status != nil {
+			statusMap[userId] = status
 		}
 	}
 

services/cache/cache.go

@@ -13,29 +13,29 @@ type Cache interface {
 	Purge()
 
 	// Add adds the given key and value to the store without an expiry.
-	Add(key, value interface{})
+	Add(key string, value interface{})
 
 	// AddWithDefaultExpires adds the given key and value to the store with the default expiry.
-	AddWithDefaultExpires(key, value interface{})
+	AddWithDefaultExpires(key string, value interface{})
 
 	// AddWithExpiresInSecs adds the given key and value to the cache with the given expiry.
-	AddWithExpiresInSecs(key, value interface{}, expireAtSecs int64)
+	AddWithExpiresInSecs(key string, value interface{}, expireAtSecs int64)
 
 	// Get returns the value stored in the cache for a key, or nil if no value is present. The ok result indicates whether value was found in the cache.
-	Get(key interface{}) (value interface{}, ok bool)
+	Get(key string) (value interface{}, ok bool)
 
 	// GetOrAdd returns the existing value for the key if present. Otherwise, it stores and returns the given value. The loaded result is true if the value was loaded, false if stored.
 	// This API intentionally deviates from the Add-only variants above for simplicity. We should simplify the entire API in the future.
-	GetOrAdd(key, value interface{}, ttl time.Duration) (actual interface{}, loaded bool)
+	GetOrAdd(key string, value interface{}, ttl time.Duration) (actual interface{}, loaded bool)
 
 	// Remove deletes the value for a key.
-	Remove(key interface{})
+	Remove(key string)
 
 	// RemoveByPrefix deletes all keys containing the given prefix string.
 	RemoveByPrefix(prefix string)
 
 	// Keys returns a slice of the keys in the cache.
-	Keys() []interface{}
+	Keys() []string
 
 	// Len returns the number of items in the cache.
 	Len() int

services/cache/lru/lru.go

@@ -23,7 +23,7 @@ import (
 type Cache struct {
 	size                   int
 	evictList              *list.List
-	items                  map[interface{}]*list.Element
+	items                  map[string]*list.Element
 	lock                   sync.RWMutex
 	name                   string
 	defaultExpiry          int64
@@ -57,7 +57,7 @@ func (c *CacheProvider) Close() {
 
 // entry is used to hold a value in the evictList.
 type entry struct {
-	key        interface{}
+	key        string
 	value      interface{}
 	expires    time.Time
 	generation int64
@@ -68,7 +68,7 @@ func New(size int) *Cache {
 	return &Cache{
 		size:      size,
 		evictList: list.New(),
-		items:     make(map[interface{}]*list.Element, size),
+		items:     make(map[string]*list.Element, size),
 	}
 }
 
@@ -91,24 +91,24 @@ func (c *Cache) Purge() {
 }
 
 // Add adds the given key and value to the store without an expiry.
-func (c *Cache) Add(key, value interface{}) {
+func (c *Cache) Add(key string, value interface{}) {
 	c.AddWithExpiresInSecs(key, value, 0)
 }
 
 // AddWithDefaultExpires adds the given key and value to the store with the default expiry.
-func (c *Cache) AddWithDefaultExpires(key, value interface{}) {
+func (c *Cache) AddWithDefaultExpires(key string, value interface{}) {
 	c.AddWithExpiresInSecs(key, value, c.defaultExpiry)
 }
 
 // AddWithExpiresInSecs adds the given key and value to the cache with the given expiry.
-func (c *Cache) AddWithExpiresInSecs(key, value interface{}, expireAtSecs int64) {
+func (c *Cache) AddWithExpiresInSecs(key string, value interface{}, expireAtSecs int64) {
 	c.lock.Lock()
 	defer c.lock.Unlock()
 
 	c.add(key, value, time.Duration(expireAtSecs)*time.Second)
 }
 
-func (c *Cache) add(key, value interface{}, ttl time.Duration) {
+func (c *Cache) add(key string, value interface{}, ttl time.Duration) {
 	var expires time.Time
 	if ttl > 0 {
 		expires = time.Now().Add(ttl)
@@ -139,14 +139,14 @@ func (c *Cache) add(key, value interface{}, ttl time.Duration) {
 }
 
 // Get returns the value stored in the cache for a key, or nil if no value is present. The ok result indicates whether value was found in the cache.
-func (c *Cache) Get(key interface{}) (value interface{}, ok bool) {
+func (c *Cache) Get(key string) (value interface{}, ok bool) {
 	c.lock.Lock()
 	defer c.lock.Unlock()
 
 	return c.getValue(key)
 }
 
-func (c *Cache) getValue(key interface{}) (value interface{}, ok bool) {
+func (c *Cache) getValue(key string) (value interface{}, ok bool) {
 	if ent, ok := c.items[key]; ok {
 		e := ent.Value.(*entry)
 
@@ -164,7 +164,7 @@ func (c *Cache) getValue(key interface{}) (value interface{}, ok bool) {
 
 // GetOrAdd returns the existing value for the key if present. Otherwise, it stores and returns the given value. The loaded result is true if the value was loaded, false if stored.
 // This API intentionally deviates from the Add-only variants above for simplicity. We should simplify the entire API in the future.
-func (c *Cache) GetOrAdd(key, value interface{}, ttl time.Duration) (actual interface{}, loaded bool) {
+func (c *Cache) GetOrAdd(key string, value interface{}, ttl time.Duration) (actual interface{}, loaded bool) {
 	c.lock.Lock()
 	defer c.lock.Unlock()
 
@@ -179,7 +179,7 @@ func (c *Cache) GetOrAdd(key, value interface{}, ttl time.Duration) (actual inte
 }
 
 // Remove deletes the value for a key.
-func (c *Cache) Remove(key interface{}) {
+func (c *Cache) Remove(key string) {
 	c.lock.Lock()
 	defer c.lock.Unlock()
 
@@ -196,8 +196,7 @@ func (c *Cache) RemoveByPrefix(prefix string) {
 	for ent := c.evictList.Back(); ent != nil; ent = ent.Prev() {
 		e := ent.Value.(*entry)
 		if e.generation == c.currentGeneration {
-			keyString := e.key.(string)
-			if strings.HasPrefix(keyString, prefix) {
+			if strings.HasPrefix(e.key, prefix) {
 				if ent, ok := c.items[e.key]; ok {
 					c.removeElement(ent)
 				}
@@ -207,11 +206,11 @@ func (c *Cache) RemoveByPrefix(prefix string) {
 }
 
 // Keys returns a slice of the keys in the cache, from oldest to newest.
-func (c *Cache) Keys() []interface{} {
+func (c *Cache) Keys() []string {
 	c.lock.RLock()
 	defer c.lock.RUnlock()
 
-	keys := make([]interface{}, c.len)
+	keys := make([]string, c.len)
 	i := 0
 	for ent := c.evictList.Back(); ent != nil; ent = ent.Prev() {
 		e := ent.Value.(*entry)

services/cache/lru/lru_test.go

@@ -11,6 +11,7 @@
 package lru
 
 import (
+	"fmt"
 	"testing"
 	"time"
 
@@ -22,56 +23,56 @@ func TestLRU(t *testing.T) {
 	l := New(128)
 
 	for i := 0; i < 256; i++ {
-		l.Add(i, i)
+		l.Add(fmt.Sprintf("%d", i), i)
 	}
 	require.Equalf(t, l.Len(), 128, "bad len: %v", l.Len())
 
 	for i, k := range l.Keys() {
 		v, ok := l.Get(k)
 		require.True(t, ok, "bad key: %v", k)
-		require.Equalf(t, v, k, "bad key: %v", k)
+		require.Equalf(t, fmt.Sprintf("%d", v), k, "bad key: %v", k)
 		require.Equalf(t, i+128, v, "bad key: %v", k)
 	}
 	for i := 0; i < 128; i++ {
-		_, ok := l.Get(i)
+		_, ok := l.Get(fmt.Sprintf("%d", i))
 		require.False(t, ok, "should be evicted")
 	}
 	for i := 128; i < 256; i++ {
-		_, ok := l.Get(i)
+		_, ok := l.Get(fmt.Sprintf("%d", i))
 		require.True(t, ok, "should not be evicted")
 	}
 	for i := 128; i < 192; i++ {
-		l.Remove(i)
-		_, ok := l.Get(i)
+		l.Remove(fmt.Sprintf("%d", i))
+		_, ok := l.Get(fmt.Sprintf("%d", i))
 		require.False(t, ok, "should be deleted")
 	}
 
-	l.Get(192) // expect 192 to be last key in l.Keys()
+	l.Get("192") // expect 192 to be last key in l.Keys()
 
 	for i, k := range l.Keys() {
-		require.Falsef(t, (i < 63 && k != i+193), "out of order key: %v", k)
-		require.Falsef(t, (i == 63 && k != 192), "out of order key: %v", k)
+		require.Falsef(t, i < 63 && k != fmt.Sprintf("%d", i+193), "out of order key: %v", k)
+		require.Falsef(t, i == 63 && k != "192", "out of order key: %v", k)
 	}
 
 	l.Purge()
 	require.Equalf(t, l.Len(), 0, "bad len: %v", l.Len())
-	_, ok := l.Get(200)
+	_, ok := l.Get("200")
 	require.False(t, ok, "should contain nothing")
 }
 
 func TestLRUExpire(t *testing.T) {
 	l := New(128)
 
-	l.AddWithExpiresInSecs(1, 1, 1)
-	l.AddWithExpiresInSecs(2, 2, 1)
-	l.AddWithExpiresInSecs(3, 3, 0)
+	l.AddWithExpiresInSecs("1", 1, 1)
+	l.AddWithExpiresInSecs("2", 2, 1)
+	l.AddWithExpiresInSecs("3", 3, 0)
 
 	time.Sleep(time.Millisecond * 2100)
 
-	r1, ok := l.Get(1)
+	r1, ok := l.Get("1")
 	require.False(t, ok, r1)
 
-	_, ok2 := l.Get(3)
+	_, ok2 := l.Get("3")
 	require.True(t, ok2, "should exist")
 }
 
@@ -79,46 +80,46 @@ func TestLRUGetOrAdd(t *testing.T) {
 	l := New(128)
 
 	// First GetOrAdd should save
-	value, loaded := l.GetOrAdd(1, 1, 0)
+	value, loaded := l.GetOrAdd("1", 1, 0)
 	assert.Equal(t, 1, value)
 	assert.False(t, loaded)
 
 	// Second GetOrAdd should load original value, ignoring new value
-	value, loaded = l.GetOrAdd(1, 10, 0)
+	value, loaded = l.GetOrAdd("1", 10, 0)
 	assert.Equal(t, 1, value)
 	assert.True(t, loaded)
 
 	// Third GetOrAdd should still load original value
-	value, loaded = l.GetOrAdd(1, 1, 0)
+	value, loaded = l.GetOrAdd("1", 1, 0)
 	assert.Equal(t, 1, value)
 	assert.True(t, loaded)
 
 	// First GetOrAdd on a new key should save
-	value, loaded = l.GetOrAdd(2, 2, 0)
+	value, loaded = l.GetOrAdd("2", 2, 0)
 	assert.Equal(t, 2, value)
 	assert.False(t, loaded)
 
-	l.Remove(1)
+	l.Remove("1")
 
 	// GetOrAdd after a remove should save
-	value, loaded = l.GetOrAdd(1, 10, 0)
+	value, loaded = l.GetOrAdd("1", 10, 0)
 	assert.Equal(t, 10, value)
 	assert.False(t, loaded)
 
 	// GetOrAdd after another key was removed should load original value for key
-	value, loaded = l.GetOrAdd(2, 2, 0)
+	value, loaded = l.GetOrAdd("2", 2, 0)
 	assert.Equal(t, 2, value)
 	assert.True(t, loaded)
 
 	// GetOrAdd should expire
-	value, loaded = l.GetOrAdd(3, 3, 500*time.Millisecond)
+	value, loaded = l.GetOrAdd("3", 3, 500*time.Millisecond)
 	assert.Equal(t, 3, value)
 	assert.False(t, loaded)
-	value, loaded = l.GetOrAdd(3, 4, 500*time.Millisecond)
+	value, loaded = l.GetOrAdd("3", 4, 500*time.Millisecond)
 	assert.Equal(t, 3, value)
 	assert.True(t, loaded)
 	time.Sleep(1 * time.Second)
-	value, loaded = l.GetOrAdd(3, 5, 500*time.Millisecond)
+	value, loaded = l.GetOrAdd("3", 5, 500*time.Millisecond)
 	assert.Equal(t, 5, value)
 	assert.False(t, loaded)
 }

store/localcachelayer/user_layer.go

@@ -56,7 +56,7 @@ func (s LocalCacheUserStore) InvalidateProfilesInChannelCacheByUser(userId strin
 		if cacheItem, ok := s.rootStore.profilesInChannelCache.Get(key); ok {
 			userMap := cacheItem.(map[string]*model.User)
 			if _, userInCache := userMap[userId]; userInCache {
-				s.rootStore.doInvalidateCacheCluster(s.rootStore.profilesInChannelCache, key.(string))
+				s.rootStore.doInvalidateCacheCluster(s.rootStore.profilesInChannelCache, key)
 				if s.rootStore.metrics != nil {
 					s.rootStore.metrics.IncrementMemCacheInvalidationCounter("Profiles in Channel - Remove by User")
 				}