From 4ba95654d0327698d68e8b036c8df056e7cb042d Mon Sep 17 00:00:00 2001 From: Evan Simkowitz Date: Wed, 19 Feb 2025 14:13:15 -0800 Subject: [PATCH 01/16] fix move & copy for prefix fs --- frontend/app/store/wshclientapi.ts | 2 +- go.mod | 2 +- pkg/remote/fileshare/fileshare.go | 18 +-- pkg/remote/fileshare/fstype/fstype.go | 8 +- pkg/remote/fileshare/fsutil/fsutil.go | 166 +++++++++++--------------- pkg/remote/fileshare/s3fs/s3fs.go | 83 +++++++------ pkg/remote/fileshare/wavefs/wavefs.go | 11 +- pkg/remote/fileshare/wshfs/wshfs.go | 4 +- pkg/wshrpc/wshclient/wshclient.go | 6 +- pkg/wshrpc/wshremote/wshremote.go | 28 +++-- pkg/wshrpc/wshrpctypes.go | 2 +- 11 files changed, 157 insertions(+), 173 deletions(-) diff --git a/frontend/app/store/wshclientapi.ts b/frontend/app/store/wshclientapi.ts index 9ca02a5dda..fba651911f 100644 --- a/frontend/app/store/wshclientapi.ts +++ b/frontend/app/store/wshclientapi.ts @@ -293,7 +293,7 @@ class RpcApiType { } // command "remotefilecopy" [call] - RemoteFileCopyCommand(client: WshClient, data: CommandFileCopyData, opts?: RpcOpts): Promise { + RemoteFileCopyCommand(client: WshClient, data: CommandFileCopyData, opts?: RpcOpts): Promise { return client.wshRpcCall("remotefilecopy", data, opts); } diff --git a/go.mod b/go.mod index b79c7fbc27..17d50db73d 100644 --- a/go.mod +++ b/go.mod @@ -34,6 +34,7 @@ require ( github.com/wavetermdev/htmltoken v0.2.0 golang.org/x/crypto v0.33.0 golang.org/x/mod v0.23.0 + golang.org/x/sync v0.11.0 golang.org/x/sys v0.30.0 golang.org/x/term v0.29.0 google.golang.org/api v0.221.0 @@ -95,7 +96,6 @@ require ( go.uber.org/atomic v1.7.0 // indirect golang.org/x/net v0.35.0 // indirect golang.org/x/oauth2 v0.26.0 // indirect - golang.org/x/sync v0.11.0 // indirect golang.org/x/text v0.22.0 // indirect golang.org/x/time v0.10.0 // indirect google.golang.org/genproto/googleapis/api v0.0.0-20241209162323-e6fa225c2576 // indirect diff --git a/pkg/remote/fileshare/fileshare.go b/pkg/remote/fileshare/fileshare.go index 9d1eed196a..c4bb084ebd 100644 --- a/pkg/remote/fileshare/fileshare.go +++ b/pkg/remote/fileshare/fileshare.go @@ -129,19 +129,11 @@ func Move(ctx context.Context, data wshrpc.CommandFileCopyData) error { return fmt.Errorf("error creating fileshare client, could not parse destination connection %s", data.DestUri) } if srcConn.Host != destConn.Host { - finfo, err := srcClient.Stat(ctx, srcConn) - if err != nil { - return fmt.Errorf("cannot stat %q: %w", data.SrcUri, err) - } - recursive := data.Opts != nil && data.Opts.Recursive - if finfo.IsDir && data.Opts != nil && !recursive { - return fmt.Errorf("cannot move directory %q to %q without recursive flag", data.SrcUri, data.DestUri) - } - err = destClient.CopyRemote(ctx, srcConn, destConn, srcClient, data.Opts) + srcIsDir, err := destClient.CopyRemote(ctx, srcConn, destConn, srcClient, data.Opts) if err != nil { return fmt.Errorf("cannot copy %q to %q: %w", data.SrcUri, data.DestUri, err) } - return srcClient.Delete(ctx, srcConn, recursive) + return srcClient.Delete(ctx, srcConn, srcIsDir) } else { return srcClient.MoveInternal(ctx, srcConn, destConn, data.Opts) } @@ -158,9 +150,11 @@ func Copy(ctx context.Context, data wshrpc.CommandFileCopyData) error { return fmt.Errorf("error creating fileshare client, could not parse destination connection %s", data.DestUri) } if srcConn.Host != destConn.Host { - return destClient.CopyRemote(ctx, srcConn, destConn, srcClient, data.Opts) + _, err := destClient.CopyRemote(ctx, srcConn, destConn, srcClient, data.Opts) + return err } else { - return srcClient.CopyInternal(ctx, srcConn, destConn, data.Opts) + _, err := srcClient.CopyInternal(ctx, srcConn, destConn, data.Opts) + return err } } diff --git a/pkg/remote/fileshare/fstype/fstype.go b/pkg/remote/fileshare/fstype/fstype.go index cc67ddeab9..4fa02ca92d 100644 --- a/pkg/remote/fileshare/fstype/fstype.go +++ b/pkg/remote/fileshare/fstype/fstype.go @@ -40,10 +40,10 @@ type FileShareClient interface { Mkdir(ctx context.Context, conn *connparse.Connection) error // Move moves the file within the same connection MoveInternal(ctx context.Context, srcConn, destConn *connparse.Connection, opts *wshrpc.FileCopyOpts) error - // Copy copies the file within the same connection - CopyInternal(ctx context.Context, srcConn, destConn *connparse.Connection, opts *wshrpc.FileCopyOpts) error - // CopyRemote copies the file between different connections - CopyRemote(ctx context.Context, srcConn, destConn *connparse.Connection, srcClient FileShareClient, opts *wshrpc.FileCopyOpts) error + // Copy copies the file within the same connection. Returns whether the copy source was a directory + CopyInternal(ctx context.Context, srcConn, destConn *connparse.Connection, opts *wshrpc.FileCopyOpts) (bool, error) + // CopyRemote copies the file between different connections. Returns whether the copy source was a directory + CopyRemote(ctx context.Context, srcConn, destConn *connparse.Connection, srcClient FileShareClient, opts *wshrpc.FileCopyOpts) (bool, error) // Delete deletes the entry at the given path Delete(ctx context.Context, conn *connparse.Connection, recursive bool) error // Join joins the given parts to the connection path diff --git a/pkg/remote/fileshare/fsutil/fsutil.go b/pkg/remote/fileshare/fsutil/fsutil.go index a6b6660557..22412584f4 100644 --- a/pkg/remote/fileshare/fsutil/fsutil.go +++ b/pkg/remote/fileshare/fsutil/fsutil.go @@ -58,53 +58,12 @@ func GetPathPrefix(conn *connparse.Connection) string { return pathPrefix } -func PrefixCopyInternal(ctx context.Context, srcConn, destConn *connparse.Connection, c fstype.FileShareClient, opts *wshrpc.FileCopyOpts, listEntriesPrefix func(ctx context.Context, host string, path string) ([]string, error), copyFunc func(ctx context.Context, host string, path string) error) error { +func PrefixCopyInternal(ctx context.Context, srcConn, destConn *connparse.Connection, c fstype.FileShareClient, opts *wshrpc.FileCopyOpts, listEntriesPrefix func(ctx context.Context, host string, path string) ([]string, error), copyFunc func(ctx context.Context, host string, path string) error) (bool, error) { log.Printf("PrefixCopyInternal: %v -> %v", srcConn.GetFullURI(), destConn.GetFullURI()) - merge := opts != nil && opts.Merge - overwrite := opts != nil && opts.Overwrite - if overwrite && merge { - return fmt.Errorf("cannot specify both overwrite and merge") - } srcHasSlash := strings.HasSuffix(srcConn.Path, fspath.Separator) - srcPath, err := CleanPathPrefix(srcConn.Path) - if err != nil { - return fmt.Errorf("error cleaning source path: %w", err) - } - destHasSlash := strings.HasSuffix(destConn.Path, fspath.Separator) - destPath, err := CleanPathPrefix(destConn.Path) - if err != nil { - return fmt.Errorf("error cleaning destination path: %w", err) - } - if !srcHasSlash { - if !destHasSlash { - destPath += fspath.Separator - } - destPath += fspath.Base(srcPath) - } - destConn.Path = destPath - destInfo, err := c.Stat(ctx, destConn) - destExists := err == nil && !destInfo.NotFound - if err != nil && !errors.Is(err, fs.ErrNotExist) { - return fmt.Errorf("error getting destination file info: %w", err) - } - - srcInfo, err := c.Stat(ctx, srcConn) + srcPath, destPath, srcInfo, err := DetermineCopyDestPath(ctx, srcConn, destConn, c, c, opts) if err != nil { - return fmt.Errorf("error getting source file info: %w", err) - } - if destExists { - if overwrite { - err = c.Delete(ctx, destConn, true) - if err != nil { - return fmt.Errorf("error deleting conflicting destination file: %w", err) - } - } else if destInfo.IsDir && srcInfo.IsDir { - if !merge { - return fmt.Errorf("destination and source are both directories, neither merge nor overwrite specified: %v", destConn.GetFullURI()) - } - } else { - return fmt.Errorf("destination already exists, overwrite not specified: %v", destConn.GetFullURI()) - } + return false, err } if srcInfo.IsDir { if !srcHasSlash { @@ -114,7 +73,7 @@ func PrefixCopyInternal(ctx context.Context, srcConn, destConn *connparse.Connec log.Printf("Copying directory: %v -> %v", srcPath, destPath) entries, err := listEntriesPrefix(ctx, srcConn.Host, srcPath) if err != nil { - return fmt.Errorf("error listing source directory: %w", err) + return false, fmt.Errorf("error listing source directory: %w", err) } tree := pathtree.NewTree(srcPath, fspath.Separator) @@ -129,7 +88,7 @@ func PrefixCopyInternal(ctx context.Context, srcConn, destConn *connparse.Connec if !srcHasSlash { prefixToRemove = fspath.Dir(srcPath) + fspath.Separator } - return tree.Walk(func(path string, numChildren int) error { + return true, tree.Walk(func(path string, numChildren int) error { // since this is a prefix filesystem, we only care about leafs if numChildren > 0 { return nil @@ -138,56 +97,15 @@ func PrefixCopyInternal(ctx context.Context, srcConn, destConn *connparse.Connec return copyFunc(ctx, path, destFilePath) }) } else { - return copyFunc(ctx, srcPath, destPath) + return false, copyFunc(ctx, srcPath, destPath) } } -func PrefixCopyRemote(ctx context.Context, srcConn, destConn *connparse.Connection, srcClient, destClient fstype.FileShareClient, destPutFile func(host string, path string, size int64, reader io.Reader) error, opts *wshrpc.FileCopyOpts) error { - merge := opts != nil && opts.Merge - overwrite := opts != nil && opts.Overwrite - if overwrite && merge { - return fmt.Errorf("cannot specify both overwrite and merge") - } - srcHasSlash := strings.HasSuffix(srcConn.Path, fspath.Separator) - destHasSlash := strings.HasSuffix(destConn.Path, fspath.Separator) - destPath, err := CleanPathPrefix(destConn.Path) - if err != nil { - return fmt.Errorf("error cleaning destination path: %w", err) - } - if !srcHasSlash { - if !destHasSlash { - destPath += fspath.Separator - } - destPath += fspath.Base(srcConn.Path) - } - destConn.Path = destPath - destInfo, err := destClient.Stat(ctx, destConn) - destExists := err == nil && !destInfo.NotFound - if err != nil && !errors.Is(err, fs.ErrNotExist) { - return fmt.Errorf("error getting destination file info: %w", err) - } - - srcInfo, err := srcClient.Stat(ctx, srcConn) - if err != nil { - return fmt.Errorf("error getting source file info: %w", err) - } - if destExists { - if overwrite { - err = destClient.Delete(ctx, destConn, true) - if err != nil { - return fmt.Errorf("error deleting conflicting destination file: %w", err) - } - } else if destInfo.IsDir && srcInfo.IsDir { - if !merge { - return fmt.Errorf("destination and source are both directories, neither merge nor overwrite specified: %v", destConn.GetFullURI()) - } - } else { - return fmt.Errorf("destination already exists, overwrite not specified: %v", destConn.GetFullURI()) - } - } +func PrefixCopyRemote(ctx context.Context, srcConn, destConn *connparse.Connection, srcClient, destClient fstype.FileShareClient, destPutFile func(host string, path string, size int64, reader io.Reader) error, opts *wshrpc.FileCopyOpts) (bool, error) { + _, destPath, srcInfo, err := DetermineCopyDestPath(ctx, srcConn, destConn, srcClient, destClient, opts) if err != nil { if !errors.Is(err, fs.ErrNotExist) { - return err + return false, err } } log.Printf("Copying: %v -> %v", srcConn.GetFullURI(), destConn.GetFullURI()) @@ -202,8 +120,8 @@ func PrefixCopyRemote(ctx context.Context, srcConn, destConn *connparse.Connecti return fmt.Errorf("protocol error: source is a directory, but only a single file is being copied") } fileName, err := CleanPathPrefix(fspath.Join(destPath, next.Name)) - if singleFile && !destHasSlash { - fileName, err = CleanPathPrefix(destConn.Path) + if singleFile { + fileName = destPath } if err != nil { return fmt.Errorf("error cleaning path: %w", err) @@ -213,9 +131,67 @@ func PrefixCopyRemote(ctx context.Context, srcConn, destConn *connparse.Connecti }) if err != nil { cancel(err) - return err + return false, err + } + return srcInfo.IsDir, nil +} + +func DetermineCopyDestPath(ctx context.Context, srcConn, destConn *connparse.Connection, srcClient, destClient fstype.FileShareClient, opts *wshrpc.FileCopyOpts) (srcPath, destPath string, srcInfo *wshrpc.FileInfo, err error) { + merge := opts != nil && opts.Merge + overwrite := opts != nil && opts.Overwrite + if overwrite && merge { + return "", "", nil, fmt.Errorf("cannot specify both overwrite and merge") + } + + srcHasSlash := strings.HasSuffix(srcConn.Path, fspath.Separator) + srcPath, err = CleanPathPrefix(srcConn.Path) + if err != nil { + return "", "", nil, fmt.Errorf("error cleaning source path: %w", err) + } + destHasSlash := strings.HasSuffix(destConn.Path, fspath.Separator) + destPath, err = CleanPathPrefix(destConn.Path) + if err != nil { + return "", "", nil, fmt.Errorf("error cleaning destination path: %w", err) + } + + srcInfo, err = srcClient.Stat(ctx, srcConn) + if err != nil { + return "", "", nil, fmt.Errorf("error getting source file info: %w", err) + } + destInfo, err := destClient.Stat(ctx, destConn) + destExists := err == nil && !destInfo.NotFound + if err != nil && !errors.Is(err, fs.ErrNotExist) { + return "", "", nil, fmt.Errorf("error getting destination file info: %w", err) + } + originalDestPath := destPath + if !srcHasSlash { + if destInfo.IsDir || (!destExists && !destHasSlash && srcInfo.IsDir) { + destPath = fspath.Join(destPath, fspath.Base(srcConn.Path)) + } + } + destConn.Path = destPath + if originalDestPath != destPath { + destInfo, err = destClient.Stat(ctx, destConn) + destExists = err == nil && !destInfo.NotFound + if err != nil && !errors.Is(err, fs.ErrNotExist) { + return "", "", nil, fmt.Errorf("error getting destination file info: %w", err) + } + } + if destExists { + if overwrite { + err = destClient.Delete(ctx, destConn, true) + if err != nil { + return "", "", nil, fmt.Errorf("error deleting conflicting destination file: %w", err) + } + } else if destInfo.IsDir && srcInfo.IsDir { + if !merge { + return "", "", nil, fmt.Errorf("destination and source are both directories, neither merge nor overwrite specified: %v", destConn.GetFullURI()) + } + } else { + return "", "", nil, fmt.Errorf("destination already exists, overwrite not specified: %v", destConn.GetFullURI()) + } } - return nil + return srcPath, destPath, srcInfo, nil } // CleanPathPrefix corrects paths for prefix filesystems (i.e. ones that don't have directories) diff --git a/pkg/remote/fileshare/s3fs/s3fs.go b/pkg/remote/fileshare/s3fs/s3fs.go index aef9404196..a764095d42 100644 --- a/pkg/remote/fileshare/s3fs/s3fs.go +++ b/pkg/remote/fileshare/s3fs/s3fs.go @@ -644,20 +644,20 @@ func (c S3Client) Mkdir(ctx context.Context, conn *connparse.Connection) error { } func (c S3Client) MoveInternal(ctx context.Context, srcConn, destConn *connparse.Connection, opts *wshrpc.FileCopyOpts) error { - err := c.CopyInternal(ctx, srcConn, destConn, opts) + isDir, err := c.CopyInternal(ctx, srcConn, destConn, opts) if err != nil { return err } - return c.Delete(ctx, srcConn, true) + return c.Delete(ctx, srcConn, isDir) } -func (c S3Client) CopyRemote(ctx context.Context, srcConn, destConn *connparse.Connection, srcClient fstype.FileShareClient, opts *wshrpc.FileCopyOpts) error { +func (c S3Client) CopyRemote(ctx context.Context, srcConn, destConn *connparse.Connection, srcClient fstype.FileShareClient, opts *wshrpc.FileCopyOpts) (bool, error) { if srcConn.Scheme == connparse.ConnectionTypeS3 && destConn.Scheme == connparse.ConnectionTypeS3 { return c.CopyInternal(ctx, srcConn, destConn, opts) } destBucket := destConn.Host if destBucket == "" || destBucket == fspath.Separator { - return fmt.Errorf("destination bucket must be specified") + return false, fmt.Errorf("destination bucket must be specified") } return fsutil.PrefixCopyRemote(ctx, srcConn, destConn, srcClient, c, func(bucket, path string, size int64, reader io.Reader) error { _, err := c.client.PutObject(ctx, &s3.PutObjectInput{ @@ -670,11 +670,11 @@ func (c S3Client) CopyRemote(ctx context.Context, srcConn, destConn *connparse.C }, opts) } -func (c S3Client) CopyInternal(ctx context.Context, srcConn, destConn *connparse.Connection, opts *wshrpc.FileCopyOpts) error { +func (c S3Client) CopyInternal(ctx context.Context, srcConn, destConn *connparse.Connection, opts *wshrpc.FileCopyOpts) (bool, error) { srcBucket := srcConn.Host destBucket := destConn.Host if srcBucket == "" || srcBucket == fspath.Separator || destBucket == "" || destBucket == fspath.Separator { - return fmt.Errorf("source and destination bucket must be specified") + return false, fmt.Errorf("source and destination bucket must be specified") } return fsutil.PrefixCopyInternal(ctx, srcConn, destConn, c, opts, func(ctx context.Context, bucket, prefix string) ([]string, error) { var entries []string @@ -697,58 +697,41 @@ func (c S3Client) CopyInternal(ctx context.Context, srcConn, destConn *connparse }) } -func (c S3Client) listFilesPrefix(ctx context.Context, input *s3.ListObjectsV2Input, fileCallback func(*types.Object) (bool, error)) error { - var err error - var output *s3.ListObjectsV2Output - objectPaginator := s3.NewListObjectsV2Paginator(c.client, input) - for objectPaginator.HasMorePages() { - output, err = objectPaginator.NextPage(ctx) - if err != nil { - var noBucket *types.NoSuchBucket - if !awsconn.CheckAccessDeniedErr(&err) && errors.As(err, &noBucket) { - err = noBucket - } - return err - } else { - for _, obj := range output.Contents { - if cont, err := fileCallback(&obj); err != nil { - return err - } else if !cont { - return nil - } - } - } - } - return nil -} - func (c S3Client) Delete(ctx context.Context, conn *connparse.Connection, recursive bool) error { + log.Printf("s3fs.Delete: %v", conn.GetFullURI()) bucket := conn.Host objectKey := conn.Path if bucket == "" || bucket == fspath.Separator { + log.Printf("Delete: bucket must be specified") return errors.Join(errors.ErrUnsupported, fmt.Errorf("bucket must be specified")) } if objectKey == "" || objectKey == fspath.Separator { + log.Printf("Delete: object key must be specified") return errors.Join(errors.ErrUnsupported, fmt.Errorf("object key must be specified")) } if recursive { + log.Printf("Deleting objects with prefix %v:%v", bucket, objectKey) if !strings.HasSuffix(objectKey, fspath.Separator) { objectKey = objectKey + fspath.Separator } - entries, err := c.client.ListObjectsV2(ctx, &s3.ListObjectsV2Input{ + objects := make([]types.ObjectIdentifier, 0) + err := c.listFilesPrefix(ctx, &s3.ListObjectsV2Input{ Bucket: aws.String(bucket), Prefix: aws.String(objectKey), + }, func(obj *types.Object) (bool, error) { + log.Printf("Deleting object %v:%v", bucket, *obj.Key) + objects = append(objects, types.ObjectIdentifier{Key: obj.Key}) + return true, nil }) if err != nil { + log.Printf("Error listing objects: %v", err) return err } - if len(entries.Contents) == 0 { + if len(objects) == 0 { + log.Printf("No objects found with prefix %v:%v", bucket, objectKey) return nil } - objects := make([]types.ObjectIdentifier, 0, len(entries.Contents)) - for _, obj := range entries.Contents { - objects = append(objects, types.ObjectIdentifier{Key: obj.Key}) - } + log.Printf("Deleting %d objects with prefix %v:%v", len(objects), bucket, objectKey) _, err = c.client.DeleteObjects(ctx, &s3.DeleteObjectsInput{ Bucket: aws.String(bucket), Delete: &types.Delete{ @@ -757,6 +740,7 @@ func (c S3Client) Delete(ctx context.Context, conn *connparse.Connection, recurs }) return err } + log.Printf("Deleting object %v:%v", bucket, objectKey) _, err := c.client.DeleteObject(ctx, &s3.DeleteObjectInput{ Bucket: aws.String(bucket), Key: aws.String(objectKey), @@ -764,6 +748,31 @@ func (c S3Client) Delete(ctx context.Context, conn *connparse.Connection, recurs return err } +func (c S3Client) listFilesPrefix(ctx context.Context, input *s3.ListObjectsV2Input, fileCallback func(*types.Object) (bool, error)) error { + var err error + var output *s3.ListObjectsV2Output + objectPaginator := s3.NewListObjectsV2Paginator(c.client, input) + for objectPaginator.HasMorePages() { + output, err = objectPaginator.NextPage(ctx) + if err != nil { + var noBucket *types.NoSuchBucket + if !awsconn.CheckAccessDeniedErr(&err) && errors.As(err, &noBucket) { + err = noBucket + } + return err + } else { + for _, obj := range output.Contents { + if cont, err := fileCallback(&obj); err != nil { + return err + } else if !cont { + return nil + } + } + } + } + return nil +} + func (c S3Client) Join(ctx context.Context, conn *connparse.Connection, parts ...string) (*wshrpc.FileInfo, error) { var joinParts []string if conn.Path == "" || conn.Path == fspath.Separator { diff --git a/pkg/remote/fileshare/wavefs/wavefs.go b/pkg/remote/fileshare/wavefs/wavefs.go index b30c4bad39..fd372bd65b 100644 --- a/pkg/remote/fileshare/wavefs/wavefs.go +++ b/pkg/remote/fileshare/wavefs/wavefs.go @@ -422,16 +422,17 @@ func (c WaveClient) MoveInternal(ctx context.Context, srcConn, destConn *connpar if srcConn.Host != destConn.Host { return fmt.Errorf("move internal, src and dest hosts do not match") } - if err := c.CopyInternal(ctx, srcConn, destConn, opts); err != nil { + isDir, err := c.CopyInternal(ctx, srcConn, destConn, opts) + if err != nil { return fmt.Errorf("error copying blockfile: %w", err) } - if err := c.Delete(ctx, srcConn, opts.Recursive); err != nil { + if err := c.Delete(ctx, srcConn, isDir); err != nil { return fmt.Errorf("error deleting blockfile: %w", err) } return nil } -func (c WaveClient) CopyInternal(ctx context.Context, srcConn, destConn *connparse.Connection, opts *wshrpc.FileCopyOpts) error { +func (c WaveClient) CopyInternal(ctx context.Context, srcConn, destConn *connparse.Connection, opts *wshrpc.FileCopyOpts) (bool, error) { return fsutil.PrefixCopyInternal(ctx, srcConn, destConn, c, opts, func(ctx context.Context, zoneId, prefix string) ([]string, error) { entryList := make([]string, 0) if err := listFilesPrefix(ctx, zoneId, prefix, func(wf *filestore.WaveFile) error { @@ -466,13 +467,13 @@ func (c WaveClient) CopyInternal(ctx context.Context, srcConn, destConn *connpar }) } -func (c WaveClient) CopyRemote(ctx context.Context, srcConn, destConn *connparse.Connection, srcClient fstype.FileShareClient, opts *wshrpc.FileCopyOpts) error { +func (c WaveClient) CopyRemote(ctx context.Context, srcConn, destConn *connparse.Connection, srcClient fstype.FileShareClient, opts *wshrpc.FileCopyOpts) (bool, error) { if srcConn.Scheme == connparse.ConnectionTypeWave && destConn.Scheme == connparse.ConnectionTypeWave { return c.CopyInternal(ctx, srcConn, destConn, opts) } zoneId := destConn.Host if zoneId == "" { - return fmt.Errorf("zoneid not found in connection") + return false, fmt.Errorf("zoneid not found in connection") } return fsutil.PrefixCopyRemote(ctx, srcConn, destConn, srcClient, c, func(zoneId, path string, size int64, reader io.Reader) error { dataBuf := make([]byte, size) diff --git a/pkg/remote/fileshare/wshfs/wshfs.go b/pkg/remote/fileshare/wshfs/wshfs.go index ae0930e864..41dfdd15f5 100644 --- a/pkg/remote/fileshare/wshfs/wshfs.go +++ b/pkg/remote/fileshare/wshfs/wshfs.go @@ -114,11 +114,11 @@ func (c WshClient) MoveInternal(ctx context.Context, srcConn, destConn *connpars return wshclient.RemoteFileMoveCommand(RpcClient, wshrpc.CommandFileCopyData{SrcUri: srcConn.GetFullURI(), DestUri: destConn.GetFullURI(), Opts: opts}, &wshrpc.RpcOpts{Route: wshutil.MakeConnectionRouteId(destConn.Host), Timeout: timeout}) } -func (c WshClient) CopyRemote(ctx context.Context, srcConn, destConn *connparse.Connection, _ fstype.FileShareClient, opts *wshrpc.FileCopyOpts) error { +func (c WshClient) CopyRemote(ctx context.Context, srcConn, destConn *connparse.Connection, _ fstype.FileShareClient, opts *wshrpc.FileCopyOpts) (bool, error) { return c.CopyInternal(ctx, srcConn, destConn, opts) } -func (c WshClient) CopyInternal(ctx context.Context, srcConn, destConn *connparse.Connection, opts *wshrpc.FileCopyOpts) error { +func (c WshClient) CopyInternal(ctx context.Context, srcConn, destConn *connparse.Connection, opts *wshrpc.FileCopyOpts) (bool, error) { if opts == nil { opts = &wshrpc.FileCopyOpts{} } diff --git a/pkg/wshrpc/wshclient/wshclient.go b/pkg/wshrpc/wshclient/wshclient.go index 1ded90d83c..243065b9cc 100644 --- a/pkg/wshrpc/wshclient/wshclient.go +++ b/pkg/wshrpc/wshclient/wshclient.go @@ -356,9 +356,9 @@ func RecordTEventCommand(w *wshutil.WshRpc, data telemetrydata.TEvent, opts *wsh } // command "remotefilecopy", wshserver.RemoteFileCopyCommand -func RemoteFileCopyCommand(w *wshutil.WshRpc, data wshrpc.CommandFileCopyData, opts *wshrpc.RpcOpts) error { - _, err := sendRpcRequestCallHelper[any](w, "remotefilecopy", data, opts) - return err +func RemoteFileCopyCommand(w *wshutil.WshRpc, data wshrpc.CommandFileCopyData, opts *wshrpc.RpcOpts) (bool, error) { + resp, err := sendRpcRequestCallHelper[bool](w, "remotefilecopy", data, opts) + return resp, err } // command "remotefiledelete", wshserver.RemoteFileDeleteCommand diff --git a/pkg/wshrpc/wshremote/wshremote.go b/pkg/wshrpc/wshremote/wshremote.go index da05953fce..4b83fa7393 100644 --- a/pkg/wshrpc/wshremote/wshremote.go +++ b/pkg/wshrpc/wshremote/wshremote.go @@ -306,7 +306,7 @@ func (impl *ServerImpl) RemoteTarStreamCommand(ctx context.Context, data wshrpc. return rtn } -func (impl *ServerImpl) RemoteFileCopyCommand(ctx context.Context, data wshrpc.CommandFileCopyData) error { +func (impl *ServerImpl) RemoteFileCopyCommand(ctx context.Context, data wshrpc.CommandFileCopyData) (bool, error) { log.Printf("RemoteFileCopyCommand: src=%s, dest=%s\n", data.SrcUri, data.DestUri) opts := data.Opts if opts == nil { @@ -319,13 +319,13 @@ func (impl *ServerImpl) RemoteFileCopyCommand(ctx context.Context, data wshrpc.C destConn, err := connparse.ParseURIAndReplaceCurrentHost(ctx, destUri) if err != nil { - return fmt.Errorf("cannot parse destination URI %q: %w", srcUri, err) + return false, fmt.Errorf("cannot parse destination URI %q: %w", srcUri, err) } destPathCleaned := filepath.Clean(wavebase.ExpandHomeDirSafe(destConn.Path)) destinfo, err := os.Stat(destPathCleaned) if err != nil { if !errors.Is(err, fs.ErrNotExist) { - return fmt.Errorf("cannot stat destination %q: %w", destPathCleaned, err) + return false, fmt.Errorf("cannot stat destination %q: %w", destPathCleaned, err) } } @@ -335,17 +335,17 @@ func (impl *ServerImpl) RemoteFileCopyCommand(ctx context.Context, data wshrpc.C if destExists && !destIsDir { if !overwrite { - return fmt.Errorf("file already exists at destination %q, use overwrite option", destPathCleaned) + return false, fmt.Errorf("file already exists at destination %q, use overwrite option", destPathCleaned) } else { err := os.Remove(destPathCleaned) if err != nil { - return fmt.Errorf("cannot remove file %q: %w", destPathCleaned, err) + return false, fmt.Errorf("cannot remove file %q: %w", destPathCleaned, err) } } } srcConn, err := connparse.ParseURIAndReplaceCurrentHost(ctx, srcUri) if err != nil { - return fmt.Errorf("cannot parse source URI %q: %w", srcUri, err) + return false, fmt.Errorf("cannot parse source URI %q: %w", srcUri, err) } copyFileFunc := func(path string, finfo fs.FileInfo, srcFile io.Reader) (int64, error) { @@ -417,15 +417,18 @@ func (impl *ServerImpl) RemoteFileCopyCommand(ctx context.Context, data wshrpc.C return finfo.Size(), nil } + srcIsDir := false + if srcConn.Host == destConn.Host { srcPathCleaned := filepath.Clean(wavebase.ExpandHomeDirSafe(srcConn.Path)) srcFileStat, err := os.Stat(srcPathCleaned) if err != nil { - return fmt.Errorf("cannot stat file %q: %w", srcPathCleaned, err) + return false, fmt.Errorf("cannot stat file %q: %w", srcPathCleaned, err) } if srcFileStat.IsDir() { + srcIsDir = true var srcPathPrefix string if destIsDir { srcPathPrefix = filepath.Dir(srcPathCleaned) @@ -450,12 +453,12 @@ func (impl *ServerImpl) RemoteFileCopyCommand(ctx context.Context, data wshrpc.C return err }) if err != nil { - return fmt.Errorf("cannot copy %q to %q: %w", srcUri, destUri, err) + return false, fmt.Errorf("cannot copy %q to %q: %w", srcUri, destUri, err) } } else { file, err := os.Open(srcPathCleaned) if err != nil { - return fmt.Errorf("cannot open file %q: %w", srcPathCleaned, err) + return false, fmt.Errorf("cannot open file %q: %w", srcPathCleaned, err) } defer utilfn.GracefulClose(file, "RemoteFileCopyCommand", srcPathCleaned) var destFilePath string @@ -466,7 +469,7 @@ func (impl *ServerImpl) RemoteFileCopyCommand(ctx context.Context, data wshrpc.C } _, err = copyFileFunc(destFilePath, srcFileStat, file) if err != nil { - return fmt.Errorf("cannot copy %q to %q: %w", srcUri, destUri, err) + return false, fmt.Errorf("cannot copy %q to %q: %w", srcUri, destUri, err) } } } else { @@ -487,6 +490,7 @@ func (impl *ServerImpl) RemoteFileCopyCommand(ctx context.Context, data wshrpc.C numFiles++ nextpath := filepath.Join(destPathCleaned, next.Name) log.Printf("RemoteFileCopyCommand: copying %q to %q\n", next.Name, nextpath) + srcIsDir = !singleFile if singleFile && !destHasSlash { // custom flag to indicate that the source is a single file, not a directory the contents of a directory nextpath = destPathCleaned @@ -500,7 +504,7 @@ func (impl *ServerImpl) RemoteFileCopyCommand(ctx context.Context, data wshrpc.C return nil }) if err != nil { - return fmt.Errorf("cannot copy %q to %q: %w", srcUri, destUri, err) + return false, fmt.Errorf("cannot copy %q to %q: %w", srcUri, destUri, err) } totalTime := time.Since(copyStart).Seconds() totalMegaBytes := float64(totalBytes) / 1024 / 1024 @@ -510,7 +514,7 @@ func (impl *ServerImpl) RemoteFileCopyCommand(ctx context.Context, data wshrpc.C } log.Printf("RemoteFileCopyCommand: done; %d files copied in %.3fs, total of %.4f MB, %.2f MB/s, %d files skipped\n", numFiles, totalTime, totalMegaBytes, rate, numSkipped) } - return nil + return srcIsDir, nil } func (impl *ServerImpl) RemoteListEntriesCommand(ctx context.Context, data wshrpc.CommandRemoteListEntriesData) chan wshrpc.RespOrErrorUnion[wshrpc.CommandRemoteListEntriesRtnData] { diff --git a/pkg/wshrpc/wshrpctypes.go b/pkg/wshrpc/wshrpctypes.go index cc4ef1e14d..00be058bf7 100644 --- a/pkg/wshrpc/wshrpctypes.go +++ b/pkg/wshrpc/wshrpctypes.go @@ -228,7 +228,7 @@ type WshRpcInterface interface { // remotes RemoteStreamFileCommand(ctx context.Context, data CommandRemoteStreamFileData) chan RespOrErrorUnion[FileData] RemoteTarStreamCommand(ctx context.Context, data CommandRemoteStreamTarData) <-chan RespOrErrorUnion[iochantypes.Packet] - RemoteFileCopyCommand(ctx context.Context, data CommandFileCopyData) error + RemoteFileCopyCommand(ctx context.Context, data CommandFileCopyData) (bool, error) RemoteListEntriesCommand(ctx context.Context, data CommandRemoteListEntriesData) chan RespOrErrorUnion[CommandRemoteListEntriesRtnData] RemoteFileInfoCommand(ctx context.Context, path string) (*FileInfo, error) RemoteFileTouchCommand(ctx context.Context, path string) error From 4b7637fb17fb0f937d67f12a63f3f370a3343117 Mon Sep 17 00:00:00 2001 From: Evan Simkowitz Date: Wed, 19 Feb 2025 15:11:27 -0800 Subject: [PATCH 02/16] fix remote copy --- .../app/view/preview/directorypreview.tsx | 14 ++++--- pkg/remote/fileshare/fsutil/fsutil.go | 42 +++++++------------ pkg/remote/fileshare/pathtree/pathtree.go | 6 +++ pkg/remote/fileshare/s3fs/s3fs.go | 5 +++ 4 files changed, 35 insertions(+), 32 deletions(-) diff --git a/frontend/app/view/preview/directorypreview.tsx b/frontend/app/view/preview/directorypreview.tsx index 2a29c1e5e7..02df998e61 100644 --- a/frontend/app/view/preview/directorypreview.tsx +++ b/frontend/app/view/preview/directorypreview.tsx @@ -48,7 +48,7 @@ type FileCopyStatus = { declare module "@tanstack/react-table" { interface TableMeta { - updateName: (path: string) => void; + updateName: (path: string, isDir: boolean) => void; newFile: () => void; newDirectory: () => void; } @@ -291,7 +291,7 @@ function DirectoryTable({ const setEntryManagerProps = useSetAtom(entryManagerOverlayPropsAtom); - const updateName = useCallback((path: string) => { + const updateName = useCallback((path: string, isDir: boolean) => { const fileName = path.split("/").at(-1); setEntryManagerProps({ entryManagerType: EntryManagerType.EditName, @@ -304,8 +304,12 @@ function DirectoryTable({ console.log(`replacing ${fileName} with ${newName}: ${path}`); fireAndForget(async () => { try { + let srcuri = await model.formatRemoteUri(path, globalStore.get); + if (isDir) { + srcuri += "/"; + } await RpcApi.FileMoveCommand(TabRpcClient, { - srcuri: await model.formatRemoteUri(path, globalStore.get), + srcuri, desturi: await model.formatRemoteUri(newPath, globalStore.get), opts: { recursive: true, @@ -547,7 +551,7 @@ function TableBody({ { label: "Rename", click: () => { - table.options.meta.updateName(finfo.path); + table.options.meta.updateName(finfo.path, finfo.isdir); }, }, { @@ -854,7 +858,7 @@ function DirectoryPreview({ model }: DirectoryPreviewProps) { }); const handleDropCopy = useCallback( - async (data: CommandFileCopyData, isDir) => { + async (data: CommandFileCopyData, isDir: boolean) => { try { await RpcApi.FileCopyCommand(TabRpcClient, data, { timeout: data.opts.timeout }); setCopyStatus(null); diff --git a/pkg/remote/fileshare/fsutil/fsutil.go b/pkg/remote/fileshare/fsutil/fsutil.go index 22412584f4..025ddca80a 100644 --- a/pkg/remote/fileshare/fsutil/fsutil.go +++ b/pkg/remote/fileshare/fsutil/fsutil.go @@ -28,7 +28,7 @@ func GetParentPath(conn *connparse.Connection) string { func GetParentPathString(hostAndPath string) string { if hostAndPath == "" || hostAndPath == fspath.Separator { - return fspath.Separator + return "" } // Remove trailing slash if present @@ -38,24 +38,9 @@ func GetParentPathString(hostAndPath string) string { lastSlash := strings.LastIndex(hostAndPath, fspath.Separator) if lastSlash <= 0 { - return fspath.Separator - } - return hostAndPath[:lastSlash+1] -} - -const minURILength = 10 // Minimum length for a valid URI (e.g., "s3://bucket") - -func GetPathPrefix(conn *connparse.Connection) string { - fullUri := conn.GetFullURI() - if fullUri == "" { return "" } - pathPrefix := fullUri - lastSlash := strings.LastIndex(fullUri, fspath.Separator) - if lastSlash > minURILength && lastSlash < len(fullUri)-1 { - pathPrefix = fullUri[:lastSlash+1] - } - return pathPrefix + return hostAndPath[:lastSlash+1] } func PrefixCopyInternal(ctx context.Context, srcConn, destConn *connparse.Connection, c fstype.FileShareClient, opts *wshrpc.FileCopyOpts, listEntriesPrefix func(ctx context.Context, host string, path string) ([]string, error), copyFunc func(ctx context.Context, host string, path string) error) (bool, error) { @@ -102,12 +87,18 @@ func PrefixCopyInternal(ctx context.Context, srcConn, destConn *connparse.Connec } func PrefixCopyRemote(ctx context.Context, srcConn, destConn *connparse.Connection, srcClient, destClient fstype.FileShareClient, destPutFile func(host string, path string, size int64, reader io.Reader) error, opts *wshrpc.FileCopyOpts) (bool, error) { + // prefix to be used if the destination is a directory. The destPath returned in the following call only applies if the destination is not a directory. + destPathPrefix, err := CleanPathPrefix(destConn.Path) + if err != nil { + return false, fmt.Errorf("error cleaning destination path: %w", err) + } + destPathPrefix += fspath.Separator + _, destPath, srcInfo, err := DetermineCopyDestPath(ctx, srcConn, destConn, srcClient, destClient, opts) if err != nil { - if !errors.Is(err, fs.ErrNotExist) { - return false, err - } + return false, err } + log.Printf("Copying: %v -> %v", srcConn.GetFullURI(), destConn.GetFullURI()) readCtx, cancel := context.WithCancelCause(ctx) defer cancel(nil) @@ -119,7 +110,7 @@ func PrefixCopyRemote(ctx context.Context, srcConn, destConn *connparse.Connecti if singleFile && srcInfo.IsDir { return fmt.Errorf("protocol error: source is a directory, but only a single file is being copied") } - fileName, err := CleanPathPrefix(fspath.Join(destPath, next.Name)) + fileName, err := CleanPathPrefix(fspath.Join(destPathPrefix, next.Name)) if singleFile { fileName = destPath } @@ -144,10 +135,7 @@ func DetermineCopyDestPath(ctx context.Context, srcConn, destConn *connparse.Con } srcHasSlash := strings.HasSuffix(srcConn.Path, fspath.Separator) - srcPath, err = CleanPathPrefix(srcConn.Path) - if err != nil { - return "", "", nil, fmt.Errorf("error cleaning source path: %w", err) - } + srcPath = srcConn.Path destHasSlash := strings.HasSuffix(destConn.Path, fspath.Separator) destPath, err = CleanPathPrefix(destConn.Path) if err != nil { @@ -179,7 +167,7 @@ func DetermineCopyDestPath(ctx context.Context, srcConn, destConn *connparse.Con } if destExists { if overwrite { - err = destClient.Delete(ctx, destConn, true) + err = destClient.Delete(ctx, destConn, destInfo.IsDir) if err != nil { return "", "", nil, fmt.Errorf("error deleting conflicting destination file: %w", err) } @@ -197,7 +185,7 @@ func DetermineCopyDestPath(ctx context.Context, srcConn, destConn *connparse.Con // CleanPathPrefix corrects paths for prefix filesystems (i.e. ones that don't have directories) func CleanPathPrefix(path string) (string, error) { if path == "" { - return "", fmt.Errorf("path is empty") + return "", nil } if strings.HasPrefix(path, fspath.Separator) { path = path[1:] diff --git a/pkg/remote/fileshare/pathtree/pathtree.go b/pkg/remote/fileshare/pathtree/pathtree.go index 5d4918fbae..f4dbb92f05 100644 --- a/pkg/remote/fileshare/pathtree/pathtree.go +++ b/pkg/remote/fileshare/pathtree/pathtree.go @@ -35,6 +35,7 @@ func NewTree(path string, delimiter string) *Tree { log.Printf("Warning: multi-character delimiter '%s' may cause unexpected behavior", delimiter) } if path != "" && !strings.HasSuffix(path, delimiter) { + log.Printf("Warning: path '%s' does not end with delimiter '%s'", path, delimiter) path += delimiter } return &Tree{ @@ -60,6 +61,7 @@ func (t *Tree) Add(path string) { relativePath = strings.TrimPrefix(path, t.RootPath) // If the path is not a child of the root path, ignore it + log.Printf("relativePath: %s", relativePath) if relativePath == path { return } @@ -68,6 +70,7 @@ func (t *Tree) Add(path string) { // If the path is already in the tree, ignore it if t.nodes[relativePath] != nil { + log.Printf("path already in tree: %s", relativePath) return } @@ -75,15 +78,18 @@ func (t *Tree) Add(path string) { // Validate path components for _, component := range components { if component == "" || component == "." || component == ".." { + log.Printf("invalid path component: %s", component) return // Skip invalid paths } } // Quick check to see if the parent path is already in the tree, in which case we can skip the loop if parent := t.tryAddToExistingParent(components); parent { + log.Printf("added to existing parent: %s", strings.Join(components, t.delimiter)) return } + log.Printf("adding new path: %s", strings.Join(components, t.delimiter)) t.addNewPath(components) } diff --git a/pkg/remote/fileshare/s3fs/s3fs.go b/pkg/remote/fileshare/s3fs/s3fs.go index a764095d42..26c90d7434 100644 --- a/pkg/remote/fileshare/s3fs/s3fs.go +++ b/pkg/remote/fileshare/s3fs/s3fs.go @@ -207,6 +207,7 @@ func (c S3Client) ReadTarStream(ctx context.Context, conn *connparse.Connection, tarPathPrefix = bucket } } else if singleFile || includeDir { + log.Printf("singleFile or includeDir; tarPathPrefix: %v", tarPathPrefix) // if we're including the directory itself, we need to remove the last part of the path tarPathPrefix = fsutil.GetParentPathString(tarPathPrefix) } @@ -295,6 +296,7 @@ func (c S3Client) ReadTarStream(ctx context.Context, conn *connparse.Connection, // Walk the tree and write the tar entries if err := tree.Walk(func(path string, numChildren int) error { + log.Printf("writing %v", path) mapEntry, isFile := objMap[path] // default vals assume entry is dir, since mapEntry might not exist @@ -738,6 +740,9 @@ func (c S3Client) Delete(ctx context.Context, conn *connparse.Connection, recurs Objects: objects, }, }) + if err != nil { + log.Printf("Error deleting objects: %v", err) + } return err } log.Printf("Deleting object %v:%v", bucket, objectKey) From 82d2bb3ae9cce338af71b82a17f03551055aead7 Mon Sep 17 00:00:00 2001 From: Evan Simkowitz Date: Wed, 19 Feb 2025 15:15:16 -0800 Subject: [PATCH 03/16] remove recursive opt from wsh copy and move --- cmd/wsh/cmd/wshcmd-file.go | 8 ++------ frontend/types/gotypes.d.ts | 1 - pkg/wshrpc/wshremote/wshremote.go | 8 -------- pkg/wshrpc/wshrpctypes.go | 3 +-- 4 files changed, 3 insertions(+), 17 deletions(-) diff --git a/cmd/wsh/cmd/wshcmd-file.go b/cmd/wsh/cmd/wshcmd-file.go index a0ca112a2a..5b18c16ade 100644 --- a/cmd/wsh/cmd/wshcmd-file.go +++ b/cmd/wsh/cmd/wshcmd-file.go @@ -418,10 +418,6 @@ func fileCpRun(cmd *cobra.Command, args []string) error { func fileMvRun(cmd *cobra.Command, args []string) error { src, dst := args[0], args[1] - recursive, err := cmd.Flags().GetBool("recursive") - if err != nil { - return err - } force, err := cmd.Flags().GetBool("force") if err != nil { return err @@ -435,9 +431,9 @@ func fileMvRun(cmd *cobra.Command, args []string) error { if err != nil { return fmt.Errorf("unable to parse dest path: %w", err) } - log.Printf("Moving %s to %s; recursive: %v, force: %v", srcPath, destPath, recursive, force) + log.Printf("Moving %s to %s; force: %v", srcPath, destPath, force) rpcOpts := &wshrpc.RpcOpts{Timeout: TimeoutYear} - err = wshclient.FileMoveCommand(RpcClient, wshrpc.CommandFileCopyData{SrcUri: srcPath, DestUri: destPath, Opts: &wshrpc.FileCopyOpts{Overwrite: force, Timeout: TimeoutYear, Recursive: recursive}}, rpcOpts) + err = wshclient.FileMoveCommand(RpcClient, wshrpc.CommandFileCopyData{SrcUri: srcPath, DestUri: destPath, Opts: &wshrpc.FileCopyOpts{Overwrite: force, Timeout: TimeoutYear}}, rpcOpts) if err != nil { return fmt.Errorf("moving file: %w", err) } diff --git a/frontend/types/gotypes.d.ts b/frontend/types/gotypes.d.ts index 2570f4b4de..15fbd91139 100644 --- a/frontend/types/gotypes.d.ts +++ b/frontend/types/gotypes.d.ts @@ -387,7 +387,6 @@ declare global { // wshrpc.FileCopyOpts type FileCopyOpts = { overwrite?: boolean; - recursive?: boolean; merge?: boolean; timeout?: number; }; diff --git a/pkg/wshrpc/wshremote/wshremote.go b/pkg/wshrpc/wshremote/wshremote.go index 4b83fa7393..f67845d1dc 100644 --- a/pkg/wshrpc/wshremote/wshremote.go +++ b/pkg/wshrpc/wshremote/wshremote.go @@ -707,7 +707,6 @@ func (impl *ServerImpl) RemoteFileMoveCommand(ctx context.Context, data wshrpc.C destUri := data.DestUri srcUri := data.SrcUri overwrite := opts != nil && opts.Overwrite - recursive := opts != nil && opts.Recursive destConn, err := connparse.ParseURIAndReplaceCurrentHost(ctx, destUri) if err != nil { @@ -735,13 +734,6 @@ func (impl *ServerImpl) RemoteFileMoveCommand(ctx context.Context, data wshrpc.C } if srcConn.Host == destConn.Host { srcPathCleaned := filepath.Clean(wavebase.ExpandHomeDirSafe(srcConn.Path)) - finfo, err := os.Stat(srcPathCleaned) - if err != nil { - return fmt.Errorf("cannot stat file %q: %w", srcPathCleaned, err) - } - if finfo.IsDir() && !recursive { - return fmt.Errorf("cannot move directory %q, recursive option not specified", srcUri) - } err = os.Rename(srcPathCleaned, destPathCleaned) if err != nil { return fmt.Errorf("cannot move file %q to %q: %w", srcPathCleaned, destPathCleaned, err) diff --git a/pkg/wshrpc/wshrpctypes.go b/pkg/wshrpc/wshrpctypes.go index 00be058bf7..b11ee5333e 100644 --- a/pkg/wshrpc/wshrpctypes.go +++ b/pkg/wshrpc/wshrpctypes.go @@ -546,8 +546,7 @@ type CommandRemoteStreamTarData struct { type FileCopyOpts struct { Overwrite bool `json:"overwrite,omitempty"` - Recursive bool `json:"recursive,omitempty"` // only used for move, always true for copy - Merge bool `json:"merge,omitempty"` // only used for copy, always false for move + Merge bool `json:"merge,omitempty"` // only used for copy, always false for move Timeout int64 `json:"timeout,omitempty"` } From bd550e4603f608d83792fa722b691daff0227001 Mon Sep 17 00:00:00 2001 From: Evan Simkowitz Date: Wed, 19 Feb 2025 15:26:30 -0800 Subject: [PATCH 04/16] Revert "remove recursive opt from wsh copy and move" This reverts commit 82d2bb3ae9cce338af71b82a17f03551055aead7. --- cmd/wsh/cmd/wshcmd-file.go | 8 ++++++-- frontend/types/gotypes.d.ts | 1 + pkg/wshrpc/wshremote/wshremote.go | 8 ++++++++ pkg/wshrpc/wshrpctypes.go | 3 ++- 4 files changed, 17 insertions(+), 3 deletions(-) diff --git a/cmd/wsh/cmd/wshcmd-file.go b/cmd/wsh/cmd/wshcmd-file.go index 5b18c16ade..a0ca112a2a 100644 --- a/cmd/wsh/cmd/wshcmd-file.go +++ b/cmd/wsh/cmd/wshcmd-file.go @@ -418,6 +418,10 @@ func fileCpRun(cmd *cobra.Command, args []string) error { func fileMvRun(cmd *cobra.Command, args []string) error { src, dst := args[0], args[1] + recursive, err := cmd.Flags().GetBool("recursive") + if err != nil { + return err + } force, err := cmd.Flags().GetBool("force") if err != nil { return err @@ -431,9 +435,9 @@ func fileMvRun(cmd *cobra.Command, args []string) error { if err != nil { return fmt.Errorf("unable to parse dest path: %w", err) } - log.Printf("Moving %s to %s; force: %v", srcPath, destPath, force) + log.Printf("Moving %s to %s; recursive: %v, force: %v", srcPath, destPath, recursive, force) rpcOpts := &wshrpc.RpcOpts{Timeout: TimeoutYear} - err = wshclient.FileMoveCommand(RpcClient, wshrpc.CommandFileCopyData{SrcUri: srcPath, DestUri: destPath, Opts: &wshrpc.FileCopyOpts{Overwrite: force, Timeout: TimeoutYear}}, rpcOpts) + err = wshclient.FileMoveCommand(RpcClient, wshrpc.CommandFileCopyData{SrcUri: srcPath, DestUri: destPath, Opts: &wshrpc.FileCopyOpts{Overwrite: force, Timeout: TimeoutYear, Recursive: recursive}}, rpcOpts) if err != nil { return fmt.Errorf("moving file: %w", err) } diff --git a/frontend/types/gotypes.d.ts b/frontend/types/gotypes.d.ts index 15fbd91139..2570f4b4de 100644 --- a/frontend/types/gotypes.d.ts +++ b/frontend/types/gotypes.d.ts @@ -387,6 +387,7 @@ declare global { // wshrpc.FileCopyOpts type FileCopyOpts = { overwrite?: boolean; + recursive?: boolean; merge?: boolean; timeout?: number; }; diff --git a/pkg/wshrpc/wshremote/wshremote.go b/pkg/wshrpc/wshremote/wshremote.go index f67845d1dc..4b83fa7393 100644 --- a/pkg/wshrpc/wshremote/wshremote.go +++ b/pkg/wshrpc/wshremote/wshremote.go @@ -707,6 +707,7 @@ func (impl *ServerImpl) RemoteFileMoveCommand(ctx context.Context, data wshrpc.C destUri := data.DestUri srcUri := data.SrcUri overwrite := opts != nil && opts.Overwrite + recursive := opts != nil && opts.Recursive destConn, err := connparse.ParseURIAndReplaceCurrentHost(ctx, destUri) if err != nil { @@ -734,6 +735,13 @@ func (impl *ServerImpl) RemoteFileMoveCommand(ctx context.Context, data wshrpc.C } if srcConn.Host == destConn.Host { srcPathCleaned := filepath.Clean(wavebase.ExpandHomeDirSafe(srcConn.Path)) + finfo, err := os.Stat(srcPathCleaned) + if err != nil { + return fmt.Errorf("cannot stat file %q: %w", srcPathCleaned, err) + } + if finfo.IsDir() && !recursive { + return fmt.Errorf("cannot move directory %q, recursive option not specified", srcUri) + } err = os.Rename(srcPathCleaned, destPathCleaned) if err != nil { return fmt.Errorf("cannot move file %q to %q: %w", srcPathCleaned, destPathCleaned, err) diff --git a/pkg/wshrpc/wshrpctypes.go b/pkg/wshrpc/wshrpctypes.go index b11ee5333e..00be058bf7 100644 --- a/pkg/wshrpc/wshrpctypes.go +++ b/pkg/wshrpc/wshrpctypes.go @@ -546,7 +546,8 @@ type CommandRemoteStreamTarData struct { type FileCopyOpts struct { Overwrite bool `json:"overwrite,omitempty"` - Merge bool `json:"merge,omitempty"` // only used for copy, always false for move + Recursive bool `json:"recursive,omitempty"` // only used for move, always true for copy + Merge bool `json:"merge,omitempty"` // only used for copy, always false for move Timeout int64 `json:"timeout,omitempty"` } From 02d272eb01eb162af430f4fe8a362b73579075b9 Mon Sep 17 00:00:00 2001 From: Evan Simkowitz Date: Wed, 19 Feb 2025 15:39:23 -0800 Subject: [PATCH 05/16] add back recursive flag, set to true for copy ops --- frontend/app/store/wshclientapi.ts | 2 +- pkg/remote/fileshare/fileshare.go | 16 +++++++++------ pkg/remote/fileshare/fstype/fstype.go | 4 ++-- pkg/remote/fileshare/fsutil/fsutil.go | 28 +++++++++++++++++---------- pkg/remote/fileshare/s3fs/s3fs.go | 18 +++++++++++------ pkg/remote/fileshare/wavefs/wavefs.go | 11 ++++++----- pkg/remote/fileshare/wshfs/wshfs.go | 4 ++-- pkg/wshrpc/wshclient/wshclient.go | 6 +++--- pkg/wshrpc/wshremote/wshremote.go | 3 ++- pkg/wshrpc/wshrpctypes.go | 2 +- 10 files changed, 57 insertions(+), 37 deletions(-) diff --git a/frontend/app/store/wshclientapi.ts b/frontend/app/store/wshclientapi.ts index fba651911f..9ca02a5dda 100644 --- a/frontend/app/store/wshclientapi.ts +++ b/frontend/app/store/wshclientapi.ts @@ -293,7 +293,7 @@ class RpcApiType { } // command "remotefilecopy" [call] - RemoteFileCopyCommand(client: WshClient, data: CommandFileCopyData, opts?: RpcOpts): Promise { + RemoteFileCopyCommand(client: WshClient, data: CommandFileCopyData, opts?: RpcOpts): Promise { return client.wshRpcCall("remotefilecopy", data, opts); } diff --git a/pkg/remote/fileshare/fileshare.go b/pkg/remote/fileshare/fileshare.go index c4bb084ebd..c155f394eb 100644 --- a/pkg/remote/fileshare/fileshare.go +++ b/pkg/remote/fileshare/fileshare.go @@ -129,11 +129,12 @@ func Move(ctx context.Context, data wshrpc.CommandFileCopyData) error { return fmt.Errorf("error creating fileshare client, could not parse destination connection %s", data.DestUri) } if srcConn.Host != destConn.Host { - srcIsDir, err := destClient.CopyRemote(ctx, srcConn, destConn, srcClient, data.Opts) + err := destClient.CopyRemote(ctx, srcConn, destConn, srcClient, data.Opts) if err != nil { return fmt.Errorf("cannot copy %q to %q: %w", data.SrcUri, data.DestUri, err) } - return srcClient.Delete(ctx, srcConn, srcIsDir) + recursive := data.Opts != nil && data.Opts.Recursive + return srcClient.Delete(ctx, srcConn, recursive) } else { return srcClient.MoveInternal(ctx, srcConn, destConn, data.Opts) } @@ -141,6 +142,11 @@ func Move(ctx context.Context, data wshrpc.CommandFileCopyData) error { func Copy(ctx context.Context, data wshrpc.CommandFileCopyData) error { log.Printf("Copy: %v", data) + opts := data.Opts + if opts == nil { + opts = &wshrpc.FileCopyOpts{} + } + opts.Recursive = true srcClient, srcConn := CreateFileShareClient(ctx, data.SrcUri) if srcConn == nil || srcClient == nil { return fmt.Errorf("error creating fileshare client, could not parse source connection %s", data.SrcUri) @@ -150,11 +156,9 @@ func Copy(ctx context.Context, data wshrpc.CommandFileCopyData) error { return fmt.Errorf("error creating fileshare client, could not parse destination connection %s", data.DestUri) } if srcConn.Host != destConn.Host { - _, err := destClient.CopyRemote(ctx, srcConn, destConn, srcClient, data.Opts) - return err + return destClient.CopyRemote(ctx, srcConn, destConn, srcClient, opts) } else { - _, err := srcClient.CopyInternal(ctx, srcConn, destConn, data.Opts) - return err + return srcClient.CopyInternal(ctx, srcConn, destConn, opts) } } diff --git a/pkg/remote/fileshare/fstype/fstype.go b/pkg/remote/fileshare/fstype/fstype.go index 4fa02ca92d..19268d566e 100644 --- a/pkg/remote/fileshare/fstype/fstype.go +++ b/pkg/remote/fileshare/fstype/fstype.go @@ -41,9 +41,9 @@ type FileShareClient interface { // Move moves the file within the same connection MoveInternal(ctx context.Context, srcConn, destConn *connparse.Connection, opts *wshrpc.FileCopyOpts) error // Copy copies the file within the same connection. Returns whether the copy source was a directory - CopyInternal(ctx context.Context, srcConn, destConn *connparse.Connection, opts *wshrpc.FileCopyOpts) (bool, error) + CopyInternal(ctx context.Context, srcConn, destConn *connparse.Connection, opts *wshrpc.FileCopyOpts) error // CopyRemote copies the file between different connections. Returns whether the copy source was a directory - CopyRemote(ctx context.Context, srcConn, destConn *connparse.Connection, srcClient FileShareClient, opts *wshrpc.FileCopyOpts) (bool, error) + CopyRemote(ctx context.Context, srcConn, destConn *connparse.Connection, srcClient FileShareClient, opts *wshrpc.FileCopyOpts) error // Delete deletes the entry at the given path Delete(ctx context.Context, conn *connparse.Connection, recursive bool) error // Join joins the given parts to the connection path diff --git a/pkg/remote/fileshare/fsutil/fsutil.go b/pkg/remote/fileshare/fsutil/fsutil.go index 025ddca80a..6c247784c6 100644 --- a/pkg/remote/fileshare/fsutil/fsutil.go +++ b/pkg/remote/fileshare/fsutil/fsutil.go @@ -21,6 +21,10 @@ import ( "github.com/wavetermdev/waveterm/pkg/wshrpc" ) +const ( + RecursiveCopyError = "recursive flag must be set to true for directory operations" +) + func GetParentPath(conn *connparse.Connection) string { hostAndPath := conn.GetPathWithHost() return GetParentPathString(hostAndPath) @@ -43,14 +47,18 @@ func GetParentPathString(hostAndPath string) string { return hostAndPath[:lastSlash+1] } -func PrefixCopyInternal(ctx context.Context, srcConn, destConn *connparse.Connection, c fstype.FileShareClient, opts *wshrpc.FileCopyOpts, listEntriesPrefix func(ctx context.Context, host string, path string) ([]string, error), copyFunc func(ctx context.Context, host string, path string) error) (bool, error) { +func PrefixCopyInternal(ctx context.Context, srcConn, destConn *connparse.Connection, c fstype.FileShareClient, opts *wshrpc.FileCopyOpts, listEntriesPrefix func(ctx context.Context, host string, path string) ([]string, error), copyFunc func(ctx context.Context, host string, path string) error) error { log.Printf("PrefixCopyInternal: %v -> %v", srcConn.GetFullURI(), destConn.GetFullURI()) srcHasSlash := strings.HasSuffix(srcConn.Path, fspath.Separator) srcPath, destPath, srcInfo, err := DetermineCopyDestPath(ctx, srcConn, destConn, c, c, opts) if err != nil { - return false, err + return err } + recursive := opts != nil && opts.Recursive if srcInfo.IsDir { + if !recursive { + return fmt.Errorf(RecursiveCopyError) + } if !srcHasSlash { srcPath += fspath.Separator } @@ -58,7 +66,7 @@ func PrefixCopyInternal(ctx context.Context, srcConn, destConn *connparse.Connec log.Printf("Copying directory: %v -> %v", srcPath, destPath) entries, err := listEntriesPrefix(ctx, srcConn.Host, srcPath) if err != nil { - return false, fmt.Errorf("error listing source directory: %w", err) + return fmt.Errorf("error listing source directory: %w", err) } tree := pathtree.NewTree(srcPath, fspath.Separator) @@ -73,7 +81,7 @@ func PrefixCopyInternal(ctx context.Context, srcConn, destConn *connparse.Connec if !srcHasSlash { prefixToRemove = fspath.Dir(srcPath) + fspath.Separator } - return true, tree.Walk(func(path string, numChildren int) error { + return tree.Walk(func(path string, numChildren int) error { // since this is a prefix filesystem, we only care about leafs if numChildren > 0 { return nil @@ -82,21 +90,21 @@ func PrefixCopyInternal(ctx context.Context, srcConn, destConn *connparse.Connec return copyFunc(ctx, path, destFilePath) }) } else { - return false, copyFunc(ctx, srcPath, destPath) + return copyFunc(ctx, srcPath, destPath) } } -func PrefixCopyRemote(ctx context.Context, srcConn, destConn *connparse.Connection, srcClient, destClient fstype.FileShareClient, destPutFile func(host string, path string, size int64, reader io.Reader) error, opts *wshrpc.FileCopyOpts) (bool, error) { +func PrefixCopyRemote(ctx context.Context, srcConn, destConn *connparse.Connection, srcClient, destClient fstype.FileShareClient, destPutFile func(host string, path string, size int64, reader io.Reader) error, opts *wshrpc.FileCopyOpts) error { // prefix to be used if the destination is a directory. The destPath returned in the following call only applies if the destination is not a directory. destPathPrefix, err := CleanPathPrefix(destConn.Path) if err != nil { - return false, fmt.Errorf("error cleaning destination path: %w", err) + return fmt.Errorf("error cleaning destination path: %w", err) } destPathPrefix += fspath.Separator _, destPath, srcInfo, err := DetermineCopyDestPath(ctx, srcConn, destConn, srcClient, destClient, opts) if err != nil { - return false, err + return err } log.Printf("Copying: %v -> %v", srcConn.GetFullURI(), destConn.GetFullURI()) @@ -122,9 +130,9 @@ func PrefixCopyRemote(ctx context.Context, srcConn, destConn *connparse.Connecti }) if err != nil { cancel(err) - return false, err + return err } - return srcInfo.IsDir, nil + return nil } func DetermineCopyDestPath(ctx context.Context, srcConn, destConn *connparse.Connection, srcClient, destClient fstype.FileShareClient, opts *wshrpc.FileCopyOpts) (srcPath, destPath string, srcInfo *wshrpc.FileInfo, err error) { diff --git a/pkg/remote/fileshare/s3fs/s3fs.go b/pkg/remote/fileshare/s3fs/s3fs.go index 26c90d7434..33f8b328f4 100644 --- a/pkg/remote/fileshare/s3fs/s3fs.go +++ b/pkg/remote/fileshare/s3fs/s3fs.go @@ -150,6 +150,7 @@ func (c S3Client) ReadStream(ctx context.Context, conn *connparse.Connection, da } func (c S3Client) ReadTarStream(ctx context.Context, conn *connparse.Connection, opts *wshrpc.FileCopyOpts) <-chan wshrpc.RespOrErrorUnion[iochantypes.Packet] { + recursive := opts != nil && opts.Recursive bucket := conn.Host if bucket == "" || bucket == "/" { return wshutil.SendErrCh[iochantypes.Packet](fmt.Errorf("bucket must be specified")) @@ -187,6 +188,10 @@ func (c S3Client) ReadTarStream(ctx context.Context, conn *connparse.Connection, // whether the operation is on a single file singleFile := singleFileResult != nil + if !singleFile && !recursive { + return wshutil.SendErrCh[iochantypes.Packet](fmt.Errorf(fsutil.RecursiveCopyError)) + } + // whether to include the directory itself in the tar includeDir := (wholeBucket && conn.Path == "") || (singleFileResult == nil && conn.Path != "" && !strings.HasSuffix(conn.Path, fspath.Separator)) @@ -646,20 +651,21 @@ func (c S3Client) Mkdir(ctx context.Context, conn *connparse.Connection) error { } func (c S3Client) MoveInternal(ctx context.Context, srcConn, destConn *connparse.Connection, opts *wshrpc.FileCopyOpts) error { - isDir, err := c.CopyInternal(ctx, srcConn, destConn, opts) + err := c.CopyInternal(ctx, srcConn, destConn, opts) if err != nil { return err } - return c.Delete(ctx, srcConn, isDir) + recursive := opts != nil && opts.Recursive + return c.Delete(ctx, srcConn, recursive) } -func (c S3Client) CopyRemote(ctx context.Context, srcConn, destConn *connparse.Connection, srcClient fstype.FileShareClient, opts *wshrpc.FileCopyOpts) (bool, error) { +func (c S3Client) CopyRemote(ctx context.Context, srcConn, destConn *connparse.Connection, srcClient fstype.FileShareClient, opts *wshrpc.FileCopyOpts) error { if srcConn.Scheme == connparse.ConnectionTypeS3 && destConn.Scheme == connparse.ConnectionTypeS3 { return c.CopyInternal(ctx, srcConn, destConn, opts) } destBucket := destConn.Host if destBucket == "" || destBucket == fspath.Separator { - return false, fmt.Errorf("destination bucket must be specified") + return fmt.Errorf("destination bucket must be specified") } return fsutil.PrefixCopyRemote(ctx, srcConn, destConn, srcClient, c, func(bucket, path string, size int64, reader io.Reader) error { _, err := c.client.PutObject(ctx, &s3.PutObjectInput{ @@ -672,11 +678,11 @@ func (c S3Client) CopyRemote(ctx context.Context, srcConn, destConn *connparse.C }, opts) } -func (c S3Client) CopyInternal(ctx context.Context, srcConn, destConn *connparse.Connection, opts *wshrpc.FileCopyOpts) (bool, error) { +func (c S3Client) CopyInternal(ctx context.Context, srcConn, destConn *connparse.Connection, opts *wshrpc.FileCopyOpts) error { srcBucket := srcConn.Host destBucket := destConn.Host if srcBucket == "" || srcBucket == fspath.Separator || destBucket == "" || destBucket == fspath.Separator { - return false, fmt.Errorf("source and destination bucket must be specified") + return fmt.Errorf("source and destination bucket must be specified") } return fsutil.PrefixCopyInternal(ctx, srcConn, destConn, c, opts, func(ctx context.Context, bucket, prefix string) ([]string, error) { var entries []string diff --git a/pkg/remote/fileshare/wavefs/wavefs.go b/pkg/remote/fileshare/wavefs/wavefs.go index fd372bd65b..441fedfe41 100644 --- a/pkg/remote/fileshare/wavefs/wavefs.go +++ b/pkg/remote/fileshare/wavefs/wavefs.go @@ -422,17 +422,18 @@ func (c WaveClient) MoveInternal(ctx context.Context, srcConn, destConn *connpar if srcConn.Host != destConn.Host { return fmt.Errorf("move internal, src and dest hosts do not match") } - isDir, err := c.CopyInternal(ctx, srcConn, destConn, opts) + err := c.CopyInternal(ctx, srcConn, destConn, opts) if err != nil { return fmt.Errorf("error copying blockfile: %w", err) } - if err := c.Delete(ctx, srcConn, isDir); err != nil { + recursive := opts != nil && opts.Recursive + if err := c.Delete(ctx, srcConn, recursive); err != nil { return fmt.Errorf("error deleting blockfile: %w", err) } return nil } -func (c WaveClient) CopyInternal(ctx context.Context, srcConn, destConn *connparse.Connection, opts *wshrpc.FileCopyOpts) (bool, error) { +func (c WaveClient) CopyInternal(ctx context.Context, srcConn, destConn *connparse.Connection, opts *wshrpc.FileCopyOpts) error { return fsutil.PrefixCopyInternal(ctx, srcConn, destConn, c, opts, func(ctx context.Context, zoneId, prefix string) ([]string, error) { entryList := make([]string, 0) if err := listFilesPrefix(ctx, zoneId, prefix, func(wf *filestore.WaveFile) error { @@ -467,13 +468,13 @@ func (c WaveClient) CopyInternal(ctx context.Context, srcConn, destConn *connpar }) } -func (c WaveClient) CopyRemote(ctx context.Context, srcConn, destConn *connparse.Connection, srcClient fstype.FileShareClient, opts *wshrpc.FileCopyOpts) (bool, error) { +func (c WaveClient) CopyRemote(ctx context.Context, srcConn, destConn *connparse.Connection, srcClient fstype.FileShareClient, opts *wshrpc.FileCopyOpts) error { if srcConn.Scheme == connparse.ConnectionTypeWave && destConn.Scheme == connparse.ConnectionTypeWave { return c.CopyInternal(ctx, srcConn, destConn, opts) } zoneId := destConn.Host if zoneId == "" { - return false, fmt.Errorf("zoneid not found in connection") + return fmt.Errorf("zoneid not found in connection") } return fsutil.PrefixCopyRemote(ctx, srcConn, destConn, srcClient, c, func(zoneId, path string, size int64, reader io.Reader) error { dataBuf := make([]byte, size) diff --git a/pkg/remote/fileshare/wshfs/wshfs.go b/pkg/remote/fileshare/wshfs/wshfs.go index 41dfdd15f5..ae0930e864 100644 --- a/pkg/remote/fileshare/wshfs/wshfs.go +++ b/pkg/remote/fileshare/wshfs/wshfs.go @@ -114,11 +114,11 @@ func (c WshClient) MoveInternal(ctx context.Context, srcConn, destConn *connpars return wshclient.RemoteFileMoveCommand(RpcClient, wshrpc.CommandFileCopyData{SrcUri: srcConn.GetFullURI(), DestUri: destConn.GetFullURI(), Opts: opts}, &wshrpc.RpcOpts{Route: wshutil.MakeConnectionRouteId(destConn.Host), Timeout: timeout}) } -func (c WshClient) CopyRemote(ctx context.Context, srcConn, destConn *connparse.Connection, _ fstype.FileShareClient, opts *wshrpc.FileCopyOpts) (bool, error) { +func (c WshClient) CopyRemote(ctx context.Context, srcConn, destConn *connparse.Connection, _ fstype.FileShareClient, opts *wshrpc.FileCopyOpts) error { return c.CopyInternal(ctx, srcConn, destConn, opts) } -func (c WshClient) CopyInternal(ctx context.Context, srcConn, destConn *connparse.Connection, opts *wshrpc.FileCopyOpts) (bool, error) { +func (c WshClient) CopyInternal(ctx context.Context, srcConn, destConn *connparse.Connection, opts *wshrpc.FileCopyOpts) error { if opts == nil { opts = &wshrpc.FileCopyOpts{} } diff --git a/pkg/wshrpc/wshclient/wshclient.go b/pkg/wshrpc/wshclient/wshclient.go index 243065b9cc..1ded90d83c 100644 --- a/pkg/wshrpc/wshclient/wshclient.go +++ b/pkg/wshrpc/wshclient/wshclient.go @@ -356,9 +356,9 @@ func RecordTEventCommand(w *wshutil.WshRpc, data telemetrydata.TEvent, opts *wsh } // command "remotefilecopy", wshserver.RemoteFileCopyCommand -func RemoteFileCopyCommand(w *wshutil.WshRpc, data wshrpc.CommandFileCopyData, opts *wshrpc.RpcOpts) (bool, error) { - resp, err := sendRpcRequestCallHelper[bool](w, "remotefilecopy", data, opts) - return resp, err +func RemoteFileCopyCommand(w *wshutil.WshRpc, data wshrpc.CommandFileCopyData, opts *wshrpc.RpcOpts) error { + _, err := sendRpcRequestCallHelper[any](w, "remotefilecopy", data, opts) + return err } // command "remotefiledelete", wshserver.RemoteFileDeleteCommand diff --git a/pkg/wshrpc/wshremote/wshremote.go b/pkg/wshrpc/wshremote/wshremote.go index 4b83fa7393..17e9e4ea20 100644 --- a/pkg/wshrpc/wshremote/wshremote.go +++ b/pkg/wshrpc/wshremote/wshremote.go @@ -19,6 +19,7 @@ import ( "github.com/wavetermdev/waveterm/pkg/remote/connparse" "github.com/wavetermdev/waveterm/pkg/remote/fileshare/fstype" + "github.com/wavetermdev/waveterm/pkg/remote/fileshare/fsutil" "github.com/wavetermdev/waveterm/pkg/remote/fileshare/wshfs" "github.com/wavetermdev/waveterm/pkg/suggestion" "github.com/wavetermdev/waveterm/pkg/util/fileutil" @@ -740,7 +741,7 @@ func (impl *ServerImpl) RemoteFileMoveCommand(ctx context.Context, data wshrpc.C return fmt.Errorf("cannot stat file %q: %w", srcPathCleaned, err) } if finfo.IsDir() && !recursive { - return fmt.Errorf("cannot move directory %q, recursive option not specified", srcUri) + return fmt.Errorf(fsutil.RecursiveCopyError) } err = os.Rename(srcPathCleaned, destPathCleaned) if err != nil { diff --git a/pkg/wshrpc/wshrpctypes.go b/pkg/wshrpc/wshrpctypes.go index 00be058bf7..cc4ef1e14d 100644 --- a/pkg/wshrpc/wshrpctypes.go +++ b/pkg/wshrpc/wshrpctypes.go @@ -228,7 +228,7 @@ type WshRpcInterface interface { // remotes RemoteStreamFileCommand(ctx context.Context, data CommandRemoteStreamFileData) chan RespOrErrorUnion[FileData] RemoteTarStreamCommand(ctx context.Context, data CommandRemoteStreamTarData) <-chan RespOrErrorUnion[iochantypes.Packet] - RemoteFileCopyCommand(ctx context.Context, data CommandFileCopyData) (bool, error) + RemoteFileCopyCommand(ctx context.Context, data CommandFileCopyData) error RemoteListEntriesCommand(ctx context.Context, data CommandRemoteListEntriesData) chan RespOrErrorUnion[CommandRemoteListEntriesRtnData] RemoteFileInfoCommand(ctx context.Context, path string) (*FileInfo, error) RemoteFileTouchCommand(ctx context.Context, path string) error From e557f3bcc6c8d5fd3af33ebdd9c2060b60b450e7 Mon Sep 17 00:00:00 2001 From: Evan Simkowitz Date: Wed, 19 Feb 2025 15:58:07 -0800 Subject: [PATCH 06/16] unify merge and overwrite errors --- .../app/view/preview/directorypreview.tsx | 7 ++- pkg/remote/fileshare/fstype/fstype.go | 9 ++- pkg/remote/fileshare/fsutil/fsutil.go | 14 ++--- pkg/remote/fileshare/s3fs/s3fs.go | 2 +- pkg/wshrpc/wshremote/wshremote.go | 56 +++++++++---------- pkg/wshrpc/wshrpctypes.go | 2 +- 6 files changed, 41 insertions(+), 49 deletions(-) diff --git a/frontend/app/view/preview/directorypreview.tsx b/frontend/app/view/preview/directorypreview.tsx index 02df998e61..3fc50a5b79 100644 --- a/frontend/app/view/preview/directorypreview.tsx +++ b/frontend/app/view/preview/directorypreview.tsx @@ -866,9 +866,10 @@ function DirectoryPreview({ model }: DirectoryPreviewProps) { console.log("copy failed:", e); const copyError = `${e}`; const allowRetry = - copyError.includes("overwrite not specified") || - copyError.includes("neither overwrite nor merge specified") || - copyError.includes("neither merge nor overwrite specified"); + copyError.includes("set overwrite flag to delete the existing file") || + copyError.includes( + "set overwrite flag to delete the existing contents or set merge flag to merge the contents" + ); const copyStatus: FileCopyStatus = { copyError, copyData: data, diff --git a/pkg/remote/fileshare/fstype/fstype.go b/pkg/remote/fileshare/fstype/fstype.go index 19268d566e..346dbdb6d4 100644 --- a/pkg/remote/fileshare/fstype/fstype.go +++ b/pkg/remote/fileshare/fstype/fstype.go @@ -14,9 +14,12 @@ import ( ) const ( - DefaultTimeout = 30 * time.Second - FileMode os.FileMode = 0644 - DirMode os.FileMode = 0755 | os.ModeDir + DefaultTimeout = 30 * time.Second + FileMode os.FileMode = 0644 + DirMode os.FileMode = 0755 | os.ModeDir + RecursiveCopyError = "recursive flag must be set for directory operations" + MergeCopyError = "directory already exists at %q, set overwrite flag to delete the existing contents or set merge flag to merge the contents" + OverwriteCopyError = "file already exists at %q, set overwrite flag to delete the existing file" ) type FileShareClient interface { diff --git a/pkg/remote/fileshare/fsutil/fsutil.go b/pkg/remote/fileshare/fsutil/fsutil.go index 6c247784c6..74aa689ff6 100644 --- a/pkg/remote/fileshare/fsutil/fsutil.go +++ b/pkg/remote/fileshare/fsutil/fsutil.go @@ -21,10 +21,6 @@ import ( "github.com/wavetermdev/waveterm/pkg/wshrpc" ) -const ( - RecursiveCopyError = "recursive flag must be set to true for directory operations" -) - func GetParentPath(conn *connparse.Connection) string { hostAndPath := conn.GetPathWithHost() return GetParentPathString(hostAndPath) @@ -57,7 +53,7 @@ func PrefixCopyInternal(ctx context.Context, srcConn, destConn *connparse.Connec recursive := opts != nil && opts.Recursive if srcInfo.IsDir { if !recursive { - return fmt.Errorf(RecursiveCopyError) + return fmt.Errorf(fstype.RecursiveCopyError) } if !srcHasSlash { srcPath += fspath.Separator @@ -179,12 +175,10 @@ func DetermineCopyDestPath(ctx context.Context, srcConn, destConn *connparse.Con if err != nil { return "", "", nil, fmt.Errorf("error deleting conflicting destination file: %w", err) } - } else if destInfo.IsDir && srcInfo.IsDir { - if !merge { - return "", "", nil, fmt.Errorf("destination and source are both directories, neither merge nor overwrite specified: %v", destConn.GetFullURI()) - } + } else if destInfo.IsDir && srcInfo.IsDir && !merge { + return "", "", nil, fmt.Errorf(fstype.MergeCopyError, destConn.GetFullURI()) } else { - return "", "", nil, fmt.Errorf("destination already exists, overwrite not specified: %v", destConn.GetFullURI()) + return "", "", nil, fmt.Errorf(fstype.OverwriteCopyError, destConn.GetFullURI()) } } return srcPath, destPath, srcInfo, nil diff --git a/pkg/remote/fileshare/s3fs/s3fs.go b/pkg/remote/fileshare/s3fs/s3fs.go index 33f8b328f4..3c685689ec 100644 --- a/pkg/remote/fileshare/s3fs/s3fs.go +++ b/pkg/remote/fileshare/s3fs/s3fs.go @@ -189,7 +189,7 @@ func (c S3Client) ReadTarStream(ctx context.Context, conn *connparse.Connection, singleFile := singleFileResult != nil if !singleFile && !recursive { - return wshutil.SendErrCh[iochantypes.Packet](fmt.Errorf(fsutil.RecursiveCopyError)) + return wshutil.SendErrCh[iochantypes.Packet](fmt.Errorf(fstype.RecursiveCopyError)) } // whether to include the directory itself in the tar diff --git a/pkg/wshrpc/wshremote/wshremote.go b/pkg/wshrpc/wshremote/wshremote.go index 17e9e4ea20..1a87f76ee9 100644 --- a/pkg/wshrpc/wshremote/wshremote.go +++ b/pkg/wshrpc/wshremote/wshremote.go @@ -19,7 +19,6 @@ import ( "github.com/wavetermdev/waveterm/pkg/remote/connparse" "github.com/wavetermdev/waveterm/pkg/remote/fileshare/fstype" - "github.com/wavetermdev/waveterm/pkg/remote/fileshare/fsutil" "github.com/wavetermdev/waveterm/pkg/remote/fileshare/wshfs" "github.com/wavetermdev/waveterm/pkg/suggestion" "github.com/wavetermdev/waveterm/pkg/util/fileutil" @@ -307,7 +306,7 @@ func (impl *ServerImpl) RemoteTarStreamCommand(ctx context.Context, data wshrpc. return rtn } -func (impl *ServerImpl) RemoteFileCopyCommand(ctx context.Context, data wshrpc.CommandFileCopyData) (bool, error) { +func (impl *ServerImpl) RemoteFileCopyCommand(ctx context.Context, data wshrpc.CommandFileCopyData) error { log.Printf("RemoteFileCopyCommand: src=%s, dest=%s\n", data.SrcUri, data.DestUri) opts := data.Opts if opts == nil { @@ -317,16 +316,19 @@ func (impl *ServerImpl) RemoteFileCopyCommand(ctx context.Context, data wshrpc.C srcUri := data.SrcUri merge := opts.Merge overwrite := opts.Overwrite + if overwrite && merge { + return fmt.Errorf("cannot specify both overwrite and merge") + } destConn, err := connparse.ParseURIAndReplaceCurrentHost(ctx, destUri) if err != nil { - return false, fmt.Errorf("cannot parse destination URI %q: %w", srcUri, err) + return fmt.Errorf("cannot parse destination URI %q: %w", srcUri, err) } destPathCleaned := filepath.Clean(wavebase.ExpandHomeDirSafe(destConn.Path)) destinfo, err := os.Stat(destPathCleaned) if err != nil { if !errors.Is(err, fs.ErrNotExist) { - return false, fmt.Errorf("cannot stat destination %q: %w", destPathCleaned, err) + return fmt.Errorf("cannot stat destination %q: %w", destPathCleaned, err) } } @@ -336,17 +338,17 @@ func (impl *ServerImpl) RemoteFileCopyCommand(ctx context.Context, data wshrpc.C if destExists && !destIsDir { if !overwrite { - return false, fmt.Errorf("file already exists at destination %q, use overwrite option", destPathCleaned) + return fmt.Errorf(fstype.OverwriteCopyError, destPathCleaned) } else { err := os.Remove(destPathCleaned) if err != nil { - return false, fmt.Errorf("cannot remove file %q: %w", destPathCleaned, err) + return fmt.Errorf("cannot remove file %q: %w", destPathCleaned, err) } } } srcConn, err := connparse.ParseURIAndReplaceCurrentHost(ctx, srcUri) if err != nil { - return false, fmt.Errorf("cannot parse source URI %q: %w", srcUri, err) + return fmt.Errorf("cannot parse source URI %q: %w", srcUri, err) } copyFileFunc := func(path string, finfo fs.FileInfo, srcFile io.Reader) (int64, error) { @@ -365,28 +367,24 @@ func (impl *ServerImpl) RemoteFileCopyCommand(ctx context.Context, data wshrpc.C return 0, fmt.Errorf("cannot stat file %q: %w", path, err) } if newdestinfo != nil && !overwrite { - return 0, fmt.Errorf("cannot create file %q, file exists at path, overwrite not specified", path) + return 0, fmt.Errorf(fstype.OverwriteCopyError, path) } - } else if !merge && !overwrite { - return 0, fmt.Errorf("cannot create directory %q, directory exists at path, neither overwrite nor merge specified", path) } else if overwrite { err := os.RemoveAll(path) if err != nil { return 0, fmt.Errorf("cannot remove directory %q: %w", path, err) } + } else if !merge { + return 0, fmt.Errorf(fstype.MergeCopyError, path) } } else { - if finfo.IsDir() { - if !overwrite { - return 0, fmt.Errorf("cannot create file %q, directory exists at path, overwrite not specified", path) - } else { - err := os.RemoveAll(path) - if err != nil { - return 0, fmt.Errorf("cannot remove directory %q: %w", path, err) - } + if !overwrite { + return 0, fmt.Errorf(fstype.OverwriteCopyError, path) + } else if finfo.IsDir() { + err := os.RemoveAll(path) + if err != nil { + return 0, fmt.Errorf("cannot remove directory %q: %w", path, err) } - } else if !overwrite { - return 0, fmt.Errorf("cannot create file %q, file exists at path, overwrite not specified", path) } } } @@ -418,18 +416,15 @@ func (impl *ServerImpl) RemoteFileCopyCommand(ctx context.Context, data wshrpc.C return finfo.Size(), nil } - srcIsDir := false - if srcConn.Host == destConn.Host { srcPathCleaned := filepath.Clean(wavebase.ExpandHomeDirSafe(srcConn.Path)) srcFileStat, err := os.Stat(srcPathCleaned) if err != nil { - return false, fmt.Errorf("cannot stat file %q: %w", srcPathCleaned, err) + return fmt.Errorf("cannot stat file %q: %w", srcPathCleaned, err) } if srcFileStat.IsDir() { - srcIsDir = true var srcPathPrefix string if destIsDir { srcPathPrefix = filepath.Dir(srcPathCleaned) @@ -454,12 +449,12 @@ func (impl *ServerImpl) RemoteFileCopyCommand(ctx context.Context, data wshrpc.C return err }) if err != nil { - return false, fmt.Errorf("cannot copy %q to %q: %w", srcUri, destUri, err) + return fmt.Errorf("cannot copy %q to %q: %w", srcUri, destUri, err) } } else { file, err := os.Open(srcPathCleaned) if err != nil { - return false, fmt.Errorf("cannot open file %q: %w", srcPathCleaned, err) + return fmt.Errorf("cannot open file %q: %w", srcPathCleaned, err) } defer utilfn.GracefulClose(file, "RemoteFileCopyCommand", srcPathCleaned) var destFilePath string @@ -470,7 +465,7 @@ func (impl *ServerImpl) RemoteFileCopyCommand(ctx context.Context, data wshrpc.C } _, err = copyFileFunc(destFilePath, srcFileStat, file) if err != nil { - return false, fmt.Errorf("cannot copy %q to %q: %w", srcUri, destUri, err) + return fmt.Errorf("cannot copy %q to %q: %w", srcUri, destUri, err) } } } else { @@ -491,7 +486,6 @@ func (impl *ServerImpl) RemoteFileCopyCommand(ctx context.Context, data wshrpc.C numFiles++ nextpath := filepath.Join(destPathCleaned, next.Name) log.Printf("RemoteFileCopyCommand: copying %q to %q\n", next.Name, nextpath) - srcIsDir = !singleFile if singleFile && !destHasSlash { // custom flag to indicate that the source is a single file, not a directory the contents of a directory nextpath = destPathCleaned @@ -505,7 +499,7 @@ func (impl *ServerImpl) RemoteFileCopyCommand(ctx context.Context, data wshrpc.C return nil }) if err != nil { - return false, fmt.Errorf("cannot copy %q to %q: %w", srcUri, destUri, err) + return fmt.Errorf("cannot copy %q to %q: %w", srcUri, destUri, err) } totalTime := time.Since(copyStart).Seconds() totalMegaBytes := float64(totalBytes) / 1024 / 1024 @@ -515,7 +509,7 @@ func (impl *ServerImpl) RemoteFileCopyCommand(ctx context.Context, data wshrpc.C } log.Printf("RemoteFileCopyCommand: done; %d files copied in %.3fs, total of %.4f MB, %.2f MB/s, %d files skipped\n", numFiles, totalTime, totalMegaBytes, rate, numSkipped) } - return srcIsDir, nil + return nil } func (impl *ServerImpl) RemoteListEntriesCommand(ctx context.Context, data wshrpc.CommandRemoteListEntriesData) chan wshrpc.RespOrErrorUnion[wshrpc.CommandRemoteListEntriesRtnData] { @@ -741,7 +735,7 @@ func (impl *ServerImpl) RemoteFileMoveCommand(ctx context.Context, data wshrpc.C return fmt.Errorf("cannot stat file %q: %w", srcPathCleaned, err) } if finfo.IsDir() && !recursive { - return fmt.Errorf(fsutil.RecursiveCopyError) + return fmt.Errorf(fstype.RecursiveCopyError) } err = os.Rename(srcPathCleaned, destPathCleaned) if err != nil { diff --git a/pkg/wshrpc/wshrpctypes.go b/pkg/wshrpc/wshrpctypes.go index cc4ef1e14d..50a2460fdd 100644 --- a/pkg/wshrpc/wshrpctypes.go +++ b/pkg/wshrpc/wshrpctypes.go @@ -547,7 +547,7 @@ type CommandRemoteStreamTarData struct { type FileCopyOpts struct { Overwrite bool `json:"overwrite,omitempty"` Recursive bool `json:"recursive,omitempty"` // only used for move, always true for copy - Merge bool `json:"merge,omitempty"` // only used for copy, always false for move + Merge bool `json:"merge,omitempty"` Timeout int64 `json:"timeout,omitempty"` } From 9583d7384afef43a30a962b1e164b7dbad2995c6 Mon Sep 17 00:00:00 2001 From: Evan Simkowitz Date: Wed, 19 Feb 2025 16:12:52 -0800 Subject: [PATCH 07/16] remove unnecessary logs --- pkg/remote/fileshare/s3fs/s3fs.go | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/pkg/remote/fileshare/s3fs/s3fs.go b/pkg/remote/fileshare/s3fs/s3fs.go index 3c685689ec..609c339dba 100644 --- a/pkg/remote/fileshare/s3fs/s3fs.go +++ b/pkg/remote/fileshare/s3fs/s3fs.go @@ -212,7 +212,6 @@ func (c S3Client) ReadTarStream(ctx context.Context, conn *connparse.Connection, tarPathPrefix = bucket } } else if singleFile || includeDir { - log.Printf("singleFile or includeDir; tarPathPrefix: %v", tarPathPrefix) // if we're including the directory itself, we need to remove the last part of the path tarPathPrefix = fsutil.GetParentPathString(tarPathPrefix) } @@ -229,7 +228,6 @@ func (c S3Client) ReadTarStream(ctx context.Context, conn *connparse.Connection, // close the objects when we're done defer func() { for key, obj := range objMap { - log.Printf("closing object %v", key) utilfn.GracefulClose(obj.Body, "s3fs", key) } }() @@ -301,7 +299,6 @@ func (c S3Client) ReadTarStream(ctx context.Context, conn *connparse.Connection, // Walk the tree and write the tar entries if err := tree.Walk(func(path string, numChildren int) error { - log.Printf("writing %v", path) mapEntry, isFile := objMap[path] // default vals assume entry is dir, since mapEntry might not exist @@ -605,13 +602,11 @@ func (c S3Client) Stat(ctx context.Context, conn *connparse.Connection) (*wshrpc func (c S3Client) PutFile(ctx context.Context, conn *connparse.Connection, data wshrpc.FileData) error { if data.At != nil { - log.Printf("PutFile: offset %d and size %d", data.At.Offset, data.At.Size) return errors.Join(errors.ErrUnsupported, fmt.Errorf("file data offset and size not supported")) } bucket := conn.Host objectKey := conn.Path if bucket == "" || bucket == "/" || objectKey == "" || objectKey == "/" { - log.Printf("PutFile: bucket and object key must be specified") return errors.Join(errors.ErrUnsupported, fmt.Errorf("bucket and object key must be specified")) } contentMaxLength := base64.StdEncoding.DecodedLen(len(data.Data64)) @@ -622,7 +617,6 @@ func (c S3Client) PutFile(ctx context.Context, conn *connparse.Connection, data decodedBody = make([]byte, contentMaxLength) contentLength, err = base64.StdEncoding.Decode(decodedBody, []byte(data.Data64)) if err != nil { - log.Printf("PutFile: error decoding data: %v", err) return err } } else { @@ -706,15 +700,12 @@ func (c S3Client) CopyInternal(ctx context.Context, srcConn, destConn *connparse } func (c S3Client) Delete(ctx context.Context, conn *connparse.Connection, recursive bool) error { - log.Printf("s3fs.Delete: %v", conn.GetFullURI()) bucket := conn.Host objectKey := conn.Path if bucket == "" || bucket == fspath.Separator { - log.Printf("Delete: bucket must be specified") return errors.Join(errors.ErrUnsupported, fmt.Errorf("bucket must be specified")) } if objectKey == "" || objectKey == fspath.Separator { - log.Printf("Delete: object key must be specified") return errors.Join(errors.ErrUnsupported, fmt.Errorf("object key must be specified")) } if recursive { @@ -732,14 +723,11 @@ func (c S3Client) Delete(ctx context.Context, conn *connparse.Connection, recurs return true, nil }) if err != nil { - log.Printf("Error listing objects: %v", err) return err } if len(objects) == 0 { - log.Printf("No objects found with prefix %v:%v", bucket, objectKey) return nil } - log.Printf("Deleting %d objects with prefix %v:%v", len(objects), bucket, objectKey) _, err = c.client.DeleteObjects(ctx, &s3.DeleteObjectsInput{ Bucket: aws.String(bucket), Delete: &types.Delete{ From d72fa854eea95419d8e4245a7e22dd10dcd228cf Mon Sep 17 00:00:00 2001 From: Evan Simkowitz Date: Wed, 19 Feb 2025 16:13:35 -0800 Subject: [PATCH 08/16] undo change in wavefs --- pkg/remote/fileshare/wavefs/wavefs.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/pkg/remote/fileshare/wavefs/wavefs.go b/pkg/remote/fileshare/wavefs/wavefs.go index 441fedfe41..46aeedcad2 100644 --- a/pkg/remote/fileshare/wavefs/wavefs.go +++ b/pkg/remote/fileshare/wavefs/wavefs.go @@ -422,8 +422,7 @@ func (c WaveClient) MoveInternal(ctx context.Context, srcConn, destConn *connpar if srcConn.Host != destConn.Host { return fmt.Errorf("move internal, src and dest hosts do not match") } - err := c.CopyInternal(ctx, srcConn, destConn, opts) - if err != nil { + if err := c.CopyInternal(ctx, srcConn, destConn, opts); err != nil { return fmt.Errorf("error copying blockfile: %w", err) } recursive := opts != nil && opts.Recursive From 9028fdb632ecbba2d3ae70bbf1e4cd50c250a395 Mon Sep 17 00:00:00 2001 From: Evan Simkowitz Date: Wed, 19 Feb 2025 16:25:57 -0800 Subject: [PATCH 09/16] switch copy error status to use new preview erroroverlay --- .../app/view/preview/directorypreview.tsx | 180 +++++------------- frontend/app/view/preview/preview.tsx | 28 +-- 2 files changed, 68 insertions(+), 140 deletions(-) diff --git a/frontend/app/view/preview/directorypreview.tsx b/frontend/app/view/preview/directorypreview.tsx index 3fc50a5b79..11fe651d48 100644 --- a/frontend/app/view/preview/directorypreview.tsx +++ b/frontend/app/view/preview/directorypreview.tsx @@ -2,9 +2,7 @@ // SPDX-License-Identifier: Apache-2.0 import { Button } from "@/app/element/button"; -import { CopyButton } from "@/app/element/copybutton"; import { Input } from "@/app/element/input"; -import { useDimensionsWithCallbackRef } from "@/app/hook/useDimensions"; import { ContextMenuModel } from "@/app/store/contextmenu"; import { atoms, getApi, globalStore } from "@/app/store/global"; import { RpcApi } from "@/app/store/wshclientapi"; @@ -732,7 +730,7 @@ function DirectoryPreview({ model }: DirectoryPreviewProps) { const blockData = useAtomValue(model.blockAtom); const finfo = useAtomValue(model.statFile); const dirPath = finfo?.path; - const [copyStatus, setCopyStatus] = useState(null); + const setErrorMsg = useSetAtom(model.errorMsgAtom); useEffect(() => { model.refreshCallback = () => { @@ -861,7 +859,6 @@ function DirectoryPreview({ model }: DirectoryPreviewProps) { async (data: CommandFileCopyData, isDir: boolean) => { try { await RpcApi.FileCopyCommand(TabRpcClient, data, { timeout: data.opts.timeout }); - setCopyStatus(null); } catch (e) { console.log("copy failed:", e); const copyError = `${e}`; @@ -870,17 +867,59 @@ function DirectoryPreview({ model }: DirectoryPreviewProps) { copyError.includes( "set overwrite flag to delete the existing contents or set merge flag to merge the contents" ); - const copyStatus: FileCopyStatus = { - copyError, - copyData: data, - allowRetry, - isDir: isDir, - }; - setCopyStatus(copyStatus); + let errorMsg: ErrorMsg; + if (allowRetry) { + errorMsg = { + status: "Confirm Overwrite File(s)", + text: "This copy operation will overwrite an existing file. Would you like to continue?", + level: "warning", + buttons: [ + { + text: "Delete Then Copy", + onClick: async () => { + await handleDropCopy( + { + srcuri: data.desturi, + desturi: data.srcuri, + opts: { + timeout: data.opts.timeout, + overwrite: true, + }, + }, + isDir + ); + }, + }, + { + text: "Sync", + onClick: async () => { + await handleDropCopy( + { + srcuri: data.srcuri, + desturi: data.desturi, + opts: { + timeout: data.opts.timeout, + merge: true, + }, + }, + isDir + ); + }, + }, + ], + }; + } else { + errorMsg = { + status: "Copy Failed", + text: copyError, + level: "error", + }; + } + setErrorMsg(errorMsg); } model.refreshCallback(); }, - [setCopyStatus, model.refreshCallback] + [model.refreshCallback] ); const [, drop] = useDrop( @@ -913,7 +952,7 @@ function DirectoryPreview({ model }: DirectoryPreviewProps) { }, // TODO: mabe add a hover option? }), - [dirPath, model.formatRemoteUri, model.refreshCallback, setCopyStatus] + [dirPath, model.formatRemoteUri, model.refreshCallback] ); useEffect(() => { @@ -1005,13 +1044,6 @@ function DirectoryPreview({ model }: DirectoryPreviewProps) { onContextMenu={(e) => handleFileContextMenu(e)} onClick={() => setEntryManagerProps(undefined)} > - {copyStatus != null && ( - - )} void; - handleDropCopy: (data: CommandFileCopyData, isDir: boolean) => Promise; - }) => { - const [overlayRefCallback, _, domRect] = useDimensionsWithCallbackRef(30); - const width = domRect?.width; - - const handleRetryCopy = React.useCallback( - async (copyOpt?: string) => { - if (!copyStatus) { - return; - } - let overwrite = copyOpt == "overwrite"; - let merge = copyOpt == "merge"; - const updatedData = { - ...copyStatus.copyData, - opts: { ...copyStatus.copyData.opts, overwrite, merge }, - }; - await handleDropCopy(updatedData, copyStatus.isDir); - }, - [copyStatus.copyData] - ); - - let statusText = "Copy Error"; - let errorMsg = `error: ${copyStatus?.copyError}`; - if (copyStatus?.allowRetry) { - statusText = "Confirm Overwrite File(s)"; - errorMsg = "This copy operation will overwrite an existing file. Would you like to continue?"; - } - - const buttonClassName = "outlined grey font-size-11 vertical-padding-3 horizontal-padding-7"; - - const handleRemoveCopyError = React.useCallback(async () => { - setCopyStatus(null); - }, [setCopyStatus]); - - const handleCopyToClipboard = React.useCallback(async () => { - await navigator.clipboard.writeText(errorMsg); - }, [errorMsg]); - - return ( -
-
-
- - -
-
- {statusText} -
- - - -
{errorMsg}
- - - {copyStatus?.allowRetry && ( -
- - {copyStatus.isDir && ( - - )} - -
- )} -
- - {!copyStatus?.allowRetry && ( -
-
- )} -
-
-
- ); - } -); - export { DirectoryPreview }; diff --git a/frontend/app/view/preview/preview.tsx b/frontend/app/view/preview/preview.tsx index 27e09c3233..e4757cc9ab 100644 --- a/frontend/app/view/preview/preview.tsx +++ b/frontend/app/view/preview/preview.tsx @@ -1216,18 +1216,22 @@ const ErrorOverlay = memo(({ errorMsg, resetOverlay }: { errorMsg: ErrorMsg; res />
{errorMsg.text}
- {errorMsg.buttons?.map((buttonDef) => ( - - ))} + {!!errorMsg.buttons && ( +
+ {errorMsg.buttons?.map((buttonDef) => ( + + ))} +
+ )} {showDismiss && ( From e1a6488a83632e064af559c090287614096492e7 Mon Sep 17 00:00:00 2001 From: Evan Simkowitz Date: Wed, 19 Feb 2025 16:59:30 -0800 Subject: [PATCH 10/16] add back isDir return from copy --- frontend/app/store/wshclientapi.ts | 2 +- .../app/view/preview/directorypreview.tsx | 28 +++------------ pkg/remote/fileshare/fileshare.go | 21 +++++++----- pkg/remote/fileshare/fstype/fstype.go | 4 +-- pkg/remote/fileshare/fsutil/fsutil.go | 34 +++++++++++-------- pkg/remote/fileshare/pathtree/pathtree.go | 11 ++---- pkg/remote/fileshare/s3fs/s3fs.go | 12 +++---- pkg/remote/fileshare/wavefs/wavefs.go | 11 +++--- pkg/remote/fileshare/wshfs/wshfs.go | 4 +-- pkg/util/tarcopy/tarcopy.go | 2 -- pkg/wshrpc/wshclient/wshclient.go | 6 ++-- pkg/wshrpc/wshremote/wshremote.go | 29 +++++++++------- pkg/wshrpc/wshrpctypes.go | 2 +- 13 files changed, 76 insertions(+), 90 deletions(-) diff --git a/frontend/app/store/wshclientapi.ts b/frontend/app/store/wshclientapi.ts index 9ca02a5dda..fba651911f 100644 --- a/frontend/app/store/wshclientapi.ts +++ b/frontend/app/store/wshclientapi.ts @@ -293,7 +293,7 @@ class RpcApiType { } // command "remotefilecopy" [call] - RemoteFileCopyCommand(client: WshClient, data: CommandFileCopyData, opts?: RpcOpts): Promise { + RemoteFileCopyCommand(client: WshClient, data: CommandFileCopyData, opts?: RpcOpts): Promise { return client.wshRpcCall("remotefilecopy", data, opts); } diff --git a/frontend/app/view/preview/directorypreview.tsx b/frontend/app/view/preview/directorypreview.tsx index 11fe651d48..76c9221d45 100644 --- a/frontend/app/view/preview/directorypreview.tsx +++ b/frontend/app/view/preview/directorypreview.tsx @@ -860,7 +860,7 @@ function DirectoryPreview({ model }: DirectoryPreviewProps) { try { await RpcApi.FileCopyCommand(TabRpcClient, data, { timeout: data.opts.timeout }); } catch (e) { - console.log("copy failed:", e); + console.warn("copy failed:", e); const copyError = `${e}`; const allowRetry = copyError.includes("set overwrite flag to delete the existing file") || @@ -877,33 +877,15 @@ function DirectoryPreview({ model }: DirectoryPreviewProps) { { text: "Delete Then Copy", onClick: async () => { - await handleDropCopy( - { - srcuri: data.desturi, - desturi: data.srcuri, - opts: { - timeout: data.opts.timeout, - overwrite: true, - }, - }, - isDir - ); + data.opts.overwrite = true; + await handleDropCopy(data, isDir); }, }, { text: "Sync", onClick: async () => { - await handleDropCopy( - { - srcuri: data.srcuri, - desturi: data.desturi, - opts: { - timeout: data.opts.timeout, - merge: true, - }, - }, - isDir - ); + data.opts.merge = true; + await handleDropCopy(data, isDir); }, }, ], diff --git a/pkg/remote/fileshare/fileshare.go b/pkg/remote/fileshare/fileshare.go index c155f394eb..a31e033b86 100644 --- a/pkg/remote/fileshare/fileshare.go +++ b/pkg/remote/fileshare/fileshare.go @@ -119,7 +119,11 @@ func Mkdir(ctx context.Context, path string) error { } func Move(ctx context.Context, data wshrpc.CommandFileCopyData) error { - log.Printf("Move: %v", data) + opts := data.Opts + if opts == nil { + opts = &wshrpc.FileCopyOpts{} + } + log.Printf("Move: srcuri: %v, desturi: %v, opts: %v", data.SrcUri, data.DestUri, opts) srcClient, srcConn := CreateFileShareClient(ctx, data.SrcUri) if srcConn == nil || srcClient == nil { return fmt.Errorf("error creating fileshare client, could not parse source connection %s", data.SrcUri) @@ -129,24 +133,23 @@ func Move(ctx context.Context, data wshrpc.CommandFileCopyData) error { return fmt.Errorf("error creating fileshare client, could not parse destination connection %s", data.DestUri) } if srcConn.Host != destConn.Host { - err := destClient.CopyRemote(ctx, srcConn, destConn, srcClient, data.Opts) + isDir, err := destClient.CopyRemote(ctx, srcConn, destConn, srcClient, opts) if err != nil { return fmt.Errorf("cannot copy %q to %q: %w", data.SrcUri, data.DestUri, err) } - recursive := data.Opts != nil && data.Opts.Recursive - return srcClient.Delete(ctx, srcConn, recursive) + return srcClient.Delete(ctx, srcConn, opts.Recursive && isDir) } else { - return srcClient.MoveInternal(ctx, srcConn, destConn, data.Opts) + return srcClient.MoveInternal(ctx, srcConn, destConn, opts) } } func Copy(ctx context.Context, data wshrpc.CommandFileCopyData) error { - log.Printf("Copy: %v", data) opts := data.Opts if opts == nil { opts = &wshrpc.FileCopyOpts{} } opts.Recursive = true + log.Printf("Copy: srcuri: %v, desturi: %v, opts: %v", data.SrcUri, data.DestUri, opts) srcClient, srcConn := CreateFileShareClient(ctx, data.SrcUri) if srcConn == nil || srcClient == nil { return fmt.Errorf("error creating fileshare client, could not parse source connection %s", data.SrcUri) @@ -156,9 +159,11 @@ func Copy(ctx context.Context, data wshrpc.CommandFileCopyData) error { return fmt.Errorf("error creating fileshare client, could not parse destination connection %s", data.DestUri) } if srcConn.Host != destConn.Host { - return destClient.CopyRemote(ctx, srcConn, destConn, srcClient, opts) + _, err := destClient.CopyRemote(ctx, srcConn, destConn, srcClient, opts) + return err } else { - return srcClient.CopyInternal(ctx, srcConn, destConn, opts) + _, err := srcClient.CopyInternal(ctx, srcConn, destConn, opts) + return err } } diff --git a/pkg/remote/fileshare/fstype/fstype.go b/pkg/remote/fileshare/fstype/fstype.go index 346dbdb6d4..2edc424bcd 100644 --- a/pkg/remote/fileshare/fstype/fstype.go +++ b/pkg/remote/fileshare/fstype/fstype.go @@ -44,9 +44,9 @@ type FileShareClient interface { // Move moves the file within the same connection MoveInternal(ctx context.Context, srcConn, destConn *connparse.Connection, opts *wshrpc.FileCopyOpts) error // Copy copies the file within the same connection. Returns whether the copy source was a directory - CopyInternal(ctx context.Context, srcConn, destConn *connparse.Connection, opts *wshrpc.FileCopyOpts) error + CopyInternal(ctx context.Context, srcConn, destConn *connparse.Connection, opts *wshrpc.FileCopyOpts) (bool, error) // CopyRemote copies the file between different connections. Returns whether the copy source was a directory - CopyRemote(ctx context.Context, srcConn, destConn *connparse.Connection, srcClient FileShareClient, opts *wshrpc.FileCopyOpts) error + CopyRemote(ctx context.Context, srcConn, destConn *connparse.Connection, srcClient FileShareClient, opts *wshrpc.FileCopyOpts) (bool, error) // Delete deletes the entry at the given path Delete(ctx context.Context, conn *connparse.Connection, recursive bool) error // Join joins the given parts to the connection path diff --git a/pkg/remote/fileshare/fsutil/fsutil.go b/pkg/remote/fileshare/fsutil/fsutil.go index 74aa689ff6..9cee220a8a 100644 --- a/pkg/remote/fileshare/fsutil/fsutil.go +++ b/pkg/remote/fileshare/fsutil/fsutil.go @@ -43,17 +43,17 @@ func GetParentPathString(hostAndPath string) string { return hostAndPath[:lastSlash+1] } -func PrefixCopyInternal(ctx context.Context, srcConn, destConn *connparse.Connection, c fstype.FileShareClient, opts *wshrpc.FileCopyOpts, listEntriesPrefix func(ctx context.Context, host string, path string) ([]string, error), copyFunc func(ctx context.Context, host string, path string) error) error { +func PrefixCopyInternal(ctx context.Context, srcConn, destConn *connparse.Connection, c fstype.FileShareClient, opts *wshrpc.FileCopyOpts, listEntriesPrefix func(ctx context.Context, host string, path string) ([]string, error), copyFunc func(ctx context.Context, host string, path string) error) (bool, error) { log.Printf("PrefixCopyInternal: %v -> %v", srcConn.GetFullURI(), destConn.GetFullURI()) srcHasSlash := strings.HasSuffix(srcConn.Path, fspath.Separator) srcPath, destPath, srcInfo, err := DetermineCopyDestPath(ctx, srcConn, destConn, c, c, opts) if err != nil { - return err + return false, err } recursive := opts != nil && opts.Recursive if srcInfo.IsDir { if !recursive { - return fmt.Errorf(fstype.RecursiveCopyError) + return false, fmt.Errorf(fstype.RecursiveCopyError) } if !srcHasSlash { srcPath += fspath.Separator @@ -62,7 +62,7 @@ func PrefixCopyInternal(ctx context.Context, srcConn, destConn *connparse.Connec log.Printf("Copying directory: %v -> %v", srcPath, destPath) entries, err := listEntriesPrefix(ctx, srcConn.Host, srcPath) if err != nil { - return fmt.Errorf("error listing source directory: %w", err) + return false, fmt.Errorf("error listing source directory: %w", err) } tree := pathtree.NewTree(srcPath, fspath.Separator) @@ -70,14 +70,14 @@ func PrefixCopyInternal(ctx context.Context, srcConn, destConn *connparse.Connec tree.Add(entry) } - /* tree.Walk will return the full path in the source bucket for each item. + /* tree.Walk will return false, the full path in the source bucket for each item. prefixToRemove specifies how much of that path we want in the destination subtree. If the source path has a trailing slash, we don't want to include the source directory itself in the destination subtree.*/ prefixToRemove := srcPath if !srcHasSlash { prefixToRemove = fspath.Dir(srcPath) + fspath.Separator } - return tree.Walk(func(path string, numChildren int) error { + return true, tree.Walk(func(path string, numChildren int) error { // since this is a prefix filesystem, we only care about leafs if numChildren > 0 { return nil @@ -86,21 +86,21 @@ func PrefixCopyInternal(ctx context.Context, srcConn, destConn *connparse.Connec return copyFunc(ctx, path, destFilePath) }) } else { - return copyFunc(ctx, srcPath, destPath) + return false, copyFunc(ctx, srcPath, destPath) } } -func PrefixCopyRemote(ctx context.Context, srcConn, destConn *connparse.Connection, srcClient, destClient fstype.FileShareClient, destPutFile func(host string, path string, size int64, reader io.Reader) error, opts *wshrpc.FileCopyOpts) error { +func PrefixCopyRemote(ctx context.Context, srcConn, destConn *connparse.Connection, srcClient, destClient fstype.FileShareClient, destPutFile func(host string, path string, size int64, reader io.Reader) error, opts *wshrpc.FileCopyOpts) (bool, error) { // prefix to be used if the destination is a directory. The destPath returned in the following call only applies if the destination is not a directory. destPathPrefix, err := CleanPathPrefix(destConn.Path) if err != nil { - return fmt.Errorf("error cleaning destination path: %w", err) + return false, fmt.Errorf("error cleaning destination path: %w", err) } destPathPrefix += fspath.Separator _, destPath, srcInfo, err := DetermineCopyDestPath(ctx, srcConn, destConn, srcClient, destClient, opts) if err != nil { - return err + return false, err } log.Printf("Copying: %v -> %v", srcConn.GetFullURI(), destConn.GetFullURI()) @@ -126,14 +126,15 @@ func PrefixCopyRemote(ctx context.Context, srcConn, destConn *connparse.Connecti }) if err != nil { cancel(err) - return err + return false, err } - return nil + return srcInfo.IsDir, nil } func DetermineCopyDestPath(ctx context.Context, srcConn, destConn *connparse.Connection, srcClient, destClient fstype.FileShareClient, opts *wshrpc.FileCopyOpts) (srcPath, destPath string, srcInfo *wshrpc.FileInfo, err error) { merge := opts != nil && opts.Merge overwrite := opts != nil && opts.Overwrite + recursive := opts != nil && opts.Recursive if overwrite && merge { return "", "", nil, fmt.Errorf("cannot specify both overwrite and merge") } @@ -171,12 +172,15 @@ func DetermineCopyDestPath(ctx context.Context, srcConn, destConn *connparse.Con } if destExists { if overwrite { - err = destClient.Delete(ctx, destConn, destInfo.IsDir) + log.Printf("Deleting existing file: %s\n", destConn.GetFullURI()) + err = destClient.Delete(ctx, destConn, destInfo.IsDir && recursive) if err != nil { return "", "", nil, fmt.Errorf("error deleting conflicting destination file: %w", err) } - } else if destInfo.IsDir && srcInfo.IsDir && !merge { - return "", "", nil, fmt.Errorf(fstype.MergeCopyError, destConn.GetFullURI()) + } else if destInfo.IsDir && srcInfo.IsDir { + if !merge { + return "", "", nil, fmt.Errorf(fstype.MergeCopyError, destConn.GetFullURI()) + } } else { return "", "", nil, fmt.Errorf(fstype.OverwriteCopyError, destConn.GetFullURI()) } diff --git a/pkg/remote/fileshare/pathtree/pathtree.go b/pkg/remote/fileshare/pathtree/pathtree.go index f4dbb92f05..3fa9735edc 100644 --- a/pkg/remote/fileshare/pathtree/pathtree.go +++ b/pkg/remote/fileshare/pathtree/pathtree.go @@ -32,10 +32,9 @@ func (n *Node) Walk(curPath string, walkFunc WalkFunc, delimiter string) error { func NewTree(path string, delimiter string) *Tree { if len(delimiter) > 1 { - log.Printf("Warning: multi-character delimiter '%s' may cause unexpected behavior", delimiter) + log.Printf("pathtree.NewTree: Warning: multi-character delimiter '%s' may cause unexpected behavior", delimiter) } if path != "" && !strings.HasSuffix(path, delimiter) { - log.Printf("Warning: path '%s' does not end with delimiter '%s'", path, delimiter) path += delimiter } return &Tree{ @@ -49,7 +48,6 @@ func NewTree(path string, delimiter string) *Tree { } func (t *Tree) Add(path string) { - log.Printf("tree.Add: path: %s", path) // Validate input if path == "" { return @@ -61,7 +59,6 @@ func (t *Tree) Add(path string) { relativePath = strings.TrimPrefix(path, t.RootPath) // If the path is not a child of the root path, ignore it - log.Printf("relativePath: %s", relativePath) if relativePath == path { return } @@ -70,7 +67,6 @@ func (t *Tree) Add(path string) { // If the path is already in the tree, ignore it if t.nodes[relativePath] != nil { - log.Printf("path already in tree: %s", relativePath) return } @@ -78,18 +74,16 @@ func (t *Tree) Add(path string) { // Validate path components for _, component := range components { if component == "" || component == "." || component == ".." { - log.Printf("invalid path component: %s", component) + log.Printf("pathtree.Add: invalid path component: %s", component) return // Skip invalid paths } } // Quick check to see if the parent path is already in the tree, in which case we can skip the loop if parent := t.tryAddToExistingParent(components); parent { - log.Printf("added to existing parent: %s", strings.Join(components, t.delimiter)) return } - log.Printf("adding new path: %s", strings.Join(components, t.delimiter)) t.addNewPath(components) } @@ -124,7 +118,6 @@ func (t *Tree) addNewPath(components []string) { } func (t *Tree) Walk(walkFunc WalkFunc) error { - log.Printf("RootPath: %s", t.RootPath) for key, child := range t.Root.Children { if err := child.Walk(t.RootPath+key, walkFunc, t.delimiter); err != nil { return err diff --git a/pkg/remote/fileshare/s3fs/s3fs.go b/pkg/remote/fileshare/s3fs/s3fs.go index 609c339dba..4db014722b 100644 --- a/pkg/remote/fileshare/s3fs/s3fs.go +++ b/pkg/remote/fileshare/s3fs/s3fs.go @@ -645,21 +645,21 @@ func (c S3Client) Mkdir(ctx context.Context, conn *connparse.Connection) error { } func (c S3Client) MoveInternal(ctx context.Context, srcConn, destConn *connparse.Connection, opts *wshrpc.FileCopyOpts) error { - err := c.CopyInternal(ctx, srcConn, destConn, opts) + isDir, err := c.CopyInternal(ctx, srcConn, destConn, opts) if err != nil { return err } recursive := opts != nil && opts.Recursive - return c.Delete(ctx, srcConn, recursive) + return c.Delete(ctx, srcConn, recursive && isDir) } -func (c S3Client) CopyRemote(ctx context.Context, srcConn, destConn *connparse.Connection, srcClient fstype.FileShareClient, opts *wshrpc.FileCopyOpts) error { +func (c S3Client) CopyRemote(ctx context.Context, srcConn, destConn *connparse.Connection, srcClient fstype.FileShareClient, opts *wshrpc.FileCopyOpts) (bool, error) { if srcConn.Scheme == connparse.ConnectionTypeS3 && destConn.Scheme == connparse.ConnectionTypeS3 { return c.CopyInternal(ctx, srcConn, destConn, opts) } destBucket := destConn.Host if destBucket == "" || destBucket == fspath.Separator { - return fmt.Errorf("destination bucket must be specified") + return false, fmt.Errorf("destination bucket must be specified") } return fsutil.PrefixCopyRemote(ctx, srcConn, destConn, srcClient, c, func(bucket, path string, size int64, reader io.Reader) error { _, err := c.client.PutObject(ctx, &s3.PutObjectInput{ @@ -672,11 +672,11 @@ func (c S3Client) CopyRemote(ctx context.Context, srcConn, destConn *connparse.C }, opts) } -func (c S3Client) CopyInternal(ctx context.Context, srcConn, destConn *connparse.Connection, opts *wshrpc.FileCopyOpts) error { +func (c S3Client) CopyInternal(ctx context.Context, srcConn, destConn *connparse.Connection, opts *wshrpc.FileCopyOpts) (bool, error) { srcBucket := srcConn.Host destBucket := destConn.Host if srcBucket == "" || srcBucket == fspath.Separator || destBucket == "" || destBucket == fspath.Separator { - return fmt.Errorf("source and destination bucket must be specified") + return false, fmt.Errorf("source and destination bucket must be specified") } return fsutil.PrefixCopyInternal(ctx, srcConn, destConn, c, opts, func(ctx context.Context, bucket, prefix string) ([]string, error) { var entries []string diff --git a/pkg/remote/fileshare/wavefs/wavefs.go b/pkg/remote/fileshare/wavefs/wavefs.go index 46aeedcad2..cbd672a1d9 100644 --- a/pkg/remote/fileshare/wavefs/wavefs.go +++ b/pkg/remote/fileshare/wavefs/wavefs.go @@ -422,17 +422,18 @@ func (c WaveClient) MoveInternal(ctx context.Context, srcConn, destConn *connpar if srcConn.Host != destConn.Host { return fmt.Errorf("move internal, src and dest hosts do not match") } - if err := c.CopyInternal(ctx, srcConn, destConn, opts); err != nil { + isDir, err := c.CopyInternal(ctx, srcConn, destConn, opts) + if err != nil { return fmt.Errorf("error copying blockfile: %w", err) } - recursive := opts != nil && opts.Recursive + recursive := opts != nil && opts.Recursive && isDir if err := c.Delete(ctx, srcConn, recursive); err != nil { return fmt.Errorf("error deleting blockfile: %w", err) } return nil } -func (c WaveClient) CopyInternal(ctx context.Context, srcConn, destConn *connparse.Connection, opts *wshrpc.FileCopyOpts) error { +func (c WaveClient) CopyInternal(ctx context.Context, srcConn, destConn *connparse.Connection, opts *wshrpc.FileCopyOpts) (bool, error) { return fsutil.PrefixCopyInternal(ctx, srcConn, destConn, c, opts, func(ctx context.Context, zoneId, prefix string) ([]string, error) { entryList := make([]string, 0) if err := listFilesPrefix(ctx, zoneId, prefix, func(wf *filestore.WaveFile) error { @@ -467,13 +468,13 @@ func (c WaveClient) CopyInternal(ctx context.Context, srcConn, destConn *connpar }) } -func (c WaveClient) CopyRemote(ctx context.Context, srcConn, destConn *connparse.Connection, srcClient fstype.FileShareClient, opts *wshrpc.FileCopyOpts) error { +func (c WaveClient) CopyRemote(ctx context.Context, srcConn, destConn *connparse.Connection, srcClient fstype.FileShareClient, opts *wshrpc.FileCopyOpts) (bool, error) { if srcConn.Scheme == connparse.ConnectionTypeWave && destConn.Scheme == connparse.ConnectionTypeWave { return c.CopyInternal(ctx, srcConn, destConn, opts) } zoneId := destConn.Host if zoneId == "" { - return fmt.Errorf("zoneid not found in connection") + return false, fmt.Errorf("zoneid not found in connection") } return fsutil.PrefixCopyRemote(ctx, srcConn, destConn, srcClient, c, func(zoneId, path string, size int64, reader io.Reader) error { dataBuf := make([]byte, size) diff --git a/pkg/remote/fileshare/wshfs/wshfs.go b/pkg/remote/fileshare/wshfs/wshfs.go index ae0930e864..41dfdd15f5 100644 --- a/pkg/remote/fileshare/wshfs/wshfs.go +++ b/pkg/remote/fileshare/wshfs/wshfs.go @@ -114,11 +114,11 @@ func (c WshClient) MoveInternal(ctx context.Context, srcConn, destConn *connpars return wshclient.RemoteFileMoveCommand(RpcClient, wshrpc.CommandFileCopyData{SrcUri: srcConn.GetFullURI(), DestUri: destConn.GetFullURI(), Opts: opts}, &wshrpc.RpcOpts{Route: wshutil.MakeConnectionRouteId(destConn.Host), Timeout: timeout}) } -func (c WshClient) CopyRemote(ctx context.Context, srcConn, destConn *connparse.Connection, _ fstype.FileShareClient, opts *wshrpc.FileCopyOpts) error { +func (c WshClient) CopyRemote(ctx context.Context, srcConn, destConn *connparse.Connection, _ fstype.FileShareClient, opts *wshrpc.FileCopyOpts) (bool, error) { return c.CopyInternal(ctx, srcConn, destConn, opts) } -func (c WshClient) CopyInternal(ctx context.Context, srcConn, destConn *connparse.Connection, opts *wshrpc.FileCopyOpts) error { +func (c WshClient) CopyInternal(ctx context.Context, srcConn, destConn *connparse.Connection, opts *wshrpc.FileCopyOpts) (bool, error) { if opts == nil { opts = &wshrpc.FileCopyOpts{} } diff --git a/pkg/util/tarcopy/tarcopy.go b/pkg/util/tarcopy/tarcopy.go index d8888719de..c2858c7917 100644 --- a/pkg/util/tarcopy/tarcopy.go +++ b/pkg/util/tarcopy/tarcopy.go @@ -73,8 +73,6 @@ func TarCopySrc(ctx context.Context, pathPrefix string) (outputChan chan wshrpc. } header.Name = path - log.Printf("TarCopySrc: header name: %v\n", header.Name) - // write header if err := tarWriter.WriteHeader(header); err != nil { return err diff --git a/pkg/wshrpc/wshclient/wshclient.go b/pkg/wshrpc/wshclient/wshclient.go index 1ded90d83c..243065b9cc 100644 --- a/pkg/wshrpc/wshclient/wshclient.go +++ b/pkg/wshrpc/wshclient/wshclient.go @@ -356,9 +356,9 @@ func RecordTEventCommand(w *wshutil.WshRpc, data telemetrydata.TEvent, opts *wsh } // command "remotefilecopy", wshserver.RemoteFileCopyCommand -func RemoteFileCopyCommand(w *wshutil.WshRpc, data wshrpc.CommandFileCopyData, opts *wshrpc.RpcOpts) error { - _, err := sendRpcRequestCallHelper[any](w, "remotefilecopy", data, opts) - return err +func RemoteFileCopyCommand(w *wshutil.WshRpc, data wshrpc.CommandFileCopyData, opts *wshrpc.RpcOpts) (bool, error) { + resp, err := sendRpcRequestCallHelper[bool](w, "remotefilecopy", data, opts) + return resp, err } // command "remotefiledelete", wshserver.RemoteFileDeleteCommand diff --git a/pkg/wshrpc/wshremote/wshremote.go b/pkg/wshrpc/wshremote/wshremote.go index 1a87f76ee9..d5546a37b8 100644 --- a/pkg/wshrpc/wshremote/wshremote.go +++ b/pkg/wshrpc/wshremote/wshremote.go @@ -306,7 +306,7 @@ func (impl *ServerImpl) RemoteTarStreamCommand(ctx context.Context, data wshrpc. return rtn } -func (impl *ServerImpl) RemoteFileCopyCommand(ctx context.Context, data wshrpc.CommandFileCopyData) error { +func (impl *ServerImpl) RemoteFileCopyCommand(ctx context.Context, data wshrpc.CommandFileCopyData) (bool, error) { log.Printf("RemoteFileCopyCommand: src=%s, dest=%s\n", data.SrcUri, data.DestUri) opts := data.Opts if opts == nil { @@ -317,18 +317,18 @@ func (impl *ServerImpl) RemoteFileCopyCommand(ctx context.Context, data wshrpc.C merge := opts.Merge overwrite := opts.Overwrite if overwrite && merge { - return fmt.Errorf("cannot specify both overwrite and merge") + return false, fmt.Errorf("cannot specify both overwrite and merge") } destConn, err := connparse.ParseURIAndReplaceCurrentHost(ctx, destUri) if err != nil { - return fmt.Errorf("cannot parse destination URI %q: %w", srcUri, err) + return false, fmt.Errorf("cannot parse destination URI %q: %w", srcUri, err) } destPathCleaned := filepath.Clean(wavebase.ExpandHomeDirSafe(destConn.Path)) destinfo, err := os.Stat(destPathCleaned) if err != nil { if !errors.Is(err, fs.ErrNotExist) { - return fmt.Errorf("cannot stat destination %q: %w", destPathCleaned, err) + return false, fmt.Errorf("cannot stat destination %q: %w", destPathCleaned, err) } } @@ -338,17 +338,17 @@ func (impl *ServerImpl) RemoteFileCopyCommand(ctx context.Context, data wshrpc.C if destExists && !destIsDir { if !overwrite { - return fmt.Errorf(fstype.OverwriteCopyError, destPathCleaned) + return false, fmt.Errorf(fstype.OverwriteCopyError, destPathCleaned) } else { err := os.Remove(destPathCleaned) if err != nil { - return fmt.Errorf("cannot remove file %q: %w", destPathCleaned, err) + return false, fmt.Errorf("cannot remove file %q: %w", destPathCleaned, err) } } } srcConn, err := connparse.ParseURIAndReplaceCurrentHost(ctx, srcUri) if err != nil { - return fmt.Errorf("cannot parse source URI %q: %w", srcUri, err) + return false, fmt.Errorf("cannot parse source URI %q: %w", srcUri, err) } copyFileFunc := func(path string, finfo fs.FileInfo, srcFile io.Reader) (int64, error) { @@ -416,15 +416,17 @@ func (impl *ServerImpl) RemoteFileCopyCommand(ctx context.Context, data wshrpc.C return finfo.Size(), nil } + srcIsDir := false if srcConn.Host == destConn.Host { srcPathCleaned := filepath.Clean(wavebase.ExpandHomeDirSafe(srcConn.Path)) srcFileStat, err := os.Stat(srcPathCleaned) if err != nil { - return fmt.Errorf("cannot stat file %q: %w", srcPathCleaned, err) + return false, fmt.Errorf("cannot stat file %q: %w", srcPathCleaned, err) } if srcFileStat.IsDir() { + srcIsDir = true var srcPathPrefix string if destIsDir { srcPathPrefix = filepath.Dir(srcPathCleaned) @@ -449,12 +451,12 @@ func (impl *ServerImpl) RemoteFileCopyCommand(ctx context.Context, data wshrpc.C return err }) if err != nil { - return fmt.Errorf("cannot copy %q to %q: %w", srcUri, destUri, err) + return false, fmt.Errorf("cannot copy %q to %q: %w", srcUri, destUri, err) } } else { file, err := os.Open(srcPathCleaned) if err != nil { - return fmt.Errorf("cannot open file %q: %w", srcPathCleaned, err) + return false, fmt.Errorf("cannot open file %q: %w", srcPathCleaned, err) } defer utilfn.GracefulClose(file, "RemoteFileCopyCommand", srcPathCleaned) var destFilePath string @@ -465,7 +467,7 @@ func (impl *ServerImpl) RemoteFileCopyCommand(ctx context.Context, data wshrpc.C } _, err = copyFileFunc(destFilePath, srcFileStat, file) if err != nil { - return fmt.Errorf("cannot copy %q to %q: %w", srcUri, destUri, err) + return false, fmt.Errorf("cannot copy %q to %q: %w", srcUri, destUri, err) } } } else { @@ -486,6 +488,7 @@ func (impl *ServerImpl) RemoteFileCopyCommand(ctx context.Context, data wshrpc.C numFiles++ nextpath := filepath.Join(destPathCleaned, next.Name) log.Printf("RemoteFileCopyCommand: copying %q to %q\n", next.Name, nextpath) + srcIsDir = !singleFile if singleFile && !destHasSlash { // custom flag to indicate that the source is a single file, not a directory the contents of a directory nextpath = destPathCleaned @@ -499,7 +502,7 @@ func (impl *ServerImpl) RemoteFileCopyCommand(ctx context.Context, data wshrpc.C return nil }) if err != nil { - return fmt.Errorf("cannot copy %q to %q: %w", srcUri, destUri, err) + return false, fmt.Errorf("cannot copy %q to %q: %w", srcUri, destUri, err) } totalTime := time.Since(copyStart).Seconds() totalMegaBytes := float64(totalBytes) / 1024 / 1024 @@ -509,7 +512,7 @@ func (impl *ServerImpl) RemoteFileCopyCommand(ctx context.Context, data wshrpc.C } log.Printf("RemoteFileCopyCommand: done; %d files copied in %.3fs, total of %.4f MB, %.2f MB/s, %d files skipped\n", numFiles, totalTime, totalMegaBytes, rate, numSkipped) } - return nil + return srcIsDir, nil } func (impl *ServerImpl) RemoteListEntriesCommand(ctx context.Context, data wshrpc.CommandRemoteListEntriesData) chan wshrpc.RespOrErrorUnion[wshrpc.CommandRemoteListEntriesRtnData] { diff --git a/pkg/wshrpc/wshrpctypes.go b/pkg/wshrpc/wshrpctypes.go index 50a2460fdd..3d629533cf 100644 --- a/pkg/wshrpc/wshrpctypes.go +++ b/pkg/wshrpc/wshrpctypes.go @@ -228,7 +228,7 @@ type WshRpcInterface interface { // remotes RemoteStreamFileCommand(ctx context.Context, data CommandRemoteStreamFileData) chan RespOrErrorUnion[FileData] RemoteTarStreamCommand(ctx context.Context, data CommandRemoteStreamTarData) <-chan RespOrErrorUnion[iochantypes.Packet] - RemoteFileCopyCommand(ctx context.Context, data CommandFileCopyData) error + RemoteFileCopyCommand(ctx context.Context, data CommandFileCopyData) (bool, error) RemoteListEntriesCommand(ctx context.Context, data CommandRemoteListEntriesData) chan RespOrErrorUnion[CommandRemoteListEntriesRtnData] RemoteFileInfoCommand(ctx context.Context, path string) (*FileInfo, error) RemoteFileTouchCommand(ctx context.Context, path string) error From 000477116e8cc89e48a6a8804d488048889a98cf Mon Sep 17 00:00:00 2001 From: Evan Simkowitz Date: Wed, 19 Feb 2025 17:04:44 -0800 Subject: [PATCH 11/16] remove logs --- pkg/wshrpc/wshremote/wshremote.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/pkg/wshrpc/wshremote/wshremote.go b/pkg/wshrpc/wshremote/wshremote.go index d5546a37b8..7b42f04496 100644 --- a/pkg/wshrpc/wshremote/wshremote.go +++ b/pkg/wshrpc/wshremote/wshremote.go @@ -322,7 +322,7 @@ func (impl *ServerImpl) RemoteFileCopyCommand(ctx context.Context, data wshrpc.C destConn, err := connparse.ParseURIAndReplaceCurrentHost(ctx, destUri) if err != nil { - return false, fmt.Errorf("cannot parse destination URI %q: %w", srcUri, err) + return false, fmt.Errorf("cannot parse destination URI %q: %w", destUri, err) } destPathCleaned := filepath.Clean(wavebase.ExpandHomeDirSafe(destConn.Path)) destinfo, err := os.Stat(destPathCleaned) @@ -390,7 +390,6 @@ func (impl *ServerImpl) RemoteFileCopyCommand(ctx context.Context, data wshrpc.C } if finfo.IsDir() { - log.Printf("RemoteFileCopyCommand: making dirs %s\n", path) err := os.MkdirAll(path, finfo.Mode()) if err != nil { return 0, fmt.Errorf("cannot create directory %q: %w", path, err) @@ -487,7 +486,6 @@ func (impl *ServerImpl) RemoteFileCopyCommand(ctx context.Context, data wshrpc.C err := tarcopy.TarCopyDest(readCtx, cancel, ioch, func(next *tar.Header, reader *tar.Reader, singleFile bool) error { numFiles++ nextpath := filepath.Join(destPathCleaned, next.Name) - log.Printf("RemoteFileCopyCommand: copying %q to %q\n", next.Name, nextpath) srcIsDir = !singleFile if singleFile && !destHasSlash { // custom flag to indicate that the source is a single file, not a directory the contents of a directory From b3096e2861ec8e2279499828397d403d1acd56ae Mon Sep 17 00:00:00 2001 From: Evan Simkowitz Date: Wed, 19 Feb 2025 17:42:07 -0800 Subject: [PATCH 12/16] add delete verify, banners for recursive rename and delete --- .../app/view/preview/directorypreview.tsx | 118 ++++++++++++------ pkg/remote/fileshare/fstype/fstype.go | 12 +- pkg/remote/fileshare/fsutil/fsutil.go | 6 +- pkg/remote/fileshare/s3fs/s3fs.go | 36 ++++-- pkg/wshrpc/wshremote/wshremote.go | 10 +- 5 files changed, 121 insertions(+), 61 deletions(-) diff --git a/frontend/app/view/preview/directorypreview.tsx b/frontend/app/view/preview/directorypreview.tsx index 76c9221d45..d0feefd75a 100644 --- a/frontend/app/view/preview/directorypreview.tsx +++ b/frontend/app/view/preview/directorypreview.tsx @@ -37,12 +37,9 @@ import "./directorypreview.scss"; const PageJumpSize = 20; -type FileCopyStatus = { - copyData: CommandFileCopyData; - copyError: string; - allowRetry: boolean; - isDir: boolean; -}; +const recursiveError = "recursive flag must be set for directory operations"; +const overwriteError = "set overwrite flag to delete the existing file"; +const mergeError = "set overwrite flag to delete the existing contents or set merge flag to merge the contents"; declare module "@tanstack/react-table" { interface TableMeta { @@ -214,6 +211,7 @@ function DirectoryTable({ newDirectory, }: DirectoryTableProps) { const fullConfig = useAtomValue(atoms.fullConfigAtom); + const setErrorMsg = useSetAtom(model.errorMsgAtom); const getIconFromMimeType = useCallback( (mimeType: string): string => { while (mimeType.length > 0) { @@ -314,11 +312,48 @@ function DirectoryTable({ }, }); } catch (e) { - const errorStatus: ErrorMsg = { - status: "Rename Failed", - text: `${e}`, - }; - globalStore.set(model.errorMsgAtom, errorStatus); + const errorText = `${e}`; + console.warn(`Rename failed: ${errorText}`); + let errorStatus: ErrorMsg; + if (errorText.includes(recursiveError)) { + errorStatus = { + status: "Confirm Rename Directory", + text: "Renaming a directory requires the recursive flag. Proceed?", + buttons: [ + { + text: "Rename Recursively", + onClick: async () => { + try { + let srcuri = await model.formatRemoteUri(path, globalStore.get); + if (isDir) { + srcuri += "/"; + } + await RpcApi.FileMoveCommand(TabRpcClient, { + srcuri, + desturi: await model.formatRemoteUri(newPath, globalStore.get), + opts: { + recursive: true, + }, + }); + } catch (e) { + const errorStatus: ErrorMsg = { + status: "Rename Failed", + text: `${e}`, + }; + globalStore.set(model.errorMsgAtom, errorStatus); + } + model.refreshCallback(); + }, + }, + ], + }; + } else { + errorStatus = { + status: "Rename Failed", + text: `${e}`, + }; + } + setErrorMsg(errorStatus); } model.refreshCallback(); }); @@ -497,6 +532,7 @@ function TableBody({ const warningBoxRef = useRef(); const rowRefs = useRef([]); const conn = useAtomValue(model.connection); + const setErrorMsg = useSetAtom(model.errorMsgAtom); useEffect(() => { if (focusIndex !== null && rowRefs.current[focusIndex] && bodyRef.current && osRef) { @@ -531,8 +567,40 @@ function TableBody({ if (finfo == null) { return; } - const normPath = finfo.path; const fileName = finfo.path.split("/").pop(); + const handleFileDelete = (recursive: boolean) => + fireAndForget(async () => { + const path = await model.formatRemoteUri(finfo.path, globalStore.get); + try { + await RpcApi.FileDeleteCommand(TabRpcClient, { + path, + recursive, + }); + } catch (e) { + const errorText = `${e}`; + console.warn(`Delete failed: ${errorText}`); + let errorStatus: ErrorMsg; + if (errorText.includes(recursiveError)) { + errorStatus = { + status: "Confirm Delete Directory", + text: "Deleting a directory requires the recursive flag. Proceed?", + buttons: [ + { + text: "Delete Recursively", + onClick: () => handleFileDelete(true), + }, + ], + }; + } else { + errorStatus = { + status: "Delete Failed", + text: `${e}`, + }; + } + setErrorMsg(errorStatus); + } + setRefreshVersion((current) => current + 1); + }); const menu: ContextMenuItem[] = [ { label: "New File", @@ -579,23 +647,7 @@ function TableBody({ }, { label: "Delete", - click: () => { - fireAndForget(async () => { - try { - await RpcApi.FileDeleteCommand(TabRpcClient, { - path: await model.formatRemoteUri(finfo.path, globalStore.get), - recursive: false, - }); - } catch (e) { - const errorStatus: ErrorMsg = { - status: "Delete Failed", - text: `${e}`, - }; - globalStore.set(model.errorMsgAtom, errorStatus); - } - setRefreshVersion((current) => current + 1); - }); - }, + click: () => handleFileDelete(false), } ); ContextMenuModel.showContextMenu(menu, e); @@ -860,13 +912,9 @@ function DirectoryPreview({ model }: DirectoryPreviewProps) { try { await RpcApi.FileCopyCommand(TabRpcClient, data, { timeout: data.opts.timeout }); } catch (e) { - console.warn("copy failed:", e); + console.warn("Copy failed:", e); const copyError = `${e}`; - const allowRetry = - copyError.includes("set overwrite flag to delete the existing file") || - copyError.includes( - "set overwrite flag to delete the existing contents or set merge flag to merge the contents" - ); + const allowRetry = copyError.includes(overwriteError) || copyError.includes(mergeError); let errorMsg: ErrorMsg; if (allowRetry) { errorMsg = { diff --git a/pkg/remote/fileshare/fstype/fstype.go b/pkg/remote/fileshare/fstype/fstype.go index 2edc424bcd..a99d1ccd87 100644 --- a/pkg/remote/fileshare/fstype/fstype.go +++ b/pkg/remote/fileshare/fstype/fstype.go @@ -14,12 +14,12 @@ import ( ) const ( - DefaultTimeout = 30 * time.Second - FileMode os.FileMode = 0644 - DirMode os.FileMode = 0755 | os.ModeDir - RecursiveCopyError = "recursive flag must be set for directory operations" - MergeCopyError = "directory already exists at %q, set overwrite flag to delete the existing contents or set merge flag to merge the contents" - OverwriteCopyError = "file already exists at %q, set overwrite flag to delete the existing file" + DefaultTimeout = 30 * time.Second + FileMode os.FileMode = 0644 + DirMode os.FileMode = 0755 | os.ModeDir + RecursiveRequiredError = "recursive flag must be set for directory operations" + MergeRequiredError = "directory already exists at %q, set overwrite flag to delete the existing contents or set merge flag to merge the contents" + OverwriteRequiredError = "file already exists at %q, set overwrite flag to delete the existing file" ) type FileShareClient interface { diff --git a/pkg/remote/fileshare/fsutil/fsutil.go b/pkg/remote/fileshare/fsutil/fsutil.go index 9cee220a8a..617bbdc706 100644 --- a/pkg/remote/fileshare/fsutil/fsutil.go +++ b/pkg/remote/fileshare/fsutil/fsutil.go @@ -53,7 +53,7 @@ func PrefixCopyInternal(ctx context.Context, srcConn, destConn *connparse.Connec recursive := opts != nil && opts.Recursive if srcInfo.IsDir { if !recursive { - return false, fmt.Errorf(fstype.RecursiveCopyError) + return false, fmt.Errorf(fstype.RecursiveRequiredError) } if !srcHasSlash { srcPath += fspath.Separator @@ -179,10 +179,10 @@ func DetermineCopyDestPath(ctx context.Context, srcConn, destConn *connparse.Con } } else if destInfo.IsDir && srcInfo.IsDir { if !merge { - return "", "", nil, fmt.Errorf(fstype.MergeCopyError, destConn.GetFullURI()) + return "", "", nil, fmt.Errorf(fstype.MergeRequiredError, destConn.GetFullURI()) } } else { - return "", "", nil, fmt.Errorf(fstype.OverwriteCopyError, destConn.GetFullURI()) + return "", "", nil, fmt.Errorf(fstype.OverwriteRequiredError, destConn.GetFullURI()) } } return srcPath, destPath, srcInfo, nil diff --git a/pkg/remote/fileshare/s3fs/s3fs.go b/pkg/remote/fileshare/s3fs/s3fs.go index 4db014722b..774f761baf 100644 --- a/pkg/remote/fileshare/s3fs/s3fs.go +++ b/pkg/remote/fileshare/s3fs/s3fs.go @@ -189,7 +189,7 @@ func (c S3Client) ReadTarStream(ctx context.Context, conn *connparse.Connection, singleFile := singleFileResult != nil if !singleFile && !recursive { - return wshutil.SendErrCh[iochantypes.Packet](fmt.Errorf(fstype.RecursiveCopyError)) + return wshutil.SendErrCh[iochantypes.Packet](fmt.Errorf(fstype.RecursiveRequiredError)) } // whether to include the directory itself in the tar @@ -708,17 +708,17 @@ func (c S3Client) Delete(ctx context.Context, conn *connparse.Connection, recurs if objectKey == "" || objectKey == fspath.Separator { return errors.Join(errors.ErrUnsupported, fmt.Errorf("object key must be specified")) } + var err error if recursive { log.Printf("Deleting objects with prefix %v:%v", bucket, objectKey) if !strings.HasSuffix(objectKey, fspath.Separator) { objectKey = objectKey + fspath.Separator } objects := make([]types.ObjectIdentifier, 0) - err := c.listFilesPrefix(ctx, &s3.ListObjectsV2Input{ + err = c.listFilesPrefix(ctx, &s3.ListObjectsV2Input{ Bucket: aws.String(bucket), Prefix: aws.String(objectKey), }, func(obj *types.Object) (bool, error) { - log.Printf("Deleting object %v:%v", bucket, *obj.Key) objects = append(objects, types.ObjectIdentifier{Key: obj.Key}) return true, nil }) @@ -734,17 +734,29 @@ func (c S3Client) Delete(ctx context.Context, conn *connparse.Connection, recurs Objects: objects, }, }) - if err != nil { - log.Printf("Error deleting objects: %v", err) - } + } else { + log.Printf("Deleting object %v:%v", bucket, objectKey) + _, err = c.client.DeleteObject(ctx, &s3.DeleteObjectInput{ + Bucket: aws.String(bucket), + Key: aws.String(objectKey), + }) + } + if err != nil { return err } - log.Printf("Deleting object %v:%v", bucket, objectKey) - _, err := c.client.DeleteObject(ctx, &s3.DeleteObjectInput{ - Bucket: aws.String(bucket), - Key: aws.String(objectKey), - }) - return err + + // verify the object was deleted + finfo, err := c.Stat(ctx, conn) + if err != nil { + return err + } + if !finfo.NotFound { + if finfo.IsDir { + return fmt.Errorf(fstype.RecursiveRequiredError) + } + return fmt.Errorf("object was not successfully deleted %v:%v", bucket, objectKey) + } + return nil } func (c S3Client) listFilesPrefix(ctx context.Context, input *s3.ListObjectsV2Input, fileCallback func(*types.Object) (bool, error)) error { diff --git a/pkg/wshrpc/wshremote/wshremote.go b/pkg/wshrpc/wshremote/wshremote.go index 7b42f04496..fc81d0672c 100644 --- a/pkg/wshrpc/wshremote/wshremote.go +++ b/pkg/wshrpc/wshremote/wshremote.go @@ -338,7 +338,7 @@ func (impl *ServerImpl) RemoteFileCopyCommand(ctx context.Context, data wshrpc.C if destExists && !destIsDir { if !overwrite { - return false, fmt.Errorf(fstype.OverwriteCopyError, destPathCleaned) + return false, fmt.Errorf(fstype.OverwriteRequiredError, destPathCleaned) } else { err := os.Remove(destPathCleaned) if err != nil { @@ -367,7 +367,7 @@ func (impl *ServerImpl) RemoteFileCopyCommand(ctx context.Context, data wshrpc.C return 0, fmt.Errorf("cannot stat file %q: %w", path, err) } if newdestinfo != nil && !overwrite { - return 0, fmt.Errorf(fstype.OverwriteCopyError, path) + return 0, fmt.Errorf(fstype.OverwriteRequiredError, path) } } else if overwrite { err := os.RemoveAll(path) @@ -375,11 +375,11 @@ func (impl *ServerImpl) RemoteFileCopyCommand(ctx context.Context, data wshrpc.C return 0, fmt.Errorf("cannot remove directory %q: %w", path, err) } } else if !merge { - return 0, fmt.Errorf(fstype.MergeCopyError, path) + return 0, fmt.Errorf(fstype.MergeRequiredError, path) } } else { if !overwrite { - return 0, fmt.Errorf(fstype.OverwriteCopyError, path) + return 0, fmt.Errorf(fstype.OverwriteRequiredError, path) } else if finfo.IsDir() { err := os.RemoveAll(path) if err != nil { @@ -736,7 +736,7 @@ func (impl *ServerImpl) RemoteFileMoveCommand(ctx context.Context, data wshrpc.C return fmt.Errorf("cannot stat file %q: %w", srcPathCleaned, err) } if finfo.IsDir() && !recursive { - return fmt.Errorf(fstype.RecursiveCopyError) + return fmt.Errorf(fstype.RecursiveRequiredError) } err = os.Rename(srcPathCleaned, destPathCleaned) if err != nil { From 4ba90d9851e1275b7c174e53120edea2a1e1df9e Mon Sep 17 00:00:00 2001 From: Evan Simkowitz Date: Wed, 19 Feb 2025 17:43:01 -0800 Subject: [PATCH 13/16] remove log --- pkg/remote/fileshare/s3fs/s3fs.go | 1 - 1 file changed, 1 deletion(-) diff --git a/pkg/remote/fileshare/s3fs/s3fs.go b/pkg/remote/fileshare/s3fs/s3fs.go index 774f761baf..18a2ea815c 100644 --- a/pkg/remote/fileshare/s3fs/s3fs.go +++ b/pkg/remote/fileshare/s3fs/s3fs.go @@ -689,7 +689,6 @@ func (c S3Client) CopyInternal(ctx context.Context, srcConn, destConn *connparse }) return entries, err }, func(ctx context.Context, srcPath, destPath string) error { - log.Printf("Copying file %v -> %v", srcBucket+"/"+srcPath, destBucket+"/"+destPath) _, err := c.client.CopyObject(ctx, &s3.CopyObjectInput{ Bucket: aws.String(destBucket), Key: aws.String(destPath), From 67cf4be4a6e0fa4bab868cf9fa4bde25fd4d0818 Mon Sep 17 00:00:00 2001 From: Evan Simkowitz Date: Wed, 19 Feb 2025 17:45:03 -0800 Subject: [PATCH 14/16] fix recursive message wshremote --- pkg/wshrpc/wshremote/wshremote.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/wshrpc/wshremote/wshremote.go b/pkg/wshrpc/wshremote/wshremote.go index fc81d0672c..afa269fc97 100644 --- a/pkg/wshrpc/wshremote/wshremote.go +++ b/pkg/wshrpc/wshremote/wshremote.go @@ -839,7 +839,7 @@ func (*ServerImpl) RemoteFileDeleteCommand(ctx context.Context, data wshrpc.Comm finfo, _ := os.Stat(cleanedPath) if finfo != nil && finfo.IsDir() { if !data.Recursive { - return fmt.Errorf("cannot delete directory %q, recursive option not specified", data.Path) + return fmt.Errorf(fstype.RecursiveRequiredError) } err = os.RemoveAll(cleanedPath) if err != nil { From 1c4bead2a143ea82a619d4ecbd8406a507a0ab52 Mon Sep 17 00:00:00 2001 From: Evan Simkowitz Date: Wed, 19 Feb 2025 17:51:13 -0800 Subject: [PATCH 15/16] fix rename handling --- .../app/view/preview/directorypreview.tsx | 110 ++++++++---------- 1 file changed, 46 insertions(+), 64 deletions(-) diff --git a/frontend/app/view/preview/directorypreview.tsx b/frontend/app/view/preview/directorypreview.tsx index d0feefd75a..7ce987d06a 100644 --- a/frontend/app/view/preview/directorypreview.tsx +++ b/frontend/app/view/preview/directorypreview.tsx @@ -298,65 +298,47 @@ function DirectoryTable({ const lastInstance = path.lastIndexOf(fileName); newPath = path.substring(0, lastInstance) + newName; console.log(`replacing ${fileName} with ${newName}: ${path}`); - fireAndForget(async () => { - try { - let srcuri = await model.formatRemoteUri(path, globalStore.get); - if (isDir) { - srcuri += "/"; - } - await RpcApi.FileMoveCommand(TabRpcClient, { - srcuri, - desturi: await model.formatRemoteUri(newPath, globalStore.get), - opts: { - recursive: true, - }, - }); - } catch (e) { - const errorText = `${e}`; - console.warn(`Rename failed: ${errorText}`); - let errorStatus: ErrorMsg; - if (errorText.includes(recursiveError)) { - errorStatus = { - status: "Confirm Rename Directory", - text: "Renaming a directory requires the recursive flag. Proceed?", - buttons: [ - { - text: "Rename Recursively", - onClick: async () => { - try { - let srcuri = await model.formatRemoteUri(path, globalStore.get); - if (isDir) { - srcuri += "/"; - } - await RpcApi.FileMoveCommand(TabRpcClient, { - srcuri, - desturi: await model.formatRemoteUri(newPath, globalStore.get), - opts: { - recursive: true, - }, - }); - } catch (e) { - const errorStatus: ErrorMsg = { - status: "Rename Failed", - text: `${e}`, - }; - globalStore.set(model.errorMsgAtom, errorStatus); - } - model.refreshCallback(); + const handleRename = (recursive: boolean) => + fireAndForget(async () => { + try { + let srcuri = await model.formatRemoteUri(path, globalStore.get); + if (isDir) { + srcuri += "/"; + } + await RpcApi.FileMoveCommand(TabRpcClient, { + srcuri, + desturi: await model.formatRemoteUri(newPath, globalStore.get), + opts: { + recursive, + }, + }); + } catch (e) { + const errorText = `${e}`; + console.warn(`Rename failed: ${errorText}`); + let errorMsg: ErrorMsg; + if (errorText.includes(recursiveError)) { + errorMsg = { + status: "Confirm Rename Directory", + text: "Renaming a directory requires the recursive flag. Proceed?", + level: "warning", + buttons: [ + { + text: "Rename Recursively", + onClick: () => handleRename(true), }, - }, - ], - }; - } else { - errorStatus = { - status: "Rename Failed", - text: `${e}`, - }; + ], + }; + } else { + errorMsg = { + status: "Rename Failed", + text: `${e}`, + }; + } + setErrorMsg(errorMsg); } - setErrorMsg(errorStatus); - } - model.refreshCallback(); - }); + model.refreshCallback(); + }); + handleRename(false); } setEntryManagerProps(undefined); }, @@ -579,11 +561,12 @@ function TableBody({ } catch (e) { const errorText = `${e}`; console.warn(`Delete failed: ${errorText}`); - let errorStatus: ErrorMsg; + let errorMsg: ErrorMsg; if (errorText.includes(recursiveError)) { - errorStatus = { + errorMsg = { status: "Confirm Delete Directory", text: "Deleting a directory requires the recursive flag. Proceed?", + level: "warning", buttons: [ { text: "Delete Recursively", @@ -592,12 +575,12 @@ function TableBody({ ], }; } else { - errorStatus = { + errorMsg = { status: "Delete Failed", text: `${e}`, }; } - setErrorMsg(errorStatus); + setErrorMsg(errorMsg); } setRefreshVersion((current) => current + 1); }); @@ -808,11 +791,10 @@ function DirectoryPreview({ model }: DirectoryPreviewProps) { ); entries = file.entries ?? []; } catch (e) { - const errorStatus: ErrorMsg = { + setErrorMsg({ status: "Cannot Read Directory", text: `${e}`, - }; - globalStore.set(model.errorMsgAtom, errorStatus); + }); } setUnfilteredData(entries); }; From 3c8d8fe9c2b211a4222c795b635a2164ced80bec Mon Sep 17 00:00:00 2001 From: Evan Simkowitz Date: Wed, 19 Feb 2025 17:55:33 -0800 Subject: [PATCH 16/16] improve error msg --- pkg/remote/fileshare/s3fs/s3fs.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/pkg/remote/fileshare/s3fs/s3fs.go b/pkg/remote/fileshare/s3fs/s3fs.go index 18a2ea815c..f6d24ef81a 100644 --- a/pkg/remote/fileshare/s3fs/s3fs.go +++ b/pkg/remote/fileshare/s3fs/s3fs.go @@ -694,7 +694,10 @@ func (c S3Client) CopyInternal(ctx context.Context, srcConn, destConn *connparse Key: aws.String(destPath), CopySource: aws.String(fspath.Join(srcBucket, srcPath)), }) - return err + if err != nil { + return fmt.Errorf("error copying %v:%v to %v:%v: %w", srcBucket, srcPath, destBucket, destPath, err) + } + return nil }) }