diff options
author | Stefan Dirix | 2015-02-26 13:51:40 +0000 |
---|---|---|
committer | Axel RICHARD | 2015-03-02 13:24:10 +0000 |
commit | 0d5159b09e90fc218583fd321922040cdcea3545 (patch) | |
tree | 308dc1761b5ae9feaf20443835bfdda31c593403 | |
parent | f6833ad75aae9922047eae5e90ae01fff8d1dd8b (diff) | |
download | org.eclipse.emf.compare-0d5159b09e90fc218583fd321922040cdcea3545.tar.gz org.eclipse.emf.compare-0d5159b09e90fc218583fd321922040cdcea3545.tar.xz org.eclipse.emf.compare-0d5159b09e90fc218583fd321922040cdcea3545.zip |
[460923] Do not filter FeatureMapChanges on removed Elements
The DefaultDiffEngine tries to minimize the number of calculated
differences. This fix modifies the DefaultDiffEngine to allow
calculation of FeatureMapChanges on removed elements. This is needed to
prevent ordering issues when contained elements are added back via
right-to-left merges.
Includes testcase.
Signed-off-by: Stefan Dirix <sdirix@eclipsesource.com>
Change-Id: Ia119ed2bf98a2730aa74c343e9138ed4bd82d35e
5 files changed, 85 insertions, 16 deletions
diff --git a/plugins/org.eclipse.emf.compare.tests/src/org/eclipse/emf/compare/tests/merge/MultipleMergeTest.java b/plugins/org.eclipse.emf.compare.tests/src/org/eclipse/emf/compare/tests/merge/MultipleMergeTest.java index 302dcc9ed..ef9e1aced 100644 --- a/plugins/org.eclipse.emf.compare.tests/src/org/eclipse/emf/compare/tests/merge/MultipleMergeTest.java +++ b/plugins/org.eclipse.emf.compare.tests/src/org/eclipse/emf/compare/tests/merge/MultipleMergeTest.java @@ -7,7 +7,7 @@ * * Contributors: * Obeo - initial API and implementation - * Stefan Dirix - bugs 441172, 452147 and 460902 + * Stefan Dirix - bugs 441172, 452147, 460902 and 460923 *******************************************************************************/ package org.eclipse.emf.compare.tests.merge; @@ -29,9 +29,11 @@ import static org.junit.Assert.assertTrue; import com.google.common.collect.ImmutableList; import com.google.common.collect.Iterators; +import com.google.common.collect.Lists; import java.io.IOException; import java.util.Collections; +import java.util.Iterator; import java.util.List; import org.eclipse.emf.common.util.BasicMonitor; @@ -1357,6 +1359,49 @@ public class MultipleMergeTest { } @Test + public void testRemoveFeatureMapR2L() throws IOException { + ResourceSetImpl resourceSet = new ResourceSetImpl(); + + final Resource left = twoWayInput.getRemoveFeatureMapR2LLeft(resourceSet); + final Resource right = twoWayInput.getRemoveFeatureMapR2LRight(resourceSet); + + final IComparisonScope scope = new DefaultComparisonScope(left, right, null); + Comparison comparison = EMFCompare.builder().build().compare(scope); + + final List<Diff> differences = comparison.getDifferences(); + + // There should be 5 differences + // 1. Remove first key (ReferenceChange) + // 2. Remove first key (FeatureMapChange) + // 3. Remove second key (ReferenceChange) + // 4. Remove second key (FeatureMapChange) + // 5. Remove NodeFeatureMapContainment (ReferenceChange) + assertEquals(5, comparison.getDifferences().size()); + + // Also test dependencies by merging the FeatureMapChanges first + Iterator<Diff> featureMapChangesIt = Iterators.filter(differences.iterator(), + instanceOf(FeatureMapChange.class)); + List<Diff> featureMapChanges = Lists.newArrayList(featureMapChangesIt); + assertEquals(2, featureMapChanges.size()); + + // Execute FeatureMapChanges first to test if they properly resolve their dependencies + for (Diff diff : featureMapChanges) { + mergerRegistry.getHighestRankingMerger(diff).copyRightToLeft(diff, new BasicMonitor()); + } + + // Execute the remaining differences + for (Diff diff : differences) { + if (!featureMapChanges.contains(diff)) { + mergerRegistry.getHighestRankingMerger(diff).copyRightToLeft(diff, new BasicMonitor()); + } + } + + // Check if no differences are left + comparison = EMFCompare.builder().build().compare(scope); + assertEquals(0, comparison.getDifferences().size()); + } + + @Test public void testMergeAllDiffsTwice() throws IOException { final IdentifierMatchInputData inputData = new IdentifierMatchInputData(); final Resource left = inputData.getExtlibraryLeft(); diff --git a/plugins/org.eclipse.emf.compare.tests/src/org/eclipse/emf/compare/tests/merge/data/TwoWayMergeInputData.java b/plugins/org.eclipse.emf.compare.tests/src/org/eclipse/emf/compare/tests/merge/data/TwoWayMergeInputData.java index cba60de1c..5f84f8abb 100644 --- a/plugins/org.eclipse.emf.compare.tests/src/org/eclipse/emf/compare/tests/merge/data/TwoWayMergeInputData.java +++ b/plugins/org.eclipse.emf.compare.tests/src/org/eclipse/emf/compare/tests/merge/data/TwoWayMergeInputData.java @@ -8,7 +8,7 @@ * Contributors: * Philip Langer - initial API and implementation * Alexandra Buzila - test data for bug 446252 - * Stefan Dirix - test data for bugs 452147, 453749 and 460902 + * Stefan Dirix - test data for bugs 452147, 453749, 460902 and 460923 *******************************************************************************/ package org.eclipse.emf.compare.tests.merge.data; @@ -115,4 +115,12 @@ public class TwoWayMergeInputData extends AbstractInputData { public Resource getFeatureMapDependencyL2RRight(ResourceSet resourceSet) throws IOException { return loadFromClassLoader("twoway/featuremapdependency/ltr/right.nodes", resourceSet); } + + public Resource getRemoveFeatureMapR2LLeft(ResourceSet resourceSet) throws IOException { + return loadFromClassLoader("twoway/removefeaturemap/rtl/left.nodes", resourceSet); + } + + public Resource getRemoveFeatureMapR2LRight(ResourceSet resourceSet) throws IOException { + return loadFromClassLoader("twoway/removefeaturemap/rtl/right.nodes", resourceSet); + } } diff --git a/plugins/org.eclipse.emf.compare.tests/src/org/eclipse/emf/compare/tests/merge/data/twoway/removefeaturemap/rtl/left.nodes b/plugins/org.eclipse.emf.compare.tests/src/org/eclipse/emf/compare/tests/merge/data/twoway/removefeaturemap/rtl/left.nodes new file mode 100644 index 000000000..41499d0dd --- /dev/null +++ b/plugins/org.eclipse.emf.compare.tests/src/org/eclipse/emf/compare/tests/merge/data/twoway/removefeaturemap/rtl/left.nodes @@ -0,0 +1,9 @@ +<?xml version="1.0" encoding="ASCII"?> +<nodes:Node + xmi:version="2.0" + xmlns:xmi="http://www.omg.org/XMI" + xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" + xmlns:nodes="http://www.eclipse.org/emf/compare/tests/nodes" + xmi:id="_root" + name="Root"> +</nodes:Node> diff --git a/plugins/org.eclipse.emf.compare.tests/src/org/eclipse/emf/compare/tests/merge/data/twoway/removefeaturemap/rtl/right.nodes b/plugins/org.eclipse.emf.compare.tests/src/org/eclipse/emf/compare/tests/merge/data/twoway/removefeaturemap/rtl/right.nodes new file mode 100644 index 000000000..2662fdac6 --- /dev/null +++ b/plugins/org.eclipse.emf.compare.tests/src/org/eclipse/emf/compare/tests/merge/data/twoway/removefeaturemap/rtl/right.nodes @@ -0,0 +1,13 @@ +<?xml version="1.0" encoding="ASCII"?> +<nodes:Node + xmi:version="2.0" + xmlns:xmi="http://www.omg.org/XMI" + xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" + xmlns:nodes="http://www.eclipse.org/emf/compare/tests/nodes" + xmi:id="_root" + name="Root"> + <containmentRef1 xsi:type="nodes:NodeFeatureMapContainment" name="n"> + <secondKey/> + <firstKey/> + </containmentRef1> +</nodes:Node> diff --git a/plugins/org.eclipse.emf.compare/src/org/eclipse/emf/compare/diff/DefaultDiffEngine.java b/plugins/org.eclipse.emf.compare/src/org/eclipse/emf/compare/diff/DefaultDiffEngine.java index fce5da029..00a8a2b28 100644 --- a/plugins/org.eclipse.emf.compare/src/org/eclipse/emf/compare/diff/DefaultDiffEngine.java +++ b/plugins/org.eclipse.emf.compare/src/org/eclipse/emf/compare/diff/DefaultDiffEngine.java @@ -7,7 +7,7 @@ * * Contributors: * Obeo - initial API and implementation - * Stefan Dirix - Bugs 450949 and 453218 + * Stefan Dirix - Bugs 450949, 453218 and 460923 *******************************************************************************/ package org.eclipse.emf.compare.diff; @@ -761,20 +761,14 @@ public class DefaultDiffEngine implements IDiffEngine { } // A value that is in the right but not in the left has been deleted or moved. - - // However, we do not want attribute changes on removed elements and in case of a FeatureMapChange - // of kind DifferenceKind.CHANGE - if (feature instanceof EReference || match.getLeft() != null) { - if (isFeatureMapMoveFromNonFeatureMapContainment(comparison, feature, diffCandidate, - leftValues, DifferenceSource.LEFT)) { - // add move change if the move originates from a non-feature-map containment. - featureChange(match, feature, diffCandidate, DifferenceKind.MOVE, DifferenceSource.LEFT); - } else if (!isFeatureMapChangeOrMove(comparison, feature, diffCandidate, leftValues, - DifferenceSource.LEFT)) { - featureChange(match, feature, diffCandidate, DifferenceKind.DELETE, DifferenceSource.LEFT); - } + if (isFeatureMapMoveFromNonFeatureMapContainment(comparison, feature, diffCandidate, leftValues, + DifferenceSource.LEFT)) { + // add move change if the move originates from a non-feature-map containment. + featureChange(match, feature, diffCandidate, DifferenceKind.MOVE, DifferenceSource.LEFT); + } else if (!isFeatureMapChangeOrMove(comparison, feature, diffCandidate, leftValues, + DifferenceSource.LEFT)) { + featureChange(match, feature, diffCandidate, DifferenceKind.DELETE, DifferenceSource.LEFT); } - } } |