From b7eb54a4f32377793bf9255c7f71acc0738df61b Mon Sep 17 00:00:00 2001 From: David Given Date: Mon, 19 Oct 2026 01:15:56 +0200 Subject: [PATCH 1/5] Add drag-and-drop into the files window. --- MODULE.bazel.lock | 10 +- .../fluxengine/gui/FilesystemPanel.java | 95 ++++++++++++++++++- .../cowlark/fluxengine/external/BUILD.bazel | 11 +++ .../gui/FilesystemPanelHeadlessTest.java | 9 ++ 4 files changed, 118 insertions(+), 7 deletions(-) diff --git a/MODULE.bazel.lock b/MODULE.bazel.lock index d0b19d173..5e457e1c0 100644 --- a/MODULE.bazel.lock +++ b/MODULE.bazel.lock @@ -21,8 +21,8 @@ "https://bcr.bazel.build/modules/apple_support/1.15.1/MODULE.bazel": "a0556fefca0b1bb2de8567b8827518f94db6a6e7e7d632b4c48dc5f865bc7c85", "https://bcr.bazel.build/modules/apple_support/1.21.0/MODULE.bazel": "ac1824ed5edf17dee2fdd4927ada30c9f8c3b520be1b5fd02a5da15bc10bff3e", "https://bcr.bazel.build/modules/apple_support/1.21.1/MODULE.bazel": "5809fa3efab15d1f3c3c635af6974044bac8a4919c62238cce06acee8a8c11f1", - "https://bcr.bazel.build/modules/apple_support/1.24.2/MODULE.bazel": "0e62471818affb9f0b26f128831d5c40b074d32e6dda5a0d3852847215a41ca4", - "https://bcr.bazel.build/modules/apple_support/1.24.2/source.json": "2c22c9827093250406c5568da6c54e6fdf0ef06238def3d99c71b12feb057a8d", + "https://bcr.bazel.build/modules/apple_support/2.5.2/MODULE.bazel": "bb1b7081e390a8d3885f642cb0073dfcf8e7a36525e079fa8f5835af7079c0fe", + "https://bcr.bazel.build/modules/apple_support/2.5.2/source.json": "dd51ee925a550e92b55eb41c4c809a967da311a2d2520241252a238a48b39c4c", "https://bcr.bazel.build/modules/bazel_features/1.1.0/MODULE.bazel": "cfd42ff3b815a5f39554d97182657f8c4b9719568eb7fded2b9135f084bf760b", "https://bcr.bazel.build/modules/bazel_features/1.1.1/MODULE.bazel": "27b8c79ef57efe08efccbd9dd6ef70d61b4798320b8d3c134fd571f78963dbcd", "https://bcr.bazel.build/modules/bazel_features/1.10.0/MODULE.bazel": "f75e8807570484a99be90abcd52b5e1f390362c258bcb73106f4544957a48101", @@ -154,9 +154,9 @@ "https://bcr.bazel.build/modules/rules_cc/0.2.0/MODULE.bazel": "b5c17f90458caae90d2ccd114c81970062946f49f355610ed89bebf954f5783c", "https://bcr.bazel.build/modules/rules_cc/0.2.13/MODULE.bazel": "eecdd666eda6be16a8d9dc15e44b5c75133405e820f620a234acc4b1fdc5aa37", "https://bcr.bazel.build/modules/rules_cc/0.2.14/MODULE.bazel": "353c99ed148887ee89c54a17d4100ae7e7e436593d104b668476019023b58df8", + "https://bcr.bazel.build/modules/rules_cc/0.2.15/MODULE.bazel": "6a0a4a75a57aa6dc888300d848053a58c6b12a29f89d4304e1c41448514ec6e8", "https://bcr.bazel.build/modules/rules_cc/0.2.17/MODULE.bazel": "1849602c86cb60da8613d2de887f9566a6d354a6df6d7009f9d04a14402f9a84", "https://bcr.bazel.build/modules/rules_cc/0.2.17/source.json": "3832f45d145354049137c0090df04629d9c2b5493dc5c2bf46f1834040133a07", - "https://bcr.bazel.build/modules/rules_cc/0.2.8/MODULE.bazel": "f1df20f0bf22c28192a794f29b501ee2018fa37a3862a1a2132ae2940a23a642", "https://bcr.bazel.build/modules/rules_foreign_cc/0.9.0/MODULE.bazel": "c9e8c682bf75b0e7c704166d79b599f93b72cfca5ad7477df596947891feeef6", "https://bcr.bazel.build/modules/rules_fuzzing/0.5.2/MODULE.bazel": "40c97d1144356f52905566c55811f13b299453a14ac7769dfba2ac38192337a8", "https://bcr.bazel.build/modules/rules_go/0.41.0/MODULE.bazel": "55861d8e8bb0e62cbd2896f60ff303f62ffcb0eddb74ecb0e5c0cbe36fc292c8", @@ -337,7 +337,7 @@ }, "@@rules_python+//python/extensions:config.bzl%config": { "general": { - "bzlTransitiveDigest": "dzD8Q2YmrP3fz8saWLHPmlwPLO91ImtTmP/c9JKTStM=", + "bzlTransitiveDigest": "L+2ZCpb7GIJqvWaXpzkfk/80d916sIFVmSPzrSm6of8=", "usagesDigest": "ZVSXMAGpD+xzVNPuvF1IoLBkty7TROO0+akMapt1pAg=", "recordedInputs": [ "REPO_MAPPING:rules_python+,bazel_tools bazel_tools", @@ -505,7 +505,7 @@ }, "@@rules_python+//python/uv:uv.bzl%uv": { "general": { - "bzlTransitiveDigest": "ijW9KS7qsIY+yBVvJ+Nr1mzwQox09j13DnE3iIwaeTM=", + "bzlTransitiveDigest": "IgkgxNrtlAec7OnaFrWoCC5NQhHcY/aIz4jsqcNMm8Y=", "usagesDigest": "H8dQoNZcoqP+Mu0tHZTi4KHATzvNkM5ePuEqoQdklIU=", "recordedInputs": [ "REPO_MAPPING:rules_python+,bazel_tools bazel_tools", diff --git a/java/com/cowlark/fluxengine/gui/FilesystemPanel.java b/java/com/cowlark/fluxengine/gui/FilesystemPanel.java index 2d8dfea9f..18399e312 100644 --- a/java/com/cowlark/fluxengine/gui/FilesystemPanel.java +++ b/java/com/cowlark/fluxengine/gui/FilesystemPanel.java @@ -37,18 +37,25 @@ import sprouts.Viewable; import swingtree.ComponentDelegate; import swingtree.UI; +import javax.swing.DropMode; import javax.swing.JButton; import javax.swing.JFileChooser; import javax.swing.JOptionPane; import javax.swing.JPanel; +import javax.swing.JTable; +import javax.swing.TransferHandler; import javax.swing.SwingUtilities; import javax.swing.event.TreeExpansionEvent; import javax.swing.event.TreeExpansionListener; import javax.swing.tree.TreePath; import javax.swing.tree.TreeSelectionModel; import java.awt.event.ActionEvent; +import java.awt.datatransfer.DataFlavor; +import java.awt.datatransfer.UnsupportedFlavorException; import java.io.File; +import java.io.IOException; import java.util.HashSet; +import java.util.List; import java.util.Set; public class FilesystemPanel extends JPanel @@ -119,6 +126,11 @@ public void treeCollapsed(TreeExpansionEvent event) treeTable.setClosedIcon(null); treeTable.putClientProperty("FlatLaf.style", "showHorizontalLines: true"); + /* Accept external files on directory rows without changing the current selection. */ + treeTable.setFillsViewportHeight(true); + treeTable.setDropMode(DropMode.ON); + treeTable.setTransferHandler(createFileDropHandler()); + filesSelected = Var.of(Tuple.of(TreePath.class)); treeSelectionModel = treeTable.getTreeSelectionModel(); treeSelectionModel.setSelectionMode(TreeSelectionModel.DISCONTIGUOUS_TREE_SELECTION); @@ -335,6 +347,81 @@ private void viewFile( }); } + DirNode getFileDropDirectory(int row) + { + /* Background drops go to root; file rows remain invalid destinations. */ + Object target; + if (row < 0) + target = treeTableModel.getRoot(); + else + { + TreePath path = treeTable.getPathForRow(row); + target = path == null ? null : path.getLastPathComponent(); + } + return target instanceof DirNode directory ? directory : null; + } + + private TransferHandler createFileDropHandler() + { + return new TransferHandler() + { + @Override + public boolean canImport(TransferSupport support) + { + /* Only copy external files into a mounted filesystem that supports writes. */ + if (!support.isDrop() + || !support.isDataFlavorSupported(DataFlavor.javaFileListFlavor) + || (support.getSourceDropActions() & COPY) == 0 + || !treeTableModel.getIsMounted().get() + || !treeTableModel.getCapabilities().get().contains(OP_PUTFILE)) + return false; + + /* Resolve the hovered directory, using root for empty table space. */ + JTable.DropLocation location = (JTable.DropLocation) support.getDropLocation(); + if (getFileDropDirectory(location.getRow()) == null) + return false; + support.setDropAction(COPY); + return true; + } + + @Override + public boolean importData(TransferSupport support) + { + if (!canImport(support)) + return false; + + /* Snapshot the destination on the EDT before handing work to the filesystem. */ + JTable.DropLocation location = (JTable.DropLocation) support.getDropLocation(); + DirNode directory = getFileDropDirectory(location.getRow()); + VfsPath parentPath = directory.vfsPath(); + try + { + /* Reject the entire drop if it contains directories or other non-files. */ + List transferred = (List) support.getTransferable() + .getTransferData(DataFlavor.javaFileListFlavor); + ImmutableList.Builder files = ImmutableList.builder(); + for (Object item : transferred) + { + if (!(item instanceof File file) || !file.isFile()) + return false; + files.add(file); + } + ImmutableList snapshot = files.build(); + if (snapshot.isEmpty()) + return false; + + /* Queue the same import operation used by the file chooser. */ + putFiles(parentPath, snapshot); + return true; + } catch (UnsupportedFlavorException | IOException e) + { + logger.warn("Cannot import dropped files", e); + return false; + } + } + }; + } + private void putFiles( ComponentDelegate delegate) { @@ -349,10 +436,14 @@ private void putFiles( if (result != JFileChooser.APPROVE_OPTION) return; - File[] selectedFiles = chooser.getSelectedFiles(); + putFiles(parentPath, ImmutableList.copyOf(chooser.getSelectedFiles())); + } + private void putFiles(VfsPath parentPath, ImmutableList files) + { + /* Read host files and write their contents on the filesystem worker thread. */ treeTableModel.queueFilesystemOperation(fs -> { - for (File file : selectedFiles) + for (File file : files) { Bytes data = Bytes.readFromFile(file.toPath()); String leafName = file.getName(); diff --git a/javatests/com/cowlark/fluxengine/external/BUILD.bazel b/javatests/com/cowlark/fluxengine/external/BUILD.bazel index 3127346ac..ee53a50ab 100644 --- a/javatests/com/cowlark/fluxengine/external/BUILD.bazel +++ b/javatests/com/cowlark/fluxengine/external/BUILD.bazel @@ -75,3 +75,14 @@ java_test( "@maven//:junit_junit", ], ) + +java_test( + name = "A2R3Test", + srcs = ["A2R3Test.java"], + deps = [ + "//java/com/cowlark/fluxengine/external", + "@maven//:junit_junit", + "@maven//:org_indunet_fastproto", + "@maven//:org_indunet_fastproto_core", + ], +) diff --git a/javatests/com/cowlark/fluxengine/gui/FilesystemPanelHeadlessTest.java b/javatests/com/cowlark/fluxengine/gui/FilesystemPanelHeadlessTest.java index 67bd806a5..4f1c67f99 100644 --- a/javatests/com/cowlark/fluxengine/gui/FilesystemPanelHeadlessTest.java +++ b/javatests/com/cowlark/fluxengine/gui/FilesystemPanelHeadlessTest.java @@ -112,6 +112,15 @@ public void setUp() throws Exception awaitLoaded(VfsPath.of("/")); } + @Test + public void backgroundDropTargetsRoot() throws Exception + { + SwingUtilities.invokeAndWait(() -> { + assertThat(panel.getFileDropDirectory(-1)).isSameInstanceAs(model.getRoot()); + assertThat(panel.getTreeTable().getFillsViewportHeight()).isTrue(); + }); + } + private void awaitIdle() throws Exception { /* Pump the EDT. */ From 15389b80d34ac1f07e7cf52af867768fa616064c Mon Sep 17 00:00:00 2001 From: David Given Date: Fri, 23 Oct 2026 22:48:28 +0200 Subject: [PATCH 2/5] Add basic drag-and-drop of files. --- .../fluxengine/gui/FilesystemPanel.java | 100 ++------ .../gui/FilesystemPanelTransferHandler.java | 231 ++++++++++++++++++ .../com/cowlark/fluxengine/gui/BUILD.bazel | 1 + .../gui/FilesystemPanelHeadlessTest.java | 109 ++++++++- 4 files changed, 357 insertions(+), 84 deletions(-) create mode 100644 java/com/cowlark/fluxengine/gui/FilesystemPanelTransferHandler.java diff --git a/java/com/cowlark/fluxengine/gui/FilesystemPanel.java b/java/com/cowlark/fluxengine/gui/FilesystemPanel.java index 18399e312..181b7baf5 100644 --- a/java/com/cowlark/fluxengine/gui/FilesystemPanel.java +++ b/java/com/cowlark/fluxengine/gui/FilesystemPanel.java @@ -42,7 +42,6 @@ import javax.swing.JFileChooser; import javax.swing.JOptionPane; import javax.swing.JPanel; -import javax.swing.JTable; import javax.swing.TransferHandler; import javax.swing.SwingUtilities; import javax.swing.event.TreeExpansionEvent; @@ -50,12 +49,9 @@ import javax.swing.tree.TreePath; import javax.swing.tree.TreeSelectionModel; import java.awt.event.ActionEvent; -import java.awt.datatransfer.DataFlavor; -import java.awt.datatransfer.UnsupportedFlavorException; +import java.awt.GraphicsEnvironment; import java.io.File; -import java.io.IOException; import java.util.HashSet; -import java.util.List; import java.util.Set; public class FilesystemPanel extends JPanel @@ -67,6 +63,7 @@ public class FilesystemPanel extends JPanel final FilesystemTreeTableModel treeTableModel; final TreeSelectionModel treeSelectionModel; final Var> filesSelected; + final Val canDoMultiFileOperation; final Set everExpanded = new HashSet<>(); JXTreeTable getTreeTable() @@ -130,6 +127,8 @@ public void treeCollapsed(TreeExpansionEvent event) treeTable.setFillsViewportHeight(true); treeTable.setDropMode(DropMode.ON); treeTable.setTransferHandler(createFileDropHandler()); + if (!GraphicsEnvironment.isHeadless()) + treeTable.setDragEnabled(true); filesSelected = Var.of(Tuple.of(TreePath.class)); treeSelectionModel = treeTable.getTreeSelectionModel(); @@ -153,7 +152,7 @@ public void treeCollapsed(TreeExpansionEvent event) !(Iterables .getOnlyElement(files) .getLastPathComponent() instanceof DirNode)); - Val canDoMultiFileOperation = Viewable.of( + canDoMultiFileOperation = Viewable.of( allowedWhenMounted, filesSelected, (mounted, files) -> isValidSelection(files) && (files.size() >= 1) && mounted); @@ -347,79 +346,9 @@ private void viewFile( }); } - DirNode getFileDropDirectory(int row) - { - /* Background drops go to root; file rows remain invalid destinations. */ - Object target; - if (row < 0) - target = treeTableModel.getRoot(); - else - { - TreePath path = treeTable.getPathForRow(row); - target = path == null ? null : path.getLastPathComponent(); - } - return target instanceof DirNode directory ? directory : null; - } - private TransferHandler createFileDropHandler() { - return new TransferHandler() - { - @Override - public boolean canImport(TransferSupport support) - { - /* Only copy external files into a mounted filesystem that supports writes. */ - if (!support.isDrop() - || !support.isDataFlavorSupported(DataFlavor.javaFileListFlavor) - || (support.getSourceDropActions() & COPY) == 0 - || !treeTableModel.getIsMounted().get() - || !treeTableModel.getCapabilities().get().contains(OP_PUTFILE)) - return false; - - /* Resolve the hovered directory, using root for empty table space. */ - JTable.DropLocation location = (JTable.DropLocation) support.getDropLocation(); - if (getFileDropDirectory(location.getRow()) == null) - return false; - support.setDropAction(COPY); - return true; - } - - @Override - public boolean importData(TransferSupport support) - { - if (!canImport(support)) - return false; - - /* Snapshot the destination on the EDT before handing work to the filesystem. */ - JTable.DropLocation location = (JTable.DropLocation) support.getDropLocation(); - DirNode directory = getFileDropDirectory(location.getRow()); - VfsPath parentPath = directory.vfsPath(); - try - { - /* Reject the entire drop if it contains directories or other non-files. */ - List transferred = (List) support.getTransferable() - .getTransferData(DataFlavor.javaFileListFlavor); - ImmutableList.Builder files = ImmutableList.builder(); - for (Object item : transferred) - { - if (!(item instanceof File file) || !file.isFile()) - return false; - files.add(file); - } - ImmutableList snapshot = files.build(); - if (snapshot.isEmpty()) - return false; - - /* Queue the same import operation used by the file chooser. */ - putFiles(parentPath, snapshot); - return true; - } catch (UnsupportedFlavorException | IOException e) - { - logger.warn("Cannot import dropped files", e); - return false; - } - } - }; + return new FilesystemPanelTransferHandler(this); } private void putFiles( @@ -439,7 +368,7 @@ private void putFiles( putFiles(parentPath, ImmutableList.copyOf(chooser.getSelectedFiles())); } - private void putFiles(VfsPath parentPath, ImmutableList files) + void putFiles(VfsPath parentPath, ImmutableList files) { /* Read host files and write their contents on the filesystem worker thread. */ treeTableModel.queueFilesystemOperation(fs -> { @@ -611,13 +540,17 @@ private void renameFile( }); } - /** - * Returns true if the selection is valid — i.e. no selected path is an - * ancestor (or duplicate) of another selected path. Selections spanning - * multiple unrelated folders are fine. - */ + /* A valid selection has unique filenames and no selected path that is an ancestor of + * another selected path. Selections spanning unrelated folders are otherwise allowed. */ private static boolean isValidSelection(Tuple paths) { + Set filenames = new HashSet<>(); + for (TreePath path : paths) + { + if (!(path.getLastPathComponent() instanceof FileNode file) + || !filenames.add(file.getDirent().filename())) + return false; + } for (int i = 0; i < paths.size(); i++) { for (int j = i + 1; j < paths.size(); j++) @@ -663,4 +596,5 @@ private static Val not(Val v1) { return v1.viewAs(Boolean.class, b -> !b); } + } diff --git a/java/com/cowlark/fluxengine/gui/FilesystemPanelTransferHandler.java b/java/com/cowlark/fluxengine/gui/FilesystemPanelTransferHandler.java new file mode 100644 index 000000000..ebb2af349 --- /dev/null +++ b/java/com/cowlark/fluxengine/gui/FilesystemPanelTransferHandler.java @@ -0,0 +1,231 @@ +package com.cowlark.fluxengine.gui; + +import static com.cowlark.fluxengine.vfs.Filesystem.Capability.OP_GETFILE; +import static com.cowlark.fluxengine.vfs.Filesystem.Capability.OP_PUTFILE; +import static com.cowlark.fluxengine.vfs.Filesystem.FileType.IS_FILE; + +import com.cowlark.fluxengine.core.Bytes; +import com.cowlark.fluxengine.vfs.VfsPath; +import com.google.common.collect.ImmutableList; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import javax.swing.JComponent; +import javax.swing.JTable; +import javax.swing.SwingUtilities; +import javax.swing.TransferHandler; +import javax.swing.tree.TreePath; +import java.awt.SecondaryLoop; +import java.awt.Toolkit; +import java.awt.datatransfer.DataFlavor; +import java.awt.datatransfer.Transferable; +import java.awt.datatransfer.UnsupportedFlavorException; +import java.io.File; +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.List; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.CompletionException; + +class FilesystemPanelTransferHandler extends TransferHandler +{ + private static final Logger logger = LoggerFactory.getLogger(FilesystemPanelTransferHandler.class); + + private final FilesystemPanel filesystemPanel; + + public FilesystemPanelTransferHandler(FilesystemPanel filesystemPanel) + { + this.filesystemPanel = filesystemPanel; + } + + private FilesystemTreeTableModel.DirNode getFileDropDirectory(int row) + { + /* Background drops go to root; file rows remain invalid destinations. */ + Object target; + if (row < 0) + target = filesystemPanel.treeTableModel.getRoot(); + else + { + TreePath path = filesystemPanel.treeTable.getPathForRow(row); + target = path == null ? null : path.getLastPathComponent(); + } + return target instanceof FilesystemTreeTableModel.DirNode directory ? directory : null; + } + + private ImmutableList getFileExportPaths() + { + /* Only selections consisting entirely of ordinary files can be dragged out. */ + if (!filesystemPanel.canDoMultiFileOperation.get() || + !filesystemPanel.treeTableModel.getCapabilities().get().contains(OP_GETFILE)) + return ImmutableList.of(); + TreePath[] selection = + filesystemPanel.treeTable.getTreeSelectionModel().getSelectionPaths(); + if (selection == null) + return ImmutableList.of(); + + ImmutableList.Builder paths = ImmutableList.builder(); + for (TreePath path : selection) + { + if (!(path.getLastPathComponent() instanceof FilesystemTreeTableModel.FileNode file) || + file instanceof FilesystemTreeTableModel.DirNode || + file.getDirent().fileType() != IS_FILE) + return ImmutableList.of(); + paths.add(file.getDirent().path()); + } + return paths.build(); + } + + private class ExportedFiles implements Transferable + { + private final ImmutableList paths; + private CompletableFuture> extracted; + + ExportedFiles(ImmutableList paths) + { + this.paths = paths; + } + + private synchronized CompletableFuture> extract() + { + if (extracted != null) + return extracted; + extracted = new CompletableFuture<>(); + filesystemPanel.treeTableModel.queueFilesystemOperation(fs -> { + try + { + /* Keep the files until application exit because native drop targets may + * copy them later. */ + Path directory = Files.createTempDirectory("fluxengine-export-"); + directory.toFile().deleteOnExit(); + ImmutableList.Builder files = ImmutableList.builder(); + for (VfsPath path : paths) + { + String name = path.getName(); + Path output = directory.resolve(name).normalize(); + if (!directory.equals(output.getParent())) + throw new IOException("Invalid export filename: " + name); + output.toFile().deleteOnExit(); + Bytes bytes = fs.getFile(path); + bytes.writeToFile(output); + files.add(output.toFile()); + } + extracted.complete(files.build()); + } catch (Exception e) + { + extracted.completeExceptionally(e); + } + }); + return extracted; + } + + @Override + public DataFlavor[] getTransferDataFlavors() + { + return new DataFlavor[]{DataFlavor.javaFileListFlavor}; + } + + @Override + public boolean isDataFlavorSupported(DataFlavor flavor) + { + return DataFlavor.javaFileListFlavor.equals(flavor); + } + + @Override + public Object getTransferData(DataFlavor flavor) + throws UnsupportedFlavorException, IOException + { + if (!isDataFlavorSupported(flavor)) + throw new UnsupportedFlavorException(flavor); + + /* Start reading only when the destination requests the dropped files. Cache + * the result because native targets may request the data more than once. */ + CompletableFuture> result = extract(); + if (SwingUtilities.isEventDispatchThread() && !result.isDone()) + { + /* Keep Swing responsive while a drop or clipboard consumer waits on + * the EDT. */ + SecondaryLoop loop = + Toolkit.getDefaultToolkit().getSystemEventQueue().createSecondaryLoop(); + result.whenComplete((files, error) -> SwingUtilities.invokeLater(() -> loop.exit())); + loop.enter(); + } + try + { + return result.join(); + } catch (CompletionException e) + { + logger.warn("Cannot extract dragged files", e.getCause()); + throw new IOException("Cannot extract files", e.getCause()); + } + } + } + + @Override + public int getSourceActions(JComponent component) + { + return getFileExportPaths().isEmpty() ? NONE : COPY; + } + + @Override + protected Transferable createTransferable(JComponent component) + { + /* Snapshot the selection before reading from the filesystem worker. */ + ImmutableList paths = getFileExportPaths(); + return paths.isEmpty() ? null : new ExportedFiles(paths); + } + + @Override + public boolean canImport(TransferSupport support) + { + /* Only copy external files into a mounted filesystem that supports writes. */ + if (!support.isDrop() || !support.isDataFlavorSupported(DataFlavor.javaFileListFlavor) || + (support.getSourceDropActions() & COPY) == 0 || + !filesystemPanel.treeTableModel.getIsMounted().get() || + !filesystemPanel.treeTableModel.getCapabilities().get().contains(OP_PUTFILE)) + return false; + + /* Resolve the hovered directory, using root for empty table space. */ + JTable.DropLocation location = (JTable.DropLocation) support.getDropLocation(); + if (getFileDropDirectory(location.getRow()) == null) + return false; + support.setDropAction(COPY); + return true; + } + + @Override + public boolean importData(TransferSupport support) + { + if (!canImport(support)) + return false; + + /* Snapshot the destination on the EDT before handing work to the filesystem. */ + JTable.DropLocation location = (JTable.DropLocation) support.getDropLocation(); + FilesystemTreeTableModel.DirNode directory = getFileDropDirectory(location.getRow()); + VfsPath parentPath = directory.vfsPath(); + try + { + /* Reject the entire drop if it contains directories or other non-files. */ + List transferred = (List) support + .getTransferable() + .getTransferData(DataFlavor.javaFileListFlavor); + ImmutableList.Builder files = ImmutableList.builder(); + for (Object item : transferred) + { + if (!(item instanceof File file) || !file.isFile()) + return false; + files.add(file); + } + ImmutableList snapshot = files.build(); + if (snapshot.isEmpty()) + return false; + + /* Queue the same import operation used by the file chooser. */ + filesystemPanel.putFiles(parentPath, snapshot); + return true; + } catch (UnsupportedFlavorException | IOException e) + { + logger.warn("Cannot import dropped files", e); + return false; + } + } +} diff --git a/javatests/com/cowlark/fluxengine/gui/BUILD.bazel b/javatests/com/cowlark/fluxengine/gui/BUILD.bazel index 999970434..b403528a9 100644 --- a/javatests/com/cowlark/fluxengine/gui/BUILD.bazel +++ b/javatests/com/cowlark/fluxengine/gui/BUILD.bazel @@ -74,6 +74,7 @@ java_test( ], jvm_flags = ["-Djava.awt.headless=true"], deps = [ + "//java/com/cowlark/fluxengine/core", "//java/com/cowlark/fluxengine/data", "//java/com/cowlark/fluxengine/gui", "//java/com/cowlark/fluxengine/vfs", diff --git a/javatests/com/cowlark/fluxengine/gui/FilesystemPanelHeadlessTest.java b/javatests/com/cowlark/fluxengine/gui/FilesystemPanelHeadlessTest.java index 4f1c67f99..740c2b957 100644 --- a/javatests/com/cowlark/fluxengine/gui/FilesystemPanelHeadlessTest.java +++ b/javatests/com/cowlark/fluxengine/gui/FilesystemPanelHeadlessTest.java @@ -5,6 +5,7 @@ import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.when; +import com.cowlark.fluxengine.core.Bytes; import com.cowlark.fluxengine.data.Disk; import com.cowlark.fluxengine.data.Image; import com.cowlark.fluxengine.gui.FilesystemTreeTableModel.DirNode; @@ -19,6 +20,12 @@ import org.mockito.junit.MockitoJUnitRunner; import sprouts.Var; import javax.swing.SwingUtilities; +import javax.swing.TransferHandler; +import java.awt.datatransfer.Clipboard; +import java.awt.datatransfer.DataFlavor; +import java.io.File; +import java.nio.file.Files; +import java.util.List; import javax.swing.tree.TreePath; import java.util.HashSet; import java.util.Set; @@ -116,11 +123,111 @@ public void setUp() throws Exception public void backgroundDropTargetsRoot() throws Exception { SwingUtilities.invokeAndWait(() -> { - assertThat(panel.getFileDropDirectory(-1)).isSameInstanceAs(model.getRoot()); + TransferHandler.TransferSupport support = + org.mockito.Mockito.mock(TransferHandler.TransferSupport.class); + javax.swing.JTable.DropLocation location = + org.mockito.Mockito.mock(javax.swing.JTable.DropLocation.class); + when(support.isDrop()).thenReturn(true); + when(support.isDataFlavorSupported(DataFlavor.javaFileListFlavor)).thenReturn(true); + when(support.getSourceDropActions()).thenReturn(TransferHandler.COPY); + when(support.getDropLocation()).thenReturn(location); + when(location.getRow()).thenReturn(-1); + assertThat(panel.getTreeTable().getTransferHandler().canImport(support)).isTrue(); + org.mockito.Mockito.verify(support).setDropAction(TransferHandler.COPY); assertThat(panel.getTreeTable().getFillsViewportHeight()).isTrue(); }); } + @Test + public void draggingFilesExportsContentsAndRejectsDirectories() throws Exception + { + /* Populate a real file entry and supply its contents through the filesystem worker. */ + VfsPath filePath = VfsPath.of("/extract.txt"); + fakeFs = org.mockito.Mockito.spy(fakeFs); + fakeFs.addFile(filePath); + org.mockito.Mockito.doReturn(new Bytes("extracted contents")) + .when(fakeFs).getFile(filePath); + SwingUtilities.invokeAndWait(() -> model.resyncPaths(Set.of(VfsPath.root()))); + awaitIdle(); + awaitLoaded(VfsPath.root()); + Clipboard clipboard = new Clipboard("test"); + + /* Export using the same TransferHandler entry points used by native drag-and-drop. */ + SwingUtilities.invokeAndWait(() -> { + DirNode root = (DirNode) model.getRoot(); + for (int i = 0; i < root.getChildCount(); i++) + { + FilesystemTreeTableModel.FsNode child = + (FilesystemTreeTableModel.FsNode) root.getChildAt(i); + if (filePath.equals(child.vfsPath())) + panel.getTreeTable().getTreeSelectionModel().setSelectionPath( + model.buildTreePath(child)); + } + TransferHandler handler = panel.getTreeTable().getTransferHandler(); + assertThat(handler.getSourceActions(panel.getTreeTable())) + .isEqualTo(TransferHandler.COPY); + handler.exportToClipboard(panel.getTreeTable(), clipboard, TransferHandler.COPY); + assertThat(clipboard.getContents(null).isDataFlavorSupported( + DataFlavor.javaFileListFlavor)).isTrue(); + panel.getTreeTable().getTreeSelectionModel().clearSelection(); + }); + org.mockito.Mockito.verify(fakeFs, org.mockito.Mockito.never()).getFile(filePath); + List files = (List) clipboard.getData( + DataFlavor.javaFileListFlavor); + assertThat(files).hasSize(1); + File exported = (File) files.get(0); + assertThat(exported.getName()).isEqualTo("extract.txt"); + assertThat(Files.readString(exported.toPath())).isEqualTo("extracted contents"); + assertThat(clipboard.getData(DataFlavor.javaFileListFlavor)).isSameInstanceAs(files); + org.mockito.Mockito.verify(fakeFs, org.mockito.Mockito.times(1)).getFile(filePath); + + /* Directory selections must never initiate extraction. */ + SwingUtilities.invokeAndWait(() -> { + panel.getTreeTable().getTreeSelectionModel().setSelectionPath( + model.buildTreePath(model.findNode(VfsPath.of("/empty")))); + assertThat(panel.getTreeTable().getTransferHandler().getSourceActions(panel.getTreeTable())) + .isEqualTo(TransferHandler.NONE); + }); + Files.delete(exported.toPath()); + Files.delete(exported.toPath().getParent()); + } + + @Test + public void draggingFilesRejectsCollidingNames() throws Exception + { + fakeFs.addFile(VfsPath.of("/same.txt")); + fakeFs.addFile(VfsPath.of("/empty/same.txt")); + SwingUtilities.invokeAndWait(() -> model.resyncPaths(Set.of(VfsPath.root()))); + awaitIdle(); + awaitLoaded(VfsPath.root()); + SwingUtilities.invokeAndWait(() -> model.requestPopulate(model.findNode(VfsPath.of("/empty")))); + awaitIdle(); + awaitLoaded(VfsPath.of("/empty")); + + SwingUtilities.invokeAndWait(() -> { + panel.getTreeTable().getTreeSelectionModel().setSelectionPaths(new TreePath[] { + model.buildTreePath(model.findNode(VfsPath.root())).pathByAddingChild( + findFile(model.findNode(VfsPath.root()), "same.txt")), + model.buildTreePath(model.findNode(VfsPath.of("/empty"))).pathByAddingChild( + findFile(model.findNode(VfsPath.of("/empty")), "same.txt")) + }); + assertThat(panel.getTreeTable().getTransferHandler().getSourceActions(panel.getTreeTable())) + .isEqualTo(TransferHandler.NONE); + }); + } + + private Object findFile(DirNode directory, String name) + { + for (int i = 0; i < directory.getChildCount(); i++) + { + FilesystemTreeTableModel.FileNode file = + (FilesystemTreeTableModel.FileNode) directory.getChildAt(i); + if (file.getDirent().filename().equals(name)) + return file; + } + throw new AssertionError("Missing file: " + name); + } + private void awaitIdle() throws Exception { /* Pump the EDT. */ From 4b8fb3f53dd167d6811412b4f1c4679c765f81ff Mon Sep 17 00:00:00 2001 From: David Given Date: Sat, 24 Oct 2026 00:22:20 +0200 Subject: [PATCH 3/5] Refactor lots of things to use CompletableFutures rather than my home grown queue thingy. --- .../fluxengine/gui/ApplicationFrame.java | 108 ++++----- .../fluxengine/gui/ConfigurationPanel.java | 158 ++++++------ .../fluxengine/gui/FilesystemExecutor.java | 84 +++++++ .../fluxengine/gui/FilesystemPanel.java | 226 +++++++++--------- .../gui/FilesystemPanelTransferHandler.java | 53 ++-- .../gui/FilesystemTreeTableModel.java | 117 ++++----- java/com/cowlark/fluxengine/gui/Gui.java | 3 +- .../fluxengine/gui/ImageViewModel.java | 35 +-- java/com/cowlark/fluxengine/gui/LogPanel.java | 5 +- java/com/cowlark/fluxengine/gui/UiUtils.java | 112 ++++++--- .../cowlark/fluxengine/vfs/Filesystem.java | 4 +- .../fluxengine/vfs/FilesystemOperation.java | 4 - .../com/cowlark/fluxengine/gui/BUILD.bazel | 22 ++ .../gui/FilesystemExecutorTest.java | 116 +++++++++ .../gui/FilesystemPanelHeadlessTest.java | 143 ++++++++--- .../cowlark/fluxengine/gui/UiUtilsTest.java | 179 ++++++++++++-- 16 files changed, 910 insertions(+), 459 deletions(-) create mode 100644 java/com/cowlark/fluxengine/gui/FilesystemExecutor.java create mode 100644 javatests/com/cowlark/fluxengine/gui/FilesystemExecutorTest.java diff --git a/java/com/cowlark/fluxengine/gui/ApplicationFrame.java b/java/com/cowlark/fluxengine/gui/ApplicationFrame.java index 49c7c83ee..4711ff1b7 100644 --- a/java/com/cowlark/fluxengine/gui/ApplicationFrame.java +++ b/java/com/cowlark/fluxengine/gui/ApplicationFrame.java @@ -1,5 +1,6 @@ package com.cowlark.fluxengine.gui; +import static com.cowlark.fluxengine.gui.UiUtils.EDT_EXECUTOR; import static com.cowlark.fluxengine.gui.UiUtils.createSplitButton; import static com.cowlark.fluxengine.gui.UiUtils.sizePts; import static swingtree.UI.Axis.HORIZONTAL; @@ -30,7 +31,6 @@ import javax.swing.JPanel; import javax.swing.JSplitPane; import javax.swing.JTabbedPane; -import javax.swing.SwingUtilities; import java.awt.Color; import java.awt.Toolkit; import java.net.URL; @@ -68,62 +68,55 @@ public class ApplicationFrame extends JFrame setTitle("FluxEngine"); int leftPaneWidth = configurationPanel.getPreferredSize().width; - UIForTabbedPane leftPane = tabbedPane().add(tab("Configuration").add(scrollPane() - .withPrefWidth(leftPaneWidth) - .withMinSize(leftPaneWidth, 0) - .withMaxSize(leftPaneWidth, Integer.MAX_VALUE) - .withVerticalScrollBarPolicy(UI.Active.ALWAYS) - .add(of(configurationPanel)))); - - UIForSplitPane topPane = - splitPane(HORIZONTAL) - .peek(pane -> { - pane.setResizeWeight(1.0); - }) - .add( - TOP, - tabbedPane() - .withMinSize(100, 0) - .add(tab("Image").add(of(imagePanel))) - .add(tab("Log").add(of(logPanel))) - .add(tab("Files").add(of(filesystemPanel)))) - .add( - BOTTOM, - tabbedPane() - .withMinSize(100, 0) - .add(tab("Visualiser").add(of(visualiserPanel)))); - - UIForPanel controlPanel = - panel("wrap 4, center, nogrid") - .add("aligny center", label("I want to")) - .add( - "aligny center", - comboBox(currentWorkflow, - Workflow.values(), - Workflow::getDisplayName)) - .add(label(":")) - .add( - "aligny center", - panel("fill, ins 0") - .withSizeExactly(sizePts(CARD_WIDTH_PTS, CARD_HEIGHT_PTS)) - .add( - "align center", - currentWorkflow, - createControlPanelCard(model))) - .add( - "aligny center", - button("Stop") - .isEnabledIf(model.getBusy()) - .withForeground(Color.RED) - .onClick(model::onEmergencyStop)); + UIForTabbedPane leftPane = + tabbedPane().add(tab("Configuration").add(scrollPane() + .withPrefWidth(leftPaneWidth) + .withMinSize(leftPaneWidth, 0) + .withMaxSize(leftPaneWidth, Integer.MAX_VALUE) + .withVerticalScrollBarPolicy(UI.Active.ALWAYS) + .add(of(configurationPanel)))); + + UIForSplitPane topPane = splitPane(HORIZONTAL).peek(pane -> { + pane.setResizeWeight(1.0); + }) + .add( + TOP, + tabbedPane().withMinSize(100, 0) + .add(tab("Image").add(of(imagePanel))) + .add(tab("Log").add(of(logPanel))) + .add(tab("Files").add(of(filesystemPanel)))) + .add( + BOTTOM, + tabbedPane().withMinSize(100, 0) + .add(tab("Visualiser").add(of(visualiserPanel)))); + + UIForPanel controlPanel = panel("wrap 4, center, nogrid") + .add("aligny center", label("I want to")) + .add( + "aligny center", + comboBox(currentWorkflow, Workflow.values(), Workflow::getDisplayName)) + .add(label(":")) + .add( + "aligny center", + panel("fill, ins 0") + .withSizeExactly(sizePts(CARD_WIDTH_PTS, CARD_HEIGHT_PTS)) + .add( + "align center", + currentWorkflow, + createControlPanelCard(model))) + .add( + "aligny center", + button("Stop") + .isEnabledIf(model.getBusy()) + .withForeground(Color.RED) + .onClick(model::onEmergencyStop)); UIForTabbedPane bottomPane = tabbedPane().add(tab("Summary").add(panel("fillx, wrap 1") .add("growx, pushx, hmin 100pt", of(summaryPanel)) .add("growx", controlPanel))); - UIForSplitPane outerSplit = splitPane(HORIZONTAL) - .peek(pane -> { + UIForSplitPane outerSplit = splitPane(HORIZONTAL).peek(pane -> { pane.setResizeWeight(0.0); pane.setDividerSize(5); pane.setContinuousLayout(true); @@ -134,25 +127,20 @@ public class ApplicationFrame extends JFrame * width without needing a manual resize or sash touch. * withPrefWidth scales via UI.scale, so the divider must * be scaled the same way. */ - SwingUtilities.invokeLater(() -> pane.setDividerLocation( - UI.scale(leftPaneWidth))); + EDT_EXECUTOR.execute(() -> pane.setDividerLocation(UI.scale(leftPaneWidth))); }) .add(TOP, leftPane) .add( BOTTOM, - splitPane(Axis.VERTICAL) - .peek(pane -> pane.setResizeWeight(1.0)) + splitPane(Axis.VERTICAL).peek(pane -> pane.setResizeWeight(1.0)) .add(TOP, topPane) .add(BOTTOM, bottomPane)); - of(this) - .withOnCloseOperation(UI.OnWindowClose.DISPOSE) - .onClose(it -> { + of(this).withOnCloseOperation(UI.OnWindowClose.DISPOSE).onClose(it -> { model.onEmergencyStop(); dispose(); System.exit(0); - }) - .peek(frame -> { + }).peek(frame -> { frame.setJMenuBar(ApplicationMenu.createMenu()); }) .add(outerSplit); diff --git a/java/com/cowlark/fluxengine/gui/ConfigurationPanel.java b/java/com/cowlark/fluxengine/gui/ConfigurationPanel.java index 189c2338e..66fac3b82 100644 --- a/java/com/cowlark/fluxengine/gui/ConfigurationPanel.java +++ b/java/com/cowlark/fluxengine/gui/ConfigurationPanel.java @@ -5,6 +5,7 @@ import static com.cowlark.fluxengine.config.OptionApplicabilityHint.HARDWARE_SOURCESINK; import static com.cowlark.fluxengine.gui.PreferencesReaderWriter.DEVICE_FLUXFILE; import static com.cowlark.fluxengine.gui.PreferencesReaderWriter.DEVICE_SERIALPORT; +import static com.cowlark.fluxengine.gui.UiUtils.EDT_EXECUTOR; import static com.google.common.base.Strings.isNullOrEmpty; import static com.google.common.collect.ImmutableMap.toImmutableMap; import static javax.swing.JFileChooser.APPROVE_OPTION; @@ -13,7 +14,6 @@ import static swingtree.UI.panel; import static swingtree.UIFactoryMethods.button; import static swingtree.UIFactoryMethods.comboBox; -import static swingtree.UIFactoryMethods.scrollPane; import static swingtree.UIFactoryMethods.separator; import static swingtree.UIFactoryMethods.textArea; @@ -21,8 +21,8 @@ import com.cowlark.fluxengine.config.OptionApplicabilityHint; import com.cowlark.fluxengine.config.OptionGroupProto; import com.cowlark.fluxengine.config.OptionProto; -import com.cowlark.fluxengine.usb.UsbFinder; import com.cowlark.fluxengine.data.Formats; +import com.cowlark.fluxengine.usb.UsbFinder; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; @@ -45,8 +45,8 @@ import javax.swing.JViewport; import javax.swing.ListCellRenderer; import javax.swing.Scrollable; -import javax.swing.SwingUtilities; import javax.swing.border.EmptyBorder; +import javax.swing.event.DocumentListener; import java.awt.Color; import java.awt.Container; import java.awt.Dimension; @@ -199,9 +199,8 @@ public Dimension getMaximumSize() { Dimension max = super.getMaximumSize(); int twoColumnMax = 340 + getInsets().left + getInsets().right; - if (max.width > twoColumnMax) { + if (max.width > twoColumnMax) max.width = twoColumnMax; - } return max; } @@ -287,11 +286,13 @@ private UIForPanel emitOptions( Var selected = options.zoomTo(getOption(optionGroup), withOption(optionGroup)); - panel = panel.add(LABEL_FORMAT, label(commentRenderer(optionGroup.getComment()))).add( - SETTING_FORMAT, comboBox( - selected, - optionGroup.getOptionList(), - ConfigurationPanel::optionRenderer)); + panel = panel + .add(LABEL_FORMAT, label(commentRenderer(optionGroup.getComment()))) + .add( + SETTING_FORMAT, comboBox( + selected, + optionGroup.getOptionList(), + ConfigurationPanel::optionRenderer)); } for (OptionProto option : config.getOptionList()) @@ -369,22 +370,26 @@ SETTING_FORMAT, comboBox( applicabilities.add(HARDWARE_SOURCESINK); if (applicabilities.contains(FLUXFILE_SOURCESINK)) - panel = panel.add(LABEL_FORMAT, label("Flux file:")).add( - SETTING_FORMAT, - UI.textField(model.getSelectedFluxFile()).isEditableIf(false).peek(field -> { - /* Show the tail of an over-long path: keep - * the caret pinned to the end, since - * alignment has no effect once the content - * overflows. */ - Runnable pin = - () -> SwingUtilities.invokeLater(() -> field.setCaretPosition(field - .getDocument() - .getLength())); - pin.run(); - Viewable - .cast(model.getSelectedFluxFile()) - .onChange(From.ALL, it -> pin.run()); - })); + panel = panel + .add(LABEL_FORMAT, label("Flux file:")) + .add( + SETTING_FORMAT, + UI + .textField(model.getSelectedFluxFile()) + .isEditableIf(false) + .peek(field -> { + /* Show the tail of an over-long path: keep + * the caret pinned to the end, since + * alignment has no effect once the content + * overflows. */ + Runnable pin = + () -> EDT_EXECUTOR.execute(() -> field.setCaretPosition( + field.getDocument().getLength())); + pin.run(); + Viewable + .cast(model.getSelectedFluxFile()) + .onChange(From.ALL, it -> pin.run()); + })); if (model.getSelectedDevice().get().equals(DEVICE_SERIALPORT)) panel = panel @@ -392,9 +397,13 @@ SETTING_FORMAT, comboBox( .add(SETTING_FORMAT, UI.textField(model.getSelectedSerialPort())); if (applicabilities.contains(HARDWARE_SOURCESINK)) - panel = panel.add(LABEL_FORMAT, label("Drive:")).add( - SETTING_FORMAT, - comboBox(model.getSelectedDrive(), DRIVES, ConfigurationPanel::driveRenderer)); + panel = panel + .add(LABEL_FORMAT, label("Drive:")) + .add( + SETTING_FORMAT, comboBox( + model.getSelectedDrive(), + DRIVES, + ConfigurationPanel::driveRenderer)); panel = emitOptions(panel, model.getOptionsForDevice(), GLOBAL_CONFIG, applicabilities); return panel; @@ -405,53 +414,50 @@ private UIForPanel buildAdvancedPane(UIForPanel { - /* Keep the outer scroll pane's preferred size in - * sync with the document so the area contracts - * to the text height (and re-expands when the - * viewport is larger via Scrollable). */ - ta.getDocument().addDocumentListener( - new javax.swing.event.DocumentListener() - { - private void update() - { - SwingUtilities.invokeLater(() -> { - revalidate(); - repaint(); - Container viewport = getParent(); - if (viewport instanceof JViewport) - viewport.revalidate(); - Container scroll = viewport != null - ? viewport.getParent() - : null; - if (scroll instanceof JScrollPane) - scroll.revalidate(); - }); - } - - @Override - public void insertUpdate( - javax.swing.event.DocumentEvent e) - { - update(); - } - - @Override - public void removeUpdate( - javax.swing.event.DocumentEvent e) - { - update(); - } - - @Override - public void changedUpdate( - javax.swing.event.DocumentEvent e) - { - update(); - } - }); - })); + "span 2, grow, push", textArea(model.getAdvancedSettings()).peek(ta -> { + /* Keep the outer scroll pane's preferred size in + * sync with the document so the area contracts + * to the text height (and re-expands when the + * viewport is larger via Scrollable). */ + ta.getDocument().addDocumentListener(new DocumentListener() + { + private void update() + { + EDT_EXECUTOR.execute(() -> { + revalidate(); + repaint(); + Container viewport = getParent(); + if (viewport instanceof JViewport) + viewport.revalidate(); + Container scroll = + viewport != null ? viewport.getParent() : null; + if (scroll instanceof JScrollPane) + scroll.revalidate(); + }); + } + + @Override + public void insertUpdate( + javax.swing.event.DocumentEvent e) + { + update(); + } + + @Override + public void removeUpdate( + javax.swing.event.DocumentEvent e) + { + update(); + } + + @Override + public void changedUpdate( + javax.swing.event.DocumentEvent e) + { + update(); + } + }); + })); } /* A renderer which shows each device entry with the placeholder icon diff --git a/java/com/cowlark/fluxengine/gui/FilesystemExecutor.java b/java/com/cowlark/fluxengine/gui/FilesystemExecutor.java new file mode 100644 index 000000000..0f79d9908 --- /dev/null +++ b/java/com/cowlark/fluxengine/gui/FilesystemExecutor.java @@ -0,0 +1,84 @@ +package com.cowlark.fluxengine.gui; + +import static com.google.common.base.Preconditions.checkState; +import static java.util.Objects.requireNonNull; + +import com.cowlark.fluxengine.core.EmergencyStopException; +import com.cowlark.fluxengine.core.FluxEngineException; +import com.cowlark.fluxengine.vfs.Filesystem; +import java.io.UncheckedIOException; +import java.util.concurrent.BlockingQueue; +import java.util.concurrent.Executor; +import java.util.concurrent.LinkedBlockingQueue; +import java.util.concurrent.RejectedExecutionException; +import java.util.function.Consumer; + +/* Runs queued tasks on the thread owned by a mounted FilesystemOperation. Each mount uses a + * separate executor so pending tasks cannot leak into a later filesystem session. */ +class FilesystemExecutor implements Executor +{ + private final BlockingQueue queue = new LinkedBlockingQueue<>(); + private volatile Filesystem filesystem; + private boolean accepting = true; + + @Override + public synchronized void execute(Runnable task) + { + if (!accepting) + throw new RejectedExecutionException("Filesystem executor has stopped"); + queue.add(requireNonNull(task)); + } + + Filesystem getFilesystem() + { + Filesystem current = filesystem; + checkState(current != null, "Filesystem is not mounted"); + return current; + } + + void run(Filesystem filesystem, Consumer onError) + { + this.filesystem = requireNonNull(filesystem); + try + { + while (true) + { + try + { + queue.take().run(); + } catch (EmergencyStopException e) + { + throw e; + } catch (UncheckedIOException e) + { + onError.accept(e.getCause()); + } catch (FluxEngineException e) + { + onError.accept(e); + } + } + } catch (InterruptedException e) + { + Thread.currentThread().interrupt(); + } finally + { + synchronized (this) + { + accepting = false; + this.filesystem = null; + queue.clear(); + } + } + } + + synchronized void stop() + { + if (!accepting) + return; + accepting = false; + /* Wake an idle worker and exit through FilesystemOperation's normal cleanup path. */ + queue.add(() -> { + throw new EmergencyStopException(); + }); + } +} diff --git a/java/com/cowlark/fluxengine/gui/FilesystemPanel.java b/java/com/cowlark/fluxengine/gui/FilesystemPanel.java index 181b7baf5..466d511a2 100644 --- a/java/com/cowlark/fluxengine/gui/FilesystemPanel.java +++ b/java/com/cowlark/fluxengine/gui/FilesystemPanel.java @@ -1,5 +1,8 @@ package com.cowlark.fluxengine.gui; +import static com.cowlark.fluxengine.gui.UiUtils.EDT_EXECUTOR; +import static com.cowlark.fluxengine.gui.UiUtils.reportError; +import static com.cowlark.fluxengine.gui.UiUtils.wrapIOException; import static com.cowlark.fluxengine.vfs.Filesystem.Capability.OP_CREATEDIR; import static com.cowlark.fluxengine.vfs.Filesystem.Capability.OP_DELETE; import static com.cowlark.fluxengine.vfs.Filesystem.Capability.OP_GETDIRENT; @@ -22,8 +25,8 @@ import com.cowlark.fluxengine.gui.FilesystemTreeTableModel.FileNode; import com.cowlark.fluxengine.gui.FilesystemTreeTableModel.FsNode; import com.cowlark.fluxengine.vfs.FileAttributes; +import com.cowlark.fluxengine.vfs.Filesystem; import com.cowlark.fluxengine.vfs.Filesystem.Capability; -import com.cowlark.fluxengine.vfs.Filesystem.Dirent; import com.cowlark.fluxengine.vfs.VfsPath; import com.google.common.collect.ImmutableList; import com.google.common.collect.Iterables; @@ -42,17 +45,18 @@ import javax.swing.JFileChooser; import javax.swing.JOptionPane; import javax.swing.JPanel; -import javax.swing.TransferHandler; import javax.swing.SwingUtilities; +import javax.swing.TransferHandler; import javax.swing.event.TreeExpansionEvent; import javax.swing.event.TreeExpansionListener; import javax.swing.tree.TreePath; import javax.swing.tree.TreeSelectionModel; -import java.awt.event.ActionEvent; import java.awt.GraphicsEnvironment; +import java.awt.event.ActionEvent; import java.io.File; import java.util.HashSet; import java.util.Set; +import java.util.concurrent.CompletableFuture; public class FilesystemPanel extends JPanel { @@ -157,35 +161,30 @@ public void treeCollapsed(TreeExpansionEvent event) filesSelected, (mounted, files) -> isValidSelection(files) && (files.size() >= 1) && mounted); - UI - .of(this) - .withLayout("fill, wrap 1, insets 5, hidemode 3") + UI.of(this).withLayout("fill, wrap 1, insets 5, hidemode 3") .add( - "growx", panel("insets 2") + "growx", + panel("insets 2") .add( - "align left", - button("Get") + "align left", button("Get") .isEnabledIf(ifCapability( canDoMultiFileOperation, OP_GETFILE)) .onClick(this::getFile)) .add( - "align left", - button("Put") + "align left", button("Put") .isEnabledIf(ifCapability( canDoSingleFileOperation, OP_PUTFILE)) .onClick(this::putFiles)) .add( - "align left", - button("Rename") + "align left", button("Rename") .isEnabledIf(ifCapability( canDoSingleFileOperation, OP_MOVE)) .onClick(this::renameFile)) .add( - "align left", - button("Delete") + "align left", button("Delete") .isEnabledIf(ifCapability( canDoMultiFileOperation, OP_DELETE)) @@ -210,7 +209,8 @@ public void treeCollapsed(TreeExpansionEvent event) .onClick(this::viewFile))) .add( "grow, push", - scrollPane().add(UI.of(treeTable)).isVisibleIf(allowedWhenMounted)) + scrollPane() + .add(UI.of(treeTable)).isVisibleIf(allowedWhenMounted)) .add( "grow, push, align " + "center, w 100%!", html("""
Filesystem not mounted @@ -218,7 +218,8 @@ public void treeCollapsed(TreeExpansionEvent event) .withHorizontalAlignment(UI.HorizontalAlignment.CENTER) .isVisibleIf(notMounted)) .add( - "growx", panel("insets 2") + "growx", + panel("insets 2") .add( "align left", button("Mount") @@ -232,23 +233,7 @@ public void treeCollapsed(TreeExpansionEvent event) .onClick(delegate -> { Set snap = snapshotExpandedVfsPaths(); treeTableModel.discard(); - treeTableModel.queueFilesystemOperation(fs -> SwingUtilities.invokeLater( - () -> { - for (VfsPath p : snap) - { - DirNode n = - treeTableModel.findNode( - p); - if (n == null || - n.getChildCount() == 0) - continue; - TreePath tp = - treeTableModel.buildTreePath( - n); - if (!treeTable.isExpanded(tp)) - treeTable.expandPath(tp); - } - })); + restoreExpandedPaths(snap); })) .add( "align left", button("Commit") @@ -307,13 +292,11 @@ private void getFile( { VfsPath vfsPath = node.getDirent().path(); String filename = node.getDirent().filename(); - UiUtils.promptAndSave( - this, "Save file", filename, saver -> { - treeTableModel.queueFilesystemOperation(fs -> { - Bytes bytes = fs.getFile(vfsPath); - saver.accept(bytes); - }); - }); + UiUtils.promptAndSave(this, "Save file", filename).completeAsync( + wrapIOException(() -> { + Filesystem fs = treeTableModel.getFilesystem(); + return fs.getFile(vfsPath); + }), treeTableModel.getExecutor()); return; } } @@ -322,13 +305,11 @@ private void getFile( ImmutableList snapshot = stream(paths) .map(p -> ((FileNode) p.getLastPathComponent()).getDirent().path()) .collect(toImmutableList()); - UiUtils.promptAndSave( - this, "Save multiple files", "files.zip", saver -> { - treeTableModel.queueFilesystemOperation(fs -> { - Bytes bytes = fs.getFiles(fs, snapshot); - saver.accept(bytes); - }); - }); + UiUtils.promptAndSave(this, "Save multiple files", "files.zip").completeAsync( + wrapIOException(() -> { + Filesystem fs = treeTableModel.getFilesystem(); + return fs.getFiles(fs, snapshot); + }), treeTableModel.getExecutor()); } private void viewFile( @@ -337,13 +318,12 @@ private void viewFile( TreePath path = Iterables.getOnlyElement(filesSelected.get()); FileNode file = (FileNode) path.getLastPathComponent(); VfsPath vfsPath = file.getDirent().path(); - treeTableModel.queueFilesystemOperation(fs -> { - Bytes data = fs.getFile(vfsPath); - SwingUtilities.invokeLater(() -> FileViewerDialogue.show( - this, - vfsPath.toString(), - data)); - }); + CompletableFuture.supplyAsync( + wrapIOException(() -> treeTableModel.getFilesystem().getFile(vfsPath)), + treeTableModel.getExecutor()).thenAcceptAsync( + data -> { + FileViewerDialogue.show(this, vfsPath.toString(), data); + }, EDT_EXECUTOR).exceptionallyAsync(reportError("Cannot view file"), EDT_EXECUTOR); } private TransferHandler createFileDropHandler() @@ -371,16 +351,20 @@ private void putFiles( void putFiles(VfsPath parentPath, ImmutableList files) { /* Read host files and write their contents on the filesystem worker thread. */ - treeTableModel.queueFilesystemOperation(fs -> { - for (File file : files) - { - Bytes data = Bytes.readFromFile(file.toPath()); - String leafName = file.getName(); - VfsPath vfsName = parentPath.resolve(leafName); - fs.putFile(vfsName, data); - } - resyncAllExpandedAsync(); - }); + CompletableFuture + .runAsync( + wrapIOException(() -> { + Filesystem fs = treeTableModel.getFilesystem(); + for (File file : files) + { + Bytes data = Bytes.readFromFile(file.toPath()); + String leafName = file.getName(); + VfsPath vfsName = parentPath.resolve(leafName); + fs.putFile(vfsName, data); + } + }), treeTableModel.getExecutor()) + .thenRunAsync(this::resyncAllExpandedAfterMutation, EDT_EXECUTOR) + .exceptionallyAsync(reportError("Cannot put files"), EDT_EXECUTOR); } private void deleteFiles( @@ -391,11 +375,15 @@ private void deleteFiles( .map(p -> ((FileNode) p.getLastPathComponent()).getDirent().path()) .collect(toImmutableList()); - treeTableModel.queueFilesystemOperation(fs -> { - for (VfsPath vfsPath : vfsPaths) - fs.deleteFileRecursively(vfsPath); - resyncAllExpandedAsync(); - }); + CompletableFuture + .runAsync( + wrapIOException(() -> { + Filesystem fs = treeTableModel.getFilesystem(); + for (VfsPath vfsPath : vfsPaths) + fs.deleteFileRecursively(vfsPath); + }), treeTableModel.getExecutor()) + .thenRunAsync(this::resyncAllExpandedAfterMutation, EDT_EXECUTOR) + .exceptionallyAsync(reportError("Cannot delete files"), EDT_EXECUTOR); } private void infoFile( @@ -404,20 +392,25 @@ private void infoFile( TreePath path = Iterables.getOnlyElement(filesSelected.get()); VfsPath vfsPath = ((FileNode) path.getLastPathComponent()).getDirent().path(); - treeTableModel.queueFilesystemOperation(fs -> { - Dirent de = fs.getDirent(vfsPath); - SwingUtilities.invokeLater(() -> FileInfoDialogue.show( - this, - "File info: " + de.path().toString(), - de - .attributes() - .entrySet() - .stream() - .collect(toImmutableMap( - e -> FileAttributes.getHumanName(e.getKey()), - e -> e.getValue())))); - }); - + CompletableFuture + .supplyAsync( + wrapIOException(() -> treeTableModel.getFilesystem().getDirent(vfsPath)), + treeTableModel.getExecutor()) + .thenAcceptAsync( + de -> { + FileInfoDialogue.show( + this, + "File info: " + de.path().toString(), + de + .attributes() + .entrySet() + .stream() + .collect(toImmutableMap( + e -> FileAttributes.getHumanName( + e.getKey()), + e -> e.getValue()))); + }, EDT_EXECUTOR) + .exceptionallyAsync(reportError("Cannot read file info"), EDT_EXECUTOR); } private void createDirectory( @@ -430,10 +423,14 @@ private void createDirectory( if (childName == null || childName.isBlank()) return; VfsPath childVfsPath = parentPath.resolve(childName); - treeTableModel.queueFilesystemOperation(fs -> { - fs.createDirectory(childVfsPath); - resyncAllExpandedAsync(); - }); + CompletableFuture + .runAsync( + wrapIOException(() -> { + Filesystem fs = treeTableModel.getFilesystem(); + fs.createDirectory(childVfsPath); + }), treeTableModel.getExecutor()) + .thenRunAsync(this::resyncAllExpandedAfterMutation, EDT_EXECUTOR) + .exceptionallyAsync(reportError("Cannot create directory"), EDT_EXECUTOR); } private DirNode getParentNodeOfSelection() @@ -501,24 +498,28 @@ void resyncAllExpandedAfterMutation() treeTableModel.mutated(); /* Re-expand snapshot paths that were auto-collapsed when empty (isLeaf became true) but * now have children after the resync. */ - treeTableModel.queueFilesystemOperation(fs -> SwingUtilities.invokeLater(() -> { - for (VfsPath p : snap) - { - DirNode n = treeTableModel.findNode(p); - if (n == null || n.getChildCount() == 0) - continue; - TreePath tp = treeTableModel.buildTreePath(n); - if (!treeTable.isExpanded(tp)) - treeTable.expandPath(tp); - } - })); + restoreExpandedPaths(snap); } - void resyncAllExpandedAsync() + private CompletableFuture restoreExpandedPaths(Set snapshot) { - /* Called from FS thread after a mutation; schedule EDT resync that preserves expansion - * and re-expands empty dirs that gained children. */ - SwingUtilities.invokeLater(this::resyncAllExpandedAfterMutation); + /* Attach the continuation before submitting the barrier, so re-expansion is queued + * behind earlier listing results even when the EDT executor runs inline. */ + CompletableFuture barrier = new CompletableFuture<>(); + CompletableFuture restored = barrier.thenRunAsync( + () -> { + for (VfsPath path : snapshot) + { + DirNode node = treeTableModel.findNode(path); + if (node == null || node.getChildCount() == 0) + continue; + TreePath treePath = treeTableModel.buildTreePath(node); + if (!treeTable.isExpanded(treePath)) + treeTable.expandPath(treePath); + } + }, EDT_EXECUTOR); + barrier.completeAsync(() -> null, treeTableModel.getExecutor()); + return restored; } private void renameFile( @@ -534,10 +535,14 @@ private void renameFile( VfsPath parentPath = parent.getDirent().path(); VfsPath oldPath = child.getDirent().path(); VfsPath newVfsPath = parentPath.resolve(newChild); - treeTableModel.queueFilesystemOperation(fs -> { - fs.moveFile(oldPath, newVfsPath); - resyncAllExpandedAsync(); - }); + CompletableFuture + .runAsync( + wrapIOException(() -> { + Filesystem fs = treeTableModel.getFilesystem(); + fs.moveFile(oldPath, newVfsPath); + }), treeTableModel.getExecutor()) + .thenRunAsync(this::resyncAllExpandedAfterMutation, EDT_EXECUTOR) + .exceptionallyAsync(reportError("Cannot rename file"), EDT_EXECUTOR); } /* A valid selection has unique filenames and no selected path that is an ancestor of @@ -547,8 +552,8 @@ private static boolean isValidSelection(Tuple paths) Set filenames = new HashSet<>(); for (TreePath path : paths) { - if (!(path.getLastPathComponent() instanceof FileNode file) - || !filenames.add(file.getDirent().filename())) + if (!(path.getLastPathComponent() instanceof FileNode file) || + !filenames.add(file.getDirent().filename())) return false; } for (int i = 0; i < paths.size(); i++) @@ -568,10 +573,11 @@ private static boolean isValidSelection(Tuple paths) private void onMount(ComponentDelegate delegate) { treeTableModel.mount(); + /* Root is lazily populated in the model, but the JXTreeTable clears expansion on * setRoot. Expand it after the EDT has processed the structure change so the initial * file list becomes visible without requiring a manual click. */ - SwingUtilities.invokeLater(() -> { + EDT_EXECUTOR.execute(() -> { Object root = treeTableModel.getRoot(); if (root != null) { diff --git a/java/com/cowlark/fluxengine/gui/FilesystemPanelTransferHandler.java b/java/com/cowlark/fluxengine/gui/FilesystemPanelTransferHandler.java index ebb2af349..40527fab6 100644 --- a/java/com/cowlark/fluxengine/gui/FilesystemPanelTransferHandler.java +++ b/java/com/cowlark/fluxengine/gui/FilesystemPanelTransferHandler.java @@ -5,6 +5,7 @@ import static com.cowlark.fluxengine.vfs.Filesystem.FileType.IS_FILE; import com.cowlark.fluxengine.core.Bytes; +import com.cowlark.fluxengine.vfs.Filesystem; import com.cowlark.fluxengine.vfs.VfsPath; import com.google.common.collect.ImmutableList; import org.slf4j.Logger; @@ -29,7 +30,8 @@ class FilesystemPanelTransferHandler extends TransferHandler { - private static final Logger logger = LoggerFactory.getLogger(FilesystemPanelTransferHandler.class); + private static final Logger logger = + LoggerFactory.getLogger(FilesystemPanelTransferHandler.class); private final FilesystemPanel filesystemPanel; @@ -89,32 +91,26 @@ private synchronized CompletableFuture> extract() { if (extracted != null) return extracted; - extracted = new CompletableFuture<>(); - filesystemPanel.treeTableModel.queueFilesystemOperation(fs -> { - try + extracted = CompletableFuture.supplyAsync(UiUtils.wrapIOException(() -> { + Filesystem fs = filesystemPanel.treeTableModel.getFilesystem(); + /* Keep the files until application exit because native drop targets may + * copy them later. */ + Path directory = Files.createTempDirectory("fluxengine-export-"); + directory.toFile().deleteOnExit(); + ImmutableList.Builder files = ImmutableList.builder(); + for (VfsPath path : paths) { - /* Keep the files until application exit because native drop targets may - * copy them later. */ - Path directory = Files.createTempDirectory("fluxengine-export-"); - directory.toFile().deleteOnExit(); - ImmutableList.Builder files = ImmutableList.builder(); - for (VfsPath path : paths) - { - String name = path.getName(); - Path output = directory.resolve(name).normalize(); - if (!directory.equals(output.getParent())) - throw new IOException("Invalid export filename: " + name); - output.toFile().deleteOnExit(); - Bytes bytes = fs.getFile(path); - bytes.writeToFile(output); - files.add(output.toFile()); - } - extracted.complete(files.build()); - } catch (Exception e) - { - extracted.completeExceptionally(e); + String name = path.getName(); + Path output = directory.resolve(name).normalize(); + if (!directory.equals(output.getParent())) + throw new IOException("Invalid export filename: " + name); + output.toFile().deleteOnExit(); + Bytes bytes = fs.getFile(path); + bytes.writeToFile(output); + files.add(output.toFile()); } - }); + return files.build(); + }), filesystemPanel.treeTableModel.getExecutor()); return extracted; } @@ -146,8 +142,11 @@ public Object getTransferData(DataFlavor flavor) * the EDT. */ SecondaryLoop loop = Toolkit.getDefaultToolkit().getSystemEventQueue().createSecondaryLoop(); - result.whenComplete((files, error) -> SwingUtilities.invokeLater(() -> loop.exit())); - loop.enter(); + result.whenCompleteAsync((files, error) -> loop.exit(), UiUtils.EDT_EXECUTOR); + + /* The executor can run inline if extraction finished before registration. */ + if (!result.isDone()) + loop.enter(); } try { diff --git a/java/com/cowlark/fluxengine/gui/FilesystemTreeTableModel.java b/java/com/cowlark/fluxengine/gui/FilesystemTreeTableModel.java index 2f03aafe4..3b8bbf903 100644 --- a/java/com/cowlark/fluxengine/gui/FilesystemTreeTableModel.java +++ b/java/com/cowlark/fluxengine/gui/FilesystemTreeTableModel.java @@ -1,15 +1,16 @@ package com.cowlark.fluxengine.gui; +import static com.cowlark.fluxengine.gui.UiUtils.EDT_EXECUTOR; import static com.cowlark.fluxengine.vfs.Filesystem.FileType.IS_DIR; import static com.google.common.collect.ImmutableList.toImmutableList; +import static com.google.common.base.Preconditions.checkState; -import com.cowlark.fluxengine.core.EmergencyStopException; import com.cowlark.fluxengine.data.Image; import com.cowlark.fluxengine.vfs.FileAttributes; +import com.cowlark.fluxengine.vfs.Filesystem; import com.cowlark.fluxengine.vfs.Filesystem.Capability; import com.cowlark.fluxengine.vfs.Filesystem.Dirent; import com.cowlark.fluxengine.vfs.FilesystemAttributes; -import com.cowlark.fluxengine.vfs.FilesystemOperation.FilesystemCaller; import com.cowlark.fluxengine.vfs.VfsPath; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; @@ -30,8 +31,8 @@ import java.util.List; import java.util.Map; import java.util.Set; -import java.util.concurrent.BlockingQueue; -import java.util.concurrent.LinkedBlockingQueue; +import java.util.concurrent.Executor; +import java.util.concurrent.CompletableFuture; import java.util.function.Function; import java.util.function.Supplier; @@ -44,7 +45,7 @@ * stores an {@code expanded} flag and never calls {@code expandPath}. *
  • Structure (which files exist, sort order, dirents) is owned by this model, but the * authoritative data is on the filesystem thread. All {@code Filesystem} I/O runs on the - * filesystem thread via {@link #queueFilesystemOperation}; all model mutations run on the + * filesystem thread via {@link #getExecutor}; all model mutations run on the * EDT.
    • *
  • Each {@code DirNode} has a {@link LoadState} driving the placeholder. A directory is only * listed when it is expanded in the view. {@link #resyncNode} re-lists a single already- @@ -66,7 +67,11 @@ public class FilesystemTreeTableModel extends DefaultTreeTableModel .build(); final ImageViewModel model; - final BlockingQueue queue = new LinkedBlockingQueue<>(); + private FilesystemExecutor filesystemExecutor = new FilesystemExecutor(); + private final Executor executor = task -> { + getFilesystem(); + filesystemExecutor.execute(task); + }; Supplier> expandedSupplier; @Getter private final Var isMounted = Var.of(false); @@ -202,8 +207,9 @@ public void setExpandedSupplier(Supplier> supplier) public void mount() { Image image = model.getDisk().mapTo(Image.class, disk -> disk.image).get(); + filesystemExecutor = new FilesystemExecutor(); model.startFilesystemOperation( - queue, image, () -> { + filesystemExecutor, image, () -> { bytesTotal.set(0); bytesUsed.set(0); isMounted.set(false); @@ -218,7 +224,7 @@ public void mount() * can expand it to trigger the first list. */ setRoot(root); insertNodeInto(new PlaceholderNode(), root, 0); - isMounted.set(true); + filesystemExecutor.execute(() -> isMounted.set(true)); mutated(); /* The root is visible expanded by default (JXTreeTable shows the placeholder row). * Populate it automatically so the initial view is not stuck on "Loading..." — still @@ -228,27 +234,37 @@ public void mount() if (SwingUtilities.isEventDispatchThread()) reset.run(); else - SwingUtilities.invokeLater(reset); + EDT_EXECUTOR.execute(reset); } /* Must be called from filesystem thread */ public void mutated() { updateFreeSpace(); - queue.add(fs -> { + filesystemExecutor.execute(() -> { + Filesystem fs = filesystemExecutor.getFilesystem(); capabilities.set(ValueSet.of(Capability.class, fs.getCapabilities())); hasPendingChanges.set(fs.needsFlushing()); }); } - public void queueFilesystemOperation(FilesystemCaller caller) + /* Access the filesystem from callbacks scheduled on the filesystem executor. */ + public Filesystem getFilesystem() { - queue.add(caller); + checkState(isMounted.get(), "Filesystem is not mounted"); + return filesystemExecutor.getFilesystem(); + } + + public Executor getExecutor() + { + getFilesystem(); + return executor; } void updateFreeSpace() { - queue.add(fs -> { + filesystemExecutor.execute(UiUtils.wrapIOException(() -> { + Filesystem fs = filesystemExecutor.getFilesystem(); ImmutableMap attrs = fs.getFilesystemMetadata(); try { @@ -264,12 +280,14 @@ void updateFreeSpace() bytesTotal.set(0); bytesUsed.set(0); } - }); + })); } public void commit() { - queue.add(fs -> fs.flushChanges()); + getExecutor().execute(UiUtils.wrapIOException(() -> { + getFilesystem().flushChanges(); + })); } public void resyncPaths(Collection paths) @@ -287,25 +305,22 @@ public void discard() Set snapshot = expandedSupplier != null ? expandedSupplier.get() : Set.of(); /* Capture immutable copy for the FS callback (still on EDT) */ Set captured = new HashSet<>(snapshot); - queue.add(fs -> { - try - { - fs.discardChanges(); - } finally - { - SwingUtilities.invokeLater(() -> { - resyncPaths(captured); - mutated(); - }); - } - }); + CompletableFuture discarded = new CompletableFuture<>(); + discarded.whenCompleteAsync((unused, error) -> { + /* Refresh after both successful and failed discards, matching the previous finally + * block. Register before submission to preserve ordering with later FS barriers. */ + resyncPaths(captured); + mutated(); + }, EDT_EXECUTOR).exceptionallyAsync(UiUtils.reportError("Cannot discard changes"), EDT_EXECUTOR); + discarded.completeAsync(UiUtils.wrapIOException(() -> { + filesystemExecutor.getFilesystem().discardChanges(); + return null; + }), filesystemExecutor); } public void unmount() { - queue.add(fs -> { - throw new EmergencyStopException(); - }); + filesystemExecutor.stop(); } @Override @@ -358,18 +373,7 @@ public void resyncNode(VfsPath path) * treeStructureChanged). */ dir.setLoadState(LoadState.LOADING); - VfsPath captured = path; - queue.add(fs -> { - try - { - ImmutableList entries = - fs.list(captured).values().stream().sorted().collect(toImmutableList()); - SwingUtilities.invokeLater(() -> applyListResult(dir, entries, null)); - } catch (Exception ex) - { - SwingUtilities.invokeLater(() -> applyListResult(dir, null, ex)); - } - }); + loadDirectory(dir, path); } /* ------------------------------------------------------------------------- @@ -388,18 +392,21 @@ void ensureLoaded(DirNode dir) if (!hasPlaceholder(dir)) insertNodeInto(new PlaceholderNode(), dir, 0); - VfsPath path = dir.vfsPath(); - queue.add(fs -> { - try - { - ImmutableList entries = - fs.list(path).values().stream().sorted().collect(toImmutableList()); - SwingUtilities.invokeLater(() -> applyListResult(dir, entries, null)); - } catch (Exception ex) - { - SwingUtilities.invokeLater(() -> applyListResult(dir, null, ex)); - } - }); + loadDirectory(dir, dir.vfsPath()); + } + + private void loadDirectory(DirNode dir, VfsPath path) + { + /* Attach the EDT callbacks before submitting the read so listing results are queued + * ahead of later filesystem barriers, even if the read completes immediately. */ + CompletableFuture> entries = new CompletableFuture<>(); + entries.thenAcceptAsync(result -> applyListResult(dir, result, null), EDT_EXECUTOR) + .exceptionallyAsync(error -> { + applyListResult(dir, null, error); + return null; + }, EDT_EXECUTOR); + entries.completeAsync(UiUtils.wrapIOException(() -> filesystemExecutor.getFilesystem() + .list(path).values().stream().sorted().collect(toImmutableList())), filesystemExecutor); } /** @@ -409,7 +416,7 @@ void ensureLoaded(DirNode dir) * @param entries sorted dirents, or {@code null} on failure * @param error non-null on failure */ - void applyListResult(DirNode dir, ImmutableList entries, Exception error) + void applyListResult(DirNode dir, ImmutableList entries, Throwable error) { assert SwingUtilities.isEventDispatchThread(); diff --git a/java/com/cowlark/fluxengine/gui/Gui.java b/java/com/cowlark/fluxengine/gui/Gui.java index 4f994c776..471d99ff4 100644 --- a/java/com/cowlark/fluxengine/gui/Gui.java +++ b/java/com/cowlark/fluxengine/gui/Gui.java @@ -3,7 +3,6 @@ import com.formdev.flatlaf.FlatDarkLaf; import com.google.common.collect.ImmutableList; import swingtree.threading.EventProcessor; -import javax.swing.SwingUtilities; import javax.swing.UIManager; import java.util.prefs.Preferences; @@ -34,7 +33,7 @@ public void run(ImmutableList args) throws Exception UIManager.setLookAndFeel(new FlatDarkLaf()); System.setProperty("apple.laf.useScreenMenuBar", "true"); - SwingUtilities.invokeLater(() -> { + UiUtils.EDT_EXECUTOR.execute(() -> { ApplicationFrame frame = new ApplicationFrame(model); frame.pack(); frame.setVisible(true); diff --git a/java/com/cowlark/fluxengine/gui/ImageViewModel.java b/java/com/cowlark/fluxengine/gui/ImageViewModel.java index 1957f9250..1fa19e6dd 100644 --- a/java/com/cowlark/fluxengine/gui/ImageViewModel.java +++ b/java/com/cowlark/fluxengine/gui/ImageViewModel.java @@ -18,7 +18,6 @@ import com.cowlark.fluxengine.algorithms.ReadWriteFluxOperation; import com.cowlark.fluxengine.config.ConfigBuilder; import com.cowlark.fluxengine.config.ConfigProto; -import com.cowlark.fluxengine.core.EmergencyStopException; import com.cowlark.fluxengine.core.FluxEngineException; import com.cowlark.fluxengine.core.LogMessage; import com.cowlark.fluxengine.core.LogMessage.ErrorLogMessage; @@ -34,7 +33,6 @@ import com.cowlark.fluxengine.vfs.BlockDevice; import com.cowlark.fluxengine.vfs.Filesystem; import com.cowlark.fluxengine.vfs.FilesystemOperation; -import com.cowlark.fluxengine.vfs.FilesystemOperation.FilesystemCaller; import com.cowlark.fluxengine.vfs.InMemoryBlockDevice; import io.reactivex.rxjava3.disposables.Disposable; import lombok.Getter; @@ -51,10 +49,8 @@ import swingtree.UI; import javax.swing.JButton; import javax.swing.JFileChooser; -import javax.swing.SwingUtilities; import java.awt.event.ActionEvent; import java.io.IOException; -import java.util.concurrent.BlockingQueue; import java.util.function.Supplier; public class ImageViewModel @@ -318,7 +314,7 @@ public void run() } void startFilesystemOperation( - BlockingQueue queue, + FilesystemExecutor executor, Image image, Runnable onExit) { @@ -329,28 +325,8 @@ void startFilesystemOperation( public void run(Filesystem filesystem) throws IOException { logger.atInfo().log("starting filesystem thread"); - try - { - while (true) - { - try - { - queue.take().accept(filesystem); - } catch (EmergencyStopException e) - { - throw e; - } catch (FluxEngineException | IOException e) - { - SwingUtilities.invokeLater(() -> UI - .message("Filesystem error: " + e.getMessage()) - .titled("Error") - .showAsError()); - } - } - } catch (InterruptedException e) - { - Thread.currentThread().interrupt(); - } + executor.run(filesystem, error -> + UiUtils.reportError("Filesystem error").apply(error)); } @Override @@ -362,11 +338,8 @@ protected BlockDevice createBlockDevice() @Override protected void onEmergencyStop() { - // Unblock queue.take() waiting in run(). logger.atInfo().log("terminating filesystem thread"); - queue.add(fs -> { - throw new EmergencyStopException(); - }); + executor.stop(); } @Override diff --git a/java/com/cowlark/fluxengine/gui/LogPanel.java b/java/com/cowlark/fluxengine/gui/LogPanel.java index f4bb3bfd2..17cb0322c 100644 --- a/java/com/cowlark/fluxengine/gui/LogPanel.java +++ b/java/com/cowlark/fluxengine/gui/LogPanel.java @@ -12,7 +12,6 @@ import sprouts.Viewables; import javax.swing.JPanel; import javax.swing.JTextArea; -import javax.swing.SwingUtilities; import java.awt.Font; import java.io.OutputStream; import java.io.PrintStream; @@ -65,7 +64,7 @@ public void write(int b) public void write(byte[] b, int off, int len) { String text = new String(b, off, len, StandardCharsets.UTF_8); - SwingUtilities.invokeLater(() -> add(text)); + UiUtils.EDT_EXECUTOR.execute(() -> add(text)); } }); } @@ -75,7 +74,7 @@ public void write(byte[] b, int off, int len) public void observe(Observable messages) { Disposable disposable = - messages.subscribe(panel -> SwingUtilities.invokeLater(() -> printingLogRenderer.add( + messages.subscribe(panel -> UiUtils.EDT_EXECUTOR.execute(() -> printingLogRenderer.add( panel))); } } diff --git a/java/com/cowlark/fluxengine/gui/UiUtils.java b/java/com/cowlark/fluxengine/gui/UiUtils.java index 8bb4ae406..12468601b 100644 --- a/java/com/cowlark/fluxengine/gui/UiUtils.java +++ b/java/com/cowlark/fluxengine/gui/UiUtils.java @@ -7,10 +7,11 @@ import static swingtree.UIFactoryMethods.splitButton; import com.cowlark.fluxengine.core.Bytes; -import com.cowlark.fluxengine.core.FluxEngineException; import io.reactivex.rxjava3.core.Scheduler; import io.reactivex.rxjava3.schedulers.Schedulers; import lombok.Builder; +import org.apache.commons.io.function.IORunnable; +import org.apache.commons.io.function.IOSupplier; import sprouts.Tuple; import sprouts.Val; import sprouts.Var; @@ -38,15 +39,27 @@ import java.io.File; import java.io.IOException; import java.io.UncheckedIOException; -import java.nio.file.InvalidPathException; import java.util.Arrays; import java.util.Map; import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.CancellationException; +import java.util.concurrent.CompletionException; +import java.util.concurrent.Executor; import java.util.function.Consumer; +import java.util.function.Function; +import java.util.function.Supplier; public class UiUtils { - public static final Scheduler EDT = Schedulers.from(SwingUtilities::invokeLater); + public static final Executor EDT_EXECUTOR = cb -> { + if (SwingUtilities.isEventDispatchThread()) + cb.run(); + else + SwingUtilities.invokeLater(cb); + }; + + public static final Scheduler EDT = Schedulers.from(EDT_EXECUTOR); static final float PIXELS_PER_POINT = 96f / 72f; static final Color HEADER_COLOUR = new Color(0x00b4d8); @@ -179,22 +192,65 @@ public static Icon createTintedIcon(BufferedImage image, String key, Color fg, C }); } - /** - * Prompts the user for a save location, then writes {@code data} to it. - */ - static void promptAndSave( + public static Runnable wrapIOException(IORunnable callback) + { + return () -> { + try + { + callback.run(); + } catch (IOException e) + { + throw new UncheckedIOException(e); + } + }; + } + + public static Supplier wrapIOException(IOSupplier callback) + { + return () -> { + try + { + return callback.get(); + } catch (IOException e) + { + throw new UncheckedIOException(e); + } + }; + } + + public static Function reportError(String title) + { + return message -> { + while (message instanceof CompletionException || message instanceof UncheckedIOException) + message = message.getCause(); + if (!(message instanceof CancellationException)) + { + String text = message.getMessage(); + EDT_EXECUTOR.execute(() -> JOptionPane.showMessageDialog( + null, text, title, JOptionPane.ERROR_MESSAGE)); + } + return null; + }; + } + + /* Prompts for a save location and returns a destination for bytes to write on the EDT. + * Cancelling the dialog or declining an overwrite cancels the future. */ + static CompletableFuture promptAndSave( Component parent, String dialogTitle, - String defaultFileName, - Consumer> dataSupplier) + String defaultFileName) { + CompletableFuture result = new CompletableFuture<>(); JFileChooser chooser = new JFileChooser(); chooser.setDialogTitle(dialogTitle); chooser.setSelectedFile(new File(defaultFileName)); - int result = chooser.showSaveDialog(parent); - if (result != JFileChooser.APPROVE_OPTION) - return; + int choice = chooser.showSaveDialog(parent); + if (choice != JFileChooser.APPROVE_OPTION) + { + result.cancel(false); + return result; + } File file = chooser.getSelectedFile(); if (file.exists()) @@ -205,34 +261,16 @@ static void promptAndSave( "Confirm Overwrite", JOptionPane.YES_NO_OPTION); if (overwrite != JOptionPane.YES_OPTION) - return; + { + result.cancel(false); + return result; + } } - Consumer saver = data -> { - SwingUtilities.invokeLater(() -> { - try - { - data.writeToFile(file.toPath()); - } catch (InvalidPathException e) - { - JOptionPane.showMessageDialog( - parent, - "Could not save file because the filename is invalid: " - + e.getMessage(), - "Save Failed", - JOptionPane.ERROR_MESSAGE); - } catch (FluxEngineException e) - { - JOptionPane.showMessageDialog( - parent, - "Could not save file: " + e.getMessage(), - "Save Failed", - JOptionPane.ERROR_MESSAGE); - } - }); - }; + result.thenAcceptAsync(data -> data.writeToFile(file.toPath()), EDT_EXECUTOR) + .exceptionallyAsync(reportError("Could not save file"), EDT_EXECUTOR); - dataSupplier.accept(saver); + return result; } public static String buildSanitisedString(byte[] source) diff --git a/java/com/cowlark/fluxengine/vfs/Filesystem.java b/java/com/cowlark/fluxengine/vfs/Filesystem.java index 4d715852d..908977f98 100644 --- a/java/com/cowlark/fluxengine/vfs/Filesystem.java +++ b/java/com/cowlark/fluxengine/vfs/Filesystem.java @@ -3,7 +3,7 @@ import com.cowlark.fluxengine.config.ConfigProto; import com.cowlark.fluxengine.core.Bytes; import com.cowlark.fluxengine.core.Logger; -import com.cowlark.fluxengine.vfs.FilesystemOperation.FilesystemCaller; +import org.apache.commons.lang3.function.FailableConsumer; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import lombok.Builder; @@ -38,7 +38,7 @@ protected Filesystem(ImmutableSet capabilities) this.capabilities = capabilities; } - public static void doWithFilesystem(ConfigProto config, FilesystemCaller callback) + public static void doWithFilesystem(ConfigProto config, FailableConsumer callback) { FilesystemOperation op = new FilesystemOperation() { diff --git a/java/com/cowlark/fluxengine/vfs/FilesystemOperation.java b/java/com/cowlark/fluxengine/vfs/FilesystemOperation.java index 4d1e56dda..24d37f26c 100644 --- a/java/com/cowlark/fluxengine/vfs/FilesystemOperation.java +++ b/java/com/cowlark/fluxengine/vfs/FilesystemOperation.java @@ -59,8 +59,4 @@ protected BlockDevice createBlockDevice() return blockDevice; } - public interface FilesystemCaller - { - void accept(Filesystem fs) throws IOException; - } } diff --git a/javatests/com/cowlark/fluxengine/gui/BUILD.bazel b/javatests/com/cowlark/fluxengine/gui/BUILD.bazel index b403528a9..ffa3e883e 100644 --- a/javatests/com/cowlark/fluxengine/gui/BUILD.bazel +++ b/javatests/com/cowlark/fluxengine/gui/BUILD.bazel @@ -61,8 +61,14 @@ java_test( srcs = ["UiUtilsTest.java"], deps = [ "//java/com/cowlark/fluxengine/gui", + "//java/com/cowlark/fluxengine/core", "@maven//:com_google_truth_truth", "@maven//:junit_junit", + "@maven//:commons_io_commons_io", + "@maven//:net_bytebuddy_byte_buddy", + "@maven//:net_bytebuddy_byte_buddy_agent", + "@maven//:org_mockito_mockito_core", + "@maven//:org_objenesis_objenesis", ], ) @@ -89,3 +95,19 @@ java_test( "@maven//:org_swinglabs_swingx_swingx_all", ], ) + +java_test( + name = "FilesystemExecutorTest", + srcs = [ + "FakeFilesystem.java", + "FilesystemExecutorTest.java", + ], + deps = [ + "//java/com/cowlark/fluxengine/core", + "//java/com/cowlark/fluxengine/gui", + "//java/com/cowlark/fluxengine/vfs", + "@maven//:com_google_guava_guava", + "@maven//:com_google_truth_truth", + "@maven//:junit_junit", + ], +) diff --git a/javatests/com/cowlark/fluxengine/gui/FilesystemExecutorTest.java b/javatests/com/cowlark/fluxengine/gui/FilesystemExecutorTest.java new file mode 100644 index 000000000..879a65961 --- /dev/null +++ b/javatests/com/cowlark/fluxengine/gui/FilesystemExecutorTest.java @@ -0,0 +1,116 @@ +package com.cowlark.fluxengine.gui; + +import static com.google.common.truth.Truth.assertThat; +import static org.junit.Assert.assertThrows; + +import com.cowlark.fluxengine.core.EmergencyStopException; +import com.cowlark.fluxengine.core.FluxEngineException; +import java.io.IOException; +import java.io.UncheckedIOException; +import java.util.ArrayList; +import java.util.List; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.RejectedExecutionException; +import java.util.concurrent.TimeUnit; +import org.junit.After; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +@RunWith(JUnit4.class) +public class FilesystemExecutorTest +{ + private final FilesystemExecutor executor = new FilesystemExecutor(); + private final FakeFilesystem filesystem = new FakeFilesystem(); + private final List errors = new ArrayList<>(); + private final CompletableFuture finished = new CompletableFuture<>(); + private Thread worker; + + private void startWorker() + { + worker = new Thread(() -> { + try + { + executor.run(filesystem, errors::add); + finished.complete(null); + } catch (EmergencyStopException e) + { + finished.complete(null); + } catch (Throwable e) + { + finished.completeExceptionally(e); + } + }); + worker.setDaemon(true); + worker.start(); + } + + @After + public void stopWorker() throws Exception + { + executor.stop(); + if (worker != null) + { + finished.get(5, TimeUnit.SECONDS); + worker.join(5000); + assertThat(worker.isAlive()).isFalse(); + } + } + + @Test + public void queuedTasksRunInOrderOnTheOperationThread() throws Exception + { + List order = new ArrayList<>(); + executor.execute(() -> order.add(1)); + executor.execute(() -> order.add(2)); + CompletableFuture task = CompletableFuture.supplyAsync(() -> { + assertThat(executor.getFilesystem()).isSameInstanceAs(filesystem); + return Thread.currentThread(); + }, executor); + assertThat(task.isDone()).isFalse(); + startWorker(); + assertThat(task.get(5, TimeUnit.SECONDS)).isSameInstanceAs(worker); + assertThat(order).containsExactly(1, 2).inOrder(); + } + + @Test + public void recoverableErrorsAreReportedAndLaterTasksStillRun() throws Exception + { + IOException ioFailure = new IOException("disk error"); + FluxEngineException failure = new FluxEngineException("filesystem error"); + executor.execute(() -> { + throw new UncheckedIOException(ioFailure); + }); + executor.execute(() -> { + throw failure; + }); + CompletableFuture subsequent = CompletableFuture.runAsync(() -> {}, executor); + startWorker(); + subsequent.get(5, TimeUnit.SECONDS); + assertThat(errors).containsExactly(ioFailure, failure).inOrder(); + } + + @Test + public void stoppingIdleWorkerClearsFilesystemAndRejectsTasks() throws Exception + { + assertThrows(IllegalStateException.class, executor::getFilesystem); + startWorker(); + CompletableFuture.runAsync(() -> {}, executor).get(5, TimeUnit.SECONDS); + executor.stop(); + finished.get(5, TimeUnit.SECONDS); + assertThrows(IllegalStateException.class, executor::getFilesystem); + assertThrows(RejectedExecutionException.class, () -> executor.execute(() -> {})); + assertThat(errors).isEmpty(); + } + + @Test + public void interruptingWorkerStopsSession() throws Exception + { + startWorker(); + CompletableFuture.runAsync(() -> {}, executor).get(5, TimeUnit.SECONDS); + worker.interrupt(); + finished.get(5, TimeUnit.SECONDS); + assertThrows(IllegalStateException.class, executor::getFilesystem); + assertThrows(RejectedExecutionException.class, () -> executor.execute(() -> {})); + } +} diff --git a/javatests/com/cowlark/fluxengine/gui/FilesystemPanelHeadlessTest.java b/javatests/com/cowlark/fluxengine/gui/FilesystemPanelHeadlessTest.java index 740c2b957..6bfcdd7a0 100644 --- a/javatests/com/cowlark/fluxengine/gui/FilesystemPanelHeadlessTest.java +++ b/javatests/com/cowlark/fluxengine/gui/FilesystemPanelHeadlessTest.java @@ -4,15 +4,18 @@ import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.when; +import static org.junit.Assert.assertThrows; import com.cowlark.fluxengine.core.Bytes; import com.cowlark.fluxengine.data.Disk; import com.cowlark.fluxengine.data.Image; import com.cowlark.fluxengine.gui.FilesystemTreeTableModel.DirNode; import com.cowlark.fluxengine.gui.FilesystemTreeTableModel.LoadState; -import com.cowlark.fluxengine.vfs.FilesystemOperation.FilesystemCaller; +import com.cowlark.fluxengine.vfs.Filesystem; import com.cowlark.fluxengine.vfs.VfsPath; import org.junit.Before; +import org.junit.After; +import com.cowlark.fluxengine.core.EmergencyStopException; import org.junit.BeforeClass; import org.junit.Test; import org.junit.runner.RunWith; @@ -24,12 +27,16 @@ import java.awt.datatransfer.Clipboard; import java.awt.datatransfer.DataFlavor; import java.io.File; +import java.io.IOException; +import java.io.UncheckedIOException; import java.nio.file.Files; import java.util.List; import javax.swing.tree.TreePath; import java.util.HashSet; import java.util.Set; -import java.util.concurrent.BlockingQueue; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.Executor; +import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicReference; @@ -42,7 +49,10 @@ public class FilesystemPanelHeadlessTest private FilesystemTreeTableModel model; private FilesystemPanel panel; private Disk disk; - private BlockingQueue capturedQueue; + private FilesystemExecutor capturedExecutor; + private Thread filesystemThread; + private Runnable onFilesystemExit; + private final AtomicReference filesystemError = new AtomicReference<>(); @BeforeClass public static void headless() @@ -53,7 +63,7 @@ public static void headless() @Before public void setUp() throws Exception { - fakeFs = new FakeFilesystem(); + fakeFs = org.mockito.Mockito.spy(new FakeFilesystem()); /* The root already contains "/"; add an empty directory "/empty". */ fakeFs.addEmptyDir(VfsPath.of("/empty")); @@ -68,31 +78,24 @@ public void setUp() throws Exception model = new FilesystemTreeTableModel(mockModel); when(mockModel.getFilesystemTreeTableModel()).thenReturn(model); - /* Capture the queue used by mount and start a fake filesystem thread. */ + /* Run the production executor with the fake filesystem on a dedicated worker. */ doAnswer(inv -> { - capturedQueue = inv.getArgument(0); - - /* The image argument is ignored for the fake filesystem. */ - Thread t = new Thread(() -> { - while (true) + capturedExecutor = inv.getArgument(0); + onFilesystemExit = inv.getArgument(2); + filesystemThread = new Thread(() -> { + try + { + capturedExecutor.run(fakeFs, error -> filesystemError.set((Exception) error)); + } catch (EmergencyStopException e) + { + /* Stopping the session unwinds the filesystem operation. */ + } finally { - try - { - FilesystemCaller c = capturedQueue.take(); - c.accept(fakeFs); - } catch (InterruptedException e) - { - Thread.currentThread().interrupt(); - break; - } catch (Exception e) - { - /* Swallow the exception as ImagerViewModel does (it shows a UI message). */ - e.printStackTrace(); - } + onFilesystemExit.run(); } }); - t.setDaemon(true); - t.start(); + filesystemThread.setDaemon(true); + filesystemThread.start(); return null; }).when(mockModel).startFilesystemOperation(any(), any(), any()); @@ -119,6 +122,58 @@ public void setUp() throws Exception awaitLoaded(VfsPath.of("/")); } + @After + public void stopFilesystemThread() throws Exception + { + if (capturedExecutor != null) + capturedExecutor.stop(); + if (filesystemThread != null) + { + filesystemThread.join(5000); + assertThat(filesystemThread.isAlive()).isFalse(); + } + } + + @Test + public void filesystemExecutorRunsCallbacksWithMountedFilesystem() throws Exception + { + assertThat(model.getFilesystem()).isSameInstanceAs(fakeFs); + CompletableFuture worker = new CompletableFuture<>(); + model.getExecutor().execute(() -> worker.complete(Thread.currentThread())); + Thread filesystemThread = worker.get(5, TimeUnit.SECONDS); + CompletableFuture result = + CompletableFuture.supplyAsync(() -> { + assertThat(Thread.currentThread()).isSameInstanceAs(filesystemThread); + return model.getFilesystem(); + }, model.getExecutor()); + assertThat(result.get(5, TimeUnit.SECONDS)).isSameInstanceAs(fakeFs); + } + + @Test + public void filesystemExecutorPreservesIoErrorHandling() throws Exception + { + IOException failure = new IOException("test filesystem failure"); + model.getExecutor().execute(() -> { + throw new UncheckedIOException(failure); + }); + CompletableFuture.runAsync(() -> {}, model.getExecutor()).get(5, TimeUnit.SECONDS); + assertThat(filesystemError.get()).isSameInstanceAs(failure); + } + + @Test + public void filesystemAccessorsRejectUnmountedFilesystem() throws Exception + { + FilesystemTreeTableModel unmounted = new FilesystemTreeTableModel(mockModel); + assertThrows(IllegalStateException.class, unmounted::getFilesystem); + assertThrows(IllegalStateException.class, unmounted::getExecutor); + + Executor executor = model.getExecutor(); + SwingUtilities.invokeAndWait(onFilesystemExit); + assertThrows(IllegalStateException.class, model::getFilesystem); + assertThrows(IllegalStateException.class, model::getExecutor); + assertThrows(IllegalStateException.class, () -> executor.execute(() -> {})); + } + @Test public void backgroundDropTargetsRoot() throws Exception { @@ -143,7 +198,6 @@ public void draggingFilesExportsContentsAndRejectsDirectories() throws Exception { /* Populate a real file entry and supply its contents through the filesystem worker. */ VfsPath filePath = VfsPath.of("/extract.txt"); - fakeFs = org.mockito.Mockito.spy(fakeFs); fakeFs.addFile(filePath); org.mockito.Mockito.doReturn(new Bytes("extracted contents")) .when(fakeFs).getFile(filePath); @@ -172,8 +226,17 @@ public void draggingFilesExportsContentsAndRejectsDirectories() throws Exception panel.getTreeTable().getTreeSelectionModel().clearSelection(); }); org.mockito.Mockito.verify(fakeFs, org.mockito.Mockito.never()).getFile(filePath); - List files = (List) clipboard.getData( - DataFlavor.javaFileListFlavor); + CompletableFuture> transferred = new CompletableFuture<>(); + UiUtils.EDT_EXECUTOR.execute(() -> { + try + { + transferred.complete((List) clipboard.getData(DataFlavor.javaFileListFlavor)); + } catch (Exception e) + { + transferred.completeExceptionally(e); + } + }); + List files = transferred.get(5, TimeUnit.SECONDS); assertThat(files).hasSize(1); File exported = (File) files.get(0); assertThat(exported.getName()).isEqualTo("extract.txt"); @@ -248,17 +311,17 @@ private void awaitIdle() throws Exception hasLoading.set(hasLoadingNode((DirNode) model.getRoot())); }); loading = hasLoading.get(); - boolean queueEmpty = capturedQueue == null || capturedQueue.isEmpty(); + CompletableFuture.runAsync(() -> {}, capturedExecutor).get(5, TimeUnit.SECONDS); /* The EDT queue must also be empty; invokeAndWait already drains it. */ - if (!loading && queueEmpty) + if (!loading) { /* Give the filesystem thread a moment to enqueue the next EDT task. */ Thread.sleep(50); /* Re-check. */ SwingUtilities.invokeAndWait(() -> hasLoading.set(hasLoadingNode((DirNode) model.getRoot()))); - if (!hasLoading.get() && (capturedQueue == null || capturedQueue.isEmpty())) + if (!hasLoading.get()) return; } Thread.sleep(20); @@ -373,6 +436,24 @@ public void expandedEmptyGainsChildIsVisible_placeholderNotCollapsedBeforePopula }); } + @Test + public void failedDirectoryListingCanBeRetried() throws Exception + { + org.mockito.Mockito.doThrow(new IOException("directory read failed")) + .doCallRealMethod().when(fakeFs).list(VfsPath.root()); + SwingUtilities.invokeAndWait(() -> model.resyncNode(VfsPath.root())); + awaitIdle(); + SwingUtilities.invokeAndWait(() -> { + DirNode root = model.findNode(VfsPath.root()); + assertThat(root.getLoadState()).isEqualTo(LoadState.NOT_LOADED); + model.requestPopulate(root); + }); + awaitIdle(); + awaitLoaded(VfsPath.root()); + SwingUtilities.invokeAndWait(() -> + assertThat(model.findNode(VfsPath.of("/empty"))).isNotNull()); + } + @Test public void discardPreservesExpansion_headless() throws Exception { diff --git a/javatests/com/cowlark/fluxengine/gui/UiUtilsTest.java b/javatests/com/cowlark/fluxengine/gui/UiUtilsTest.java index e70d9357a..f3ba7ce90 100644 --- a/javatests/com/cowlark/fluxengine/gui/UiUtilsTest.java +++ b/javatests/com/cowlark/fluxengine/gui/UiUtilsTest.java @@ -1,8 +1,27 @@ package com.cowlark.fluxengine.gui; import static com.google.common.truth.Truth.assertThat; +import static org.junit.Assert.assertThrows; +import com.cowlark.fluxengine.core.Bytes; +import org.apache.commons.io.function.IORunnable; +import org.apache.commons.io.function.IOSupplier; +import java.io.IOException; +import java.io.UncheckedIOException; import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.CompletionException; +import java.util.concurrent.CancellationException; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.function.Supplier; +import javax.swing.JFileChooser; +import javax.swing.JOptionPane; +import javax.swing.SwingUtilities; +import org.mockito.MockedConstruction; +import org.mockito.MockedStatic; +import org.mockito.Mockito; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; @@ -10,6 +29,124 @@ @RunWith(JUnit4.class) public class UiUtilsTest { + @Test + public void wrapIOExceptionDefersCallbacksAndPreservesResults() + { + AtomicBoolean called = new AtomicBoolean(); + Runnable runnable = UiUtils.wrapIOException(() -> { + called.set(true); + }); + assertThat(called.get()).isFalse(); + runnable.run(); + assertThat(called.get()).isTrue(); + + called.set(false); + Supplier supplier = UiUtils.wrapIOException(() -> { + called.set(true); + return "result"; + }); + assertThat(called.get()).isFalse(); + assertThat(supplier.get()).isEqualTo("result"); + assertThat(called.get()).isTrue(); + } + + @Test + public void wrapIOExceptionPreservesOriginalCause() + { + IOException failure = new IOException("disk error"); + IORunnable runnable = () -> { + throw failure; + }; + IOSupplier supplier = () -> { + throw failure; + }; + assertThat(assertThrows(UncheckedIOException.class, + UiUtils.wrapIOException(runnable)::run).getCause()).isSameInstanceAs(failure); + assertThat(assertThrows(UncheckedIOException.class, + UiUtils.wrapIOException(supplier)::get).getCause()).isSameInstanceAs(failure); + } + + @Test + public void wrapIOExceptionLeavesRuntimeFailuresUnchanged() + { + IllegalStateException failure = new IllegalStateException("failure"); + IORunnable runnable = () -> { + throw failure; + }; + IOSupplier supplier = () -> { + throw failure; + }; + assertThat(assertThrows(IllegalStateException.class, + UiUtils.wrapIOException(runnable)::run)).isSameInstanceAs(failure); + assertThat(assertThrows(IllegalStateException.class, + UiUtils.wrapIOException(supplier)::get)).isSameInstanceAs(failure); + } + + @Test + public void reportErrorUnwrapsFailuresAndIgnoresCancellation() throws Exception + { + SwingUtilities.invokeAndWait(() -> { + try (MockedStatic dialogs = Mockito.mockStatic(JOptionPane.class)) + { + CompletableFuture failed = CompletableFuture.failedFuture( + new CompletionException(new UncheckedIOException(new IOException("disk error")))) + .exceptionallyAsync(UiUtils.reportError("Read failed"), UiUtils.EDT_EXECUTOR); + assertThat(failed.join()).isNull(); + dialogs.verify(() -> JOptionPane.showMessageDialog( + null, "disk error", "Read failed", JOptionPane.ERROR_MESSAGE)); + CompletableFuture.failedFuture(new CompletionException(new CancellationException())) + .exceptionallyAsync(UiUtils.reportError("Cancelled"), UiUtils.EDT_EXECUTOR).join(); + dialogs.verifyNoMoreInteractions(); + } + }); + } + + @Test + public void promptAndSaveReturnsDestination() throws Exception + { + Path directory = Files.createTempDirectory("fluxengine-save-test-"); + Path output = directory.resolve("saved.txt"); + try (MockedConstruction choosers = Mockito.mockConstruction( + JFileChooser.class, (chooser, context) -> { + Mockito.when(chooser.showSaveDialog(null)).thenReturn(JFileChooser.APPROVE_OPTION); + Mockito.when(chooser.getSelectedFile()).thenReturn(output.toFile()); + })) + { + CompletableFuture result = + UiUtils.promptAndSave(null, "Save file", "default.txt"); + assertThat(result.isDone()).isFalse(); + assertThat(Files.exists(output)).isFalse(); + Mockito.verify(choosers.constructed().get(0)).showSaveDialog(null); + result.completeAsync(() -> new Bytes("saved contents"), Runnable::run); + SwingUtilities.invokeAndWait(() -> {}); + assertThat(Files.readString(output)).isEqualTo("saved contents"); + } finally + { + Files.deleteIfExists(output); + Files.delete(directory); + } + } + + @Test + public void promptAndSaveCancelsFutureWhenDialogIsCancelled() + { + try (MockedConstruction choosers = Mockito.mockConstruction( + JFileChooser.class, (chooser, context) -> + Mockito.when(chooser.showSaveDialog(null)) + .thenReturn(JFileChooser.CANCEL_OPTION))) + { + CompletableFuture result = + UiUtils.promptAndSave(null, "Save file", "default.txt"); + assertThat(result.isCancelled()).isTrue(); + AtomicBoolean extracted = new AtomicBoolean(); + result.completeAsync(() -> { + extracted.set(true); + return new Bytes("must not be extracted"); + }, Runnable::run); + assertThat(extracted.get()).isFalse(); + } + } + private static byte[] bs(String s) { return s.getBytes(StandardCharsets.ISO_8859_1); @@ -93,17 +230,17 @@ public void crlfIsConvertedToSingleLf() @Test public void mixedLineEndingsAreNormalised() { - // CRLF + LF + /* CRLF + LF */ assertThat(UiUtils.buildSanitisedString(bs("a\r\n\nb"))).isEqualTo("a\n\nb"); - // LF + CRLF + /* LF + CRLF */ assertThat(UiUtils.buildSanitisedString(bs("a\n\r\nb"))).isEqualTo("a\n\nb"); - // CR + LF + /* CR + LF */ assertThat(UiUtils.buildSanitisedString(bs("a\r\nb\nc\rd"))).isEqualTo("a\nb\nc\nd"); - // All three forms interleaved + /* All three forms interleaved */ assertThat(UiUtils.buildSanitisedString(bs("x\r\ny\nz\ra"))).isEqualTo("x\ny\nz\na"); assertThat(UiUtils.buildSanitisedString(bs("a\r\nb\rc\nd"))).isEqualTo("a\nb\nc\nd"); assertThat(UiUtils.buildSanitisedString(bs("\r\n\n\r"))).isEqualTo("\n\n\n"); - // Raw bytes: CRLF as 13,10 + /* Raw bytes: CRLF as 13,10 */ assertThat(UiUtils.buildSanitisedString(bs(13, 10, 10, 13))).isEqualTo("\n\n\n"); } @@ -113,14 +250,14 @@ public void nonPrintableCharactersAreEscaped() assertThat(UiUtils.buildSanitisedString(bs(0))).isEqualTo("\\x00"); assertThat(UiUtils.buildSanitisedString(bs(1))).isEqualTo("\\x01"); assertThat(UiUtils.buildSanitisedString(bs(0x1f))).isEqualTo("\\x1f"); - // \t (0x09) and \n (0x0a) are not escaped; \r (0x0d) becomes \n - assertThat(UiUtils.buildSanitisedString(bs(0x08))).isEqualTo("\\x08"); // backspace - assertThat(UiUtils.buildSanitisedString(bs(0x0b))).isEqualTo("\\x0b"); // vertical tab - assertThat(UiUtils.buildSanitisedString(bs(0x0c))).isEqualTo("\\x0c"); // form feed + /* \t (0x09) and \n (0x0a) are not escaped; \r (0x0d) becomes \n */ + assertThat(UiUtils.buildSanitisedString(bs(0x08))).isEqualTo("\\x08"); /* backspace */ + assertThat(UiUtils.buildSanitisedString(bs(0x0b))).isEqualTo("\\x0b"); /* vertical tab */ + assertThat(UiUtils.buildSanitisedString(bs(0x0c))).isEqualTo("\\x0c"); /* form feed */ assertThat(UiUtils.buildSanitisedString(bs(0x0e))).isEqualTo("\\x0e"); - // DEL (127) is not printable + /* DEL (127) is not printable */ assertThat(UiUtils.buildSanitisedString(bs(0x7f))).isEqualTo("\\x7f"); - // NUL via string conversion + /* NUL via string conversion */ assertThat(UiUtils.buildSanitisedString(bs("\u0000"))).isEqualTo("\\x00"); assertThat(UiUtils.buildSanitisedString(bs("\u0001"))).isEqualTo("\\x01"); } @@ -131,13 +268,13 @@ public void charactersAbove127AreEscaped() assertThat(UiUtils.buildSanitisedString(bs(0x80))).isEqualTo("\\x80"); assertThat(UiUtils.buildSanitisedString(bs(0xff))).isEqualTo("\\xff"); assertThat(UiUtils.buildSanitisedString(bs(0xa0))).isEqualTo("\\xa0"); - // Check that all bytes 128..255 produce \xXX in lower-case hex + /* Check that all bytes 128..255 produce \xXX in lower-case hex */ assertThat(UiUtils.buildSanitisedString(bs(0x80, 0x81, 0xfe))) .isEqualTo("\\x80\\x81\\xfe"); - // High bytes via ISO-8859-1 string + /* High bytes via ISO-8859-1 string */ assertThat(UiUtils.buildSanitisedString(bs("\u0080"))).isEqualTo("\\x80"); assertThat(UiUtils.buildSanitisedString(bs("\u00ff"))).isEqualTo("\\xff"); - // Ensure signed byte handling: (byte)0x80 == -128 should still be \x80 + /* Ensure signed byte handling: (byte)0x80 == -128 should still be \x80 */ assertThat(UiUtils.buildSanitisedString(new byte[] {(byte) 0x80})).isEqualTo("\\x80"); assertThat(UiUtils.buildSanitisedString(new byte[] {(byte) 0xff})).isEqualTo("\\xff"); } @@ -145,13 +282,13 @@ public void charactersAbove127AreEscaped() @Test public void printableBoundaryIsCorrect() { - // 32 (space) and 126 (~) are the boundaries and should be preserved + /* 32 (space) and 126 (~) are the boundaries and should be preserved */ assertThat(UiUtils.buildSanitisedString(bs(" "))).isEqualTo(" "); assertThat(UiUtils.buildSanitisedString(bs("~"))).isEqualTo("~"); assertThat(UiUtils.buildSanitisedString(bs(32, 126))).isEqualTo(" ~"); assertThat(UiUtils.buildSanitisedString(bs(32))).isEqualTo(" "); assertThat(UiUtils.buildSanitisedString(bs(126))).isEqualTo("~"); - // 31 and 127 should be escaped + /* 31 and 127 should be escaped */ assertThat(UiUtils.buildSanitisedString(bs(31))).isEqualTo("\\x1f"); assertThat(UiUtils.buildSanitisedString(bs(127))).isEqualTo("\\x7f"); } @@ -163,7 +300,7 @@ public void mixedPrintableAndEscaped() .isEqualTo("a\\x00b\nc\nd\\x7fe\\x80f"); assertThat(UiUtils.buildSanitisedString(bs("Hello\u0001World\r\nNext\u007fLine\u00ff"))) .isEqualTo("Hello\\x01World\nNext\\x7fLine\\xff"); - // Raw bytes mixed + /* Raw bytes mixed */ assertThat(UiUtils.buildSanitisedString(bs('a', 0, 'b', 10, 'c', 13, 10, 'd', 0x7f, 'e', 0x80, 'f'))) .isEqualTo("a\\x00b\nc\nd\\x7fe\\x80f"); } @@ -172,7 +309,7 @@ public void mixedPrintableAndEscaped() public void hexFormattingIsLowerCaseZeroPadded() { assertThat(UiUtils.buildSanitisedString(bs(0x01))).isEqualTo("\\x01"); - assertThat(UiUtils.buildSanitisedString(bs("\n"))).isEqualTo("\n"); // LF is preserved, not escaped + assertThat(UiUtils.buildSanitisedString(bs("\n"))).isEqualTo("\n"); /* LF is preserved, not escaped */ assertThat(UiUtils.buildSanitisedString(bs(0x0f))).isEqualTo("\\x0f"); assertThat(UiUtils.buildSanitisedString(bs(0x10))).isEqualTo("\\x10"); assertThat(UiUtils.buildSanitisedString(bs(0x00))).isEqualTo("\\x00"); @@ -181,12 +318,12 @@ public void hexFormattingIsLowerCaseZeroPadded() @Test public void byteArrayHandlesUnsignedCorrectly() { - // 255 via signed byte -1 + /* 255 via signed byte -1 */ assertThat(UiUtils.buildSanitisedString(new byte[] {(byte) 255})).isEqualTo("\\xff"); - // Sequence of high bytes + /* Sequence of high bytes */ assertThat(UiUtils.buildSanitisedString(new byte[] {(byte) 0x80, (byte) 0x90, (byte) 0xa0, (byte) 0xff})) .isEqualTo("\\x80\\x90\\xa0\\xff"); - // Printable still works with signed bytes: 'A' is 65 + /* Printable still works with signed bytes: 'A' is 65 */ assertThat(UiUtils.buildSanitisedString(new byte[] {65, 66, 67})).isEqualTo("ABC"); } } From 2436e7a6407ca64c9508fb3a2e72f54452993cbe Mon Sep 17 00:00:00 2001 From: David Given Date: Sat, 24 Oct 2026 00:33:50 +0200 Subject: [PATCH 4/5] The workflow selector is stored in preferences. --- .../fluxengine/gui/ApplicationFrame.java | 6 ++- java/com/cowlark/fluxengine/gui/Gui.java | 2 +- .../gui/PreferencesReaderWriter.java | 17 ++++++- .../gui/PreferencesReaderWriterTest.java | 45 +++++++++++++++++++ 4 files changed, 66 insertions(+), 4 deletions(-) diff --git a/java/com/cowlark/fluxengine/gui/ApplicationFrame.java b/java/com/cowlark/fluxengine/gui/ApplicationFrame.java index 4711ff1b7..4ee212b0a 100644 --- a/java/com/cowlark/fluxengine/gui/ApplicationFrame.java +++ b/java/com/cowlark/fluxengine/gui/ApplicationFrame.java @@ -1,6 +1,7 @@ package com.cowlark.fluxengine.gui; import static com.cowlark.fluxengine.gui.UiUtils.EDT_EXECUTOR; +import static com.cowlark.fluxengine.gui.PreferencesReaderWriter.WORKFLOW; import static com.cowlark.fluxengine.gui.UiUtils.createSplitButton; import static com.cowlark.fluxengine.gui.UiUtils.sizePts; import static swingtree.UI.Axis.HORIZONTAL; @@ -50,11 +51,12 @@ public class ApplicationFrame extends JFrame private final FilesystemPanel filesystemPanel; private final ImageViewModel model; - private final Var currentWorkflow = Var.of(Workflow.DISK_READING); + private final Var currentWorkflow; - ApplicationFrame(ImageViewModel model) + ApplicationFrame(ImageViewModel model, PreferencesReaderWriter preferences) { this.model = model; + currentWorkflow = preferences.makeEnumPreference(WORKFLOW, Workflow.DISK_READING); summaryPanel = new SummaryPanel(model); logPanel = new LogPanel(model); imagePanel = new ImagePanel(model); diff --git a/java/com/cowlark/fluxengine/gui/Gui.java b/java/com/cowlark/fluxengine/gui/Gui.java index 471d99ff4..f2fa8f824 100644 --- a/java/com/cowlark/fluxengine/gui/Gui.java +++ b/java/com/cowlark/fluxengine/gui/Gui.java @@ -34,7 +34,7 @@ public void run(ImmutableList args) throws Exception System.setProperty("apple.laf.useScreenMenuBar", "true"); UiUtils.EDT_EXECUTOR.execute(() -> { - ApplicationFrame frame = new ApplicationFrame(model); + ApplicationFrame frame = new ApplicationFrame(model, preferencesReaderWriter); frame.pack(); frame.setVisible(true); }); diff --git a/java/com/cowlark/fluxengine/gui/PreferencesReaderWriter.java b/java/com/cowlark/fluxengine/gui/PreferencesReaderWriter.java index 7ef2dbb2b..59d539657 100644 --- a/java/com/cowlark/fluxengine/gui/PreferencesReaderWriter.java +++ b/java/com/cowlark/fluxengine/gui/PreferencesReaderWriter.java @@ -1,8 +1,12 @@ package com.cowlark.fluxengine.gui; import com.google.common.base.Splitter; +import com.google.common.base.Enums; import sprouts.Association; import sprouts.Pair; +import sprouts.From; +import sprouts.Var; +import sprouts.Viewable; import java.net.URLDecoder; import java.net.URLEncoder; import java.nio.charset.StandardCharsets; @@ -16,6 +20,7 @@ public class PreferencesReaderWriter static final String FORMAT = "format"; static final String DEVICE = "device"; static final String DRIVE = "drive"; + static final String WORKFLOW = "workflow"; static final String ADVANCED_SETTINGS = "advanced_settings"; static final String DEVICE_FLUXFILE = "fluxfile"; @@ -53,7 +58,7 @@ Association getOptions(String key) .withKeyValueSeparator(Splitter.on('=').limit(2)) .split(optionsString); - // Decode URL-encoded keys and values + /* Decode URL-encoded keys and values. */ return rawMap .entrySet() .stream() @@ -72,6 +77,16 @@ void setOptions(String key, Association options) preferences.put(key, optionsString); } + > Var makeEnumPreference(String name, E defaultValue) + { + E initial = Enums.getIfPresent(defaultValue.getDeclaringClass(), + getStringPreference(name, defaultValue.name())).or(defaultValue); + Var value = Var.of(initial); + Viewable.cast(value).onChange(From.VIEW, change -> + setStringPreference(name, change.currentValue().orElseThrowUnchecked().name())); + return value; + } + String getStringPreference(String name, String defaultValue) { return preferences.get(name, defaultValue); diff --git a/javatests/com/cowlark/fluxengine/gui/PreferencesReaderWriterTest.java b/javatests/com/cowlark/fluxengine/gui/PreferencesReaderWriterTest.java index 36a22996c..a915b5b88 100644 --- a/javatests/com/cowlark/fluxengine/gui/PreferencesReaderWriterTest.java +++ b/javatests/com/cowlark/fluxengine/gui/PreferencesReaderWriterTest.java @@ -9,6 +9,9 @@ import org.mockito.Mock; import org.mockito.junit.MockitoJUnitRunner; import sprouts.Association; +import sprouts.From; +import sprouts.Var; +import com.cowlark.fluxengine.gui.ApplicationFrame.Workflow; import java.util.prefs.Preferences; @RunWith(MockitoJUnitRunner.class) @@ -25,6 +28,48 @@ private static Association assoc(String... pairs) return result; } + @Test + public void workflowPreferenceRestoresSavedSelection() + { + when(preferences.get("workflow", "DISK_READING")).thenReturn("IMAGE_PROCESSING"); + writer = new PreferencesReaderWriter(preferences); + + assertThat(writer.makeEnumPreference("workflow", Workflow.DISK_READING).get()) + .isEqualTo(Workflow.IMAGE_PROCESSING); + } + + @Test + public void workflowPreferenceUsesDefaultWhenMissing() + { + when(preferences.get("workflow", "DISK_READING")).thenReturn("DISK_READING"); + writer = new PreferencesReaderWriter(preferences); + + assertThat(writer.makeEnumPreference("workflow", Workflow.DISK_READING).get()) + .isEqualTo(Workflow.DISK_READING); + } + + @Test + public void workflowPreferenceUsesDefaultForUnknownValue() + { + when(preferences.get("workflow", "DISK_READING")).thenReturn("OLD_WORKFLOW"); + writer = new PreferencesReaderWriter(preferences); + + assertThat(writer.makeEnumPreference("workflow", Workflow.DISK_READING).get()) + .isEqualTo(Workflow.DISK_READING); + } + + @Test + public void workflowPreferencePersistsSelectionChanges() + { + when(preferences.get("workflow", "DISK_READING")).thenReturn("DISK_READING"); + writer = new PreferencesReaderWriter(preferences); + Var workflow = writer.makeEnumPreference("workflow", Workflow.DISK_READING); + + workflow.set(From.VIEW, Workflow.DISK_WRITING); + + verify(preferences).put("workflow", "DISK_WRITING"); + } + @Test public void setOptionsForFormatEncodesIntoPreferences() { From 178af4f0d46173cdb8a6abe72ec8f44cf9e94298 Mon Sep 17 00:00:00 2001 From: David Given Date: Sat, 24 Oct 2026 00:49:56 +0200 Subject: [PATCH 5/5] Remember the current directory inside file dialogues. --- .../fluxengine/gui/ConfigurationPanel.java | 6 +- .../fluxengine/gui/FilesystemPanel.java | 6 +- .../fluxengine/gui/ImageViewModel.java | 8 +-- .../gui/PreferencesReaderWriter.java | 20 +++++++ java/com/cowlark/fluxengine/gui/UiUtils.java | 5 +- .../gui/PreferencesReaderWriterTest.java | 57 +++++++++++++++++++ .../cowlark/fluxengine/gui/UiUtilsTest.java | 10 +++- 7 files changed, 96 insertions(+), 16 deletions(-) diff --git a/java/com/cowlark/fluxengine/gui/ConfigurationPanel.java b/java/com/cowlark/fluxengine/gui/ConfigurationPanel.java index 66fac3b82..499f4d1d1 100644 --- a/java/com/cowlark/fluxengine/gui/ConfigurationPanel.java +++ b/java/com/cowlark/fluxengine/gui/ConfigurationPanel.java @@ -53,7 +53,6 @@ import java.awt.Rectangle; import java.awt.event.ActionEvent; import java.awt.image.BufferedImage; -import java.io.File; import java.util.Collection; import java.util.HashSet; import java.util.Optional; @@ -518,10 +517,7 @@ yield label(String.format( private void onUseFluxFile(ComponentDelegate delegate) { - JFileChooser fileChooser = new JFileChooser(); - String oldFile = model.getSelectedFluxFile().get(); - if (!isNullOrEmpty(oldFile)) - fileChooser.setCurrentDirectory(new File(oldFile).getParentFile()); + JFileChooser fileChooser = model.getPreferencesReaderWriter().createFileChooser(); fileChooser.setDialogTitle("Open flux file"); fileChooser.setApproveButtonText("Select"); if (fileChooser.showOpenDialog(this) == APPROVE_OPTION) diff --git a/java/com/cowlark/fluxengine/gui/FilesystemPanel.java b/java/com/cowlark/fluxengine/gui/FilesystemPanel.java index 466d511a2..aff970cdf 100644 --- a/java/com/cowlark/fluxengine/gui/FilesystemPanel.java +++ b/java/com/cowlark/fluxengine/gui/FilesystemPanel.java @@ -292,7 +292,7 @@ private void getFile( { VfsPath vfsPath = node.getDirent().path(); String filename = node.getDirent().filename(); - UiUtils.promptAndSave(this, "Save file", filename).completeAsync( + UiUtils.promptAndSave(this, model.getPreferencesReaderWriter(), "Save file", filename).completeAsync( wrapIOException(() -> { Filesystem fs = treeTableModel.getFilesystem(); return fs.getFile(vfsPath); @@ -305,7 +305,7 @@ private void getFile( ImmutableList snapshot = stream(paths) .map(p -> ((FileNode) p.getLastPathComponent()).getDirent().path()) .collect(toImmutableList()); - UiUtils.promptAndSave(this, "Save multiple files", "files.zip").completeAsync( + UiUtils.promptAndSave(this, model.getPreferencesReaderWriter(), "Save multiple files", "files.zip").completeAsync( wrapIOException(() -> { Filesystem fs = treeTableModel.getFilesystem(); return fs.getFiles(fs, snapshot); @@ -337,7 +337,7 @@ private void putFiles( DirNode parent = getParentNodeOfSelection(); VfsPath parentPath = parent.getDirent().path(); - JFileChooser chooser = new JFileChooser(); + JFileChooser chooser = model.getPreferencesReaderWriter().createFileChooser(); chooser.setDialogTitle("Open files"); chooser.setMultiSelectionEnabled(true); diff --git a/java/com/cowlark/fluxengine/gui/ImageViewModel.java b/java/com/cowlark/fluxengine/gui/ImageViewModel.java index 1fa19e6dd..b0f8cba76 100644 --- a/java/com/cowlark/fluxengine/gui/ImageViewModel.java +++ b/java/com/cowlark/fluxengine/gui/ImageViewModel.java @@ -62,7 +62,7 @@ public class ImageViewModel # decoder.retries=0 """; - private final PreferencesReaderWriter preferencesReaderWriter; + @Getter private final PreferencesReaderWriter preferencesReaderWriter; @Getter private Var statusMessage = Var.of("Ready"); @Getter private Var selectedFormat; @@ -217,7 +217,7 @@ public void run() void onLoadDiskImage(ComponentDelegate delegate) { - JFileChooser fileChooser = new JFileChooser(); + JFileChooser fileChooser = preferencesReaderWriter.createFileChooser(); fileChooser.setDialogTitle("Load image file"); fileChooser.setApproveButtonText("Load"); if (fileChooser.showOpenDialog(null) != APPROVE_OPTION) @@ -244,7 +244,7 @@ void onSaveDiskImage(ComponentDelegate delegate) { Image image = getDisk().get().image; - JFileChooser fileChooser = new JFileChooser(); + JFileChooser fileChooser = preferencesReaderWriter.createFileChooser(); fileChooser.setDialogTitle("Save image file"); fileChooser.setApproveButtonText("Save"); if (fileChooser.showSaveDialog(null) != APPROVE_OPTION) @@ -269,7 +269,7 @@ void onSaveDiskFlux(ComponentDelegate delegate) Disk disk = getDisk().get(); FluxSource fluxSource = new MemoryFluxSource(disk); - JFileChooser fileChooser = new JFileChooser(); + JFileChooser fileChooser = preferencesReaderWriter.createFileChooser(); fileChooser.setDialogTitle("Save flux file"); fileChooser.setApproveButtonText("Save"); if (fileChooser.showSaveDialog(null) != APPROVE_OPTION) diff --git a/java/com/cowlark/fluxengine/gui/PreferencesReaderWriter.java b/java/com/cowlark/fluxengine/gui/PreferencesReaderWriter.java index 59d539657..0569583a9 100644 --- a/java/com/cowlark/fluxengine/gui/PreferencesReaderWriter.java +++ b/java/com/cowlark/fluxengine/gui/PreferencesReaderWriter.java @@ -1,5 +1,7 @@ package com.cowlark.fluxengine.gui; +import static com.google.common.base.Strings.isNullOrEmpty; + import com.google.common.base.Splitter; import com.google.common.base.Enums; import sprouts.Association; @@ -7,6 +9,8 @@ import sprouts.From; import sprouts.Var; import sprouts.Viewable; +import javax.swing.JFileChooser; +import java.io.File; import java.net.URLDecoder; import java.net.URLEncoder; import java.nio.charset.StandardCharsets; @@ -21,6 +25,7 @@ public class PreferencesReaderWriter static final String DEVICE = "device"; static final String DRIVE = "drive"; static final String WORKFLOW = "workflow"; + static final String CURRENT_DIRECTORY = "current_directory"; static final String ADVANCED_SETTINGS = "advanced_settings"; static final String DEVICE_FLUXFILE = "fluxfile"; @@ -77,6 +82,21 @@ void setOptions(String key, Association options) preferences.put(key, optionsString); } + JFileChooser createFileChooser() + { + JFileChooser chooser = new JFileChooser(); + String currentDirectory = getStringPreference(CURRENT_DIRECTORY, ""); + if (!isNullOrEmpty(currentDirectory)) + chooser.setCurrentDirectory(new File(currentDirectory)); + /* Remember navigation even if the user subsequently cancels the dialog. */ + chooser.addPropertyChangeListener(JFileChooser.DIRECTORY_CHANGED_PROPERTY, change -> { + File directory = (File) change.getNewValue(); + if (directory != null) + setStringPreference(CURRENT_DIRECTORY, directory.getAbsolutePath()); + }); + return chooser; + } + > Var makeEnumPreference(String name, E defaultValue) { E initial = Enums.getIfPresent(defaultValue.getDeclaringClass(), diff --git a/java/com/cowlark/fluxengine/gui/UiUtils.java b/java/com/cowlark/fluxengine/gui/UiUtils.java index 12468601b..8521aaf79 100644 --- a/java/com/cowlark/fluxengine/gui/UiUtils.java +++ b/java/com/cowlark/fluxengine/gui/UiUtils.java @@ -237,13 +237,14 @@ public static Function reportError(String title) * Cancelling the dialog or declining an overwrite cancels the future. */ static CompletableFuture promptAndSave( Component parent, + PreferencesReaderWriter preferences, String dialogTitle, String defaultFileName) { CompletableFuture result = new CompletableFuture<>(); - JFileChooser chooser = new JFileChooser(); + JFileChooser chooser = preferences.createFileChooser(); chooser.setDialogTitle(dialogTitle); - chooser.setSelectedFile(new File(defaultFileName)); + chooser.setSelectedFile(new File(chooser.getCurrentDirectory(), defaultFileName)); int choice = chooser.showSaveDialog(parent); if (choice != JFileChooser.APPROVE_OPTION) diff --git a/javatests/com/cowlark/fluxengine/gui/PreferencesReaderWriterTest.java b/javatests/com/cowlark/fluxengine/gui/PreferencesReaderWriterTest.java index a915b5b88..aafd5195e 100644 --- a/javatests/com/cowlark/fluxengine/gui/PreferencesReaderWriterTest.java +++ b/javatests/com/cowlark/fluxengine/gui/PreferencesReaderWriterTest.java @@ -5,6 +5,13 @@ import static org.mockito.Mockito.when; import org.junit.Test; +import org.mockito.ArgumentCaptor; +import org.mockito.MockedConstruction; +import org.mockito.Mockito; +import javax.swing.JFileChooser; +import java.beans.PropertyChangeEvent; +import java.beans.PropertyChangeListener; +import java.io.File; import org.junit.runner.RunWith; import org.mockito.Mock; import org.mockito.junit.MockitoJUnitRunner; @@ -28,6 +35,56 @@ private static Association assoc(String... pairs) return result; } + @Test + public void fileChooserRestoresLastDirectory() + { + File directory = new File("saved-directory").getAbsoluteFile(); + when(preferences.get("current_directory", "")).thenReturn(directory.getPath()); + writer = new PreferencesReaderWriter(preferences); + try (MockedConstruction choosers = Mockito.mockConstruction(JFileChooser.class)) + { + JFileChooser chooser = writer.createFileChooser(); + verify(chooser).setCurrentDirectory(directory); + } + } + + @Test + public void fileChooserNavigationIsRememberedEvenWhenCancelled() + { + File directory = new File("last-directory").getAbsoluteFile(); + when(preferences.get("current_directory", "")).thenReturn(""); + writer = new PreferencesReaderWriter(preferences); + try (MockedConstruction choosers = Mockito.mockConstruction(JFileChooser.class)) + { + JFileChooser chooser = writer.createFileChooser(); + ArgumentCaptor listener = + ArgumentCaptor.forClass(PropertyChangeListener.class); + verify(chooser).addPropertyChangeListener( + Mockito.eq(JFileChooser.DIRECTORY_CHANGED_PROPERTY), listener.capture()); + listener.getValue().propertyChange(new PropertyChangeEvent( + chooser, JFileChooser.DIRECTORY_CHANGED_PROPERTY, null, directory)); + when(chooser.showOpenDialog(null)).thenReturn(JFileChooser.CANCEL_OPTION); + assertThat(chooser.showOpenDialog(null)).isEqualTo(JFileChooser.CANCEL_OPTION); + verify(preferences).put("current_directory", directory.getPath()); + + when(preferences.get("current_directory", "")).thenReturn(directory.getPath()); + JFileChooser next = new PreferencesReaderWriter(preferences).createFileChooser(); + verify(next).setCurrentDirectory(directory); + } + } + + @Test + public void fileChooserKeepsSystemDefaultWhenNoDirectoryIsSaved() + { + when(preferences.get("current_directory", "")).thenReturn(""); + writer = new PreferencesReaderWriter(preferences); + try (MockedConstruction choosers = Mockito.mockConstruction(JFileChooser.class)) + { + JFileChooser chooser = writer.createFileChooser(); + verify(chooser, Mockito.never()).setCurrentDirectory(Mockito.any()); + } + } + @Test public void workflowPreferenceRestoresSavedSelection() { diff --git a/javatests/com/cowlark/fluxengine/gui/UiUtilsTest.java b/javatests/com/cowlark/fluxengine/gui/UiUtilsTest.java index f3ba7ce90..23223cdf3 100644 --- a/javatests/com/cowlark/fluxengine/gui/UiUtilsTest.java +++ b/javatests/com/cowlark/fluxengine/gui/UiUtilsTest.java @@ -16,6 +16,7 @@ import java.util.concurrent.CancellationException; import java.util.concurrent.atomic.AtomicBoolean; import java.util.function.Supplier; +import java.util.prefs.Preferences; import javax.swing.JFileChooser; import javax.swing.JOptionPane; import javax.swing.SwingUtilities; @@ -29,6 +30,8 @@ @RunWith(JUnit4.class) public class UiUtilsTest { + private final PreferencesReaderWriter preferences = + new PreferencesReaderWriter(Mockito.mock(Preferences.class)); @Test public void wrapIOExceptionDefersCallbacksAndPreservesResults() { @@ -110,13 +113,16 @@ public void promptAndSaveReturnsDestination() throws Exception JFileChooser.class, (chooser, context) -> { Mockito.when(chooser.showSaveDialog(null)).thenReturn(JFileChooser.APPROVE_OPTION); Mockito.when(chooser.getSelectedFile()).thenReturn(output.toFile()); + Mockito.when(chooser.getCurrentDirectory()).thenReturn(directory.toFile()); })) { CompletableFuture result = - UiUtils.promptAndSave(null, "Save file", "default.txt"); + UiUtils.promptAndSave(null, preferences, "Save file", "default.txt"); assertThat(result.isDone()).isFalse(); assertThat(Files.exists(output)).isFalse(); Mockito.verify(choosers.constructed().get(0)).showSaveDialog(null); + Mockito.verify(choosers.constructed().get(0)).setSelectedFile( + directory.resolve("default.txt").toFile()); result.completeAsync(() -> new Bytes("saved contents"), Runnable::run); SwingUtilities.invokeAndWait(() -> {}); assertThat(Files.readString(output)).isEqualTo("saved contents"); @@ -136,7 +142,7 @@ public void promptAndSaveCancelsFutureWhenDialogIsCancelled() .thenReturn(JFileChooser.CANCEL_OPTION))) { CompletableFuture result = - UiUtils.promptAndSave(null, "Save file", "default.txt"); + UiUtils.promptAndSave(null, preferences, "Save file", "default.txt"); assertThat(result.isCancelled()).isTrue(); AtomicBoolean extracted = new AtomicBoolean(); result.completeAsync(() -> {