Skip to content

Commit 9e10036

Browse files
georgweissgeorgweiss
authored
Merge pull request #3353 from ControlSystemStudio/CSSTUDIO-3140
Fix update of save&restore snapshot meta-data
2 parents aa5173a + 2970883 commit 9e10036

File tree

3 files changed

+47
-59
lines changed

3 files changed

+47
-59
lines changed

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

Lines changed: 6 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -357,8 +357,6 @@ public Filter fromString(String s) {
357357
pasteMenuItem.setOnAction(ae -> pasteFromClipboard());
358358

359359
contextMenu.getItems().addAll(menuItems);
360-
361-
362360
treeView.setContextMenu(contextMenu);
363361

364362
loadTreeData();
@@ -440,33 +438,19 @@ private String getSavedFilterName() {
440438
/**
441439
* Expands the specified {@link Node}. In order to maintain the list of child {@link Node}s between repeated
442440
* expand/collapse actions, this method will query the service for the current list of child {@link Node}s and
443-
* then update the tree view accordingly, i.e. add {@link Node}s that are not yet present, and remove those that
444-
* have been removed.
441+
* then update the tree view accordingly.
445442
*
446443
* @param targetItem {@link TreeItem<Node>} on which the operation is performed.
447444
*/
448445
protected void expandTreeNode(TreeItem<Node> targetItem) {
449446
List<Node> childNodes = saveAndRestoreService.getChildNodes(targetItem.getValue());
450-
List<String> childNodeIds = childNodes.stream().map(Node::getUniqueId).toList();
451-
List<String> existingNodeIds =
452-
targetItem.getChildren().stream().map(item -> item.getValue().getUniqueId()).toList();
453-
List<TreeItem<Node>> itemsToAdd = new ArrayList<>();
454-
childNodes.forEach(n -> {
455-
if (!existingNodeIds.contains(n.getUniqueId())) {
456-
itemsToAdd.add(createTreeItem(n));
457-
}
458-
});
459-
List<TreeItem<Node>> itemsToRemove = new ArrayList<>();
460-
targetItem.getChildren().forEach(item -> {
461-
if (!childNodeIds.contains(item.getValue().getUniqueId())) {
462-
itemsToRemove.add(item);
463-
}
464-
});
465-
466-
targetItem.getChildren().addAll(itemsToAdd);
467-
targetItem.getChildren().removeAll(itemsToRemove);
447+
List<TreeItem<Node>> list =
448+
childNodes.stream().map(n -> createTreeItem(n)).toList();
449+
targetItem.getChildren().setAll(list);
468450
targetItem.getChildren().sort(treeNodeComparator);
469451
targetItem.setExpanded(true);
452+
treeView.getSelectionModel().clearSelection();
453+
treeView.getSelectionModel().select(null);
470454
}
471455

472456
/**

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

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -191,6 +191,9 @@ public void initialize() {
191191
snapshotDataDirty.set(newValue != null && (snapshotNodeProperty.isNull().get() || snapshotNodeProperty.isNotNull().get() && !newValue.equals(snapshotNodeProperty.get().getDescription())))));
192192

193193
saveSnapshotButton.disableProperty().bind(Bindings.createBooleanBinding(() ->
194+
// TODO: support save (=update) a composite snapshot from the snapshot view. In the meanwhile, disable save button.
195+
snapshotNodeProperty.isNull().get() ||
196+
snapshotNodeProperty.get().getNodeType().equals(NodeType.COMPOSITE_SNAPSHOT) ||
194197
snapshotDataDirty.not().get() ||
195198
snapshotNameProperty.isEmpty().get() ||
196199
snapshotCommentProperty.isEmpty().get() ||
@@ -289,14 +292,7 @@ public void initialize() {
289292

290293
snapshotNodeProperty.addListener((ob, old, node) -> {
291294
if (node != null) {
292-
Platform.runLater(() -> {
293-
snapshotNameProperty.set(node.getName());
294-
snapshotCommentProperty.set(node.getDescription());
295-
createdDateTextProperty.set(node.getCreated() != null ? TimestampFormats.SECONDS_FORMAT.format(node.getCreated().toInstant()) : null);
296-
lastModifiedDateTextProperty.set(node.getLastModified() != null ? TimestampFormats.SECONDS_FORMAT.format(node.getLastModified().toInstant()) : null);
297-
createdByTextProperty.set(node.getUserName());
298-
filterToolbar.disableProperty().set(node.getName() == null);
299-
});
295+
updateUi(node);
300296
}
301297
});
302298

@@ -382,6 +378,18 @@ public SimpleBooleanProperty getSnapshotRestorableProperty() {
382378

383379
public void setSnapshotNode(Node node) {
384380
snapshotNodeProperty.set(node);
381+
updateUi(node);
382+
}
383+
384+
private void updateUi(Node node){
385+
Platform.runLater(() -> {
386+
snapshotNameProperty.set(node.getName());
387+
snapshotCommentProperty.set(node.getDescription());
388+
createdDateTextProperty.set(node.getCreated() != null ? TimestampFormats.SECONDS_FORMAT.format(node.getCreated().toInstant()) : null);
389+
lastModifiedDateTextProperty.set(node.getLastModified() != null ? TimestampFormats.SECONDS_FORMAT.format(node.getLastModified().toInstant()) : null);
390+
createdByTextProperty.set(node.getUserName());
391+
filterToolbar.disableProperty().set(node.getName() == null);
392+
});
385393
}
386394

387395
private void parseAndUpdateThreshold(String value) {

‎services/save-and-restore/src/main/java/org/phoebus/service/saveandrestore/persistence/dao/impl/elasticsearch/ElasticsearchDAO.java‎

Lines changed: 25 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,6 @@
4242
import org.springframework.util.LinkedMultiValueMap;
4343
import org.springframework.util.MultiValueMap;
4444

45-
import java.lang.annotation.Inherited;
4645
import java.util.ArrayList;
4746
import java.util.Collections;
4847
import java.util.Comparator;
@@ -129,7 +128,7 @@ public List<Node> getNodes(List<String> uniqueNodeIds) {
129128
}
130129

131130
/**
132-
* {@inheritDoc}
131+
* {@inheritDoc}
133132
*/
134133
@Override
135134
@Deprecated
@@ -138,12 +137,12 @@ public void deleteNode(String nodeId) {
138137
}
139138

140139
/**
141-
* {@inheritDoc}
140+
* {@inheritDoc}
142141
*/
143142
@Override
144-
public void deleteNodes(List<String> nodeIds){
143+
public void deleteNodes(List<String> nodeIds) {
145144
List<Node> nodes = new ArrayList<>();
146-
for(String nodeId : nodeIds){
145+
for (String nodeId : nodeIds) {
147146
Node nodeToDelete = getNode(nodeId);
148147
if (nodeToDelete == null) {
149148
throw new NodeNotFoundException("Cannot delete non-existing node");
@@ -318,17 +317,17 @@ public Node moveNodes(List<String> nodeIds, String targetId, String userName) {
318317
throw new RuntimeException("Parent node of source node " + sourceNodes.get(0).getUniqueId() + " not found. Should not happen.");
319318
}
320319

321-
if (targetNode.getChildNodes() != null){
320+
if (targetNode.getChildNodes() != null) {
322321
List<Node> targetsChildNodes = new ArrayList<>();
323-
for(String parentChildNode : targetNode.getChildNodes()){
322+
for (String parentChildNode : targetNode.getChildNodes()) {
324323
Optional<ESTreeNode> targetChildNodeOptional = elasticsearchTreeRepository.findById(parentChildNode);
325-
if(targetChildNodeOptional.isEmpty()){ // Should not happen, but ignore if it does.
324+
if (targetChildNodeOptional.isEmpty()) { // Should not happen, but ignore if it does.
326325
continue;
327326
}
328327
targetsChildNodes.add(targetChildNodeOptional.get().getNode());
329328
}
330-
for(Node sourceNode : sourceNodes){
331-
for(Node targetChildNode : targetsChildNodes) {
329+
for (Node sourceNode : sourceNodes) {
330+
for (Node targetChildNode : targetsChildNodes) {
332331
if (targetChildNode.getName().equals(sourceNode.getName()) && targetChildNode.getNodeType().equals(sourceNode.getNodeType())) {
333332
throw new IllegalArgumentException("Cannot move, at least one source node has same name and type as a target child node");
334333
}
@@ -758,15 +757,16 @@ public Snapshot updateSnapshot(Snapshot snapshot) {
758757

759758
snapshot.getSnapshotNode().setNodeType(NodeType.SNAPSHOT); // Force node type
760759
SnapshotData newSnapshotData;
760+
Snapshot newSnapshot = new Snapshot();
761761
try {
762762
newSnapshotData = snapshotDataRepository.save(snapshot.getSnapshotData());
763+
Node updatedNode = updateNode(snapshot.getSnapshotNode(), false);
764+
newSnapshot.setSnapshotData(newSnapshotData);
765+
newSnapshot.setSnapshotNode(updatedNode);
763766
} catch (Exception e) {
764767
throw new RuntimeException(e);
765768
}
766769

767-
Snapshot newSnapshot = new Snapshot();
768-
newSnapshot.setSnapshotData(newSnapshotData);
769-
newSnapshot.setSnapshotNode(snapshot.getSnapshotNode());
770770

771771
return newSnapshot;
772772
}
@@ -810,7 +810,8 @@ public Node findParentFromPathElements(Node parentNode, String[] splitPath, int
810810

811811
/**
812812
* Checks if a {@link Node} is present in a subtree. This is called recursively.
813-
* @param startNode {@link Node} id from which the search will start.
813+
*
814+
* @param startNode {@link Node} id from which the search will start.
814815
* @param nodeToLookFor Self-explanatory.
815816
* @return <code>true</code> if the #nodeToLookFor is found in the subtree, otherwise <code>false</code>.
816817
*/
@@ -1079,23 +1080,19 @@ private boolean checkCompositeSnapshotReferencedNodeType(String nodeId) {
10791080
}
10801081
}
10811082
return true;
1082-
} else if (node.getNodeType().equals(NodeType.SNAPSHOT)) {
1083-
return true;
1084-
} else {
1085-
return false;
1086-
}
1083+
} else return node.getNodeType().equals(NodeType.SNAPSHOT);
10871084
}
10881085

10891086
@Override
10901087
public SearchResult search(MultiValueMap<String, String> searchParameters) {
10911088
return searchInternal(searchParameters);
10921089
}
10931090

1094-
private SearchResult searchInternal(MultiValueMap<String, String> searchParameters){
1091+
private SearchResult searchInternal(MultiValueMap<String, String> searchParameters) {
10951092
// Did client specify search on pv name(s)?
1096-
if(searchParameters.keySet().stream().anyMatch(k -> k.strip().toLowerCase().contains("pvs"))){
1093+
if (searchParameters.keySet().stream().anyMatch(k -> k.strip().toLowerCase().contains("pvs"))) {
10971094
List<ConfigurationData> configurationDataList = configurationDataRepository.searchOnPvName(searchParameters);
1098-
if(configurationDataList.isEmpty()){
1095+
if (configurationDataList.isEmpty()) {
10991096
// No matching configurations found, return empty SearchResult
11001097
return new SearchResult(0, Collections.emptyList());
11011098
}
@@ -1104,8 +1101,7 @@ private SearchResult searchInternal(MultiValueMap<String, String> searchParamete
11041101
augmented.putAll(searchParameters);
11051102
augmented.put("uniqueid", uniqueIds);
11061103
return elasticsearchTreeRepository.search(augmented);
1107-
}
1108-
else{
1104+
} else {
11091105
return elasticsearchTreeRepository.search(searchParameters);
11101106
}
11111107
}
@@ -1228,7 +1224,7 @@ protected String determineNewNodeName(Node sourceNode, List<Node> targetParentCh
12281224
// Filter to make sure only nodes of same type are considered.
12291225
targetParentChildNodes = targetParentChildNodes.stream().filter(n -> n.getNodeType().equals(sourceNode.getNodeType())).collect(Collectors.toList());
12301226
List<String> targetParentChildNodeNames = targetParentChildNodes.stream().map(Node::getName).collect(Collectors.toList());
1231-
if(!targetParentChildNodeNames.contains(sourceNode.getName())){
1227+
if (!targetParentChildNodeNames.contains(sourceNode.getName())) {
12321228
return sourceNode.getName();
12331229
}
12341230
String newNodeBaseName = sourceNode.getName();
@@ -1240,7 +1236,7 @@ protected String determineNewNodeName(Node sourceNode, List<Node> targetParentCh
12401236
Pattern pattern = Pattern.compile(newNodeBaseName + "(\\scopy(\\s\\d*)?$)");
12411237
for (Node targetChildNode : targetParentChildNodes) {
12421238
String targetChildNodeName = targetChildNode.getName();
1243-
if(pattern.matcher(targetChildNodeName).matches()){
1239+
if (pattern.matcher(targetChildNodeName).matches()) {
12441240
nodeNameCopies.add(targetChildNodeName);
12451241
}
12461242
}
@@ -1267,11 +1263,11 @@ protected String determineNewNodeName(Node sourceNode, List<Node> targetParentCh
12671263
/**
12681264
* Compares {@link Node} names for the purpose of ordering.
12691265
*/
1270-
public static class NodeNameComparator implements Comparator<String>{
1266+
public static class NodeNameComparator implements Comparator<String> {
12711267

12721268
@Override
1273-
public int compare(String s1, String s2){
1274-
if(s1.endsWith("copy") || s2.endsWith("copy")){
1269+
public int compare(String s1, String s2) {
1270+
if (s1.endsWith("copy") || s2.endsWith("copy")) {
12751271
return s1.compareTo(s2);
12761272
}
12771273
int copyIndex1 = s1.indexOf("copy");

0 commit comments

Comments
 (0)