diff --git a/backend/s3/s3.go b/backend/s3/s3.go index 1898cf969..eb5e879c2 100644 --- a/backend/s3/s3.go +++ b/backend/s3/s3.go @@ -1923,20 +1923,52 @@ func NewFs(ctx context.Context, name, root string, m configmap.Mapper) (fs.Fs, e newRoot, leaf := path.Split(oldRoot) f.setRoot(newRoot) _, err := f.NewObject(ctx, leaf) - if errors.Is(err, fs.ErrorObjectNotFound) { + switch { + case err == nil: + // It is a file so return an fs which points to the parent + return f, fs.ErrorIsFile + case errors.Is(err, fs.ErrorObjectNotFound): // File doesn't exist or is a directory so return old f f.setRoot(oldRoot) return f, nil + default: + // We couldn't HEAD the object so now attempt to list it + hasChildren, listErr := f.hasChildren(ctx, leaf) + if listErr != nil { + fs.Debugf(f, "Couldn't check %q for children after HEAD failed (%v): %v", oldRoot, err, listErr) + } + // If it listed and has children it must be a directory + if hasChildren { + f.setRoot(oldRoot) + return f, nil + } + // If it has no children it is either a file or an empty directory. We can't + // tell these two cases apart. We choose file which is more likely, and + // return an fs which points to the parent. + return f, fs.ErrorIsFile } - if err != nil { - return nil, err - } - // return an error with an fs which points to the parent - return f, fs.ErrorIsFile } return f, nil } +// hasChildren reports whether the directory dir contains any objects. +func (f *Fs) hasChildren(ctx context.Context, dir string) (found bool, err error) { + bucket, directory := f.split(dir) + err = f.list(ctx, listOpt{ + bucket: bucket, + directory: directory, + prefix: f.rootDirectory, + recurse: true, + }, func(remote string, object *types.Object, versionID *string, isDirectory bool) error { + found = true + return errEndList // stop after the first object + }) + if err != nil && err != fs.ErrorDirNotFound { + return false, err + } + return found, nil +} + // getMetaDataListing gets the metadata from the object unconditionally from the listing // // This is needed to find versioned objects from their paths. diff --git a/backend/s3/s3_internal_test.go b/backend/s3/s3_internal_test.go index 5a67ffbdb..1a719d899 100644 --- a/backend/s3/s3_internal_test.go +++ b/backend/s3/s3_internal_test.go @@ -144,6 +144,31 @@ func (f *Fs) InternalTestNoHead(t *testing.T) { } +func (f *Fs) InternalTestHasChildren(t *testing.T) { + ctx := context.Background() + contents := random.String(100) + item := fstest.NewItem("has-children/file.txt", contents, fstest.Time("2001-05-06T04:05:06.499999999Z")) + obj := fstests.PutTestContents(ctx, t, f, &item, contents, true) + defer func() { + assert.NoError(t, obj.Remove(ctx)) + }() + + // A prefix with an object under it is a directory + found, err := f.hasChildren(ctx, "has-children") + require.NoError(t, err) + assert.True(t, found, "prefix with an object should report children") + + // The object itself is a file so has no children + found, err = f.hasChildren(ctx, "has-children/file.txt") + require.NoError(t, err) + assert.False(t, found, "a file should not report children") + + // A path which doesn't exist has no children + found, err = f.hasChildren(ctx, "has-children/does-not-exist") + require.NoError(t, err) + assert.False(t, found, "a missing path should not report children") +} + func TestVersionLess(t *testing.T) { key1 := "key1" key2 := "key2" @@ -785,6 +810,7 @@ func (f *Fs) InternalTestObjectLock(t *testing.T) { func (f *Fs) InternalTest(t *testing.T) { t.Run("Metadata", f.InternalTestMetadata) t.Run("NoHead", f.InternalTestNoHead) + t.Run("HasChildren", f.InternalTestHasChildren) t.Run("Versions", f.InternalTestVersions) t.Run("ObjectLock", f.InternalTestObjectLock) }