fs/cache: fix moveto/copyto remote:file remote:file2

Before this change, if the cache was given a source `remote:file` it
stored `remote:` with the error `fs.ErrorIsFile` attached. This meant
that if it `remote:` was subsequently looked up it would return the
`fs.ErrorIsFile` error.

This broke `moveto remote:file remote:file2` as moveto would lookup
`remote:` from the second argument and erroneously get the
`fs.ErrorIsFile` error.

This likely broke other commands too.

This was broken in

4c9836035 fs/cache: Add Pin and Unpin and canonicalised lookup

Which was released in v1.52.0

The fix is to make a new cache entry for `remote:` with no error
attached in the case that the original call returned `fs.ErrorIsFile`.
This commit is contained in:
Nick Craig-Wood 2020-07-02 10:15:47 +01:00
parent 50e36fb482
commit 8d5bc7f28b
2 changed files with 48 additions and 7 deletions

18
fs/cache/cache.go vendored
View File

@ -56,12 +56,20 @@ func GetFn(fsString string, create func(fsString string) (fs.Fs, error)) (f fs.F
if created {
canonicalName := fs.ConfigString(f)
if canonicalName != fsString {
fs.Debugf(nil, "fs cache: renaming cache item %q to be canonical %q", fsString, canonicalName)
value, found := c.Rename(fsString, canonicalName)
if found {
f = value.(fs.Fs)
// Note that if err == fs.ErrorIsFile at this moment
// then we can't rename the remote as it will have the
// wrong error status, we need to add a new one.
if err == nil {
fs.Debugf(nil, "fs cache: renaming cache item %q to be canonical %q", fsString, canonicalName)
value, found := c.Rename(fsString, canonicalName)
if found {
f = value.(fs.Fs)
}
addMapping(fsString, canonicalName)
} else {
fs.Debugf(nil, "fs cache: adding new entry for parent of %q, %q", fsString, canonicalName)
Put(canonicalName, f)
}
addMapping(fsString, canonicalName)
}
}
return f, err

View File

@ -23,7 +23,7 @@ func mockNewFs(t *testing.T) (func(), func(path string) (fs.Fs, error)) {
switch path {
case "mock:/":
return mockfs.NewFs("mock", "/"), nil
case "mock:/file.txt":
case "mock:/file.txt", "mock:file.txt":
return mockfs.NewFs("mock", "/"), fs.ErrorIsFile
case "mock:/error":
return nil, errSentinel
@ -64,13 +64,46 @@ func TestGetFile(t *testing.T) {
require.Equal(t, fs.ErrorIsFile, err)
require.NotNil(t, f)
assert.Equal(t, 1, c.Entries())
assert.Equal(t, 2, c.Entries())
f2, err := GetFn("mock:/file.txt", create)
require.Equal(t, fs.ErrorIsFile, err)
require.NotNil(t, f2)
assert.Equal(t, f, f2)
// check parent is there too
f2, err = GetFn("mock:/", create)
require.Nil(t, err)
require.NotNil(t, f2)
assert.Equal(t, f, f2)
}
func TestGetFile2(t *testing.T) {
cleanup, create := mockNewFs(t)
defer cleanup()
assert.Equal(t, 0, c.Entries())
f, err := GetFn("mock:file.txt", create)
require.Equal(t, fs.ErrorIsFile, err)
require.NotNil(t, f)
assert.Equal(t, 2, c.Entries())
f2, err := GetFn("mock:file.txt", create)
require.Equal(t, fs.ErrorIsFile, err)
require.NotNil(t, f2)
assert.Equal(t, f, f2)
// check parent is there too
f2, err = GetFn("mock:/", create)
require.Nil(t, err)
require.NotNil(t, f2)
assert.Equal(t, f, f2)
}
func TestGetError(t *testing.T) {