diff --git a/README.md b/README.md index 227c921..4845a28 100644 --- a/README.md +++ b/README.md @@ -217,15 +217,15 @@ For example, let's assume you want to load users from a database, you could prob create the list of results, with nulls filling in for missing values. ```java - MapBatchLoader mapBatchLoader = new MapBatchLoader() { + MappedBatchLoaderWithContext mapBatchLoader = new MappedBatchLoaderWithContext() { @Override - public CompletionStage> load(List userIds, BatchLoaderEnvironment environment) { + public CompletionStage> load(Set userIds, BatchLoaderEnvironment environment) { SecurityCtx callCtx = environment.getContext(); - return CompletableFuture.supplyAsync(() -> userManager.loadMapOfUsersById(callCtx, userIds)); + return CompletableFuture.supplyAsync(() -> userManager.loadMapOfUsersByIds(callCtx, userIds)); } }; - DataLoader userLoader = DataLoader.newDataLoader(mapBatchLoader); + DataLoader userLoader = DataLoader.newMappedDataLoader(mapBatchLoader); // ... ``` diff --git a/src/main/java/org/dataloader/DataLoaderHelper.java b/src/main/java/org/dataloader/DataLoaderHelper.java index 277b810..d1e4be5 100644 --- a/src/main/java/org/dataloader/DataLoaderHelper.java +++ b/src/main/java/org/dataloader/DataLoaderHelper.java @@ -6,8 +6,10 @@ import java.util.ArrayList; import java.util.Collection; import java.util.HashMap; +import java.util.LinkedHashSet; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.concurrent.CompletableFuture; import java.util.concurrent.CompletionStage; import java.util.stream.Collectors; @@ -274,10 +276,11 @@ private CompletionStage> invokeListBatchLoader(List keys, BatchLoader @SuppressWarnings("unchecked") private CompletionStage> invokeMapBatchLoader(List keys, BatchLoaderEnvironment environment) { CompletionStage> loadResult; + Set setOfKeys = new LinkedHashSet<>(keys); if (batchLoadFunction instanceof MappedBatchLoaderWithContext) { - loadResult = ((MappedBatchLoaderWithContext) batchLoadFunction).load(keys, environment); + loadResult = ((MappedBatchLoaderWithContext) batchLoadFunction).load(setOfKeys, environment); } else { - loadResult = ((MappedBatchLoader) batchLoadFunction).load(keys); + loadResult = ((MappedBatchLoader) batchLoadFunction).load(setOfKeys); } CompletionStage> mapBatchLoad = nonNull(loadResult, "Your batch loader function MUST return a non null CompletionStage promise"); return mapBatchLoad.thenApply(map -> { diff --git a/src/main/java/org/dataloader/MappedBatchLoader.java b/src/main/java/org/dataloader/MappedBatchLoader.java index b230e7f..4b489fa 100644 --- a/src/main/java/org/dataloader/MappedBatchLoader.java +++ b/src/main/java/org/dataloader/MappedBatchLoader.java @@ -18,19 +18,19 @@ import java.util.List; import java.util.Map; +import java.util.Set; import java.util.concurrent.CompletionStage; /** - * A function that is invoked for batch loading a map of of data values indicated by the provided list of keys. The + * A function that is invoked for batch loading a map of of data values indicated by the provided set of keys. The * function returns a promise of a map of results of individual load requests. *

* There are a few constraints that must be upheld: *

    *
  • The keys MUST be able to be first class keys in a Java map. Get your equals() and hashCode() methods in order
  • - *
  • The caller of the {@link org.dataloader.DataLoader} that uses this batch loader function MUSt be able to cope with + *
  • The caller of the {@link org.dataloader.DataLoader} that uses this batch loader function MUST be able to cope with * null values coming back as results *
  • - *
  • The function MUST be resilient to the same key being presented twice.
  • *
*

* This form is useful when you don't have a 1:1 mapping of keys to values or when null is an acceptable value for a missing value. @@ -50,9 +50,6 @@ *

