This is an automated email from the ASF dual-hosted git repository. desruisseaux pushed a commit to branch geoapi-4.0 in repository https://gitbox.apache.org/repos/asf/sis.git
The following commit(s) were added to refs/heads/geoapi-4.0 by this push: new b957357 Better recording of recently used file (ordered by file closed instead of file opened, remove from menu the opened files). b957357 is described below commit b957357d51d50e7aa8e3aca9a1a8754853ebfa2c Author: Martin Desruisseaux <martin.desruisse...@geomatys.com> AuthorDate: Tue Nov 2 13:57:47 2021 +0100 Better recording of recently used file (ordered by file closed instead of file opened, remove from menu the opened files). --- .../main/java/org/apache/sis/gui/RecentFiles.java | 55 +++++--- .../java/org/apache/sis/gui/dataset/LoadEvent.java | 31 +---- .../dataset/{LoadEvent.java => ResourceEvent.java} | 41 ++++-- .../apache/sis/gui/dataset/ResourceExplorer.java | 29 ++++- .../org/apache/sis/gui/dataset/ResourceTree.java | 141 ++++++++++++++------- .../apache/sis/internal/gui/ResourceLoader.java | 2 +- 6 files changed, 190 insertions(+), 109 deletions(-) diff --git a/application/sis-javafx/src/main/java/org/apache/sis/gui/RecentFiles.java b/application/sis-javafx/src/main/java/org/apache/sis/gui/RecentFiles.java index 7a5a9fd..42ec7f6 100644 --- a/application/sis-javafx/src/main/java/org/apache/sis/gui/RecentFiles.java +++ b/application/sis-javafx/src/main/java/org/apache/sis/gui/RecentFiles.java @@ -25,7 +25,7 @@ import javafx.event.EventHandler; import javafx.scene.control.Menu; import javafx.scene.control.MenuItem; import javafx.collections.ObservableList; -import org.apache.sis.gui.dataset.LoadEvent; +import org.apache.sis.gui.dataset.ResourceEvent; import org.apache.sis.gui.dataset.ResourceExplorer; import org.apache.sis.internal.gui.Resources; import org.apache.sis.internal.gui.RecentChoices; @@ -36,7 +36,7 @@ import org.apache.sis.util.ArraysExt; * Manages a list of recently opened files. The list of files is initialized from user preferences. * * @author Martin Desruisseaux (Geomatys) - * @version 1.1 + * @version 1.2 * @since 1.1 */ final class RecentFiles implements EventHandler<ActionEvent> { @@ -65,9 +65,9 @@ final class RecentFiles implements EventHandler<ActionEvent> { } /** - * Creates a menu for a list of recently used files. The menu is initialized with a list of files - * fetched for user preferences. The user preferences are updated when {@link #opened(LoadEvent)} - * is invoked. + * Creates a menu for a list of recently used files. + * The menu is initialized with a list of files fetched from user preferences. + * The user preferences are updated when {@link #touched(ResourceEvent, boolean)} is invoked. */ static Menu create(final ResourceExplorer explorer, final Resources localized) { final Menu menu = new Menu(localized.getString(Resources.Keys.OpenRecentFile)); @@ -82,7 +82,8 @@ final class RecentFiles implements EventHandler<ActionEvent> { } } handler.items.setAll(ArraysExt.resize(items, n)); - explorer.setOnResourceLoaded(handler::opened); + explorer.setOnResourceLoaded((e) -> handler.touched(e, false)); + explorer.setOnResourceClosed((e) -> handler.touched(e, true)); return menu; } @@ -93,15 +94,17 @@ final class RecentFiles implements EventHandler<ActionEvent> { final MenuItem item = new MenuItem(file.getName()); item.setUserData(file); item.setOnAction(this); + item.setDisable(!file.exists()); return item; } /** - * Notifies that a file has been opened. A new menu items is created for the specified file - * and is inserted at the beginning of the list of recent files. If the list of recent files - * has more than {@value #MAX_COUNT} elements, the latest item is discarded. + * Notifies that a file is opened or closed. If opened, the file item (if any) is temporarily removed. + * If closed, a new menu items is created for the specified file and is inserted at the beginning of + * the list of recent files. If the list of recent files has more than {@value #MAX_COUNT} elements, + * the latest item is discarded. */ - public void opened(final LoadEvent event) { + private void touched(final ResourceEvent event, final boolean closed) { final Path path = event.getResourcePath(); final File file; try { @@ -113,7 +116,7 @@ final class RecentFiles implements EventHandler<ActionEvent> { final int size = items.size(); /* * Verifies if an item already exists for the given file. - * If yes, we will just move it. + * If yes, we will remove it (if opening) or move it to the top (if closing). */ MenuItem item = null; for (int i=0; i<size; i++) { @@ -122,23 +125,35 @@ final class RecentFiles implements EventHandler<ActionEvent> { break; } } - if (item == null) { - if (size >= MAX_COUNT) { - item = items.remove(size-1); - item.setText(file.getName()); - item.setUserData(file); - } else { - item = createItem(file); + /* + * The menu item has been removed. Reinsert it at the top of the list only if the file has been closed. + * But save in the preferences in all cases, in case the application shutdown without closing the files. + */ + final StringJoiner s = new StringJoiner(System.lineSeparator()); + int count = 0; + if (closed) { + if (item == null) { + if (size >= MAX_COUNT) { + item = items.remove(size-1); + item.setText(file.getName()); + item.setUserData(file); + item.setDisable(false); + } else { + item = createItem(file); + } } + items.add(0, item); + } else { + s.add(file.getPath()); // Explicit saved because that item is no longer in the menu. + count = 1; } - items.add(0, item); /* * At this point the menu items has been updated. * Now save the file list in user preferences. */ - final StringJoiner s = new StringJoiner(System.lineSeparator()); for (final MenuItem i : items) { s.add(((File) i.getUserData()).getPath()); + if (++count >= MAX_COUNT) break; } RecentChoices.setFiles(s.toString()); } diff --git a/application/sis-javafx/src/main/java/org/apache/sis/gui/dataset/LoadEvent.java b/application/sis-javafx/src/main/java/org/apache/sis/gui/dataset/LoadEvent.java index 6e0ed8d..5b97839 100644 --- a/application/sis-javafx/src/main/java/org/apache/sis/gui/dataset/LoadEvent.java +++ b/application/sis-javafx/src/main/java/org/apache/sis/gui/dataset/LoadEvent.java @@ -17,50 +17,31 @@ package org.apache.sis.gui.dataset; import java.nio.file.Path; -import javafx.event.Event; -import javafx.event.EventType; /** * Event sent when a resource is loaded. * * @author Martin Desruisseaux (Geomatys) - * @version 1.1 + * @version 1.2 * @since 1.1 + * + * @deprecated Renamed {@link ResourceEvent}. */ -public final class LoadEvent extends Event { +@Deprecated +public final class LoadEvent extends ResourceEvent { /** * For cross-version compatibility. */ private static final long serialVersionUID = 5935085957507976585L; /** - * The only valid type for the load event. - */ - private static final EventType<LoadEvent> LOAD = new EventType<>("LOAD"); - - /** - * Path to the resource being loaded. - */ - private final Path path; - - /** * Creates a new event. * * @param source the source of this event. * @param path path to the file being loaded. */ LoadEvent(final ResourceTree source, final Path path) { - super(source, null, LOAD); - this.path = path; - } - - /** - * Returns the path to the resource being loaded. - * - * @return path to the resource being loaded. - */ - public Path getResourcePath() { - return path; + super(source, path, LOADED); } } diff --git a/application/sis-javafx/src/main/java/org/apache/sis/gui/dataset/LoadEvent.java b/application/sis-javafx/src/main/java/org/apache/sis/gui/dataset/ResourceEvent.java similarity index 53% copy from application/sis-javafx/src/main/java/org/apache/sis/gui/dataset/LoadEvent.java copy to application/sis-javafx/src/main/java/org/apache/sis/gui/dataset/ResourceEvent.java index 6e0ed8d..fdd75cb 100644 --- a/application/sis-javafx/src/main/java/org/apache/sis/gui/dataset/LoadEvent.java +++ b/application/sis-javafx/src/main/java/org/apache/sis/gui/dataset/ResourceEvent.java @@ -22,25 +22,41 @@ import javafx.event.EventType; /** - * Event sent when a resource is loaded. + * Event sent when a resource is loaded or closed. The {@linkplain #getSource() source} + * of this event are the {@link ResourceTree} instance on which handlers are registered. + * The event contains a {@link Path} to the resource opened or closed. * * @author Martin Desruisseaux (Geomatys) - * @version 1.1 - * @since 1.1 + * @version 1.2 + * + * @see ResourceTree#onResourceLoaded + * @see ResourceTree#onResourceClosed + * + * @since 1.2 */ -public final class LoadEvent extends Event { +@SuppressWarnings("CloneableImplementsClone") +public class ResourceEvent extends Event { /** * For cross-version compatibility. */ - private static final long serialVersionUID = 5935085957507976585L; + private static final long serialVersionUID = -425980517754215310L; /** - * The only valid type for the load event. + * The type for load events. + * + * @see ResourceTree#onResourceLoaded + */ + static final EventType<ResourceEvent> LOADED = new EventType<>("LOADED"); + + /** + * The type for close events. + * + * @see ResourceTree#onResourceClosed */ - private static final EventType<LoadEvent> LOAD = new EventType<>("LOAD"); + static final EventType<ResourceEvent> CLOSED = new EventType<>("CLOSED"); /** - * Path to the resource being loaded. + * Path to the resource being loaded or closed. */ private final Path path; @@ -49,16 +65,17 @@ public final class LoadEvent extends Event { * * @param source the source of this event. * @param path path to the file being loaded. + * @param type the type of event. */ - LoadEvent(final ResourceTree source, final Path path) { - super(source, null, LOAD); + ResourceEvent(final ResourceTree source, final Path path, final EventType<ResourceEvent> type) { + super(source, null, type); this.path = path; } /** - * Returns the path to the resource being loaded. + * Returns the path to the resource being loaded or closed. * - * @return path to the resource being loaded. + * @return path to the resource being loaded or closed. */ public Path getResourcePath() { return path; diff --git a/application/sis-javafx/src/main/java/org/apache/sis/gui/dataset/ResourceExplorer.java b/application/sis-javafx/src/main/java/org/apache/sis/gui/dataset/ResourceExplorer.java index 7156284..a5516b7 100644 --- a/application/sis-javafx/src/main/java/org/apache/sis/gui/dataset/ResourceExplorer.java +++ b/application/sis-javafx/src/main/java/org/apache/sis/gui/dataset/ResourceExplorer.java @@ -264,7 +264,7 @@ public class ResourceExplorer extends WindowManager { * * @return current function to be called after a resource has been loaded, or {@code null} if none. */ - public EventHandler<LoadEvent> getOnResourceLoaded() { + public EventHandler<ResourceEvent> getOnResourceLoaded() { return resources.onResourceLoaded.get(); } @@ -275,11 +275,36 @@ public class ResourceExplorer extends WindowManager { * * @param handler new function to be called after a resource has been loaded, or {@code null} if none. */ - public void setOnResourceLoaded(final EventHandler<LoadEvent> handler) { + public void setOnResourceLoaded(final EventHandler<ResourceEvent> handler) { resources.onResourceLoaded.set(handler); } /** + * Returns the function to be called when a resource is closed. + * This is an accessor for the {@link ResourceTree#onResourceClosed} property value. + * + * @return current function to be called when a resource is closed, or {@code null} if none. + * + * @since 1.2 + */ + public EventHandler<ResourceEvent> getOnResourceClosed() { + return resources.onResourceClosed.get(); + } + + /** + * Specifies a function to be called when a resource is closed. + * This is a setter for the {@link ResourceTree#onResourceClosed} property value. + * If this method is never invoked, then the default value is {@code null}. + * + * @param handler new function to be called when a resource is closed, or {@code null} if none. + * + * @since 1.2 + */ + public void setOnResourceClosed(final EventHandler<ResourceEvent> handler) { + resources.onResourceClosed.set(handler); + } + + /** * Loads all given sources in background threads and add them to the resource tree. * The given collection typically contains files to load, * but may also contain {@link Resource} instances to add directly. diff --git a/application/sis-javafx/src/main/java/org/apache/sis/gui/dataset/ResourceTree.java b/application/sis-javafx/src/main/java/org/apache/sis/gui/dataset/ResourceTree.java index f79ab40..a4fc1d9 100644 --- a/application/sis-javafx/src/main/java/org/apache/sis/gui/dataset/ResourceTree.java +++ b/application/sis-javafx/src/main/java/org/apache/sis/gui/dataset/ResourceTree.java @@ -69,6 +69,7 @@ import org.apache.sis.internal.gui.Resources; import org.apache.sis.internal.gui.Styles; import org.apache.sis.internal.system.Modules; import org.apache.sis.internal.util.Strings; +import org.apache.sis.storage.DataStoreProvider; import org.apache.sis.util.logging.Logging; @@ -107,8 +108,20 @@ public class ResourceTree extends TreeView<Resource> { * The default value is {@code null}. * * @see #loadResource(Object) + * @see ResourceEvent#LOADED */ - public final ObjectProperty<EventHandler<LoadEvent>> onResourceLoaded; + public final ObjectProperty<EventHandler<ResourceEvent>> onResourceLoaded; + + /** + * Function to be called after a resource has been closed from a file or URL. + * The default value is {@code null}. + * + * @see #removeAndClose(Resource) + * @see ResourceEvent#CLOSED + * + * @since 1.2 + */ + public final ObjectProperty<EventHandler<ResourceEvent>> onResourceClosed; /** * Creates a new tree of resources with initially no resource to show. @@ -120,6 +133,7 @@ public class ResourceTree extends TreeView<Resource> { setOnDragOver(ResourceTree::onDragOver); setOnDragDropped(this::onDragDropped); onResourceLoaded = new SimpleObjectProperty<>(this, "onResourceLoaded"); + onResourceClosed = new SimpleObjectProperty<>(this, "onResourceClosed"); } /** @@ -186,10 +200,10 @@ public class ResourceTree extends TreeView<Resource> { addTo = (Root) root; } else { final TreeItem<Resource> group = new TreeItem<>(); - addTo = new Root(group, root); - group.setValue(addTo); - setRoot(group); setShowRoot(false); + setRoot(group); // Also detach `item` from the TreeView root. + addTo = new Root(group, item); // Pseudo-resource for a group of data stores. + group.setValue(addTo); } return addTo.add(resource); } @@ -222,13 +236,12 @@ public class ResourceTree extends TreeView<Resource> { addResource((Resource) source); } else { final ResourceLoader loader = new ResourceLoader(source); - final Resource existing = loader.fromCache(); + final DataStore existing = loader.fromCache(); if (existing != null) { addResource(existing); } else { loader.setOnSucceeded((event) -> { - addResource((Resource) event.getSource().getValue()); - notifyLoaded(source); + addLoadedResource((DataStore) event.getSource().getValue(), source); }); loader.setOnFailed((event) -> ExceptionReporter.show(this, event)); BackgroundThreads.execute(loader); @@ -238,23 +251,28 @@ public class ResourceTree extends TreeView<Resource> { } /** - * Notifies {@link #onResourceLoaded} handler that a resource at the given path has been loaded. + * Adds the given store as a resource, then notifies {@link #onResourceLoaded} + * handler that a resource at the given path has been loaded. * This method is invoked from JavaFX thread. - * - * <p>Those notifications are not used by {@code ResourceTree} itself. - * They are useful to other packages, for example for managing a list of opened files.</p> */ - private void notifyLoaded(final Object source) { - final EventHandler<LoadEvent> handler = onResourceLoaded.getValue(); + private void addLoadedResource(final DataStore store, final Object source) { + addResource(store); + final EventHandler<ResourceEvent> handler = onResourceLoaded.getValue(); if (handler != null) { final Path path; try { path = IOUtilities.toPathOrNull(source); } catch (IllegalArgumentException | FileSystemNotFoundException e) { - Logging.recoverableException(Logging.getLogger(Modules.APPLICATION), ResourceTree.class, "loadResource", e); + recoverableException("loadResource", e); return; } if (path != null) { + /* + * Following call should be quick because it starts the search from last item. + * A `NullPointerException` or `ClassCastException` here would be a bug in our + * wrapping of resources. + */ + ((Item) findOrRemove(store, false)).path = path; handler.handle(new LoadEvent(this, path)); } } @@ -318,6 +336,11 @@ public class ResourceTree extends TreeView<Resource> { * Children of {@link Aggregate} resource and not scanned. * If the given resource can not be removed, then this method does nothing.</p> * + * <h4>Notifications</h4> + * If {@link #onResourceClosed} has a non-null value, the {@link EventHandler} will be notified. + * The notification may happen in same time that the resource is closing in a background thread. + * If an exception occurs while closing the resource, the error is reported in a dialog box. + * * @param resource the resource to remove. Null values are ignored. * * @see #setResource(Resource) @@ -325,9 +348,27 @@ public class ResourceTree extends TreeView<Resource> { * @see ResourceExplorer#removeAndClose(Resource) */ public void removeAndClose(final Resource resource) { - if (findOrRemove(resource, true)) { - if (resource instanceof DataStore) { - ResourceLoader.removeAndClose((DataStore) resource, this); + final TreeItem<Resource> item = findOrRemove(resource, true); + if (item != null && resource instanceof DataStore) { + final DataStore store = (DataStore) resource; + ResourceLoader.removeAndClose(store, this); + final EventHandler<ResourceEvent> handler = onResourceClosed.get(); + if (handler != null) { + Path path = null; + if (item instanceof Item) { + path = ((Item) item).path; + } + if (path == null) try { + path = store.getOpenParameters() + .map((p) -> IOUtilities.toPathOrNull(p.parameter(DataStoreProvider.LOCATION).getValue())) + .orElse(null); + } catch (IllegalArgumentException | FileSystemNotFoundException e) { + // Ignore because the location parameter is optional. + recoverableException("removeAndClose", e); + } + if (path != null) { + handler.handle(new ResourceEvent(this, path, ResourceEvent.CLOSED)); + } } } } @@ -337,9 +378,9 @@ public class ResourceTree extends TreeView<Resource> { * * @param resource the resource to search of remove, or {@code null}. * @param remove {@code true} for removing the resource, or {@code false} for checking only. - * @return whether the resource has been found in the roots. + * @return the item wrapping the resource, or {@code null} if the resource has been found in the roots. */ - private boolean findOrRemove(final Resource resource, final boolean remove) { + private TreeItem<Resource> findOrRemove(final Resource resource, final boolean remove) { assert Platform.isFxApplicationThread(); if (resource != null) { /* @@ -365,19 +406,15 @@ public class ResourceTree extends TreeView<Resource> { if (remove) { setRoot(null); } - return true; + return item; } if (root instanceof Root) { - if (remove) { - return ((Root) root).remove(resource); - } else { - return ((Root) root).contains(resource); - } + return ((Root) root).contains(resource, remove); } } } } - return false; + return null; } /** @@ -475,6 +512,13 @@ public class ResourceTree extends TreeView<Resource> { } /** + * Reports an ignorable exception in the given method. + */ + private static void recoverableException(final String method, final Exception e) { + Logging.recoverableException(Logging.getLogger(Modules.APPLICATION), ResourceTree.class, method, e); + } + + /** * Reports an unexpected but non-fatal exception in the given method. */ static void unexpectedException(final String method, final Exception e) { @@ -566,7 +610,7 @@ public class ResourceTree extends TreeView<Resource> { * If the resource is one of the "root" resources, add a menu for removing it. * If we find that the cell already has a menu, we do not need to build it again. */ - if (tree.findOrRemove(resource, false)) { + if (tree.findOrRemove(resource, false) != null) { menu = getContextMenu(); if (menu == null) { menu = new ContextMenu(); @@ -617,6 +661,12 @@ public class ResourceTree extends TreeView<Resource> { */ private static final class Item extends TreeItem<Resource> { /** + * The path to the resource, or {@code null} if none or unknown. This is used for notifications only; + * this information does not play an important role for {@link ResourceTree} itself. + */ + Path path; + + /** * The text of this node, computed and cached when first needed. * Computation is done by invoking {@link #findLabel(Resource, Locale)} in a background thread. * @@ -785,6 +835,7 @@ public class ResourceTree extends TreeView<Resource> { * Invoked in JavaFX thread if children can not be loaded. */ @Override + @SuppressWarnings("unchecked") protected void failed() { Item.super.getChildren().setAll(new Item(getException())); } @@ -795,7 +846,7 @@ public class ResourceTree extends TreeView<Resource> { /** - * The root resource when there is more than one resources to display. + * The root pseudo-resource when there is more than one resources to display. * This root node should be hidden in the {@link ResourceTree}. */ private static final class Root implements Aggregate { @@ -805,31 +856,33 @@ public class ResourceTree extends TreeView<Resource> { private final List<TreeItem<Resource>> components; /** - * Creates a new aggregate which is going to be wrapped in the given item. - * Caller should invoke {@code group.setValue(root)} after this constructor. + * Creates a new aggregate which is going to be wrapped in the given node. + * Caller shall invoke {@code group.setValue(root)} after this constructor. * - * @param group the new tree root which will contain "real" resources. + * @param group the new tree root which will contain "real" resources. * @param previous the previous root, to be added in the new group. */ - Root(final TreeItem<Resource> group, final Resource previous) { + Root(final TreeItem<Resource> group, final TreeItem<Resource> previous) { components = group.getChildren(); - add(previous); + components.add(previous); } /** - * Returns whether this root contains the given resource as a direct child. + * Checks whether this root contains the given resource as a direct child. * This method does not search recursively in sub-trees. * * @param resource the resource to search. - * @return whether the given resource is present. + * @param remove whether to remove the resource if found. + * @return the resource wrapper, or {@code null} if not found. */ - boolean contains(final Resource resource) { + TreeItem<Resource> contains(final Resource resource, final boolean remove) { for (int i=components.size(); --i >= 0;) { - if (components.get(i).getValue() == resource) { - return true; + final TreeItem<Resource> item = components.get(i); + if (item.getValue() == resource) { + return remove ? components.remove(i) : item; } } - return false; + return null; } /** @@ -848,16 +901,6 @@ public class ResourceTree extends TreeView<Resource> { } /** - * Removes the given resource if presents. - * - * @param resource the resource to remove. - * @return whether the resource has been removed. - */ - boolean remove(final Resource resource) { - return components.removeIf((i) -> i.getValue() == resource); - } - - /** * Returns a read-only view of the components. This method is not used directly by {@link ResourceTree} * but is defined in case a user invoke {@link ResourceTree#getResource()}. For this reason, it is not * worth to cache the list created in this method. diff --git a/application/sis-javafx/src/main/java/org/apache/sis/internal/gui/ResourceLoader.java b/application/sis-javafx/src/main/java/org/apache/sis/internal/gui/ResourceLoader.java index 1191b40..10e44f2 100644 --- a/application/sis-javafx/src/main/java/org/apache/sis/internal/gui/ResourceLoader.java +++ b/application/sis-javafx/src/main/java/org/apache/sis/internal/gui/ResourceLoader.java @@ -71,7 +71,7 @@ import org.apache.sis.gui.DataViewer; * @since 1.1 * @module */ -public final class ResourceLoader extends Task<Resource> { +public final class ResourceLoader extends Task<DataStore> { /** * The cache of previously loaded resources. * Used for avoiding to load the same resource twice.