Skip to content

Commit

Permalink
Do not shadow directories (#1817)
Browse files Browse the repository at this point in the history 
Fixes #1813

RELEASE_NOTES=[BUGFIX] Do not shadow directories

Signed-off-by: Dominik Schulz <dominik.schulz@gauner.org>
  • Loading branch information
@dominikschulz
dominikschulz authored Mar 8, 2021
1 parent e54b2a5 commit cf6020b
Show file tree
Hide file tree
Showing 2 changed files with 103 additions and 47 deletions.
18 changes: 13 additions & 5 deletions internal/store/root/move.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ func (r *Store) move(ctx context.Context, from, to string, delete bool) error {
return fmt.Errorf("destination is a file")
}

if err := r.moveFromTo(ctx, subFrom, from, to, fromPrefix, srcIsDir, delete); err != nil {
if err := r.moveFromTo(ctx, subFrom, from, to, fromPrefix, srcIsDir, dstIsDir, delete); err != nil {
return err
}
if err := subFrom.Storage().Commit(ctx, fmt.Sprintf("Move from %s to %s", from, to)); delete && err != nil {
Expand All @@ -63,7 +63,7 @@ func (r *Store) move(ctx context.Context, from, to string, delete bool) error {

if err := subFrom.Storage().Push(ctx, "", ""); err != nil {
if errors.Is(err, store.ErrGitNotInit) {
msg := "Warning: git is not initialized for this.storage. Ignoring auto-push option\n" +
msg := "Warning: git is not initialized for this storage. Ignoring auto-push option\n" +
"Run: gopass git init"
debug.Log(msg)
return nil
Expand All @@ -79,7 +79,7 @@ func (r *Store) move(ctx context.Context, from, to string, delete bool) error {
if !subFrom.Equals(subTo) {
if err := subTo.Storage().Push(ctx, "", ""); err != nil {
if errors.Is(err, store.ErrGitNotInit) {
msg := "Warning: git is not initialized for this.storage. Ignoring auto-push option\n" +
msg := "Warning: git is not initialized for this storage. Ignoring auto-push option\n" +
"Run: gopass git init"
debug.Log(msg)
return nil
Expand All @@ -96,10 +96,12 @@ func (r *Store) move(ctx context.Context, from, to string, delete bool) error {
return nil
}

func (r *Store) moveFromTo(ctx context.Context, subFrom *leaf.Store, from, to, fromPrefix string, srcIsDir, delete bool) error {
func (r *Store) moveFromTo(ctx context.Context, subFrom *leaf.Store, from, to, fromPrefix string, srcIsDir, dstIsDir, delete bool) error {
ctx = ctxutil.WithGitCommit(ctx, false)

entries := []string{from}
// if the source is a directory we enumerate all it's children
// and move them one by one.
if r.IsDir(ctx, from) {
var err error
entries, err = subFrom.List(ctx, fromPrefix)
Expand All @@ -111,6 +113,8 @@ func (r *Store) moveFromTo(ctx context.Context, subFrom *leaf.Store, from, to, f
return fmt.Errorf("no entries")
}

debug.Log("Moving %q to %q (entries: %+v)", from, to, entries)

for _, src := range entries {
dst := to
if srcIsDir {
Expand All @@ -120,8 +124,12 @@ func (r *Store) moveFromTo(ctx context.Context, subFrom *leaf.Store, from, to, f
} else {
dst = path.Join(to, path.Base(from), strings.TrimPrefix(src, from))
}
} else {
if dstIsDir || strings.HasSuffix(to, "/") {
dst = path.Join(to, path.Base(src))
}
}
debug.Log("Moving %s (%s) => %s (%s) (sid:%t, delete:%t)\n", from, src, to, dst, srcIsDir, delete)
debug.Log("Moving %q (%q) => %q (%q) (sid:%t, did:%t, delete:%t)\n", from, src, to, dst, srcIsDir, dstIsDir, delete)

content, err := r.Get(ctx, src)
if err != nil {
Expand Down
132 changes: 90 additions & 42 deletions internal/store/root/move_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,43 @@ import (
"github.com/stretchr/testify/require"
)

func TestMoveShadow(t *testing.T) {
u := gptest.NewUnitTester(t)
u.Entries = []string{
"old/www/foo",
"old/www/bar",
}

require.NoError(t, u.InitStore(""))
defer u.Remove()

ctx := context.Background()
ctx = ctxutil.WithAlwaysYes(ctx, true)
ctx = ctxutil.WithHidden(ctx, true)

rs, err := createRootStore(ctx, u)
require.NoError(t, err)
assert.NoError(t, rs.Delete(ctx, "foo"))

// -> move old/www/foo www/dir/foo => OK
assert.NoError(t, rs.Move(ctx, "old/www/foo", "www/dir/foo"))
entries, err := rs.List(ctx, tree.INF)
require.NoError(t, err)
require.Equal(t, []string{
"old/www/bar",
"www/dir/foo",
}, entries)

// -> move old/www/bar www/ => OK
assert.NoError(t, rs.Move(ctx, "old/www/bar", "www/"))
entries, err = rs.List(ctx, tree.INF)
require.NoError(t, err)
require.Equal(t, []string{
"www/bar",
"www/dir/foo",
}, entries)
}

func TestMove(t *testing.T) {
u := gptest.NewUnitTester(t)
u.Entries = []string{
Expand All @@ -39,8 +76,10 @@ func TestMove(t *testing.T) {
"foo/baz",
"misc/zab",
}, entries)

// -> move foo/ misc/zab => ERROR: misc/zab is a file
assert.Error(t, rs.Move(ctx, "foo/", "misc/zab"))

// -> move foo misc/zab => ERROR: misc/zab is a file
assert.Error(t, rs.Move(ctx, "foo", "misc/zab"))

Expand Down Expand Up @@ -125,56 +164,65 @@ func TestCopy(t *testing.T) {
assert.NoError(t, rs.Delete(ctx, "foo"))

// Initial state:
entries, err := rs.List(ctx, tree.INF)
require.NoError(t, err)
assert.Equal(t, []string{
"foo/bar",
"foo/baz",
"misc/zab",
}, entries)
t.Run("initial state", func(t *testing.T) {
entries, err := rs.List(ctx, tree.INF)
require.NoError(t, err)
assert.Equal(t, []string{
"foo/bar",
"foo/baz",
"misc/zab",
}, entries)
})

// -> copy foo/ misc/zab => ERROR: misc/zab is a file
assert.Error(t, rs.Copy(ctx, "foo/", "misc/zab"))
// -> copy foo misc/zab => ERROR: misc/zab is a file
assert.Error(t, rs.Copy(ctx, "foo", "misc/zab"))

// -> copy foo/ misc => OK
assert.NoError(t, rs.Copy(ctx, "foo", "misc"))
entries, err = rs.List(ctx, tree.INF)
require.NoError(t, err)
assert.Equal(t, []string{
"foo/bar",
"foo/baz",
"misc/foo/bar",
"misc/foo/baz",
"misc/zab",
}, entries)
t.Run("copy foo misc", func(t *testing.T) {
assert.NoError(t, rs.Copy(ctx, "foo", "misc"))
entries, err := rs.List(ctx, tree.INF)
require.NoError(t, err)
assert.Equal(t, []string{
"foo/bar",
"foo/baz",
"misc/foo/bar",
"misc/foo/baz",
"misc/zab",
}, entries)
})

// -> copy misc/foo/ bar/ => OK
assert.NoError(t, rs.Copy(ctx, "misc/foo/", "bar/"))
entries, err = rs.List(ctx, tree.INF)
require.NoError(t, err)
assert.Equal(t, []string{
"bar/bar",
"bar/baz",
"foo/bar",
"foo/baz",
"misc/foo/bar",
"misc/foo/baz",
"misc/zab",
}, entries)
t.Run("copy misc/foo/ bar/", func(t *testing.T) {
assert.NoError(t, rs.Copy(ctx, "misc/foo/", "bar/"))
entries, err := rs.List(ctx, tree.INF)
require.NoError(t, err)
assert.Equal(t, []string{
"bar/bar",
"bar/baz",
"foo/bar",
"foo/baz",
"misc/foo/bar",
"misc/foo/baz",
"misc/zab",
}, entries)
})

// -> copy misc/zab bar/foo/zab => OK
assert.NoError(t, rs.Copy(ctx, "misc/zab", "bar/foo/zab"))
entries, err = rs.List(ctx, tree.INF)
require.NoError(t, err)
assert.Equal(t, []string{
"bar/bar",
"bar/baz",
"bar/foo/zab",
"foo/bar",
"foo/baz",
"misc/foo/bar",
"misc/foo/baz",
"misc/zab",
}, entries)
t.Run("copy misc/zab bar/foo/zab", func(t *testing.T) {
assert.NoError(t, rs.Copy(ctx, "misc/zab", "bar/foo/zab"))
entries, err := rs.List(ctx, tree.INF)
require.NoError(t, err)
assert.Equal(t, []string{
"bar/bar",
"bar/baz",
"bar/foo/zab",
"foo/bar",
"foo/baz",
"misc/foo/bar",
"misc/foo/baz",
"misc/zab",
}, entries)
})
}

0 comments on commit cf6020b

Please sign in to comment.