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.

See #3270
Original pull request: #3271
This commit is contained in:
Mark Paluch
2025-05-08 10:51:55 +02:00
parent 6360ef83b4
commit 79c625d543
9 changed files with 123 additions and 180 deletions

View File

@@ -20,9 +20,8 @@ import java.lang.reflect.Method;
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.core.RepositoryInformation;
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 class AotQueryMethodGenerationContext {
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 class AotQueryMethodGenerationContext {
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,23 +132,13 @@ public class AotQueryMethodGenerationContext {
return TypeName.get(getReturnType().getType());
}
/**
* 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
*/
public String 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
* 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) {
@@ -161,9 +156,8 @@ public class AotQueryMethodGenerationContext {
* {@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;
@@ -185,12 +179,12 @@ public class AotQueryMethodGenerationContext {
}
/**
* 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) {
@@ -204,13 +198,13 @@ public class AotQueryMethodGenerationContext {
}
/**
* 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;
@@ -237,7 +231,7 @@ public class AotQueryMethodGenerationContext {
List<String> result = new ArrayList<>();
for (Parameter parameter : queryMethod.getParameters().getBindableParameters()) {
getParameterName(parameter.getIndex());
result.add(getParameterName(parameter.getIndex()));
}
return result;
@@ -250,45 +244,50 @@ public class AotQueryMethodGenerationContext {
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());

View File

@@ -52,6 +52,8 @@ import org.springframework.javapoet.TypeSpec;
*/
class AotRepositoryBuilder {
private static final Log logger = LogFactory.getLog(AotRepositoryBuilder.class);
private final RepositoryInformation repositoryInformation;
private final String moduleName;
private final ProjectionFactory projectionFactory;
@@ -74,7 +76,7 @@ class AotRepositoryBuilder {
.initializer("$T.getLog($T.class)", TypeName.get(LogFactory.class), this.generationMetadata.getTargetTypeName())
.build());
this.customizer = (info, metadata, builder) -> {};
this.customizer = (info, builder) -> {};
}
/**
@@ -146,15 +148,21 @@ class AotRepositoryBuilder {
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
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);
@@ -273,11 +281,10 @@ class AotRepositoryBuilder {
/**
* 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);
}

View File

@@ -33,7 +33,6 @@ import org.springframework.javapoet.TypeName;
* @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 class AotRepositoryConstructorBuilder {
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 class AotRepositoryConstructorBuilder {
/**
* 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 class AotRepositoryConstructorBuilder {
* 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 @@ public class AotRepositoryConstructorBuilder {
customizer.customize(repositoryInformation, builder);
for (Entry<String, ConstructorArgument> 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 @@ public class AotRepositoryConstructorBuilder {
*/
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);
}
}

View File

@@ -29,10 +29,13 @@ import org.springframework.javapoet.FieldSpec;
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<String, FieldSpec> fields = new HashMap<>(3);
@@ -59,18 +62,6 @@ public class AotRepositoryFragmentMetadata {
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 class AotRepositoryFragmentMetadata {
fields.put(fieldSpec.name, fieldSpec);
}
Map<String, FieldSpec> getFields() {
public Map<String, FieldSpec> getFields() {
return fields;
}
@@ -87,10 +78,6 @@ public class AotRepositoryFragmentMetadata {
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));
}

View File

@@ -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();
}
}

View File

@@ -31,10 +31,16 @@ class LocalVariableNameFactory implements VariableNameFactory {
private final MultiValueMap<String, String> variables;
LocalVariableNameFactory(Iterable<String> 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 @@ class LocalVariableNameFactory implements VariableNameFactory {
return new LocalVariableNameFactory(predefinedVariables);
}
LocalVariableNameFactory(Iterable<String> predefinedVariableNames) {
variables = new LinkedMultiValueMap<>();
predefinedVariableNames.forEach(varName -> variables.add(varName, varName));
}
@Override
public String generateName(String intendedVariableName) {
@@ -84,4 +84,5 @@ class LocalVariableNameFactory implements VariableNameFactory {
}
return suggestTargetName(suggested, counter + 1);
}
}

View File

@@ -35,11 +35,13 @@ import org.springframework.javapoet.TypeName;
* Metadata about an AOT Repository method.
*
* @author Christoph Strobl
* @author Mark Paluch
* @since 4.0
*/
class MethodMetadata {
private final Map<String, ParameterSpec> methodArguments = new LinkedHashMap<>();
private final Map<String, String> localVariables = new LinkedHashMap<>();
private final ResolvableType actualReturnType;
private final ResolvableType returnType;
@@ -90,6 +92,10 @@ class MethodMetadata {
return null;
}
Map<String, String> getLocalVariables() {
return localVariables;
}
private void initParameters(RepositoryInformation repositoryInformation, Method method,
ParameterNameDiscoverer nameDiscoverer) {

View File

@@ -136,7 +136,7 @@ public class RepositoryContributor {
/**
* 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) {
}

View File

@@ -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 @@ import org.springframework.data.util.TypeInformation;
* Tests targeting {@link AotQueryMethodGenerationContext}.
*
* @author Christoph Strobl
* @author Mark Paluch
*/
class AotQueryMethodGenerationContextUnitTests {
@@ -39,8 +41,8 @@ class AotQueryMethodGenerationContextUnitTests {
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 {