Skip to content

Commit

Permalink
Fix ControllerConstraintHandlerTest for JDK client (#9299)
Browse files Browse the repository at this point in the history
This fixes the StreamTest and FilterErrorTest TCK tests that were failing in #9299

Closes #9223
Closes #9300
  • Loading branch information
timyates authored Jun 22, 2023
1 parent a0b9724 commit a666245
Show file tree
Hide file tree
Showing 4 changed files with 126 additions and 18 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,11 @@
import io.micronaut.http.HttpResponse;
import io.micronaut.http.HttpStatus;
import io.micronaut.http.MediaType;
import io.micronaut.http.codec.CodecException;
import io.micronaut.http.codec.MediaTypeCodec;
import io.micronaut.http.codec.MediaTypeCodecRegistry;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import java.nio.charset.Charset;
import java.nio.charset.StandardCharsets;
Expand All @@ -45,6 +48,8 @@
@Experimental
public class HttpResponseAdapter<O> implements HttpResponse<O> {

private static final Logger LOG = LoggerFactory.getLogger(HttpResponseAdapter.class);

private final java.net.http.HttpResponse<byte[]> httpResponse;
@NonNull
private final Argument<O> bodyType;
Expand Down Expand Up @@ -93,21 +98,47 @@ public Optional<O> getBody() {
return convertBytes(getContentType().orElse(null), httpResponse.body(), bodyType);
}

@Override
public <T> Optional<T> getBody(Argument<T> type) {
return convertBytes(getContentType().orElse(null), httpResponse.body(), type);
}

private <T> Optional convertBytes(@Nullable MediaType contentType, byte[] bytes, Argument<T> type) {
if (type != null && mediaTypeCodecRegistry != null && contentType != null) {
if (CharSequence.class.isAssignableFrom(type.getType())) {
final boolean isOptional = type.getType() == Optional.class;
final Argument finalArgument = isOptional ? type.getFirstTypeVariable().orElse(type) : type;

if (mediaTypeCodecRegistry != null && contentType != null) {
if (CharSequence.class.isAssignableFrom(finalArgument.getType())) {
Charset charset = contentType.getCharset().orElse(StandardCharsets.UTF_8);
return Optional.of(new String(bytes, charset));
} else if (type.getType() == byte[].class) {
return Optional.of(bytes);
var converted = Optional.of(new String(bytes, charset));
// If the requested type is an Optional, then we need to wrap the result again
return isOptional ? Optional.of(converted) : converted;
} else if (finalArgument.getType() == byte[].class) {
var converted = Optional.of(bytes);
// If the requested type is an Optional, then we need to wrap the result again
return isOptional ? Optional.of(converted) : converted;
} else {
Optional<MediaTypeCodec> foundCodec = mediaTypeCodecRegistry.findCodec(contentType);
if (foundCodec.isPresent()) {
return foundCodec.map(codec -> codec.decode(type, bytes));
try {
var converted = foundCodec.map(codec -> codec.decode(finalArgument, bytes));
return isOptional ? Optional.of(converted) : converted;
} catch (CodecException e) {
if (LOG.isDebugEnabled()) {
var message = e.getMessage();
LOG.debug("Error decoding body for type [{}] from '{}'. Attempting fallback.", type, contentType);
LOG.debug("CodecException Message was: {}", message == null ? "null" : message.replace("\n", ""));
}
}
}
}
}
// last chance, try type conversion
return type != null ? conversionService.convert(bytes, ConversionContext.of(type)) : Optional.empty();
var converted = conversionService.convert(bytes, ConversionContext.of(finalArgument));
if (isOptional) {
return Optional.of(converted);
} else {
return converted;
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
package io.micronaut.http.client.jdk

import io.micronaut.context.annotation.Property
import io.micronaut.context.annotation.Requires
import io.micronaut.http.annotation.Controller
import io.micronaut.http.annotation.Get
import io.micronaut.http.annotation.QueryValue
import io.micronaut.http.client.annotation.Client
import io.micronaut.test.extensions.spock.annotation.MicronautTest
import jakarta.inject.Inject
import spock.lang.Issue
import spock.lang.Specification

@MicronautTest
@Property(name = "spec.name", value = "RawStringHandlingSpec")
class RawStringHandlingSpec extends Specification {

@Inject
RawClient client

@Issue("https://github.com/micronaut-projects/micronaut-core/issues/9223")
void "test raw return type handling"() {
when:
def result = client.string()

then:
result.get() == "Hello World"

when:
result = client.noString()

then:
result.empty

when:
result = client.optional('present')

then:
result.get() == "Hello World"

when:
result = client.optional('absent')

then:
result.empty
}

@Controller("/raw")
@Requires(property = "spec.name", value = "RawStringHandlingSpec")
static class RawController {

@Get(value = "/string")
String string() {
"Hello World"
}

@Get(value = "/no-string")
String noString() {
null
}

@Get(value = "/optional")
Optional<String> optional(@QueryValue String name) {
if (name == 'present') {
return Optional.of("Hello World")
} else {
return Optional.empty()
}
}
}

@Client("/")
@Requires(property = "spec.name", value = "RawStringHandlingSpec")
static interface RawClient {

@Get("/raw/string")
Optional<String> string()

@Get("/raw/no-string")
Optional<String> noString()

@Get("/raw/optional")
Optional<String> optional(@QueryValue String name)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@
"checkstyle:DesignForExtension"
})
public class ControllerConstraintHandlerTest {

public static final String SPEC_NAME = "ControllerConstraintHandlerTest";
private static final HttpResponseAssertion TEAPOT_ASSERTION = HttpResponseAssertion.builder()
.status(HttpStatus.I_AM_A_TEAPOT)
Expand Down Expand Up @@ -78,11 +79,9 @@ void testPojoWithNullable() throws IOException {
asserts(SPEC_NAME,
HttpRequest.POST("/constraints-via-on-error-method/with-at-nullable", "{\"username\":\"\",\"password\":\"secret\"}"),
(server, request) -> AssertionUtils.assertThrows(server, request, TEAPOT_ASSERTION));

asserts(SPEC_NAME,
HttpRequest.POST("/constraints-via-on-error-method/with-at-nullable", "{\"password\":\"secret\"}"),
(server, request) -> AssertionUtils.assertThrows(server, request, TEAPOT_ASSERTION));

}

@Test
Expand All @@ -96,11 +95,9 @@ void testWithPojoWithoutAnnotations() throws IOException {
asserts(SPEC_NAME,
HttpRequest.POST("/constraints-via-handler", "{\"username\":\"\",\"password\":\"secret\"}"),
(server, request) -> AssertionUtils.assertThrows(server, request, constraintAssertion("must not be blank\"")));

asserts(SPEC_NAME,
HttpRequest.POST("/constraints-via-on-error-method", "{\"username\":\"\",\"password\":\"secret\"}"),
(server, request) -> AssertionUtils.assertThrows(server, request, TEAPOT_ASSERTION));

asserts(SPEC_NAME,
HttpRequest.POST("/constraints-via-on-error-method", "{\"password\":\"secret\"}"),
(server, request) -> AssertionUtils.assertThrows(server, request, TEAPOT_ASSERTION));
Expand All @@ -114,12 +111,9 @@ void testPojoWithNonNullAnnotation() throws IOException {
asserts(SPEC_NAME,
HttpRequest.POST("/constraints-via-handler/with-non-null", "{\"username\":\"\",\"password\":\"secret\"}"),
(server, request) -> AssertionUtils.assertThrows(server, request, constraintAssertion("must not be blank\"")));


asserts(SPEC_NAME,
HttpRequest.POST("/constraints-via-on-error-method/with-non-null", "{\"username\":\"\",\"password\":\"secret\"}"),
(server, request) -> AssertionUtils.assertThrows(server, request, TEAPOT_ASSERTION));

asserts(SPEC_NAME,
HttpRequest.POST("/constraints-via-on-error-method/with-non-null", "{\"password\":\"secret\"}"),
(server, request) -> AssertionUtils.assertThrows(server, request, TEAPOT_ASSERTION));
Expand All @@ -130,8 +124,8 @@ private static HttpResponseAssertion constraintAssertion(String expectedMessage)
.status(HttpStatus.BAD_REQUEST)
.assertResponse(response -> {
Optional<String> json = response.getBody(Argument.of(String.class));
assertTrue(json.isPresent());
assertTrue(json.get().contains(expectedMessage));
assertTrue(json.isPresent(), "response.getBody(Argument.of(String.class)) should be present");
assertTrue(json.get().contains(expectedMessage), "Body '" + json.get() + "' should contain '" + expectedMessage + "'");
}).build();
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
package io.micronaut.http.server.tck.netty.tests;

import org.junit.platform.suite.api.ExcludeClassNamePatterns;
import org.junit.platform.suite.api.ExcludeTags;
import org.junit.platform.suite.api.SelectPackages;
import org.junit.platform.suite.api.Suite;
Expand All @@ -10,6 +9,5 @@
@SelectPackages("io.micronaut.http.server.tck.tests")
@SuiteDisplayName("HTTP Server TCK for Javanet client")
@ExcludeTags("multipart") // Multipart not supported by HttpClient
@ExcludeClassNamePatterns("io.micronaut.http.server.tck.tests.constraintshandler.ControllerConstraintHandlerTest")
public class JdkHttpServerTestSuite {
}

0 comments on commit a666245

Please sign in to comment.