Remotecache: Migrates get/set calls to use bytearrays and remove get/set functions (#63525)

Signed-off-by: bergquist <carl.bergquist@gmail.com>
This commit is contained in:
Carl Bergquist 2023-03-08 17:08:57 +01:00 committed by GitHub
parent a5133d61b5
commit 7c55dbf37d
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 34 additions and 54 deletions

View File

@ -55,12 +55,6 @@ func ProvideService(cfg *setting.Cfg, sqlStore db.DB, secretsService secrets.Ser
// so any struct added to the cache needs to be registered with `remotecache.Register`
// ex `remotecache.Register(CacheableStruct{})`
type CacheStorage interface {
// Get reads object from Cache
Get(ctx context.Context, key string) (interface{}, error)
// Set sets an object into the cache. if `expire` is set to zero it will default to 24h
Set(ctx context.Context, key string, value interface{}, expire time.Duration) error
// GetByteArray gets the cache value as an byte array
GetByteArray(ctx context.Context, key string) ([]byte, error)
@ -83,11 +77,6 @@ type RemoteCache struct {
Cfg *setting.Cfg
}
// Get reads object from Cache
func (ds *RemoteCache) Get(ctx context.Context, key string) (interface{}, error) {
return ds.client.Get(ctx, key)
}
// GetByteArray returns the cached value as an byte array
func (ds *RemoteCache) GetByteArray(ctx context.Context, key string) ([]byte, error) {
return ds.client.GetByteArray(ctx, key)
@ -95,16 +84,11 @@ func (ds *RemoteCache) GetByteArray(ctx context.Context, key string) ([]byte, er
// SetByteArray stored the byte array in the cache
func (ds *RemoteCache) SetByteArray(ctx context.Context, key string, value []byte, expire time.Duration) error {
return ds.client.SetByteArray(ctx, key, value, expire)
}
// Set sets an object into the cache. if `expire` is set to zero it will default to 24h
func (ds *RemoteCache) Set(ctx context.Context, key string, value interface{}, expire time.Duration) error {
if expire == 0 {
expire = defaultMaxCacheExpiration
}
return ds.client.Set(ctx, key, value, expire)
return ds.client.SetByteArray(ctx, key, value, expire)
}
// Delete object from cache
@ -208,15 +192,9 @@ type prefixCacheStorage struct {
prefix string
}
func (pcs *prefixCacheStorage) Get(ctx context.Context, key string) (interface{}, error) {
return pcs.cache.Get(ctx, pcs.prefix+key)
}
func (pcs *prefixCacheStorage) GetByteArray(ctx context.Context, key string) ([]byte, error) {
return pcs.cache.GetByteArray(ctx, pcs.prefix+key)
}
func (pcs *prefixCacheStorage) Set(ctx context.Context, key string, value interface{}, expire time.Duration) error {
return pcs.cache.Set(ctx, pcs.prefix+key, value, expire)
}
func (pcs *prefixCacheStorage) SetByteArray(ctx context.Context, key string, value []byte, expire time.Duration) error {
return pcs.cache.SetByteArray(ctx, pcs.prefix+key, value, expire)
}

View File

@ -65,15 +65,15 @@ func runCountTestsForClient(t *testing.T, opts *setting.RemoteCacheOptions, sqls
}
t.Run("can count items", func(t *testing.T) {
cacheableStruct := CacheableStruct{String: "hej", Int64: 2000}
cacheableValue := []byte("hej hej")
err := client.Set(context.Background(), "pref-key1", cacheableStruct, 0)
err := client.SetByteArray(context.Background(), "pref-key1", cacheableValue, 0)
require.NoError(t, err)
err = client.Set(context.Background(), "pref-key2", cacheableStruct, 0)
err = client.SetByteArray(context.Background(), "pref-key2", cacheableValue, 0)
require.NoError(t, err)
err = client.Set(context.Background(), "key3-not-pref", cacheableStruct, 0)
err = client.SetByteArray(context.Background(), "key3-not-pref", cacheableValue, 0)
require.NoError(t, err)
n, errC := client.Count(context.Background(), "pref-")
@ -89,37 +89,34 @@ func runCountTestsForClient(t *testing.T, opts *setting.RemoteCacheOptions, sqls
}
func canPutGetAndDeleteCachedObjects(t *testing.T, client CacheStorage) {
cacheableStruct := CacheableStruct{String: "hej", Int64: 2000}
dataToCache := []byte("some bytes")
err := client.Set(context.Background(), "key1", cacheableStruct, 0)
err := client.SetByteArray(context.Background(), "key1", dataToCache, 0)
assert.Equal(t, err, nil, "expected nil. got: ", err)
data, err := client.Get(context.Background(), "key1")
data, err := client.GetByteArray(context.Background(), "key1")
assert.Equal(t, err, nil)
s, ok := data.(CacheableStruct)
assert.Equal(t, ok, true)
assert.Equal(t, s.String, "hej")
assert.Equal(t, s.Int64, int64(2000))
assert.Equal(t, string(data), "some bytes")
err = client.Delete(context.Background(), "key1")
assert.Equal(t, err, nil)
_, err = client.Get(context.Background(), "key1")
_, err = client.GetByteArray(context.Background(), "key1")
assert.Equal(t, err, ErrCacheItemNotFound)
}
func canNotFetchExpiredItems(t *testing.T, client CacheStorage) {
cacheableStruct := CacheableStruct{String: "hej", Int64: 2000}
dataToCache := []byte("some bytes")
err := client.Set(context.Background(), "key1", cacheableStruct, time.Second)
err := client.SetByteArray(context.Background(), "key1", dataToCache, time.Second)
assert.Equal(t, err, nil)
// not sure how this can be avoided when testing redis/memcached :/
<-time.After(time.Second + time.Millisecond)
// should not be able to read that value since its expired
_, err = client.Get(context.Background(), "key1")
_, err = client.GetByteArray(context.Background(), "key1")
assert.Equal(t, err, ErrCacheItemNotFound)
}
@ -133,16 +130,16 @@ func TestCachePrefix(t *testing.T) {
prefixCache := &prefixCacheStorage{cache: cache, prefix: "test/"}
// Set a value (with a prefix)
err := prefixCache.Set(context.Background(), "foo", "bar", time.Hour)
err := prefixCache.SetByteArray(context.Background(), "foo", []byte("bar"), time.Hour)
require.NoError(t, err)
// Get a value (with a prefix)
v, err := prefixCache.Get(context.Background(), "foo")
v, err := prefixCache.GetByteArray(context.Background(), "foo")
require.NoError(t, err)
require.Equal(t, "bar", v)
require.Equal(t, "bar", string(v))
// Get a value directly from the underlying cache, ensure the prefix is in the key
v, err = cache.Get(context.Background(), "test/foo")
v, err = cache.GetByteArray(context.Background(), "test/foo")
require.NoError(t, err)
require.Equal(t, "bar", v)
require.Equal(t, "bar", string(v))
// Get a value directly from the underlying cache without a prefix, should not be there
_, err = cache.Get(context.Background(), "foo")
require.Error(t, err)

View File

@ -96,5 +96,5 @@ func (a *AnonSessionService) TagSession(ctx context.Context, httpReq *http.Reque
a.localCache.SetDefault(key, struct{}{})
return a.remoteCache.Set(ctx, key, key, thirtyDays)
return a.remoteCache.SetByteArray(ctx, key, []byte(key), thirtyDays)
}

View File

@ -2,6 +2,7 @@ package clients
import (
"context"
"encoding/binary"
"encoding/hex"
"fmt"
"hash/fnv"
@ -49,8 +50,8 @@ func ProvideProxy(cfg *setting.Cfg, cache proxyCache, userSrv user.Service, clie
}
type proxyCache interface {
Get(ctx context.Context, key string) (interface{}, error)
Set(ctx context.Context, key string, value interface{}, expire time.Duration) error
GetByteArray(ctx context.Context, key string) ([]byte, error)
SetByteArray(ctx context.Context, key string, value []byte, expire time.Duration) error
}
type Proxy struct {
@ -82,14 +83,16 @@ func (c *Proxy) Authenticate(ctx context.Context, r *authn.Request) (*authn.Iden
if ok {
// See if we have cached the user id, in that case we can fetch the signed-in user and skip sync.
// Error here means that we could not find anything in cache, so we can proceed as usual
if entry, err := c.cache.Get(ctx, cacheKey); err == nil {
if entry, err := c.cache.GetByteArray(ctx, cacheKey); err == nil {
uid := int64(binary.LittleEndian.Uint64(entry))
usr, err := c.userSrv.GetSignedInUserWithCacheCtx(ctx, &user.GetSignedInUserQuery{
UserID: entry.(int64),
UserID: uid,
OrgID: r.OrgID,
})
if err != nil {
c.log.FromContext(ctx).Warn("Could not resolved cached user", "error", err, "userId", entry.(int64))
c.log.FromContext(ctx).Warn("Could not resolved cached user", "error", err, "userId", string(entry))
}
// if we for some reason cannot find the user we proceed with the normal flow, authenticate with ProxyClient
@ -133,8 +136,10 @@ func (c *Proxy) Hook(ctx context.Context, identity *authn.Identity, r *authn.Req
}
c.log.FromContext(ctx).Debug("Cache proxy user", "userId", id)
if err := c.cache.Set(ctx, identity.ClientParams.CacheAuthProxyKey, id, time.Duration(c.cfg.AuthProxySyncTTL)*time.Minute); err != nil {
c.log.FromContext(ctx).Warn("Failed to cache proxy user", "error", err, "userId", id)
bytes := make([]byte, 8)
binary.LittleEndian.PutUint64(bytes, uint64(id))
if err := c.cache.SetByteArray(ctx, identity.ClientParams.CacheAuthProxyKey, bytes, time.Duration(c.cfg.AuthProxySyncTTL)*time.Minute); err != nil {
c.log.Warn("failed to cache proxy user", "error", err, "userId", id)
}
return nil

View File

@ -178,13 +178,13 @@ var _ proxyCache = new(fakeCache)
type fakeCache struct {
expectedErr error
expectedItem interface{}
expectedItem []byte
}
func (f fakeCache) Get(ctx context.Context, key string) (interface{}, error) {
func (f fakeCache) GetByteArray(ctx context.Context, key string) ([]byte, error) {
return f.expectedItem, f.expectedErr
}
func (f fakeCache) Set(ctx context.Context, key string, value interface{}, expire time.Duration) error {
func (f fakeCache) SetByteArray(ctx context.Context, key string, value []byte, expire time.Duration) error {
return f.expectedErr
}