From 49a3b56481bfcdd0bbf853fce3151108ad296bdf Mon Sep 17 00:00:00 2001
From: "Greg L. Turnquist" <gturnquist@vmware.com>
Date: Wed, 13 Apr 2022 11:01:19 -0500
Subject: [PATCH] Convert Iterable<ID> to Collection<ID> for
 deleteAllByIdInBatch.

JpaRepository accepts Iterable<ID> for bulk deletes. But some JPA providers require Collection<ID> instead. To avoid breaking any APIs, convert the incoming argument if it's not already a Collection.

See #2242.
---
 .../support/SimpleJpaRepository.java          | 13 ++++++-
 .../EclipseLinkJpaRepositoryTests.java        | 13 ++++---
 .../support/JpaRepositoryTests.java           | 34 ++++++++++++++++++-
 3 files changed, 54 insertions(+), 6 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 367ee9f47..8d804ee69 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
@@ -25,6 +25,8 @@ 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;
@@ -239,7 +241,16 @@ public class SimpleJpaRepository<T, ID> implements JpaRepositoryImplementation<T
 					entityInformation.getIdAttribute().getName());
 
 			Query query = em.createQuery(queryString);
-			query.setParameter("ids", ids);
+			/**
+			 * Some JPA providers require {@code ids} to be a {@link Collection} so we must convert if it's not already.
+			 */
+			if (Collection.class.isInstance(ids)) {
+				query.setParameter("ids", ids);
+			} else {
+				Collection<ID> idsCollection = StreamSupport.stream(ids.spliterator(), false)
+						.collect(Collectors.toCollection(ArrayList::new));
+				query.setParameter("ids", idsCollection);
+			}
 
 			query.executeUpdate();
 		}
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 cfc2cb958..ab7f6a5e0 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" })
@@ -128,6 +131,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<SampleEntityPK> ids = new Iterable<SampleEntityPK>() {
+
+			private List<SampleEntityPK> ids = Arrays.asList(new SampleEntityPK("one", "eins"),
+					new SampleEntityPK("three", "drei"));
+
+			@NotNull
+			@Override
+			public Iterator<SampleEntityPK> iterator() {
+				return ids.iterator();
+			}
+		};
+
+		repository.deleteAllByIdInBatch(ids);
+		assertThat(repository.findAll()).containsExactly(two);
+	}
+
 	private interface SampleEntityRepository extends JpaRepository<SampleEntity, SampleEntityPK> {
 
 	}