Skip to content

Commit 5a03004

Browse files
georgweissgeorgweiss
authored
Merge pull request #2585 from ControlSystemStudio/save-n-restore-fix
Bug fixes in save&restore
2 parents 6f1c7e0 + 2cd0cba commit 5a03004

File tree

8 files changed

+115
-120
lines changed

8 files changed

+115
-120
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/ContextMenuSnapshot.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,7 @@ public ContextMenuSnapshot(SaveAndRestoreController saveAndRestoreController,
8383
//
8484
setOnShowing(event -> {
8585
saveAndRestoreController.configureGoldenItem(tagGoldenMenuItem);
86+
runChecks();
8687
});
8788

8889
getItems().addAll(deleteNodesMenuItem,

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

Lines changed: 11 additions & 8 deletions
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());
@@ -413,14 +413,17 @@ protected void compareSnapshot() {
413413
* @param node The snapshot used in the comparison.
414414
*/
415415
protected void compareSnapshot(Node node) {
416-
try {
417-
SnapshotTab currentTab = (SnapshotTab) tabPane.getSelectionModel().getSelectedItem();
418-
if (currentTab == null) {
419-
return;
416+
Tab tab = tabPane.getSelectionModel().getSelectedItem();
417+
if(tab == null){
418+
return;
419+
}
420+
if (tab instanceof SnapshotTab) {
421+
try {
422+
SnapshotTab currentTab = (SnapshotTab) tab;
423+
currentTab.addSnapshot(node);
424+
} catch (Exception e) {
425+
LOG.log(Level.WARNING, "Failed to compare snapshot", e);
420426
}
421-
currentTab.addSnapshot(node);
422-
} catch (Exception e) {
423-
LOG.log(Level.WARNING, "Failed to compare snapshot", e);
424427
}
425428
}
426429

‎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

0 commit comments

Comments
 (0)