Skip to main content
summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorPhilip Langer2014-10-11 18:14:20 +0000
committerLaurent Goubet2014-10-14 09:49:39 +0000
commitb32dfec137438c9242a1e674e9fec19ac659681d (patch)
tree6f79cc0bad79974dbda14052cecda5c3d88425c4
parent223fb7874e850c3e8d267bd96fae09d3ee6938c7 (diff)
downloadorg.eclipse.emf.compare-b32dfec137438c9242a1e674e9fec19ac659681d.tar.gz
org.eclipse.emf.compare-b32dfec137438c9242a1e674e9fec19ac659681d.tar.xz
org.eclipse.emf.compare-b32dfec137438c9242a1e674e9fec19ac659681d.zip
[446739] Fixes obtaining the target feature in three-way merge of a move
We now make sure that the target feature is correctly determined. This prevents a NullPointerException thrown during a three-way merge of a move, in which the target containment feature is not available in the type of the original containment feature. Bug: 446739 Signed-off-by: Philip Langer <planger@eclipsesource.com> Change-Id: I98a7f37bb5e43f10ddaf4a042893ed4773bf25b9
-rw-r--r--plugins/org.eclipse.emf.compare.tests/src/org/eclipse/emf/compare/tests/merge/ThreeWayBatchMergingTest.java150
-rw-r--r--plugins/org.eclipse.emf.compare.tests/src/org/eclipse/emf/compare/tests/merge/data/ThreeWayMergeInputData.java31
-rw-r--r--plugins/org.eclipse.emf.compare.tests/src/org/eclipse/emf/compare/tests/merge/data/threeway/movedifferentcontainmentfeature/origin.nodes9
-rw-r--r--plugins/org.eclipse.emf.compare.tests/src/org/eclipse/emf/compare/tests/merge/data/threeway/movedifferentcontainmentfeature/side-of-move.nodes10
-rw-r--r--plugins/org.eclipse.emf.compare.tests/src/org/eclipse/emf/compare/tests/merge/data/threeway/movedifferentcontainmentfeature/unchanged.nodes9
-rw-r--r--plugins/org.eclipse.emf.compare.tests/src/org/eclipse/emf/compare/tests/suite/AllTests.java3
-rw-r--r--plugins/org.eclipse.emf.compare/src/org/eclipse/emf/compare/internal/utils/DiffUtil.java32
7 files changed, 231 insertions, 13 deletions
diff --git a/plugins/org.eclipse.emf.compare.tests/src/org/eclipse/emf/compare/tests/merge/ThreeWayBatchMergingTest.java b/plugins/org.eclipse.emf.compare.tests/src/org/eclipse/emf/compare/tests/merge/ThreeWayBatchMergingTest.java
new file mode 100644
index 000000000..8c5659cfc
--- /dev/null
+++ b/plugins/org.eclipse.emf.compare.tests/src/org/eclipse/emf/compare/tests/merge/ThreeWayBatchMergingTest.java
@@ -0,0 +1,150 @@
+/*******************************************************************************
+ * Copyright (c) 2014 EclipseSource Muenchen GmbH and others.
+ * 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:
+ * Philip Langer - initial API and implementation
+ *******************************************************************************/
+package org.eclipse.emf.compare.tests.merge;
+
+import static org.eclipse.emf.compare.DifferenceSource.LEFT;
+import static org.eclipse.emf.compare.DifferenceSource.RIGHT;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.List;
+
+import org.eclipse.emf.common.notify.Notifier;
+import org.eclipse.emf.common.util.BasicMonitor;
+import org.eclipse.emf.common.util.EList;
+import org.eclipse.emf.compare.Comparison;
+import org.eclipse.emf.compare.ConflictKind;
+import org.eclipse.emf.compare.Diff;
+import org.eclipse.emf.compare.DifferenceSource;
+import org.eclipse.emf.compare.EMFCompare;
+import org.eclipse.emf.compare.merge.BatchMerger;
+import org.eclipse.emf.compare.merge.IBatchMerger;
+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.ThreeWayMergeInputData;
+import org.eclipse.emf.compare.tests.nodes.NodesPackage;
+import org.eclipse.emf.ecore.resource.Resource;
+import org.junit.Test;
+
+/**
+ * Tests three-way comparison of {@link NodesPackage nodes models} and subsequent merging using the
+ * {@link BatchMerger}.
+ *
+ * @author Philip Langer <planger@eclipsesource.com>
+ */
+public class ThreeWayBatchMergingTest {
+
+ final private ThreeWayMergeInputData input = new ThreeWayMergeInputData();
+
+ final private IMerger.Registry mergerRegistry = IMerger.RegistryImpl.createStandaloneInstance();
+
+ final private IBatchMerger merger = new BatchMerger(mergerRegistry);
+
+ /**
+ * Tests a scenario in which a model element is moved from one container into another, whereas the
+ * original container's type does not provide the containment reference that is used as the target of the
+ * move. This lead to an NPE (cf. bug #446739). In this test, the move is applied on the left-hand side.
+ *
+ * @throws IOException
+ * if {@link ThreeWayMergeInputData} fails to load the test models.
+ */
+ @Test
+ public void mergingMoveToDifferentContainmentFeatureLeft() throws IOException {
+ final Resource origin = input.getMoveToDifferentContainmentFeatureOrigin();
+ final Resource left = input.getMoveToDifferentContainmentFeatureMove();
+ final Resource right = input.getMoveToDifferentContainmentFeatureUnchanged();
+ assertUnchanged(new DefaultComparisonScope(left, right, origin), RIGHT);
+ batchMergeAndAssertEquality(new DefaultComparisonScope(left, right, origin), LEFT);
+ }
+
+ /**
+ * Tests a scenario in which a model element is moved from one container into another, whereas the
+ * original container's type does not provide the containment reference that is used as the target of the
+ * move. This lead to an NPE (cf. bug #446739). In this test, the move is applied on the right-hand side.
+ *
+ * @throws IOException
+ * if {@link ThreeWayMergeInputData} fails to load the test models.
+ */
+ @Test
+ public void mergingMoveToDifferentContainmentFeatureRight() throws IOException {
+ final Resource origin = input.getMoveToDifferentContainmentFeatureOrigin();
+ final Resource left = input.getMoveToDifferentContainmentFeatureUnchanged();
+ final Resource right = input.getMoveToDifferentContainmentFeatureMove();
+ assertUnchanged(new DefaultComparisonScope(left, right, origin), LEFT);
+ batchMergeAndAssertEquality(new DefaultComparisonScope(left, right, origin), RIGHT);
+ }
+
+ private void assertUnchanged(IComparisonScope scope, DifferenceSource side) {
+ final Notifier supposeUnchanged;
+ if (LEFT.equals(side)) {
+ supposeUnchanged = scope.getLeft();
+ } else {
+ supposeUnchanged = scope.getRight();
+ }
+ final IComparisonScope scopeForAssert = createTwoWayScopeWithOrigin(scope, supposeUnchanged);
+ EList<Diff> diffsForAssertion = compare(scopeForAssert).getDifferences();
+ assertEquals(0, diffsForAssertion.size());
+ }
+
+ private IComparisonScope createTwoWayScopeWithOrigin(IComparisonScope scope, final Notifier notifier) {
+ return new DefaultComparisonScope(scope.getOrigin(), notifier, null);
+ }
+
+ private void batchMergeAndAssertEquality(IComparisonScope scope, DifferenceSource side) {
+ batchMerge(scope, side);
+ assertEqualityOfLeftAndRight(scope);
+ }
+
+ private void batchMerge(IComparisonScope scope, DifferenceSource side) {
+ Comparison comparison = compare(scope);
+ final EList<Diff> differences = comparison.getDifferences();
+
+ if (LEFT.equals(side)) {
+ final List<Diff> leftDiffs = filterDiffs(differences, LEFT, false);
+ merger.copyAllLeftToRight(leftDiffs, new BasicMonitor());
+ } else {
+ final List<Diff> rightDiffs = filterDiffs(differences, RIGHT, false);
+ merger.copyAllRightToLeft(rightDiffs, new BasicMonitor());
+ }
+ }
+
+ private Comparison compare(IComparisonScope scope) {
+ return EMFCompare.builder().build().compare(scope);
+ }
+
+ private List<Diff> filterDiffs(List<Diff> diffs, DifferenceSource source, boolean filterConflicting) {
+ List<Diff> filteredDiffs = new ArrayList<Diff>();
+ for (Diff diff : diffs) {
+ if (source.equals(diff.getSource()) && (!isConflicting(diff) || !filterConflicting)) {
+ filteredDiffs.add(diff);
+ }
+ }
+ return filteredDiffs;
+ }
+
+ private boolean isConflicting(Diff diff) {
+ return diff.getConflict() != null && ConflictKind.REAL.equals(diff.getConflict().getKind());
+ }
+
+ private void assertEqualityOfLeftAndRight(IComparisonScope scope) {
+ final Notifier left = scope.getLeft();
+ final Notifier right = scope.getRight();
+ final IComparisonScope assertScope = new DefaultComparisonScope(left, right, null);
+ final Comparison assertComparison = compare(assertScope);
+ final EList<Diff> assertDiffs = assertComparison.getDifferences();
+
+ assertTrue(assertDiffs.isEmpty());
+ }
+
+}
diff --git a/plugins/org.eclipse.emf.compare.tests/src/org/eclipse/emf/compare/tests/merge/data/ThreeWayMergeInputData.java b/plugins/org.eclipse.emf.compare.tests/src/org/eclipse/emf/compare/tests/merge/data/ThreeWayMergeInputData.java
new file mode 100644
index 000000000..2ad71d797
--- /dev/null
+++ b/plugins/org.eclipse.emf.compare.tests/src/org/eclipse/emf/compare/tests/merge/data/ThreeWayMergeInputData.java
@@ -0,0 +1,31 @@
+/*******************************************************************************
+ * Copyright (c) 2014 EclipseSource Muenchen GmbH and others.
+ * 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:
+ * Philip Langer - initial API and implementation
+ *******************************************************************************/
+package org.eclipse.emf.compare.tests.merge.data;
+
+import java.io.IOException;
+
+import org.eclipse.emf.compare.tests.framework.AbstractInputData;
+import org.eclipse.emf.ecore.resource.Resource;
+
+@SuppressWarnings("nls")
+public class ThreeWayMergeInputData extends AbstractInputData {
+ public Resource getMoveToDifferentContainmentFeatureOrigin() throws IOException {
+ return loadFromClassLoader("threeway/movedifferentcontainmentfeature/origin.nodes");
+ }
+
+ public Resource getMoveToDifferentContainmentFeatureMove() throws IOException {
+ return loadFromClassLoader("threeway/movedifferentcontainmentfeature/side-of-move.nodes");
+ }
+
+ public Resource getMoveToDifferentContainmentFeatureUnchanged() throws IOException {
+ return loadFromClassLoader("threeway/movedifferentcontainmentfeature/unchanged.nodes");
+ }
+}
diff --git a/plugins/org.eclipse.emf.compare.tests/src/org/eclipse/emf/compare/tests/merge/data/threeway/movedifferentcontainmentfeature/origin.nodes b/plugins/org.eclipse.emf.compare.tests/src/org/eclipse/emf/compare/tests/merge/data/threeway/movedifferentcontainmentfeature/origin.nodes
new file mode 100644
index 000000000..5771610c5
--- /dev/null
+++ b/plugins/org.eclipse.emf.compare.tests/src/org/eclipse/emf/compare/tests/merge/data/threeway/movedifferentcontainmentfeature/origin.nodes
@@ -0,0 +1,9 @@
+<?xml version="1.0" encoding="ASCII"?>
+<nodes:NodeMultipleContainment
+ xmi:version="2.0"
+ xmlns:xmi="http://www.omg.org/XMI"
+ xmlns:nodes="http://www.eclipse.org/emf/compare/tests/nodes"
+ name="Root">
+ <containmentRef2 xsi:type="nodes:Node" name="A"/>
+ <containmentRef2 xsi:type="nodes:Node" name="B"/>
+</nodes:NodeMultipleContainment>
diff --git a/plugins/org.eclipse.emf.compare.tests/src/org/eclipse/emf/compare/tests/merge/data/threeway/movedifferentcontainmentfeature/side-of-move.nodes b/plugins/org.eclipse.emf.compare.tests/src/org/eclipse/emf/compare/tests/merge/data/threeway/movedifferentcontainmentfeature/side-of-move.nodes
new file mode 100644
index 000000000..0ddb5e4db
--- /dev/null
+++ b/plugins/org.eclipse.emf.compare.tests/src/org/eclipse/emf/compare/tests/merge/data/threeway/movedifferentcontainmentfeature/side-of-move.nodes
@@ -0,0 +1,10 @@
+<?xml version="1.0" encoding="ASCII"?>
+<nodes:NodeMultipleContainment
+ xmi:version="2.0"
+ xmlns:xmi="http://www.omg.org/XMI"
+ xmlns:nodes="http://www.eclipse.org/emf/compare/tests/nodes"
+ name="Root">
+ <containmentRef2 xsi:type="nodes:Node" name="B">
+ <containmentRef1 xsi:type="nodes:Node" name="A"/>
+ </containmentRef2>
+</nodes:NodeMultipleContainment>
diff --git a/plugins/org.eclipse.emf.compare.tests/src/org/eclipse/emf/compare/tests/merge/data/threeway/movedifferentcontainmentfeature/unchanged.nodes b/plugins/org.eclipse.emf.compare.tests/src/org/eclipse/emf/compare/tests/merge/data/threeway/movedifferentcontainmentfeature/unchanged.nodes
new file mode 100644
index 000000000..5771610c5
--- /dev/null
+++ b/plugins/org.eclipse.emf.compare.tests/src/org/eclipse/emf/compare/tests/merge/data/threeway/movedifferentcontainmentfeature/unchanged.nodes
@@ -0,0 +1,9 @@
+<?xml version="1.0" encoding="ASCII"?>
+<nodes:NodeMultipleContainment
+ xmi:version="2.0"
+ xmlns:xmi="http://www.omg.org/XMI"
+ xmlns:nodes="http://www.eclipse.org/emf/compare/tests/nodes"
+ name="Root">
+ <containmentRef2 xsi:type="nodes:Node" name="A"/>
+ <containmentRef2 xsi:type="nodes:Node" name="B"/>
+</nodes:NodeMultipleContainment>
diff --git a/plugins/org.eclipse.emf.compare.tests/src/org/eclipse/emf/compare/tests/suite/AllTests.java b/plugins/org.eclipse.emf.compare.tests/src/org/eclipse/emf/compare/tests/suite/AllTests.java
index ca5805b02..01df42167 100644
--- a/plugins/org.eclipse.emf.compare.tests/src/org/eclipse/emf/compare/tests/suite/AllTests.java
+++ b/plugins/org.eclipse.emf.compare.tests/src/org/eclipse/emf/compare/tests/suite/AllTests.java
@@ -41,6 +41,7 @@ import org.eclipse.emf.compare.tests.merge.IndividualMergeOutOfScopeValuesTest;
import org.eclipse.emf.compare.tests.merge.IndividualMergeTest;
import org.eclipse.emf.compare.tests.merge.MultipleMergeTest;
import org.eclipse.emf.compare.tests.merge.PseudoConflictMergeTest;
+import org.eclipse.emf.compare.tests.merge.ThreeWayBatchMergingTest;
import org.eclipse.emf.compare.tests.merge.TwoWayBatchMergingTest;
import org.eclipse.emf.compare.tests.nodes.NodesPackage;
import org.eclipse.emf.compare.tests.nodes.util.NodesResourceFactoryImpl;
@@ -72,7 +73,7 @@ import org.junit.runners.Suite.SuiteClasses;
ConflictMergeTest.class, PseudoConflictMergeTest.class, ProximityIndexTest.class, AllRCPTests.class,
FeatureMaps2wayMergeTest.class, FeatureMaps3wayMergeTest.class, FeatureMapsConflictsMergeTest.class,
FeatureMapsPseudoConflictsMergeTest.class, TwoWayBatchMergingTest.class, EqualityHelperTest.class,
- FeatureFilterTest.class })
+ FeatureFilterTest.class, ThreeWayBatchMergingTest.class })
public class AllTests {
/**
* Standalone launcher for all of compare's tests.
diff --git a/plugins/org.eclipse.emf.compare/src/org/eclipse/emf/compare/internal/utils/DiffUtil.java b/plugins/org.eclipse.emf.compare/src/org/eclipse/emf/compare/internal/utils/DiffUtil.java
index 1b1118320..c0d038807 100644
--- a/plugins/org.eclipse.emf.compare/src/org/eclipse/emf/compare/internal/utils/DiffUtil.java
+++ b/plugins/org.eclipse.emf.compare/src/org/eclipse/emf/compare/internal/utils/DiffUtil.java
@@ -7,7 +7,7 @@
*
* Contributors:
* Obeo - initial API and implementation
- * Philip Langer - Fixes for bug 440679, 441258, 442439, 443504, and refactorings
+ * Philip Langer - Fixes for bug 440679, 441258, 442439, 443504, 446739, and refactorings
*******************************************************************************/
package org.eclipse.emf.compare.internal.utils;
@@ -1065,18 +1065,9 @@ public final class DiffUtil {
final EStructuralFeature diffFeature = getChangedFeature(diff);
final Object diffValue = getChangedValue(diff);
- if (isContainmentReferenceMove(diff)) {
+ if (isContainmentReferenceMove(diff) && isTargetOnTheRight(diff, rightToLeft)) {
final Match valueMatch = comparison.getMatch((EObject)diffValue);
- if (rightToLeft && DifferenceSource.LEFT == diff.getSource() && valueMatch.getRight() != null) {
- targetFeature = valueMatch.getRight().eContainingFeature();
- } else if (!rightToLeft && DifferenceSource.RIGHT == diff.getSource()
- && valueMatch.getLeft() != null) {
- targetFeature = valueMatch.getRight().eContainingFeature();
- } else if (valueMatch.getOrigin() != null) {
- targetFeature = valueMatch.getOrigin().eContainingFeature();
- } else {
- targetFeature = diffFeature;
- }
+ targetFeature = valueMatch.getRight().eContainingFeature();
} else {
targetFeature = diffFeature;
}
@@ -1100,6 +1091,23 @@ public final class DiffUtil {
}
/**
+ * Specifies whether the target container, target feature, and target value of the current merging is the
+ * right-hand side. This depends on the source of the {@code diff} and the direction of merging as
+ * specified in {@code rightToLeft}.
+ *
+ * @param diff
+ * The diff to check.
+ * @param rightToLeft
+ * Direction of the merging. {@code true} if the merge is to be done on the left side, making
+ * 'target' the right side, {@code false} otherwise.
+ * @return <code>true</code> if the target is on the right side, <code>false</code> otherwise.
+ */
+ private static boolean isTargetOnTheRight(Diff diff, boolean rightToLeft) {
+ return (rightToLeft && DifferenceSource.LEFT == diff.getSource())
+ || (!rightToLeft && DifferenceSource.RIGHT == diff.getSource());
+ }
+
+ /**
* When computing the insertion index of an element in a list, we need to ignore all elements present in
* that list that feature unresolved Diffs on the same feature.
*

Back to the top