Skip to content

Commit d8078e0

Browse files
bbakermanbbakerman
committed
more tests and more resilient setCache calls
1 parent 38b3755 commit d8078e0

File tree

3 files changed

+121
-22
lines changed

3 files changed

+121
-22
lines changed

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

Lines changed: 20 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -455,20 +455,26 @@ private CompletableFuture<List<Try<V>>> getFromValueCache(List<K> keys) {
455455
}
456456

457457
private CompletionStage<List<V>> setToValueCache(List<K> keys, CompletionStage<List<V>> assembledValues) {
458-
boolean completeValueAfterCacheSet = loaderOptions.getValueCacheOptions().isCompleteValueAfterCacheSet();
459-
if (completeValueAfterCacheSet) {
460-
return assembledValues.thenCompose(values -> nonNull(valueCache
461-
.setValues(keys, values), () -> "Your ValueCache.setValues function MUST return a non null promise")
462-
// we dont trust the set cache to give us the values back - we have them - lets use them
463-
// if the cache set fails - then they wont be in cache and maybe next time they will
464-
.handle((ignored, setExIgnored) -> values));
465-
} else {
466-
return assembledValues.thenApply(values -> {
467-
// no one is waiting for the set to happen here so if its truly async
468-
// it will happen eventually but no result will be dependant on it
469-
valueCache.setValues(keys, values);
470-
return values;
471-
});
458+
try {
459+
boolean completeValueAfterCacheSet = loaderOptions.getValueCacheOptions().isCompleteValueAfterCacheSet();
460+
if (completeValueAfterCacheSet) {
461+
return assembledValues.thenCompose(values -> nonNull(valueCache
462+
.setValues(keys, values), () -> "Your ValueCache.setValues function MUST return a non null promise")
463+
// we dont trust the set cache to give us the values back - we have them - lets use them
464+
// if the cache set fails - then they wont be in cache and maybe next time they will
465+
.handle((ignored, setExIgnored) -> values));
466+
} else {
467+
return assembledValues.thenApply(values -> {
468+
// no one is waiting for the set to happen here so if its truly async
469+
// it will happen eventually but no result will be dependant on it
470+
valueCache.setValues(keys, values);
471+
return values;
472+
});
473+
}
474+
} catch (RuntimeException ignored) {
475+
// if we cant set values back into the cache - so be it - this must be a faulty
476+
// ValueCache implementation
477+
return assembledValues;
472478
}
473479
}
474480

‎src/test/java/org/dataloader/DataLoaderValueCacheTest.java‎

Lines changed: 94 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44
import com.github.benmanes.caffeine.cache.Caffeine;
55
import org.dataloader.fixtures.CaffeineValueCache;
66
import org.dataloader.fixtures.CustomValueCache;
7-
import org.dataloader.fixtures.TestKit;
87
import org.dataloader.impl.DataLoaderAssertionException;
98
import org.junit.Test;
109

@@ -20,6 +19,8 @@
2019
import static org.awaitility.Awaitility.await;
2120
import static org.dataloader.DataLoaderOptions.newOptions;
2221
import static org.dataloader.fixtures.TestKit.idLoader;
22+
import static org.dataloader.fixtures.TestKit.snooze;
23+
import static org.dataloader.fixtures.TestKit.sort;
2324
import static org.dataloader.impl.CompletableFutureKit.failedFuture;
2425
import static org.hamcrest.Matchers.equalTo;
2526
import static org.junit.Assert.assertArrayEquals;
@@ -48,6 +49,7 @@ public void test_by_default_we_have_no_value_caching() {
4849
assertThat(loadCalls, equalTo(singletonList(asList("a", "b"))));
4950

5051
// futures are still cached but not values
52+
loadCalls.clear();
5153

5254
fA = identityLoader.load("a");
5355
fB = identityLoader.load("b");
@@ -59,8 +61,7 @@ public void test_by_default_we_have_no_value_caching() {
5961
assertThat(fA.join(), equalTo("a"));
6062
assertThat(fB.join(), equalTo("b"));
6163

62-
assertThat(loadCalls, equalTo(singletonList(asList("a", "b"))));
63-
64+
assertThat(loadCalls, equalTo(emptyList()));
6465
}
6566

6667
@Test
@@ -213,12 +214,12 @@ public void caching_can_take_some_time_complete() {
213214
public CompletableFuture<Object> get(String key) {
214215
if (key.startsWith("miss")) {
215216
return CompletableFuture.supplyAsync(() -> {
216-
TestKit.snooze(1000);
217+
snooze(1000);
217218
throw new IllegalStateException("no a in cache");
218219
});
219220
} else {
220221
return CompletableFuture.supplyAsync(() -> {
221-
TestKit.snooze(1000);
222+
snooze(1000);
222223
return key;
223224
});
224225
}
@@ -261,7 +262,7 @@ public CompletableFuture<List<Try<Object>>> getValues(List<String> keys) {
261262
}
262263
}
263264
return CompletableFuture.supplyAsync(() -> {
264-
TestKit.snooze(1000);
265+
snooze(1000);
265266
return cacheCalls;
266267
});
267268
}
@@ -364,4 +365,91 @@ public CompletableFuture<Object> get(String key) {
364365
assertThat(getCalls.get(), equalTo(0));
365366
assertTrue(customValueCache.asMap().isEmpty());
366367
}
368+
369+
@Test
370+
public void if_everything_is_cached_no_batching_happens() {
371+
AtomicInteger getCalls = new AtomicInteger();
372+
AtomicInteger setCalls = new AtomicInteger();
373+
CustomValueCache customValueCache = new CustomValueCache() {
374+
375+
@Override
376+
public CompletableFuture<Object> get(String key) {
377+
getCalls.incrementAndGet();
378+
return super.get(key);
379+
}
380+
381+
@Override
382+
public CompletableFuture<List<Object>> setValues(List<String> keys, List<Object> values) {
383+
setCalls.incrementAndGet();
384+
return super.setValues(keys, values);
385+
}
386+
};
387+
customValueCache.asMap().put("a", "cachedA");
388+
customValueCache.asMap().put("b", "cachedB");
389+
customValueCache.asMap().put("c", "cachedC");
390+
391+
List<List<String>> loadCalls = new ArrayList<>();
392+
DataLoaderOptions options = newOptions().setValueCache(customValueCache).setCachingEnabled(true);
393+
DataLoader<String, String> identityLoader = idLoader(options, loadCalls);
394+
395+
CompletableFuture<String> fA = identityLoader.load("a");
396+
CompletableFuture<String> fB = identityLoader.load("b");
397+
CompletableFuture<String> fC = identityLoader.load("c");
398+
399+
await().until(identityLoader.dispatch()::isDone);
400+
401+
assertThat(fA.join(), equalTo("cachedA"));
402+
assertThat(fB.join(), equalTo("cachedB"));
403+
assertThat(fC.join(), equalTo("cachedC"));
404+
405+
assertThat(loadCalls, equalTo(emptyList()));
406+
assertThat(getCalls.get(), equalTo(3));
407+
assertThat(setCalls.get(), equalTo(0));
408+
}
409+
410+
411+
@Test
412+
public void if_batching_is_off_it_still_can_cache() {
413+
AtomicInteger getCalls = new AtomicInteger();
414+
AtomicInteger setCalls = new AtomicInteger();
415+
CustomValueCache customValueCache = new CustomValueCache() {
416+
417+
@Override
418+
public CompletableFuture<Object> get(String key) {
419+
getCalls.incrementAndGet();
420+
return super.get(key);
421+
}
422+
423+
@Override
424+
public CompletableFuture<List<Object>> setValues(List<String> keys, List<Object> values) {
425+
setCalls.incrementAndGet();
426+
return super.setValues(keys, values);
427+
}
428+
};
429+
customValueCache.asMap().put("a", "cachedA");
430+
431+
List<List<String>> loadCalls = new ArrayList<>();
432+
DataLoaderOptions options = newOptions().setValueCache(customValueCache).setCachingEnabled(true).setBatchingEnabled(false);
433+
DataLoader<String, String> identityLoader = idLoader(options, loadCalls);
434+
435+
CompletableFuture<String> fA = identityLoader.load("a");
436+
CompletableFuture<String> fB = identityLoader.load("b");
437+
CompletableFuture<String> fC = identityLoader.load("c");
438+
439+
assertTrue(fA.isDone()); // with batching off they are dispatched immediately
440+
assertTrue(fB.isDone());
441+
assertTrue(fC.isDone());
442+
443+
await().until(identityLoader.dispatch()::isDone);
444+
445+
assertThat(fA.join(), equalTo("cachedA"));
446+
assertThat(fB.join(), equalTo("b"));
447+
assertThat(fC.join(), equalTo("c"));
448+
449+
assertThat(loadCalls, equalTo(asList(singletonList("b"), singletonList("c"))));
450+
assertThat(getCalls.get(), equalTo(3));
451+
assertThat(setCalls.get(), equalTo(2));
452+
453+
assertThat(sort(customValueCache.asMap().values()), equalTo(sort(asList("b", "c", "cachedA"))));
454+
}
367455
}

‎src/test/java/org/dataloader/fixtures/TestKit.java‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,8 @@
99
import java.util.Collection;
1010
import java.util.List;
1111
import java.util.concurrent.CompletableFuture;
12-
import java.util.stream.Collectors;
1312

13+
import static java.util.stream.Collectors.toList;
1414
import static org.dataloader.impl.CompletableFutureKit.failedFuture;
1515

1616
public class TestKit {
@@ -26,7 +26,7 @@ public static <K, V> BatchLoader<K, V> keysAsValues(List<List<K>> loadCalls) {
2626
@SuppressWarnings("unchecked")
2727
List<V> values = keys.stream()
2828
.map(k -> (V) k)
29-
.collect(Collectors.toList());
29+
.collect(toList());
3030
return CompletableFuture.completedFuture(values);
3131
};
3232
}
@@ -62,4 +62,9 @@ public static void snooze(int millis) {
6262
throw new RuntimeException(e);
6363
}
6464
}
65+
66+
67+
public static <T> List<T> sort(Collection<? extends T> collection) {
68+
return collection.stream().sorted().collect(toList());
69+
}
6570
}

0 commit comments

Comments
 (0)