Skip to content

Commit a35cea9

Browse files
jxblumjxblum
committed
Fix bug in SimpleGemfireRepository.deleteAll running in a client/server topology with a server-side PARTITION Region.
Resolves spring-projectsgh-512.
1 parent a642fad commit a35cea9

File tree

5 files changed

+351
-33
lines changed

5 files changed

+351
-33
lines changed

‎spring-data-geode/src/main/java/org/springframework/data/gemfire/repository/support/SimpleGemfireRepository.java

+55-9
Original file line numberDiff line numberDiff line change
@@ -22,12 +22,13 @@
2222
import java.util.Map;
2323
import java.util.Objects;
2424
import java.util.Optional;
25+
import java.util.Set;
2526
import java.util.stream.Collectors;
2627
import java.util.stream.StreamSupport;
2728

28-
import org.apache.geode.cache.Cache;
2929
import org.apache.geode.cache.CacheTransactionManager;
3030
import org.apache.geode.cache.DataPolicy;
31+
import org.apache.geode.cache.GemFireCache;
3132
import org.apache.geode.cache.Region;
3233
import org.apache.geode.cache.query.SelectResults;
3334

@@ -41,6 +42,7 @@
4142
import org.springframework.data.gemfire.repository.query.QueryString;
4243
import org.springframework.data.gemfire.repository.query.support.PagingUtils;
4344
import org.springframework.data.gemfire.util.CollectionUtils;
45+
import org.springframework.data.gemfire.util.RegionUtils;
4446
import org.springframework.data.gemfire.util.SpringUtils;
4547
import org.springframework.data.repository.CrudRepository;
4648
import org.springframework.data.repository.PagingAndSortingRepository;
@@ -140,6 +142,9 @@ public SimpleGemfireRepository(@NonNull GemfireTemplate template,
140142
return this.template;
141143
}
142144

