diff --git a/changelog/unreleased/mountpoint-shares.md b/changelog/unreleased/mountpoint-shares.md new file mode 100644 index 0000000000..339682f84b --- /dev/null +++ b/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 diff --git a/internal/grpc/services/sharesstorageprovider/sharesstorageprovider.go b/internal/grpc/services/sharesstorageprovider/sharesstorageprovider.go index ec8f2f63b8..59c15ff539 100644 --- a/internal/grpc/services/sharesstorageprovider/sharesstorageprovider.go +++ b/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 diff --git a/internal/grpc/services/sharesstorageprovider/sharesstorageprovider_test.go b/internal/grpc/services/sharesstorageprovider/sharesstorageprovider_test.go index 8e1c28ead9..1fa1a215ff 100644 --- a/internal/grpc/services/sharesstorageprovider/sharesstorageprovider_test.go +++ b/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()) diff --git a/pkg/storagespace/storagespace.go b/pkg/storagespace/storagespace.go index 24454b3bed..b42c46f8f9 100644 --- a/pkg/storagespace/storagespace.go +++ b/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 +// $! or +// ! in case the provider ID is empty or +// 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: // $! -> $, diff --git a/pkg/storagespace/storagespace_test.go b/pkg/storagespace/storagespace_test.go index d07d8737be..636d419c33 100644 --- a/pkg/storagespace/storagespace_test.go +++ b/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