From 7f0ed79430efeb4e0755a5ebdf0f91b013d2b170 Mon Sep 17 00:00:00 2001
From: lgoubet
Date: Thu, 23 May 2019 14:31:31 +0200
Subject: Merging single-valued containment changes can lead to dangling
elements
Change-Id: I122badf27faa6f987a0849e046ff48c595f3c9cd
---
.../merge/DanglingReferenceAfterMergeTest.java | 164 +++++++++++++++++++++
.../tests/merge/data/IndividualDiffInputData.java | 12 ++
.../tests/merge/data/danglingpostmerge/left.nodes | 9 ++
.../merge/data/danglingpostmerge/origin.nodes | 9 ++
.../tests/merge/data/danglingpostmerge/right.nodes | 9 ++
.../emf/compare/merge/ReferenceChangeMerger.java | 6 +-
6 files changed, 206 insertions(+), 3 deletions(-)
create mode 100644 plugins/org.eclipse.emf.compare.tests/src/org/eclipse/emf/compare/tests/merge/DanglingReferenceAfterMergeTest.java
create mode 100644 plugins/org.eclipse.emf.compare.tests/src/org/eclipse/emf/compare/tests/merge/data/danglingpostmerge/left.nodes
create mode 100644 plugins/org.eclipse.emf.compare.tests/src/org/eclipse/emf/compare/tests/merge/data/danglingpostmerge/origin.nodes
create mode 100644 plugins/org.eclipse.emf.compare.tests/src/org/eclipse/emf/compare/tests/merge/data/danglingpostmerge/right.nodes
(limited to 'plugins')
diff --git a/plugins/org.eclipse.emf.compare.tests/src/org/eclipse/emf/compare/tests/merge/DanglingReferenceAfterMergeTest.java b/plugins/org.eclipse.emf.compare.tests/src/org/eclipse/emf/compare/tests/merge/DanglingReferenceAfterMergeTest.java
new file mode 100644
index 000000000..7e4bc1b8a
--- /dev/null
+++ b/plugins/org.eclipse.emf.compare.tests/src/org/eclipse/emf/compare/tests/merge/DanglingReferenceAfterMergeTest.java
@@ -0,0 +1,164 @@
+/*******************************************************************************
+ * Copyright (c) 2019 Obeo.
+ * All rights reserved. This program and the accompanying materials
+ * are made available under the terms of the Eclipse Public License v1.0
+ * which accompanies this distribution, and is available at
+ * http://www.eclipse.org/legal/epl-v10.html
+ *
+ * Contributors:
+ * Obeo - initial API and implementation
+ *******************************************************************************/
+package org.eclipse.emf.compare.tests.merge;
+
+import static com.google.common.base.Predicates.and;
+import static org.eclipse.emf.compare.utils.EMFComparePredicates.changedReference;
+import static org.eclipse.emf.compare.utils.EMFComparePredicates.fromSide;
+import static org.eclipse.emf.compare.utils.EMFComparePredicates.ofKind;
+import static org.eclipse.emf.compare.utils.EMFComparePredicates.onEObject;
+import static org.eclipse.emf.compare.utils.EMFComparePredicates.referenceValueMatch;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+
+import com.google.common.base.Predicate;
+import com.google.common.collect.Iterators;
+
+import java.io.IOException;
+import java.util.Arrays;
+import java.util.Iterator;
+import java.util.List;
+
+import org.eclipse.emf.common.util.BasicMonitor;
+import org.eclipse.emf.common.util.TreeIterator;
+import org.eclipse.emf.compare.Comparison;
+import org.eclipse.emf.compare.Diff;
+import org.eclipse.emf.compare.DifferenceKind;
+import org.eclipse.emf.compare.DifferenceSource;
+import org.eclipse.emf.compare.DifferenceState;
+import org.eclipse.emf.compare.EMFCompare;
+import org.eclipse.emf.compare.merge.BatchMerger;
+import org.eclipse.emf.compare.merge.IMerger;
+import org.eclipse.emf.compare.scope.DefaultComparisonScope;
+import org.eclipse.emf.compare.scope.IComparisonScope;
+import org.eclipse.emf.compare.tests.merge.data.IndividualDiffInputData;
+import org.eclipse.emf.ecore.EObject;
+import org.eclipse.emf.ecore.EStructuralFeature;
+import org.eclipse.emf.ecore.resource.Resource;
+import org.eclipse.emf.ecore.util.EcoreUtil;
+import org.junit.Test;
+
+/**
+ * This class double-checks that there are no dangling references after a batch merge.
+ *
+ * A particular case that has been isolated presenting such an occurrence is when the user changes the element
+ * contained in a single-valued containment reference while this value is referenced through non-containment
+ * features.
+ *
+ *
+ * @author lgoubet
+ */
+@SuppressWarnings("nls")
+public class DanglingReferenceAfterMergeTest {
+ private IndividualDiffInputData input = new IndividualDiffInputData();
+
+ private final BatchMerger batchMerger = new BatchMerger(IMerger.RegistryImpl.createStandaloneInstance());
+
+ @Test
+ public void testDanglingReferencePostMergeLtR() throws IOException {
+ Resource ancestor = input.getDanglingPostMergeAncestor();
+ Resource left = input.getDanglingPostMergeLeft();
+ Resource right = input.getDanglingPostMergeRight();
+
+ IComparisonScope scope = new DefaultComparisonScope(left, right, ancestor);
+ Comparison comparison = EMFCompare.builder().build().compare(scope);
+
+ List differences = comparison.getDifferences();
+ assertEquals(3, differences.size());
+
+ Diff changedReference = Iterators.find(differences.iterator(),
+ and(fromSide(DifferenceSource.RIGHT),
+ changedReference("root.container.ClassA", "singleValuedReference",
+ "root.referencedContainer.ClassB", "root.referencedContainer.ClassC")));
+ Diff addedClassC = Iterators.find(differences.iterator(),
+ and(fromSide(DifferenceSource.RIGHT), addedToSingleValuedReference("root.referencedContainer",
+ "singleValueContainment", "root.referencedContainer.ClassC")));
+ Diff removedClassB = Iterators.find(differences.iterator(),
+ and(fromSide(DifferenceSource.RIGHT),
+ removedFromSingleValuedReference("root.referencedContainer", "singleValueContainment",
+ "root.referencedContainer.ClassB")));
+
+ batchMerger.copyAllLeftToRight(Arrays.asList(addedClassC), new BasicMonitor());
+ assertNoDangling(right);
+ assertNotNull(getNodeNamed(right, "ClassB"));
+ assertEquals(DifferenceState.DISCARDED, changedReference.getState());
+ assertEquals(DifferenceState.DISCARDED, addedClassC.getState());
+ assertEquals(DifferenceState.DISCARDED, removedClassB.getState());
+ }
+
+ @Test
+ public void testDanglingReferencePostMergeRtL() throws IOException {
+ Resource ancestor = input.getDanglingPostMergeAncestor();
+ Resource left = input.getDanglingPostMergeLeft();
+ Resource right = input.getDanglingPostMergeRight();
+
+ IComparisonScope scope = new DefaultComparisonScope(left, right, ancestor);
+ Comparison comparison = EMFCompare.builder().build().compare(scope);
+
+ List differences = comparison.getDifferences();
+ assertEquals(3, differences.size());
+
+ Diff changedReference = Iterators.find(differences.iterator(),
+ and(fromSide(DifferenceSource.RIGHT),
+ changedReference("root.container.ClassA", "singleValuedReference",
+ "root.referencedContainer.ClassB", "root.referencedContainer.ClassC")));
+ Diff addedClassC = Iterators.find(differences.iterator(),
+ and(fromSide(DifferenceSource.RIGHT), addedToSingleValuedReference("root.referencedContainer",
+ "singleValueContainment", "root.referencedContainer.ClassC")));
+ Diff removedClassB = Iterators.find(differences.iterator(),
+ and(fromSide(DifferenceSource.RIGHT),
+ removedFromSingleValuedReference("root.referencedContainer", "singleValueContainment",
+ "root.referencedContainer.ClassB")));
+
+ batchMerger.copyAllRightToLeft(Arrays.asList(addedClassC), new BasicMonitor());
+ assertNoDangling(left);
+ assertNotNull(getNodeNamed(left, "ClassC"));
+ assertEquals(DifferenceState.MERGED, changedReference.getState());
+ assertEquals(DifferenceState.MERGED, addedClassC.getState());
+ assertEquals(DifferenceState.MERGED, removedClassB.getState());
+ }
+
+ private void assertNoDangling(Resource res) {
+ TreeIterator iterator = res.getAllContents();
+ while (iterator.hasNext()) {
+ EObject next = iterator.next();
+ for (EObject o : next.eCrossReferences()) {
+ assertNotNull(o.eResource());
+ }
+ }
+ }
+
+ @SuppressWarnings("unchecked")
+ private Predicate super Diff> addedToSingleValuedReference(String qualifiedName, String referenceName,
+ String addedQualifiedName) {
+ return and(ofKind(DifferenceKind.ADD), onEObject(qualifiedName),
+ referenceValueMatch(referenceName, addedQualifiedName, false));
+ }
+
+ @SuppressWarnings("unchecked")
+ private Predicate super Diff> removedFromSingleValuedReference(String qualifiedName,
+ String referenceName, String addedQualifiedName) {
+ return and(ofKind(DifferenceKind.DELETE), onEObject(qualifiedName),
+ referenceValueMatch(referenceName, addedQualifiedName, false));
+ }
+
+ private EObject getNodeNamed(Resource res, String name) {
+ final Iterator iterator = EcoreUtil.getAllProperContents(res, false);
+ while (iterator.hasNext()) {
+ final EObject next = iterator.next();
+ final EStructuralFeature nameFeature = next.eClass().getEStructuralFeature("name");
+ if (nameFeature != null && name.equals(next.eGet(nameFeature))) {
+ return next;
+ }
+ }
+ return null;
+ }
+}
diff --git a/plugins/org.eclipse.emf.compare.tests/src/org/eclipse/emf/compare/tests/merge/data/IndividualDiffInputData.java b/plugins/org.eclipse.emf.compare.tests/src/org/eclipse/emf/compare/tests/merge/data/IndividualDiffInputData.java
index 805f5d1b6..92c69b74d 100644
--- a/plugins/org.eclipse.emf.compare.tests/src/org/eclipse/emf/compare/tests/merge/data/IndividualDiffInputData.java
+++ b/plugins/org.eclipse.emf.compare.tests/src/org/eclipse/emf/compare/tests/merge/data/IndividualDiffInputData.java
@@ -820,4 +820,16 @@ public class IndividualDiffInputData extends AbstractInputData {
public Resource getMoveConflictAndPseudoConflictImplicationsRight() throws IOException {
return loadFromClassLoader("dependencies/bug_484579/right.uml");
}
+
+ public Resource getDanglingPostMergeAncestor() throws IOException {
+ return loadFromClassLoader("danglingpostmerge/origin.nodes");
+ }
+
+ public Resource getDanglingPostMergeLeft() throws IOException {
+ return loadFromClassLoader("danglingpostmerge/left.nodes");
+ }
+
+ public Resource getDanglingPostMergeRight() throws IOException {
+ return loadFromClassLoader("danglingpostmerge/right.nodes");
+ }
}
diff --git a/plugins/org.eclipse.emf.compare.tests/src/org/eclipse/emf/compare/tests/merge/data/danglingpostmerge/left.nodes b/plugins/org.eclipse.emf.compare.tests/src/org/eclipse/emf/compare/tests/merge/data/danglingpostmerge/left.nodes
new file mode 100644
index 000000000..337f3daa9
--- /dev/null
+++ b/plugins/org.eclipse.emf.compare.tests/src/org/eclipse/emf/compare/tests/merge/data/danglingpostmerge/left.nodes
@@ -0,0 +1,9 @@
+
+
+
+
+
+
+
+
+
diff --git a/plugins/org.eclipse.emf.compare.tests/src/org/eclipse/emf/compare/tests/merge/data/danglingpostmerge/origin.nodes b/plugins/org.eclipse.emf.compare.tests/src/org/eclipse/emf/compare/tests/merge/data/danglingpostmerge/origin.nodes
new file mode 100644
index 000000000..337f3daa9
--- /dev/null
+++ b/plugins/org.eclipse.emf.compare.tests/src/org/eclipse/emf/compare/tests/merge/data/danglingpostmerge/origin.nodes
@@ -0,0 +1,9 @@
+
+
+
+
+
+
+
+
+
diff --git a/plugins/org.eclipse.emf.compare.tests/src/org/eclipse/emf/compare/tests/merge/data/danglingpostmerge/right.nodes b/plugins/org.eclipse.emf.compare.tests/src/org/eclipse/emf/compare/tests/merge/data/danglingpostmerge/right.nodes
new file mode 100644
index 000000000..12a13b8ea
--- /dev/null
+++ b/plugins/org.eclipse.emf.compare.tests/src/org/eclipse/emf/compare/tests/merge/data/danglingpostmerge/right.nodes
@@ -0,0 +1,9 @@
+
+
+
+
+
+
+
+
+
diff --git a/plugins/org.eclipse.emf.compare/src/org/eclipse/emf/compare/merge/ReferenceChangeMerger.java b/plugins/org.eclipse.emf.compare/src/org/eclipse/emf/compare/merge/ReferenceChangeMerger.java
index f4ff37304..d71c2f3cc 100644
--- a/plugins/org.eclipse.emf.compare/src/org/eclipse/emf/compare/merge/ReferenceChangeMerger.java
+++ b/plugins/org.eclipse.emf.compare/src/org/eclipse/emf/compare/merge/ReferenceChangeMerger.java
@@ -411,7 +411,7 @@ public class ReferenceChangeMerger extends AbstractMerger {
expectedValue = diff.getValue();
}
} else if (rightToLeft) {
- if (reference.isContainment() || valueMatch.getLeft() == null) {
+ if (valueMatch.getLeft() == null) {
expectedValue = createCopy(diff.getValue());
valueMatch.setLeft(expectedValue);
needXmiId = true;
@@ -419,7 +419,7 @@ public class ReferenceChangeMerger extends AbstractMerger {
expectedValue = valueMatch.getLeft();
}
} else {
- if (reference.isContainment() || valueMatch.getRight() == null) {
+ if (valueMatch.getRight() == null) {
expectedValue = createCopy(diff.getValue());
valueMatch.setRight(expectedValue);
needXmiId = true;
@@ -441,7 +441,7 @@ public class ReferenceChangeMerger extends AbstractMerger {
if (needXmiId) {
// Copy XMI ID when applicable.
final Resource initialResource = diff.getValue().eResource();
- final Resource targetResource = expectedValue.eResource();
+ final Resource targetResource = expectedContainer.eResource();
if (initialResource instanceof XMIResource && targetResource instanceof XMIResource) {
((XMIResource)targetResource).setID(expectedValue,
((XMIResource)initialResource).getID(diff.getValue()));
--
cgit v1.2.3