[505950] Fix filter not hiding directly pseudo-conflicting diffs

Bug: 505950
Change-Id: I21b12daa33ec8b536bf49da2337c54e2890be1fe
Signed-off-by: Mathieu Cartaud <mathieu.cartaud@obeo.fr>

9 files changed, 146 insertions, 27 deletions

diff --git a/plugins/org.eclipse.emf.compare.rcp.ui.tests/META-INF/MANIFEST.MF b/plugins/org.eclipse.emf.compare.rcp.ui.tests/META-INF/MANIFEST.MF
index 172d73c65..db1261ff5 100644
--- a/plugins/org.eclipse.emf.compare.rcp.ui.tests/META-INF/MANIFEST.MF
+++ b/plugins/org.eclipse.emf.compare.rcp.ui.tests/META-INF/MANIFEST.MF
@@ -12,7 +12,8 @@ Require-Bundle: org.eclipse.core.runtime,
  org.eclipse.emf.compare.tests;bundle-version="2.1.0",
  org.eclipse.emf.compare.rcp;bundle-version="2.1.0",
  org.eclipse.emf.compare.rcp.ui;bundle-version="3.0.0",
- org.eclipse.jface
+ org.eclipse.jface,
+ org.eclipse.emf.compare.ide.ui.tests.framework
 Bundle-RequiredExecutionEnvironment: J2SE-1.5
 Bundle-Vendor: %providerName
 Import-Package: com.google.common.base;version="[11.0.0,16.0.0)",
