Skip to content

Commit

Permalink
Do not try to delete blobs when deleting a directory node
Browse files Browse the repository at this point in the history
Directories don't have blobs associated with them.
  • Loading branch information
aduffeck committed Feb 16, 2021
1 parent 03c7b7f commit a48811a
Show file tree
Hide file tree
Showing 4 changed files with 61 additions and 5 deletions.
2 changes: 1 addition & 1 deletion pkg/storage/fs/ocis/blobstore/blobstore.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ func (bs *Blobstore) Download(key string) (io.ReadCloser, error) {
func (bs *Blobstore) Delete(key string) error {
err := os.Remove(bs.path(key))
if err != nil {
return errors.Wrapf(err, "could not delete blod '%s'", key)
return errors.Wrapf(err, "could not delete blob '%s'", key)
}
return nil
}
Expand Down
8 changes: 7 additions & 1 deletion pkg/storage/utils/decomposedfs/testhelpers/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ func NewTestEnv() (*TestEnv, error) {
}
lookup := &decomposedfs.Lookup{Options: o}
permissions := &mocks.PermissionsChecker{}
permissions.On("HasPermission", mock.Anything, mock.Anything, mock.Anything).Return(true, nil).Twice() // Permissions required for setup below
permissions.On("HasPermission", mock.Anything, mock.Anything, mock.Anything).Return(true, nil).Times(3) // Permissions required for setup below
bs := &treemocks.Blobstore{}
tree := tree.New(o.Root, true, true, lookup, bs)
fs, err := decomposedfs.New(o, lookup, permissions, tree)
Expand Down Expand Up @@ -143,6 +143,12 @@ func NewTestEnv() (*TestEnv, error) {
return nil, err
}

// Create emptydir
err = fs.CreateDir(ctx, "emptydir")
if err != nil {
return nil, err
}

return &TestEnv{
Root: tmpRoot,
Fs: fs,
Expand Down
13 changes: 10 additions & 3 deletions pkg/storage/utils/decomposedfs/tree/tree.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ package tree

import (
"context"
"fmt"
"io"
"os"
"path/filepath"
Expand Down Expand Up @@ -399,9 +400,11 @@ func (t *Tree) PurgeRecycleItemFunc(ctx context.Context, key string) (*node.Node
}

// delete blob from blobstore
if err = t.DeleteBlob(rn.BlobID); err != nil {
log.Error().Err(err).Str("trashItem", trashItem).Msg("error deleting trash item blob")
return err
if rn.BlobID != "" {
if err = t.DeleteBlob(rn.BlobID); err != nil {
log.Error().Err(err).Str("trashItem", trashItem).Msg("error deleting trash item blob")
return err
}
}

// delete item link in trash
Expand Down Expand Up @@ -515,6 +518,10 @@ func (t *Tree) ReadBlob(key string) (io.ReadCloser, error) {

// DeleteBlob deletes a blob from the blobstore
func (t *Tree) DeleteBlob(key string) error {
if key == "" {
return fmt.Errorf("could not delete blob, empty key was given")
}

return t.blobstore.Delete(key)
}

Expand Down
43 changes: 43 additions & 0 deletions pkg/storage/utils/decomposedfs/tree/tree_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -152,4 +152,47 @@ var _ = Describe("Tree", func() {
})
})
})

Context("with an empty directory", func() {
var (
n *node.Node
)

JustBeforeEach(func() {
var err error
n, err = env.Lookup.NodeFromPath(env.Ctx, "emptydir")
Expect(err).ToNot(HaveOccurred())
})

Context("that was deleted", func() {
var (
trashPath string
)

JustBeforeEach(func() {
trashPath = path.Join(env.Root, "trash", env.Owner.Id.OpaqueId, n.ID)
Expect(t.Delete(env.Ctx, n)).To(Succeed())
})

Describe("PurgeRecycleItemFunc", func() {
JustBeforeEach(func() {
_, err := os.Stat(trashPath)
Expect(err).ToNot(HaveOccurred())

_, purgeFunc, err := t.PurgeRecycleItemFunc(env.Ctx, env.Owner.Id.OpaqueId+":"+n.ID)
Expect(err).ToNot(HaveOccurred())
Expect(purgeFunc()).To(Succeed())
})

It("removes the file from the trash", func() {
_, err := os.Stat(trashPath)
Expect(err).To(HaveOccurred())
})

It("does not try to delete a blob from the blobstore", func() {
env.Blobstore.AssertNotCalled(GinkgoT(), "Delete", mock.AnythingOfType("string"))
})
})
})
})
})

0 comments on commit a48811a

Please sign in to comment.