From 936c1809cd7b500a664d9d9449c7f5d1aba2ce48 Mon Sep 17 00:00:00 2001 From: Sam Brannen Date: Tue, 20 Jun 2023 17:47:01 +0200 Subject: [PATCH] Support private init/destroy methods in AOT mode Closes gh-30692 --- ...BeanDefinitionPropertiesCodeGenerator.java | 2 + .../AbstractAutowireCapableBeanFactory.java | 21 +++-- .../support/DisposableBeanAdapter.java | 20 +++-- ...stroyAnnotationBeanPostProcessorTests.java | 75 +++++++++++++++- .../InitDestroyMethodLifecycleTests.java | 86 ++++++++++++++++++- 5 files changed, 190 insertions(+), 14 deletions(-) diff --git a/spring-beans/src/main/java/org/springframework/beans/factory/aot/BeanDefinitionPropertiesCodeGenerator.java b/spring-beans/src/main/java/org/springframework/beans/factory/aot/BeanDefinitionPropertiesCodeGenerator.java index 72dbf9ebd580..2f1fcbcef9d3 100644 --- a/spring-beans/src/main/java/org/springframework/beans/factory/aot/BeanDefinitionPropertiesCodeGenerator.java +++ b/spring-beans/src/main/java/org/springframework/beans/factory/aot/BeanDefinitionPropertiesCodeGenerator.java @@ -72,6 +72,7 @@ * * @author Phillip Webb * @author Stephane Nicoll + * @author Sam Brannen * @since 6.0 */ class BeanDefinitionPropertiesCodeGenerator { @@ -138,6 +139,7 @@ private void addInitDestroyMethods(Builder code, } private void addInitDestroyHint(Class beanUserClass, String methodName) { + // TODO Handle fully-qualified method names Method method = ReflectionUtils.findMethod(beanUserClass, methodName); if (method != null) { this.hints.reflection().registerMethod(method, ExecutableMode.INVOKE); 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..2c0d70f9e381 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 @@ -1840,18 +1840,29 @@ protected void invokeInitMethods(String beanName, Object bean, @Nullable RootBea protected void invokeCustomInitMethod(String beanName, Object bean, RootBeanDefinition mbd, String initMethodName) throws Throwable { + Class methodDeclaringClass = bean.getClass(); + String methodName = initMethodName; + + // Parse fully-qualified method name into class and method name. + int indexOfDot = initMethodName.lastIndexOf('.'); + if (indexOfDot > 0) { + String className = initMethodName.substring(0, indexOfDot); + methodName = initMethodName.substring(indexOfDot + 1); + methodDeclaringClass = ClassUtils.forName(className, bean.getClass().getClassLoader()); + } + Method initMethod = (mbd.isNonPublicAccessAllowed() ? - BeanUtils.findMethod(bean.getClass(), initMethodName) : - ClassUtils.getMethodIfAvailable(bean.getClass(), initMethodName)); + BeanUtils.findMethod(methodDeclaringClass, methodName) : + ClassUtils.getMethodIfAvailable(bean.getClass(), methodName)); if (initMethod == null) { if (mbd.isEnforceInitMethod()) { throw new BeanDefinitionValidationException("Could not find an init method named '" + - initMethodName + "' on bean with name '" + beanName + "'"); + methodName + "' on bean with name '" + beanName + "'"); } else { if (logger.isTraceEnabled()) { - logger.trace("No default init method named '" + initMethodName + + logger.trace("No default init method named '" + methodName + "' found on bean with name '" + beanName + "'"); } // Ignore non-existent default lifecycle methods. @@ -1860,7 +1871,7 @@ protected void invokeCustomInitMethod(String beanName, Object bean, RootBeanDefi } if (logger.isTraceEnabled()) { - logger.trace("Invoking init method '" + initMethodName + "' on bean with name '" + beanName + "'"); + logger.trace("Invoking init method '" + methodName + "' on bean with name '" + beanName + "'"); } Method methodToInvoke = ClassUtils.getInterfaceMethodIfPossible(initMethod, bean.getClass()); 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 0d2bc2adeb22..2eef7297d551 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 @@ -254,20 +254,30 @@ else if (this.destroyMethodNames != null) { @Nullable private Method determineDestroyMethod(String name) { try { - Class beanClass = this.bean.getClass(); - Method destroyMethod = findDestroyMethod(beanClass, name); + Class methodDeclaringClass = this.bean.getClass(); + String methodName = name; + + // Parse fully-qualified method name into class and method name. + int indexOfDot = name.lastIndexOf('.'); + if (indexOfDot > 0) { + String className = name.substring(0, indexOfDot); + methodName = name.substring(indexOfDot + 1); + methodDeclaringClass = ClassUtils.forName(className, methodDeclaringClass.getClassLoader()); + } + + Method destroyMethod = findDestroyMethod(methodDeclaringClass, methodName); if (destroyMethod != null) { return destroyMethod; } - for (Class beanInterface : beanClass.getInterfaces()) { - destroyMethod = findDestroyMethod(beanInterface, name); + for (Class beanInterface : this.bean.getClass().getInterfaces()) { + destroyMethod = findDestroyMethod(beanInterface, methodName); if (destroyMethod != null) { return destroyMethod; } } return null; } - catch (IllegalArgumentException ex) { + catch (ClassNotFoundException | IllegalArgumentException ex) { throw new BeanDefinitionValidationException("Could not find unique destroy method on bean with name '" + this.beanName + ": " + ex.getMessage()); } diff --git a/spring-beans/src/test/java/org/springframework/beans/factory/annotation/InitDestroyAnnotationBeanPostProcessorTests.java b/spring-beans/src/test/java/org/springframework/beans/factory/annotation/InitDestroyAnnotationBeanPostProcessorTests.java index 9856e9d1cf9c..2f6787f0cd0f 100644 --- a/spring-beans/src/test/java/org/springframework/beans/factory/annotation/InitDestroyAnnotationBeanPostProcessorTests.java +++ b/spring-beans/src/test/java/org/springframework/beans/factory/annotation/InitDestroyAnnotationBeanPostProcessorTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2022 the original author or authors. + * Copyright 2002-2023 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 org.junit.jupiter.api.Test; +import org.springframework.beans.factory.DisposableBean; +import org.springframework.beans.factory.InitializingBean; import org.springframework.beans.factory.support.AbstractBeanDefinition; import org.springframework.beans.factory.support.DefaultListableBeanFactory; import org.springframework.beans.factory.support.RegisteredBean; @@ -29,12 +31,15 @@ import org.springframework.beans.testfixture.beans.factory.generator.lifecycle.MultiInitDestroyBean; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.SoftAssertions.assertSoftly; /** * Tests for {@link InitDestroyAnnotationBeanPostProcessor}. * + * @since 6.0 * @author Stephane Nicoll * @author Phillip Webb + * @author Sam Brannen */ class InitDestroyAnnotationBeanPostProcessorTests { @@ -109,6 +114,29 @@ void processAheadOfTimeWhenHasMultipleInitDestroyAnnotationsAddsAllMethodNames() assertThat(mergedBeanDefinition.getDestroyMethodNames()).containsExactly("anotherDestroyMethod", "destroyMethod"); } + @Test + void processAheadOfTimeWithMultipleLevelsOfPublicAndPrivateInitAndDestroyMethods() { + RootBeanDefinition beanDefinition = new RootBeanDefinition(CustomAnnotatedPrivateSameNameInitDestroyBean.class); + beanDefinition.setInitMethodNames("afterPropertiesSet", "customInit"); + beanDefinition.setDestroyMethodNames("destroy", "customDestroy"); + processAheadOfTime(beanDefinition); + RootBeanDefinition mergedBeanDefinition = getMergedBeanDefinition(); + assertSoftly(softly -> { + softly.assertThat(mergedBeanDefinition.getInitMethodNames()).containsExactly( + "afterPropertiesSet", + "customInit", + CustomAnnotatedPrivateInitDestroyBean.class.getName() + ".privateInit", // fully-qualified private method + CustomAnnotatedPrivateSameNameInitDestroyBean.class.getName() + ".privateInit" // fully-qualified private method + ); + softly.assertThat(mergedBeanDefinition.getDestroyMethodNames()).containsExactly( + "destroy", + "customDestroy", + CustomAnnotatedPrivateSameNameInitDestroyBean.class.getName() + ".privateDestroy", // fully-qualified private method + CustomAnnotatedPrivateInitDestroyBean.class.getName() + ".privateDestroy" // fully-qualified private method + ); + }); + } + private void processAheadOfTime(RootBeanDefinition beanDefinition) { RegisteredBean registeredBean = registerBean(beanDefinition); assertThat(createAotBeanPostProcessor().processAheadOfTime(registeredBean)).isNull(); @@ -133,4 +161,49 @@ private InitDestroyAnnotationBeanPostProcessor createAotBeanPostProcessor() { static class NoInitDestroyBean {} + static class CustomInitDestroyBean { + + public void customInit() { + } + + public void customDestroy() { + } + } + + static class CustomInitializingDisposableBean extends CustomInitDestroyBean + implements InitializingBean, DisposableBean { + + @Override + public void afterPropertiesSet() { + } + + @Override + public void destroy() { + } + } + + static class CustomAnnotatedPrivateInitDestroyBean extends CustomInitializingDisposableBean { + + @Init + private void privateInit() { + } + + @Destroy + private void privateDestroy() { + } + } + + static class CustomAnnotatedPrivateSameNameInitDestroyBean extends CustomAnnotatedPrivateInitDestroyBean { + + @Init + @SuppressWarnings("unused") + private void privateInit() { + } + + @Destroy + @SuppressWarnings("unused") + private void privateDestroy() { + } + } + } 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..2529b5bbdc59 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 @@ -18,15 +18,23 @@ import java.util.ArrayList; import java.util.List; +import java.util.function.BiConsumer; import jakarta.annotation.PostConstruct; import jakarta.annotation.PreDestroy; import org.junit.jupiter.api.Test; +import org.springframework.aot.test.generate.TestGenerationContext; 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.context.ApplicationContextInitializer; +import org.springframework.context.aot.ApplicationContextAotGenerator; +import org.springframework.context.support.GenericApplicationContext; +import org.springframework.core.test.tools.CompileWithForkedClassLoader; +import org.springframework.core.test.tools.Compiled; +import org.springframework.core.test.tools.TestCompiler; import static org.assertj.core.api.Assertions.assertThat; @@ -112,11 +120,58 @@ void jsr250AnnotationsWithCustomPrivateInitDestroyMethods() { @Test void jsr250AnnotationsWithCustomSameMethodNames() { 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 + @CompileWithForkedClassLoader + void jsr250AnnotationsWithCustomSameMethodNamesWithAotProcessingAndAotRuntime() { + Class beanClass = CustomAnnotatedPrivateSameNameInitDestroyBean.class; + GenericApplicationContext applicationContext = new GenericApplicationContext(); + + DefaultListableBeanFactory beanFactory = applicationContext.getDefaultListableBeanFactory(); + AnnotationConfigUtils.registerAnnotationConfigProcessors(beanFactory); + + RootBeanDefinition beanDefinition = new RootBeanDefinition(beanClass); + beanDefinition.setInitMethodName("customInit"); + beanDefinition.setDestroyMethodName("customDestroy"); + beanFactory.registerBeanDefinition("test", beanDefinition); + + testCompiledResult(applicationContext, (initializer, compiled) -> { + GenericApplicationContext aotApplicationContext = createApplicationContext(initializer); + CustomAnnotatedPrivateSameNameInitDestroyBean bean = aotApplicationContext.getBean("test", beanClass); + + assertThat(bean.initMethods).as("init-methods").containsExactly( + "afterPropertiesSet", + "customInit", + "@PostConstruct.privateCustomInit1", + "@PostConstruct.sameNameCustomInit1" + ); + + aotApplicationContext.close(); + assertThat(bean.destroyMethods).as("destroy-methods").containsExactly( + "destroy", + "customDestroy", + "@PreDestroy.sameNameCustomDestroy1", + "@PreDestroy.privateCustomDestroy1" + ); + }); } @Test @@ -142,6 +197,31 @@ private static DefaultListableBeanFactory createBeanFactoryAndRegisterBean(Class return beanFactory; } + private static GenericApplicationContext createApplicationContext( + ApplicationContextInitializer initializer) { + + GenericApplicationContext context = new GenericApplicationContext(); + initializer.initialize(context); + context.refresh(); + return context; + } + + @SuppressWarnings("unchecked") + private static void testCompiledResult(GenericApplicationContext applicationContext, + BiConsumer, Compiled> result) { + + TestCompiler.forSystem().with(processAheadOfTime(applicationContext)).compile(compiled -> + result.accept(compiled.getInstance(ApplicationContextInitializer.class), compiled)); + } + + private static TestGenerationContext processAheadOfTime(GenericApplicationContext applicationContext) { + ApplicationContextAotGenerator generator = new ApplicationContextAotGenerator(); + TestGenerationContext generationContext = new TestGenerationContext(); + generator.processAheadOfTime(applicationContext, generationContext); + generationContext.writeGeneratedContent(); + return generationContext; + } + static class InitDestroyBean {