From 2fd83aa764a522e52b4e8fd3bf9630fc0518939b Mon Sep 17 00:00:00 2001 From: Sam Brannen Date: Wed, 21 Jun 2023 13:44:01 +0200 Subject: [PATCH] Polish InitDestroyAnnotationBeanPostProcessor internals, etc. --- framework-platform/framework-platform.gradle | 1 + ...nitDestroyAnnotationBeanPostProcessor.java | 104 +++++++++--------- .../AbstractAutowireCapableBeanFactory.java | 6 +- spring-context/spring-context.gradle | 1 + .../InitDestroyMethodLifecycleTests.java | 38 +++++-- 5 files changed, 85 insertions(+), 65 deletions(-) diff --git a/framework-platform/framework-platform.gradle b/framework-platform/framework-platform.gradle index fca0e6e74854..1083a9334ce5 100644 --- a/framework-platform/framework-platform.gradle +++ b/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") diff --git a/spring-beans/src/main/java/org/springframework/beans/factory/annotation/InitDestroyAnnotationBeanPostProcessor.java b/spring-beans/src/main/java/org/springframework/beans/factory/annotation/InitDestroyAnnotationBeanPostProcessor.java index 952f84afe1f6..bd30995a0c38 100644 --- a/spring-beans/src/main/java/org/springframework/beans/factory/annotation/InitDestroyAnnotationBeanPostProcessor.java +++ b/spring-beans/src/main/java/org/springframework/beans/factory/annotation/InitDestroyAnnotationBeanPostProcessor.java @@ -24,7 +24,6 @@ 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 @@ * @author Juergen Hoeller * @author Stephane Nicoll * @author Phillip Webb + * @author Sam Brannen * @since 2.5 * @see #setInitAnnotationType * @see #setDestroyAnnotationType @@ -153,7 +153,7 @@ public int getOrder() { @Override public void postProcessMergedBeanDefinition(RootBeanDefinition beanDefinition, Class beanType, String beanName) { - findInjectionMetadata(beanDefinition, beanType); + findLifecycleMetadata(beanDefinition, beanType); } @Override @@ -161,7 +161,7 @@ public void postProcessMergedBeanDefinition(RootBeanDefinition beanDefinition, C 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 BeanRegistrationAotContribution processAheadOfTime(RegisteredBean registe 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 detectedElements) { - Stream detectedNames = detectedElements.stream().map(LifecycleElement::getIdentifier); + private static String[] safeMerge(@Nullable String[] existingNames, Collection detectedMethods) { + Stream detectedNames = detectedMethods.stream().map(LifecycleMethod::getIdentifier); Stream mergedNames = (existingNames != null ? Stream.concat(Stream.of(existingNames), detectedNames) : detectedNames); return mergedNames.distinct().toArray(String[]::new); @@ -217,12 +217,14 @@ public void postProcessBeforeDestruction(Object bean, String beanName) throws Be 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 @@ private LifecycleMetadata findLifecycleMetadata(Class clazz) { } 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 initMethods = new ArrayList<>(); - List destroyMethods = new ArrayList<>(); + List initMethods = new ArrayList<>(); + List destroyMethods = new ArrayList<>(); Class targetClass = clazz; do { - final List currInitMethods = new ArrayList<>(); - final List currDestroyMethods = new ArrayList<>(); + final List currInitMethods = new ArrayList<>(); + final List currDestroyMethods = new ArrayList<>(); ReflectionUtils.doWithLocalMethods(targetClass, method -> { if (this.initAnnotationType != null && method.isAnnotationPresent(this.initAnnotationType)) { - LifecycleElement element = new LifecycleElement(method); - currInitMethods.add(element); + currInitMethods.add(new LifecycleMethod(method)); 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 @@ private class LifecycleMetadata { private final Class targetClass; - private final Collection initMethods; + private final Collection initMethods; - private final Collection destroyMethods; + private final Collection destroyMethods; @Nullable - private volatile Set checkedInitMethods; + private volatile Set checkedInitMethods; @Nullable - private volatile Set checkedDestroyMethods; + private volatile Set checkedDestroyMethods; - public LifecycleMetadata(Class targetClass, Collection initMethods, - Collection destroyMethods) { + public LifecycleMetadata(Class targetClass, Collection initMethods, + Collection destroyMethods) { this.targetClass = targetClass; this.initMethods = initMethods; @@ -331,23 +332,23 @@ public LifecycleMetadata(Class targetClass, Collection init } public void checkInitDestroyMethods(RootBeanDefinition beanDefinition) { - Set checkedInitMethods = new LinkedHashSet<>(this.initMethods.size()); - for (LifecycleElement element : this.initMethods) { - String methodIdentifier = element.getIdentifier(); + Set checkedInitMethods = new LinkedHashSet<>(this.initMethods.size()); + for (LifecycleMethod lifecycleMethod : this.initMethods) { + 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 checkedDestroyMethods = new LinkedHashSet<>(this.destroyMethods.size()); - for (LifecycleElement element : this.destroyMethods) { - String methodIdentifier = element.getIdentifier(); + Set checkedDestroyMethods = new LinkedHashSet<>(this.destroyMethods.size()); + for (LifecycleMethod lifecycleMethod : this.destroyMethods) { + 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 void checkInitDestroyMethods(RootBeanDefinition beanDefinition) { } public void invokeInitMethods(Object target, String beanName) throws Throwable { - Collection checkedInitMethods = this.checkedInitMethods; - Collection initMethodsToIterate = + Collection checkedInitMethods = this.checkedInitMethods; + Collection 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 checkedDestroyMethods = this.checkedDestroyMethods; - Collection destroyMethodsToUse = + Collection checkedDestroyMethods = this.checkedDestroyMethods; + Collection 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 checkedDestroyMethods = this.checkedDestroyMethods; - Collection destroyMethodsToUse = + Collection checkedDestroyMethods = this.checkedDestroyMethods; + Collection destroyMethodsToUse = (checkedDestroyMethods != null ? checkedDestroyMethods : this.destroyMethods); return !destroyMethodsToUse.isEmpty(); } @@ -395,17 +396,17 @@ public boolean hasDestroyMethods() { /** - * 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 String getIdentifier() { 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 true; - } - if (!(other instanceof LifecycleElement otherElement)) { - return false; - } - return (this.identifier.equals(otherElement.identifier)); + return (this == other || (other instanceof LifecycleMethod that && + this.identifier.equals(that.identifier))); } @Override diff --git a/spring-beans/src/main/java/org/springframework/beans/factory/support/AbstractAutowireCapableBeanFactory.java b/spring-beans/src/main/java/org/springframework/beans/factory/support/AbstractAutowireCapableBeanFactory.java index 7e264e411e1a..0aa1f3fb4a9b 100644 --- a/spring-beans/src/main/java/org/springframework/beans/factory/support/AbstractAutowireCapableBeanFactory.java +++ b/spring-beans/src/main/java/org/springframework/beans/factory/support/AbstractAutowireCapableBeanFactory.java @@ -1832,9 +1832,9 @@ protected void invokeInitMethods(String beanName, Object bean, @Nullable RootBea /** * Invoke the specified custom init method on the given bean. - * Called by invokeInitMethods. - *

