From cbb81e32d07286e7216e1a6ab7fb102b1da24639 Mon Sep 17 00:00:00 2001 From: Pritham Marupaka Date: Tue, 21 Jan 2025 18:37:19 -0500 Subject: [PATCH] handle empty body --- .../java/dialogue/serde/ConjureBodySerDe.java | 8 +++-- .../serde/JacksonEmptyContainerLoader.java | 31 +++++++++++++++++ .../dialogue/serde/ConjureBodySerDeTest.java | 16 +++++++++ .../EndpointErrorsConjureBodySerDeTest.java | 33 ++++++++++++++++++- 4 files changed, 84 insertions(+), 4 deletions(-) diff --git a/dialogue-serde/src/main/java/com/palantir/conjure/java/dialogue/serde/ConjureBodySerDe.java b/dialogue-serde/src/main/java/com/palantir/conjure/java/dialogue/serde/ConjureBodySerDe.java index 845a0b159..253d04faf 100644 --- a/dialogue-serde/src/main/java/com/palantir/conjure/java/dialogue/serde/ConjureBodySerDe.java +++ b/dialogue-serde/src/main/java/com/palantir/conjure/java/dialogue/serde/ConjureBodySerDe.java @@ -249,14 +249,16 @@ private static final class EncodingDeserializerForEndpointRegistry implements private final ImmutableList> encodings; private final EndpointErrorDecoder endpointErrorDecoder; private final Optional acceptValue; - private final Supplier> emptyInstance; + private final Supplier> emptyInstance; private final TypeMarker token; + private final TypeMarker successTypeMarker; EncodingDeserializerForEndpointRegistry( List encodingsSortedByWeight, EmptyContainerDeserializer empty, TypeMarker token, DeserializerArgs deserializersForEndpoint) { + this.successTypeMarker = deserializersForEndpoint.successType(); this.encodings = encodingsSortedByWeight.stream() .map(encoding -> new EncodingDeserializerContainer<>(encoding, deserializersForEndpoint.successType())) @@ -267,7 +269,7 @@ private static final class EncodingDeserializerForEndpointRegistry implements .filter(encoding -> encoding.supportsContentType("application/json")) .findFirst()); this.token = token; - this.emptyInstance = Suppliers.memoize(() -> empty.tryGetEmptyInstance(token)); + this.emptyInstance = Suppliers.memoize(() -> empty.tryGetEmptyInstance(successTypeMarker)); // Encodings are applied to the accept header in the order of preference based on the provided list. this.acceptValue = Optional.of(encodingsSortedByWeight.stream() .map(Encoding::getContentType) @@ -284,7 +286,7 @@ public T deserialize(Response response) { // TODO(dfox): what if we get a 204 for a non-optional type??? // TODO(dfox): support http200 & body=null // TODO(dfox): what if we were expecting an empty list but got {}? - Optional maybeEmptyInstance = emptyInstance.get(); + Optional maybeEmptyInstance = emptyInstance.get(); if (maybeEmptyInstance.isPresent()) { return maybeEmptyInstance.get(); } diff --git a/dialogue-serde/src/main/java/com/palantir/conjure/java/dialogue/serde/JacksonEmptyContainerLoader.java b/dialogue-serde/src/main/java/com/palantir/conjure/java/dialogue/serde/JacksonEmptyContainerLoader.java index 5f03e3399..3a9a16c3c 100644 --- a/dialogue-serde/src/main/java/com/palantir/conjure/java/dialogue/serde/JacksonEmptyContainerLoader.java +++ b/dialogue-serde/src/main/java/com/palantir/conjure/java/dialogue/serde/JacksonEmptyContainerLoader.java @@ -63,6 +63,11 @@ private Optional constructEmptyInstance(Type type, TypeMarker origina return jacksonInstance; } + Method noArgStaticFactoryMethod = getFirstNoArgCreatorStaticMethod(type); + if (noArgStaticFactoryMethod != null) { + return invokeNoArgStaticFactoryMethod(noArgStaticFactoryMethod); + } + // fallback to manual reflection to handle aliases of optionals (and aliases of aliases of optionals) Method method = getJsonCreatorStaticMethod(type); if (method != null) { @@ -152,6 +157,32 @@ private static Method getJsonCreatorStaticMethod(Type type) { return null; } + @Nullable + private static Method getFirstNoArgCreatorStaticMethod(Type type) { + if (type instanceof Class) { + Class clazz = (Class) type; + for (Method method : clazz.getMethods()) { + if (Modifier.isStatic(method.getModifiers()) + && method.getParameterCount() == 0 + && method.getAnnotation(JsonCreator.class) != null) { + return method; + } + } + } + return null; + } + + private static Optional invokeNoArgStaticFactoryMethod(Method method) { + try { + return Optional.ofNullable(method.invoke(null)); + } catch (IllegalAccessException | InvocationTargetException e) { + if (log.isDebugEnabled()) { + log.debug("Reflection instantiation failed", e); + } + return Optional.empty(); + } + } + private static Optional invokeStaticFactoryMethod(Method method, Object parameter) { try { return Optional.ofNullable(method.invoke(null, parameter)); diff --git a/dialogue-serde/src/test/java/com/palantir/conjure/java/dialogue/serde/ConjureBodySerDeTest.java b/dialogue-serde/src/test/java/com/palantir/conjure/java/dialogue/serde/ConjureBodySerDeTest.java index 03879e807..d6ca5439e 100644 --- a/dialogue-serde/src/test/java/com/palantir/conjure/java/dialogue/serde/ConjureBodySerDeTest.java +++ b/dialogue-serde/src/test/java/com/palantir/conjure/java/dialogue/serde/ConjureBodySerDeTest.java @@ -20,6 +20,7 @@ import static org.assertj.core.api.Assertions.assertThatExceptionOfType; import static org.assertj.core.api.Assertions.assertThatThrownBy; +import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.ObjectMapper; import com.google.common.collect.ImmutableList; @@ -69,6 +70,21 @@ public void testRequestOptionalEmpty() { assertThat(value).isEmpty(); } + @Test + public void testRequestCustomEmpty() { + record EmptyRecord() { + @JsonCreator + public static EmptyRecord create() { + return new EmptyRecord(); + } + } + TestResponse response = new TestResponse().code(204); + BodySerDe serializers = conjureBodySerDe("application/json"); + EmptyRecord value = + serializers.deserializer(new TypeMarker() {}).deserialize(response); + assertThat(value).isNotNull(); + } + private ConjureBodySerDe conjureBodySerDe(String... contentTypes) { return new ConjureBodySerDe( Arrays.stream(contentTypes) diff --git a/dialogue-serde/src/test/java/com/palantir/conjure/java/dialogue/serde/EndpointErrorsConjureBodySerDeTest.java b/dialogue-serde/src/test/java/com/palantir/conjure/java/dialogue/serde/EndpointErrorsConjureBodySerDeTest.java index 858e9df20..771c3018c 100644 --- a/dialogue-serde/src/test/java/com/palantir/conjure/java/dialogue/serde/EndpointErrorsConjureBodySerDeTest.java +++ b/dialogue-serde/src/test/java/com/palantir/conjure/java/dialogue/serde/EndpointErrorsConjureBodySerDeTest.java @@ -59,6 +59,18 @@ public class EndpointErrorsConjureBodySerDeTest { private static final ObjectMapper MAPPER = ObjectMappers.newServerObjectMapper(); + @Generated("by conjure-java") + private sealed interface EmptyBodyEndpointReturnBaseType permits EmptyReturnValue, ErrorReturnValue {} + + @Generated("by conjure-java") + record EmptyReturnValue() implements EmptyBodyEndpointReturnBaseType { + @JsonCreator + public static EmptyReturnValue create() { + return new EmptyReturnValue(); + } + + } + @Generated("by conjure-java") private sealed interface EndpointReturnBaseType permits ExpectedReturnValue, ErrorReturnValue {} @@ -80,7 +92,8 @@ record ErrorForEndpointArgs( @JsonProperty("complexArg") @Safe ComplexArg complexArg, @JsonProperty("optionalArg") @Safe Optional optionalArg) {} - static final class ErrorReturnValue extends EndpointError implements EndpointReturnBaseType { + static final class ErrorReturnValue extends EndpointError + implements EndpointReturnBaseType, EmptyBodyEndpointReturnBaseType { @JsonCreator ErrorReturnValue( @JsonProperty("errorCode") String errorCode, @@ -209,6 +222,24 @@ public void testDeserializeExpectedValue() { assertThat(value).isEqualTo(new ExpectedReturnValue(expectedString)); } + @Test + public void testDeserializeEmptyBody() { + // Given + TestResponse response = new TestResponse().code(204); + BodySerDe serializers = conjureBodySerDe("application/json", "text/plain"); + DeserializerArgs deserializerArgs = + DeserializerArgs.builder() + .baseType(new TypeMarker<>() {}) + .success(new TypeMarker() {}) + .error("Default:FailedPrecondition", new TypeMarker() {}) + .build(); + // When + EmptyBodyEndpointReturnBaseType value = + serializers.deserializer(deserializerArgs).deserialize(response); + // Then + assertThat(value).isEqualTo(new EmptyReturnValue()); + } + // Ensure that the supplied JSON encoding is used when available. @Test public void testDeserializeWithCustomEncoding() throws JsonProcessingException {