[Fix] DefaultPriorityHandler now takes the artifact type into account
This commit fixes a long-standing bug that caused Eclipse Capra to select
the incorrect ArtifactHandler in certain situations. Essentially, up until
now, the DefaultPriorityHandler created a hierarchy of ArtifactHandlers
and always selected the one with the highest priority, independent of the
concrete artifact that was handled. This could lead to situations where
an incorrect ArtifactHandler was selected. With the new commit, the
DefaultPriorityHandler only returns an ArtifactHandler that fits the
artifact. The test cases test for this now.
Change-Id: I6a422552a7dd2ac650a603cc7134efa639a2a11d
diff --git a/bundles/org.eclipse.capra.generic.priority/src/org/eclipse/capra/generic/priority/DefaultPriorityHandler.java b/bundles/org.eclipse.capra.generic.priority/src/org/eclipse/capra/generic/priority/DefaultPriorityHandler.java
index e205c8a..859b705 100644
--- a/bundles/org.eclipse.capra.generic.priority/src/org/eclipse/capra/generic/priority/DefaultPriorityHandler.java
+++ b/bundles/org.eclipse.capra.generic.priority/src/org/eclipse/capra/generic/priority/DefaultPriorityHandler.java
@@ -30,15 +30,15 @@
@Override
public <T> IArtifactHandler<? extends T> getSelectedHandler(
Collection<? extends IArtifactHandler<? extends T>> handlers, Object artifact) {
- return handlers.stream().max(new ArtifactHandlerPriorityComparator()).get();
+ return handlers.stream().filter(handler -> handler.canHandleArtifact(artifact))
+ .max(new ArtifactHandlerPriorityComparator()).get();
}
/**
* A comparator that compares two classes by whether the handled classes are
- * assignable to each other. If instance A is of a type that is a superclass
- * or superinterface of instance B, it will return A>B. A=B if the
- * assignment works in both directions or if the assignment does not work at
- * all.
+ * assignable to each other. If instance A is of a type that is a superclass or
+ * superinterface of instance B, it will return A>B. A=B if the assignment works
+ * in both directions or if the assignment does not work at all.
*/
private class ArtifactHandlerPriorityComparator implements Comparator<IArtifactHandler<?>> {
diff --git a/tests/org.eclipse.capra.testsuite/META-INF/MANIFEST.MF b/tests/org.eclipse.capra.testsuite/META-INF/MANIFEST.MF
index b75e66f..03740a5 100644
--- a/tests/org.eclipse.capra.testsuite/META-INF/MANIFEST.MF
+++ b/tests/org.eclipse.capra.testsuite/META-INF/MANIFEST.MF
@@ -27,6 +27,7 @@
org.eclipse.jface,
org.eclipse.osgi,
org.eclipse.swt,
+ org.eclipse.mylyn.builds.core,
org.eclipse.capra.generic.artifactmodel,
org.eclipse.capra.generic.tracemodel,
org.eclipse.capra.generic.priority,
diff --git a/tests/org.eclipse.capra.testsuite/src/org/eclipse/capra/testsuite/TestDefaultPriorityHandler.java b/tests/org.eclipse.capra.testsuite/src/org/eclipse/capra/testsuite/TestDefaultPriorityHandler.java
index 980a94b..8e6a762 100644
--- a/tests/org.eclipse.capra.testsuite/src/org/eclipse/capra/testsuite/TestDefaultPriorityHandler.java
+++ b/tests/org.eclipse.capra.testsuite/src/org/eclipse/capra/testsuite/TestDefaultPriorityHandler.java
@@ -13,12 +13,19 @@
*******************************************************************************/
package org.eclipse.capra.testsuite;
+import static org.junit.Assert.assertEquals;
+
import java.util.ArrayList;
import java.util.Collection;
import org.eclipse.capra.core.handlers.IArtifactHandler;
import org.eclipse.capra.core.handlers.PriorityHandler;
import org.eclipse.capra.core.helpers.ExtensionPointHelper;
+import org.eclipse.capra.handler.hudson.BuildElementHandler;
+import org.eclipse.capra.handler.hudson.TestElementHandler;
+import org.eclipse.mylyn.builds.core.IBuildElement;
+import org.eclipse.mylyn.builds.core.ITestCase;
+import org.eclipse.mylyn.builds.internal.core.BuildFactory;
import org.junit.Assert;
import org.junit.Before;
import org.junit.Test;
@@ -31,6 +38,12 @@
private PriorityHandler priorityHandler;
+ @SuppressWarnings("restriction")
+ private IBuildElement buildElement = BuildFactory.eINSTANCE.createBuildPlan();
+
+ @SuppressWarnings("restriction")
+ private ITestCase testCase = BuildFactory.eINSTANCE.createTestCase();
+
@Before
public void setup() {
priorityHandler = ExtensionPointHelper.getPriorityHandler().get();
@@ -42,28 +55,34 @@
}
@Test
+ public void testCorrectSelection() {
+ assertEquals(BuildElementHandler.class, priorityHandler
+ .getSelectedHandler(ExtensionPointHelper.getArtifactHandlers(), buildElement).getClass());
+ assertEquals(TestElementHandler.class,
+ priorityHandler.getSelectedHandler(ExtensionPointHelper.getArtifactHandlers(), testCase).getClass());
+ }
+
+ @Test
public void testPrioritiesOrder() {
Collection<IArtifactHandler<?>> handlers = new ArrayList<>();
handlers.add(emfHandler);
handlers.add(buildElementHandler);
- Assert.assertEquals(buildElementHandler, priorityHandler.getSelectedHandler(handlers, null));
+ // First, without the testCaseHandler, test case is picked up by EMF (fallback)
+ Assert.assertEquals(buildElementHandler, priorityHandler.getSelectedHandler(handlers, buildElement));
+ Assert.assertEquals(emfHandler, priorityHandler.getSelectedHandler(handlers, testCase));
+
+ // Now, we add the testCaseHandler and we should see a change in priority
+ handlers.add(testElementHandler);
+ Assert.assertEquals(testElementHandler, priorityHandler.getSelectedHandler(handlers, testCase));
// Turn order around
handlers.clear();
- handlers.add(buildElementHandler);
- handlers.add(emfHandler);
- Assert.assertEquals(buildElementHandler, priorityHandler.getSelectedHandler(handlers, null));
- }
-
- @Test
- public void testPrioritiesConflict() {
- Collection<IArtifactHandler<?>> handlers = new ArrayList<>();
handlers.add(testElementHandler);
handlers.add(buildElementHandler);
- // PriorityHandler should choose first element in list in case of a
- // conflict
- Assert.assertEquals(testElementHandler, priorityHandler.getSelectedHandler(handlers, null));
+ handlers.add(emfHandler);
+ Assert.assertEquals(buildElementHandler, priorityHandler.getSelectedHandler(handlers, buildElement));
+ Assert.assertEquals(testElementHandler, priorityHandler.getSelectedHandler(handlers, testCase));
}
}
