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/node/node.go

@@ -1055,6 +1055,15 @@ func ReadBlobSizeAttr(path string) (int64, error) {
 	return blobSize, nil
 }
 
+// ReadBlobIDAttr reads the blobsize from the xattrs
+func ReadBlobIDAttr(path string) (string, error) {
+	attr, err := xattrs.Get(path, xattrs.BlobIDAttr)
+	if err != nil {
+		return "", errors.Wrapf(err, "error reading blobid xattr")
+	}
+	return attr, nil
+}
+
 func (n *Node) hasUserShares(ctx context.Context) bool {
 	g, err := n.ListGrantees(ctx)
 	if err != 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, all its children and all their revisions
 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,20 +538,23 @@ func (t *Tree) PurgeRecycleItemFunc(ctx context.Context, spaceid, key string, pa
 		return nil, nil, err
 	}
 
-	fn := func() error {
-		// delete the actual node
-		// TODO recursively delete children
-		if err := os.RemoveAll(deletedNodePath); err != nil {
-			log.Error().Err(err).Str("deletedNodePath", deletedNodePath).Msg("error deleting trash node")
-			return 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
+		}
+	}
 
-		// delete blob from blobstore
-		if rn.BlobID != "" {
-			if err = t.DeleteBlob(rn); err != nil {
-				log.Error().Err(err).Str("trashItem", trashItem).Msg("error deleting trash item blob")
-				return err
-			}
+	fn := func() error {
+		if err := t.removeNode(deletedNodePath, rn); err != nil {
+			return err
 		}
 
 		// delete item link in trash
@@ -560,12 +563,66 @@ 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 := t.removeNode(n.InternalPath(), n); err != nil {
+				return err
+			}
+
+		}
+
 		return nil
 	}
 
 	return rn, fn, nil
 }
 
+func (t *Tree) removeNode(path string, n *node.Node) error {
+	// delete the actual node
+	if err := utils.RemoveItem(path); err != nil {
+		log.Error().Err(err).Str("path", path).Msg("error node")
+		return err
+	}
+
+	// delete blob from blobstore
+	if n.BlobID != "" {
+		if err := t.DeleteBlob(n); err != nil {
+			log.Error().Err(err).Str("blobID", n.BlobID).Msg("error deleting nodes blob")
+			return err
+		}
+	}
+
+	// delete revisions
+	revs, err := filepath.Glob(n.InternalPath() + node.RevisionIDDelimiter + "*")
+	if err != nil {
+		log.Error().Err(err).Str("path", n.InternalPath()+node.RevisionIDDelimiter+"*").Msg("glob failed badly")
+		return err
+	}
+	for _, rev := range revs {
+		bID, err := node.ReadBlobIDAttr(rev)
+		if err != nil {
+			log.Error().Err(err).Str("revision", rev).Msg("error reading blobid attribute")
+			return err
+		}
+
+		if err := utils.RemoveItem(rev); err != nil {
+			log.Error().Err(err).Str("revision", rev).Msg("error removing revision node")
+			return err
+		}
+
+		if bID != "" {
+			if err := t.DeleteBlob(&node.Node{SpaceID: n.SpaceID, BlobID: bID}); err != nil {
+				log.Error().Err(err).Str("revision", rev).Str("blobID", bID).Msg("error removing revision node blob")
+				return err
+			}
+		}
+
+	}
+
+	return nil
+}
+
 // Propagate propagates changes to the root of the tree
 func (t *Tree) Propagate(ctx context.Context, n *node.Node) (err error) {
 	sublog := appctx.GetLogger(ctx).With().Interface("node", n).Logger()
@@ -866,3 +923,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
+		}
+
+	}
+
+}