handle empty body

palantir · Jan 21, 2025 · cbb81e3 · cbb81e3
1 parent ae9dfff
commit cbb81e3
Show file tree

Hide file tree

Showing 4 changed files with 84 additions and 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
@@ -249,14 +249,16 @@ private static final class EncodingDeserializerForEndpointRegistry<T> implements
         private final ImmutableList<EncodingDeserializerContainer<? extends T>> encodings;
         private final EndpointErrorDecoder<T> endpointErrorDecoder;
         private final Optional<String> acceptValue;
-        private final Supplier<Optional<T>> emptyInstance;
+        private final Supplier<Optional<? extends T>> emptyInstance;
         private final TypeMarker<T> token;
+        private final TypeMarker<? extends T> successTypeMarker;
 
         EncodingDeserializerForEndpointRegistry(
                 List<Encoding> encodingsSortedByWeight,
                 EmptyContainerDeserializer empty,
                 TypeMarker<T> token,
                 DeserializerArgs<T> 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<T> 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<T> maybeEmptyInstance = emptyInstance.get();
+                    Optional<? extends T> maybeEmptyInstance = emptyInstance.get();
                     if (maybeEmptyInstance.isPresent()) {
                         return maybeEmptyInstance.get();
                     }

diff --git a/...e/src/main/java/com/palantir/conjure/java/dialogue/serde/JacksonEmptyContainerLoader.java b/...e/src/main/java/com/palantir/conjure/java/dialogue/serde/JacksonEmptyContainerLoader.java
@@ -63,6 +63,11 @@ private Optional<Object> 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<Object> 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<Object> invokeStaticFactoryMethod(Method method, Object parameter) {
         try {
             return Optional.ofNullable(method.invoke(null, parameter));

diff --git a/...ue-serde/src/test/java/com/palantir/conjure/java/dialogue/serde/ConjureBodySerDeTest.java b/...ue-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<EmptyRecord>() {}).deserialize(response);
+        assertThat(value).isNotNull();
+    }
+
     private ConjureBodySerDe conjureBodySerDe(String... contentTypes) {
         return new ConjureBodySerDe(
                 Arrays.stream(contentTypes)

diff --git a/...est/java/com/palantir/conjure/java/dialogue/serde/EndpointErrorsConjureBodySerDeTest.java b/...est/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<Integer> optionalArg) {}
 
-    static final class ErrorReturnValue extends EndpointError<ErrorForEndpointArgs> implements EndpointReturnBaseType {
+    static final class ErrorReturnValue extends EndpointError<ErrorForEndpointArgs>
+            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<EmptyBodyEndpointReturnBaseType> deserializerArgs =
+                DeserializerArgs.<EmptyBodyEndpointReturnBaseType>builder()
+                        .baseType(new TypeMarker<>() {})
+                        .success(new TypeMarker<EmptyReturnValue>() {})
+                        .error("Default:FailedPrecondition", new TypeMarker<ErrorReturnValue>() {})
+                        .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 {