diff --git a/spring-test/src/main/java/org/springframework/test/context/junit/jupiter/ParameterAutowireUtils.java b/spring-test/src/main/java/org/springframework/test/context/junit/jupiter/ParameterAutowireUtils.java index 502393e98f..2c81b450b0 100644 --- a/spring-test/src/main/java/org/springframework/test/context/junit/jupiter/ParameterAutowireUtils.java +++ b/spring-test/src/main/java/org/springframework/test/context/junit/jupiter/ParameterAutowireUtils.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2016 the original author or authors. + * Copyright 2002-2018 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -18,6 +18,8 @@ package org.springframework.test.context.junit.jupiter; import java.lang.annotation.Annotation; import java.lang.reflect.AnnotatedElement; +import java.lang.reflect.Constructor; +import java.lang.reflect.Executable; import java.lang.reflect.Parameter; import java.util.Optional; @@ -32,6 +34,7 @@ import org.springframework.core.MethodParameter; import org.springframework.core.annotation.AnnotatedElementUtils; import org.springframework.core.annotation.SynthesizingMethodParameter; import org.springframework.lang.Nullable; +import org.springframework.util.ClassUtils; /** * Collection of utilities related to autowiring of individual method parameters. @@ -43,6 +46,26 @@ import org.springframework.lang.Nullable; */ abstract class ParameterAutowireUtils { + private static final AnnotatedElement EMPTY_ANNOTATED_ELEMENT = new AnnotatedElement() { + + @Override + public T getAnnotation(Class annotationClass) { + return null; + } + + @Override + public Annotation[] getAnnotations() { + return new Annotation[0]; + } + + @Override + public Annotation[] getDeclaredAnnotations() { + return new Annotation[0]; + } + }; + + + private ParameterAutowireUtils() { /* no-op */ } @@ -54,13 +77,18 @@ abstract class ParameterAutowireUtils { * {@link ApplicationContext} (or a sub-type thereof) or is annotated or * meta-annotated with {@link Autowired @Autowired}, * {@link Qualifier @Qualifier}, or {@link Value @Value}. + * @param parameter the parameter whose dependency should be autowired + * @param parameterIndex the index of the parameter * @see #resolveDependency(Parameter, Class, ApplicationContext) */ - static boolean isAutowirable(Parameter parameter) { - return ApplicationContext.class.isAssignableFrom(parameter.getType()) - || AnnotatedElementUtils.hasAnnotation(parameter, Autowired.class) - || AnnotatedElementUtils.hasAnnotation(parameter, Qualifier.class) - || AnnotatedElementUtils.hasAnnotation(parameter, Value.class); + static boolean isAutowirable(Parameter parameter, int parameterIndex) { + if (ApplicationContext.class.isAssignableFrom(parameter.getType())) { + return true; + } + AnnotatedElement annotatedParameter = getEffectiveAnnotatedParameter(parameter, parameterIndex); + return AnnotatedElementUtils.hasAnnotation(annotatedParameter, Autowired.class) + || AnnotatedElementUtils.hasAnnotation(annotatedParameter, Qualifier.class) + || AnnotatedElementUtils.hasAnnotation(annotatedParameter, Value.class); } /** @@ -77,6 +105,7 @@ abstract class ParameterAutowireUtils { *

If an explicit qualifier is not declared, the name of the parameter * will be used as the qualifier for resolving ambiguities. * @param parameter the parameter whose dependency should be resolved + * @param parameterIndex the index of the parameter * @param containingClass the concrete class that contains the parameter; this may * differ from the class that declares the parameter in that it may be a subclass * thereof, potentially substituting type variables @@ -90,8 +119,9 @@ abstract class ParameterAutowireUtils { * @see AutowireCapableBeanFactory#resolveDependency(DependencyDescriptor, String) */ @Nullable - static Object resolveDependency(Parameter parameter, Class containingClass, ApplicationContext applicationContext) { - boolean required = findMergedAnnotation(parameter, Autowired.class).map(Autowired::required).orElse(true); + static Object resolveDependency(Parameter parameter, int parameterIndex, Class containingClass, ApplicationContext applicationContext) { + AnnotatedElement annotatedParameter = getEffectiveAnnotatedParameter(parameter, parameterIndex); + boolean required = findMergedAnnotation(annotatedParameter, Autowired.class).map(Autowired::required).orElse(true); MethodParameter methodParameter = SynthesizingMethodParameter.forParameter(parameter); DependencyDescriptor descriptor = new DependencyDescriptor(methodParameter, required); descriptor.setContainingClass(containingClass); @@ -102,4 +132,44 @@ abstract class ParameterAutowireUtils { return Optional.ofNullable(AnnotatedElementUtils.findMergedAnnotation(element, annotationType)); } + /** + * Due to a bug in {@code javac} on JDK versions prior to JDK 9, looking up + * annotations directly on a {@link Parameter} will fail for inner class + * constructors. + * + *

Bug in javac in JDK < 9

+ *

The parameter annotations array in the compiled byte code excludes an entry + * for the implicit enclosing instance parameter for an inner class + * constructor. + * + *

Workaround

+ *

This method provides a workaround for this off-by-one error by allowing the + * caller to access annotations on the preceding {@link Parameter} object (i.e., + * {@code index - 1}). If the supplied {@code index} is zero, this method returns + * an empty {@code AnnotatedElement}. + * + *

WARNING

+ *

The {@code AnnotatedElement} returned by this method should never be cast and + * treated as a {@code Parameter} since the metadata (e.g., {@link Parameter#getName()}, + * {@link Parameter#getType()}, etc.) will not match those for the declared parameter + * at the given index in an inner class constructor. + * + * @return the supplied {@code parameter} or the effective {@code Parameter} + * if the aforementioned bug is in effect + */ + private static AnnotatedElement getEffectiveAnnotatedParameter(Parameter parameter, int index) { + Executable executable = parameter.getDeclaringExecutable(); + + if (executable instanceof Constructor && + ClassUtils.isInnerClass(executable.getDeclaringClass()) && + executable.getParameterAnnotations().length == executable.getParameterCount() - 1) { + + // Bug in javac in JDK <9: annotation array excludes enclosing instance parameter + // for inner classes, so access it with the actual parameter index lowered by 1 + return (index == 0) ? EMPTY_ANNOTATED_ELEMENT : executable.getParameters()[index - 1]; + } + + return parameter; + } + } diff --git a/spring-test/src/main/java/org/springframework/test/context/junit/jupiter/SpringExtension.java b/spring-test/src/main/java/org/springframework/test/context/junit/jupiter/SpringExtension.java index 4b5b433098..f8b6249786 100644 --- a/spring-test/src/main/java/org/springframework/test/context/junit/jupiter/SpringExtension.java +++ b/spring-test/src/main/java/org/springframework/test/context/junit/jupiter/SpringExtension.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2017 the original author or authors. + * Copyright 2002-2018 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -155,10 +155,11 @@ public class SpringExtension implements BeforeAllCallback, AfterAllCallback, Tes @Override public boolean supportsParameter(ParameterContext parameterContext, ExtensionContext extensionContext) { Parameter parameter = parameterContext.getParameter(); + int index = parameterContext.getIndex(); Executable executable = parameter.getDeclaringExecutable(); return (executable instanceof Constructor && AnnotatedElementUtils.hasAnnotation(executable, Autowired.class)) || - ParameterAutowireUtils.isAutowirable(parameter); + ParameterAutowireUtils.isAutowirable(parameter, index); } /** @@ -172,9 +173,10 @@ public class SpringExtension implements BeforeAllCallback, AfterAllCallback, Tes @Nullable public Object resolveParameter(ParameterContext parameterContext, ExtensionContext extensionContext) { Parameter parameter = parameterContext.getParameter(); + int index = parameterContext.getIndex(); Class testClass = extensionContext.getRequiredTestClass(); ApplicationContext applicationContext = getApplicationContext(extensionContext); - return ParameterAutowireUtils.resolveDependency(parameter, testClass, applicationContext); + return ParameterAutowireUtils.resolveDependency(parameter, index, testClass, applicationContext); } diff --git a/spring-test/src/test/java/org/springframework/test/context/junit/jupiter/nested/NestedTestsWithConstructorInjectionWithSpringAndJUnitJupiterTestCase.java b/spring-test/src/test/java/org/springframework/test/context/junit/jupiter/nested/NestedTestsWithConstructorInjectionWithSpringAndJUnitJupiterTestCase.java index d07e10641a..6ccc6ddb22 100644 --- a/spring-test/src/test/java/org/springframework/test/context/junit/jupiter/nested/NestedTestsWithConstructorInjectionWithSpringAndJUnitJupiterTestCase.java +++ b/spring-test/src/test/java/org/springframework/test/context/junit/jupiter/nested/NestedTestsWithConstructorInjectionWithSpringAndJUnitJupiterTestCase.java @@ -18,12 +18,14 @@ package org.springframework.test.context.junit.jupiter.nested; import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.TestInfo; import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.beans.factory.annotation.Qualifier; +import org.springframework.beans.factory.annotation.Value; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.test.context.junit.SpringJUnitJupiterTestSuite; -import org.springframework.test.context.junit.jupiter.DisabledIf; import org.springframework.test.context.junit.jupiter.SpringExtension; import org.springframework.test.context.junit.jupiter.SpringJUnitConfig; import org.springframework.test.context.junit.jupiter.nested.NestedTestsWithConstructorInjectionWithSpringAndJUnitJupiterTestCase.TopLevelConfig; @@ -33,7 +35,7 @@ import static org.junit.jupiter.api.Assertions.*; /** * Integration tests that verify support for {@code @Nested} test classes in conjunction * with the {@link SpringExtension} in a JUnit Jupiter environment ... when using - * constructor injection as opposed to field injection. + * constructor injection as opposed to field injection (see SPR-16653). * *

* To run these tests in an IDE that does not have built-in support for the JUnit @@ -49,8 +51,7 @@ class NestedTestsWithConstructorInjectionWithSpringAndJUnitJupiterTestCase { final String foo; - @Autowired - NestedTestsWithConstructorInjectionWithSpringAndJUnitJupiterTestCase(String foo) { + NestedTestsWithConstructorInjectionWithSpringAndJUnitJupiterTestCase(TestInfo testInfo, @Autowired String foo) { this.foo = foo; } @@ -65,8 +66,6 @@ class NestedTestsWithConstructorInjectionWithSpringAndJUnitJupiterTestCase { final String bar; - // Only fails on JDK 8 if the parameter is annotated with @Autowired. - // Works if the constructor itself is annotated with @Autowired. @Autowired AutowiredConstructor(String bar) { this.bar = bar; @@ -81,15 +80,10 @@ class NestedTestsWithConstructorInjectionWithSpringAndJUnitJupiterTestCase { @Nested @SpringJUnitConfig(NestedConfig.class) - @DisabledIf(expression = "#{systemProperties['java.version'].startsWith('1.8')}", // - reason = "Disabled on Java 8 due to a bug in javac in JDK 8") - // See https://github.com/junit-team/junit5/issues/1345 class AutowiredConstructorParameter { final String bar; - // Only fails on JDK 8 if the parameter is annotated with @Autowired. - // Works if the constructor itself is annotated with @Autowired. AutowiredConstructorParameter(@Autowired String bar) { this.bar = bar; } @@ -101,6 +95,43 @@ class NestedTestsWithConstructorInjectionWithSpringAndJUnitJupiterTestCase { } } + @Nested + @SpringJUnitConfig(NestedConfig.class) + class QualifiedConstructorParameter { + + final String bar; + + QualifiedConstructorParameter(TestInfo testInfo, @Qualifier("bar") String s) { + this.bar = s; + } + + @Test + void nestedTest() throws Exception { + assertEquals("foo", foo); + assertEquals("bar", bar); + } + } + + @Nested + @SpringJUnitConfig(NestedConfig.class) + class SpelConstructorParameter { + + final String bar; + final int answer; + + SpelConstructorParameter(@Autowired String bar, TestInfo testInfo, @Value("#{ 6 * 7 }") int answer) { + this.bar = bar; + this.answer = answer; + } + + @Test + void nestedTest() throws Exception { + assertEquals("foo", foo); + assertEquals("bar", bar); + assertEquals(42, answer); + } + } + // ------------------------------------------------------------------------- @Configuration