From 977321639affb31e5b237c609a9df3a094691cdf Mon Sep 17 00:00:00 2001 From: jameskleeh Date: Thu, 31 May 2018 14:25:58 -0400 Subject: [PATCH 1/2] Correctly support Nullable for method value, field value, and field bean injections. Add annotationMetadataProvider to ArgumentConversionContext --- .../convert/ArgumentConversionContext.java | 9 +++- .../inject/field/nullableinjection/A.java | 19 ++++++++ .../inject/field/nullableinjection/B.java | 28 +++++++++++ .../inject/field/nullableinjection/C.java | 27 +++++++++++ .../FieldNullableInjectionSpec.groovy | 48 +++++++++++++++++++ .../inject/value/nullablevalue/A.java | 32 +++++++++++++ .../nullablevalue/NullableValueSpec.groovy | 25 ++++++++++ .../context/AbstractBeanDefinition.java | 18 +++++-- .../session/binder/SessionArgumentBinder.java | 2 +- 9 files changed, 203 insertions(+), 5 deletions(-) create mode 100644 inject-java/src/test/groovy/io/micronaut/inject/field/nullableinjection/A.java create mode 100644 inject-java/src/test/groovy/io/micronaut/inject/field/nullableinjection/B.java create mode 100644 inject-java/src/test/groovy/io/micronaut/inject/field/nullableinjection/C.java create mode 100644 inject-java/src/test/groovy/io/micronaut/inject/field/nullableinjection/FieldNullableInjectionSpec.groovy create mode 100644 inject-java/src/test/groovy/io/micronaut/inject/value/nullablevalue/A.java create mode 100644 inject-java/src/test/groovy/io/micronaut/inject/value/nullablevalue/NullableValueSpec.groovy diff --git a/core/src/main/java/io/micronaut/core/convert/ArgumentConversionContext.java b/core/src/main/java/io/micronaut/core/convert/ArgumentConversionContext.java index 86ebe69b248..4d8d0dde2a5 100644 --- a/core/src/main/java/io/micronaut/core/convert/ArgumentConversionContext.java +++ b/core/src/main/java/io/micronaut/core/convert/ArgumentConversionContext.java @@ -16,6 +16,8 @@ package io.micronaut.core.convert; +import io.micronaut.core.annotation.AnnotationMetadata; +import io.micronaut.core.annotation.AnnotationMetadataProvider; import io.micronaut.core.type.Argument; /** @@ -25,10 +27,15 @@ * @author Graeme Rocher * @since 1.0 */ -public interface ArgumentConversionContext extends ConversionContext { +public interface ArgumentConversionContext extends ConversionContext, AnnotationMetadataProvider { /** * @return The {@link Argument} being converted */ Argument getArgument(); + + @Override + default AnnotationMetadata getAnnotationMetadata() { + return getArgument().getAnnotationMetadata(); + } } diff --git a/inject-java/src/test/groovy/io/micronaut/inject/field/nullableinjection/A.java b/inject-java/src/test/groovy/io/micronaut/inject/field/nullableinjection/A.java new file mode 100644 index 00000000000..8aa598a47aa --- /dev/null +++ b/inject-java/src/test/groovy/io/micronaut/inject/field/nullableinjection/A.java @@ -0,0 +1,19 @@ +/* + * Copyright 2017-2018 original authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.micronaut.inject.field.nullableinjection; + +public interface A { +} diff --git a/inject-java/src/test/groovy/io/micronaut/inject/field/nullableinjection/B.java b/inject-java/src/test/groovy/io/micronaut/inject/field/nullableinjection/B.java new file mode 100644 index 00000000000..bcc41ffd67e --- /dev/null +++ b/inject-java/src/test/groovy/io/micronaut/inject/field/nullableinjection/B.java @@ -0,0 +1,28 @@ +/* + * Copyright 2017-2018 original authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.micronaut.inject.field.nullableinjection; + +import javax.annotation.Nullable; +import javax.inject.Inject; + +public class B { + + @Inject @Nullable protected A a; + + public A getA() { + return this.a; + } +} diff --git a/inject-java/src/test/groovy/io/micronaut/inject/field/nullableinjection/C.java b/inject-java/src/test/groovy/io/micronaut/inject/field/nullableinjection/C.java new file mode 100644 index 00000000000..728b3ad9dea --- /dev/null +++ b/inject-java/src/test/groovy/io/micronaut/inject/field/nullableinjection/C.java @@ -0,0 +1,27 @@ +/* + * Copyright 2017-2018 original authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.micronaut.inject.field.nullableinjection; + +import javax.inject.Inject; + +public class C { + + @Inject protected A a; + + public A getA() { + return this.a; + } +} diff --git a/inject-java/src/test/groovy/io/micronaut/inject/field/nullableinjection/FieldNullableInjectionSpec.groovy b/inject-java/src/test/groovy/io/micronaut/inject/field/nullableinjection/FieldNullableInjectionSpec.groovy new file mode 100644 index 00000000000..f0f3045ce12 --- /dev/null +++ b/inject-java/src/test/groovy/io/micronaut/inject/field/nullableinjection/FieldNullableInjectionSpec.groovy @@ -0,0 +1,48 @@ +/* + * Copyright 2017-2018 original authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.micronaut.inject.field.nullableinjection + +import io.micronaut.context.BeanContext +import io.micronaut.context.DefaultBeanContext +import io.micronaut.context.exceptions.DependencyInjectionException +import spock.lang.Specification + +class FieldNullableInjectionSpec extends Specification { + + void "test nullable injection with field"() { + given: + BeanContext context = new DefaultBeanContext() + context.start() + + when:"A bean is obtained that has a constructor with @Inject" + B b = context.getBean(B) + + then:"The implementation is not injected, but null is" + b.a == null + } + + void "test normal injection still fails"() { + given: + BeanContext context = new DefaultBeanContext() + context.start() + + when:"A bean is obtained that has a constructor with @Inject" + context.getBean(C) + + then:"The bean is not found" + thrown(DependencyInjectionException) + } +} \ No newline at end of file diff --git a/inject-java/src/test/groovy/io/micronaut/inject/value/nullablevalue/A.java b/inject-java/src/test/groovy/io/micronaut/inject/value/nullablevalue/A.java new file mode 100644 index 00000000000..57e276de433 --- /dev/null +++ b/inject-java/src/test/groovy/io/micronaut/inject/value/nullablevalue/A.java @@ -0,0 +1,32 @@ +package io.micronaut.inject.value.nullablevalue; + +import io.micronaut.context.annotation.Value; + +import javax.annotation.Nullable; +import javax.inject.Inject; +import javax.inject.Singleton; + +@Singleton +public class A { + + public final String nullConstructorArg; + public final String nonNullConstructorArg; + public String nullMethodArg; + public String nonNullMethodArg; + public @Value("${doesnt.exist}") @Nullable String nullField; + public @Value("${exists.x}") String nonNullField; + + + public A(@Value("${doesnt.exist}") @Nullable String nullConstructorArg, + @Value("${exists.x}") String nonNullConstructorArg) { + this.nullConstructorArg = nullConstructorArg; + this.nonNullConstructorArg = nonNullConstructorArg; + } + + @Inject + void injectedMethod(@Value("${doesnt.exist}") @Nullable String nullMethodArg, + @Value("${exists.x}") String nonNullMethodArg) { + this.nullMethodArg = nullMethodArg; + this.nonNullMethodArg = nonNullMethodArg; + } +} diff --git a/inject-java/src/test/groovy/io/micronaut/inject/value/nullablevalue/NullableValueSpec.groovy b/inject-java/src/test/groovy/io/micronaut/inject/value/nullablevalue/NullableValueSpec.groovy new file mode 100644 index 00000000000..dc3cb97bcc9 --- /dev/null +++ b/inject-java/src/test/groovy/io/micronaut/inject/value/nullablevalue/NullableValueSpec.groovy @@ -0,0 +1,25 @@ +package io.micronaut.inject.value.nullablevalue + +import io.micronaut.context.ApplicationContext +import spock.lang.Specification + +class NullableValueSpec extends Specification { + + void "test value with nullable"() { + given: + ApplicationContext context = ApplicationContext.run( + ["exists.x":"fromConfig"], "test" + ) + + when: + A a = context.getBean(A) + + then: + a.nullField == null + a.nonNullField == "fromConfig" + a.nullConstructorArg == null + a.nonNullConstructorArg == "fromConfig" + a.nullMethodArg == null + a.nonNullMethodArg == "fromConfig" + } +} diff --git a/inject/src/main/java/io/micronaut/context/AbstractBeanDefinition.java b/inject/src/main/java/io/micronaut/context/AbstractBeanDefinition.java index c806f93d8db..c55059e7573 100644 --- a/inject/src/main/java/io/micronaut/context/AbstractBeanDefinition.java +++ b/inject/src/main/java/io/micronaut/context/AbstractBeanDefinition.java @@ -748,9 +748,10 @@ protected final Object getValueForMethodArgument(BeanResolutionContext resolutio } else { if (value.isPresent()) { return value.get(); - } else if (!Iterable.class.isAssignableFrom(argumentType) && !Map.class.isAssignableFrom(argumentType)) { - throw new DependencyInjectionException(resolutionContext, injectionPoint, conversionContext, valString); } else { + if (argument.getDeclaredAnnotation(Nullable.class) != null) { + return null; + } throw new DependencyInjectionException(resolutionContext, injectionPoint, conversionContext, valString); } } @@ -1134,7 +1135,14 @@ protected final Object getValueForField(BeanResolutionContext resolutionContext, if (fieldType == Optional.class) { return resolveOptionalObject(value); } else { - return value.orElseThrow(() -> new DependencyInjectionException(resolutionContext, injectionPoint, "Error resolving field value [" + valString + "]. Property doesn't exist or cannot be converted")); + if (value.isPresent()) { + return value.get(); + } else { + if (fieldArgument.getDeclaredAnnotation(Nullable.class) != null) { + return null; + } + throw new DependencyInjectionException(resolutionContext, injectionPoint, "Error resolving field value [" + valString + "]. Property doesn't exist or cannot be converted"); + } } } } else { @@ -1288,6 +1296,10 @@ protected final Object getBeanForField(BeanResolutionContext resolutionContext, path.pop(); return bean; } catch (NoSuchBeanException e) { + if (injectionPoint.getDeclaredAnnotation(Nullable.class) != null) { + path.pop(); + return null; + } throw new DependencyInjectionException(resolutionContext, injectionPoint, e); } } diff --git a/session/src/main/java/io/micronaut/session/binder/SessionArgumentBinder.java b/session/src/main/java/io/micronaut/session/binder/SessionArgumentBinder.java index 12ed4da7efb..5986fd00699 100644 --- a/session/src/main/java/io/micronaut/session/binder/SessionArgumentBinder.java +++ b/session/src/main/java/io/micronaut/session/binder/SessionArgumentBinder.java @@ -74,7 +74,7 @@ public ArgumentBinder.BindingResult bind(ArgumentConversionContext existing; } else { // create a new session store it in the attribute - if (context.getAnnotation(Nullable.class) == null) { + if (!context.isAnnotationPresent(Nullable.class)) { Session newSession = sessionStore.newSession(); attrs.put(HttpSessionFilter.SESSION_ATTRIBUTE, newSession); return () -> Optional.of(newSession); From 796f73158084b6bd52e8e981035cdeeab43b95bc Mon Sep 17 00:00:00 2001 From: jameskleeh Date: Thu, 31 May 2018 14:43:10 -0400 Subject: [PATCH 2/2] Attempt to fix EventListenerSpec on travis --- .../security/events/EventListenerSpec.groovy | 23 +++++++++++-------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/security/src/test/groovy/io/micronaut/security/events/EventListenerSpec.groovy b/security/src/test/groovy/io/micronaut/security/events/EventListenerSpec.groovy index ab7a35b8afd..6cb2c3bdf14 100644 --- a/security/src/test/groovy/io/micronaut/security/events/EventListenerSpec.groovy +++ b/security/src/test/groovy/io/micronaut/security/events/EventListenerSpec.groovy @@ -40,6 +40,7 @@ import org.reactivestreams.Publisher import spock.lang.AutoCleanup import spock.lang.Shared import spock.lang.Specification +import spock.util.concurrent.PollingConditions import javax.inject.Singleton @@ -62,8 +63,9 @@ class EventListenerSpec extends Specification { then: thrown(HttpClientResponseException) - embeddedServer.applicationContext.getBean(LoginFailedEventListener).events.size() == - old(embeddedServer.applicationContext.getBean(LoginFailedEventListener).events.size()) + 1 + new PollingConditions().eventually { + embeddedServer.applicationContext.getBean(LoginFailedEventListener).events.size() == 1 + } } def "successful login publishes LoginSuccessfulEvent"() { @@ -72,8 +74,9 @@ class EventListenerSpec extends Specification { client.toBlocking().exchange(request) then: - embeddedServer.applicationContext.getBean(LoginSuccessfulEventListener).events.size() == - old(embeddedServer.applicationContext.getBean(LoginSuccessfulEventListener).events.size()) + 1 + new PollingConditions().eventually { + embeddedServer.applicationContext.getBean(LoginSuccessfulEventListener).events.size() == 1 + } } def "accessing a secured endpoints, validates Basic auth token and triggers TokenValidatedEvent"() { @@ -82,8 +85,9 @@ class EventListenerSpec extends Specification { client.toBlocking().exchange(request) then: - embeddedServer.applicationContext.getBean(TokenValidatedEventListener).events.size() == - old(embeddedServer.applicationContext.getBean(TokenValidatedEventListener).events.size()) + 1 + new PollingConditions().eventually { + embeddedServer.applicationContext.getBean(TokenValidatedEventListener).events.size() == 1 + } } def "invoking logout triggers LogoutEvent"() { @@ -93,9 +97,10 @@ class EventListenerSpec extends Specification { then: thrown(HttpClientResponseException) - embeddedServer.applicationContext.getBean(LogoutEventListener).events.size() == - old(embeddedServer.applicationContext.getBean(LogoutEventListener).events.size()) + 1 - (embeddedServer.applicationContext.getBean(LogoutEventListener).events*.getSource() as List).find { it.name == 'user'} + new PollingConditions().eventually { + embeddedServer.applicationContext.getBean(LogoutEventListener).events.size() == 1 + (embeddedServer.applicationContext.getBean(LogoutEventListener).events*.getSource() as List).any { it.name == 'user'} + } } @Requires(property = "spec.name", value = "eventlistener")