Skip to content

Commit

Permalink
MODSIDECAR-96 Replaced Vert.x WebClient with Vert.x HttpClient for al…
Browse files Browse the repository at this point in the history 
…l forwarded requests. However, WebClient is still used for REST interactions, including communication with Keycloak and retrieving module boot configurations.
  • Loading branch information
@vgema
vgema committed Feb 25, 2025
1 parent f54df27 commit 055ab6f
Show file tree
Hide file tree
Showing 2 changed files with 69 additions and 47 deletions.
13 changes: 10 additions & 3 deletions src/main/java/org/folio/sidecar/service/routing/RequestForwardingService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import static org.folio.sidecar.integration.okapi.OkapiHeaders.REQUEST_ID;
import static org.folio.sidecar.utils.RoutingUtils.getRequestId;

import io.netty.handler.codec.http.QueryStringEncoder;
import io.vertx.core.Future;
import io.vertx.core.http.HttpClient;
import io.vertx.core.http.HttpClientRequest;
Expand Down Expand Up @@ -109,9 +110,13 @@ private void forwardRequest(RoutingContext rc, String absUri, HttpClient httpCli
HttpServerRequest httpServerRequest = rc.request();
URI httpUri = URI.create(absUri);

QueryStringEncoder encoder = new QueryStringEncoder(httpUri.getPath());
httpServerRequest.params().forEach(encoder::addParam);

// Create an HTTP request
Future<HttpClientRequest> httpClientRequestFuture = httpClient.request(httpServerRequest.method(),
httpUri.getPort(), httpUri.getHost(), httpUri.getPath())
Future<HttpClientRequest> request = httpClient.request(httpServerRequest.method(),
httpUri.getPort(), httpUri.getHost(), encoder.toString());
Future<HttpClientRequest> httpClientRequestFuture = request
.timeout(httpProperties.getTimeout(), TimeUnit.MILLISECONDS);
httpClientRequestFuture.onSuccess(httpClientRequest -> {

Expand Down Expand Up @@ -140,7 +145,9 @@ private void forwardRequest(RoutingContext rc, String absUri, HttpClient httpCli
log.trace("Handle the HTTP client response by streaming the output back to the server");
handleSuccessfulResponse(rc, response);
transactionLogHandler.log(rc, response, httpClientRequest);
});
}).onFailure(error -> errorHandler.sendErrorResponse(
rc, new InternalServerErrorException("Failed to proxy request", error)));

