From 9d8f2210f6d7d6e722a85abe290370e766a47acd Mon Sep 17 00:00:00 2001 From: Pritham Marupaka Date: Mon, 6 Jan 2025 11:55:42 -0500 Subject: [PATCH] wip --- .../java/dialogue/serde/ConjureBodySerDe.java | 161 +++++------------- .../dialogue/serde/DefaultConjureRuntime.java | 1 - .../dialogue/serde/EndpointErrorDecoder.java | 99 ++++++----- .../java/dialogue/serde/ErrorDecoder.java | 136 ++------------- .../dialogue/serde/BinaryEncodingTest.java | 2 - .../dialogue/serde/ConjureBodySerDeTest.java | 5 - .../dialogue/serde/DefaultClientsTest.java | 1 - .../EndpointErrorsConjureBodySerDeTest.java | 3 +- 8 files changed, 112 insertions(+), 296 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 8e67cddd2..66a124267 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 @@ -43,6 +43,7 @@ import java.io.OutputStream; import java.lang.reflect.Type; import java.util.ArrayList; +import java.util.Collections; import java.util.Comparator; import java.util.List; import java.util.Optional; @@ -52,7 +53,6 @@ /** * items: * - we don't want to use `String` for the error identifier. Let's create an `ErrorName` class. - * - re-consider using a map for the deserializersForEndpointBaseType field. is there a more direct way to get this info */ /** Package private internal API. */ @@ -65,7 +65,7 @@ final class ConjureBodySerDe implements BodySerDe { private final Deserializer> optionalBinaryInputStreamDeserializer; private final Deserializer emptyBodyDeserializer; private final LoadingCache> serializers; - private final LoadingCache> deserializers; + private final LoadingCache> deserializers; private final EmptyContainerDeserializer emptyContainerDeserializer; /** @@ -75,32 +75,48 @@ final class ConjureBodySerDe implements BodySerDe { */ ConjureBodySerDe( List rawEncodings, - ErrorDecoder errorDecoder, EmptyContainerDeserializer emptyContainerDeserializer, CaffeineSpec cacheSpec) { List encodings = decorateEncodings(rawEncodings); this.encodingsSortedByWeight = sortByWeight(encodings); Preconditions.checkArgument(encodings.size() > 0, "At least one Encoding is required"); + // note(pm): why do the weighted encoding thing? can we just pass in the default encoding? this.defaultEncoding = encodings.get(0).encoding(); this.emptyContainerDeserializer = emptyContainerDeserializer; - this.binaryInputStreamDeserializer = new EncodingDeserializerRegistry<>( + this.binaryInputStreamDeserializer = new EncodingDeserializerForEndpointRegistry<>( ImmutableList.of(BinaryEncoding.INSTANCE), - errorDecoder, emptyContainerDeserializer, - BinaryEncoding.MARKER); - this.optionalBinaryInputStreamDeserializer = new EncodingDeserializerRegistry<>( + BinaryEncoding.MARKER, + DeserializerArgs.builder() + .withBaseType(BinaryEncoding.MARKER) + .withExpectedResult(BinaryEncoding.MARKER) + .build()); + this.optionalBinaryInputStreamDeserializer = new EncodingDeserializerForEndpointRegistry<>( ImmutableList.of(BinaryEncoding.INSTANCE), - errorDecoder, emptyContainerDeserializer, - BinaryEncoding.OPTIONAL_MARKER); - this.emptyBodyDeserializer = new EmptyBodyDeserializer(errorDecoder); + BinaryEncoding.OPTIONAL_MARKER, + DeserializerArgs.>builder() + .withBaseType(BinaryEncoding.OPTIONAL_MARKER) + .withExpectedResult(BinaryEncoding.OPTIONAL_MARKER) + .build()); + this.emptyBodyDeserializer = + new EmptyBodyDeserializer(new EndpointErrorDecoder<>(Collections.emptyMap(), Optional.empty())); // 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) .build(type -> new EncodingSerializerRegistry<>(defaultEncoding, TypeMarker.of(type))); - this.deserializers = Caffeine.from(cacheSpec) - .build(type -> new EncodingDeserializerRegistry<>( - encodingsSortedByWeight, errorDecoder, emptyContainerDeserializer, TypeMarker.of(type))); + this.deserializers = Caffeine.from(cacheSpec).build(type -> buildCacheEntry(TypeMarker.of(type))); + } + + private EncodingDeserializerForEndpointRegistry buildCacheEntry(TypeMarker typeMarker) { + return new EncodingDeserializerForEndpointRegistry<>( + encodingsSortedByWeight, + emptyContainerDeserializer, + typeMarker, + DeserializerArgs.builder() + .withBaseType(typeMarker) + .withExpectedResult(typeMarker) + .build()); } private static List decorateEncodings(List input) { @@ -235,108 +251,7 @@ private static final class EncodingSerializerContainer { } } - private static final class EncodingDeserializerRegistry implements Deserializer { - - private static final SafeLogger log = SafeLoggerFactory.get(EncodingDeserializerRegistry.class); - private final ImmutableList> encodings; - private final ErrorDecoder errorDecoder; - private final Optional acceptValue; - private final Supplier> emptyInstance; - private final TypeMarker token; - - EncodingDeserializerRegistry( - List encodings, - ErrorDecoder errorDecoder, - EmptyContainerDeserializer empty, - TypeMarker token) { - this.encodings = encodings.stream() - .map(encoding -> new EncodingDeserializerContainer<>(encoding, token)) - .collect(ImmutableList.toImmutableList()); - this.errorDecoder = errorDecoder; - 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. - this.acceptValue = - Optional.of(encodings.stream().map(Encoding::getContentType).collect(Collectors.joining(", "))); - } - - @Override - public T deserialize(Response response) { - boolean closeResponse = true; - try { - if (errorDecoder.isError(response)) { - throw errorDecoder.decode(response); - } else if (response.code() == 204) { - // 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(); - if (maybeEmptyInstance.isPresent()) { - return maybeEmptyInstance.get(); - } - throw new SafeRuntimeException( - "Unable to deserialize non-optional response type from 204", SafeArg.of("type", token)); - } - - Optional contentType = response.getFirstHeader(HttpHeaders.CONTENT_TYPE); - if (!contentType.isPresent()) { - throw new SafeIllegalArgumentException( - "Response is missing Content-Type header", - SafeArg.of("received", response.headers().keySet())); - } - Encoding.Deserializer deserializer = getResponseDeserializer(contentType.get()); - T deserialized = deserializer.deserialize(response.body()); - // deserializer has taken on responsibility for closing the response body - closeResponse = false; - return deserialized; - } catch (IOException e) { - throw new SafeRuntimeException( - "Failed to deserialize response stream", - e, - SafeArg.of("contentType", response.getFirstHeader(HttpHeaders.CONTENT_TYPE)), - SafeArg.of("type", token)); - } finally { - if (closeResponse) { - response.close(); - } - } - } - - @Override - public Optional accepts() { - return acceptValue; - } - - /** Returns the {@link EncodingDeserializerContainer} to use to deserialize the request body. */ - @SuppressWarnings("ForLoopReplaceableByForEach") - // performance sensitive code avoids iterator allocation - Encoding.Deserializer getResponseDeserializer(String contentType) { - for (int i = 0; i < encodings.size(); i++) { - EncodingDeserializerContainer container = encodings.get(i); - if (container.encoding.supportsContentType(contentType)) { - return container.deserializer; - } - } - return throwingDeserializer(contentType); - } - - private Encoding.Deserializer 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)); - }; - } - } - private static final class EncodingDeserializerForEndpointRegistry implements Deserializer { - private static final SafeLogger log = SafeLoggerFactory.get(EncodingDeserializerForEndpointRegistry.class); private final ImmutableList> encodings; private final EndpointErrorDecoder endpointErrorDecoder; @@ -353,8 +268,11 @@ private static final class EncodingDeserializerForEndpointRegistry implements .map(encoding -> new EncodingDeserializerContainer<>( encoding, deserializersForEndpoint.expectedResultType())) .collect(ImmutableList.toImmutableList()); - this.endpointErrorDecoder = - new EndpointErrorDecoder<>(deserializersForEndpoint.errorNameToTypeMarker(), encodings); + this.endpointErrorDecoder = new EndpointErrorDecoder<>( + deserializersForEndpoint.errorNameToTypeMarker(), + encodings.stream() + .filter(encoding -> encoding.supportsContentType("application/json")) + .findAny()); 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. @@ -367,7 +285,6 @@ public T deserialize(Response response) { boolean closeResponse = true; try { if (endpointErrorDecoder.isError(response)) { - // TODO(pm): This needs to return T for the new deserializer API, but throw an exception for the old return endpointErrorDecoder.decode(response); } else if (response.code() == 204) { Optional maybeEmptyInstance = emptyInstance.get(); @@ -457,10 +374,10 @@ public String toString() { } private static final class EmptyBodyDeserializer implements Deserializer { - private final ErrorDecoder errorDecoder; + private final EndpointErrorDecoder endpointErrorDecoder; - EmptyBodyDeserializer(ErrorDecoder errorDecoder) { - this.errorDecoder = errorDecoder; + EmptyBodyDeserializer(EndpointErrorDecoder endpointErrorDecoder) { + this.endpointErrorDecoder = endpointErrorDecoder; } @Override @@ -468,8 +385,8 @@ private static final class EmptyBodyDeserializer implements Deserializer { public Void deserialize(Response response) { // We should not fail if a server that previously returned nothing starts returning a response try (Response unused = response) { - if (errorDecoder.isError(response)) { - throw errorDecoder.decode(response); + if (endpointErrorDecoder.isError(response)) { + endpointErrorDecoder.decode(response); } return null; } diff --git a/dialogue-serde/src/main/java/com/palantir/conjure/java/dialogue/serde/DefaultConjureRuntime.java b/dialogue-serde/src/main/java/com/palantir/conjure/java/dialogue/serde/DefaultConjureRuntime.java index 3e4766fda..befd4c350 100644 --- a/dialogue-serde/src/main/java/com/palantir/conjure/java/dialogue/serde/DefaultConjureRuntime.java +++ b/dialogue-serde/src/main/java/com/palantir/conjure/java/dialogue/serde/DefaultConjureRuntime.java @@ -45,7 +45,6 @@ public final class DefaultConjureRuntime implements ConjureRuntime { private DefaultConjureRuntime(Builder builder) { this.bodySerDe = new ConjureBodySerDe( builder.encodings.isEmpty() ? DEFAULT_ENCODINGS : builder.encodings, - ErrorDecoder.INSTANCE, Encodings.emptyContainerDeserializer(), DEFAULT_SERDE_CACHE_SPEC); } diff --git a/dialogue-serde/src/main/java/com/palantir/conjure/java/dialogue/serde/EndpointErrorDecoder.java b/dialogue-serde/src/main/java/com/palantir/conjure/java/dialogue/serde/EndpointErrorDecoder.java index f2ecfdfcc..2642f7af9 100644 --- a/dialogue-serde/src/main/java/com/palantir/conjure/java/dialogue/serde/EndpointErrorDecoder.java +++ b/dialogue-serde/src/main/java/com/palantir/conjure/java/dialogue/serde/EndpointErrorDecoder.java @@ -29,6 +29,7 @@ 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; @@ -48,21 +49,33 @@ 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; -// TODO(pm): public because maybe we need to expose this in the dialogue annotations. What does that do? -// T is the base type of the endpoint response. It's a union of the result type and all of the error types. -public final class EndpointErrorDecoder { +/** + * Extracts the error from a {@link Response}. + *

If the error's name is in the {@link #errorNameToJsonDeserializerMap}, 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. + * + * @param the base type of the endpoint response. It's a union of the result type and all the error types. + */ +final class EndpointErrorDecoder { private static final SafeLogger log = SafeLoggerFactory.get(EndpointErrorDecoder.class); private static final ObjectMapper MAPPER = ObjectMappers.newClientObjectMapper(); - private final Map> errorNameToTypeMap; - private final List encodings; - - public EndpointErrorDecoder(Map> errorNameToTypeMap, List encodings) { - this.errorNameToTypeMap = errorNameToTypeMap; - this.encodings = encodings; + private final Map> errorNameToJsonDeserializerMap; + + EndpointErrorDecoder( + Map> errorNameToTypeMap, Optional maybeJsonEncoding) { + this.errorNameToJsonDeserializerMap = maybeJsonEncoding + .>>map( + jsonEncoding -> errorNameToTypeMap.entrySet().stream() + .collect(Collectors.toMap( + Map.Entry::getKey, entry -> jsonEncoding.deserializer(entry.getValue())))) + .orElseGet(Collections::emptyMap); } public boolean isError(Response response) { @@ -76,15 +89,12 @@ public T decode(Response response) { try { return decodeInternal(response); } catch (Exception e) { - // TODO(pm): do we want to add the diagnostic information to the result type as well? e.addSuppressed(diagnostic(response)); throw e; } } - // performance sensitive code avoids iterator allocation - @SuppressWarnings({"checkstyle:CyclomaticComplexity", "ForLoopReplaceableByForEach"}) - private T decodeInternal(Response response) { + Optional checkCode(Response response) { int code = response.code(); switch (code) { case 308: @@ -95,7 +105,7 @@ private T decodeInternal(Response response) { UnknownRemoteException remoteException = new UnknownRemoteException(code, ""); remoteException.initCause( QosException.retryOther(qosReason(response), new URL(locationHeader))); - throw remoteException; + return Optional.of(remoteException); } catch (MalformedURLException e) { log.error( "Failed to parse location header for QosException.RetryOther", @@ -108,15 +118,25 @@ private T decodeInternal(Response response) { } break; case 429: - throw response.getFirstHeader(HttpHeaders.RETRY_AFTER) + return Optional.of(response.getFirstHeader(HttpHeaders.RETRY_AFTER) .map(Longs::tryParse) .map(Duration::ofSeconds) .map(duration -> QosException.throttle(qosReason(response), duration)) - .orElseGet(() -> QosException.throttle(qosReason(response))); + .orElseGet(() -> QosException.throttle(qosReason(response)))); case 503: - throw QosException.unavailable(qosReason(response)); + return Optional.of(QosException.unavailable(qosReason(response))); } + return Optional.empty(); + } + // performance sensitive code avoids iterator allocation + @SuppressWarnings({"checkstyle:CyclomaticComplexity", "ForLoopReplaceableByForEach"}) + private T decodeInternal(Response response) { + Optional maybeQosException = checkCode(response); + if (maybeQosException.isPresent()) { + throw maybeQosException.get(); + } + int code = response.code(); String body; try { body = toString(response.body()); @@ -127,27 +147,25 @@ private T decodeInternal(Response response) { } Optional contentType = response.getFirstHeader(HttpHeaders.CONTENT_TYPE); - // Use a factory: given contentType, create the deserailizer. - // We need Encoding.Deserializer here. That depends on the encoding. - if (contentType.isPresent() && Encodings.matchesContentType("application/json", contentType.get())) { + String jsonContentType = "application/json"; + if (contentType.isPresent() && Encodings.matchesContentType(jsonContentType, contentType.get())) { try { JsonNode node = MAPPER.readTree(body); - if (node.get("errorName") != null) { - // TODO(pm): Update this to use some struct instead of errorName. - TypeMarker container = Optional.ofNullable( - errorNameToTypeMap.get(node.get("errorName").asText())) - .orElseThrow(); - for (int i = 0; i < encodings.size(); i++) { - Encoding encoding = encodings.get(i); - if (encoding.supportsContentType(contentType.get())) { - return encoding.deserializer(container) - .deserialize(new ByteArrayInputStream(body.getBytes(StandardCharsets.UTF_8))); - } - } - } else { - SerializableError serializableError = MAPPER.readValue(body, SerializableError.class); - throw new RemoteException(serializableError, code); + JsonNode errorNameNode = node.get("errorName"); + if (errorNameNode == null) { + throwRemoteException(body, code); + } + Optional> maybeDeserializer = + Optional.ofNullable(errorNameToJsonDeserializerMap.get(errorNameNode.asText())); + if (maybeDeserializer.isEmpty()) { + throwRemoteException(body, code); } + return maybeDeserializer + .get() + .deserialize(new ByteArrayInputStream(body.getBytes(StandardCharsets.UTF_8))); + } catch (RemoteException remoteException) { + // rethrow the created remote exception + throw remoteException; } catch (Exception e) { throw new UnknownRemoteException(code, body); } @@ -156,17 +174,22 @@ private T decodeInternal(Response response) { throw new UnknownRemoteException(code, body); } - private static String toString(InputStream body) throws IOException { + private static void throwRemoteException(String body, int code) throws IOException { + SerializableError serializableError = MAPPER.readValue(body, SerializableError.class); + throw new RemoteException(serializableError, code); + } + + static String toString(InputStream body) throws IOException { try (Reader reader = new InputStreamReader(body, StandardCharsets.UTF_8)) { return CharStreams.toString(reader); } } - private static ResponseDiagnostic diagnostic(Response response) { + static ResponseDiagnostic diagnostic(Response response) { return new ResponseDiagnostic(diagnosticArgs(response)); } - private static ImmutableList> diagnosticArgs(Response response) { + static ImmutableList> diagnosticArgs(Response response) { ImmutableList.Builder> args = ImmutableList.>builder().add(SafeArg.of("status", response.code())); recordHeader(HttpHeaders.SERVER, response, args); recordHeader(HttpHeaders.CONTENT_TYPE, response, args); diff --git a/dialogue-serde/src/main/java/com/palantir/conjure/java/dialogue/serde/ErrorDecoder.java b/dialogue-serde/src/main/java/com/palantir/conjure/java/dialogue/serde/ErrorDecoder.java index 50642aea0..127f9d0ad 100644 --- a/dialogue-serde/src/main/java/com/palantir/conjure/java/dialogue/serde/ErrorDecoder.java +++ b/dialogue-serde/src/main/java/com/palantir/conjure/java/dialogue/serde/ErrorDecoder.java @@ -17,35 +17,16 @@ package com.palantir.conjure.java.dialogue.serde; import com.fasterxml.jackson.databind.ObjectMapper; -import com.google.common.collect.ImmutableList; -import com.google.common.io.CharStreams; import com.google.common.net.HttpHeaders; -import com.google.common.primitives.Longs; -import com.palantir.conjure.java.api.errors.QosException; -import com.palantir.conjure.java.api.errors.QosReason; -import com.palantir.conjure.java.api.errors.QosReasons; -import com.palantir.conjure.java.api.errors.QosReasons.QosResponseDecodingAdapter; 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.serialization.ObjectMappers; import com.palantir.dialogue.Response; -import com.palantir.logsafe.Arg; -import com.palantir.logsafe.SafeArg; -import com.palantir.logsafe.SafeLoggable; -import com.palantir.logsafe.UnsafeArg; -import com.palantir.logsafe.exceptions.SafeExceptions; import com.palantir.logsafe.logger.SafeLogger; import com.palantir.logsafe.logger.SafeLoggerFactory; import java.io.IOException; -import java.io.InputStream; -import java.io.InputStreamReader; -import java.io.Reader; -import java.net.MalformedURLException; -import java.net.URL; -import java.nio.charset.StandardCharsets; -import java.time.Duration; -import java.util.List; +import java.util.Collections; import java.util.Optional; /** @@ -59,17 +40,19 @@ 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()); public boolean isError(Response response) { - return 300 <= response.code() && response.code() <= 599; + return ENDPOINT_ERROR_DECODER.isError(response); } public RuntimeException decode(Response response) { if (log.isDebugEnabled()) { - log.debug("Received an error response", diagnosticArgs(response)); + log.debug("Received an error response", EndpointErrorDecoder.diagnosticArgs(response)); } RuntimeException result = decodeInternal(response); - result.addSuppressed(diagnostic(response)); + result.addSuppressed(EndpointErrorDecoder.diagnostic(response)); return result; } @@ -77,41 +60,15 @@ private RuntimeException decodeInternal(Response response) { // TODO(rfink): What about HTTP/101 switching protocols? // TODO(rfink): What about HEAD requests? - int code = response.code(); - switch (code) { - case 308: - Optional location = response.getFirstHeader(HttpHeaders.LOCATION); - if (location.isPresent()) { - String locationHeader = location.get(); - try { - UnknownRemoteException remoteException = new UnknownRemoteException(code, ""); - remoteException.initCause( - QosException.retryOther(qosReason(response), new URL(locationHeader))); - return remoteException; - } catch (MalformedURLException e) { - log.error( - "Failed to parse location header for QosException.RetryOther", - UnsafeArg.of("locationHeader", locationHeader), - e); - } - } else { - log.error("Retrieved HTTP status code 308 without Location header, cannot perform " - + "redirect. This appears to be a server-side protocol violation."); - } - break; - case 429: - return response.getFirstHeader(HttpHeaders.RETRY_AFTER) - .map(Longs::tryParse) - .map(Duration::ofSeconds) - .map(duration -> QosException.throttle(qosReason(response), duration)) - .orElseGet(() -> QosException.throttle(qosReason(response))); - case 503: - return QosException.unavailable(qosReason(response)); + Optional maybeQosException = ENDPOINT_ERROR_DECODER.checkCode(response); + if (maybeQosException.isPresent()) { + return maybeQosException.get(); } + int code = response.code(); String body; try { - body = toString(response.body()); + body = EndpointErrorDecoder.toString(response.body()); } catch (NullPointerException | IOException e) { UnknownRemoteException exception = new UnknownRemoteException(code, ""); exception.initCause(e); @@ -130,75 +87,4 @@ private RuntimeException decodeInternal(Response response) { return new UnknownRemoteException(code, body); } - - private static String toString(InputStream body) throws IOException { - try (Reader reader = new InputStreamReader(body, StandardCharsets.UTF_8)) { - return CharStreams.toString(reader); - } - } - - private static ResponseDiagnostic diagnostic(Response response) { - return new ResponseDiagnostic(diagnosticArgs(response)); - } - - private static ImmutableList> diagnosticArgs(Response response) { - ImmutableList.Builder> args = ImmutableList.>builder().add(SafeArg.of("status", response.code())); - recordHeader(HttpHeaders.SERVER, response, args); - recordHeader(HttpHeaders.CONTENT_TYPE, response, args); - recordHeader(HttpHeaders.CONTENT_LENGTH, response, args); - recordHeader(HttpHeaders.CONNECTION, response, args); - recordHeader(HttpHeaders.DATE, response, args); - recordHeader("x-envoy-response-flags", response, args); - recordHeader("x-envoy-response-code-details", response, args); - recordHeader("Response-Flags", response, args); - recordHeader("Response-Code-Details", response, args); - return args.build(); - } - - private static void recordHeader(String header, Response response, ImmutableList.Builder> args) { - response.getFirstHeader(header).ifPresent(server -> args.add(SafeArg.of(header, server))); - } - - private static final class ResponseDiagnostic extends RuntimeException implements SafeLoggable { - - private static final String SAFE_MESSAGE = "Response Diagnostic Information"; - - private final ImmutableList> args; - - ResponseDiagnostic(ImmutableList> args) { - super(SafeExceptions.renderMessage(SAFE_MESSAGE, args.toArray(new Arg[0]))); - this.args = args; - } - - @Override - public String getLogMessage() { - return SAFE_MESSAGE; - } - - @Override - public List> getArgs() { - return args; - } - - @Override - @SuppressWarnings("UnsynchronizedOverridesSynchronized") // nop - public Throwable fillInStackTrace() { - // no-op: stack trace generation is expensive, this type exists - // to simply associate diagnostic information with a failure. - return this; - } - } - - private static QosReason qosReason(Response response) { - return QosReasons.parseFromResponse(response, DialogueQosResponseDecodingAdapter.INSTANCE); - } - - private enum DialogueQosResponseDecodingAdapter implements QosResponseDecodingAdapter { - INSTANCE; - - @Override - public Optional getFirstHeader(Response response, String headerName) { - return response.getFirstHeader(headerName); - } - } } diff --git a/dialogue-serde/src/test/java/com/palantir/conjure/java/dialogue/serde/BinaryEncodingTest.java b/dialogue-serde/src/test/java/com/palantir/conjure/java/dialogue/serde/BinaryEncodingTest.java index 0d13b0b4a..fe8a9bcd2 100644 --- a/dialogue-serde/src/test/java/com/palantir/conjure/java/dialogue/serde/BinaryEncodingTest.java +++ b/dialogue-serde/src/test/java/com/palantir/conjure/java/dialogue/serde/BinaryEncodingTest.java @@ -34,7 +34,6 @@ public void testBinary() throws IOException { TestResponse response = new TestResponse().code(200).contentType("application/octet-stream"); BodySerDe serializers = new ConjureBodySerDe( ImmutableList.of(WeightedEncoding.of(new ConjureBodySerDeTest.StubEncoding("application/json"))), - ErrorDecoder.INSTANCE, Encodings.emptyContainerDeserializer(), DefaultConjureRuntime.DEFAULT_SERDE_CACHE_SPEC); InputStream deserialized = serializers.inputStreamDeserializer().deserialize(response); @@ -58,7 +57,6 @@ public void testBinary_optional_present() throws IOException { TestResponse response = new TestResponse().code(200).contentType("application/octet-stream"); BodySerDe serializers = new ConjureBodySerDe( ImmutableList.of(WeightedEncoding.of(new ConjureBodySerDeTest.StubEncoding("application/json"))), - ErrorDecoder.INSTANCE, Encodings.emptyContainerDeserializer(), DefaultConjureRuntime.DEFAULT_SERDE_CACHE_SPEC); Optional maybe = 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 684c5acdd..03879e807 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 @@ -52,8 +52,6 @@ public class ConjureBodySerDeTest { private static final TypeMarker TYPE = new TypeMarker() {}; private static final TypeMarker> OPTIONAL_TYPE = new TypeMarker>() {}; - private ErrorDecoder errorDecoder = ErrorDecoder.INSTANCE; - @Test public void testRequestContentType() throws IOException { @@ -76,7 +74,6 @@ private ConjureBodySerDe conjureBodySerDe(String... contentTypes) { Arrays.stream(contentTypes) .map(c -> WeightedEncoding.of(new StubEncoding(c))) .collect(ImmutableList.toImmutableList()), - errorDecoder, Encodings.emptyContainerDeserializer(), DefaultConjureRuntime.DEFAULT_SERDE_CACHE_SPEC); } @@ -115,7 +112,6 @@ public void testAcceptBasedOnWeight() throws IOException { BodySerDe serializers = new ConjureBodySerDe( ImmutableList.of(WeightedEncoding.of(plain, .5), WeightedEncoding.of(json, 1)), - ErrorDecoder.INSTANCE, Encodings.emptyContainerDeserializer(), DefaultConjureRuntime.DEFAULT_SERDE_CACHE_SPEC); // first encoding is default @@ -174,7 +170,6 @@ public void if_deserialize_throws_response_is_still_closed() { TestResponse response = new TestResponse().code(200).contentType("application/json"); BodySerDe serializers = new ConjureBodySerDe( ImmutableList.of(WeightedEncoding.of(BrokenEncoding.INSTANCE)), - ErrorDecoder.INSTANCE, Encodings.emptyContainerDeserializer(), DefaultConjureRuntime.DEFAULT_SERDE_CACHE_SPEC); assertThatThrownBy(() -> serializers.deserializer(TYPE).deserialize(response)) diff --git a/dialogue-serde/src/test/java/com/palantir/conjure/java/dialogue/serde/DefaultClientsTest.java b/dialogue-serde/src/test/java/com/palantir/conjure/java/dialogue/serde/DefaultClientsTest.java index a8ac9ab0d..58f1e4631 100644 --- a/dialogue-serde/src/test/java/com/palantir/conjure/java/dialogue/serde/DefaultClientsTest.java +++ b/dialogue-serde/src/test/java/com/palantir/conjure/java/dialogue/serde/DefaultClientsTest.java @@ -80,7 +80,6 @@ public final class DefaultClientsTest { private Response response = new TestResponse(); private BodySerDe bodySerde = new ConjureBodySerDe( DefaultConjureRuntime.DEFAULT_ENCODINGS, - ErrorDecoder.INSTANCE, Encodings.emptyContainerDeserializer(), DefaultConjureRuntime.DEFAULT_SERDE_CACHE_SPEC); private final SettableFuture responseFuture = SettableFuture.create(); 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 296bb193d..3a83e796f 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 @@ -48,7 +48,6 @@ @ExtendWith(MockitoExtension.class) public class EndpointErrorsConjureBodySerDeTest { private static final ObjectMapper MAPPER = ObjectMappers.newServerObjectMapper(); - private ErrorDecoder errorDecoder = ErrorDecoder.INSTANCE; @Generated("by conjure-java") private sealed interface EndpointReturnBaseType permits StringReturn, ErrorForEndpoint {} @@ -144,6 +143,7 @@ public void testDeserializeCustomErrors() throws IOException { EndpointErrorsConjureBodySerDeTest.EndpointReturnBaseType value = serializers.deserializer(deserializerArgs).deserialize(response); + assertThat(value).isInstanceOf(ErrorForEndpoint.class); assertThat(value) .extracting("errorCode", "errorName", "errorInstanceId", "args") .containsExactly( @@ -175,7 +175,6 @@ private ConjureBodySerDe conjureBodySerDe(String... contentTypes) { Arrays.stream(contentTypes) .map(c -> WeightedEncoding.of(new TypeReturningStubEncoding(c))) .collect(ImmutableList.toImmutableList()), - errorDecoder, Encodings.emptyContainerDeserializer(), DefaultConjureRuntime.DEFAULT_SERDE_CACHE_SPEC); }