Migrated to pure Java CompletableFuture<List<V>>

Author: bbakerman

README.md

@@ -5,14 +5,14 @@
 [ ![Download](https://api.bintray.com/packages/bbakerman/maven/java-dataloader/images/download.svg) ](https://bintray.com/bbakerman/maven/java-dataloader/_latestVersion)
 
 
-This small and simple utility library is a Pure Java 8 port of [Facebook DataLoader](https://github.com/facebook/dataloader). 
+This small and simple utility library is a pure Java 8 port of [Facebook DataLoader](https://github.com/facebook/dataloader). 
 
 It can serve as integral part of your application's data layer to provide a
 consistent API over various back-ends and reduce message communication overhead through batching and caching.
 
 An important use case for `java-dataloader` is improving the efficiency of GraphQL query execution.  Graphql fields
 are resolved in a independent manner and with a true graph of objects, you may be fetching the same object many times.  
-Also a naive implementation of graphql data fetchers can easily lead to the dreaded  "n+1" fetch problem. 
+A naive implementation of graphql data fetchers can easily lead to the dreaded  "n+1" fetch problem. 
 
 There are many other use cases where you can also benefit from using this utility.
 
@@ -67,15 +67,13 @@ a list of keys
 
         BatchLoader<Long, User> userBatchLoader = new BatchLoader<Long, User>() {
             @Override
-            public PromisedValues<User> load(List<Long> userIds) {
-                List<CompletableFuture<User>> futures = userIds.stream()
-                        .map(userId ->
-                                CompletableFuture.supplyAsync(() ->
-                                        userManager.loadUsersById(userId)))
-                        .collect(Collectors.toList());
-                return PromisedValues.allOf(futures);
+            public CompletionStage<List<User>> load(List<Long> userIds) {
+                return CompletableFuture.supplyAsync(() -> {
+                    return userManager.loadUsersById(userIds);
+                });
             }
         };
+
         DataLoader<Long, User> userLoader = new DataLoader<>(userBatchLoader);
 
 ```
@@ -88,6 +86,7 @@ You can then use it to load values which will be `CompleteableFuture` promises t
 
 or you can use it to compose future computations as follows.  The key requirement is that you call
 `dataloader.dispatch()` or its variant `dataloader.dispatchAndJoin()` at some point in order to make the underlying calls happen to the batch loader.
+
 In this version of data loader, this does not happen automatically.  More on this in [Manual dispatching](#manual-dispatching) .
 
 ```java
@@ -126,12 +125,47 @@ maintain minimal outgoing data requests.
 
 In the example above, the first call to dispatch will cause the batched user keys (1 and 2) to be fired at the BatchLoader function to load 2 users.
 
-Since each `thenAccept` callback made more calls to `userLoader` to get the "user they they invited", another 2 user keys are fired at the BatchLoader function for them.    
+Since each `thenAccept` callback made more calls to `userLoader` to get the "user they they invited", another 2 user keys are given at the `BatchLoader` 
+function for them.    
 
-In this case the `userLoader.dispatchAndJoin()` is used to fire a dispatch call, wait for it (aka join it), see if the data loader has more batched entries, (which is does)
-and then it repeats this until the data loader internal queue of keys is empty.  At this point we have made 2 batched calls instead of the niave 4 calls we might have made if
+In this case the `userLoader.dispatchAndJoin()` is used to make a dispatch call, wait for it (aka join it), see if the data loader has more batched entries, (which is does)
+and then it repeats this until the data loader internal queue of keys is empty.  At this point we have made 2 batched calls instead of the naive 4 calls we might have made if
 we did not "batch" the calls to load data.
 
+## Batching requires batch backing APIs
+
+You will notice in our BatchLoader example that the backing service had the ability to get a list of users give a list of user ids in one call.
+ 
+```java
+            public CompletionStage<List<User>> load(List<Long> userIds) {
+                return CompletableFuture.supplyAsync(() -> {
+                    return userManager.loadUsersById(userIds);
+                });
+            }
+```
+ 
+ This is important consideration.  By using `dataloader` you have batched up the request for N keys in a list of keys that can be retrieved at one time.
+ If you don't have batch backing services, then you cant be as efficient as possible if you then have to make N calls for each key.
+ 
+ ```java
+        BatchLoader<Long, User> lessEfficientUserBatchLoader = new BatchLoader<Long, User>() {
+            @Override
+            public CompletionStage<List<User>> load(List<Long> userIds) {
+                return CompletableFuture.supplyAsync(() -> {
+                    //
+                    // notice how it makes N calls to load by single user id out of the batch of N keys
+                    //
+                    return userIds.stream()
+                            .map(id -> userManager.loadUserById(id))
+                            .collect(Collectors.toList());
+                });
+            }
+        };
+
+```
+ 
+That said, with key caching turn on (the default), it may still be more efficient using `dataloader` than without it
+
 ## Differences to reference implementation
 
 ### Manual dispatching
@@ -162,6 +196,23 @@ and there are also gains to this different mode of operation:
 However, with batch execution control comes responsibility! If you forget to make the call to `dispatch()` then the futures
 in the load request queue will never be batched, and thus _will never complete_! So be careful when crafting your loader designs.
 
+### Error object is not a thing in Java
+
+In the reference JS implementation if the batch loader returns an `Error` object back then the `loadKey()` promise is rejected
+with that error.  This allows fine grain (per object in the list)  sets of error.  If I ask for keys A,B,C and B errors out the promise
+for B can contain a specific eror. 
+
+This is not (easily) possible in a Java implementation
+
+A batch loader function is defined as `BatchLoader<K, V>` meaning for a key of type `K` it returns a value of type `V`.  It can just return
+some `Exception` as an object of type `V` since Java is type safe.
+  
+We could have made it return an `List<Either<V,Exception>>` but this greatly complicates the method signatures and so this aspect was not ported.
+   
+Instead when the batch loader function is called, it returns a promise of `List<V>` and if that promise fails in aggregate then that is what
+is reported back.   
+
+
 ## Let's get started!
 
 ### Installing
@@ -214,13 +265,15 @@ itself.  All the heavy lifting has been done by this project : [vertx-dataloader
 including the extensive testing.
 
 This particular port was done to reduce the dependency on Vertx and to write a pure Java 8 implementation with no dependencies and also
-to use the more normative Java CompletableFuture.  [vertx-core](http://vertx.io/docs/vertx-core/java/) is not a lightweight library by any means
+to use the more normative Java CompletableFuture.  
+
+[vertx-core](http://vertx.io/docs/vertx-core/java/) is not a lightweight library by any means
 so having a pure Java 8 implementation is very desirable.
 
 
 This library is entirely inspired by the great works of [Lee Byron](https://github.com/leebyron) and
 [Nicholas Schrock](https://github.com/schrockn) from [Facebook](https://www.facebook.com/) whom we would like to thank, and
-especially @leebyron for taking the time and effort to provide 100% coverage on the codebase. A set of tests which
+especially @leebyron for taking the time and effort to provide 100% coverage on the codebase. The original set of tests 
 were also ported.
 
 

src/main/java/org/dataloader/BatchLoader.java

@@ -17,18 +17,27 @@
 package org.dataloader;
 
 import java.util.List;
+import java.util.concurrent.CompletionStage;
 
 /**
  * A function that is invoked for batch loading a list of data values indicated by the provided list of keys. The
- * function returns a {@link PromisedValues} to aggregate results of individual load requests.
+ * function returns a promise of a list of results of individual load requests.
  *
  * There are a few constraints that must be upheld:
  * <ul>
  * <li>The list of values must be the same size as the list of keys.</li>
  * <li>Each index in the list of values must correspond to the same index in the list of keys.</li>
  * </ul>
  *
- * For example, if your batch function was provided the list of keys: [ 2, 9, 6, 1 ], and loading from a back-end service returned the values:
+ * For example, if your batch function was provided the list of keys:
+ *
+ * <pre>
+ *  [
+ *      2, 9, 6, 1
+ *  ]
+ * </pre>
+ *
+ * and loading from a back-end service returned this list of  values:
  *
  * <pre>
  *  [
@@ -38,10 +47,13 @@
  *  ]
  * </pre>
  *
- * The back-end service returned results in a different order than we requested, likely because it was more efficient for it to do so. Also, it omitted a result for key 6, which we can interpret as no value
- * existing for that key.
+ * then the batch loader function contract has been broken.
+ *
+ * The back-end service returned results in a different order than we requested, likely because it was more efficient for it to
+ * do so. Also, it omitted a result for key 6, which we may interpret as no value existing for that key.
  *
- * To uphold the constraints of the batch function, it must return an List of values the same length as the List of keys, and re-order them to ensure each index aligns with the original keys [ 2, 9, 6, 1 ]:
+ * To uphold the constraints of the batch function, it must return an List of values the same length as
+ * the List of keys, and re-order them to ensure each index aligns with the original keys [ 2, 9, 6, 1 ]:
  *
  * <pre>
  *  [
@@ -56,16 +68,17 @@
  * @param <V> type parameter indicating the type of values returned
  *
  * @author <a href="https://github.com/aschrijver/">Arnold Schrijver</a>
+ * @author <a href="https://github.com/bbakerman/">Brad Baker</a>
  */
 @FunctionalInterface
 public interface BatchLoader<K, V> {
 
     /**
-     * Called to batch load the provided keys and return a {@link PromisedValues} which is a promise to a list of values
+     * Called to batch load the provided keys and return a promise to a list of values
      *
      * @param keys the collection of keys to load
      *
      * @return a promise of the values for those keys
      */
-    PromisedValues<V> load(List<K> keys);
+    CompletionStage<List<V>> load(List<K> keys);
 }

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

@@ -33,6 +33,7 @@
  * @param <V> type parameter indicating the type of the data that is cached
  *
  * @author <a href="https://github.com/aschrijver/">Arnold Schrijver</a>
+ * @author <a href="https://github.com/bbakerman/">Brad Baker</a>
  */
 public interface CacheMap<U, V> {
 
@@ -41,6 +42,7 @@ public interface CacheMap<U, V> {
      *
      * @param <U> type parameter indicating the type of the cache keys
      * @param <V> type parameter indicating the type of the data that is cached
+     *
      * @return the cache map
      */
     static <U, V> CacheMap<U, CompletableFuture<V>> simpleMap() {
@@ -51,6 +53,7 @@ static <U, V> CacheMap<U, CompletableFuture<V>> simpleMap() {
      * Checks whether the specified key is contained in the cach map.
      *
      * @param key the key to check
+     *
      * @return {@code true} if the cache contains the key, {@code false} otherwise
      */
     boolean containsKey(U key);
@@ -62,6 +65,7 @@ static <U, V> CacheMap<U, CompletableFuture<V>> simpleMap() {
      * so be sure to check {@link CacheMap#containsKey(Object)} first.
      *
      * @param key the key to retrieve
+     *
      * @return the cached value, or {@code null} if not found (depends on cache implementation)
      */
     V get(U key);
@@ -71,6 +75,7 @@ static <U, V> CacheMap<U, CompletableFuture<V>> simpleMap() {
      *
      * @param key   the key to cache
      * @param value the value to cache
+     *
      * @return the cache map for fluent coding
      */
     CacheMap<U, V> set(U key, V value);
@@ -79,6 +84,7 @@ static <U, V> CacheMap<U, CompletableFuture<V>> simpleMap() {
      * Deletes the entry with the specified key from the cache map, if it exists.
      *
      * @param key the key to delete
+     *
      * @return the cache map for fluent coding
      */
     CacheMap<U, V> delete(U key);