Rework file deletion

changelog/unreleased/fix-blob-deletion.md

@@ -0,0 +1,6 @@
+Bugfix: Actually remove blobs when purging
+
+Blobs were not being deleted properly on purge.
+Now if a folder gets purged all its children will be deleted
+
+https://github.com/cs3org/reva/pull/2868

pkg/storage/fs/ocis/blobstore/blobstore.go

@@ -26,6 +26,7 @@ import (
 
 	"github.com/cs3org/reva/v2/pkg/storage/utils/decomposedfs/lookup"
 	"github.com/cs3org/reva/v2/pkg/storage/utils/decomposedfs/node"
+	"github.com/cs3org/reva/v2/pkg/utils"
 	"github.com/pkg/errors"
 )
 
@@ -79,8 +80,7 @@ func (bs *Blobstore) Download(node *node.Node) (io.ReadCloser, error) {
 
 // Delete deletes a blob from the blobstore
 func (bs *Blobstore) Delete(node *node.Node) error {
-	err := os.Remove(bs.path(node))
-	if err != nil {
+	if err := utils.RemoveItem(bs.path(node)); err != nil {
 		return errors.Wrapf(err, "could not delete blob '%s'", bs.path(node))
 	}
 	return nil

pkg/storage/utils/decomposedfs/recycle.go

@@ -288,11 +288,12 @@ func (fs *Decomposedfs) RestoreRecycleItem(ctx context.Context, ref *provider.Re
 	return restoreFunc()
 }
 
-// PurgeRecycleItem purges the specified item
+// PurgeRecycleItem purges the specified item and all its children
 func (fs *Decomposedfs) PurgeRecycleItem(ctx context.Context, ref *provider.Reference, key, relativePath string) error {
 	if ref == nil {
 		return errtypes.BadRequest("missing reference, needs a space id")
 	}
+
 	rn, purgeFunc, err := fs.tp.PurgeRecycleItemFunc(ctx, ref.ResourceId.OpaqueId, key, relativePath)
 	if err != nil {
 		if errors.Is(err, iofs.ErrNotExist) {
@@ -321,6 +322,17 @@ func (fs *Decomposedfs) EmptyRecycle(ctx context.Context, ref *provider.Referenc
 	if ref == nil || ref.ResourceId == nil || ref.ResourceId.OpaqueId == "" {
 		return errtypes.BadRequest("spaceid must be set")
 	}
+
+	items, err := fs.ListRecycle(ctx, ref, "", "/")
+	if err != nil {
+		return err
+	}
+
+	for _, i := range items {
+		if err := fs.PurgeRecycleItem(ctx, ref, i.Key, ""); err != nil {
+			return err
+		}
+	}
 	// TODO what permission should we check? we could check the root node of the user? or the owner permissions on his home root node?
 	// The current impl will wipe your own trash. or when no user provided the trash of 'root'
 	return os.RemoveAll(fs.getRecycleRoot(ctx, ref.ResourceId.StorageId))

pkg/storage/utils/decomposedfs/tree/tree.go

@@ -538,10 +538,24 @@ func (t *Tree) PurgeRecycleItemFunc(ctx context.Context, spaceid, key string, pa
 		return nil, nil, err
 	}
 
+	// only the root node is trashed, the rest is still in normal file system
+	children, err := os.ReadDir(deletedNodePath)
+	var nodes []*node.Node
+	for _, c := range children {
+		n, _, _, _, err := t.readRecycleItem(ctx, spaceid, key, filepath.Join(path, c.Name()))
+		if err != nil {
+			return nil, nil, err
+		}
+		nodes, err = appendChildren(ctx, n, nodes)
+		if err != nil {
+			return nil, nil, err
+		}
+	}
+
 	fn := func() error {
 		// delete the actual node
 		// TODO recursively delete children
-		if err := os.RemoveAll(deletedNodePath); err != nil {
+		if err := utils.RemoveItem(deletedNodePath); err != nil {
 			log.Error().Err(err).Str("deletedNodePath", deletedNodePath).Msg("error deleting trash node")
 			return err
 		}
@@ -560,6 +574,23 @@ func (t *Tree) PurgeRecycleItemFunc(ctx context.Context, spaceid, key string, pa
 			return err
 		}
 
+		// delete children
+		for i := len(nodes) - 1; i >= 0; i-- {
+			n := nodes[i]
+			if err := utils.RemoveItem(n.InternalPath()); err != nil {
+				log.Error().Err(err).Str("deletedNodePath", deletedNodePath).Msg("error deleting trash node")
+				return err
+			}
+			if n.BlobID != "" {
+				if err = t.DeleteBlob(n); err != nil {
+					log.Error().Err(err).Str("trashItem", trashItem).Msg("error deleting item blob")
+					return err
+				}
+
+			}
+
+		}
+
 		return nil
 	}
 
@@ -866,3 +897,29 @@ func (t *Tree) readRecycleItem(ctx context.Context, spaceID, key, path string) (
 
 	return
 }
+
+// appendChildren appends `n` and all its children to `nodes`
+func appendChildren(ctx context.Context, n *node.Node, nodes []*node.Node) ([]*node.Node, error) {
+	nodes = append(nodes, n)
+
+	children, err := os.ReadDir(n.InternalPath())
+	if err != nil {
+		// TODO: How to differentiate folders from files?
+		return nodes, nil
+	}
+
+	for _, c := range children {
+		cn, err := n.Child(ctx, c.Name())
+		if err != nil {
+			// continue?
+			return nil, err
+		}
+		nodes, err = appendChildren(ctx, cn, nodes)
+		if err != nil {
+			// continue?
+			return nil, err
+		}
+	}
+
+	return nodes, nil
+}

pkg/utils/utils.go

@@ -23,6 +23,7 @@ import (
 	"net"
 	"net/http"
 	"net/url"
+	"os"
 	"os/user"
 	"path"
 	"path/filepath"
@@ -368,3 +369,21 @@ func ExistsInOpaque(o *types.Opaque, key string) bool {
 	_, ok := o.Map[key]
 	return ok
 }
+
+// RemoveItem removes the given item, its children and all empty parent folders
+func RemoveItem(path string) error {
+	if err := os.RemoveAll(path); err != nil {
+		return err
+	}
+
+	for {
+		path = filepath.Dir(path)
+		if err := os.Remove(path); err != nil {
+			// remove will fail when the dir is not empty.
+			// We can exit in that case
+			return nil
+		}
+
+	}
+
+}