Skip to content

Commit 5523c12

Browse files
committed
DATAGEODE-4 - Polish.
1 parent d018192 commit 5523c12

16 files changed

+92
-79
lines changed

src/main/java/org/springframework/data/gemfire/GemfireCacheUtils.java

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@
2626
import org.apache.geode.IncompatibleSystemException;
2727
import org.apache.geode.InternalGemFireException;
2828
import org.apache.geode.InvalidValueException;
29-
import org.apache.geode.LicenseException;
3029
import org.apache.geode.NoSystemException;
3130
import org.apache.geode.SystemConnectException;
3231
import org.apache.geode.SystemIsRunningException;
@@ -194,9 +193,6 @@ public static DataAccessException convertGemfireAccessException(GemFireException
194193
if (ex instanceof CopyException) {
195194
return new GemfireSystemException(ex);
196195
}
197-
if (ex instanceof org.apache.geode.cache.EntryNotFoundInRegion) {
198-
return new DataRetrievalFailureException(ex.getMessage(), ex);
199-
}
200196
if (ex instanceof FunctionException) {
201197
return new InvalidDataAccessApiUsageException(ex.getMessage(), ex);
202198
}
@@ -224,9 +220,6 @@ public static DataAccessException convertGemfireAccessException(GemFireException
224220
if (ex instanceof LeaseExpiredException) {
225221
return new PessimisticLockingFailureException(ex.getMessage(), ex);
226222
}
227-
if (ex instanceof LicenseException) {
228-
return new GemfireSystemException(ex);
229-
}
230223
if (ex instanceof NoSystemException) {
231224
return new GemfireSystemException(ex);
232225
}

src/main/java/org/springframework/data/gemfire/repository/GemfireRepository.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,17 +13,17 @@
1313
* See the License for the specific language governing permissions and
1414
* limitations under the License.
1515
*/
16+
1617
package org.springframework.data.gemfire.repository;
1718

1819
import org.springframework.data.domain.Sort;
1920
import org.springframework.data.repository.CrudRepository;
2021

2122
/**
22-
* Gemfire-specific extension of the {@link CrudRepository} interface.
23+
* GemFire specific extension of the Spring Data {@link CrudRepository} interface.
2324
*
2425
* @author Oliver Gierke
2526
* @author John Blum
26-
* @see java.io.Serializable
2727
* @see org.springframework.data.repository.CrudRepository
2828
*/
2929
public interface GemfireRepository<T, ID> extends CrudRepository<T, ID> {

src/main/java/org/springframework/data/gemfire/repository/Wrapper.java

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,19 +13,21 @@
1313
* See the License for the specific language governing permissions and
1414
* limitations under the License.
1515
*/
16+
1617
package org.springframework.data.gemfire.repository;
1718

1819
import lombok.NonNull;
1920
import lombok.Value;
2021

2122
/**
22-
* Simple value object to hold an entity alongside an external key the entity shall be stored under.
23-
*
23+
* Simple value object holding an entity along with the external key in which the entity will be mapped.
24+
*
2425
* @author Oliver Gierke
2526
*/
2627
@Value
2728
public class Wrapper<T, KEY> {
2829

2930
T entity;
3031
@NonNull KEY key;
32+
3133
}

src/main/java/org/springframework/data/gemfire/repository/query/DefaultGemfireEntityInformation.java

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,8 @@
2121
import org.springframework.data.repository.core.support.PersistentEntityInformation;
2222

2323
/**
24-
* Implementation of {@link GemfireEntityInformation} to return the region name stored in the backing
25-
* {@link PersistentEntity}.
24+
* Implementation of {@link GemfireEntityInformation} and Spring Data's {@link PersistentEntityInformation}
25+
* that returns the Region name associated with the {@link PersistentEntity}.
2626
*
2727
* @author Oliver Gierke
2828
* @author John Blum
@@ -53,5 +53,4 @@ public DefaultGemfireEntityInformation(GemfirePersistentEntity<T> entity) {
5353
public String getRegionName() {
5454
return entity.getRegionName();
5555
}
56-
5756
}

src/main/java/org/springframework/data/gemfire/repository/query/GemfireEntityInformation.java

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,15 +13,17 @@
1313
* See the License for the specific language governing permissions and
1414
* limitations under the License.
1515
*/
16+
1617
package org.springframework.data.gemfire.repository.query;
1718

1819
import org.apache.geode.cache.Region;
1920
import org.springframework.data.repository.core.EntityInformation;
2021

2122
/**
22-
* {@link EntityInformation} to capture Gemfire specific information.
23+
* {@link EntityInformation} capturing GemFire specific information.
2324
*
2425
* @author Oliver Gierke
26+
* @see org.springframework.data.repository.core.EntityInformation
2527
*/
2628
public interface GemfireEntityInformation<T, ID> extends EntityInformation<T, ID> {
2729

@@ -31,4 +33,5 @@ public interface GemfireEntityInformation<T, ID> extends EntityInformation<T, ID
3133
* @return the name of the {@link Region} the entity is held in.
3234
*/
3335
String getRegionName();
36+
3437
}

src/main/java/org/springframework/data/gemfire/repository/support/GemfireRepositoryFactory.java

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,8 @@
1616

1717
package org.springframework.data.gemfire.repository.support;
1818

19-
import static org.springframework.data.gemfire.util.RuntimeExceptionFactory.*;
19+
import static org.springframework.data.gemfire.util.RuntimeExceptionFactory.newIllegalArgumentException;
20+
import static org.springframework.data.gemfire.util.RuntimeExceptionFactory.newIllegalStateException;
2021

2122
import java.io.Serializable;
2223
import java.lang.reflect.Method;
@@ -82,9 +83,10 @@ public GemfireRepositoryFactory(Iterable<Region<?, ?>> regions,
8283
@Override
8384
@SuppressWarnings("unchecked")
8485
public <T, ID> GemfireEntityInformation<T, ID> getEntityInformation(Class<T> domainClass) {
85-
86+
8687
GemfirePersistentEntity<T> entity = (GemfirePersistentEntity<T>) mappingContext.getPersistentEntity(domainClass)
87-
.orElseThrow(() -> newIllegalArgumentException("Unable to resolve PersistentEntity for type [%s]", domainClass));
88+
.orElseThrow(() -> newIllegalArgumentException("Unable to resolve PersistentEntity for type [%s]",
89+
domainClass));
8890

8991
return new DefaultGemfireEntityInformation<>(entity);
9092
}
@@ -95,37 +97,39 @@ public <T, ID> GemfireEntityInformation<T, ID> getEntityInformation(Class<T> dom
9597
*/
9698
@Override
9799
protected Object getTargetRepository(RepositoryInformation repositoryInformation) {
98-
GemfireEntityInformation<?, Serializable> entityInformation = getEntityInformation(
99-
repositoryInformation.getDomainType());
100+
101+
GemfireEntityInformation<?, Serializable> entityInformation =
102+
getEntityInformation(repositoryInformation.getDomainType());
100103

101104
GemfireTemplate gemfireTemplate = getTemplate(repositoryInformation);
102105

103106
return getTargetRepositoryViaReflection(repositoryInformation, gemfireTemplate, entityInformation);
104107
}
105108

106109
GemfireTemplate getTemplate(RepositoryMetadata metadata) {
110+
107111
GemfirePersistentEntity<?> entity = mappingContext.getPersistentEntity(metadata.getDomainType())
108112
.orElseThrow(() -> newIllegalArgumentException("Unable to resolve PersistentEntity for type [%s]",
109113
metadata.getDomainType()));
110114

111115
String entityRegionName = entity.getRegionName();
112116
String repositoryRegionName = getRepositoryRegionName(metadata.getRepositoryInterface());
113-
String regionName = (StringUtils.hasText(repositoryRegionName) ? repositoryRegionName : entityRegionName);
117+
String resolvedRegionName = StringUtils.hasText(repositoryRegionName) ? repositoryRegionName : entityRegionName;
114118

115-
Region<?, ?> region = regions.getRegion(regionName);
119+
Region<?, ?> region = regions.getRegion(resolvedRegionName);
116120

117121
if (region == null) {
118-
throw new IllegalStateException(String.format("No Region '%1$s' found for domain class %2$s;"
122+
throw newIllegalStateException("No Region [%1$s] was found for domain class [%2$s];"
119123
+ " Make sure you have configured a GemFire Region of that name in your application context",
120-
regionName, metadata.getDomainType().getName()));
124+
resolvedRegionName, metadata.getDomainType().getName());
121125
}
122126

123127
Class<?> regionKeyType = region.getAttributes().getKeyConstraint();
124128
Class<?> entityIdType = metadata.getIdType();
125129

126130
if (regionKeyType != null && entity.getIdProperty() != null) {
127131
Assert.isTrue(regionKeyType.isAssignableFrom(entityIdType), String.format(
128-
"The Region referenced only supports keys of type %1$s, but the entity to be stored has an id of type %2$s",
132+
"The Region referenced only supports keys of type [%1$s], but the entity to be stored has an id of type [%2$s]",
129133
regionKeyType.getName(), entityIdType.getName()));
130134
}
131135

src/main/java/org/springframework/data/gemfire/repository/support/SimpleGemfireRepository.java

Lines changed: 7 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@
1616

1717
package org.springframework.data.gemfire.repository.support;
1818

19-
import java.io.Serializable;
2019
import java.util.Collection;
2120
import java.util.HashMap;
2221
import java.util.List;
@@ -54,7 +53,7 @@
5453
* @see org.apache.geode.cache.Cache
5554
* @see org.apache.geode.cache.Region
5655
*/
57-
public class SimpleGemfireRepository<T, ID extends Serializable> implements GemfireRepository<T, ID> {
56+
public class SimpleGemfireRepository<T, ID> implements GemfireRepository<T, ID> {
5857

5958
private final EntityInformation<T, ID> entityInformation;
6059

@@ -80,7 +79,6 @@ public SimpleGemfireRepository(GemfireTemplate template, EntityInformation<T, ID
8079
*/
8180
@Override
8281
public <U extends T> U save(U entity) {
83-
8482
ID id = entityInformation.getRequiredId(entity);
8583

8684
template.put(id, entity);
@@ -94,13 +92,10 @@ public <U extends T> U save(U entity) {
9492
*/
9593
@Override
9694
public <U extends T> Iterable<U> saveAll(Iterable<U> entities) {
97-
9895
Map<ID, U> entitiesToSave = new HashMap<>();
9996

100-
entities.forEach(entity -> {
101-
entitiesToSave.put(entityInformation.getRequiredId(entity), entity);
102-
});
103-
97+
entities.forEach(entity -> entitiesToSave.put(entityInformation.getRequiredId(entity), entity));
98+
10499
template.putAll(entitiesToSave);
105100

106101
return entitiesToSave.values();
@@ -113,7 +108,9 @@ public <U extends T> Iterable<U> saveAll(Iterable<U> entities) {
113108
@Override
114109
public T save(Wrapper<T, ID> wrapper) {
115110
T entity = wrapper.getEntity();
111+
116112
template.put(wrapper.getKey(), entity);
113+
117114
return entity;
118115
}
119116

@@ -180,10 +177,9 @@ public Iterable<T> findAll(Sort sort) {
180177
*/
181178
@Override
182179
public Collection<T> findAllById(Iterable<ID> ids) {
183-
184-
List<ID> parameters = Streamable.of(ids).stream().collect(StreamUtils.toUnmodifiableList());
180+
List<ID> keys = Streamable.of(ids).stream().collect(StreamUtils.toUnmodifiableList());
185181

186-
return CollectionUtils.<ID, T>nullSafeMap(template.getAll(parameters)).values().stream()
182+
return CollectionUtils.<ID, T>nullSafeMap(template.getAll(keys)).values().stream()
187183
.filter(Objects::nonNull).collect(Collectors.toList());
188184
}
189185

src/test/java/org/springframework/data/gemfire/client/GemFireDataSourceIntegrationTests.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,10 @@
1010
* an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the
1111
* specific language governing permissions and limitations under the License.
1212
*/
13+
1314
package org.springframework.data.gemfire.client;
1415

15-
import static org.assertj.core.api.Assertions.*;
16+
import static org.assertj.core.api.Assertions.assertThat;
1617

1718
import java.util.Arrays;
1819
import java.util.List;

src/test/java/org/springframework/data/gemfire/repository/sample/AnimalRepositoryTest.java

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616

1717
package org.springframework.data.gemfire.repository.sample;
1818

19-
import static org.assertj.core.api.Assertions.*;
19+
import static org.assertj.core.api.Assertions.assertThat;
2020

2121
import java.util.Optional;
2222

@@ -27,20 +27,20 @@
2727
import org.springframework.test.context.junit4.SpringRunner;
2828

2929
/**
30-
* The AnimalRepositoryTest class is a test suite of test cases testing the functionality behind PR #55 involving
31-
* persisting application domain object/entities to multiple Regions in GemFire's Cache.
30+
* Integration test testing the functionality behind PR #55 involving persisting application domain object/entities
31+
* to multiple Regions in a GemFire Cache.
3232
*
3333
* @author Stuart Williams
3434
* @author John Blum
3535
* @see org.junit.Test
3636
* @see org.junit.runner.RunWith
3737
* @see org.springframework.test.context.ContextConfiguration
38-
* @see org.springframework.test.context.junit4.SpringJUnit4ClassRunner
39-
* @since 1.4.0
38+
* @see org.springframework.test.context.junit4.SpringRunner
4039
* @link https://github.com/spring-projects/spring-data-gemfire/pull/55
40+
* @since 1.4.0
4141
*/
42-
@ContextConfiguration
4342
@RunWith(SpringRunner.class)
43+
@ContextConfiguration
4444
@SuppressWarnings("unused")
4545
public class AnimalRepositoryTest {
4646

@@ -58,7 +58,7 @@ protected static Animal newAnimal(long id, String name) {
5858
}
5959

6060
@Test
61-
public void testEntityStoredInMultipleRegions() {
61+
public void entityStoredInMultipleRegionsIsSuccessful() {
6262
Animal felix = newAnimal(1, "Felix");
6363
Animal leo = newAnimal(2, "Leo");
6464
Animal cerberus = newAnimal(3, "Cerberus");

src/test/java/org/springframework/data/gemfire/repository/sample/IncompatibleRegionKeyEntityIdAnimalRepositoryTest.java

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -39,15 +39,16 @@
3939
*/
4040
public class IncompatibleRegionKeyEntityIdAnimalRepositoryTest {
4141

42-
protected static final String APPLICATION_CONTEXT_CONFIG_LOCATION = String.format("%1$s%2$s%1$s%3$s",
42+
private static final String APPLICATION_CONTEXT_CONFIG_LOCATION = String.format("%1$s%2$s%1$s%3$s",
4343
File.separator, AnimalRepositoryTest.class.getPackage().getName().replace('.', File.separatorChar),
44-
"IncompatibleRegionKeyEntityIdAnimalRepositoryTest-context.xml");
44+
String.format("%s-context.xml", IncompatibleRegionKeyEntityIdAnimalRepositoryTest.class.getSimpleName()));
4545

4646
@Test(expected = IllegalArgumentException.class)
4747
public void storeAnimalHavingLongIdInRabbitsRegionWithStringKey() {
4848
try {
49-
ConfigurableApplicationContext applicationContext = new ClassPathXmlApplicationContext(
50-
APPLICATION_CONTEXT_CONFIG_LOCATION);
49+
ConfigurableApplicationContext applicationContext =
50+
new ClassPathXmlApplicationContext(APPLICATION_CONTEXT_CONFIG_LOCATION);
51+
5152
applicationContext.getBean(RabbitRepository.class);
5253
}
5354
// NOTE the ClassCastException thrown from GemFire is unexpected; this is not correct and the identifying type
@@ -62,8 +63,9 @@ public void storeAnimalHavingLongIdInRabbitsRegionWithStringKey() {
6263
catch (BeanCreationException expected) {
6364
//expected.printStackTrace(System.err);
6465
assertTrue(expected.getCause() instanceof IllegalArgumentException);
65-
assertEquals(String.format("The Region referenced only supports keys of type %1$s, but the entity to be stored has an id of type %2$s",
66+
assertEquals(String.format("The Region referenced only supports keys of type [%1$s], but the entity to be stored has an id of type [%2$s]",
6667
String.class.getName(), Long.class.getName()), expected.getCause().getMessage());
68+
6769
throw (IllegalArgumentException) expected.getCause();
6870
}
6971
}

src/test/java/org/springframework/data/gemfire/repository/sample/PlantRepositoryTest.java

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -38,24 +38,26 @@
3838
*/
3939
public class PlantRepositoryTest {
4040

41-
protected static final String APPLICATION_CONTEXT_CONFIG_LOCATION = String.format("%1$s%2$s%1$s%3$s",
41+
private static final String APPLICATION_CONTEXT_CONFIG_LOCATION = String.format("%1$s%2$s%1$s%3$s",
4242
File.separator, PlantRepositoryTest.class.getPackage().getName().replace('.', File.separatorChar),
4343
"PlantRepositoryTest-context.xml");
4444

4545
@Test(expected = IllegalArgumentException.class)
4646
public void storePlantHavingStringIdInPlantsRegionWithLongKey() {
4747
try {
48-
ConfigurableApplicationContext context = new ClassPathXmlApplicationContext(
49-
APPLICATION_CONTEXT_CONFIG_LOCATION);
48+
ConfigurableApplicationContext context =
49+
new ClassPathXmlApplicationContext(APPLICATION_CONTEXT_CONFIG_LOCATION);
50+
5051
context.getBean(PlantRepository.class);
5152
}
5253
// NOTE technically, the IllegalArgumentException for incompatible Region 'Key' and Entity ID is thrown
5354
// when the Spring container starts up and the Repository beans are created.
5455
catch (BeanCreationException expected) {
5556
//expected.printStackTrace(System.err);
5657
assertTrue(expected.getCause() instanceof IllegalArgumentException);
57-
assertEquals(String.format("The Region referenced only supports keys of type %1$s, but the entity to be stored has an id of type %2$s",
58+
assertEquals(String.format("The Region referenced only supports keys of type [%1$s], but the entity to be stored has an id of type [%2$s]",
5859
Long.class.getName(), String.class.getName()), expected.getCause().getMessage());
60+
5961
throw (IllegalArgumentException) expected.getCause();
6062
}
6163
}

0 commit comments

Comments
 (0)