add util function to split storage space id

Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>
cs3org · Jul 8, 2021 · ce7845b · ce7845b
1 parent a5d4def
commit ce7845b
Show file tree

Hide file tree

Showing 4 changed files with 31 additions and 20 deletions.
diff --git a/internal/grpc/services/gateway/storageprovider.go b/internal/grpc/services/gateway/storageprovider.go
@@ -133,16 +133,16 @@ func (s *svc) ListStorageSpaces(ctx context.Context, req *provider.ListStorageSp
 
 	if id != nil {
 		// query that specific storage provider
-		parts := strings.SplitN(id.OpaqueId, "!", 2)
-		if len(parts) != 2 {
+		storageid, opaqeid, err := utils.SplitStorageSpaceID(id.OpaqueId)
+		if err != nil {
 			return &provider.ListStorageSpacesResponse{
 				Status: status.NewInvalidArg(ctx, "space id must be separated by !"),
 			}, nil
 		}
 		res, err := c.GetStorageProviders(ctx, &registry.GetStorageProvidersRequest{
 			Ref: &provider.Reference{ResourceId: &provider.ResourceId{
-				StorageId: parts[0], // FIXME REFERENCE the StorageSpaceId is a storageid + an opaqueid
-				OpaqueId:  parts[1],
+				StorageId: storageid,
+				OpaqueId:  opaqeid,
 			}},
 		})
 		if err != nil {
@@ -264,15 +264,15 @@ func (s *svc) UpdateStorageSpace(ctx context.Context, req *provider.UpdateStorag
 func (s *svc) DeleteStorageSpace(ctx context.Context, req *provider.DeleteStorageSpaceRequest) (*provider.DeleteStorageSpaceResponse, error) {
 	log := appctx.GetLogger(ctx)
 	// TODO: needs to be fixed
-	parts := strings.SplitN(req.Id.OpaqueId, "!", 2)
-	if len(parts) != 2 {
+	storageid, opaqeid, err := utils.SplitStorageSpaceID(req.Id.OpaqueId)
+	if err != nil {
 		return &provider.DeleteStorageSpaceResponse{
 			Status: status.NewInvalidArg(ctx, "space id must be separated by !"),
 		}, nil
 	}
 	c, err := s.find(ctx, &provider.Reference{ResourceId: &provider.ResourceId{
-		StorageId: parts[0], // FIXME REFERENCE the StorageSpaceId is a storageid + a opaqueid
-		OpaqueId:  parts[1],
+		StorageId: storageid,
+		OpaqueId:  opaqeid,
 	}})
 	if err != nil {
 		return &provider.DeleteStorageSpaceResponse{

diff --git a/pkg/rhttp/datatx/manager/spaces/spaces.go b/pkg/rhttp/datatx/manager/spaces/spaces.go
@@ -31,6 +31,7 @@ import (
 	"github.com/cs3org/reva/pkg/rhttp/datatx/utils/download"
 	"github.com/cs3org/reva/pkg/rhttp/router"
 	"github.com/cs3org/reva/pkg/storage"
+	"github.com/cs3org/reva/pkg/utils"
 	"github.com/mitchellh/mapstructure"
 	"github.com/pkg/errors"
 )
@@ -83,21 +84,18 @@ func (m *manager) Handler(fs storage.FS) (http.Handler, error) {
 
 			// TODO refactor: pass Reference to Upload & GetOrHeadFile
 			// build a storage space reference
-			parts := strings.SplitN(spaceID, "!", 2)
-			if len(parts) != 2 {
+			storageid, opaqeid, err := utils.SplitStorageSpaceID(spaceID)
+			if err != nil {
 				sublog.Error().Msg("space id must be separated by !")
 				w.WriteHeader(http.StatusBadRequest)
 				return
 			}
 
 			ref := &provider.Reference{
-				ResourceId: &provider.ResourceId{
-					StorageId: parts[0],
-					OpaqueId:  parts[1],
-				},
-				Path: fn,
+				ResourceId: &provider.ResourceId{StorageId: storageid, OpaqueId: opaqeid},
+				Path:       fn,
 			}
-			err := fs.Upload(ctx, ref, r.Body)
+			err = fs.Upload(ctx, ref, r.Body)
 			switch v := err.(type) {
 			case nil:
 				w.WriteHeader(http.StatusOK)

diff --git a/pkg/rhttp/datatx/utils/download/download.go b/pkg/rhttp/datatx/utils/download/download.go
@@ -26,7 +26,6 @@ import (
 	"net/http"
 	"path"
 	"strconv"
-	"strings"
 
 	provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1"
 	"github.com/cs3org/reva/pkg/appctx"
@@ -55,10 +54,10 @@ func GetOrHeadFile(w http.ResponseWriter, r *http.Request, fs storage.FS, spaceI
 		ref = &provider.Reference{Path: path.Join("/", fn)}
 	} else {
 		// build a storage space reference
-		parts := strings.SplitN(spaceID, "!", 2)
-		if len(parts) == 2 {
+		storageid, opaqeid, err := utils.SplitStorageSpaceID(spaceID)
+		if err != nil {
 			ref = &provider.Reference{
-				ResourceId: &provider.ResourceId{StorageId: parts[0], OpaqueId: parts[1]},
+				ResourceId: &provider.ResourceId{StorageId: storageid, OpaqueId: opaqeid},
 				// ensure the relative path starts with '.'
 				Path: utils.MakeRelativePath(fn),
 			}

diff --git a/pkg/utils/utils.go b/pkg/utils/utils.go
@@ -19,6 +19,7 @@
 package utils
 
 import (
+	"fmt"
 	"math/rand"
 	"net"
 	"net/http"
@@ -227,3 +228,16 @@ func MakeRelativePath(p string) string {
 	}
 	return "." + p
 }
+
+// SplitStorageSpaceID can be used to split `storagespaceid` into `storageid` and `nodeid`
+// Currently they are built using `<storageid>!<nodeid>` in the decomposedfs, but other drivers might return different ids.
+// any place in the code that relies on this function should instead use the storage registry to look up the responsible storage provider.
+// Note: This would in effect change the storage registry into a storage space registry.
+func SplitStorageSpaceID(ssid string) (storageid, nodeid string, err error) {
+	// query that specific storage provider
+	parts := strings.SplitN(ssid, "!", 2)
+	if len(parts) != 2 {
+		return "", "", fmt.Errorf("storage space id must be separated by '!'")
+	}
+	return parts[0], parts[1], nil
+}