Skip to content

Phase 2 , getting Apache 5 compilation and Junit ready along with clearing Checkstyles and spotbug issues #6100

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.

Sign up for GitHub

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

Open
wants to merge 2 commits into
base: feature/master/apache5x
Choose a base branch
from
Open

Phase 2 , getting Apache 5 compilation and Junit ready along with clearing Checkstyles and spotbug issues #6100

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6234d36
Phase 2 , getting Apache 5 compilation and Junit ready along with cle…
joviegas May 13, 2025
9e766f7
Handle comments from review
joviegas May 16, 2025
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
2 changes: 2 additions & 0 deletions build-tools/src/main/resources/software/amazon/awssdk/spotbugs-suppressions.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -285,6 +285,7 @@
<Class name="software.amazon.awssdk.core.util.DefaultSdkAutoConstructMap"/>
<Class name="software.amazon.awssdk.http.nio.netty.internal.http2.FlushOnReadHandler"/>
<Class name="software.amazon.awssdk.http.apache.internal.conn.IdleConnectionReaper"/>
<Class name="software.amazon.awssdk.http.apache5.internal.conn.IdleConnectionReaper"/>
<Class name="software.amazon.awssdk.eventnotifications.s3.internal.DefaultS3EventNotificationWriter"/>
<Class name="software.amazon.awssdk.policybuilder.iam.internal.DefaultIamPolicyWriter"/>
</Or>
Expand Down Expand Up @@ -337,6 +338,7 @@
<Class name="~software\.amazon\.awssdk\.utils\.CompletableFutureUtils" />
<Class name="~software\.amazon\.awssdk\.metrics\.publishers\.cloudwatch\.CloudWatchMetricPublisher" />
<Class name="~software\.amazon\.awssdk\.http\.apache\.internal\.conn\.IdleConnectionReaper\$ReaperTask" />
<Class name="~software\.amazon\.awssdk\.http\.apache5\.internal\.conn\.IdleConnectionReaper\$ReaperTask" />
<Class name="~software\.amazon\.awssdk\.core\.internal\.retry\.RateLimitingTokenBucket" />
<Class name="~software\.amazon\.awssdk\.core\.internal\.waiters\.WaiterExecutor" />
<Class name="~software\.amazon\.awssdk\.regions\.internal\.util\.EC2MetadataUtils" />
Expand Down
39 changes: 39 additions & 0 deletions http-clients/apache5-client/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,12 +43,51 @@
<artifactId>httpcore5</artifactId>
<version>5.3.4</version>
</dependency>
<dependency>
<groupId>software.amazon.awssdk</groupId>
<artifactId>utils</artifactId>
<version>${awsjavasdk.version}</version>
</dependency>
<dependency>
<groupId>software.amazon.awssdk</groupId>
<artifactId>metrics-spi</artifactId>
<version>${awsjavasdk.version}</version>
</dependency>
<dependency>
<groupId>software.amazon.awssdk</groupId>
<artifactId>annotations</artifactId>
<version>${awsjavasdk.version}</version>
</dependency>

<dependency>
<groupId>software.amazon.awssdk</groupId>
<artifactId>http-client-tests</artifactId>
<version>${awsjavasdk.version}</version>
<scope>test</scope>
</dependency>
<!-- Added for WIRE logging details while testing-->
<dependency>
<groupId>org.apache.logging.log4j</groupId>
<artifactId>log4j-api</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.apache.logging.log4j</groupId>
<artifactId>log4j-core</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.apache.logging.log4j</groupId>
<artifactId>log4j-slf4j-impl</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.slf4j</groupId>
<artifactId>jcl-over-slf4j</artifactId>
<scope>test</scope>
<version>${slf4j.version}</version>
</dependency>

<dependency>
<groupId>org.junit.jupiter</groupId>
<artifactId>junit-jupiter</artifactId>
Expand Down
123 changes: 67 additions & 56 deletions ...s/apache5-client/src/main/java/software/amazon/awssdk/http/apache5/Apache5HttpClient.java
View file
Open in desktop

Large diffs are not rendered by default.

84 changes: 28 additions & 56 deletions ...java/software/amazon/awssdk/http/apache5/internal/RepeatableInputStreamRequestEntity.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,8 @@