145+
/**
146+
* @inheritDoc
147+
*/
143148
@Override
144149
public <U extends T> U save(@NonNull U entity) {
145150

@@ -155,6 +160,9 @@ public <U extends T> U save(@NonNull U entity) {
155160
return entity;
156161
}
157162

163+
/**
164+
* @inheritDoc
165+
*/
158166
@Override
159167
public T save(@NonNull Wrapper<T, ID> wrapper) {
160168

@@ -170,6 +178,9 @@ public T save(@NonNull Wrapper<T, ID> wrapper) {
170178
return entity;
171179
}
172180

181+
/**
182+
* @inheritDoc
183+
*/
173184
@Override
174185
public <U extends T> Iterable<U> saveAll(@NonNull Iterable<U> entities) {
175186

@@ -223,6 +234,9 @@ public boolean existsById(ID id) {
223234
return findById(id).isPresent();
224235
}
225236

237+
/**
238+
* @inheritDoc
239+
*/
226240
@Override
227241
public @NonNull Iterable<T> findAll() {
228242

@@ -234,6 +248,9 @@ public boolean existsById(ID id) {
234248
return toList(selectResults);
235249
}
236250

251+
/**
252+
* @inheritDoc
253+
*/
237254
@Override
238255
public Page<T> findAll(@NonNull Pageable pageable) {
239256

@@ -244,6 +261,9 @@ public Page<T> findAll(@NonNull Pageable pageable) {
244261
return toPage(results, pageable);
245262
}
246263

264+
/**
265+
* @inheritDoc
266+
*/
247267
@Override
248268
public @NonNull Iterable<T> findAll(@NonNull Sort sort) {
249269

@@ -256,6 +276,9 @@ public Page<T> findAll(@NonNull Pageable pageable) {
256276
return toList(selectResults);
257277
}
258278

279+
/**
280+
* @inheritDoc
281+
*/
259282
@Override
260283
public @NonNull Iterable<T> findAllById(@NonNull Iterable<ID> ids) {
261284

@@ -274,6 +297,9 @@ public Page<T> findAll(@NonNull Pageable pageable) {
274297
return values;
275298
}
276299

300+
/**
301+
* @inheritDoc
302+
*/
277303
@Override
278304
public Optional<T> findById(@NonNull ID id) {
279305

@@ -284,11 +310,17 @@ public Optional<T> findById(@NonNull ID id) {
284310
return Optional.ofNullable(value);
285311
}
286312

313+
/**
314+
* @inheritDoc
315+
*/
287316
@Override
288317
public void delete(@NonNull T entity) {
289318
deleteById(getEntityInformation().getRequiredId(entity));
290319
}
291320

321+
/**
322+
* @inheritDoc
323+
*/
292324
@Override
293325
public void deleteAll() {
294326

@@ -305,39 +337,53 @@ public void deleteAll() {
305337
});
306338
}
307339

340+
/**
341+
* @inheritDoc
342+
*/
308343
@Override
309344
public void deleteAll(@NonNull Iterable<? extends T> entities) {
310345
CollectionUtils.nullSafeIterable(entities).forEach(this::delete);
311346
}
312347

348+
/**
349+
* @inheritDoc
350+
*/
313351
@Override
314352
public void deleteById(@NonNull ID id) {
315353
getTemplate().remove(id);
316354
}
317355

318-
boolean isPartitioned(Region<?, ?> region) {
356+
boolean isPartitioned(@Nullable Region<?, ?> region) {
319357

320358
return region != null
321359
&& region.getAttributes() != null
322360
&& isPartitioned(region.getAttributes().getDataPolicy());
323361
}
324362

325-
boolean isPartitioned(DataPolicy dataPolicy) {
363+
boolean isPartitioned(@Nullable DataPolicy dataPolicy) {
326364
return dataPolicy != null && dataPolicy.withPartitioning();
327365
}
328366

329-
boolean isTransactionPresent(Region<?, ?> region) {
367+
boolean isTransactionPresent(@Nullable Region<?, ?> region) {
330368

331-
return region.getRegionService() instanceof Cache
332-
&& isTransactionPresent(((Cache) region.getRegionService()).getCacheTransactionManager());
369+
return region != null
370+
&& region.getRegionService() instanceof GemFireCache
371+
&& isTransactionPresent(((GemFireCache) region.getRegionService()).getCacheTransactionManager());
333372
}
334373

335-
boolean isTransactionPresent(CacheTransactionManager cacheTransactionManager) {
374+
boolean isTransactionPresent(@Nullable CacheTransactionManager cacheTransactionManager) {
336375
return cacheTransactionManager != null && cacheTransactionManager.exists();
337376
}
338377

339-
<K> void doRegionClear(Region<K, ?> region) {
340-
region.removeAll(region.keySet());
378+
<K> void doRegionClear(@NonNull Region<K, ?> region) {
379+
region.removeAll(resolveRegionKeys(region));
380+
}
381+
382+
@NonNull <K> Set<K> resolveRegionKeys(@NonNull Region<K, ?> region) {
383+
384+
return RegionUtils.isClient(region) ? region.keySetOnServer()
385+
: RegionUtils.isServer(region) ? region.keySet()
386+
: Collections.emptySet();
341387
}
342388

343389
@NonNull List<T> toList(@Nullable Iterable<T> iterable) {

‎spring-data-geode/src/main/java/org/springframework/data/gemfire/util/CacheUtils.java

+19-15
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,8 @@
2828
import org.apache.geode.distributed.DistributedSystem;
2929
import org.apache.geode.internal.cache.GemFireCacheImpl;
3030

31+
import org.springframework.lang.NonNull;
32+
import org.springframework.lang.Nullable;
3133
import org.springframework.util.StringUtils;
3234

3335
/**
@@ -51,10 +53,9 @@ public abstract class CacheUtils extends DistributedSystemUtils {
5153

5254
public static final String DEFAULT_POOL_NAME = "DEFAULT";
5355

54-
@SuppressWarnings("all")
55-
public static boolean isClient(GemFireCache cache) {
56+
public static boolean isClient(@Nullable GemFireCache cache) {
5657

57-
boolean client = (cache instanceof ClientCache);
58+
boolean client = cache instanceof ClientCache;
5859

5960
if (cache instanceof GemFireCacheImpl) {
6061
client &= ((GemFireCacheImpl) cache).isClient();
@@ -63,23 +64,27 @@ public static boolean isClient(GemFireCache cache) {
6364
return client;
6465
}
6566

66-
public static boolean isDefaultPool(Pool pool) {
67-
return Optional.ofNullable(pool).map(Pool::getName).filter(CacheUtils::isDefaultPool).isPresent();
67+
public static boolean isDefaultPool(@Nullable Pool pool) {
68+
69+
return Optional.ofNullable(pool)
70+
.map(Pool::getName)
71+
.filter(CacheUtils::isDefaultPool)
72+
.isPresent();
6873
}
6974

70-
public static boolean isNotDefaultPool(Pool pool) {
75+
public static boolean isNotDefaultPool(@Nullable Pool pool) {
7176
return !isDefaultPool(pool);
7277
}
7378

74-
public static boolean isDefaultPool(String poolName) {
79+
public static boolean isDefaultPool(@Nullable String poolName) {
7580
return DEFAULT_POOL_NAME.equals(poolName);
7681
}
7782

78-
public static boolean isNotDefaultPool(String poolName) {
83+
public static boolean isNotDefaultPool(@Nullable String poolName) {
7984
return !isDefaultPool(poolName);
8085
}
8186

82-
public static boolean isDurable(ClientCache clientCache) {
87+
public static boolean isDurable(@Nullable ClientCache clientCache) {
8388

8489
// NOTE: Technically, the following code snippet would be more useful/valuable but is not "testable"!
8590
//((InternalDistributedSystem) distributedSystem).getConfig().getDurableClientId();
@@ -93,10 +98,9 @@ public static boolean isDurable(ClientCache clientCache) {
9398
.isPresent();
9499
}
95100

96-
@SuppressWarnings("all")
97-
public static boolean isPeer(GemFireCache cache) {
101+
public static boolean isPeer(@Nullable GemFireCache cache) {
98102

99-
boolean peer = (cache instanceof Cache);
103+
boolean peer = cache instanceof Cache;
100104

101105
if (cache instanceof GemFireCacheImpl) {
102106
peer &= !((GemFireCacheImpl) cache).isClient();
@@ -109,17 +113,17 @@ public static boolean close() {
109113
return close(resolveGemFireCache());
110114
}
111115

112-
public static boolean close(GemFireCache gemfireCache) {
116+
public static boolean close(@NonNull GemFireCache gemfireCache) {
113117
return close(gemfireCache, () -> {});
114118
}
115119

116-
public static boolean close(GemFireCache gemfireCache, Runnable shutdownHook) {
120+
public static boolean close(@NonNull GemFireCache gemfireCache, @Nullable Runnable shutdownHook) {
117121

118122
try {
119123
gemfireCache.close();
120124
return true;
121125
}
122-
catch (Exception ignore) {
126+
catch (Throwable ignore) {
123127
return false;
124128
}
125129
finally {

‎spring-data-geode/src/main/java/org/springframework/data/gemfire/util/RegionUtils.java

+12-1
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,7 @@ public static boolean close(Region<?, ?> region) {
113113
* @return a boolean indicating whether the target {@link Region} is a {@literal client} {@link Region}.
114114
* @see org.apache.geode.cache.Region
115115
*/
116-
public static boolean isClient(Region<?, ?> region) {
116+
public static boolean isClient(@Nullable Region<?, ?> region) {
117117

118118
return Optional.ofNullable(region)
119119
.map(Region::getAttributes)
@@ -175,4 +175,15 @@ public static String toRegionPath(@Nullable Region<?, ?> region) {
175175
public static String toRegionPath(String regionName) {
176176
return String.format("%1$s%2$s", Region.SEPARATOR, regionName);
177177
}
178+
179+
/**
180+
* Determines whether the target {@link Region} is a {@literal server-side} {@link Region}.
181+
*
182+
* @param region {@link Region} to evaluate.
183+
* @return a boolean indicating whether the target {@link Region} is a {@literal server-side} {@link Region}.
184+
* @see org.apache.geode.cache.Region
185+
*/
186+
public static boolean isServer(@Nullable Region<?, ?> region) {
187+
return region != null && !isClient(region);
188+
}
178189
}

0 commit comments

Comments
 (0)