* This means that if 10 keys are asked for then {@link DataLoader#dispatch()} will return a promise of 10 value results and each * of the {@link org.dataloader.DataLoader#load(Object)} will complete with a value, null or an exception. - *

- * When caching is disabled, its possible for the same key to be presented in the list of keys more than once. Your map - * batch loader function needs to be resilient to this situation. * * @param type parameter indicating the type of keys to use for data load requests. * @param type parameter indicating the type of values returned @@ -63,10 +60,10 @@ public interface MappedBatchLoader { /** * Called to batch load the provided keys and return a promise to a map of values. * - * @param keys the collection of keys to load + * @param keys the set of keys to load * * @return a promise to a map of values for those keys */ @SuppressWarnings("unused") - CompletionStage> load(List keys); + CompletionStage> load(Set keys); } diff --git a/src/main/java/org/dataloader/MappedBatchLoaderWithContext.java b/src/main/java/org/dataloader/MappedBatchLoaderWithContext.java index a899fae..6e3a2f0 100644 --- a/src/main/java/org/dataloader/MappedBatchLoaderWithContext.java +++ b/src/main/java/org/dataloader/MappedBatchLoaderWithContext.java @@ -18,6 +18,7 @@ import java.util.List; import java.util.Map; +import java.util.Set; import java.util.concurrent.CompletionStage; /** @@ -32,11 +33,11 @@ public interface MappedBatchLoaderWithContext { /** * Called to batch load the provided keys and return a promise to a map of values. * - * @param keys the collection of keys to load + * @param keys the set of keys to load * @param environment the calling environment * * @return a promise to a map of values for those keys */ @SuppressWarnings("unused") - CompletionStage> load(List keys, BatchLoaderEnvironment environment); + CompletionStage> load(Set keys, BatchLoaderEnvironment environment); } diff --git a/src/test/java/ReadmeExamples.java b/src/test/java/ReadmeExamples.java index 0f0d18d..0803728 100644 --- a/src/test/java/ReadmeExamples.java +++ b/src/test/java/ReadmeExamples.java @@ -15,6 +15,7 @@ import java.util.ArrayList; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.concurrent.CompletableFuture; import java.util.concurrent.CompletionStage; import java.util.stream.Collectors; @@ -105,9 +106,9 @@ private CompletionStage> callDatabaseForResults(SecurityCtx callCtx private void mapBatchLoader() { MappedBatchLoaderWithContext mapBatchLoader = new MappedBatchLoaderWithContext() { @Override - public CompletionStage> load(List userIds, BatchLoaderEnvironment environment) { + public CompletionStage> load(Set userIds, BatchLoaderEnvironment environment) { SecurityCtx callCtx = environment.getContext(); - return CompletableFuture.supplyAsync(() -> userManager.loadMapOfUsersById(callCtx, userIds)); + return CompletableFuture.supplyAsync(() -> userManager.loadMapOfUsersByIds(callCtx, userIds)); } }; diff --git a/src/test/java/org/dataloader/DataLoaderMapBatchLoaderTest.java b/src/test/java/org/dataloader/DataLoaderMapBatchLoaderTest.java index 8e83f82..1a436c1 100644 --- a/src/test/java/org/dataloader/DataLoaderMapBatchLoaderTest.java +++ b/src/test/java/org/dataloader/DataLoaderMapBatchLoaderTest.java @@ -9,6 +9,7 @@ import java.util.Map; import java.util.concurrent.CompletableFuture; import java.util.concurrent.ExecutionException; +import java.util.concurrent.atomic.AtomicInteger; import static java.util.Arrays.asList; import static java.util.Collections.singletonList; @@ -33,12 +34,14 @@ public class DataLoaderMapBatchLoaderTest { MappedBatchLoader evensOnlyMappedBatchLoader = (keys) -> { Map mapOfResults = new HashMap<>(); - for (int i = 0; i < keys.size(); i++) { - String k = keys.get(i); + + AtomicInteger index = new AtomicInteger(); + keys.forEach(k -> { + int i = index.getAndIncrement(); if (i % 2 == 0) { mapOfResults.put(k, k); } - } + }); return CompletableFuture.completedFuture(mapOfResults); }; @@ -153,7 +156,9 @@ public void should_work_with_duplicate_keys_when_caching_disabled() throws Execu assertThat(future1.get(), equalTo("A")); assertThat(future2.get(), equalTo("B")); assertThat(future3.get(), equalTo("A")); - assertThat(loadCalls, equalTo(singletonList(asList("A", "B", "A")))); + + // the map batch functions use a set of keys as input and hence remove duplicates unlike list variant + assertThat(loadCalls, equalTo(singletonList(asList("A", "B")))); } @Test diff --git a/src/test/java/org/dataloader/DataLoaderWithTryTest.java b/src/test/java/org/dataloader/DataLoaderWithTryTest.java index b926841..b2127e6 100644 --- a/src/test/java/org/dataloader/DataLoaderWithTryTest.java +++ b/src/test/java/org/dataloader/DataLoaderWithTryTest.java @@ -8,6 +8,7 @@ import java.util.List; import java.util.Map; import java.util.concurrent.CompletableFuture; +import java.util.stream.Collectors; import static java.util.Arrays.asList; import static java.util.Collections.singletonList; @@ -45,7 +46,7 @@ public void should_handle_Trys_coming_back_from_mapped_batchLoader() throws Exce List> batchKeyCalls = new ArrayList<>(); MappedBatchLoaderWithContext> batchLoader = (keys, environment) -> { - batchKeyCalls.add(keys); + batchKeyCalls.add(new ArrayList<>(keys)); Map> result = new HashMap<>(); for (String key : keys) { diff --git a/src/test/java/org/dataloader/fixtures/UserManager.java b/src/test/java/org/dataloader/fixtures/UserManager.java index 95b712e..24fee0d 100644 --- a/src/test/java/org/dataloader/fixtures/UserManager.java +++ b/src/test/java/org/dataloader/fixtures/UserManager.java @@ -4,6 +4,7 @@ import java.util.LinkedHashMap; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.stream.Collectors; @SuppressWarnings({"unused", "SpellCheckingInspection", "NonAsciiCharacters"}) @@ -51,7 +52,7 @@ public List loadUsersById(List userIds) { return userIds.stream().map(this::loadUserById).collect(Collectors.toList()); } - public Map loadMapOfUsersById(SecurityCtx callCtx, List userIds) { + public Map loadMapOfUsersByIds(SecurityCtx callCtx, Set userIds) { Map map = new HashMap<>(); userIds.forEach(userId -> { User user = loadUserById(userId);