diff --git a/changelog/unreleased/fix-blob-deletion.md b/changelog/unreleased/fix-blob-deletion.md new file mode 100644 index 0000000000..5d24c94936 --- /dev/null +++ b/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 diff --git a/pkg/storage/fs/ocis/blobstore/blobstore.go b/pkg/storage/fs/ocis/blobstore/blobstore.go index 97dc6ca111..8fbdf30b44 100644 --- a/pkg/storage/fs/ocis/blobstore/blobstore.go +++ b/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 diff --git a/pkg/storage/utils/decomposedfs/node/node.go b/pkg/storage/utils/decomposedfs/node/node.go index 0df5047d41..48645be677 100644 --- a/pkg/storage/utils/decomposedfs/node/node.go +++ b/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 { diff --git a/pkg/storage/utils/decomposedfs/recycle.go b/pkg/storage/utils/decomposedfs/recycle.go index 36e3fea073..bc25cb4a01 100644 --- a/pkg/storage/utils/decomposedfs/recycle.go +++ b/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)) diff --git a/pkg/storage/utils/decomposedfs/tree/tree.go b/pkg/storage/utils/decomposedfs/tree/tree.go index d239275ce1..e3461d78d8 100644 --- a/pkg/storage/utils/decomposedfs/tree/tree.go +++ b/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 +} diff --git a/pkg/utils/utils.go b/pkg/utils/utils.go index 9b3c7ffb29..3f049c9d45 100644 --- a/pkg/utils/utils.go +++ b/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 + } + + } + +}