On Fri, 21 Mar 2025 13:45:24 GMT, Volkan Yazici <vyaz...@openjdk.org> wrote:
> Adds `HttpResponse::connectionLabel` method that provides an identifier for
> the connection.
>
> **Implementation note:** The feature is facilitated by
> `HttpConnection::label`, which should not be confused with
> `HttpConnection::id`. This distinction is explained in the JavaDoc of both
> properties.
src/java.net.http/share/classes/jdk/internal/net/http/HttpResponseImpl.java
line 47:
> 45:
> 46: final int responseCode;
> 47: private final Optional<String> connectionLabel;
There's a precedent for `previousResponse` below, but we could fix that in a
different cleanup PR, and avoid introducing new fields of type `Optional`.
Suggestion:
private final String connectionLabel;
src/java.net.http/share/classes/jdk/internal/net/http/HttpResponseImpl.java
line 63:
> 61: Exchange<T> exch) {
> 62: this.responseCode = response.statusCode();
> 63: this.connectionLabel = connectionLabel(exch);
Suggestion:
this.connectionLabel = connectionLabel(exch).orElse(null);
src/java.net.http/share/classes/jdk/internal/net/http/HttpResponseImpl.java
line 88:
> 86: return Optional.empty();
> 87: }
> 88: return Optional.of(connection.label());
Suggestion:
return Optional.ofNullable(exchange)
.map(e -> e.exchImpl)
.map(ExchangeImpl::connection)
.map(HttpConnection::label);
src/java.net.http/share/classes/jdk/internal/net/http/HttpResponseImpl.java
line 98:
> 96: @Override
> 97: public Optional<String> connectionLabel() {
> 98: return connectionLabel;
Suggestion:
return Optional.ofNullable(connectionLabel);
test/jdk/java/net/httpclient/HttpResponseConnectionLabelTest.java line 87:
> 85: // Primary server-client pairs
> 86:
> 87: private static final ServerRequestPair PRI_HTTP1 =
> ServerRequestPair.of(HttpClient.Version.HTTP_1_1, false);
Add an import for `HttpClient.Version`
test/jdk/java/net/httpclient/HttpResponseConnectionLabelTest.java line 243:
> 241: if (HttpClient.Version.HTTP_2.equals(pair.server.getVersion())) {
> 242: return;
> 243: }
Instead you could send a first request to warm-up the client/server and get a
connection in the pool.
Then you could assert that if the version is HTTP/2, the connectionLabels are
identical (instead of different)
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/24154#discussion_r2007835413
PR Review Comment: https://git.openjdk.org/jdk/pull/24154#discussion_r2007833459
PR Review Comment: https://git.openjdk.org/jdk/pull/24154#discussion_r2007828641
PR Review Comment: https://git.openjdk.org/jdk/pull/24154#discussion_r2007836560
PR Review Comment: https://git.openjdk.org/jdk/pull/24154#discussion_r2007786550
PR Review Comment: https://git.openjdk.org/jdk/pull/24154#discussion_r2007803802
