diff --git a/internal/grpc/services/sharesstorageprovider/sharesstorageprovider.go b/internal/grpc/services/sharesstorageprovider/sharesstorageprovider.go index d0d1b1fa5c..207992f515 100644 --- a/internal/grpc/services/sharesstorageprovider/sharesstorageprovider.go +++ b/internal/grpc/services/sharesstorageprovider/sharesstorageprovider.go @@ -591,12 +591,6 @@ func (s *service) Move(ctx context.Context, req *provider.MoveRequest) (*provide Interface("destination", req.Destination). Msg("sharesstorageprovider: Got Move request") - if !utils.ResourceIDEqual(req.Source.ResourceId, req.Destination.ResourceId) { - return &provider.MoveResponse{ - Status: status.NewUnimplemented(ctx, nil, "sharesstorageprovider: can not move between shares"), - }, nil - } - // TODO moving inside a shared tree should just be a forward of the move // but when do we rename a mounted share? Does that request even hit us? // - the registry needs to invalidate the alias @@ -650,6 +644,12 @@ func (s *service) Move(ctx context.Context, req *provider.MoveRequest) (*provide }, nil } + if dstReceivedShare.Share.Id.OpaqueId != srcReceivedShare.Share.Id.OpaqueId { + return &provider.MoveResponse{ + Status: status.NewUnimplemented(ctx, nil, "sharesstorageprovider: can not move between shares"), + }, nil + } + gatewayClient, err := s.gatewaySelector.Next() if err != nil { return nil, err diff --git a/internal/grpc/services/sharesstorageprovider/sharesstorageprovider_test.go b/internal/grpc/services/sharesstorageprovider/sharesstorageprovider_test.go index 98db834907..eac7ac8035 100644 --- a/internal/grpc/services/sharesstorageprovider/sharesstorageprovider_test.go +++ b/internal/grpc/services/sharesstorageprovider/sharesstorageprovider_test.go @@ -89,8 +89,8 @@ var _ = Describe("Sharesstorageprovider", func() { OpaqueId: "shareid", }, ResourceId: &sprovider.ResourceId{ - StorageId: "share1-storageid", - SpaceId: "share1-storageid", + StorageId: "storageid", + SpaceId: "storageid", OpaqueId: "shareddir", }, Permissions: &collaboration.SharePermissions{ @@ -102,7 +102,7 @@ var _ = Describe("Sharesstorageprovider", func() { }, }, MountPoint: &sprovider.Reference{ - Path: "oldname", + Path: "share1-shareddir", }, } @@ -110,12 +110,12 @@ var _ = Describe("Sharesstorageprovider", func() { State: collaboration.ShareState_SHARE_STATE_ACCEPTED, Share: &collaboration.Share{ Id: &collaboration.ShareId{ - OpaqueId: "shareidtwo", + OpaqueId: "shareid2", }, ResourceId: &sprovider.ResourceId{ - StorageId: "share1-storageid", - SpaceId: "share1-storageid", - OpaqueId: "shareddir", + StorageId: "storageid", + SpaceId: "storageid", + OpaqueId: "shareddir2", }, Permissions: &collaboration.SharePermissions{ Permissions: &sprovider.ResourcePermissions{ @@ -126,7 +126,7 @@ var _ = Describe("Sharesstorageprovider", func() { }, }, MountPoint: &sprovider.Reference{ - Path: "share1-shareddir", + Path: "share2-shareddir2", }, } @@ -298,6 +298,12 @@ var _ = Describe("Sharesstorageprovider", func() { }) } return resp + case utils.ResourceIDEqual(req.Ref.ResourceId, BaseShareTwo.Share.ResourceId): + resp := &sprovider.ListContainerResponse{ + Status: status.NewOK(context.Background()), + Infos: []*sprovider.ResourceInfo{}, + } + return resp case utils.ResourceIDEqual(req.Ref.ResourceId, BaseShareTwo.Share.ResourceId): return &sprovider.ListContainerResponse{ Status: status.NewOK(context.Background()), @@ -772,11 +778,11 @@ var _ = Describe("Sharesstorageprovider", func() { req := &sprovider.MoveRequest{ Source: &sprovider.Reference{ ResourceId: ShareJail, - Path: "./oldname", + Path: "./share1-shareddir", }, Destination: &sprovider.Reference{ ResourceId: ShareJail, - Path: "./newname", + Path: "./share1-shareddir-renamed", }, } res, err := s.Move(ctx, req) @@ -790,17 +796,37 @@ var _ = Describe("Sharesstorageprovider", func() { It("refuses to move a file between shares", func() { req := &sprovider.MoveRequest{ Source: &sprovider.Reference{ - Path: "/shares/share1-shareddir/share1-shareddir-file", + ResourceId: ShareJail, + Path: "./share1-shareddir/share1-shareddir-file", }, Destination: &sprovider.Reference{ - Path: "/shares/share2-shareddir/share2-shareddir-file", + ResourceId: ShareJail, + Path: "./share2-shareddir2/share1-shareddir-file", }, } res, err := s.Move(ctx, req) gatewayClient.AssertNotCalled(GinkgoT(), "Move", mock.Anything, mock.Anything) Expect(err).ToNot(HaveOccurred()) Expect(res).ToNot(BeNil()) - Expect(res.Status.Code).To(Equal(rpc.Code_CODE_INVALID_ARGUMENT)) + Expect(res.Status.Code).To(Equal(rpc.Code_CODE_UNIMPLEMENTED)) + }) + + It("refuses to move a file between shares resolving to the same space", func() { + req := &sprovider.MoveRequest{ + Source: &sprovider.Reference{ + ResourceId: ShareJail, + Path: "./share1-shareddir/share1-shareddir-file", + }, + Destination: &sprovider.Reference{ + ResourceId: ShareJail, + Path: "./share2-shareddir2/share1-shareddir-file", + }, + } + res, err := s.Move(ctx, req) + gatewayClient.AssertNotCalled(GinkgoT(), "Move", mock.Anything, mock.Anything) + Expect(err).ToNot(HaveOccurred()) + Expect(res).ToNot(BeNil()) + Expect(res.Status.Code).To(Equal(rpc.Code_CODE_UNIMPLEMENTED)) }) It("moves a file", func() {