Merge pull request #3507 from ControlSystemStudio/snapshot_controller_refactoring

Fix close checks in save&restore

Author: georgweiss

app/save-and-restore/app/doc/images/filter-pv-items.png

@@ -324,12 +324,25 @@ Prior to restore user has the option to:
 
 Restoring from a composite snapshot works in the same manner as the restore operation from a single-snapshot.
 
+Filter PV items in list
+^^^^^^^^^^^^^^^^^^^^^^^
+The list of items in the snapshot view can be filtered based on the PV name. See screenshot for highlighted UI element
+where user may specify a string pattern to match PV names. Non-matching items will be hidden from the list view and
+**also excluded from a restore operation**.
+
+To filter the view without excluding PV items from a restore operation, user needs to tick the “Preserve selection…” checkbox.
+
+.. image:: images/filter-pv-items.png
+   :width: 80%
+
+
 Restore from context menu
 ^^^^^^^^^^^^^^^^^^^^^^^^^
 
 User may invoke a restore operation (from client or from service) from context menu items in the tree
-view or in the search-and-filer view. In this case user will not have the possibility to unselect specific PVs.
-However, PV items configured as read-only will not be restored.
+view or in the search-and-filter view. In this case user will not have the possibility to deselect specific PVs.
+Filtering/exclusion based on PV name will not be possible either.
+However, PV items configured as read-only will always be excluded from a restore operation.
 
 Restore result
 ^^^^^^^^^^^^^^

app/save-and-restore/app/doc/index.rst

