s3: fix --s3-versions when copying a single object

Before this change, if --s3-versions was enabled, then copying a
single object from a subdirectory would fail.

This was due to an incorrect comparison in the NewFs code.

This fixes the change and introduces a new unit tests.

backend/drive/drive.go

@@ -1210,7 +1210,6 @@ func newFs(ctx context.Context, name, path string, m configmap.Mapper) (*Fs, err
 		WriteMimeType:           true,
 		CanHaveEmptyDirectories: true,
 		ServerSideAcrossConfigs: opt.ServerSideAcrossConfigs,
-		FilterAware:             true,
 	}).Fill(ctx, f)
 
 	// Create a new authorized Drive client.

backend/drive/drive_internal_test.go

@@ -518,9 +518,6 @@ func (f *Fs) InternalTestCopyID(t *testing.T) {
 
 // TestIntegration/FsMkdir/FsPutFiles/Internal/AgeQuery
 func (f *Fs) InternalTestAgeQuery(t *testing.T) {
-	// Check set up for filtering
-	assert.True(t, f.Features().FilterAware)
-
 	opt := &filter.Opt{}
 	err := opt.MaxAge.Set("1h")
 	assert.NoError(t, err)

backend/local/local.go

@@ -300,7 +300,6 @@ func NewFs(ctx context.Context, name, root string, m configmap.Mapper) (fs.Fs, e
 		ReadMetadata:            true,
 		WriteMetadata:           true,
 		UserMetadata:            xattrSupported, // can only R/W general purpose metadata if xattrs are supported
-		FilterAware:             true,
 	}).Fill(ctx, f)
 	if opt.FollowSymlinks {
 		f.lstat = os.Stat

backend/local/local_internal_test.go

@@ -378,9 +378,6 @@ func TestFilter(t *testing.T) {
 	r.WriteFile("excluded", "excluded file", when)
 	f := r.Flocal.(*Fs)
 
-	// Check set up for filtering
-	assert.True(t, f.Features().FilterAware)
-
 	// Add a filter
 	ctx, fi := filter.AddConfig(ctx)
 	require.NoError(t, fi.AddRule("+ included"))

backend/s3/s3.go

@@ -2758,7 +2758,8 @@ func (f *Fs) getMetaDataListing(ctx context.Context, wantRemote string) (info *s
 		if isDirectory {
 			return nil
 		}
-		if wantRemote != gotRemote {
+		// compare the base name only since the listing will have a prefix
+		if path.Base(wantRemote) != path.Base(gotRemote) {
 			return nil
 		}
 		info = object

backend/s3/s3_internal_test.go

@@ -6,12 +6,16 @@ import (
 	"context"
 	"crypto/md5"
 	"fmt"
+	"path"
+	"strings"
 	"testing"
 	"time"
 
 	"github.com/aws/aws-sdk-go/aws"
 	"github.com/aws/aws-sdk-go/service/s3"
 	"github.com/rclone/rclone/fs"
+	"github.com/rclone/rclone/fs/cache"
+	"github.com/rclone/rclone/fs/fspath"
 	"github.com/rclone/rclone/fs/hash"
 	"github.com/rclone/rclone/fstest"
 	"github.com/rclone/rclone/fstest/fstests"
@@ -250,7 +254,7 @@ func (f *Fs) InternalTestVersions(t *testing.T) {
 	time.Sleep(2 * time.Second)
 
 	// Create an object
-	const fileName = "test-versions.txt"
+	const fileName = "versions/test-versions.txt"
 	contents := random.String(100)
 	item := fstest.NewItem(fileName, contents, fstest.Time("2001-05-06T04:05:06.499999999Z"))
 	obj := fstests.PutTestContents(ctx, t, f, &item, contents, true)
@@ -280,7 +284,7 @@ func (f *Fs) InternalTestVersions(t *testing.T) {
 		}()
 
 		// Read the contents
-		entries, err := f.List(ctx, "")
+		entries, err := f.List(ctx, "versions")
 		require.NoError(t, err)
 		tests := 0
 		var fileNameVersion string
@@ -295,12 +299,24 @@ func (f *Fs) InternalTestVersions(t *testing.T) {
 				t.Run("ReadVersion", func(t *testing.T) {
 					assert.Equal(t, contents, fstests.ReadObject(ctx, t, entry.(fs.Object), -1))
 				})
+				t.Run("NewFs", func(t *testing.T) {
+					// Check we can find the object with NewFs
+					fPath := fs.ConfigString(f)
+					fPath = strings.Replace(fPath, ":", ",versions=true:", 1)
+					subFPath := fspath.JoinRootPath(fPath, entry.Remote())
+					subF, err := cache.Get(ctx, subFPath)
+					require.Equal(t, fs.ErrorIsFile, err, "Remote %q didn't find a file", subFPath)
+					require.NotNil(t, subF)
+					o, err := subF.NewObject(ctx, path.Base(entry.Remote()))
+					require.NoError(t, err)
+					assert.Equal(t, contents, fstests.ReadObject(ctx, t, o, -1))
+				})
 				assert.WithinDuration(t, obj.(*Object).lastModified, versionTime, time.Second, "object time must be with 1 second of version time")
 				fileNameVersion = remote
 				tests++
 			}
 		}
-		assert.Equal(t, 2, tests, "object missing from listing")
+		assert.Equal(t, 2, tests, "object missing from listing: %v", entries)
 
 		// Check we can read the object with a version suffix
 		t.Run("NewObject", func(t *testing.T) {

cmd/serve/webdav/webdav.go

@@ -377,21 +377,18 @@ func (fi FileInfo) ETag(ctx context.Context) (etag string, err error) {
 // ContentType returns a content type for the FileInfo
 func (fi FileInfo) ContentType(ctx context.Context) (contentType string, err error) {
 	// defer log.Trace(fi, "")("etag=%q, err=%v", &contentType, &err)
-	if fi.IsDir() {
+	node, ok := (fi.FileInfo).(vfs.Node)
+	if !ok {
+		fs.Errorf(fi, "Expecting vfs.Node, got %T", fi.FileInfo)
+		return "application/octet-stream", nil
+	}
+	entry := node.DirEntry()
+	switch x := entry.(type) {
+	case fs.Object:
+		return fs.MimeType(ctx, x), nil
+	case fs.Directory:
 		return "inode/directory", nil
 	}
-	if node, ok := (fi.FileInfo).(vfs.Node); !ok {
-		fs.Errorf(fi, "Expecting vfs.Node, got %T", fi.FileInfo)
-	} else {
-		entry := node.DirEntry()
-		switch x := entry.(type) {
-		case nil:
-			// object hasn't been uploaded yet if entry is nil
-		case fs.Object:
-			return fs.MimeType(ctx, x), nil
-		default:
-			fs.Errorf(fi, "Expecting fs.Object or nil, got %T", entry)
-		}
-	}
-	return fs.MimeTypeFromName(fi.Name()), nil
+	fs.Errorf(fi, "Expecting fs.Object or fs.Directory, got %T", entry)
+	return "application/octet-stream", nil
 }

docs/content/docs.md

@@ -1868,22 +1868,13 @@ By default, rclone doesn't keep track of renamed files, so if you
 rename a file locally then sync it to a remote, rclone will delete the
 old file on the remote and upload a new copy.
 
-An rclone sync with `--track-renames` runs like a normal sync, but keeps
-track of objects which exist in the destination but not in the source
-(which would normally be deleted), and which objects exist in the
-source but not the destination (which would normally be transferred).
-These objects are then candidates for renaming.
+If you use this flag, and the remote supports server-side copy or
+server-side move, and the source and destination have a compatible
+hash, then this will track renames during `sync`
+operations and perform renaming server-side.
 
-After the sync, rclone matches up the source only and destination only
-objects using the `--track-renames-strategy` specified and either
-renames the destination object or transfers the source and deletes the
-destination object. `--track-renames` is stateless like all of
-rclone's syncs.
-
-To use this flag the destination must support server-side copy or
-server-side move, and to use a hash based `--track-renames-strategy`
-(the default) the source and the destination must have a compatible
-hash.
+Files will be matched by size and hash - if both match then a rename
+will be considered.
 
 If the destination does not support server-side copy or move, rclone
 will fall back to the default behaviour and log an error level message
@@ -1901,7 +1892,7 @@ Note also that `--track-renames` is incompatible with
 
 ### --track-renames-strategy (hash,modtime,leaf,size) ###
 
-This option changes the file matching criteria for `--track-renames`.
+This option changes the matching criteria for `--track-renames`.
 
 The matching is controlled by a comma separated selection of these tokens:
 
@@ -1910,15 +1901,15 @@ The matching is controlled by a comma separated selection of these tokens:
 - `leaf` - the name of the file not including its directory name
 - `size` - the size of the file (this is always enabled)
 
-The default option is `hash`.
-
-Using `--track-renames-strategy modtime,leaf` would match files
+So using `--track-renames-strategy modtime,leaf` would match files
 based on modification time, the leaf of the file name and the size
 only.
 
 Using `--track-renames-strategy modtime` or `leaf` can enable
 `--track-renames` support for encrypted destinations.
 
+If nothing is specified, the default option is matching by `hash`es.
+
 Note that the `hash` strategy is not supported with encrypted destinations.
 
 ### --delete-(before,during,after) ###

fs/features.go

@@ -29,7 +29,6 @@ type Features struct {
 	ReadMetadata            bool // can read metadata from objects
 	WriteMetadata           bool // can write metadata to objects
 	UserMetadata            bool // can read/write general purpose metadata
-	FilterAware             bool // can make use of filters if provided for listing
 
 	// Purge all files in the directory specified
 	//
@@ -321,7 +320,6 @@ func (ft *Features) Mask(ctx context.Context, f Fs) *Features {
 	// ft.IsLocal = ft.IsLocal && mask.IsLocal Don't propagate IsLocal
 	ft.SlowModTime = ft.SlowModTime && mask.SlowModTime
 	ft.SlowHash = ft.SlowHash && mask.SlowHash
-	ft.FilterAware = ft.FilterAware && mask.FilterAware
 
 	if mask.Purge == nil {
 		ft.Purge = nil

fs/march/march.go

@@ -83,7 +83,7 @@ func (m *March) makeListDir(ctx context.Context, f fs.Fs, includeAll bool) listD
 	if !(ci.UseListR && f.Features().ListR != nil) && // !--fast-list active and
 		!(ci.NoTraverse && fi.HaveFilesFrom()) { // !(--files-from and --no-traverse)
 		return func(dir string) (entries fs.DirEntries, err error) {
-			dirCtx := filter.SetUseFilter(m.Ctx, f.Features().FilterAware && !includeAll) // make filter-aware backends constrain List
+			dirCtx := filter.SetUseFilter(m.Ctx, !includeAll) // make filter-aware backends constrain List
 			return list.DirSorted(dirCtx, f, includeAll, dir)
 		}
 	}
@@ -100,7 +100,7 @@ func (m *March) makeListDir(ctx context.Context, f fs.Fs, includeAll bool) listD
 		mu.Lock()
 		defer mu.Unlock()
 		if !started {
-			dirCtx := filter.SetUseFilter(m.Ctx, f.Features().FilterAware && !includeAll) // make filter-aware backends constrain List
+			dirCtx := filter.SetUseFilter(m.Ctx, !includeAll) // make filter-aware backends constrain List
 			dirs, dirsErr = walk.NewDirTree(dirCtx, f, m.Dir, includeAll, ci.MaxDepth)
 			started = true
 		}

fs/walk/walk.go

@@ -64,7 +64,7 @@ type Func func(path string, entries fs.DirEntries, err error) error
 func Walk(ctx context.Context, f fs.Fs, path string, includeAll bool, maxLevel int, fn Func) error {
 	ci := fs.GetConfig(ctx)
 	fi := filter.GetConfig(ctx)
-	ctx = filter.SetUseFilter(ctx, f.Features().FilterAware && !includeAll) // make filter-aware backends constrain List
+	ctx = filter.SetUseFilter(ctx, !includeAll) // make filter-aware backends constrain List
 	if ci.NoTraverse && fi.HaveFilesFrom() {
 		return walkR(ctx, f, path, includeAll, maxLevel, fn, fi.MakeListR(ctx, f.NewObject))
 	}
@@ -158,7 +158,7 @@ func ListR(ctx context.Context, f fs.Fs, path string, includeAll bool, maxLevel
 		fi.UsesDirectoryFilters() { // ...using any directory filters
 		return listRwalk(ctx, f, path, includeAll, maxLevel, listType, fn)
 	}
-	ctx = filter.SetUseFilter(ctx, f.Features().FilterAware && !includeAll) // make filter-aware backends constrain List
+	ctx = filter.SetUseFilter(ctx, !includeAll) // make filter-aware backends constrain List
 	return listR(ctx, f, path, includeAll, listType, fn, doListR, listType.Dirs() && f.Features().BucketBased)
 }