Skip to content
This repository has been archived by the owner on Nov 26, 2024. It is now read-only.

Commit

Permalink
Merge pull request #631 from FortnoxAB/content-type-charset
Browse files Browse the repository at this point in the history 
Disregard charset in Content-Type when parsing response
  • Loading branch information
@flowertwig
flowertwig authored Oct 27, 2023
2 parents cfc9d91 + b3ec501 commit fb95475
Show file tree
Hide file tree
Showing 2 changed files with 91 additions and 47 deletions.
27 changes: 15 additions & 12 deletions client/src/main/java/se/fortnox/reactivewizard/client/HttpClient.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import com.fasterxml.jackson.databind.type.TypeFactory;
import com.google.common.collect.Sets;
import io.netty.handler.codec.http.HttpResponseStatus;
import io.netty.handler.codec.http.HttpUtil;
import io.netty.handler.timeout.ReadTimeoutException;
import jakarta.inject.Inject;
import org.reactivestreams.Publisher;
Expand Down Expand Up @@ -233,7 +234,7 @@ private static <T> Mono<Response<T>> flattenResponse(Mono<Response<Flux<T>>> res
*/
public static <T> Mono<Response<Flux<T>>> getFullResponse(Flux<T> source) {
Optional<Mono<Response<Flux<T>>>> responseFlux = ReactiveDecorator.getDecoration(source);
if (!responseFlux.isPresent()) {
if (responseFlux.isEmpty()) {
throw new IllegalArgumentException("Must be used with Flux returned from api call");
}
return responseFlux.get();
Expand All @@ -248,7 +249,7 @@ public static <T> Mono<Response<Flux<T>>> getFullResponse(Flux<T> source) {
*/
public static <T> Mono<Response<T>> getFullResponse(Mono<T> source) {
Optional<Mono<Response<Flux<T>>>> responseFlux = ReactiveDecorator.getDecoration(source);
if (!responseFlux.isPresent()) {
if (responseFlux.isEmpty()) {
throw new IllegalArgumentException("Must be used with Mono returned from api call");
}
return flattenResponse(responseFlux.get());
Expand Down Expand Up @@ -281,7 +282,10 @@ protected Flux<Object> parseResponseSingle(Method method, RwHttpClientResponse r
}

private String resolveContentType(Method method, RwHttpClientResponse response) {
String contentType = response.getHttpClientResponse().responseHeaders().get(CONTENT_TYPE);
String contentType = Optional.ofNullable(response.getHttpClientResponse().responseHeaders().get(CONTENT_TYPE))
.map(HttpUtil::getMimeType)
.map(CharSequence::toString)
.orElse(null);

// Override response content-type if resource method is annotated with a non-empty @Produces
JaxRsMeta jaxRsMeta = new JaxRsMeta(method);
Expand Down Expand Up @@ -433,13 +437,13 @@ protected void addContent(Method method, Object[] arguments, RequestBuilder requ
}
}
}
if (output.length() > 0) {
if (!output.isEmpty()) {
requestBuilder.setContent(output.toString());
}
}

protected void addFormParamToOutput(StringBuilder output, Object value, FormParam formParam) {
if (output.length() != 0) {
if (!output.isEmpty()) {
output.append("&");
}
output.append(formParam.value()).append("=").append(urlEncode(value.toString()));
Expand Down Expand Up @@ -495,7 +499,7 @@ private String formatHeaders(reactor.netty.http.client.HttpClientResponse client

private HttpClient.DetailedError getDetailedError(String data, Throwable cause) {
HttpClient.DetailedError detailedError = new HttpClient.DetailedError(cause);
if (data != null && data.length() > 0) {
if (data != null && !data.isEmpty()) {
try {
objectMapper.readerForUpdating(detailedError).readValue(data);
} catch (IOException e) {
Expand Down Expand Up @@ -666,13 +670,12 @@ protected String getPath(Method method, Object[] arguments, JaxRsMeta meta) {
private List<BeanParamProperty> getBeanParamGetters(Class beanParamType) {
List<BeanParamProperty> result = new ArrayList<>();
for (Field field : getDeclaredFieldsFromClassAndAncestors(beanParamType)) {
Optional<Function<Object, Object>> getter = ReflectionUtil.getter(beanParamType, field.getName());
if (getter.isPresent()) {
Optional<Function<Object, Object>> optionalGetter = ReflectionUtil.getter(beanParamType, field.getName());
optionalGetter.ifPresent(getter ->
result.add(new BeanParamProperty(
getter.get(),
getter,
field.getAnnotations()
));
}
)));
}
return result;
}
Expand All @@ -681,7 +684,7 @@ private List<BeanParamProperty> getBeanParamGetters(Class beanParamType) {
* Recursive function getting all declared fields from the passed in class and its ancestors.
*
* @param clazz the clazz fetching fields from
* @return list of fields
* @return set of fields
*/
private static Set<Field> getDeclaredFieldsFromClassAndAncestors(Class clazz) {
final HashSet<Field> declaredFields = new HashSet<>(asList(clazz.getDeclaredFields()));
Expand Down
111 changes: 76 additions & 35 deletions client/src/test/java/se/fortnox/reactivewizard/client/HttpClientTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@ private TestResource getHttpProxyWithClientReturningEmpty(DisposableServer dispo
}

protected void withServer(Consumer<DisposableServer> serverConsumer) {
server = startServer(HttpResponseStatus.OK, "\"OK\"");
server = startServer(OK, "\"OK\"");
serverConsumer.accept(server);
}

Expand Down Expand Up @@ -329,7 +329,7 @@ void shouldNotRetryIfEmptyReturnedOnPost() {
}

@Test
void shouldNotReportUnhealthyWhenPoolIsExhaustedRatherSequentiatingTheRequests() throws URISyntaxException {
void shouldNotReportUnhealthyWhenPoolIsExhaustedRatherSequentiatingTheRequests() {

AtomicBoolean wasUnhealthy = new AtomicBoolean(false);

Expand All @@ -350,7 +350,7 @@ public boolean logStatus(Object key, boolean currentStatus) {
config.setRetryCount(0);
TestResource resource = getHttpProxy(config);

List<Observable<String>> results = new ArrayList<Observable<String>>();
List<Observable<String>> results = new ArrayList<>();
for (int i = 0; i < 10; i++) {
results.add(resource.getHello());
}
Expand Down Expand Up @@ -419,7 +419,7 @@ void shouldReportHealthyWhenPoolIsNotExhausted() {
withServer(server -> {
TestResource resource = getHttpProxy(server.port(), 5);

List<Observable<String>> results = new ArrayList<Observable<String>>();
List<Observable<String>> results = new ArrayList<>();
for (int i = 0; i < 5; i++) {
results.add(resource.getHello());
}
Expand Down Expand Up @@ -480,14 +480,13 @@ void shouldReturnErrorJsonBody() {

TestResource resource = getHttpProxy(server.port());
StepVerifier.create(resource.getHelloMono())
.verifyErrorSatisfies(error -> {
.verifyErrorSatisfies(error ->
assertThat(error)
.isInstanceOf(WebException.class)
.hasFieldOrPropertyWithValue("body", errorJson)
.cause()
.isInstanceOf(HttpClient.DetailedError.class)
.hasMessage("My message");
});
.hasMessage("My message"));
}

@Test
Expand All @@ -497,14 +496,13 @@ void shouldReturnErrorNonJsonBody() {

TestResource resource = getHttpProxy(server.port());
StepVerifier.create(resource.getHelloMono())
.verifyErrorSatisfies(error -> {
.verifyErrorSatisfies(error ->
assertThat(error)
.isInstanceOf(WebException.class)
.hasFieldOrPropertyWithValue("body", errorNonJson)
.cause()
.isInstanceOf(HttpClient.DetailedError.class)
.hasMessage(errorNonJson);
});
.hasMessage(errorNonJson));
}

@Test
Expand Down Expand Up @@ -594,22 +592,22 @@ void shouldSupportBeanParamRecord() throws Exception {

@Test
void shouldSupportSingleSource() {
server = startServer(HttpResponseStatus.OK, "\"OK\"");
server = startServer(OK, "\"OK\"");

TestResource resource = getHttpProxy(server.port());
resource.getSingle().toBlocking().value();
}

@Test
void shouldHandleLargeResponses() {
server = startServer(HttpResponseStatus.OK, generateLargeString(10));
server = startServer(OK, generateLargeString(10));
TestResource resource = getHttpProxy(server.port());
resource.getHello().toBlocking().single();
}

@Test
void shouldThrowIllegalArgumentExceptionWhenPathParamIsNull() {
server = startServer(HttpResponseStatus.OK, generateLargeString(10));
server = startServer(OK, generateLargeString(10));
TestResource resource = getHttpProxy(server.port());
assertThatExceptionOfType(IllegalArgumentException.class)
.isThrownBy(() -> resource.getPathParam(null).block())
Expand All @@ -618,7 +616,7 @@ void shouldThrowIllegalArgumentExceptionWhenPathParamIsNull() {

@Test
void shouldLogErrorOnTooLargeResponse() {
server = startServer(HttpResponseStatus.OK, generateLargeString(11));
server = startServer(OK, generateLargeString(11));
TestResource resource = getHttpProxy(server.port());
assertThatExceptionOfType(WebException.class)
.isThrownBy(() -> resource.getHello().toBlocking().single())
Expand Down Expand Up @@ -704,7 +702,7 @@ void shouldRedactSensitiveHeaderInLogsAndExceptionMessage() throws Exception {

@Test
void shouldReturnBadRequestOnTooLargeResponses() throws URISyntaxException {
server = startServer(HttpResponseStatus.OK, "\"derp\"");
server = startServer(OK, "\"derp\"");
HttpClientConfig config = new HttpClientConfig("127.0.0.1:" + server.port());
config.setMaxResponseSize(5);
TestResource resource = getHttpProxy(config);
Expand All @@ -718,7 +716,7 @@ void shouldReturnBadRequestOnTooLargeResponses() throws URISyntaxException {

@Test
void shouldSupportByteArrayResponse() {
server = startServer(HttpResponseStatus.OK, "hej");
server = startServer(OK, "hej");
TestResource resource = getHttpProxy(server.port());
byte[] result = resource.getAsBytes().toBlocking().single();
assertThat(new String(result)).isEqualTo("hej");
Expand Down Expand Up @@ -852,7 +850,7 @@ void shouldThrowNettyReadTimeoutIfRequestTakesLongerThanClientIsConfigured() {
@Test
void shouldHandleMultipleChunks() {
server = HttpServer.create().port(0).handle((request, response) -> {
response.status(HttpResponseStatus.OK);
response.status(OK);
return response.sendString(just("\"he")
.concatWith(defer(() -> just("llo\"")))
.concatWith(defer(Flux::empty)));
Expand Down Expand Up @@ -923,18 +921,18 @@ private DisposableServer createTestServer(Consumer<String> serverLog) {
return HttpServer.create().host("localhost").port(0).handle((request, response) -> {
serverLog.accept(request.fullPath());
if (request.fullPath().equals("/hello/servertest/fast")) {
response.status(HttpResponseStatus.OK);
response.status(OK);
return response.sendString(just("\"fast\""));
}
if (request.fullPath().equals("/hello/servertest/slowHeaders")) {
return Flux.defer(() -> {
response.status(HttpResponseStatus.OK);
response.status(OK);
return response.sendString(just("\"slowHeaders\""));
})
.delaySubscription(Duration.ofMillis(5000));
}
if (request.fullPath().equals("/hello/servertest/slowBody")) {
response.status(HttpResponseStatus.OK);
response.status(OK);
return response.sendString(
just("\"slowBody: ")
.concatWith(just("1", "2", "3").delaySequence(Duration.ofMillis(10000)))
Expand Down Expand Up @@ -988,16 +986,14 @@ void shouldShouldNotGivePoolExhaustedIfServerDoesNotCloseConnection() throws URI

try {
server = HttpServer.create().port(0)
.handle((request, response) -> {
return Flux.defer(() -> {
response.status(HttpResponseStatus.OK);
.handle((request, response) ->
Flux.defer(() -> {
response.status(OK);
return response.sendString(Mono.just("\"hello\""));
})
.delaySubscription(Duration.ofMillis(50000))
.doOnError(e -> {
e.printStackTrace();
});
}).bindNow();
.doOnError(Throwable::printStackTrace)
).bindNow();

// this config ensures that the autocleanup will run before the hystrix timeout
HttpClientConfig config = new HttpClientConfig("localhost:" + server.port());
Expand All @@ -1022,7 +1018,7 @@ void shouldShouldNotGivePoolExhaustedIfServerDoesNotCloseConnection() throws URI
@Test
void willRequestWithMultipleCookies() {
Consumer<HttpServerRequest> reqLog = mock(Consumer.class);
server = startServer(OK, "", reqLog::accept);
server = startServer(OK, "", reqLog);

String cookie1Value = "stub1";
String cookie2Value = "stub2";
Expand Down Expand Up @@ -1235,7 +1231,7 @@ void shouldAllowBodyInPostPutDeletePatchCalls() {
void shouldBeAbleToSendAndReceiveRecordBody() {
server = HttpServer.create().port(0).handle((request, response) ->
request.receive().aggregate().flatMap(buf ->
response.status(HttpResponseStatus.OK).send(Mono.just(buf)).then()
response.status(OK).send(Mono.just(buf)).then()
)).bindNow();

var resource = getHttpProxy(server.port());
Expand Down Expand Up @@ -1275,7 +1271,7 @@ void shouldSetTimeoutOnResource() throws URISyntaxException {
@Test
void shouldExecutePreRequestHooks() throws URISyntaxException {
server = HttpServer.create().port(0).handle((request, response) -> {
response.status(HttpResponseStatus.OK);
response.status(OK);
return response.sendString(Mono.just("\"hi\""));
}).bindNow();

Expand All @@ -1291,9 +1287,9 @@ void shouldExecutePreRequestHooks() throws URISyntaxException {
TestResource resource = client.create(TestResource.class);
resource.getHello().toBlocking().single();

verify(preRequestHook, times(1)).apply((TestUtil.matches(requestBuilder -> {
assertThat(requestBuilder.getFullUrl()).isEqualToIgnoringCase(url + "/hello");
})));
verify(preRequestHook, times(1)).apply((TestUtil.matches(requestBuilder ->
assertThat(requestBuilder.getFullUrl()).isEqualToIgnoringCase(url + "/hello")
)));
}

@Test
Expand Down Expand Up @@ -1427,7 +1423,7 @@ private Injector injectorWithProgrammaticHttpClientConfig(HttpClientConfig httpC
@Test
void assertRequestContainsHost() {
Consumer<HttpServerRequest> reqLog = mock(Consumer.class);
server = startServer(OK, "", reqLog::accept);
server = startServer(OK, "", reqLog);

String host = "localhost";

Expand All @@ -1442,7 +1438,7 @@ void assertRequestContainsHost() {
@Test
void assertRequestContainsHostFromHeaderParam() {
Consumer<HttpServerRequest> reqLog = mock(Consumer.class);
server = startServer(OK, "", reqLog::accept);
server = startServer(OK, "", reqLog);

String host = "globalhost";

Expand Down Expand Up @@ -1523,6 +1519,48 @@ void shouldFailIfBodyIsNotStringOrBytes() {
}
}

@Test
void shouldDisregardCharsetInContentTypeWhenComparingToResourceAnnotation() {
String someRecordJson = """
[{
"param1": "a",
"param2": "b"
},
{
"param1": "c",
"param2": "d"
}
]""";
server = HttpServer.create().port(0).handle((request, response) ->
response.status(OK)
.header("Content-Type", "application/json; charset=utf-8")
.sendString(Mono.just(someRecordJson))
.then()).bindNow();
TestResource resource = getHttpProxy(server.port());

StepVerifier.create(resource.producesJson())
.expectNext(new TestResource.SomeRecord("a", "b"))
.expectNext(new TestResource.SomeRecord("c", "d"))
.verifyComplete();
loggingVerifier.assertThatLogs()
.noneSatisfy(logEvent ->
assertThat(logEvent.getMessage().getFormattedMessage())
.contains("does not match the Content-Type"));
}

@Test
void shouldHandleMissingContentTypeInResponse() {
server = HttpServer.create().port(0).handle((request, response) ->
response.status(OK)
.sendString(Mono.just("[]"))
.then()).bindNow();
TestResource resource = getHttpProxy(server.port());

StepVerifier.create(resource.producesJson())
.expectNextCount(1)
.verifyComplete();
}

@Path("/hello")
public interface TestResource {

Expand Down Expand Up @@ -1656,6 +1694,9 @@ Observable<String> postForm(@FormParam("paramA") String a,
@Consumes("application/xml")
Observable<Void> sendXml(Pojo pojo);

@GET
Flux<SomeRecord> producesJson();

}

static class Wrapper {
Expand Down

0 comments on commit fb95475

Please sign in to comment.