[451365] Removes conflicts and implications of refining diffs

The AbstractChangeFactory propagates conflicts from refining diffs to
refined diffs. For OpaqueElementBodyChanges there is a specific conflict
detection, so conflicts of refining changes are not relevant.
Especially, since the typcial refining diffs often have pseudo
conflicts. If these pseudo conflicts are then propagated to the
OpaqueElementBodyChanges, they will be filtered in the structured diff
viewer.

To address this issue, we now remove all propagated conflicts from
OpaqueElementBodyChanges.

Also, we used the implies relationship to make sure that the refining
changes are handled when the OpaqueElementBodyChanges is merged.
However, this had implications on the user interface of the structure
merge viewer.

To address this issue, we removed those implications but now we have to
remove the refining diffs from the merge dependencies; otherwise, when
the opaque element body change is merged, AbstractMerger would first
invoke the merge of the refining diffs as they are considered as merge
dependencies, which causes the merge of the entire
OpaqueElementBodyChange anyway, and then pursue the merge of the
OpaqueElementBodyChange, so that it would ultimately be performed twice.
Therefore, we remove the refining diffs from the merge dependencies by
overriding #getDirectmergeDependencies(Diff, boolean).

Bug: 451365
Change-Id: Ib61e2564665f83c9d076293706df63e024b29e63
Signed-off-by: Philip Langer <planger@eclipsesource.com>

3 files changed, 50 insertions, 39 deletions

diff --git a/plugins/org.eclipse.emf.compare.uml2.tests/src/org/eclipse/emf/compare/uml2/tests/opaque/OpaqueElementBodyChangeMergeTest.java b/plugins/org.eclipse.emf.compare.uml2.tests/src/org/eclipse/emf/compare/uml2/tests/opaque/OpaqueElementBodyChangeMergeTest.java
index 3f86f6fc7..444da5f97 100644
--- a/plugins/org.eclipse.emf.compare.uml2.tests/src/org/eclipse/emf/compare/uml2/tests/opaque/OpaqueElementBodyChangeMergeTest.java
+++ b/plugins/org.eclipse.emf.compare.uml2.tests/src/org/eclipse/emf/compare/uml2/tests/opaque/OpaqueElementBodyChangeMergeTest.java
@@ -11,9 +11,13 @@
 package org.eclipse.emf.compare.uml2.tests.opaque;
 
 import static com.google.common.base.Predicates.and;
+import static com.google.common.collect.Iterables.any;
 import static com.google.common.collect.Iterables.filter;
+import static com.google.common.collect.Iterables.size;
+import static org.eclipse.emf.compare.DifferenceSource.LEFT;
+import static org.eclipse.emf.compare.DifferenceSource.RIGHT;
+import static org.eclipse.emf.compare.utils.EMFComparePredicates.fromSide;
 import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
 
 import com.google.common.base.Predicate;
@@ -26,7 +30,6 @@ import org.eclipse.emf.compare.Comparison;
 import org.eclipse.emf.compare.Conflict;
 import org.eclipse.emf.compare.ConflictKind;
 import org.eclipse.emf.compare.Diff;
-import org.eclipse.emf.compare.DifferenceSource;
 import org.eclipse.emf.compare.merge.IMerger;
 import org.eclipse.emf.compare.uml2.internal.OpaqueElementBodyChange;
 import org.eclipse.emf.compare.uml2.tests.AbstractUMLInputData;
@@ -103,15 +106,15 @@ public class OpaqueElementBodyChangeMergeTest extends AbstractUMLTest {
 		}
 	};
 
