Polish InitDestroyAnnotationBeanPostProcessor internals, etc.

framework-platform/framework-platform.gradle

@@ -82,6 +82,7 @@ dependencies {
 		api("jakarta.websocket:jakarta.websocket-api:2.1.0")
 		api("jakarta.websocket:jakarta.websocket-client-api:2.1.0")
 		api("jakarta.xml.bind:jakarta.xml.bind-api:3.0.1")
+		api("javax.annotation:javax.annotation-api:1.3.2")
 		api("javax.cache:cache-api:1.1.1")
 		api("javax.money:money-api:1.1")
 		api("jaxen:jaxen:1.2.0")

spring-beans/src/main/java/org/springframework/beans/factory/annotation/InitDestroyAnnotationBeanPostProcessor.java

@@ -24,7 +24,6 @@ import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Method;
 import java.lang.reflect.Modifier;
 import java.util.ArrayList;
-import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.LinkedHashSet;
@@ -79,6 +78,7 @@ import org.springframework.util.ReflectionUtils;
  * @author Juergen Hoeller
  * @author Stephane Nicoll
  * @author Phillip Webb
+ * @author Sam Brannen
  * @since 2.5
  * @see #setInitAnnotationType
  * @see #setDestroyAnnotationType
@@ -153,7 +153,7 @@ public class InitDestroyAnnotationBeanPostProcessor implements DestructionAwareB
 
 	@Override
 	public void postProcessMergedBeanDefinition(RootBeanDefinition beanDefinition, Class<?> beanType, String beanName) {
-		findInjectionMetadata(beanDefinition, beanType);
+		findLifecycleMetadata(beanDefinition, beanType);
 	}
 
 	@Override
@@ -161,7 +161,7 @@ public class InitDestroyAnnotationBeanPostProcessor implements DestructionAwareB
 	public BeanRegistrationAotContribution processAheadOfTime(RegisteredBean registeredBean) {
 		RootBeanDefinition beanDefinition = registeredBean.getMergedBeanDefinition();
 		beanDefinition.resolveDestroyMethodIfNecessary();
-		LifecycleMetadata metadata = findInjectionMetadata(beanDefinition, registeredBean.getBeanClass());
+		LifecycleMetadata metadata = findLifecycleMetadata(beanDefinition, registeredBean.getBeanClass());
 		if (!CollectionUtils.isEmpty(metadata.initMethods)) {
 			String[] initMethodNames = safeMerge(beanDefinition.getInitMethodNames(), metadata.initMethods);
 			beanDefinition.setInitMethodNames(initMethodNames);
@@ -173,14 +173,14 @@ public class InitDestroyAnnotationBeanPostProcessor implements DestructionAwareB
 		return null;
 	}
 
-	private LifecycleMetadata findInjectionMetadata(RootBeanDefinition beanDefinition, Class<?> beanType) {
+	private LifecycleMetadata findLifecycleMetadata(RootBeanDefinition beanDefinition, Class<?> beanType) {
 		LifecycleMetadata metadata = findLifecycleMetadata(beanType);
 		metadata.checkInitDestroyMethods(beanDefinition);
 		return metadata;
 	}
 
-	private String[] safeMerge(@Nullable String[] existingNames, Collection<LifecycleElement> detectedElements) {
+	private static String[] safeMerge(@Nullable String[] existingNames, Collection<LifecycleMethod> detectedMethods) {
-		Stream<String> detectedNames = detectedElements.stream().map(LifecycleElement::getIdentifier);
+		Stream<String> detectedNames = detectedMethods.stream().map(LifecycleMethod::getIdentifier);
 		Stream<String> mergedNames = (existingNames != null ?
 				Stream.concat(Stream.of(existingNames), detectedNames) : detectedNames);
 		return mergedNames.distinct().toArray(String[]::new);
@@ -217,12 +217,14 @@ public class InitDestroyAnnotationBeanPostProcessor implements DestructionAwareB
 			if (logger.isDebugEnabled()) {
 				logger.warn(msg, ex.getTargetException());
 			}
-			else {
+			else if (logger.isWarnEnabled()) {
 				logger.warn(msg + ": " + ex.getTargetException());
 			}
 		}
 		catch (Throwable ex) {
-			logger.warn("Failed to invoke destroy method on bean with name '" + beanName + "'", ex);
+			if (logger.isWarnEnabled()) {
+				logger.warn("Failed to invoke destroy method on bean with name '" + beanName + "'", ex);
+			}
 		}
 	}
 
@@ -253,28 +255,27 @@ public class InitDestroyAnnotationBeanPostProcessor implements DestructionAwareB
 	}
 
 	private LifecycleMetadata buildLifecycleMetadata(final Class<?> clazz) {
-		if (!AnnotationUtils.isCandidateClass(clazz, Arrays.asList(this.initAnnotationType, this.destroyAnnotationType))) {
+		if (!AnnotationUtils.isCandidateClass(clazz, List.of(this.initAnnotationType, this.destroyAnnotationType))) {
 			return this.emptyLifecycleMetadata;
 		}
 
-		List<LifecycleElement> initMethods = new ArrayList<>();
+		List<LifecycleMethod> initMethods = new ArrayList<>();
-		List<LifecycleElement> destroyMethods = new ArrayList<>();
+		List<LifecycleMethod> destroyMethods = new ArrayList<>();
 		Class<?> targetClass = clazz;
 
 		do {
-			final List<LifecycleElement> currInitMethods = new ArrayList<>();
+			final List<LifecycleMethod> currInitMethods = new ArrayList<>();
-			final List<LifecycleElement> currDestroyMethods = new ArrayList<>();
+			final List<LifecycleMethod> currDestroyMethods = new ArrayList<>();
 
 			ReflectionUtils.doWithLocalMethods(targetClass, method -> {
 				if (this.initAnnotationType != null && method.isAnnotationPresent(this.initAnnotationType)) {
-					LifecycleElement element = new LifecycleElement(method);
+					currInitMethods.add(new LifecycleMethod(method));
-					currInitMethods.add(element);
 					if (logger.isTraceEnabled()) {
 						logger.trace("Found init method on class [" + clazz.getName() + "]: " + method);
 					}
 				}
 				if (this.destroyAnnotationType != null && method.isAnnotationPresent(this.destroyAnnotationType)) {
-					currDestroyMethods.add(new LifecycleElement(method));
+					currDestroyMethods.add(new LifecycleMethod(method));
 					if (logger.isTraceEnabled()) {
 						logger.trace("Found destroy method on class [" + clazz.getName() + "]: " + method);
 					}
@@ -312,18 +313,18 @@ public class InitDestroyAnnotationBeanPostProcessor implements DestructionAwareB
 
 		private final Class<?> targetClass;
 
-		private final Collection<LifecycleElement> initMethods;
+		private final Collection<LifecycleMethod> initMethods;
 
-		private final Collection<LifecycleElement> destroyMethods;
+		private final Collection<LifecycleMethod> destroyMethods;
 
 		@Nullable
-		private volatile Set<LifecycleElement> checkedInitMethods;
+		private volatile Set<LifecycleMethod> checkedInitMethods;
 
 		@Nullable
-		private volatile Set<LifecycleElement> checkedDestroyMethods;
+		private volatile Set<LifecycleMethod> checkedDestroyMethods;
 
-		public LifecycleMetadata(Class<?> targetClass, Collection<LifecycleElement> initMethods,
+		public LifecycleMetadata(Class<?> targetClass, Collection<LifecycleMethod> initMethods,
-				Collection<LifecycleElement> destroyMethods) {
+				Collection<LifecycleMethod> destroyMethods) {
 
 			this.targetClass = targetClass;
 			this.initMethods = initMethods;
@@ -331,23 +332,23 @@ public class InitDestroyAnnotationBeanPostProcessor implements DestructionAwareB
 		}
 
 		public void checkInitDestroyMethods(RootBeanDefinition beanDefinition) {
-			Set<LifecycleElement> checkedInitMethods = new LinkedHashSet<>(this.initMethods.size());
+			Set<LifecycleMethod> checkedInitMethods = new LinkedHashSet<>(this.initMethods.size());
-			for (LifecycleElement element : this.initMethods) {
+			for (LifecycleMethod lifecycleMethod : this.initMethods) {
-				String methodIdentifier = element.getIdentifier();
+				String methodIdentifier = lifecycleMethod.getIdentifier();
 				if (!beanDefinition.isExternallyManagedInitMethod(methodIdentifier)) {
 					beanDefinition.registerExternallyManagedInitMethod(methodIdentifier);
-					checkedInitMethods.add(element);
+					checkedInitMethods.add(lifecycleMethod);
 					if (logger.isTraceEnabled()) {
 						logger.trace("Registered init method on class [" + this.targetClass.getName() + "]: " + methodIdentifier);
 					}
 				}
 			}
-			Set<LifecycleElement> checkedDestroyMethods = new LinkedHashSet<>(this.destroyMethods.size());
+			Set<LifecycleMethod> checkedDestroyMethods = new LinkedHashSet<>(this.destroyMethods.size());
-			for (LifecycleElement element : this.destroyMethods) {
+			for (LifecycleMethod lifecycleMethod : this.destroyMethods) {
-				String methodIdentifier = element.getIdentifier();
+				String methodIdentifier = lifecycleMethod.getIdentifier();
 				if (!beanDefinition.isExternallyManagedDestroyMethod(methodIdentifier)) {
 					beanDefinition.registerExternallyManagedDestroyMethod(methodIdentifier);
-					checkedDestroyMethods.add(element);
+					checkedDestroyMethods.add(lifecycleMethod);
 					if (logger.isTraceEnabled()) {
 						logger.trace("Registered destroy method on class [" + this.targetClass.getName() + "]: " + methodIdentifier);
 					}
@@ -358,36 +359,36 @@ public class InitDestroyAnnotationBeanPostProcessor implements DestructionAwareB
 		}
 
 		public void invokeInitMethods(Object target, String beanName) throws Throwable {
-			Collection<LifecycleElement> checkedInitMethods = this.checkedInitMethods;
+			Collection<LifecycleMethod> checkedInitMethods = this.checkedInitMethods;
-			Collection<LifecycleElement> initMethodsToIterate =
+			Collection<LifecycleMethod> initMethodsToIterate =
 					(checkedInitMethods != null ? checkedInitMethods : this.initMethods);
 			if (!initMethodsToIterate.isEmpty()) {
-				for (LifecycleElement element : initMethodsToIterate) {
+				for (LifecycleMethod lifecycleMethod : initMethodsToIterate) {
 					if (logger.isTraceEnabled()) {
-						logger.trace("Invoking init method on bean '" + beanName + "': " + element.getMethod());
+						logger.trace("Invoking init method on bean '" + beanName + "': " + lifecycleMethod.getMethod());
 					}
-					element.invoke(target);
+					lifecycleMethod.invoke(target);
 				}
 			}
 		}
 
 		public void invokeDestroyMethods(Object target, String beanName) throws Throwable {
-			Collection<LifecycleElement> checkedDestroyMethods = this.checkedDestroyMethods;
+			Collection<LifecycleMethod> checkedDestroyMethods = this.checkedDestroyMethods;
-			Collection<LifecycleElement> destroyMethodsToUse =
+			Collection<LifecycleMethod> destroyMethodsToUse =
 					(checkedDestroyMethods != null ? checkedDestroyMethods : this.destroyMethods);
 			if (!destroyMethodsToUse.isEmpty()) {
-				for (LifecycleElement element : destroyMethodsToUse) {
+				for (LifecycleMethod lifecycleMethod : destroyMethodsToUse) {
 					if (logger.isTraceEnabled()) {
-						logger.trace("Invoking destroy method on bean '" + beanName + "': " + element.getMethod());
+						logger.trace("Invoking destroy method on bean '" + beanName + "': " + lifecycleMethod.getMethod());
 					}
-					element.invoke(target);
+					lifecycleMethod.invoke(target);
 				}
 			}
 		}
 
 		public boolean hasDestroyMethods() {
-			Collection<LifecycleElement> checkedDestroyMethods = this.checkedDestroyMethods;
+			Collection<LifecycleMethod> checkedDestroyMethods = this.checkedDestroyMethods;
-			Collection<LifecycleElement> destroyMethodsToUse =
+			Collection<LifecycleMethod> destroyMethodsToUse =
 					(checkedDestroyMethods != null ? checkedDestroyMethods : this.destroyMethods);
 			return !destroyMethodsToUse.isEmpty();
 		}
@@ -395,17 +396,17 @@ public class InitDestroyAnnotationBeanPostProcessor implements DestructionAwareB
 
 
 	/**
-	 * Class representing injection information about an annotated method.
+	 * Class representing an annotated init or destroy method.
 	 */
-	private static class LifecycleElement {
+	private static class LifecycleMethod {
 
 		private final Method method;
 
 		private final String identifier;
 
-		public LifecycleElement(Method method) {
+		public LifecycleMethod(Method method) {
 			if (method.getParameterCount() != 0) {
-				throw new IllegalStateException("Lifecycle method annotation requires a no-arg method: " + method);
+				throw new IllegalStateException("Lifecycle annotation requires a no-arg method: " + method);
 			}
 			this.method = method;
 			this.identifier = (Modifier.isPrivate(method.getModifiers()) ?
@@ -422,18 +423,13 @@ public class InitDestroyAnnotationBeanPostProcessor implements DestructionAwareB
 
 		public void invoke(Object target) throws Throwable {
 			ReflectionUtils.makeAccessible(this.method);
-			this.method.invoke(target, (Object[]) null);
+			this.method.invoke(target);
 		}
 
 		@Override
 		public boolean equals(@Nullable Object other) {
-			if (this == other) {
+			return (this == other || (other instanceof LifecycleMethod that &&
-				return true;
+					this.identifier.equals(that.identifier)));
-			}
-			if (!(other instanceof LifecycleElement otherElement)) {
-				return false;
-			}
-			return (this.identifier.equals(otherElement.identifier));
 		}
 
 		@Override

spring-beans/src/main/java/org/springframework/beans/factory/support/AbstractAutowireCapableBeanFactory.java

@@ -1832,9 +1832,9 @@ public abstract class AbstractAutowireCapableBeanFactory extends AbstractBeanFac
 
 	/**
 	 * Invoke the specified custom init method on the given bean.
-	 * Called by invokeInitMethods.
+	 * <p>Called by {@link #invokeInitMethods(String, Object, RootBeanDefinition)}.
-	 * <p>Can be overridden in subclasses for custom resolution of init
+	 * <p>Can be overridden in subclasses for custom resolution of init methods
-	 * methods with arguments.
+	 * with arguments.
 	 * @see #invokeInitMethods
 	 */
 	protected void invokeCustomInitMethod(String beanName, Object bean, RootBeanDefinition mbd, String initMethodName)

spring-context/spring-context.gradle

@@ -36,6 +36,7 @@ dependencies {
 	testImplementation("org.apache.commons:commons-pool2")
 	testImplementation("org.awaitility:awaitility")
 	testImplementation("jakarta.inject:jakarta.inject-tck")
+	testImplementation("javax.annotation:javax.annotation-api")
 	testImplementation("org.jetbrains.kotlinx:kotlinx-coroutines-core")
 	testRuntimeOnly("jakarta.xml.bind:jakarta.xml.bind-api")
 	testRuntimeOnly("org.glassfish:jakarta.el")

spring-context/src/test/java/org/springframework/context/annotation/InitDestroyMethodLifecycleTests.java

@@ -25,6 +25,7 @@ import org.junit.jupiter.api.Test;
 
 import org.springframework.beans.factory.DisposableBean;
 import org.springframework.beans.factory.InitializingBean;
+import org.springframework.beans.factory.annotation.InitDestroyAnnotationBeanPostProcessor;
 import org.springframework.beans.factory.support.DefaultListableBeanFactory;
 import org.springframework.beans.factory.support.RootBeanDefinition;
 
@@ -80,7 +81,7 @@ class InitDestroyMethodLifecycleTests {
 	}
 
 	@Test
-	void jsr250Annotations() {
+	void jakartaAnnotations() {
 		Class<?> beanClass = CustomAnnotatedInitDestroyBean.class;
 		DefaultListableBeanFactory beanFactory = createBeanFactoryAndRegisterBean(beanClass, "customInit", "customDestroy");
 		CustomAnnotatedInitDestroyBean bean = beanFactory.getBean(CustomAnnotatedInitDestroyBean.class);
@@ -90,7 +91,7 @@ class InitDestroyMethodLifecycleTests {
 	}
 
 	@Test
-	void jsr250AnnotationsWithShadowedMethods() {
+	void jakartaAnnotationsWithShadowedMethods() {
 		Class<?> beanClass = CustomAnnotatedInitDestroyWithShadowedMethodsBean.class;
 		DefaultListableBeanFactory beanFactory = createBeanFactoryAndRegisterBean(beanClass, "customInit", "customDestroy");
 		CustomAnnotatedInitDestroyWithShadowedMethodsBean bean = beanFactory.getBean(CustomAnnotatedInitDestroyWithShadowedMethodsBean.class);
@@ -100,7 +101,7 @@ class InitDestroyMethodLifecycleTests {
 	}
 
 	@Test
-	void jsr250AnnotationsWithCustomPrivateInitDestroyMethods() {
+	void jakartaAnnotationsWithCustomPrivateInitDestroyMethods() {
 		Class<?> beanClass = CustomAnnotatedPrivateInitDestroyBean.class;
 		DefaultListableBeanFactory beanFactory = createBeanFactoryAndRegisterBean(beanClass, "customInit1", "customDestroy1");
 		CustomAnnotatedPrivateInitDestroyBean bean = beanFactory.getBean(CustomAnnotatedPrivateInitDestroyBean.class);
@@ -110,13 +111,25 @@ class InitDestroyMethodLifecycleTests {
 	}
 
 	@Test
-	void jsr250AnnotationsWithCustomSameMethodNames() {
+	void jakartaAnnotationsCustomPrivateInitDestroyMethodsWithTheSameMethodNames() {
 		Class<?> beanClass = CustomAnnotatedPrivateSameNameInitDestroyBean.class;
-		DefaultListableBeanFactory beanFactory = createBeanFactoryAndRegisterBean(beanClass, "customInit1", "customDestroy1");
+		DefaultListableBeanFactory beanFactory = createBeanFactoryAndRegisterBean(beanClass, "customInit", "customDestroy");
 		CustomAnnotatedPrivateSameNameInitDestroyBean bean = beanFactory.getBean(CustomAnnotatedPrivateSameNameInitDestroyBean.class);
-		assertThat(bean.initMethods).as("init-methods").containsExactly("@PostConstruct.privateCustomInit1", "@PostConstruct.sameNameCustomInit1", "afterPropertiesSet");
+
+		assertThat(bean.initMethods).as("init-methods").containsExactly(
+				"@PostConstruct.privateCustomInit1",
+				"@PostConstruct.sameNameCustomInit1",
+				"afterPropertiesSet",
+				"customInit"
+			);
+
 		beanFactory.destroySingletons();
-		assertThat(bean.destroyMethods).as("destroy-methods").containsExactly("@PreDestroy.sameNameCustomDestroy1", "@PreDestroy.privateCustomDestroy1", "destroy");
+		assertThat(bean.destroyMethods).as("destroy-methods").containsExactly(
+				"@PreDestroy.sameNameCustomDestroy1",
+				"@PreDestroy.privateCustomDestroy1",
+				"destroy",
+				"customDestroy"
+			);
 	}
 
 	@Test
@@ -134,10 +147,19 @@ class InitDestroyMethodLifecycleTests {
 			String initMethodName, String destroyMethodName) {
 
 		DefaultListableBeanFactory beanFactory = new DefaultListableBeanFactory();
+		beanFactory.addBeanPostProcessor(new CommonAnnotationBeanPostProcessor());
+
+		// Configure and register an InitDestroyAnnotationBeanPostProcessor as
+		// done in AnnotationConfigUtils.registerAnnotationConfigProcessors()
+		// for an ApplicatonContext.
+		InitDestroyAnnotationBeanPostProcessor initDestroyBpp = new InitDestroyAnnotationBeanPostProcessor();
+		initDestroyBpp.setInitAnnotationType(javax.annotation.PostConstruct.class);
+		initDestroyBpp.setDestroyAnnotationType(javax.annotation.PreDestroy.class);
+		beanFactory.addBeanPostProcessor(initDestroyBpp);
+
 		RootBeanDefinition beanDefinition = new RootBeanDefinition(beanClass);
 		beanDefinition.setInitMethodName(initMethodName);
 		beanDefinition.setDestroyMethodName(destroyMethodName);
-		beanFactory.addBeanPostProcessor(new CommonAnnotationBeanPostProcessor());
 		beanFactory.registerBeanDefinition("lifecycleTestBean", beanDefinition);
 		return beanFactory;
 	}