Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deserialize endpoint errors #2443

Open
wants to merge 18 commits into
base: develop
Choose a base branch
from
Prev Previous commit
Next Next commit
Support inputStreamDeserializer and optionalInputStreamDeserializer
  • Loading branch information
Pritham Marupaka committed Jan 23, 2025
commit 01a1589e67b1ed28dfb6841357af2f01ce0102b7
Original file line number Diff line number Diff line change
@@ -47,8 +47,10 @@
import java.util.Comparator;
import java.util.List;
import java.util.Optional;
import java.util.function.Function;
import java.util.function.Supplier;
import java.util.stream.Collectors;
import javax.annotation.Nullable;

/** Package private internal API. */
final class ConjureBodySerDe implements BodySerDe {
@@ -60,7 +62,7 @@ final class ConjureBodySerDe implements BodySerDe {
private final Deserializer<Optional<InputStream>> optionalBinaryInputStreamDeserializer;
private final Deserializer<Void> emptyBodyDeserializer;
private final LoadingCache<Type, Serializer<?>> serializers;
private final LoadingCache<Type, EncodingDeserializerForEndpointRegistry<?>> deserializers;
private final LoadingCache<Type, EncodingDeserializerForEndpointRegistry<?, ?>> deserializers;
private final EmptyContainerDeserializer emptyContainerDeserializer;

/**
@@ -77,15 +79,15 @@ final class ConjureBodySerDe implements BodySerDe {
Preconditions.checkArgument(encodings.size() > 0, "At least one Encoding is required");
this.defaultEncoding = encodings.get(0).encoding();
this.emptyContainerDeserializer = emptyContainerDeserializer;
this.binaryInputStreamDeserializer = new EncodingDeserializerForEndpointRegistry<>(
this.binaryInputStreamDeserializer = EncodingDeserializerForEndpointRegistry.create(
ImmutableList.of(BinaryEncoding.INSTANCE),
emptyContainerDeserializer,
BinaryEncoding.MARKER,
DeserializerArgs.<InputStream>builder()
.baseType(BinaryEncoding.MARKER)
.success(BinaryEncoding.MARKER)
.build());
this.optionalBinaryInputStreamDeserializer = new EncodingDeserializerForEndpointRegistry<>(
this.optionalBinaryInputStreamDeserializer = EncodingDeserializerForEndpointRegistry.create(
ImmutableList.of(BinaryEncoding.INSTANCE),
emptyContainerDeserializer,
BinaryEncoding.OPTIONAL_MARKER,
@@ -101,8 +103,8 @@ final class ConjureBodySerDe implements BodySerDe {
this.deserializers = Caffeine.from(cacheSpec).build(type -> buildCacheEntry(TypeMarker.of(type)));
}

private <T> EncodingDeserializerForEndpointRegistry<?> buildCacheEntry(TypeMarker<T> typeMarker) {
return new EncodingDeserializerForEndpointRegistry<>(
private <T> EncodingDeserializerForEndpointRegistry<?, ?> buildCacheEntry(TypeMarker<T> typeMarker) {
return EncodingDeserializerForEndpointRegistry.create(
encodingsSortedByWeight,
emptyContainerDeserializer,
typeMarker,
@@ -143,11 +145,8 @@ public <T> Deserializer<T> deserializer(TypeMarker<T> token) {
@Override
@SuppressWarnings("unchecked")
public <T> Deserializer<T> deserializer(DeserializerArgs<T> deserializerArgs) {
return new EncodingDeserializerForEndpointRegistry<>(
encodingsSortedByWeight,
emptyContainerDeserializer,
(TypeMarker<T>) deserializerArgs.baseType(),
deserializerArgs);
return EncodingDeserializerForEndpointRegistry.create(
encodingsSortedByWeight, emptyContainerDeserializer, deserializerArgs.baseType(), deserializerArgs);
}

@Override
@@ -160,11 +159,34 @@ public Deserializer<InputStream> inputStreamDeserializer() {
return binaryInputStreamDeserializer;
}

@Override
@SuppressWarnings("unchecked")
public <T> Deserializer<T> inputStreamDeserializer(DeserializerArgs<T> deserializerArgs) {
return new EncodingDeserializerForEndpointRegistry<>(
encodingsSortedByWeight,
emptyContainerDeserializer,
deserializerArgs.baseType(),
deserializerArgs,
BinaryEncoding.MARKER,
is -> createSuccessType(deserializerArgs.successType(), is));
}

@Override
public Deserializer<Optional<InputStream>> optionalInputStreamDeserializer() {
return optionalBinaryInputStreamDeserializer;
}

@Override
public <T> Deserializer<T> optionalInputStreamDeserializer(DeserializerArgs<T> deserializerArgs) {
return new EncodingDeserializerForEndpointRegistry<>(
encodingsSortedByWeight,
emptyContainerDeserializer,
deserializerArgs.baseType(),
deserializerArgs,
BinaryEncoding.OPTIONAL_MARKER,
ois -> createSuccessType(deserializerArgs.successType(), ois));
}

@Override
public RequestBody serialize(BinaryRequestBody value) {
Preconditions.checkNotNull(value, "A BinaryRequestBody value is required");
@@ -244,24 +266,41 @@ private static final class EncodingSerializerContainer<T> {
}
}

private static final class EncodingDeserializerForEndpointRegistry<T> implements Deserializer<T> {
private static final class EncodingDeserializerForEndpointRegistry<S, T> implements Deserializer<T> {
private static final SafeLogger log = SafeLoggerFactory.get(EncodingDeserializerForEndpointRegistry.class);
private final ImmutableList<EncodingDeserializerContainer<? extends T>> encodings;
private final ImmutableList<EncodingDeserializerContainer<? extends S>> encodings;
private final EndpointErrorDecoder<T> endpointErrorDecoder;
private final Optional<String> acceptValue;
private final Supplier<Optional<? extends T>> emptyInstance;
private final TypeMarker<T> token;
private final TypeMarker<? extends T> successTypeMarker;
private final @Nullable Function<S, T> transform;

EncodingDeserializerForEndpointRegistry(
@SuppressWarnings("unchecked")
static <T> EncodingDeserializerForEndpointRegistry<T, T> create(
List<Encoding> encodingsSortedByWeight,
EmptyContainerDeserializer empty,
TypeMarker<T> token,
DeserializerArgs<T> deserializersForEndpoint) {
return new EncodingDeserializerForEndpointRegistry<>(
encodingsSortedByWeight,
empty,
token,
deserializersForEndpoint,
(TypeMarker<T>) deserializersForEndpoint.successType(),
null);
}

EncodingDeserializerForEndpointRegistry(
List<Encoding> encodingsSortedByWeight,
EmptyContainerDeserializer empty,
TypeMarker<T> token,
DeserializerArgs<T> deserializersForEndpoint,
TypeMarker<S> intermediateResult,
@Nullable Function<S, T> transform) {
this.successTypeMarker = deserializersForEndpoint.successType();
this.encodings = encodingsSortedByWeight.stream()
.map(encoding ->
new EncodingDeserializerContainer<>(encoding, deserializersForEndpoint.successType()))
.map(encoding -> new EncodingDeserializerContainer<>(encoding, intermediateResult))
.collect(ImmutableList.toImmutableList());
this.endpointErrorDecoder = new EndpointErrorDecoder<>(
deserializersForEndpoint.errorNameToTypeMarker(),
@@ -270,13 +309,14 @@ private static final class EncodingDeserializerForEndpointRegistry<T> implements
.findFirst());
this.token = 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)
.collect(Collectors.joining(", ")));
this.transform = transform;
}

@Override
@SuppressWarnings("unchecked")
public T deserialize(Response response) {
boolean closeResponse = true;
try {
@@ -300,11 +340,21 @@ public T deserialize(Response response) {
"Response is missing Content-Type header",
SafeArg.of("received", response.headers().keySet()));
}
Encoding.Deserializer<? extends T> deserializer = getResponseDeserializer(contentType.get());
T deserialized = deserializer.deserialize(response.body());
Encoding.Deserializer<? extends S> deserializer = getResponseDeserializer(contentType.get());
S deserialized = deserializer.deserialize(response.body());
// deserializer has taken on responsibility for closing the response body
closeResponse = false;
return deserialized;
if (transform == null) {
return (T) deserialized;
}
T responseValue = transform.apply(deserialized);
if (responseValue == null) {
throw new SafeRuntimeException(
"Failed to create success type",
SafeArg.of("type", token),
SafeArg.of("deserialized", deserialized));
}
return responseValue;
} catch (IOException e) {
throw new SafeRuntimeException(
"Failed to deserialize response stream",
@@ -326,30 +376,27 @@ public Optional<String> accepts() {
/** Returns the {@link EncodingDeserializerContainer} to use to deserialize the request body. */
@SuppressWarnings("ForLoopReplaceableByForEach")
// performance sensitive code avoids iterator allocation
Encoding.Deserializer<? extends T> getResponseDeserializer(String contentType) {
Encoding.Deserializer<? extends S> getResponseDeserializer(String contentType) {
for (int i = 0; i < encodings.size(); i++) {
EncodingDeserializerContainer<? extends T> container = encodings.get(i);
EncodingDeserializerContainer<? extends S> container = encodings.get(i);
if (container.encoding.supportsContentType(contentType)) {
return container.deserializer;
}
}
return throwingDeserializer(contentType);
}

private Encoding.Deserializer<T> throwingDeserializer(String contentType) {
return new Encoding.Deserializer<T>() {
@Override
public T deserialize(InputStream input) {
try {
input.close();
} catch (RuntimeException | IOException e) {
log.warn("Failed to close InputStream", e);
}
throw new SafeRuntimeException(
"Unsupported Content-Type",
SafeArg.of("received", contentType),
SafeArg.of("supportedEncodings", encodings));
private Encoding.Deserializer<S> throwingDeserializer(String contentType) {
return input -> {
try {
input.close();
} catch (RuntimeException | IOException e) {
log.warn("Failed to close InputStream", e);
}
throw new SafeRuntimeException(
"Unsupported Content-Type",
SafeArg.of("received", contentType),
SafeArg.of("supportedEncodings", encodings));
};
}
}
@@ -400,4 +447,35 @@ public String toString() {
return "EmptyBodyDeserializer{}";
}
}

@SuppressWarnings("unchecked")
@Nullable
private static <T> T createSuccessType(TypeMarker<T> successT, InputStream inputStream) {
if (successT.getType() instanceof Class<?>) {
try {
return (T) ((Class<?>) successT.getType())
.getConstructor(InputStream.class)
.newInstance(inputStream);
} catch (ReflectiveOperationException e) {
throw new SafeRuntimeException("Failed to create success type", e);
}
}
return null;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in the else case, we should throw rather than returning a null ref that will cause something else to throw in a more confusing way later on!

I might suggest splitting this up a bit as well: We should be able to do the validation on successT.getType() once when the deserializer is first created, as well as the .getConstructor(<arg>), that way we only need to execute newInstance on a per-response basis.

}

@SuppressWarnings("unchecked")
@Nullable
private static <T> T createSuccessType(TypeMarker<T> successT, Optional<InputStream> optionalInputStream) {
if (successT.getType() instanceof Class<?>) {
try {
return (T) ((Class<?>) successT.getType())
// TODO(pm): check that the param is Optional<InputStream>
.getConstructor(Optional.class)
.newInstance(optionalInputStream);
} catch (ReflectiveOperationException e) {
throw new SafeRuntimeException("Failed to create success type", e);
}
}
return null;
}
}
Original file line number Diff line number Diff line change
@@ -24,6 +24,7 @@
import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.google.common.collect.ImmutableList;
import com.google.errorprone.annotations.MustBeClosed;
import com.palantir.conjure.java.api.errors.CheckedServiceException;
import com.palantir.conjure.java.api.errors.ErrorType;
import com.palantir.conjure.java.api.errors.RemoteException;
@@ -43,6 +44,7 @@
import com.palantir.logsafe.Unsafe;
import com.palantir.logsafe.UnsafeArg;
import java.io.IOException;
import java.io.InputStream;
import java.util.Arrays;
import java.util.List;
import java.util.Optional;
@@ -73,6 +75,9 @@ public static EmptyReturnValue create() {
@Generated("by conjure-java")
private sealed interface EndpointReturnBaseType permits ExpectedReturnValue, ErrorReturnValue {}

@Generated("by conjure-java")
private sealed interface EndpointBinaryReturnBaseType permits BinaryReturnValue, ErrorReturnValue {}

@Generated("by conjure-java")
record ExpectedReturnValue(String value) implements EndpointReturnBaseType {
@JsonCreator
@@ -81,6 +86,13 @@ public static ExpectedReturnValue create(String value) {
}
}

@Generated("by conjure-java")
record BinaryReturnValue(@MustBeClosed InputStream value) implements EndpointBinaryReturnBaseType {
public BinaryReturnValue {
Preconditions.checkArgumentNotNull(value, "value cannot be null");
}
}

@Generated("by conjure-java")
record ComplexArg(int foo, String bar) {}

@@ -92,7 +104,9 @@ record ErrorForEndpointArgs(
@JsonProperty("optionalArg") @Safe Optional<Integer> optionalArg) {}

static final class ErrorReturnValue extends EndpointError<ErrorForEndpointArgs>
implements EndpointReturnBaseType, EmptyBodyEndpointReturnBaseType {
implements EndpointErrorsConjureBodySerDeTest.EndpointReturnBaseType,
EndpointErrorsConjureBodySerDeTest.EmptyBodyEndpointReturnBaseType,
EndpointErrorsConjureBodySerDeTest.EndpointBinaryReturnBaseType {
@JsonCreator
ErrorReturnValue(
@JsonProperty("errorCode") String errorCode,
@@ -137,7 +151,7 @@ public void testDeserializeCustomError(String supportedContentType) throws IOExc
BodySerDe serializers = conjureBodySerDe(supportedContentType);
DeserializerArgs<EndpointReturnBaseType> deserializerArgs = DeserializerArgs.<EndpointReturnBaseType>builder()
.baseType(new TypeMarker<>() {})
.success(new TypeMarker<ExpectedReturnValue>() {})
.success(new TypeMarker<EndpointErrorsConjureBodySerDeTest.ExpectedReturnValue>() {})
.error("Default:FailedPrecondition", new TypeMarker<ErrorReturnValue>() {})
.build();

@@ -221,6 +235,38 @@ public void testDeserializeExpectedValue() {
assertThat(value).isEqualTo(new ExpectedReturnValue(expectedString));
}

@Test
public void testDeserializeBinaryValue() {
// Given
byte[] is = new byte[] {1, 2, 3};
TestResponse response =
new TestResponse(is).contentType("application/octet-stream").code(200);
// BodySerDe serializers = conjureBodySerDe("application/octet-stream", "text/plain");

BodySerDe serializers = new ConjureBodySerDe(
ImmutableList.of(WeightedEncoding.of(BinaryEncoding.INSTANCE)),
Encodings.emptyContainerDeserializer(),
DefaultConjureRuntime.DEFAULT_SERDE_CACHE_SPEC);

DeserializerArgs<EndpointBinaryReturnBaseType> deserializerArgs =
DeserializerArgs.<EndpointBinaryReturnBaseType>builder()
.baseType(new TypeMarker<>() {})
.success(new TypeMarker<BinaryReturnValue>() {})
.error("Default:FailedPrecondition", new TypeMarker<ErrorReturnValue>() {})
.build();
// When
EndpointBinaryReturnBaseType value =
serializers.inputStreamDeserializer(deserializerArgs).deserialize(response);
// Then
assertThat(value).isInstanceOfSatisfying(BinaryReturnValue.class, binaryReturnValue -> {
try {
assertThat(binaryReturnValue.value.readAllBytes()).isEqualTo(is);
} catch (IOException e) {
throw new RuntimeException(e);
}
});
}

@Test
public void testDeserializeEmptyBody() {
// Given
@@ -229,7 +275,7 @@ public void testDeserializeEmptyBody() {
DeserializerArgs<EmptyBodyEndpointReturnBaseType> deserializerArgs =
DeserializerArgs.<EmptyBodyEndpointReturnBaseType>builder()
.baseType(new TypeMarker<>() {})
.success(new TypeMarker<EmptyReturnValue>() {})
.success(new TypeMarker<EndpointErrorsConjureBodySerDeTest.EmptyReturnValue>() {})
.error("Default:FailedPrecondition", new TypeMarker<ErrorReturnValue>() {})
.build();
// When
Original file line number Diff line number Diff line change
@@ -43,9 +43,13 @@ public interface BodySerDe {
*/
Deserializer<InputStream> inputStreamDeserializer();

<T> Deserializer<T> inputStreamDeserializer(DeserializerArgs<T> deserializerArgs);
Copy link
Contributor Author

@mpritham mpritham Jan 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also added methods to deserialize input streams and optional input streams. The idea is that the type T.Success here has a constructor with InputStream as the only parameter, and on a successful binary response body, a SuccessT(response.body()) will be returned.


/** Same as {@link #inputStreamDeserializer()} with support for 204 responses. */
Deserializer<Optional<InputStream>> optionalInputStreamDeserializer();

<T> Deserializer<T> optionalInputStreamDeserializer(DeserializerArgs<T> deserializerArgs);

/** Serializes a {@link BinaryRequestBody} to <pre>application/octet-stream</pre>. */
RequestBody serialize(BinaryRequestBody value);
}