diff --git a/org.osgi.test.junit5/src/main/java/org/osgi/test/junit5/inject/InjectingExtension.java b/org.osgi.test.junit5/src/main/java/org/osgi/test/junit5/inject/InjectingExtension.java index 32410f3cb..0ff897997 100644 --- a/org.osgi.test.junit5/src/main/java/org/osgi/test/junit5/inject/InjectingExtension.java +++ b/org.osgi.test.junit5/src/main/java/org/osgi/test/junit5/inject/InjectingExtension.java @@ -106,25 +106,21 @@ protected boolean supportsField(Field field, ExtensionContext extensionContext) if (!isAnnotated(field, annotation())) { return false; } + if ((field.getModifiers() & disallowedFieldModifiers()) != 0) { + throw new ExtensionConfigurationException(String.format("Field %s must not be %s for annotation @%s.", + field.getName(), Modifier.toString(field.getModifiers() & disallowedFieldModifiers()), + annotation().getSimpleName())); + } + TargetType targetType = TargetType.of(field); try { - if (!supportsType(targetType, extensionContext)) { - return false; - } + return supportsType(targetType, extensionContext); } catch (ParameterResolutionException pre) { // Convert to ExtensionConfigurationException for field ExtensionConfigurationException ece = new ExtensionConfigurationException(pre.getMessage(), pre.getCause()); ece.setStackTrace(pre.getStackTrace()); throw ece; } - - if ((field.getModifiers() & disallowedFieldModifiers()) != 0) { - throw new ExtensionConfigurationException( - String.format("Field %s must not be %s for annotation @%s.", field.getName(), - Modifier.toString(field.getModifiers() & disallowedFieldModifiers()), - annotation().getSimpleName())); - } - return true; } /** @@ -139,15 +135,14 @@ public boolean supportsParameter(ParameterContext parameterContext, ExtensionCon return false; } TargetType targetType = TargetType.of(parameterContext.getParameter()); - if (!supportsType(targetType, extensionContext)) { - return false; - } - return true; + return supportsType(targetType, extensionContext); } /** * Determine if this extender supports resolution for the specified - * {@link TargetType} for the specified {@link ExtensionContext}. + * {@link TargetType} for the specified {@link ExtensionContext}. The + * default implementation checks through all of the types that were passed + * to the constructor. */ protected boolean supportsType(TargetType targetType, ExtensionContext extensionContext) throws ParameterResolutionException { 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 index b9f86c807..0d05fdca8 100644 --- 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 @@ -62,52 +62,72 @@ public ServiceExtension() { } @Override - protected boolean supportsType(TargetType targetType, ExtensionContext extensionContext) - throws ParameterResolutionException { - extractValueType(targetType); + protected boolean supportsType(TargetType targetType, ExtensionContext extensionContext) { return true; } - private Class extractValueType(TargetType targetType) { - Type serviceType; + private Type extractServiceType(TargetType targetType, InjectService injectService) + throws ParameterResolutionException { + Type valueType, upperBoundType = null; if (targetType.matches(List.class) || targetType.matches(ServiceAware.class)) { if (targetType.hasParameterizedTypes()) { - serviceType = targetType.getFirstGenericTypes() + valueType = targetType.getFirstGenericTypes() .get(); - if (serviceType instanceof WildcardType) { - serviceType = Object.class; + if (valueType instanceof WildcardType) { + final WildcardType wild = (WildcardType) valueType; + upperBoundType = (wild.getUpperBounds().length > 0) ? wild.getUpperBounds()[0] : Object.class; } } else { - serviceType = Object.class; + valueType = Object.class; } } else { - serviceType = targetType.getGenericType(); + valueType = targetType.getGenericType(); + } + if (upperBoundType == null) { + upperBoundType = valueType; } - if (!(serviceType instanceof Class)) { + if (!(upperBoundType 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())); + targetType.getName(), upperBoundType.getTypeName(), annotation().getSimpleName())); } + final Class upperBoundClass = (Class) upperBoundType; - return (Class) serviceType; + final Class serviceClass = injectService.service(); + if (serviceClass.equals(annotation())) { + return upperBoundClass; + } + + if (!upperBoundClass.isAssignableFrom(serviceClass)) { + throw new ParameterResolutionException(String.format( + "Element %s has service type %s for annotation @%s but field expects %s.", targetType.getName(), + serviceClass.getName(), annotation().getSimpleName(), valueType.getTypeName())); + } + if (valueType instanceof WildcardType) { + final WildcardType wild = (WildcardType) valueType; + if (wild.getLowerBounds().length > 0) { + final Type lowerBoundType = wild.getLowerBounds()[0]; + if (!(lowerBoundType instanceof Class)) { + throw new ParameterResolutionException(String.format( + "Element %s has an unsupported lower bound %s for annotation @%s. Service must have non-generic type.", + targetType.getName(), lowerBoundType.getTypeName(), annotation().getSimpleName())); + } + if (!serviceClass.isAssignableFrom((Class) lowerBoundType)) { + throw new ParameterResolutionException(String.format( + "Element %s has service type %s for annotation @%s but field expects %s.", targetType.getName(), + serviceClass.getName(), annotation().getSimpleName(), valueType.getTypeName())); + } + } + } + + return serviceClass; } @Override protected Object resolveValue(TargetType targetType, InjectService injectService, ExtensionContext extensionContext) throws ParameterResolutionException { - final Class valueClass = extractValueType(targetType); - final Class serviceClass = injectService.service(); + final Type serviceType = extractServiceType(targetType, injectService); - Type serviceType; - if (serviceClass.equals(annotation())) { - serviceType = valueClass; - } else if (!valueClass.isAssignableFrom(serviceClass)) { - throw new ParameterResolutionException(String.format( - "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/org.osgi.test.junit5/src/test/java/org/osgi/test/junit5/test/service/ServiceExtension_SanityCheckingTest.java b/org.osgi.test.junit5/src/test/java/org/osgi/test/junit5/test/service/ServiceExtension_SanityCheckingTest.java index c7e28cbbf..8938e0cb0 100644 --- a/org.osgi.test.junit5/src/test/java/org/osgi/test/junit5/test/service/ServiceExtension_SanityCheckingTest.java +++ b/org.osgi.test.junit5/src/test/java/org/osgi/test/junit5/test/service/ServiceExtension_SanityCheckingTest.java @@ -63,9 +63,24 @@ void myParameterTest(@InjectService ServiceAware> param) {} } + static class ServiceAwareOfNonRawUpperBoundType { + @SuppressWarnings("unused") + @Test + void myParameterTest(@InjectService + ServiceAware> param) {} + } + + static class ServiceAwareOfNonRawLowerBoundType { + @SuppressWarnings("unused") + @Test + void myParameterTest(@InjectService(service = AtomicReference.class) + ServiceAware> param) {} + } + @ParameterizedTest @ValueSource(classes = { - NonRawParameterType.class, ListOfNonRawParameterType.class, ServiceAwareOfNonRawParameterType.class + NonRawParameterType.class, ListOfNonRawParameterType.class, ServiceAwareOfNonRawParameterType.class, + ServiceAwareOfNonRawUpperBoundType.class, ServiceAwareOfNonRawLowerBoundType.class }) void annotatedParameter_withNonRawType_throwsException(Class test) { assertThatTest(test).isInstanceOf(ParameterResolutionException.class) @@ -109,9 +124,21 @@ static class MismatchedServiceType_ServiceAware extends TestBase { ServiceAware bc; } + static class MismatchedServiceType_ServiceAware_UpperWildcard extends TestBase { + // AtomicBoolean is not a subclass of Date + @InjectService(service = AtomicBoolean.class) + ServiceAware bc; + } + + static class MismatchedServiceType_ServiceAware_LowerWildcard extends TestBase { + // AtomicBoolean is not a superclass of Date + @InjectService(service = AtomicBoolean.class) + ServiceAware bc; + } + @ParameterizedTest @ValueSource(classes = { - MismatchedServiceType.class, MismatchedServiceType_List.class, MismatchedServiceType_ServiceAware.class + MismatchedServiceType.class, MismatchedServiceType_List.class, MismatchedServiceType_ServiceAware.class, }) void annotatedField_withExplicitServiceType_thatDoesntMatchField_throwsException(Class clazz) { assertThatTest(clazz).isInstanceOf(ExtensionConfigurationException.class) @@ -120,6 +147,22 @@ void annotatedField_withExplicitServiceType_thatDoesntMatchField_throwsException "@InjectService"); } + @Test + void annotatedField_withExplicitServiceType_andUpperBoundThatDoesntMatch_throwsException() { + assertThatTest(MismatchedServiceType_ServiceAware_UpperWildcard.class) + .isInstanceOf(ExtensionConfigurationException.class) + .hasMessageContainingAll("bc", "service type " + AtomicBoolean.class.getName(), + "expects ? extends " + Date.class.getName(), "@InjectService"); + } + + @Test + void annotatedField_withExplicitServiceType_andLowerBoundThatDoesntMatch_throwsException() { + assertThatTest(MismatchedServiceType_ServiceAware_LowerWildcard.class) + .isInstanceOf(ExtensionConfigurationException.class) + .hasMessageContainingAll("bc", "service type " + AtomicBoolean.class.getName(), + "expects ? super " + Date.class.getName(), "@InjectService"); + } + static class MismatchedServiceType_Parameter { @Test void myParameterTest(@InjectService(service = AtomicBoolean.class) diff --git a/org.osgi.test.junit5/src/test/java/org/osgi/test/junit5/test/service/ServiceListTest.java b/org.osgi.test.junit5/src/test/java/org/osgi/test/junit5/test/service/ServiceListTest.java index 209c8b666..3cf0ab0f4 100644 --- a/org.osgi.test.junit5/src/test/java/org/osgi/test/junit5/test/service/ServiceListTest.java +++ b/org.osgi.test.junit5/src/test/java/org/osgi/test/junit5/test/service/ServiceListTest.java @@ -37,6 +37,12 @@ public class ServiceListTest { BundleContext bc; @InjectService List logServices; + @InjectService(service = LogService.class) + List unboundedWildcardLogService; + @InjectService + List upperWildcardLogService; + @InjectService(service = LogService.class) + List lowerWildcardLogService; @InjectService(cardinality = 0) List testServices; @@ -46,8 +52,25 @@ public class ServiceListTest { @Test public void testField() throws Exception { - assertThat(logServices).size() - .isEqualTo(1); + assertThat(logServices).hasSize(1); + } + + @Test + public void testUpperWildcard() { + assertThat(upperWildcardLogService).hasSize(1) + .isEqualTo(logServices); + } + + @Test + public void testLowerWildcard() { + assertThat(lowerWildcardLogService).hasSize(1) + .isEqualTo(logServices); + } + + @Test + public void testUnboundedWildcard() { + assertThat(unboundedWildcardLogService).hasSize(1) + .isEqualTo(logServices); } @Test