From 68e28a5f5958f9ff74c76183006e5cc2623a0765 Mon Sep 17 00:00:00 2001 From: Olga Maciaszek-Sharma Date: Tue, 19 Jul 2022 19:35:55 +0200 Subject: [PATCH 1/2] Handle inferred init/destroy method consistently See gh-28843 --- .../BeanDefinitionPropertiesCodeGenerator.java | 15 +++++++++------ ...eanDefinitionPropertiesCodeGeneratorTests.java | 13 +++++++++++++ 2 files changed, 22 insertions(+), 6 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 cc30e470c56..65310628f75 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 @@ -21,6 +21,7 @@ import java.beans.IntrospectionException; import java.beans.Introspector; import java.beans.PropertyDescriptor; import java.lang.reflect.Method; +import java.util.Arrays; import java.util.Collections; import java.util.HashMap; import java.util.Map; @@ -73,6 +74,7 @@ import org.springframework.util.StringUtils; * * @author Phillip Webb * @author Stephane Nicoll + * @author Olga Maciaszek-Sharma * @since 6.0 */ class BeanDefinitionPropertiesCodeGenerator { @@ -143,12 +145,13 @@ class BeanDefinitionPropertiesCodeGenerator { Class beanType = ClassUtils .getUserClass(beanDefinition.getResolvableType().toClass()); Builder arguments = CodeBlock.builder(); - for (int i = 0; i < methodNames.length; i++) { - String methodName = methodNames[i]; - if (!AbstractBeanDefinition.INFER_METHOD.equals(methodName)) { - arguments.add((i != 0) ? ", $S" : "$S", methodName); - addInitDestroyHint(beanType, methodName); - } + String[] filteredMethodNames = Arrays.stream(methodNames) + .filter(methodName -> !AbstractBeanDefinition.INFER_METHOD.equals(methodName)) + .toArray(String[]::new); + for (int i = 0; i < filteredMethodNames.length; i++) { + String methodName = filteredMethodNames[i]; + arguments.add((i != 0) ? ", $S" : "$S", methodName); + addInitDestroyHint(beanType, methodName); } if (!arguments.isEmpty()) { builder.addStatement(format, BEAN_DEFINITION_VARIABLE, arguments.build()); diff --git a/spring-beans/src/test/java/org/springframework/beans/factory/aot/BeanDefinitionPropertiesCodeGeneratorTests.java b/spring-beans/src/test/java/org/springframework/beans/factory/aot/BeanDefinitionPropertiesCodeGeneratorTests.java index 2b1e78d3040..76ddb895af5 100644 --- a/spring-beans/src/test/java/org/springframework/beans/factory/aot/BeanDefinitionPropertiesCodeGeneratorTests.java +++ b/spring-beans/src/test/java/org/springframework/beans/factory/aot/BeanDefinitionPropertiesCodeGeneratorTests.java @@ -57,6 +57,7 @@ import static org.assertj.core.api.Assertions.assertThat; * * @author Phillip Webb * @author Stephane Nicoll + * @author Olga Maciaszek-Sharma */ class BeanDefinitionPropertiesCodeGeneratorTests { @@ -394,6 +395,18 @@ class BeanDefinitionPropertiesCodeGeneratorTests { }); } + @Test + void inferredMethodsAtTheBeginning() { + this.beanDefinition.setInitMethodNames(AbstractBeanDefinition.INFER_METHOD, "init"); + this.beanDefinition.setDestroyMethodNames(AbstractBeanDefinition.INFER_METHOD, "destroy"); + compile((actual, compiled) -> { + assertThat(compiled.getSourceFile().getContent()) + .contains("beanDefinition.setInitMethodNames(\"init\");"); + assertThat(compiled.getSourceFile().getContent()) + .contains("beanDefinition.setDestroyMethodNames(\"destroy\");"); + }); + } + private void compile(BiConsumer result) { compile(attribute -> true, result); } From 85173b5de3273c70367605447500fffb2cc53f5a Mon Sep 17 00:00:00 2001 From: Stephane Nicoll Date: Wed, 20 Jul 2022 10:41:49 +0200 Subject: [PATCH 2/2] Polish "Handle inferred init/destroy method consistently" See gh-28843 --- ...BeanDefinitionPropertiesCodeGenerator.java | 31 +++++++++---------- ...efinitionPropertiesCodeGeneratorTests.java | 26 +++++++++------- 2 files changed, 28 insertions(+), 29 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 65310628f75..83274e6c111 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 @@ -24,6 +24,7 @@ import java.lang.reflect.Method; import java.util.Arrays; import java.util.Collections; import java.util.HashMap; +import java.util.List; import java.util.Map; import java.util.Objects; import java.util.function.BiFunction; @@ -74,7 +75,6 @@ import org.springframework.util.StringUtils; * * @author Phillip Webb * @author Stephane Nicoll - * @author Olga Maciaszek-Sharma * @since 6.0 */ class BeanDefinitionPropertiesCodeGenerator { @@ -144,17 +144,14 @@ class BeanDefinitionPropertiesCodeGenerator { if (!ObjectUtils.isEmpty(methodNames)) { Class beanType = ClassUtils .getUserClass(beanDefinition.getResolvableType().toClass()); - Builder arguments = CodeBlock.builder(); - String[] filteredMethodNames = Arrays.stream(methodNames) - .filter(methodName -> !AbstractBeanDefinition.INFER_METHOD.equals(methodName)) - .toArray(String[]::new); - for (int i = 0; i < filteredMethodNames.length; i++) { - String methodName = filteredMethodNames[i]; - arguments.add((i != 0) ? ", $S" : "$S", methodName); - addInitDestroyHint(beanType, methodName); - } - if (!arguments.isEmpty()) { - builder.addStatement(format, BEAN_DEFINITION_VARIABLE, arguments.build()); + List filteredMethodNames = Arrays.stream(methodNames) + .filter(candidate -> !AbstractBeanDefinition.INFER_METHOD.equals(candidate)) + .toList(); + if (!ObjectUtils.isEmpty(filteredMethodNames)) { + filteredMethodNames.forEach(methodName -> addInitDestroyHint(beanType, methodName)); + CodeBlock arguments = CodeBlock.join(filteredMethodNames.stream() + .map(name -> CodeBlock.of("$S", name)).toList(), ", "); + builder.addStatement(format, BEAN_DEFINITION_VARIABLE, arguments); } } } @@ -277,11 +274,11 @@ class BeanDefinitionPropertiesCodeGenerator { private Object toRole(int value) { return switch (value) { - case BeanDefinition.ROLE_INFRASTRUCTURE -> CodeBlock.builder() - .add("$T.ROLE_INFRASTRUCTURE", BeanDefinition.class).build(); - case BeanDefinition.ROLE_SUPPORT -> CodeBlock.builder() - .add("$T.ROLE_SUPPORT", BeanDefinition.class).build(); - default -> value; + case BeanDefinition.ROLE_INFRASTRUCTURE -> CodeBlock.builder() + .add("$T.ROLE_INFRASTRUCTURE", BeanDefinition.class).build(); + case BeanDefinition.ROLE_SUPPORT -> CodeBlock.builder() + .add("$T.ROLE_SUPPORT", BeanDefinition.class).build(); + default -> value; }; } diff --git a/spring-beans/src/test/java/org/springframework/beans/factory/aot/BeanDefinitionPropertiesCodeGeneratorTests.java b/spring-beans/src/test/java/org/springframework/beans/factory/aot/BeanDefinitionPropertiesCodeGeneratorTests.java index 76ddb895af5..035a63b24da 100644 --- a/spring-beans/src/test/java/org/springframework/beans/factory/aot/BeanDefinitionPropertiesCodeGeneratorTests.java +++ b/spring-beans/src/test/java/org/springframework/beans/factory/aot/BeanDefinitionPropertiesCodeGeneratorTests.java @@ -245,6 +245,13 @@ class BeanDefinitionPropertiesCodeGeneratorTests { assertHasMethodInvokeHints(InitDestroyBean.class, methodNames); } + @Test + void setInitMethodWithInferredMethodFirst() { + this.beanDefinition.setInitMethodNames(AbstractBeanDefinition.INFER_METHOD, "init"); + compile((actual, compiled) -> assertThat(compiled.getSourceFile().getContent()) + .contains("beanDefinition.setInitMethodNames(\"init\");")); + } + @Test void setDestroyMethodWhenDestroyInitMethod() { this.beanDefinition.setTargetType(InitDestroyBean.class); @@ -274,6 +281,13 @@ class BeanDefinitionPropertiesCodeGeneratorTests { assertHasMethodInvokeHints(InitDestroyBean.class, methodNames); } + @Test + void setDestroyMethodWithInferredMethodFirst() { + this.beanDefinition.setDestroyMethodNames(AbstractBeanDefinition.INFER_METHOD, "destroy"); + compile((actual, compiled) -> assertThat(compiled.getSourceFile().getContent()) + .contains("beanDefinition.setDestroyMethodNames(\"destroy\");")); + } + private void assertHasMethodInvokeHints(Class beanType, String... methodNames) { assertThat(methodNames).allMatch(methodName -> RuntimeHintsPredicates.reflection() .onMethod(beanType, methodName).invoke() @@ -395,18 +409,6 @@ class BeanDefinitionPropertiesCodeGeneratorTests { }); } - @Test - void inferredMethodsAtTheBeginning() { - this.beanDefinition.setInitMethodNames(AbstractBeanDefinition.INFER_METHOD, "init"); - this.beanDefinition.setDestroyMethodNames(AbstractBeanDefinition.INFER_METHOD, "destroy"); - compile((actual, compiled) -> { - assertThat(compiled.getSourceFile().getContent()) - .contains("beanDefinition.setInitMethodNames(\"init\");"); - assertThat(compiled.getSourceFile().getContent()) - .contains("beanDefinition.setDestroyMethodNames(\"destroy\");"); - }); - } - private void compile(BiConsumer result) { compile(attribute -> true, result); }