@@ -34,7 +34,10 @@ public class Messages {
     public static String cannotCompareTitle;
     public static String closeConfigurationWarning;
     public static String closeCompositeSnapshotWarning;
-    public static String closeTabPrompt;
+    public static String closeSnapshotWarning;
+    public static String closeCompositeSnapshotTabPrompt;
+    public static String closeConfigurationTabPrompt;
+    public static String closeSnapshotTabPrompt;
     public static String compositeSnapshotConsistencyCheckFailed;
     public static String contextMenuAddTag;
     @Deprecated
@@ -120,7 +123,6 @@ public class Messages {
 
     public static String overwrite;
     public static String paste;
-    public static String promptCloseSnapshotTabContent;
     public static String promptDeleteSelectedTitle;
     public static String promptDeleteSelectedHeader;
     public static String promptDeleteSelectedContent;

app/save-and-restore/app/src/main/java/org/phoebus/applications/saveandrestore/Messages.java

@@ -18,14 +18,15 @@
 
 package org.phoebus.applications.saveandrestore;
 
+import javafx.application.Platform;
 import javafx.fxml.FXMLLoader;
 import org.phoebus.applications.saveandrestore.ui.SaveAndRestoreController;
 import org.phoebus.framework.nls.NLS;
 import org.phoebus.framework.persistence.Memento;
 import org.phoebus.framework.spi.AppDescriptor;
 import org.phoebus.framework.spi.AppInstance;
 import org.phoebus.security.tokens.ScopedAuthenticationToken;
-import org.phoebus.ui.docking.DockItem;
+import org.phoebus.ui.docking.DockItemWithInput;
 import org.phoebus.ui.docking.DockPane;
 
 import java.net.URI;
@@ -37,9 +38,10 @@
 
 public class SaveAndRestoreInstance implements AppInstance {
 
+
     private final AppDescriptor appDescriptor;
     private final SaveAndRestoreController saveAndRestoreController;
-    private DockItem dockItem;
+    private DockItemWithInput dockItem;
 
     public static SaveAndRestoreInstance INSTANCE;
 
@@ -53,21 +55,65 @@ public SaveAndRestoreInstance(AppDescriptor appDescriptor) {
             ResourceBundle resourceBundle = NLS.getMessages(Messages.class);
             loader.setResources(resourceBundle);
             loader.setLocation(SaveAndRestoreApplication.class.getResource("ui/SaveAndRestoreUI.fxml"));
-            dockItem = new DockItem(this, loader.load());
+            dockItem = new DockItemWithInput(this, loader.load(), null, null, null);
         } catch (Exception e) {
             Logger.getLogger(SaveAndRestoreInstance.class.getName()).log(Level.SEVERE, "Failed loading fxml", e);
         }
 
         saveAndRestoreController = loader.getController();
+
+        // When user closes Phoebus...
         dockItem.addCloseCheck(() -> {
-            saveAndRestoreController.handleTabClosed();
-            INSTANCE = null;
-            return CompletableFuture.completedFuture(true);
+            boolean mayClose = mayClose();
+            if (mayClose) {
+                saveAndRestoreController.handleTabClosed();
+                INSTANCE = null;
+            }
+            return CompletableFuture.completedFuture(mayClose);
+        });
+
+        // When user closes save&restore app...
+        dockItem.setOnCloseRequest(event -> {
+            if (mayClose()) {
+                saveAndRestoreController.handleTabClosed();
+                INSTANCE = null;
+            } else {
+                event.consume();
+            }
         });
 
         DockPane.getActiveDockPane().addTab(dockItem);
     }
 
+    /**
+     * Checks with each tab if it may be closed.
+     *
+     * @return <code>false</code> if any of the tabs contains unsaved data, otherwise <code>true</code>.
+     */
+    private boolean mayClose() {
+        if (Platform.isFxApplicationThread()) {
+            return saveAndRestoreController.doCloseCheck();
+        } else {
+            CompletableFuture<Boolean> completableFuture = new CompletableFuture<>();
+            Platform.runLater(() -> {
+                try {
+                    boolean okToClose = saveAndRestoreController.doCloseCheck();
+                    completableFuture.complete(okToClose);
+                } catch (Exception e) {
+                    Logger.getLogger(SaveAndRestoreInstance.class.getName()).log(Level.SEVERE,
+                        "Got exception when checking if OK to close", e);
+                    completableFuture.complete(true);
+                }
+            });
+            try {
+                return completableFuture.get();
+            } catch (Exception e) {
+                Logger.getLogger(SaveAndRestoreInstance.class.getName()).log(Level.WARNING, "Got exception when waiting for close check evaluation", e);
+                return true;
+            }
+        }
+    }
+
     @Override
     public AppDescriptor getAppDescriptor() {
         return appDescriptor;

app/save-and-restore/app/src/main/java/org/phoebus/applications/saveandrestore/SaveAndRestoreInstance.java

@@ -76,8 +76,14 @@ public SimpleStringProperty getUserIdentity() {
     protected void handleWebSocketMessage(SaveAndRestoreWebSocketMessage<?> webSocketMessage){
     }
 
+    /**
+     * Performs suitable cleanup, e.g. close web socket and PVs (where applicable).
+     */
+    public abstract void handleTabClosed();
 
-    protected boolean handleTabClosed(){
-        return true;
-    }
+    /**
+     * Checks if the tab may be closed, e.g. if data managed in the UI has been saved.
+     * @return <code>false</code> if tab contains unsaved data, otherwise <code>true</code>
+     */
+    public abstract boolean doCloseCheck();
 }

app/save-and-restore/app/src/main/java/org/phoebus/applications/saveandrestore/ui/SaveAndRestoreBaseController.java

@@ -937,11 +937,11 @@ public void saveLocalState() {
     }
 
     @Override
-    public boolean handleTabClosed() {
+    public void handleTabClosed() {
+        tabPane.getTabs().forEach(t -> ((SaveAndRestoreTab)t).handleTabClosed());
         saveLocalState();
         webSocketClientService.closeWebSocket();
         filterActivators.forEach(FilterActivator::stop);
-        return true;
     }
 
     /**
@@ -1436,7 +1436,7 @@ private void addOptionalLoggingMenuItem() {
     }
 
     @Override
-    public void handleWebSocketMessage(SaveAndRestoreWebSocketMessage saveAndRestoreWebSocketMessage) {
+    public void handleWebSocketMessage(SaveAndRestoreWebSocketMessage<?> saveAndRestoreWebSocketMessage) {
         switch (saveAndRestoreWebSocketMessage.messageType()) {
             case NODE_ADDED -> nodeAdded((String) saveAndRestoreWebSocketMessage.payload());
             case NODE_REMOVED -> nodeRemoved((String) saveAndRestoreWebSocketMessage.payload());
@@ -1496,4 +1496,14 @@ private void handleWebSocketDisconnected() {
         serviceConnected.setValue(false);
         saveLocalState();
     }
+
+    @Override
+    public boolean doCloseCheck(){
+        for(Tab tab : tabPane.getTabs()){
+            if(!((SaveAndRestoreTab)tab).doCloseCheck()){
+                return false;
+            }
+        }
+        return true;
+    }
 }

app/save-and-restore/app/src/main/java/org/phoebus/applications/saveandrestore/ui/SaveAndRestoreController.java

@@ -38,7 +38,6 @@
 public abstract class SaveAndRestoreTab extends Tab implements WebSocketMessageHandler {
 
     protected SaveAndRestoreBaseController controller;
-    protected WebSocketClientService webSocketClientService;
 
     public SaveAndRestoreTab() {
         ContextMenu contextMenu = new ContextMenu();
@@ -63,17 +62,13 @@ public SaveAndRestoreTab() {
         contextMenu.getItems().addAll(closeAll, closeOthers);
         setContextMenu(contextMenu);
 
-        webSocketClientService = WebSocketClientService.getInstance();
-
         setOnCloseRequest(event -> {
-            if (!controller.handleTabClosed()) {
-                event.consume();
+            if (doCloseCheck()) {
+                handleTabClosed();
             } else {
-                webSocketClientService.removeWebSocketMessageHandler(this);
+                event.consume();
             }
         });
-
-        webSocketClientService.addWebSocketMessageHandler(this);
     }
 
     /**
@@ -89,4 +84,19 @@ public void secureStoreChanged(List<ScopedAuthenticationToken> validTokens) {
     public void handleWebSocketMessage(SaveAndRestoreWebSocketMessage<?> saveAndRestoreWebSocketMessage) {
 
     }
+
+    /**
+     * Performs suitable cleanup, e.g. close web socket and PVs (where applicable).
+     */
+    public void handleTabClosed() {
+        controller.handleTabClosed();
+    }
+
+    /**
+     * Checks if the tab may be closed, e.g. if data managed in the UI has been saved.
+     * @return <code>false</code> if tab contains unsaved data, otherwise <code>true</code>
+     */
+    public boolean doCloseCheck() {
+        return controller.doCloseCheck();
+    }
 }

app/save-and-restore/app/src/main/java/org/phoebus/applications/saveandrestore/ui/SaveAndRestoreTab.java

@@ -67,6 +67,7 @@
 import org.phoebus.framework.jobs.JobManager;
 import org.phoebus.framework.selection.SelectionService;
 import org.phoebus.ui.application.ContextMenuHelper;
+import org.phoebus.ui.dialog.DialogHelper;
 import org.phoebus.ui.dialog.ExceptionDetailsErrorDialog;
 import org.phoebus.ui.javafx.FocusUtil;
 import org.phoebus.ui.javafx.ImageCache;
@@ -86,7 +87,7 @@ public class ConfigurationController extends SaveAndRestoreBaseController implem
 
     @FXML
     @SuppressWarnings("unused")
-    private BorderPane root;
+    private BorderPane borderPane;
 
     @FXML
     @SuppressWarnings("unused")
@@ -485,7 +486,7 @@ public void loadConfiguration(final Node node) {
             try {
                 configurationData = saveAndRestoreService.getConfiguration(node.getUniqueId());
             } catch (Exception e) {
-                Platform.runLater(() -> ExceptionDetailsErrorDialog.openError(root, Messages.errorGeneric, Messages.errorUnableToRetrieveData, e));
+                Platform.runLater(() -> ExceptionDetailsErrorDialog.openError(borderPane, Messages.errorGeneric, Messages.errorUnableToRetrieveData, e));
                 return;
             }
 
@@ -513,24 +514,28 @@ public void loadConfiguration(final Node node) {
     }
 
     /**
-     * Handles clean-up when the associated {@link ConfigurationTab} is closed.
      * A check is made if content is dirty, in which case user is prompted to cancel or close anyway.
      *
      * @return <code>true</code> if content is not dirty or user chooses to close anyway,
      * otherwise <code>false</code>.
      */
     @Override
-    public boolean handleTabClosed() {
+    public boolean doCloseCheck() {
         if (dirty.get()) {
             Alert alert = new Alert(Alert.AlertType.CONFIRMATION);
-            alert.setTitle(Messages.closeTabPrompt);
+            alert.setTitle(Messages.closeConfigurationTabPrompt);
             alert.setContentText(Messages.closeConfigurationWarning);
+            DialogHelper.positionDialog(alert, borderPane, -200, -200);
             Optional<ButtonType> result = alert.showAndWait();
             return result.isPresent() && result.get().equals(ButtonType.OK);
-        } else {
-            webSocketClientService.removeWebSocketMessageHandler(this);
-            return true;
         }
+
+        return true;
+    }
+
+    @Override
+    public void handleTabClosed(){
+        webSocketClientService.removeWebSocketMessageHandler(this);
     }
 
     @Override

app/save-and-restore/app/src/main/java/org/phoebus/applications/saveandrestore/ui/configuration/ConfigurationController.java

@@ -75,12 +75,4 @@ public SearchAndFilterTab(SaveAndRestoreController saveAndRestoreController) {
         setText(Messages.search);
         setGraphic(new ImageView(ImageCache.getImage(ImageCache.class, "/icons/sar-search_18x18.png")));
     }
-
-    public void filterActivated(String filterName){
-        ((SearchAndFilterViewController)controller).filterActivated(filterName);
-    }
-
-    public void filterDeactivated(String filterName){
-        ((SearchAndFilterViewController)controller).filterDeactivated(filterName);
-    }
 }

app/save-and-restore/app/src/main/java/org/phoebus/applications/saveandrestore/ui/search/SearchAndFilterTab.java

@@ -648,11 +648,13 @@ public void handleWebSocketMessage(SaveAndRestoreWebSocketMessage<?> saveAndRest
         }
     }
 
-    public void filterActivated(String filterName){
-
+    @Override
+    public void handleTabClosed(){
+        webSocketClientService.removeWebSocketMessageHandler(this);
     }
 
-    public void filterDeactivated(String filterName){
-
+    @Override
+    public boolean doCloseCheck(){
+        return true;
     }
 }