diff --git a/plugins/org.eclipse.emf.compare.rcp.ui.tests/src/org/eclipse/emf/compare/rcp/ui/tests/structuremergeviewer/filters/TechnicalitiesFilterTests.java b/plugins/org.eclipse.emf.compare.rcp.ui.tests/src/org/eclipse/emf/compare/rcp/ui/tests/structuremergeviewer/filters/TechnicalitiesFilterTests.java
new file mode 100644
index 000000000..9941219de
--- /dev/null
+++ b/plugins/org.eclipse.emf.compare.rcp.ui.tests/src/org/eclipse/emf/compare/rcp/ui/tests/structuremergeviewer/filters/TechnicalitiesFilterTests.java
@@ -0,0 +1,86 @@
+/*******************************************************************************
+ * Copyright (c) 2016 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.rcp.ui.tests.structuremergeviewer.filters;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+import java.io.IOException;
+
+import org.eclipse.emf.compare.Comparison;
+import org.eclipse.emf.compare.ide.ui.tests.framework.RuntimeTestRunner;
+import org.eclipse.emf.compare.ide.ui.tests.framework.annotations.Compare;
+import org.eclipse.emf.compare.rcp.ui.internal.structuremergeviewer.filters.impl.TechnicalitiesFilter;
+import org.eclipse.emf.compare.rcp.ui.internal.structuremergeviewer.groups.impl.ThreeWayComparisonGroupProvider;
+import org.eclipse.emf.compare.rcp.ui.internal.structuremergeviewer.groups.impl.ThreeWayComparisonGroupProvider.ConflictsGroupImpl;
+import org.eclipse.emf.ecore.EObject;
+import org.eclipse.emf.ecore.EReference;
+import org.eclipse.emf.ecore.util.ECrossReferenceAdapter;
+import org.eclipse.emf.edit.tree.TreeNode;
+import org.eclipse.emf.edit.tree.TreePackage;
+import org.junit.runner.RunWith;
+
+import com.google.common.base.Predicate;
+
+@SuppressWarnings("restriction")
+@RunWith(RuntimeTestRunner.class)
+public class TechnicalitiesFilterTests {
+
+	/**
+	 * This test checks that the following type of pseudo conflicts is filtered by the Technicalities filter.
+	 * 
+	 * <pre>
+	 * Ancestor: - root
+	 * 				- class1
+	 * 					- attribute1
+	 * 
+	 * Left: - root
+	 * 
+	 * Right: - root
+	 * 			- class1
+	 * </pre>
+	 * 
+	 * @throws IOException
+	 */
+	@Compare(left = "data/predicates/technicalities/directPseudoConflict/left.ecore", right = "data/predicates/technicalities/directPseudoConflict/right.ecore", ancestor = "data/predicates/technicalities/directPseudoConflict/ancestor.ecore")
+	public void testDirectPseudoConflictingDiffFilter(Comparison comparison) throws IOException {
+		ConflictsGroupImpl group = new ThreeWayComparisonGroupProvider.ConflictsGroupImpl(comparison,
+				"Conflict", new TestECrossReferenceAdapter());
+		group.buildSubTree();
+		assertEquals(1, group.getChildren().size());
+
+		TreeNode conflictNode = group.getChildren().get(0);
+		assertEquals(1, conflictNode.getChildren().size());
+
+		TreeNode matchNode = conflictNode.getChildren().get(0);
+		assertEquals(2, matchNode.getChildren().size());
+
+		TechnicalitiesFilter filter = new TechnicalitiesFilter();
+		Predicate<? super EObject> selected = filter.getPredicateWhenSelected();
+		Predicate<? super EObject> unselected = filter.getPredicateWhenUnselected();
+
+		TreeNode diffNode1 = matchNode.getChildren().get(0);
+		assertTrue(selected.apply(diffNode1));
+		assertFalse(unselected.apply(diffNode1));
+
+		TreeNode diffNode2 = matchNode.getChildren().get(1);
+		assertTrue(selected.apply(diffNode2));
+		assertFalse(unselected.apply(diffNode2));
+	}
+
+	private class TestECrossReferenceAdapter extends ECrossReferenceAdapter {
+		@Override
+		protected boolean isIncluded(EReference eReference) {
+			return eReference == TreePackage.Literals.TREE_NODE__DATA;
+		}
+	}
+}
diff --git a/plugins/org.eclipse.emf.compare.rcp.ui.tests/src/org/eclipse/emf/compare/rcp/ui/tests/structuremergeviewer/filters/data/predicates/technicalities/directPseudoConflict/ancestor.ecore b/plugins/org.eclipse.emf.compare.rcp.ui.tests/src/org/eclipse/emf/compare/rcp/ui/tests/structuremergeviewer/filters/data/predicates/technicalities/directPseudoConflict/ancestor.ecore
new file mode 100644
index 000000000..2d1ac9c5f
--- /dev/null
+++ b/plugins/org.eclipse.emf.compare.rcp.ui.tests/src/org/eclipse/emf/compare/rcp/ui/tests/structuremergeviewer/filters/data/predicates/technicalities/directPseudoConflict/ancestor.ecore
@@ -0,0 +1,7 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<ecore:EPackage xmi:version="2.0" xmlns:xmi="http://www.omg.org/XMI" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+    xmlns:ecore="http://www.eclipse.org/emf/2002/Ecore" name="root" nsURI="root" nsPrefix="root">
+  <eClassifiers xsi:type="ecore:EClass" name="Class1">
+    <eStructuralFeatures xsi:type="ecore:EAttribute" name="attribute1"/>
+  </eClassifiers>
+</ecore:EPackage>
diff --git a/plugins/org.eclipse.emf.compare.rcp.ui.tests/src/org/eclipse/emf/compare/rcp/ui/tests/structuremergeviewer/filters/data/predicates/technicalities/directPseudoConflict/left.ecore b/plugins/org.eclipse.emf.compare.rcp.ui.tests/src/org/eclipse/emf/compare/rcp/ui/tests/structuremergeviewer/filters/data/predicates/technicalities/directPseudoConflict/left.ecore
new file mode 100644
index 000000000..96e3d2312
--- /dev/null
+++ b/plugins/org.eclipse.emf.compare.rcp.ui.tests/src/org/eclipse/emf/compare/rcp/ui/tests/structuremergeviewer/filters/data/predicates/technicalities/directPseudoConflict/left.ecore
@@ -0,0 +1,3 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<ecore:EPackage xmi:version="2.0" xmlns:xmi="http://www.omg.org/XMI" xmlns:ecore="http://www.eclipse.org/emf/2002/Ecore"
+    name="root" nsURI="root" nsPrefix="root"/>
diff --git a/plugins/org.eclipse.emf.compare.rcp.ui.tests/src/org/eclipse/emf/compare/rcp/ui/tests/structuremergeviewer/filters/data/predicates/technicalities/directPseudoConflict/right.ecore b/plugins/org.eclipse.emf.compare.rcp.ui.tests/src/org/eclipse/emf/compare/rcp/ui/tests/structuremergeviewer/filters/data/predicates/technicalities/directPseudoConflict/right.ecore
new file mode 100644
index 000000000..38398404a
--- /dev/null
+++ b/plugins/org.eclipse.emf.compare.rcp.ui.tests/src/org/eclipse/emf/compare/rcp/ui/tests/structuremergeviewer/filters/data/predicates/technicalities/directPseudoConflict/right.ecore
@@ -0,0 +1,5 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<ecore:EPackage xmi:version="2.0" xmlns:xmi="http://www.omg.org/XMI" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+    xmlns:ecore="http://www.eclipse.org/emf/2002/Ecore" name="root" nsURI="root" nsPrefix="root">
+  <eClassifiers xsi:type="ecore:EClass" name="Class1"/>
+</ecore:EPackage>
diff --git a/plugins/org.eclipse.emf.compare.rcp.ui.tests/src/org/eclipse/emf/compare/rcp/ui/tests/structuremergeviewer/groups/ConflictsGroupTest.java b/plugins/org.eclipse.emf.compare.rcp.ui.tests/src/org/eclipse/emf/compare/rcp/ui/tests/structuremergeviewer/groups/ConflictsGroupTest.java
index b3947caf2..f5cc65a29 100644
--- a/plugins/org.eclipse.emf.compare.rcp.ui.tests/src/org/eclipse/emf/compare/rcp/ui/tests/structuremergeviewer/groups/ConflictsGroupTest.java
+++ b/plugins/org.eclipse.emf.compare.rcp.ui.tests/src/org/eclipse/emf/compare/rcp/ui/tests/structuremergeviewer/groups/ConflictsGroupTest.java
@@ -350,9 +350,8 @@ public class ConflictsGroupTest extends AbstractTestTreeNodeItemProviderAdapter
 
 	/**
 	 * Tests that a refined diff that is refined by one diff involved in a pseudo conflict
-	 * {@link ConflictKind#PSEUDO} and one diff involved in no conflict is shown in the conflicts group and in
-	 * the left or right diff group. This test is related to <a
-	 * href="https://bugs.eclipse.org/bugs/show_bug.cgi?id=501864"</a>
+	 * {@link ConflictKind#PSEUDO} and one diff involved in no conflict is shown only in the side group. This
+	 * test is related to <a href="https://bugs.eclipse.org/bugs/show_bug.cgi?id=501864"</a>
 	 */
 	@Test
 	public void testRefinedDiffsWithOnlyOnePseudoConflictAreInDiffGroupOnly() {
@@ -403,7 +402,7 @@ public class ConflictsGroupTest extends AbstractTestTreeNodeItemProviderAdapter
 		assertEquals(1, diffNodesLeftFiltered.size());
 		final List<? extends TreeNode> subDiffNodesLeftFiltered = applyTechnicalitiesFilter(
 				diffNodesLeftFiltered.get(0).getChildren());
-		assertEquals(2, subDiffNodesLeftFiltered.size());
+		assertEquals(1, subDiffNodesLeftFiltered.size());
 
 		// Build right side
 		final BasicDifferenceGroupImpl rightSide = buildDifferenceGroup(scenario.comparison,
@@ -422,7 +421,7 @@ public class ConflictsGroupTest extends AbstractTestTreeNodeItemProviderAdapter
 		assertEquals(1, diffNodesRightFiltered.size());
 		final List<? extends TreeNode> subDiffNodesRightFiltered = applyTechnicalitiesFilter(
 				diffNodesRightFiltered.get(0).getChildren());
-		assertEquals(2, subDiffNodesRightFiltered.size());
+		assertEquals(1, subDiffNodesRightFiltered.size());
 	}
 
 	/**
diff --git a/plugins/org.eclipse.emf.compare.rcp.ui.tests/src/org/eclipse/emf/compare/rcp/ui/tests/suite/AllTests.java b/plugins/org.eclipse.emf.compare.rcp.ui.tests/src/org/eclipse/emf/compare/rcp/ui/tests/suite/AllTests.java
index 6ec0dfeaa..00b5af1a6 100644
--- a/plugins/org.eclipse.emf.compare.rcp.ui.tests/src/org/eclipse/emf/compare/rcp/ui/tests/suite/AllTests.java
+++ b/plugins/org.eclipse.emf.compare.rcp.ui.tests/src/org/eclipse/emf/compare/rcp/ui/tests/suite/AllTests.java
@@ -18,6 +18,7 @@ import org.eclipse.emf.compare.rcp.ui.tests.match.RCPMatchEngineFactoryRegistryT
 import org.eclipse.emf.compare.rcp.ui.tests.mergeviewer.item.MergeViewerItemFeatureMapsTest;
 import org.eclipse.emf.compare.rcp.ui.tests.mergeviewer.item.MergeViewerItemPseudoConflictTest;
 import org.eclipse.emf.compare.rcp.ui.tests.mergeviewer.item.MergeViewerItemTest;
+import org.eclipse.emf.compare.rcp.ui.tests.structuremergeviewer.filters.TechnicalitiesFilterTests;
 import org.eclipse.emf.compare.rcp.ui.tests.structuremergeviewer.filters.TestFeatureMapDifferencesFilter;
 import org.eclipse.emf.compare.rcp.ui.tests.structuremergeviewer.groups.ConflictsGroupTest;
 import org.eclipse.emf.compare.rcp.ui.tests.structuremergeviewer.groups.TestBasicDifferenceGroupImpl;
@@ -44,7 +45,7 @@ import junit.textui.TestRunner;
 		MergeViewerItemPseudoConflictTest.class, MergeViewerItemFeatureMapsTest.class,
 		TestBasicDifferenceGroupImpl.class, BugsTestSuite.class, TestFeatureMapDifferencesFilter.class,
 		RCPMatchEngineFactoryRegistryTest.class, ThreeWayComparisonGroupProviderTest.class,
-		ConflictsGroupTest.class, MatchAccessorTest.class, })
+		ConflictsGroupTest.class, MatchAccessorTest.class, TechnicalitiesFilterTests.class, })
 public class AllTests {
 	/**
 	 * Launches the test with the given arguments.
diff --git a/plugins/org.eclipse.emf.compare.rcp.ui/src/org/eclipse/emf/compare/rcp/ui/internal/structuremergeviewer/filters/impl/TechnicalitiesFilter.java b/plugins/org.eclipse.emf.compare.rcp.ui/src/org/eclipse/emf/compare/rcp/ui/internal/structuremergeviewer/filters/impl/TechnicalitiesFilter.java
index 2fcaa5c5a..c660a761e 100644
--- a/plugins/org.eclipse.emf.compare.rcp.ui/src/org/eclipse/emf/compare/rcp/ui/internal/structuremergeviewer/filters/impl/TechnicalitiesFilter.java
+++ b/plugins/org.eclipse.emf.compare.rcp.ui/src/org/eclipse/emf/compare/rcp/ui/internal/structuremergeviewer/filters/impl/TechnicalitiesFilter.java
@@ -14,11 +14,7 @@ package org.eclipse.emf.compare.rcp.ui.internal.structuremergeviewer.filters.imp
 
 import static com.google.common.base.Predicates.or;
 import static com.google.common.collect.Iterators.any;
-import static org.eclipse.emf.compare.ConflictKind.PSEUDO;
-import static org.eclipse.emf.compare.ConflictKind.REAL;
-import static org.eclipse.emf.compare.utils.EMFComparePredicates.allAtomicRefining;
-import static org.eclipse.emf.compare.utils.EMFComparePredicates.hasConflict;
-import static org.eclipse.emf.compare.utils.EMFComparePredicates.hasNoDirectOrIndirectConflict;
+import static org.eclipse.emf.compare.utils.EMFComparePredicates.canBeConsideredAsPseudoConflicting;
 
 import com.google.common.base.Predicate;
 
@@ -87,7 +83,7 @@ public class TechnicalitiesFilter extends AbstractDifferenceFilter {
 				if (data instanceof Diff) {
 					Diff diff = (Diff)data;
 					if (diff.getMatch().getComparison().isThreeWay()) {
-						ret = isConsideredAsPseudoConflicting(diff);
+						ret = canBeConsideredAsPseudoConflicting().apply(diff);
 					}
 				}
 			}
@@ -96,20 +92,6 @@ public class TechnicalitiesFilter extends AbstractDifferenceFilter {
 	};
 
 	/**
-	 * Specifies whether the given diff can be considered as part of a pseudo conflict, but not of a real
-	 * conflict.
-	 * 
-	 * @param diff
-	 *            The diff to check.
-	 * @return <code>true</code> if it has a direct pseudo-conflict or of all its atomic refining diffs are in
-	 *         pseudo-conflict.
-	 */
-	private static boolean isConsideredAsPseudoConflicting(Diff diff) {
-		return allAtomicRefining(hasConflict(PSEUDO)).apply(diff)
-				&& hasNoDirectOrIndirectConflict(REAL).apply(diff);
-	}
-
-	/**
 	 * The predicate use to filter identical elements.
 	 */
 	private static final Predicate<? super EObject> PREDICATE_IDENTICAL_ELEMENTS = new Predicate<EObject>() {
diff --git a/plugins/org.eclipse.emf.compare/src/org/eclipse/emf/compare/utils/EMFComparePredicates.java b/plugins/org.eclipse.emf.compare/src/org/eclipse/emf/compare/utils/EMFComparePredicates.java
index 1c480d426..070ecd6c2 100644
--- a/plugins/org.eclipse.emf.compare/src/org/eclipse/emf/compare/utils/EMFComparePredicates.java
+++ b/plugins/org.eclipse.emf.compare/src/org/eclipse/emf/compare/utils/EMFComparePredicates.java
@@ -19,6 +19,7 @@ import static com.google.common.base.Predicates.not;
 import static com.google.common.base.Predicates.or;
 import static com.google.common.collect.Iterables.all;
 import static com.google.common.collect.Iterables.any;
+import static org.eclipse.emf.compare.ConflictKind.PSEUDO;
 import static org.eclipse.emf.compare.ConflictKind.REAL;
 import static org.eclipse.emf.compare.DifferenceKind.ADD;
 import static org.eclipse.emf.compare.internal.utils.ComparisonUtil.isDeleteOrUnsetDiff;
@@ -1200,6 +1201,40 @@ public final class EMFComparePredicates {
 	}
 
 	/**
+	 * Check whether a diff is not refined and has a direct conflict of (one of) the given type(s).
+	 * 
+	 * @param kinds
+	 *            Type(s) of the conflict(s) we seek.
+	 * @return The created predicate.
+	 * @since 3.4
+	 */
+	public static Predicate<Diff> isNotRefinedDirectlyConflicting(final ConflictKind... kinds) {
+		return new Predicate<Diff>() {
+			public boolean apply(Diff input) {
+				return input != null && input.getConflict() != null && input.getRefinedBy().isEmpty()
+						&& Arrays.asList(kinds).contains(input.getConflict().getKind());
+			}
+		};
+	}
+
+	/**
+	 * This predicate can be used to test if a diff is in a pseudo conflict. Several cases are possible:
+	 * 
+	 * <pre>
+	 * - if the diff is not a refined diff and has a direct pseudo conflict (i.e. diff.getConflict = a pseudo conflict)
+	 * - if the diff is a refined diff and all its refining diffs are in pseudo conflicts.
+	 * 		The refining diffs must not be part of a real conflict  directly or indirectly
+	 * </pre>
+	 * 
+	 * @return the predicate
+	 * @since 3.4
+	 */
+	public static Predicate<Diff> canBeConsideredAsPseudoConflicting() {
+		return or(isNotRefinedDirectlyConflicting(PSEUDO),
+				and(allAtomicRefining(hasConflict(PSEUDO)), hasNoDirectOrIndirectConflict(REAL)));
+	}
+
+	/**
 	 * This predicate can be used to check whether a diff is in a conflict directly or indirectly.
 	 * <p>
 	 * A diff is directly in a conflict if it {@link #hasConflict(ConflictKind...) has a conflict}. A diff is