From c917d2d5b41567902b417874e248af9859e5b972 Mon Sep 17 00:00:00 2001 From: Nick Craig-Wood Date: Mon, 5 Sep 2022 18:56:11 +0100 Subject: [PATCH] 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/s3/s3.go | 3 ++- backend/s3/s3_internal_test.go | 22 +++++++++++++++++++--- 2 files changed, 21 insertions(+), 4 deletions(-) diff --git a/backend/s3/s3.go b/backend/s3/s3.go index 7b3a81a29..ae96e239d 100644 --- a/backend/s3/s3.go +++ b/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 diff --git a/backend/s3/s3_internal_test.go b/backend/s3/s3_internal_test.go index 9d22ca69b..75e2bcec4 100644 --- a/backend/s3/s3_internal_test.go +++ b/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) {