Support DI of individual constructor args in @nested tests

Prior to this commit it was impossible to have an individual
constructor argument in a @nested (i.e., inner) test class injected via
@Autowired, @qualifier, or @value.

This is due to a bug in javac on JDK versions prior to 9, whereby
annotation lookups performed directly via the
java.lang.reflect.Parameter API fail for inner class constructors.

Specifically, the parameter annotations array in the compiled byte code
for the user's test class excludes an entry for the implicit enclosing
instance parameter for an inner class constructor.

This commit introduces a workaround in ParameterAutowireUtils for this
off-by-one error by transparently looking up annotations on the
preceding Parameter object (i.e., index - 1). In addition, this commit
relies on the change recently introduced in MethodParameter in order to
compensate for the same JDK bug (see SPR-16652).

Issue: SPR-16653

Author: sbrannen

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 @@
 
 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.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 @@
  */
 abstract class ParameterAutowireUtils {
 
+	private static final AnnotatedElement EMPTY_ANNOTATED_ELEMENT = new AnnotatedElement() {
+
+		@Override
+		public <T extends Annotation> T getAnnotation(Class<T> 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 @@ private 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 @@ static boolean isAutowirable(Parameter parameter) {
 	 * <p>If an explicit <em>qualifier</em> 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 @@ static boolean isAutowirable(Parameter parameter) {
 	 * @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 @@ private static <A extends Annotation> Optional<A> findMergedAnnotation(Annotated
 		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.
+	 *
+	 * <h4>Bug in javac in JDK &lt; 9</h4>
+	 * <p>The parameter annotations array in the compiled byte code excludes an entry
+	 * for the implicit <em>enclosing instance</em> parameter for an inner class
+	 * constructor.
+	 *
+	 * <h4>Workaround</h4>
+	 * <p>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}.
+	 *
+	 * <h4>WARNING</h4>
+	 * <p>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 <em>effective</em> {@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;
+	}
+
 }

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 void afterEach(ExtensionContext context) throws Exception {
 	@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 boolean supportsParameter(ParameterContext parameterContext, ExtensionCon
 	@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);
 	}
 
 

spring-test/src/test/java/org/springframework/test/context/junit/jupiter/nested/NestedTestsWithConstructorInjectionWithSpringAndJUnitJupiterTestCase.java

@@ -18,12 +18,14 @@
 
 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 @@
 /**
  * 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).
  *
  * <p>
  * 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 AutowiredConstructor {
 
 		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 @@ void nestedTest() throws Exception {
 
 	@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 @@ void nestedTest() throws Exception {
 		}
 	}
 
+	@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