Add a provider id to mountpoint spaces as well

changelog/unreleased/mountpoint-shares.md

@@ -0,0 +1,5 @@
+Bugfix: Fix space ids of mountpoint shares
+
+We fixed mountpoint share ids by adding a provider id just like with other space types as well.
+
+https://github.com/cs3org/reva/pull/2829

internal/grpc/services/sharesstorageprovider/sharesstorageprovider.go

@@ -56,11 +56,13 @@ func init() {
 type config struct {
 	GatewayAddr               string `mapstructure:"gateway_addr"`
 	UserShareProviderEndpoint string `mapstructure:"usershareprovidersvc"`
+	MountID                   string `mapstructure:"mount_id"`
 }
 
 type service struct {
 	gateway              gateway.GatewayAPIClient
 	sharesProviderClient collaboration.CollaborationAPIClient
+	mountID              string
 }
 
 func (s *service) Close() error {
@@ -93,14 +95,15 @@ func NewDefault(m map[string]interface{}, _ *grpc.Server) (rgrpc.Service, error)
 		return nil, errors.Wrap(err, "sharesstorageprovider: error getting UserShareProvider client")
 	}
 
-	return New(gateway, client)
+	return New(gateway, client, c.MountID)
 }
 
 // New returns a new instance of the SharesStorageProvider service
-func New(gateway gateway.GatewayAPIClient, c collaboration.CollaborationAPIClient) (rgrpc.Service, error) {
+func New(gateway gateway.GatewayAPIClient, c collaboration.CollaborationAPIClient, mountID string) (rgrpc.Service, error) {
 	s := &service{
 		gateway:              gateway,
 		sharesProviderClient: c,
+		mountID:              mountID,
 	}
 	return s, nil
 }
@@ -393,7 +396,7 @@ func (s *service) ListStorageSpaces(ctx context.Context, req *provider.ListStora
 					space := &provider.StorageSpace{
 						Opaque: opaque,
 						Id: &provider.StorageSpaceId{
-							OpaqueId: virtualRootID.StorageId + "!" + virtualRootID.OpaqueId,
+							OpaqueId: storagespace.FormatID(s.mountID, virtualRootID.StorageId, virtualRootID.OpaqueId),
 						},
 						SpaceType: "virtual",
 						//Owner:     &userv1beta1.User{Id: receivedShare.Share.Owner}, // FIXME actually, the mount point belongs to the recipient
@@ -420,7 +423,7 @@ func (s *service) ListStorageSpaces(ctx context.Context, req *provider.ListStora
 				space := &provider.StorageSpace{
 					Opaque: opaque,
 					Id: &provider.StorageSpaceId{
-						OpaqueId: root.StorageId + "!" + root.OpaqueId,
+						OpaqueId: storagespace.FormatID("", root.StorageId, root.OpaqueId),
 					},
 					SpaceType: "grant",
 					Owner:     &userv1beta1.User{Id: receivedShare.Share.Owner},
@@ -466,7 +469,7 @@ func (s *service) ListStorageSpaces(ctx context.Context, req *provider.ListStora
 				space := &provider.StorageSpace{
 					Opaque: opaque,
 					Id: &provider.StorageSpaceId{
-						OpaqueId: root.StorageId + "!" + root.OpaqueId,
+						OpaqueId: storagespace.FormatID(s.mountID, root.StorageId, root.OpaqueId),
 					},
 					SpaceType: "mountpoint",
 					Owner:     &userv1beta1.User{Id: receivedShare.Share.Owner}, // FIXME actually, the mount point belongs to the recipient

internal/grpc/services/sharesstorageprovider/sharesstorageprovider_test.go

@@ -112,6 +112,7 @@ var _ = Describe("Sharesstorageprovider", func() {
 	var (
 		config = map[string]interface{}{
 			"gateway_addr": "127.0.0.1:1234",
+			"mount_id":     "mountid",
 			"driver":       "json",
 			"drivers": map[string]map[string]interface{}{
 				"json": {},
@@ -240,7 +241,7 @@ var _ = Describe("Sharesstorageprovider", func() {
 	})
 
 	JustBeforeEach(func() {
-		p, err := provider.New(gw, sharesProviderClient)
+		p, err := provider.New(gw, sharesProviderClient, "mountid")
 		Expect(err).ToNot(HaveOccurred())
 		s = p.(sprovider.ProviderAPIServer)
 		Expect(s).ToNot(BeNil())

pkg/storagespace/storagespace.go

@@ -87,6 +87,18 @@ func FormatStorageID(providerID, spaceID string) string {
 	return strings.Join([]string{providerID, spaceID}, _storageIDDelimiter)
 }
 
+// FormatID converts provider, storage and opaqueID into the string format
+// <providerid>$<storageid>!<opaquaid> or
+// <storageid>!<opaquaid> in case the provider ID is empty or
+// <opaquaid> in case the storage ID is empty or
+func FormatID(providerID, storageID, opaqueID string) string {
+	id := opaqueID
+	if storageID != "" {
+		id = storageID + _idDelimiter + opaqueID
+	}
+	return FormatStorageID(providerID, id)
+}
+
 // ParseID parses a storage space ID and returns a storageprovider ResourceId.
 // The accepted formats are:
 // <providerid>$<spaceid>!<nodeid> 	-> <providerid>$<spaceid>, <nodeid>

pkg/storagespace/storagespace_test.go

@@ -125,6 +125,42 @@ func TestFormatStorageID(t *testing.T) {
 	}
 }
 
+func TestFormatId(t *testing.T) {
+	tests := []struct {
+		pid         string
+		sid         string
+		oid         string
+		expectation string
+	}{
+		{
+			"",
+			"",
+			"oid",
+			"oid",
+		},
+		{
+			"",
+			"sid",
+			"oid",
+			"sid!oid",
+		},
+		{
+			"pid",
+			"sid",
+			"oid",
+			"pid$sid!oid",
+		},
+	}
+
+	for _, tt := range tests {
+		id := FormatID(tt.pid, tt.sid, tt.oid)
+
+		if id != tt.expectation {
+			t.Errorf("Expected id %s got %s", tt.expectation, id)
+		}
+	}
+}
+
 func TestParseStorageSpaceReference(t *testing.T) {
 	tests := []struct {
 		sRef      string