[ServiceExtension] Sanity check InjectService.service()

Fixes #495 Signed-off-by: Fr Jeremy Krieg <fr.jkrieg@greekwelfaresa.org.au>
eclipse-osgi-technology · Mar 29, 2022 · dd03f92 · dd03f92
1 parent 0f9b54f
commit dd03f92
Show file tree

Hide file tree

Showing 2 changed files with 80 additions and 34 deletions.
diff --git a/org.osgi.test.junit5/src/main/java/org/osgi/test/junit5/service/ServiceExtension.java b/org.osgi.test.junit5/src/main/java/org/osgi/test/junit5/service/ServiceExtension.java
@@ -64,6 +64,11 @@ public ServiceExtension() {
 	@Override
 	protected boolean supportsType(TargetType targetType, ExtensionContext extensionContext)
 		throws ParameterResolutionException {
+		extractValueType(targetType);
+		return true;
+	}
+
+	private Class<?> extractValueType(TargetType targetType) {
 		Type serviceType;
 		if (targetType.matches(List.class) || targetType.matches(ServiceAware.class)) {
 			if (targetType.hasParameterizedTypes()) {
@@ -78,42 +83,31 @@ protected boolean supportsType(TargetType targetType, ExtensionContext extension
 		} else {
 			serviceType = targetType.getGenericType();
 		}
-
 		if (!(serviceType instanceof Class)) {
 			throw new ParameterResolutionException(String.format(
 				"Element %s has an unsupported type %s for annotation @%s. Service must have non-generic type.",
 				targetType.getName(), serviceType.getTypeName(), annotation().getSimpleName()));
 		}
 
-		return true;
+		return (Class<?>) serviceType;
 	}
 
 	@Override
 	protected Object resolveValue(TargetType targetType, InjectService injectService, ExtensionContext extensionContext)
 		throws ParameterResolutionException {
-		Type serviceType = injectService.service();
-		if (serviceType.equals(annotation())) {
-			if (targetType.matches(List.class) || targetType.matches(ServiceAware.class)) {
-				if (targetType.hasParameterizedTypes()) {
-					serviceType = targetType.getFirstGenericTypes()
-						.get();
-					if (serviceType instanceof WildcardType) {
-						serviceType = Object.class;
-					}
-				} else {
-					serviceType = Object.class;
-				}
-			} else {
-				serviceType = targetType.getType();
-			}
-		}
+		final Class<?> valueClass = extractValueType(targetType);
+		final Class<?> serviceClass = injectService.service();
 
-		if (!(serviceType instanceof Class)) {
+		Type serviceType;
+		if (serviceClass.equals(annotation())) {
+			serviceType = valueClass;
+		} else if (!valueClass.isAssignableFrom(serviceClass)) {
 			throw new ParameterResolutionException(String.format(
-				"Element %s has an unsupported type %s for annotation @%s. Service must have non-generic type.",
-				targetType.getName(), serviceType.getTypeName(), annotation().getSimpleName()));
+				"Element %s has service type %s for annotation @%s but field expects %s.", targetType.getName(),
+				serviceClass.getName(), annotation().getSimpleName(), valueClass.getTypeName()));
+		} else {
+			serviceType = serviceClass;
 		}
-
 		ServiceConfiguration<?> configuration = getServiceConfiguration((Class<?>) serviceType, injectService.filter(),
 			injectService.filterArguments(), injectService.cardinality(), injectService.timeout(), extensionContext);
 

diff --git a/.../src/test/java/org/osgi/test/junit5/test/service/ServiceExtension_SanityCheckingTest.java b/.../src/test/java/org/osgi/test/junit5/test/service/ServiceExtension_SanityCheckingTest.java
@@ -22,6 +22,7 @@
 
 import java.util.Date;
 import java.util.List;
+import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.concurrent.atomic.AtomicReference;
 
 import org.junit.jupiter.api.Test;
@@ -44,19 +45,22 @@ void myTest() {}
 	static class NonRawParameterType {
 		@SuppressWarnings("unused")
 		@Test
-		void myParameterTest(@InjectService AtomicReference<?> param) {}
+		void myParameterTest(@InjectService
+		AtomicReference<?> param) {}
 	}
 
 	static class ListOfNonRawParameterType {
 		@SuppressWarnings("unused")
 		@Test
-		void myParameterTest(@InjectService List<AtomicReference<?>> param) {}
+		void myParameterTest(@InjectService
+		List<AtomicReference<?>> param) {}
 	}
 
 	static class ServiceAwareOfNonRawParameterType {
 		@SuppressWarnings("unused")
 		@Test
-		void myParameterTest(@InjectService ServiceAware<AtomicReference<?>> param) {}
+		void myParameterTest(@InjectService
+		ServiceAware<AtomicReference<?>> param) {}
 	}
 
 	@ParameterizedTest
@@ -71,10 +75,6 @@ void annotatedParameter_withNonRawType_throwsException(Class<?> test) {
 	static class FinalField extends TestBase {
 		@InjectService
 		final Date bc = null;
-
-		@Override
-		@Test
-		void myTest() {}
 	}
 
 	@Test
@@ -85,16 +85,68 @@ void annotatedField_thatIsFinal_throwsException() {
 
 	static class PrivateField extends TestBase {
 		@InjectService
-		private Date bc = null;
-
-		@Override
-		@Test
-		void myTest() {}
+		private Date bc;
 	}
 
 	@Test
 	void annotatedField_thatIsPrivate_throwsException() {
 		assertThatTest(PrivateField.class).isInstanceOf(ExtensionConfigurationException.class)
 			.hasMessageContainingAll("bc", "must not be private", "@InjectService");
 	}
+
+	static class MismatchedServiceType extends TestBase {
+		@InjectService(service = AtomicBoolean.class)
+		Date bc;
+	}
+
+	static class MismatchedServiceType_List extends TestBase {
+		@InjectService(service = AtomicBoolean.class)
+		List<Date> bc;
+	}
+
+	static class MismatchedServiceType_ServiceAware extends TestBase {
+		@InjectService(service = AtomicBoolean.class)
+		ServiceAware<Date> bc;
+	}
+
+	@ParameterizedTest
+	@ValueSource(classes = {
+		MismatchedServiceType.class, MismatchedServiceType_List.class, MismatchedServiceType_ServiceAware.class
+	})
+	void annotatedField_withExplicitServiceType_thatDoesntMatchField_throwsException(Class<?> clazz) {
+		assertThatTest(clazz).isInstanceOf(ExtensionConfigurationException.class)
+			.hasMessageContainingAll("bc", "service type " + AtomicBoolean.class.getName(),
+				"expects " + Date.class.getName(),
+				"@InjectService");
+	}
+
+	static class MismatchedServiceType_Parameter {
+		@Test
+		void myParameterTest(@InjectService(service = AtomicBoolean.class)
+		Date bc) {}
+	}
+
+	static class MismatchedServiceType_Parameter_List {
+		@Test
+		void myParameterTest(@InjectService(service = AtomicBoolean.class)
+		List<Date> bc) {}
+	}
+
+	static class MismatchedServiceType_Parameter_ServiceAware {
+		@Test
+		void myParameterTest(@InjectService(service = AtomicBoolean.class)
+		ServiceAware<Date> bc) {}
+	}
+
+	@ParameterizedTest
+	@ValueSource(classes = {
+		MismatchedServiceType_Parameter.class, MismatchedServiceType_Parameter_List.class,
+		MismatchedServiceType_Parameter_ServiceAware.class
+	})
+	void annotatedParameter_withExplicitServiceType_thatDoesntMatchParameter_throwsException(Class<?> testClass) {
+		assertThatTest(testClass)
+			.isInstanceOf(ParameterResolutionException.class)
+			.hasMessageContainingAll("service type " + AtomicBoolean.class.getName(),
+				"expects " + Date.class.getName(), "@InjectService");
+	}
 }