Skip to content

Commit 4b7637f

Browse files
esimkowitzesimkowitz
committed
fix remote copy
1 parent 4ba9565 commit 4b7637f

File tree

4 files changed

+35
-32
lines changed

4 files changed

+35
-32
lines changed

‎frontend/app/view/preview/directorypreview.tsx

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ type FileCopyStatus = {
4848

4949
declare module "@tanstack/react-table" {
5050
interface TableMeta<TData extends RowData> {
51-
updateName: (path: string) => void;
51+
updateName: (path: string, isDir: boolean) => void;
5252
newFile: () => void;
5353
newDirectory: () => void;
5454
}
@@ -291,7 +291,7 @@ function DirectoryTable({
291291

292292
const setEntryManagerProps = useSetAtom(entryManagerOverlayPropsAtom);
293293

294-
const updateName = useCallback((path: string) => {
294+
const updateName = useCallback((path: string, isDir: boolean) => {
295295
const fileName = path.split("/").at(-1);
296296
setEntryManagerProps({
297297
entryManagerType: EntryManagerType.EditName,
@@ -304,8 +304,12 @@ function DirectoryTable({
304304
console.log(`replacing ${fileName} with ${newName}: ${path}`);
305305
fireAndForget(async () => {
306306
try {
307+
let srcuri = await model.formatRemoteUri(path, globalStore.get);
308+
if (isDir) {
309+
srcuri += "/";
310+
}
307311
await RpcApi.FileMoveCommand(TabRpcClient, {
308-
srcuri: await model.formatRemoteUri(path, globalStore.get),
312+
srcuri,
309313
desturi: await model.formatRemoteUri(newPath, globalStore.get),
310314
opts: {
311315
recursive: true,
@@ -547,7 +551,7 @@ function TableBody({
547551
{
548552
label: "Rename",
549553
click: () => {
550-
table.options.meta.updateName(finfo.path);
554+
table.options.meta.updateName(finfo.path, finfo.isdir);
551555
},
552556
},
553557
{
@@ -854,7 +858,7 @@ function DirectoryPreview({ model }: DirectoryPreviewProps) {
854858
});
855859

856860
const handleDropCopy = useCallback(
857-
async (data: CommandFileCopyData, isDir) => {
861+
async (data: CommandFileCopyData, isDir: boolean) => {
858862
try {
859863
await RpcApi.FileCopyCommand(TabRpcClient, data, { timeout: data.opts.timeout });
860864
setCopyStatus(null);

‎pkg/remote/fileshare/fsutil/fsutil.go

Lines changed: 15 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ func GetParentPath(conn *connparse.Connection) string {
2828

2929
func GetParentPathString(hostAndPath string) string {
3030
if hostAndPath == "" || hostAndPath == fspath.Separator {
31-
return fspath.Separator
31+
return ""
3232
}
3333

3434
// Remove trailing slash if present
@@ -38,24 +38,9 @@ func GetParentPathString(hostAndPath string) string {
3838

3939
lastSlash := strings.LastIndex(hostAndPath, fspath.Separator)
4040
if lastSlash <= 0 {
41-
return fspath.Separator
42-
}
43-
return hostAndPath[:lastSlash+1]
44-
}
45-
46-
const minURILength = 10 // Minimum length for a valid URI (e.g., "s3://bucket")
47-
48-
func GetPathPrefix(conn *connparse.Connection) string {
49-
fullUri := conn.GetFullURI()
50-
if fullUri == "" {
5141
return ""
5242
}
53-
pathPrefix := fullUri
54-
lastSlash := strings.LastIndex(fullUri, fspath.Separator)
55-
if lastSlash > minURILength && lastSlash < len(fullUri)-1 {
56-
pathPrefix = fullUri[:lastSlash+1]
57-
}
58-
return pathPrefix
43+
return hostAndPath[:lastSlash+1]
5944
}
6045

6146
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
10287
}
10388

10489
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) {
90+
// 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.
91+
destPathPrefix, err := CleanPathPrefix(destConn.Path)
92+
if err != nil {
93+
return false, fmt.Errorf("error cleaning destination path: %w", err)
94+
}
95+
destPathPrefix += fspath.Separator
96+
10597
_, destPath, srcInfo, err := DetermineCopyDestPath(ctx, srcConn, destConn, srcClient, destClient, opts)
10698
if err != nil {
107-
if !errors.Is(err, fs.ErrNotExist) {
108-
return false, err
109-
}
99+
return false, err
110100
}
101+
111102
log.Printf("Copying: %v -> %v", srcConn.GetFullURI(), destConn.GetFullURI())
112103
readCtx, cancel := context.WithCancelCause(ctx)
113104
defer cancel(nil)
@@ -119,7 +110,7 @@ func PrefixCopyRemote(ctx context.Context, srcConn, destConn *connparse.Connecti
119110
if singleFile && srcInfo.IsDir {
120111
return fmt.Errorf("protocol error: source is a directory, but only a single file is being copied")
121112
}
122-
fileName, err := CleanPathPrefix(fspath.Join(destPath, next.Name))
113+
fileName, err := CleanPathPrefix(fspath.Join(destPathPrefix, next.Name))
123114
if singleFile {
124115
fileName = destPath
125116
}
@@ -144,10 +135,7 @@ func DetermineCopyDestPath(ctx context.Context, srcConn, destConn *connparse.Con
144135
}
145136

146137
srcHasSlash := strings.HasSuffix(srcConn.Path, fspath.Separator)
147-
srcPath, err = CleanPathPrefix(srcConn.Path)
148-
if err != nil {
149-
return "", "", nil, fmt.Errorf("error cleaning source path: %w", err)
150-
}
138+
srcPath = srcConn.Path
151139
destHasSlash := strings.HasSuffix(destConn.Path, fspath.Separator)
152140
destPath, err = CleanPathPrefix(destConn.Path)
153141
if err != nil {
@@ -179,7 +167,7 @@ func DetermineCopyDestPath(ctx context.Context, srcConn, destConn *connparse.Con
179167
}
180168
if destExists {
181169
if overwrite {
182-
err = destClient.Delete(ctx, destConn, true)
170+
err = destClient.Delete(ctx, destConn, destInfo.IsDir)
183171
if err != nil {
184172
return "", "", nil, fmt.Errorf("error deleting conflicting destination file: %w", err)
185173
}
@@ -197,7 +185,7 @@ func DetermineCopyDestPath(ctx context.Context, srcConn, destConn *connparse.Con
197185
// CleanPathPrefix corrects paths for prefix filesystems (i.e. ones that don't have directories)
198186
func CleanPathPrefix(path string) (string, error) {
199187
if path == "" {
200-
return "", fmt.Errorf("path is empty")
188+
return "", nil
201189
}
202190
if strings.HasPrefix(path, fspath.Separator) {
203191
path = path[1:]

‎pkg/remote/fileshare/pathtree/pathtree.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ func NewTree(path string, delimiter string) *Tree {
3535
log.Printf("Warning: multi-character delimiter '%s' may cause unexpected behavior", delimiter)
3636
}
3737
if path != "" && !strings.HasSuffix(path, delimiter) {
38+
log.Printf("Warning: path '%s' does not end with delimiter '%s'", path, delimiter)
3839
path += delimiter
3940
}
4041
return &Tree{
@@ -60,6 +61,7 @@ func (t *Tree) Add(path string) {
6061
relativePath = strings.TrimPrefix(path, t.RootPath)
6162

6263
// If the path is not a child of the root path, ignore it
64+
log.Printf("relativePath: %s", relativePath)
6365
if relativePath == path {
6466
return
6567
}
@@ -68,22 +70,26 @@ func (t *Tree) Add(path string) {
6870

6971
// If the path is already in the tree, ignore it
7072
if t.nodes[relativePath] != nil {
73+
log.Printf("path already in tree: %s", relativePath)
7174
return
7275
}
7376

7477
components := strings.Split(relativePath, t.delimiter)
7578
// Validate path components
7679
for _, component := range components {
7780
if component == "" || component == "." || component == ".." {
81+
log.Printf("invalid path component: %s", component)
7882
return // Skip invalid paths
7983
}
8084
}
8185

8286
// Quick check to see if the parent path is already in the tree, in which case we can skip the loop
8387
if parent := t.tryAddToExistingParent(components); parent {
88+
log.Printf("added to existing parent: %s", strings.Join(components, t.delimiter))
8489
return
8590
}
8691

92+
log.Printf("adding new path: %s", strings.Join(components, t.delimiter))
8793
t.addNewPath(components)
8894
}
8995

‎pkg/remote/fileshare/s3fs/s3fs.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -207,6 +207,7 @@ func (c S3Client) ReadTarStream(ctx context.Context, conn *connparse.Connection,
207207
tarPathPrefix = bucket
208208
}
209209
} else if singleFile || includeDir {
210+
log.Printf("singleFile or includeDir; tarPathPrefix: %v", tarPathPrefix)
210211
// if we're including the directory itself, we need to remove the last part of the path
211212
tarPathPrefix = fsutil.GetParentPathString(tarPathPrefix)
212213
}
@@ -295,6 +296,7 @@ func (c S3Client) ReadTarStream(ctx context.Context, conn *connparse.Connection,
295296

296297
// Walk the tree and write the tar entries
297298
if err := tree.Walk(func(path string, numChildren int) error {
299+
log.Printf("writing %v", path)
298300
mapEntry, isFile := objMap[path]
299301

300302
// 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
738740
Objects: objects,
739741
},
740742
})
743+
if err != nil {
744+
log.Printf("Error deleting objects: %v", err)
745+
}
741746
return err
742747
}
743748
log.Printf("Deleting object %v:%v", bucket, objectKey)

0 commit comments

Comments
 (0)