Skip to content

Fix update of save&restore snapshot meta-data #3353

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
georgweiss merged 2 commits into from
Apr 5, 2025
Merged

Fix update of save&restore snapshot meta-data #3353

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
fb298de
Fix update of save&restore snapshot meta-data
georgweiss Apr 4, 2025
2970883
Removed debug print
georgweiss Apr 4, 2025
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
28 changes: 6 additions & 22 deletions ...pp/src/main/java/org/phoebus/applications/saveandrestore/ui/SaveAndRestoreController.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -357,8 +357,6 @@ public Filter fromString(String s) {
pasteMenuItem.setOnAction(ae -> pasteFromClipboard());

contextMenu.getItems().addAll(menuItems);


treeView.setContextMenu(contextMenu);

loadTreeData();
Expand Down Expand Up @@ -440,33 +438,19 @@ private String getSavedFilterName() {
/**
* Expands the specified {@link Node}. In order to maintain the list of child {@link Node}s between repeated
* expand/collapse actions, this method will query the service for the current list of child {@link Node}s and
* then update the tree view accordingly, i.e. add {@link Node}s that are not yet present, and remove those that
* have been removed.
* then update the tree view accordingly.
*
* @param targetItem {@link TreeItem<Node>} on which the operation is performed.
*/
protected void expandTreeNode(TreeItem<Node> targetItem) {
List<Node> childNodes = saveAndRestoreService.getChildNodes(targetItem.getValue());
List<String> childNodeIds = childNodes.stream().map(Node::getUniqueId).toList();
List<String> existingNodeIds =
targetItem.getChildren().stream().map(item -> item.getValue().getUniqueId()).toList();
List<TreeItem<Node>> itemsToAdd = new ArrayList<>();
childNodes.forEach(n -> {
if (!existingNodeIds.contains(n.getUniqueId())) {
itemsToAdd.add(createTreeItem(n));
}
});
List<TreeItem<Node>> itemsToRemove = new ArrayList<>();
targetItem.getChildren().forEach(item -> {
if (!childNodeIds.contains(item.getValue().getUniqueId())) {
itemsToRemove.add(item);
}
});

targetItem.getChildren().addAll(itemsToAdd);
targetItem.getChildren().removeAll(itemsToRemove);
List<TreeItem<Node>> list =
childNodes.stream().map(n -> createTreeItem(n)).toList();
targetItem.getChildren().setAll(list);
targetItem.getChildren().sort(treeNodeComparator);
targetItem.setExpanded(true);
treeView.getSelectionModel().clearSelection();
treeView.getSelectionModel().select(null);
}

/**
Expand Down
24 changes: 16 additions & 8 deletions ...a/org/phoebus/applications/saveandrestore/ui/snapshot/SnapshotControlsViewController.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,9 @@ public void initialize() {
snapshotDataDirty.set(newValue != null && (snapshotNodeProperty.isNull().get() || snapshotNodeProperty.isNotNull().get() && !newValue.equals(snapshotNodeProperty.get().getDescription())))));

saveSnapshotButton.disableProperty().bind(Bindings.createBooleanBinding(() ->
// TODO: support save (=update) a composite snapshot from the snapshot view. In the meanwhile, disable save button.
Comment thread
abrahamwolk marked this conversation as resolved.
snapshotNodeProperty.isNull().get() ||
snapshotNodeProperty.get().getNodeType().equals(NodeType.COMPOSITE_SNAPSHOT) ||
snapshotDataDirty.not().get() ||
snapshotNameProperty.isEmpty().get() ||
snapshotCommentProperty.isEmpty().get() ||
Expand Down Expand Up @@ -289,14 +292,7 @@ public void initialize() {

snapshotNodeProperty.addListener((ob, old, node) -> {
if (node != null) {
Platform.runLater(() -> {
snapshotNameProperty.set(node.getName());
snapshotCommentProperty.set(node.getDescription());
createdDateTextProperty.set(node.getCreated() != null ? TimestampFormats.SECONDS_FORMAT.format(node.getCreated().toInstant()) : null);
lastModifiedDateTextProperty.set(node.getLastModified() != null ? TimestampFormats.SECONDS_FORMAT.format(node.getLastModified().toInstant()) : null);
createdByTextProperty.set(node.getUserName());
filterToolbar.disableProperty().set(node.getName() == null);
});
updateUi(node);
}
});

Expand Down Expand Up @@ -382,6 +378,18 @@ public SimpleBooleanProperty getSnapshotRestorableProperty() {

public void setSnapshotNode(Node node) {
snapshotNodeProperty.set(node);
updateUi(node);
}

private void updateUi(Node node){
Platform.runLater(() -> {
snapshotNameProperty.set(node.getName());
snapshotCommentProperty.set(node.getDescription());
createdDateTextProperty.set(node.getCreated() != null ? TimestampFormats.SECONDS_FORMAT.format(node.getCreated().toInstant()) : null);
lastModifiedDateTextProperty.set(node.getLastModified() != null ? TimestampFormats.SECONDS_FORMAT.format(node.getLastModified().toInstant()) : null);
createdByTextProperty.set(node.getUserName());
filterToolbar.disableProperty().set(node.getName() == null);
});
}

private void parseAndUpdateThreshold(String value) {
Expand Down
54 changes: 25 additions & 29 deletions ...g/phoebus/service/saveandrestore/persistence/dao/impl/elasticsearch/ElasticsearchDAO.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@
import org.springframework.util.LinkedMultiValueMap;
import org.springframework.util.MultiValueMap;

import java.lang.annotation.Inherited;
import java.util.ArrayList;
import java.util.Collections;
import java.util.Comparator;
Expand Down Expand Up @@ -129,7 +128,7 @@ public List<Node> getNodes(List<String> uniqueNodeIds) {
}

/**
* {@inheritDoc}
* {@inheritDoc}
*/
@Override
@Deprecated
Expand All @@ -138,12 +137,12 @@ public void deleteNode(String nodeId) {
}

/**
* {@inheritDoc}
* {@inheritDoc}
*/
@Override
public void deleteNodes(List<String> nodeIds){
public void deleteNodes(List<String> nodeIds) {
List<Node> nodes = new ArrayList<>();
for(String nodeId : nodeIds){
for (String nodeId : nodeIds) {
Node nodeToDelete = getNode(nodeId);
if (nodeToDelete == null) {
throw new NodeNotFoundException("Cannot delete non-existing node");
Expand Down Expand Up @@ -318,17 +317,17 @@ public Node moveNodes(List<String> nodeIds, String targetId, String userName) {
throw new RuntimeException("Parent node of source node " + sourceNodes.get(0).getUniqueId() + " not found. Should not happen.");
}

if (targetNode.getChildNodes() != null){
if (targetNode.getChildNodes() != null) {
List<Node> targetsChildNodes = new ArrayList<>();
for(String parentChildNode : targetNode.getChildNodes()){
for (String parentChildNode : targetNode.getChildNodes()) {
Optional<ESTreeNode> targetChildNodeOptional = elasticsearchTreeRepository.findById(parentChildNode);
if(targetChildNodeOptional.isEmpty()){ // Should not happen, but ignore if it does.
if (targetChildNodeOptional.isEmpty()) { // Should not happen, but ignore if it does.
continue;
}
targetsChildNodes.add(targetChildNodeOptional.get().getNode());
}
for(Node sourceNode : sourceNodes){
for(Node targetChildNode : targetsChildNodes) {
for (Node sourceNode : sourceNodes) {
for (Node targetChildNode : targetsChildNodes) {
if (targetChildNode.getName().equals(sourceNode.getName()) && targetChildNode.getNodeType().equals(sourceNode.getNodeType())) {
throw new IllegalArgumentException("Cannot move, at least one source node has same name and type as a target child node");
}
Expand Down Expand Up @@ -758,15 +757,16 @@ public Snapshot updateSnapshot(Snapshot snapshot) {

snapshot.getSnapshotNode().setNodeType(NodeType.SNAPSHOT); // Force node type
SnapshotData newSnapshotData;
Snapshot newSnapshot = new Snapshot();
try {
newSnapshotData = snapshotDataRepository.save(snapshot.getSnapshotData());
Node updatedNode = updateNode(snapshot.getSnapshotNode(), false);
newSnapshot.setSnapshotData(newSnapshotData);
newSnapshot.setSnapshotNode(updatedNode);
} catch (Exception e) {
throw new RuntimeException(e);
}

Snapshot newSnapshot = new Snapshot();
newSnapshot.setSnapshotData(newSnapshotData);
newSnapshot.setSnapshotNode(snapshot.getSnapshotNode());

return newSnapshot;
}
Expand Down Expand Up @@ -810,7 +810,8 @@ public Node findParentFromPathElements(Node parentNode, String[] splitPath, int

/**
* Checks if a {@link Node} is present in a subtree. This is called recursively.
* @param startNode {@link Node} id from which the search will start.
*
* @param startNode {@link Node} id from which the search will start.
* @param nodeToLookFor Self-explanatory.
* @return <code>true</code> if the #nodeToLookFor is found in the subtree, otherwise <code>false</code>.
*/
Expand Down Expand Up @@ -1079,23 +1080,19 @@ private boolean checkCompositeSnapshotReferencedNodeType(String nodeId) {
}
}
return true;
} else if (node.getNodeType().equals(NodeType.SNAPSHOT)) {
return true;
} else {
return false;
}
} else return node.getNodeType().equals(NodeType.SNAPSHOT);
}

@Override
public SearchResult search(MultiValueMap<String, String> searchParameters) {
return searchInternal(searchParameters);
}

private SearchResult searchInternal(MultiValueMap<String, String> searchParameters){
private SearchResult searchInternal(MultiValueMap<String, String> searchParameters) {
// Did client specify search on pv name(s)?
if(searchParameters.keySet().stream().anyMatch(k -> k.strip().toLowerCase().contains("pvs"))){
if (searchParameters.keySet().stream().anyMatch(k -> k.strip().toLowerCase().contains("pvs"))) {
List<ConfigurationData> configurationDataList = configurationDataRepository.searchOnPvName(searchParameters);
if(configurationDataList.isEmpty()){
if (configurationDataList.isEmpty()) {
// No matching configurations found, return empty SearchResult
return new SearchResult(0, Collections.emptyList());
}
Expand All @@ -1104,8 +1101,7 @@ private SearchResult searchInternal(MultiValueMap<String, String> searchParamete
augmented.putAll(searchParameters);
augmented.put("uniqueid", uniqueIds);
return elasticsearchTreeRepository.search(augmented);
}
else{
} else {
return elasticsearchTreeRepository.search(searchParameters);
}
}
Expand Down Expand Up @@ -1228,7 +1224,7 @@ protected String determineNewNodeName(Node sourceNode, List<Node> targetParentCh
// Filter to make sure only nodes of same type are considered.
targetParentChildNodes = targetParentChildNodes.stream().filter(n -> n.getNodeType().equals(sourceNode.getNodeType())).collect(Collectors.toList());
List<String> targetParentChildNodeNames = targetParentChildNodes.stream().map(Node::getName).collect(Collectors.toList());
if(!targetParentChildNodeNames.contains(sourceNode.getName())){
if (!targetParentChildNodeNames.contains(sourceNode.getName())) {
return sourceNode.getName();
}
String newNodeBaseName = sourceNode.getName();
Expand All @@ -1240,7 +1236,7 @@ protected String determineNewNodeName(Node sourceNode, List<Node> targetParentCh
Pattern pattern = Pattern.compile(newNodeBaseName + "(\\scopy(\\s\\d*)?$)");
for (Node targetChildNode : targetParentChildNodes) {
String targetChildNodeName = targetChildNode.getName();
if(pattern.matcher(targetChildNodeName).matches()){
if (pattern.matcher(targetChildNodeName).matches()) {
nodeNameCopies.add(targetChildNodeName);
}
}
Expand All @@ -1267,11 +1263,11 @@ protected String determineNewNodeName(Node sourceNode, List<Node> targetParentCh
/**
* Compares {@link Node} names for the purpose of ordering.
*/
public static class NodeNameComparator implements Comparator<String>{
public static class NodeNameComparator implements Comparator<String> {

@Override
public int compare(String s1, String s2){
if(s1.endsWith("copy") || s2.endsWith("copy")){
public int compare(String s1, String s2) {
if (s1.endsWith("copy") || s2.endsWith("copy")) {
return s1.compareTo(s2);
}
int copyIndex1 = s1.indexOf("copy");
Expand Down