diff --git a/src/main/java/org/springframework/data/gemfire/repository/support/SimpleGemfireRepository.java b/src/main/java/org/springframework/data/gemfire/repository/support/SimpleGemfireRepository.java index c31bb249..40b58d62 100644 --- a/src/main/java/org/springframework/data/gemfire/repository/support/SimpleGemfireRepository.java +++ b/src/main/java/org/springframework/data/gemfire/repository/support/SimpleGemfireRepository.java @@ -18,6 +18,7 @@ package org.springframework.data.gemfire.repository.support; import java.util.Collection; import java.util.HashMap; +import java.util.Iterator; import java.util.List; import java.util.Map; import java.util.Objects; @@ -60,24 +61,25 @@ public class SimpleGemfireRepository implements GemfireRepository private final GemfireTemplate template; /** - * Creates a new {@link SimpleGemfireRepository}. + * Constructs a new instance of {@link SimpleGemfireRepository} initialized with the {@link GemfireTemplate} + * and {@link EntityInformation}. * - * @param template must not be {@literal null}. - * @param entityInformation must not be {@literal null}. + * @param template {@link GemfireTemplate} used to perform basic data access operations and simple OQL queries; + * must not be {@literal null}. + * @param entityInformation {@link EntityInformation} used to describe the entity; must not be {@literal null}. + * @throws IllegalArgumentException if {@link GemfireTemplate} or {@link EntityInformation} is {@literal null}. + * @see org.springframework.data.gemfire.GemfireTemplate + * @see org.springframework.data.repository.core.EntityInformation */ public SimpleGemfireRepository(GemfireTemplate template, EntityInformation entityInformation) { - Assert.notNull(template, "Template must not be null"); + Assert.notNull(template, "GemfireTemplate must not be null"); Assert.notNull(entityInformation, "EntityInformation must not be null"); this.template = template; this.entityInformation = entityInformation; } - /* - * (non-Javadoc) - * @see org.springframework.data.repository.CrudRepository#save(S) - */ @Override public U save(U entity) { @@ -88,10 +90,16 @@ public class SimpleGemfireRepository implements GemfireRepository return entity; } - /* - * (non-Javadoc) - * @see org.springframework.data.repository.CrudRepository#save(java.lang.Iterable) - */ + @Override + public T save(Wrapper wrapper) { + + T entity = wrapper.getEntity(); + + this.template.put(wrapper.getKey(), entity); + + return entity; + } + @Override public Iterable saveAll(Iterable entities) { @@ -104,55 +112,31 @@ public class SimpleGemfireRepository implements GemfireRepository return entitiesToSave.values(); } - /* - * (non-Javadoc) - * @see org.springframework.data.gemfire.repository.GemfireRepository#save(org.springframework.data.gemfire.repository.Wrapper) - */ - @Override - public T save(Wrapper wrapper) { - - T entity = wrapper.getEntity(); - - this.template.put(wrapper.getKey(), entity); - - return entity; - } - - /* - * (non-Javadoc) - * @see org.springframework.data.repository.CrudRepository#count() - */ @Override public long count() { - SelectResults results = - this.template.find(String.format("SELECT count(*) FROM %s", this.template.getRegion().getFullPath())); + String countQuery = String.format("SELECT count(*) FROM %s", this.template.getRegion().getFullPath()); - return Long.valueOf(results.iterator().next()); + SelectResults results = this.template.find(countQuery); + + return Optional.ofNullable(results) + .map(SelectResults::iterator) + .filter(Iterator::hasNext) + .map(Iterator::next) + .map(Long::valueOf) + .orElse(0L); } - /* - * (non-Javadoc) - * @see org.springframework.data.repository.CrudRepository#existsById(java.lang.Object) - */ @Override public boolean existsById(ID id) { return findById(id).isPresent(); } - /* - * (non-Javadoc) - * @see org.springframework.data.repository.CrudRepository#findById(java.lang.Object) - */ @Override public Optional findById(ID id) { return Optional.ofNullable(this.template.get(id)); } - /* - * (non-Javadoc) - * @see org.springframework.data.repository.CrudRepository#findAll() - */ @Override public Collection findAll() { @@ -162,10 +146,6 @@ public class SimpleGemfireRepository implements GemfireRepository return results.asList(); } - /* - * (non-Javadoc) - * @see org.springframework.data.gemfire.repository.GemfireRepository.sort(:org.springframework.data.domain.Sort) - */ @Override public Iterable findAll(Sort sort) { @@ -178,10 +158,6 @@ public class SimpleGemfireRepository implements GemfireRepository return selectResults.asList(); } - /* - * (non-Javadoc) - * @see org.springframework.data.repository.CrudRepository#findAllById(java.lang.Iterable) - */ @Override public Collection findAllById(Iterable ids) { @@ -191,80 +167,45 @@ public class SimpleGemfireRepository implements GemfireRepository .filter(Objects::nonNull).collect(Collectors.toList()); } - /* - * (non-Javadoc) - * @see org.springframework.data.repository.CrudRepository#deleteById(java.lang.Object) - */ @Override public void deleteById(ID id) { this.template.remove(id); } - /* - * (non-Javadoc) - * @see org.springframework.data.repository.CrudRepository#delete(java.lang.Object) - */ @Override public void delete(T entity) { deleteById(this.entityInformation.getRequiredId(entity)); } - /* - * (non-Javadoc) - * @see org.springframework.data.repository.CrudRepository#delete(java.lang.Iterable) - */ @Override public void deleteAll(Iterable entities) { entities.forEach(this::delete); } - /* - * (non-Javadoc) - * @see org.apache.geode.cache.Region#getAttributes() - * @see org.apache.geode.cache.RegionAttributes#getDataPolicy() - */ boolean isPartitioned(Region region) { return region != null && region.getAttributes() != null && isPartitioned(region.getAttributes().getDataPolicy()); } - /* - * (non-Javadoc) - * @see org.apache.geode.cache.DataPolicy#withPartitioning() - */ boolean isPartitioned(DataPolicy dataPolicy) { return dataPolicy != null && dataPolicy.withPartitioning(); } - /* - * (non-Javadoc) - * @see org.apache.geode.cache.Region#getRegionService() - * @see org.apache.geode.cache.Cache#getCacheTransactionManager() - */ boolean isTransactionPresent(Region region) { return region.getRegionService() instanceof Cache && isTransactionPresent(((Cache) region.getRegionService()).getCacheTransactionManager()); } - /* - * (non-Javadoc) - * @see org.apache.geode.cache.CacheTransactionManager#exists() - */ boolean isTransactionPresent(CacheTransactionManager cacheTransactionManager) { return cacheTransactionManager != null && cacheTransactionManager.exists(); } - /* (non-Javadoc) */ void doRegionClear(Region region) { region.removeAll(region.keySet()); } - /* - * (non-Javadoc) - * @see org.springframework.data.repository.CrudRepository#deleteAll() - */ @Override public void deleteAll() { this.template.execute((GemfireCallback) region -> { diff --git a/src/test/java/org/springframework/data/gemfire/repository/support/SimpleGemfireRepositoryUnitTests.java b/src/test/java/org/springframework/data/gemfire/repository/support/SimpleGemfireRepositoryUnitTests.java index 95813acb..a4756690 100644 --- a/src/test/java/org/springframework/data/gemfire/repository/support/SimpleGemfireRepositoryUnitTests.java +++ b/src/test/java/org/springframework/data/gemfire/repository/support/SimpleGemfireRepositoryUnitTests.java @@ -13,12 +13,9 @@ * See the License for the specific language governing permissions and * limitations under the License. */ - package org.springframework.data.gemfire.repository.support; import static org.assertj.core.api.Assertions.assertThat; -import static org.hamcrest.Matchers.is; -import static org.hamcrest.Matchers.nullValue; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.doAnswer; @@ -28,6 +25,8 @@ import static org.mockito.Mockito.mock; import static org.mockito.Mockito.spy; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoInteractions; +import static org.mockito.Mockito.verifyNoMoreInteractions; import static org.mockito.Mockito.when; import java.util.ArrayList; @@ -44,41 +43,43 @@ import java.util.function.Function; import java.util.stream.Collectors; import java.util.stream.Stream; +import org.junit.Test; +import org.mockito.invocation.InvocationOnMock; +import org.mockito.stubbing.Answer; + import org.apache.geode.cache.Cache; import org.apache.geode.cache.CacheTransactionManager; import org.apache.geode.cache.DataPolicy; import org.apache.geode.cache.Region; import org.apache.geode.cache.RegionAttributes; import org.apache.geode.cache.query.SelectResults; -import org.junit.Rule; -import org.junit.Test; -import org.junit.rules.ExpectedException; -import org.mockito.invocation.InvocationOnMock; -import org.mockito.stubbing.Answer; + import org.springframework.data.gemfire.GemfireTemplate; import org.springframework.data.gemfire.repository.Wrapper; import org.springframework.data.gemfire.repository.sample.Animal; import org.springframework.data.repository.core.EntityInformation; /** - * Unit tests for {@link SimpleGemfireRepository}. + * Unit Tests for {@link SimpleGemfireRepository}. * * @author John Blum - * @see org.junit.Rule + * @see java.util.function.Function + * @see java.util.stream.Stream * @see org.junit.Test * @see org.mockito.Mockito + * @see org.apache.geode.cache.Cache + * @see org.apache.geode.cache.Region * @see org.springframework.data.gemfire.GemfireTemplate * @see org.springframework.data.gemfire.repository.Wrapper * @see org.springframework.data.gemfire.repository.support.SimpleGemfireRepository + * @see org.springframework.data.repository.core.EntityInformation * @since 1.4.5 */ -@SuppressWarnings("unchecked") +@SuppressWarnings({ "rawtypes", "unchecked" }) public class SimpleGemfireRepositoryUnitTests { - @Rule - public ExpectedException exception = ExpectedException.none(); - protected Map asMap(Iterable animals) { + Map animalMap = new HashMap<>(); for (Animal animal : animals) { @@ -89,14 +90,20 @@ public class SimpleGemfireRepositoryUnitTests { } protected Animal newAnimal(String name) { + Animal animal = new Animal(); + animal.setName(name); + return animal; } protected Animal newAnimal(Long id, String name) { + Animal animal = newAnimal(name); + animal.setId(id); + return animal; } @@ -105,6 +112,7 @@ public class SimpleGemfireRepositoryUnitTests { } protected Cache mockCache(String name, boolean transactionExists) { + Cache mockCache = mock(Cache.class, String.format("%s.MockCache", name)); CacheTransactionManager mockCacheTransactionManager = mock(CacheTransactionManager.class, @@ -117,13 +125,15 @@ public class SimpleGemfireRepositoryUnitTests { } protected EntityInformation mockEntityInformation() { + EntityInformation mockEntityInformation = mock(EntityInformation.class); doAnswer(new Answer() { + private final AtomicLong idSequence = new AtomicLong(0L); @Override - public Long answer(InvocationOnMock invocation) throws Throwable { + public Long answer(InvocationOnMock invocation) { Animal argument = invocation.getArgument(0); argument.setId(resolveId(argument.getId())); return argument.getId(); @@ -133,6 +143,7 @@ public class SimpleGemfireRepositoryUnitTests { private Long resolveId(Long id) { return (id != null ? id : idSequence.incrementAndGet()); } + }).when(mockEntityInformation).getRequiredId(any(Animal.class)); return mockEntityInformation; @@ -143,6 +154,7 @@ public class SimpleGemfireRepositoryUnitTests { } protected Region mockRegion(String name) { + Region mockRegion = mock(Region.class, String.format("%s.MockRegion", name)); when(mockRegion.getName()).thenReturn(name); @@ -152,6 +164,7 @@ public class SimpleGemfireRepositoryUnitTests { } protected Region mockRegion(String name, Cache mockCache, DataPolicy dataPolicy) { + Region mockRegion = mockRegion(name); when(mockRegion.getRegionService()).thenReturn(mockCache); @@ -166,25 +179,54 @@ public class SimpleGemfireRepositoryUnitTests { } @Test - public void constructSimpleGemfireRepositoryWithNullTemplateThrowsIllegalArgumentException() { - exception.expect(IllegalArgumentException.class); - exception.expectCause(is(nullValue(Throwable.class))); - exception.expectMessage("Template must not be null"); + public void constructsSimpleGemfireRepositorySuccessfully() { - new SimpleGemfireRepository<>(null, mockEntityInformation()); + Region mockRegion = mock(Region.class); + + GemfireTemplate template = spy(new GemfireTemplate(mockRegion)); + + EntityInformation mockEntityInformation = mock(EntityInformation.class); + + SimpleGemfireRepository repository = new SimpleGemfireRepository(template, mockEntityInformation); + + assertThat(repository).isNotNull(); + + verifyNoInteractions(template, mockRegion, mockEntityInformation); } - @Test - public void constructSimpleGemfireRepositoryWithNullEntityInformationThrowsIllegalArgumentException() { - exception.expect(IllegalArgumentException.class); - exception.expectCause(is(nullValue(Throwable.class))); - exception.expectMessage("EntityInformation must not be null"); + @Test(expected = IllegalArgumentException.class) + public void constructSimpleGemfireRepositoryWithNullTemplateThrowsIllegalArgumentException() { - new SimpleGemfireRepository<>(newGemfireTemplate(mockRegion()), null); + try { + new SimpleGemfireRepository<>(null, mockEntityInformation()); + } + catch (IllegalArgumentException expected) { + + assertThat(expected).hasMessage("GemfireTemplate must not be null"); + assertThat(expected).hasNoCause(); + + throw expected; + } + } + + @Test(expected = IllegalArgumentException.class) + public void constructSimpleGemfireRepositoryWithNullEntityInformationThrowsIllegalArgumentException() { + + try { + new SimpleGemfireRepository<>(newGemfireTemplate(mockRegion()), null); + } + catch (IllegalArgumentException expected) { + + assertThat(expected).hasMessage("EntityInformation must not be null"); + assertThat(expected).hasNoCause(); + + throw expected; + } } @Test public void saveEntityIsCorrect() { + Region mockRegion = mockRegion(); SimpleGemfireRepository repository = @@ -237,21 +279,98 @@ public class SimpleGemfireRepositoryUnitTests { @Test public void countReturnsNumberOfRegionEntries() { - Region mockRegion = mockRegion("Example"); - GemfireTemplate template = spy(newGemfireTemplate(mockRegion)); + SelectResults mockSelectResults = mock(SelectResults.class); + Region mockRegion = mockRegion("Example"); + + GemfireTemplate template = spy(newGemfireTemplate(mockRegion)); + doReturn(mockSelectResults).when(template).find(eq("SELECT count(*) FROM /Example")); when(mockSelectResults.iterator()).thenReturn(Collections.singletonList(21).iterator()); - SimpleGemfireRepository repository = new SimpleGemfireRepository<>( - template, mockEntityInformation()); + SimpleGemfireRepository repository = + new SimpleGemfireRepository<>(template, mockEntityInformation()); - assertThat(repository.count()).isEqualTo(21); + assertThat(repository).isNotNull(); + assertThat(repository.count()).isEqualTo(21L); + verify(template, times(1)).getRegion(); verify(mockRegion, times(1)).getFullPath(); verify(template, times(1)).find(eq("SELECT count(*) FROM /Example")); verify(mockSelectResults, times(1)).iterator(); + verifyNoMoreInteractions(mockRegion, mockSelectResults, template); + } + + @Test + public void countWhenSelectResultsAreNullIsNullSafeAndReturnsZero() { + + Region mockRegion = mockRegion("Example"); + + GemfireTemplate template = spy(newGemfireTemplate(mockRegion)); + + doReturn(null).when(template).find(eq("SELECT count(*) FROM /Example")); + + SimpleGemfireRepository repository = + new SimpleGemfireRepository<>(template, mockEntityInformation()); + + assertThat(repository).isNotNull(); + assertThat(repository.count()).isEqualTo(0L); + + verify(template, times(1)).getRegion(); + verify(mockRegion, times(1)).getFullPath(); + verify(template, times(1)).find(eq("SELECT count(*) FROM /Example")); + verifyNoMoreInteractions(mockRegion, template); + } + + @Test + public void countWhenSelectResultsIteratorIsNullIsNullSafeAndReturnsZero() { + + SelectResults mockSelectResults = mock(SelectResults.class); + + Region mockRegion = mockRegion("Example"); + + GemfireTemplate template = spy(newGemfireTemplate(mockRegion)); + + doReturn(mockSelectResults).when(template).find(eq("SELECT count(*) FROM /Example")); + doReturn(null).when(mockSelectResults).iterator(); + + SimpleGemfireRepository repository = + new SimpleGemfireRepository<>(template, mockEntityInformation()); + + assertThat(repository).isNotNull(); + assertThat(repository.count()).isEqualTo(0L); + + verify(template, times(1)).getRegion(); + verify(mockRegion, times(1)).getFullPath(); + verify(template, times(1)).find(eq("SELECT count(*) FROM /Example")); + verify(mockSelectResults, times(1)).iterator(); + verifyNoMoreInteractions(mockRegion, mockSelectResults, template); + } + + @Test + public void countWhenSelectResultsIteratorIsEmptyReturnsZero() { + + SelectResults mockSelectResults = mock(SelectResults.class); + + Region mockRegion = mockRegion("Example"); + + GemfireTemplate template = spy(newGemfireTemplate(mockRegion)); + + doReturn(mockSelectResults).when(template).find(eq("SELECT count(*) FROM /Example")); + doReturn(Collections.emptyIterator()).when(mockSelectResults).iterator(); + + SimpleGemfireRepository repository = + new SimpleGemfireRepository<>(template, mockEntityInformation()); + + assertThat(repository).isNotNull(); + assertThat(repository.count()).isEqualTo(0L); + + verify(template, times(1)).getRegion(); + verify(mockRegion, times(1)).getFullPath(); + verify(template, times(1)).find(eq("SELECT count(*) FROM /Example")); + verify(mockSelectResults, times(1)).iterator(); + verifyNoMoreInteractions(mockRegion, mockSelectResults, template); } @Test