From fb17dd9fe76ccf456bfe992d5ed1dbf80f94a1fb Mon Sep 17 00:00:00 2001 From: Nick Craig-Wood Date: Fri, 13 Mar 2026 16:47:08 +0000 Subject: [PATCH] dropbox: make shared_folders mode read-write - fixes #9221 Previously shared_folders mode blocked all write operations with "not supported in shared files mode". This adds full read-write support by using namespace-scoped API clients for all operations. --- backend/dropbox/dropbox.go | 475 ++++++++++++++++++++--- backend/dropbox/dropbox_internal_test.go | 311 +++++++++++---- 2 files changed, 673 insertions(+), 113 deletions(-) diff --git a/backend/dropbox/dropbox.go b/backend/dropbox/dropbox.go index 0f7815a22..e042bf70d 100644 --- a/backend/dropbox/dropbox.go +++ b/backend/dropbox/dropbox.go @@ -29,6 +29,7 @@ import ( "path" "regexp" "strings" + "sync" "time" "unicode/utf8" @@ -229,7 +230,10 @@ will be interpreted as the name of shared folder and rclone will list and access files within it using the shared folder's namespace. This works for all shared folder types including team folders. -Note that write operations (upload, delete, etc.) are not supported in this mode. +Write operations (upload, delete, mkdir, copy, move) are supported within +shared folders. Server-side copy and move operations only work within the +same shared folder - cross-shared-folder operations will fall back to +upload/download. See also --dropbox-root-namespace for an alternative way to work with shared folders.`, @@ -317,23 +321,25 @@ type Options struct { // Fs represents a remote dropbox server type Fs struct { - name string // name of this remote - root string // the path we are working on - opt Options // parsed options - ci *fs.ConfigInfo // global config - features *fs.Features // optional features - srv files.Client // the connection to the dropbox server - svc files.Client // the connection to the dropbox server (unauthorized) - sharing sharing.Client // as above, but for generating sharing links - users users.Client // as above, but for accessing user information - team team.Client // for the Teams API - slashRoot string // root with "/" prefix, lowercase - slashRootSlash string // root with "/" prefix and postfix, lowercase - pacer *fs.Pacer // To pace the API calls - ns string // The namespace we are using or "" for none - batcher *batcher.Batcher[*files.UploadSessionFinishArg, *files.FileMetadata] - exportExts []exportExtension - cfg dropbox.Config // SDK config for creating per-namespace clients + name string // name of this remote + root string // the path we are working on + opt Options // parsed options + ci *fs.ConfigInfo // global config + features *fs.Features // optional features + srv files.Client // the connection to the dropbox server + svc files.Client // the connection to the dropbox server (unauthorized) + sharing sharing.Client // as above, but for generating sharing links + users users.Client // as above, but for accessing user information + team team.Client // for the Teams API + slashRoot string // root with "/" prefix, lowercase + slashRootSlash string // root with "/" prefix and postfix, lowercase + pacer *fs.Pacer // To pace the API calls + ns string // The namespace we are using or "" for none + batcher *batcher.Batcher[*files.UploadSessionFinishArg, *files.FileMetadata] + exportExts []exportExtension + cfg dropbox.Config // SDK config for creating per-namespace clients + sharedFolderMu sync.Mutex // protects sharedFolderIDs + sharedFolderIDs map[string]string // cache of shared folder name → namespace ID } type exportType int @@ -373,6 +379,33 @@ func (o *Object) fileSrv() files.Client { return o.fs.srv } +// sharedFolderSrvAndPath resolves an rclone path in shared_folders mode +// to a namespace-scoped files client and the API path within that namespace. +// The rclone path format is "SharedFolderName/sub/path/file.txt". +func (f *Fs) sharedFolderSrvAndPath(ctx context.Context, rclonePath string) (srv files.Client, apiPath string, nsID string, err error) { + firstDir, subPath, _ := strings.Cut(rclonePath, "/") + nsID, err = f.findSharedFolder(ctx, firstDir) + if err != nil { + return nil, "", "", err + } + srv = files.New(f.cfg.WithNamespaceID(nsID)) + if subPath != "" { + apiPath = f.opt.Enc.FromStandardPath("/" + subPath) + } + return srv, apiPath, nsID, nil +} + +// nsPath returns the encoded API path for use with namespace-scoped clients. +// In shared_folders mode the object's remote is "SharedFolder/sub/file.txt" +// but the API path within the namespace is just "/sub/file.txt". +func (o *Object) nsPath() string { + if o.nsID != "" { + _, subPath, _ := strings.Cut(o.remote, "/") + return o.fs.opt.Enc.FromStandardPath("/" + subPath) + } + return o.fs.opt.Enc.FromStandardPath(o.remotePath()) +} + // Name of the remote (as passed into NewFs) func (f *Fs) Name() string { return f.name @@ -575,7 +608,8 @@ func NewFs(ctx context.Context, name, root string, m configmap.Mapper) (fs.Fs, e if f.opt.SharedFolders { f.setRoot(root) if f.root == "" { - return f, nil // our root it empty so we probably want to list shared folders + f.features.Fill(ctx, f) + return f, nil // our root is empty so we probably want to list shared folders } dir := path.Dir(f.root) @@ -1024,21 +1058,38 @@ func (f *Fs) listDir(ctx context.Context, srv files.Client, root string, dir str // findSharedFolder find the id for a given shared folder name // somewhat annoyingly there is no endpoint to query a shared folder by it's name // so our only option is to iterate over all shared folders +// +// Results are cached for the lifetime of the Fs. func (f *Fs) findSharedFolder(ctx context.Context, name string) (id string, err error) { - errFoundFile := errors.New("found file") - err = f.listSharedFolders(ctx, func(entry fs.DirEntry) error { - if entry.(*fs.Dir).Remote() == name { - id = entry.(*fs.Dir).ID() - return errFoundFile + f.sharedFolderMu.Lock() + if f.sharedFolderIDs != nil { + if cachedID, ok := f.sharedFolderIDs[name]; ok { + f.sharedFolderMu.Unlock() + return cachedID, nil } + } + f.sharedFolderMu.Unlock() + + // Not cached - list all shared folders and populate the cache + found := make(map[string]string) + err = f.listSharedFolders(ctx, func(entry fs.DirEntry) error { + d := entry.(*fs.Dir) + found[d.Remote()] = d.ID() return nil }) - if errors.Is(err, errFoundFile) { - return id, nil - } else if err != nil { + if err != nil { return "", err } - return "", fs.ErrorDirNotFound + + f.sharedFolderMu.Lock() + f.sharedFolderIDs = found + f.sharedFolderMu.Unlock() + + id, ok := found[name] + if !ok { + return "", fs.ErrorDirNotFound + } + return id, nil } // mountSharedFolder mount a shared folder to the root namespace @@ -1196,7 +1247,7 @@ func (f *Fs) ListP(ctx context.Context, dir string, callback fs.ListRCallback) ( // // The new object may have been created if an error is returned func (f *Fs) Put(ctx context.Context, in io.Reader, src fs.ObjectInfo, options ...fs.OpenOption) (fs.Object, error) { - if f.opt.SharedFiles || f.opt.SharedFolders { + if f.opt.SharedFiles { return nil, errNotSupportedInSharedMode } // Temporary Object under construction @@ -1204,6 +1255,17 @@ func (f *Fs) Put(ctx context.Context, in io.Reader, src fs.ObjectInfo, options . fs: f, remote: src.Remote(), } + if f.opt.SharedFolders { + firstDir, _, ok := strings.Cut(src.Remote(), "/") + if !ok { + return nil, errors.New("can't upload to shared folder root") + } + nsID, err := f.findSharedFolder(ctx, firstDir) + if err != nil { + return nil, err + } + o.nsID = nsID + } return o, o.Update(ctx, in, src, options...) } @@ -1214,9 +1276,12 @@ func (f *Fs) PutStream(ctx context.Context, in io.Reader, src fs.ObjectInfo, opt // Mkdir creates the container if it doesn't exist func (f *Fs) Mkdir(ctx context.Context, dir string) error { - if f.opt.SharedFiles || f.opt.SharedFolders { + if f.opt.SharedFiles { return errNotSupportedInSharedMode } + if f.opt.SharedFolders { + return f.mkdirSharedFolder(ctx, dir) + } root := path.Join(f.slashRoot, dir) // can't create or run metadata on root @@ -1247,6 +1312,48 @@ func (f *Fs) Mkdir(ctx context.Context, dir string) error { return err } +// mkdirSharedFolder creates a directory within a shared folder. +// Creating shared folders themselves is not supported. +func (f *Fs) mkdirSharedFolder(ctx context.Context, dir string) error { + if dir == "" { + return nil // root of shared folders listing - nothing to create + } + if !strings.Contains(dir, "/") { + // This is just a shared folder name - it already exists + // (you can't create shared folders via the files API) + _, err := f.findSharedFolder(ctx, dir) + return err + } + srv, apiPath, _, err := f.sharedFolderSrvAndPath(ctx, dir) + if err != nil { + return err + } + if apiPath == "" { + return nil // root of shared folder already exists + } + // Check if directory exists by trying to get metadata + var entry files.IsMetadata + err = f.pacer.Call(func() (bool, error) { + entry, err = srv.GetMetadata(&files.GetMetadataArg{Path: apiPath}) + return shouldRetry(ctx, err) + }) + if err == nil { + if _, ok := entry.(*files.FolderMetadata); ok { + return nil // directory exists already + } + } + // Create it + arg := files.CreateFolderArg{Path: apiPath} + if cErr := checkPathLength(arg.Path); cErr != nil { + return cErr + } + err = f.pacer.Call(func() (bool, error) { + _, err = srv.CreateFolderV2(&arg) + return shouldRetry(ctx, err) + }) + return err +} + // purgeCheck removes the root directory, if check is set then it // refuses to do so if it has anything in func (f *Fs) purgeCheck(ctx context.Context, dir string, check bool) (err error) { @@ -1296,12 +1403,73 @@ func (f *Fs) purgeCheck(ctx context.Context, dir string, check bool) (err error) // // Returns an error if it isn't empty func (f *Fs) Rmdir(ctx context.Context, dir string) error { - if f.opt.SharedFiles || f.opt.SharedFolders { + if f.opt.SharedFiles { return errNotSupportedInSharedMode } + if f.opt.SharedFolders { + return f.rmdirSharedFolder(ctx, dir) + } return f.purgeCheck(ctx, dir, true) } +// rmdirSharedFolder removes a directory within a shared folder. +// Removing the shared folder itself (top-level dir) is not allowed. +func (f *Fs) rmdirSharedFolder(ctx context.Context, dir string) error { + if dir == "" || !strings.Contains(dir, "/") { + return errors.New("can't remove shared folder itself - only subdirectories within it") + } + return f.purgeCheckSharedFolder(ctx, dir, true) +} + +// purgeCheckSharedFolder removes a directory within a shared folder. +// If check is true, it verifies the directory is empty first. +func (f *Fs) purgeCheckSharedFolder(ctx context.Context, dir string, check bool) error { + if dir == "" || !strings.Contains(dir, "/") { + return errors.New("can't remove shared folder itself - only subdirectories within it") + } + srv, apiPath, _, err := f.sharedFolderSrvAndPath(ctx, dir) + if err != nil { + return err + } + if apiPath == "" { + return errors.New("can't remove shared folder root") + } + if check { + // Check directory exists + var entry files.IsMetadata + err = f.pacer.Call(func() (bool, error) { + entry, err = srv.GetMetadata(&files.GetMetadataArg{Path: apiPath}) + return shouldRetry(ctx, err) + }) + if err != nil { + return fmt.Errorf("Rmdir: %w", err) + } + if _, ok := entry.(*files.FolderMetadata); !ok { + return fmt.Errorf("Rmdir: %w", fs.ErrorIsFile) + } + // Check directory is empty + arg := files.NewListFolderArg(apiPath) + arg.Recursive = false + var res *files.ListFolderResult + err = f.pacer.Call(func() (bool, error) { + res, err = srv.ListFolder(arg) + return shouldRetry(ctx, err) + }) + if err != nil { + return fmt.Errorf("Rmdir: %w", err) + } + if len(res.Entries) != 0 { + return errors.New("directory not empty") + } + } + // Remove it + err = f.pacer.Call(func() (bool, error) { + _, err = srv.DeleteV2(&files.DeleteArg{Path: apiPath}) + return shouldRetry(ctx, err) + }) + return err +} + // Precision returns the precision func (f *Fs) Precision() time.Duration { return time.Second @@ -1323,6 +1491,10 @@ func (f *Fs) Copy(ctx context.Context, src fs.Object, remote string) (dst fs.Obj return nil, fs.ErrorCantCopy } + if f.opt.SharedFolders { + return f.copySharedFolder(ctx, srcObj, remote) + } + // Find and remove existing object cleanup, err := operations.RemoveExisting(ctx, f, remote, "server side copy") if err != nil { @@ -1365,12 +1537,76 @@ func (f *Fs) Copy(ctx context.Context, src fs.Object, remote string) (dst fs.Obj return dstObj, nil } +// copySharedFolder copies an object within shared folders. +// Both source and destination must be in the same shared folder namespace. +func (f *Fs) copySharedFolder(ctx context.Context, srcObj *Object, remote string) (fs.Object, error) { + srcFirstDir, _, _ := strings.Cut(srcObj.remote, "/") + dstFirstDir, _, ok := strings.Cut(remote, "/") + if !ok { + return nil, fs.ErrorCantCopy + } + srcNsID, err := f.findSharedFolder(ctx, srcFirstDir) + if err != nil { + return nil, fs.ErrorCantCopy + } + dstNsID, err := f.findSharedFolder(ctx, dstFirstDir) + if err != nil { + return nil, fs.ErrorCantCopy + } + if srcNsID != dstNsID { + return nil, fs.ErrorCantCopy + } + srv := files.New(f.cfg.WithNamespaceID(srcNsID)) + + // Find and remove existing object + cleanup, err := operations.RemoveExisting(ctx, f, remote, "server side copy") + if err != nil { + return nil, err + } + defer cleanup(&err) + + dstObj := &Object{ + fs: f, + remote: remote, + nsID: srcNsID, + } + + _, srcSubPath, _ := strings.Cut(srcObj.remote, "/") + _, dstSubPath, _ := strings.Cut(remote, "/") + arg := files.RelocationArg{ + RelocationPath: files.RelocationPath{ + FromPath: f.opt.Enc.FromStandardPath("/" + srcSubPath), + ToPath: f.opt.Enc.FromStandardPath("/" + dstSubPath), + }, + } + var result *files.RelocationResult + err = f.pacer.Call(func() (bool, error) { + result, err = srv.CopyV2(&arg) + return shouldRetry(ctx, err) + }) + if err != nil { + return nil, fmt.Errorf("copy failed: %w", err) + } + fileInfo, ok := result.Metadata.(*files.FileMetadata) + if !ok { + return nil, fs.ErrorNotAFile + } + err = dstObj.setMetadataFromEntry(fileInfo) + if err != nil { + return nil, fmt.Errorf("copy failed: %w", err) + } + return dstObj, nil +} + // Purge deletes all the files and the container // // Optional interface: Only implement this if you have a way of // deleting all the files quicker than just running Remove() on the // result of List() func (f *Fs) Purge(ctx context.Context, dir string) (err error) { + if f.opt.SharedFolders { + return f.purgeCheckSharedFolder(ctx, dir, false) + } return f.purgeCheck(ctx, dir, false) } @@ -1390,6 +1626,10 @@ func (f *Fs) Move(ctx context.Context, src fs.Object, remote string) (fs.Object, return nil, fs.ErrorCantMove } + if f.opt.SharedFolders { + return f.moveSharedFolder(ctx, srcObj, remote) + } + // Temporary Object under construction dstObj := &Object{ fs: f, @@ -1435,6 +1675,67 @@ func (f *Fs) Move(ctx context.Context, src fs.Object, remote string) (fs.Object, return dstObj, nil } +// moveSharedFolder moves an object within shared folders. +// Both source and destination must be in the same shared folder namespace. +func (f *Fs) moveSharedFolder(ctx context.Context, srcObj *Object, remote string) (fs.Object, error) { + srcFirstDir, _, _ := strings.Cut(srcObj.remote, "/") + dstFirstDir, _, ok := strings.Cut(remote, "/") + if !ok { + return nil, fs.ErrorCantMove + } + srcNsID, err := f.findSharedFolder(ctx, srcFirstDir) + if err != nil { + return nil, fs.ErrorCantMove + } + dstNsID, err := f.findSharedFolder(ctx, dstFirstDir) + if err != nil { + return nil, fs.ErrorCantMove + } + if srcNsID != dstNsID { + return nil, fs.ErrorCantMove + } + srv := files.New(f.cfg.WithNamespaceID(srcNsID)) + + dstObj := &Object{ + fs: f, + remote: remote, + nsID: srcNsID, + } + + _, srcSubPath, _ := strings.Cut(srcObj.remote, "/") + _, dstSubPath, _ := strings.Cut(remote, "/") + arg := files.RelocationArg{ + RelocationPath: files.RelocationPath{ + FromPath: f.opt.Enc.FromStandardPath("/" + srcSubPath), + ToPath: f.opt.Enc.FromStandardPath("/" + dstSubPath), + }, + } + var result *files.RelocationResult + err = f.pacer.Call(func() (bool, error) { + result, err = srv.MoveV2(&arg) + switch e := err.(type) { + case files.MoveV2APIError: + if e.EndpointError != nil && e.EndpointError.FromLookup != nil && e.EndpointError.FromLookup.Tag == files.LookupErrorNotFound { + fs.Debugf(srcObj, "Retrying move on %v error", err) + return true, err + } + } + return shouldRetry(ctx, err) + }) + if err != nil { + return nil, fmt.Errorf("move failed: %w", err) + } + fileInfo, ok := result.Metadata.(*files.FileMetadata) + if !ok { + return nil, fs.ErrorNotAFile + } + err = dstObj.setMetadataFromEntry(fileInfo) + if err != nil { + return nil, fmt.Errorf("move failed: %w", err) + } + return dstObj, nil +} + // PublicLink adds a "readable by anyone with link" permission on the given file or folder. func (f *Fs) PublicLink(ctx context.Context, remote string, expire fs.Duration, unlink bool) (link string, err error) { absPath := f.opt.Enc.FromStandardPath(path.Join(f.slashRoot, remote)) @@ -1522,6 +1823,11 @@ func (f *Fs) DirMove(ctx context.Context, src fs.Fs, srcRemote, dstRemote string fs.Debugf(srcFs, "Can't move directory - not same remote type") return fs.ErrorCantDirMove } + + if f.opt.SharedFolders { + return f.dirMoveSharedFolder(ctx, srcFs, srcRemote, dstRemote) + } + srcPath := path.Join(srcFs.slashRoot, srcRemote) dstPath := path.Join(f.slashRoot, dstRemote) @@ -1554,6 +1860,57 @@ func (f *Fs) DirMove(ctx context.Context, src fs.Fs, srcRemote, dstRemote string return nil } +// dirMoveSharedFolder moves a directory within shared folders. +// Both source and destination must be in the same shared folder namespace. +func (f *Fs) dirMoveSharedFolder(ctx context.Context, srcFs *Fs, srcRemote, dstRemote string) error { + srcFirstDir, srcSubPath, _ := strings.Cut(srcRemote, "/") + dstFirstDir, dstSubPath, _ := strings.Cut(dstRemote, "/") + if srcSubPath == "" || dstSubPath == "" { + return errors.New("can't move shared folder itself - only subdirectories within it") + } + srcNsID, err := srcFs.findSharedFolder(ctx, srcFirstDir) + if err != nil { + return fs.ErrorCantDirMove + } + dstNsID, err := f.findSharedFolder(ctx, dstFirstDir) + if err != nil { + return fs.ErrorCantDirMove + } + if srcNsID != dstNsID { + return fs.ErrorCantDirMove + } + srv := files.New(f.cfg.WithNamespaceID(srcNsID)) + + // Check if destination exists + dstAPIPath := f.opt.Enc.FromStandardPath("/" + dstSubPath) + var entry files.IsMetadata + err = f.pacer.Call(func() (bool, error) { + entry, err = srv.GetMetadata(&files.GetMetadataArg{Path: dstAPIPath}) + return shouldRetry(ctx, err) + }) + if err == nil { + if _, ok := entry.(*files.FolderMetadata); ok { + return fs.ErrorDirExists + } + } + + // Do the move + arg := files.RelocationArg{ + RelocationPath: files.RelocationPath{ + FromPath: f.opt.Enc.FromStandardPath("/" + srcSubPath), + ToPath: dstAPIPath, + }, + } + err = f.pacer.Call(func() (bool, error) { + _, err = srv.MoveV2(&arg) + return shouldRetry(ctx, err) + }) + if err != nil { + return fmt.Errorf("MoveDir failed: %w", err) + } + return nil +} + // About gets quota information func (f *Fs) About(ctx context.Context) (usage *fs.Usage, err error) { var q *users.SpaceUsage @@ -1818,7 +2175,7 @@ func (o *Object) ID() string { // Hash returns the dropbox special hash func (o *Object) Hash(ctx context.Context, t hash.Type) (string, error) { - if o.fs.opt.SharedFiles || o.fs.opt.SharedFolders { + if o.fs.opt.SharedFiles { return "", errNotSupportedInSharedMode } if t != DbHashType { @@ -1879,6 +2236,29 @@ func (o *Object) setMetadataFromEntry(info *files.FileMetadata) error { // Reads the entry for a file from dropbox func (o *Object) readEntry(ctx context.Context) (*files.FileMetadata, error) { + if o.nsID != "" { + srv := o.fileSrv() + var entry files.IsMetadata + var err error + err = o.fs.pacer.Call(func() (bool, error) { + entry, err = srv.GetMetadata(&files.GetMetadataArg{Path: o.nsPath()}) + return shouldRetry(ctx, err) + }) + if err != nil { + switch e := err.(type) { + case files.GetMetadataAPIError: + if e.EndpointError != nil && e.EndpointError.Path != nil && e.EndpointError.Path.Tag == files.LookupErrorNotFound { + return nil, fs.ErrorObjectNotFound + } + } + return nil, err + } + fileInfo, ok := entry.(*files.FileMetadata) + if !ok { + return nil, fs.ErrorNotAFile + } + return fileInfo, nil + } return o.fs.getFileMetadata(ctx, o.remotePath()) } @@ -2010,11 +2390,11 @@ func (o *Object) Open(ctx context.Context, options ...fs.OpenOption) (in io.Read // Will introduce two additional network requests to start and finish the session. // If the size is unknown (i.e. -1) the method incurs one additional // request to the Dropbox API that does not carry a payload to close the append session. -func (o *Object) uploadChunked(ctx context.Context, in0 io.Reader, commitInfo *files.CommitInfo, size int64) (entry *files.FileMetadata, err error) { +func (o *Object) uploadChunked(ctx context.Context, in0 io.Reader, srv files.Client, commitInfo *files.CommitInfo, size int64, useBatching bool) (entry *files.FileMetadata, err error) { // start upload var res *files.UploadSessionStartResult err = o.fs.pacer.Call(func() (bool, error) { - res, err = o.fs.srv.UploadSessionStart(&files.UploadSessionStartArg{}, nil) + res, err = srv.UploadSessionStart(&files.UploadSessionStartArg{}, nil) return shouldRetry(ctx, err) }) if err != nil { @@ -2051,7 +2431,7 @@ func (o *Object) uploadChunked(ctx context.Context, in0 io.Reader, commitInfo *f if _, err = chunk.Seek(skip, io.SeekStart); err != nil { return false, err } - err = o.fs.srv.UploadSessionAppendV2(&appendArg, chunk) + err = srv.UploadSessionAppendV2(&appendArg, chunk) // after session is started, we retry everything if err != nil { // Check for incorrect offset error and retry with new offset @@ -2105,12 +2485,12 @@ func (o *Object) uploadChunked(ctx context.Context, in0 io.Reader, commitInfo *f } // If we are batching then we should have written all the data now // store the commit info now for a batch commit - if o.fs.batcher.Batching() { + if useBatching { return o.fs.batcher.Commit(ctx, o.remote, args) } err = o.fs.pacer.Call(func() (bool, error) { - entry, err = o.fs.srv.UploadSessionFinish(args, nil) + entry, err = srv.UploadSessionFinish(args, nil) if retry, err := shouldRetryExclude(ctx, err); !retry { return retry, err } @@ -2154,14 +2534,15 @@ func checkPathLength(name string) (err error) { // // The new object may have been created if an error is returned func (o *Object) Update(ctx context.Context, in io.Reader, src fs.ObjectInfo, options ...fs.OpenOption) error { - if o.fs.opt.SharedFiles || o.fs.opt.SharedFolders { + if o.fs.opt.SharedFiles { return errNotSupportedInSharedMode } + uploadPath := o.nsPath() remote := o.remotePath() if ignoredFiles.MatchString(remote) { return fserrors.NoRetryError(fmt.Errorf("file name %q is disallowed - not uploading", path.Base(remote))) } - commitInfo := files.NewCommitInfo(o.fs.opt.Enc.FromStandardPath(o.remotePath())) + commitInfo := files.NewCommitInfo(uploadPath) commitInfo.Mode.Tag = "overwrite" // The Dropbox API only accepts timestamps in UTC with second precision. clientModified := src.ModTime(ctx).UTC().Round(time.Second) @@ -2171,14 +2552,17 @@ func (o *Object) Update(ctx context.Context, in io.Reader, src fs.ObjectInfo, op return cErr } + srv := o.fileSrv() size := src.Size() + // Disable batching for shared folder objects since the batch commit uses f.srv + useBatching := o.fs.batcher.Batching() && o.nsID == "" var err error var entry *files.FileMetadata - if size > int64(o.fs.opt.ChunkSize) || size < 0 || o.fs.batcher.Batching() { - entry, err = o.uploadChunked(ctx, in, commitInfo, size) + if size > int64(o.fs.opt.ChunkSize) || size < 0 || useBatching { + entry, err = o.uploadChunked(ctx, in, srv, commitInfo, size, useBatching) } else { err = o.fs.pacer.CallNoRetry(func() (bool, error) { - entry, err = o.fs.srv.Upload(&files.UploadArg{CommitInfo: *commitInfo}, in) + entry, err = srv.Upload(&files.UploadArg{CommitInfo: *commitInfo}, in) return shouldRetry(ctx, err) }) } @@ -2199,12 +2583,13 @@ func (o *Object) Update(ctx context.Context, in io.Reader, src fs.ObjectInfo, op // Remove an object func (o *Object) Remove(ctx context.Context) (err error) { - if o.fs.opt.SharedFiles || o.fs.opt.SharedFolders { + if o.fs.opt.SharedFiles { return errNotSupportedInSharedMode } + srv := o.fileSrv() err = o.fs.pacer.Call(func() (bool, error) { - _, err = o.fs.srv.DeleteV2(&files.DeleteArg{ - Path: o.fs.opt.Enc.FromStandardPath(o.remotePath()), + _, err = srv.DeleteV2(&files.DeleteArg{ + Path: o.nsPath(), }) return shouldRetry(ctx, err) }) diff --git a/backend/dropbox/dropbox_internal_test.go b/backend/dropbox/dropbox_internal_test.go index 32dda4505..c3c1e3dde 100644 --- a/backend/dropbox/dropbox_internal_test.go +++ b/backend/dropbox/dropbox_internal_test.go @@ -1,6 +1,7 @@ package dropbox import ( + "bytes" "context" "io" "strings" @@ -12,6 +13,7 @@ import ( "github.com/dropbox/dropbox-sdk-go-unofficial/v6/dropbox/files" "github.com/dropbox/dropbox-sdk-go-unofficial/v6/dropbox/sharing" "github.com/rclone/rclone/fs" + "github.com/rclone/rclone/fs/object" "github.com/rclone/rclone/fstest/fstests" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -99,42 +101,25 @@ func (f *Fs) InternalTestPaperExport(t *testing.T) { require.Contains(t, text, excerpt) } } -func (f *Fs) InternalTestSharedFolderList(t *testing.T) { + +// InternalTestSharedFolder tests both read and write operations on shared folders. +// It uses a single shared folder setup to avoid re-sharing between subtests +// (Dropbox has eventual consistency issues with share/unshare cycles). +func (f *Fs) InternalTestSharedFolder(t *testing.T) { ctx := context.Background() - - // Create a subfolder and a file inside it using the normal API. - // The test Fs root is something like /rclone-test-xxxxx so the - // shared folder will be the test root directory itself. testDir := f.slashRoot - testFileName := "shared-folder-test-file.txt" - testFilePath := testDir + "/" + testFileName - - // Upload a small test file - content := "hello shared folder" - uploadArg := files.NewUploadArg(testFilePath) - uploadArg.Mode = &files.WriteMode{Tagged: dropbox.Tagged{Tag: files.WriteModeOverwrite}} - var err error - err = f.pacer.Call(func() (bool, error) { - _, err = f.srv.Upload(uploadArg, strings.NewReader(content)) - return shouldRetry(ctx, err) - }) - require.NoError(t, err) - defer func() { - // Clean up test file - _, _ = f.srv.DeleteV2(files.NewDeleteArg(testFilePath)) - }() // Share the test root folder folderName := f.opt.Enc.ToStandardName(strings.TrimPrefix(testDir, "/")) shareArg := sharing.NewShareFolderArg(testDir) var sharedFolderID string var shareLaunch *sharing.ShareFolderLaunch + var err error err = f.pacer.Call(func() (bool, error) { shareLaunch, err = f.sharing.ShareFolder(shareArg) return shouldRetry(ctx, err) }) if err != nil { - // If the folder is already shared, find its ID sharedFolderID, err = f.findSharedFolder(ctx, folderName) require.NoError(t, err, "ShareFolder failed and couldn't find existing share") } else { @@ -142,7 +127,6 @@ func (f *Fs) InternalTestSharedFolderList(t *testing.T) { case sharing.ShareFolderLaunchComplete: sharedFolderID = shareLaunch.Complete.SharedFolderId case sharing.ShareFolderLaunchAsyncJobId: - // Poll for completion pollArg := async.PollArg{AsyncJobId: shareLaunch.AsyncJobId} for range 30 { time.Sleep(time.Second) @@ -163,7 +147,6 @@ func (f *Fs) InternalTestSharedFolderList(t *testing.T) { require.NotEmpty(t, sharedFolderID) defer func() { - // Unshare the folder when done unshareArg := sharing.NewUnshareFolderArg(sharedFolderID) unshareArg.LeaveACopy = true _ = f.pacer.Call(func() (bool, error) { @@ -172,64 +155,256 @@ func (f *Fs) InternalTestSharedFolderList(t *testing.T) { }) }() - // Now test listing with shared_folders mode. - // Create a copy of the Fs with SharedFolders enabled. - sharedFs := *f - sharedFs.opt.SharedFolders = true + sf := *f //nolint:govet // intentional copy; mutex is zero-valued and unused before this point + sf.opt.SharedFolders = true + sf.features = (&fs.Features{ + CaseInsensitive: true, + ReadMimeType: false, + CanHaveEmptyDirectories: true, + }).Fill(ctx, &sf) + sharedFs := &sf - // Test 1: listing root should include the shared folder - var rootEntries fs.DirEntries - err = sharedFs.ListP(ctx, "", func(entries fs.DirEntries) error { - rootEntries = append(rootEntries, entries...) - return nil + // Upload a test file using the normal API for listing tests + testFileName := "shared-folder-test-file.txt" + testFilePath := testDir + "/" + testFileName + content := "hello shared folder" + uploadArg := files.NewUploadArg(testFilePath) + uploadArg.Mode = &files.WriteMode{Tagged: dropbox.Tagged{Tag: files.WriteModeOverwrite}} + err = f.pacer.Call(func() (bool, error) { + _, err = f.srv.Upload(uploadArg, strings.NewReader(content)) + return shouldRetry(ctx, err) }) require.NoError(t, err) + defer func() { + _, _ = f.srv.DeleteV2(files.NewDeleteArg(testFilePath)) + }() - found := false - for _, entry := range rootEntries { - if entry.Remote() == folderName { - found = true - break + t.Run("List", func(t *testing.T) { + // listing root should include the shared folder + var rootEntries fs.DirEntries + err = sharedFs.ListP(ctx, "", func(entries fs.DirEntries) error { + rootEntries = append(rootEntries, entries...) + return nil + }) + require.NoError(t, err) + + found := false + for _, entry := range rootEntries { + if entry.Remote() == folderName { + found = true + break + } } - } - assert.True(t, found, "shared folder %q not found in root listing (have %v)", folderName, rootEntries) + assert.True(t, found, "shared folder %q not found in root listing (have %v)", folderName, rootEntries) - // Test 2: listing the shared folder should show its contents - var dirEntries fs.DirEntries - err = sharedFs.ListP(ctx, folderName, func(entries fs.DirEntries) error { - dirEntries = append(dirEntries, entries...) - return nil + // listing the shared folder should show its contents + var dirEntries fs.DirEntries + err = sharedFs.ListP(ctx, folderName, func(entries fs.DirEntries) error { + dirEntries = append(dirEntries, entries...) + return nil + }) + require.NoError(t, err) + + foundFile := false + expectedRemote := folderName + "/" + testFileName + for _, entry := range dirEntries { + if entry.Remote() == expectedRemote { + foundFile = true + break + } + } + assert.True(t, foundFile, "file %q not found in shared folder listing (have %v)", expectedRemote, dirEntries) }) - require.NoError(t, err) - foundFile := false - expectedRemote := folderName + "/" + testFileName - for _, entry := range dirEntries { - if entry.Remote() == expectedRemote { - foundFile = true - break + t.Run("NewObject", func(t *testing.T) { + expectedRemote := folderName + "/" + testFileName + obj, err := sharedFs.NewObject(ctx, expectedRemote) + require.NoError(t, err, "NewObject failed for shared folder file") + assert.Equal(t, expectedRemote, obj.Remote()) + assert.Equal(t, int64(len(content)), obj.Size()) + }) + + t.Run("Open", func(t *testing.T) { + expectedRemote := folderName + "/" + testFileName + obj, err := sharedFs.NewObject(ctx, expectedRemote) + require.NoError(t, err) + rc, err := obj.Open(ctx) + require.NoError(t, err, "Open failed for shared folder file") + buf, err := io.ReadAll(rc) + require.NoError(t, rc.Close()) + require.NoError(t, err) + assert.Equal(t, content, string(buf)) + }) + + t.Run("Put", func(t *testing.T) { + // Upload a file via Put + content := "hello from shared folder put" + remote := folderName + "/test-put-file.txt" + src := object.NewStaticObjectInfo(remote, time.Now(), int64(len(content)), true, nil, nil) + obj, err := sharedFs.Put(ctx, bytes.NewReader([]byte(content)), src) + require.NoError(t, err) + assert.Equal(t, remote, obj.Remote()) + assert.Equal(t, int64(len(content)), obj.Size()) + + // Verify we can read back the content + rc, err := obj.Open(ctx) + require.NoError(t, err) + buf, err := io.ReadAll(rc) + require.NoError(t, rc.Close()) + require.NoError(t, err) + assert.Equal(t, content, string(buf)) + + // Clean up + require.NoError(t, obj.Remove(ctx)) + }) + + t.Run("Hash", func(t *testing.T) { + // Upload a file and check its hash + content := "hash test content" + remote := folderName + "/test-hash-file.txt" + src := object.NewStaticObjectInfo(remote, time.Now(), int64(len(content)), true, nil, nil) + obj, err := sharedFs.Put(ctx, bytes.NewReader([]byte(content)), src) + require.NoError(t, err) + defer func() { _ = obj.Remove(ctx) }() + + hash, err := obj.Hash(ctx, DbHashType) + require.NoError(t, err) + assert.NotEmpty(t, hash, "hash should not be empty") + }) + + t.Run("Mkdir", func(t *testing.T) { + // Create a subdirectory within the shared folder + subDir := folderName + "/test-mkdir-subdir" + err := sharedFs.Mkdir(ctx, subDir) + require.NoError(t, err) + + // Verify it appears in the listing + var entries fs.DirEntries + err = sharedFs.ListP(ctx, folderName, func(e fs.DirEntries) error { + entries = append(entries, e...) + return nil + }) + require.NoError(t, err) + found := false + for _, entry := range entries { + if entry.Remote() == subDir { + found = true + break + } } - } - assert.True(t, foundFile, "file %q not found in shared folder listing (have %v)", expectedRemote, dirEntries) + assert.True(t, found, "subdirectory %q not found in listing", subDir) - // Test 3: NewObject should find a file inside a shared folder - obj, err := sharedFs.NewObject(ctx, expectedRemote) - require.NoError(t, err, "NewObject failed for shared folder file") - assert.Equal(t, expectedRemote, obj.Remote()) - assert.Equal(t, int64(len(content)), obj.Size()) + // Clean up + require.NoError(t, sharedFs.Rmdir(ctx, subDir)) + }) - // Test 4: Open should be able to read the file contents - rc, err := obj.Open(ctx) - require.NoError(t, err, "Open failed for shared folder file") - buf, err := io.ReadAll(rc) - require.NoError(t, rc.Close()) - require.NoError(t, err) - assert.Equal(t, content, string(buf)) + t.Run("Rmdir", func(t *testing.T) { + // Create and then remove a subdirectory + subDir := folderName + "/test-rmdir-subdir" + require.NoError(t, sharedFs.Mkdir(ctx, subDir)) + require.NoError(t, sharedFs.Rmdir(ctx, subDir)) + + // Verify it's gone + var entries fs.DirEntries + err := sharedFs.ListP(ctx, folderName, func(e fs.DirEntries) error { + entries = append(entries, e...) + return nil + }) + require.NoError(t, err) + for _, entry := range entries { + assert.NotEqual(t, subDir, entry.Remote(), "subdirectory should have been removed") + } + }) + + t.Run("RmdirSharedFolderRoot", func(t *testing.T) { + // Removing a shared folder itself should fail + err := sharedFs.Rmdir(ctx, folderName) + assert.Error(t, err, "should not be able to remove shared folder itself") + }) + + t.Run("Remove", func(t *testing.T) { + // Upload and then delete a file + content := "file to remove" + remote := folderName + "/test-remove-file.txt" + src := object.NewStaticObjectInfo(remote, time.Now(), int64(len(content)), true, nil, nil) + obj, err := sharedFs.Put(ctx, bytes.NewReader([]byte(content)), src) + require.NoError(t, err) + + err = obj.Remove(ctx) + require.NoError(t, err) + + // Verify it's gone + _, err = sharedFs.NewObject(ctx, remote) + assert.ErrorIs(t, err, fs.ErrorObjectNotFound) + }) + + t.Run("Copy", func(t *testing.T) { + // Upload a file, then server-side copy it within the same shared folder + content := "file to copy" + srcRemote := folderName + "/test-copy-src.txt" + dstRemote := folderName + "/test-copy-dst.txt" + + src := object.NewStaticObjectInfo(srcRemote, time.Now(), int64(len(content)), true, nil, nil) + srcObj, err := sharedFs.Put(ctx, bytes.NewReader([]byte(content)), src) + require.NoError(t, err) + defer func() { _ = srcObj.Remove(ctx) }() + + dstObj, err := sharedFs.Copy(ctx, srcObj, dstRemote) + require.NoError(t, err) + defer func() { _ = dstObj.Remove(ctx) }() + + assert.Equal(t, dstRemote, dstObj.Remote()) + assert.Equal(t, int64(len(content)), dstObj.Size()) + + // Verify content + rc, err := dstObj.Open(ctx) + require.NoError(t, err) + buf, err := io.ReadAll(rc) + require.NoError(t, rc.Close()) + require.NoError(t, err) + assert.Equal(t, content, string(buf)) + }) + + t.Run("Move", func(t *testing.T) { + // Upload a file, then server-side move it within the same shared folder + content := "file to move" + srcRemote := folderName + "/test-move-src.txt" + dstRemote := folderName + "/test-move-dst.txt" + + src := object.NewStaticObjectInfo(srcRemote, time.Now(), int64(len(content)), true, nil, nil) + srcObj, err := sharedFs.Put(ctx, bytes.NewReader([]byte(content)), src) + require.NoError(t, err) + + dstObj, err := sharedFs.Move(ctx, srcObj, dstRemote) + require.NoError(t, err) + defer func() { _ = dstObj.Remove(ctx) }() + + assert.Equal(t, dstRemote, dstObj.Remote()) + + // Source should be gone + _, err = sharedFs.NewObject(ctx, srcRemote) + assert.ErrorIs(t, err, fs.ErrorObjectNotFound) + }) + + t.Run("Purge", func(t *testing.T) { + // Create a subdir with a file, then purge + subDir := folderName + "/test-purge-subdir" + require.NoError(t, sharedFs.Mkdir(ctx, subDir)) + + content := "file in purge dir" + remote := subDir + "/file.txt" + src := object.NewStaticObjectInfo(remote, time.Now(), int64(len(content)), true, nil, nil) + _, err := sharedFs.Put(ctx, bytes.NewReader([]byte(content)), src) + require.NoError(t, err) + + err = sharedFs.Purge(ctx, subDir) + require.NoError(t, err) + }) } func (f *Fs) InternalTest(t *testing.T) { t.Run("PaperExport", f.InternalTestPaperExport) - t.Run("SharedFolderList", f.InternalTestSharedFolderList) + t.Run("SharedFolder", f.InternalTestSharedFolder) } var _ fstests.InternalTester = (*Fs)(nil)