From 4e0161151689fd9b90efacf375a4a8e16810089f Mon Sep 17 00:00:00 2001 From: Jens Schauder Date: Wed, 11 Nov 2020 07:09:55 +0100 Subject: [PATCH 1/5] DATAJPA-1818 - Prepare branch --- pom.xml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pom.xml b/pom.xml index d5f7e25385..00f3470b73 100644 --- a/pom.xml +++ b/pom.xml @@ -5,7 +5,7 @@ org.springframework.data spring-data-jpa - 2.5.0-SNAPSHOT + 2.5.0-DATAJPA-1818-SNAPSHOT Spring Data JPA Spring Data module for JPA repositories. @@ -25,7 +25,7 @@ 5.4.8.Final 2.19.1 org.hibernate - 2.5.0-SNAPSHOT + 2.4.0-DATACMNS-800-SNAPSHOT spring.data.jpa From 12c6a8deafa61d8e43a502e51cfa9850380a8ecb Mon Sep 17 00:00:00 2001 From: Jens Schauder Date: Wed, 11 Nov 2020 07:20:08 +0100 Subject: [PATCH 2/5] DATAJPA-1818 - Implements CrudRepository.delete(Iterable ids). --- .../support/SimpleJpaRepository.java | 10 +++++ .../jpa/repository/UserRepositoryTests.java | 43 +++++++++++++------ 2 files changed, 39 insertions(+), 14 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 f9181959b5..a416ee7e8a 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 @@ -218,6 +218,16 @@ public void deleteAll(Iterable entities) { } } + @Override + public void deleteAllById(Iterable ids) { + + Assert.notNull(ids, "Ids must not be null!"); + + for (ID id : ids) { + deleteById(id); + } + } + /* * (non-Javadoc) * @see org.springframework.data.jpa.repository.JpaRepository#deleteInBatch(java.lang.Iterable) 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 dedd3d546e..d2ff181402 100644 --- a/src/test/java/org/springframework/data/jpa/repository/UserRepositoryTests.java +++ b/src/test/java/org/springframework/data/jpa/repository/UserRepositoryTests.java @@ -15,6 +15,7 @@ */ package org.springframework.data.jpa.repository; +import static java.util.Arrays.*; import static org.assertj.core.api.Assertions.*; import static org.springframework.data.domain.Example.*; import static org.springframework.data.domain.ExampleMatcher.*; @@ -151,7 +152,7 @@ void findsAllByGivenIds() { flushTestUsers(); - assertThat(repository.findAllById(Arrays.asList(firstUser.getId(), secondUser.getId()))).contains(firstUser, + assertThat(repository.findAllById(asList(firstUser.getId(), secondUser.getId()))).contains(firstUser, secondUser); } @@ -166,7 +167,7 @@ void testReadByIdReturnsNullForNotFoundEntities() { @Test void savesCollectionCorrectly() throws Exception { - assertThat(repository.saveAll(Arrays.asList(firstUser, secondUser, thirdUser))).hasSize(3).contains(firstUser, + assertThat(repository.saveAll(asList(firstUser, secondUser, thirdUser))).hasSize(3).contains(firstUser, secondUser, thirdUser); } @@ -238,13 +239,13 @@ void returnsAllIgnoreCaseSortedCorrectly() throws Exception { } @Test - void deleteColletionOfEntities() { + void deleteCollectionOfEntities() { flushTestUsers(); long before = repository.count(); - repository.deleteAll(Arrays.asList(firstUser, secondUser)); + repository.deleteAll(asList(firstUser, secondUser)); assertThat(repository.existsById(firstUser.getId())).isFalse(); assertThat(repository.existsById(secondUser.getId())).isFalse(); @@ -252,13 +253,27 @@ void deleteColletionOfEntities() { } @Test - void batchDeleteColletionOfEntities() { + void batchDeleteCollectionOfEntities() { flushTestUsers(); long before = repository.count(); - repository.deleteInBatch(Arrays.asList(firstUser, secondUser)); + repository.deleteInBatch(asList(firstUser, secondUser)); + + assertThat(repository.existsById(firstUser.getId())).isFalse(); + assertThat(repository.existsById(secondUser.getId())).isFalse(); + assertThat(repository.count()).isEqualTo(before - 2); + } + + @Test // DATAJPA-1818 + void deleteCollectionOfEntitiesById() { + + flushTestUsers(); + + long before = repository.count(); + + repository.deleteAllById(asList(firstUser.getId(), secondUser.getId())); assertThat(repository.existsById(firstUser.getId())).isFalse(); assertThat(repository.existsById(secondUser.getId())).isFalse(); @@ -603,7 +618,7 @@ void executesQueryMethodWithDeepTraversalCorrectly() throws Exception { firstUser.setManager(secondUser); thirdUser.setManager(firstUser); - repository.saveAll(Arrays.asList(firstUser, thirdUser)); + repository.saveAll(asList(firstUser, thirdUser)); assertThat(repository.findByManagerLastname("Arrasz")).containsOnly(firstUser); assertThat(repository.findByManagerLastname("Gierke")).containsOnly(thirdUser); @@ -616,7 +631,7 @@ void executesFindByColleaguesLastnameCorrectly() throws Exception { firstUser.addColleague(secondUser); thirdUser.addColleague(firstUser); - repository.saveAll(Arrays.asList(firstUser, thirdUser)); + repository.saveAll(asList(firstUser, thirdUser)); assertThat(repository.findByColleaguesLastname(secondUser.getLastname())).containsOnly(firstUser); @@ -1178,7 +1193,7 @@ void findByElementCollectionAttribute() { flushTestUsers(); - List result = repository.findByAttributesIn(new HashSet<>(Arrays.asList("cool", "hip"))); + List result = repository.findByAttributesIn(new HashSet<>(asList("cool", "hip"))); assertThat(result).containsOnly(firstUser, secondUser); } @@ -1731,7 +1746,7 @@ void findAllByExampleWithAssociation() { firstUser.setManager(secondUser); thirdUser.setManager(firstUser); - repository.saveAll(Arrays.asList(firstUser, thirdUser)); + repository.saveAll(asList(firstUser, thirdUser)); User manager = new User(); manager.setLastname("Arrasz"); @@ -1854,7 +1869,7 @@ void findAllByExampleWithIncludeNull() { fifthUser.setFirstname(firstUser.getFirstname()); fifthUser.setLastname(firstUser.getLastname()); - repository.saveAll(Arrays.asList(firstUser, fifthUser)); + repository.saveAll(asList(firstUser, fifthUser)); User prototype = new User(); prototype.setFirstname(firstUser.getFirstname()); @@ -2242,7 +2257,7 @@ void findByElementCollectionInAttributeIgnoreCase() { flushTestUsers(); - List result = repository.findByAttributesIgnoreCaseIn(new HashSet<>(Arrays.asList("cOOl", "hIP"))); + List result = repository.findByAttributesIgnoreCaseIn(new HashSet<>(asList("cOOl", "hIP"))); assertThat(result).containsOnly(firstUser, secondUser); } @@ -2256,7 +2271,7 @@ void findByElementCollectionNotInAttributeIgnoreCase() { flushTestUsers(); - List result = repository.findByAttributesIgnoreCaseNotIn(Arrays.asList("CooL", "HIp")); + List result = repository.findByAttributesIgnoreCaseNotIn(asList("CooL", "HIp")); assertThat(result).containsOnly(thirdUser); } @@ -2284,7 +2299,7 @@ void findByElementCollectionInAttributeIgnoreCaseWithNulls() { flushTestUsers(); - List result = repository.findByAttributesIgnoreCaseIn(Arrays.asList("cOOl", null)); + List result = repository.findByAttributesIgnoreCaseIn(asList("cOOl", null)); assertThat(result).containsOnly(firstUser); } From 7f28d2524c95155935970cd954b1469a49826ab8 Mon Sep 17 00:00:00 2001 From: Jens Schauder Date: Tue, 24 Nov 2020 10:11:34 +0100 Subject: [PATCH 3/5] DATAJPA-1818 - Add JpaRepository.deleteAllByIdInBatch(Iterable ids). --- .../data/jpa/repository/JpaRepository.java | 13 +++++++++++++ .../data/jpa/repository/query/QueryUtils.java | 1 + .../repository/support/SimpleJpaRepository.java | 17 +++++++++++++++++ .../repository/support/JpaRepositoryTests.java | 17 +++++++++++++++-- 4 files changed, 46 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/springframework/data/jpa/repository/JpaRepository.java b/src/main/java/org/springframework/data/jpa/repository/JpaRepository.java index 8bf0f5a88c..1769b50520 100644 --- a/src/main/java/org/springframework/data/jpa/repository/JpaRepository.java +++ b/src/main/java/org/springframework/data/jpa/repository/JpaRepository.java @@ -84,6 +84,19 @@ public interface JpaRepository extends PagingAndSortingRepository, */ void deleteInBatch(Iterable entities); + + /** + * Deletes the entities identified by the given ids using a single {@link Query}. + * + * This kind of operation leaves JPAs first level cache and the database out of sync. + * Consider flushing the `EntityManager` before calling this method. + * + * @param ids + * + * @since 3.0 + */ + void deleteAllByIdInBatch(Iterable ids); + /** * Deletes all entities in a batch call. */ diff --git a/src/main/java/org/springframework/data/jpa/repository/query/QueryUtils.java b/src/main/java/org/springframework/data/jpa/repository/query/QueryUtils.java index f59aeb5857..c779073d13 100644 --- a/src/main/java/org/springframework/data/jpa/repository/query/QueryUtils.java +++ b/src/main/java/org/springframework/data/jpa/repository/query/QueryUtils.java @@ -86,6 +86,7 @@ public abstract class QueryUtils { public static final String COUNT_QUERY_STRING = "select count(%s) from %s x"; public static final String DELETE_ALL_QUERY_STRING = "delete from %s x"; + public static final String DELETE_ALL_QUERY_BY_ID_STRING = "delete from %s x where %s in :ids"; // Used Regex/Unicode categories (see https://www.unicode.org/reports/tr18/#General_Category_Property): // Z Separator 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 a416ee7e8a..45aff86929 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 @@ -246,6 +246,23 @@ public void deleteInBatch(Iterable entities) { .executeUpdate(); } + @Override + public void deleteAllByIdInBatch(Iterable ids) { + + Assert.notNull(ids, "Ids must not be null!"); + + if (!ids.iterator().hasNext()) { + return; + } + + String queryString = String.format(DELETE_ALL_QUERY_BY_ID_STRING, entityInformation.getEntityName(), entityInformation.getIdAttribute().getName()); + + Query query = em.createQuery(queryString); + query.setParameter("ids", ids); + + query.executeUpdate(); + } + /* * (non-Javadoc) * @see org.springframework.data.repository.Repository#deleteAll() 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 feb7ae1e34..69e3611ad6 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 @@ -114,11 +114,24 @@ void executesExistsForEntityWithIdClass() { assertThat(idClassRepository.existsById(id)).isTrue(); } - private static interface SampleEntityRepository extends JpaRepository { + @Test // DATAJPA-1818 + void deleteAllByIdInBatch() { + + 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(); + + repository.deleteAllByIdInBatch(Arrays.asList(new SampleEntityPK("one", "eins"),new SampleEntityPK("three", "drei"))); + assertThat(repository.findAll()).containsExactly(two); + } + + private interface SampleEntityRepository extends JpaRepository { } - private static interface SampleWithIdClassRepository + private interface SampleWithIdClassRepository extends CrudRepository { } From e466cdbfbca2a4d36353e672acf44f902024918d Mon Sep 17 00:00:00 2001 From: Jens Schauder Date: Tue, 24 Nov 2020 10:31:21 +0100 Subject: [PATCH 4/5] DATAJPA-1818 - Ignoring tests for EclipseLink. Handling of IN-Queries in Eclipselink is still broken. --- .../data/jpa/repository/support/SimpleJpaRepository.java | 3 ++- .../repository/support/EclipseLinkJpaRepositoryTests.java | 8 ++++++++ 2 files changed, 10 insertions(+), 1 deletion(-) 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 45aff86929..c28498b67a 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 @@ -255,7 +255,8 @@ public void deleteAllByIdInBatch(Iterable ids) { return; } - String queryString = String.format(DELETE_ALL_QUERY_BY_ID_STRING, entityInformation.getEntityName(), entityInformation.getIdAttribute().getName()); + String queryTemplate = DELETE_ALL_QUERY_BY_ID_STRING; + String queryString = String.format(queryTemplate, entityInformation.getEntityName(), entityInformation.getIdAttribute().getName()); Query query = em.createQuery(queryString); query.setParameter("ids", ids); 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 62990eee34..a53b2f9345 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 @@ -15,6 +15,7 @@ */ package org.springframework.data.jpa.repository.support; +import org.junit.jupiter.api.Disabled; import org.springframework.test.context.ContextConfiguration; /** @@ -25,4 +26,11 @@ @ContextConfiguration("classpath:eclipselink.xml") class EclipseLinkJpaRepositoryTests extends JpaRepositoryTests { + @Override + /** + * Ignored until https://bugs.eclipse.org/bugs/show_bug.cgi?id=349477 is resolved. + */ + void deleteAllByIdInBatch() { + super.deleteAllByIdInBatch(); + } } From 833445b28084de09823718e3ffddd7e0450c4d92 Mon Sep 17 00:00:00 2001 From: Jens Schauder Date: Tue, 24 Nov 2020 10:19:46 +0100 Subject: [PATCH 5/5] DATAJPA-1818 - Rename JpaRepository.deleteInBatch to match names in CrudRepository --- .../data/jpa/repository/JpaRepository.java | 22 ++++++++++++++++--- .../support/SimpleJpaRepository.java | 2 +- .../jpa/repository/UserRepositoryTests.java | 3 +-- 3 files changed, 21 insertions(+), 6 deletions(-) diff --git a/src/main/java/org/springframework/data/jpa/repository/JpaRepository.java b/src/main/java/org/springframework/data/jpa/repository/JpaRepository.java index 1769b50520..029f1d997c 100644 --- a/src/main/java/org/springframework/data/jpa/repository/JpaRepository.java +++ b/src/main/java/org/springframework/data/jpa/repository/JpaRepository.java @@ -77,12 +77,28 @@ public interface JpaRepository extends PagingAndSortingRepository, S saveAndFlush(S entity); /** - * Deletes the given entities in a batch which means it will create a single {@link Query}. Assume that we will clear - * the {@link javax.persistence.EntityManager} after the call. + * Deletes the given entities in a batch which means it will create a single {@link Query}. + * + * This kind of operation leaves JPAs first level cache and the database out of sync. + * Consider flushing the `EntityManager` before calling this method. * * @param entities + * @deprecated Use {@link #deleteAllInBatch(Iterable)} instead. + */ + @Deprecated + default void deleteInBatch(Iterable entities){deleteAllInBatch(entities);} + + /** + * Deletes the given entities in a batch which means it will create a single {@link Query}. + * + * This kind of operation leaves JPAs first level cache and the database out of sync. + * Consider flushing the `EntityManager` before calling this method. + * + * @param entities + * + * @since 3.0 */ - void deleteInBatch(Iterable entities); + void deleteAllInBatch(Iterable entities); /** 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 c28498b67a..3dfea791b6 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 @@ -234,7 +234,7 @@ public void deleteAllById(Iterable ids) { */ @Transactional @Override - public void deleteInBatch(Iterable entities) { + public void deleteAllInBatch(Iterable entities) { Assert.notNull(entities, "Entities must not be null!"); 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 d2ff181402..0cd58c1179 100644 --- a/src/test/java/org/springframework/data/jpa/repository/UserRepositoryTests.java +++ b/src/test/java/org/springframework/data/jpa/repository/UserRepositoryTests.java @@ -25,7 +25,6 @@ import static org.springframework.data.jpa.domain.sample.UserSpecifications.*; import java.util.ArrayList; -import java.util.Arrays; import java.util.Collection; import java.util.Collections; import java.util.HashSet; @@ -259,7 +258,7 @@ void batchDeleteCollectionOfEntities() { long before = repository.count(); - repository.deleteInBatch(asList(firstUser, secondUser)); + repository.deleteAllInBatch(asList(firstUser, secondUser)); assertThat(repository.existsById(firstUser.getId())).isFalse(); assertThat(repository.existsById(secondUser.getId())).isFalse();