Skip to content

Commit 6c821c9

Browse files
CharlesDuboisSAPKavithaSiva
CharlesDuboisSAP
and
KavithaSiva
authored
chore: Migrate connectivity-destination-service to HttpClient5 (#1028)
Co-authored-by: KavithaSiva <kavitha.sivakumar@sap.com>
1 parent 81490de commit 6c821c9

11 files changed

Lines changed: 493 additions & 283 deletions

File tree

‎cloudplatform/connectivity-destination-service/pom.xml‎

Lines changed: 9 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@
4343
</dependency>
4444
<dependency>
4545
<groupId>com.sap.cloud.sdk.cloudplatform</groupId>
46-
<artifactId>connectivity-apache-httpclient4</artifactId>
46+
<artifactId>connectivity-apache-httpclient5</artifactId>
4747
</dependency>
4848
<!-- Included by default to serve as the default implementation -->
4949
<dependency>
@@ -97,26 +97,14 @@
9797
<groupId>commons-io</groupId>
9898
<artifactId>commons-io</artifactId>
9999
</dependency>
100-
<dependency>
101-
<groupId>org.apache.httpcomponents</groupId>
102-
<artifactId>httpcore</artifactId>
103-
<exclusions>
104-
<exclusion>
105-
<artifactId>commons-logging</artifactId>
106-
<groupId>commons-logging</groupId>
107-
</exclusion>
108-
</exclusions>
109-
</dependency>
110-
<dependency>
111-
<groupId>org.apache.httpcomponents</groupId>
112-
<artifactId>httpclient</artifactId>
113-
<exclusions>
114-
<exclusion>
115-
<artifactId>commons-logging</artifactId>
116-
<groupId>commons-logging</groupId>
117-
</exclusion>
118-
</exclusions>
119-
</dependency>
100+
<dependency>
101+
<groupId>org.apache.httpcomponents.core5</groupId>
102+
<artifactId>httpcore5</artifactId>
103+
</dependency>
104+
<dependency>
105+
<groupId>org.apache.httpcomponents.client5</groupId>
106+
<artifactId>httpclient5</artifactId>
107+
</dependency>
120108
<dependency>
121109
<groupId>io.vavr</groupId>
122110
<artifactId>vavr</artifactId>

‎cloudplatform/connectivity-destination-service/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/AuthTokenHeaderProvider.java‎

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111

1212
import javax.annotation.Nonnull;
1313

14-
import org.apache.http.HttpHeaders;
14+
import org.apache.hc.core5.http.HttpHeaders;
1515

1616
import com.google.common.base.Strings;
1717
import com.sap.cloud.sdk.cloudplatform.connectivity.exception.DestinationAccessException;
@@ -96,13 +96,9 @@ private static List<Header> getDestinationHeaders( @Nonnull final List<Destinati
9696

9797
private static boolean isAuthTokenExpected( @Nonnull final AuthenticationType authType )
9898
{
99-
switch( authType ) {
100-
case NO_AUTHENTICATION:
101-
case PRINCIPAL_PROPAGATION:
102-
case CLIENT_CERTIFICATE_AUTHENTICATION:
103-
return false;
104-
default:
105-
return true;
106-
}
99+
return switch( authType ) {
100+
case NO_AUTHENTICATION, PRINCIPAL_PROPAGATION, CLIENT_CERTIFICATE_AUTHENTICATION -> false;
101+
default -> true;
102+
};
107103
}
108104
}

‎cloudplatform/connectivity-destination-service/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/DestinationServiceAdapter.java‎

Lines changed: 47 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -13,11 +13,11 @@
1313
import javax.annotation.Nonnull;
1414
import javax.annotation.Nullable;
1515

16-
import org.apache.http.HttpResponse;
17-
import org.apache.http.HttpStatus;
18-
import org.apache.http.StatusLine;
19-
import org.apache.http.client.methods.HttpGet;
20-
import org.apache.http.client.methods.HttpUriRequest;
16+
import org.apache.hc.client5.http.classic.methods.HttpGet;
17+
import org.apache.hc.client5.http.impl.classic.BasicHttpClientResponseHandler;
18+
import org.apache.hc.core5.http.ClassicHttpRequest;
19+
import org.apache.hc.core5.http.ClassicHttpResponse;
20+
import org.apache.hc.core5.http.HttpStatus;
2121

2222
import com.google.common.base.Strings;
2323
import com.sap.cloud.environment.servicebinding.api.DefaultServiceBindingAccessor;
@@ -32,6 +32,7 @@
3232
import io.vavr.control.Try;
3333
import lombok.AccessLevel;
3434
import lombok.Getter;
35+
import lombok.RequiredArgsConstructor;
3536
import lombok.extern.slf4j.Slf4j;
3637

3738
@Slf4j
@@ -126,53 +127,62 @@ String getConfigurationAsJson(
126127
serviceDestinationLoader.apply(strategy.behalf()),
127128
() -> "Destination for Destination Service on behalf of " + strategy.behalf() + " not found.");
128129

129-
final HttpUriRequest request = prepareRequest(servicePath, strategy);
130+
final ClassicHttpRequest request = prepareRequest(servicePath, strategy);
130131

131-
final HttpResponse response;
132132
try {
133-
response = HttpClientAccessor.getHttpClient(serviceDestination).execute(request);
133+
return ApacheHttpClient5Accessor
134+
.getHttpClient(serviceDestination)
135+
.execute(request, new DestinationHttpClientResponseHandler(request));
134136
}
135137
catch( final IOException e ) {
136138
throw new DestinationAccessException(e);
137139
}
138-
return handleResponse(request, response);
139140
}
140141

