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

@@ -2,26 +2,79 @@
 package cache
 
 import (
+	"sync"
+
 	"github.com/rclone/rclone/fs"
 	"github.com/rclone/rclone/lib/cache"
 )
 
 var (
 	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

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())
 }