DATAJPA-461 - Polishing of binding implementation for StringQueries.

We now always create a ParameterBinding for all parameters to simplify the client code so that it can safely always lookup bindings and apply them.

Changed the setup of the regular expression to work with the keywords provided by the binding types to ease future extensions.

Added integration test to quickly verify the EclipseLink bug we're running into now for further reference.

Original pull request: #56.
This commit is contained in:
Oliver Gierke
2014-02-11 15:07:02 +01:00
parent f8b0917c90
commit 1d4b57f684
7 changed files with 115 additions and 66 deletions

View File

@@ -40,8 +40,8 @@ public class ParameterBinder {
/**
* Creates a new {@link ParameterBinder}.
*
* @param parameters
* @param values
* @param parameters must not be {@literal null}.
* @param values must not be {@literal null}.
*/
public ParameterBinder(JpaParameters parameters, Object[] values) {
@@ -55,7 +55,6 @@ public class ParameterBinder {
}
ParameterBinder(JpaParameters parameters) {
this(parameters, new Object[0]);
}
@@ -128,12 +127,10 @@ public class ParameterBinder {
return;
}
Object valueToUse = value;
if (hasNamedParameter(query) && parameter.isNamedParameter()) {
query.setParameter(parameter.getName(), valueToUse);
query.setParameter(parameter.getName(), value);
} else {
query.setParameter(position, valueToUse);
query.setParameter(position, value);
}
}
@@ -144,10 +141,13 @@ public class ParameterBinder {
* @return
*/
public Query bindAndPrepare(Query query) {
return bindAndPrepare(query, parameters);
}
boolean hasNamedParameter(Query query) {
return QueryUtils.hasNamedParameter(query);
}
private Query bindAndPrepare(Query query, Parameters<?, ?> parameters) {
Query result = bind(query);
@@ -161,9 +161,4 @@ public class ParameterBinder {
return result;
}
boolean hasNamedParameter(Query query) {
return QueryUtils.hasNamedParameter(query);
}
}

View File

@@ -97,18 +97,20 @@ class StringQuery {
/**
* Returns the {@link ParameterBinding} for the given name.
*
* @param name
* @param name must not be {@literal null} or empty.
* @return
*/
public ParameterBinding getBindingFor(String name) {
for (StringQuery.ParameterBinding binding : bindings) {
Assert.hasText(name, "Name must not be null or empty!");
for (ParameterBinding binding : bindings) {
if (binding.hasName(name)) {
return binding;
}
}
return null;
throw new IllegalArgumentException(String.format("No parameter binding found for name %s!", name));
}
/**
@@ -118,13 +120,14 @@ class StringQuery {
* @return
*/
public ParameterBinding getBindingFor(int position) {
for (ParameterBinding binding : bindings) {
if (binding.hasPosition(position)) {
return binding;
}
}
return null;
throw new IllegalArgumentException(String.format("No parameter binding found for position %s!", position));
}
/**
@@ -132,7 +135,7 @@ class StringQuery {
*
* @author Thomas Darimont
*/
static enum ParameterBindingParser {
private static enum ParameterBindingParser {
INSTANCE;
@@ -142,9 +145,19 @@ class StringQuery {
static {
List<String> keywords = new ArrayList<String>();
for (ParameterBindingType type : ParameterBindingType.values()) {
if (type.getKeyword() != null) {
keywords.add(type.getKeyword());
}
}
StringBuilder builder = new StringBuilder();
builder.append("(?<=(like|in))"); // parameterBindingType -> starts with "like" or "in"
builder.append("(?: )+"); // some whitespace
builder.append("(?<=(");
builder.append(StringUtils.collectionToDelimitedString(keywords, "|")); // keywords
builder.append("))");
builder.append("(?: )?"); // some whitespace
builder.append("(");
builder.append("%?(\\?(\\d+))%?"); // position parameter
builder.append("|"); // or
@@ -173,7 +186,8 @@ class StringQuery {
String parameterName = parameterIndexString != null ? null : matcher.group(6);
Integer parameterIndex = parameterIndexString == null ? null : Integer.valueOf(parameterIndexString);
switch (ParameterBindingKind.of(matcher.group(1))) {
switch (ParameterBindingType.of(matcher.group(1))) {
case LIKE:
Type likeType = LikeParameterBinding.getLikeTypeFrom(matcher.group(2));
@@ -187,7 +201,6 @@ class StringQuery {
}
result = StringUtils.replace(result, matcher.group(2), replacement);
break;
case IN:
@@ -198,11 +211,15 @@ class StringQuery {
checkAndRegister(new InParameterBinding(parameterName), bindings);
}
result = query;
break;
case AS_IS: // fall-through we don't need a special parameter binding for the given parameter.
default:
;
bindings.add(parameterIndex != null ? new ParameterBinding(parameterIndex) : new ParameterBinding(
parameterName));
result = query;
}
}
@@ -221,25 +238,44 @@ class StringQuery {
}
/**
* An enum for the supported parameter binding kinds.
* An enum for the different types of bindings.
*
* @author Thomas Darimont
* @author Oliver Gierke
*/
enum ParameterBindingKind {
private static enum ParameterBindingType {
LIKE, IN, AS_IS;
// Trailing whitespace is intentional to reflect that the keywords must be used with at least one whitespace
// character, while = does not.
LIKE("like "), IN("in "), AS_IS("=");
private final String keyword;
private ParameterBindingType(String keyword) {
this.keyword = keyword;
}
/**
* Return the appropriate {@link ParameterBindingKind} for the given {@link String}. Returns {@value #AS_IS} in
* case no other {@link ParameterBindingKind} could be found.
* Returns the keyword that will tirgger the binding type or {@literal null} if the type is not triggered by a
* keyword.
*
* @return the keyword
*/
public String getKeyword() {
return keyword;
}
/**
* Return the appropriate {@link ParameterBindingType} for the given {@link String}. Returns {@keyword
* #AS_IS} in case no other {@link ParameterBindingType} could be found.
*
* @param parameterBindingKindName
* @return
*/
static ParameterBindingKind of(String parameterBindingKindName) {
static ParameterBindingType of(String parameterBindingKindName) {
for (ParameterBindingKind type : values()) {
if (type.name().equalsIgnoreCase(parameterBindingKindName)) {
for (ParameterBindingType type : values()) {
if (type.name().equalsIgnoreCase(parameterBindingKindName.trim())) {
return type;
}
}
@@ -321,6 +357,10 @@ class StringQuery {
return position;
}
/*
* (non-Javadoc)
* @see java.lang.Object#hashCode()
*/
@Override
public int hashCode() {
@@ -332,6 +372,10 @@ class StringQuery {
return result;
}
/*
* (non-Javadoc)
* @see java.lang.Object#equals(java.lang.Object)
*/
@Override
public boolean equals(Object obj) {
@@ -393,16 +437,7 @@ class StringQuery {
* @see org.springframework.data.jpa.repository.query.StringQuery.ParameterBinding#prepare(java.lang.Object)
*/
@Override
public Object prepare(Object valueToBind) {
return convertArrayToCollectionIfNecessary(valueToBind);
}
/**
* Returns the given value as collection if it is an array or as is if not.
*
* @return
*/
private Object convertArrayToCollectionIfNecessary(Object value) {
public Object prepare(Object value) {
if (!ObjectUtils.isArray(value)) {
return value;
@@ -472,19 +507,20 @@ class StringQuery {
}
/**
* Returns the type of the {@link LikeParameterBinding}.
* Returns the {@link Type} of the binding.
*
* @return
* @return the type
*/
public Type getType() {
return type;
}
/**
* Prepares the given raw value according to the like type.
* Prepares the given raw keyword according to the like type.
*
* @param value
* @param keyword
*/
@Override
public Object prepare(Object value) {
if (value == null) {

View File

@@ -57,18 +57,8 @@ public class StringQueryParameterBinder extends ParameterBinder {
@Override
protected void bind(Query jpaQuery, JpaParameter methodParameter, Object value, int position) {
Object valueToBind = value;
if (query.hasParameterBindings()) {
ParameterBinding binding = getBindingFor(jpaQuery, position, methodParameter);
if (binding != null) {
valueToBind = binding.prepare(valueToBind);
}
}
super.bind(jpaQuery, methodParameter, valueToBind, position);
ParameterBinding binding = getBindingFor(jpaQuery, position, methodParameter);
super.bind(jpaQuery, methodParameter, binding.prepare(value), position);
}
/**
@@ -77,7 +67,7 @@ public class StringQueryParameterBinder extends ParameterBinder {
* @param jpaQuery must not be {@literal null}.
* @param position
* @param methodParameter must not be {@literal null}.
* @return the {@link LikeParameterBinding} for the given parameters or {@literal null} if none available.
* @return the {@link ParameterBinding} for the given parameters or {@literal null} if none available.
*/
private ParameterBinding getBindingFor(Query jpaQuery, int position, Parameter methodParameter) {
@@ -93,6 +83,10 @@ public class StringQueryParameterBinder extends ParameterBinder {
}
}
return null;
// We should actually reject parameters unavailable, but as EclipseLink doesn't implement ….getParameter(int) for
// native queries correctly we need to fall back to an indexed parameter
// @see https://bugs.eclipse.org/bugs/show_bug.cgi?id=427892
return new ParameterBinding(position);
}
}

View File

@@ -1,5 +1,5 @@
/*
* Copyright 2013 the original author or authors.
* Copyright 2013-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.
@@ -15,6 +15,8 @@
*/
package org.springframework.data.jpa.infrastructure;
import org.junit.Ignore;
import org.junit.Test;
import org.springframework.test.context.ContextConfiguration;
/**
@@ -25,4 +27,13 @@ import org.springframework.test.context.ContextConfiguration;
@ContextConfiguration("classpath:eclipselink.xml")
public class EclipseLinkMetamodelIntegrationTests extends MetamodelIntegrationTests {
/**
* TODO: Remove, once https://bugs.eclipse.org/bugs/show_bug.cgi?id=427892 is fixed.
*/
@Test
@Ignore
@Override
public void canAccessParametersByIndexForNativeQueries() {
}
}

View File

@@ -1,5 +1,5 @@
/*
* Copyright 2013 the original author or authors.
* Copyright 2013-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.
@@ -20,6 +20,7 @@ import static org.junit.Assert.*;
import javax.persistence.EntityManager;
import javax.persistence.PersistenceContext;
import javax.persistence.Query;
import javax.persistence.criteria.CriteriaBuilder;
import javax.persistence.criteria.CriteriaQuery;
import javax.persistence.criteria.Path;
@@ -42,8 +43,7 @@ import org.springframework.test.context.junit4.SpringJUnit4ClassRunner;
@ContextConfiguration({ "classpath:infrastructure.xml" })
public class MetamodelIntegrationTests {
@PersistenceContext
EntityManager em;
@PersistenceContext EntityManager em;
@Test
public void considersOneToOneAttributeAnAssociation() {
@@ -66,4 +66,12 @@ public class MetamodelIntegrationTests {
assertThat(path.getModel().getBindableType(), is(BindableType.ENTITY_TYPE));
}
@Test
public void canAccessParametersByIndexForNativeQueries() {
Query query = em.createNativeQuery("SELECT u from User u where u.lastname = ?1");
assertThat(query.getParameter(1), is(notNullValue()));
}
}

View File

@@ -1,5 +1,5 @@
/*
* Copyright 2013 the original author or authors.
* Copyright 2013-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.
@@ -23,7 +23,10 @@ import org.springframework.data.jpa.repository.query.StringQuery.LikeParameterBi
import org.springframework.data.repository.query.parser.Part.Type;
/**
* Unit tests for {@link LikeParameterBinding}.
*
* @author Oliver Gierke
* @author Thomas Darimont
*/
public class LikeBindingUnitTests {

View File

@@ -124,10 +124,11 @@ public class StringQueryUnitTests {
assertThat(query.getQueryString(), is(queryString));
List<ParameterBinding> bindings = query.getParameterBindings();
assertThat(bindings, hasSize(2));
assertThat(bindings, hasSize(3));
assertNamedBinding(InParameterBinding.class, "ids", bindings.get(0));
assertNamedBinding(InParameterBinding.class, "names", bindings.get(1));
assertNamedBinding(ParameterBinding.class, "bar", bindings.get(2));
}
/**
@@ -161,10 +162,11 @@ public class StringQueryUnitTests {
assertThat(query.getQueryString(), is(queryString));
List<ParameterBinding> bindings = query.getParameterBindings();
assertThat(bindings, hasSize(2));
assertThat(bindings, hasSize(3));
assertPositionalBinding(InParameterBinding.class, 1, bindings.get(0));
assertPositionalBinding(InParameterBinding.class, 2, bindings.get(1));
assertPositionalBinding(ParameterBinding.class, 3, bindings.get(2));
}
/**