Skip to content

Commit

Permalink
[ServiceExtension] Better wildcard support.
Browse files Browse the repository at this point in the history 
Fixes #494.

Signed-off-by: Fr Jeremy Krieg <fr.jkrieg@greekwelfaresa.org.au>
  • Loading branch information
@kriegfrj
kriegfrj committed Mar 28, 2022
1 parent 1e2d488 commit 1fb6f07
Show file tree
Hide file tree
Showing 4 changed files with 159 additions and 47 deletions.
62 changes: 43 additions & 19 deletions org.osgi.test.junit5/src/main/java/org/osgi/test/junit5/inject/InjectingExtension.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
import static java.util.Objects.requireNonNull;
import static java.util.stream.Collectors.joining;
import static org.junit.platform.commons.support.AnnotationSupport.findAnnotation;
import static org.junit.platform.commons.support.AnnotationSupport.isAnnotated;
import static org.osgi.test.common.inject.FieldInjector.findAnnotatedFields;
import static org.osgi.test.common.inject.FieldInjector.findAnnotatedNonStaticFields;
import static org.osgi.test.common.inject.FieldInjector.setField;
Expand All @@ -32,6 +31,7 @@
import java.lang.reflect.Parameter;
import java.util.Arrays;
import java.util.List;
import java.util.Optional;

import org.junit.jupiter.api.TestInstance;
import org.junit.jupiter.api.TestInstance.Lifecycle;
Expand Down Expand Up @@ -103,28 +103,26 @@ protected int disallowedFieldModifiers() {
* {@link Field} for the specified {@link ExtensionContext}.
*/
protected boolean supportsField(Field field, ExtensionContext extensionContext) {
if (!isAnnotated(field, annotation())) {
Optional<INJECTION> injectionOpt = findAnnotation(field, annotation());

if (!injectionOpt.isPresent()) {
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, injectionOpt.get(), 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;
}

/**
Expand All @@ -135,19 +133,22 @@ protected boolean supportsField(Field field, ExtensionContext extensionContext)
@Override
public boolean supportsParameter(ParameterContext parameterContext, ExtensionContext extensionContext)
throws ParameterResolutionException {
if (!parameterContext.isAnnotated(annotation())) {
Optional<INJECTION> injection = parameterContext.findAnnotation(annotation());
if (!injection.isPresent()) {
return false;
}
TargetType targetType = TargetType.of(parameterContext.getParameter());
if (!supportsType(targetType, extensionContext)) {
return false;
}
return true;
return supportsType(targetType, injection.get(), 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. This method is not called directly - it is called by
* the default implementation of
* {@link #supportsType(TargetType, INJECTION, ExtensionContext)}. It is
* retained for backward compatibility.
*/
protected boolean supportsType(TargetType targetType, ExtensionContext extensionContext)
throws ParameterResolutionException {
Expand All @@ -165,6 +166,29 @@ protected boolean supportsType(TargetType targetType, ExtensionContext extension
return true;
}

/**
* Determine if this extender supports resolution for the specified
* {@link TargetType} for the specified annotation and
* {@link ExtensionContext}. The default implementation simply delegates to
* {@link #supportsType(TargetType, ExtensionContext)}.<br>
* This method is not called directly - it is called from
* {@link #supportsParameter} and {@link supportsField}. It is only invoked
* if the field/parameter is annotated with an annotation owned by this
* injector.
*
* @param targetType the type of the field/parameter that is the target of
* the injected object
* @param annotation the annotation on the injected field/parameter (never
* <tt>null</tt>).
* @param extensionContext the extension context for the active test where
* this field/parameter is being injected.
* @return <tt>true</tt> if the target type is supported by the injector,
* <tt>false</tt> if not.
*/
protected boolean supportsType(TargetType targetType, INJECTION annotation, ExtensionContext extensionContext) {
return supportsType(targetType, extensionContext);
}

/**
* Resolve the value for the specified {@link Field} for the specified
* {@link ExtensionContext}.
Expand Down
70 changes: 46 additions & 24 deletions org.osgi.test.junit5/src/main/java/org/osgi/test/junit5/service/ServiceExtension.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,52 +62,74 @@ public ServiceExtension() {
}

@Override
protected boolean supportsType(TargetType targetType, ExtensionContext extensionContext)
protected boolean supportsType(TargetType targetType, InjectService injectService,
ExtensionContext extensionContext)
throws ParameterResolutionException {
extractValueType(targetType);
extractServiceType(targetType, injectService);
return true;
}

private Class<?> extractValueType(TargetType targetType) {
Type serviceType;
private Type extractServiceType(TargetType targetType, InjectService injectService) {
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 (!(serviceType instanceof Class)) {
if (upperBoundType == null) {
upperBoundType = valueType;
}
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;

final Class<?> serviceClass = injectService.service();
if (serviceClass.equals(annotation())) {
return upperBoundClass;
}

return (Class<?>) serviceType;
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);

Expand Down
47 changes: 45 additions & 2 deletions .../src/test/java/org/osgi/test/junit5/test/service/ServiceExtension_SanityCheckingTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,9 +63,24 @@ void myParameterTest(@InjectService
ServiceAware<AtomicReference<?>> param) {}
}

static class ServiceAwareOfNonRawUpperBoundType {
@SuppressWarnings("unused")
@Test
void myParameterTest(@InjectService
ServiceAware<? extends AtomicReference<?>> param) {}
}

static class ServiceAwareOfNonRawLowerBoundType {
@SuppressWarnings("unused")
@Test
void myParameterTest(@InjectService(service = AtomicReference.class)
ServiceAware<? super AtomicReference<?>> 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)
Expand Down Expand Up @@ -109,9 +124,21 @@ static class MismatchedServiceType_ServiceAware extends TestBase {
ServiceAware<Date> bc;
}

static class MismatchedServiceType_ServiceAware_UpperWildcard extends TestBase {
// AtomicBoolean is not a subclass of Date
@InjectService(service = AtomicBoolean.class)
ServiceAware<? extends Date> bc;
}

static class MismatchedServiceType_ServiceAware_LowerWildcard extends TestBase {
// AtomicBoolean is not a superclass of Date
@InjectService(service = AtomicBoolean.class)
ServiceAware<? super Date> 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)
Expand All @@ -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)
Expand Down
27 changes: 25 additions & 2 deletions org.osgi.test.junit5/src/test/java/org/osgi/test/junit5/test/service/ServiceListTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,12 @@ public class ServiceListTest {
BundleContext bc;
@InjectService
List<LogService> logServices;
@InjectService(service = LogService.class)
List<?> unboundedWildcardLogService;
@InjectService
List<? extends LogService> upperWildcardLogService;
@InjectService(service = LogService.class)
List<? super LogService> lowerWildcardLogService;

@InjectService(cardinality = 0)
List<TestServiceWithMultipleCardinality> testServices;
Expand All @@ -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
Expand Down

0 comments on commit 1fb6f07

Please sign in to comment.