Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add config option to disable query validation #3642

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

<groupId>org.springframework.data</groupId>
<artifactId>spring-data-jpa-parent</artifactId>
<version>3.4.0-SNAPSHOT</version>
<version>3.4.x-1690-SNAPSHOT</version>
<packaging>pom</packaging>

<name>Spring Data JPA Parent</name>
Expand Down
4 changes: 2 additions & 2 deletions spring-data-envers/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,12 @@

<groupId>org.springframework.data</groupId>
<artifactId>spring-data-envers</artifactId>
<version>3.4.0-SNAPSHOT</version>
<version>3.4.x-1690-SNAPSHOT</version>

<parent>
<groupId>org.springframework.data</groupId>
<artifactId>spring-data-jpa-parent</artifactId>
<version>3.4.0-SNAPSHOT</version>
<version>3.4.x-1690-SNAPSHOT</version>
<relativePath>../pom.xml</relativePath>
</parent>

Expand Down
2 changes: 1 addition & 1 deletion spring-data-jpa-distribution/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
<parent>
<groupId>org.springframework.data</groupId>
<artifactId>spring-data-jpa-parent</artifactId>
<version>3.4.0-SNAPSHOT</version>
<version>3.4.x-1690-SNAPSHOT</version>
<relativePath>../pom.xml</relativePath>
</parent>

Expand Down
2 changes: 1 addition & 1 deletion spring-data-jpa-performance/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
<parent>
<groupId>org.springframework.data</groupId>
<artifactId>spring-data-jpa-parent</artifactId>
<version>3.4.0-SNAPSHOT</version>
<version>3.4.x-1690-SNAPSHOT</version>
<relativePath>../pom.xml</relativePath>
</parent>

Expand Down
4 changes: 2 additions & 2 deletions spring-data-jpa/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

<groupId>org.springframework.data</groupId>
<artifactId>spring-data-jpa</artifactId>
<version>3.4.0-SNAPSHOT</version>
<version>3.4.x-1690-SNAPSHOT</version>

<name>Spring Data JPA</name>
<description>Spring Data module for JPA repositories.</description>
Expand All @@ -15,7 +15,7 @@
<parent>
<groupId>org.springframework.data</groupId>
<artifactId>spring-data-jpa-parent</artifactId>
<version>3.4.0-SNAPSHOT</version>
<version>3.4.x-1690-SNAPSHOT</version>
<relativePath>../pom.xml</relativePath>
</parent>

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ public class JpaRepositoryConfigExtension extends RepositoryConfigurationExtensi
private static final String ENABLE_DEFAULT_TRANSACTIONS_ATTRIBUTE = "enableDefaultTransactions";
private static final String JPA_METAMODEL_CACHE_CLEANUP_CLASSNAME = "org.springframework.data.jpa.util.JpaMetamodelCacheCleanup";
private static final String ESCAPE_CHARACTER_PROPERTY = "escapeCharacter";
private static final String QUERY_VALIDATION = "queryValidation";

private final Map<Object, String> entityManagerRefs = new LinkedHashMap<>();

Expand Down Expand Up @@ -118,6 +119,7 @@ public void postProcess(BeanDefinitionBuilder builder, RepositoryConfigurationSo
builder.addPropertyValue("transactionManager", transactionManagerRef.orElse(DEFAULT_TRANSACTION_MANAGER_BEAN_NAME));
builder.addPropertyReference("entityManager", entityManagerRefs.get(source));
builder.addPropertyValue(ESCAPE_CHARACTER_PROPERTY, getEscapeCharacter(source).orElse('\\'));
getQueryValidation(source).ifPresent(it -> builder.addPropertyValue(QUERY_VALIDATION, it));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't need that one since we're not updating @EnableJpaRepositories

builder.addPropertyReference("mappingContext", JPA_MAPPING_CONTEXT_BEAN_NAME);
}

Expand All @@ -139,6 +141,15 @@ private static Optional<Character> getEscapeCharacter(RepositoryConfigurationSou
}
}

private static Optional<Boolean> getQueryValidation(RepositoryConfigurationSource source) {

try {
return source.getAttribute(QUERY_VALIDATION, Boolean.class);
} catch (IllegalArgumentException ___) {
return Optional.empty();
}
}

