From 2ef1b156a7ae0aea0e78b48fa97ead78989c63d7 Mon Sep 17 00:00:00 2001 From: Jens Schauder Date: Mon, 29 Apr 2019 15:13:22 +0200 Subject: [PATCH] DATAJPA-1534 - Improved backward compatibility. Incorporates review feedback by @mp911de. --- .../QueryByExamplePredicateBuilder.java | 16 ++++++++++++-- .../jpa/repository/query/EscapeCharacter.java | 4 ++++ .../support/JpaRepositoryFactory.java | 2 +- .../support/JpaRepositoryFactoryBean.java | 2 +- .../support/SimpleJpaRepository.java | 2 +- ...eryByExamplePredicateBuilderUnitTests.java | 22 +++++++++---------- .../JpaCountQueryCreatorIntegrationTests.java | 2 +- .../JpaQueryLookupStrategyUnitTests.java | 6 ++--- .../ParameterExpressionProviderTests.java | 2 +- ...meterMetadataProviderIntegrationTests.java | 2 +- .../ParameterMetadataProviderUnitTests.java | 2 +- .../PartTreeJpaQueryIntegrationTests.java | 12 +++++----- 12 files changed, 45 insertions(+), 29 deletions(-) diff --git a/src/main/java/org/springframework/data/jpa/convert/QueryByExamplePredicateBuilder.java b/src/main/java/org/springframework/data/jpa/convert/QueryByExamplePredicateBuilder.java index 58ff56d73..1a2b71692 100644 --- a/src/main/java/org/springframework/data/jpa/convert/QueryByExamplePredicateBuilder.java +++ b/src/main/java/org/springframework/data/jpa/convert/QueryByExamplePredicateBuilder.java @@ -72,11 +72,23 @@ public class QueryByExamplePredicateBuilder { * @param root must not be {@literal null}. * @param cb must not be {@literal null}. * @param example must not be {@literal null}. - * @param escapeCharacter + * @return never {@literal null}. + */ + public static Predicate getPredicate(Root root, CriteriaBuilder cb, Example example) { + return getPredicate(root, cb, example, EscapeCharacter.DEFAULT); + } + + /** + * Extract the {@link Predicate} representing the {@link Example}. + * + * @param root must not be {@literal null}. + * @param cb must not be {@literal null}. + * @param example must not be {@literal null}. + * @param escapeCharacter Must not be {@literal null}. * @return never {@literal null}. */ public static Predicate getPredicate(Root root, CriteriaBuilder cb, Example example, - EscapeCharacter escapeCharacter) { + EscapeCharacter escapeCharacter) { Assert.notNull(root, "Root must not be null!"); Assert.notNull(cb, "CriteriaBuilder must not be null!"); diff --git a/src/main/java/org/springframework/data/jpa/repository/query/EscapeCharacter.java b/src/main/java/org/springframework/data/jpa/repository/query/EscapeCharacter.java index e382276b0..01ee4f3b3 100644 --- a/src/main/java/org/springframework/data/jpa/repository/query/EscapeCharacter.java +++ b/src/main/java/org/springframework/data/jpa/repository/query/EscapeCharacter.java @@ -18,6 +18,7 @@ package org.springframework.data.jpa.repository.query; import lombok.Value; import java.util.Arrays; +import java.util.List; /** * A value type encapsulating an escape character for LIKE queries and the actually usage of it in escaping @@ -29,6 +30,9 @@ import java.util.Arrays; @Value(staticConstructor = "of") public class EscapeCharacter { + public static final EscapeCharacter DEFAULT = EscapeCharacter.of('\\'); + private static final List TO_REPLACE = Arrays.asList("_", "%"); + char escapeCharacter; /** diff --git a/src/main/java/org/springframework/data/jpa/repository/support/JpaRepositoryFactory.java b/src/main/java/org/springframework/data/jpa/repository/support/JpaRepositoryFactory.java index 708463079..30b1b4b19 100644 --- a/src/main/java/org/springframework/data/jpa/repository/support/JpaRepositoryFactory.java +++ b/src/main/java/org/springframework/data/jpa/repository/support/JpaRepositoryFactory.java @@ -59,7 +59,7 @@ public class JpaRepositoryFactory extends RepositoryFactorySupport { private final QueryExtractor extractor; private final CrudMethodMetadataPostProcessor crudMethodMetadataPostProcessor; - private EscapeCharacter escapeCharacter = EscapeCharacter.of('\\'); + private EscapeCharacter escapeCharacter = EscapeCharacter.DEFAULT; /** * Creates a new {@link JpaRepositoryFactory}. diff --git a/src/main/java/org/springframework/data/jpa/repository/support/JpaRepositoryFactoryBean.java b/src/main/java/org/springframework/data/jpa/repository/support/JpaRepositoryFactoryBean.java index 9f4872f42..f980c946a 100644 --- a/src/main/java/org/springframework/data/jpa/repository/support/JpaRepositoryFactoryBean.java +++ b/src/main/java/org/springframework/data/jpa/repository/support/JpaRepositoryFactoryBean.java @@ -39,7 +39,7 @@ public class JpaRepositoryFactoryBean, S, ID extends extends TransactionalRepositoryFactoryBeanSupport { private EntityManager entityManager; - private EscapeCharacter escapeCharacter = EscapeCharacter.of('\\'); + private EscapeCharacter escapeCharacter = EscapeCharacter.DEFAULT; /** * Creates a new {@link JpaRepositoryFactoryBean} for the given repository interface. diff --git a/src/main/java/org/springframework/data/jpa/repository/support/SimpleJpaRepository.java b/src/main/java/org/springframework/data/jpa/repository/support/SimpleJpaRepository.java index 7454bed15..90d6272ed 100644 --- a/src/main/java/org/springframework/data/jpa/repository/support/SimpleJpaRepository.java +++ b/src/main/java/org/springframework/data/jpa/repository/support/SimpleJpaRepository.java @@ -84,7 +84,7 @@ public class SimpleJpaRepository private final PersistenceProvider provider; private CrudMethodMetadata metadata; - private EscapeCharacter escapeCharacter; + private EscapeCharacter escapeCharacter = EscapeCharacter.DEFAULT; /** * Creates a new {@link SimpleJpaRepository} to manage objects of the given {@link JpaEntityInformation}. diff --git a/src/test/java/org/springframework/data/jpa/convert/QueryByExamplePredicateBuilderUnitTests.java b/src/test/java/org/springframework/data/jpa/convert/QueryByExamplePredicateBuilderUnitTests.java index 5277ddcac..471cc0ef6 100644 --- a/src/test/java/org/springframework/data/jpa/convert/QueryByExamplePredicateBuilderUnitTests.java +++ b/src/test/java/org/springframework/data/jpa/convert/QueryByExamplePredicateBuilderUnitTests.java @@ -120,22 +120,22 @@ public class QueryByExamplePredicateBuilderUnitTests { @Test(expected = IllegalArgumentException.class) // DATAJPA-218 public void getPredicateShouldThrowExceptionOnNullRoot() { - QueryByExamplePredicateBuilder.getPredicate(null, cb, of(new Person()), EscapeCharacter.of('\\')); + QueryByExamplePredicateBuilder.getPredicate(null, cb, of(new Person()), EscapeCharacter.DEFAULT); } @Test(expected = IllegalArgumentException.class) // DATAJPA-218 public void getPredicateShouldThrowExceptionOnNullCriteriaBuilder() { - QueryByExamplePredicateBuilder.getPredicate(root, null, of(new Person()), EscapeCharacter.of('\\')); + QueryByExamplePredicateBuilder.getPredicate(root, null, of(new Person()), EscapeCharacter.DEFAULT); } @Test(expected = IllegalArgumentException.class) // DATAJPA-218 public void getPredicateShouldThrowExceptionOnNullExample() { - QueryByExamplePredicateBuilder.getPredicate(root, null, null, EscapeCharacter.of('\\')); + QueryByExamplePredicateBuilder.getPredicate(root, null, null, EscapeCharacter.DEFAULT); } @Test // DATAJPA-218 public void emptyCriteriaListShouldResultTruePredicate() { - assertThat(QueryByExamplePredicateBuilder.getPredicate(root, cb, of(new Person()), EscapeCharacter.of('\\')), + assertThat(QueryByExamplePredicateBuilder.getPredicate(root, cb, of(new Person()), EscapeCharacter.DEFAULT), equalTo(truePredicate)); } @@ -145,7 +145,7 @@ public class QueryByExamplePredicateBuilderUnitTests { Person p = new Person(); p.firstname = "foo"; - assertThat(QueryByExamplePredicateBuilder.getPredicate(root, cb, of(p), EscapeCharacter.of('\\')), + assertThat(QueryByExamplePredicateBuilder.getPredicate(root, cb, of(p), EscapeCharacter.DEFAULT), equalTo(dummyPredicate)); verify(cb, times(1)).equal(any(Expression.class), eq("foo")); } @@ -161,7 +161,7 @@ public class QueryByExamplePredicateBuilderUnitTests { exception.expectCause(IsInstanceOf. instanceOf(IllegalArgumentException.class)); exception.expectMessage("Unexpected path type"); - QueryByExamplePredicateBuilder.getPredicate(root, cb, of(p), EscapeCharacter.of('\\')); + QueryByExamplePredicateBuilder.getPredicate(root, cb, of(p), EscapeCharacter.DEFAULT); } @Test // DATAJPA-218 @@ -171,7 +171,7 @@ public class QueryByExamplePredicateBuilderUnitTests { p.firstname = "foo"; p.age = 2L; - assertThat(QueryByExamplePredicateBuilder.getPredicate(root, cb, of(p), EscapeCharacter.of('\\')), + assertThat(QueryByExamplePredicateBuilder.getPredicate(root, cb, of(p), EscapeCharacter.DEFAULT), equalTo(andPredicate)); verify(cb, times(1)).equal(any(Expression.class), eq("foo")); @@ -187,7 +187,7 @@ public class QueryByExamplePredicateBuilderUnitTests { Example example = of(person, ExampleMatcher.matchingAny()); - assertThat(QueryByExamplePredicateBuilder.getPredicate(root, cb, example, EscapeCharacter.of('\\')), + assertThat(QueryByExamplePredicateBuilder.getPredicate(root, cb, example, EscapeCharacter.DEFAULT), equalTo(orPredicate)); verify(cb, times(1)).or(Matchers.anyVararg()); @@ -206,7 +206,7 @@ public class QueryByExamplePredicateBuilderUnitTests { .withStringMatcher(ExampleMatcher.StringMatcher.CONTAINING) // ); - QueryByExamplePredicateBuilder.getPredicate(root, cb, example, EscapeCharacter.of('\\')); + QueryByExamplePredicateBuilder.getPredicate(root, cb, example, EscapeCharacter.DEFAULT); verify(cb, times(1)).like(any(Expression.class), eq("%f\\\\o\\_o%"), eq('\\')); } @@ -225,7 +225,7 @@ public class QueryByExamplePredicateBuilderUnitTests { .withStringMatcher(ExampleMatcher.StringMatcher.STARTING) // ); - QueryByExamplePredicateBuilder.getPredicate(root, cb, example, EscapeCharacter.of('\\')); + QueryByExamplePredicateBuilder.getPredicate(root, cb, example, EscapeCharacter.DEFAULT); verify(cb, times(1)).like(any(Expression.class), eq("f\\\\o\\_o%"), eq('\\')); } @@ -243,7 +243,7 @@ public class QueryByExamplePredicateBuilderUnitTests { .withStringMatcher(ExampleMatcher.StringMatcher.ENDING) // ); - QueryByExamplePredicateBuilder.getPredicate(root, cb, example, EscapeCharacter.of('\\')); + QueryByExamplePredicateBuilder.getPredicate(root, cb, example, EscapeCharacter.DEFAULT); verify(cb, times(1)).like(any(Expression.class), eq("%f\\\\o\\_o"), eq('\\')); } diff --git a/src/test/java/org/springframework/data/jpa/repository/query/JpaCountQueryCreatorIntegrationTests.java b/src/test/java/org/springframework/data/jpa/repository/query/JpaCountQueryCreatorIntegrationTests.java index 2beeedd0c..ae3718f5a 100644 --- a/src/test/java/org/springframework/data/jpa/repository/query/JpaCountQueryCreatorIntegrationTests.java +++ b/src/test/java/org/springframework/data/jpa/repository/query/JpaCountQueryCreatorIntegrationTests.java @@ -60,7 +60,7 @@ public class JpaCountQueryCreatorIntegrationTests { PartTree tree = new PartTree("findDistinctByRolesIn", User.class); ParameterMetadataProvider metadataProvider = new ParameterMetadataProvider(entityManager.getCriteriaBuilder(), - queryMethod.getParameters(), provider, EscapeCharacter.of('\\')); + queryMethod.getParameters(), provider, EscapeCharacter.DEFAULT); JpaCountQueryCreator creator = new JpaCountQueryCreator(tree, queryMethod.getResultProcessor().getReturnedType(), entityManager.getCriteriaBuilder(), metadataProvider); diff --git a/src/test/java/org/springframework/data/jpa/repository/query/JpaQueryLookupStrategyUnitTests.java b/src/test/java/org/springframework/data/jpa/repository/query/JpaQueryLookupStrategyUnitTests.java index f4ff6c893..0b0675791 100644 --- a/src/test/java/org/springframework/data/jpa/repository/query/JpaQueryLookupStrategyUnitTests.java +++ b/src/test/java/org/springframework/data/jpa/repository/query/JpaQueryLookupStrategyUnitTests.java @@ -50,7 +50,7 @@ import org.springframework.data.repository.query.QueryLookupStrategy.Key; /** * Unit tests for {@link JpaQueryLookupStrategy}. - * + * * @author Oliver Gierke * @author Thomas Darimont */ @@ -80,7 +80,7 @@ public class JpaQueryLookupStrategyUnitTests { public void invalidAnnotatedQueryCausesException() throws Exception { QueryLookupStrategy strategy = JpaQueryLookupStrategy.create(em, Key.CREATE_IF_NOT_FOUND, extractor, - EVALUATION_CONTEXT_PROVIDER, EscapeCharacter.of('\\')); + EVALUATION_CONTEXT_PROVIDER, EscapeCharacter.DEFAULT); Method method = UserRepository.class.getMethod("findByFoo", String.class); RepositoryMetadata metadata = new DefaultRepositoryMetadata(UserRepository.class); @@ -99,7 +99,7 @@ public class JpaQueryLookupStrategyUnitTests { public void sholdThrowMorePreciseExceptionIfTryingToUsePaginationInNativeQueries() throws Exception { QueryLookupStrategy strategy = JpaQueryLookupStrategy.create(em, Key.CREATE_IF_NOT_FOUND, extractor, - EVALUATION_CONTEXT_PROVIDER, EscapeCharacter.of('\\')); + EVALUATION_CONTEXT_PROVIDER, EscapeCharacter.DEFAULT); Method method = UserRepository.class.getMethod("findByInvalidNativeQuery", String.class, Pageable.class); RepositoryMetadata metadata = new DefaultRepositoryMetadata(UserRepository.class); diff --git a/src/test/java/org/springframework/data/jpa/repository/query/ParameterExpressionProviderTests.java b/src/test/java/org/springframework/data/jpa/repository/query/ParameterExpressionProviderTests.java index 12bcd6373..fc6ae1973 100644 --- a/src/test/java/org/springframework/data/jpa/repository/query/ParameterExpressionProviderTests.java +++ b/src/test/java/org/springframework/data/jpa/repository/query/ParameterExpressionProviderTests.java @@ -58,7 +58,7 @@ public class ParameterExpressionProviderTests { CriteriaBuilder builder = em.getCriteriaBuilder(); PersistenceProvider persistenceProvider = PersistenceProvider.fromEntityManager(em); - ParameterMetadataProvider provider = new ParameterMetadataProvider(builder, accessor, persistenceProvider, EscapeCharacter.of('\\')); + ParameterMetadataProvider provider = new ParameterMetadataProvider(builder, accessor, persistenceProvider, EscapeCharacter.DEFAULT); ParameterExpression expression = provider.next(part, Comparable.class).getExpression(); assertThat(expression.getParameterType(), is(typeCompatibleWith(int.class))); } diff --git a/src/test/java/org/springframework/data/jpa/repository/query/ParameterMetadataProviderIntegrationTests.java b/src/test/java/org/springframework/data/jpa/repository/query/ParameterMetadataProviderIntegrationTests.java index 8acc863d6..b77ff8402 100644 --- a/src/test/java/org/springframework/data/jpa/repository/query/ParameterMetadataProviderIntegrationTests.java +++ b/src/test/java/org/springframework/data/jpa/repository/query/ParameterMetadataProviderIntegrationTests.java @@ -81,7 +81,7 @@ public class ParameterMetadataProviderIntegrationTests { simulateDiscoveredParametername(parameters, 0, "name"); return new ParameterMetadataProvider(em.getCriteriaBuilder(), parameters, - PersistenceProvider.fromEntityManager(em), EscapeCharacter.of('\\')); + PersistenceProvider.fromEntityManager(em), EscapeCharacter.DEFAULT); } @SuppressWarnings("unchecked") diff --git a/src/test/java/org/springframework/data/jpa/repository/query/ParameterMetadataProviderUnitTests.java b/src/test/java/org/springframework/data/jpa/repository/query/ParameterMetadataProviderUnitTests.java index 7886db227..f57fff216 100644 --- a/src/test/java/org/springframework/data/jpa/repository/query/ParameterMetadataProviderUnitTests.java +++ b/src/test/java/org/springframework/data/jpa/repository/query/ParameterMetadataProviderUnitTests.java @@ -44,7 +44,7 @@ public class ParameterMetadataProviderUnitTests { Parameters parameters = mock(Parameters.class, RETURNS_DEEP_STUBS); ParameterMetadataProvider metadataProvider = new ParameterMetadataProvider(builder, parameters, - persistenceProvider, EscapeCharacter.of('\\')); + persistenceProvider, EscapeCharacter.DEFAULT); exception.expect(IllegalArgumentException.class); exception.expectMessage("parameter"); diff --git a/src/test/java/org/springframework/data/jpa/repository/query/PartTreeJpaQueryIntegrationTests.java b/src/test/java/org/springframework/data/jpa/repository/query/PartTreeJpaQueryIntegrationTests.java index 1cc56c948..e4972f082 100644 --- a/src/test/java/org/springframework/data/jpa/repository/query/PartTreeJpaQueryIntegrationTests.java +++ b/src/test/java/org/springframework/data/jpa/repository/query/PartTreeJpaQueryIntegrationTests.java @@ -54,7 +54,7 @@ import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; /** * Integration tests for {@link PartTreeJpaQuery}. - * + * * @author Oliver Gierke * @author Mark Paluch */ @@ -79,7 +79,7 @@ public class PartTreeJpaQueryIntegrationTests { public void test() throws Exception { JpaQueryMethod queryMethod = getQueryMethod("findByFirstname", String.class, Pageable.class); - PartTreeJpaQuery jpaQuery = new PartTreeJpaQuery(queryMethod, entityManager, provider, EscapeCharacter.of('\\')); + PartTreeJpaQuery jpaQuery = new PartTreeJpaQuery(queryMethod, entityManager, provider, EscapeCharacter.DEFAULT); jpaQuery.createQuery(new Object[] { "Matthews", new PageRequest(0, 1) }); jpaQuery.createQuery(new Object[] { "Matthews", new PageRequest(0, 1) }); @@ -103,7 +103,7 @@ public class PartTreeJpaQueryIntegrationTests { public void recreatesQueryIfNullValueIsGiven() throws Exception { JpaQueryMethod queryMethod = getQueryMethod("findByFirstname", String.class, Pageable.class); - PartTreeJpaQuery jpaQuery = new PartTreeJpaQuery(queryMethod, entityManager, provider, EscapeCharacter.of('\\')); + PartTreeJpaQuery jpaQuery = new PartTreeJpaQuery(queryMethod, entityManager, provider, EscapeCharacter.DEFAULT); Query query = jpaQuery.createQuery(new Object[] { "Matthews", new PageRequest(0, 1) }); @@ -118,7 +118,7 @@ public class PartTreeJpaQueryIntegrationTests { public void shouldLimitExistsProjectionQueries() throws Exception { JpaQueryMethod queryMethod = getQueryMethod("existsByFirstname", String.class); - PartTreeJpaQuery jpaQuery = new PartTreeJpaQuery(queryMethod, entityManager, provider, EscapeCharacter.of('\\')); + PartTreeJpaQuery jpaQuery = new PartTreeJpaQuery(queryMethod, entityManager, provider, EscapeCharacter.DEFAULT); Query query = jpaQuery.createQuery(new Object[] { "Matthews" }); @@ -129,7 +129,7 @@ public class PartTreeJpaQueryIntegrationTests { public void shouldSelectAliasedIdForExistsProjectionQueries() throws Exception { JpaQueryMethod queryMethod = getQueryMethod("existsByFirstname", String.class); - PartTreeJpaQuery jpaQuery = new PartTreeJpaQuery(queryMethod, entityManager, provider, EscapeCharacter.of('\\')); + PartTreeJpaQuery jpaQuery = new PartTreeJpaQuery(queryMethod, entityManager, provider, EscapeCharacter.DEFAULT); Query query = jpaQuery.createQuery(new Object[] { "Matthews" }); @@ -172,7 +172,7 @@ public class PartTreeJpaQueryIntegrationTests { JpaQueryMethod queryMethod = getQueryMethod(methodName, parameterTypes); PartTreeJpaQuery jpaQuery = new PartTreeJpaQuery(queryMethod, entityManager, - PersistenceProvider.fromEntityManager(entityManager), EscapeCharacter.of('\\')); + PersistenceProvider.fromEntityManager(entityManager), EscapeCharacter.DEFAULT); jpaQuery.createQuery(values); }