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 addedToSingleValuedReference(String qualifiedName, String referenceName, + String addedQualifiedName) { + return and(ofKind(DifferenceKind.ADD), onEObject(qualifiedName), + referenceValueMatch(referenceName, addedQualifiedName, false)); + } + + @SuppressWarnings("unchecked") + private Predicate 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