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 27afe22
Show file tree
Hide file tree
Showing 5 changed files with 74 additions and 80 deletions.
12 changes: 6 additions & 6 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,16 +82,16 @@ 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()));
Expand All @@ -108,8 +108,8 @@ private <T> EncodingDeserializerForEndpointRegistry<?> buildCacheEntry(TypeMarke
emptyContainerDeserializer,
typeMarker,
DeserializerArgs.<T>builder()
.withBaseType(typeMarker)
.withExpectedResult(typeMarker)
.baseType(typeMarker)
.success(typeMarker)
.build());
}

Expand Down
9 changes: 4 additions & 5 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 @@ -19,6 +19,8 @@
import com.fasterxml.jackson.databind.JsonNode;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Maps;
import com.google.common.io.CharStreams;
import com.google.common.net.HttpHeaders;
import com.google.common.primitives.Longs;
Expand Down Expand Up @@ -53,7 +55,6 @@
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.stream.Collectors;

/**
* Extracts the error from a {@link Response}.
Expand All @@ -71,10 +72,8 @@ final class EndpointErrorDecoder<T> {
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()))))
.<Map<String, Encoding.Deserializer<? extends T>>>map(jsonEncoding ->
ImmutableMap.copyOf(Maps.transformValues(errorNameToTypeMap, jsonEncoding::deserializer)))
.orElseGet(Collections::emptyMap);
}

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
79 changes: 42 additions & 37 deletions dialogue-serde/src/test/java/com/palantir/conjure/java/dialogue/serde/ErrorDecoderTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.ValueSource;
import org.junit.jupiter.params.provider.EnumSource;
import org.mockito.junit.jupiter.MockitoExtension;

@ExtendWith(MockitoExtension.class)
Expand All @@ -68,13 +68,18 @@ private static String createServiceException(ServiceException exception) {
}
}

public enum DecoderType {
LEGACY,
ENDPOINT
}

private static final ErrorDecoder decoder = ErrorDecoder.INSTANCE;
private static final EndpointErrorDecoder<?> endpointErrorDecoder =
new EndpointErrorDecoder<>(Collections.emptyMap(), Optional.empty());

@ParameterizedTest
@ValueSource(booleans = {true, false})
public void extractsRemoteExceptionForAllErrorCodes(boolean isLegacyErrorDecoder) {
@EnumSource(DecoderType.class)
public void extractsRemoteExceptionForAllErrorCodes(DecoderType decoderType) {
for (int code : ImmutableList.of(300, 400, 404, 500)) {
Response response =
TestResponse.withBody(SERIALIZED_EXCEPTION).code(code).contentType("application/json");
Expand All @@ -100,7 +105,7 @@ public void extractsRemoteExceptionForAllErrorCodes(boolean isLegacyErrorDecoder
+ ")");
};

if (isLegacyErrorDecoder) {
if (decoderType == DecoderType.LEGACY) {
assertThat(decoder.isError(response)).isTrue();
RuntimeException result = decoder.decode(response);
assertThat(result).isInstanceOfSatisfying(RemoteException.class, validationFunction);
Expand All @@ -114,8 +119,8 @@ public void extractsRemoteExceptionForAllErrorCodes(boolean isLegacyErrorDecoder
}

@ParameterizedTest
@ValueSource(booleans = {true, false})
public void testQos503(boolean isLegacyErrorDecoder) {
@EnumSource(DecoderType.class)
public void testQos503(DecoderType decoderType) {
Response response = TestResponse.withBody(SERIALIZED_EXCEPTION).code(503);

Consumer<RuntimeException> validationFunction = exception -> {
Expand All @@ -124,7 +129,7 @@ public void testQos503(boolean isLegacyErrorDecoder) {
});
};

if (isLegacyErrorDecoder) {
if (decoderType == DecoderType.LEGACY) {
assertThat(decoder.isError(response)).isTrue();
RuntimeException result = decoder.decode(response);
assertThat(result).isInstanceOfSatisfying(RuntimeException.class, validationFunction);
Expand All @@ -137,8 +142,8 @@ public void testQos503(boolean isLegacyErrorDecoder) {
}

@ParameterizedTest
@ValueSource(booleans = {true, false})
public void testQos503WithMetadata(boolean isLegacyErrorDecoder) {
@EnumSource(DecoderType.class)
public void testQos503WithMetadata(DecoderType decoderType) {
Response response = TestResponse.withBody(SERIALIZED_EXCEPTION)
.code(503)
.withHeader("Qos-Retry-Hint", "do-not-retry")
Expand All @@ -155,7 +160,7 @@ public void testQos503WithMetadata(boolean isLegacyErrorDecoder) {
});
};

if (isLegacyErrorDecoder) {
if (decoderType == DecoderType.LEGACY) {
assertThat(decoder.isError(response)).isTrue();
RuntimeException result = decoder.decode(response);
assertThat(result).isInstanceOfSatisfying(RuntimeException.class, validationFunction);
Expand All @@ -168,8 +173,8 @@ public void testQos503WithMetadata(boolean isLegacyErrorDecoder) {
}

@ParameterizedTest
@ValueSource(booleans = {true, false})
public void testQos429(boolean isLegacyErrorDecoder) {
@EnumSource(DecoderType.class)
public void testQos429(DecoderType decoderType) {
Response response = TestResponse.withBody(SERIALIZED_EXCEPTION).code(429);

Consumer<RuntimeException> validationFunction = exception -> {
Expand All @@ -179,7 +184,7 @@ public void testQos429(boolean isLegacyErrorDecoder) {
});
};

if (isLegacyErrorDecoder) {
if (decoderType == DecoderType.LEGACY) {
assertThat(decoder.isError(response)).isTrue();
RuntimeException result = decoder.decode(response);
assertThat(result).isInstanceOfSatisfying(RuntimeException.class, validationFunction);
Expand All @@ -192,8 +197,8 @@ public void testQos429(boolean isLegacyErrorDecoder) {
}

@ParameterizedTest
@ValueSource(booleans = {true, false})
public void testQos429_retryAfter(boolean isLegacyErrorDecoder) {
@EnumSource(DecoderType.class)
public void testQos429_retryAfter(DecoderType decoderType) {
Response response =
TestResponse.withBody(SERIALIZED_EXCEPTION).code(429).withHeader(HttpHeaders.RETRY_AFTER, "3");

Expand All @@ -204,7 +209,7 @@ public void testQos429_retryAfter(boolean isLegacyErrorDecoder) {
});
};

if (isLegacyErrorDecoder) {
if (decoderType == DecoderType.LEGACY) {
assertThat(decoder.isError(response)).isTrue();
RuntimeException result = decoder.decode(response);
assertThat(result).isInstanceOfSatisfying(RuntimeException.class, validationFunction);
Expand All @@ -217,8 +222,8 @@ public void testQos429_retryAfter(boolean isLegacyErrorDecoder) {
}

@ParameterizedTest
@ValueSource(booleans = {true, false})
public void testQos429_retryAfter_invalid(boolean isLegacyErrorDecoder) {
@EnumSource(DecoderType.class)
public void testQos429_retryAfter_invalid(DecoderType decoderType) {
Response response =
TestResponse.withBody(SERIALIZED_EXCEPTION).code(429).withHeader(HttpHeaders.RETRY_AFTER, "bad");

Expand All @@ -229,7 +234,7 @@ public void testQos429_retryAfter_invalid(boolean isLegacyErrorDecoder) {
});
};

if (isLegacyErrorDecoder) {
if (decoderType == DecoderType.LEGACY) {
assertThat(decoder.isError(response)).isTrue();
RuntimeException result = decoder.decode(response);
assertThat(result).isInstanceOfSatisfying(RuntimeException.class, validationFunction);
Expand All @@ -242,8 +247,8 @@ public void testQos429_retryAfter_invalid(boolean isLegacyErrorDecoder) {
}

@ParameterizedTest
@ValueSource(booleans = {true, false})
public void testQos308_noLocation(boolean isLegacyErrorDecoder) {
@EnumSource(DecoderType.class)
public void testQos308_noLocation(DecoderType decoderType) {
Response response = TestResponse.withBody(SERIALIZED_EXCEPTION).code(308);

Consumer<RuntimeException> validationFunction = exception -> {
Expand All @@ -252,7 +257,7 @@ public void testQos308_noLocation(boolean isLegacyErrorDecoder) {
});
};

if (isLegacyErrorDecoder) {
if (decoderType == DecoderType.LEGACY) {
assertThat(decoder.isError(response)).isTrue();
RuntimeException result = decoder.decode(response);
assertThat(result).isInstanceOfSatisfying(RuntimeException.class, validationFunction);
Expand All @@ -265,8 +270,8 @@ public void testQos308_noLocation(boolean isLegacyErrorDecoder) {
}

@ParameterizedTest
@ValueSource(booleans = {true, false})
public void testQos308_invalidLocation(boolean isLegacyErrorDecoder) {
@EnumSource(DecoderType.class)
public void testQos308_invalidLocation(DecoderType decoderType) {
Response response =
TestResponse.withBody(SERIALIZED_EXCEPTION).code(308).withHeader(HttpHeaders.LOCATION, "invalid");

Expand All @@ -276,7 +281,7 @@ public void testQos308_invalidLocation(boolean isLegacyErrorDecoder) {
});
};

if (isLegacyErrorDecoder) {
if (decoderType == DecoderType.LEGACY) {
assertThat(decoder.isError(response)).isTrue();
RuntimeException result = decoder.decode(response);
assertThat(result).isInstanceOfSatisfying(RuntimeException.class, validationFunction);
Expand All @@ -289,8 +294,8 @@ public void testQos308_invalidLocation(boolean isLegacyErrorDecoder) {
}

@ParameterizedTest
@ValueSource(booleans = {true, false})
public void testQos308(boolean isLegacyErrorDecoder) {
@EnumSource(ErrorDecoderTest.DecoderType.class)
public void testQos308(DecoderType decoderType) {
String expectedLocation = "https://localhost";
Response response = TestResponse.withBody(SERIALIZED_EXCEPTION)
.code(308)
Expand All @@ -306,7 +311,7 @@ public void testQos308(boolean isLegacyErrorDecoder) {
});
};

if (isLegacyErrorDecoder) {
if (decoderType == DecoderType.LEGACY) {
assertThat(decoder.isError(response)).isTrue();
RuntimeException result = decoder.decode(response);
assertThat(result).isInstanceOfSatisfying(RuntimeException.class, validationFunction);
Expand Down Expand Up @@ -339,16 +344,16 @@ public void cannotDecodeNonJsonMediaTypes() {
}

@ParameterizedTest
@ValueSource(booleans = {true, false})
public void doesNotHandleUnparseableBody(boolean isLegacyErrorDecoder) {
@EnumSource(DecoderType.class)
public void doesNotHandleUnparseableBody(DecoderType decoderType) {
Response response = TestResponse.withBody("not json").code(500).contentType("application/json/");

Consumer<UnknownRemoteException> validationFunction = exception -> {
assertThat(exception.getStatus()).isEqualTo(500);
assertThat(exception.getBody()).isEqualTo("not json");
};

if (isLegacyErrorDecoder) {
if (decoderType == DecoderType.LEGACY) {
RuntimeException result = decoder.decode(response);
assertThat(result).isInstanceOfSatisfying(UnknownRemoteException.class, validationFunction);
} else {
Expand All @@ -372,8 +377,8 @@ public void doesNotHandleNullBody() {
}

@ParameterizedTest
@ValueSource(booleans = {true, false})
public void handlesUnexpectedJson(boolean isLegacyErrorDecoder) {
@EnumSource(DecoderType.class)
public void handlesUnexpectedJson(DecoderType decoderType) {
Response response = TestResponse.withBody("{\"error\":\"some-unknown-json\"}")
.code(502)
.contentType("application/json");
Expand All @@ -383,7 +388,7 @@ public void handlesUnexpectedJson(boolean isLegacyErrorDecoder) {
assertThat(expected.getBody()).isEqualTo("{\"error\":\"some-unknown-json\"}");
assertThat(expected.getMessage()).isEqualTo("Response status: 502");
};
if (isLegacyErrorDecoder) {
if (decoderType == DecoderType.LEGACY) {
assertThat(decoder.decode(response))
.isInstanceOfSatisfying(UnknownRemoteException.class, validationFunction);
} else {
Expand All @@ -394,8 +399,8 @@ public void handlesUnexpectedJson(boolean isLegacyErrorDecoder) {
}

@ParameterizedTest
@ValueSource(booleans = {true, false})
public void handlesJsonWithEncoding(boolean isLegacyErrorDecoder) {
@EnumSource(DecoderType.class)
public void handlesJsonWithEncoding(DecoderType decoderType) {
int code = 500;
Response response =
TestResponse.withBody(SERIALIZED_EXCEPTION).code(code).contentType("application/json; charset=utf-8");
Expand All @@ -408,7 +413,7 @@ public void handlesJsonWithEncoding(boolean isLegacyErrorDecoder) {
assertThat(exception.getError().errorName()).isEqualTo(ErrorType.FAILED_PRECONDITION.name());
};

if (isLegacyErrorDecoder) {
if (decoderType == DecoderType.LEGACY) {
assertThat(decoder.decode(response)).isInstanceOfSatisfying(RemoteException.class, validationFunction);
} else {
assertThatExceptionOfType(RemoteException.class)
Expand Down
Loading

0 comments on commit 27afe22

Please sign in to comment.