From 0612b2513f47f6d27ace8967aa33a584533422c5 Mon Sep 17 00:00:00 2001 From: Christoph Strobl Date: Wed, 16 Apr 2025 08:07:48 +0200 Subject: [PATCH 1/5] 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 dacd58a6b61f1cab7751b61d24f0f01e8336ea4d Mon Sep 17 00:00:00 2001 From: Christoph Strobl Date: Wed, 16 Apr 2025 10:01:27 +0200 Subject: [PATCH 2/5] 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 cb93ce28357674329fe055535b43a09ec8e22306 Mon Sep 17 00:00:00 2001 From: Christoph Strobl Date: Fri, 2 May 2025 08:14:15 +0200 Subject: [PATCH 3/5] 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 7b5f4763fe60799af962fb95c892caf58fed3298 Mon Sep 17 00:00:00 2001 From: Christoph Strobl Date: Fri, 2 May 2025 10:19:19 +0200 Subject: [PATCH 4/5] 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 4a48edc8b3a018e23db57ee1e844ef98d497b177 Mon Sep 17 00:00:00 2001 From: Christoph Strobl Date: Fri, 2 May 2025 10:50:10 +0200 Subject: [PATCH 5/5] 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); + } +}