Skip to content

Commit 3a16d9c

Browse files
authored
Merge pull request #146 from AlexandreCarlton/dont-check-key-is-present-before-cache-map-get
Remove CacheMap#containsKey before #get
2 parents bd101a2 + 12a73d3 commit 3a16d9c

File tree

2 files changed

+16
-9
lines changed

2 files changed

+16
-9
lines changed

src/main/java/org/dataloader/CacheMap.java

+1-2
Original file line numberDiff line numberDiff line change
@@ -65,8 +65,7 @@ static <K, V> CacheMap<K, V> simpleMap() {
6565
/**
6666
* Gets the specified key from the cache map.
6767
* <p>
68-
* May throw an exception if the key does not exist, depending on the cache map implementation that is used,
69-
* so be sure to check {@link CacheMap#containsKey(Object)} first.
68+
* May throw an exception if the key does not exist, depending on the cache map implementation that is used.
7069
*
7170
* @param key the key to retrieve
7271
*

src/main/java/org/dataloader/DataLoaderHelper.java

+15-7
Original file line numberDiff line numberDiff line change
@@ -110,9 +110,13 @@ Optional<CompletableFuture<V>> getIfPresent(K key) {
110110
boolean cachingEnabled = loaderOptions.cachingEnabled();
111111
if (cachingEnabled) {
112112
Object cacheKey = getCacheKey(nonNull(key));
113-
if (futureCache.containsKey(cacheKey)) {
114-
stats.incrementCacheHitCount(new IncrementCacheHitCountStatisticsContext<>(key));
115-
return Optional.of(futureCache.get(cacheKey));
113+
try {
114+
CompletableFuture<V> cacheValue = futureCache.get(cacheKey);
115+
if (cacheValue != null) {
116+
stats.incrementCacheHitCount(new IncrementCacheHitCountStatisticsContext<>(key));
117+
return Optional.of(cacheValue);
118+
}
119+
} catch (Exception ignored) {
116120
}
117121
}
118122
}
@@ -307,10 +311,14 @@ private void possiblyClearCacheEntriesOnExceptions(List<K> keys) {
307311
private CompletableFuture<V> loadFromCache(K key, Object loadContext, boolean batchingEnabled) {
308312
final Object cacheKey = loadContext == null ? getCacheKey(key) : getCacheKeyWithContext(key, loadContext);
309313

310-
if (futureCache.containsKey(cacheKey)) {
311-
// We already have a promise for this key, no need to check value cache or queue up load
312-
stats.incrementCacheHitCount(new IncrementCacheHitCountStatisticsContext<>(key, loadContext));
313-
return futureCache.get(cacheKey);
314+
try {
315+
CompletableFuture<V> cacheValue = futureCache.get(cacheKey);
316+
if (cacheValue != null) {
317+
// We already have a promise for this key, no need to check value cache or queue up load
318+
stats.incrementCacheHitCount(new IncrementCacheHitCountStatisticsContext<>(key, loadContext));
319+
return cacheValue;
320+
}
321+
} catch (Exception ignored) {
314322
}
315323

316324
CompletableFuture<V> loadCallFuture = queueOrInvokeLoader(key, loadContext, batchingEnabled, true);

0 commit comments

Comments
 (0)