8217627: HttpClient: The API documentation of BodySubscribers::mapping promotes bad behavior
Summary: The API documentation is updated to steer away from blocking in the mapper function, and an alternative is suggested.
Reviewed-by: chegar
--- a/src/java.net.http/share/classes/java/net/http/HttpResponse.java Fri Jan 25 18:12:06 2019 +0100
+++ b/src/java.net.http/share/classes/java/net/http/HttpResponse.java Fri Jan 25 18:13:25 2019 +0000
@@ -47,6 +47,7 @@
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 @@
*
* <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
- * in the following example which uses a well-known JSON parser to
+ * response body type, including blocking {@link InputStream}.
+ * 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 @@
* }
* });
* return downstream;
- * } }</pre>
+ * } }</pre>
*
* @param <T> the upstream body type
* @param <U> the type of the body subscriber returned
--- a/test/jdk/java/net/httpclient/examples/JavadocExamples.java Fri Jan 25 18:12:06 2019 +0100
+++ b/test/jdk/java/net/httpclient/examples/JavadocExamples.java Fri Jan 25 18:13:25 2019 +0000
@@ -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.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.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 @@
.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 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;
+ }
+
+ }
+
}