Skip to content

Commit

Permalink
Merge pull request #497 from kriegfrj/serviceextension-assignment-com…
Browse files Browse the repository at this point in the history
…patibility

[ServiceExtension] Better sanity checking and wildcard support
  • Loading branch information
kriegfrj authored Mar 30, 2022
2 parents c70f2c8 + c66a797 commit 96dcbde
Show file tree
Hide file tree
Showing 4 changed files with 192 additions and 64 deletions.
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
Original file line number Diff line number Diff line change
Expand Up @@ -62,57 +62,72 @@ public ServiceExtension() {
}

@Override
protected boolean supportsType(TargetType targetType, ExtensionContext extensionContext)
protected boolean supportsType(TargetType targetType, ExtensionContext extensionContext) {
return true;
}

private Type extractServiceType(TargetType targetType, InjectService injectService)
throws ParameterResolutionException {
Type serviceType;
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 (!(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;

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

@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;
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()));
}
} else {
serviceType = targetType.getType();
}
}

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 serviceClass;
}

@Override
protected Object resolveValue(TargetType targetType, InjectService injectService, ExtensionContext extensionContext)
throws ParameterResolutionException {
final Type serviceType = extractServiceType(targetType, injectService);

ServiceConfiguration<?> configuration = getServiceConfiguration((Class<?>) serviceType, injectService.filter(),
injectService.filterArguments(), injectService.cardinality(), injectService.timeout(), extensionContext);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -44,24 +45,42 @@ 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) {}
}

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 All @@ -71,10 +90,6 @@ void annotatedParameter_withNonRawType_throwsException(Class<?> test) {
static class FinalField extends TestBase {
@InjectService
final Date bc = null;

@Override
@Test
void myTest() {}
}

@Test
Expand All @@ -85,16 +100,96 @@ 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;
}

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,
})
void annotatedField_withExplicitServiceType_thatDoesntMatchField_throwsException(Class<?> clazz) {
assertThatTest(clazz).isInstanceOf(ExtensionConfigurationException.class)
.hasMessageContainingAll("bc", "service type " + AtomicBoolean.class.getName(),
"expects " + Date.class.getName(),
"@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)
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");
}
}
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 96dcbde

Please sign in to comment.