Fix recycle to different location (#1541)

cs3org · Apr 19, 2021 · 65208c2 · 65208c2
1 parent f08ed45
commit 65208c2
Show file tree

Hide file tree

Showing 15 changed files with 110 additions and 52 deletions.
diff --git a/changelog/unreleased/fix-restore-to-different-location.md b/changelog/unreleased/fix-restore-to-different-location.md
@@ -0,0 +1,8 @@
+Bugfix: Allow for restoring recycle items to different locations
+
+The CS3 APIs specify a way to restore a recycle item to a different location
+than the original by setting the `restore_path` field in the
+`RestoreRecycleItemRequest`. This field had not been considered until now.
+
+https://github.com/cs3org/reva/pull/1541
+https://cs3org.github.io/cs3apis/
diff --git a/internal/grpc/services/storageprovider/storageprovider.go b/internal/grpc/services/storageprovider/storageprovider.go
@@ -816,7 +816,7 @@ func (s *service) ListRecycle(ctx context.Context, req *provider.ListRecycleRequ
 
 func (s *service) RestoreRecycleItem(ctx context.Context, req *provider.RestoreRecycleItemRequest) (*provider.RestoreRecycleItemResponse, error) {
 	// TODO(labkode): CRITICAL: fill recycle info with storage provider.
-	if err := s.storage.RestoreRecycleItem(ctx, req.Key); err != nil {
+	if err := s.storage.RestoreRecycleItem(ctx, req.Key, req.RestorePath); err != nil {
 		var st *rpc.Status
 		switch err.(type) {
 		case errtypes.IsNotFound:

diff --git a/pkg/storage/fs/owncloud/owncloud.go b/pkg/storage/fs/owncloud/owncloud.go
@@ -2152,7 +2152,7 @@ func (fs *ocfs) ListRecycle(ctx context.Context) ([]*provider.RecycleItem, error
 	return items, nil
 }
 
-func (fs *ocfs) RestoreRecycleItem(ctx context.Context, key string) error {
+func (fs *ocfs) RestoreRecycleItem(ctx context.Context, key, restorePath string) error {
 	// TODO check permission? on what? user must be the owner?
 	log := appctx.GetLogger(ctx)
 	rp, err := fs.getRecyclePath(ctx)
@@ -2167,16 +2167,17 @@ func (fs *ocfs) RestoreRecycleItem(ctx context.Context, key string) error {
 		return nil
 	}
 
-	origin := "/"
-	if v, err := xattr.Get(src, trashOriginPrefix); err != nil {
-		log.Error().Err(err).Str("key", key).Str("path", src).Msg("could not read origin")
-	} else {
-		origin = filepath.Clean(string(v))
+	if restorePath == "" {
+		v, err := xattr.Get(src, trashOriginPrefix)
+		if err != nil {
+			log.Error().Err(err).Str("key", key).Str("path", src).Msg("could not read origin")
+		}
+		restorePath = filepath.Join("/", filepath.Clean(string(v)), strings.TrimSuffix(filepath.Base(src), suffix))
 	}
-	tgt := fs.toInternalPath(ctx, filepath.Join("/", origin, strings.TrimSuffix(filepath.Base(src), suffix)))
+	tgt := fs.toInternalPath(ctx, restorePath)
 	// move back to original location
 	if err := os.Rename(src, tgt); err != nil {
-		log.Error().Err(err).Str("key", key).Str("origin", origin).Str("src", src).Str("tgt", tgt).Msg("could not restore item")
+		log.Error().Err(err).Str("key", key).Str("restorePath", restorePath).Str("src", src).Str("tgt", tgt).Msg("could not restore item")
 		return errors.Wrap(err, "ocfs: could not restore item")
 	}
 	// unset trash origin location in metadata

diff --git a/pkg/storage/fs/s3/s3.go b/pkg/storage/fs/s3/s3.go
@@ -656,6 +656,6 @@ func (fs *s3FS) ListRecycle(ctx context.Context) ([]*provider.RecycleItem, error
 	return nil, errtypes.NotSupported("list recycle")
 }
 
-func (fs *s3FS) RestoreRecycleItem(ctx context.Context, restoreKey string) error {
+func (fs *s3FS) RestoreRecycleItem(ctx context.Context, restoreKey, restorePath string) error {
 	return errtypes.NotSupported("restore recycle")
 }
diff --git a/pkg/storage/storage.go b/pkg/storage/storage.go
@@ -43,7 +43,7 @@ type FS interface {
 	DownloadRevision(ctx context.Context, ref *provider.Reference, key string) (io.ReadCloser, error)
 	RestoreRevision(ctx context.Context, ref *provider.Reference, key string) error
 	ListRecycle(ctx context.Context) ([]*provider.RecycleItem, error)
-	RestoreRecycleItem(ctx context.Context, key string) error
+	RestoreRecycleItem(ctx context.Context, key, restorePath string) error
 	PurgeRecycleItem(ctx context.Context, key string) error
 	EmptyRecycle(ctx context.Context) error
 	GetPathByID(ctx context.Context, id *provider.ResourceId) (string, error)

diff --git a/pkg/storage/utils/decomposedfs/decomposedfs.go b/pkg/storage/utils/decomposedfs/decomposedfs.go
@@ -63,7 +63,7 @@ type Tree interface {
 	// CreateReference(ctx context.Context, node *node.Node, targetURI *url.URL) error
 	Move(ctx context.Context, oldNode *node.Node, newNode *node.Node) (err error)
 	Delete(ctx context.Context, node *node.Node) (err error)
-	RestoreRecycleItemFunc(ctx context.Context, key string) (*node.Node, func() error, error)
+	RestoreRecycleItemFunc(ctx context.Context, key, restorePath string) (*node.Node, func() error, error)
 	PurgeRecycleItemFunc(ctx context.Context, key string) (*node.Node, func() error, error)
 
 	WriteBlob(key string, reader io.Reader) error

diff --git a/pkg/storage/utils/decomposedfs/recycle.go b/pkg/storage/utils/decomposedfs/recycle.go
@@ -146,8 +146,8 @@ func (fs *Decomposedfs) ListRecycle(ctx context.Context) (items []*provider.Recy
 }
 
 // RestoreRecycleItem restores the specified item
-func (fs *Decomposedfs) RestoreRecycleItem(ctx context.Context, key string) error {
-	rn, restoreFunc, err := fs.tp.RestoreRecycleItemFunc(ctx, key)
+func (fs *Decomposedfs) RestoreRecycleItem(ctx context.Context, key, restorePath string) error {
+	rn, restoreFunc, err := fs.tp.RestoreRecycleItemFunc(ctx, key, restorePath)
 	if err != nil {
 		return err
 	}

diff --git a/pkg/storage/utils/decomposedfs/tree/tree.go b/pkg/storage/utils/decomposedfs/tree/tree.go
@@ -345,16 +345,20 @@ func (t *Tree) Delete(ctx context.Context, n *node.Node) (err error) {
 }
 
 // RestoreRecycleItemFunc returns a node and a function to restore it from the trash
-func (t *Tree) RestoreRecycleItemFunc(ctx context.Context, key string) (*node.Node, func() error, error) {
+func (t *Tree) RestoreRecycleItemFunc(ctx context.Context, key, restorePath string) (*node.Node, func() error, error) {
 	rn, trashItem, deletedNodePath, origin, err := t.readRecycleItem(ctx, key)
 	if err != nil {
 		return nil, nil, err
 	}
 
+	if restorePath == "" {
+		restorePath = origin
+	}
+
 	fn := func() error {
 		// link to origin
 		var n *node.Node
-		n, err = t.lookup.NodeFromPath(ctx, origin)
+		n, err = t.lookup.NodeFromPath(ctx, restorePath)
 		if err != nil {
 			return err
 		}

diff --git a/pkg/storage/utils/decomposedfs/tree/tree_test.go b/pkg/storage/utils/decomposedfs/tree/tree_test.go
@@ -55,12 +55,13 @@ var _ = Describe("Tree", func() {
 
 	Context("with an existingfile", func() {
 		var (
-			n *node.Node
+			n            *node.Node
+			originalPath = "dir1/file1"
 		)
 
 		JustBeforeEach(func() {
 			var err error
-			n, err = env.Lookup.NodeFromPath(env.Ctx, "/dir1/file1")
+			n, err = env.Lookup.NodeFromPath(env.Ctx, originalPath)
 			Expect(err).ToNot(HaveOccurred())
 		})
 
@@ -135,18 +136,41 @@ var _ = Describe("Tree", func() {
 					Expect(err).ToNot(HaveOccurred())
 					_, err = os.Stat(n.InternalPath())
 					Expect(err).To(HaveOccurred())
+				})
 
-					_, restoreFunc, err := t.RestoreRecycleItemFunc(env.Ctx, env.Owner.Id.OpaqueId+":"+n.ID)
+				It("restores the file to its original location if the targetPath is empty", func() {
+					_, restoreFunc, err := t.RestoreRecycleItemFunc(env.Ctx, env.Owner.Id.OpaqueId+":"+n.ID, "")
 					Expect(err).ToNot(HaveOccurred())
+
 					Expect(restoreFunc()).To(Succeed())
+
+					originalNode, err := env.Lookup.NodeFromPath(env.Ctx, originalPath)
+					Expect(err).ToNot(HaveOccurred())
+					Expect(originalNode.Exists).To(BeTrue())
 				})
 
-				It("restores the file to its original location", func() {
-					_, err := os.Stat(n.InternalPath())
+				It("restores files to different locations", func() {
+					_, restoreFunc, err := t.RestoreRecycleItemFunc(env.Ctx, env.Owner.Id.OpaqueId+":"+n.ID, "dir1/newLocation")
+					Expect(err).ToNot(HaveOccurred())
+
+					Expect(restoreFunc()).To(Succeed())
+
+					newNode, err := env.Lookup.NodeFromPath(env.Ctx, "dir1/newLocation")
 					Expect(err).ToNot(HaveOccurred())
+					Expect(newNode.Exists).To(BeTrue())
+
+					originalNode, err := env.Lookup.NodeFromPath(env.Ctx, originalPath)
+					Expect(err).ToNot(HaveOccurred())
+					Expect(originalNode.Exists).To(BeFalse())
 				})
+
 				It("removes the file from the trash", func() {
-					_, err := os.Stat(trashPath)
+					_, restoreFunc, err := t.RestoreRecycleItemFunc(env.Ctx, env.Owner.Id.OpaqueId+":"+n.ID, "")
+					Expect(err).ToNot(HaveOccurred())
+
+					Expect(restoreFunc()).To(Succeed())
+
+					_, err = os.Stat(trashPath)
 					Expect(err).To(HaveOccurred())
 				})
 			})

diff --git a/pkg/storage/utils/eosfs/eosfs.go b/pkg/storage/utils/eosfs/eosfs.go
@@ -1284,7 +1284,7 @@ func (fs *eosfs) ListRecycle(ctx context.Context) ([]*provider.RecycleItem, erro
 	return recycleEntries, nil
 }
 
-func (fs *eosfs) RestoreRecycleItem(ctx context.Context, key string) error {
+func (fs *eosfs) RestoreRecycleItem(ctx context.Context, key, restorePath string) error {
 	u, err := getUser(ctx)
 	if err != nil {
 		return errors.Wrap(err, "eos: no user in ctx")

diff --git a/pkg/storage/utils/localfs/localfs.go b/pkg/storage/utils/localfs/localfs.go
@@ -1179,7 +1179,7 @@ func (fs *localfs) ListRecycle(ctx context.Context) ([]*provider.RecycleItem, er
 	return items, nil
 }
 
-func (fs *localfs) RestoreRecycleItem(ctx context.Context, restoreKey string) error {
+func (fs *localfs) RestoreRecycleItem(ctx context.Context, restoreKey, restorePath string) error {
 
 	suffix := path.Ext(restoreKey)
 	if len(suffix) == 0 || !strings.HasPrefix(suffix, ".d") {
@@ -1191,14 +1191,17 @@ func (fs *localfs) RestoreRecycleItem(ctx context.Context, restoreKey string) er
 		return errors.Wrap(err, "localfs: invalid key")
 	}
 
-	var originalPath string
-	if fs.isShareFolder(ctx, filePath) {
-		originalPath = fs.wrapReferences(ctx, filePath)
-	} else {
-		originalPath = fs.wrap(ctx, filePath)
+	var localRestorePath string
+	switch {
+	case restorePath != "":
+		localRestorePath = fs.wrap(ctx, restorePath)
+	case fs.isShareFolder(ctx, filePath):
+		localRestorePath = fs.wrapReferences(ctx, filePath)
+	default:
+		localRestorePath = fs.wrap(ctx, filePath)
 	}
 
-	if _, err = os.Stat(originalPath); err == nil {
+	if _, err = os.Stat(localRestorePath); err == nil {
 		return errors.New("localfs: can't restore - file already exists at original path")
 	}
 
@@ -1210,7 +1213,7 @@ func (fs *localfs) RestoreRecycleItem(ctx context.Context, restoreKey string) er
 		return errors.Wrap(err, "localfs: error stating "+rp)
 	}
 
-	if err := os.Rename(rp, originalPath); err != nil {
+	if err := os.Rename(rp, localRestorePath); err != nil {
 		return errors.Wrap(err, "ocfs: could not restore item")
 	}
 
@@ -1219,7 +1222,7 @@ func (fs *localfs) RestoreRecycleItem(ctx context.Context, restoreKey string) er
 		return errors.Wrap(err, "localfs: error adding entry to DB")
 	}
 
-	return fs.propagate(ctx, originalPath)
+	return fs.propagate(ctx, localRestorePath)
 }
 
 func (fs *localfs) propagate(ctx context.Context, leafPath string) error {

diff --git a/tests/acceptance/expected-failures-on-OCIS-storage.md b/tests/acceptance/expected-failures-on-OCIS-storage.md
@@ -3,13 +3,6 @@
 ### File
 Basic file management like up and download, move, copy, properties, quota, trash, versions and chunking.
 
-#### [Implement Trashbin Feature for ocis storage](https://github.com/owncloud/product/issues/209)
-
--   [apiWebdavEtagPropagation2/restoreFromTrash.feature:48](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiWebdavEtagPropagation2/restoreFromTrash.feature#L48)
--   [apiWebdavEtagPropagation2/restoreFromTrash.feature:49](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiWebdavEtagPropagation2/restoreFromTrash.feature#L49)
--   [apiWebdavEtagPropagation2/restoreFromTrash.feature:90](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiWebdavEtagPropagation2/restoreFromTrash.feature#L90)
--   [apiWebdavEtagPropagation2/restoreFromTrash.feature:91](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiWebdavEtagPropagation2/restoreFromTrash.feature#L91)
-
 #### [QA trashcan cannot delete a deep tree](https://github.com/owncloud/ocis/issues/1077)
 -   [apiTrashbin/trashbinDelete.feature:107](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiTrashbin/trashbinDelete.feature#L107)
 -   [apiTrashbin/trashbinDelete.feature:123](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiTrashbin/trashbinDelete.feature#L123)

diff --git a/tests/acceptance/expected-failures-on-OWNCLOUD-storage.md b/tests/acceptance/expected-failures-on-OWNCLOUD-storage.md
@@ -3,12 +3,6 @@
 ### File
 Basic file management like up and download, move, copy, properties, quota, trash, versions and chunking.
 
-#### [Implement Trashbin Feature for ocis storage](https://github.com/owncloud/product/issues/209)
--   [apiWebdavEtagPropagation2/restoreFromTrash.feature:48](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiWebdavEtagPropagation2/restoreFromTrash.feature#L48)
--   [apiWebdavEtagPropagation2/restoreFromTrash.feature:49](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiWebdavEtagPropagation2/restoreFromTrash.feature#L49)
--   [apiWebdavEtagPropagation2/restoreFromTrash.feature:90](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiWebdavEtagPropagation2/restoreFromTrash.feature#L90)
--   [apiWebdavEtagPropagation2/restoreFromTrash.feature:91](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiWebdavEtagPropagation2/restoreFromTrash.feature#L91)
-
 #### [QA trashcan cannot delete a deep tree](https://github.com/owncloud/ocis/issues/1077)
 -   [apiTrashbin/trashbinDelete.feature:107](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiTrashbin/trashbinDelete.feature#L107)
 -   [apiTrashbin/trashbinDelete.feature:123](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiTrashbin/trashbinDelete.feature#L123)

diff --git a/tests/acceptance/expected-failures-on-S3NG-storage.md b/tests/acceptance/expected-failures-on-S3NG-storage.md
@@ -3,13 +3,6 @@
 ### File
 Basic file management like up and download, move, copy, properties, quota, trash, versions and chunking.
 
-#### [Implement Trashbin Feature for ocis storage](https://github.com/owncloud/product/issues/209)
-
--   [apiWebdavEtagPropagation2/restoreFromTrash.feature:48](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiWebdavEtagPropagation2/restoreFromTrash.feature#L48)
--   [apiWebdavEtagPropagation2/restoreFromTrash.feature:49](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiWebdavEtagPropagation2/restoreFromTrash.feature#L49)
--   [apiWebdavEtagPropagation2/restoreFromTrash.feature:90](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiWebdavEtagPropagation2/restoreFromTrash.feature#L90)
--   [apiWebdavEtagPropagation2/restoreFromTrash.feature:91](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiWebdavEtagPropagation2/restoreFromTrash.feature#L91)
-
 #### [QA trashcan cannot delete a deep tree](https://github.com/owncloud/ocis/issues/1077)
 -   [apiTrashbin/trashbinDelete.feature:107](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiTrashbin/trashbinDelete.feature#L107)
 -   [apiTrashbin/trashbinDelete.feature:123](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiTrashbin/trashbinDelete.feature#L123)

diff --git a/tests/integration/grpc/storageprovider_test.go b/tests/integration/grpc/storageprovider_test.go
@@ -360,6 +360,44 @@ var _ = Describe("storage providers", func() {
 			Expect(statRes.Status.Code).To(Equal(rpcv1beta1.Code_CODE_OK))
 		})
 
+		It("restores resources to a different location", func() {
+			restoreRef := &storagep.Reference{
+				Spec: &storagep.Reference_Path{Path: "/subdirRestored"},
+			}
+			By("deleting an item")
+			res, err := serviceClient.Delete(ctx, &storagep.DeleteRequest{Ref: subdirRef})
+			Expect(err).ToNot(HaveOccurred())
+			Expect(res.Status.Code).To(Equal(rpcv1beta1.Code_CODE_OK))
+
+			By("listing the recycle items")
+			listRes, err := serviceClient.ListRecycle(ctx, &storagep.ListRecycleRequest{})
+			Expect(err).ToNot(HaveOccurred())
+			Expect(listRes.Status.Code).To(Equal(rpcv1beta1.Code_CODE_OK))
+
+			Expect(len(listRes.RecycleItems)).To(Equal(1))
+			item := listRes.RecycleItems[0]
+			Expect(item.Path).To(Equal(subdirPath))
+
+			By("restoring the item to a different location")
+			statRes, err := serviceClient.Stat(ctx, &storagep.StatRequest{Ref: restoreRef})
+			Expect(err).ToNot(HaveOccurred())
+			Expect(statRes.Status.Code).To(Equal(rpcv1beta1.Code_CODE_NOT_FOUND))
+
+			restoreRes, err := serviceClient.RestoreRecycleItem(ctx,
+				&storagep.RestoreRecycleItemRequest{
+					Ref:         subdirRef,
+					Key:         item.Key,
+					RestorePath: "/subdirRestored",
+				},
+			)
+			Expect(err).ToNot(HaveOccurred())
+			Expect(restoreRes.Status.Code).To(Equal(rpcv1beta1.Code_CODE_OK))
+
+			statRes, err = serviceClient.Stat(ctx, &storagep.StatRequest{Ref: restoreRef})
+			Expect(err).ToNot(HaveOccurred())
+			Expect(statRes.Status.Code).To(Equal(rpcv1beta1.Code_CODE_OK))
+		})
+
 		It("purges recycle items resources", func() {
 			By("deleting an item")
 			res, err := serviceClient.Delete(ctx, &storagep.DeleteRequest{Ref: subdirRef})