DATAJPA-415 - Polishing.

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.

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<Object> list = new ArrayList<Object>(len);
-			for (int i = 0; i < len; i++) {
-				list.add(Array.get(value, i));
-			}
-			result = list;
+		int length = Array.getLength(value);
+		Collection<Object> result = new ArrayList<Object>(length);
+
+		for (int i = 0; i < length; i++) {
+			result.add(Array.get(value, i));
 		}
 
 		return result;

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() {
+
+	}
 }

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<User> resultList = query.getResultList();
 		assertThat(resultList.size(), is(2));
 	}
+
+	/**
+	 * Ignored until https://issues.apache.org/jira/browse/OPENJPA-2018 gets fixed.
+	 */
+	@Override
+	public void invokesQueryWithVarargsParametersCorrectly() {
+
+	}
 }

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<User> 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<User> executeSpecWithSort(Sort sort) {
 
 		flushTestUsers();

src/test/java/org/springframework/data/jpa/repository/sample/UserRepository.java

@@ -272,6 +272,11 @@ public interface UserRepository extends JpaRepository<User, Integer>, JpaSpecifi
 	@Query("select u.firstname from User u where u.lastname = ?1")
 	List<String> findFirstnamesByLastname(String lastname);
 
+	/**
+	 * @see DATAJPA-415
+	 */
+	Collection<User> findByIdIn(@Param("ids") Integer... ids);
+
 	/**
 	 * @see DATAJPA-415
 	 */