Fix Statcache issue

changelog/unreleased/fix-statcache.md

@@ -0,0 +1,5 @@
+Bugfix: Fix Statcache removing
+
+Removing from statcache didn't work correctly with different setups. Unified and fixed
+
+https://github.com/cs3org/reva/pull/2351

internal/grpc/services/gateway/storageprovider.go

@@ -205,12 +205,6 @@ func (s *svc) CreateStorageSpace(ctx context.Context, req *provider.CreateStorag
 		}, nil
 	}
 
-	var r *provider.ResourceId
-	if createRes.StorageSpace != nil {
-		r = createRes.StorageSpace.Root
-	}
-
-	RemoveFromCache(s.statCache, ctxpkg.ContextMustGetUser(ctx), r)
 	return createRes, nil
 }
 

internal/grpc/services/gateway/storageprovidercache.go

@@ -28,6 +28,7 @@ import (
 	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"
+	"github.com/cs3org/reva/pkg/utils"
 	"google.golang.org/grpc"
 )
 
@@ -40,19 +41,34 @@ func userKey(ctx context.Context, ref *provider.Reference) string {
 	if !ok {
 		return ""
 	}
-	return u.Id.OpaqueId + "!" + ref.ResourceId.StorageId + "!" + ref.ResourceId.OpaqueId + "!" + ref.Path
+	return "uid:" + u.Id.OpaqueId + "!sid:" + ref.ResourceId.StorageId + "!oid:" + ref.ResourceId.OpaqueId + "!path:" + ref.Path
 }
 
 // RemoveFromCache removes a reference from the cache
 func RemoveFromCache(cache *ttlcache.Cache, user *userpb.User, res *provider.ResourceId) {
-	remove := user.Id.OpaqueId
+	uid := "uid:" + user.Id.OpaqueId
+	sid := ""
+	oid := ""
 	if res != nil {
-		remove += "!" + res.StorageId + "!" + res.OpaqueId
+		sid = "sid:" + res.StorageId
+		oid = "oid:" + res.OpaqueId
 	}
 
-	for _, key := range cache.GetKeys() {
-		if strings.HasPrefix(key, remove) {
+	keys := cache.GetKeys()
+	for _, key := range keys {
+		if strings.Contains(key, uid) {
 			_ = cache.Remove(key)
+			continue
+		}
+
+		if sid != "" && strings.Contains(key, sid) {
+			_ = cache.Remove(key)
+			continue
+		}
+
+		if oid != "" && strings.Contains(key, oid) {
+			_ = cache.Remove(key)
+			continue
 		}
 	}
 }
@@ -86,6 +102,11 @@ func (c *cachedAPIClient) Stat(ctx context.Context, in *provider.StatRequest, op
 		return resp, nil
 	case key == "":
 		return resp, nil
+	case strings.Contains(key, "sid:"+utils.ShareStorageProviderID):
+		// We cannot cache shares at the moment:
+		// we do not know when to invalidate them
+		// FIXME: find a way to cache/invalidate them too
+		return resp, nil
 	default:
 		b, err := json.Marshal(resp)
 		if err != nil {

internal/grpc/services/gateway/usershareprovider.go

@@ -147,8 +147,7 @@ func (s *svc) RemoveShare(ctx context.Context, req *collaboration.RemoveShareReq
 		}
 	}
 
-	// TODO: How to find the resourceId?
-	RemoveFromCache(s.statCache, ctxpkg.ContextMustGetUser(ctx), nil)
+	RemoveFromCache(s.statCache, ctxpkg.ContextMustGetUser(ctx), share.ResourceId)
 	return res, nil
 }
 

internal/grpc/services/publicstorageprovider/publicstorageprovider.go

@@ -48,10 +48,6 @@ func init() {
 	rgrpc.Register("publicstorageprovider", New)
 }
 
-// StorageID is used to identify resources handled by the public storage provider.
-// Used in the publiclink scope
-const StorageID = "7993447f-687f-490d-875c-ac95e89a62a4"
-
 type config struct {
 	GatewayAddr string `mapstructure:"gateway_addr"`
 }
@@ -321,7 +317,7 @@ func (s *service) ListStorageSpaces(ctx context.Context, req *provider.ListStora
 			}
 		case provider.ListStorageSpacesRequest_Filter_TYPE_ID:
 			spaceid, _ := utils.SplitStorageSpaceID(f.GetId().OpaqueId)
-			if spaceid != StorageID {
+			if spaceid != utils.PublicStorageProviderID {
 				return &provider.ListStorageSpacesResponse{
 					Status: &rpc.Status{Code: rpc.Code_CODE_OK},
 				}, nil
@@ -333,13 +329,13 @@ func (s *service) ListStorageSpaces(ctx context.Context, req *provider.ListStora
 		Status: &rpc.Status{Code: rpc.Code_CODE_OK},
 		StorageSpaces: []*provider.StorageSpace{{
 			Id: &provider.StorageSpaceId{
-				OpaqueId: StorageID,
+				OpaqueId: utils.PublicStorageProviderID,
 			},
 			SpaceType: "public",
 			// return the actual resource id?
 			Root: &provider.ResourceId{
-				StorageId: StorageID,
-				OpaqueId:  StorageID,
+				StorageId: utils.PublicStorageProviderID,
+				OpaqueId:  utils.PublicStorageProviderID,
 			},
 			Name:  "Public shares",
 			Mtime: &typesv1beta1.Timestamp{}, // do we need to update it?
@@ -576,7 +572,7 @@ func (s *service) augmentStatResponse(ctx context.Context, res *provider.StatRes
 
 // setPublicStorageID encodes the actual spaceid and nodeid as an opaqueid in the publicstorageprovider space
 func (s *service) setPublicStorageID(info *provider.ResourceInfo, shareToken string) {
-	info.Id.StorageId = StorageID
+	info.Id.StorageId = utils.PublicStorageProviderID
 	info.Id.OpaqueId = shareToken + "/" + info.Id.OpaqueId
 }
 

internal/grpc/services/sharesstorageprovider/sharesstorageprovider.go

@@ -314,7 +314,7 @@ func (s *service) ListStorageSpaces(ctx context.Context, req *provider.ListStora
 			}
 		case provider.ListStorageSpacesRequest_Filter_TYPE_ID:
 			spaceid, _ := utils.SplitStorageSpaceID(f.GetId().OpaqueId)
-			if spaceid != "a0ca6a90-a365-4782-871e-d44447bbc668" {
+			if spaceid != utils.ShareStorageProviderID {
 				return &provider.ListStorageSpacesResponse{
 					// a specific id was requested, return not found instead of empty list
 					Status: &rpc.Status{Code: rpc.Code_CODE_NOT_FOUND},
@@ -349,7 +349,7 @@ func (s *service) ListStorageSpaces(ctx context.Context, req *provider.ListStora
 			// return the actual resource id
 			//Root: lsRes.Shares[i].Share.ResourceId,
 			Root: &provider.ResourceId{
-				StorageId: "a0ca6a90-a365-4782-871e-d44447bbc668",
+				StorageId: utils.ShareStorageProviderID,
 				OpaqueId:  lsRes.Shares[i].Share.ResourceId.OpaqueId,
 			},
 			// TODO in the future the spaces registry will handle the alias for share spaces.
@@ -425,7 +425,7 @@ func (s *service) Delete(ctx context.Context, req *provider.DeleteRequest) (*pro
 	}
 
 	// the root of a share always has the path "."
-	if req.Ref.ResourceId.StorageId == "a0ca6a90-a365-4782-871e-d44447bbc668" && req.Ref.Path == "." {
+	if req.Ref.ResourceId.StorageId == utils.ShareStorageProviderID && req.Ref.Path == "." {
 		err := s.rejectReceivedShare(ctx, receivedShare)
 		if err != nil {
 			return &provider.DeleteResponse{
@@ -470,7 +470,7 @@ func (s *service) Move(ctx context.Context, req *provider.MoveRequest) (*provide
 	// can we do a rename
 	if utils.ResourceIDEqual(req.Source.ResourceId, req.Destination.ResourceId) &&
 		// only if we are responsible for the space
-		req.Source.ResourceId.StorageId == "a0ca6a90-a365-4782-871e-d44447bbc668" &&
+		req.Source.ResourceId.StorageId == utils.ShareStorageProviderID &&
 		// only if the source path has no path segment
 		req.Source.Path == "." &&
 		// only if the destination is a dot followed by a single path segment, e.g. './new'

internal/grpc/services/sharesstorageprovider/sharesstorageprovider_test.go

@@ -47,7 +47,7 @@ var (
 		State: collaboration.ShareState_SHARE_STATE_ACCEPTED,
 		Share: &collaboration.Share{
 			ResourceId: &sprovider.ResourceId{
-				StorageId: "a0ca6a90-a365-4782-871e-d44447bbc668", // <- sharestorageproviderid
+				StorageId: utils.ShareStorageProviderID,
 				OpaqueId:  "shareddir",
 			},
 			Permissions: &collaboration.SharePermissions{
@@ -63,7 +63,7 @@ var (
 		State: collaboration.ShareState_SHARE_STATE_ACCEPTED,
 		Share: &collaboration.Share{
 			ResourceId: &sprovider.ResourceId{
-				StorageId: "a0ca6a90-a365-4782-871e-d44447bbc668",
+				StorageId: utils.ShareStorageProviderID,
 				OpaqueId:  "shareddir",
 			},
 			Permissions: &collaboration.SharePermissions{

pkg/utils/utils.go

@@ -48,6 +48,12 @@ var (
 	// GlobalRegistry configures a service registry globally accessible. It defaults to a memory registry. The usage of
 	// globals is not encouraged, and this is a workaround until the PR is out of a draft state.
 	GlobalRegistry registry.Registry = memory.New(map[string]interface{}{})
+
+	// ShareStorageProviderID is the id used by the sharestorageprovider
+	ShareStorageProviderID = "a0ca6a90-a365-4782-871e-d44447bbc668"
+
+	// PublicStorageProviderID is the id used by the sharestorageprovider
+	PublicStorageProviderID = "7993447f-687f-490d-875c-ac95e89a62a4"
 )
 
 // Skip  evaluates whether a source endpoint contains any of the prefixes.