diff --git a/changelog/unreleased/fix-move-of-shared-file.md b/changelog/unreleased/fix-move-of-shared-file.md new file mode 100644 index 0000000000..b4df5e43c1 --- /dev/null +++ b/changelog/unreleased/fix-move-of-shared-file.md @@ -0,0 +1,5 @@ +Bugfix: Fix moving of a shared file + +As the share receiver, moving a shared file to another share was not possible. + +https://github.com/cs3org/reva/pull/2026 diff --git a/internal/grpc/services/gateway/storageprovider.go b/internal/grpc/services/gateway/storageprovider.go index 5dd46af077..d96bf898db 100644 --- a/internal/grpc/services/gateway/storageprovider.go +++ b/internal/grpc/services/gateway/storageprovider.go @@ -1029,37 +1029,61 @@ func (s *svc) Move(ctx context.Context, req *provider.MoveRequest) (*provider.Mo dshareName, dshareChild := s.splitShare(ctx, dp) log.Debug().Msgf("srcpath:%s dstpath:%s srcsharename:%s srcsharechild: %s dstsharename:%s dstsharechild:%s ", p, dp, shareName, shareChild, dshareName, dshareChild) - if shareName != dshareName { - err := errtypes.BadRequest("gateway: move: src and dst points to different targets") + srcStatReq := &provider.StatRequest{Ref: &provider.Reference{Path: shareName}} + srcStatRes, err := s.stat(ctx, srcStatReq) + if err != nil { return &provider.MoveResponse{ - Status: status.NewStatusFromErrType(ctx, "gateway: error moving", err), + Status: status.NewInternal(ctx, err, "gateway: error stating ref:"+srcStatReq.Ref.String()), }, nil + } + if srcStatRes.Status.Code != rpc.Code_CODE_OK { + return &provider.MoveResponse{ + Status: srcStatRes.Status, + }, nil } - statReq := &provider.StatRequest{Ref: &provider.Reference{Path: shareName}} - statRes, err := s.stat(ctx, statReq) + dstStatReq := &provider.StatRequest{Ref: &provider.Reference{Path: dshareName}} + dstStatRes, err := s.stat(ctx, dstStatReq) if err != nil { return &provider.MoveResponse{ - Status: status.NewInternal(ctx, err, "gateway: error stating ref:"+statReq.Ref.String()), + Status: status.NewInternal(ctx, err, "gateway: error stating ref:"+srcStatReq.Ref.String()), }, nil } - if statRes.Status.Code != rpc.Code_CODE_OK { + if dstStatRes.Status.Code != rpc.Code_CODE_OK { return &provider.MoveResponse{ - Status: statRes.Status, + Status: srcStatRes.Status, }, nil } - ri, protocol, err := s.checkRef(ctx, statRes.Info) + srcRi, srcProtocol, err := s.checkRef(ctx, srcStatRes.Info) if err != nil { return &provider.MoveResponse{ - Status: status.NewStatusFromErrType(ctx, "error resolving reference "+statRes.Info.Target, err), + Status: status.NewStatusFromErrType(ctx, "error resolving reference "+srcStatRes.Info.Target, err), }, nil } - if protocol == "webdav" { - err = s.webdavRefMove(ctx, statRes.Info.Target, shareChild, dshareChild) + if srcProtocol == "webdav" { + err = s.webdavRefMove(ctx, dstStatRes.Info.Target, shareChild, dshareChild) + if err != nil { + return &provider.MoveResponse{ + Status: status.NewInternal(ctx, err, "gateway: error moving resource on webdav host: "+p), + }, nil + } + return &provider.MoveResponse{ + Status: status.NewOK(ctx), + }, nil + } + dstRi, dstProtocol, err := s.checkRef(ctx, dstStatRes.Info) + if err != nil { + return &provider.MoveResponse{ + Status: status.NewStatusFromErrType(ctx, "error resolving reference "+srcStatRes.Info.Target, err), + }, nil + } + + if dstProtocol == "webdav" { + err = s.webdavRefMove(ctx, dstStatRes.Info.Target, shareChild, dshareChild) if err != nil { return &provider.MoveResponse{ Status: status.NewInternal(ctx, err, "gateway: error moving resource on webdav host: "+p), @@ -1071,10 +1095,10 @@ func (s *svc) Move(ctx context.Context, req *provider.MoveRequest) (*provider.Mo } src := &provider.Reference{ - Path: path.Join(ri.Path, shareChild), + Path: path.Join(srcRi.Path, shareChild), } dst := &provider.Reference{ - Path: path.Join(ri.Path, dshareChild), + Path: path.Join(dstRi.Path, dshareChild), } req.Source = src @@ -1089,14 +1113,14 @@ func (s *svc) Move(ctx context.Context, req *provider.MoveRequest) (*provider.Mo } func (s *svc) move(ctx context.Context, req *provider.MoveRequest) (*provider.MoveResponse, error) { - srcList, err := s.findProviders(ctx, req.Source) + srcProviders, err := s.findProviders(ctx, req.Source) if err != nil { return &provider.MoveResponse{ Status: status.NewStatusFromErrType(ctx, "move src="+req.Source.String(), err), }, nil } - dstList, err := s.findProviders(ctx, req.Destination) + dstProviders, err := s.findProviders(ctx, req.Destination) if err != nil { return &provider.MoveResponse{ Status: status.NewStatusFromErrType(ctx, "move dst="+req.Destination.String(), err), @@ -1104,27 +1128,27 @@ func (s *svc) move(ctx context.Context, req *provider.MoveRequest) (*provider.Mo } // if providers are not the same we do not implement cross storage move yet. - if len(srcList) != 1 || len(dstList) != 1 { + if len(srcProviders) != 1 || len(dstProviders) != 1 { res := &provider.MoveResponse{ Status: status.NewUnimplemented(ctx, nil, "gateway: cross storage copy not yet implemented"), } return res, nil } - srcP, dstP := srcList[0], dstList[0] + srcProvider, dstProvider := srcProviders[0], dstProviders[0] // if providers are not the same we do not implement cross storage copy yet. - if srcP.Address != dstP.Address { + if srcProvider.Address != dstProvider.Address { res := &provider.MoveResponse{ Status: status.NewUnimplemented(ctx, nil, "gateway: cross storage copy not yet implemented"), } return res, nil } - c, err := s.getStorageProviderClient(ctx, srcP) + c, err := s.getStorageProviderClient(ctx, srcProvider) if err != nil { return &provider.MoveResponse{ - Status: status.NewInternal(ctx, err, "error connecting to storage provider="+srcP.Address), + Status: status.NewInternal(ctx, err, "error connecting to storage provider="+srcProvider.Address), }, nil } diff --git a/tests/acceptance/expected-failures-on-OCIS-storage.md b/tests/acceptance/expected-failures-on-OCIS-storage.md index 25ba737c35..9712ee1f27 100644 --- a/tests/acceptance/expected-failures-on-OCIS-storage.md +++ b/tests/acceptance/expected-failures-on-OCIS-storage.md @@ -303,12 +303,6 @@ Synchronization features like etag propagation, setting mtime and locking files ### Share File and sync features in a shared scenario -#### [etags don't change for a share receiver](https://github.com/owncloud/product/issues/243) -- [apiWebdavEtagPropagation1/moveFileFolder.feature:244](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiWebdavEtagPropagation1/moveFileFolder.feature#L244) -- [apiWebdavEtagPropagation1/moveFileFolder.feature:245](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiWebdavEtagPropagation1/moveFileFolder.feature#L245) -- [apiWebdavEtagPropagation1/moveFileFolder.feature:314](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiWebdavEtagPropagation1/moveFileFolder.feature#L314) -- [apiWebdavEtagPropagation1/moveFileFolder.feature:315](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiWebdavEtagPropagation1/moveFileFolder.feature#L315) - ### [Different response containing exact and non exact match in response of getting sharees](https://github.com/owncloud/ocis/issues/2376) - [apiSharees/sharees.feature:350](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiSharees/sharees.feature#L350) - [apiSharees/sharees.feature:351](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiSharees/sharees.feature#L351) @@ -659,7 +653,6 @@ _requires a [CS3 user provisioning api that can update the quota for a user](htt #### [not possible to move file into a received folder](https://github.com/owncloud/ocis/issues/764) - [apiShareOperationsToShares1/changingFilesShare.feature:24](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiShareOperationsToShares1/changingFilesShare.feature#L24) - [apiShareOperationsToShares1/changingFilesShare.feature:25](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiShareOperationsToShares1/changingFilesShare.feature#L25) -- [apiShareOperationsToShares1/changingFilesShare.feature:66](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiShareOperationsToShares1/changingFilesShare.feature#L66) - [apiShareOperationsToShares1/changingFilesShare.feature:82](https://github.com/owncloud/core/blob/master/test/acceptance/features/apiShareOperationsToShares1/changingFilesShare.feature#L82) - [apiShareOperationsToShares1/changingFilesShare.feature:98](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiShareOperationsToShares1/changingFilesShare.feature#L98) diff --git a/tests/acceptance/expected-failures-on-S3NG-storage.md b/tests/acceptance/expected-failures-on-S3NG-storage.md index 423d8aa226..431c61561e 100644 --- a/tests/acceptance/expected-failures-on-S3NG-storage.md +++ b/tests/acceptance/expected-failures-on-S3NG-storage.md @@ -306,12 +306,6 @@ Synchronization features like etag propagation, setting mtime and locking files ### Share File and sync features in a shared scenario -#### [etags don't change for a share receiver](https://github.com/owncloud/product/issues/243) -- [apiWebdavEtagPropagation1/moveFileFolder.feature:244](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiWebdavEtagPropagation1/moveFileFolder.feature#L244) -- [apiWebdavEtagPropagation1/moveFileFolder.feature:245](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiWebdavEtagPropagation1/moveFileFolder.feature#L245) -- [apiWebdavEtagPropagation1/moveFileFolder.feature:314](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiWebdavEtagPropagation1/moveFileFolder.feature#L314) -- [apiWebdavEtagPropagation1/moveFileFolder.feature:315](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiWebdavEtagPropagation1/moveFileFolder.feature#L315) - ### [Different response containing exact and non exact match in response of getting sharees](https://github.com/owncloud/ocis/issues/2376) - [apiSharees/sharees.feature:350](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiSharees/sharees.feature#L350) - [apiSharees/sharees.feature:351](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiSharees/sharees.feature#L351) @@ -633,7 +627,6 @@ _requires a [CS3 user provisioning api that can update the quota for a user](htt - [apiShareOperationsToShares1/changingFilesShare.feature:24](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiShareOperationsToShares1/changingFilesShare.feature#L24) - [apiShareOperationsToShares1/changingFilesShare.feature:25](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiShareOperationsToShares1/changingFilesShare.feature#L25) -- [apiShareOperationsToShares1/changingFilesShare.feature:66](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiShareOperationsToShares1/changingFilesShare.feature#L66) - [apiShareOperationsToShares1/changingFilesShare.feature:82](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiShareOperationsToShares1/changingFilesShare.feature#L82) - [apiShareOperationsToShares1/changingFilesShare.feature:98](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiShareOperationsToShares1/changingFilesShare.feature#L98)