From 9541eb9223e2448a30648d15bf5d598b362f7089 Mon Sep 17 00:00:00 2001 From: John Blum Date: Mon, 30 Nov 2020 16:25:36 -0800 Subject: [PATCH] DATAGEODE-387 - Re-implement SimpleGemfireRepository.deleteAllById(:Iterable) in terms of GemfireTemplate.removeAll(:Collection) and ultimately Region.removeAll(:Collection). --- .../support/SimpleGemfireRepository.java | 35 +++++---- ...mpleGemfireRepositoryIntegrationTests.java | 51 +++++++------ ...epositoryTransactionalIntegrationTest.java | 11 ++- .../SimpleGemfireRepositoryUnitTests.java | 72 +++++++++++++++++++ 4 files changed, 129 insertions(+), 40 deletions(-) diff --git a/spring-data-geode/src/main/java/org/springframework/data/gemfire/repository/support/SimpleGemfireRepository.java b/spring-data-geode/src/main/java/org/springframework/data/gemfire/repository/support/SimpleGemfireRepository.java index 02e2a301..959a43c0 100644 --- a/spring-data-geode/src/main/java/org/springframework/data/gemfire/repository/support/SimpleGemfireRepository.java +++ b/spring-data-geode/src/main/java/org/springframework/data/gemfire/repository/support/SimpleGemfireRepository.java @@ -22,6 +22,7 @@ import java.util.List; import java.util.Map; import java.util.Objects; import java.util.Optional; +import java.util.Set; import java.util.stream.Collectors; import java.util.stream.StreamSupport; @@ -72,6 +73,8 @@ import org.slf4j.LoggerFactory; */ public class SimpleGemfireRepository implements GemfireRepository { + protected static final String SELECT_COUNT_OQL_QUERY = "SELECT count(*) FROM %s"; + private final EntityInformation entityInformation; private final GemfireTemplate template; @@ -132,7 +135,7 @@ public class SimpleGemfireRepository implements GemfireRepository /** * Returns a reference to the {@link GemfireTemplate} used by this {@link GemfireRepository} to perform basic - * CRUD data access operations and simple OQL queries. + * CRUD and simple OQL queries data access operations * * @return a reference to the {@link GemfireTemplate} used by this {@link GemfireRepository}. * @see org.springframework.data.gemfire.GemfireTemplate @@ -200,7 +203,7 @@ public class SimpleGemfireRepository implements GemfireRepository public long count() { String regionPath = getRegion().getFullPath(); - String countQuery = String.format("SELECT count(*) FROM %s", regionPath); + String countQuery = String.format(SELECT_COUNT_OQL_QUERY, regionPath); SelectResults results = getTemplate().find(countQuery); @@ -285,21 +288,11 @@ public class SimpleGemfireRepository implements GemfireRepository return Optional.ofNullable(value); } - @Override - public void deleteById(@NonNull ID id) { - getTemplate().remove(id); - } - @Override public void delete(@NonNull T entity) { deleteById(getEntityInformation().getRequiredId(entity)); } - @Override - public void deleteAllById(@NonNull Iterable ids) { - CollectionUtils.nullSafeIterable(ids).forEach(this::deleteById); - } - @Override public void deleteAll() { @@ -321,6 +314,24 @@ public class SimpleGemfireRepository implements GemfireRepository CollectionUtils.nullSafeIterable(entities).forEach(this::delete); } + @Override + public void deleteAllById(@NonNull Iterable ids) { + + Set keys = + StreamSupport.stream(CollectionUtils.nullSafeIterable(ids).spliterator(), false) + .filter(Objects::nonNull) + .collect(Collectors.toSet()); + + if (!keys.isEmpty()) { + getTemplate().removeAll(keys); + } + } + + @Override + public void deleteById(@NonNull ID id) { + getTemplate().remove(id); + } + boolean isPartitioned(Region region) { return region != null diff --git a/spring-data-geode/src/test/java/org/springframework/data/gemfire/repository/support/SimpleGemfireRepositoryIntegrationTests.java b/spring-data-geode/src/test/java/org/springframework/data/gemfire/repository/support/SimpleGemfireRepositoryIntegrationTests.java index 9677fe83..ac6f9aa1 100644 --- a/spring-data-geode/src/test/java/org/springframework/data/gemfire/repository/support/SimpleGemfireRepositoryIntegrationTests.java +++ b/spring-data-geode/src/test/java/org/springframework/data/gemfire/repository/support/SimpleGemfireRepositoryIntegrationTests.java @@ -72,9 +72,11 @@ public class SimpleGemfireRepositoryIntegrationTests { static final String GEMFIRE_LOG_LEVEL = "warning"; - @Autowired private GemfireTemplate template; + @Autowired + private GemfireTemplate template; - @Resource(name = "People") private Region people; + @Resource(name = "People") + private Region people; private RegionClearListener regionClearListener; @@ -108,6 +110,31 @@ public class SimpleGemfireRepositoryIntegrationTests { assertThat(this.regionClearListener.eventFired).isTrue(); } + @Test // DATAGEODE-387 + public void deleteAllById() { + + assertThat(this.repository.count()).isEqualTo(0); + + List people = Arrays.asList( + new Person(1L, "Jon", "Doe"), + new Person(2L, "Jane", "Doe"), + new Person(3L, "Cookie", "Doe"), + new Person(4L, "Pie", "Doe"), + new Person(5L, "Sour", "Doe") + ); + + people.forEach(person -> this.template.put(person.getId(), person)); + + assertThat(this.repository.count()).isEqualTo(5); + + this.repository.deleteAllById(Arrays.asList(1L, 2L)); + + assertThat(this.repository.count()).isEqualTo(3L); + assertThat(this.repository.findAll()) // + .extracting(Person::getFirstname) // + .containsExactlyInAnyOrder("Cookie", "Pie", "Sour"); + } + @Test public void findAllPaged() { @@ -223,26 +250,6 @@ public class SimpleGemfireRepositoryIntegrationTests { assertThat(this.repository.findAll()).isEmpty(); } - @Test // DATAGEODE-387 - public void deleteAllById() { - - assertThat(this.repository.count()).isEqualTo(0); - - List people = Arrays.asList(new Person(1L, "Jon", "Doe"), new Person(2L, "Jane", "Doe"), - new Person(3L, "Cookie", "Doe"), new Person(4L, "Pie", "Doe"), new Person(5L, "Sour", "Doe")); - - people.forEach(person -> this.template.put(person.getId(), person)); - - assertThat(this.repository.count()).isEqualTo(5); - - this.repository.deleteAllById(Arrays.asList(1L, 2L)); - - assertThat(this.repository.count()).isEqualTo(3L); - assertThat(this.repository.findAll()) // - .extracting(Person::getFirstname) // - .containsExactlyInAnyOrder("Cookie", "Pie", "Sour"); - } - @Test public void saveEntities() { diff --git a/spring-data-geode/src/test/java/org/springframework/data/gemfire/repository/support/SimpleGemfireRepositoryTransactionalIntegrationTest.java b/spring-data-geode/src/test/java/org/springframework/data/gemfire/repository/support/SimpleGemfireRepositoryTransactionalIntegrationTest.java index cc9c10b4..2576b1f4 100644 --- a/spring-data-geode/src/test/java/org/springframework/data/gemfire/repository/support/SimpleGemfireRepositoryTransactionalIntegrationTest.java +++ b/spring-data-geode/src/test/java/org/springframework/data/gemfire/repository/support/SimpleGemfireRepositoryTransactionalIntegrationTest.java @@ -13,7 +13,6 @@ * See the License for the specific language governing permissions and * limitations under the License. */ - package org.springframework.data.gemfire.repository.support; import static org.junit.Assert.assertEquals; @@ -28,13 +27,13 @@ import java.util.concurrent.atomic.AtomicLong; import javax.annotation.Resource; -import org.apache.geode.cache.Region; - import org.junit.After; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; +import org.apache.geode.cache.Region; + import org.springframework.beans.factory.annotation.Autowired; import org.springframework.data.gemfire.GemfireTemplate; import org.springframework.data.gemfire.mapping.GemfireMappingContext; @@ -51,7 +50,7 @@ import org.springframework.transaction.support.TransactionCallbackWithoutResult; import org.springframework.transaction.support.TransactionTemplate; /** - * Integration tests testing the {@link SimpleGemfireRepository} class and SDC Repository abstraction implementation + * Integration Tests testing the {@link SimpleGemfireRepository} class and SDC Repository abstraction implementation * in the context of GemFire "Cache" Transactions. * * @author John Blum @@ -75,7 +74,7 @@ public class SimpleGemfireRepositoryTransactionalIntegrationTest { private CustomerService customerService; @Resource(name = "Customers") - private Region customers; + private Region customers; static Customer createCustomer(String firstName, String lastName) { @@ -102,7 +101,7 @@ public class SimpleGemfireRepositoryTransactionalIntegrationTest { @Test public void testDeleteAll() { - Collection expectedCustomers = new ArrayList(4); + Collection expectedCustomers = new ArrayList<>(4); expectedCustomers.add(createCustomer("Jon", "Doe")); expectedCustomers.add(createCustomer("Jane", "Doe")); diff --git a/spring-data-geode/src/test/java/org/springframework/data/gemfire/repository/support/SimpleGemfireRepositoryUnitTests.java b/spring-data-geode/src/test/java/org/springframework/data/gemfire/repository/support/SimpleGemfireRepositoryUnitTests.java index 93fa0704..4eed8067 100644 --- a/spring-data-geode/src/test/java/org/springframework/data/gemfire/repository/support/SimpleGemfireRepositoryUnitTests.java +++ b/spring-data-geode/src/test/java/org/springframework/data/gemfire/repository/support/SimpleGemfireRepositoryUnitTests.java @@ -23,6 +23,7 @@ import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.inOrder; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.spy; import static org.mockito.Mockito.times; @@ -47,6 +48,7 @@ import java.util.stream.Collectors; import java.util.stream.Stream; import org.junit.Test; +import org.mockito.InOrder; import org.mockito.invocation.InvocationOnMock; import org.mockito.stubbing.Answer; @@ -979,6 +981,76 @@ public class SimpleGemfireRepositoryUnitTests { verify(mockRegion, times(1)).removeAll(eq(keys)); } + @Test + public void deleteAllByIdWithKeys() { + + Collection ids = Arrays.asList(1, 2, 3); + Collection keys = new HashSet(ids); + + Region mockRegion = mockRegion("Example"); + + GemfireTemplate template = spy(newGemfireTemplate(mockRegion)); + + SimpleGemfireRepository repository = new SimpleGemfireRepository(template, mockEntityInformation()); + + assertThat(repository).isNotNull(); + assertThat(repository.getTemplate()).isEqualTo(template); + assertThat(template.getRegion()).isEqualTo(mockRegion); + + repository.deleteAllById(ids); + + InOrder order = inOrder(mockRegion, template); + + order.verify(template, times(1)).removeAll(eq(keys)); + order.verify(mockRegion, times(1)).removeAll(eq(keys)); + + verifyNoMoreInteractions(mockRegion); + } + + @Test + public void deleteAllByIdWithMixedKeys() { + + Collection ids = Arrays.asList(1, null, 3); + Collection keys = CollectionUtils.asSet(1, 3); + + Region mockRegion = mockRegion("Example"); + + GemfireTemplate template = spy(newGemfireTemplate(mockRegion)); + + SimpleGemfireRepository repository = new SimpleGemfireRepository(template, mockEntityInformation()); + + assertThat(repository).isNotNull(); + assertThat(repository.getTemplate()).isEqualTo(template); + assertThat(template.getRegion()).isEqualTo(mockRegion); + + repository.deleteAllById(ids); + + InOrder order = inOrder(mockRegion, template); + + order.verify(template, times(1)).removeAll(eq(keys)); + order.verify(mockRegion, times(1)).removeAll(eq(keys)); + + verifyNoMoreInteractions(mockRegion); + } + + @Test + public void deleteAllByIdWithNullCollectionOfKeys() { + + Region mockRegion = mockRegion("Example"); + + GemfireTemplate template = spy(newGemfireTemplate(mockRegion)); + + SimpleGemfireRepository repository = new SimpleGemfireRepository(template, mockEntityInformation()); + + assertThat(repository).isNotNull(); + assertThat(repository.getTemplate()).isEqualTo(template); + assertThat(template.getRegion()).isEqualTo(mockRegion); + + repository.deleteAllById(null); + + verifyNoInteractions(mockRegion); + } + @Test public void toListFromIterable() {