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 29, 2022
1 parent dd03f92 commit c66a797
Show file tree
Hide file tree
Showing 4 changed files with 127 additions and 45 deletions.
27 changes: 11 additions & 16 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 @@ -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;
}

/**
Expand All @@ -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 {
Expand Down
71 changes: 46 additions & 25 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,73 @@ 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;
Type 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);

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 c66a797

Please sign in to comment.