Skip to content

Commit

Permalink
review changes
Browse files Browse the repository at this point in the history
  • Loading branch information
Pritham Marupaka committed Jan 9, 2025
1 parent f00bd5f commit 0dafd35
Show file tree
Hide file tree
Showing 6 changed files with 97 additions and 114 deletions.
27 changes: 10 additions & 17 deletions dialogue-serde/src/main/java/com/palantir/conjure/java/dialogue/serde/ConjureBodySerDe.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,19 +82,18 @@ final class ConjureBodySerDe implements BodySerDe {
emptyContainerDeserializer,
BinaryEncoding.MARKER,
DeserializerArgs.<InputStream>builder()
.withBaseType(BinaryEncoding.MARKER)
.withExpectedResult(BinaryEncoding.MARKER)
.baseType(BinaryEncoding.MARKER)
.success(BinaryEncoding.MARKER)
.build());
this.optionalBinaryInputStreamDeserializer = new EncodingDeserializerForEndpointRegistry<>(
ImmutableList.of(BinaryEncoding.INSTANCE),
emptyContainerDeserializer,
BinaryEncoding.OPTIONAL_MARKER,
DeserializerArgs.<Optional<InputStream>>builder()
.withBaseType(BinaryEncoding.OPTIONAL_MARKER)
.withExpectedResult(BinaryEncoding.OPTIONAL_MARKER)
.baseType(BinaryEncoding.OPTIONAL_MARKER)
.success(BinaryEncoding.OPTIONAL_MARKER)
.build());
this.emptyBodyDeserializer =
new EmptyBodyDeserializer(new EndpointErrorDecoder<>(Collections.emptyMap(), Optional.empty()));
this.emptyBodyDeserializer = new EmptyBodyDeserializer(new EndpointErrorDecoder<>(Collections.emptyMap()));
// Class unloading: Not supported, Jackson keeps strong references to the types
// it sees: https://github.com/FasterXML/jackson-databind/issues/489
this.serializers = Caffeine.from(cacheSpec)
Expand All @@ -108,8 +107,8 @@ private <T> EncodingDeserializerForEndpointRegistry<?> buildCacheEntry(TypeMarke
emptyContainerDeserializer,
typeMarker,
DeserializerArgs.<T>builder()
.withBaseType(typeMarker)
.withExpectedResult(typeMarker)
.baseType(typeMarker)
.success(typeMarker)
.build());
}

