From 4c98360356be75f456f9c57f6d5a05f437801ffc Mon Sep 17 00:00:00 2001 From: Nick Craig-Wood Date: Fri, 1 May 2020 12:49:34 +0100 Subject: [PATCH] fs/cache: Add Pin and Unpin and canonicalised lookup Before this change we stored cached Fs under the config string the user gave us. As the result of fs.ConfigString() can often be different after the backend has canonicalised the paths this meant that we could not look up backends in the cache reliably. After this change we store cached Fs under their config string as returned from fs.ConfigString(f) after the Fs has been created. We also store a map of user to canonical names (where they are different) so the users can look up Fs under the names they passed to rclone too. This change along with Pin and Unpin is necessary so we can look up the Fs in use reliably in the `backend/command` remote control interface. --- fs/cache/cache.go | 61 ++++++++++++++++++++++++++++++++-- fs/cache/cache_test.go | 74 +++++++++++++++++++++++++++++++++--------- 2 files changed, 116 insertions(+), 19 deletions(-) diff --git a/fs/cache/cache.go b/fs/cache/cache.go index 1a8482bf8..8b39051c0 100644 --- a/fs/cache/cache.go +++ b/fs/cache/cache.go @@ -2,26 +2,79 @@ package cache import ( + "sync" + "github.com/rclone/rclone/fs" "github.com/rclone/rclone/lib/cache" ) var ( - c = cache.New() + c = cache.New() + mu sync.Mutex // mutex to protect remap + remap = map[string]string{} // map user supplied names to canonical names ) +// Lookup fsString in the mapping from user supplied names to +// canonical names and return the canonical form +func canonicalize(fsString string) string { + mu.Lock() + canonicalName, ok := remap[fsString] + mu.Unlock() + if !ok { + return fsString + } + fs.Debugf(nil, "fs cache: switching user supplied name %q for canonical name %q", fsString, canonicalName) + return canonicalName +} + +// Put in a mapping from fsString => canonicalName if they are different +func addMapping(fsString, canonicalName string) { + if canonicalName == fsString { + return + } + mu.Lock() + remap[fsString] = canonicalName + mu.Unlock() +} + // GetFn gets a fs.Fs named fsString either from the cache or creates // it afresh with the create function func GetFn(fsString string, create func(fsString string) (fs.Fs, error)) (f fs.Fs, err error) { + fsString = canonicalize(fsString) + created := false value, err := c.Get(fsString, func(fsString string) (f interface{}, ok bool, err error) { f, err = create(fsString) ok = err == nil || err == fs.ErrorIsFile + created = ok return f, ok, err }) if err != nil && err != fs.ErrorIsFile { return nil, err } - return value.(fs.Fs), err + f = value.(fs.Fs) + // Check we stored the Fs at the canonical name + 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) + } + addMapping(fsString, canonicalName) + } + } + return f, err +} + +// Pin f into the cache until Unpin is called +func Pin(f fs.Fs) { + c.Pin(fs.ConfigString(f)) +} + +// Unpin f from the cache +func Unpin(f fs.Fs) { + c.Pin(fs.ConfigString(f)) } // Get gets a fs.Fs named fsString either from the cache or creates it afresh @@ -31,7 +84,9 @@ func Get(fsString string) (f fs.Fs, err error) { // Put puts an fs.Fs named fsString into the cache func Put(fsString string, f fs.Fs) { - c.Put(fsString, f) + canonicalName := fs.ConfigString(f) + c.Put(canonicalName, f) + addMapping(fsString, canonicalName) } // Clear removes everything from the cahce diff --git a/fs/cache/cache_test.go b/fs/cache/cache_test.go index a7391c3f8..e261eda15 100644 --- a/fs/cache/cache_test.go +++ b/fs/cache/cache_test.go @@ -2,7 +2,6 @@ package cache import ( "errors" - "fmt" "testing" "github.com/rclone/rclone/fs" @@ -18,18 +17,19 @@ var ( func mockNewFs(t *testing.T) (func(), func(path string) (fs.Fs, error)) { called = 0 - create := func(path string) (fs.Fs, error) { + create := func(path string) (f fs.Fs, err error) { assert.Equal(t, 0, called) called++ switch path { - case "/": - return mockfs.NewFs("mock", "mock"), nil - case "/file.txt": - return mockfs.NewFs("mock", "mock"), fs.ErrorIsFile - case "/error": + case "mock:/": + return mockfs.NewFs("mock", "/"), nil + case "mock:/file.txt": + return mockfs.NewFs("mock", "/"), fs.ErrorIsFile + case "mock:/error": return nil, errSentinel } - panic(fmt.Sprintf("Unknown path %q", path)) + t.Fatalf("Unknown path %q", path) + panic("unreachable") } cleanup := func() { c.Clear() @@ -43,12 +43,12 @@ func TestGet(t *testing.T) { assert.Equal(t, 0, c.Entries()) - f, err := GetFn("/", create) + f, err := GetFn("mock:/", create) require.NoError(t, err) assert.Equal(t, 1, c.Entries()) - f2, err := GetFn("/", create) + f2, err := GetFn("mock:/", create) require.NoError(t, err) assert.Equal(t, f, f2) @@ -60,13 +60,13 @@ func TestGetFile(t *testing.T) { assert.Equal(t, 0, c.Entries()) - f, err := GetFn("/file.txt", create) + f, err := GetFn("mock:/file.txt", create) require.Equal(t, fs.ErrorIsFile, err) require.NotNil(t, f) assert.Equal(t, 1, c.Entries()) - f2, err := GetFn("/file.txt", create) + f2, err := GetFn("mock:/file.txt", create) require.Equal(t, fs.ErrorIsFile, err) require.NotNil(t, f2) @@ -79,7 +79,7 @@ func TestGetError(t *testing.T) { assert.Equal(t, 0, c.Entries()) - f, err := GetFn("/error", create) + f, err := GetFn("mock:/error", create) require.Equal(t, errSentinel, err) require.Equal(t, nil, f) @@ -90,17 +90,59 @@ func TestPut(t *testing.T) { cleanup, create := mockNewFs(t) defer cleanup() - f := mockfs.NewFs("mock", "mock") + f := mockfs.NewFs("mock", "/alien") assert.Equal(t, 0, c.Entries()) - Put("/alien", f) + Put("mock:/alien", f) assert.Equal(t, 1, c.Entries()) - fNew, err := GetFn("/alien", create) + fNew, err := GetFn("mock:/alien", create) require.NoError(t, err) require.Equal(t, f, fNew) assert.Equal(t, 1, c.Entries()) + + // Check canonicalisation + + Put("mock:/alien/", f) + + fNew, err = GetFn("mock:/alien/", create) + require.NoError(t, err) + require.Equal(t, f, fNew) + + assert.Equal(t, 1, c.Entries()) + +} + +func TestPin(t *testing.T) { + cleanup, create := mockNewFs(t) + defer cleanup() + + // Test pinning and unpinning non existent + f := mockfs.NewFs("mock", "/alien") + Pin(f) + Unpin(f) + + // Now test pinning an existing + f2, err := GetFn("mock:/", create) + require.NoError(t, err) + Pin(f2) + Unpin(f2) +} + +func TestClear(t *testing.T) { + cleanup, create := mockNewFs(t) + defer cleanup() + + // Create something + _, err := GetFn("mock:/", create) + require.NoError(t, err) + + assert.Equal(t, 1, c.Entries()) + + Clear() + + assert.Equal(t, 0, c.Entries()) }