diff --git a/java/com/cowlark/fluxengine/gui/ApplicationFrame.java b/java/com/cowlark/fluxengine/gui/ApplicationFrame.java index 49c7c83e..4ee212b0 100644 --- a/java/com/cowlark/fluxengine/gui/ApplicationFrame.java +++ b/java/com/cowlark/fluxengine/gui/ApplicationFrame.java @@ -1,5 +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; @@ -30,7 +32,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; @@ -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); @@ -68,62 +70,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 +129,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 189c2338..499f4d1d 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,15 +45,14 @@ 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; 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; @@ -199,9 +198,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 +285,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 +369,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 +396,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 +413,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 @@ -512,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/FilesystemExecutor.java b/java/com/cowlark/fluxengine/gui/FilesystemExecutor.java new file mode 100644 index 00000000..0f79d990 --- /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 2d8dfea9..aff970cd 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; @@ -37,19 +40,23 @@ 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.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.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 { @@ -60,6 +67,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() @@ -119,6 +127,13 @@ 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()); + if (!GraphicsEnvironment.isHeadless()) + treeTable.setDragEnabled(true); + filesSelected = Var.of(Tuple.of(TreePath.class)); treeSelectionModel = treeTable.getTreeSelectionModel(); treeSelectionModel.setSelectionMode(TreeSelectionModel.DISCONTIGUOUS_TREE_SELECTION); @@ -141,40 +156,35 @@ 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); - 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)) @@ -199,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 @@ -207,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") @@ -221,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") @@ -296,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, model.getPreferencesReaderWriter(), "Save file", filename).completeAsync( + wrapIOException(() -> { + Filesystem fs = treeTableModel.getFilesystem(); + return fs.getFile(vfsPath); + }), treeTableModel.getExecutor()); return; } } @@ -311,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, model.getPreferencesReaderWriter(), "Save multiple files", "files.zip").completeAsync( + wrapIOException(() -> { + Filesystem fs = treeTableModel.getFilesystem(); + return fs.getFiles(fs, snapshot); + }), treeTableModel.getExecutor()); } private void viewFile( @@ -326,13 +318,17 @@ 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() + { + return new FilesystemPanelTransferHandler(this); } private void putFiles( @@ -341,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); @@ -349,18 +345,26 @@ private void putFiles( if (result != JFileChooser.APPROVE_OPTION) return; - File[] selectedFiles = chooser.getSelectedFiles(); + putFiles(parentPath, ImmutableList.copyOf(chooser.getSelectedFiles())); + } - treeTableModel.queueFilesystemOperation(fs -> { - for (File file : selectedFiles) - { - Bytes data = Bytes.readFromFile(file.toPath()); - String leafName = file.getName(); - VfsPath vfsName = parentPath.resolve(leafName); - fs.putFile(vfsName, data); - } - resyncAllExpandedAsync(); - }); + void putFiles(VfsPath parentPath, ImmutableList files) + { + /* Read host files and write their contents on the filesystem worker thread. */ + 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( @@ -371,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( @@ -384,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( @@ -410,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() @@ -481,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( @@ -514,19 +535,27 @@ 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); } - /** - * 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++) @@ -544,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) { @@ -572,4 +602,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 00000000..40527fab --- /dev/null +++ b/java/com/cowlark/fluxengine/gui/FilesystemPanelTransferHandler.java @@ -0,0 +1,230 @@ +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.Filesystem; +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 = 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) + { + 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; + } + + @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.whenCompleteAsync((files, error) -> loop.exit(), UiUtils.EDT_EXECUTOR); + + /* The executor can run inline if extraction finished before registration. */ + if (!result.isDone()) + 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/java/com/cowlark/fluxengine/gui/FilesystemTreeTableModel.java b/java/com/cowlark/fluxengine/gui/FilesystemTreeTableModel.java index 2f03aafe..3b8bbf90 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 4f994c77..f2fa8f82 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,8 +33,8 @@ public void run(ImmutableList args) throws Exception UIManager.setLookAndFeel(new FlatDarkLaf()); System.setProperty("apple.laf.useScreenMenuBar", "true"); - SwingUtilities.invokeLater(() -> { - ApplicationFrame frame = new ApplicationFrame(model); + UiUtils.EDT_EXECUTOR.execute(() -> { + ApplicationFrame frame = new ApplicationFrame(model, preferencesReaderWriter); frame.pack(); frame.setVisible(true); }); diff --git a/java/com/cowlark/fluxengine/gui/ImageViewModel.java b/java/com/cowlark/fluxengine/gui/ImageViewModel.java index 1957f925..b0f8cba7 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 @@ -66,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; @@ -221,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) @@ -248,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) @@ -273,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) @@ -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 f4bb3bfd..17cb0322 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/PreferencesReaderWriter.java b/java/com/cowlark/fluxengine/gui/PreferencesReaderWriter.java index 7ef2dbb2..0569583a 100644 --- a/java/com/cowlark/fluxengine/gui/PreferencesReaderWriter.java +++ b/java/com/cowlark/fluxengine/gui/PreferencesReaderWriter.java @@ -1,8 +1,16 @@ 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; import sprouts.Pair; +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; @@ -16,6 +24,8 @@ 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 CURRENT_DIRECTORY = "current_directory"; static final String ADVANCED_SETTINGS = "advanced_settings"; static final String DEVICE_FLUXFILE = "fluxfile"; @@ -53,7 +63,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 +82,31 @@ 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(), + 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/java/com/cowlark/fluxengine/gui/UiUtils.java b/java/com/cowlark/fluxengine/gui/UiUtils.java index 8bb4ae40..8521aaf7 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,66 @@ 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, + PreferencesReaderWriter preferences, String dialogTitle, - String defaultFileName, - Consumer> dataSupplier) + String defaultFileName) { - JFileChooser chooser = new JFileChooser(); + CompletableFuture result = new CompletableFuture<>(); + JFileChooser chooser = preferences.createFileChooser(); chooser.setDialogTitle(dialogTitle); - chooser.setSelectedFile(new File(defaultFileName)); + chooser.setSelectedFile(new File(chooser.getCurrentDirectory(), 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 +262,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 4d715852..908977f9 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 4d1e56dd..24d37f26 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 99997043..ffa3e883 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", ], ) @@ -74,6 +80,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", @@ -88,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 00000000..879a6596 --- /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 67bd806a..6bfcdd7a 100644 --- a/javatests/com/cowlark/fluxengine/gui/FilesystemPanelHeadlessTest.java +++ b/javatests/com/cowlark/fluxengine/gui/FilesystemPanelHeadlessTest.java @@ -4,14 +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; @@ -19,10 +23,20 @@ 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.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; @@ -35,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() @@ -46,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")); @@ -61,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()); @@ -112,6 +122,175 @@ 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 + { + SwingUtilities.invokeAndWait(() -> { + 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.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); + 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"); + 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. */ @@ -132,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); @@ -257,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/PreferencesReaderWriterTest.java b/javatests/com/cowlark/fluxengine/gui/PreferencesReaderWriterTest.java index 36a22996..aafd5195 100644 --- a/javatests/com/cowlark/fluxengine/gui/PreferencesReaderWriterTest.java +++ b/javatests/com/cowlark/fluxengine/gui/PreferencesReaderWriterTest.java @@ -5,10 +5,20 @@ 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; 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 +35,98 @@ 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() + { + 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() { diff --git a/javatests/com/cowlark/fluxengine/gui/UiUtilsTest.java b/javatests/com/cowlark/fluxengine/gui/UiUtilsTest.java index e70d9357..23223cdf 100644 --- a/javatests/com/cowlark/fluxengine/gui/UiUtilsTest.java +++ b/javatests/com/cowlark/fluxengine/gui/UiUtilsTest.java @@ -1,8 +1,28 @@ 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 java.util.prefs.Preferences; +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 +30,129 @@ @RunWith(JUnit4.class) public class UiUtilsTest { + private final PreferencesReaderWriter preferences = + new PreferencesReaderWriter(Mockito.mock(Preferences.class)); + @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()); + Mockito.when(chooser.getCurrentDirectory()).thenReturn(directory.toFile()); + })) + { + CompletableFuture result = + 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"); + } 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, preferences, "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 +236,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 +256,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 +274,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 +288,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 +306,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 +315,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 +324,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"); } }