From 72774135a7fda447efdd5a83850e2e21c273bd91 Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Wed, 4 Sep 2024 10:02:05 +0200 Subject: [PATCH] Polishing. Remove Preprocessor interface. Add property accessors to RelationalQueryLookupStrategy. Reuse property accessors instead of loosely coupled object access. See #1856 Original pull request #1863 --- .../query/StringBasedJdbcQuery.java | 14 ++++---- .../support/JdbcQueryLookupStrategy.java | 32 +++++++------------ .../support/R2dbcRepositoryFactory.java | 20 +++++------- .../repository/query/QueryPreprocessor.java | 29 ----------------- .../RelationalQueryLookupStrategy.java | 19 +++++------ .../support/TableNameQueryPreprocessor.java | 15 +++++---- 6 files changed, 46 insertions(+), 83 deletions(-) delete mode 100644 spring-data-relational/src/main/java/org/springframework/data/relational/repository/query/QueryPreprocessor.java diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/repository/query/StringBasedJdbcQuery.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/repository/query/StringBasedJdbcQuery.java index 6949ea3681c..7e8da647eea 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/repository/query/StringBasedJdbcQuery.java +++ b/spring-data-jdbc/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/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/repository/support/JdbcQueryLookupStrategy.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/repository/support/JdbcQueryLookupStrategy.java index 999481d57bb..b1f3f5bd9f5 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/repository/support/JdbcQueryLookupStrategy.java +++ b/spring-data-jdbc/src/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 createMapper(Class returnedObjectType) { - RelationalPersistentEntity persistentEntity = context.getPersistentEntity(returnedObjectType); + RelationalPersistentEntity persistentEntity = getMappingContext().getPersistentEntity(returnedObjectType); if (persistentEntity == null) { return (RowMapper) 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/spring-data-r2dbc/src/main/java/org/springframework/data/r2dbc/repository/support/R2dbcRepositoryFactory.java b/spring-data-r2dbc/src/main/java/org/springframework/data/r2dbc/repository/support/R2dbcRepositoryFactory.java index d3299a29a7d..3d49bf38044 100644 --- a/spring-data-r2dbc/src/main/java/org/springframework/data/r2dbc/repository/support/R2dbcRepositoryFactory.java +++ b/spring-data-r2dbc/src/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 RelationalEntityInformation getEntityInformation(Class } /** - * {@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 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/spring-data-relational/src/main/java/org/springframework/data/relational/repository/query/QueryPreprocessor.java b/spring-data-relational/src/main/java/org/springframework/data/relational/repository/query/QueryPreprocessor.java deleted file mode 100644 index 4508e21d7de..00000000000 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/repository/query/QueryPreprocessor.java +++ /dev/null @@ -1,29 +0,0 @@ -/* - * 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.relational.repository.query; - -public interface QueryPreprocessor { - - QueryPreprocessor NOOP = new QueryPreprocessor() { - - @Override - public String transform(String query) { - return query; - } - }; - String transform(String query); -} diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/repository/support/RelationalQueryLookupStrategy.java b/spring-data-relational/src/main/java/org/springframework/data/relational/repository/support/RelationalQueryLookupStrategy.java index de78eeea58a..5c1f0aaea39 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/repository/support/RelationalQueryLookupStrategy.java +++ b/spring-data-relational/src/main/java/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 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/spring-data-relational/src/main/java/org/springframework/data/relational/repository/support/TableNameQueryPreprocessor.java b/spring-data-relational/src/main/java/org/springframework/data/relational/repository/support/TableNameQueryPreprocessor.java index d54999f535c..3cda34475c5 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/repository/support/TableNameQueryPreprocessor.java +++ b/spring-data-relational/src/main/java/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();