From 3590075a46663ff83cd4b1772d6ba720b91ba70c Mon Sep 17 00:00:00 2001 From: Thomas Darimont Date: Fri, 14 Nov 2014 16:18:24 +0100 Subject: [PATCH] DATAJPA-606 - Improve handling of empty arrays and collections in query parameters. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Introduced potentiallyConvertEmptyCollection method to PersistenceProvider for provider specific handling of empty collections. Special handling is performed for Hibernate and EclipseLink. Conversion is performed in ParameterMetadata.prepare(…). Renamed ParameterMetadataProvider.accessor to bindableParameterValues. Original pull request: #123. --- .../jpa/provider/PersistenceProvider.java | 29 ++++++ .../query/ParameterMetadataProvider.java | 88 ++++++++++++++----- .../repository/query/PartTreeJpaQuery.java | 13 ++- .../jpa/repository/UserRepositoryTests.java | 53 ++++++++++- .../ParameterExpressionProviderTests.java | 6 +- .../jpa/repository/sample/UserRepository.java | 15 ++++ 6 files changed, 177 insertions(+), 27 deletions(-) diff --git a/src/main/java/org/springframework/data/jpa/provider/PersistenceProvider.java b/src/main/java/org/springframework/data/jpa/provider/PersistenceProvider.java index dd1177047..aff2820e0 100644 --- a/src/main/java/org/springframework/data/jpa/provider/PersistenceProvider.java +++ b/src/main/java/org/springframework/data/jpa/provider/PersistenceProvider.java @@ -19,6 +19,7 @@ import static org.springframework.data.jpa.provider.JpaClassUtils.*; import static org.springframework.data.jpa.provider.PersistenceProvider.Constants.*; import java.util.Arrays; +import java.util.Collection; import java.util.Collections; import javax.persistence.EntityManager; @@ -85,6 +86,14 @@ public enum PersistenceProvider implements QueryExtractor, ProxyIdAccessor { public Object getIdentifierFrom(Object entity) { return ((HibernateProxy) entity).getHibernateLazyInitializer().getIdentifier(); } + + /* (non-Javadoc) + * @see org.springframework.data.jpa.provider.PersistenceProvider#potentiallyConvertEmptyCollection(java.util.Collection) + */ + @Override + public Collection potentiallyConvertEmptyCollection(Collection collection) { + return collection == null || collection.isEmpty() ? null : collection; + } }, /** @@ -114,6 +123,14 @@ public enum PersistenceProvider implements QueryExtractor, ProxyIdAccessor { public Object getIdentifierFrom(Object entity) { return null; } + + /* (non-Javadoc) + * @see org.springframework.data.jpa.provider.PersistenceProvider#potentiallyConvertEmptyCollection(java.util.Collection) + */ + @Override + public Collection potentiallyConvertEmptyCollection(Collection collection) { + return collection == null || collection.isEmpty() ? null : collection; + } }, /** @@ -281,4 +298,16 @@ public enum PersistenceProvider implements QueryExtractor, ProxyIdAccessor { public String getCountQueryPlaceholder() { return "x"; } + + /** + * Potentially converts an empty collection to the appropriate representation of this {@link PersistenceProvider}, + * since some JPA providers cannot correctly handle empty collections. + * + * @see DATAJPA-606 + * @param collection + * @return + */ + public Collection potentiallyConvertEmptyCollection(Collection collection) { + return collection; + } } diff --git a/src/main/java/org/springframework/data/jpa/repository/query/ParameterMetadataProvider.java b/src/main/java/org/springframework/data/jpa/repository/query/ParameterMetadataProvider.java index aa191a961..5fe789945 100644 --- a/src/main/java/org/springframework/data/jpa/repository/query/ParameterMetadataProvider.java +++ b/src/main/java/org/springframework/data/jpa/repository/query/ParameterMetadataProvider.java @@ -1,5 +1,5 @@ /* - * Copyright 2011-2013 the original author or authors. + * Copyright 2011-2014 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. @@ -25,6 +25,7 @@ import java.util.List; import javax.persistence.criteria.CriteriaBuilder; import javax.persistence.criteria.ParameterExpression; +import org.springframework.data.jpa.provider.PersistenceProvider; import org.springframework.data.repository.query.Parameter; import org.springframework.data.repository.query.Parameters; import org.springframework.data.repository.query.ParametersParameterAccessor; @@ -39,36 +40,66 @@ import org.springframework.util.ObjectUtils; * Helper class to allow easy creation of {@link ParameterMetadata}s. * * @author Oliver Gierke + * @author Thomas Darimont */ class ParameterMetadataProvider { private final CriteriaBuilder builder; private final Iterator parameters; private final List> expressions; - private Iterator accessor; + private final Iterator bindableParameterValues; + private final PersistenceProvider persistenceProvider; /** * Creates a new {@link ParameterMetadataProvider} from the given {@link CriteriaBuilder} and - * {@link ParametersParameterAccessor}. + * {@link ParametersParameterAccessor} with support for parameter value customizations via {@link PersistenceProvider} + * . + * + * @param builder must not be {@literal null}. + * @param accessor must not be {@literal null}. + * @param provider must not be {@literal null}. + */ + public ParameterMetadataProvider(CriteriaBuilder builder, ParametersParameterAccessor accessor, + PersistenceProvider provider) { + + this(builder, accessor.iterator(), accessor.getParameters(), provider); + } + + /** + * Creates a new {@link ParameterMetadataProvider} from the given {@link CriteriaBuilder} and {@link Parameters} with + * support for parameter value customizations via {@link PersistenceProvider}. * * @param builder must not be {@literal null}. * @param parameters must not be {@literal null}. + * @param provider must not be {@literal null}. */ - public ParameterMetadataProvider(CriteriaBuilder builder, ParametersParameterAccessor accessor) { + public ParameterMetadataProvider(CriteriaBuilder builder, Parameters parameters, PersistenceProvider provider) { - this(builder, accessor.getParameters()); - Assert.notNull(accessor); - this.accessor = accessor.iterator(); + this(builder, null, parameters, provider); } - public ParameterMetadataProvider(CriteriaBuilder builder, Parameters parameters) { + /** + * Creates a new {@link ParameterMetadataProvider} from the given {@link CriteriaBuilder} an {@link Iterable} of all + * bindable parameter values, and {@link Parameters} with support for parameter value customizations via + * {@link PersistenceProvider}. + * + * @param builder must not be {@literal null}. + * @param bindableParameterValues may be {@literal null}. + * @param parameters must not be {@literal null}. + * @param provider must not be {@literal null}. + */ + private ParameterMetadataProvider(CriteriaBuilder builder, Iterator bindableParameterValues, + Parameters parameters, PersistenceProvider provider) { Assert.notNull(builder); + Assert.notNull(parameters); + Assert.notNull(provider); this.builder = builder; this.parameters = parameters.getBindableParameters().iterator(); this.expressions = new ArrayList>(); - this.accessor = null; + this.bindableParameterValues = bindableParameterValues; + this.persistenceProvider = provider; } /** @@ -131,22 +162,38 @@ class ParameterMetadataProvider { ParameterExpression expression = name == null ? builder.parameter(reifiedType) : builder.parameter(reifiedType, name); ParameterMetadata value = new ParameterMetadata(expression, part.getType(), - accessor == null ? ParameterMetadata.PLACEHOLDER : accessor.next()); + bindableParameterValues == null ? ParameterMetadata.PLACEHOLDER : bindableParameterValues.next(), + this.persistenceProvider); expressions.add(value); return value; } + /** + * @author Oliver Gierke + * @author Thomas Darimont + * @param + */ static class ParameterMetadata { static final Object PLACEHOLDER = new Object(); - private final ParameterExpression expression; private final Type type; + private final ParameterExpression expression; + private final PersistenceProvider persistenceProvider; - public ParameterMetadata(ParameterExpression expression, Type type, Object value) { + /** + * Creates a new {@link ParameterMetadata}. + * + * @param expression + * @param type + * @param value + * @param provider + */ + public ParameterMetadata(ParameterExpression expression, Type type, Object value, PersistenceProvider provider) { this.expression = expression; + this.persistenceProvider = provider; this.type = value == null && Type.SIMPLE_PROPERTY.equals(type) ? Type.IS_NULL : type; } @@ -171,27 +218,28 @@ class ParameterMetadataProvider { /** * Prepares the object before it's actually bound to the {@link javax.persistence.Query;}. * - * @param parameter must not be {@literal null}. + * @param value must not be {@literal null}. * @return */ - public Object prepare(Object parameter) { + public Object prepare(Object value) { - Assert.notNull(parameter); + Assert.notNull(value); switch (type) { case STARTING_WITH: - return String.format("%s%%", parameter.toString()); + return String.format("%s%%", value.toString()); case ENDING_WITH: - return String.format("%%%s", parameter.toString()); + return String.format("%%%s", value.toString()); case CONTAINING: - return String.format("%%%s%%", parameter.toString()); + return String.format("%%%s%%", value.toString()); default: - return Collection.class.equals(expression.getJavaType()) ? toCollection(parameter) : parameter; + return Collection.class.isAssignableFrom(expression.getJavaType()) ? persistenceProvider + .potentiallyConvertEmptyCollection(toCollection(value)) : value; } } /** - * Return sthe given argument as {@link Collection} which means it will return it as is if it's a + * Returns the given argument as {@link Collection} which means it will return it as is if it's a * {@link Collections}, turn an array into an {@link ArrayList} or simply wrap any other value into a single element * {@link Collections}. * diff --git a/src/main/java/org/springframework/data/jpa/repository/query/PartTreeJpaQuery.java b/src/main/java/org/springframework/data/jpa/repository/query/PartTreeJpaQuery.java index 32546aeed..6156f6802 100644 --- a/src/main/java/org/springframework/data/jpa/repository/query/PartTreeJpaQuery.java +++ b/src/main/java/org/springframework/data/jpa/repository/query/PartTreeJpaQuery.java @@ -24,6 +24,7 @@ import javax.persistence.criteria.CriteriaBuilder; import javax.persistence.criteria.CriteriaQuery; import org.springframework.data.domain.Sort; +import org.springframework.data.jpa.provider.PersistenceProvider; import org.springframework.data.jpa.repository.query.JpaQueryExecution.DeleteExecution; import org.springframework.data.jpa.repository.query.ParameterMetadataProvider.ParameterMetadata; import org.springframework.data.repository.query.ParametersParameterAccessor; @@ -96,6 +97,7 @@ public class PartTreeJpaQuery extends AbstractJpaQuery { * Query preparer to create {@link CriteriaQuery} instances and potentially cache them. * * @author Oliver Gierke + * @author Thomas Darimont */ private class QueryPreparer { @@ -185,9 +187,10 @@ public class PartTreeJpaQuery extends AbstractJpaQuery { EntityManager entityManager = getEntityManager(); CriteriaBuilder builder = entityManager.getCriteriaBuilder(); + PersistenceProvider persistenceProvider = PersistenceProvider.fromEntityManager(entityManager); - ParameterMetadataProvider provider = accessor == null ? new ParameterMetadataProvider(builder, parameters) - : new ParameterMetadataProvider(builder, accessor); + ParameterMetadataProvider provider = accessor == null ? new ParameterMetadataProvider(builder, parameters, + persistenceProvider) : new ParameterMetadataProvider(builder, accessor, persistenceProvider); return new JpaQueryCreator(tree, domainClass, builder, provider); } @@ -219,6 +222,7 @@ public class PartTreeJpaQuery extends AbstractJpaQuery { * Special {@link QueryPreparer} to create count queries. * * @author Oliver Gierke + * @author Thomas Darimont */ private class CountQueryPreparer extends QueryPreparer { @@ -235,9 +239,10 @@ public class PartTreeJpaQuery extends AbstractJpaQuery { EntityManager entityManager = getEntityManager(); CriteriaBuilder builder = entityManager.getCriteriaBuilder(); + PersistenceProvider persistenceProvider = PersistenceProvider.fromEntityManager(entityManager); - ParameterMetadataProvider provider = accessor == null ? new ParameterMetadataProvider(builder, parameters) - : new ParameterMetadataProvider(builder, accessor); + ParameterMetadataProvider provider = accessor == null ? new ParameterMetadataProvider(builder, parameters, + persistenceProvider) : new ParameterMetadataProvider(builder, accessor, persistenceProvider); return new JpaCountQueryCreator(tree, domainClass, builder, provider); } diff --git a/src/test/java/org/springframework/data/jpa/repository/UserRepositoryTests.java b/src/test/java/org/springframework/data/jpa/repository/UserRepositoryTests.java index a3caceda1..5f2392856 100644 --- a/src/test/java/org/springframework/data/jpa/repository/UserRepositoryTests.java +++ b/src/test/java/org/springframework/data/jpa/repository/UserRepositoryTests.java @@ -1751,8 +1751,8 @@ public class UserRepositoryTests { assertThat(users.getContent(), hasSize(2)); assertThat(users.getContent().get(0), is(thirdUser)); assertThat(users.getContent().get(1), is(fourthUser)); - } - + } + /** * @see DATAJPA-629 */ @@ -1767,6 +1767,55 @@ public class UserRepositoryTests { assertThat(users.get(0), is(secondUser)); } + /** + * @see DATAJPA-606 + */ + @Test + public void findByEmptyCollectionOfStrings() throws Exception { + + flushTestUsers(); + + List users = repository.findByAttributesIn(new HashSet()); + assertThat(users, hasSize(0)); + } + + /** + * @see DATAJPA-606 + */ + @Test + public void findByEmptyCollectionOfIntegers() throws Exception { + + flushTestUsers(); + + List users = repository.findByAgeIn(Arrays. asList()); + assertThat(users, hasSize(0)); + } + + /** + * @see DATAJPA-606 + */ + @Test + public void findByEmptyArrayOfIntegers() throws Exception { + + flushTestUsers(); + + List users = repository.queryByAgeIn(new Integer[0]); + assertThat(users, hasSize(0)); + } + + /** + * @see DATAJPA-606 + */ + @Test + public void findByAgeWithEmptyArrayOfIntegersOrFirstName() { + + flushTestUsers(); + + List users = repository.queryByAgeInOrFirstname(new Integer[0], secondUser.getFirstname()); + assertThat(users, hasSize(1)); + assertThat(users.get(0), is(secondUser)); + } + private Page executeSpecWithSort(Sort sort) { flushTestUsers(); 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 c52796664..988d639c7 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 @@ -7,11 +7,13 @@ import java.lang.reflect.Method; import javax.persistence.EntityManager; import javax.persistence.PersistenceContext; +import javax.persistence.criteria.CriteriaBuilder; import javax.persistence.criteria.ParameterExpression; import org.junit.Test; import org.junit.runner.RunWith; import org.springframework.data.jpa.domain.sample.User; +import org.springframework.data.jpa.provider.PersistenceProvider; import org.springframework.data.repository.query.DefaultParameters; import org.springframework.data.repository.query.Parameters; import org.springframework.data.repository.query.ParametersParameterAccessor; @@ -42,7 +44,9 @@ public class ParameterExpressionProviderTests { ParametersParameterAccessor accessor = new ParametersParameterAccessor(parameters, new Object[] { 1 }); Part part = new Part("IdGreaterThan", User.class); - ParameterMetadataProvider provider = new ParameterMetadataProvider(em.getCriteriaBuilder(), accessor); + CriteriaBuilder builder = em.getCriteriaBuilder(); + PersistenceProvider persistenceProvider = PersistenceProvider.fromEntityManager(em); + ParameterMetadataProvider provider = new ParameterMetadataProvider(builder, accessor, persistenceProvider); 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/sample/UserRepository.java b/src/test/java/org/springframework/data/jpa/repository/sample/UserRepository.java index 9c6d02861..b9f2ce83a 100644 --- a/src/test/java/org/springframework/data/jpa/repository/sample/UserRepository.java +++ b/src/test/java/org/springframework/data/jpa/repository/sample/UserRepository.java @@ -534,4 +534,19 @@ public interface UserRepository extends JpaRepository, JpaSpecifi @Query("select u from #{#entityName} u where u.firstname = ?#{[0]} and u.lastname = ?#{[1]}") List findUsersByFirstnameForSpELExpressionWithParameterIndexOnlyWithEntityExpression(String firstname, String lastname); + + /** + * DATAJPA-606 + */ + List findByAgeIn(Collection ages); + + /** + * DATAJPA-606 + */ + List queryByAgeIn(Integer[] ages); + + /** + * DATAJPA-606 + */ + List queryByAgeInOrFirstname(Integer[] ages, String firstname); }