From 48e947dffbc145960ae7e4669892aa115c9f1804 Mon Sep 17 00:00:00 2001 From: Moritz Becker Date: Sat, 26 Nov 2016 10:26:31 +0100 Subject: [PATCH] DATAJPA-433 - Fix IN expressions for non empty collections in EclipseLink. We now convert parameter to collections and use `ParameterExpression>` to declare the parameter. This utilises the fix for https://bugs.eclipse.org/bugs/show_bug.cgi?id=349477. It does break empty collections in EclipseLink which so far worked in 2.6 but didn't work 2.7 but support for only empty collection isn't of much use. Original pull request: #185. --- .../jpa/repository/query/JpaQueryCreator.java | 7 +- .../support/SimpleJpaRepository.java | 14 +++- ...lipseLinkNamespaceUserRepositoryTests.java | 73 +++---------------- 3 files changed, 26 insertions(+), 68 deletions(-) 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 e5bb9cd55..58d9cba73 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 @@ -53,6 +53,7 @@ import org.springframework.util.Assert; * @author Michael Cramer * @author Mark Paluch * @author Reda.Housni-Alaoui + * @author Moritz Becker */ public class JpaQueryCreator extends AbstractQueryCreator, Predicate> { @@ -268,9 +269,11 @@ public class JpaQueryCreator extends AbstractQueryCreator>) provider.next(part, Collection.class).getExpression()).not(); case IN: - return getTypedPath(root, part).in(provider.next(part, Collection.class).getExpression()); + // cast required for eclipselink workaround, see DATAJPA-433 + return getTypedPath(root, part).in((Expression>) provider.next(part, Collection.class).getExpression()); case STARTING_WITH: case ENDING_WITH: case CONTAINING: diff --git a/src/main/java/org/springframework/data/jpa/repository/support/SimpleJpaRepository.java b/src/main/java/org/springframework/data/jpa/repository/support/SimpleJpaRepository.java index 3081d6d87..708226336 100644 --- a/src/main/java/org/springframework/data/jpa/repository/support/SimpleJpaRepository.java +++ b/src/main/java/org/springframework/data/jpa/repository/support/SimpleJpaRepository.java @@ -18,6 +18,7 @@ package org.springframework.data.jpa.repository.support; import static org.springframework.data.jpa.repository.query.QueryUtils.*; import java.util.ArrayList; +import java.util.Collection; import java.util.Collections; import java.util.List; import java.util.Map; @@ -69,6 +70,7 @@ import org.springframework.util.Assert; * @author Stefan Fussenegger * @author Jens Schauder * @author David Madden + * @author Moritz Becker * @param the type of the entity to handle * @param the type of the entity's identifier */ @@ -347,10 +349,16 @@ public class SimpleJpaRepository implements JpaRepositoryImplementation idCollection = new ArrayList(); + for (ID id : ids) { + idCollection.add(id); + } + ByIdsSpecification specification = new ByIdsSpecification(entityInformation); TypedQuery query = getQuery(specification, Sort.unsorted()); - return query.setParameter(specification.parameter, ids).getResultList(); + return query.setParameter(specification.parameter, idCollection).getResultList(); } /* @@ -778,7 +786,7 @@ public class SimpleJpaRepository implements JpaRepositoryImplementation entityInformation; - @Nullable ParameterExpression parameter; + @Nullable ParameterExpression> parameter; ByIdsSpecification(JpaEntityInformation entityInformation) { this.entityInformation = entityInformation; @@ -791,7 +799,7 @@ public class SimpleJpaRepository implements JpaRepositoryImplementation root, CriteriaQuery query, CriteriaBuilder cb) { Path path = root.get(entityInformation.getIdAttribute()); - parameter = cb.parameter(Iterable.class); + parameter = (ParameterExpression>) (ParameterExpression) cb.parameter(Collection.class); return path.in(parameter); } } 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 074ff3427..755133b78 100644 --- a/src/test/java/org/springframework/data/jpa/repository/EclipseLinkNamespaceUserRepositoryTests.java +++ b/src/test/java/org/springframework/data/jpa/repository/EclipseLinkNamespaceUserRepositoryTests.java @@ -33,37 +33,17 @@ import org.springframework.test.context.ContextConfiguration; * @author Oliver Gierke * @author Thomas Darimont * @author Jens Schauder + * @author Moritz Becker */ @ContextConfiguration(value = "classpath:eclipselink.xml") public class EclipseLinkNamespaceUserRepositoryTests extends NamespaceUserRepositoryTests { - /** - * Ignored until https://bugs.eclipse.org/bugs/show_bug.cgi?id=349477 is resolved. - */ - @Override - public void findsAllByGivenIds() { - - } - - /** - * Ignored until https://bugs.eclipse.org/bugs/show_bug.cgi?id=349477 is resolved. - */ - @Override - public void handlesIterableOfIdsCorrectly() { - - } - - /** - * Ignored until https://bugs.eclipse.org/bugs/show_bug.cgi?id=349477 is resolved. - */ - @Override - public void invokesQueryWithVarargsParametersCorrectly() {} - /** * Ignored until https://bugs.eclipse.org/bugs/show_bug.cgi?id=422450 is resolved. */ @Override - public void sortByAssociationPropertyShouldUseLeftOuterJoin() {} + public void sortByAssociationPropertyShouldUseLeftOuterJoin() { + } /** * Ignored until https://bugs.eclipse.org/bugs/show_bug.cgi?id=422450 is resolved. @@ -123,56 +103,29 @@ public class EclipseLinkNamespaceUserRepositoryTests extends NamespaceUserReposi public void bindsNativeQueryResultsToProjectionByName() {} /** - * Ignores the test for EclipseLink 2.7.2. Reconsider once https://bugs.eclipse.org/bugs/show_bug.cgi?id=533240 is - * fixed. + * Ignores the test. Reconsider once https://bugs.eclipse.org/bugs/show_bug.cgi?id=533240 is fixed. */ @Override - @Test // DATAJPA-1314 - public void findByEmptyArrayOfIntegers() throws Exception { - - assumeNotEclipseLink2_7_2plus(); - - super.findByEmptyArrayOfIntegers(); - } + public void findByEmptyArrayOfIntegers() throws Exception {} /** - * Ignores the test for EclipseLink 2.7.2. Reconsider once https://bugs.eclipse.org/bugs/show_bug.cgi?id=533240 is - * fixed. + * Ignores the test. Reconsider once https://bugs.eclipse.org/bugs/show_bug.cgi?id=533240 is fixed. */ @Override - @Test // DATAJPA-1314 public void findByAgeWithEmptyArrayOfIntegersOrFirstName() { - - assumeNotEclipseLink2_7_2plus(); - - super.findByAgeWithEmptyArrayOfIntegersOrFirstName(); } /** - * Ignores the test for EclipseLink 2.7.2. Reconsider once https://bugs.eclipse.org/bugs/show_bug.cgi?id=533240 is - * fixed. + * Ignores the test. Reconsider once https://bugs.eclipse.org/bugs/show_bug.cgi?id=533240 is fixed. */ @Override - @Test // DATAJPA-1314 - public void findByEmptyCollectionOfIntegers() throws Exception { - - assumeNotEclipseLink2_7_2plus(); - - super.findByEmptyCollectionOfIntegers(); - } + public void findByEmptyCollectionOfIntegers() throws Exception {} /** - * Ignores the test for EclipseLink 2.7.2. Reconsider once https://bugs.eclipse.org/bugs/show_bug.cgi?id=533240 is - * fixed. + * Ignores the test. Reconsider once https://bugs.eclipse.org/bugs/show_bug.cgi?id=533240 is fixed. */ @Override - @Test // DATAJPA-1314 - public void findByEmptyCollectionOfStrings() throws Exception { - - assumeNotEclipseLink2_7_2plus(); - - super.findByEmptyCollectionOfStrings(); - } + public void findByEmptyCollectionOfStrings() throws Exception {} /** * Ignores the test for EclipseLink. @@ -181,10 +134,4 @@ public class EclipseLinkNamespaceUserRepositoryTests extends NamespaceUserReposi @Test @Ignore public void savingUserThrowsAnException() {} - - private void assumeNotEclipseLink2_7_2plus() { - - Assume.assumeFalse("Empty collections seem to be broken in EclipseLink 2.7.2+", - Version.parse(getVersion()).isGreaterThanOrEqualTo(new Version(2, 7, 2))); - } }