Expand Down Expand Up @@ -259,16 +258,10 @@ private static final class EncodingDeserializerForEndpointRegistry<T> implements
TypeMarker<T> token,
DeserializerArgs<T> deserializersForEndpoint) {
this.encodings = encodings.stream()
.map(encoding -> new EncodingDeserializerContainer<>(
encoding, deserializersForEndpoint.expectedResultType()))
.map(encoding ->
new EncodingDeserializerContainer<>(encoding, deserializersForEndpoint.successType()))
.collect(ImmutableList.toImmutableList());
this.endpointErrorDecoder = new EndpointErrorDecoder<>(
deserializersForEndpoint.errorNameToTypeMarker(),
// Errors are expected to be JSON objects. See
// https://palantir.github.io/conjure/#/docs/spec/wire?id=_55-conjure-errors.
encodings.stream()
.filter(encoding -> encoding.supportsContentType("application/json"))
.findAny());
this.endpointErrorDecoder = new EndpointErrorDecoder<>(deserializersForEndpoint.errorNameToTypeMarker());
this.token = token;
this.emptyInstance = Suppliers.memoize(() -> empty.tryGetEmptyInstance(token));
// Encodings are applied to the accept header in the order of preference based on the provided list.
Expand Down
36 changes: 16 additions & 20 deletions ...ue-serde/src/main/java/com/palantir/conjure/java/dialogue/serde/EndpointErrorDecoder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@
import com.palantir.conjure.java.api.errors.RemoteException;
import com.palantir.conjure.java.api.errors.SerializableError;
import com.palantir.conjure.java.api.errors.UnknownRemoteException;
import com.palantir.conjure.java.dialogue.serde.Encoding.Deserializer;
import com.palantir.conjure.java.serialization.ObjectMappers;
import com.palantir.dialogue.Response;
import com.palantir.dialogue.TypeMarker;
Expand All @@ -49,15 +48,13 @@
import java.net.URL;
import java.nio.charset.StandardCharsets;
import java.time.Duration;
import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.stream.Collectors;

/**
* Extracts the error from a {@link Response}.
* <p>If the error's name is in the {@link #errorNameToJsonDeserializerMap}, this class attempts to deserialize the
* <p>If the error's name is in the {@link #errorNameToTypeMap}, this class attempts to deserialize the
* {@link Response} body as JSON, to the error type. Otherwise, a {@link RemoteException} is thrown. If the
* {@link Response} does not adhere to the expected format, an {@link UnknownRemoteException} is thrown.
*
Expand All @@ -66,16 +63,13 @@
final class EndpointErrorDecoder<T> {
private static final SafeLogger log = SafeLoggerFactory.get(EndpointErrorDecoder.class);
private static final ObjectMapper MAPPER = ObjectMappers.newClientObjectMapper();
private final Map<String, Encoding.Deserializer<? extends T>> errorNameToJsonDeserializerMap;

EndpointErrorDecoder(
Map<String, TypeMarker<? extends T>> errorNameToTypeMap, Optional<Encoding> maybeJsonEncoding) {
this.errorNameToJsonDeserializerMap = maybeJsonEncoding
.<Map<String, Encoding.Deserializer<? extends T>>>map(
jsonEncoding -> errorNameToTypeMap.entrySet().stream()
.collect(Collectors.toMap(
Map.Entry::getKey, entry -> jsonEncoding.deserializer(entry.getValue()))))
.orElseGet(Collections::emptyMap);
// Errors are expected to be JSON objects. See
// https://palantir.github.io/conjure/#/docs/spec/wire?id=_55-conjure-errors.
private static final Encoding JSON_ENCODING = Encodings.json();
private final Map<String, TypeMarker<? extends T>> errorNameToTypeMap;

EndpointErrorDecoder(Map<String, TypeMarker<? extends T>> errorNameToTypeMap) {
this.errorNameToTypeMap = errorNameToTypeMap;
}

public boolean isError(Response response) {
Expand Down Expand Up @@ -153,13 +147,13 @@ private T decodeInternal(Response response) {
if (errorNameNode == null) {
throwRemoteException(body, code);
}
Optional<Deserializer<? extends T>> maybeDeserializer =
Optional.ofNullable(errorNameToJsonDeserializerMap.get(errorNameNode.asText()));
if (maybeDeserializer.isEmpty()) {
Optional<TypeMarker<? extends T>> maybeType =
Optional.ofNullable(errorNameToTypeMap.get(errorNameNode.asText()));
if (maybeType.isEmpty()) {
throwRemoteException(body, code);
}
return maybeDeserializer
.get()
return JSON_ENCODING
.deserializer(maybeType.get())
.deserialize(new ByteArrayInputStream(body.getBytes(StandardCharsets.UTF_8)));
} catch (RemoteException remoteException) {
// rethrow the created remote exception
Expand All @@ -173,7 +167,9 @@ private T decodeInternal(Response response) {
}

private static void throwRemoteException(String body, int code) throws IOException {
SerializableError serializableError = MAPPER.readValue(body, SerializableError.class);
SerializableError serializableError = JSON_ENCODING
.deserializer(new TypeMarker<SerializableError>() {})
.deserialize(new ByteArrayInputStream(body.getBytes(StandardCharsets.UTF_8)));
throw new RemoteException(serializableError, code);
}

Expand Down
2 changes: 1 addition & 1 deletion dialogue-serde/src/main/java/com/palantir/conjure/java/dialogue/serde/ErrorDecoder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ public enum ErrorDecoder {
private static final SafeLogger log = SafeLoggerFactory.get(ErrorDecoder.class);
private static final ObjectMapper MAPPER = ObjectMappers.newClientObjectMapper();
private static final EndpointErrorDecoder<?> ENDPOINT_ERROR_DECODER =
new EndpointErrorDecoder<>(Collections.emptyMap(), Optional.empty());
new EndpointErrorDecoder<>(Collections.emptyMap());

public boolean isError(Response response) {
return ENDPOINT_ERROR_DECODER.isError(response);
Expand Down
16 changes: 8 additions & 8 deletions ...est/java/com/palantir/conjure/java/dialogue/serde/EndpointErrorsConjureBodySerDeTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -116,9 +116,9 @@ public void testDeserializeCustomError() throws IOException {
.code(500);
BodySerDe serializers = conjureBodySerDe("application/json", "text/plain");
DeserializerArgs<EndpointReturnBaseType> deserializerArgs = DeserializerArgs.<EndpointReturnBaseType>builder()
.withBaseType(new TypeMarker<>() {})
.withExpectedResult(new TypeMarker<ExpectedReturnValue>() {})
.withErrorType("Default:FailedPrecondition", new TypeMarker<ErrorReturnValue>() {})
.baseType(new TypeMarker<>() {})
.success(new TypeMarker<ExpectedReturnValue>() {})
.error("Default:FailedPrecondition", new TypeMarker<ErrorReturnValue>() {})
.build();

// When
Expand Down Expand Up @@ -155,8 +155,8 @@ public void testDeserializingUndefinedErrorFallsbackToSerializableError() throws
.code(500);
BodySerDe serializers = conjureBodySerDe("application/json", "text/plain");
DeserializerArgs<EndpointReturnBaseType> deserializerArgs = DeserializerArgs.<EndpointReturnBaseType>builder()
.withBaseType(new TypeMarker<>() {})
.withExpectedResult(new TypeMarker<ExpectedReturnValue>() {})
.baseType(new TypeMarker<>() {})
.success(new TypeMarker<ExpectedReturnValue>() {})
// Note: no error types are registered.
.build();

Expand Down Expand Up @@ -190,9 +190,9 @@ public void testDeserializeExpectedValue() {
.code(200);
BodySerDe serializers = conjureBodySerDe("application/json", "text/plain");
DeserializerArgs<EndpointReturnBaseType> deserializerArgs = DeserializerArgs.<EndpointReturnBaseType>builder()
.withBaseType(new TypeMarker<>() {})
.withExpectedResult(new TypeMarker<ExpectedReturnValue>() {})
.withErrorType("Default:FailedPrecondition", new TypeMarker<ErrorReturnValue>() {})
.baseType(new TypeMarker<>() {})
.success(new TypeMarker<ExpectedReturnValue>() {})
.error("Default:FailedPrecondition", new TypeMarker<ErrorReturnValue>() {})
.build();
// When
EndpointReturnBaseType value =
Expand Down
Loading

0 comments on commit 0dafd35

Please sign in to comment.