Repository: googleforgames/open-saves (go)

Issue:
Add caching for Store entities
**Is your feature request related to a problem? Please describe.**
No.

**Describe the solution you'd like**
Add caching for store entities in Open Saves, similar to what we have for record, Same TTL.

**Describe alternatives you've considered**
Caching the results of the request on the client since the store entities never changes in our implementation.

**Additional context**
Add any other context or screenshots about the feature request here.


Pull request:
feat: implement store caching
**What type of PR is this?**

/kind feat


**What this PR does / Why we need it**:

This PR enables store caching.

**Which issue(s) this PR fixes**:

Closes #404 

**Special notes for your reviewer**:
Since I'm not able to run the full test scenario in the gcp project I'm relying on the CI or someone to manually run it

Patch:
diff --git a/docs/contributing.md b/docs/contributing.md
index 8fce163..f21be1a 100644
--- a/docs/contributing.md
+++ b/docs/contributing.md
@@ -23,7 +23,7 @@ use GitHub pull requests for this purpose. Consult
 information on using pull requests.
 
 When opening a pull request, please enter the title with a matching
-[conentional commit](https://www.conventionalcommits.org/en/v1.0.0/) tag.
+[conventional commit](https://www.conventionalcommits.org/en/v1.0.0/) tag.
 For example, a patch to fix a bug should be titles as "fix: server crash with malformed json."
 
 ## Community Guidelines
diff --git a/internal/app/server/open_saves.go b/internal/app/server/open_saves.go
index e39434c..1374ea5 100644
--- a/internal/app/server/open_saves.go
+++ b/internal/app/server/open_saves.go
@@ -103,6 +103,7 @@ func (s *openSavesServer) CreateStore(ctx context.Context, req *pb.CreateStoreRe
 		return nil, status.Convert(err).Err()
 	}
 	log.Debugf("Created store: %+v", store)
+	s.storeCache(ctx, &store)
 	return newStore.ToProto(), nil
 }
 
@@ -143,12 +144,11 @@ func (s *openSavesServer) DeleteRecord(ctx context.Context, req *pb.DeleteRecord
 }
 
 func (s *openSavesServer) GetStore(ctx context.Context, req *pb.GetStoreRequest) (*pb.Store, error) {
-	store, err := s.metaDB.GetStore(ctx, req.GetKey())
+	str, err := s.getStoreAndCache(ctx, req.GetKey())
 	if err != nil {
-		log.Warnf("GetStore failed for store (%s): %v", req.GetKey(), err)
-		return nil, status.Convert(err).Err()
+		return nil, err
 	}
-	return store.ToProto(), nil
+	return str.ToProto(), nil
 }
 
 func (s *openSavesServer) ListStores(ctx context.Context, req *pb.ListStoresRequest) (*pb.ListStoresResponse, error) {
@@ -1012,6 +1012,33 @@ func (s *openSavesServer) cacheRecord(ctx context.Context, r *record.Record, hin
 	return err
 }
 
+func (s *openSavesServer) getStoreAndCache(ctx context.Context, storeKey string) (*store.Store, error) {
+	st := new(store.Store)
+	if err := s.cacheStore.Get(ctx, store.CacheKey(storeKey), st); err == nil {
+		log.Debug("store cache hit")
+		return st, nil
+	}
+	log.Debug("store cache miss")
+
+	str, err := s.metaDB.GetStore(ctx, storeKey)
+	if err != nil {
+		log.Warnf("GetStore failed for store (%s): %v",
+			storeKey, err)
+		return nil, status.Convert(err).Err()
+	}
+	s.storeCache(ctx, str)
+	log.Tracef("Got store %+v", s)
+	return str, nil
+}
+
+func (s *openSavesServer) storeCache(ctx context.Context, st *store.Store) error {
+	var err error
+	if err = s.cacheStore.Set(ctx, st); err != nil {
+		log.Warnf("failed to encode record for cache for store (%s): %v", st.Key, err)
+	}
+	return err
+}
+
 // shouldCache returns whether or not Open Saves should try to store
 // the record in the cache store. Default behavior is to cache
 // if hint is not specified.
diff --git a/internal/pkg/metadb/store/store.go b/internal/pkg/metadb/store/store.go
index 565f406..d54e1e9 100644
--- a/internal/pkg/metadb/store/store.go
+++ b/internal/pkg/metadb/store/store.go
@@ -16,8 +16,10 @@ package store
 
 import (
 	"cloud.google.com/go/datastore"
+	"fmt"
 	pb "github.com/googleforgames/open-saves/api"
 	"github.com/googleforgames/open-saves/internal/pkg/metadb/timestamps"
+	"github.com/vmihailenco/msgpack/v5"
 )
 
 // Store represents a Open Saves store in the metadata database.
@@ -49,6 +51,12 @@ func (s *Store) ToProto() *pb.Store {
 	}
 }
 
+// CacheKey returns a cache key string to manage cached entries.
+// concatenates "store" + store keys separated by a dash.
+func CacheKey(storeKey string) string {
+	return fmt.Sprintf("store:%s", storeKey)
+}
+
 // These functions need to be implemented here instead of the datastore package because
 // go doesn't permit to define additional receivers in another package.
 // Save and Load replicates the default behaviors, however, they are required
@@ -72,6 +80,29 @@ func (s *Store) LoadKey(k *datastore.Key) error {
 	return nil
 }
 
+// Cacheable implementations
+
+// CacheKey  returns cache formatted key
+func (s *Store) CacheKey() string {
+	return CacheKey(s.Key)
+}
+
+// DecodeBytes deserializes the byte slice given by by.
+func (s *Store) DecodeBytes(by []byte) error {
+	return msgpack.Unmarshal(by, s)
+}
+
+// EncodeBytes returns a serialized byte slice of the object.
+func (s *Store) EncodeBytes() ([]byte, error) {
+	b, err := msgpack.Marshal(s)
+	if err != nil {
+		return nil, err
+	}
+	return b, nil
+}
+
+// Proto
+
 // FromProto creates a new Store instance from a proto.
 // Passing nil returns a zero-initialized Store.
 func FromProto(p *pb.Store) *Store {


Test patch:
diff --git a/internal/app/server/open_saves_test.go b/internal/app/server/open_saves_test.go
index 77862b0..c9253f6 100644
--- a/internal/app/server/open_saves_test.go
+++ b/internal/app/server/open_saves_test.go
@@ -17,6 +17,7 @@ package server
 import (
 	"context"
 	"fmt"
+	"github.com/googleforgames/open-saves/internal/pkg/metadb/store"
 	"io"
 	"net"
 	"reflect"
@@ -397,6 +398,26 @@ func TestOpenSaves_CreateGetDeleteStore(t *testing.T) {
 	assert.Equal(t, store.GetUpdatedAt(), store2.GetUpdatedAt())
 }
 
+func TestOpenSaves_GetCreateStoreFromCache(t *testing.T) {
+	ctx := context.Background()
+	server, listener := getOpenSavesServer(ctx, t, "gcp")
+	_, client := getTestClient(ctx, t, listener)
+	storeKey := uuid.NewString()
+	str := &pb.Store{
+		Key:     storeKey,
+		Name:    "test-GetCreateStore-store-cache",
+		Tags:    []string{"tag1"},
+		OwnerId: "owner",
+	}
+	setupTestStore(ctx, t, client, str)
+	// should be already cached
+	cacheKey := store.CacheKey(storeKey)
+	storeFromCache := new(store.Store)
+	err := server.cacheStore.Get(ctx, cacheKey, storeFromCache)
+	assert.NoError(t, err, "should have retrieved store from cache after Create")
+
+}
+
 func TestOpenSaves_CreateGetDeleteRecord(t *testing.T) {
 	ctx := context.Background()
 	_, listener := getOpenSavesServer(ctx, t, "gcp")
diff --git a/internal/pkg/metadb/store/store_test.go b/internal/pkg/metadb/store/store_test.go
index 0839191..e256d55 100644
--- a/internal/pkg/metadb/store/store_test.go
+++ b/internal/pkg/metadb/store/store_test.go
@@ -88,3 +88,49 @@ func TestStore_LoadKey(t *testing.T) {
 	assert.NoError(t, store.LoadKey(key))
 	assert.Equal(t, "testkey", store.Key)
 }
+
+func TestStore_CacheKey(t *testing.T) {
+	s := &Store{
+		Key: "abc",
+	}
+	key := s.CacheKey()
+	assert.Equal(t, "store:abc", key)
+}
+
+func TestStore_SerializeRecord(t *testing.T) {
+	rr := []*Store{
+		{
+			Timestamps: timestamps.Timestamps{
+				CreatedAt: time.Unix(100, 0),
+				UpdatedAt: time.Unix(110, 0),
+			},
+		},
+		{
+			Key:     "some-key",
+			Name:    "some-store",
+			Tags:    []string{"tag1", "tag2"},
+			OwnerID: "owner",
+			Timestamps: timestamps.Timestamps{
+				CreatedAt: time.Unix(100, 0),
+				UpdatedAt: time.Unix(110, 0),
+			},
+		},
+		{
+			Key:     "some-key",
+			OwnerID: "new-owner",
+			Tags:    []string{"tag1", "tag2"},
+			Timestamps: timestamps.Timestamps{
+				CreatedAt: time.Unix(100, 0),
+				UpdatedAt: time.Unix(110, 0),
+			},
+		},
+	}
+
+	for _, s := range rr {
+		e, err := s.EncodeBytes()
+		assert.NoError(t, err)
+		decoded := new(Store)
+		assert.NoError(t, decoded.DecodeBytes(e))
+		assert.Equal(t, s, decoded)
+	}
+}