API Cleanup: SpaceID (#3006)

* Update CS3Apis * adapt the storagespace methods and tests * adapt the providers and sharing * split spaceID on DeleteStorageSpace * owncloudsql: use spaceid instead of storageid Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de> * make sharesstorageprovider tolerate legacy resourceid Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de> * make ocs sharing fill opaqueid when deleting a space membership Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de> * ocdav: tolerate missing sharesstorageproviderid Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de> * bump version of cs3api-validator Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de> * sharesstorageprovider prevent nil Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de> Co-authored-by: Jörn Friedrich Dreyer <jfd@butonic.de>
cs3org · Jul 12, 2022 · 6602900 · 6602900
1 parent 3be25cf
commit 6602900
Show file tree

Hide file tree

Showing 67 changed files with 560 additions and 659 deletions.
diff --git a/.drone.star b/.drone.star
@@ -808,7 +808,7 @@ def cs3ApiValidatorOcis():
             },
             {
                 "name": "cs3api-validator-ocis",
-                "image": "owncloud/cs3api-validator:0.1.0",
+                "image": "owncloud/cs3api-validator:0.2.0",
                 "commands": [
                     "/usr/bin/cs3api-validator /var/lib/cs3api-validator --endpoint=revad-services:19000",
                 ],
@@ -861,7 +861,7 @@ def cs3ApiValidatorS3NG():
             },
             {
                 "name": "cs3api-validator-S3NG",
-                "image": "owncloud/cs3api-validator:0.1.0",
+                "image": "owncloud/cs3api-validator:0.2.0",
                 "commands": [
                     "/usr/bin/cs3api-validator /var/lib/cs3api-validator --endpoint=revad-services:19000",
                 ],

diff --git a/Makefile b/Makefile
@@ -58,8 +58,8 @@ test-integration: build-ci
 
 # This is needed for osx because this os does not support static linking
 # Use the build target without the static flag
-test-integration-osx: build
-	cd tests/integration && go test -race ./...
+test-integration-osx: off build
+	cd tests/integration && go test -race -v ./...
 
 litmus-test-old: build
 	cd tests/oc-integration-tests/local && ../../../cmd/revad/revad -c frontend.toml &

diff --git a/changelog/unreleased/space-id.md b/changelog/unreleased/space-id.md
@@ -0,0 +1,5 @@
+Change: Use spaceID on the cs3api
+
+We introduced a new spaceID field on the cs3api to implement the spaces feature in a cleaner way.
+
+https://github.com/cs3org/reva/pull/3006
diff --git a/cmd/reva/upload.go b/cmd/reva/upload.go
@@ -218,7 +218,7 @@ func uploadCommand() *command {
 
 		info := res2.Info
 
-		fmt.Printf("File uploaded: %s:%s %d %s\n", info.Id.StorageId, info.Id.OpaqueId, info.Size, info.Path)
+		fmt.Printf("File uploaded: %s %d %s\n", info.Id, info.Size, info.Path)
 
 		return nil
 	}

diff --git a/go.mod b/go.mod
@@ -18,7 +18,7 @@ require (
 	github.com/cheggaaa/pb v1.0.29
 	github.com/coreos/go-oidc v2.2.1+incompatible
 	github.com/cs3org/cato v0.0.0-20200828125504-e418fc54dd5e
-	github.com/cs3org/go-cs3apis v0.0.0-20220512100524-551800f020d8
+	github.com/cs3org/go-cs3apis v0.0.0-20220621145831-c38cca0796c2
 	github.com/cubewise-code/go-mime v0.0.0-20200519001935-8c5762b177d8
 	github.com/dgraph-io/ristretto v0.1.0
 	github.com/eventials/go-tus v0.0.0-20200718001131-45c7ec8f5d59
@@ -90,7 +90,6 @@ require (
 go 1.16
 
 replace (
-	github.com/cs3org/go-cs3apis => github.com/micbar/go-cs3apis v0.0.0-20220617090231-703c04619761 // temp fork
 	github.com/eventials/go-tus => github.com/andrewmostello/go-tus v0.0.0-20200314041820-904a9904af9a
 	github.com/oleiade/reflections => github.com/oleiade/reflections v1.0.1
 )
diff --git a/go.sum b/go.sum
@@ -216,6 +216,8 @@ github.com/creack/pty v1.1.9/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ3
 github.com/creack/pty v1.1.11/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ33E=
 github.com/cs3org/cato v0.0.0-20200828125504-e418fc54dd5e h1:tqSPWQeueWTKnJVMJffz4pz0o1WuQxJ28+5x5JgaHD8=
 github.com/cs3org/cato v0.0.0-20200828125504-e418fc54dd5e/go.mod h1:XJEZ3/EQuI3BXTp/6DUzFr850vlxq11I6satRtz0YQ4=
+github.com/cs3org/go-cs3apis v0.0.0-20220621145831-c38cca0796c2 h1:o/ovJzS4pyL/rZgp0MtC4Q7JIle5DikimilTLBw2TjY=
+github.com/cs3org/go-cs3apis v0.0.0-20220621145831-c38cca0796c2/go.mod h1:UXha4TguuB52H14EMoSsCqDj7k8a/t7g4gVP+bgY5LY=
 github.com/cubewise-code/go-mime v0.0.0-20200519001935-8c5762b177d8 h1:Z9lwXumT5ACSmJ7WGnFl+OMLLjpz5uR2fyz7dC255FI=
 github.com/cubewise-code/go-mime v0.0.0-20200519001935-8c5762b177d8/go.mod h1:4abs/jPXcmJzYoYGF91JF9Uq9s/KL5n1jvFDix8KcqY=
 github.com/cyberdelia/templates v0.0.0-20141128023046-ca7fffd4298c/go.mod h1:GyV+0YP4qX0UQ7r2MoYZ+AvYDp12OF5yg4q8rGnyNh4=
@@ -667,8 +669,6 @@ github.com/matttproud/golang_protobuf_extensions v1.0.1 h1:4hp9jkHxhMHkqkrB3Ix0j
 github.com/matttproud/golang_protobuf_extensions v1.0.1/go.mod h1:D8He9yQNgCq6Z5Ld7szi9bcBfOoFv/3dc6xSMkL2PC0=
 github.com/maxymania/go-system v0.0.0-20170110133659-647cc364bf0b h1:Q53idHrTuQDDHyXaxZ6pUl0I9uyD6Z6uKFK3ocX6LzI=
 github.com/maxymania/go-system v0.0.0-20170110133659-647cc364bf0b/go.mod h1:KirJrATYGbTyUwVR26xIkaipRqRcMRXBf8N5dacvGus=
-github.com/micbar/go-cs3apis v0.0.0-20220617090231-703c04619761 h1:vpejExni287rOeBsJxzCDSYJ4d981KXq7HnnaCt2Wn4=
-github.com/micbar/go-cs3apis v0.0.0-20220617090231-703c04619761/go.mod h1:UXha4TguuB52H14EMoSsCqDj7k8a/t7g4gVP+bgY5LY=
 github.com/miekg/dns v1.0.14/go.mod h1:W1PPwlIAgtquWBMBEV9nkV9Cazfe8ScdGz/Lj7v3Nrg=
 github.com/miekg/dns v1.1.26/go.mod h1:bPDLeHnStXmXAq1m/Ch/hvfNHr14JKNPMBo3VZKjuso=
 github.com/miekg/dns v1.1.40/go.mod h1:KNUDUusw/aVsxyTYZM1oqvCicbwhgbNgztCETuNZ7xM=

diff --git a/internal/grpc/interceptors/auth/scope.go b/internal/grpc/interceptors/auth/scope.go
@@ -237,9 +237,16 @@ func extractRefFromListProvidersReq(v *registry.ListStorageProvidersRequest) (*p
 	ref := &provider.Reference{}
 	if v.Opaque != nil && v.Opaque.Map != nil {
 		if e, ok := v.Opaque.Map["storage_id"]; ok {
-			ref.ResourceId = &provider.ResourceId{
-				StorageId: string(e.Value),
+			if ref.ResourceId == nil {
+				ref.ResourceId = &provider.ResourceId{}
+			}
+			ref.ResourceId.StorageId = string(e.Value)
+		}
+		if e, ok := v.Opaque.Map["space_id"]; ok {
+			if ref.ResourceId == nil {
+				ref.ResourceId = &provider.ResourceId{}
 			}
+			ref.ResourceId.SpaceId = string(e.Value)
 		}
 		if e, ok := v.Opaque.Map["opaque_id"]; ok {
 			if ref.ResourceId == nil {

diff --git a/internal/grpc/services/gateway/storageprovider.go b/internal/grpc/services/gateway/storageprovider.go
@@ -231,11 +231,11 @@ func (s *svc) ListStorageSpaces(ctx context.Context, req *provider.ListStorageSp
 	for _, f := range req.Filters {
 		switch f.Type {
 		case provider.ListStorageSpacesRequest_Filter_TYPE_ID:
-			sid, oid, err := storagespace.SplitID(f.GetId().OpaqueId)
+			sid, spid, oid, err := storagespace.SplitID(f.GetId().OpaqueId)
 			if err != nil {
 				continue
 			}
-			filters["storage_id"], filters["opaque_id"] = sid, oid
+			filters["storage_id"], filters["space_id"], filters["opaque_id"] = sid, spid, oid
 		case provider.ListStorageSpacesRequest_Filter_TYPE_OWNER:
 			filters["owner_idp"] = f.GetOwner().Idp
 			filters["owner_id"] = f.GetOwner().OpaqueId
@@ -318,17 +318,14 @@ func (s *svc) DeleteStorageSpace(ctx context.Context, req *provider.DeleteStorag
 		_, purge = opaque.Map["purge"]
 	}
 
-	storageid, opaqeid, err := storagespace.SplitID(req.Id.OpaqueId)
+	rid, err := storagespace.ParseID(req.Id.OpaqueId)
 	if err != nil {
 		return &provider.DeleteStorageSpaceResponse{
-			Status: status.NewStatusFromErrType(ctx, fmt.Sprintf("gateway could not split space id %s", req.GetId().GetOpaqueId()), err),
+			Status: status.NewStatusFromErrType(ctx, fmt.Sprintf("gateway could not parse space id %s", req.GetId().GetOpaqueId()), err),
 		}, nil
 	}
 
-	ref := &provider.Reference{ResourceId: &provider.ResourceId{
-		StorageId: storageid,
-		OpaqueId:  opaqeid,
-	}}
+	ref := &provider.Reference{ResourceId: &rid}
 	c, _, err := s.find(ctx, ref)
 	if err != nil {
 		return &provider.DeleteStorageSpaceResponse{
@@ -359,7 +356,7 @@ func (s *svc) DeleteStorageSpace(ctx context.Context, req *provider.DeleteStorag
 	log.Debug().Msg("purging storage space")
 	// List all shares in this storage space
 	lsRes, err := s.ListShares(ctx, &collaborationv1beta1.ListSharesRequest{
-		Filters: []*collaborationv1beta1.Filter{share.StorageIDFilter(storageid)},
+		Filters: []*collaborationv1beta1.Filter{share.SpaceIDFilter(id.SpaceId)}, // FIXME rename the filter? @c0rby
 	})
 	switch {
 	case err != nil:
@@ -384,7 +381,7 @@ func (s *svc) DeleteStorageSpace(ctx context.Context, req *provider.DeleteStorag
 
 	// List all public shares in this storage space
 	lpsRes, err := s.ListPublicShares(ctx, &linkv1beta1.ListPublicSharesRequest{
-		Filters: []*linkv1beta1.ListPublicSharesRequest_Filter{publicshare.StorageIDFilter(storageid)},
+		Filters: []*linkv1beta1.ListPublicSharesRequest_Filter{publicshare.StorageIDFilter(id.SpaceId)}, // FIXME rename the filter? @c0rby
 	})
 	switch {
 	case err != nil:
@@ -1102,7 +1099,9 @@ func (s *svc) findSpaces(ctx context.Context, ref *provider.Reference) ([]*regis
 	case ref == nil:
 		return nil, errtypes.BadRequest("missing reference")
 	case ref.ResourceId != nil:
-		// no action needed in that case
+		if ref.ResourceId.OpaqueId == "" {
+			ref.ResourceId.OpaqueId = ref.ResourceId.SpaceId
+		}
 	case ref.Path != "": //  TODO implement a mount path cache in the registry?
 		// nothing to do here either
 	default:
@@ -1115,6 +1114,7 @@ func (s *svc) findSpaces(ctx context.Context, ref *provider.Reference) ([]*regis
 	}
 	if ref.ResourceId != nil {
 		filters["storage_id"] = ref.ResourceId.StorageId
+		filters["space_id"] = ref.ResourceId.SpaceId
 		filters["opaque_id"] = ref.ResourceId.OpaqueId
 	}
 
@@ -1132,7 +1132,9 @@ func (s *svc) findSingleSpace(ctx context.Context, ref *provider.Reference) ([]*
 	case ref == nil:
 		return nil, errtypes.BadRequest("missing reference")
 	case ref.ResourceId != nil:
-		// no action needed in that case
+		if ref.ResourceId.OpaqueId == "" {
+			ref.ResourceId.OpaqueId = ref.ResourceId.SpaceId
+		}
 	case ref.Path != "": //  TODO implement a mount path cache in the registry?
 		// nothing to do here either
 	default:
@@ -1146,6 +1148,7 @@ func (s *svc) findSingleSpace(ctx context.Context, ref *provider.Reference) ([]*
 	}
 	if ref.ResourceId != nil {
 		filters["storage_id"] = ref.ResourceId.StorageId
+		filters["space_id"] = ref.ResourceId.SpaceId
 		filters["opaque_id"] = ref.ResourceId.OpaqueId
 	}
 
@@ -1208,6 +1211,7 @@ func unwrap(ref *provider.Reference, mountPoint string, root *provider.ResourceI
 	return &provider.Reference{
 		ResourceId: &provider.ResourceId{
 			StorageId: ref.ResourceId.StorageId,
+			SpaceId:   ref.ResourceId.SpaceId,
 			OpaqueId:  ref.ResourceId.OpaqueId,
 		},
 		Path: ref.Path,

diff --git a/internal/grpc/services/gateway/storageprovidercache.go b/internal/grpc/services/gateway/storageprovidercache.go
@@ -105,7 +105,7 @@ func (c Caches) RemoveStat(user *userpb.User, res *provider.ResourceId) {
 	sid := ""
 	oid := ""
 	if res != nil {
-		sid = "sid:" + res.StorageId
+		sid = "sid:" + res.SpaceId
 		oid = "oid:" + res.OpaqueId
 	}
 
@@ -133,7 +133,7 @@ func (c Caches) RemoveListStorageProviders(res *provider.ResourceId) {
 	if res == nil {
 		return
 	}
-	sid := res.StorageId
+	sid := res.SpaceId
 
 	cache := c[listproviders]
 	for _, key := range cache.GetKeys() {
@@ -182,9 +182,9 @@ func (c *cachedRegistryClient) ListStorageProviders(ctx context.Context, in *reg
 
 	user := ctxpkg.ContextMustGetUser(ctx)
 
-	storageID := sdk.DecodeOpaqueMap(in.Opaque)["storage_id"]
+	spaceID := sdk.DecodeOpaqueMap(in.Opaque)["space_id"]
 
-	key := user.GetId().GetOpaqueId() + "!" + storageID
+	key := user.GetId().GetOpaqueId() + "!" + spaceID
 	if key != "!" {
 		s := &registry.ListStorageProvidersResponse{}
 		if err := pullFromCache(cache, key, s); err == nil {
@@ -198,9 +198,9 @@ func (c *cachedRegistryClient) ListStorageProviders(ctx context.Context, in *reg
 		return nil, err
 	case resp.Status.Code != rpc.Code_CODE_OK:
 		return resp, nil
-	case storageID == "":
+	case spaceID == "":
 		return resp, nil
-	case storageID == utils.ShareStorageProviderID: // TODO do we need to compare providerid and spaceid separately?
+	case spaceID == utils.ShareStorageSpaceID: // TODO do we need to compare providerid and spaceid separately?
 		return resp, nil
 	default:
 		return resp, pushToCache(cache, key, resp)
@@ -230,15 +230,15 @@ type cachedAPIClient struct {
 // a key looks like: uid:1234-1233!sid:5678-5677!oid:9923-9934!path:/path/to/source
 // as you see it adds "uid:"/"sid:"/"oid:" prefixes to the uuids so they can be differentiated
 func statKey(user *userpb.User, ref *provider.Reference, metaDataKeys, fieldMaskPaths []string) string {
-	if ref == nil || ref.ResourceId == nil || ref.ResourceId.StorageId == "" {
+	if ref == nil || ref.ResourceId == nil || ref.ResourceId.SpaceId == "" {
 		return ""
 	}
 
 	key := strings.Builder{}
 	key.WriteString("uid:")
 	key.WriteString(user.Id.OpaqueId)
 	key.WriteString("!sid:")
-	key.WriteString(ref.ResourceId.StorageId)
+	key.WriteString(ref.ResourceId.SpaceId)
 	key.WriteString("!oid:")
 	key.WriteString(ref.ResourceId.OpaqueId)
 	key.WriteString("!path:")

diff --git a/internal/grpc/services/gateway/usershareprovider.go b/internal/grpc/services/gateway/usershareprovider.go
@@ -32,7 +32,6 @@ import (
 	"github.com/cs3org/reva/v2/pkg/rgrpc/status"
 	"github.com/cs3org/reva/v2/pkg/rgrpc/todo/pool"
 	"github.com/cs3org/reva/v2/pkg/storage/utils/grants"
-	"github.com/cs3org/reva/v2/pkg/storagespace"
 	"github.com/cs3org/reva/v2/pkg/utils"
 	"github.com/pkg/errors"
 )
@@ -717,11 +716,11 @@ func refIsSpaceRoot(ref *provider.ResourceId) bool {
 	if ref == nil {
 		return false
 	}
-	if ref.StorageId == "" || ref.OpaqueId == "" {
+	if ref.SpaceId == "" || ref.OpaqueId == "" {
 		return false
 	}
-	_, sid := storagespace.SplitStorageID(ref.GetStorageId())
-	return sid == ref.OpaqueId
+
+	return ref.SpaceId == ref.OpaqueId
 }
 
 func shareIsSpaceRoot(key *collaboration.ShareKey) bool {