8217627: HttpClient: The API documentation of BodySubscribers::mapping promotes bad behavior

The API documentation is updated to steer away from blocking in the mapper function, and an alternative is suggested.

Reviewed-by: chegar

src/java.net.http/share/classes/java/net/http/HttpResponse.java

@@ -47,6 +47,7 @@ import java.util.concurrent.Flow.Publisher;
 import java.util.concurrent.Flow.Subscription;
 import java.util.function.Consumer;
 import java.util.function.Function;
+import java.util.function.Supplier;
 import java.util.stream.Stream;
 import javax.net.ssl.SSLSession;
 import jdk.internal.net.http.BufferingSubscriber;
@@ -1282,17 +1283,26 @@ public interface HttpResponse<T> {
          *
          * <p> The mapping function is executed using the client's {@linkplain
          * HttpClient#executor() executor}, and can therefore be used to map any
-         * response body type, including blocking {@link InputStream}, as shown
+         * response body type, including blocking {@link InputStream}.
-         * in the following example which uses a well-known JSON parser to
+         * However, performing any blocking operation in the mapper function
+         * runs the risk of blocking the executor's thread for an unknown
+         * amount of time (at least until the blocking operation finishes),
+         * which may end up starving the executor of available threads.
+         * Therefore, in the case where mapping to the desired type might
+         * block (e.g. by reading on the {@code InputStream}), then mapping
+         * to a {@link java.util.function.Supplier Supplier} of the desired
+         * type and deferring the blocking operation until {@link Supplier#get()
+         * Supplier::get} is invoked by the caller's thread should be preferred,
+         * as shown in the following example which uses a well-known JSON parser to
          * convert an {@code InputStream} into any annotated Java type.
          *
          * <p>For example:
-         * <pre> {@code  public static <W> BodySubscriber<W> asJSON(Class<W> targetType) {
+         * <pre> {@code  public static <W> BodySubscriber<Supplier<W>> asJSON(Class<W> targetType) {
          *     BodySubscriber<InputStream> upstream = BodySubscribers.ofInputStream();
          *
-         *     BodySubscriber<W> downstream = BodySubscribers.mapping(
+         *     BodySubscriber<Supplier<W>> downstream = BodySubscribers.mapping(
          *           upstream,
-         *           (InputStream is) -> {
+         *           (InputStream is) -> () -> {
          *               try (InputStream stream = is) {
          *                   ObjectMapper objectMapper = new ObjectMapper();
          *                   return objectMapper.readValue(stream, targetType);
@@ -1301,7 +1311,7 @@ public interface HttpResponse<T> {
          *               }
          *           });
          *    return downstream;
-         * } }</pre>
+         *  } }</pre>
          *
          * @param <T> the upstream body type
          * @param <U> the type of the body subscriber returned

test/jdk/java/net/httpclient/examples/JavadocExamples.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2018, 2019, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -24,6 +24,8 @@
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.PrintStream;
+import java.io.UncheckedIOException;
+import java.lang.reflect.UndeclaredThrowableException;
 import java.net.Authenticator;
 import java.net.InetSocketAddress;
 import java.net.ProxySelector;
@@ -36,6 +38,7 @@ import java.net.http.HttpRequest.BodyPublishers;
 import java.net.http.HttpResponse;
 import java.net.http.HttpResponse.BodyHandler;
 import java.net.http.HttpResponse.BodyHandlers;
+import java.net.http.HttpResponse.BodySubscriber;
 import java.net.http.HttpResponse.BodySubscribers;
 import java.nio.file.Path;
 import java.nio.file.Paths;
@@ -44,6 +47,7 @@ import java.util.Collections;
 import java.util.List;
 import java.util.concurrent.CopyOnWriteArrayList;
 import java.util.concurrent.Flow;
+import java.util.function.Supplier;
 import java.util.regex.Pattern;
 import static java.nio.charset.StandardCharsets.UTF_8;
 
@@ -177,6 +181,11 @@ public class JavadocExamples {
                 .send(request, responseInfo ->
                         BodySubscribers.mapping(BodySubscribers.ofString(UTF_8), String::getBytes));
 
+        // Maps an InputStream to a Foo object.
+        HttpResponse<Supplier<Foo>> response9 = client.send(request,
+                (resp) -> FromMappingSubscriber.asJSON(Foo.class));
+        String resp = response9.body().get().asString();
+
     }
 
     /**
@@ -290,4 +299,51 @@ public class JavadocExamples {
         }
     }
 
+    public static class Foo {
+        byte[] bytes;
+        public Foo(byte[] bytes) {
+            this.bytes = bytes;
+        }
+        public String asString() {
+            return new String(bytes, UTF_8);
+        }
+    }
+
+    static class ObjectMapper {
+        <W> W readValue(InputStream is, Class<W> targetType)
+            throws IOException
+        {
+                byte[] bytes = is.readAllBytes();
+                return map(bytes, targetType);
+        }
+
+        static <W> W map(byte[] bytes, Class<W> targetType) {
+            try {
+                return targetType.getConstructor(byte[].class).newInstance(bytes);
+            } catch (RuntimeException | Error x) {
+                throw x;
+            } catch (Exception x) {
+                throw new UndeclaredThrowableException(x);
+            }
+        }
+    }
+
+    static class FromMappingSubscriber {
+        public static <W> BodySubscriber<Supplier<W>> asJSON(Class<W> targetType) {
+            BodySubscriber<InputStream> upstream = BodySubscribers.ofInputStream();
+
+            BodySubscriber<Supplier<W>> downstream = BodySubscribers.mapping(
+                    upstream, (InputStream is) -> () -> {
+                        try (InputStream stream = is) {
+                            ObjectMapper objectMapper = new ObjectMapper();
+                            return objectMapper.readValue(stream, targetType);
+                        } catch (IOException e) {
+                            throw new UncheckedIOException(e);
+                        }
+                    });
+            return downstream;
+        }
+
+    }
+
 }