-	private static final Predicate<Diff> IS_RIGHT_SOURCE = new Predicate<Diff>() {
-		public boolean apply(Diff diff) {
-			return DifferenceSource.RIGHT.equals(diff.getSource());
+	private static final Predicate<Conflict> IS_REAL_CONFLICT = new Predicate<Conflict>() {
+		public boolean apply(Conflict conflict) {
+			return ConflictKind.REAL.equals(conflict.getKind());
 		}
 	};
 
-	private static final Predicate<Diff> IS_LEFT_SOURCE = new Predicate<Diff>() {
-		public boolean apply(Diff diff) {
-			return DifferenceSource.LEFT.equals(diff.getSource());
+	private static final Predicate<Conflict> CONCERNS_OPAQUE_ELEMENT_BODY_CHANGE = new Predicate<Conflict>() {
+		public boolean apply(Conflict conflict) {
+			return any(conflict.getDifferences(), IS_OPAQUE_ELEMENT_CHANGE);
 		}
 	};
 
@@ -188,7 +191,7 @@ public class OpaqueElementBodyChangeMergeTest extends AbstractUMLTest {
 		Resource right = input.getA2Right();
 
 		Comparison comparison = compare(left, right, origin);
-		assertOneRealConflict(comparison);
+		assertOneRealConflictOnOpaqueElementBodyChange(comparison);
 	}
 
 	@Test
@@ -198,7 +201,7 @@ public class OpaqueElementBodyChangeMergeTest extends AbstractUMLTest {
 		Resource right = input.getA3Right();
 
 		Comparison comparison = compare(left, right, origin);
-		assertOneRealConflict(comparison);
+		assertOneRealConflictOnOpaqueElementBodyChange(comparison);
 	}
 
 	@Test
@@ -392,7 +395,7 @@ public class OpaqueElementBodyChangeMergeTest extends AbstractUMLTest {
 		Resource right = input.getA7Right();
 
 		Comparison comparison = compare(left, right, origin);
-		assertOneRealConflict(comparison);
+		assertOneRealConflictOnOpaqueElementBodyChange(comparison);
 	}
 
 	@Test
@@ -402,7 +405,7 @@ public class OpaqueElementBodyChangeMergeTest extends AbstractUMLTest {
 		Resource right = input.getA8Right();
 
 		Comparison comparison = compare(left, right, origin);
-		assertOneRealConflict(comparison);
+		assertOneRealConflictOnOpaqueElementBodyChange(comparison);
 	}
 
 	@Test
@@ -476,7 +479,7 @@ public class OpaqueElementBodyChangeMergeTest extends AbstractUMLTest {
 		Resource right = input.getA11Right();
 
 		Comparison comparison = compare(left, right, origin);
-		assertOneRealConflict(comparison);
+		assertOneRealConflictOnOpaqueElementBodyChange(comparison);
 	}
 
 	@Test
@@ -486,7 +489,7 @@ public class OpaqueElementBodyChangeMergeTest extends AbstractUMLTest {
 		Resource right = input.getA12Right();
 
 		Comparison comparison = compare(left, right, origin);
-		assertOneRealConflict(comparison);
+		assertOneRealConflictOnOpaqueElementBodyChange(comparison);
 	}
 
 	@Test
@@ -595,7 +598,7 @@ public class OpaqueElementBodyChangeMergeTest extends AbstractUMLTest {
 	private void applyRightOpaqueElementBodyChangesToLeft(Comparison comparison) {
 		final EList<Diff> allDifferences = comparison.getDifferences();
 		final Iterable<Diff> rightOpaqueElementBodyChanges = filter(allDifferences, and(
-				IS_OPAQUE_ELEMENT_CHANGE, IS_RIGHT_SOURCE));
+				IS_OPAQUE_ELEMENT_CHANGE, fromSide(RIGHT)));
 		for (Diff diff : rightOpaqueElementBodyChanges) {
 			IMerger merger = getMergerRegistry().getHighestRankingMerger(diff);
 			merger.copyRightToLeft(diff, new BasicMonitor());
@@ -605,7 +608,7 @@ public class OpaqueElementBodyChangeMergeTest extends AbstractUMLTest {
 	private void revertLeftOpaqueElementBodyChanges(Comparison comparison) {
 		final EList<Diff> allDifferences = comparison.getDifferences();
 		final Iterable<Diff> leftOpaqueElementBodyChanges = filter(allDifferences, and(
-				IS_OPAQUE_ELEMENT_CHANGE, IS_LEFT_SOURCE));
+				IS_OPAQUE_ELEMENT_CHANGE, fromSide(LEFT)));
 		for (Diff diff : leftOpaqueElementBodyChanges) {
 			IMerger merger = getMergerRegistry().getHighestRankingMerger(diff);
 			merger.copyRightToLeft(diff, new BasicMonitor());
@@ -615,7 +618,7 @@ public class OpaqueElementBodyChangeMergeTest extends AbstractUMLTest {
 	private void revertRightOpaqueElementBodyChanges(Comparison comparison) {
 		final EList<Diff> allDifferences = comparison.getDifferences();
 		final Iterable<Diff> rightOpaqueElementBodyChanges = filter(allDifferences, and(
-				IS_OPAQUE_ELEMENT_CHANGE, IS_RIGHT_SOURCE));
+				IS_OPAQUE_ELEMENT_CHANGE, fromSide(RIGHT)));
 		for (Diff diff : rightOpaqueElementBodyChanges) {
 			IMerger merger = getMergerRegistry().getHighestRankingMerger(diff);
 			merger.copyLeftToRight(diff, new BasicMonitor());
@@ -625,7 +628,7 @@ public class OpaqueElementBodyChangeMergeTest extends AbstractUMLTest {
 	private void applyLeftOpaqueElementBodyChangesToRight(Comparison comparison) {
 		final EList<Diff> allDifferences = comparison.getDifferences();
 		final Iterable<Diff> leftOpaqueElementBodyChanges = filter(allDifferences, and(
-				IS_OPAQUE_ELEMENT_CHANGE, IS_LEFT_SOURCE));
+				IS_OPAQUE_ELEMENT_CHANGE, fromSide(LEFT)));
 		for (Diff diff : leftOpaqueElementBodyChanges) {
 			IMerger merger = getMergerRegistry().getHighestRankingMerger(diff);
 			merger.copyLeftToRight(diff, new BasicMonitor());
@@ -633,23 +636,14 @@ public class OpaqueElementBodyChangeMergeTest extends AbstractUMLTest {
 	}
 
 	private void assertNoRealConflict(Comparison comparison) {
-		for (Conflict conflict : comparison.getConflicts()) {
-			assertFalse(isRealConflict(conflict));
-		}
-	}
-
-	private void assertOneRealConflict(Comparison comparison) {
-		int numberOfRealConflicts = 0;
-		for (Conflict conflict : comparison.getConflicts()) {
-			if (isRealConflict(conflict)) {
-				numberOfRealConflicts++;
-			}
-		}
-		assertEquals(1, numberOfRealConflicts);
+		assertEquals(0, size(filter(comparison.getConflicts(), IS_REAL_CONFLICT)));
+		assertEquals(0, size(filter(comparison.getConflicts(), CONCERNS_OPAQUE_ELEMENT_BODY_CHANGE)));
 	}
 
-	private boolean isRealConflict(Conflict conflict) {
-		return ConflictKind.REAL.equals(conflict.getKind());
+	private void assertOneRealConflictOnOpaqueElementBodyChange(Comparison comparison) {
+		assertEquals(1, size(filter(comparison.getConflicts(), IS_REAL_CONFLICT)));
+		assertEquals(1, size(filter(comparison.getConflicts(), and(IS_REAL_CONFLICT,
+				CONCERNS_OPAQUE_ELEMENT_BODY_CHANGE))));
 	}
 
 	@Override
diff --git a/plugins/org.eclipse.emf.compare.uml2/src/org/eclipse/emf/compare/uml2/internal/merge/OpaqueElementBodyChangeMerger.java b/plugins/org.eclipse.emf.compare.uml2/src/org/eclipse/emf/compare/uml2/internal/merge/OpaqueElementBodyChangeMerger.java
index c53ba5045..2311c3b18 100644
--- a/plugins/org.eclipse.emf.compare.uml2/src/org/eclipse/emf/compare/uml2/internal/merge/OpaqueElementBodyChangeMerger.java
+++ b/plugins/org.eclipse.emf.compare.uml2/src/org/eclipse/emf/compare/uml2/internal/merge/OpaqueElementBodyChangeMerger.java
@@ -13,6 +13,7 @@ package org.eclipse.emf.compare.uml2.internal.merge;
 import com.google.common.base.Optional;
 
 import java.util.List;
+import java.util.Set;
 
 import org.eclipse.emf.compare.AttributeChange;
 import org.eclipse.emf.compare.Comparison;
@@ -32,9 +33,9 @@ import org.eclipse.emf.ecore.EObject;
  * OpaqueExpressions.
  * <p>
  * This merger handles all {@link Diff differences} that are either {@link OpaqueElementBodyChange opaque
- * element body changes} itself or that are AttributeChanges refining {@link OpaqueElementBodyChange opaque
- * element body changes}. Note that this merger forces the merging of the entire
- * {@link OpaqueElementBodyChange}, also if it is only invoked for one {@link AttributeChange} that refines a
+ * element body changes} themselves or that are {@link AttributeChange attribute changes} refining opaque
+ * element body changes. Note that this merger forces the merging of the entire
+ * {@link OpaqueElementBodyChange}, also if it is invoked only for one {@link AttributeChange} that refines a
  * {@link OpaqueElementBodyChange}.
  * </p>
  * 
@@ -398,7 +399,6 @@ public class OpaqueElementBodyChangeMerger extends AttributeChangeMerger {
 
 		move(languages, languageValueToMove, insertionIndex);
 		move(bodies, bodyValueToMove, insertionIndex);
-
 	}
 
 	/**
@@ -432,4 +432,16 @@ public class OpaqueElementBodyChangeMerger extends AttributeChangeMerger {
 		}
 	}
 
+	@Override
+	public Set<Diff> getDirectMergeDependencies(Diff diff, boolean mergeRightToLeft) {
+		/*
+		 * We take care of merging the refining diffs in this merger anyway, so we remove them from the direct
+		 * merge dependencies to avoid being called for them again before we even have completed the merge of
+		 * the body change itself.
+		 */
+		Set<Diff> dependencies = super.getDirectMergeDependencies(diff, mergeRightToLeft);
+		dependencies.removeAll(diff.getRefinedBy());
+		return dependencies;
+	}
+
 }
diff --git a/plugins/org.eclipse.emf.compare.uml2/src/org/eclipse/emf/compare/uml2/internal/postprocessor/extension/UMLOpaqueElementBodyChangeFactory.java b/plugins/org.eclipse.emf.compare.uml2/src/org/eclipse/emf/compare/uml2/internal/postprocessor/extension/UMLOpaqueElementBodyChangeFactory.java
index c13d10a2e..22945f373 100644
--- a/plugins/org.eclipse.emf.compare.uml2/src/org/eclipse/emf/compare/uml2/internal/postprocessor/extension/UMLOpaqueElementBodyChangeFactory.java
+++ b/plugins/org.eclipse.emf.compare.uml2/src/org/eclipse/emf/compare/uml2/internal/postprocessor/extension/UMLOpaqueElementBodyChangeFactory.java
@@ -264,12 +264,17 @@ public class UMLOpaqueElementBodyChangeFactory extends AbstractUMLChangeFactory
 	@Override
 	public Diff create(Diff input) {
 		final OpaqueElementBodyChange extension = (OpaqueElementBodyChange)super.create(input);
+
 		// getAffectedLanguage must yield a value at this point, otherwise we wouldn't have
 		// returned true when handle was called
 		extension.setLanguage(getAffectedLanguage((AttributeChange)input).get());
 		extension.setKind(computeDifferenceKind(extension));
-		extension.setSource(input.getSource());
-		extension.getImplies().addAll(extension.getRefinedBy());
+
+		// remove conflict from extension that has been inferred from refined differences (cf.
+		// AbstractChangeFactory#create()) because we add specific conflicts for OpaqueElementBodyChanges
+		// later in the post processor
+		extension.setConflict(null);
+
 		return extension;
 	}