mattcasters commented on code in PR #8876:
URL: https://github.com/apache/hop/pull/8876#discussion_r4249059642
##########
ui/src/main/java/org/apache/hop/ui/hopgui/vfs/explorer/VfsFileExplorerLocation.java:
##########
@@ -302,10 +359,171 @@ public VfsFileExplorerLocation(Composite parent,
VfsFileExplorer explorer) {
detailsToolbar.dispose();
bookmarksToolbar.dispose();
});
- refreshBookmarks();
+ refreshPlaces();
updateToolbar();
}
+ /**
+ * Drag folders from the tree and rows from the file list, and drop on a
folder. The paths travel
+ * as a {@link FileTransfer}, as in the File Explorer perspective, so files
dragged between the
+ * two or from the operating system are accepted too. A drop copies unless a
move is asked for:
+ * the source and target are often different stores.
+ */
+ private void installDragAndDrop() {
+ FileTransfer fileTransfer = FileTransfer.getInstance();
+ Transfer[] transfers = {fileTransfer};
+
+ DragSource treeSource = new DragSource(tree, DND.DROP_COPY |
DND.DROP_MOVE);
+ treeSource.setTransfer(transfers);
+ treeSource.addDragListener(dragListener(this::selectedFolderEntries));
+ DragSource tableSource = new DragSource(table, DND.DROP_COPY |
DND.DROP_MOVE);
+ tableSource.setTransfer(transfers);
+ tableSource.addDragListener(dragListener(this::selectedFileEntries));
+
+ DropTarget treeTarget = new DropTarget(tree, DND.DROP_COPY |
DND.DROP_MOVE);
+ treeTarget.setTransfer(transfers);
+ treeTarget.addDropListener(
+ dropListener(
+ event -> {
+ FolderNode node = event.item instanceof TreeItem item ?
nodeOf(item) : null;
+ return node == null ? null : node.uri;
+ }));
+ DropTarget tableTarget = new DropTarget(table, DND.DROP_COPY |
DND.DROP_MOVE);
+ tableTarget.setTransfer(transfers);
+ tableTarget.addDropListener(
+ dropListener(
+ event -> {
+ // A folder row is the target. Anywhere else in the list is the
folder being viewed.
+ if (event.item instanceof TableItem item && !item.isDisposed()) {
+ int index = table.indexOf(item);
+ if (index >= 0 && index < shownRows.size() &&
shownRows.get(index).isFolder()) {
+ return shownRows.get(index).getUri();
+ }
+ }
+ return currentFolderUri();
+ }));
+ }
+
+ private DragSourceAdapter dragListener(
+ java.util.function.Supplier<List<VfsFileTransfer.Entry>> selection) {
+ return new DragSourceAdapter() {
+ @Override
+ public void dragStart(DragSourceEvent event) {
+ List<String> uris = urisOf(selection.get());
+ event.doit = !uris.isEmpty();
+ draggedUris = uris;
+ if (EnvironmentUtils.getInstance().isWeb()) {
+ // RAP has no native drag image to fall back on.
+ event.image = GuiResource.getInstance().getImageFolder();
+ }
+ }
+
+ @Override
+ public void dragSetData(DragSourceEvent event) {
+ // Read the selection again rather than the paths kept at drag start:
Hop Web can finish
+ // the drag before it asks for the data, and dragFinished has cleared
them by then.
+ if (FileTransfer.getInstance().isSupportedType(event.dataType)) {
+ List<String> uris = urisOf(selection.get());
+ event.doit = !uris.isEmpty();
+ event.data = uris.toArray(new String[0]);
+ }
+ }
+
+ @Override
+ public void dragFinished(DragSourceEvent event) {
+ draggedUris = List.of();
+ }
+ };
+ }
+
+ private DropTargetAdapter dropListener(
+ java.util.function.Function<DropTargetEvent, String> targetOf) {
+ return new DropTargetAdapter() {
+ @Override
+ public void dragEnter(DropTargetEvent event) {
+ if (event.detail == DND.DROP_DEFAULT) {
Review Comment:
**[bug]** An unmodified drop moves the files and deletes the originals on
Linux and macOS, which contradicts the manual ("A drop copies the files: the
originals stay where they are") and the comment above `installDragAndDrop`.
`dragEnter` only rewrites `DND.DROP_DEFAULT` to copy. GTK and Cocoa turn that
default into `DROP_MOVE` before the listener runs whenever the drop target
style is `DROP_COPY | DROP_MOVE` and does not include `DROP_DEFAULT`
(`DropTarget.setEventData`). No modifier key is held, so `event.detail` is
already `DROP_MOVE` and this branch does nothing. `dragOver` keeps `DROP_MOVE`,
and `drop` then uses `VfsFileTransfer.Mode.MOVE`, which `moveTo`s or copies and
then deletes the source. That includes a drag from the operating-system file
manager into the explorer: the local files are removed after the VFS copy.
Ctrl+drag is a copy; Shift+drag is also a move, so there is no unmodified copy
gesture. Windows happens to work because it applies the MOVE default only after
the listener, and this listener still sees `DROP_DEFAULT`.
**Suggestion:** On `dragEnter` and `dragOver`, force `DROP_COPY` unless the
user is actually holding the platform move modifier. Do not treat "MOVE is in
the allowed operations" as a request to move. Add a UI test or a headless check
that an unmodified drop uses `Mode.COPY`.
##########
ui/src/main/java/org/apache/hop/ui/hopgui/vfs/explorer/VfsFileExplorerLocation.java:
##########
@@ -1432,52 +2061,62 @@ private void pasteClipboard() {
destinationUri,
entries,
mode,
- BaseMessages.getString(PKG, operationKey, entries.get(0).getName()));
+ BaseMessages.getString(PKG, operationKey, entries.get(0).getName()),
+ clipboard);
}
private void copyIntoFolder(
String destinationUri,
List<VfsFileTransfer.Entry> entries,
VfsFileTransfer.Mode mode,
String description) {
+ copyIntoFolder(destinationUri, entries, mode, description, null);
+ }
+
+ /**
+ * Copy or move {@code entries} into a folder in the background. A move
pasted from {@code
+ * clipboard} empties it: the files are no longer where it points.
+ */
+ private void copyIntoFolder(
+ String destinationUri,
+ List<VfsFileTransfer.Entry> entries,
+ VfsFileTransfer.Mode mode,
+ String description,
+ VfsFileClipboard clipboard) {
if (StringUtils.isBlank(destinationUri) || entries.isEmpty()) {
return;
}
runChange(
description,
destinationUri,
- () -> {
+ operation -> {
FileObject destination = HopVfs.getFileObject(destinationUri,
variables);
if (!destination.isFolder()) {
throw new IllegalStateException(destinationUri);
}
List<String> errors = new ArrayList<>();
- boolean overwrite = false;
- boolean asked = false;
+ VfsFileTransfer.ConflictResolver resolver = conflictResolver();
int done = 0;
for (VfsFileTransfer.Entry entry : entries) {
- if (Thread.currentThread().isInterrupted()) {
- break;
+ if (operation.isCancelled()) {
+ throw new CancellationException();
}
FileObject source = HopVfs.getFileObject(entry.getUri(),
variables);
- VfsFileTransfer.Refusal refusal = VfsFileTransfer.refusal(source,
destination);
- if (refusal == VfsFileTransfer.Refusal.EXISTS) {
- if (!asked) {
- overwrite = askOverwrite();
- asked = true;
- }
- if (!overwrite) {
- continue;
- }
- } else if (refusal != VfsFileTransfer.Refusal.NONE) {
- errors.add(transferMessage(refusal, entry.getName()));
- continue;
+ try {
+ VfsFileTransfer.transfer(
+ source,
+ destination,
+ mode,
+ resolver,
+ operation::isCancelled,
+ operation.progress());
+ done++;
+ } catch (VfsFileTransfer.RefusedException e) {
+ errors.add(transferMessage(e.getRefusal(), e.getName()));
}
- VfsFileTransfer.transfer(source, destination, mode, overwrite);
- done++;
}
- if (mode == VfsFileTransfer.Mode.MOVE && done > 0) {
- VfsFileClipboard.clear();
+ if (clipboard != null && mode == VfsFileTransfer.Mode.MOVE && done >
0) {
Review Comment:
**[bug]** A cut is cleared, or left unusable, after a paste that did not
move every file. `done` is incremented whenever `transfer` returns, and `place`
returns normally for Skip as well as for a successful replace. Skip or Skip all
on a cut therefore clears the clipboard even though every source is still in
place. A later `IOException` or `CancellationException` is not caught in this
loop, so it skips the `clear` and also skips every entry after the failure.
Files already moved are gone, but the clipboard still points at the old URIs.
The next paste opens the missing file, throws (not a `RefusedException`), and
aborts before the entries that were not moved yet. The user cannot finish the
cut without selecting the survivors again, and the error does not say which
names already moved.
**Suggestion:** Count only entries that were actually moved. On Skip, leave
that entry on the clipboard. On cancel or a hard failure, drop the entries
already moved and keep the rest, or keep going and report each failure the same
way `RefusedException` is reported. Add the multi-item "some refused / some
skipped" paste test the issue asked for, including a cut.
##########
ui/src/main/java/org/apache/hop/ui/hopgui/vfs/explorer/VfsFileTransfer.java:
##########
@@ -102,32 +165,129 @@ public static Refusal refusal(FileObject source,
FileObject destinationFolder)
return Refusal.INTO_ITSELF;
}
if (destination.exists()) {
- return Refusal.EXISTS;
+ return destination.isFolder() == source.isFolder() ? Refusal.EXISTS :
Refusal.OTHER_KIND;
}
return Refusal.NONE;
}
/**
* Copy or move {@code source} into {@code destinationFolder}, keeping the
base name. An existing
- * destination is replaced only when {@code overwrite} is true.
+ * folder is merged. Each existing file goes to {@code resolver}.
+ *
+ * @throws RefusedException when the source cannot be placed at all
+ * @throws CancellationException when {@code cancelled} turns true or the
resolver cancels
*/
public static void transfer(
- FileObject source, FileObject destinationFolder, Mode mode, boolean
overwrite)
- throws FileSystemException {
+ FileObject source,
+ FileObject destinationFolder,
+ Mode mode,
+ ConflictResolver resolver,
+ BooleanSupplier cancelled)
+ throws IOException {
+ transfer(source, destinationFolder, mode, resolver, cancelled,
Progress.NONE);
+ }
+
+ /** As {@link #transfer(FileObject, FileObject, Mode, ConflictResolver,
BooleanSupplier)}. */
+ public static void transfer(
+ FileObject source,
+ FileObject destinationFolder,
+ Mode mode,
+ ConflictResolver resolver,
+ BooleanSupplier cancelled,
+ Progress progress)
+ throws IOException {
Refusal refusal = refusal(source, destinationFolder);
+ String name = source == null ? "" : source.getName().getBaseName();
if (refusal == Refusal.SAME
|| refusal == Refusal.INTO_ITSELF
- || (refusal == Refusal.EXISTS && !overwrite)) {
- throw new IllegalStateException(refusal.name());
+ || refusal == Refusal.OTHER_KIND) {
+ throw new RefusedException(refusal, name);
}
- FileObject destination =
destinationFolder.resolveFile(source.getName().getBaseName());
- if (refusal == Refusal.EXISTS) {
- delete(destination);
+ place(
+ source,
+ destinationFolder.resolveFile(name),
+ mode,
+ resolver,
+ cancelled,
+ progress == null ? Progress.NONE : progress);
+ }
+
+ private static void place(
+ FileObject source,
+ FileObject target,
+ Mode mode,
+ ConflictResolver resolver,
+ BooleanSupplier cancelled,
+ Progress progress)
+ throws IOException {
+ checkCancelled(cancelled);
+ if (!target.exists()) {
+ if (mode == Mode.MOVE && source.canRenameTo(target)) {
+ source.moveTo(target);
+ progress.fileDone();
+ } else {
+ copyTree(source, target, cancelled, progress);
+ if (mode == Mode.MOVE) {
+ delete(source);
+ }
+ }
+ return;
+ }
+ if (source.isFolder() != target.isFolder()) {
+ throw new RefusedException(Refusal.OTHER_KIND,
source.getName().getBaseName());
+ }
+ if (!source.isFolder()) {
+ Resolution resolution = resolver == null ? Resolution.SKIP :
resolver.resolve(source, target);
+ if (resolution == Resolution.CANCEL) {
+ throw new CancellationException();
+ }
+ if (resolution == Resolution.REPLACE) {
+ replaceFile(source, target, mode, cancelled, progress);
+ }
+ return;
+ }
+ for (FileObject child : source.getChildren()) {
+ place(
+ child,
+ target.resolveFile(child.getName().getBaseName()),
+ mode,
+ resolver,
+ cancelled,
+ progress);
}
+ if (mode == Mode.MOVE && source.getChildren().length == 0) {
+ // Skipped files stay behind, and so does the folder that holds them.
+ source.delete();
+ }
+ }
+
+ /**
+ * Replace the existing file {@code target} with {@code source}. The new
content goes to a hidden
+ * file next to the target first, so a failed or stopped copy leaves the
original untouched.
+ */
+ public static void replaceFile(
+ FileObject source, FileObject target, Mode mode, BooleanSupplier
cancelled)
+ throws IOException {
+ replaceFile(source, target, mode, cancelled, Progress.NONE);
+ }
+
+ /** As {@link #replaceFile(FileObject, FileObject, Mode, BooleanSupplier)},
counting bytes. */
+ public static void replaceFile(
+ FileObject source, FileObject target, Mode mode, BooleanSupplier
cancelled, Progress progress)
+ throws IOException {
+ FileObject temp = target.getParent().resolveFile(TEMP_PREFIX +
UUID.randomUUID() + ".tmp");
+ // copyFile removes a partly written temp file itself.
+ copyFile(source, temp, cancelled, progress == null ? Progress.NONE :
progress);
+ try {
+ target.delete();
Review Comment:
**[suggestion]** `replaceFile` deletes the destination before
`temp.moveTo(target)`. The temp copy is complete, so a stopped or failed copy
still leaves the original, which matches the issue. If `moveTo` then fails, the
name the user had is already gone and the new bytes exist only as a hidden
`.hop-transfer-*.tmp` sibling. On object stores `moveTo` is itself a copy plus
delete, so that second copy can fail after the original key was removed. The
failure dialog does not mention the temp name, and hidden files are off by
default, so the file looks deleted.
**Suggestion:** Rename the original aside, move the temp onto the real name,
then delete the backup. If the move fails, put the backup back and delete the
temp. If a temp must be left behind, include its name in the operation error.
##########
ui/src/main/java/org/apache/hop/ui/hopgui/vfs/explorer/VfsFileExplorerLocation.java:
##########
@@ -302,10 +359,171 @@ public VfsFileExplorerLocation(Composite parent,
VfsFileExplorer explorer) {
detailsToolbar.dispose();
bookmarksToolbar.dispose();
});
- refreshBookmarks();
+ refreshPlaces();
updateToolbar();
}
+ /**
+ * Drag folders from the tree and rows from the file list, and drop on a
folder. The paths travel
+ * as a {@link FileTransfer}, as in the File Explorer perspective, so files
dragged between the
+ * two or from the operating system are accepted too. A drop copies unless a
move is asked for:
+ * the source and target are often different stores.
+ */
+ private void installDragAndDrop() {
+ FileTransfer fileTransfer = FileTransfer.getInstance();
+ Transfer[] transfers = {fileTransfer};
+
+ DragSource treeSource = new DragSource(tree, DND.DROP_COPY |
DND.DROP_MOVE);
Review Comment:
**[bug]** A Hop Web user without `explorer.write` can still write by
dragging out of this explorer. The new drag source offers `FileTransfer` with
copy and move and does not check `canWrite()`. The in-explorer drop target does
call `checkWrite()`, but the File Explorer perspective drop
(`ExplorerPerspective` drop listener, `copyFrom` / `moveFile`) never checks
`EXPLORER_WRITE`. That drop was already unsafe for drags inside the
perspective; this change newly lets a read-only session drag a project file, a
server file, or a named connection (S3, FTP, and so on) onto the perspective
tree. Combined with issue 1, that drag is a move on Linux and macOS, so it
deletes the source as well as writing the destination. Disabling the paste,
cut, and delete buttons does not close this path.
**Suggestion:** Refuse `dragStart` when `HopSecurity.allows(EXPLORER_WRITE)`
is false, or make the perspective drop call the same
`HopSecurityUi.check(Permission.EXPLORER_WRITE)` guard as
`getSelectedFolderPath` before it copies or moves. A read-only user may still
copy a path or download when `file.export` allows it; they must not be able to
create or delete files through a drop.
##########
ui/src/main/java/org/apache/hop/ui/hopgui/vfs/explorer/VfsFileExplorerLocation.java:
##########
@@ -772,11 +1133,127 @@ public void uploadSelected() {
String description =
files.size() == 1
? BaseMessages.getString(
- PKG, "VfsFileExplorer.Operation.Uploading",
files.get(0).getName())
- : BaseMessages.getString(PKG,
"VfsFileExplorer.Operation.Uploading.Many", files.size());
+ PKG, "VfsFileExplorer.Operation.Copying",
files.get(0).getName())
+ : BaseMessages.getString(PKG,
"VfsFileExplorer.Operation.Copying.Many", files.size());
copyIntoFolder(destination, files, VfsFileTransfer.Mode.COPY, description);
}
+ /**
+ * Hop Web: send the selected file to the browser. It is read into a
server-side temporary file
+ * first, because a remote file cannot be streamed to the browser directly.
+ */
+ private void downloadToBrowser() {
+ if (!HopSecurityUi.check(Permission.FILE_EXPORT)) {
+ return;
+ }
+ VfsFileRow row = singleFileRow();
+ if (row == null) {
+ return;
+ }
+ String sourceUri = row.getUri();
+ String name = row.getName();
+ Display display = getDisplay();
+ runChange(
+ BaseMessages.getString(PKG, "VfsFileExplorer.Operation.Downloading",
name),
+ sourceUri,
+ false,
+ operation -> {
+ Path local = Files.createTempFile("hop-vfs-download-", ".tmp");
Review Comment:
**[suggestion]** Hop Web download does not observe Stop and does not use the
session temp directory. `Files.copy` runs to completion with no
`operation.isCancelled()` check, then `asyncExec` still calls
`BrowserFileTransferFacade.download` after the user has stopped the operation.
The temp file is created with `Files.createTempFile` in the JVM temp dir, not
under `HopWebUserFilePlugin.getSessionTempDirectory()`. `deleteLocalQuietly`
claims the session folder will remove it, but that cleanup never sees this
path. If the display is disposed before the `asyncExec` runs, or the delete
fails, the server copy is leaked outside the session directory.
**Suggestion:** Copy into the session temp directory, stop between chunks
the same way `VfsFileTransfer.copyFile` does, and skip `download` when the
operation is already cancelled. Delete the temp in a `finally` on the worker if
the UI hand-off does not run.
##########
ui/src/main/java/org/apache/hop/ui/hopgui/vfs/explorer/VfsFileExplorerLocation.java:
##########
@@ -302,10 +359,171 @@ public VfsFileExplorerLocation(Composite parent,
VfsFileExplorer explorer) {
detailsToolbar.dispose();
bookmarksToolbar.dispose();
});
- refreshBookmarks();
+ refreshPlaces();
updateToolbar();
}
+ /**
+ * Drag folders from the tree and rows from the file list, and drop on a
folder. The paths travel
+ * as a {@link FileTransfer}, as in the File Explorer perspective, so files
dragged between the
+ * two or from the operating system are accepted too. A drop copies unless a
move is asked for:
+ * the source and target are often different stores.
+ */
+ private void installDragAndDrop() {
+ FileTransfer fileTransfer = FileTransfer.getInstance();
+ Transfer[] transfers = {fileTransfer};
+
+ DragSource treeSource = new DragSource(tree, DND.DROP_COPY |
DND.DROP_MOVE);
+ treeSource.setTransfer(transfers);
+ treeSource.addDragListener(dragListener(this::selectedFolderEntries));
+ DragSource tableSource = new DragSource(table, DND.DROP_COPY |
DND.DROP_MOVE);
+ tableSource.setTransfer(transfers);
+ tableSource.addDragListener(dragListener(this::selectedFileEntries));
+
+ DropTarget treeTarget = new DropTarget(tree, DND.DROP_COPY |
DND.DROP_MOVE);
+ treeTarget.setTransfer(transfers);
+ treeTarget.addDropListener(
+ dropListener(
+ event -> {
+ FolderNode node = event.item instanceof TreeItem item ?
nodeOf(item) : null;
+ return node == null ? null : node.uri;
+ }));
+ DropTarget tableTarget = new DropTarget(table, DND.DROP_COPY |
DND.DROP_MOVE);
+ tableTarget.setTransfer(transfers);
+ tableTarget.addDropListener(
+ dropListener(
+ event -> {
+ // A folder row is the target. Anywhere else in the list is the
folder being viewed.
+ if (event.item instanceof TableItem item && !item.isDisposed()) {
+ int index = table.indexOf(item);
+ if (index >= 0 && index < shownRows.size() &&
shownRows.get(index).isFolder()) {
+ return shownRows.get(index).getUri();
+ }
+ }
+ return currentFolderUri();
+ }));
+ }
+
+ private DragSourceAdapter dragListener(
+ java.util.function.Supplier<List<VfsFileTransfer.Entry>> selection) {
+ return new DragSourceAdapter() {
+ @Override
+ public void dragStart(DragSourceEvent event) {
+ List<String> uris = urisOf(selection.get());
+ event.doit = !uris.isEmpty();
+ draggedUris = uris;
+ if (EnvironmentUtils.getInstance().isWeb()) {
+ // RAP has no native drag image to fall back on.
+ event.image = GuiResource.getInstance().getImageFolder();
+ }
+ }
+
+ @Override
+ public void dragSetData(DragSourceEvent event) {
+ // Read the selection again rather than the paths kept at drag start:
Hop Web can finish
+ // the drag before it asks for the data, and dragFinished has cleared
them by then.
+ if (FileTransfer.getInstance().isSupportedType(event.dataType)) {
+ List<String> uris = urisOf(selection.get());
+ event.doit = !uris.isEmpty();
+ event.data = uris.toArray(new String[0]);
+ }
+ }
+
+ @Override
+ public void dragFinished(DragSourceEvent event) {
+ draggedUris = List.of();
+ }
+ };
+ }
+
+ private DropTargetAdapter dropListener(
+ java.util.function.Function<DropTargetEvent, String> targetOf) {
+ return new DropTargetAdapter() {
+ @Override
+ public void dragEnter(DropTargetEvent event) {
+ if (event.detail == DND.DROP_DEFAULT) {
+ event.detail = (event.operations & DND.DROP_COPY) != 0 ?
DND.DROP_COPY : DND.DROP_NONE;
+ }
+ }
+
+ @Override
+ public void dragOperationChanged(DropTargetEvent event) {
+ dragEnter(event);
+ }
+
+ @Override
+ public void dragOver(DropTargetEvent event) {
+ event.feedback = DND.FEEDBACK_SELECT | DND.FEEDBACK_SCROLL |
DND.FEEDBACK_EXPAND;
+ String target = targetOf.apply(event);
+ if (!canWrite() || StringUtils.isBlank(target) ||
!canDropInto(draggedUris, target)) {
+ event.detail = DND.DROP_NONE;
+ } else if (event.detail == DND.DROP_NONE || event.detail ==
DND.DROP_DEFAULT) {
+ event.detail = (event.operations & DND.DROP_COPY) != 0 ?
DND.DROP_COPY : DND.DROP_MOVE;
+ }
+ }
+
+ @Override
+ public void drop(DropTargetEvent event) {
+ String target = targetOf.apply(event);
+ if (!(event.data instanceof String[] paths)
+ || paths.length == 0
+ || StringUtils.isBlank(target)
+ || !checkWrite()) {
+ event.detail = DND.DROP_NONE;
+ return;
+ }
+ List<String> uris = List.of(paths);
+ if (!canDropInto(uris, target)) {
+ event.detail = DND.DROP_NONE;
+ return;
+ }
+ List<VfsFileTransfer.Entry> entries = new ArrayList<>();
+ for (String uri : uris) {
+ entries.add(new VfsFileTransfer.Entry(uri, titleOf(uri), false));
Review Comment:
**[suggestion]** Internal drags are de-duplicated by `selectedFileEntries` /
`selectedFolderEntries` before the paths are put on the transfer, but the drop
rebuilds every entry with `folder` false and never calls `withoutNested`. A
drop from the file manager or the File Explorer perspective can contain both a
folder and a file or folder inside it. Those are copied as siblings, and a move
can take the child out of the folder before the folder itself is moved. The
false flag also means a later clipboard-style nested check cannot see the
directory.
**Suggestion:** Before `copyIntoFolder`, stat the paths (or carry the folder
bit) and run `VfsFileTransfer.withoutNested`, so a directory and a path inside
it are one transfer.
##########
ui/src/main/java/org/apache/hop/ui/hopgui/vfs/explorer/VfsFileTransfer.java:
##########
@@ -171,4 +331,67 @@ public static boolean isInside(FileObject folder,
FileObject candidate)
}
return folder.getName().isDescendent(candidate.getName());
}
+
+ /**
+ * Copy a file or a folder with everything in it to {@code target}, which
does not exist yet.
+ * Stops between chunks when {@code cancelled} turns true, and removes a
partly written file.
+ */
+ public static void copy(
+ FileObject source, FileObject target, BooleanSupplier cancelled,
Progress progress)
+ throws IOException {
+ copyTree(source, target, cancelled, progress == null ? Progress.NONE :
progress);
+ }
+
+ private static void copyTree(
+ FileObject source, FileObject target, BooleanSupplier cancelled,
Progress progress)
+ throws IOException {
+ checkCancelled(cancelled);
+ if (!source.isFolder()) {
+ copyFile(source, target, cancelled, progress);
+ return;
+ }
+ target.createFolder();
Review Comment:
**[suggestion]** Stop deletes the file that was mid-write (`copyFile`'s
catch calls `deleteQuietly`), but a folder copy already called `createFolder`
and may have finished earlier children. Those stay behind when
`CancellationException` unwinds `copyTree`. A stopped move does not delete the
source, so the user is left with a partial extra tree and no signal of which
names finished. The new test only covers a single file.
**Suggestion:** On cancel, delete a destination folder this call created
(and the children it copied into it) when the copy did not finish. Leave an
existing folder that was being merged alone, apart from the partial file
already removed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
