drive: strip trailing slashes in shortcut command #4098

This also fixes typo in the name of the function, and allows making
shortcuts from the root directory which are useful in cross drive
shortcut creation.

This also adds a basic suite of tests for creating listing, removing
shortcuts.

backend/drive/drive.go

@@ -2787,14 +2787,26 @@ func (f *Fs) changeServiceAccountFile(file string) (err error) {
 // Create a shortcut from (f, srcPath) to (dstFs, dstPath)
 //
 // Will not overwrite existing files
-func (f *Fs) makeShorcut(ctx context.Context, srcPath string, dstFs *Fs, dstPath string) (o fs.Object, err error) {
+func (f *Fs) makeShortcut(ctx context.Context, srcPath string, dstFs *Fs, dstPath string) (o fs.Object, err error) {
 	srcFs := f
+	srcPath = strings.Trim(srcPath, "/")
+	dstPath = strings.Trim(dstPath, "/")
+	if dstPath == "" {
+		return nil, errors.New("shortcut destination can't be root directory")
+	}
 
 	// Find source
-	srcObj, err := srcFs.NewObject(ctx, srcPath)
 	var srcID string
 	isDir := false
-	if err != nil {
+	if srcPath == "" {
+		// source is root directory
+		err = f.dirCache.FindRoot(ctx, false)
+		if err != nil {
+			return nil, err
+		}
+		srcID = f.dirCache.RootID()
+		isDir = true
+	} else if srcObj, err := srcFs.NewObject(ctx, srcPath); err != nil {
 		if err != fs.ErrorNotAFile {
 			return nil, errors.Wrap(err, "can't find source")
 		}
@@ -2804,7 +2816,6 @@ func (f *Fs) makeShorcut(ctx context.Context, srcPath string, dstFs *Fs, dstPath
 			return nil, errors.Wrap(err, "failed to find source dir")
 		}
 		isDir = true
-
 	} else {
 		// source was a file
 		srcID = srcObj.(*Object).id
@@ -2963,7 +2974,7 @@ func (f *Fs) Command(ctx context.Context, name string, arg []string, opt map[str
 				return nil, errors.New("target is not a drive backend")
 			}
 		}
-		return f.makeShorcut(ctx, arg[0], dstFs, arg[1])
+		return f.makeShortcut(ctx, arg[0], dstFs, arg[1])
 	default:
 		return nil, fs.ErrorCommandNotFound
 	}

backend/drive/drive_internal_test.go

@@ -14,6 +14,7 @@ import (
 	"github.com/pkg/errors"
 	_ "github.com/rclone/rclone/backend/local"
 	"github.com/rclone/rclone/fs"
+	"github.com/rclone/rclone/fs/hash"
 	"github.com/rclone/rclone/fs/operations"
 	"github.com/rclone/rclone/fstest/fstests"
 	"github.com/stretchr/testify/assert"
@@ -268,6 +269,98 @@ func (f *Fs) InternalTestDocumentLink(t *testing.T) {
 	}
 }
 
+// TestIntegration/FsMkdir/FsPutFiles/Internal/Shortcuts
+func (f *Fs) InternalTestShortcuts(t *testing.T) {
+	const (
+		// from fstest/fstests/fstests.go
+		existingDir    = "hello? sausage"
+		existingFile   = "file name.txt"
+		existingSubDir = "êé"
+	)
+	ctx := context.Background()
+	srcObj, err := f.NewObject(ctx, existingFile)
+	require.NoError(t, err)
+	srcHash, err := srcObj.Hash(ctx, hash.MD5)
+	require.NoError(t, err)
+	assert.NotEqual(t, "", srcHash)
+	t.Run("Errors", func(t *testing.T) {
+		_, err := f.makeShortcut(ctx, "", f, "")
+		assert.Error(t, err)
+		assert.Contains(t, err.Error(), "can't be root")
+
+		_, err = f.makeShortcut(ctx, "notfound", f, "dst")
+		assert.Error(t, err)
+		assert.Contains(t, err.Error(), "can't find source")
+
+		_, err = f.makeShortcut(ctx, existingFile, f, existingFile)
+		assert.Error(t, err)
+		assert.Contains(t, err.Error(), "not overwriting")
+		assert.Contains(t, err.Error(), "existing file")
+
+		_, err = f.makeShortcut(ctx, existingFile, f, existingDir)
+		assert.Error(t, err)
+		assert.Contains(t, err.Error(), "not overwriting")
+		assert.Contains(t, err.Error(), "existing directory")
+	})
+	t.Run("File", func(t *testing.T) {
+		dstObj, err := f.makeShortcut(ctx, existingFile, f, "shortcut.txt")
+		require.NoError(t, err)
+		require.NotNil(t, dstObj)
+		assert.Equal(t, "shortcut.txt", dstObj.Remote())
+		dstHash, err := dstObj.Hash(ctx, hash.MD5)
+		require.NoError(t, err)
+		assert.Equal(t, srcHash, dstHash)
+		require.NoError(t, dstObj.Remove(ctx))
+	})
+	t.Run("Dir", func(t *testing.T) {
+		dstObj, err := f.makeShortcut(ctx, existingDir, f, "shortcutdir")
+		require.NoError(t, err)
+		require.Nil(t, dstObj)
+		entries, err := f.List(ctx, "shortcutdir")
+		require.NoError(t, err)
+		require.Equal(t, 1, len(entries))
+		require.Equal(t, "shortcutdir/"+existingSubDir, entries[0].Remote())
+		require.NoError(t, f.Rmdir(ctx, "shortcutdir"))
+	})
+	t.Run("Command", func(t *testing.T) {
+		_, err := f.Command(ctx, "shortcut", []string{"one"}, nil)
+		require.Error(t, err)
+		require.Contains(t, err.Error(), "need exactly 2 arguments")
+
+		_, err = f.Command(ctx, "shortcut", []string{"one", "two"}, map[string]string{
+			"target": "doesnotexistremote:",
+		})
+		require.Error(t, err)
+		require.Contains(t, err.Error(), "couldn't find target")
+
+		_, err = f.Command(ctx, "shortcut", []string{"one", "two"}, map[string]string{
+			"target": ".",
+		})
+		require.Error(t, err)
+		require.Contains(t, err.Error(), "target is not a drive backend")
+
+		dstObjI, err := f.Command(ctx, "shortcut", []string{existingFile, "shortcut2.txt"}, map[string]string{
+			"target": fs.ConfigString(f),
+		})
+		require.NoError(t, err)
+		dstObj := dstObjI.(*Object)
+		assert.Equal(t, "shortcut2.txt", dstObj.Remote())
+		dstHash, err := dstObj.Hash(ctx, hash.MD5)
+		require.NoError(t, err)
+		assert.Equal(t, srcHash, dstHash)
+		require.NoError(t, dstObj.Remove(ctx))
+
+		dstObjI, err = f.Command(ctx, "shortcut", []string{existingFile, "shortcut3.txt"}, nil)
+		require.NoError(t, err)
+		dstObj = dstObjI.(*Object)
+		assert.Equal(t, "shortcut3.txt", dstObj.Remote())
+		dstHash, err = dstObj.Hash(ctx, hash.MD5)
+		require.NoError(t, err)
+		assert.Equal(t, srcHash, dstHash)
+		require.NoError(t, dstObj.Remove(ctx))
+	})
+}
+
 func (f *Fs) InternalTest(t *testing.T) {
 	// These tests all depend on each other so run them as nested tests
 	t.Run("DocumentImport", func(t *testing.T) {
@@ -282,6 +375,7 @@ func (f *Fs) InternalTest(t *testing.T) {
 			})
 		})
 	})
+	t.Run("Shortcuts", f.InternalTestShortcuts)
 }
 
 var _ fstests.InternalTester = (*Fs)(nil)