Activate Statcache (#2329)

* define endpoints on which the cache needs to be cleaned

* add changelog item

* copy the struct before caching

* use json marshaling to avoid pointer magic

* pass references to RemoveFromCache

* don't defer Cache cleaning

* don't pass path to cache key

* clean cache before creating home

* Revert "don't pass path to cache key"

This reverts commit 9c7a830.

* try to find the problem

* Revert "don't defer Cache cleaning"

This reverts commit 8026f2a.

* Revert "Revert "don't defer Cache cleaning""

This reverts commit 5319e08.

* don't panic when creating spaces

* pass user to RemoveFromCache func directly

* first draft for remove logic

* refine remove logic

* add resourceid when creating space

* refine create space remove logic

changelog/unreleased/activate-statcache.md

@@ -0,0 +1,6 @@
+Change: activate the statcache
+
+Activates the cache of stat request/responses in the gateway.
+Fixes tests that are failing because of it
+
+https://github.com/cs3org/reva/pull/2329

internal/grpc/services/gateway/publicshareprovider.go

@@ -24,6 +24,7 @@ import (
 	rpc "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1"
 	link "github.com/cs3org/go-cs3apis/cs3/sharing/link/v1beta1"
 	"github.com/cs3org/reva/pkg/appctx"
+	ctxpkg "github.com/cs3org/reva/pkg/ctx"
 	"github.com/cs3org/reva/pkg/rgrpc/todo/pool"
 	"github.com/pkg/errors"
 )
@@ -42,6 +43,7 @@ func (s *svc) CreatePublicShare(ctx context.Context, req *link.CreatePublicShare
 		return nil, err
 	}
 
+	RemoveFromCache(s.statCache, ctxpkg.ContextMustGetUser(ctx), res.Share.ResourceId)
 	return res, nil
 }
 
@@ -57,6 +59,8 @@ func (s *svc) RemovePublicShare(ctx context.Context, req *link.RemovePublicShare
 	if err != nil {
 		return nil, err
 	}
+	// TODO: How to find out the resourceId?
+	RemoveFromCache(s.statCache, ctxpkg.ContextMustGetUser(ctx), nil)
 	return res, nil
 }
 
@@ -134,5 +138,6 @@ func (s *svc) UpdatePublicShare(ctx context.Context, req *link.UpdatePublicShare
 	if err != nil {
 		return nil, errors.Wrap(err, "error updating share")
 	}
+	RemoveFromCache(s.statCache, ctxpkg.ContextMustGetUser(ctx), res.Share.ResourceId)
 	return res, nil
 }

internal/grpc/services/gateway/storageprovider.go

@@ -204,6 +204,13 @@ func (s *svc) CreateStorageSpace(ctx context.Context, req *provider.CreateStorag
 			Status: status.NewInternal(ctx, err, "error calling CreateStorageSpace"),
 		}, nil
 	}
+
+	var r *provider.ResourceId
+	if createRes.StorageSpace != nil {
+		r = createRes.StorageSpace.Root
+	}
+
+	RemoveFromCache(s.statCache, ctxpkg.ContextMustGetUser(ctx), r)
 	return createRes, nil
 }
 
@@ -330,6 +337,7 @@ func (s *svc) UpdateStorageSpace(ctx context.Context, req *provider.UpdateStorag
 			Status: status.NewInternal(ctx, err, "error calling UpdateStorageSpace"),
 		}, nil
 	}
+	RemoveFromCache(s.statCache, ctxpkg.ContextMustGetUser(ctx), res.StorageSpace.Root)
 	return res, nil
 }
 
@@ -354,6 +362,8 @@ func (s *svc) DeleteStorageSpace(ctx context.Context, req *provider.DeleteStorag
 			Status: status.NewInternal(ctx, err, "error calling DeleteStorageSpace"),
 		}, nil
 	}
+
+	RemoveFromCache(s.statCache, ctxpkg.ContextMustGetUser(ctx), &provider.ResourceId{OpaqueId: req.Id.OpaqueId})
 	return res, nil
 }
 
@@ -529,6 +539,7 @@ func (s *svc) InitiateFileUpload(ctx context.Context, req *provider.InitiateFile
 		}
 	}
 
