Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Attempt to read an unconsumed response entity to allow connection caching #8943

Merged
merged 2 commits into from
Jul 16, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2022, 2023 Oracle and/or its affiliates.
* Copyright (c) 2022, 2024 Oracle and/or its affiliates.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -17,6 +17,7 @@
package io.helidon.webclient.http1;

import java.io.InputStream;
import java.lang.System.Logger.Level;
import java.util.List;
import java.util.OptionalLong;
import java.util.ServiceLoader;
Expand All @@ -27,6 +28,7 @@
import io.helidon.common.HelidonServiceLoader;
import io.helidon.common.LazyValue;
import io.helidon.common.buffers.BufferData;
import io.helidon.common.buffers.DataReader;
import io.helidon.common.media.type.ParserMode;
import io.helidon.http.ClientRequestHeaders;
import io.helidon.http.ClientResponseHeaders;
Expand Down Expand Up @@ -153,7 +155,7 @@ public void close() {
if (headers().contains(HeaderValues.CONNECTION_CLOSE)) {
connection.closeResource();
} else {
if (entityFullyRead || entityLength == 0) {
if (entityFullyRead || entityLength == 0 || consumeUnreadEntity()) {
connection.releaseResource();
} else {
connection.closeResource();
Expand Down Expand Up @@ -186,6 +188,34 @@ ClientConnection connection() {
return connection;
}

/**
* Attempts to consume an unread entity for the purpose of re-using a cached
* connection. Only works for length-prefixed responses and when the entity
* has been loaded and has not been partially read. This method shall never
* block on a read operation.
*
* @return {@code true} if consumed, {@code false} otherwise
*/
private boolean consumeUnreadEntity() {
if (entityLength == ENTITY_LENGTH_CHUNKED) {
return false;
}
DataReader reader = connection.reader();
if (reader.available() != entityLength) {
return false;
}
try {
for (long i = 0; i < entityLength; i++) {
spericas marked this conversation as resolved.
Show resolved Hide resolved
reader.read();
}
entityFullyRead = true;
return true;
} catch (RuntimeException e) {
LOGGER.log(Level.DEBUG, "Exception while consuming entity", e);
return false;
}
}

private ReadableEntity entity(ClientRequestHeaders requestHeaders,
ClientResponseHeaders responseHeaders) {
if (inputStream == null) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2023 Oracle and/or its affiliates.
* Copyright (c) 2023, 2024 Oracle and/or its affiliates.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -255,6 +255,37 @@ void testConnectionQueueDequeue() {
}
}

@Test
void testConnectionCachingUnreadEntity() {
ClientConnection connectionNow;
ClientConnection connectionPrior = null;
for (int i = 0; i < 5; ++i) {
HttpClientRequest request = injectedHttp1client.put("/test");
// connection will be dequeued if queue is not empty
WebClient webClient = WebClient.create();
Http1ClientConfig clientConfig = Http1ClientConfig.create();
Http1ClientImpl http1Client = new Http1ClientImpl(webClient, clientConfig);
connectionNow = http1Client
.connectionCache()
.connection(http1Client,
Duration.ZERO,
injectedHttp1client.prototype().tls(),
Proxy.noProxy(),
request.resolvedUri(),
request.headers(),
true);
request.connection(connectionNow);
// submitted entity is echoed back but not consumed here
HttpClientResponse response = request.submit("this is an entity");
// connection will be queued up
response.close();
if (connectionPrior != null) {
assertThat(connectionNow, is(connectionPrior));
}
connectionPrior = connectionNow;
}
}

@Test
void testConnectionQueueSizeLimit() {
int connectionQueueSize = injectedHttp1client.prototype().connectionCacheSize();
Expand Down