Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions app/save-and-restore/app/doc/index.rst
Original file line number Diff line number Diff line change
Expand Up @@ -470,8 +470,8 @@ are supported:

* | Open a configuration, snapshot or composite snapshot node in the Save-And-Restore application.
| This can be used to quickly access a particular node in order to invoke a restore operation.
* | Open a named filter in the Save-And-Restore application.
| This will open/show the search and filter view and automatically perform the search associated with the named filter.
* | Open a named filter in the Save-And-Restore Filter View application.
| This will open/show the Filter View application and automatically perform the search associated with the named filter.
| This feature can be used to quickly navigate from a Display Builder screen to a view containing a set of commonly used snapshots.

Configuring actions
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
/*
* Copyright (C) 2024 European Spallation Source ERIC.
*/

package org.phoebus.applications.saveandrestore;

import org.phoebus.framework.spi.AppInstance;
import org.phoebus.framework.spi.AppResourceDescriptor;

import java.net.URI;

/**
* Application showing list of save-and-restore nodes matching a particular filter. Its UI is a subset of the
* search and filter view of the {@link SaveAndRestoreApplication} UI.
*/
public class FilterViewApplication implements AppResourceDescriptor {

public static final String NAME = "saveandrestorefilterview";
public static final String DISPLAY_NAME = "SAR Filter";

@Override
public String getDisplayName() {
return DISPLAY_NAME;
}

@Override
public String getName() {
return NAME;
}

@Override
public AppInstance create() {
return create(null);
}

@Override
public AppInstance create(URI uri) {
if(FilterViewInstance.INSTANCE == null){
FilterViewInstance.INSTANCE = new FilterViewInstance(this);
}
else{
FilterViewInstance.INSTANCE.raise();
}

if(uri != null){
FilterViewInstance.INSTANCE.openResource(uri);
}

return FilterViewInstance.INSTANCE;
}

public AppInstance getInstance(){
return FilterViewInstance.INSTANCE;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
/*
* Copyright (C) 2024 European Spallation Source ERIC.
*/

package org.phoebus.applications.saveandrestore;

import javafx.fxml.FXMLLoader;
import org.phoebus.applications.saveandrestore.ui.search.SearchResultTableViewController;
import org.phoebus.framework.nls.NLS;
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.DockPane;

import java.net.URI;
import java.net.URLDecoder;
import java.nio.charset.StandardCharsets;
import java.util.List;
import java.util.ResourceBundle;
import java.util.concurrent.CompletableFuture;
import java.util.logging.Level;
import java.util.logging.Logger;

/**
* Instance maintaining a list of save-and-restore nodes matching a particular filter. Its UI is a subset of the
* search and filter view of the {@link SaveAndRestoreApplication} UI.
*/
public class FilterViewInstance implements AppInstance {

private final AppDescriptor appDescriptor;
public static FilterViewInstance INSTANCE;
private DockItem dockItem;
private final SearchResultTableViewController searchResultTableViewController;

public FilterViewInstance(AppDescriptor appDescriptor) {
this.appDescriptor = appDescriptor;

dockItem = null;

FXMLLoader loader = new FXMLLoader();
try {
ResourceBundle resourceBundle = NLS.getMessages(Messages.class);
loader.setResources(resourceBundle);
loader.setLocation(FilterViewApplication.class.getResource("/org/phoebus/applications/saveandrestore/ui/search/SearchResultTableView.fxml"));
dockItem = new DockItem(this, loader.load());
} catch (Exception e) {
Logger.getLogger(SaveAndRestoreApplication.class.getName()).log(Level.SEVERE, "Failed loading fxml", e);
}

searchResultTableViewController = loader.getController();
dockItem.addCloseCheck(() -> {
INSTANCE = null;
return CompletableFuture.completedFuture(true);
});

DockPane.getActiveDockPane().addTab(dockItem);
}

@Override
public AppDescriptor getAppDescriptor() {
return appDescriptor;
}

public void openResource(URI uri) {
searchResultTableViewController.loadFilter(URLDecoder.decode(uri.getPath().substring(1), StandardCharsets.UTF_8));
}

public void raise() {
dockItem.select();
}

public void secureStoreChanged(List<ScopedAuthenticationToken> validTokens) {
searchResultTableViewController.secureStoreChanged(validTokens);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
import org.csstudio.display.builder.model.Widget;
import org.csstudio.display.builder.model.spi.ActionHandler;
import org.csstudio.display.builder.model.spi.ActionInfo;
import org.phoebus.applications.saveandrestore.SaveAndRestoreApplication;
import org.phoebus.applications.saveandrestore.FilterViewApplication;
import org.phoebus.framework.workbench.ApplicationService;

import java.net.URI;
Expand All @@ -17,7 +17,7 @@

/**
* {@link ActionHandler} implementation for opening a save-and-restore {@link org.phoebus.applications.saveandrestore.model.search.Filter}
* in the save-and-restore app.
* in the {@link FilterViewApplication} app.
*/
public class OpenFilterActionHandler implements ActionHandler {

Expand All @@ -27,9 +27,10 @@ public void handleAction(Widget sourceWidget, ActionInfo actionInfo) {

Platform.runLater(() -> {
try {
ApplicationService.createInstance(SaveAndRestoreApplication.NAME, URI.create("file:/" +
URLEncoder.encode(openFilterAction.getFilterId(), StandardCharsets.UTF_8) + "?app=saveandrestore&action=" + OpenFilterAction.OPEN_SAR_FILTER));
ApplicationService.createInstance(FilterViewApplication.NAME, URI.create("file:/" +
URLEncoder.encode(openFilterAction.getFilterId(), StandardCharsets.UTF_8)));
} catch (Exception e) {

throw new RuntimeException(e);
}
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@

package org.phoebus.applications.saveandrestore.authentication;

import org.phoebus.applications.saveandrestore.FilterViewApplication;
import org.phoebus.applications.saveandrestore.FilterViewInstance;
import org.phoebus.applications.saveandrestore.SaveAndRestoreApplication;
import org.phoebus.applications.saveandrestore.SaveAndRestoreInstance;
import org.phoebus.framework.spi.AppDescriptor;
Expand All @@ -38,15 +40,26 @@ public class SecureStoreChangeHandlerImpl implements SecureStoreChangeHandler {
*/
@Override
public void secureStoreChanged(List<ScopedAuthenticationToken> validTokens) {
AppDescriptor appDescriptor = ApplicationService.findApplication(SaveAndRestoreApplication.NAME);
if (appDescriptor != null && appDescriptor instanceof SaveAndRestoreApplication) {
SaveAndRestoreApplication saveAndRestoreApplication = (SaveAndRestoreApplication) appDescriptor;
AppDescriptor saveAndRestoreAppDescriptor = ApplicationService.findApplication(SaveAndRestoreApplication.NAME);
if (saveAndRestoreAppDescriptor != null && saveAndRestoreAppDescriptor instanceof SaveAndRestoreApplication) {
SaveAndRestoreApplication saveAndRestoreApplication = (SaveAndRestoreApplication) saveAndRestoreAppDescriptor;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The cast can be made type-safe by changing the the if-statement to
if (saveAndRestoreAppDescriptor instanceof SaveAndRestoreApplication saveAndRestoreApplication) { ...

Copy link
Copy Markdown
Collaborator Author

@georgweiss georgweiss Feb 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How is it not type-safe the way it stands?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if (x instance of ABC)
{
   ABC abc = (ABC) x;

is type-safe.
.. except one could have typos and accidentally do this:

if (x instance of ABC)
{
   XYZ abc = (XYZ) x;

In the newfangled syntax, the type is only written once

if (x instance ABC abc)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As a downside, the new syntax is ugly.
While at it, the null-check isn't necessary. if (x instanceof XXX) is false for x=null, so can be shorter and more readable .. which does free up space for the new ugly syntax

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@georgweiss: You are right that casting (in Java) is type-safe. However, a runtime-exception will be thrown if the cast doesn't succeed, and the compiler doesn't check that the runtime exception is caught.

With the pattern matching construct the compiler can give better guarantees: in the body of the then-clause of the if-expression it is known that the cast succeeded and that the variable has the correct type. If the cast didn't succeed for some reason, this can, e.g., be handled in an else-clause (where the variable that was bound in the then-clause isn't bound). This allows the programmer to handle all cases and for the compiler to type-check the program without any risk of ClassCastException from the cast at runtime.

In simple cases such as this, the main advantage is to guard against typos as @kasemir pointed out. In more complicated cases it can help the programmer to know that all cases that can arise have been covered in the code.

SaveAndRestoreInstance saveAndRestoreInstance = (SaveAndRestoreInstance) saveAndRestoreApplication.getInstance();
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This cast can be made type-safe by changing the return type of SaveAndRestoreApplication.getInstance() to SaveAndRestoreInstance instead of AppInstance.

// Save&restore app may not be launched (yet)
if(saveAndRestoreInstance == null){
return;
}
saveAndRestoreInstance.secureStoreChanged(validTokens);
}

AppDescriptor filterViewAppDescriptor = ApplicationService.findApplication(FilterViewApplication.NAME);
if (filterViewAppDescriptor != null && filterViewAppDescriptor instanceof FilterViewApplication) {
FilterViewApplication filterViewApplication = (FilterViewApplication) filterViewAppDescriptor;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As above, this cast can be made type-safe by changing the if-statement to: if (filterViewAppDescriptor instanceof FilterViewApplication filterViewApplication) { ...

FilterViewInstance filterViewInstance = (FilterViewInstance) filterViewApplication.getInstance();
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As above, this can be made type-safe by changing the return type of FilterViewInstance.getInstance() to FilterViewInstance.

// Save&restore filter view app may not be launched (yet)
if(filterViewInstance == null){
return;
}
filterViewInstance.secureStoreChanged(validTokens);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,6 @@
import org.phoebus.applications.saveandrestore.model.Node;
import org.phoebus.applications.saveandrestore.model.NodeType;
import org.phoebus.applications.saveandrestore.model.Tag;
import org.phoebus.applications.saveandrestore.model.TagData;
import org.phoebus.applications.saveandrestore.model.search.Filter;
import org.phoebus.applications.saveandrestore.model.search.SearchQueryUtil;
import org.phoebus.applications.saveandrestore.model.search.SearchQueryUtil.Keys;
Expand Down Expand Up @@ -1097,7 +1096,7 @@ private void removeMovedNodes(TreeItem<Node> parentTreeItem, List<Node> nodes) {
* <li>Launch the search/filter view to show a filter search result.</li>
* </ul>
*
* @param uri An {@link URI} on the form file:/unique-id?action=[open-node|open-filter]&app=saveandrestore, where unique-id is the
* @param uri An {@link URI} on the form file:/unique-id?action=[open_sar_node|open_sar_filter]&app=saveandrestore, where unique-id is the
* unique id of a {@link Node} or the unique id (name) of a {@link Filter}. If action is not sepcified,
* it defaults to open-node.
*/
Expand Down Expand Up @@ -1126,9 +1125,6 @@ public void openResource(URI uri) {
case OpenNodeAction.OPEN_SAR_NODE:
openNode(uri.getPath().substring(1));
break;
case OpenFilterAction.OPEN_SAR_FILTER:
openSearchWindowForFilter(URLDecoder.decode(uri.getPath().substring(1), StandardCharsets.UTF_8));
break;
default:
logger.log(Level.WARNING, "Action '" + action + "' not supported");
}
Expand Down Expand Up @@ -1383,6 +1379,7 @@ public boolean compareSnapshotsPossible() {
!selectedItem.getValue().getUniqueId().equals(configAndSnapshotNode[1].getUniqueId());
}


@Override
public void secureStoreChanged(List<ScopedAuthenticationToken> validTokens) {
super.secureStoreChanged(validTokens);
Expand Down Expand Up @@ -1480,7 +1477,7 @@ public void configureContextMenu(ContextMenuEvent e) {
contextMenu.getItems().addAll(menuItems);

// If logbook has been configured, add the Create Log menu item
if(LogbookPreferences.is_supported){
if (LogbookPreferences.is_supported) {
contextMenu.getItems().add(new SeparatorMenuItem());
SelectionService.getInstance().setSelection(SaveAndRestoreApplication.NAME,
selectedItemsProperty.size() == 1 ? List.of(selectedItemsProperty.get(0)) : Collections.emptyList());
Expand Down Expand Up @@ -1508,36 +1505,4 @@ public void configureContextMenu(ContextMenuEvent e) {
!selectedItemsProperty.get(0).getNodeType().equals(NodeType.COMPOSITE_SNAPSHOT)));
configureTagContextMenu(tagWithComment);
}

/**
* Adds a tag to the {@link Node}s contained in <code>tagData</code>
*
* @param tagData Data object consumed by service
*/
public void addTag(TagData tagData) {
try {
SaveAndRestoreService.getInstance().addTag(tagData);
} catch (Exception e) {
ExceptionDetailsErrorDialog.openError(Messages.errorGeneric,
Messages.errorAddTagFailed,
e);
Logger.getLogger(SaveAndRestoreController.class.getName()).log(Level.SEVERE, "Failed to add tag");
}
}

/**
* Removes a tag from the {@link Node}s contained in <code>tagData</code>
*
* @param tagData Data object consumed by service
*/
public void deleteTag(TagData tagData) {
try {
SaveAndRestoreService.getInstance().deleteTag(tagData);
} catch (Exception e) {
ExceptionDetailsErrorDialog.openError(Messages.errorGeneric,
Messages.errorDeleteTagFailed,
e);
logger.log(Level.SEVERE, "Failed to delete tag");
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,11 @@ public SaveAndRestoreTab() {
setContextMenu(contextMenu);
}

/**
* Called when user has signed in or out.
*
* @param validTokens List of valid {@link ScopedAuthenticationToken}s.
*/
public void secureStoreChanged(List<ScopedAuthenticationToken> validTokens) {
controller.secureStoreChanged(validTokens);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,15 @@
import org.phoebus.applications.saveandrestore.Messages;
import org.phoebus.applications.saveandrestore.model.Node;
import org.phoebus.applications.saveandrestore.model.NodeType;
import org.phoebus.applications.saveandrestore.ui.SaveAndRestoreBaseController;
import org.phoebus.applications.saveandrestore.ui.SaveAndRestoreController;
import org.phoebus.ui.javafx.ImageCache;

import java.util.function.Consumer;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unused import.


public class CompareSnapshotsMenuItem extends SaveAndRestoreMenuItem {

public CompareSnapshotsMenuItem(SaveAndRestoreController saveAndRestoreController, ObservableList<Node> selectedItemsProperty, Runnable onAction) {
public CompareSnapshotsMenuItem(SaveAndRestoreBaseController saveAndRestoreController, ObservableList<Node> selectedItemsProperty, Runnable onAction) {
super(saveAndRestoreController, selectedItemsProperty, onAction);
setText(Messages.contextMenuCompareSnapshots);
setGraphic(ImageCache.getImageView(ImageCache.class, "/icons/save-and-restore/compare.png"));
Expand All @@ -25,6 +26,6 @@ public CompareSnapshotsMenuItem(SaveAndRestoreController saveAndRestoreControlle
public void configure() {
disableProperty().set(selectedItemsProperty.size() != 1 ||
!selectedItemsProperty.get(0).getNodeType().equals(NodeType.SNAPSHOT) ||
!saveAndRestoreController.compareSnapshotsPossible());
!((SaveAndRestoreController)saveAndRestoreController).compareSnapshotsPossible());
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here saveAndRestoreController is cast to SaveAndRestoreController, but the constructor takes as argument a SaveAndRestoreBaseController. Is this correct?

}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,15 @@
import org.phoebus.applications.saveandrestore.Messages;
import org.phoebus.applications.saveandrestore.model.Node;
import org.phoebus.applications.saveandrestore.model.NodeType;
import org.phoebus.applications.saveandrestore.ui.SaveAndRestoreBaseController;
import org.phoebus.applications.saveandrestore.ui.SaveAndRestoreController;
import org.phoebus.ui.javafx.ImageCache;

import java.util.function.Consumer;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unused import.


public class CopyMenuItem extends SaveAndRestoreMenuItem {

public CopyMenuItem(SaveAndRestoreController saveAndRestoreController, ObservableList<Node> selectedItemsProperty, Runnable onAction) {
public CopyMenuItem(SaveAndRestoreBaseController saveAndRestoreController, ObservableList<Node> selectedItemsProperty, Runnable onAction) {
super(saveAndRestoreController, selectedItemsProperty, onAction);
setText(Messages.copy);
setGraphic(ImageCache.getImageView(ImageCache.class, "/icons/copy.png"));
Expand All @@ -25,7 +26,7 @@ public CopyMenuItem(SaveAndRestoreController saveAndRestoreController, Observabl
public void configure() {
disableProperty().set(saveAndRestoreController.getUserIdentity().isNull().get() ||
allFoldersOrRootFolder(selectedItemsProperty) ||
!saveAndRestoreController.mayCopy());
!((SaveAndRestoreController)saveAndRestoreController).mayCopy());
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As above, here saveAndRestoreController is cast to SaveAndRestoreController, but the constructor takes as argument a SaveAndRestoreBaseController. Is this correct?

}

private boolean allFoldersOrRootFolder(ObservableList<Node> selectedItemsProperty) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,15 @@
import javafx.collections.ObservableList;
import org.phoebus.applications.saveandrestore.Messages;
import org.phoebus.applications.saveandrestore.model.Node;
import org.phoebus.applications.saveandrestore.ui.SaveAndRestoreBaseController;
import org.phoebus.applications.saveandrestore.ui.SaveAndRestoreController;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unused import.

import org.phoebus.ui.javafx.ImageCache;

import java.util.function.Consumer;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unused import.


public class CopyUniqueIdToClipboardMenuItem extends SaveAndRestoreMenuItem {

public CopyUniqueIdToClipboardMenuItem(SaveAndRestoreController saveAndRestoreController, ObservableList<Node> selectedItemsProperty, Runnable onAction) {
public CopyUniqueIdToClipboardMenuItem(SaveAndRestoreBaseController saveAndRestoreController, ObservableList<Node> selectedItemsProperty, Runnable onAction) {
super(saveAndRestoreController, selectedItemsProperty, onAction);
setText(Messages.copyUniqueIdToClipboard);
setGraphic(ImageCache.getImageView(ImageCache.class, "/icons/copy.png"));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,15 @@
import org.phoebus.applications.saveandrestore.Messages;
import org.phoebus.applications.saveandrestore.model.Node;
import org.phoebus.applications.saveandrestore.model.NodeType;
import org.phoebus.applications.saveandrestore.ui.SaveAndRestoreBaseController;
import org.phoebus.applications.saveandrestore.ui.SaveAndRestoreController;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unused import.

import org.phoebus.ui.javafx.ImageCache;

import java.util.function.Consumer;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unused import.


public class CreateSnapshotMenuItem extends SaveAndRestoreMenuItem {

public CreateSnapshotMenuItem(SaveAndRestoreController saveAndRestoreController, ObservableList<Node> selectedItemsProperty, Runnable onAction) {
public CreateSnapshotMenuItem(SaveAndRestoreBaseController saveAndRestoreController, ObservableList<Node> selectedItemsProperty, Runnable onAction) {
super(saveAndRestoreController, selectedItemsProperty, onAction);
setText(Messages.contextMenuCreateSnapshot);
setGraphic(ImageCache.getImageView(ImageCache.class, "/icons/save-and-restore/snapshot.png"));
Expand Down
Loading