+	RemoveFromCache(s.statCache, ctxpkg.ContextMustGetUser(ctx), req.Ref.ResourceId)
 	return &gateway.InitiateFileUploadResponse{
 		Opaque:    storageRes.Opaque,
 		Status:    storageRes.Status,
@@ -571,6 +582,7 @@ func (s *svc) CreateContainer(ctx context.Context, req *provider.CreateContainer
 		return nil, errors.Wrap(err, "gateway: error calling CreateContainer")
 	}
 
+	RemoveFromCache(s.statCache, ctxpkg.ContextMustGetUser(ctx), req.Ref.ResourceId)
 	return res, nil
 }
 
@@ -595,6 +607,7 @@ func (s *svc) Delete(ctx context.Context, req *provider.DeleteRequest) (*provide
 		return nil, errors.Wrap(err, "gateway: error calling Delete")
 	}
 
+	RemoveFromCache(s.statCache, ctxpkg.ContextMustGetUser(ctx), req.Ref.ResourceId)
 	return res, nil
 }
 
@@ -635,6 +648,8 @@ func (s *svc) Move(ctx context.Context, req *provider.MoveRequest) (*provider.Mo
 		}
 	}
 
+	RemoveFromCache(s.statCache, ctxpkg.ContextMustGetUser(ctx), req.Source.ResourceId)
+	RemoveFromCache(s.statCache, ctxpkg.ContextMustGetUser(ctx), req.Destination.ResourceId)
 	return c.Move(ctx, req)
 }
 
@@ -657,6 +672,7 @@ func (s *svc) SetArbitraryMetadata(ctx context.Context, req *provider.SetArbitra
 		return nil, errors.Wrap(err, "gateway: error calling SetArbitraryMetadata")
 	}
 
+	RemoveFromCache(s.statCache, ctxpkg.ContextMustGetUser(ctx), req.Ref.ResourceId)
 	return res, nil
 }
 
@@ -679,6 +695,7 @@ func (s *svc) UnsetArbitraryMetadata(ctx context.Context, req *provider.UnsetArb
 		return nil, errors.Wrap(err, "gateway: error calling UnsetArbitraryMetadata")
 	}
 
+	RemoveFromCache(s.statCache, ctxpkg.ContextMustGetUser(ctx), req.Ref.ResourceId)
 	return res, nil
 }
 
@@ -1055,6 +1072,7 @@ func (s *svc) RestoreFileVersion(ctx context.Context, req *provider.RestoreFileV
 		return nil, errors.Wrap(err, "gateway: error calling RestoreFileVersion")
 	}
 
+	RemoveFromCache(s.statCache, ctxpkg.ContextMustGetUser(ctx), req.Ref.ResourceId)
 	return res, nil
 }
 
@@ -1297,6 +1315,12 @@ func (s *svc) RestoreRecycleItem(ctx context.Context, req *provider.RestoreRecyc
 	if err != nil {
 		return nil, errors.Wrap(err, "gateway: error calling RestoreRecycleItem")
 	}
+
+	RemoveFromCache(s.statCache, ctxpkg.ContextMustGetUser(ctx), req.Ref.ResourceId)
+	if req.RestoreRef != nil {
+		RemoveFromCache(s.statCache, ctxpkg.ContextMustGetUser(ctx), req.RestoreRef.ResourceId)
+	}
+
 	return res, nil
 }
 
@@ -1316,6 +1340,8 @@ func (s *svc) PurgeRecycle(ctx context.Context, req *provider.PurgeRecycleReques
 	if err != nil {
 		return nil, errors.Wrap(err, "gateway: error calling PurgeRecycle")
 	}
+
+	RemoveFromCache(s.statCache, ctxpkg.ContextMustGetUser(ctx), req.Ref.ResourceId)
 	return res, nil
 }
 
@@ -1390,8 +1416,7 @@ func (s *svc) getStorageProviderClient(_ context.Context, p *registry.ProviderIn
 		return nil, err
 	}
 
