From 33216534bec6e6ecf25fb7e6e55c0ddac5d22ec3 Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Tue, 22 Oct 2013 15:03:32 +0200 Subject: [PATCH] DATAJPA-415 - Polishing. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Simplified implementation of ParameterBinder.convertToCollectionIfNecessary(…). Added a simpler test case for plain query execution and ignored that for EclipseLink and OpenJpa as it fails with both the EclipseLink and OpenJpa versions we currently rely on. See the ignored test cases for links to bug reports. Original pull request: #45. --- .../jpa/repository/query/ParameterBinder.java | 23 +++++---- ...lipseLinkNamespaceUserRepositoryTests.java | 8 ++++ .../OpenJpaNamespaceUserRepositoryTests.java | 14 ++++-- .../jpa/repository/UserRepositoryTests.java | 48 ++++++++++++------- .../jpa/repository/sample/UserRepository.java | 5 ++ 5 files changed, 65 insertions(+), 33 deletions(-) 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 607460be2..a1afa8c60 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 @@ -27,6 +27,7 @@ import org.springframework.data.domain.Sort; import org.springframework.data.jpa.repository.query.JpaParameters.JpaParameter; import org.springframework.data.repository.query.Parameters; import org.springframework.util.Assert; +import org.springframework.util.ObjectUtils; /** * {@link ParameterBinder} is used to bind method parameters to a {@link Query}. This is usually done whenever an @@ -141,23 +142,21 @@ public class ParameterBinder { } /** - * In order to avoid errors like: IllegalArgumentException: Encountered array-valued parameter binding, but was - * expecting [java.lang.Integer]. + * Returns the given value as collection if it is an array or as is if not. * - * @see DATAJPA-415 - * @throws Exception + * @return */ private Object convertArrayToCollectionIfNecessary(Object value) { - Object result = value; + if (!ObjectUtils.isArray(value)) { + return value; + } - if (result != null && result.getClass().isArray()) { - int len = Array.getLength(value); - Collection list = new ArrayList(len); - for (int i = 0; i < len; i++) { - list.add(Array.get(value, i)); - } - result = list; + int length = Array.getLength(value); + Collection result = new ArrayList(length); + + for (int i = 0; i < length; i++) { + result.add(Array.get(value, i)); } return result; diff --git a/src/test/java/org/springframework/data/jpa/repository/EclipseLinkNamespaceUserRepositoryTests.java b/src/test/java/org/springframework/data/jpa/repository/EclipseLinkNamespaceUserRepositoryTests.java index 7327b1dcf..213aba40f 100644 --- a/src/test/java/org/springframework/data/jpa/repository/EclipseLinkNamespaceUserRepositoryTests.java +++ b/src/test/java/org/springframework/data/jpa/repository/EclipseLinkNamespaceUserRepositoryTests.java @@ -63,4 +63,12 @@ public class EclipseLinkNamespaceUserRepositoryTests extends NamespaceUserReposi public void shouldGenerateLeftOuterJoinInfindAllWithPaginationAndSortOnNestedPropertyPath() { super.shouldGenerateLeftOuterJoinInfindAllWithPaginationAndSortOnNestedPropertyPath(); } + + /** + * Ignored until https://bugs.eclipse.org/bugs/show_bug.cgi?id=349477 is resolved. + */ + @Override + public void invokesQueryWithVarargsParametersCorrectly() { + + } } diff --git a/src/test/java/org/springframework/data/jpa/repository/OpenJpaNamespaceUserRepositoryTests.java b/src/test/java/org/springframework/data/jpa/repository/OpenJpaNamespaceUserRepositoryTests.java index c079aafed..92edaefb2 100644 --- a/src/test/java/org/springframework/data/jpa/repository/OpenJpaNamespaceUserRepositoryTests.java +++ b/src/test/java/org/springframework/data/jpa/repository/OpenJpaNamespaceUserRepositoryTests.java @@ -44,8 +44,7 @@ import org.springframework.test.context.ContextConfiguration; @ContextConfiguration("classpath:openjpa.xml") public class OpenJpaNamespaceUserRepositoryTests extends NamespaceUserRepositoryTests { - @PersistenceContext - EntityManager em; + @PersistenceContext EntityManager em; /** * Ignored until https://issues.apache.org/jira/browse/OPENJPA-2018 gets fixed. @@ -60,8 +59,7 @@ public class OpenJpaNamespaceUserRepositoryTests extends NamespaceUserRepository * Ignored until https://issues.apache.org/jira/browse/OPENJPA-2018 gets fixed. */ @Override - public void handlesIterableOfIdsCorrectly() { - } + public void handlesIterableOfIdsCorrectly() {} @Test public void checkQueryValidationWithOpenJpa() { @@ -105,4 +103,12 @@ public class OpenJpaNamespaceUserRepositoryTests extends NamespaceUserRepository List resultList = query.getResultList(); assertThat(resultList.size(), is(2)); } + + /** + * Ignored until https://issues.apache.org/jira/browse/OPENJPA-2018 gets fixed. + */ + @Override + public void invokesQueryWithVarargsParametersCorrectly() { + + } } 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 103177104..2f88e441d 100644 --- a/src/test/java/org/springframework/data/jpa/repository/UserRepositoryTests.java +++ b/src/test/java/org/springframework/data/jpa/repository/UserRepositoryTests.java @@ -68,7 +68,7 @@ import org.springframework.transaction.annotation.Transactional; * * @author Oliver Gierke * @author Kevin Raymond - * @author Thomas Darimont + * @author Thomas Darimont */ @RunWith(SpringJUnit4ClassRunner.class) @ContextConfiguration("classpath:application-context.xml") @@ -1106,22 +1106,36 @@ public class UserRepositoryTests { assertThat(result, is(firstUser)); } - /** - * @see DATAJPA-415 - */ - @Test - public void shouldSupportModifyingQueryWithVarArgs() { - - flushTestUsers(); - - repository.updateUserActiveState(false, firstUser.getId(), secondUser.getId(), thirdUser.getId(), - fourthUser.getId()); - - long expectedCount = repository.count(); - assertThat(repository.findByActiveFalse().size(), is((int) expectedCount)); - assertThat(repository.findByActiveTrue().size(), is((int) 0)); - } - + /** + * @see DATAJPA-415 + */ + @Test + public void invokesQueryWithVarargsParametersCorrectly() { + + flushTestUsers(); + + Collection result = repository.findByIdIn(firstUser.getId(), secondUser.getId()); + + assertThat(result, hasSize(2)); + assertThat(result, hasItems(firstUser, secondUser)); + } + + /** + * @see DATAJPA-415 + */ + @Test + public void shouldSupportModifyingQueryWithVarArgs() { + + flushTestUsers(); + + repository.updateUserActiveState(false, firstUser.getId(), secondUser.getId(), thirdUser.getId(), + fourthUser.getId()); + + long expectedCount = repository.count(); + assertThat(repository.findByActiveFalse().size(), is((int) expectedCount)); + assertThat(repository.findByActiveTrue().size(), is(0)); + } + private Page executeSpecWithSort(Sort sort) { flushTestUsers(); 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 9335eb450..74ef492a7 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 @@ -272,6 +272,11 @@ public interface UserRepository extends JpaRepository, JpaSpecifi @Query("select u.firstname from User u where u.lastname = ?1") List findFirstnamesByLastname(String lastname); + /** + * @see DATAJPA-415 + */ + Collection findByIdIn(@Param("ids") Integer... ids); + /** * @see DATAJPA-415 */