From 14a69eac4344f535449df7f0a42e5368d1c23870 Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Tue, 22 Feb 2011 21:51:09 +0100 Subject: [PATCH] DATAJPA-29, DATAJPA-30 - Fixed invalid page metadata in pagination queries. JpaCountQueryCreator now correctly applies the predicate. Added test cases and support for 'In' keywords. --- .../query/JpaCountQueryCreator.java | 2 +- .../jpa/repository/query/JpaQueryCreator.java | 19 +++++ .../jpa/repository/query/JpaQueryPart.java | 83 ------------------- .../jpa/repository/query/ParameterBinder.java | 3 +- .../repository/UserRepositoryFinderTests.java | 51 +++++++----- .../query/JpaQueryMethodUnitTests.java | 2 +- .../jpa/repository/sample/UserRepository.java | 7 +- 7 files changed, 59 insertions(+), 108 deletions(-) delete mode 100644 src/main/java/org/springframework/data/jpa/repository/query/JpaQueryPart.java diff --git a/src/main/java/org/springframework/data/jpa/repository/query/JpaCountQueryCreator.java b/src/main/java/org/springframework/data/jpa/repository/query/JpaCountQueryCreator.java index 9ab545177..e8fe1cd22 100644 --- a/src/main/java/org/springframework/data/jpa/repository/query/JpaCountQueryCreator.java +++ b/src/main/java/org/springframework/data/jpa/repository/query/JpaCountQueryCreator.java @@ -63,6 +63,6 @@ public class JpaCountQueryCreator extends JpaQueryCreator { protected CriteriaQuery complete(Predicate predicate, Sort sort, CriteriaQuery query, CriteriaBuilder builder, Root root) { - return query.select(builder.count(root)); + return query.select(builder.count(root)).where(predicate); } } diff --git a/src/main/java/org/springframework/data/jpa/repository/query/JpaQueryCreator.java b/src/main/java/org/springframework/data/jpa/repository/query/JpaQueryCreator.java index 1d5fea671..d3962bc15 100644 --- a/src/main/java/org/springframework/data/jpa/repository/query/JpaQueryCreator.java +++ b/src/main/java/org/springframework/data/jpa/repository/query/JpaQueryCreator.java @@ -15,6 +15,8 @@ */ package org.springframework.data.jpa.repository.query; +import java.util.Arrays; +import java.util.Collection; import java.util.Iterator; import javax.persistence.EntityManager; @@ -34,6 +36,7 @@ import org.springframework.data.repository.query.parser.Part; import org.springframework.data.repository.query.parser.PartTree; import org.springframework.data.repository.query.parser.Property; import org.springframework.util.Assert; +import org.springframework.util.CollectionUtils; /** @@ -175,6 +178,8 @@ public class JpaQueryCreator extends return path.isNull(); case IS_NOT_NULL: return path.isNotNull(); + case IN: + return path.in(nextAsCollection(iterator)); case LIKE: return builder.like(root. get(part.getProperty() .toDotPath()), iterator.next().toString()); @@ -248,4 +253,18 @@ public class JpaQueryCreator extends return (Comparable) next; } + + + private Collection nextAsCollection(Iterator iterator) { + + Object next = iterator.next(); + + if (next instanceof Collection) { + return (Collection) next; + } else if (next.getClass().isArray()) { + return CollectionUtils.arrayToList(next); + } + + return Arrays.asList(next); + } } diff --git a/src/main/java/org/springframework/data/jpa/repository/query/JpaQueryPart.java b/src/main/java/org/springframework/data/jpa/repository/query/JpaQueryPart.java deleted file mode 100644 index 35f95c99c..000000000 --- a/src/main/java/org/springframework/data/jpa/repository/query/JpaQueryPart.java +++ /dev/null @@ -1,83 +0,0 @@ -/* - * Copyright 2008-2011 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. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package org.springframework.data.jpa.repository.query; - -import org.springframework.data.repository.query.Parameter; -import org.springframework.data.repository.query.parser.Part; -import org.springframework.util.Assert; - - -/** - * JPA specific {@link Part}. Allows creating JPQL snippets via - * {@link #createQueryPart(Type, String, Parameter)}. - * - * @author Oliver Gierke - */ -class JpaQueryPart extends Part { - - /** - * @param part - * @param method - * @param parameter - */ - public JpaQueryPart(String part, Class method) { - - super(part, method); - } - - - /** - * Returns the query part. - * - * @return - */ - public String getQueryPart(Parameter parameter) { - - return createQueryPart(getType(), getProperty().toDotPath(), parameter); - } - - - /** - * Create the actual query part for the given property. Creates a simple - * assignment of the following shape by default. {@code x.$ - * property} ${operator} ${parameterPlaceholder}}. - * - * @param property the actual clean property - * @param parameters - * @param index - * @return - */ - private String createQueryPart(Type type, String property, - Parameter parameter) { - - switch (type) { - case BETWEEN: - String first = parameter.getPlaceholder(); - String second = parameter.getNext().getPlaceholder(); - - return String.format("x.%s between %s and %s", property, first, - second); - case IS_NOT_NULL: - return String.format("x.%s is not null", property); - case IS_NULL: - return String.format("x.%s is null", property); - default: - Assert.notNull(parameter); - return String.format("x.%s %s %s", property, type.getOperator(), - parameter.getPlaceholder()); - } - } -} diff --git a/src/main/java/org/springframework/data/jpa/repository/query/ParameterBinder.java b/src/main/java/org/springframework/data/jpa/repository/query/ParameterBinder.java index fccb62bd3..33503d5f3 100644 --- a/src/main/java/org/springframework/data/jpa/repository/query/ParameterBinder.java +++ b/src/main/java/org/springframework/data/jpa/repository/query/ParameterBinder.java @@ -108,6 +108,7 @@ public class ParameterBinder { public Query bind(Query query) { int methodParameterPosition = 0; + int queryParameterPosition = 1; for (Parameter parameter : parameters) { @@ -118,7 +119,7 @@ public class ParameterBinder { if (hasNamedParameter(query) && parameter.isNamedParameter()) { query.setParameter(parameter.getName(), value); } else { - query.setParameter(parameter.getIndex() + 1, value); + query.setParameter(queryParameterPosition++, value); } } diff --git a/src/test/java/org/springframework/data/jpa/repository/UserRepositoryFinderTests.java b/src/test/java/org/springframework/data/jpa/repository/UserRepositoryFinderTests.java index 93d3bdcce..1a997e095 100644 --- a/src/test/java/org/springframework/data/jpa/repository/UserRepositoryFinderTests.java +++ b/src/test/java/org/springframework/data/jpa/repository/UserRepositoryFinderTests.java @@ -49,27 +49,22 @@ public class UserRepositoryFinderTests { @Autowired UserRepository userRepository; - User firstUser, secondUser; + User dave, carter, oliver; @Before public void setUp() { // This one matches both criterias - firstUser = new User(); - firstUser.setEmailAddress("foo"); - firstUser.setLastname("bar"); - firstUser.setFirstname("foobar"); - - userRepository.save(firstUser); + dave = new User("Dave", "Matthews", "dave@dmband.com"); + userRepository.save(dave); // This one matches only the second one - secondUser = new User(); - secondUser.setEmailAddress("bar"); - secondUser.setLastname("foo"); - secondUser.setFirstname("foobar"); + carter = new User("Carter", "Beauford", "carter@dmband.com"); + userRepository.save(carter); - userRepository.save(secondUser); + oliver = new User("Oliver August", "Matthews", "oliver@dmband.com"); + userRepository.save(oliver); } @@ -79,8 +74,10 @@ public class UserRepositoryFinderTests { @Test public void testSimpleCustomCreatedFinder() { - User user = userRepository.findByEmailAddressAndLastname("foo", "bar"); - assertEquals(firstUser, user); + User user = + userRepository.findByEmailAddressAndLastname("dave@dmband.com", + "Matthews"); + assertEquals(dave, user); } @@ -104,13 +101,13 @@ public class UserRepositoryFinderTests { public void testAndOrFinder() { List users = - userRepository.findByEmailAddressAndLastnameOrFirstname("bar", - "foo", "foobar"); + userRepository.findByEmailAddressAndLastnameOrFirstname( + "dave@dmband.com", "Matthews", "Carter"); assertNotNull(users); assertEquals(2, users.size()); - assertTrue(users.contains(firstUser)); - assertTrue(users.contains(secondUser)); + assertTrue(users.contains(dave)); + assertTrue(users.contains(carter)); } @@ -118,7 +115,8 @@ public class UserRepositoryFinderTests { public void executesPagingMethodToPageCorrectly() throws Exception { Page page = - userRepository.findByFirstname(new PageRequest(0, 1), "foobar"); + userRepository + .findByLastname(new PageRequest(0, 1), "Matthews"); assertThat(page.getNumberOfElements(), is(1)); assertThat(page.getTotalElements(), is(2L)); assertThat(page.getTotalPages(), is(2)); @@ -129,7 +127,20 @@ public class UserRepositoryFinderTests { public void executesPagingMethodToListCorrectly() throws Exception { List list = - userRepository.findByFirstname("foobar", new PageRequest(0, 1)); + userRepository.findByFirstname("Carter", new PageRequest(0, 1)); assertThat(list.size(), is(1)); } + + + @Test + public void testname() throws Exception { + + Page page = + userRepository.findByFirstnameIn(new PageRequest(0, 1), "Dave", + "Oliver August"); + + assertThat(page.getNumberOfElements(), is(1)); + assertThat(page.getTotalElements(), is(2L)); + assertThat(page.getTotalPages(), is(2)); + } } diff --git a/src/test/java/org/springframework/data/jpa/repository/query/JpaQueryMethodUnitTests.java b/src/test/java/org/springframework/data/jpa/repository/query/JpaQueryMethodUnitTests.java index 4840c1583..0106e8d1a 100644 --- a/src/test/java/org/springframework/data/jpa/repository/query/JpaQueryMethodUnitTests.java +++ b/src/test/java/org/springframework/data/jpa/repository/query/JpaQueryMethodUnitTests.java @@ -198,7 +198,7 @@ public class JpaQueryMethodUnitTests { throws Exception { Method method = - UserRepository.class.getMethod("findByFirstname", + UserRepository.class.getMethod("findByLastname", Pageable.class, String.class); when(extractor.canExtractQuery()).thenReturn(false); 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 eb1845ce0..e8aacf9f3 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 @@ -115,10 +115,10 @@ public interface UserRepository extends JpaRepository, * parameter to be regarded on query execution. * * @param pageable - * @param firstname + * @param lastname * @return */ - Page findByFirstname(Pageable pageable, String firstname); + Page findByLastname(Pageable pageable, String lastname); /** @@ -133,6 +133,9 @@ public interface UserRepository extends JpaRepository, List findByFirstname(String firstname, Pageable pageable); + Page findByFirstnameIn(Pageable pageable, String... firstnames); + + /** * Manipulating query to set all {@link User}'s names to the given one. *