From fb298de68acb490543f6437093857c3d15662fde Mon Sep 17 00:00:00 2001 From: georgweiss Date: Fri, 4 Apr 2025 10:08:19 +0200 Subject: [PATCH 1/2] Fix update of save&restore snapshot meta-data --- .../ui/SaveAndRestoreController.java | 32 ++++------- .../SnapshotControlsViewController.java | 24 ++++++--- .../impl/elasticsearch/ElasticsearchDAO.java | 54 +++++++++---------- 3 files changed, 51 insertions(+), 59 deletions(-) diff --git a/app/save-and-restore/app/src/main/java/org/phoebus/applications/saveandrestore/ui/SaveAndRestoreController.java b/app/save-and-restore/app/src/main/java/org/phoebus/applications/saveandrestore/ui/SaveAndRestoreController.java index ffc9f4444d..ef238e8b5a 100644 --- a/app/save-and-restore/app/src/main/java/org/phoebus/applications/saveandrestore/ui/SaveAndRestoreController.java +++ b/app/save-and-restore/app/src/main/java/org/phoebus/applications/saveandrestore/ui/SaveAndRestoreController.java @@ -357,10 +357,12 @@ public Filter fromString(String s) { pasteMenuItem.setOnAction(ae -> pasteFromClipboard()); contextMenu.getItems().addAll(menuItems); - - treeView.setContextMenu(contextMenu); + treeView.getSelectionModel().selectedItemProperty().addListener((a,b,c) -> { + System.out.println(); + }); + loadTreeData(); } @@ -440,33 +442,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} on which the operation is performed. */ protected void expandTreeNode(TreeItem targetItem) { List childNodes = saveAndRestoreService.getChildNodes(targetItem.getValue()); - List childNodeIds = childNodes.stream().map(Node::getUniqueId).toList(); - List existingNodeIds = - targetItem.getChildren().stream().map(item -> item.getValue().getUniqueId()).toList(); - List> itemsToAdd = new ArrayList<>(); - childNodes.forEach(n -> { - if (!existingNodeIds.contains(n.getUniqueId())) { - itemsToAdd.add(createTreeItem(n)); - } - }); - List> 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> 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); } /** diff --git a/app/save-and-restore/app/src/main/java/org/phoebus/applications/saveandrestore/ui/snapshot/SnapshotControlsViewController.java b/app/save-and-restore/app/src/main/java/org/phoebus/applications/saveandrestore/ui/snapshot/SnapshotControlsViewController.java index 6e75603298..0c9b1d2776 100644 --- a/app/save-and-restore/app/src/main/java/org/phoebus/applications/saveandrestore/ui/snapshot/SnapshotControlsViewController.java +++ b/app/save-and-restore/app/src/main/java/org/phoebus/applications/saveandrestore/ui/snapshot/SnapshotControlsViewController.java @@ -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. + snapshotNodeProperty.isNull().get() || + snapshotNodeProperty.get().getNodeType().equals(NodeType.COMPOSITE_SNAPSHOT) || snapshotDataDirty.not().get() || snapshotNameProperty.isEmpty().get() || snapshotCommentProperty.isEmpty().get() || @@ -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); } }); @@ -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) { diff --git a/services/save-and-restore/src/main/java/org/phoebus/service/saveandrestore/persistence/dao/impl/elasticsearch/ElasticsearchDAO.java b/services/save-and-restore/src/main/java/org/phoebus/service/saveandrestore/persistence/dao/impl/elasticsearch/ElasticsearchDAO.java index 01d4527614..df239e71cc 100644 --- a/services/save-and-restore/src/main/java/org/phoebus/service/saveandrestore/persistence/dao/impl/elasticsearch/ElasticsearchDAO.java +++ b/services/save-and-restore/src/main/java/org/phoebus/service/saveandrestore/persistence/dao/impl/elasticsearch/ElasticsearchDAO.java @@ -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; @@ -129,7 +128,7 @@ public List getNodes(List uniqueNodeIds) { } /** - * {@inheritDoc} + * {@inheritDoc} */ @Override @Deprecated @@ -138,12 +137,12 @@ public void deleteNode(String nodeId) { } /** - * {@inheritDoc} + * {@inheritDoc} */ @Override - public void deleteNodes(List nodeIds){ + public void deleteNodes(List nodeIds) { List 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"); @@ -318,17 +317,17 @@ public Node moveNodes(List 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 targetsChildNodes = new ArrayList<>(); - for(String parentChildNode : targetNode.getChildNodes()){ + for (String parentChildNode : targetNode.getChildNodes()) { Optional 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"); } @@ -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; } @@ -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 true if the #nodeToLookFor is found in the subtree, otherwise false. */ @@ -1079,11 +1080,7 @@ 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 @@ -1091,11 +1088,11 @@ public SearchResult search(MultiValueMap searchParameters) { return searchInternal(searchParameters); } - private SearchResult searchInternal(MultiValueMap searchParameters){ + private SearchResult searchInternal(MultiValueMap 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 configurationDataList = configurationDataRepository.searchOnPvName(searchParameters); - if(configurationDataList.isEmpty()){ + if (configurationDataList.isEmpty()) { // No matching configurations found, return empty SearchResult return new SearchResult(0, Collections.emptyList()); } @@ -1104,8 +1101,7 @@ private SearchResult searchInternal(MultiValueMap searchParamete augmented.putAll(searchParameters); augmented.put("uniqueid", uniqueIds); return elasticsearchTreeRepository.search(augmented); - } - else{ + } else { return elasticsearchTreeRepository.search(searchParameters); } } @@ -1228,7 +1224,7 @@ protected String determineNewNodeName(Node sourceNode, List 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 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(); @@ -1240,7 +1236,7 @@ protected String determineNewNodeName(Node sourceNode, List 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); } } @@ -1267,11 +1263,11 @@ protected String determineNewNodeName(Node sourceNode, List targetParentCh /** * Compares {@link Node} names for the purpose of ordering. */ - public static class NodeNameComparator implements Comparator{ + public static class NodeNameComparator implements Comparator { @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"); From 2970883dc4af1aef2012255ce9a005ef6585d602 Mon Sep 17 00:00:00 2001 From: georgweiss Date: Fri, 4 Apr 2025 13:21:40 +0200 Subject: [PATCH 2/2] Removed debug print --- .../saveandrestore/ui/SaveAndRestoreController.java | 4 ---- 1 file changed, 4 deletions(-) diff --git a/app/save-and-restore/app/src/main/java/org/phoebus/applications/saveandrestore/ui/SaveAndRestoreController.java b/app/save-and-restore/app/src/main/java/org/phoebus/applications/saveandrestore/ui/SaveAndRestoreController.java index ef238e8b5a..952fe132e5 100644 --- a/app/save-and-restore/app/src/main/java/org/phoebus/applications/saveandrestore/ui/SaveAndRestoreController.java +++ b/app/save-and-restore/app/src/main/java/org/phoebus/applications/saveandrestore/ui/SaveAndRestoreController.java @@ -359,10 +359,6 @@ public Filter fromString(String s) { contextMenu.getItems().addAll(menuItems); treeView.setContextMenu(contextMenu); - treeView.getSelectionModel().selectedItemProperty().addListener((a,b,c) -> { - System.out.println(); - }); - loadTreeData(); }