diff --git a/plugins/de.cau.cs.kieler.klighd.incremental/src/de/cau/cs/kieler/klighd/incremental/IncrementalUpdateStrategy.java b/plugins/de.cau.cs.kieler.klighd.incremental/src/de/cau/cs/kieler/klighd/incremental/IncrementalUpdateStrategy.java index 1e97efe72..31d01d59e 100644 --- a/plugins/de.cau.cs.kieler.klighd.incremental/src/de/cau/cs/kieler/klighd/incremental/IncrementalUpdateStrategy.java +++ b/plugins/de.cau.cs.kieler.klighd.incremental/src/de/cau/cs/kieler/klighd/incremental/IncrementalUpdateStrategy.java @@ -72,8 +72,6 @@ public void update(final KNode baseModel, final KNode newModel, final ViewContex KGraphMerger merger = new KGraphMerger(comparison, new KGraphDataFilter()); merger.merge(); - UIDAdapters.removeAdapter(baseModel); - UIDAdapters.removeAdapter(newModel); } catch (RuntimeException e) { final String msg = "KLighD: Incremental update of diagram failed."; @@ -83,6 +81,8 @@ public void update(final KNode baseModel, final KNode newModel, final ViewContex // if incremental updating failed, apply the SimpleUpdateStrategy fallback(baseModel, newModel, viewContext); } + UIDAdapters.removeAdapter(baseModel); + UIDAdapters.removeAdapter(newModel); } private void fallback(final KNode baseModel, final KNode newModel, 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 7c29068ed..89658a941 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 @@ -119,9 +119,14 @@ private void removeNode(final KNode node) { * Add new from the new model to the base model. */ private void handleAddedNodes() { - for (KNode node : comparison.getAddedNodes()) { - addNode(node); - } + // Before adding the nodes we have to make sure they are added in the same order as they appear in the + // containment list of their parent to ensure correct generation and mapping of ID-less elements. + comparison.getAddedNodes().stream().sorted( + (KNode n1, KNode n2) -> n1.getParent().getChildren().indexOf(n1) + - n2.getParent().getChildren().indexOf(n2) + ).forEachOrdered( + (KNode node) -> addNode(node) + ); // Add edges after adding the nodes to ensure that all targets are available. for (KNode node : comparison.getAddedNodes()) { handleEdges(comparison.lookupBaseNode(node), node); @@ -160,7 +165,6 @@ private void addNode(final KNode node) { */ private void handleMatchedNodes() { for (ValueDifference diff : comparison.getMatchedNodes()) { - // TODO Maybe check if update is really necessary updateKnode(diff.leftValue(), diff.rightValue()); } } 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 4174f9d0e..5c3e62f74 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 @@ -157,6 +157,62 @@ public void testAddNode() { Assert.assertSame(newNodePosition, baseNodePosition); } + /** + * Tests adding multiple nodes. Checks if the nodes are added and if they are added at the correct positions. + */ + @Test + public void testAddMultipleNodes() { + final KNode baseGraph = KGraphUtil.createInitializedNode(); + final KNode nodeA0 = KGraphUtil.createInitializedNode(); + addIdentifier(nodeA0, "nodeA"); + nodeA0.setParent(baseGraph); + final KNode newGraph = KGraphUtil.createInitializedNode(); + final KNode nodeA1 = KGraphUtil.createInitializedNode(); + addIdentifier(nodeA1, "nodeA"); + nodeA1.setParent(newGraph); + + final KNode newNode0 = KGraphUtil.createInitializedNode(); + final KNode newNode1 = KGraphUtil.createInitializedNode(); + final KNode newNode2 = KGraphUtil.createInitializedNode(); + final EObject newNodeSource0 = new EObjectImpl() { }; + final EObject newNodeSource1 = new EObjectImpl() { }; + final EObject newNodeSource2 = new EObjectImpl() { }; + newNode0.setProperty(KlighdInternalProperties.MODEL_ELEMEMT, newNodeSource0); + newNode1.setProperty(KlighdInternalProperties.MODEL_ELEMEMT, newNodeSource1); + newNode2.setProperty(KlighdInternalProperties.MODEL_ELEMEMT, newNodeSource2); + final int newNode0Position = 0; + final int newNode1Position = 1; + final int newNode2Position = 2; + + newGraph.getChildren().add(newNode0Position, newNode0); + newGraph.getChildren().add(newNode1Position, newNode1); + newGraph.getChildren().add(newNode2Position, newNode2); + + 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 nodes are in the updated base model. + EObject baseNewNode0 = viewContext.getTargetElements(newNodeSource0).stream().findFirst().orElse(null); + EObject baseNewNode1 = viewContext.getTargetElements(newNodeSource1).stream().findFirst().orElse(null); + EObject baseNewNode2 = viewContext.getTargetElements(newNodeSource2).stream().findFirst().orElse(null); + Assert.assertNotNull(baseNewNode0); + Assert.assertNotNull(baseNewNode1); + Assert.assertNotNull(baseNewNode2); + Assert.assertTrue(baseNewNode0 instanceof KNode); + Assert.assertTrue(baseNewNode1 instanceof KNode); + Assert.assertTrue(baseNewNode2 instanceof KNode); + // Assert that the new nodes are in the same position in the updated base graph as they are in the new graph. + int baseNode0Position = viewContext.getViewModel().getChildren().indexOf(baseNewNode0); + int baseNode1Position = viewContext.getViewModel().getChildren().indexOf(baseNewNode1); + int baseNode2Position = viewContext.getViewModel().getChildren().indexOf(baseNewNode2); + Assert.assertSame(newNode0Position, baseNode0Position); + Assert.assertSame(newNode1Position, baseNode1Position); + Assert.assertSame(newNode2Position, baseNode2Position); + } + /** * Tests removing a node. */ @@ -679,4 +735,120 @@ private void removeEdge(KEdge edge) { edge.setTargetPort(null); } + /** + * Tests adding connected nodes without IDs as in KLighD issue 48. + * See https://github.com/kieler/KLighD/issues/48 + */ + @Test + public void testIssue48() { + // Create the base graph with two connected nodes. + final KNode baseGraph = KGraphUtil.createInitializedNode(); + final KNode baseNode3 = KGraphUtil.createInitializedNode(); + final KNode baseNode2 = KGraphUtil.createInitializedNode(); + baseNode3.setParent(baseGraph); + baseNode2.setParent(baseGraph); + + final KPort basePort2In = KGraphUtil.createInitializedPort(); + basePort2In.setNode(baseNode2); + final KPort basePort2Out = KGraphUtil.createInitializedPort(); + basePort2Out.setNode(baseNode2); + + final KPort basePort3In = KGraphUtil.createInitializedPort(); + basePort3In.setNode(baseNode3); + final KPort basePort3Out = KGraphUtil.createInitializedPort(); + basePort3Out.setNode(baseNode3); + + final KEdge baseEdge23 = KGraphUtil.createInitializedEdge(); + baseEdge23.setSource(baseNode2); + baseEdge23.setTarget(baseNode3); + + // Create the new graph with five connected nodes and ports. + final KNode newGraph = KGraphUtil.createInitializedNode(); + final KNode newNodeC = KGraphUtil.createInitializedNode(); + final KNode newNode1 = KGraphUtil.createInitializedNode(); + final KNode newNode3 = KGraphUtil.createInitializedNode(); + final KNode newNodeL = KGraphUtil.createInitializedNode(); + final KNode newNode2 = KGraphUtil.createInitializedNode(); + newNodeC.setParent(newGraph); + newNode1.setParent(newGraph); + newNode3.setParent(newGraph); + newNodeL.setParent(newGraph); + newNode2.setParent(newGraph); + final EObject newNodeSourceC = new EObjectImpl() { }; + final EObject newNodeSource1 = new EObjectImpl() { }; + final EObject newNodeSource3 = new EObjectImpl() { }; + final EObject newNodeSourceL = new EObjectImpl() { }; + final EObject newNodeSource2 = new EObjectImpl() { }; + newNodeC.setProperty(KlighdInternalProperties.MODEL_ELEMEMT, newNodeSourceC); + newNode1.setProperty(KlighdInternalProperties.MODEL_ELEMEMT, newNodeSource1); + newNode3.setProperty(KlighdInternalProperties.MODEL_ELEMEMT, newNodeSource3); + newNodeL.setProperty(KlighdInternalProperties.MODEL_ELEMEMT, newNodeSourceL); + newNode2.setProperty(KlighdInternalProperties.MODEL_ELEMEMT, newNodeSource2); + + final KPort newPortCOut = KGraphUtil.createInitializedPort(); + newPortCOut.setNode(newNodeC); + final KPort newPort1In = KGraphUtil.createInitializedPort(); + newPort1In.setNode(newNode1); + final KPort newPort1Out = KGraphUtil.createInitializedPort(); + newPort1Out.setNode(newNode1); + final KPort newPort3In = KGraphUtil.createInitializedPort(); + newPort3In.setNode(newNode3); + final KPort newPort3Out = KGraphUtil.createInitializedPort(); + newPort3Out.setNode(newNode3); + final KPort newPortLIn = KGraphUtil.createInitializedPort(); + newPortLIn.setNode(newNodeL); + final KPort newPortLOut = KGraphUtil.createInitializedPort(); + newPortLOut.setNode(newNodeL); + final KPort newPort2In = KGraphUtil.createInitializedPort(); + newPort2In.setNode(newNode2); + final KPort newPort2Out = KGraphUtil.createInitializedPort(); + newPort2Out.setNode(newNode2); + + final KEdge newEdgeC1 = KGraphUtil.createInitializedEdge(); + newEdgeC1.setSource(newNodeC); + newEdgeC1.setSourcePort(newPortCOut); + newEdgeC1.setTarget(newNode1); + newEdgeC1.setTargetPort(newPort1In); + final KEdge newEdge1L = KGraphUtil.createInitializedEdge(); + newEdge1L.setSource(newNode1); + newEdge1L.setSourcePort(newPort1Out); + newEdge1L.setTarget(newNodeL); + newEdge1L.setTargetPort(newPortLIn); + final KEdge newEdge3L = KGraphUtil.createInitializedEdge(); + newEdge3L.setSource(newNode3); + newEdge3L.setSourcePort(newPort3Out); + newEdge3L.setTarget(newNodeL); + newEdge3L.setTargetPort(newPortLIn); + final KEdge newEdgeL2 = KGraphUtil.createInitializedEdge(); + newEdgeL2.setSource(newNodeL); + newEdgeL2.setSourcePort(newPortLOut); + newEdgeL2.setTarget(newNode2); + newEdgeL2.setTargetPort(newPort2In); + + // The test for these graphs. + + 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 nodes are in the updated base model. + EObject baseNewNodeC = viewContext.getTargetElements(newNodeSourceC).stream().findFirst().orElse(null); + EObject baseNewNode1 = viewContext.getTargetElements(newNodeSource1).stream().findFirst().orElse(null); + EObject baseNewNode3 = viewContext.getTargetElements(newNodeSource3).stream().findFirst().orElse(null); + EObject baseNewNodeL = viewContext.getTargetElements(newNodeSourceL).stream().findFirst().orElse(null); + EObject baseNewNode2 = viewContext.getTargetElements(newNodeSource2).stream().findFirst().orElse(null); + Assert.assertNotNull(baseNewNodeC); + Assert.assertNotNull(baseNewNode1); + Assert.assertNotNull(baseNewNode3); // This one failed before the fix because of the ordering. + Assert.assertNotNull(baseNewNodeL); + Assert.assertNotNull(baseNewNode2); + Assert.assertTrue(baseNewNodeC instanceof KNode); + Assert.assertTrue(baseNewNode1 instanceof KNode); + Assert.assertTrue(baseNewNode3 instanceof KNode); + Assert.assertTrue(baseNewNodeL instanceof KNode); + Assert.assertTrue(baseNewNode2 instanceof KNode); + } + }