import java.io.ByteArrayInputStream;
import java.io.IOException;
import java.io.InputStream;
import java.io.OutputStream;
import java.util.Optional;
import org.apache.hc.core5.http.ContentType;
import org.apache.hc.core5.http.io.entity.BasicHttpEntity;
import org.apache.hc.core5.http.io.entity.InputStreamEntity;
import software.amazon.awssdk.annotations.SdkInternalApi;
Expand All @@ -47,20 +46,12 @@ public class RepeatableInputStreamRequestEntity extends BasicHttpEntity {
*/
private boolean firstAttempt = true;

/**
* True if the "Transfer-Encoding:chunked" header is present
*/
private boolean isChunked;

/**
* The underlying InputStreamEntity being delegated to
*/
private InputStreamEntity inputStreamRequestEntity;

/**
* The InputStream containing the content to write out
*/
private InputStream content;

/**
* Record the original exception if we do attempt a retry, so that if the
Expand All @@ -80,33 +71,31 @@ public class RepeatableInputStreamRequestEntity extends BasicHttpEntity {
* @param request The details of the request being written out (content type,
* content length, and content).
*/
public RepeatableInputStreamRequestEntity(final HttpExecuteRequest request) {
isChunked = request.httpRequest().matchingHeaders(TRANSFER_ENCODING).contains(CHUNKED);
setChunked(isChunked);

/*
* If we don't specify a content length when we instantiate our
* InputStreamRequestEntity, then HttpClient will attempt to
* buffer the entire stream contents into memory to determine
* the content length.
*/
long contentLength = request.httpRequest().firstMatchingHeader("Content-Length")
.map(this::parseContentLength)
.orElse(-1L);

content = getContent(request.contentStreamProvider());
// TODO v2 MetricInputStreamEntity
inputStreamRequestEntity = new InputStreamEntity(content, contentLength);
setContent(content);
setContentLength(contentLength);

request.httpRequest().firstMatchingHeader("Content-Type").ifPresent(contentType -> {
inputStreamRequestEntity.setContentType(contentType);
setContentType(contentType);
});
public RepeatableInputStreamRequestEntity(HttpExecuteRequest request) {
super(request.contentStreamProvider().map(ContentStreamProvider::newStream)
.orElseGet(() -> new ByteArrayInputStream(new byte[0])),
getContentLengthFromRequest(request),
getContentTypeFromRequest(request),
null,
request.httpRequest().matchingHeaders(TRANSFER_ENCODING).contains(CHUNKED));

inputStreamRequestEntity = new InputStreamEntity(getContent(),
getContentLengthFromRequest(request),
getContentTypeFromRequest(request));
}


private static ContentType getContentTypeFromRequest(HttpExecuteRequest request) {
return request.httpRequest().firstMatchingHeader("Content-Type").map(ContentType::create).orElse(null);
}

private long parseContentLength(String contentLength) {
private static Long getContentLengthFromRequest(HttpExecuteRequest request) {
return request.httpRequest().firstMatchingHeader("Content-Length")
.map(RepeatableInputStreamRequestEntity::parseContentLength)
.orElse(-1L);
}

private static long parseContentLength(String contentLength) {
try {
return Long.parseLong(contentLength);
} catch (NumberFormatException nfe) {
Expand All @@ -115,25 +104,8 @@ private long parseContentLength(String contentLength) {
}
}

/**
* @return The request content input stream or an empty input stream if there is no content.
*/
private InputStream getContent(Optional<ContentStreamProvider> contentStreamProvider) {
return contentStreamProvider.map(ContentStreamProvider::newStream).orElseGet(() -> new ByteArrayInputStream(new byte[0]));
}

@Override
public boolean isChunked() {
return isChunked;
}

/**
* Returns true if the underlying InputStream supports marking/reseting or
* if the underlying InputStreamRequestEntity is repeatable.
*/
@Override
public boolean isRepeatable() {
return content.markSupported() || inputStreamRequestEntity.isRepeatable();
private boolean isRepeatableStream() {
return getContent().markSupported() || inputStreamRequestEntity.isRepeatable();
}

/**
Expand All @@ -149,8 +121,8 @@ public boolean isRepeatable() {
@Override
public void writeTo(OutputStream output) throws IOException {
try {
if (!firstAttempt && isRepeatable()) {
content.reset();
if (!firstAttempt && isRepeatableStream()) {
getContent().reset();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any reason we are calling getContent() instead of saving it as a member variable? ContentStreamProvider#newStream could be very expensive for some custom implementations.

}

firstAttempt = false;
Expand Down
1 change: 0 additions & 1 deletion ...ient/src/main/java/software/amazon/awssdk/http/apache5/internal/SdkProxyRoutePlanner.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
import org.apache.hc.client5.http.impl.routing.DefaultRoutePlanner;
import org.apache.hc.core5.http.HttpException;
import org.apache.hc.core5.http.HttpHost;
import org.apache.hc.core5.http.HttpRequest;
import org.apache.hc.core5.http.protocol.HttpContext;
import software.amazon.awssdk.annotations.SdkInternalApi;

Expand Down
52 changes: 25 additions & 27 deletions ...ava/software/amazon/awssdk/http/apache5/internal/conn/ClientConnectionManagerFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,17 +16,20 @@
package software.amazon.awssdk.http.apache5.internal.conn;

import java.io.IOException;
import java.util.concurrent.TimeUnit;
import org.apache.hc.client5.http.ConnectionRequest;
import org.apache.hc.client5.http.HttpRoute;
import org.apache.hc.client5.http.io.ConnectionEndpoint;
import org.apache.hc.client5.http.io.HttpClientConnectionManager;
import org.apache.hc.core5.http.io.HttpClientConnection;
import org.apache.hc.client5.http.io.LeaseRequest;
import org.apache.hc.core5.http.protocol.HttpContext;
import org.apache.hc.core5.io.CloseMode;
import org.apache.hc.core5.util.TimeValue;
import org.apache.hc.core5.util.Timeout;
import software.amazon.awssdk.annotations.SdkInternalApi;

@SdkInternalApi
public final class ClientConnectionManagerFactory {


private ClientConnectionManagerFactory() {
}

Expand All @@ -44,7 +47,7 @@ public static HttpClientConnectionManager wrap(HttpClientConnectionManager orig)
}

/**
* Further wraps {@link ConnectionRequest} to capture performance metrics.
* Further wraps {@link LeaseRequest} to capture performance metrics.
*/
private static class InstrumentedHttpClientConnectionManager extends DelegatingHttpClientConnectionManager {

Expand All @@ -53,10 +56,12 @@ private InstrumentedHttpClientConnectionManager(HttpClientConnectionManager dele
}

@Override
public ConnectionRequest requestConnection(HttpRoute route, Object state) {
ConnectionRequest connectionRequest = super.requestConnection(route, state);
public LeaseRequest lease(String id, HttpRoute route, Timeout requestTimeout, Object state) {
LeaseRequest connectionRequest = super.lease(id, route, requestTimeout, state);
return ClientConnectionRequestFactory.wrap(connectionRequest);
}


}

/**
Expand All @@ -71,44 +76,37 @@ protected DelegatingHttpClientConnectionManager(HttpClientConnectionManager dele
}

@Override
public ConnectionRequest requestConnection(HttpRoute route, Object state) {
return delegate.requestConnection(route, state);
public LeaseRequest lease(String id, HttpRoute route, Timeout requestTimeout, Object state) {
return delegate.lease(id, route, requestTimeout, state);
}

@Override
public void releaseConnection(HttpClientConnection conn, Object newState, long validDuration, TimeUnit timeUnit) {
delegate.releaseConnection(conn, newState, validDuration, timeUnit);
}
public void release(ConnectionEndpoint endpoint, Object newState, TimeValue validDuration) {
delegate.release(endpoint, newState, validDuration);

@Override
public void connect(HttpClientConnection conn, HttpRoute route, int connectTimeout, HttpContext context)
throws IOException {
delegate.connect(conn, route, connectTimeout, context);
}

@Override
public void upgrade(HttpClientConnection conn, HttpRoute route, HttpContext context) throws IOException {
delegate.upgrade(conn, route, context);
}
public void connect(ConnectionEndpoint endpoint, TimeValue connectTimeout, HttpContext context) throws IOException {
delegate.connect(endpoint, connectTimeout, context);

@Override
public void routeComplete(HttpClientConnection conn, HttpRoute route, HttpContext context) throws IOException {
delegate.routeComplete(conn, route, context);
}

@Override
public void closeIdleConnections(long idletime, TimeUnit timeUnit) {
delegate.closeIdleConnections(idletime, timeUnit);
public void upgrade(ConnectionEndpoint endpoint, HttpContext context) throws IOException {
delegate.upgrade(endpoint, context);
}

@Override
public void closeExpiredConnections() {
delegate.closeExpiredConnections();
public void close(CloseMode closeMode) {
delegate.close(closeMode);

}

@Override
public void shutdown() {
delegate.shutdown();
public void close() throws IOException {
delegate.close();

}
}
}
Loading
Loading