From b8c4a037f4ec152865e18e638d28b0a47c7eeaa4 Mon Sep 17 00:00:00 2001 From: Niklas Rentz Date: Fri, 1 Sep 2023 16:43:17 +0200 Subject: [PATCH] incremental: fixed wrong handling of new/old nodes in edges. Added a more complex test for the incremental update with multiple new elements. Something is off with the ID generation, possibly causing further issues (see the test and the FIXME in the KGraphMerger), but solution still seems to work for the previous issue for now. --- .../incremental/merge/KGraphMerger.java | 5 +- .../klighd/test/IncrementalUpdateTest.java | 113 ++++++++++++++++++ 2 files changed, 117 insertions(+), 1 deletion(-) diff --git a/plugins/de.cau.cs.kieler.klighd.incremental/src/de/cau/cs/kieler/klighd/incremental/merge/KGraphMerger.java b/plugins/de.cau.cs.kieler.klighd.incremental/src/de/cau/cs/kieler/klighd/incremental/merge/KGraphMerger.java index 610ccadd6..725e6fdd8 100644 --- a/plugins/de.cau.cs.kieler.klighd.incremental/src/de/cau/cs/kieler/klighd/incremental/merge/KGraphMerger.java +++ b/plugins/de.cau.cs.kieler.klighd.incremental/src/de/cau/cs/kieler/klighd/incremental/merge/KGraphMerger.java @@ -137,7 +137,6 @@ private void addEdges(KNode newEdgeContainer) { private void handleMatchedEdges() { Set nodesWithMatchedEdges = new HashSet<>(); for (ValueDifference diff : comparison.getMatchedEdges()) { - nodesWithMatchedEdges.add(diff.leftValue().getSource()); nodesWithMatchedEdges.add(diff.rightValue().getSource()); } nodesWithMatchedEdges.forEach( @@ -232,6 +231,10 @@ private void addNode(final KNode node) { int oldPosition = node.getParent().getChildren().indexOf(node); KNode copiedNode = EcoreUtil.copy(node); baseParent.getChildren().add(oldPosition, copiedNode); + // FIXME: if the added node also has a new edge, the target of that will not be set up yet and cause a + // dangling reference here, possibly breaking further equality checks depending on correct IDs. + // One occurring issue: target port of new edge will have a wrong ID with the dangle string and can + // therefore not be found in the base model, causing the port to not be connected. comparison.getBaseAdapter().generateIDs(copiedNode); } } diff --git a/test/de.cau.cs.kieler.klighd.test/src/de/cau/cs/kieler/klighd/test/IncrementalUpdateTest.java b/test/de.cau.cs.kieler.klighd.test/src/de/cau/cs/kieler/klighd/test/IncrementalUpdateTest.java index 7e8d0e492..e0e5bcbfa 100644 --- a/test/de.cau.cs.kieler.klighd.test/src/de/cau/cs/kieler/klighd/test/IncrementalUpdateTest.java +++ b/test/de.cau.cs.kieler.klighd.test/src/de/cau/cs/kieler/klighd/test/IncrementalUpdateTest.java @@ -51,6 +51,34 @@ private ViewContext createViewContext() { return new ViewContext((IDiagramWorkbenchPart) null, null); } + /* + * method for programatically creating the following KGraph in kgt notion: + * + * kgraph nodeRoot + * + * knode nodeA { + * kport portA1 + * kport portA2 + * + * klabel labelA1 "labelA1" + * klabel labelA2 "labelA2" + * + * kedge (->nodeB) // edgeAB + * kedge (->nodeC) // edgeAC + * + * kedge (:portA1->nodeB:portB1) // edgeA1B1 + * kedge (:portA1->nodeB:portB1) // edgeA1B1_2 + * } + * knode nodeB { + * kport portB1 + * + * kedge (->nodeC) // edgeBC + * } + * knode nodeC { + * kedge (->nodeB) // edgeCB + * } + * + */ private KNode createTestGraph() { final KNode nodeRoot = KGraphUtil.createInitializedNode(); addIdentifier(nodeRoot, "nodeRoot"); @@ -558,6 +586,91 @@ public void testAddEdgeOnPort() { Assert.assertSame(newEdgeTargetPosition, baseEdgeTargetPosition); } + /** + * Tests adding an edge to an added port of an added node. Checks if the elements are added and if the edge is + * connected to the new port at the correct positions in various reference lists. + */ + @Test + public void testAddEdgeNewNodeToNewTargetPort() { + final KNode baseGraph = createTestGraph(); + final KNode newGraph = createTestGraph(); + + // Create a new source node on index 0. + final KNode newSourceNode = KGraphUtil.createInitializedNode(); + final EObject newSourceNodeSource = new EObjectImpl() { }; + newSourceNode.setProperty(KlighdInternalProperties.MODEL_ELEMEMT, newSourceNodeSource); + final int newSourceNodePosition = 0; + + newGraph.getChildren().add(newSourceNodePosition, newSourceNode); + + // Create the new target port on another node with index 1. + final KPort newTargetPort = KGraphUtil.createInitializedPort(); + final EObject newTargetPortSource = new EObjectImpl() { }; + newTargetPort.setProperty(KlighdInternalProperties.MODEL_ELEMEMT, newTargetPortSource); + final int targetNodePosition = 1; + final int newTargetPortPosition = 0; + + final KNode targetNode = newGraph.getChildren().get(targetNodePosition); + targetNode.getPorts().add(newTargetPortPosition, newTargetPort); + + // Create a new edge from the new source node to the new target port. + final KEdge newEdge = KGraphUtil.createInitializedEdge(); + final EObject newEdgeSource = new EObjectImpl() { }; + newEdge.setProperty(KlighdInternalProperties.MODEL_ELEMEMT, newEdgeSource); + + newEdge.setSource(newSourceNode); + newEdge.setTargetPort(newTargetPort); + newEdge.setTarget(targetNode); + final int newEdgeSourcePosition = 0; + final int newEdgeTargetPortPosition = 0; + final int newEdgeTargetPosition = 0; + newSourceNode.getOutgoingEdges().move(newEdgeSourcePosition, newEdge); + newTargetPort.getEdges().move(newEdgeTargetPortPosition, newEdge); + targetNode.getIncomingEdges().move(newEdgeTargetPosition, newEdge); + + final ViewContext viewContext = createViewContext(); + // Initialize the view context with the base graph. + INCREMENTAL_UPDATE_STRATEGY.update(viewContext.getViewModel(), baseGraph, viewContext); + // Update with the new graph. + INCREMENTAL_UPDATE_STRATEGY.update(viewContext.getViewModel(), newGraph, viewContext); + + // Assert the new node is in the updated base model. + EObject baseNewNode = viewContext.getTargetElements(newSourceNodeSource).stream().findFirst().orElse(null); + Assert.assertNotNull(baseNewNode); + Assert.assertTrue(baseNewNode instanceof KNode); + + // Assert the new port is in the updated base model. + EObject baseNewPort = viewContext.getTargetElements(newTargetPortSource).stream().findFirst().orElse(null); + Assert.assertNotNull(baseNewPort); + Assert.assertTrue(baseNewPort instanceof KPort); + + // Assert the new edge is in the updated base model. + EObject baseNewEdge = viewContext.getTargetElements(newEdgeSource).stream().findFirst().orElse(null); + Assert.assertNotNull(baseNewEdge); + Assert.assertTrue(baseNewEdge instanceof KEdge); + + + // Assert that the new node is in the same positions in the updated base graph as it is in the new graph. + int baseNodePosition = viewContext.getViewModel().getChildren().indexOf(baseNewNode); + Assert.assertSame(newSourceNodePosition, baseNodePosition); + + // Assert that the new port is in the same positions in the updated base graph as it is in the new graph. + int basePortPosition = viewContext.getViewModel().getChildren().get(targetNodePosition).getPorts() + .indexOf(baseNewPort); + Assert.assertSame(newTargetPortPosition, basePortPosition); + + // Assert that the new edge is in the same positions in the updated base graph as it is in the new graph. + int baseEdgeSourcePosition = viewContext.getViewModel().getChildren().get(newSourceNodePosition) + .getOutgoingEdges().indexOf(baseNewEdge); + Assert.assertSame(newEdgeSourcePosition, baseEdgeSourcePosition); + int baseEdgeTargetPosition = viewContext.getViewModel().getChildren().get(targetNodePosition).getIncomingEdges() + .indexOf(baseNewEdge); + Assert.assertSame(newEdgeTargetPosition, baseEdgeTargetPosition); + int baseEdgeTargetPortPosition = viewContext.getViewModel().getChildren().get(targetNodePosition).getPorts() + .get(newTargetPortPosition).getEdges().indexOf(baseNewEdge); + Assert.assertSame(newEdgeTargetPortPosition, baseEdgeTargetPortPosition); + } + /** * Tests removing an edge plainly on a node. */