From a7d5fbfbea1b1c4948a8f8cc5541f90bd2a1d972 Mon Sep 17 00:00:00 2001 From: Sam Brannen Date: Tue, 1 Mar 2022 13:27:37 +0100 Subject: [PATCH 1/6] Fix log messages for init/destroy method registration --- .../annotation/InitDestroyAnnotationBeanPostProcessor.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) 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 f76a03b8dc9d..97e65f155816 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 @@ -1,5 +1,5 @@ /* - * Copyright 2002-2019 the original author or authors. + * Copyright 2002-2022 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. @@ -302,7 +302,7 @@ public void checkConfigMembers(RootBeanDefinition beanDefinition) { beanDefinition.registerExternallyManagedInitMethod(methodIdentifier); checkedInitMethods.add(element); if (logger.isTraceEnabled()) { - logger.trace("Registered init method on class [" + this.targetClass.getName() + "]: " + element); + logger.trace("Registered init method on class [" + this.targetClass.getName() + "]: " + methodIdentifier); } } } @@ -313,7 +313,7 @@ public void checkConfigMembers(RootBeanDefinition beanDefinition) { beanDefinition.registerExternallyManagedDestroyMethod(methodIdentifier); checkedDestroyMethods.add(element); if (logger.isTraceEnabled()) { - logger.trace("Registered destroy method on class [" + this.targetClass.getName() + "]: " + element); + logger.trace("Registered destroy method on class [" + this.targetClass.getName() + "]: " + methodIdentifier); } } } From f96872404d9ab495a2821955b649fec9a28f92ed Mon Sep 17 00:00:00 2001 From: Sam Brannen Date: Tue, 1 Mar 2022 15:02:57 +0100 Subject: [PATCH 2/6] Ensure private init/destroy method is invoked only once Closes gh-28083 --- .../AbstractAutowireCapableBeanFactory.java | 4 +- .../support/DisposableBeanAdapter.java | 5 +- .../factory/support/RootBeanDefinition.java | 63 ++++++++++++++++++- 3 files changed, 67 insertions(+), 5 deletions(-) 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 7a94a91c86a8..c1e764b22a5f 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 @@ -1844,7 +1844,7 @@ protected void invokeInitMethods(String beanName, Object bean, @Nullable RootBea throws Throwable { boolean isInitializingBean = (bean instanceof InitializingBean); - if (isInitializingBean && (mbd == null || !mbd.isExternallyManagedInitMethod("afterPropertiesSet"))) { + if (isInitializingBean && (mbd == null || !mbd.hasAnyExternallyManagedInitMethod("afterPropertiesSet"))) { if (logger.isTraceEnabled()) { logger.trace("Invoking afterPropertiesSet() on bean with name '" + beanName + "'"); } @@ -1868,7 +1868,7 @@ protected void invokeInitMethods(String beanName, Object bean, @Nullable RootBea String initMethodName = mbd.getInitMethodName(); if (StringUtils.hasLength(initMethodName) && !(isInitializingBean && "afterPropertiesSet".equals(initMethodName)) && - !mbd.isExternallyManagedInitMethod(initMethodName)) { + !mbd.hasAnyExternallyManagedInitMethod(initMethodName)) { invokeCustomInitMethod(beanName, bean, mbd); } } diff --git a/spring-beans/src/main/java/org/springframework/beans/factory/support/DisposableBeanAdapter.java b/spring-beans/src/main/java/org/springframework/beans/factory/support/DisposableBeanAdapter.java index b5fdef4dd813..4317ae901c9b 100644 --- a/spring-beans/src/main/java/org/springframework/beans/factory/support/DisposableBeanAdapter.java +++ b/spring-beans/src/main/java/org/springframework/beans/factory/support/DisposableBeanAdapter.java @@ -52,6 +52,7 @@ * @author Juergen Hoeller * @author Costin Leau * @author Stephane Nicoll + * @author Sam Brannen * @since 2.0 * @see AbstractBeanFactory * @see org.springframework.beans.factory.DisposableBean @@ -109,12 +110,12 @@ public DisposableBeanAdapter(Object bean, String beanName, RootBeanDefinition be this.beanName = beanName; this.nonPublicAccessAllowed = beanDefinition.isNonPublicAccessAllowed(); this.invokeDisposableBean = (bean instanceof DisposableBean && - !beanDefinition.isExternallyManagedDestroyMethod(DESTROY_METHOD_NAME)); + !beanDefinition.hasAnyExternallyManagedDestroyMethod(DESTROY_METHOD_NAME)); String destroyMethodName = inferDestroyMethodIfNecessary(bean, beanDefinition); if (destroyMethodName != null && !(this.invokeDisposableBean && DESTROY_METHOD_NAME.equals(destroyMethodName)) && - !beanDefinition.isExternallyManagedDestroyMethod(destroyMethodName)) { + !beanDefinition.hasAnyExternallyManagedDestroyMethod(destroyMethodName)) { this.invokeAutoCloseable = (bean instanceof AutoCloseable && CLOSE_METHOD_NAME.equals(destroyMethodName)); if (!this.invokeAutoCloseable) { diff --git a/spring-beans/src/main/java/org/springframework/beans/factory/support/RootBeanDefinition.java b/spring-beans/src/main/java/org/springframework/beans/factory/support/RootBeanDefinition.java index f4fdafa3767e..13638768efa1 100644 --- a/spring-beans/src/main/java/org/springframework/beans/factory/support/RootBeanDefinition.java +++ b/spring-beans/src/main/java/org/springframework/beans/factory/support/RootBeanDefinition.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2021 the original author or authors. + * Copyright 2002-2022 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. @@ -49,6 +49,7 @@ * * @author Rod Johnson * @author Juergen Hoeller + * @author Sam Brannen * @see GenericBeanDefinition * @see ChildBeanDefinition */ @@ -479,6 +480,36 @@ public boolean isExternallyManagedInitMethod(String initMethod) { } } + /** + * Determine if the given method name indicates an externally managed + * initialization method, regardless of method visibility. + *

In contrast to {@link #isExternallyManagedInitMethod(String)}, this + * method also returns {@code true} if there is a {@code private} external + * init method that has been + * {@linkplain #registerExternallyManagedInitMethod(String) registered} + * using a fully qualified method name instead of a simple method name. + * @since 5.3.17 + */ + boolean hasAnyExternallyManagedInitMethod(String initMethod) { + synchronized (this.postProcessingLock) { + if (isExternallyManagedInitMethod(initMethod)) { + return true; + } + if (this.externallyManagedInitMethods != null) { + for (String candidate : this.externallyManagedInitMethods) { + int indexOfDot = candidate.lastIndexOf("."); + if (indexOfDot >= 0) { + String methodName = candidate.substring(indexOfDot + 1); + if (methodName.equals(initMethod)) { + return true; + } + } + } + } + return false; + } + } + /** * Return all externally managed initialization methods (as an immutable Set). * @since 5.3.11 @@ -513,6 +544,36 @@ public boolean isExternallyManagedDestroyMethod(String destroyMethod) { } } + /** + * Determine if the given method name indicates an externally managed + * destruction method, regardless of method visibility. + *

In contrast to {@link #isExternallyManagedDestroyMethod(String)}, this + * method also returns {@code true} if there is a {@code private} external + * destroy method that has been + * {@linkplain #registerExternallyManagedDestroyMethod(String) registered} + * using a fully qualified method name instead of a simple method name. + * @since 5.3.17 + */ + boolean hasAnyExternallyManagedDestroyMethod(String destroyMethod) { + synchronized (this.postProcessingLock) { + if (isExternallyManagedDestroyMethod(destroyMethod)) { + return true; + } + if (this.externallyManagedDestroyMethods != null) { + for (String candidate : this.externallyManagedDestroyMethods) { + int indexOfDot = candidate.lastIndexOf("."); + if (indexOfDot >= 0) { + String methodName = candidate.substring(indexOfDot + 1); + if (methodName.equals(destroyMethod)) { + return true; + } + } + } + } + return false; + } + } + /** * Return all externally managed destruction methods (as an immutable Set). * @since 5.3.11 From af14eea1ef76576acd07ba90cd0a656bd6b31969 Mon Sep 17 00:00:00 2001 From: Vikey Chen Date: Tue, 1 Mar 2022 00:49:43 +0800 Subject: [PATCH 3/6] Introduce tests for gh-28083 --- .../Spr3775InitDestroyLifecycleTests.java | 49 +++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/spring-context/src/test/java/org/springframework/context/annotation/Spr3775InitDestroyLifecycleTests.java b/spring-context/src/test/java/org/springframework/context/annotation/Spr3775InitDestroyLifecycleTests.java index 0fb249595f43..8fb07dc306f9 100644 --- a/spring-context/src/test/java/org/springframework/context/annotation/Spr3775InitDestroyLifecycleTests.java +++ b/spring-context/src/test/java/org/springframework/context/annotation/Spr3775InitDestroyLifecycleTests.java @@ -149,6 +149,30 @@ public void testJsr250AnnotationsWithShadowedMethods() { bean.destroyMethods); } + @Test + public void testJsr250AnnotationsWithCustomPrivateInitDestroyMethods() { + Class beanClass = CustomAnnotatedPrivateInitDestroyBean.class; + DefaultListableBeanFactory beanFactory = createBeanFactoryAndRegisterBean(beanClass, "customInit1", "customDestroy1"); + CustomAnnotatedPrivateInitDestroyBean bean = + (CustomAnnotatedPrivateInitDestroyBean) beanFactory.getBean(LIFECYCLE_TEST_BEAN); + assertMethodOrdering("init-methods", Arrays.asList("privateCustomInit1","afterPropertiesSet"), bean.initMethods); + beanFactory.destroySingletons(); + assertMethodOrdering("destroy-methods", Arrays.asList("privateCustomDestroy1","destroy"), bean.destroyMethods); + } + + @Test + public void testJsr250AnnotationsWithCustomSameMethodNames() { + Class beanClass = CustomAnnotatedPrivateSameNameInitDestroyBean.class; + DefaultListableBeanFactory beanFactory = createBeanFactoryAndRegisterBean(beanClass, "customInit1", "customDestroy1"); + CustomAnnotatedPrivateSameNameInitDestroyBean bean = + (CustomAnnotatedPrivateSameNameInitDestroyBean) beanFactory.getBean(LIFECYCLE_TEST_BEAN); + assertMethodOrdering("init-methods", + Arrays.asList("privateCustomInit1","afterPropertiesSet","sameNameCustomInit1"), bean.initMethods); + beanFactory.destroySingletons(); + assertMethodOrdering("destroy-methods", + Arrays.asList("privateCustomDestroy1","destroy","sameNameCustomDestroy1"), bean.destroyMethods); + } + @Test public void testAllLifecycleMechanismsAtOnce() { final Class beanClass = AllInOneBean.class; @@ -205,6 +229,31 @@ public void customDestroy() throws Exception { } } + public static class CustomAnnotatedPrivateInitDestroyBean extends CustomInitializingDisposableBean{ + + @PostConstruct + private void customInit1() throws Exception { + this.initMethods.add("privateCustomInit1"); + } + + @PreDestroy + private void customDestroy1() throws Exception { + this.destroyMethods.add("privateCustomDestroy1"); + } + + } + + public static class CustomAnnotatedPrivateSameNameInitDestroyBean extends CustomAnnotatedPrivateInitDestroyBean { + + private void customInit1() throws Exception { + this.initMethods.add("sameNameCustomInit1"); + } + + private void customDestroy1() throws Exception { + this.destroyMethods.add("sameNameCustomDestroy1"); + } + + } public static class CustomInitializingDisposableBean extends CustomInitDestroyBean implements InitializingBean, DisposableBean { From a524857bd548dbb3245771a374e9ef609dfae7c0 Mon Sep 17 00:00:00 2001 From: Sam Brannen Date: Tue, 1 Mar 2022 15:22:34 +0100 Subject: [PATCH 4/6] Fix init/destroy lifecycle method tests See gh-28083 --- .../Spr3775InitDestroyLifecycleTests.java | 28 +++++++++++-------- 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/spring-context/src/test/java/org/springframework/context/annotation/Spr3775InitDestroyLifecycleTests.java b/spring-context/src/test/java/org/springframework/context/annotation/Spr3775InitDestroyLifecycleTests.java index 8fb07dc306f9..4de6652e65a3 100644 --- a/spring-context/src/test/java/org/springframework/context/annotation/Spr3775InitDestroyLifecycleTests.java +++ b/spring-context/src/test/java/org/springframework/context/annotation/Spr3775InitDestroyLifecycleTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2019 the original author or authors. + * Copyright 2002-2022 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,9 +155,9 @@ public void testJsr250AnnotationsWithCustomPrivateInitDestroyMethods() { DefaultListableBeanFactory beanFactory = createBeanFactoryAndRegisterBean(beanClass, "customInit1", "customDestroy1"); CustomAnnotatedPrivateInitDestroyBean bean = (CustomAnnotatedPrivateInitDestroyBean) beanFactory.getBean(LIFECYCLE_TEST_BEAN); - assertMethodOrdering("init-methods", Arrays.asList("privateCustomInit1","afterPropertiesSet"), bean.initMethods); + assertMethodOrdering(beanClass, "init-methods", Arrays.asList("@PostConstruct.privateCustomInit1", "afterPropertiesSet"), bean.initMethods); beanFactory.destroySingletons(); - assertMethodOrdering("destroy-methods", Arrays.asList("privateCustomDestroy1","destroy"), bean.destroyMethods); + assertMethodOrdering(beanClass, "destroy-methods", Arrays.asList("@PreDestroy.privateCustomDestroy1", "destroy"), bean.destroyMethods); } @Test @@ -166,11 +166,11 @@ public void testJsr250AnnotationsWithCustomSameMethodNames() { DefaultListableBeanFactory beanFactory = createBeanFactoryAndRegisterBean(beanClass, "customInit1", "customDestroy1"); CustomAnnotatedPrivateSameNameInitDestroyBean bean = (CustomAnnotatedPrivateSameNameInitDestroyBean) beanFactory.getBean(LIFECYCLE_TEST_BEAN); - assertMethodOrdering("init-methods", - Arrays.asList("privateCustomInit1","afterPropertiesSet","sameNameCustomInit1"), bean.initMethods); + assertMethodOrdering(beanClass, "init-methods", + Arrays.asList("@PostConstruct.privateCustomInit1", "@PostConstruct.sameNameCustomInit1", "afterPropertiesSet"), bean.initMethods); beanFactory.destroySingletons(); - assertMethodOrdering("destroy-methods", - Arrays.asList("privateCustomDestroy1","destroy","sameNameCustomDestroy1"), bean.destroyMethods); + assertMethodOrdering(beanClass, "destroy-methods", + Arrays.asList("@PreDestroy.sameNameCustomDestroy1", "@PreDestroy.privateCustomDestroy1", "destroy"), bean.destroyMethods); } @Test @@ -229,28 +229,32 @@ public void customDestroy() throws Exception { } } - public static class CustomAnnotatedPrivateInitDestroyBean extends CustomInitializingDisposableBean{ + public static class CustomAnnotatedPrivateInitDestroyBean extends CustomInitializingDisposableBean { @PostConstruct private void customInit1() throws Exception { - this.initMethods.add("privateCustomInit1"); + this.initMethods.add("@PostConstruct.privateCustomInit1"); } @PreDestroy private void customDestroy1() throws Exception { - this.destroyMethods.add("privateCustomDestroy1"); + this.destroyMethods.add("@PreDestroy.privateCustomDestroy1"); } } public static class CustomAnnotatedPrivateSameNameInitDestroyBean extends CustomAnnotatedPrivateInitDestroyBean { + @PostConstruct + @SuppressWarnings("unused") private void customInit1() throws Exception { - this.initMethods.add("sameNameCustomInit1"); + this.initMethods.add("@PostConstruct.sameNameCustomInit1"); } + @PreDestroy + @SuppressWarnings("unused") private void customDestroy1() throws Exception { - this.destroyMethods.add("sameNameCustomDestroy1"); + this.destroyMethods.add("@PreDestroy.sameNameCustomDestroy1"); } } From dcdea986f6316344705aa0b70d79a4c60ab96121 Mon Sep 17 00:00:00 2001 From: Sam Brannen Date: Tue, 1 Mar 2022 15:43:25 +0100 Subject: [PATCH 5/6] Polish init/destroy lifecycle method tests See gh-28083 --- .../InitDestroyMethodLifecycleTests.java | 279 +++++++++++++++ .../Spr3775InitDestroyLifecycleTests.java | 325 ------------------ 2 files changed, 279 insertions(+), 325 deletions(-) create mode 100644 spring-context/src/test/java/org/springframework/context/annotation/InitDestroyMethodLifecycleTests.java delete mode 100644 spring-context/src/test/java/org/springframework/context/annotation/Spr3775InitDestroyLifecycleTests.java 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 new file mode 100644 index 000000000000..034452bc0512 --- /dev/null +++ b/spring-context/src/test/java/org/springframework/context/annotation/InitDestroyMethodLifecycleTests.java @@ -0,0 +1,279 @@ +/* + * Copyright 2002-2022 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. + * You may obtain a copy of the License at + * + * https://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 org.springframework.context.annotation; + +import java.util.ArrayList; +import java.util.List; + +import javax.annotation.PostConstruct; +import javax.annotation.PreDestroy; + +import org.junit.jupiter.api.Test; + +import org.springframework.beans.factory.DisposableBean; +import org.springframework.beans.factory.InitializingBean; +import org.springframework.beans.factory.support.DefaultListableBeanFactory; +import org.springframework.beans.factory.support.RootBeanDefinition; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * Tests which verify expected init and destroy bean lifecycle + * behavior as requested in + * SPR-3775. + * + *

Specifically, combinations of the following are tested: + *

+ * + * @author Sam Brannen + * @since 2.5 + */ +class InitDestroyMethodLifecycleTests { + + @Test + void initDestroyMethods() { + Class beanClass = InitDestroyBean.class; + DefaultListableBeanFactory beanFactory = createBeanFactoryAndRegisterBean(beanClass, "afterPropertiesSet", "destroy"); + InitDestroyBean bean = beanFactory.getBean(InitDestroyBean.class); + assertThat(bean.initMethods).as("init-methods").containsExactly("afterPropertiesSet"); + beanFactory.destroySingletons(); + assertThat(bean.destroyMethods).as("destroy-methods").containsExactly("destroy"); + } + + @Test + void initializingDisposableInterfaces() { + Class beanClass = CustomInitializingDisposableBean.class; + DefaultListableBeanFactory beanFactory = createBeanFactoryAndRegisterBean(beanClass, "customInit", "customDestroy"); + CustomInitializingDisposableBean bean = beanFactory.getBean(CustomInitializingDisposableBean.class); + assertThat(bean.initMethods).as("init-methods").containsExactly("afterPropertiesSet", "customInit"); + beanFactory.destroySingletons(); + assertThat(bean.destroyMethods).as("destroy-methods").containsExactly("destroy", "customDestroy"); + } + + @Test + void initializingDisposableInterfacesWithShadowedMethods() { + Class beanClass = InitializingDisposableWithShadowedMethodsBean.class; + DefaultListableBeanFactory beanFactory = createBeanFactoryAndRegisterBean(beanClass, "afterPropertiesSet", "destroy"); + InitializingDisposableWithShadowedMethodsBean bean = beanFactory.getBean(InitializingDisposableWithShadowedMethodsBean.class); + assertThat(bean.initMethods).as("init-methods").containsExactly("InitializingBean.afterPropertiesSet"); + beanFactory.destroySingletons(); + assertThat(bean.destroyMethods).as("destroy-methods").containsExactly("DisposableBean.destroy"); + } + + @Test + void jsr250Annotations() { + Class beanClass = CustomAnnotatedInitDestroyBean.class; + DefaultListableBeanFactory beanFactory = createBeanFactoryAndRegisterBean(beanClass, "customInit", "customDestroy"); + CustomAnnotatedInitDestroyBean bean = beanFactory.getBean(CustomAnnotatedInitDestroyBean.class); + assertThat(bean.initMethods).as("init-methods").containsExactly("postConstruct", "afterPropertiesSet", "customInit"); + beanFactory.destroySingletons(); + assertThat(bean.destroyMethods).as("destroy-methods").containsExactly("preDestroy", "destroy", "customDestroy"); + } + + @Test + void jsr250AnnotationsWithShadowedMethods() { + Class beanClass = CustomAnnotatedInitDestroyWithShadowedMethodsBean.class; + DefaultListableBeanFactory beanFactory = createBeanFactoryAndRegisterBean(beanClass, "customInit", "customDestroy"); + CustomAnnotatedInitDestroyWithShadowedMethodsBean bean = beanFactory.getBean(CustomAnnotatedInitDestroyWithShadowedMethodsBean.class); + assertThat(bean.initMethods).as("init-methods").containsExactly("@PostConstruct.afterPropertiesSet", "customInit"); + beanFactory.destroySingletons(); + assertThat(bean.destroyMethods).as("destroy-methods").containsExactly("@PreDestroy.destroy", "customDestroy"); + } + + @Test + void jsr250AnnotationsWithCustomPrivateInitDestroyMethods() { + Class beanClass = CustomAnnotatedPrivateInitDestroyBean.class; + DefaultListableBeanFactory beanFactory = createBeanFactoryAndRegisterBean(beanClass, "customInit1", "customDestroy1"); + CustomAnnotatedPrivateInitDestroyBean bean = beanFactory.getBean(CustomAnnotatedPrivateInitDestroyBean.class); + assertThat(bean.initMethods).as("init-methods").containsExactly("@PostConstruct.privateCustomInit1", "afterPropertiesSet"); + beanFactory.destroySingletons(); + assertThat(bean.destroyMethods).as("destroy-methods").containsExactly("@PreDestroy.privateCustomDestroy1", "destroy"); + } + + @Test + void jsr250AnnotationsWithCustomSameMethodNames() { + Class beanClass = CustomAnnotatedPrivateSameNameInitDestroyBean.class; + DefaultListableBeanFactory beanFactory = createBeanFactoryAndRegisterBean(beanClass, "customInit1", "customDestroy1"); + CustomAnnotatedPrivateSameNameInitDestroyBean bean = beanFactory.getBean(CustomAnnotatedPrivateSameNameInitDestroyBean.class); + assertThat(bean.initMethods).as("init-methods").containsExactly("@PostConstruct.privateCustomInit1", "@PostConstruct.sameNameCustomInit1", "afterPropertiesSet"); + beanFactory.destroySingletons(); + assertThat(bean.destroyMethods).as("destroy-methods").containsExactly("@PreDestroy.sameNameCustomDestroy1", "@PreDestroy.privateCustomDestroy1", "destroy"); + } + + @Test + void allLifecycleMechanismsAtOnce() { + Class beanClass = AllInOneBean.class; + DefaultListableBeanFactory beanFactory = createBeanFactoryAndRegisterBean(beanClass, "afterPropertiesSet", "destroy"); + AllInOneBean bean = beanFactory.getBean(AllInOneBean.class); + assertThat(bean.initMethods).as("init-methods").containsExactly("afterPropertiesSet"); + beanFactory.destroySingletons(); + assertThat(bean.destroyMethods).as("destroy-methods").containsExactly("destroy"); + } + + + private static DefaultListableBeanFactory createBeanFactoryAndRegisterBean(Class beanClass, + String initMethodName, String destroyMethodName) { + + DefaultListableBeanFactory beanFactory = new DefaultListableBeanFactory(); + RootBeanDefinition beanDefinition = new RootBeanDefinition(beanClass); + beanDefinition.setInitMethodName(initMethodName); + beanDefinition.setDestroyMethodName(destroyMethodName); + beanFactory.addBeanPostProcessor(new CommonAnnotationBeanPostProcessor()); + beanFactory.registerBeanDefinition("lifecycleTestBean", beanDefinition); + return beanFactory; + } + + + static class InitDestroyBean { + + final List initMethods = new ArrayList<>(); + final List destroyMethods = new ArrayList<>(); + + + public void afterPropertiesSet() throws Exception { + this.initMethods.add("afterPropertiesSet"); + } + + public void destroy() throws Exception { + this.destroyMethods.add("destroy"); + } + } + + static class InitializingDisposableWithShadowedMethodsBean extends InitDestroyBean implements + InitializingBean, DisposableBean { + + @Override + public void afterPropertiesSet() throws Exception { + this.initMethods.add("InitializingBean.afterPropertiesSet"); + } + + @Override + public void destroy() throws Exception { + this.destroyMethods.add("DisposableBean.destroy"); + } + } + + + static class CustomInitDestroyBean { + + final List initMethods = new ArrayList<>(); + final List destroyMethods = new ArrayList<>(); + + public void customInit() throws Exception { + this.initMethods.add("customInit"); + } + + public void customDestroy() throws Exception { + this.destroyMethods.add("customDestroy"); + } + } + + static class CustomAnnotatedPrivateInitDestroyBean extends CustomInitializingDisposableBean { + + @PostConstruct + private void customInit1() throws Exception { + this.initMethods.add("@PostConstruct.privateCustomInit1"); + } + + @PreDestroy + private void customDestroy1() throws Exception { + this.destroyMethods.add("@PreDestroy.privateCustomDestroy1"); + } + } + + static class CustomAnnotatedPrivateSameNameInitDestroyBean extends CustomAnnotatedPrivateInitDestroyBean { + + @PostConstruct + @SuppressWarnings("unused") + private void customInit1() throws Exception { + this.initMethods.add("@PostConstruct.sameNameCustomInit1"); + } + + @PreDestroy + @SuppressWarnings("unused") + private void customDestroy1() throws Exception { + this.destroyMethods.add("@PreDestroy.sameNameCustomDestroy1"); + } + } + + static class CustomInitializingDisposableBean extends CustomInitDestroyBean + implements InitializingBean, DisposableBean { + + @Override + public void afterPropertiesSet() throws Exception { + this.initMethods.add("afterPropertiesSet"); + } + + @Override + public void destroy() throws Exception { + this.destroyMethods.add("destroy"); + } + } + + static class CustomAnnotatedInitDestroyBean extends CustomInitializingDisposableBean { + + @PostConstruct + public void postConstruct() throws Exception { + this.initMethods.add("postConstruct"); + } + + @PreDestroy + public void preDestroy() throws Exception { + this.destroyMethods.add("preDestroy"); + } + } + + static class CustomAnnotatedInitDestroyWithShadowedMethodsBean extends CustomInitializingDisposableBean { + + @PostConstruct + @Override + public void afterPropertiesSet() throws Exception { + this.initMethods.add("@PostConstruct.afterPropertiesSet"); + } + + @PreDestroy + @Override + public void destroy() throws Exception { + this.destroyMethods.add("@PreDestroy.destroy"); + } + } + + static class AllInOneBean implements InitializingBean, DisposableBean { + + final List initMethods = new ArrayList<>(); + final List destroyMethods = new ArrayList<>(); + + @PostConstruct + @Override + public void afterPropertiesSet() throws Exception { + this.initMethods.add("afterPropertiesSet"); + } + + @PreDestroy + @Override + public void destroy() throws Exception { + this.destroyMethods.add("destroy"); + } + } + +} diff --git a/spring-context/src/test/java/org/springframework/context/annotation/Spr3775InitDestroyLifecycleTests.java b/spring-context/src/test/java/org/springframework/context/annotation/Spr3775InitDestroyLifecycleTests.java deleted file mode 100644 index 4de6652e65a3..000000000000 --- a/spring-context/src/test/java/org/springframework/context/annotation/Spr3775InitDestroyLifecycleTests.java +++ /dev/null @@ -1,325 +0,0 @@ -/* - * Copyright 2002-2022 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. - * You may obtain a copy of the License at - * - * https://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 org.springframework.context.annotation; - -import java.util.ArrayList; -import java.util.Arrays; -import java.util.List; - -import javax.annotation.PostConstruct; -import javax.annotation.PreDestroy; - -import org.apache.commons.logging.Log; -import org.apache.commons.logging.LogFactory; -import org.junit.jupiter.api.Test; - -import org.springframework.beans.factory.DisposableBean; -import org.springframework.beans.factory.InitializingBean; -import org.springframework.beans.factory.support.DefaultListableBeanFactory; -import org.springframework.beans.factory.support.RootBeanDefinition; -import org.springframework.util.ObjectUtils; - -import static org.assertj.core.api.Assertions.assertThat; - -/** - *

- * JUnit-3.8-based unit test which verifies expected init and - * destroy bean lifecycle behavior as requested in SPR-3775. - *

- *

- * Specifically, combinations of the following are tested: - *

- *
    - *
  • {@link InitializingBean} & {@link DisposableBean} interfaces
    • - *
  • Custom {@link RootBeanDefinition#getInitMethodName() init} & - * {@link RootBeanDefinition#getDestroyMethodName() destroy} methods
    • - *
  • JSR 250's {@link javax.annotation.PostConstruct @PostConstruct} & - * {@link javax.annotation.PreDestroy @PreDestroy} annotations
    • - *
- * - * @author Sam Brannen - * @since 2.5 - */ -public class Spr3775InitDestroyLifecycleTests { - - private static final Log logger = LogFactory.getLog(Spr3775InitDestroyLifecycleTests.class); - - /** LIFECYCLE_TEST_BEAN. */ - private static final String LIFECYCLE_TEST_BEAN = "lifecycleTestBean"; - - - private void debugMethods(Class clazz, String category, List methodNames) { - if (logger.isDebugEnabled()) { - logger.debug(clazz.getSimpleName() + ": " + category + ": " + methodNames); - } - } - - private void assertMethodOrdering(Class clazz, String category, List expectedMethods, - List actualMethods) { - debugMethods(clazz, category, actualMethods); - assertThat(ObjectUtils.nullSafeEquals(expectedMethods, actualMethods)).as("Verifying " + category + ": expected<" + expectedMethods + "> but got<" + actualMethods + ">.").isTrue(); - } - - private DefaultListableBeanFactory createBeanFactoryAndRegisterBean(final Class beanClass, - final String initMethodName, final String destroyMethodName) { - DefaultListableBeanFactory beanFactory = new DefaultListableBeanFactory(); - RootBeanDefinition beanDefinition = new RootBeanDefinition(beanClass); - beanDefinition.setInitMethodName(initMethodName); - beanDefinition.setDestroyMethodName(destroyMethodName); - beanFactory.addBeanPostProcessor(new CommonAnnotationBeanPostProcessor()); - beanFactory.registerBeanDefinition(LIFECYCLE_TEST_BEAN, beanDefinition); - return beanFactory; - } - - @Test - public void testInitDestroyMethods() { - final Class beanClass = InitDestroyBean.class; - final DefaultListableBeanFactory beanFactory = createBeanFactoryAndRegisterBean(beanClass, - "afterPropertiesSet", "destroy"); - final InitDestroyBean bean = (InitDestroyBean) beanFactory.getBean(LIFECYCLE_TEST_BEAN); - assertMethodOrdering(beanClass, "init-methods", Arrays.asList("afterPropertiesSet"), bean.initMethods); - beanFactory.destroySingletons(); - assertMethodOrdering(beanClass, "destroy-methods", Arrays.asList("destroy"), bean.destroyMethods); - } - - @Test - public void testInitializingDisposableInterfaces() { - final Class beanClass = CustomInitializingDisposableBean.class; - final DefaultListableBeanFactory beanFactory = createBeanFactoryAndRegisterBean(beanClass, "customInit", - "customDestroy"); - final CustomInitializingDisposableBean bean = (CustomInitializingDisposableBean) beanFactory.getBean(LIFECYCLE_TEST_BEAN); - assertMethodOrdering(beanClass, "init-methods", Arrays.asList("afterPropertiesSet", "customInit"), - bean.initMethods); - beanFactory.destroySingletons(); - assertMethodOrdering(beanClass, "destroy-methods", Arrays.asList("destroy", "customDestroy"), - bean.destroyMethods); - } - - @Test - public void testInitializingDisposableInterfacesWithShadowedMethods() { - final Class beanClass = InitializingDisposableWithShadowedMethodsBean.class; - final DefaultListableBeanFactory beanFactory = createBeanFactoryAndRegisterBean(beanClass, - "afterPropertiesSet", "destroy"); - final InitializingDisposableWithShadowedMethodsBean bean = (InitializingDisposableWithShadowedMethodsBean) beanFactory.getBean(LIFECYCLE_TEST_BEAN); - assertMethodOrdering(beanClass, "init-methods", Arrays.asList("InitializingBean.afterPropertiesSet"), - bean.initMethods); - beanFactory.destroySingletons(); - assertMethodOrdering(beanClass, "destroy-methods", Arrays.asList("DisposableBean.destroy"), bean.destroyMethods); - } - - @Test - public void testJsr250Annotations() { - final Class beanClass = CustomAnnotatedInitDestroyBean.class; - final DefaultListableBeanFactory beanFactory = createBeanFactoryAndRegisterBean(beanClass, "customInit", - "customDestroy"); - final CustomAnnotatedInitDestroyBean bean = (CustomAnnotatedInitDestroyBean) beanFactory.getBean(LIFECYCLE_TEST_BEAN); - assertMethodOrdering(beanClass, "init-methods", Arrays.asList("postConstruct", "afterPropertiesSet", - "customInit"), bean.initMethods); - beanFactory.destroySingletons(); - assertMethodOrdering(beanClass, "destroy-methods", Arrays.asList("preDestroy", "destroy", "customDestroy"), - bean.destroyMethods); - } - - @Test - public void testJsr250AnnotationsWithShadowedMethods() { - final Class beanClass = CustomAnnotatedInitDestroyWithShadowedMethodsBean.class; - final DefaultListableBeanFactory beanFactory = createBeanFactoryAndRegisterBean(beanClass, "customInit", - "customDestroy"); - final CustomAnnotatedInitDestroyWithShadowedMethodsBean bean = (CustomAnnotatedInitDestroyWithShadowedMethodsBean) beanFactory.getBean(LIFECYCLE_TEST_BEAN); - assertMethodOrdering(beanClass, "init-methods", - Arrays.asList("@PostConstruct.afterPropertiesSet", "customInit"), bean.initMethods); - beanFactory.destroySingletons(); - assertMethodOrdering(beanClass, "destroy-methods", Arrays.asList("@PreDestroy.destroy", "customDestroy"), - bean.destroyMethods); - } - - @Test - public void testJsr250AnnotationsWithCustomPrivateInitDestroyMethods() { - Class beanClass = CustomAnnotatedPrivateInitDestroyBean.class; - DefaultListableBeanFactory beanFactory = createBeanFactoryAndRegisterBean(beanClass, "customInit1", "customDestroy1"); - CustomAnnotatedPrivateInitDestroyBean bean = - (CustomAnnotatedPrivateInitDestroyBean) beanFactory.getBean(LIFECYCLE_TEST_BEAN); - assertMethodOrdering(beanClass, "init-methods", Arrays.asList("@PostConstruct.privateCustomInit1", "afterPropertiesSet"), bean.initMethods); - beanFactory.destroySingletons(); - assertMethodOrdering(beanClass, "destroy-methods", Arrays.asList("@PreDestroy.privateCustomDestroy1", "destroy"), bean.destroyMethods); - } - - @Test - public void testJsr250AnnotationsWithCustomSameMethodNames() { - Class beanClass = CustomAnnotatedPrivateSameNameInitDestroyBean.class; - DefaultListableBeanFactory beanFactory = createBeanFactoryAndRegisterBean(beanClass, "customInit1", "customDestroy1"); - CustomAnnotatedPrivateSameNameInitDestroyBean bean = - (CustomAnnotatedPrivateSameNameInitDestroyBean) beanFactory.getBean(LIFECYCLE_TEST_BEAN); - assertMethodOrdering(beanClass, "init-methods", - Arrays.asList("@PostConstruct.privateCustomInit1", "@PostConstruct.sameNameCustomInit1", "afterPropertiesSet"), bean.initMethods); - beanFactory.destroySingletons(); - assertMethodOrdering(beanClass, "destroy-methods", - Arrays.asList("@PreDestroy.sameNameCustomDestroy1", "@PreDestroy.privateCustomDestroy1", "destroy"), bean.destroyMethods); - } - - @Test - public void testAllLifecycleMechanismsAtOnce() { - final Class beanClass = AllInOneBean.class; - final DefaultListableBeanFactory beanFactory = createBeanFactoryAndRegisterBean(beanClass, - "afterPropertiesSet", "destroy"); - final AllInOneBean bean = (AllInOneBean) beanFactory.getBean(LIFECYCLE_TEST_BEAN); - assertMethodOrdering(beanClass, "init-methods", Arrays.asList("afterPropertiesSet"), bean.initMethods); - beanFactory.destroySingletons(); - assertMethodOrdering(beanClass, "destroy-methods", Arrays.asList("destroy"), bean.destroyMethods); - } - - - public static class InitDestroyBean { - - final List initMethods = new ArrayList<>(); - final List destroyMethods = new ArrayList<>(); - - - public void afterPropertiesSet() throws Exception { - this.initMethods.add("afterPropertiesSet"); - } - - public void destroy() throws Exception { - this.destroyMethods.add("destroy"); - } - } - - public static class InitializingDisposableWithShadowedMethodsBean extends InitDestroyBean implements - InitializingBean, DisposableBean { - - @Override - public void afterPropertiesSet() throws Exception { - this.initMethods.add("InitializingBean.afterPropertiesSet"); - } - - @Override - public void destroy() throws Exception { - this.destroyMethods.add("DisposableBean.destroy"); - } - } - - - public static class CustomInitDestroyBean { - - final List initMethods = new ArrayList<>(); - final List destroyMethods = new ArrayList<>(); - - public void customInit() throws Exception { - this.initMethods.add("customInit"); - } - - public void customDestroy() throws Exception { - this.destroyMethods.add("customDestroy"); - } - } - - public static class CustomAnnotatedPrivateInitDestroyBean extends CustomInitializingDisposableBean { - - @PostConstruct - private void customInit1() throws Exception { - this.initMethods.add("@PostConstruct.privateCustomInit1"); - } - - @PreDestroy - private void customDestroy1() throws Exception { - this.destroyMethods.add("@PreDestroy.privateCustomDestroy1"); - } - - } - - public static class CustomAnnotatedPrivateSameNameInitDestroyBean extends CustomAnnotatedPrivateInitDestroyBean { - - @PostConstruct - @SuppressWarnings("unused") - private void customInit1() throws Exception { - this.initMethods.add("@PostConstruct.sameNameCustomInit1"); - } - - @PreDestroy - @SuppressWarnings("unused") - private void customDestroy1() throws Exception { - this.destroyMethods.add("@PreDestroy.sameNameCustomDestroy1"); - } - - } - - public static class CustomInitializingDisposableBean extends CustomInitDestroyBean - implements InitializingBean, DisposableBean { - - @Override - public void afterPropertiesSet() throws Exception { - this.initMethods.add("afterPropertiesSet"); - } - - @Override - public void destroy() throws Exception { - this.destroyMethods.add("destroy"); - } - } - - - public static class CustomAnnotatedInitDestroyBean extends CustomInitializingDisposableBean { - - @PostConstruct - public void postConstruct() throws Exception { - this.initMethods.add("postConstruct"); - } - - @PreDestroy - public void preDestroy() throws Exception { - this.destroyMethods.add("preDestroy"); - } - } - - - public static class CustomAnnotatedInitDestroyWithShadowedMethodsBean extends CustomInitializingDisposableBean { - - @PostConstruct - @Override - public void afterPropertiesSet() throws Exception { - this.initMethods.add("@PostConstruct.afterPropertiesSet"); - } - - @PreDestroy - @Override - public void destroy() throws Exception { - this.destroyMethods.add("@PreDestroy.destroy"); - } - } - - - public static class AllInOneBean implements InitializingBean, DisposableBean { - - final List initMethods = new ArrayList<>(); - final List destroyMethods = new ArrayList<>(); - - @Override - @PostConstruct - public void afterPropertiesSet() throws Exception { - this.initMethods.add("afterPropertiesSet"); - } - - @Override - @PreDestroy - public void destroy() throws Exception { - this.destroyMethods.add("destroy"); - } - } - -} From d67034f99b5dad82e2daecc1e31ce99b1c551593 Mon Sep 17 00:00:00 2001 From: Sam Brannen Date: Tue, 1 Mar 2022 16:18:46 +0100 Subject: [PATCH 6/6] Document semantics for externally managed init/destroy methods This commit introduces Javadoc to explain the difference between init/destroy method names when such methods are private, namely that a private method is registered via its qualified method name; whereas, a non-private method is registered via its simple name. See gh-28083 --- .../factory/support/RootBeanDefinition.java | 52 ++++++++++++++----- 1 file changed, 39 insertions(+), 13 deletions(-) diff --git a/spring-beans/src/main/java/org/springframework/beans/factory/support/RootBeanDefinition.java b/spring-beans/src/main/java/org/springframework/beans/factory/support/RootBeanDefinition.java index 13638768efa1..f93c08f3d548 100644 --- a/spring-beans/src/main/java/org/springframework/beans/factory/support/RootBeanDefinition.java +++ b/spring-beans/src/main/java/org/springframework/beans/factory/support/RootBeanDefinition.java @@ -437,7 +437,7 @@ public void registerExternallyManagedConfigMember(Member configMember) { } /** - * Check whether the given method or field is an externally managed configuration member. + * Determine if the given method or field is an externally managed configuration member. */ public boolean isExternallyManagedConfigMember(Member configMember) { synchronized (this.postProcessingLock) { @@ -447,7 +447,7 @@ public boolean isExternallyManagedConfigMember(Member configMember) { } /** - * Return all externally managed configuration methods and fields (as an immutable Set). + * Get all externally managed configuration methods and fields (as an immutable Set). * @since 5.3.11 */ public Set getExternallyManagedConfigMembers() { @@ -459,7 +459,15 @@ public Set getExternallyManagedConfigMembers() { } /** - * Register an externally managed configuration initialization method. + * Register an externally managed configuration initialization method — + * for example, a method annotated with JSR-250's + * {@link javax.annotation.PostConstruct} annotation. + *

The supplied {@code initMethod} may be the + * {@linkplain Method#getName() simple method name} for non-private methods or the + * {@linkplain org.springframework.util.ClassUtils#getQualifiedMethodName(Method) + * qualified method name} for {@code private} methods. A qualified name is + * necessary for {@code private} methods in order to disambiguate between + * multiple private methods with the same name within a class hierarchy. */ public void registerExternallyManagedInitMethod(String initMethod) { synchronized (this.postProcessingLock) { @@ -471,7 +479,10 @@ public void registerExternallyManagedInitMethod(String initMethod) { } /** - * Check whether the given method name indicates an externally managed initialization method. + * Determine if the given method name indicates an externally managed + * initialization method. + *

See {@link #registerExternallyManagedInitMethod} for details + * regarding the format for the supplied {@code initMethod}. */ public boolean isExternallyManagedInitMethod(String initMethod) { synchronized (this.postProcessingLock) { @@ -484,10 +495,10 @@ public boolean isExternallyManagedInitMethod(String initMethod) { * Determine if the given method name indicates an externally managed * initialization method, regardless of method visibility. *

In contrast to {@link #isExternallyManagedInitMethod(String)}, this - * method also returns {@code true} if there is a {@code private} external - * init method that has been + * method also returns {@code true} if there is a {@code private} externally + * managed initialization method that has been * {@linkplain #registerExternallyManagedInitMethod(String) registered} - * using a fully qualified method name instead of a simple method name. + * using a qualified method name instead of a simple method name. * @since 5.3.17 */ boolean hasAnyExternallyManagedInitMethod(String initMethod) { @@ -512,6 +523,8 @@ boolean hasAnyExternallyManagedInitMethod(String initMethod) { /** * Return all externally managed initialization methods (as an immutable Set). + *

See {@link #registerExternallyManagedInitMethod} for details + * regarding the format for the initialization methods in the returned set. * @since 5.3.11 */ public Set getExternallyManagedInitMethods() { @@ -523,7 +536,15 @@ public Set getExternallyManagedInitMethods() { } /** - * Register an externally managed configuration destruction method. + * Register an externally managed configuration destruction method — + * for example, a method annotated with JSR-250's + * {@link javax.annotation.PreDestroy} annotation. + *

The supplied {@code destroyMethod} may be the + * {@linkplain Method#getName() simple method name} for non-private methods or the + * {@linkplain org.springframework.util.ClassUtils#getQualifiedMethodName(Method) + * qualified method name} for {@code private} methods. A qualified name is + * necessary for {@code private} methods in order to disambiguate between + * multiple private methods with the same name within a class hierarchy. */ public void registerExternallyManagedDestroyMethod(String destroyMethod) { synchronized (this.postProcessingLock) { @@ -535,7 +556,10 @@ public void registerExternallyManagedDestroyMethod(String destroyMethod) { } /** - * Check whether the given method name indicates an externally managed destruction method. + * Determine if the given method name indicates an externally managed + * destruction method. + *

See {@link #registerExternallyManagedDestroyMethod} for details + * regarding the format for the supplied {@code destroyMethod}. */ public boolean isExternallyManagedDestroyMethod(String destroyMethod) { synchronized (this.postProcessingLock) { @@ -548,10 +572,10 @@ public boolean isExternallyManagedDestroyMethod(String destroyMethod) { * Determine if the given method name indicates an externally managed * destruction method, regardless of method visibility. *

In contrast to {@link #isExternallyManagedDestroyMethod(String)}, this - * method also returns {@code true} if there is a {@code private} external - * destroy method that has been + * method also returns {@code true} if there is a {@code private} externally + * managed destruction method that has been * {@linkplain #registerExternallyManagedDestroyMethod(String) registered} - * using a fully qualified method name instead of a simple method name. + * using a qualified method name instead of a simple method name. * @since 5.3.17 */ boolean hasAnyExternallyManagedDestroyMethod(String destroyMethod) { @@ -575,7 +599,9 @@ boolean hasAnyExternallyManagedDestroyMethod(String destroyMethod) { } /** - * Return all externally managed destruction methods (as an immutable Set). + * Get all externally managed destruction methods (as an immutable Set). + *

See {@link #registerExternallyManagedDestroyMethod} for details + * regarding the format for the destruction methods in the returned set. * @since 5.3.11 */ public Set getExternallyManagedDestroyMethods() {