// End the request when the file stream finishes
httpServerRequest.endHandler(v -> {
log.trace("End the request when the file stream finishes");
Expand Down
103 changes: 59 additions & 44 deletions src/test/java/org/folio/sidecar/service/routing/RequestForwardingServiceTest.java
View file
Original file line number Diff line number Diff line change
@@ -1,31 +1,32 @@
package org.folio.sidecar.service.routing;

import static io.vertx.core.Future.failedFuture;
import static io.vertx.core.Future.succeededFuture;
import static io.vertx.core.http.HttpMethod.GET;
import static jakarta.ws.rs.core.HttpHeaders.CONTENT_TYPE;
import static jakarta.ws.rs.core.HttpHeaders.USER_AGENT;
import static jakarta.ws.rs.core.MediaType.APPLICATION_JSON;
import static org.apache.http.HttpStatus.SC_OK;
import static org.assertj.core.api.Assertions.assertThat;
import static org.folio.sidecar.support.TestConstants.MODULE_URL_TLS;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;

import io.netty.handler.codec.http.QueryStringEncoder;
import io.smallrye.mutiny.TimeoutException;
import io.vertx.core.Future;
import io.vertx.core.MultiMap;
import io.vertx.core.buffer.Buffer;
import io.vertx.core.http.HttpClient;
import io.vertx.core.http.HttpClientRequest;
import io.vertx.core.http.HttpClientResponse;
import io.vertx.core.http.HttpServerRequest;
import io.vertx.core.http.HttpServerResponse;
import io.vertx.core.http.impl.headers.HeadersMultiMap;
import io.vertx.ext.web.RoutingContext;
import io.vertx.ext.web.client.HttpRequest;
import io.vertx.ext.web.client.HttpResponse;
import io.vertx.ext.web.client.WebClient;
import jakarta.ws.rs.InternalServerErrorException;
import java.util.function.Consumer;
import org.folio.sidecar.configuration.properties.HttpProperties;
Expand Down Expand Up @@ -53,12 +54,16 @@ class RequestForwardingServiceTest {
private final String absoluteUrl = TestConstants.MODULE_URL + PATH;

@InjectMocks private RequestForwardingService service;
@Mock private WebClient webClient;
@Mock private HttpClient httpClient;
@Mock private ErrorHandler errorHandler;
@Mock private HttpRequest<Buffer> httpRequest;

@Mock private HttpClientRequest httpClientRequest;
@Mock private HttpResponse<Buffer> httpResponse;
@Mock private Buffer buffer;
@Mock private HttpClientResponse httpClientResponse;
@Mock private MultiMap headers;

@Mock private MultiMap headersResponse;
@Mock private SidecarSignatureService sidecarSignatureService;
@Mock private HttpProperties httpProperties;
@Mock private WebClientConfig webClientConfig;
Expand All @@ -73,15 +78,17 @@ class RequestForwardingServiceTest {
@Test
void forward_positive() {
var routingContext = routingContext(RequestForwardingServiceTest::withHttpResponse);
QueryStringEncoder encoder = new QueryStringEncoder(PATH);
routingContext.request().params().forEach(encoder::addParam);

when(webClient.requestAbs(GET, absoluteUrl)).thenReturn(httpRequest);
prepareHttpRequestMocks(routingContext);
prepareHttpResponseMocks(buffer);
when(httpClient.request(GET, 8081, "sc-foo", encoder.toString())).thenReturn(
Future.succeededFuture(httpClientRequest));
prepareHttpRequestMocks(routingContext, httpClientRequest);
prepareHttpResponseMocks(httpClientResponse);

var response = routingContext.response();
when(response.headers()).thenReturn(headers);
when(headers.addAll(responseHeadersMapCaptor.capture())).thenReturn(headers);
when(response.end(buffer)).thenReturn(succeededFuture());
when(response.headers()).thenReturn(headersResponse);
when(headersResponse.addAll(responseHeadersMapCaptor.capture())).thenReturn(headersResponse);

service.forwardIngress(routingContext, absoluteUrl);

Expand All @@ -97,7 +104,7 @@ void forward_positive() {
assertThat(responseHeaders.get("tst-header")).isEqualTo("tst-value");
assertThat(responseHeaders.get(CONTENT_TYPE)).isEqualTo(APPLICATION_JSON);
assertThat(responseHeaders.get(TestConstants.SIDECAR_SIGNATURE_HEADER)).isNull();
assertThat(queryParamCaptor.getAllValues()).containsExactly("query", "name==test", "offset", "10", "size", "50");

assertThat(requestIdCaptor.getValue()).isNotEmpty().matches("\\d{6}/foo");

verify(sidecarSignatureService).removeSignature(any(HttpServerResponse.class));
Expand All @@ -111,16 +118,19 @@ void forwardEgress_positive() {
var egressTlsMock = mock(WebClientConfig.TlsSettings.class);
when(egressSettingsMock.tls()).thenReturn(egressTlsMock);
when(egressTlsMock.enabled()).thenReturn(true);

when(webClient.requestAbs(GET, MODULE_URL_TLS + PATH)).thenReturn(httpRequest);
QueryStringEncoder encoder = new QueryStringEncoder(PATH);
var routingContext = routingContext(RequestForwardingServiceTest::withHttpResponse);
prepareHttpRequestMocks(routingContext);
prepareHttpResponseMocks(buffer);
routingContext.request().params().forEach(encoder::addParam);

when(httpClient.request(GET, 8081, "sc-foo", encoder.toString())).thenReturn(
Future.succeededFuture(httpClientRequest));

prepareHttpRequestMocks(routingContext, httpClientRequest);
prepareHttpResponseMocks(httpClientResponse);

var response = routingContext.response();
when(response.headers()).thenReturn(headers);
when(headers.addAll(responseHeadersMapCaptor.capture())).thenReturn(headers);
when(response.end(buffer)).thenReturn(succeededFuture());
when(response.headers()).thenReturn(headersResponse);
when(headersResponse.addAll(responseHeadersMapCaptor.capture())).thenReturn(headersResponse);

service.forwardEgress(routingContext, absoluteUrl);

Expand All @@ -136,7 +146,6 @@ void forwardEgress_positive() {
assertThat(responseHeaders.get("tst-header")).isEqualTo("tst-value");
assertThat(responseHeaders.get(CONTENT_TYPE)).isEqualTo(APPLICATION_JSON);
assertThat(responseHeaders.get(TestConstants.SIDECAR_SIGNATURE_HEADER)).isNull();
assertThat(queryParamCaptor.getAllValues()).containsExactly("query", "name==test", "offset", "10", "size", "50");
assertThat(requestIdCaptor.getValue()).isNotEmpty().matches("\\d{6}/foo");

verify(sidecarSignatureService).removeSignature(any(HttpServerResponse.class));
Expand All @@ -146,14 +155,18 @@ void forwardEgress_positive() {
void forward_positive_nullBodyBuffer() {
var routingContext = routingContext(RequestForwardingServiceTest::withHttpResponse);

when(webClient.requestAbs(GET, absoluteUrl)).thenReturn(httpRequest);
prepareHttpRequestMocks(routingContext);
prepareHttpResponseMocks(null);
QueryStringEncoder encoder = new QueryStringEncoder(PATH);

routingContext.request().params().forEach(encoder::addParam);

when(httpClient.request(GET, 8081, "sc-foo", encoder.toString())).thenReturn(
Future.succeededFuture(httpClientRequest));
prepareHttpRequestMocks(routingContext, httpClientRequest);
prepareHttpResponseMocks(httpClientResponse);

var response = routingContext.response();
when(response.headers()).thenReturn(headers);
when(headers.addAll(responseHeadersMapCaptor.capture())).thenReturn(headers);
when(response.end()).thenReturn(succeededFuture());
when(response.headers()).thenReturn(headersResponse);
when(headersResponse.addAll(responseHeadersMapCaptor.capture())).thenReturn(headersResponse);

service.forwardIngress(routingContext, absoluteUrl);

Expand All @@ -168,7 +181,7 @@ void forward_positive_nullBodyBuffer() {
assertThat(responseHeaders.get("tst-header")).isEqualTo("tst-value");
assertThat(responseHeaders.get(CONTENT_TYPE)).isEqualTo(APPLICATION_JSON);

assertThat(queryParamCaptor.getAllValues()).containsExactly("query", "name==test", "offset", "10", "size", "50");
// assertThat(queryParamCaptor.getAllValues()).containsExactly("query", "name==test", "offset", "10", "size", "50");
assertThat(requestIdCaptor.getValue()).isNotEmpty().matches("\\d{6}/foo");
assertThat(responseHeaders.get(TestConstants.SIDECAR_SIGNATURE_HEADER)).isNull();

Expand All @@ -180,32 +193,34 @@ void forward_negative() {
var routingContext = routingContext(rc -> {});
var error = new TimeoutException();

when(webClient.requestAbs(GET, absoluteUrl)).thenReturn(httpRequest);
QueryStringEncoder encoder = new QueryStringEncoder(PATH);
routingContext.request().params().forEach(encoder::addParam);

when(httpClient.request(GET, 8081, "sc-foo", encoder.toString()))
.thenReturn(Future.succeededFuture(httpClientRequest));
when(httpProperties.getTimeout()).thenReturn(TIMEOUT);
when(httpProperties.getTimeout()).thenReturn(TIMEOUT);
when(httpRequest.timeout(TIMEOUT)).thenReturn(httpRequest);
when(httpRequest.putHeaders(any(MultiMap.class))).thenReturn(httpRequest);
when(httpRequest.addQueryParam(anyString(), anyString())).thenReturn(httpRequest);
when(httpRequest.putHeader(eq(OkapiHeaders.REQUEST_ID), anyString())).thenReturn(httpRequest);
when(httpRequest.sendStream(routingContext.request())).thenReturn(failedFuture(error));
when(httpClientRequest.headers()).thenReturn(headers);
when(headers.addAll(requestHeadersMapCaptor.capture())).thenReturn(headers);
when(headers.add(eq(OkapiHeaders.REQUEST_ID), requestIdCaptor.capture())).thenReturn(headers);
when(httpClientRequest.response()).thenReturn(failedFuture(error));

service.forwardIngress(routingContext, absoluteUrl);

verify(errorHandler).sendErrorResponse(eq(routingContext), any(InternalServerErrorException.class));
}

private void prepareHttpResponseMocks(Buffer buffer) {
when(httpResponse.headers()).thenReturn(responseHeaders());
when(httpResponse.statusCode()).thenReturn(SC_OK);
when(httpResponse.bodyAsBuffer()).thenReturn(buffer);
private void prepareHttpResponseMocks(HttpClientResponse httpClientResponse) {
when(httpClientResponse.headers()).thenReturn(responseHeaders());
when(httpClientResponse.statusCode()).thenReturn(SC_OK);
}

private void prepareHttpRequestMocks(RoutingContext routingContext) {
private void prepareHttpRequestMocks(RoutingContext routingContext, HttpClientRequest httpClientRequest) {
when(httpProperties.getTimeout()).thenReturn(TIMEOUT);
when(httpRequest.timeout(TIMEOUT)).thenReturn(httpRequest);
when(httpRequest.putHeaders(requestHeadersMapCaptor.capture())).thenReturn(httpRequest);
when(httpRequest.addQueryParam(queryParamCaptor.capture(), queryParamCaptor.capture())).thenReturn(httpRequest);
when(httpRequest.putHeader(eq(OkapiHeaders.REQUEST_ID), requestIdCaptor.capture())).thenReturn(httpRequest);
when(httpRequest.sendStream(routingContext.request())).thenReturn(succeededFuture(httpResponse));
when(httpClientRequest.headers()).thenReturn(headers);
when(headers.addAll(requestHeadersMapCaptor.capture())).thenReturn(headers);
when(headers.add(eq(OkapiHeaders.REQUEST_ID), requestIdCaptor.capture())).thenReturn(headers);
when(httpClientRequest.response()).thenReturn(Future.succeededFuture(httpClientResponse));
}

private static RoutingContext routingContext(Consumer<RoutingContext> rcConsumer) {
Expand Down

0 comments on commit 055ab6f

Please sign in to comment.