Can be overridden in subclasses for custom resolution of init - * methods with arguments. + *

Called by {@link #invokeInitMethods(String, Object, RootBeanDefinition)}. + *

Can be overridden in subclasses for custom resolution of init methods + * with arguments. * @see #invokeInitMethods */ protected void invokeCustomInitMethod(String beanName, Object bean, RootBeanDefinition mbd, String initMethodName) diff --git a/spring-context/spring-context.gradle b/spring-context/spring-context.gradle index 04d4e1e8c72f..3b93d1bc0c75 100644 --- a/spring-context/spring-context.gradle +++ b/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") diff --git a/spring-context/src/test/java/org/springframework/context/annotation/InitDestroyMethodLifecycleTests.java b/spring-context/src/test/java/org/springframework/context/annotation/InitDestroyMethodLifecycleTests.java index a481ce58b54c..13f5de72ca56 100644 --- a/spring-context/src/test/java/org/springframework/context/annotation/InitDestroyMethodLifecycleTests.java +++ b/spring-context/src/test/java/org/springframework/context/annotation/InitDestroyMethodLifecycleTests.java @@ -25,6 +25,7 @@ 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 @@ void initializingDisposableInterfacesWithShadowedMethods() { } @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 @@ void jsr250Annotations() { } @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 @@ void jsr250AnnotationsWithShadowedMethods() { } @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 @@ void jsr250AnnotationsWithCustomPrivateInitDestroyMethods() { } @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 @@ private static DefaultListableBeanFactory createBeanFactoryAndRegisterBean(Class 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; }