@Override
public void postProcess(BeanDefinitionBuilder builder, AnnotationRepositoryConfigurationSource config) {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,15 +38,15 @@ enum JpaQueryFactory {
*/
AbstractJpaQuery fromMethodWithQueryString(JpaQueryMethod method, EntityManager em, String queryString,
@Nullable String countQueryString, QueryRewriter queryRewriter,
ValueExpressionDelegate valueExpressionDelegate) {
ValueExpressionDelegate valueExpressionDelegate, boolean validation) {

if (method.isScrollQuery()) {
throw QueryCreationException.create(method, "Scroll queries are not supported using String-based queries");
}

return method.isNativeQuery()
? new NativeJpaQuery(method, em, queryString, countQueryString, queryRewriter, valueExpressionDelegate)
: new SimpleJpaQuery(method, em, queryString, countQueryString, queryRewriter, valueExpressionDelegate);
: new SimpleJpaQuery(method, em, queryString, countQueryString, queryRewriter, valueExpressionDelegate, validation);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -113,19 +113,21 @@ protected abstract RepositoryQuery resolveQuery(JpaQueryMethod method, QueryRewr
private static class CreateQueryLookupStrategy extends AbstractQueryLookupStrategy {

private final EscapeCharacter escape;
private final boolean validate;

public CreateQueryLookupStrategy(EntityManager em, JpaQueryMethodFactory queryMethodFactory,
QueryRewriterProvider queryRewriterProvider, EscapeCharacter escape) {
QueryRewriterProvider queryRewriterProvider, EscapeCharacter escape, boolean validate) {

super(em, queryMethodFactory, queryRewriterProvider);

this.escape = escape;
this.validate = validate;
}

@Override
protected RepositoryQuery resolveQuery(JpaQueryMethod method, QueryRewriter queryRewriter, EntityManager em,
NamedQueries namedQueries) {
return new PartTreeJpaQuery(method, em, escape);
return new PartTreeJpaQuery(method, em, escape, validate);
}
}

Expand All @@ -140,20 +142,24 @@ protected RepositoryQuery resolveQuery(JpaQueryMethod method, QueryRewriter quer
private static class DeclaredQueryLookupStrategy extends AbstractQueryLookupStrategy {

private final ValueExpressionDelegate valueExpressionDelegate;
private final boolean validation;

/**
* Creates a new {@link DeclaredQueryLookupStrategy}.
*
* @param em must not be {@literal null}.
* @param queryMethodFactory must not be {@literal null}.
* @param evaluationContextProvider must not be {@literal null}.
* @param delegate must not be {@literal null}.
* @param queryRewriterProvider must not be {@literal null}.
* @param validation indicate if queries are validated on startup.
*/
public DeclaredQueryLookupStrategy(EntityManager em, JpaQueryMethodFactory queryMethodFactory,
ValueExpressionDelegate delegate, QueryRewriterProvider queryRewriterProvider) {
ValueExpressionDelegate delegate, QueryRewriterProvider queryRewriterProvider, boolean validation) {

super(em, queryMethodFactory, queryRewriterProvider);

this.valueExpressionDelegate = delegate;
this.validation = validation;
}

@Override
Expand All @@ -172,13 +178,13 @@ protected RepositoryQuery resolveQuery(JpaQueryMethod method, QueryRewriter quer
}

return JpaQueryFactory.INSTANCE.fromMethodWithQueryString(method, em, method.getRequiredAnnotatedQuery(),
getCountQuery(method, namedQueries, em), queryRewriter, valueExpressionDelegate);
getCountQuery(method, namedQueries, em), queryRewriter, valueExpressionDelegate, validation);
}

String name = method.getNamedQueryName();
if (namedQueries.hasQuery(name)) {
return JpaQueryFactory.INSTANCE.fromMethodWithQueryString(method, em, namedQueries.getQuery(name),
getCountQuery(method, namedQueries, em), queryRewriter, valueExpressionDelegate);
getCountQuery(method, namedQueries, em), queryRewriter, valueExpressionDelegate, validation);
}

RepositoryQuery query = NamedQuery.lookupFrom(method, em);
Expand Down Expand Up @@ -285,6 +291,22 @@ public static QueryLookupStrategy create(EntityManager em, JpaQueryMethodFactory
queryRewriterProvider, escape);
}

/**
* Creates a {@link QueryLookupStrategy} for the given {@link EntityManager} and {@link Key}.
*
* @param em must not be {@literal null}.
* @param queryMethodFactory must not be {@literal null}.
* @param key may be {@literal null}.
* @param delegate must not be {@literal null}.
* @param queryRewriterProvider must not be {@literal null}.
* @param escape must not be {@literal null}.
*/
public static QueryLookupStrategy create(EntityManager em, JpaQueryMethodFactory queryMethodFactory,
@Nullable Key key, ValueExpressionDelegate delegate, QueryRewriterProvider queryRewriterProvider,
EscapeCharacter escape) {
return create(em, queryMethodFactory, key, delegate, queryRewriterProvider, escape, true);
}

/**
* Creates a {@link QueryLookupStrategy} for the given {@link EntityManager} and {@link Key}.
*
Expand All @@ -297,18 +319,18 @@ public static QueryLookupStrategy create(EntityManager em, JpaQueryMethodFactory
*/
public static QueryLookupStrategy create(EntityManager em, JpaQueryMethodFactory queryMethodFactory,
@Nullable Key key, ValueExpressionDelegate delegate, QueryRewriterProvider queryRewriterProvider,
EscapeCharacter escape) {
EscapeCharacter escape, boolean queryValidation) {

Assert.notNull(em, "EntityManager must not be null");
Assert.notNull(delegate, "ValueExpressionDelegate must not be null");

return switch (key != null ? key : Key.CREATE_IF_NOT_FOUND) {
case CREATE -> new CreateQueryLookupStrategy(em, queryMethodFactory, queryRewriterProvider, escape);
case CREATE -> new CreateQueryLookupStrategy(em, queryMethodFactory, queryRewriterProvider, escape, queryValidation);
case USE_DECLARED_QUERY ->
new DeclaredQueryLookupStrategy(em, queryMethodFactory, delegate, queryRewriterProvider);
new DeclaredQueryLookupStrategy(em, queryMethodFactory, delegate, queryRewriterProvider, queryValidation);
case CREATE_IF_NOT_FOUND -> new CreateIfNotFoundQueryLookupStrategy(em, queryMethodFactory,
new CreateQueryLookupStrategy(em, queryMethodFactory, queryRewriterProvider, escape),
new DeclaredQueryLookupStrategy(em, queryMethodFactory, delegate, queryRewriterProvider),
new CreateQueryLookupStrategy(em, queryMethodFactory, queryRewriterProvider, escape, queryValidation),
new DeclaredQueryLookupStrategy(em, queryMethodFactory, delegate, queryRewriterProvider, queryValidation),
queryRewriterProvider);
default -> throw new IllegalArgumentException(String.format("Unsupported query lookup strategy %s", key));
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ public class PartTreeJpaQuery extends AbstractJpaQuery {
* @param em must not be {@literal null}.
*/
PartTreeJpaQuery(JpaQueryMethod method, EntityManager em) {
this(method, em, EscapeCharacter.DEFAULT);
this(method, em, EscapeCharacter.DEFAULT, true);
}

/**
Expand All @@ -81,7 +81,7 @@ public class PartTreeJpaQuery extends AbstractJpaQuery {
* @param em must not be {@literal null}.
* @param escape character used for escaping characters used as patterns in LIKE-expressions.
*/
PartTreeJpaQuery(JpaQueryMethod method, EntityManager em, EscapeCharacter escape) {
PartTreeJpaQuery(JpaQueryMethod method, EntityManager em, EscapeCharacter escape, boolean validate) {

super(method, em);

Expand All @@ -99,7 +99,9 @@ public class PartTreeJpaQuery extends AbstractJpaQuery {
try {

this.tree = new PartTree(method.getName(), domainClass);
validate(tree, parameters, method.toString());
if(validate) {
validate(tree, parameters, method.toString());
}
this.countQuery = new CountQueryPreparer(recreationRequired);
this.query = tree.isCountProjection() ? countQuery : new QueryPreparer(recreationRequired);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import jakarta.persistence.EntityManager;
import jakarta.persistence.Query;

import org.springframework.core.SpringProperties;
import org.springframework.data.jpa.repository.QueryRewriter;
import org.springframework.data.repository.query.RepositoryQuery;
import org.springframework.data.repository.query.ValueExpressionDelegate;
Expand Down Expand Up @@ -45,8 +46,8 @@ final class SimpleJpaQuery extends AbstractStringBasedJpaQuery {
* @param valueExpressionDelegate must not be {@literal null}
*/
public SimpleJpaQuery(JpaQueryMethod method, EntityManager em, @Nullable String countQueryString,
QueryRewriter queryRewriter, ValueExpressionDelegate valueExpressionDelegate) {
this(method, em, method.getRequiredAnnotatedQuery(), countQueryString, queryRewriter, valueExpressionDelegate);
QueryRewriter queryRewriter, ValueExpressionDelegate valueExpressionDelegate, boolean validation) {
this(method, em, method.getRequiredAnnotatedQuery(), countQueryString, queryRewriter, valueExpressionDelegate, validation);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably a QueryValidator object would make sense instead of a magic boolean. I can handle that during the merge.

}

/**
Expand All @@ -60,15 +61,17 @@ public SimpleJpaQuery(JpaQueryMethod method, EntityManager em, @Nullable String
* @param valueExpressionDelegate must not be {@literal null}
*/
public SimpleJpaQuery(JpaQueryMethod method, EntityManager em, String queryString, @Nullable String countQueryString, QueryRewriter queryRewriter,
ValueExpressionDelegate valueExpressionDelegate) {
ValueExpressionDelegate valueExpressionDelegate, boolean validation) {

super(method, em, queryString, countQueryString, queryRewriter, valueExpressionDelegate);

validateQuery(getQuery().getQueryString(), "Validation failed for query for method %s", method);
if(validation) {
validateQuery(getQuery().getQueryString(), "Validation failed for query for method %s", method);

if (method.isPageQuery()) {
validateQuery(getCountQuery().getQueryString(),
if (method.isPageQuery()) {
validateQuery(getCountQuery().getQueryString(),
String.format("Count query validation failed for method %s", method));
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
public class DefaultJpaContext implements JpaContext {

private final MultiValueMap<Class<?>, EntityManager> entityManagers;
private boolean validateQueries;

/**
* Creates a new {@link DefaultJpaContext} for the given {@link Set} of {@link EntityManager}s.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import org.springframework.beans.BeanUtils;
import org.springframework.beans.BeansException;
import org.springframework.beans.factory.BeanFactory;
import org.springframework.core.SpringProperties;
import org.springframework.dao.InvalidDataAccessApiUsageException;
import org.springframework.data.jpa.projection.CollectionAwareProjectionFactory;
import org.springframework.data.jpa.provider.PersistenceProvider;
Expand Down Expand Up @@ -90,6 +91,7 @@ public class JpaRepositoryFactory extends RepositoryFactorySupport {
private EscapeCharacter escapeCharacter = EscapeCharacter.DEFAULT;
private JpaQueryMethodFactory queryMethodFactory;
private QueryRewriterProvider queryRewriterProvider;
private boolean queryValidation;

/**
* Creates a new {@link JpaRepositoryFactory}.
Expand Down Expand Up @@ -120,6 +122,8 @@ public JpaRepositoryFactory(EntityManager entityManager) {
}

this.crudMethodMetadata = crudMethodMetadataPostProcessor.getCrudMethodMetadata();
String p = SpringProperties.getProperty("spring.data.jpa.query.validate");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's not apply any defaulting from inside our library.

this.queryValidation = p == null || Boolean.parseBoolean(p);
}

@Override
Expand Down Expand Up @@ -167,6 +171,15 @@ public void setEscapeCharacter(EscapeCharacter escapeCharacter) {
this.escapeCharacter = escapeCharacter;
}

/**
* Configures if spring based queries should be validated on creation
*
* @param queryValidation a character used for escaping in certain like expressions.
* @since 3.4
*/
public void setQueryValidation(boolean queryValidation) {
this.queryValidation = queryValidation;
}
/**
* Configures the {@link JpaQueryMethodFactory} to be used. Defaults to {@link DefaultJpaQueryMethodFactory}.
*
Expand Down Expand Up @@ -240,7 +253,7 @@ protected Optional<QueryLookupStrategy> getQueryLookupStrategy(@Nullable Key key
ValueExpressionDelegate valueExpressionDelegate) {
return Optional.of(JpaQueryLookupStrategy.create(entityManager, queryMethodFactory, key,
new CachingValueExpressionDelegate(valueExpressionDelegate),
queryRewriterProvider, escapeCharacter));
queryRewriterProvider, escapeCharacter, queryValidation));
}


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ public class JpaRepositoryFactoryBean<T extends Repository<S, ID>, S, ID>
private @Nullable EntityManager entityManager;
private EntityPathResolver entityPathResolver;
private EscapeCharacter escapeCharacter = EscapeCharacter.DEFAULT;
private Boolean queryValidation;
private JpaQueryMethodFactory queryMethodFactory;

/**
Expand Down Expand Up @@ -116,6 +117,9 @@ protected RepositoryFactorySupport createRepositoryFactory(EntityManager entityM
JpaRepositoryFactory jpaRepositoryFactory = new JpaRepositoryFactory(entityManager);
jpaRepositoryFactory.setEntityPathResolver(entityPathResolver);
jpaRepositoryFactory.setEscapeCharacter(escapeCharacter);
if(queryValidation != null) {
jpaRepositoryFactory.setQueryValidation(queryValidation);
}

if (queryMethodFactory != null) {
jpaRepositoryFactory.setQueryMethodFactory(queryMethodFactory);
Expand All @@ -136,4 +140,12 @@ public void setEscapeCharacter(char escapeCharacter) {

this.escapeCharacter = EscapeCharacter.of(escapeCharacter);
}

/**
* @param queryValidation
* @since 3.4
*/
public void setQueryValidation(Boolean queryValidation) {
this.queryValidation = queryValidation;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ void createsNormalQueryForJpaManagedReturnTypes() throws Exception {

JpaQueryMethod method = getMethod("findRolesByEmailAddress", String.class);
AbstractStringBasedJpaQuery jpaQuery = new SimpleJpaQuery(method, mock, null, QueryRewriter.IdentityQueryRewriter.INSTANCE,
ValueExpressionDelegate.create());
ValueExpressionDelegate.create(), true);

jpaQuery.createJpaQuery(method.getAnnotatedQuery(), Sort.unsorted(), null,
method.getResultProcessor().getReturnedType());
Expand Down
Loading