From 3fbaa4c0b0e8076b9bc135fd51966f06dc207408 Mon Sep 17 00:00:00 2001 From: Nick Craig-Wood Date: Mon, 6 Sep 2021 13:54:08 +0100 Subject: [PATCH] backends: make NewObject return fs.ErrorIsDir if possible This changes the interface to NewObject so that if NewObject is called on a directory then it should return fs.ErrorIsDir if possible without doing any extra work, otherwise fs.ErrorObjectNotFound. Tested on integration test server with: go run integration-test.go -tests backend -run TestIntegration/FsMkdir/FsPutFiles/FsNewObjectDir -branch fix-stat -maxtries 1 --- backend/box/box.go | 3 +++ backend/drive/drive.go | 6 +++--- backend/dropbox/dropbox.go | 3 +++ backend/filefabric/filefabric.go | 2 +- backend/jottacloud/jottacloud.go | 8 +++++--- backend/koofr/koofr.go | 2 +- backend/local/local.go | 2 +- backend/mailru/mailru.go | 2 +- backend/mega/mega.go | 4 ++-- backend/onedrive/onedrive.go | 2 +- backend/putio/object.go | 2 +- backend/sftp/sftp.go | 2 +- backend/sharefile/sharefile.go | 8 ++++++-- backend/webdav/webdav.go | 4 ++-- backend/yandex/yandex.go | 4 +++- backend/zoho/zoho.go | 2 +- fs/fs.go | 1 + fs/types.go | 4 ++++ fstest/fstests/fstests.go | 4 ++-- 19 files changed, 42 insertions(+), 23 deletions(-) diff --git a/backend/box/box.go b/backend/box/box.go index 9ec830a98..2af0a857e 100644 --- a/backend/box/box.go +++ b/backend/box/box.go @@ -1184,6 +1184,9 @@ func (o *Object) Size() int64 { // setMetaData sets the metadata from info func (o *Object) setMetaData(info *api.Item) (err error) { + if info.Type == api.ItemTypeFolder { + return fs.ErrorIsDir + } if info.Type != api.ItemTypeFile { return errors.Wrapf(fs.ErrorNotAFile, "%q is %q", o.remote, info.Type) } diff --git a/backend/drive/drive.go b/backend/drive/drive.go index 72207c381..bdc80bc79 100755 --- a/backend/drive/drive.go +++ b/backend/drive/drive.go @@ -1374,7 +1374,7 @@ func (f *Fs) newObjectWithExportInfo( } switch { case info.MimeType == driveFolderType: - return nil, fs.ErrorNotAFile + return nil, fs.ErrorIsDir case info.MimeType == shortcutMimeType: // We can only get here if f.opt.SkipShortcuts is set // and not from a listing. This is unlikely. @@ -2922,7 +2922,7 @@ func (f *Fs) makeShortcut(ctx context.Context, srcPath string, dstFs *Fs, dstPat } isDir = true } else if srcObj, err := srcFs.NewObject(ctx, srcPath); err != nil { - if err != fs.ErrorNotAFile { + if err != fs.ErrorIsDir { return nil, errors.Wrap(err, "can't find source") } // source was a directory @@ -2942,7 +2942,7 @@ func (f *Fs) makeShortcut(ctx context.Context, srcPath string, dstFs *Fs, dstPat if err != fs.ErrorObjectNotFound { if err == nil { err = errors.New("existing file") - } else if err == fs.ErrorNotAFile { + } else if err == fs.ErrorIsDir { err = errors.New("existing directory") } return nil, errors.Wrap(err, "not overwriting shortcut target") diff --git a/backend/dropbox/dropbox.go b/backend/dropbox/dropbox.go index 70bd15181..efcaa34f1 100755 --- a/backend/dropbox/dropbox.go +++ b/backend/dropbox/dropbox.go @@ -624,6 +624,9 @@ func (f *Fs) getFileMetadata(ctx context.Context, filePath string) (fileInfo *fi } fileInfo, ok := entry.(*files.FileMetadata) if !ok { + if _, ok = entry.(*files.FolderMetadata); ok { + return nil, fs.ErrorIsDir + } return nil, fs.ErrorNotAFile } return fileInfo, nil diff --git a/backend/filefabric/filefabric.go b/backend/filefabric/filefabric.go index 9bc95ddfa..d515c4cc8 100644 --- a/backend/filefabric/filefabric.go +++ b/backend/filefabric/filefabric.go @@ -1089,7 +1089,7 @@ func (o *Object) Size() int64 { // setMetaData sets the metadata from info func (o *Object) setMetaData(info *api.Item) (err error) { if info.Type != api.ItemTypeFile { - return errors.Wrapf(fs.ErrorNotAFile, "%q is %q", o.remote, info.Type) + return fs.ErrorIsDir } o.hasMetaData = true o.size = info.Size diff --git a/backend/jottacloud/jottacloud.go b/backend/jottacloud/jottacloud.go index 5c6b802fd..4c2134857 100644 --- a/backend/jottacloud/jottacloud.go +++ b/backend/jottacloud/jottacloud.go @@ -599,7 +599,9 @@ func (f *Fs) readMetaDataForPath(ctx context.Context, path string) (info *api.Jo if err != nil { return nil, errors.Wrap(err, "read metadata failed") } - if result.XMLName.Local != "file" { + if result.XMLName.Local == "folder" { + return nil, fs.ErrorIsDir + } else if result.XMLName.Local != "file" { return nil, fs.ErrorNotAFile } return &result, nil @@ -762,7 +764,7 @@ func NewFs(ctx context.Context, name, root string, m configmap.Mapper) (fs.Fs, e // Renew the token in the background f.tokenRenewer = oauthutil.NewRenew(f.String(), ts, func() error { _, err := f.readMetaDataForPath(ctx, "") - if err == fs.ErrorNotAFile { + if err == fs.ErrorNotAFile || err == fs.ErrorIsDir { err = nil } return err @@ -784,7 +786,7 @@ func NewFs(ctx context.Context, name, root string, m configmap.Mapper) (fs.Fs, e } _, err := f.NewObject(context.TODO(), remote) if err != nil { - if errors.Cause(err) == fs.ErrorObjectNotFound || errors.Cause(err) == fs.ErrorNotAFile { + if uErr := errors.Cause(err); uErr == fs.ErrorObjectNotFound || uErr == fs.ErrorNotAFile || uErr == fs.ErrorIsDir { // File doesn't exist so return old f f.root = root return f, nil diff --git a/backend/koofr/koofr.go b/backend/koofr/koofr.go index 58eea15f7..d2d6339c0 100644 --- a/backend/koofr/koofr.go +++ b/backend/koofr/koofr.go @@ -344,7 +344,7 @@ func (f *Fs) NewObject(ctx context.Context, remote string) (obj fs.Object, err e return nil, translateErrorsObject(err) } if info.Type == "dir" { - return nil, fs.ErrorNotAFile + return nil, fs.ErrorIsDir } return &Object{ fs: f, diff --git a/backend/local/local.go b/backend/local/local.go index 65782308f..c932d605c 100644 --- a/backend/local/local.go +++ b/backend/local/local.go @@ -401,7 +401,7 @@ func (f *Fs) newObjectWithInfo(remote string, info os.FileInfo) (fs.Object, erro } if o.mode.IsDir() { - return nil, errors.Wrapf(fs.ErrorNotAFile, "%q", remote) + return nil, fs.ErrorIsDir } return o, nil } diff --git a/backend/mailru/mailru.go b/backend/mailru/mailru.go index 8a437cf37..74eb73444 100644 --- a/backend/mailru/mailru.go +++ b/backend/mailru/mailru.go @@ -1958,7 +1958,7 @@ func (o *Object) readMetaData(ctx context.Context, force bool) error { } newObj, ok := entry.(*Object) if !ok || dirSize >= 0 { - return fs.ErrorNotAFile + return fs.ErrorIsDir } if newObj.remote != o.remote { return fmt.Errorf("File %q path has changed to %q", o.remote, newObj.remote) diff --git a/backend/mega/mega.go b/backend/mega/mega.go index 36d41842e..d732d6559 100644 --- a/backend/mega/mega.go +++ b/backend/mega/mega.go @@ -303,7 +303,7 @@ func (f *Fs) findObject(rootNode *mega.Node, file string) (node *mega.Node, err if err == mega.ENOENT { return nil, fs.ErrorObjectNotFound } else if err == nil && node.GetType() != mega.FILE { - return nil, fs.ErrorNotAFile + return nil, fs.ErrorIsDir // all other node types are directories } return node, err } @@ -958,7 +958,7 @@ func (o *Object) Size() int64 { // setMetaData sets the metadata from info func (o *Object) setMetaData(info *mega.Node) (err error) { if info.GetType() != mega.FILE { - return fs.ErrorNotAFile + return fs.ErrorIsDir // all other node types are directories } o.info = info return nil diff --git a/backend/onedrive/onedrive.go b/backend/onedrive/onedrive.go index 556629d91..2cd47706c 100755 --- a/backend/onedrive/onedrive.go +++ b/backend/onedrive/onedrive.go @@ -1715,7 +1715,7 @@ func (o *Object) Size() int64 { // setMetaData sets the metadata from info func (o *Object) setMetaData(info *api.Item) (err error) { if info.GetFolder() != nil { - return errors.Wrapf(fs.ErrorNotAFile, "%q", o.remote) + return fs.ErrorIsDir } o.hasMetaData = true o.size = info.GetSize() diff --git a/backend/putio/object.go b/backend/putio/object.go index a4a1e8416..acc988c58 100644 --- a/backend/putio/object.go +++ b/backend/putio/object.go @@ -151,7 +151,7 @@ func (o *Object) readEntry(ctx context.Context) (f *putio.File, err error) { return nil, err } if resp.File.IsDir() { - return nil, fs.ErrorNotAFile + return nil, fs.ErrorIsDir } return &resp.File, err } diff --git a/backend/sftp/sftp.go b/backend/sftp/sftp.go index d1141e8e6..4f063c75b 100644 --- a/backend/sftp/sftp.go +++ b/backend/sftp/sftp.go @@ -1383,7 +1383,7 @@ func (o *Object) stat(ctx context.Context) error { return errors.Wrap(err, "stat failed") } if info.IsDir() { - return errors.Wrapf(fs.ErrorNotAFile, "%q", o.remote) + return fs.ErrorIsDir } o.setMetadata(info) return nil diff --git a/backend/sharefile/sharefile.go b/backend/sharefile/sharefile.go index 0c5404c2d..c8cbdf06c 100644 --- a/backend/sharefile/sharefile.go +++ b/backend/sharefile/sharefile.go @@ -337,8 +337,12 @@ func (f *Fs) readMetaDataForIDPath(ctx context.Context, id, path string, directo if directoriesOnly && item.Type != api.ItemTypeFolder { return nil, fs.ErrorIsFile } - if filesOnly && item.Type != api.ItemTypeFile { - return nil, fs.ErrorNotAFile + if filesOnly { + if item.Type == api.ItemTypeFolder { + return nil, fs.ErrorIsDir + } else if item.Type != api.ItemTypeFile { + return nil, fs.ErrorNotAFile + } } return &item, nil } diff --git a/backend/webdav/webdav.go b/backend/webdav/webdav.go index 86678bfad..76dabff0c 100644 --- a/backend/webdav/webdav.go +++ b/backend/webdav/webdav.go @@ -317,7 +317,7 @@ func (f *Fs) readMetaDataForPath(ctx context.Context, path string, depth string) return nil, fs.ErrorObjectNotFound } if itemIsDir(&item) { - return nil, fs.ErrorNotAFile + return nil, fs.ErrorIsDir } return &item.Props, nil } @@ -469,7 +469,7 @@ func NewFs(ctx context.Context, name, root string, m configmap.Mapper) (fs.Fs, e } _, err := f.NewObject(ctx, remote) if err != nil { - if errors.Cause(err) == fs.ErrorObjectNotFound || errors.Cause(err) == fs.ErrorNotAFile { + if errors.Cause(err) == fs.ErrorObjectNotFound || errors.Cause(err) == fs.ErrorIsDir { // File doesn't exist so return old f f.root = root return f, nil diff --git a/backend/yandex/yandex.go b/backend/yandex/yandex.go index 051a630f9..d1fa3321b 100644 --- a/backend/yandex/yandex.go +++ b/backend/yandex/yandex.go @@ -949,7 +949,9 @@ func (o *Object) readMetaData(ctx context.Context) (err error) { if err != nil { return err } - if info.ResourceType != "file" { + if info.ResourceType == "dir" { + return fs.ErrorIsDir + } else if info.ResourceType != "file" { return fs.ErrorNotAFile } return o.setMetaData(info) diff --git a/backend/zoho/zoho.go b/backend/zoho/zoho.go index f6454fec5..117ba74e3 100644 --- a/backend/zoho/zoho.go +++ b/backend/zoho/zoho.go @@ -1121,7 +1121,7 @@ func (o *Object) Size() int64 { // setMetaData sets the metadata from info func (o *Object) setMetaData(info *api.Item) (err error) { if info.Attributes.IsFolder { - return fs.ErrorNotAFile + return fs.ErrorIsDir } o.hasMetaData = true o.size = info.Attributes.StorageInfo.Size diff --git a/fs/fs.go b/fs/fs.go index 6ae75fd13..fb4885d34 100644 --- a/fs/fs.go +++ b/fs/fs.go @@ -37,6 +37,7 @@ var ( ErrorListAborted = errors.New("list aborted") ErrorListBucketRequired = errors.New("bucket or container name is needed in remote") ErrorIsFile = errors.New("is a file not a directory") + ErrorIsDir = errors.New("is a directory not a file") ErrorNotAFile = errors.New("is not a regular file") ErrorNotDeleting = errors.New("not deleting files as there were IO errors") ErrorNotDeletingDirs = errors.New("not deleting directories as there were IO errors") diff --git a/fs/types.go b/fs/types.go index a7cbba457..b340ce67b 100644 --- a/fs/types.go +++ b/fs/types.go @@ -28,6 +28,10 @@ type Fs interface { // NewObject finds the Object at remote. If it can't be found // it returns the error ErrorObjectNotFound. + // + // If remote points to a directory then it should return + // ErrorIsDir if possible without doing any extra work, + // otherwise ErrorObjectNotFound. NewObject(ctx context.Context, remote string) (Object, error) // Put in to the remote path with the modTime given of the given size diff --git a/fstest/fstests/fstests.go b/fstest/fstests/fstests.go index a5fc12e3e..31648e81e 100644 --- a/fstest/fstests/fstests.go +++ b/fstest/fstests/fstests.go @@ -1044,13 +1044,13 @@ func Run(t *testing.T, opt *Opt) { fstest.CheckListing(t, f, []fstest.Item{file1, file2}) }) - // TestFsNewObjectDir tests NewObject on a directory which should produce an error + // TestFsNewObjectDir tests NewObject on a directory which should produce fs.ErrorIsDir if possible or fs.ErrorObjectNotFound if not t.Run("FsNewObjectDir", func(t *testing.T) { skipIfNotOk(t) dir := path.Dir(file2.Path) obj, err := f.NewObject(ctx, dir) assert.Nil(t, obj) - assert.NotNil(t, err) + assert.True(t, err == fs.ErrorIsDir || err == fs.ErrorObjectNotFound, fmt.Sprintf("Wrong error: expecting fs.ErrorIsDir or fs.ErrorObjectNotFound but got: %#v", err)) }) // TestFsPurge tests Purge