Skip to content

Commit 3b509bb

Browse files
georgweissgeorgweiss
committed
Bug fixes in save&restore
1 parent d9ca4d3 commit 3b509bb

File tree

7 files changed

+104
-113
lines changed

7 files changed

+104
-113
lines changed

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
import org.phoebus.applications.saveandrestore.ui.SaveAndRestoreService;
3535
import org.phoebus.applications.saveandrestore.ui.configuration.ConfigurationFromSelectionController;
3636
import org.phoebus.core.types.ProcessVariable;
37+
import org.phoebus.framework.nls.NLS;
3738
import org.phoebus.framework.selection.Selection;
3839
import org.phoebus.ui.javafx.ImageCache;
3940
import org.phoebus.ui.spi.ContextMenuEntry;
@@ -42,6 +43,7 @@
4243
import java.time.ZoneId;
4344
import java.time.format.DateTimeFormatter;
4445
import java.util.List;
46+
import java.util.ResourceBundle;
4547
import java.util.logging.Level;
4648
import java.util.logging.Logger;
4749

@@ -91,6 +93,8 @@ public void call(final Selection selection)
9193

9294
try {
9395
FXMLLoader loader = new FXMLLoader();
96+
ResourceBundle resourceBundle = NLS.getMessages(Messages.class);
97+
loader.setResources(resourceBundle);
9498
loader.setLocation(SaveAndRestoreApplication.class.getResource("ui/configuration/ConfigurationFromSelection.fxml"));
9599
Parent root = loader.load();
96100
Stage dialog = new Stage();

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

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -152,15 +152,18 @@ public void updateItem(Node node, boolean empty) {
152152
}
153153
// Use custom layout as this makes it easier to set opacity
154154
HBox hBox = new HBox();
155-
if (!saveAndRestoreController.matchesFilter(node)) {
155+
// saveAndRestoreController is null if configuration management is accessed from context menu
156+
if (saveAndRestoreController != null && !saveAndRestoreController.matchesFilter(node)) {
156157
hBox.setOpacity(0.4);
157158
}
158159
StringBuffer stringBuffer = new StringBuffer();
159160
String comment = node.getDescription();
160161
if (comment != null && !comment.isEmpty()) {
161162
stringBuffer.append(comment).append(System.lineSeparator());
162163
}
163-
stringBuffer.append(TimestampFormats.SECONDS_FORMAT.format(node.getCreated().toInstant())).append(" (").append(node.getUserName()).append(")");
164+
if(node.getCreated() != null){ // Happens if configuration management is accessed from context menu
165+
stringBuffer.append(TimestampFormats.SECONDS_FORMAT.format(node.getCreated().toInstant())).append(" (").append(node.getUserName()).append(")");
166+
}
164167
// Tooltip with at least date and user id is set on all tree items
165168
setTooltip(new Tooltip(stringBuffer.toString()));
166169
switch (node.getNodeType()) {

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -305,7 +305,7 @@ protected TreeItem<Node> call() {
305305
// Check if there is a save tree structure. Also check that the first node id (=tree root)
306306
// has the same unique id as the actual root node retrieved from the remote service. This check
307307
// is needed to handle the case when the client connects to a different save-and-restore service.
308-
if (savedTreeViewStructure != null && savedTreeViewStructure.get(0).equals(rootNode.getUniqueId())) {
308+
if (savedTreeViewStructure != null && !savedTreeViewStructure.isEmpty() && savedTreeViewStructure.get(0).equals(rootNode.getUniqueId())) {
309309
HashMap<String, List<TreeItem<Node>>> childNodesMap = new HashMap<>();
310310
savedTreeViewStructure.forEach(s -> {
311311
List<Node> childNodes = saveAndRestoreService.getChildNodes(Node.builder().uniqueId(s).build());

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

Lines changed: 63 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@
1818
*/
1919
package org.phoebus.applications.saveandrestore.ui.configuration;
2020

21+
import javafx.application.Platform;
22+
import javafx.beans.binding.Bindings;
2123
import javafx.beans.property.SimpleBooleanProperty;
2224
import javafx.beans.property.SimpleIntegerProperty;
2325
import javafx.beans.property.SimpleObjectProperty;
@@ -55,14 +57,12 @@
5557

5658
import java.net.URL;
5759
import java.text.NumberFormat;
58-
import java.time.Instant;
59-
import java.time.ZoneId;
60-
import java.time.format.DateTimeFormatter;
6160
import java.util.ArrayList;
6261
import java.util.List;
6362
import java.util.Map;
6463
import java.util.Optional;
6564
import java.util.ResourceBundle;
65+
import java.util.logging.Level;
6666
import java.util.logging.Logger;
6767

6868
/**
@@ -84,8 +84,6 @@ private static class TableRowEntry {
8484
private ConfigPv pv;
8585
}
8686

87-
private static final DateTimeFormatter configurationTimeFormat = DateTimeFormatter.ofPattern("yyyy-MM-dd HH:mm:ss").withZone(ZoneId.systemDefault());
88-
8987
private final List<String> nodeListInFolder = new ArrayList<>();
9088

9189
@FXML
@@ -95,10 +93,10 @@ private static class TableRowEntry {
9593
private Button browseButton;
9694

9795
@FXML
98-
private TextField configurationName;
96+
private TextField configurationNameField;
9997

10098
@FXML
101-
private TextArea description;
99+
private TextArea descriptionTextArea;
102100

103101
@FXML
104102
private Label numSelectedLabel;
@@ -137,6 +135,14 @@ public void disableConfigurationSelectionInBrowsing() {
137135

138136
private SimpleObjectProperty<Node> createdConfiguration = null;
139137

138+
private final SimpleStringProperty configurationName = new SimpleStringProperty();
139+
140+
private final SimpleStringProperty description = new SimpleStringProperty();
141+
142+
private final SimpleStringProperty locationProperty = new SimpleStringProperty();
143+
144+
private final SimpleBooleanProperty updateOfExistingConfiguration = new SimpleBooleanProperty();
145+
140146

141147
public void setCreatedConfigurationProperty(SimpleObjectProperty<Node> createdConfiguration) {
142148
this.createdConfiguration = createdConfiguration;
@@ -147,43 +153,30 @@ public void initialize(URL location, ResourceBundle resources) {
147153
targetNode.addListener((observableValue, node, newNode) -> {
148154
if (newNode != null) {
149155
try {
156+
nodeListInFolder.clear();
157+
configurationNameField.promptTextProperty().set(null);
150158
if (newNode.getNodeType() == NodeType.CONFIGURATION) {
151-
configurationName.setText(newNode.getName());
152-
description.setText(newNode.getDescription());
153-
154-
configurationName.setEditable(false);
155-
description.setEditable(true);
156-
159+
configurationName.set(newNode.getName());
160+
description.set(newNode.getDescription());
157161
Node parentNode = saveAndRestoreService.getParentNode(newNode.getUniqueId());
158-
locationTextField.setText(DirectoryUtilities.CreateLocationString(parentNode, false));
159-
160-
nodeListInFolder.clear();
162+
locationProperty.set(DirectoryUtilities.CreateLocationString(parentNode, false));
161163
saveAndRestoreService.getChildNodes(parentNode).forEach(item -> nodeListInFolder.add(item.getName()));
162-
163-
saveButton.setDisable(false);
164-
164+
updateOfExistingConfiguration.set(true);
165165
} else {
166-
configurationName.setText("");
167-
description.setText("");
168-
169-
configurationName.setEditable(true);
170-
description.setEditable(true);
171-
172-
locationTextField.setText(DirectoryUtilities.CreateLocationString(newNode, false));
173-
174-
nodeListInFolder.clear();
166+
configurationName.set(null);
167+
description.set(null);
168+
locationProperty.set(DirectoryUtilities.CreateLocationString(newNode, false));
175169
saveAndRestoreService.getChildNodes(newNode).forEach(item -> nodeListInFolder.add(item.getName()));
176-
170+
updateOfExistingConfiguration.set(false);
177171
}
178-
configurationName.getStyleClass().remove("input-error");
179-
configurationName.setTooltip(null);
172+
configurationNameField.getStyleClass().remove("input-error");
180173
} catch (Exception e) {
181-
e.printStackTrace();
174+
LOGGER.log(Level.WARNING, "Failed to handle selection of target node", e);
182175
}
183176
}
184177
});
185178

186-
targetNode.set(saveAndRestoreService.getChildNodes(saveAndRestoreService.getRootNode()).get(0));
179+
locationTextField.textProperty().bind(locationProperty);
187180

188181
browseButton.setOnAction(action -> {
189182
try {
@@ -208,25 +201,22 @@ public void initialize(URL location, ResourceBundle resources) {
208201
targetNode.set(selectedNode);
209202
}
210203
} catch (Exception e) {
211-
e.printStackTrace();
204+
LOGGER.log(Level.WARNING, "Unable to launch UI", e);
212205
}
213206
});
214207

215-
configurationName.setPromptText(configurationTimeFormat.format(Instant.now()));
216-
configurationName.getStylesheets().add(getClass().getResource("/style.css").toExternalForm());
217-
configurationName.textProperty().addListener((observableValue, oldName, newName) -> {
218-
saveButton.setDisable(nodeListInFolder.contains(newName));
219-
220-
if (saveButton.isDisabled()) {
221-
configurationName.getStyleClass().add("input-error");
222-
configurationName.setTooltip(new Tooltip(Messages.toolTipConfigurationExists + (!isDisabledConfigurationSelectionInBrowsing ? System.lineSeparator() + Messages.toolTipConfigurationExistsOption : "")));
208+
configurationNameField.getStylesheets().add(getClass().getResource("/style.css").toExternalForm());
209+
configurationNameField.textProperty().addListener((observableValue, oldName, newName) -> {
210+
if (nodeListInFolder.contains(newName)) {
211+
configurationNameField.getStyleClass().add("input-error");
212+
configurationNameField.setTooltip(new Tooltip(Messages.toolTipConfigurationExists + (!isDisabledConfigurationSelectionInBrowsing ? System.lineSeparator() + Messages.toolTipConfigurationExistsOption : "")));
223213
} else {
224-
configurationName.getStyleClass().remove("input-error");
225-
configurationName.setTooltip(null);
214+
configurationNameField.getStyleClass().remove("input-error");
215+
configurationNameField.setTooltip(null);
226216
}
227217
});
228218

229-
description.setPromptText("Configuration created at " + configurationTimeFormat.format(Instant.now()));
219+
configurationNameField.textProperty().bindBidirectional(configurationName);
230220

231221
selectColumn.setReorderable(false);
232222
selectColumn.setCellFactory(CheckBoxTableCell.forTableColumn(selectColumn));
@@ -275,6 +265,28 @@ public void initialize(URL location, ResourceBundle resources) {
275265
});
276266

277267
numSelected.addListener((observableValue, number, newValue) -> numSelectedLabel.setText(NumberFormat.getIntegerInstance().format(newValue)));
268+
269+
// Cannot save until location, config name and description are set.
270+
saveButton.disableProperty().bind(Bindings.createBooleanBinding(() -> targetNode.get() == null ||
271+
configurationName.get() == null || configurationName.get().isEmpty() ||
272+
description.get() == null || description.get().isEmpty(),
273+
targetNode, configurationName, description));
274+
275+
// Make config name field non-editable if location is not set, or if user is updating existing configuration
276+
configurationNameField.editableProperty().bind(Bindings.createBooleanBinding(() -> targetNode.get() != null
277+
&& updateOfExistingConfiguration.not().get(),
278+
targetNode,
279+
updateOfExistingConfiguration));
280+
281+
282+
// Make description text area non-editable if location is not set, or if user is updating existing configuration
283+
descriptionTextArea.editableProperty().bind(Bindings.createBooleanBinding(() -> targetNode.get() != null
284+
&& updateOfExistingConfiguration.not().get(),
285+
targetNode,
286+
updateOfExistingConfiguration));
287+
descriptionTextArea.textProperty().bindBidirectional(description);
288+
289+
Platform.runLater(() -> browseButton.requestFocus());
278290
}
279291

280292
public void setSelection(List<ProcessVariable> pvList) {
@@ -298,8 +310,10 @@ public void setData(Node targetNode, String name, String description, List<Map<S
298310
this.targetNode.set(targetNode);
299311
}
300312

301-
this.configurationName.setText(name);
302-
this.description.setText(description);
313+
//this.configurationNameField.setText(name);
314+
//this.descriptionTextArea.setText(description);
315+
this.configurationName.set(name);
316+
this.description.set(description);
303317

304318
for (Map<String, String> entry : entries) {
305319
final TableRowEntry rowEntry = new TableRowEntry();
@@ -338,7 +352,7 @@ private void save(ActionEvent ae) {
338352
if (selectedNode.getNodeType() == NodeType.FOLDER) {
339353
Node newConfigurationBuild = Node.builder()
340354
.nodeType(NodeType.CONFIGURATION)
341-
.name(configurationName.getText().trim().isEmpty() ? configurationName.getPromptText() : configurationName.getText().trim())
355+
.name(configurationNameField.getText().trim().isEmpty() ? configurationNameField.getPromptText() : configurationNameField.getText().trim())
342356
.build();
343357

344358
try {
@@ -347,7 +361,7 @@ private void save(ActionEvent ae) {
347361
configurationData.setPvList(pvs);
348362

349363
Configuration configuration = new Configuration();
350-
newConfigurationBuild.setDescription(description.getText().trim().isEmpty() ? description.getPromptText() : description.getText().trim());
364+
newConfigurationBuild.setDescription(descriptionTextArea.getText().trim().isEmpty() ? descriptionTextArea.getPromptText() : descriptionTextArea.getText().trim());
351365
configuration.setConfigurationNode(newConfigurationBuild);
352366
configuration.setConfigurationData(configurationData);
353367

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

Lines changed: 17 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -1,23 +1,23 @@
11
/**
22
* Copyright (C) 2020 Facility for Rare Isotope Beams
3-
*
3+
* <p>
44
* This program is free software: you can redistribute it and/or modify
55
* it under the terms of the GNU General Public License as published by
66
* the Free Software Foundation, either version 3 of the License, or
77
* (at your option) any later version.
8-
*
8+
* <p>
99
* This program is distributed in the hope that it will be useful,
1010
* but WITHOUT ANY WARRANTY; without even the implied warranty of
1111
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
1212
* GNU General Public License for more details.
13-
*
13+
* <p>
1414
* You should have received a copy of the GNU General Public License
1515
* along with this program. If not, see <https://www.gnu.org/licenses/>.
16-
*
16+
* <p>
1717
* Contact Information: Facility for Rare Isotope Beam,
18-
* Michigan State University,
19-
* East Lansing, MI 48824-1321
20-
* http://frib.msu.edu
18+
* Michigan State University,
19+
* East Lansing, MI 48824-1321
20+
* http://frib.msu.edu
2121
*/
2222
package org.phoebus.applications.saveandrestore.ui.configuration;
2323

@@ -33,10 +33,11 @@
3333
import org.phoebus.applications.saveandrestore.Messages;
3434
import org.phoebus.applications.saveandrestore.model.Node;
3535
import org.phoebus.applications.saveandrestore.model.NodeType;
36-
import org.phoebus.applications.saveandrestore.ui.SaveAndRestoreService;
3736
import org.phoebus.applications.saveandrestore.ui.BrowserTreeCell;
37+
import org.phoebus.applications.saveandrestore.ui.SaveAndRestoreService;
3838

3939
import java.net.URL;
40+
import java.util.Comparator;
4041
import java.util.List;
4142
import java.util.Optional;
4243
import java.util.ResourceBundle;
@@ -88,20 +89,9 @@ public void initialize(URL url, ResourceBundle resourceBundle) {
8889
if (selectedTreeItem == null) {
8990
return;
9091
}
91-
9292
Node selectedNode = selectedTreeItem.getValue();
93-
if (selectedNode.getUniqueId().equals(saveAndRestoreService.getRootNode().getUniqueId())
94-
|| selectedNode.getNodeType().equals(NodeType.FOLDER)) {
95-
chooseButton.setDisable(true);
96-
} else {
97-
chooseButton.setDisable(false);
98-
}
99-
100-
if (selectedNode.getNodeType().equals(NodeType.FOLDER)) {
101-
createFolderButton.setDisable(false);
102-
} else {
103-
createFolderButton.setDisable(true);
104-
}
93+
chooseButton.setDisable(selectedNode.getUniqueId().equals(saveAndRestoreService.getRootNode().getUniqueId()));
94+
createFolderButton.setDisable(!selectedNode.getNodeType().equals(NodeType.FOLDER));
10595
});
10696

10797
treeView.getSelectionModel().selectFirst();
@@ -125,37 +115,25 @@ public void initialize(URL url, ResourceBundle resourceBundle) {
125115
private void RecursiveAddNode(TreeItem<Node> parentItem) {
126116
List<Node> childNodes = saveAndRestoreService.getChildNodes(parentItem.getValue());
127117
List<TreeItem<Node>> childItems = childNodes.stream()
128-
.filter(node -> isDisabledConfigurationSelection ? !(node.getNodeType().equals(NodeType.CONFIGURATION) || node.getNodeType().equals(NodeType.SNAPSHOT)) : !node.getNodeType().equals(NodeType.SNAPSHOT) )
118+
.filter(node -> isDisabledConfigurationSelection ? !(node.getNodeType().equals(NodeType.CONFIGURATION) || node.getNodeType().equals(NodeType.SNAPSHOT)) : !node.getNodeType().equals(NodeType.SNAPSHOT))
129119
.map(node -> {
130120
TreeItem<Node> treeItem = createNode(node);
131-
if (node.getNodeType().equals(NodeType.FOLDER)) {
132-
treeItem.getChildren().add(createCreateConfiguration());
133-
}
134121
RecursiveAddNode(treeItem);
135122
return treeItem;
136123
}).collect(Collectors.toList());
137-
parentItem.getChildren().addAll(childItems);
124+
List<TreeItem<Node>> sorted = childItems.stream().sorted(Comparator.comparing(TreeItem::getValue)).collect(Collectors.toList());
125+
parentItem.getChildren().addAll(sorted);
138126
}
139127

140-
private TreeItem<Node> createNode(final Node node){
141-
return new TreeItem<>(node){
128+
private TreeItem<Node> createNode(final Node node) {
129+
return new TreeItem<>(node) {
142130
@Override
143-
public boolean isLeaf(){
131+
public boolean isLeaf() {
144132
return getChildren().isEmpty();
145133
}
146134
};
147135
}
148136

149-
private TreeItem<Node> createCreateConfiguration() {
150-
Node newConfiguration = Node.builder()
151-
.nodeType(NodeType.CONFIGURATION)
152-
.name("Create a new configuration")
153-
.uniqueId("0")
154-
.build();
155-
156-
return createNode(newConfiguration);
157-
}
158-
159137
private void createNewFolder(TreeItem<Node> parentTreeItem) {
160138
List<String> existingFolderNames =
161139
parentTreeItem.getChildren().stream()
@@ -185,7 +163,6 @@ private void createNewFolder(TreeItem<Node> parentTreeItem) {
185163
Node newTreeNode = saveAndRestoreService
186164
.createNode(parentTreeItem.getValue().getUniqueId(), newFolderNode);
187165
TreeItem<Node> treeItem = createNode(newTreeNode);
188-
treeItem.getChildren().add(createCreateConfiguration());
189166
parentTreeItem.getChildren().add(treeItem);
190167
parentTreeItem.setExpanded(true);
191168
} catch (Exception e) {

0 commit comments

Comments
 (0)