DATAJPA-606 - Improve handling of empty arrays and collections in query parameters.

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.
This commit is contained in:
Thomas Darimont
2014-11-14 16:18:24 +01:00
committed by Oliver Gierke
parent 36a6f958f2
commit 3590075a46
6 changed files with 177 additions and 27 deletions

View File

@@ -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 <T> Collection<T> potentiallyConvertEmptyCollection(Collection<T> 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 <T> Collection<T> potentiallyConvertEmptyCollection(Collection<T> 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 <T> Collection<T> potentiallyConvertEmptyCollection(Collection<T> collection) {
return collection;
}
}

View File

@@ -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<? extends Parameter> parameters;
private final List<ParameterMetadata<?>> expressions;
private Iterator<Object> accessor;
private final Iterator<Object> 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<Object> 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<ParameterMetadata<?>>();
this.accessor = null;
this.bindableParameterValues = bindableParameterValues;
this.persistenceProvider = provider;
}
/**
@@ -131,22 +162,38 @@ class ParameterMetadataProvider {
ParameterExpression<T> expression = name == null ? builder.parameter(reifiedType) : builder.parameter(reifiedType,
name);
ParameterMetadata<T> value = new ParameterMetadata<T>(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 <T>
*/
static class ParameterMetadata<T> {
static final Object PLACEHOLDER = new Object();
private final ParameterExpression<T> expression;
private final Type type;
private final ParameterExpression<T> expression;
private final PersistenceProvider persistenceProvider;
public ParameterMetadata(ParameterExpression<T> expression, Type type, Object value) {
/**
* Creates a new {@link ParameterMetadata}.
*
* @param expression
* @param type
* @param value
* @param provider
*/
public ParameterMetadata(ParameterExpression<T> 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}.
*

View File

@@ -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);
}

View File

@@ -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<User> users = repository.findByAttributesIn(new HashSet<String>());
assertThat(users, hasSize(0));
}
/**
* @see DATAJPA-606
*/
@Test
public void findByEmptyCollectionOfIntegers() throws Exception {
flushTestUsers();
List<User> users = repository.findByAgeIn(Arrays.<Integer> asList());
assertThat(users, hasSize(0));
}
/**
* @see DATAJPA-606
*/
@Test
public void findByEmptyArrayOfIntegers() throws Exception {
flushTestUsers();
List<User> users = repository.queryByAgeIn(new Integer[0]);
assertThat(users, hasSize(0));
}
/**
* @see DATAJPA-606
*/
@Test
public void findByAgeWithEmptyArrayOfIntegersOrFirstName() {
flushTestUsers();
List<User> users = repository.queryByAgeInOrFirstname(new Integer[0], secondUser.getFirstname());
assertThat(users, hasSize(1));
assertThat(users.get(0), is(secondUser));
}
private Page<User> executeSpecWithSort(Sort sort) {
flushTestUsers();

View File

@@ -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<? extends Comparable> expression = provider.next(part, Comparable.class).getExpression();
assertThat(expression.getParameterType(), is(typeCompatibleWith(int.class)));
}

View File

@@ -534,4 +534,19 @@ public interface UserRepository extends JpaRepository<User, Integer>, JpaSpecifi
@Query("select u from #{#entityName} u where u.firstname = ?#{[0]} and u.lastname = ?#{[1]}")
List<User> findUsersByFirstnameForSpELExpressionWithParameterIndexOnlyWithEntityExpression(String firstname,
String lastname);
/**
* DATAJPA-606
*/
List<User> findByAgeIn(Collection<Integer> ages);
/**
* DATAJPA-606
*/
List<User> queryByAgeIn(Integer[] ages);
/**
* DATAJPA-606
*/
List<User> queryByAgeInOrFirstname(Integer[] ages, String firstname);
}