Polishing.

Remove Preprocessor interface. Add property accessors to RelationalQueryLookupStrategy. Reuse property accessors instead of loosely coupled object access. See #1856 Original pull request #1863
spring-projects · Sep 4, 2024 · 7277413 · 7277413
1 parent f937738
commit 7277413
Show file tree

Hide file tree

Showing 6 changed files with 46 additions and 83 deletions.
diff --git a/...bc/src/main/java/org/springframework/data/jdbc/repository/query/StringBasedJdbcQuery.java b/...bc/src/main/java/org/springframework/data/jdbc/repository/query/StringBasedJdbcQuery.java
@@ -35,7 +35,6 @@
 import org.springframework.data.jdbc.core.mapping.JdbcValue;
 import org.springframework.data.jdbc.support.JdbcUtil;
 import org.springframework.data.relational.core.mapping.RelationalMappingContext;
-import org.springframework.data.relational.repository.query.QueryPreprocessor;
 import org.springframework.data.relational.repository.query.RelationalParameterAccessor;
 import org.springframework.data.relational.repository.query.RelationalParametersParameterAccessor;
 import org.springframework.data.repository.query.Parameter;
@@ -113,33 +112,34 @@ public StringBasedJdbcQuery(JdbcQueryMethod queryMethod, NamedParameterJdbcOpera
 	public StringBasedJdbcQuery(JdbcQueryMethod queryMethod, NamedParameterJdbcOperations operations,
 			RowMapperFactory rowMapperFactory, JdbcConverter converter,
 			QueryMethodEvaluationContextProvider evaluationContextProvider) {
-		this(queryMethod, operations, rowMapperFactory, converter, evaluationContextProvider, QueryPreprocessor.NOOP.transform(queryMethod.getRequiredQuery()));
+		this(queryMethod.getRequiredQuery(), queryMethod, operations, rowMapperFactory, converter,
+				evaluationContextProvider);
 	}
 
 	/**
 	 * Creates a new {@link StringBasedJdbcQuery} for the given {@link JdbcQueryMethod}, {@link RelationalMappingContext}
 	 * and {@link RowMapperFactory}.
 	 *
+	 * @param query must not be {@literal null} or empty.
 	 * @param queryMethod must not be {@literal null}.
 	 * @param operations must not be {@literal null}.
 	 * @param rowMapperFactory must not be {@literal null}.
 	 * @param converter must not be {@literal null}.
 	 * @param evaluationContextProvider must not be {@literal null}.
-	 * @param query
 	 * @since 3.4
 	 */
-	public StringBasedJdbcQuery(JdbcQueryMethod queryMethod, NamedParameterJdbcOperations operations,
-								RowMapperFactory rowMapperFactory, JdbcConverter converter,
-								QueryMethodEvaluationContextProvider evaluationContextProvider, String query) {
+	public StringBasedJdbcQuery(String query, JdbcQueryMethod queryMethod, NamedParameterJdbcOperations operations,
+			RowMapperFactory rowMapperFactory, JdbcConverter converter,
+			QueryMethodEvaluationContextProvider evaluationContextProvider) {
 
 		super(queryMethod, operations);
 
+		Assert.hasText(query, "Query must not be null or empty");
 		Assert.notNull(rowMapperFactory, "RowMapperFactory must not be null");
 
 		this.converter = converter;
 		this.rowMapperFactory = rowMapperFactory;
 
-
 		if (queryMethod.isSliceQuery()) {
 			throw new UnsupportedOperationException(
 					"Slice queries are not supported using string-based queries; Offending method: " + queryMethod);

diff --git a/...c/main/java/org/springframework/data/jdbc/repository/support/JdbcQueryLookupStrategy.java b/...c/main/java/org/springframework/data/jdbc/repository/support/JdbcQueryLookupStrategy.java
@@ -66,10 +66,9 @@ abstract class JdbcQueryLookupStrategy extends RelationalQueryLookupStrategy {
 	private static final Log LOG = LogFactory.getLog(JdbcQueryLookupStrategy.class);
 
 	private final ApplicationEventPublisher publisher;
-	private final @Nullable EntityCallbacks callbacks;
 	private final RelationalMappingContext context;
+	private final @Nullable EntityCallbacks callbacks;
 	private final JdbcConverter converter;
-	private final Dialect dialect;
 	private final QueryMappingConfiguration queryMappingConfiguration;
 	private final NamedParameterJdbcOperations operations;
 	@Nullable private final BeanFactory beanfactory;
@@ -83,24 +82,25 @@ abstract class JdbcQueryLookupStrategy extends RelationalQueryLookupStrategy {
 		super(context, dialect);
 
 		Assert.notNull(publisher, "ApplicationEventPublisher must not be null");
-		Assert.notNull(context, "RelationalMappingContext must not be null");
 		Assert.notNull(converter, "JdbcConverter must not be null");
-		Assert.notNull(dialect, "Dialect must not be null");
 		Assert.notNull(queryMappingConfiguration, "QueryMappingConfiguration must not be null");
 		Assert.notNull(operations, "NamedParameterJdbcOperations must not be null");
 		Assert.notNull(evaluationContextProvider, "QueryMethodEvaluationContextProvier must not be null");
 
+		this.context = context;
 		this.publisher = publisher;
 		this.callbacks = callbacks;
-		this.context = context;
 		this.converter = converter;
-		this.dialect = dialect;
 		this.queryMappingConfiguration = queryMappingConfiguration;
 		this.operations = operations;
 		this.beanfactory = beanfactory;
 		this.evaluationContextProvider = evaluationContextProvider;
 	}
 
+	public RelationalMappingContext getMappingContext() {
+		return context;
+	}
+
 	/**
 	 * {@link QueryLookupStrategy} to create a query from the method name.
 	 *
@@ -124,7 +124,7 @@ public RepositoryQuery resolveQuery(Method method, RepositoryMetadata repository
 
 			JdbcQueryMethod queryMethod = getJdbcQueryMethod(method, repositoryMetadata, projectionFactory, namedQueries);
 
-			return new PartTreeJdbcQuery(getContext(), queryMethod, getDialect(), getConverter(), getOperations(),
+			return new PartTreeJdbcQuery(getMappingContext(), queryMethod, getDialect(), getConverter(), getOperations(),
 					this::createMapper);
 		}
 	}
@@ -161,8 +161,8 @@ public RepositoryQuery resolveQuery(Method method, RepositoryMetadata repository
 
 				String queryString = evaluateTableExpressions(repositoryMetadata, queryMethod.getRequiredQuery());
 
-				StringBasedJdbcQuery query = new StringBasedJdbcQuery(queryMethod, getOperations(), this::createMapper,
-						getConverter(), evaluationContextProvider, queryString);
+				StringBasedJdbcQuery query = new StringBasedJdbcQuery(queryString, queryMethod, getOperations(),
+						this::createMapper, getConverter(), evaluationContextProvider);
 				query.setBeanFactory(getBeanFactory());
 				return query;
 			}
@@ -224,7 +224,7 @@ public RepositoryQuery resolveQuery(Method method, RepositoryMetadata repository
 	 */
 	JdbcQueryMethod getJdbcQueryMethod(Method method, RepositoryMetadata repositoryMetadata,
 			ProjectionFactory projectionFactory, NamedQueries namedQueries) {
-		return new JdbcQueryMethod(method, repositoryMetadata, projectionFactory, namedQueries, context);
+		return new JdbcQueryMethod(method, repositoryMetadata, projectionFactory, namedQueries, getMappingContext());
 	}
 
 	/**
@@ -277,18 +277,10 @@ public static QueryLookupStrategy create(@Nullable Key key, ApplicationEventPubl
 		}
 	}
 
-	RelationalMappingContext getContext() {
-		return context;
-	}
-
 	JdbcConverter getConverter() {
 		return converter;
 	}
 
-	Dialect getDialect() {
-		return dialect;
-	}
-
 	NamedParameterJdbcOperations getOperations() {
 		return operations;
 	}
@@ -301,7 +293,7 @@ BeanFactory getBeanFactory() {
 	@SuppressWarnings("unchecked")
 	RowMapper<Object> createMapper(Class<?> returnedObjectType) {
 
-		RelationalPersistentEntity<?> persistentEntity = context.getPersistentEntity(returnedObjectType);
+		RelationalPersistentEntity<?> persistentEntity = getMappingContext().getPersistentEntity(returnedObjectType);
 
 		if (persistentEntity == null) {
 			return (RowMapper<Object>) SingleColumnRowMapper.newInstance(returnedObjectType,
@@ -319,7 +311,7 @@ private RowMapper<?> determineDefaultMapper(Class<?> returnedObjectType) {
 			return configuredQueryMapper;
 
 		EntityRowMapper<?> defaultEntityRowMapper = new EntityRowMapper<>( //
-				context.getRequiredPersistentEntity(returnedObjectType), //
+				getMappingContext().getRequiredPersistentEntity(returnedObjectType), //
 				converter //
 		);
 

diff --git a/...c/main/java/org/springframework/data/r2dbc/repository/support/R2dbcRepositoryFactory.java b/...c/main/java/org/springframework/data/r2dbc/repository/support/R2dbcRepositoryFactory.java
@@ -112,8 +112,7 @@ protected Object getTargetRepository(RepositoryInformation information) {
 		RelationalEntityInformation<?, ?> entityInformation = getEntityInformation(information.getDomainType(),
 				information);
 
-		return getTargetRepositoryViaReflection(information, entityInformation,
-				operations, this.converter);
+		return getTargetRepositoryViaReflection(information, entityInformation, operations, this.converter);
 	}
 
 	@Override
@@ -138,7 +137,7 @@ private <T, ID> RelationalEntityInformation<T, ID> getEntityInformation(Class<T>
 	}
 
 	/**
-	 * {@link QueryLookupStrategy} to create R2DBC queries..
+	 * {@link QueryLookupStrategy} to create R2DBC queries.
 	 *
 	 * @author Mark Paluch
 	 * @author Jens Schauder
@@ -167,21 +166,18 @@ private static class R2dbcQueryLookupStrategy extends RelationalQueryLookupStrat
 		public RepositoryQuery resolveQuery(Method method, RepositoryMetadata metadata, ProjectionFactory factory,
 				NamedQueries namedQueries) {
 
-			MappingContext<? extends RelationalPersistentEntity<?>, ? extends RelationalPersistentProperty> mappingContext = this.converter.getMappingContext();
-
-			R2dbcQueryMethod queryMethod = new R2dbcQueryMethod(method, metadata, factory,
-					mappingContext);
+			R2dbcQueryMethod queryMethod = new R2dbcQueryMethod(method, metadata, factory, getMappingContext());
 			String namedQueryName = queryMethod.getNamedQueryName();
 
 			if (namedQueries.hasQuery(namedQueryName) || queryMethod.hasAnnotatedQuery()) {
 
-				String query = namedQueries.hasQuery(namedQueryName) ? namedQueries.getQuery(namedQueryName) : queryMethod.getRequiredAnnotatedQuery();
-				query  = evaluateTableExpressions(metadata, query);
+				String query = namedQueries.hasQuery(namedQueryName) ? namedQueries.getQuery(namedQueryName)
+						: queryMethod.getRequiredAnnotatedQuery();
+				query = evaluateTableExpressions(metadata, query);
 
 				return new StringBasedR2dbcQuery(query, queryMethod, this.entityOperations, this.converter,
-						this.dataAccessStrategy,
-						parser, this.evaluationContextProvider);
-
+						this.dataAccessStrategy, parser, this.evaluationContextProvider);
+
 			} else {
 				return new PartTreeR2dbcQuery(queryMethod, this.entityOperations, this.converter, this.dataAccessStrategy);
 			}

diff --git a/...src/main/java/org/springframework/data/relational/repository/query/QueryPreprocessor.java b/...src/main/java/org/springframework/data/relational/repository/query/QueryPreprocessor.java
diff --git a/...org/springframework/data/relational/repository/support/RelationalQueryLookupStrategy.java b/...org/springframework/data/relational/repository/support/RelationalQueryLookupStrategy.java
@@ -20,8 +20,6 @@
 import org.springframework.data.relational.core.dialect.Dialect;
 import org.springframework.data.relational.core.mapping.RelationalPersistentEntity;
 import org.springframework.data.relational.core.mapping.RelationalPersistentProperty;
-import org.springframework.data.relational.core.sql.SqlIdentifier;
-import org.springframework.data.relational.repository.query.QueryPreprocessor;
 import org.springframework.data.repository.core.RepositoryMetadata;
 import org.springframework.data.repository.query.QueryLookupStrategy;
 import org.springframework.util.Assert;
@@ -48,17 +46,20 @@ protected RelationalQueryLookupStrategy(
 		this.dialect = dialect;
 	}
 
-	protected String evaluateTableExpressions(RepositoryMetadata repositoryMetadata, String queryString) {
+	public MappingContext<? extends RelationalPersistentEntity<?>, ? extends RelationalPersistentProperty> getMappingContext() {
+		return context;
+	}
 
-		return prepareQueryPreprocessor(repositoryMetadata).transform(queryString);
+	public Dialect getDialect() {
+		return dialect;
 	}
 
-	private QueryPreprocessor prepareQueryPreprocessor(RepositoryMetadata repositoryMetadata) {
+	protected String evaluateTableExpressions(RepositoryMetadata repositoryMetadata, String queryString) {
+
+		TableNameQueryPreprocessor preprocessor = new TableNameQueryPreprocessor(
+				context.getRequiredPersistentEntity(repositoryMetadata.getDomainType()), dialect);
 
-		SqlIdentifier tableName = context.getPersistentEntity(repositoryMetadata.getDomainType()).getTableName();
-		SqlIdentifier qualifiedTableName = context.getPersistentEntity(repositoryMetadata.getDomainType())
-				.getQualifiedTableName();
-		return new TableNameQueryPreprocessor(tableName, qualifiedTableName, dialect);
+		return preprocessor.transform(queryString);
 	}
 
 }
diff --git a/...va/org/springframework/data/relational/repository/support/TableNameQueryPreprocessor.java b/...va/org/springframework/data/relational/repository/support/TableNameQueryPreprocessor.java
@@ -16,23 +16,23 @@
 
 package org.springframework.data.relational.repository.support;
 
+import java.util.regex.Pattern;
+
 import org.springframework.data.relational.core.dialect.Dialect;
+import org.springframework.data.relational.core.mapping.RelationalPersistentEntity;
 import org.springframework.data.relational.core.sql.SqlIdentifier;
-import org.springframework.data.relational.repository.query.QueryPreprocessor;
 import org.springframework.expression.Expression;
 import org.springframework.expression.ParserContext;
 import org.springframework.expression.spel.standard.SpelExpressionParser;
 import org.springframework.expression.spel.support.StandardEvaluationContext;
 import org.springframework.util.Assert;
 
-import java.util.regex.Pattern;
-
 /**
  * Replaces SpEL expressions based on table names in query strings.
  *
  * @author Jens Schauder
  */
-class TableNameQueryPreprocessor implements QueryPreprocessor {
+class TableNameQueryPreprocessor {
 
 	private static final String EXPRESSION_PARAMETER = "$1#{";
 	private static final String QUOTED_EXPRESSION_PARAMETER = "$1__HASH__{";
@@ -44,7 +44,11 @@ class TableNameQueryPreprocessor implements QueryPreprocessor {
 	private final SqlIdentifier qualifiedTableName;
 	private final Dialect dialect;
 
-	public TableNameQueryPreprocessor(SqlIdentifier tableName, SqlIdentifier qualifiedTableName, Dialect dialect) {
+	public TableNameQueryPreprocessor(RelationalPersistentEntity<?> entity, Dialect dialect) {
+		this(entity.getTableName(), entity.getQualifiedTableName(), dialect);
+	}
+
+	TableNameQueryPreprocessor(SqlIdentifier tableName, SqlIdentifier qualifiedTableName, Dialect dialect) {
 
 		Assert.notNull(tableName, "TableName must not be null");
 		Assert.notNull(qualifiedTableName, "QualifiedTableName must not be null");
@@ -55,7 +59,6 @@ public TableNameQueryPreprocessor(SqlIdentifier tableName, SqlIdentifier qualifi
 		this.dialect = dialect;
 	}
 
-	@Override
 	public String transform(String query) {
 
 		StandardEvaluationContext evaluationContext = new StandardEvaluationContext();