From e62cafb19097674bbac8f8caabb5be4d02fb34f0 Mon Sep 17 00:00:00 2001 From: "Greg L. Turnquist" Date: Wed, 13 Apr 2022 11:01:19 -0500 Subject: [PATCH] Convert Iterable to Collection for deleteAllByIdInBatch. JpaRepository accepts Iterable for bulk deletes. But some JPA providers require Collection instead. To avoid breaking any APIs, convert the incoming argument if it's not already a Collection. See #2242. --- .../support/SimpleJpaRepository.java | 64 +++++++++++-------- .../EclipseLinkJpaRepositoryTests.java | 13 ++-- .../support/JpaRepositoryTests.java | 34 +++++++++- 3 files changed, 79 insertions(+), 32 deletions(-) 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 acbab99f4..ee1a78eda 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 @@ -15,6 +15,32 @@ */ 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.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.function.Function; +import java.util.stream.Collectors; +import java.util.stream.StreamSupport; + +import javax.persistence.EntityManager; +import javax.persistence.LockModeType; +import javax.persistence.NoResultException; +import javax.persistence.Parameter; +import javax.persistence.Query; +import javax.persistence.TypedQuery; +import javax.persistence.criteria.CriteriaBuilder; +import javax.persistence.criteria.CriteriaQuery; +import javax.persistence.criteria.ParameterExpression; +import javax.persistence.criteria.Path; +import javax.persistence.criteria.Predicate; +import javax.persistence.criteria.Root; + import org.springframework.dao.EmptyResultDataAccessException; import org.springframework.data.domain.Example; import org.springframework.data.domain.Page; @@ -37,29 +63,6 @@ import org.springframework.stereotype.Repository; import org.springframework.transaction.annotation.Transactional; import org.springframework.util.Assert; -import javax.persistence.EntityManager; -import javax.persistence.LockModeType; -import javax.persistence.NoResultException; -import javax.persistence.Parameter; -import javax.persistence.Query; -import javax.persistence.TypedQuery; -import javax.persistence.criteria.CriteriaBuilder; -import javax.persistence.criteria.CriteriaQuery; -import javax.persistence.criteria.ParameterExpression; -import javax.persistence.criteria.Path; -import javax.persistence.criteria.Predicate; -import javax.persistence.criteria.Root; -import java.util.ArrayList; -import java.util.Collection; -import java.util.Collections; -import java.util.HashMap; -import java.util.List; -import java.util.Map; -import java.util.Optional; -import java.util.function.Function; - -import static org.springframework.data.jpa.repository.query.QueryUtils.*; - /** * Default implementation of the {@link org.springframework.data.repository.CrudRepository} interface. This will offer * you a more sophisticated interface than the plain {@link EntityManager} . @@ -237,7 +240,16 @@ public class SimpleJpaRepository implements JpaRepositoryImplementation idsCollection = StreamSupport.stream(ids.spliterator(), false) + .collect(Collectors.toCollection(ArrayList::new)); + query.setParameter("ids", idsCollection); + } query.executeUpdate(); } @@ -325,7 +337,6 @@ public class SimpleJpaRepository implements JpaRepositoryImplementation implements JpaRepositoryImplementation List findAll(Example example, Sort sort) { - return getQuery(new ExampleSpecification<>(example, escapeCharacter), example.getProbeType(), sort) - .getResultList(); + return getQuery(new ExampleSpecification<>(example, escapeCharacter), example.getProbeType(), sort).getResultList(); } /* diff --git a/src/test/java/org/springframework/data/jpa/repository/support/EclipseLinkJpaRepositoryTests.java b/src/test/java/org/springframework/data/jpa/repository/support/EclipseLinkJpaRepositoryTests.java index 2574770e7..bb16ec76c 100644 --- a/src/test/java/org/springframework/data/jpa/repository/support/EclipseLinkJpaRepositoryTests.java +++ b/src/test/java/org/springframework/data/jpa/repository/support/EclipseLinkJpaRepositoryTests.java @@ -22,15 +22,20 @@ import org.springframework.test.context.ContextConfiguration; * Integration tests to execute {@link JpaRepositoryTests} against EclipseLink. * * @author Oliver Gierke + * @author Greg Turnquist */ @ContextConfiguration("classpath:eclipselink.xml") class EclipseLinkJpaRepositoryTests extends JpaRepositoryTests { @Override - /** - * Ignored until https://bugs.eclipse.org/bugs/show_bug.cgi?id=349477 is resolved. - */ + @Disabled("https://bugs.eclipse.org/bugs/show_bug.cgi?id=349477") void deleteAllByIdInBatch() { - super.deleteAllByIdInBatch(); + // disabled + } + + @Override + @Disabled("https://bugs.eclipse.org/bugs/show_bug.cgi?id=349477") + void deleteAllByIdInBatchShouldConvertAnIterableToACollection() { + // disabled } } diff --git a/src/test/java/org/springframework/data/jpa/repository/support/JpaRepositoryTests.java b/src/test/java/org/springframework/data/jpa/repository/support/JpaRepositoryTests.java index 036267e6b..cecfc60d1 100644 --- a/src/test/java/org/springframework/data/jpa/repository/support/JpaRepositoryTests.java +++ b/src/test/java/org/springframework/data/jpa/repository/support/JpaRepositoryTests.java @@ -18,15 +18,17 @@ package org.springframework.data.jpa.repository.support; import static org.assertj.core.api.Assertions.*; import java.util.Arrays; +import java.util.Iterator; +import java.util.List; import java.util.Optional; import javax.persistence.EntityManager; import javax.persistence.PersistenceContext; +import org.jetbrains.annotations.NotNull; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; - import org.springframework.data.jpa.domain.sample.PersistableWithIdClass; import org.springframework.data.jpa.domain.sample.PersistableWithIdClassPK; import org.springframework.data.jpa.domain.sample.SampleEntity; @@ -43,6 +45,7 @@ import org.springframework.transaction.annotation.Transactional; * @author Oliver Gierke * @author Thomas Darimont * @author Jens Schauder + * @author Greg Turnquist */ @ExtendWith(SpringExtension.class) @ContextConfiguration({ "classpath:infrastructure.xml" }) @@ -127,6 +130,35 @@ class JpaRepositoryTests { assertThat(repository.findAll()).containsExactly(two); } + @Test // GH-2242 + void deleteAllByIdInBatchShouldConvertAnIterableToACollection() { + + SampleEntity one = new SampleEntity("one", "eins"); + SampleEntity two = new SampleEntity("two", "zwei"); + SampleEntity three = new SampleEntity("three", "drei"); + repository.saveAll(Arrays.asList(one, two, three)); + repository.flush(); + + /** + * Wrap a {@link List} inside an {@link Iterable} to verify that {@link SimpleJpaRepository} can properly convert a + * pure {@link Iterable} to a {@link Collection}. + **/ + Iterable ids = new Iterable() { + + private List ids = Arrays.asList(new SampleEntityPK("one", "eins"), + new SampleEntityPK("three", "drei")); + + @NotNull + @Override + public Iterator iterator() { + return ids.iterator(); + } + }; + + repository.deleteAllByIdInBatch(ids); + assertThat(repository.findAll()).containsExactly(two); + } + private interface SampleEntityRepository extends JpaRepository { }