fix moving of a shared file between share (#2026)

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

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,42 +1113,42 @@ 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),
 		}, nil
 	}
 
 	// 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
 	}
 

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)
@@ -653,7 +647,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)
 

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)
@@ -627,7 +621,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)