From 3d08dc1eaabbf9836d26173bc68af58989232fd0 Mon Sep 17 00:00:00 2001 From: John Blum Date: Thu, 16 Oct 2014 13:59:36 -0700 Subject: [PATCH] SGF-343 - Optimize the SDG implementation of CrudRepository.save(Iterable enttiies) to use GemFire's Region.putAll(Map values) operation. --- .../support/SimpleGemfireRepository.java | 127 ++++---- .../data/gemfire/GemfireTemplateTest.java | 7 +- ...impleGemfireRepositoryIntegrationTest.java | 26 +- .../SimpleGemfireRepositoryUnitTest.java | 303 ++++++++++++++++++ .../gemfire/test/support/CollectionUtils.java | 10 + .../data/gemfire/basic-template.xml | 13 +- 6 files changed, 416 insertions(+), 70 deletions(-) create mode 100644 src/test/java/org/springframework/data/gemfire/repository/support/SimpleGemfireRepositoryUnitTest.java 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 1ef481be..dacc219a 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 @@ -3,7 +3,9 @@ package org.springframework.data.gemfire.repository.support; import java.io.Serializable; import java.util.ArrayList; import java.util.Collection; +import java.util.HashMap; import java.util.List; +import java.util.Map; import org.springframework.data.domain.Sort; import org.springframework.data.gemfire.GemfireCallback; @@ -18,10 +20,15 @@ import com.gemstone.gemfire.cache.Region; import com.gemstone.gemfire.cache.query.SelectResults; /** - * Basic repository implementation. + * Basic Repository implementation for GemFire. * * @author Oliver Gierke * @author David Turanski + * @author John Blum + * @see java.io.Serializable + * @see org.springframework.data.gemfire.GemfireTemplate + * @see org.springframework.data.gemfire.repository.GemfireRepository + * @see com.gemstone.gemfire.cache.Region */ public class SimpleGemfireRepository implements GemfireRepository { @@ -36,7 +43,6 @@ public class SimpleGemfireRepository implements Gemf * @param entityInformation must not be {@literal null}. */ public SimpleGemfireRepository(GemfireTemplate template, EntityInformation entityInformation) { - Assert.notNull(template); Assert.notNull(entityInformation); @@ -58,41 +64,63 @@ public class SimpleGemfireRepository implements Gemf /* * (non-Javadoc) * - * @see - * org.springframework.data.repository.CrudRepository#save(java.lang.Iterable - * ) + * @see org.springframework.data.repository.CrudRepository#save(java.lang.Iterable) */ @Override public Iterable save(Iterable entities) { - List result = new ArrayList(); + Map result = new HashMap(); + for (U entity : entities) { - result.add(save(entity)); + result.put(entityInformation.getId(entity), entity); } - return result; + + template.putAll(result); + + return result.values(); + } + + /* + * (non-Javadoc) + * + * @see + * org.springframework.data.gemfire.repository.GemfireRepository#save( + * org.springframework.data.gemfire.repository.Wrapper) + */ + @Override + public T save(Wrapper wrapper) { + return template.put(wrapper.getKey(), wrapper.getEntity()); + } + + /* + * (non-Javadoc) + * + * @see org.springframework.data.repository.CrudRepository#count() + */ + @Override + public long count() { + SelectResults results = template.find("SELECT count(*) FROM " + template.getRegion().getFullPath()); + return (long) results.iterator().next(); } /* * (non-Javadoc) * - * @see org.springframework.data.repository.CrudRepository#findOne(java.io. - * Serializable) + * @see org.springframework.data.repository.CrudRepository#exists(java.io.Serializable) + */ + @Override + public boolean exists(ID id) { + return (findOne(id) != null); + } + + /* + * (non-Javadoc) + * + * @see org.springframework.data.repository.CrudRepository#findOne(java.io.Serializable) */ @Override @SuppressWarnings("unchecked") public T findOne(ID id) { - Object object = template.get(id); - return (T) object; - } - - /* - * (non-Javadoc) - * - * @see org.springframework.data.repository.CrudRepository#exists(java.io. - * Serializable) - */ - @Override - public boolean exists(ID id) { - return findOne(id) != null; + return (T) template.get(id); } /* @@ -102,13 +130,13 @@ public class SimpleGemfireRepository implements Gemf */ @Override public Collection findAll() { - SelectResults results = template.find("select * from " + template.getRegion().getFullPath()); + SelectResults results = template.find("SELECT * FROM " + template.getRegion().getFullPath()); return results.asList(); } /* * (non-Javadoc) - * @see org.springframework.data.gemfire.repository.GemfireRepository.sor(:org.springframework.data.domain.Sort) + * @see org.springframework.data.gemfire.repository.GemfireRepository.sort(:org.springframework.data.domain.Sort) */ @Override public Iterable findAll(final Sort sort) { @@ -124,15 +152,13 @@ public class SimpleGemfireRepository implements Gemf /* * (non-Javadoc) * - * @see - * org.springframework.data.repository.CrudRepository#findAll(java.lang. - * Iterable) + * @see org.springframework.data.repository.CrudRepository#findAll(java.lang.Iterable) */ @Override @SuppressWarnings("unchecked") public Collection findAll(Iterable ids) { - List parameters = new ArrayList(); + for (ID id : ids) { parameters.add(id); } @@ -142,33 +168,30 @@ public class SimpleGemfireRepository implements Gemf /* * (non-Javadoc) - * - * @see org.springframework.data.repository.CrudRepository#count() + * + * @see org.springframework.data.repository.CrudRepository#delete(java.io.Serializable) */ @Override - public long count() { - SelectResults results = template.find("select count(*) from " + template.getRegion().getFullPath()); - return (long)results.iterator().next(); + public void delete(ID id) { + template.remove(id); } /* * (non-Javadoc) * * @see - * org.springframework.data.repository.CrudRepository#delete(java.lang.Object - * ) + * org.springframework.data.repository.CrudRepository#delete(java.lang.Object) */ @Override public void delete(T entity) { - template.remove(entityInformation.getId(entity)); + delete(entityInformation.getId(entity)); } /* * (non-Javadoc) * * @see - * org.springframework.data.repository.CrudRepository#delete(java.lang.Iterable - * ) + * org.springframework.data.repository.CrudRepository#delete(java.lang.Iterable) */ @Override public void delete(Iterable entities) { @@ -184,7 +207,6 @@ public class SimpleGemfireRepository implements Gemf */ @Override public void deleteAll() { - template.execute(new GemfireCallback() { @Override @SuppressWarnings("rawtypes") @@ -192,7 +214,8 @@ public class SimpleGemfireRepository implements Gemf //clear() does not work for partitioned regions try { region.clear(); - } catch (UnsupportedOperationException e) { + } + catch (UnsupportedOperationException e) { for (Object key : region.keySet()) { region.remove(key); } @@ -203,26 +226,4 @@ public class SimpleGemfireRepository implements Gemf }); } - /* - * (non-Javadoc) - * - * @see org.springframework.data.repository.CrudRepository#delete(java.io. - * Serializable) - */ - @Override - public void delete(ID id) { - template.remove(id); - } - - /* - * (non-Javadoc) - * - * @see - * org.springframework.data.gemfire.repository.GemfireRepository#save(org - * .springframework.data.gemfire.repository.Wrapper) - */ - @Override - public T save(Wrapper wrapper) { - return template.put(wrapper.getKey(), wrapper.getEntity()); - } } diff --git a/src/test/java/org/springframework/data/gemfire/GemfireTemplateTest.java b/src/test/java/org/springframework/data/gemfire/GemfireTemplateTest.java index b89d9a7b..56c25208 100644 --- a/src/test/java/org/springframework/data/gemfire/GemfireTemplateTest.java +++ b/src/test/java/org/springframework/data/gemfire/GemfireTemplateTest.java @@ -58,10 +58,11 @@ import com.gemstone.gemfire.cache.query.SelectResults; import com.gemstone.gemfire.cache.query.TypeMismatchException; /** - * The GemfireTemplateTest class is a test suite of test case testing the contract and functionality of the - * GemfirTemplate class. + * The GemfireTemplateTest class is a test suite of test cases testing the contract and functionality of the SDG + * GemfireTemplate class. * * @author Costin Leau + * @author David Turanski * @author John Blum * @see org.junit.Test * @see org.junit.runner.RunWith @@ -69,8 +70,8 @@ import com.gemstone.gemfire.cache.query.TypeMismatchException; * @see org.springframework.data.gemfire.test.GemfireTestApplicationContextInitializer * @see org.springframework.test.context.ContextConfiguration */ -@ContextConfiguration(locations="basic-template.xml", initializers=GemfireTestApplicationContextInitializer.class) @RunWith(SpringJUnit4ClassRunner.class) +@ContextConfiguration(locations="basic-template.xml", initializers=GemfireTestApplicationContextInitializer.class) @SuppressWarnings("unused") public class GemfireTemplateTest { diff --git a/src/test/java/org/springframework/data/gemfire/repository/support/SimpleGemfireRepositoryIntegrationTest.java b/src/test/java/org/springframework/data/gemfire/repository/support/SimpleGemfireRepositoryIntegrationTest.java index 98793f32..ad39fc8a 100644 --- a/src/test/java/org/springframework/data/gemfire/repository/support/SimpleGemfireRepositoryIntegrationTest.java +++ b/src/test/java/org/springframework/data/gemfire/repository/support/SimpleGemfireRepositoryIntegrationTest.java @@ -19,13 +19,13 @@ import static org.hamcrest.Matchers.hasItems; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.not; import static org.hamcrest.Matchers.nullValue; +import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertThat; import static org.junit.Assert.assertTrue; import java.util.Arrays; import java.util.Collection; - import javax.annotation.Resource; import org.junit.Before; @@ -48,6 +48,9 @@ import com.gemstone.gemfire.cache.util.CacheListenerAdapter; * Integration tests for {@link SimpleGemfireRepository}. * * @author Oliver Gierke + * @author John Blum + * @see org.junit.Test + * @see org.springframework.data.gemfire.repository.support.SimpleGemfireRepository */ @RunWith(SpringJUnit4ClassRunner.class) @ContextConfiguration("../../basic-template.xml") @@ -63,8 +66,8 @@ public class SimpleGemfireRepositoryIntegrationTest { RegionClearListener regionClearListener; - @SuppressWarnings("unchecked") @Before + @SuppressWarnings("unchecked") public void setUp() { simpleRegion.clear(); regionClearListener = new RegionClearListener(); @@ -128,6 +131,24 @@ public class SimpleGemfireRepositoryIntegrationTest { assertThat(result, not(hasItems(dave))); } + @Test + public void testSaveEntities() { + assertTrue(template.getRegion().isEmpty()); + + Person johnBlum = new Person(1l, "John", "Blum"); + Person jonBloom = new Person(2l, "Jon", "Bloom"); + Person juanBlume = new Person(3l, "Juan", "Blume"); + + repository.save(Arrays.asList(johnBlum, jonBloom, juanBlume)); + + assertFalse(template.getRegion().isEmpty()); + assertEquals(3, template.getRegion().size()); + + assertEquals(johnBlum, template.get(johnBlum.id)); + assertEquals(jonBloom, template.get(jonBloom.id)); + assertEquals(juanBlume, template.get(juanBlume.id)); + } + @SuppressWarnings("rawtypes") public static class RegionClearListener extends CacheListenerAdapter { public boolean eventFired; @@ -137,4 +158,5 @@ public class SimpleGemfireRepositoryIntegrationTest { eventFired = true; } } + } diff --git a/src/test/java/org/springframework/data/gemfire/repository/support/SimpleGemfireRepositoryUnitTest.java b/src/test/java/org/springframework/data/gemfire/repository/support/SimpleGemfireRepositoryUnitTest.java new file mode 100644 index 00000000..e06a50de --- /dev/null +++ b/src/test/java/org/springframework/data/gemfire/repository/support/SimpleGemfireRepositoryUnitTest.java @@ -0,0 +1,303 @@ +/* + * Copyright 2010-2013 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * 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; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; +import static org.mockito.Matchers.any; +import static org.mockito.Matchers.eq; +import static org.mockito.Mockito.doAnswer; +import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collection; +import java.util.HashMap; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.concurrent.atomic.AtomicLong; + +import org.junit.Test; +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.gemfire.test.support.CollectionUtils; +import org.springframework.data.repository.core.EntityInformation; + +import com.gemstone.gemfire.cache.Region; + +/** + * The SimpleGemfireRepositoryUnitTest class is a test suite of test cases testing the contract and functionality + * of the SimpleGemfireRepository class. + * + * @author John Blum + * @see org.junit.Test + * @see org.mockito.Mockito + * @see org.springframework.data.gemfire.repository.support.SimpleGemfireRepository + * @since 1.4.5 + */ +@SuppressWarnings("unchecked") +public class SimpleGemfireRepositoryUnitTest { + + protected Map asMap(Iterable animals) { + Map animalMap = new HashMap(); + for (Animal animal : animals) { + animalMap.put(animal.getId(), animal); + } + return animalMap; + } + + protected Animal createAnimal(final String name) { + Animal animal = new Animal(); + animal.setName(name); + return animal; + } + + protected Animal createAnimal(final Long id, final String name) { + Animal animal = createAnimal(name); + animal.setId(id); + return animal; + } + + protected GemfireTemplate createGemfireTemplate(final Region region) { + return new GemfireTemplate(region); + } + + protected EntityInformation mockEntityInformation() { + EntityInformation mockEntityInformation = mock(EntityInformation.class); + + doAnswer(new Answer() { + private final AtomicLong idSequence = new AtomicLong(0l); + @Override public Long answer(final InvocationOnMock invocation) throws Throwable { + Animal argument = Animal.class.cast(invocation.getArguments()[0]); + Long id = argument.getId(); + id = (id != null ? id : idSequence.incrementAndGet()); + argument.setId(id); + return id; + + } + }).when(mockEntityInformation).getId(any(Animal.class)); + + return mockEntityInformation; + } + + @Test + public void testSave() { + Region mockRegion = mock(Region.class, "testSave"); + + SimpleGemfireRepository repository = new SimpleGemfireRepository( + createGemfireTemplate(mockRegion), mockEntityInformation()); + + Animal dog = repository.save(createAnimal("dog")); + + assertNotNull(dog); + assertEquals(1l, dog.getId().longValue()); + assertEquals("dog", dog.getName()); + + verify(mockRegion, times(1)).put(eq(1l), eq(dog)); + } + + @Test + public void testSaveEntities() { + List animals = new ArrayList(3); + + animals.add(createAnimal("bird")); + animals.add(createAnimal("cat")); + animals.add(createAnimal("dog")); + + Region mockRegion = mock(Region.class, "testSaveAll"); + + SimpleGemfireRepository repository = new SimpleGemfireRepository( + createGemfireTemplate(mockRegion), mockEntityInformation()); + + Iterable savedAnimals = repository.save(animals); + + assertNotNull(savedAnimals); + + verify(mockRegion, times(1)).putAll(eq(asMap(savedAnimals))); + } + + @Test + public void testSaveWrapper() { + Animal dog = createAnimal(1l, "dog"); + + Wrapper dogWrapper = new Wrapper(dog, dog.getId()); + + Region mockRegion = mock(Region.class, "testSaveWrapper"); + + SimpleGemfireRepository repository = new SimpleGemfireRepository( + createGemfireTemplate(mockRegion), mockEntityInformation()); + + repository.save(dogWrapper); + + verify(mockRegion, times(1)).put(eq(dog.getId()), eq(dog)); + } + + @Test + public void testExists() { + final Animal dog = createAnimal(1l, "dog"); + + Region mockRegion = mock(Region.class, "testFindOne"); + + when(mockRegion.get(any(Long.class))).then(new Answer() { + @Override public Animal answer(final InvocationOnMock invocation) throws Throwable { + return (dog.getId().equals(invocation.getArguments()[0]) ? dog : null); + } + }); + + SimpleGemfireRepository repository = new SimpleGemfireRepository( + createGemfireTemplate(mockRegion), mockEntityInformation()); + + assertTrue(repository.exists(1l)); + assertFalse(repository.exists(10l)); + } + + @Test + public void testFindOne() { + final Animal dog = createAnimal(1l, "dog"); + + Region mockRegion = mock(Region.class, "testFindOne"); + + when(mockRegion.get(any(Long.class))).then(new Answer() { + @Override public Animal answer(final InvocationOnMock invocation) throws Throwable { + return (dog.getId().equals(invocation.getArguments()[0]) ? dog : null); + } + }); + + SimpleGemfireRepository repository = new SimpleGemfireRepository( + createGemfireTemplate(mockRegion), mockEntityInformation()); + + assertEquals(dog, repository.findOne(1l)); + assertNull(repository.findOne(10l)); + } + + @Test + public void testFindAll() { + long id = 0; + + final List animals = new ArrayList(3); + + animals.add(createAnimal(++id, "bird")); + animals.add(createAnimal(++id, "cat")); + animals.add(createAnimal(++id, "dog")); + + Region mockRegion = mock(Region.class, "testSaveAll"); + + when(mockRegion.getAll(any(Collection.class))).then(new Answer>() { + @Override public Map answer(final InvocationOnMock invocation) throws Throwable { + Collection keys = (Collection) invocation.getArguments()[0]; + Map result = new HashMap(keys.size()); + for (Animal animal : animals) { + if (keys.contains(animal.getId())) { + result.put(animal.getId(), animal); + } + } + return result; + } + }); + + SimpleGemfireRepository repository = new SimpleGemfireRepository( + createGemfireTemplate(mockRegion), mockEntityInformation()); + + Collection animalsFound = repository.findAll(Arrays.asList(1l, 3l)); + + assertNotNull(animalsFound); + assertEquals(2, animalsFound.size()); + assertTrue(animalsFound.containsAll(CollectionUtils.subList(animals, 0, 2))); + + verify(mockRegion, times(1)).getAll(eq(Arrays.asList(1l, 3l))); + } + + @Test + public void testDeleteById() { + Region mockRegion = mock(Region.class, "testDeleteById"); + + SimpleGemfireRepository repository = new SimpleGemfireRepository( + createGemfireTemplate(mockRegion), mockEntityInformation()); + + repository.delete(1l); + + verify(mockRegion, times(1)).remove(eq(1l)); + } + + @Test + public void testDeleteEntity() { + Region mockRegion = mock(Region.class, "testDeleteEntity"); + + SimpleGemfireRepository repository = new SimpleGemfireRepository( + createGemfireTemplate(mockRegion), mockEntityInformation()); + + repository.delete(createAnimal(1l, "dog")); + + verify(mockRegion, times(1)).remove(eq(1l)); + } + + @Test + public void testDeleteEntities() { + Region mockRegion = mock(Region.class, "testDeleteEntities"); + + SimpleGemfireRepository repository = new SimpleGemfireRepository( + createGemfireTemplate(mockRegion), mockEntityInformation()); + + repository.delete(Arrays.asList(createAnimal(1l, "bird"), createAnimal(2l, "cat"), createAnimal(3l, "dog"))); + + verify(mockRegion, times(1)).remove(eq(1l)); + verify(mockRegion, times(1)).remove(eq(2l)); + verify(mockRegion, times(1)).remove(eq(3l)); + } + + @Test + public void testDeleteAllWithClear() { + Region mockRegion = mock(Region.class, "testDeleteEntities"); + + SimpleGemfireRepository repository = new SimpleGemfireRepository( + createGemfireTemplate(mockRegion), mockEntityInformation()); + + repository.deleteAll(); + + verify(mockRegion, times(1)).clear(); + } + + @Test + public void testDeleteAllWithKeys() { + Region mockRegion = mock(Region.class, "testDeleteEntities"); + + doThrow(new UnsupportedOperationException("Not Implemented!")).when(mockRegion).clear(); + when(mockRegion.keySet()).thenReturn(new HashSet(Arrays.asList(1l, 2l, 3l))); + + SimpleGemfireRepository repository = new SimpleGemfireRepository( + createGemfireTemplate(mockRegion), mockEntityInformation()); + + repository.deleteAll(); + + verify(mockRegion, times(1)).clear(); + verify(mockRegion, times(1)).remove(eq(1l)); + verify(mockRegion, times(1)).remove(eq(2l)); + verify(mockRegion, times(1)).remove(eq(3l)); + } + +} diff --git a/src/test/java/org/springframework/data/gemfire/test/support/CollectionUtils.java b/src/test/java/org/springframework/data/gemfire/test/support/CollectionUtils.java index 86bcf4cd..768c9251 100644 --- a/src/test/java/org/springframework/data/gemfire/test/support/CollectionUtils.java +++ b/src/test/java/org/springframework/data/gemfire/test/support/CollectionUtils.java @@ -16,8 +16,10 @@ package org.springframework.data.gemfire.test.support; +import java.util.ArrayList; import java.util.Enumeration; import java.util.Iterator; +import java.util.List; /** * The CollectionUtils class is a utility class for working with the Java Collections Framework. @@ -46,4 +48,12 @@ public abstract class CollectionUtils { }; } + public static List subList(final List source, final int... indices) { + List result = new ArrayList(indices.length); + for (int index : indices) { + result.add(source.get(index)); + } + return result; + } + } diff --git a/src/test/resources/org/springframework/data/gemfire/basic-template.xml b/src/test/resources/org/springframework/data/gemfire/basic-template.xml index 695b1796..60b0e7ee 100644 --- a/src/test/resources/org/springframework/data/gemfire/basic-template.xml +++ b/src/test/resources/org/springframework/data/gemfire/basic-template.xml @@ -2,12 +2,21 @@ - + + SpringGemFireBasicTemplate + 0 + warning + + +