141-
@Nonnull
142-
private static String handleResponse( final HttpUriRequest request, final HttpResponse response )
142+
@RequiredArgsConstructor( access = AccessLevel.PRIVATE )
143+
static class DestinationHttpClientResponseHandler extends BasicHttpClientResponseHandler
143144
{
144-
final StatusLine status = response.getStatusLine();
145-
final int statusCode = status.getStatusCode();
146-
final String reasonPhrase = status.getReasonPhrase();
145+
final ClassicHttpRequest request;
147146

148-
log.debug("Destination service returned HTTP status {} ({})", statusCode, reasonPhrase);
147+
@Override
148+
public String handleResponse( final ClassicHttpResponse response )
149+
{
150+
final int statusCode = response.getCode();
151+
final String reasonPhrase = response.getReasonPhrase();
149152

150-
Try<String> maybeBody = Try.of(() -> HttpEntityUtil.getResponseBody(response));
151-
if( maybeBody.isFailure() ) {
152-
final var ex =
153-
new DestinationAccessException("Failed to read body from HTTP response", maybeBody.getCause());
154-
maybeBody = Try.failure(ex);
155-
}
153+
log.debug("Destination service returned HTTP status {} ({})", statusCode, reasonPhrase);
156154

157-
if( statusCode == HttpStatus.SC_OK ) {
158-
final var ex = new DestinationAccessException("Failed to get destinations: no body returned in response.");
159-
maybeBody = maybeBody.filter(it -> !Strings.isNullOrEmpty(it), () -> ex);
160-
return maybeBody.get();
161-
}
155+
Try<String> maybeBody = Try.of(() -> handleEntity(response.getEntity()));
156+
if( maybeBody.isFailure() ) {
157+
final var ex =
158+
new DestinationAccessException("Failed to read body from HTTP response", maybeBody.getCause());
159+
maybeBody = Try.failure(ex);
160+
}
162161

163-
final String requestUri = request.getURI().getPath();
164-
if( statusCode == HttpStatus.SC_NOT_FOUND ) {
165-
throw new DestinationNotFoundException(null, "Destination could not be found for path " + requestUri + ".");
162+
if( statusCode == HttpStatus.SC_OK ) {
163+
final var ex =
164+
new DestinationAccessException("Failed to get destinations: no body returned in response.");
165+
maybeBody = maybeBody.filter(it -> !Strings.isNullOrEmpty(it), () -> ex);
166+
return maybeBody.get();
167+
}
168+
169+
final String requestUri = request.getPath();
170+
if( statusCode == HttpStatus.SC_NOT_FOUND ) {
171+
throw new DestinationNotFoundException(
172+
null,
173+
"Destination could not be found for path " + requestUri + ".");
174+
}
175+
final String message =
176+
"Failed to get destinations: destination service responded with HTTP status %s (%S) at '%s'."
177+
.formatted(statusCode, reasonPhrase, requestUri);
178+
final String messageWithBody =
179+
message + " Body: %s".formatted(maybeBody.getOrElseGet(Throwable::getMessage));
180+
log.error(messageWithBody);
181+
throw new DestinationAccessException(message);
166182
}
167-
final String message =
168-
"Failed to get destinations: destination service responded with HTTP status %s (%S) at '%s'."
169-
.formatted(statusCode, reasonPhrase, requestUri);
170-
final String messageWithBody = message + " Body: %s".formatted(maybeBody.getOrElseGet(Throwable::getMessage));
171-
log.error(messageWithBody);
172-
throw new DestinationAccessException(message);
173183
}
174184

175-
private HttpUriRequest prepareRequest( final String servicePath, final DestinationRetrievalStrategy strategy )
185+
private ClassicHttpRequest prepareRequest( final String servicePath, final DestinationRetrievalStrategy strategy )
176186
{
177187
final URI requestUri;
178188
try {
@@ -183,7 +193,7 @@ private HttpUriRequest prepareRequest( final String servicePath, final Destinati
183193
}
184194

185195
log.debug("Querying Destination Service via URI {}.", requestUri);
186-
final HttpUriRequest request = new HttpGet(requestUri);
196+
final ClassicHttpRequest request = new HttpGet(requestUri);
187197

188198
if( !servicePath.startsWith(DestinationService.PATH_DEFAULT)
189199
&& !servicePath.startsWith(DestinationService.PATH_V2) ) {

‎cloudplatform/connectivity-destination-service/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/TransparentProxy.java‎

Lines changed: 21 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -11,11 +11,13 @@
1111
import javax.annotation.Nonnull;
1212

1313
import org.apache.commons.lang3.exception.ExceptionUtils;
14-
import org.apache.http.HttpMessage;
15-
import org.apache.http.HttpResponse;
16-
import org.apache.http.HttpStatus;
17-
import org.apache.http.client.HttpClient;
18-
import org.apache.http.client.methods.HttpHead;
14+
import org.apache.hc.client5.http.classic.HttpClient;
15+
import org.apache.hc.client5.http.classic.methods.HttpHead;
16+
import org.apache.hc.core5.http.ClassicHttpResponse;
17+
import org.apache.hc.core5.http.HttpHost;
18+
import org.apache.hc.core5.http.HttpMessage;
19+
import org.apache.hc.core5.http.HttpResponse;
20+
import org.apache.hc.core5.http.HttpStatus;
1921

2022
import com.sap.cloud.sdk.cloudplatform.connectivity.exception.DestinationAccessException;
2123
import com.sap.cloud.sdk.cloudplatform.connectivity.exception.DestinationNotFoundException;
@@ -76,7 +78,7 @@ public class TransparentProxy implements DestinationLoader
7678
private static final String SET_COOKIE_HEADER = "Set-Cookie";
7779
private static final Integer DEFAULT_PORT = 80;
7880
private static final String SCHEME_SEPARATOR = "://";
79-
private static final String HTTP_SCHEME = org.apache.http.HttpHost.DEFAULT_SCHEME_NAME + SCHEME_SEPARATOR;
81+
private static final String HTTP_SCHEME = HttpHost.DEFAULT_SCHEME.getId() + SCHEME_SEPARATOR;
8082
private static final String PORT_SEPARATOR = ":";
8183
private static final String HOST_CONTAINS_PATH_ERROR_MESSAGE_TEMPLATE =
8284
"Host '%s' contains a path '%s'. Paths are not allowed in host registration.";
@@ -279,7 +281,7 @@ private static String normalizeHostWithScheme( @Nonnull final String host )
279281
private static TransparentProxyDestination verifyDestination(
280282
@Nonnull final TransparentProxyDestination destination )
281283
{
282-
final HttpClient httpClient = HttpClientAccessor.getHttpClient(destination);
284+
final HttpClient httpClient = ApacheHttpClient5Accessor.getHttpClient(destination);
283285
final URI destinationUri = URI.create(uri);
284286
final HttpHead headRequest = new HttpHead(destinationUri);
285287
final String destinationName = getDestinationName(destination, destinationUri);
@@ -288,28 +290,18 @@ private static TransparentProxyDestination verifyDestination(
288290
.debug(
289291
"Performing HEAD request to destination with name {} to verify the destination exists",
290292
destinationName);
291-
final Supplier<HttpResponse> tpDestinationVerifierSupplier = prepareSupplier(httpClient, headRequest);
292-
HttpResponse response = null;
293-
try {
294-
response = ResilienceDecorator.executeSupplier(tpDestinationVerifierSupplier, resilienceConfiguration);
293+
final Supplier<ClassicHttpResponse> tpDestinationVerifierSupplier = prepareSupplier(httpClient, headRequest);
294+
try(
295+
ClassicHttpResponse response =
296+
ResilienceDecorator.executeSupplier(tpDestinationVerifierSupplier, resilienceConfiguration) ) {
295297
verifyTransparentProxyResponse(response, destinationName);
296298
}
297-
catch( final ResilienceRuntimeException e ) {
299+
catch( final ResilienceRuntimeException | IOException e ) {
298300
if( hasCauseAssignableFrom(e, DestinationNotFoundException.class) ) {
299301
throw new DestinationNotFoundException(e);
300302
}
301303
throw new DestinationAccessException(e);
302304
}
303-
finally {
304-
if( response != null ) {
305-
try {
306-
org.apache.http.util.EntityUtils.consume(response.getEntity());
307-
}
308-
catch( IOException e ) {
309-
log.warn("Failed to close HTTP response", e);
310-
}
311-
}
312-
}
313305

314306
return destination;
315307
}
@@ -340,15 +332,15 @@ private static void verifyTransparentProxyResponse( final HttpResponse response,
340332
throw new DestinationAccessException(FAILED_TO_VERIFY_DESTINATION + "Response is null.");
341333
}
342334
if( response.containsHeader(SET_COOKIE_HEADER) ) {
343-
final org.apache.http.Header[] header = response.getHeaders(SET_COOKIE_HEADER);
335+
final org.apache.hc.core5.http.Header[] header = response.getHeaders(SET_COOKIE_HEADER);
344336
final List<String> cookieNames = Arrays.stream(header).map(h -> h.getValue().split("=", 2)[0]).toList();
345337
log
346338
.warn(
347339
"received set-cookie headers as part of destination health check. This is unexpected and may have side effects for your application. The following cookies were set: {}",
348340
cookieNames);
349341
}
350342

351-
final int statusCode = response.getStatusLine().getStatusCode();
343+
final int statusCode = response.getCode();
352344
final String errorInternalCode = getHeaderValue(response, X_ERROR_INTERNAL_CODE_HEADER);
353345
final String errorMessage = getHeaderValue(response, X_ERROR_MESSAGE_HEADER);
354346
final String errorOrigin = getHeaderValue(response, X_ERROR_ORIGIN_HEADER);
@@ -378,11 +370,14 @@ private static void verifyTransparentProxyResponse( final HttpResponse response,
378370
}
379371

380372
@Nonnull
381-
private static Supplier<HttpResponse> prepareSupplier( final HttpClient httpClient, final HttpHead headRequest )
373+
private static
374+
Supplier<ClassicHttpResponse>
375+
prepareSupplier( final HttpClient httpClient, final HttpHead headRequest )
382376
{
383377
return () -> {
384378
try {
385-
return httpClient.execute(headRequest);
379+
// migration from apache httpclient4 to httpclient5 by possibly adding a response handler to httpClient.execute(headRequest);
380+
return httpClient.execute(headRequest, classicHttpResponse -> classicHttpResponse);
386381
}
387382
catch( final IOException e ) {
388383
throw new DestinationAccessException(FAILED_TO_VERIFY_DESTINATION, e);

‎cloudplatform/connectivity-destination-service/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/AuthTokenHeaderProviderTest.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
import java.util.Arrays;
99
import java.util.Collection;
1010

11-
import org.apache.http.HttpHeaders;
11+
import org.apache.hc.core5.http.HttpHeaders;
1212
import org.junit.jupiter.api.AfterEach;
1313
import org.junit.jupiter.api.BeforeEach;
1414
import org.junit.jupiter.api.DisplayName;

0 commit comments

Comments
 (0)