Merge pull request #146 from AlexandreCarlton/dont-check-key-is-prese…

…nt-before-cache-map-get Remove CacheMap#containsKey before #get
graphql-java · Apr 29, 2024 · 3a16d9c · 3a16d9c
2 parents bd101a2 + 12a73d3
commit 3a16d9c
Show file tree

Hide file tree

Showing 2 changed files with 16 additions and 9 deletions.
diff --git a/src/main/java/org/dataloader/CacheMap.java b/src/main/java/org/dataloader/CacheMap.java
@@ -65,8 +65,7 @@ static <K, V> CacheMap<K, V> simpleMap() {
     /**
      * Gets the specified key from the cache map.
      * <p>
-     * May throw an exception if the key does not exist, depending on the cache map implementation that is used,
-     * so be sure to check {@link CacheMap#containsKey(Object)} first.
+     * May throw an exception if the key does not exist, depending on the cache map implementation that is used.
      *
      * @param key the key to retrieve
      *

diff --git a/src/main/java/org/dataloader/DataLoaderHelper.java b/src/main/java/org/dataloader/DataLoaderHelper.java
@@ -110,9 +110,13 @@ Optional<CompletableFuture<V>> getIfPresent(K key) {
             boolean cachingEnabled = loaderOptions.cachingEnabled();
             if (cachingEnabled) {
                 Object cacheKey = getCacheKey(nonNull(key));
-                if (futureCache.containsKey(cacheKey)) {
-                    stats.incrementCacheHitCount(new IncrementCacheHitCountStatisticsContext<>(key));
-                    return Optional.of(futureCache.get(cacheKey));
+                try {
+                    CompletableFuture<V> cacheValue = futureCache.get(cacheKey);
+                    if (cacheValue != null) {
+                        stats.incrementCacheHitCount(new IncrementCacheHitCountStatisticsContext<>(key));
+                        return Optional.of(cacheValue);
+                    }
+                } catch (Exception ignored) {
                 }
             }
         }
@@ -307,10 +311,14 @@ private void possiblyClearCacheEntriesOnExceptions(List<K> keys) {
     private CompletableFuture<V> loadFromCache(K key, Object loadContext, boolean batchingEnabled) {
         final Object cacheKey = loadContext == null ? getCacheKey(key) : getCacheKeyWithContext(key, loadContext);
 
-        if (futureCache.containsKey(cacheKey)) {
-            // We already have a promise for this key, no need to check value cache or queue up load
-            stats.incrementCacheHitCount(new IncrementCacheHitCountStatisticsContext<>(key, loadContext));
-            return futureCache.get(cacheKey);
+        try {
+            CompletableFuture<V> cacheValue = futureCache.get(cacheKey);
+            if (cacheValue != null) {
+                // We already have a promise for this key, no need to check value cache or queue up load
+                stats.incrementCacheHitCount(new IncrementCacheHitCountStatisticsContext<>(key, loadContext));
+                return cacheValue;
+            }
+        } catch (Exception ignored) {
         }
 
         CompletableFuture<V> loadCallFuture = queueOrInvokeLoader(key, loadContext, batchingEnabled, true);