-	// return Cached(c, s.statCache), nil
-	return c, nil
+	return Cached(c, s.statCache), nil
 }
 
 /*

internal/grpc/services/gateway/storageprovidercache.go

@@ -20,8 +20,11 @@ package gateway
 
 import (
 	"context"
+	"encoding/json"
+	"strings"
 
 	"github.com/ReneKroon/ttlcache/v2"
+	userpb "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1"
 	rpc "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1"
 	provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1"
 	ctxpkg "github.com/cs3org/reva/pkg/ctx"
@@ -40,6 +43,20 @@ func userKey(ctx context.Context, ref *provider.Reference) string {
 	return u.Id.OpaqueId + "!" + ref.ResourceId.StorageId + "!" + ref.ResourceId.OpaqueId + "!" + ref.Path
 }
 
+// RemoveFromCache removes a reference from the cache
+func RemoveFromCache(cache *ttlcache.Cache, user *userpb.User, res *provider.ResourceId) {
+	remove := user.Id.OpaqueId
+	if res != nil {
+		remove += "!" + res.StorageId + "!" + res.OpaqueId
+	}
+
+	for _, key := range cache.GetKeys() {
+		if strings.HasPrefix(key, remove) {
+			_ = cache.Remove(key)
+		}
+	}
+}
+
 // Cached stores responses from the storageprovider inmemory so it doesn't need to do the same request over and over again
 func Cached(c provider.ProviderAPIClient, statCache *ttlcache.Cache) provider.ProviderAPIClient {
 	return &cachedAPIClient{c: c, statCache: statCache}
@@ -56,7 +73,9 @@ func (c *cachedAPIClient) Stat(ctx context.Context, in *provider.StatRequest, op
 	if key != "" {
 		r, err := c.statCache.Get(key)
 		if err == nil {
-			return r.(*provider.StatResponse), nil
+			s := &provider.StatResponse{}
+			err = json.Unmarshal(r.([]byte), s)
+			return s, err
 		}
 	}
 	resp, err := c.c.Stat(ctx, in, opts...)
@@ -68,7 +87,11 @@ func (c *cachedAPIClient) Stat(ctx context.Context, in *provider.StatRequest, op
 	case key == "":
 		return resp, nil
 	default:
-		_ = c.statCache.Set(key, resp)
+		b, err := json.Marshal(resp)
+		if err != nil {
+			return resp, nil
+		}
+		_ = c.statCache.Set(key, b)
 		return resp, nil
 	}
 }

internal/grpc/services/gateway/usershareprovider.go

@@ -22,6 +22,7 @@ import (
 	"context"
 	"path"
 
+	ctxpkg "github.com/cs3org/reva/pkg/ctx"
 	rtrace "github.com/cs3org/reva/pkg/trace"
 	"github.com/cs3org/reva/pkg/utils"
 
@@ -88,6 +89,7 @@ func (s *svc) CreateShare(ctx context.Context, req *collaboration.CreateShareReq
 		}
 	}
 
+	RemoveFromCache(s.statCache, ctxpkg.ContextMustGetUser(ctx), req.ResourceInfo.Id)
 	return res, nil
 }
 
@@ -145,6 +147,8 @@ func (s *svc) RemoveShare(ctx context.Context, req *collaboration.RemoveShareReq
 		}
 	}
 
+	// TODO: How to find the resourceId?
+	RemoveFromCache(s.statCache, ctxpkg.ContextMustGetUser(ctx), nil)
 	return res, nil
 }
 
@@ -228,6 +232,7 @@ func (s *svc) UpdateShare(ctx context.Context, req *collaboration.UpdateShareReq
 		}
 	}
 
+	RemoveFromCache(s.statCache, ctxpkg.ContextMustGetUser(ctx), res.Share.ResourceId)
 	return res, nil
 }
 
@@ -304,6 +309,7 @@ func (s *svc) UpdateReceivedShare(ctx context.Context, req *collaboration.Update
 		}, nil
 	}
 
+	RemoveFromCache(s.statCache, ctxpkg.ContextMustGetUser(ctx), req.Share.Share.ResourceId)
 	return c.UpdateReceivedShare(ctx, req)
 }
 

tests/integration/grpc/gateway_storageprovider_static_test.go

@@ -134,8 +134,8 @@ var _ = Describe("gateway using a static registry and a shard setup", func() {
 			Expect(statRes.Status.Code).To(Equal(rpcv1beta1.Code_CODE_NOT_FOUND))
 
 			res, err := serviceClient.CreateHome(marieCtx, &storagep.CreateHomeRequest{})
-			Expect(res.Status.Code).To(Equal(rpcv1beta1.Code_CODE_OK))
 			Expect(err).ToNot(HaveOccurred())
+			Expect(res.Status.Code).To(Equal(rpcv1beta1.Code_CODE_OK))
 
 			statRes, err = serviceClient.Stat(marieCtx, &storagep.StatRequest{Ref: homeRef})
 			Expect(err).ToNot(HaveOccurred())