From 1b7257a8b532a7de8b4fabf5ffc5cc8e34353bf9 Mon Sep 17 00:00:00 2001 From: Christoph Strobl Date: Wed, 16 Apr 2025 08:07:48 +0200 Subject: [PATCH 1/8] Prepare issue branch. --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index a6dc167a03..1ba673449d 100644 --- a/pom.xml +++ b/pom.xml @@ -5,7 +5,7 @@ org.springframework.data spring-data-commons - 4.0.0-SNAPSHOT + 4.0.x-GH-3270-SNAPSHOT Spring Data Core Core Spring concepts underpinning every Spring Data module. From c8e16583f7a38f810b0a2594b525aa2c36b6b0d3 Mon Sep 17 00:00:00 2001 From: Christoph Strobl Date: Wed, 16 Apr 2025 10:01:27 +0200 Subject: [PATCH 2/8] Add LocalVariableNameFactory. Add a variable name factory that considers predefined names and resolves name clashes. Expose variable name clash resolution via the generation context of a single method. --- .../AotQueryMethodGenerationContext.java | 17 +++- .../generate/LocalVariableNameFactory.java | 82 +++++++++++++++++++ .../aot/generate/VariableNameFactory.java | 36 ++++++++ .../LocalVariableNameFactoryUnitTests.java | 62 ++++++++++++++ 4 files changed, 194 insertions(+), 3 deletions(-) create mode 100644 src/main/java/org/springframework/data/repository/aot/generate/LocalVariableNameFactory.java create mode 100644 src/main/java/org/springframework/data/repository/aot/generate/VariableNameFactory.java create mode 100644 src/test/java/org/springframework/data/repository/aot/generate/LocalVariableNameFactoryUnitTests.java diff --git a/src/main/java/org/springframework/data/repository/aot/generate/AotQueryMethodGenerationContext.java b/src/main/java/org/springframework/data/repository/aot/generate/AotQueryMethodGenerationContext.java index 0dd0806637..fc4fb6a6eb 100644 --- a/src/main/java/org/springframework/data/repository/aot/generate/AotQueryMethodGenerationContext.java +++ b/src/main/java/org/springframework/data/repository/aot/generate/AotQueryMethodGenerationContext.java @@ -24,11 +24,11 @@ import javax.lang.model.element.Modifier; import org.jspecify.annotations.Nullable; - import org.springframework.core.ResolvableType; import org.springframework.core.annotation.MergedAnnotation; import org.springframework.core.annotation.MergedAnnotationSelectors; import org.springframework.core.annotation.MergedAnnotations; +import org.springframework.data.repository.aot.generate.VariableNameFactory.VariableName; import org.springframework.data.repository.core.RepositoryInformation; import org.springframework.data.repository.query.Parameter; import org.springframework.data.repository.query.QueryMethod; @@ -54,6 +54,7 @@ public class AotQueryMethodGenerationContext { private final AotRepositoryFragmentMetadata targetTypeMetadata; private final MethodMetadata targetMethodMetadata; private final CodeBlocks codeBlocks; + private final VariableNameFactory variableNameFactory; AotQueryMethodGenerationContext(RepositoryInformation repositoryInformation, Method method, QueryMethod queryMethod, AotRepositoryFragmentMetadata targetTypeMetadata) { @@ -65,6 +66,7 @@ public class AotQueryMethodGenerationContext { this.targetTypeMetadata = targetTypeMetadata; this.targetMethodMetadata = new MethodMetadata(repositoryInformation, method); this.codeBlocks = new CodeBlocks(targetTypeMetadata); + this.variableNameFactory = LocalVariableNameFactory.forMethod(targetMethodMetadata); } AotRepositoryFragmentMetadata getTargetTypeMetadata() { @@ -127,6 +129,16 @@ public TypeName getReturnTypeName() { return TypeName.get(getReturnType().getType()); } + /** + * Suggests naming clash free variant for the given intended variable name within the local method context. + * + * @param variableName the intended variable name. + * @return the suggested VariableName + */ + public VariableName suggestLocalVariableName(String variableName) { + return variableNameFactory.generateName(variableName); + } + /** * Returns the required parameter name for the {@link Parameter#isBindable() bindable parameter} at the given * {@code parameterIndex} or throws {@link IllegalArgumentException} if the parameter cannot be determined by its @@ -274,8 +286,7 @@ public String getParameterNameOf(Class type) { return null; } - List> entries = new ArrayList<>( - targetMethodMetadata.getMethodArguments().entrySet()); + List> entries = new ArrayList<>(targetMethodMetadata.getMethodArguments().entrySet()); if (position < entries.size()) { return entries.get(position).getKey(); } diff --git a/src/main/java/org/springframework/data/repository/aot/generate/LocalVariableNameFactory.java b/src/main/java/org/springframework/data/repository/aot/generate/LocalVariableNameFactory.java new file mode 100644 index 0000000000..63b56d1c28 --- /dev/null +++ b/src/main/java/org/springframework/data/repository/aot/generate/LocalVariableNameFactory.java @@ -0,0 +1,82 @@ +/* + * Copyright 2025 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.data.repository.aot.generate; + +import java.util.Set; + +import org.springframework.util.LinkedMultiValueMap; +import org.springframework.util.MultiValueMap; + +/** + * {@link VariableNameFactory} implementation keeping track of defined names resolving name clashes using internal + * counter appending {@code _%d} to a suggested name in case of a clash. + * + * @author Christoph Strobl + * @since 4.0 + */ +class LocalVariableNameFactory implements VariableNameFactory { + + private final MultiValueMap variables; + + static LocalVariableNameFactory forMethod(MethodMetadata methodMetadata) { + return of(methodMetadata.getMethodArguments().keySet()); + } + + static LocalVariableNameFactory empty() { + return of(Set.of()); + } + + static LocalVariableNameFactory of(Set variables) { + return new LocalVariableNameFactory(variables); + } + + LocalVariableNameFactory(Iterable predefinedVariableNames) { + + variables = new LinkedMultiValueMap<>(); + for (String parameterName : predefinedVariableNames) { + variables.add(parameterName, new VariableName(parameterName)); + } + } + + @Override + public VariableName generateName(String suggestedName) { + + if (!variables.containsKey(suggestedName)) { + VariableName variableName = new VariableName(suggestedName); + variables.add(suggestedName, variableName); + return variableName; + } + + String targetName = suggestTargetName(suggestedName); + VariableName variableName = new VariableName(suggestedName, targetName); + variables.add(suggestedName, variableName); + variables.add(targetName, variableName); + return variableName; + } + + String suggestTargetName(String suggested) { + return suggestTargetName(suggested, 1); + } + + String suggestTargetName(String suggested, int counter) { + + String targetName = "%s_%s".formatted(suggested, counter); + if (!variables.containsKey(targetName)) { + return targetName; + } + return suggestTargetName(suggested, counter + 1); + } +} diff --git a/src/main/java/org/springframework/data/repository/aot/generate/VariableNameFactory.java b/src/main/java/org/springframework/data/repository/aot/generate/VariableNameFactory.java new file mode 100644 index 0000000000..1111474b04 --- /dev/null +++ b/src/main/java/org/springframework/data/repository/aot/generate/VariableNameFactory.java @@ -0,0 +1,36 @@ +/* + * Copyright 2025 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.data.repository.aot.generate; + +/** + * @author Christoph Strobl + * @since 4.0 + */ +public interface VariableNameFactory { + + VariableName generateName(String suggestedName); + + record VariableName(String source, String target) { + + public VariableName(String source) { + this(source, source); + } + + public String name() { + return target; + } + } +} diff --git a/src/test/java/org/springframework/data/repository/aot/generate/LocalVariableNameFactoryUnitTests.java b/src/test/java/org/springframework/data/repository/aot/generate/LocalVariableNameFactoryUnitTests.java new file mode 100644 index 0000000000..63cfc110a1 --- /dev/null +++ b/src/test/java/org/springframework/data/repository/aot/generate/LocalVariableNameFactoryUnitTests.java @@ -0,0 +1,62 @@ +/* + * Copyright 2025 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.data.repository.aot.generate; + +import static org.assertj.core.api.Assertions.assertThat; + +import java.util.Set; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +/** + * @author Christoph Strobl + */ +class LocalVariableNameFactoryUnitTests { + + LocalVariableNameFactory variableNameFactory; + + @BeforeEach + void beforeEach() { + variableNameFactory = LocalVariableNameFactory.of(Set.of("firstname", "lastname", "sort")); + } + + @Test // GH-3270 + void resolvesNameClashesInNames() { + + assertThat(variableNameFactory.generateName("name").name()).isEqualTo("name"); + assertThat(variableNameFactory.generateName("name").name()).isEqualTo("name_1"); + assertThat(variableNameFactory.generateName("name").name()).isEqualTo("name_2"); + assertThat(variableNameFactory.generateName("name1").name()).isEqualTo("name1"); + assertThat(variableNameFactory.generateName("name3").name()).isEqualTo("name3"); + assertThat(variableNameFactory.generateName("name3").name()).isEqualTo("name3_1"); + assertThat(variableNameFactory.generateName("name4_1").name()).isEqualTo("name4_1"); + assertThat(variableNameFactory.generateName("name4").name()).isEqualTo("name4"); + assertThat(variableNameFactory.generateName("name4_1_1").name()).isEqualTo("name4_1_1"); + assertThat(variableNameFactory.generateName("name4_1").name()).isEqualTo("name4_1_2"); + assertThat(variableNameFactory.generateName("name4_1").name()).isEqualTo("name4_1_3"); + } + + @Test // GH-3270 + void considersPredefinedNames() { + assertThat(variableNameFactory.generateName("firstname").name()).isEqualTo("firstname_1"); + } + + @Test // GH-3270 + void considersCase() { + assertThat(variableNameFactory.generateName("firstName").name()).isEqualTo("firstName"); + } +} From 4c9260b9a145d2f96dcbdcf0abc962726ee05266 Mon Sep 17 00:00:00 2001 From: Christoph Strobl Date: Fri, 2 May 2025 08:14:15 +0200 Subject: [PATCH 3/8] Use plain string instead of wrapper --- .../AotQueryMethodGenerationContext.java | 3 +-- .../generate/LocalVariableNameFactory.java | 18 ++++++------- .../aot/generate/VariableNameFactory.java | 12 +-------- .../LocalVariableNameFactoryUnitTests.java | 26 +++++++++---------- 4 files changed, 23 insertions(+), 36 deletions(-) diff --git a/src/main/java/org/springframework/data/repository/aot/generate/AotQueryMethodGenerationContext.java b/src/main/java/org/springframework/data/repository/aot/generate/AotQueryMethodGenerationContext.java index fc4fb6a6eb..5994e61b5d 100644 --- a/src/main/java/org/springframework/data/repository/aot/generate/AotQueryMethodGenerationContext.java +++ b/src/main/java/org/springframework/data/repository/aot/generate/AotQueryMethodGenerationContext.java @@ -28,7 +28,6 @@ import org.springframework.core.annotation.MergedAnnotation; import org.springframework.core.annotation.MergedAnnotationSelectors; import org.springframework.core.annotation.MergedAnnotations; -import org.springframework.data.repository.aot.generate.VariableNameFactory.VariableName; import org.springframework.data.repository.core.RepositoryInformation; import org.springframework.data.repository.query.Parameter; import org.springframework.data.repository.query.QueryMethod; @@ -135,7 +134,7 @@ public TypeName getReturnTypeName() { * @param variableName the intended variable name. * @return the suggested VariableName */ - public VariableName suggestLocalVariableName(String variableName) { + public String suggestLocalVariableName(String variableName) { return variableNameFactory.generateName(variableName); } diff --git a/src/main/java/org/springframework/data/repository/aot/generate/LocalVariableNameFactory.java b/src/main/java/org/springframework/data/repository/aot/generate/LocalVariableNameFactory.java index 63b56d1c28..adf3e8e0ff 100644 --- a/src/main/java/org/springframework/data/repository/aot/generate/LocalVariableNameFactory.java +++ b/src/main/java/org/springframework/data/repository/aot/generate/LocalVariableNameFactory.java @@ -29,7 +29,7 @@ */ class LocalVariableNameFactory implements VariableNameFactory { - private final MultiValueMap variables; + private final MultiValueMap variables; static LocalVariableNameFactory forMethod(MethodMetadata methodMetadata) { return of(methodMetadata.getMethodArguments().keySet()); @@ -47,24 +47,22 @@ static LocalVariableNameFactory of(Set variables) { variables = new LinkedMultiValueMap<>(); for (String parameterName : predefinedVariableNames) { - variables.add(parameterName, new VariableName(parameterName)); + variables.add(parameterName, parameterName); } } @Override - public VariableName generateName(String suggestedName) { + public String generateName(String suggestedName) { if (!variables.containsKey(suggestedName)) { - VariableName variableName = new VariableName(suggestedName); - variables.add(suggestedName, variableName); - return variableName; + variables.add(suggestedName, suggestedName); + return suggestedName; } String targetName = suggestTargetName(suggestedName); - VariableName variableName = new VariableName(suggestedName, targetName); - variables.add(suggestedName, variableName); - variables.add(targetName, variableName); - return variableName; + variables.add(suggestedName, targetName); + variables.add(targetName, targetName); + return targetName; } String suggestTargetName(String suggested) { diff --git a/src/main/java/org/springframework/data/repository/aot/generate/VariableNameFactory.java b/src/main/java/org/springframework/data/repository/aot/generate/VariableNameFactory.java index 1111474b04..b992052d1f 100644 --- a/src/main/java/org/springframework/data/repository/aot/generate/VariableNameFactory.java +++ b/src/main/java/org/springframework/data/repository/aot/generate/VariableNameFactory.java @@ -21,16 +21,6 @@ */ public interface VariableNameFactory { - VariableName generateName(String suggestedName); + String generateName(String suggestedName); - record VariableName(String source, String target) { - - public VariableName(String source) { - this(source, source); - } - - public String name() { - return target; - } - } } diff --git a/src/test/java/org/springframework/data/repository/aot/generate/LocalVariableNameFactoryUnitTests.java b/src/test/java/org/springframework/data/repository/aot/generate/LocalVariableNameFactoryUnitTests.java index 63cfc110a1..df1e3c61b8 100644 --- a/src/test/java/org/springframework/data/repository/aot/generate/LocalVariableNameFactoryUnitTests.java +++ b/src/test/java/org/springframework/data/repository/aot/generate/LocalVariableNameFactoryUnitTests.java @@ -37,26 +37,26 @@ void beforeEach() { @Test // GH-3270 void resolvesNameClashesInNames() { - assertThat(variableNameFactory.generateName("name").name()).isEqualTo("name"); - assertThat(variableNameFactory.generateName("name").name()).isEqualTo("name_1"); - assertThat(variableNameFactory.generateName("name").name()).isEqualTo("name_2"); - assertThat(variableNameFactory.generateName("name1").name()).isEqualTo("name1"); - assertThat(variableNameFactory.generateName("name3").name()).isEqualTo("name3"); - assertThat(variableNameFactory.generateName("name3").name()).isEqualTo("name3_1"); - assertThat(variableNameFactory.generateName("name4_1").name()).isEqualTo("name4_1"); - assertThat(variableNameFactory.generateName("name4").name()).isEqualTo("name4"); - assertThat(variableNameFactory.generateName("name4_1_1").name()).isEqualTo("name4_1_1"); - assertThat(variableNameFactory.generateName("name4_1").name()).isEqualTo("name4_1_2"); - assertThat(variableNameFactory.generateName("name4_1").name()).isEqualTo("name4_1_3"); + assertThat(variableNameFactory.generateName("name")).isEqualTo("name"); + assertThat(variableNameFactory.generateName("name")).isEqualTo("name_1"); + assertThat(variableNameFactory.generateName("name")).isEqualTo("name_2"); + assertThat(variableNameFactory.generateName("name1")).isEqualTo("name1"); + assertThat(variableNameFactory.generateName("name3")).isEqualTo("name3"); + assertThat(variableNameFactory.generateName("name3")).isEqualTo("name3_1"); + assertThat(variableNameFactory.generateName("name4_1")).isEqualTo("name4_1"); + assertThat(variableNameFactory.generateName("name4")).isEqualTo("name4"); + assertThat(variableNameFactory.generateName("name4_1_1")).isEqualTo("name4_1_1"); + assertThat(variableNameFactory.generateName("name4_1")).isEqualTo("name4_1_2"); + assertThat(variableNameFactory.generateName("name4_1")).isEqualTo("name4_1_3"); } @Test // GH-3270 void considersPredefinedNames() { - assertThat(variableNameFactory.generateName("firstname").name()).isEqualTo("firstname_1"); + assertThat(variableNameFactory.generateName("firstname")).isEqualTo("firstname_1"); } @Test // GH-3270 void considersCase() { - assertThat(variableNameFactory.generateName("firstName").name()).isEqualTo("firstName"); + assertThat(variableNameFactory.generateName("firstName")).isEqualTo("firstName"); } } From bf670a68133392b4736a92009cfce8115980a28f Mon Sep 17 00:00:00 2001 From: Christoph Strobl Date: Fri, 2 May 2025 10:19:19 +0200 Subject: [PATCH 4/8] Reduce visibility & add documentation (well a little at least) --- .../generate/LocalVariableNameFactory.java | 41 +++++++++++-------- .../aot/generate/VariableNameFactory.java | 16 +++++++- .../LocalVariableNameFactoryUnitTests.java | 10 +++++ 3 files changed, 48 insertions(+), 19 deletions(-) diff --git a/src/main/java/org/springframework/data/repository/aot/generate/LocalVariableNameFactory.java b/src/main/java/org/springframework/data/repository/aot/generate/LocalVariableNameFactory.java index adf3e8e0ff..419d63b74d 100644 --- a/src/main/java/org/springframework/data/repository/aot/generate/LocalVariableNameFactory.java +++ b/src/main/java/org/springframework/data/repository/aot/generate/LocalVariableNameFactory.java @@ -21,8 +21,8 @@ import org.springframework.util.MultiValueMap; /** - * {@link VariableNameFactory} implementation keeping track of defined names resolving name clashes using internal - * counter appending {@code _%d} to a suggested name in case of a clash. + * Non thread safe {@link VariableNameFactory} implementation keeping track of defined names resolving name clashes + * using internal counters appending {@code _%d} to a suggested name in case of a clash. * * @author Christoph Strobl * @since 4.0 @@ -31,36 +31,43 @@ class LocalVariableNameFactory implements VariableNameFactory { private final MultiValueMap variables; + /** + * Create a new {@link LocalVariableNameFactory} considering available {@link MethodMetadata#getMethodArguments() + * method arguments}. + * + * @param methodMetadata source metadata + * @return new instance of {@link LocalVariableNameFactory}. + */ static LocalVariableNameFactory forMethod(MethodMetadata methodMetadata) { return of(methodMetadata.getMethodArguments().keySet()); } - static LocalVariableNameFactory empty() { - return of(Set.of()); - } - - static LocalVariableNameFactory of(Set variables) { - return new LocalVariableNameFactory(variables); + /** + * Create a new {@link LocalVariableNameFactory} with a predefined set of initial variable names. + * + * @param predefinedVariables variables already known to be used in the given context. + * @return new instance of {@link LocalVariableNameFactory}. + */ + static LocalVariableNameFactory of(Set predefinedVariables) { + return new LocalVariableNameFactory(predefinedVariables); } LocalVariableNameFactory(Iterable predefinedVariableNames) { variables = new LinkedMultiValueMap<>(); - for (String parameterName : predefinedVariableNames) { - variables.add(parameterName, parameterName); - } + predefinedVariableNames.forEach(varName -> variables.add(varName, varName)); } @Override - public String generateName(String suggestedName) { + public String generateName(String intendedVariableName) { - if (!variables.containsKey(suggestedName)) { - variables.add(suggestedName, suggestedName); - return suggestedName; + if (!variables.containsKey(intendedVariableName)) { + variables.add(intendedVariableName, intendedVariableName); + return intendedVariableName; } - String targetName = suggestTargetName(suggestedName); - variables.add(suggestedName, targetName); + String targetName = suggestTargetName(intendedVariableName); + variables.add(intendedVariableName, targetName); variables.add(targetName, targetName); return targetName; } diff --git a/src/main/java/org/springframework/data/repository/aot/generate/VariableNameFactory.java b/src/main/java/org/springframework/data/repository/aot/generate/VariableNameFactory.java index b992052d1f..c5e4047fce 100644 --- a/src/main/java/org/springframework/data/repository/aot/generate/VariableNameFactory.java +++ b/src/main/java/org/springframework/data/repository/aot/generate/VariableNameFactory.java @@ -15,12 +15,24 @@ */ package org.springframework.data.repository.aot.generate; +import org.springframework.lang.CheckReturnValue; + /** + * Name factory for generating clash free variable names checking an intended name against predefined and already used + * ones. + * * @author Christoph Strobl * @since 4.0 */ -public interface VariableNameFactory { +interface VariableNameFactory { - String generateName(String suggestedName); + /** + * Compare and potentially generate a new name for the given intended variable name. + * + * @param intendedVariableName must not be {@literal null}. + * @return the {@literal intendedVariableName} if no naming clash detected or a clash free generated name. + */ + @CheckReturnValue + String generateName(String intendedVariableName); } diff --git a/src/test/java/org/springframework/data/repository/aot/generate/LocalVariableNameFactoryUnitTests.java b/src/test/java/org/springframework/data/repository/aot/generate/LocalVariableNameFactoryUnitTests.java index df1e3c61b8..3ca63aaa58 100644 --- a/src/test/java/org/springframework/data/repository/aot/generate/LocalVariableNameFactoryUnitTests.java +++ b/src/test/java/org/springframework/data/repository/aot/generate/LocalVariableNameFactoryUnitTests.java @@ -50,6 +50,16 @@ void resolvesNameClashesInNames() { assertThat(variableNameFactory.generateName("name4_1")).isEqualTo("name4_1_3"); } + @Test // GH-3270 + void worksWithVariablesContainingUnderscores() { + + assertThat(variableNameFactory.generateName("first_name")).isEqualTo("first_name"); + assertThat(variableNameFactory.generateName("first_name")).isEqualTo("first_name_1"); + assertThat(variableNameFactory.generateName("first_name")).isEqualTo("first_name_2"); + assertThat(variableNameFactory.generateName("first_name_3")).isEqualTo("first_name_3"); + assertThat(variableNameFactory.generateName("first_name")).isEqualTo("first_name_4"); + } + @Test // GH-3270 void considersPredefinedNames() { assertThat(variableNameFactory.generateName("firstname")).isEqualTo("firstname_1"); From b0ebc7fed873281ef27de19ce528ab2ccef0b0b0 Mon Sep 17 00:00:00 2001 From: Christoph Strobl Date: Fri, 2 May 2025 10:50:10 +0200 Subject: [PATCH 5/8] Move method parameter initialization to MethodMetadata. --- .../AotQueryMethodGenerationContext.java | 27 ++---- .../generate/AotRepositoryMethodBuilder.java | 24 ------ .../aot/generate/MethodMetadata.java | 48 +++++++++-- ...QueryMethodGenerationContextUnitTests.java | 70 +++++++++++++++ .../aot/generate/MethodMetadataUnitTests.java | 85 +++++++++++++++++++ 5 files changed, 202 insertions(+), 52 deletions(-) create mode 100644 src/test/java/org/springframework/data/repository/aot/generate/AotQueryMethodGenerationContextUnitTests.java create mode 100644 src/test/java/org/springframework/data/repository/aot/generate/MethodMetadataUnitTests.java diff --git a/src/main/java/org/springframework/data/repository/aot/generate/AotQueryMethodGenerationContext.java b/src/main/java/org/springframework/data/repository/aot/generate/AotQueryMethodGenerationContext.java index 5994e61b5d..abe49df347 100644 --- a/src/main/java/org/springframework/data/repository/aot/generate/AotQueryMethodGenerationContext.java +++ b/src/main/java/org/springframework/data/repository/aot/generate/AotQueryMethodGenerationContext.java @@ -19,7 +19,6 @@ import java.lang.reflect.Method; import java.util.ArrayList; import java.util.List; -import java.util.Map.Entry; import javax.lang.model.element.Modifier; @@ -64,8 +63,8 @@ public class AotQueryMethodGenerationContext { this.repositoryInformation = repositoryInformation; this.targetTypeMetadata = targetTypeMetadata; this.targetMethodMetadata = new MethodMetadata(repositoryInformation, method); - this.codeBlocks = new CodeBlocks(targetTypeMetadata); this.variableNameFactory = LocalVariableNameFactory.forMethod(targetMethodMetadata); + this.codeBlocks = new CodeBlocks(targetTypeMetadata); } AotRepositoryFragmentMetadata getTargetTypeMetadata() { @@ -129,7 +128,7 @@ public TypeName getReturnTypeName() { } /** - * Suggests naming clash free variant for the given intended variable name within the local method context. + * Suggest naming clash free variant for the given intended variable name within the local method context. * * @param variableName the intended variable name. * @return the suggested VariableName @@ -238,7 +237,7 @@ public List getBindableParameterNames() { List result = new ArrayList<>(); for (Parameter parameter : queryMethod.getParameters().getBindableParameters()) { - parameter.getName().map(result::add); + getParameterName(parameter.getIndex()); } return result; @@ -248,14 +247,7 @@ public List getBindableParameterNames() { * @return list of all parameter names (including non-bindable special parameters). */ public List getAllParameterNames() { - - List result = new ArrayList<>(); - - for (Parameter parameter : queryMethod.getParameters()) { - parameter.getName().map(result::add); - } - - return result; + return targetMethodMetadata.getMethodArguments().keySet().stream().toList(); } public boolean hasField(String fieldName) { @@ -280,16 +272,7 @@ public String getParameterNameOf(Class type) { } public @Nullable String getParameterName(int position) { - - if (0 > position) { - return null; - } - - List> entries = new ArrayList<>(targetMethodMetadata.getMethodArguments().entrySet()); - if (position < entries.size()) { - return entries.get(position).getKey(); - } - return null; + return targetMethodMetadata.getParameterName(position); } public void addParameter(ParameterSpec parameter) { diff --git a/src/main/java/org/springframework/data/repository/aot/generate/AotRepositoryMethodBuilder.java b/src/main/java/org/springframework/data/repository/aot/generate/AotRepositoryMethodBuilder.java index fdea9bf60a..0994f234cd 100644 --- a/src/main/java/org/springframework/data/repository/aot/generate/AotRepositoryMethodBuilder.java +++ b/src/main/java/org/springframework/data/repository/aot/generate/AotRepositoryMethodBuilder.java @@ -16,7 +16,6 @@ package org.springframework.data.repository.aot.generate; import java.lang.reflect.Method; -import java.lang.reflect.Parameter; import java.lang.reflect.TypeVariable; import java.util.function.BiConsumer; import java.util.function.Function; @@ -24,13 +23,8 @@ import javax.lang.model.element.Modifier; -import org.springframework.core.DefaultParameterNameDiscoverer; -import org.springframework.core.MethodParameter; -import org.springframework.core.ResolvableType; -import org.springframework.data.repository.core.RepositoryInformation; import org.springframework.javapoet.CodeBlock; import org.springframework.javapoet.MethodSpec; -import org.springframework.javapoet.ParameterSpec; import org.springframework.javapoet.TypeName; import org.springframework.javapoet.TypeVariableName; import org.springframework.util.StringUtils; @@ -50,25 +44,7 @@ class AotRepositoryMethodBuilder { private BiConsumer customizer = (context, body) -> {}; AotRepositoryMethodBuilder(AotQueryMethodGenerationContext context) { - this.context = context; - initParameters(context.getMethod(), context.getRepositoryInformation()); - } - - private void initParameters(Method method, RepositoryInformation repositoryInformation) { - - ResolvableType repositoryInterface = ResolvableType.forClass(repositoryInformation.getRepositoryInterface()); - - for (Parameter parameter : method.getParameters()) { - - MethodParameter methodParameter = MethodParameter.forParameter(parameter); - methodParameter.initParameterNameDiscovery(new DefaultParameterNameDiscoverer()); - ResolvableType resolvableParameterType = ResolvableType.forMethodParameter(methodParameter, repositoryInterface); - - TypeName parameterType = TypeName.get(resolvableParameterType.getType()); - - this.context.addParameter(ParameterSpec.builder(parameterType, methodParameter.getParameterName()).build()); - } } /** diff --git a/src/main/java/org/springframework/data/repository/aot/generate/MethodMetadata.java b/src/main/java/org/springframework/data/repository/aot/generate/MethodMetadata.java index a69ad48aed..a9a65a300d 100644 --- a/src/main/java/org/springframework/data/repository/aot/generate/MethodMetadata.java +++ b/src/main/java/org/springframework/data/repository/aot/generate/MethodMetadata.java @@ -16,12 +16,16 @@ package org.springframework.data.repository.aot.generate; import java.lang.reflect.Method; +import java.util.ArrayList; import java.util.LinkedHashMap; +import java.util.List; import java.util.Map; import java.util.Map.Entry; import org.jspecify.annotations.Nullable; - +import org.springframework.core.DefaultParameterNameDiscoverer; +import org.springframework.core.MethodParameter; +import org.springframework.core.ParameterNameDiscoverer; import org.springframework.core.ResolvableType; import org.springframework.data.repository.core.RepositoryInformation; import org.springframework.javapoet.ParameterSpec; @@ -31,6 +35,7 @@ * Metadata about an AOT Repository method. * * @author Christoph Strobl + * @since 4.0 */ class MethodMetadata { @@ -38,10 +43,11 @@ class MethodMetadata { private final ResolvableType actualReturnType; private final ResolvableType returnType; - public MethodMetadata(RepositoryInformation repositoryInformation, Method method) { + MethodMetadata(RepositoryInformation repositoryInformation, Method method) { this.returnType = repositoryInformation.getReturnType(method).toResolvableType(); this.actualReturnType = ResolvableType.forType(repositoryInformation.getReturnedDomainClass(method)); + this.initParameters(repositoryInformation, method, new DefaultParameterNameDiscoverer()); } @Nullable @@ -54,20 +60,50 @@ public String getParameterNameOf(Class type) { return null; } - public ResolvableType getReturnType() { + ResolvableType getReturnType() { return returnType; } - public ResolvableType getActualReturnType() { + ResolvableType getActualReturnType() { return actualReturnType; } - public void addParameter(ParameterSpec parameterSpec) { + void addParameter(ParameterSpec parameterSpec) { this.methodArguments.put(parameterSpec.name, parameterSpec); } - public Map getMethodArguments() { + Map getMethodArguments() { return methodArguments; } + @Nullable + String getParameterName(int position) { + + if (0 > position) { + return null; + } + + List> entries = new ArrayList<>(methodArguments.entrySet()); + if (position < entries.size()) { + return entries.get(position).getKey(); + } + return null; + } + + private void initParameters(RepositoryInformation repositoryInformation, Method method, + ParameterNameDiscoverer nameDiscoverer) { + + ResolvableType repositoryInterface = ResolvableType.forClass(repositoryInformation.getRepositoryInterface()); + + for (java.lang.reflect.Parameter parameter : method.getParameters()) { + + MethodParameter methodParameter = MethodParameter.forParameter(parameter); + methodParameter.initParameterNameDiscovery(nameDiscoverer); + ResolvableType resolvableParameterType = ResolvableType.forMethodParameter(methodParameter, repositoryInterface); + + TypeName parameterType = TypeName.get(resolvableParameterType.getType()); + + addParameter(ParameterSpec.builder(parameterType, methodParameter.getParameterName()).build()); + } + } } diff --git a/src/test/java/org/springframework/data/repository/aot/generate/AotQueryMethodGenerationContextUnitTests.java b/src/test/java/org/springframework/data/repository/aot/generate/AotQueryMethodGenerationContextUnitTests.java new file mode 100644 index 0000000000..e76187303e --- /dev/null +++ b/src/test/java/org/springframework/data/repository/aot/generate/AotQueryMethodGenerationContextUnitTests.java @@ -0,0 +1,70 @@ +/* + * Copyright 2025 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.data.repository.aot.generate; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.eq; + +import java.lang.reflect.Method; + +import org.junit.jupiter.api.Test; +import org.mockito.Mockito; +import org.springframework.data.domain.Pageable; +import org.springframework.data.repository.core.RepositoryInformation; +import org.springframework.data.repository.query.QueryMethod; +import org.springframework.data.util.TypeInformation; + +/** + * Tests targeting {@link AotQueryMethodGenerationContext}. + * + * @author Christoph Strobl + */ +class AotQueryMethodGenerationContextUnitTests { + + @Test // GH-3270 + void suggestLocalVariableNameConsidersMethodArguments() throws NoSuchMethodException { + + AotQueryMethodGenerationContext ctx = ctxFor("reservedParameterMethod"); + + assertThat(ctx.suggestLocalVariableName("foo")).isEqualTo("foo"); + assertThat(ctx.suggestLocalVariableName("arg0")).isNotIn("arg0", "arg1", "arg2"); + } + + AotQueryMethodGenerationContext ctxFor(String methodName) throws NoSuchMethodException { + + Method target = null; + for (Method m : DummyRepo.class.getMethods()) { + if (m.getName().equals(methodName)) { + target = m; + break; + } + } + + if (target == null) { + throw new NoSuchMethodException(methodName); + } + + RepositoryInformation ri = Mockito.mock(RepositoryInformation.class); + Mockito.doReturn(TypeInformation.of(target.getReturnType())).when(ri).getReturnType(eq(target)); + + return new AotQueryMethodGenerationContext(ri, target, Mockito.mock(QueryMethod.class), + Mockito.mock(AotRepositoryFragmentMetadata.class)); + } + + private interface DummyRepo { + String reservedParameterMethod(Object arg0, Pageable arg1, Object arg2); + } +} diff --git a/src/test/java/org/springframework/data/repository/aot/generate/MethodMetadataUnitTests.java b/src/test/java/org/springframework/data/repository/aot/generate/MethodMetadataUnitTests.java new file mode 100644 index 0000000000..8cc981251a --- /dev/null +++ b/src/test/java/org/springframework/data/repository/aot/generate/MethodMetadataUnitTests.java @@ -0,0 +1,85 @@ +/* + * Copyright 2025 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.data.repository.aot.generate; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.eq; + +import java.lang.reflect.Method; + +import org.junit.jupiter.api.Test; +import org.mockito.Mockito; +import org.springframework.data.domain.Pageable; +import org.springframework.data.repository.core.RepositoryInformation; +import org.springframework.data.util.TypeInformation; + +/** + * Unit tests for {@link MethodMetadata}. + * + * @author Christoph Strobl + */ +class MethodMetadataUnitTests { + + @Test // GH-3270 + void getParameterNameByIndex() throws NoSuchMethodException { + + MethodMetadata metadata = methodMetadataFor("threeArgsMethod"); + + assertThat(metadata.getParameterName(0)).isEqualTo("arg0"); + assertThat(metadata.getParameterName(1)).isEqualTo("arg1"); + assertThat(metadata.getParameterName(2)).isEqualTo("arg2"); + } + + @Test // GH-3270 + void getParameterNameByNonExistingIndex() throws NoSuchMethodException { + + MethodMetadata metadata = methodMetadataFor("threeArgsMethod"); + + assertThat(metadata.getParameterName(-1)).isNull(); + assertThat(metadata.getParameterName(3)).isNull(); + } + + @Test // GH-3270 + void getParameterNameForNoArgsMethod() throws NoSuchMethodException { + assertThat(methodMetadataFor("noArgsMethod").getParameterName(0)).isNull(); + } + + static MethodMetadata methodMetadataFor(String methodName) throws NoSuchMethodException { + + Method target = null; + for (Method m : DummyRepo.class.getMethods()) { + if (m.getName().equals(methodName)) { + target = m; + break; + } + } + + if (target == null) { + throw new NoSuchMethodException(methodName); + } + + RepositoryInformation ri = Mockito.mock(RepositoryInformation.class); + Mockito.doReturn(TypeInformation.of(target.getReturnType())).when(ri).getReturnType(eq(target)); + return new MethodMetadata(ri, target); + } + + private interface DummyRepo { + + String noArgsMethod(); + + String threeArgsMethod(Object arg0, Pageable arg1, Object arg2); + } +} From 25102c56caf39d42c6aa03c6ee1c0e87fa3c0fa4 Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Thu, 8 May 2025 10:51:55 +0200 Subject: [PATCH 6/8] Polishing. Refine local variable handling to logical and physical naming where the logical name is used in AOT contributor code while the physical name is rendered. --- .../generate/AotQueryMethodGenerationContext.java | 11 ++++++----- .../aot/generate/AotRepositoryBuilder.java | 11 ++++++++++- .../aot/generate/LocalVariableNameFactory.java | 15 ++++++++------- .../repository/aot/generate/MethodMetadata.java | 6 ++++++ .../AotQueryMethodGenerationContextUnitTests.java | 10 ++++++---- 5 files changed, 36 insertions(+), 17 deletions(-) diff --git a/src/main/java/org/springframework/data/repository/aot/generate/AotQueryMethodGenerationContext.java b/src/main/java/org/springframework/data/repository/aot/generate/AotQueryMethodGenerationContext.java index abe49df347..b4275f2a23 100644 --- a/src/main/java/org/springframework/data/repository/aot/generate/AotQueryMethodGenerationContext.java +++ b/src/main/java/org/springframework/data/repository/aot/generate/AotQueryMethodGenerationContext.java @@ -128,13 +128,14 @@ public TypeName getReturnTypeName() { } /** - * Suggest naming clash free variant for the given intended variable name within the local method context. + * Obtain a naming-clash free variant for the given logical variable name within the local method context. Returns the + * target variable name when called multiple times with the same {@code variableName}. * - * @param variableName the intended variable name. - * @return the suggested VariableName + * @param variableName the logical variable name. + * @return the variable name used in the generated code. */ - public String suggestLocalVariableName(String variableName) { - return variableNameFactory.generateName(variableName); + public String localVariable(String variableName) { + return targetMethodMetadata.getLocalVariables().computeIfAbsent(variableName, variableNameFactory::generateName); } /** diff --git a/src/main/java/org/springframework/data/repository/aot/generate/AotRepositoryBuilder.java b/src/main/java/org/springframework/data/repository/aot/generate/AotRepositoryBuilder.java index d26fd21f37..958511266f 100644 --- a/src/main/java/org/springframework/data/repository/aot/generate/AotRepositoryBuilder.java +++ b/src/main/java/org/springframework/data/repository/aot/generate/AotRepositoryBuilder.java @@ -52,6 +52,8 @@ */ class AotRepositoryBuilder { + private static final Log logger = LogFactory.getLog(AotRepositoryBuilder.class); + private final RepositoryInformation repositoryInformation; private final String moduleName; private final ProjectionFactory projectionFactory; @@ -146,7 +148,14 @@ public AotBundle build() { return it.getDeclaringClass().getName(); }).thenComparing(Method::getName).thenComparing(Method::getParameterCount).thenComparing(Method::toString)) .forEach(method -> { - contributeMethod(method, repositoryComposition, methodMetadata, builder); + try { + contributeMethod(method, repositoryComposition, methodMetadata, builder); + } catch (RuntimeException e) { + if (logger.isErrorEnabled()) { + logger.error("Failed to contribute Repository method [%s.%s]" + .formatted(repositoryInformation.getRepositoryInterface().getName(), method.getName()), e); + } + } }); // write fields at the end so we make sure to capture things added by methods diff --git a/src/main/java/org/springframework/data/repository/aot/generate/LocalVariableNameFactory.java b/src/main/java/org/springframework/data/repository/aot/generate/LocalVariableNameFactory.java index 419d63b74d..01d0186432 100644 --- a/src/main/java/org/springframework/data/repository/aot/generate/LocalVariableNameFactory.java +++ b/src/main/java/org/springframework/data/repository/aot/generate/LocalVariableNameFactory.java @@ -31,10 +31,16 @@ class LocalVariableNameFactory implements VariableNameFactory { private final MultiValueMap variables; + LocalVariableNameFactory(Iterable predefinedVariableNames) { + + variables = new LinkedMultiValueMap<>(); + predefinedVariableNames.forEach(varName -> variables.add(varName, varName)); + } + /** * Create a new {@link LocalVariableNameFactory} considering available {@link MethodMetadata#getMethodArguments() * method arguments}. - * + * * @param methodMetadata source metadata * @return new instance of {@link LocalVariableNameFactory}. */ @@ -52,12 +58,6 @@ static LocalVariableNameFactory of(Set predefinedVariables) { return new LocalVariableNameFactory(predefinedVariables); } - LocalVariableNameFactory(Iterable predefinedVariableNames) { - - variables = new LinkedMultiValueMap<>(); - predefinedVariableNames.forEach(varName -> variables.add(varName, varName)); - } - @Override public String generateName(String intendedVariableName) { @@ -84,4 +84,5 @@ String suggestTargetName(String suggested, int counter) { } return suggestTargetName(suggested, counter + 1); } + } diff --git a/src/main/java/org/springframework/data/repository/aot/generate/MethodMetadata.java b/src/main/java/org/springframework/data/repository/aot/generate/MethodMetadata.java index a9a65a300d..dd9885933c 100644 --- a/src/main/java/org/springframework/data/repository/aot/generate/MethodMetadata.java +++ b/src/main/java/org/springframework/data/repository/aot/generate/MethodMetadata.java @@ -35,11 +35,13 @@ * Metadata about an AOT Repository method. * * @author Christoph Strobl + * @author Mark Paluch * @since 4.0 */ class MethodMetadata { private final Map methodArguments = new LinkedHashMap<>(); + private final Map localVariables = new LinkedHashMap<>(); private final ResolvableType actualReturnType; private final ResolvableType returnType; @@ -90,6 +92,10 @@ String getParameterName(int position) { return null; } + Map getLocalVariables() { + return localVariables; + } + private void initParameters(RepositoryInformation repositoryInformation, Method method, ParameterNameDiscoverer nameDiscoverer) { diff --git a/src/test/java/org/springframework/data/repository/aot/generate/AotQueryMethodGenerationContextUnitTests.java b/src/test/java/org/springframework/data/repository/aot/generate/AotQueryMethodGenerationContextUnitTests.java index e76187303e..8f63192006 100644 --- a/src/test/java/org/springframework/data/repository/aot/generate/AotQueryMethodGenerationContextUnitTests.java +++ b/src/test/java/org/springframework/data/repository/aot/generate/AotQueryMethodGenerationContextUnitTests.java @@ -15,13 +15,14 @@ */ package org.springframework.data.repository.aot.generate; -import static org.assertj.core.api.Assertions.assertThat; -import static org.mockito.ArgumentMatchers.eq; +import static org.assertj.core.api.Assertions.*; +import static org.mockito.ArgumentMatchers.*; import java.lang.reflect.Method; import org.junit.jupiter.api.Test; import org.mockito.Mockito; + import org.springframework.data.domain.Pageable; import org.springframework.data.repository.core.RepositoryInformation; import org.springframework.data.repository.query.QueryMethod; @@ -31,6 +32,7 @@ * Tests targeting {@link AotQueryMethodGenerationContext}. * * @author Christoph Strobl + * @author Mark Paluch */ class AotQueryMethodGenerationContextUnitTests { @@ -39,8 +41,8 @@ void suggestLocalVariableNameConsidersMethodArguments() throws NoSuchMethodExcep AotQueryMethodGenerationContext ctx = ctxFor("reservedParameterMethod"); - assertThat(ctx.suggestLocalVariableName("foo")).isEqualTo("foo"); - assertThat(ctx.suggestLocalVariableName("arg0")).isNotIn("arg0", "arg1", "arg2"); + assertThat(ctx.localVariable("foo")).isEqualTo("foo"); + assertThat(ctx.localVariable("arg0")).isNotIn("arg0", "arg1", "arg2"); } AotQueryMethodGenerationContext ctxFor(String methodName) throws NoSuchMethodException { From b21987153dc0ad92701a5adf2b0da86461496573 Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Thu, 8 May 2025 11:34:30 +0200 Subject: [PATCH 7/8] Polishing. Remove CodeBlocks, reduce visibility where possible. --- .../AotQueryMethodGenerationContext.java | 118 +++++++++--------- .../aot/generate/AotRepositoryBuilder.java | 12 +- .../AotRepositoryConstructorBuilder.java | 39 +++--- .../AotRepositoryFragmentMetadata.java | 25 +--- .../repository/aot/generate/CodeBlocks.java | 66 ---------- .../aot/generate/RepositoryContributor.java | 2 +- 6 files changed, 93 insertions(+), 169 deletions(-) delete mode 100644 src/main/java/org/springframework/data/repository/aot/generate/CodeBlocks.java diff --git a/src/main/java/org/springframework/data/repository/aot/generate/AotQueryMethodGenerationContext.java b/src/main/java/org/springframework/data/repository/aot/generate/AotQueryMethodGenerationContext.java index b4275f2a23..076a66c8c8 100644 --- a/src/main/java/org/springframework/data/repository/aot/generate/AotQueryMethodGenerationContext.java +++ b/src/main/java/org/springframework/data/repository/aot/generate/AotQueryMethodGenerationContext.java @@ -20,9 +20,8 @@ import java.util.ArrayList; import java.util.List; -import javax.lang.model.element.Modifier; - import org.jspecify.annotations.Nullable; + import org.springframework.core.ResolvableType; import org.springframework.core.annotation.MergedAnnotation; import org.springframework.core.annotation.MergedAnnotationSelectors; @@ -31,8 +30,6 @@ import org.springframework.data.repository.query.Parameter; import org.springframework.data.repository.query.QueryMethod; import org.springframework.data.repository.query.ReturnedType; -import org.springframework.javapoet.FieldSpec; -import org.springframework.javapoet.ParameterSpec; import org.springframework.javapoet.TypeName; import org.springframework.util.ObjectUtils; @@ -51,7 +48,6 @@ public class AotQueryMethodGenerationContext { private final RepositoryInformation repositoryInformation; private final AotRepositoryFragmentMetadata targetTypeMetadata; private final MethodMetadata targetMethodMetadata; - private final CodeBlocks codeBlocks; private final VariableNameFactory variableNameFactory; AotQueryMethodGenerationContext(RepositoryInformation repositoryInformation, Method method, QueryMethod queryMethod, @@ -64,11 +60,6 @@ public class AotQueryMethodGenerationContext { this.targetTypeMetadata = targetTypeMetadata; this.targetMethodMetadata = new MethodMetadata(repositoryInformation, method); this.variableNameFactory = LocalVariableNameFactory.forMethod(targetMethodMetadata); - this.codeBlocks = new CodeBlocks(targetTypeMetadata); - } - - AotRepositoryFragmentMetadata getTargetTypeMetadata() { - return targetTypeMetadata; } MethodMetadata getTargetMethodMetadata() { @@ -79,12 +70,18 @@ public RepositoryInformation getRepositoryInformation() { return repositoryInformation; } - public Method getMethod() { - return method; + /** + * Obtain the field name by type. + * + * @param type + * @return + */ + public @Nullable String fieldNameOf(Class type) { + return targetTypeMetadata.fieldNameOf(type); } - public CodeBlocks codeBlocks() { - return codeBlocks; + public Method getMethod() { + return method; } /** @@ -112,10 +109,18 @@ public ReturnedType getReturnedType() { return queryMethod.getResultProcessor().getReturnedType(); } + /** + * @return the actual returned domain type. + * @see org.springframework.data.repository.core.RepositoryMetadata#getReturnedDomainClass(Method) + */ public ResolvableType getActualReturnType() { return targetMethodMetadata.getActualReturnType(); } + /** + * @return the query method return type. + * @see org.springframework.data.repository.core.RepositoryMetadata#getReturnType(Method) + */ public ResolvableType getReturnType() { return targetMethodMetadata.getReturnType(); } @@ -127,24 +132,13 @@ public TypeName getReturnTypeName() { return TypeName.get(getReturnType().getType()); } - /** - * Obtain a naming-clash free variant for the given logical variable name within the local method context. Returns the - * target variable name when called multiple times with the same {@code variableName}. - * - * @param variableName the logical variable name. - * @return the variable name used in the generated code. - */ - public String localVariable(String variableName) { - return targetMethodMetadata.getLocalVariables().computeIfAbsent(variableName, variableNameFactory::generateName); - } - /** * Returns the required parameter name for the {@link Parameter#isBindable() bindable parameter} at the given * {@code parameterIndex} or throws {@link IllegalArgumentException} if the parameter cannot be determined by its * index. * * @param parameterIndex the zero-based parameter index as used in the query to reference bindable parameters. - * @return the parameter name. + * @return the method parameter name. */ public String getRequiredBindableParameterName(int parameterIndex) { @@ -162,9 +156,8 @@ public String getRequiredBindableParameterName(int parameterIndex) { * {@code parameterIndex} or {@code null} if the parameter cannot be determined by its index. * * @param parameterIndex the zero-based parameter index as used in the query to reference bindable parameters. - * @return the parameter name. + * @return the method parameter name. */ - // TODO: Simplify?! public @Nullable String getBindableParameterName(int parameterIndex) { int bindable = 0; @@ -186,12 +179,12 @@ public String getRequiredBindableParameterName(int parameterIndex) { } /** - * Returns the required parameter name for the {@link Parameter#isBindable() bindable parameter} at the given - * {@code parameterName} or throws {@link IllegalArgumentException} if the parameter cannot be determined by its - * index. + * Returns the required parameter name for the {@link Parameter#isBindable() bindable parameter} at the given logical + * {@code parameterName} or throws {@link IllegalArgumentException} if the parameter cannot be determined by its name. * * @param parameterName the parameter name as used in the query to reference bindable parameters. - * @return the parameter name. + * @return the method parameter name. + * @see org.springframework.data.repository.query.Param */ public String getRequiredBindableParameterName(String parameterName) { @@ -205,13 +198,13 @@ public String getRequiredBindableParameterName(String parameterName) { } /** - * Returns the required parameter name for the {@link Parameter#isBindable() bindable parameter} at the given - * {@code parameterName} or {@code null} if the parameter cannot be determined by its index. + * Returns the required parameter name for the {@link Parameter#isBindable() bindable parameter} at the given logical + * {@code parameterName} or {@code null} if the parameter cannot be determined by its name. * * @param parameterName the parameter name as used in the query to reference bindable parameters. - * @return the parameter name. + * @return the method parameter name. + * @see org.springframework.data.repository.query.Param */ - // TODO: Simplify?! public @Nullable String getBindableParameterName(String parameterName) { int totalIndex = 0; @@ -238,7 +231,7 @@ public List getBindableParameterNames() { List result = new ArrayList<>(); for (Parameter parameter : queryMethod.getParameters().getBindableParameters()) { - getParameterName(parameter.getIndex()); + result.add(getParameterName(parameter.getIndex())); } return result; @@ -251,45 +244,50 @@ public List getAllParameterNames() { return targetMethodMetadata.getMethodArguments().keySet().stream().toList(); } - public boolean hasField(String fieldName) { - return targetTypeMetadata.hasField(fieldName); - } - - public void addField(String fieldName, TypeName type, Modifier... modifiers) { - targetTypeMetadata.addField(fieldName, type, modifiers); - } - - public void addField(FieldSpec fieldSpec) { - targetTypeMetadata.addField(fieldSpec); - } - - public @Nullable String fieldNameOf(Class type) { - return targetTypeMetadata.fieldNameOf(type); - } - - @Nullable - public String getParameterNameOf(Class type) { - return targetMethodMetadata.getParameterNameOf(type); + /** + * Obtain a naming-clash free variant for the given logical variable name within the local method context. Returns the + * target variable name when called multiple times with the same {@code variableName}. + * + * @param variableName the logical variable name. + * @return the variable name used in the generated code. + */ + public String localVariable(String variableName) { + return targetMethodMetadata.getLocalVariables().computeIfAbsent(variableName, variableNameFactory::generateName); } + /** + * Returns the parameter name for the method parameter at {@code position}. + * + * @param position zero-indexed parameter position. + * @return + * @see Method#getParameters() + */ public @Nullable String getParameterName(int position) { return targetMethodMetadata.getParameterName(position); } - public void addParameter(ParameterSpec parameter) { - this.targetMethodMetadata.addParameter(parameter); - } - + /** + * @return the parameter name for the {@link org.springframework.data.domain.Sort sort parameter} or {@code null} if + * the method does not declare a sort parameter. + */ @Nullable public String getSortParameterName() { return getParameterName(queryMethod.getParameters().getSortIndex()); } + /** + * @return the parameter name for the {@link org.springframework.data.domain.Pageable pageable parameter} or + * {@code null} if the method does not declare a pageable parameter. + */ @Nullable public String getPageableParameterName() { return getParameterName(queryMethod.getParameters().getPageableIndex()); } + /** + * @return the parameter name for the {@link org.springframework.data.domain.Limit limit parameter} or {@code null} if + * the method does not declare a limit parameter. + */ @Nullable public String getLimitParameterName() { return getParameterName(queryMethod.getParameters().getLimitIndex()); diff --git a/src/main/java/org/springframework/data/repository/aot/generate/AotRepositoryBuilder.java b/src/main/java/org/springframework/data/repository/aot/generate/AotRepositoryBuilder.java index 958511266f..84f344331b 100644 --- a/src/main/java/org/springframework/data/repository/aot/generate/AotRepositoryBuilder.java +++ b/src/main/java/org/springframework/data/repository/aot/generate/AotRepositoryBuilder.java @@ -76,7 +76,7 @@ private AotRepositoryBuilder(RepositoryInformation repositoryInformation, String .initializer("$T.getLog($T.class)", TypeName.get(LogFactory.class), this.generationMetadata.getTargetTypeName()) .build()); - this.customizer = (info, metadata, builder) -> {}; + this.customizer = (info, builder) -> {}; } /** @@ -162,8 +162,7 @@ public AotBundle build() { generationMetadata.getFields().values().forEach(builder::addField); // finally customize the file itself - this.customizer.customize(repositoryInformation, generationMetadata, builder); - + this.customizer.customize(repositoryInformation, builder); JavaFile javaFile = JavaFile.builder(packageName(), builder.build()).build(); AotRepositoryMetadata metadata = getAotRepositoryMetadata(methodMetadata); @@ -282,11 +281,10 @@ public interface ClassCustomizer { /** * Apply customization ot the AOT repository fragment class after it has been defined. * - * @param information repository information. - * @param metadata metadata of the AOT repository fragment. - * @param builder the actual builder. + * @param information the repository information that is used for the AOT fragment. + * @param builder the class builder to be customized. */ - void customize(RepositoryInformation information, AotRepositoryFragmentMetadata metadata, TypeSpec.Builder builder); + void customize(RepositoryInformation information, TypeSpec.Builder builder); } diff --git a/src/main/java/org/springframework/data/repository/aot/generate/AotRepositoryConstructorBuilder.java b/src/main/java/org/springframework/data/repository/aot/generate/AotRepositoryConstructorBuilder.java index e30252bae8..51b08d63c0 100644 --- a/src/main/java/org/springframework/data/repository/aot/generate/AotRepositoryConstructorBuilder.java +++ b/src/main/java/org/springframework/data/repository/aot/generate/AotRepositoryConstructorBuilder.java @@ -33,7 +33,6 @@ * @author Mark Paluch * @since 4.0 */ -// TODO: extract constructor contributor in a similar way to MethodContributor. public class AotRepositoryConstructorBuilder { private final RepositoryInformation repositoryInformation; @@ -41,18 +40,17 @@ public class AotRepositoryConstructorBuilder { private ConstructorCustomizer customizer = (info, builder) -> {}; - AotRepositoryConstructorBuilder(RepositoryInformation repositoryInformation, - AotRepositoryFragmentMetadata metadata) { + AotRepositoryConstructorBuilder(RepositoryInformation repositoryInformation, AotRepositoryFragmentMetadata metadata) { this.repositoryInformation = repositoryInformation; this.metadata = metadata; } /** - * Add constructor parameter. + * Add constructor parameter and create a field storing its value. * - * @param parameterName - * @param type + * @param parameterName name of the parameter. + * @param type parameter type. */ public void addParameter(String parameterName, Class type) { @@ -61,14 +59,15 @@ public void addParameter(String parameterName, Class type) { addParameter(parameterName, TypeName.get(type)); return; } + addParameter(parameterName, ParameterizedTypeName.get(type, resolvableType.resolveGenerics())); } /** - * Add constructor parameter and create a field for it. + * Add constructor parameter and create a field storing its value. * - * @param parameterName - * @param type + * @param parameterName name of the parameter. + * @param type parameter type. */ public void addParameter(String parameterName, TypeName type) { addParameter(parameterName, type, true); @@ -77,13 +76,15 @@ public void addParameter(String parameterName, TypeName type) { /** * Add constructor parameter. * - * @param parameterName - * @param type + * @param parameterName name of the parameter. + * @param type parameter type. + * @param createField whether to create a field for the parameter and assign its value to the field. */ public void addParameter(String parameterName, TypeName type, boolean createField) { this.metadata.addConstructorArgument(parameterName, type, createField ? parameterName : null); - if(createField) { + + if (createField) { this.metadata.addField(parameterName, type, Modifier.PRIVATE, Modifier.FINAL); } } @@ -92,7 +93,7 @@ public void addParameter(String parameterName, TypeName type, boolean createFiel * Add constructor customizer. Customizer is invoked after adding constructor arguments and before assigning * constructor arguments to fields. * - * @param customizer + * @param customizer the customizer with direct access to the {@link MethodSpec.Builder constructor builder}. */ public void customize(ConstructorCustomizer customizer) { this.customizer = customizer; @@ -109,9 +110,8 @@ MethodSpec buildConstructor() { customizer.customize(repositoryInformation, builder); for (Entry parameter : this.metadata.getConstructorArguments().entrySet()) { - if(parameter.getValue().isForLocalField()) { - builder.addStatement("this.$N = $N", parameter.getKey(), - parameter.getKey()); + if (parameter.getValue().isForLocalField()) { + builder.addStatement("this.$N = $N", parameter.getKey(), parameter.getKey()); } } @@ -123,7 +123,14 @@ MethodSpec buildConstructor() { */ public interface ConstructorCustomizer { + /** + * Customize the constructor. + * + * @param information the repository information that is used for the AOT fragment. + * @param builder the constructor builder to be customized. + */ void customize(RepositoryInformation information, MethodSpec.Builder builder); + } } diff --git a/src/main/java/org/springframework/data/repository/aot/generate/AotRepositoryFragmentMetadata.java b/src/main/java/org/springframework/data/repository/aot/generate/AotRepositoryFragmentMetadata.java index 0e96884206..8d51fac38a 100644 --- a/src/main/java/org/springframework/data/repository/aot/generate/AotRepositoryFragmentMetadata.java +++ b/src/main/java/org/springframework/data/repository/aot/generate/AotRepositoryFragmentMetadata.java @@ -29,10 +29,13 @@ import org.springframework.javapoet.TypeName; /** + * Metadata for a repository fragment. + * * @author Christoph Strobl + * @author Mark Paluch + * @since 4.0 */ -// TODO: Can we make this package-private? -public class AotRepositoryFragmentMetadata { +class AotRepositoryFragmentMetadata { private final ClassName className; private final Map fields = new HashMap<>(3); @@ -59,18 +62,6 @@ public ClassName getTargetTypeName() { return className; } - public String getTargetTypeSimpleName() { - return className.simpleName(); - } - - public String getTargetTypePackageName() { - return className.packageName(); - } - - public boolean hasField(String fieldName) { - return fields.containsKey(fieldName); - } - public void addField(String fieldName, TypeName type, Modifier... modifiers) { fields.put(fieldName, FieldSpec.builder(type, fieldName, modifiers).build()); } @@ -79,7 +70,7 @@ public void addField(FieldSpec fieldSpec) { fields.put(fieldSpec.name, fieldSpec); } - Map getFields() { + public Map getFields() { return fields; } @@ -87,10 +78,6 @@ public Map getConstructorArguments() { return constructorArguments; } - public void addConstructorArgument(String parameterName, TypeName type) { - addConstructorArgument(parameterName, type, parameterName); - } - public void addConstructorArgument(String parameterName, TypeName type, @Nullable String fieldName) { this.constructorArguments.put(parameterName, new ConstructorArgument(parameterName, type, fieldName)); } diff --git a/src/main/java/org/springframework/data/repository/aot/generate/CodeBlocks.java b/src/main/java/org/springframework/data/repository/aot/generate/CodeBlocks.java deleted file mode 100644 index 7da35aa904..0000000000 --- a/src/main/java/org/springframework/data/repository/aot/generate/CodeBlocks.java +++ /dev/null @@ -1,66 +0,0 @@ -/* - * Copyright 2025 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.data.repository.aot.generate; - -import org.apache.commons.logging.Log; -import org.springframework.javapoet.CodeBlock; -import org.springframework.util.ObjectUtils; -import org.springframework.util.StringUtils; - -/** - * Helper to write contextual pieces of code during code generation. - * - * @author Christoph Strobl - */ -@Deprecated(forRemoval = true) -public class CodeBlocks { - - private final AotRepositoryFragmentMetadata metadata; - - CodeBlocks(AotRepositoryFragmentMetadata metadata) { - this.metadata = metadata; - } - - /** - * @param message the logging message. - * @param args optional args to apply to the message. - * @return a {@link CodeBlock} containing a debug level guarded logging statement. - */ - public CodeBlock logDebug(String message, Object... args) { - return log("debug", message, args); - } - - /** - * @param level the log level eg. `debug`. - * @param message the message to print/ - * @param args optional args to be applied to the message. - * @return a {@link CodeBlock} containing a level guarded logging statement. - */ - private CodeBlock log(String level, String message, Object... args) { - - CodeBlock.Builder builder = CodeBlock.builder(); - builder.beginControlFlow("if($L.is$LEnabled())", metadata.fieldNameOf(Log.class), StringUtils.capitalize(level)); - if (ObjectUtils.isEmpty(args)) { - builder.addStatement("$L.$L($S)", metadata.fieldNameOf(Log.class), level, message); - } else { - builder.addStatement("$L.$L($S.formatted($L))", metadata.fieldNameOf(Log.class), level, message, - StringUtils.arrayToCommaDelimitedString(args)); - } - builder.endControlFlow(); - return builder.build(); - } - -} diff --git a/src/main/java/org/springframework/data/repository/aot/generate/RepositoryContributor.java b/src/main/java/org/springframework/data/repository/aot/generate/RepositoryContributor.java index 6c66c7ffee..ef79269325 100644 --- a/src/main/java/org/springframework/data/repository/aot/generate/RepositoryContributor.java +++ b/src/main/java/org/springframework/data/repository/aot/generate/RepositoryContributor.java @@ -136,7 +136,7 @@ private static String getRepositoryJsonFileName(Class repositoryInterface) { /** * Customization hook for store implementations to customize class after building the entire class. */ - protected void customizeClass(RepositoryInformation information, AotRepositoryFragmentMetadata metadata, + protected void customizeClass(RepositoryInformation information, TypeSpec.Builder builder) { } From da9e0870d1a7c22a2580dfb31d6ed7e4f465868b Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Thu, 8 May 2025 15:34:01 +0200 Subject: [PATCH 8/8] Reduce API surface, expose interfaces only. --- .../aot/generate/AotRepositoryBuilder.java | 48 +++---- .../generate/AotRepositoryClassBuilder.java | 49 +++++++ .../AotRepositoryConstructorBuilder.java | 71 +--------- .../RepositoryConstructorBuilder.java | 124 ++++++++++++++++++ .../aot/generate/RepositoryContributor.java | 9 +- .../AotRepositoryBuilderUnitTests.java | 9 +- .../MethodCapturingRepositoryContributor.java | 7 +- .../RepositoryContributorUnitTests.java | 6 +- 8 files changed, 216 insertions(+), 107 deletions(-) create mode 100644 src/main/java/org/springframework/data/repository/aot/generate/AotRepositoryClassBuilder.java create mode 100644 src/main/java/org/springframework/data/repository/aot/generate/RepositoryConstructorBuilder.java diff --git a/src/main/java/org/springframework/data/repository/aot/generate/AotRepositoryBuilder.java b/src/main/java/org/springframework/data/repository/aot/generate/AotRepositoryBuilder.java index 84f344331b..d7c0c9dd96 100644 --- a/src/main/java/org/springframework/data/repository/aot/generate/AotRepositoryBuilder.java +++ b/src/main/java/org/springframework/data/repository/aot/generate/AotRepositoryBuilder.java @@ -22,6 +22,7 @@ import java.util.LinkedHashMap; import java.util.List; import java.util.Map; +import java.util.function.Consumer; import javax.lang.model.element.Modifier; @@ -43,6 +44,7 @@ import org.springframework.javapoet.MethodSpec; import org.springframework.javapoet.TypeName; import org.springframework.javapoet.TypeSpec; +import org.springframework.util.Assert; /** * Builder for AOT repository fragments. @@ -59,9 +61,9 @@ class AotRepositoryBuilder { private final ProjectionFactory projectionFactory; private final AotRepositoryFragmentMetadata generationMetadata; - private @Nullable ConstructorCustomizer constructorCustomizer; + private @Nullable Consumer constructorCustomizer; private @Nullable MethodContributorFactory methodContributorFactory; - private ClassCustomizer customizer; + private Consumer classCustomizer; private AotRepositoryBuilder(RepositoryInformation repositoryInformation, String moduleName, ProjectionFactory projectionFactory) { @@ -76,7 +78,7 @@ private AotRepositoryBuilder(RepositoryInformation repositoryInformation, String .initializer("$T.getLog($T.class)", TypeName.get(LogFactory.class), this.generationMetadata.getTargetTypeName()) .build()); - this.customizer = (info, builder) -> {}; + this.classCustomizer = (builder) -> {}; } /** @@ -93,14 +95,14 @@ public static AotRepositoryBuilder forRepository(RepositoryInformation informati } /** - * Configure a {@link ClassCustomizer} customizer. + * Configure a {@link AotRepositoryConstructorBuilder} customizer. * * @param classCustomizer must not be {@literal null}. * @return {@code this}. */ - public AotRepositoryBuilder withClassCustomizer(ClassCustomizer classCustomizer) { + public AotRepositoryBuilder withClassCustomizer(Consumer classCustomizer) { - this.customizer = classCustomizer; + this.classCustomizer = classCustomizer; return this; } @@ -110,7 +112,8 @@ public AotRepositoryBuilder withClassCustomizer(ClassCustomizer classCustomizer) * @param constructorCustomizer must not be {@literal null}. * @return {@code this}. */ - public AotRepositoryBuilder withConstructorCustomizer(ConstructorCustomizer constructorCustomizer) { + public AotRepositoryBuilder withConstructorCustomizer( + Consumer constructorCustomizer) { this.constructorCustomizer = constructorCustomizer; return this; @@ -162,7 +165,12 @@ public AotBundle build() { generationMetadata.getFields().values().forEach(builder::addField); // finally customize the file itself - this.customizer.customize(repositoryInformation, builder); + this.classCustomizer.accept(customizer -> { + + Assert.notNull(customizer, "ClassCustomizer must not be null"); + customizer.customize(builder); + }); + JavaFile javaFile = JavaFile.builder(packageName(), builder.build()).build(); AotRepositoryMetadata metadata = getAotRepositoryMetadata(methodMetadata); @@ -171,11 +179,11 @@ public AotBundle build() { private MethodSpec buildConstructor() { - AotRepositoryConstructorBuilder constructorBuilder = new AotRepositoryConstructorBuilder(repositoryInformation, + RepositoryConstructorBuilder constructorBuilder = new RepositoryConstructorBuilder( generationMetadata); if (constructorCustomizer != null) { - constructorCustomizer.customize(constructorBuilder); + constructorCustomizer.accept(constructorBuilder); } return constructorBuilder.buildConstructor(); @@ -213,8 +221,7 @@ private void contributeMethod(Method method, RepositoryComposition repositoryCom if (repositoryInformation.isQueryMethod(method) && methodContributorFactory != null) { - MethodContributor contributor = methodContributorFactory.create(method, - repositoryInformation); + MethodContributor contributor = methodContributorFactory.create(method); if (contributor != null) { @@ -273,20 +280,6 @@ public ProjectionFactory getProjectionFactory() { return projectionFactory; } - /** - * Customizer interface to customize the AOT repository fragment class after it has been defined. - */ - public interface ClassCustomizer { - - /** - * Apply customization ot the AOT repository fragment class after it has been defined. - * - * @param information the repository information that is used for the AOT fragment. - * @param builder the class builder to be customized. - */ - void customize(RepositoryInformation information, TypeSpec.Builder builder); - - } /** * Customizer interface to customize the AOT repository fragment constructor through @@ -313,12 +306,11 @@ public interface MethodContributorFactory { * Apply customization ot the AOT repository fragment constructor. * * @param method the method to be contributed. - * @param information repository information. * @return the {@link MethodContributor} to be used. Can be {@literal null} if the method and method metadata should * not be contributed. */ @Nullable - MethodContributor create(Method method, RepositoryInformation information); + MethodContributor create(Method method); } diff --git a/src/main/java/org/springframework/data/repository/aot/generate/AotRepositoryClassBuilder.java b/src/main/java/org/springframework/data/repository/aot/generate/AotRepositoryClassBuilder.java new file mode 100644 index 0000000000..9c37487ec7 --- /dev/null +++ b/src/main/java/org/springframework/data/repository/aot/generate/AotRepositoryClassBuilder.java @@ -0,0 +1,49 @@ +/* + * Copyright 2025 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.data.repository.aot.generate; + +import org.springframework.javapoet.TypeSpec; + +/** + * Builder for AOT repository fragment classes. + * + * @author Mark Paluch + * @since 4.0 + */ +public interface AotRepositoryClassBuilder { + + /** + * Add a class customizer. Customizer is invoked after building the class. + * + * @param customizer the customizer with direct access to the {@link TypeSpec.Builder type builder}. + */ + void customize(ClassCustomizer customizer); + + /** + * Customizer interface to customize the AOT repository fragment class after it has been defined. + */ + interface ClassCustomizer { + + /** + * Apply customization ot the AOT repository fragment class after it has been defined. + * + * @param builder the class builder to be customized. + */ + void customize(TypeSpec.Builder builder); + + } + +} diff --git a/src/main/java/org/springframework/data/repository/aot/generate/AotRepositoryConstructorBuilder.java b/src/main/java/org/springframework/data/repository/aot/generate/AotRepositoryConstructorBuilder.java index 51b08d63c0..45d964d076 100644 --- a/src/main/java/org/springframework/data/repository/aot/generate/AotRepositoryConstructorBuilder.java +++ b/src/main/java/org/springframework/data/repository/aot/generate/AotRepositoryConstructorBuilder.java @@ -15,15 +15,7 @@ */ package org.springframework.data.repository.aot.generate; -import java.util.Map.Entry; - -import javax.lang.model.element.Modifier; - -import org.springframework.core.ResolvableType; -import org.springframework.data.repository.aot.generate.AotRepositoryFragmentMetadata.ConstructorArgument; -import org.springframework.data.repository.core.RepositoryInformation; import org.springframework.javapoet.MethodSpec; -import org.springframework.javapoet.ParameterizedTypeName; import org.springframework.javapoet.TypeName; /** @@ -33,18 +25,7 @@ * @author Mark Paluch * @since 4.0 */ -public class AotRepositoryConstructorBuilder { - - private final RepositoryInformation repositoryInformation; - private final AotRepositoryFragmentMetadata metadata; - - private ConstructorCustomizer customizer = (info, builder) -> {}; - - AotRepositoryConstructorBuilder(RepositoryInformation repositoryInformation, AotRepositoryFragmentMetadata metadata) { - - this.repositoryInformation = repositoryInformation; - this.metadata = metadata; - } +public interface AotRepositoryConstructorBuilder { /** * Add constructor parameter and create a field storing its value. @@ -52,16 +33,7 @@ public class AotRepositoryConstructorBuilder { * @param parameterName name of the parameter. * @param type parameter type. */ - public void addParameter(String parameterName, Class type) { - - ResolvableType resolvableType = ResolvableType.forClass(type); - if (!resolvableType.hasGenerics() || !resolvableType.hasResolvableGenerics()) { - addParameter(parameterName, TypeName.get(type)); - return; - } - - addParameter(parameterName, ParameterizedTypeName.get(type, resolvableType.resolveGenerics())); - } + void addParameter(String parameterName, Class type); /** * Add constructor parameter and create a field storing its value. @@ -69,7 +41,7 @@ public void addParameter(String parameterName, Class type) { * @param parameterName name of the parameter. * @param type parameter type. */ - public void addParameter(String parameterName, TypeName type) { + default void addParameter(String parameterName, TypeName type) { addParameter(parameterName, type, true); } @@ -80,14 +52,7 @@ public void addParameter(String parameterName, TypeName type) { * @param type parameter type. * @param createField whether to create a field for the parameter and assign its value to the field. */ - public void addParameter(String parameterName, TypeName type, boolean createField) { - - this.metadata.addConstructorArgument(parameterName, type, createField ? parameterName : null); - - if (createField) { - this.metadata.addField(parameterName, type, Modifier.PRIVATE, Modifier.FINAL); - } - } + void addParameter(String parameterName, TypeName type, boolean createField); /** * Add constructor customizer. Customizer is invoked after adding constructor arguments and before assigning @@ -95,41 +60,19 @@ public void addParameter(String parameterName, TypeName type, boolean createFiel * * @param customizer the customizer with direct access to the {@link MethodSpec.Builder constructor builder}. */ - public void customize(ConstructorCustomizer customizer) { - this.customizer = customizer; - } - - MethodSpec buildConstructor() { - - MethodSpec.Builder builder = MethodSpec.constructorBuilder().addModifiers(Modifier.PUBLIC); - - for (Entry parameter : this.metadata.getConstructorArguments().entrySet()) { - builder.addParameter(parameter.getValue().typeName(), parameter.getKey()); - } - - customizer.customize(repositoryInformation, builder); - - for (Entry parameter : this.metadata.getConstructorArguments().entrySet()) { - if (parameter.getValue().isForLocalField()) { - builder.addStatement("this.$N = $N", parameter.getKey(), parameter.getKey()); - } - } - - return builder.build(); - } + void customize(ConstructorCustomizer customizer); /** * Customizer for the AOT repository constructor. */ - public interface ConstructorCustomizer { + interface ConstructorCustomizer { /** * Customize the constructor. * - * @param information the repository information that is used for the AOT fragment. * @param builder the constructor builder to be customized. */ - void customize(RepositoryInformation information, MethodSpec.Builder builder); + void customize(MethodSpec.Builder builder); } diff --git a/src/main/java/org/springframework/data/repository/aot/generate/RepositoryConstructorBuilder.java b/src/main/java/org/springframework/data/repository/aot/generate/RepositoryConstructorBuilder.java new file mode 100644 index 0000000000..fabb0dba56 --- /dev/null +++ b/src/main/java/org/springframework/data/repository/aot/generate/RepositoryConstructorBuilder.java @@ -0,0 +1,124 @@ +/* + * Copyright 2024 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.data.repository.aot.generate; + +import java.util.Map.Entry; + +import javax.lang.model.element.Modifier; + +import org.springframework.core.ResolvableType; +import org.springframework.data.repository.aot.generate.AotRepositoryFragmentMetadata.ConstructorArgument; +import org.springframework.javapoet.MethodSpec; +import org.springframework.javapoet.ParameterizedTypeName; +import org.springframework.javapoet.TypeName; +import org.springframework.util.Assert; + +/** + * Builder for AOT Repository Constructors. + * + * @author Christoph Strobl + * @author Mark Paluch + * @since 4.0 + */ +class RepositoryConstructorBuilder implements AotRepositoryConstructorBuilder { + + private final AotRepositoryFragmentMetadata metadata; + + private ConstructorCustomizer customizer = (builder) -> {}; + + RepositoryConstructorBuilder(AotRepositoryFragmentMetadata metadata) { + this.metadata = metadata; + } + + /** + * Add constructor parameter and create a field storing its value. + * + * @param parameterName name of the parameter. + * @param type parameter type. + */ + @Override + public void addParameter(String parameterName, Class type) { + + ResolvableType resolvableType = ResolvableType.forClass(type); + if (!resolvableType.hasGenerics() || !resolvableType.hasResolvableGenerics()) { + addParameter(parameterName, TypeName.get(type)); + return; + } + + addParameter(parameterName, ParameterizedTypeName.get(type, resolvableType.resolveGenerics())); + } + + /** + * Add constructor parameter and create a field storing its value. + * + * @param parameterName name of the parameter. + * @param type parameter type. + */ + @Override + public void addParameter(String parameterName, TypeName type) { + addParameter(parameterName, type, true); + } + + /** + * Add constructor parameter. + * + * @param parameterName name of the parameter. + * @param type parameter type. + * @param createField whether to create a field for the parameter and assign its value to the field. + */ + @Override + public void addParameter(String parameterName, TypeName type, boolean createField) { + + this.metadata.addConstructorArgument(parameterName, type, createField ? parameterName : null); + + if (createField) { + this.metadata.addField(parameterName, type, Modifier.PRIVATE, Modifier.FINAL); + } + } + + /** + * Add constructor customizer. Customizer is invoked after adding constructor arguments and before assigning + * constructor arguments to fields. + * + * @param customizer the customizer with direct access to the {@link MethodSpec.Builder constructor builder}. + */ + @Override + public void customize(ConstructorCustomizer customizer) { + + Assert.notNull(customizer, "ConstructorCustomizer must not be null"); + this.customizer = customizer; + } + + public MethodSpec buildConstructor() { + + MethodSpec.Builder builder = MethodSpec.constructorBuilder().addModifiers(Modifier.PUBLIC); + + for (Entry parameter : this.metadata.getConstructorArguments().entrySet()) { + builder.addParameter(parameter.getValue().typeName(), parameter.getKey()); + } + + customizer.customize(builder); + + for (Entry parameter : this.metadata.getConstructorArguments().entrySet()) { + if (parameter.getValue().isForLocalField()) { + builder.addStatement("this.$N = $N", parameter.getKey(), parameter.getKey()); + } + } + + return builder.build(); + } + +} diff --git a/src/main/java/org/springframework/data/repository/aot/generate/RepositoryContributor.java b/src/main/java/org/springframework/data/repository/aot/generate/RepositoryContributor.java index ef79269325..b02572379b 100644 --- a/src/main/java/org/springframework/data/repository/aot/generate/RepositoryContributor.java +++ b/src/main/java/org/springframework/data/repository/aot/generate/RepositoryContributor.java @@ -31,6 +31,7 @@ import org.springframework.data.repository.query.QueryMethod; import org.springframework.javapoet.JavaFile; import org.springframework.javapoet.TypeName; +import org.springframework.util.StringUtils; import org.springframework.javapoet.TypeSpec; /** @@ -136,23 +137,21 @@ private static String getRepositoryJsonFileName(Class repositoryInterface) { /** * Customization hook for store implementations to customize class after building the entire class. */ - protected void customizeClass(RepositoryInformation information, - TypeSpec.Builder builder) { + protected void customizeClass(AotRepositoryClassBuilder builder) { } /** * Customization hook for store implementations to customize the fragment constructor after building the constructor. */ - protected void customizeConstructor(AotRepositoryConstructorBuilder constructorBuilder) { + protected void customizeConstructor(AotRepositoryConstructorBuilder builder) { } /** * Customization hook for store implementations to contribute a query method. */ - protected @Nullable MethodContributor contributeQueryMethod(Method method, - RepositoryInformation repositoryInformation) { + protected @Nullable MethodContributor contributeQueryMethod(Method method) { return null; } diff --git a/src/test/java/org/springframework/data/repository/aot/generate/AotRepositoryBuilderUnitTests.java b/src/test/java/org/springframework/data/repository/aot/generate/AotRepositoryBuilderUnitTests.java index 1ac8d043b3..51dee1e568 100644 --- a/src/test/java/org/springframework/data/repository/aot/generate/AotRepositoryBuilderUnitTests.java +++ b/src/test/java/org/springframework/data/repository/aot/generate/AotRepositoryBuilderUnitTests.java @@ -96,7 +96,7 @@ void appliesCtorCodeBlock() { AotRepositoryBuilder repoBuilder = AotRepositoryBuilder.forRepository(repositoryInformation, "Commons", new SpelAwareProxyProjectionFactory()); repoBuilder.withConstructorCustomizer(ctor -> { - ctor.customize((info, code) -> { + ctor.customize((code) -> { code.addStatement("throw new $T($S)", IllegalStateException.class, "initialization error"); }); }); @@ -110,7 +110,9 @@ void appliesClassCustomizations() { AotRepositoryBuilder repoBuilder = AotRepositoryBuilder.forRepository(repositoryInformation, "Commons", new SpelAwareProxyProjectionFactory()); - repoBuilder.withClassCustomizer((info, metadata, clazz) -> { + repoBuilder.withClassCustomizer((builder) -> { + + builder.customize(clazz -> { clazz.addField(Float.class, "f", Modifier.PRIVATE, Modifier.STATIC); clazz.addField(Double.class, "d", Modifier.PUBLIC); @@ -119,6 +121,7 @@ void appliesClassCustomizations() { clazz.addAnnotation(Repository.class); clazz.addMethod(MethodSpec.methodBuilder("oops").build()); + }); }); assertThat(repoBuilder.build().javaFile().toString()) // @@ -138,7 +141,7 @@ void appliesQueryMethodContributor() { AotRepositoryBuilder repoBuilder = AotRepositoryBuilder.forRepository(repositoryInformation, "Commons", new SpelAwareProxyProjectionFactory()); - repoBuilder.withQueryMethodContributor((method, info) -> { + repoBuilder.withQueryMethodContributor((method) -> { return new MethodContributor<>(mock(QueryMethod.class), null) { diff --git a/src/test/java/org/springframework/data/repository/aot/generate/MethodCapturingRepositoryContributor.java b/src/test/java/org/springframework/data/repository/aot/generate/MethodCapturingRepositoryContributor.java index 033c7fbe18..109e2e37c2 100644 --- a/src/test/java/org/springframework/data/repository/aot/generate/MethodCapturingRepositoryContributor.java +++ b/src/test/java/org/springframework/data/repository/aot/generate/MethodCapturingRepositoryContributor.java @@ -15,15 +15,15 @@ */ package org.springframework.data.repository.aot.generate; -import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.*; import java.lang.reflect.Method; import java.util.List; import org.assertj.core.api.MapAssert; import org.jspecify.annotations.Nullable; + import org.springframework.data.repository.config.AotRepositoryContext; -import org.springframework.data.repository.core.RepositoryInformation; import org.springframework.data.repository.query.QueryMethod; import org.springframework.util.LinkedMultiValueMap; import org.springframework.util.MultiValueMap; @@ -41,8 +41,7 @@ public MethodCapturingRepositoryContributor(AotRepositoryContext repositoryConte } @Override - protected @Nullable MethodContributor contributeQueryMethod(Method method, - RepositoryInformation repositoryInformation) { + protected @Nullable MethodContributor contributeQueryMethod(Method method) { capturedInvocations.add(method.getName(), method); return null; } diff --git a/src/test/java/org/springframework/data/repository/aot/generate/RepositoryContributorUnitTests.java b/src/test/java/org/springframework/data/repository/aot/generate/RepositoryContributorUnitTests.java index 9156704008..1c0d254bd3 100644 --- a/src/test/java/org/springframework/data/repository/aot/generate/RepositoryContributorUnitTests.java +++ b/src/test/java/org/springframework/data/repository/aot/generate/RepositoryContributorUnitTests.java @@ -58,10 +58,10 @@ void createsCompilableClassStub() { RepositoryContributor repositoryContributor = new RepositoryContributor(aotContext) { @Override - protected @Nullable MethodContributor contributeQueryMethod(Method method, - RepositoryInformation repositoryInformation) { + protected @Nullable MethodContributor contributeQueryMethod(Method method) { - return MethodContributor.forQueryMethod(new QueryMethod(method, repositoryInformation, getProjectionFactory())) + return MethodContributor + .forQueryMethod(new QueryMethod(method, getRepositoryInformation(), getProjectionFactory())) .withMetadata(new QueryMetadata() { @Override