Skip to content

Commit abdf579

Browse files
author
Pritham Marupaka
committed
wip: review changes
1 parent 01a1589 commit abdf579

File tree

2 files changed

+175
-51
lines changed

2 files changed

+175
-51
lines changed

‎dialogue-serde/src/main/java/com/palantir/conjure/java/dialogue/serde/ConjureBodySerDe.java

+57-35
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,8 @@
4141
import java.io.IOException;
4242
import java.io.InputStream;
4343
import java.io.OutputStream;
44+
import java.lang.reflect.Constructor;
45+
import java.lang.reflect.ParameterizedType;
4446
import java.lang.reflect.Type;
4547
import java.util.ArrayList;
4648
import java.util.Collections;
@@ -168,7 +170,7 @@ public <T> Deserializer<T> inputStreamDeserializer(DeserializerArgs<T> deseriali
168170
deserializerArgs.baseType(),
169171
deserializerArgs,
170172
BinaryEncoding.MARKER,
171-
is -> createSuccessType(deserializerArgs.successType(), is));
173+
(Function<InputStream, T>) createSuccessTypeFunctionForInputStream(deserializerArgs.successType()));
172174
}
173175

174176
@Override
@@ -177,14 +179,16 @@ public Deserializer<Optional<InputStream>> optionalInputStreamDeserializer() {
177179
}
178180

179181
@Override
182+
@SuppressWarnings("unchecked")
180183
public <T> Deserializer<T> optionalInputStreamDeserializer(DeserializerArgs<T> deserializerArgs) {
181184
return new EncodingDeserializerForEndpointRegistry<>(
182185
encodingsSortedByWeight,
183186
emptyContainerDeserializer,
184187
deserializerArgs.baseType(),
185188
deserializerArgs,
186189
BinaryEncoding.OPTIONAL_MARKER,
187-
ois -> createSuccessType(deserializerArgs.successType(), ois));
190+
(Function<Optional<InputStream>, T>)
191+
createSuccessTypeFunctionForOptionalInputStream(deserializerArgs.successType()));
188192
}
189193

190194
@Override
@@ -271,9 +275,8 @@ private static final class EncodingDeserializerForEndpointRegistry<S, T> impleme
271275
private final ImmutableList<EncodingDeserializerContainer<? extends S>> encodings;
272276
private final EndpointErrorDecoder<T> endpointErrorDecoder;
273277
private final Optional<String> acceptValue;
274-
private final Supplier<Optional<? extends T>> emptyInstance;
278+
private final Supplier<Optional<? extends S>> emptyInstance;
275279
private final TypeMarker<T> token;
276-
private final TypeMarker<? extends T> successTypeMarker;
277280
private final @Nullable Function<S, T> transform;
278281

279282
@SuppressWarnings("unchecked")
@@ -298,7 +301,6 @@ static <T> EncodingDeserializerForEndpointRegistry<T, T> create(
298301
DeserializerArgs<T> deserializersForEndpoint,
299302
TypeMarker<S> intermediateResult,
300303
@Nullable Function<S, T> transform) {
301-
this.successTypeMarker = deserializersForEndpoint.successType();
302304
this.encodings = encodingsSortedByWeight.stream()
303305
.map(encoding -> new EncodingDeserializerContainer<>(encoding, intermediateResult))
304306
.collect(ImmutableList.toImmutableList());
@@ -308,7 +310,7 @@ static <T> EncodingDeserializerForEndpointRegistry<T, T> create(
308310
.filter(encoding -> encoding.supportsContentType("application/json"))
309311
.findFirst());
310312
this.token = token;
311-
this.emptyInstance = Suppliers.memoize(() -> empty.tryGetEmptyInstance(successTypeMarker));
313+
this.emptyInstance = Suppliers.memoize(() -> empty.tryGetEmptyInstance(intermediateResult));
312314
this.acceptValue = Optional.of(encodingsSortedByWeight.stream()
313315
.map(Encoding::getContentType)
314316
.collect(Collectors.joining(", ")));
@@ -326,9 +328,12 @@ public T deserialize(Response response) {
326328
// TODO(dfox): what if we get a 204 for a non-optional type???
327329
// TODO(dfox): support http200 & body=null
328330
// TODO(dfox): what if we were expecting an empty list but got {}?
329-
Optional<? extends T> maybeEmptyInstance = emptyInstance.get();
331+
Optional<? extends S> maybeEmptyInstance = emptyInstance.get();
330332
if (maybeEmptyInstance.isPresent()) {
331-
return maybeEmptyInstance.get();
333+
if (transform == null) {
334+
return (T) maybeEmptyInstance.get();
335+
}
336+
return transform.apply(maybeEmptyInstance.get());
332337
}
333338
throw new SafeRuntimeException(
334339
"Unable to deserialize non-optional response type from 204", SafeArg.of("type", token));
@@ -347,14 +352,7 @@ public T deserialize(Response response) {
347352
if (transform == null) {
348353
return (T) deserialized;
349354
}
350-
T responseValue = transform.apply(deserialized);
351-
if (responseValue == null) {
352-
throw new SafeRuntimeException(
353-
"Failed to create success type",
354-
SafeArg.of("type", token),
355-
SafeArg.of("deserialized", deserialized));
356-
}
357-
return responseValue;
355+
return transform.apply(deserialized);
358356
} catch (IOException e) {
359357
throw new SafeRuntimeException(
360358
"Failed to deserialize response stream",
@@ -449,33 +447,57 @@ public String toString() {
449447
}
450448

451449
@SuppressWarnings("unchecked")
452-
@Nullable
453-
private static <T> T createSuccessType(TypeMarker<T> successT, InputStream inputStream) {
454-
if (successT.getType() instanceof Class<?>) {
450+
private static <T> Function<InputStream, T> createSuccessTypeFunctionForInputStream(TypeMarker<T> successT) {
451+
return successTypeCreatorFactory(successT, successType -> {
455452
try {
456-
return (T) ((Class<?>) successT.getType())
457-
.getConstructor(InputStream.class)
458-
.newInstance(inputStream);
459-
} catch (ReflectiveOperationException e) {
460-
throw new SafeRuntimeException("Failed to create success type", e);
453+
return ((Class<T>) successType.getType()).getConstructor(InputStream.class);
454+
} catch (ReflectiveOperationException ex) {
455+
throw new SafeRuntimeException("Failed to create success type", ex);
461456
}
462-
}
463-
return null;
457+
});
464458
}
465459

466460
@SuppressWarnings("unchecked")
467-
@Nullable
468-
private static <T> T createSuccessType(TypeMarker<T> successT, Optional<InputStream> optionalInputStream) {
469-
if (successT.getType() instanceof Class<?>) {
461+
private static <T> Function<Optional<InputStream>, T> createSuccessTypeFunctionForOptionalInputStream(
462+
TypeMarker<T> successT) {
463+
return successTypeCreatorFactory(successT, successType -> {
470464
try {
471-
return (T) ((Class<?>) successT.getType())
472-
// TODO(pm): check that the param is Optional<InputStream>
473-
.getConstructor(Optional.class)
474-
.newInstance(optionalInputStream);
465+
Class<T> clazz = (Class<T>) successType.getType();
466+
for (Constructor<?> ctor : clazz.getConstructors()) {
467+
if (ctor.getParameterCount() != 1) {
468+
continue;
469+
}
470+
Type paramType = ctor.getGenericParameterTypes()[0];
471+
if (paramType instanceof ParameterizedType parameterizedType) {
472+
if (parameterizedType.getRawType().equals(Optional.class)
473+
&& parameterizedType.getActualTypeArguments()[0].equals(InputStream.class)) {
474+
return (Constructor<T>) ctor;
475+
}
476+
}
477+
}
478+
} catch (SecurityException ex) {
479+
throw new SafeRuntimeException("Failed to create success type", ex);
480+
}
481+
throw new SafeRuntimeException(
482+
"Failed to create success type. Could not find constructor with Optional<InputStream> parameter");
483+
});
484+
}
485+
486+
private static <S, T> Function<S, T> successTypeCreatorFactory(
487+
TypeMarker<T> successT, Function<TypeMarker<T>, Constructor<T>> ctorExtractor) {
488+
if (!(successT.getType() instanceof Class<?>)) {
489+
throw new SafeRuntimeException("Failed to create success type", SafeArg.of("type", successT));
490+
}
491+
Constructor<T> ctor = ctorExtractor.apply(successT);
492+
if (ctor == null) {
493+
throw new SafeRuntimeException("Failed to create success type", SafeArg.of("type", successT));
494+
}
495+
return ctorParam -> {
496+
try {
497+
return ctor.newInstance(ctorParam);
475498
} catch (ReflectiveOperationException e) {
476499
throw new SafeRuntimeException("Failed to create success type", e);
477500
}
478-
}
479-
return null;
501+
};
480502
}
481503
}

‎dialogue-serde/src/test/java/com/palantir/conjure/java/dialogue/serde/EndpointErrorsConjureBodySerDeTest.java

+118-16
Original file line numberDiff line numberDiff line change
@@ -48,12 +48,18 @@
4848
import java.util.Arrays;
4949
import java.util.List;
5050
import java.util.Optional;
51+
import java.util.function.Supplier;
52+
import java.util.stream.Stream;
5153
import javax.annotation.Nullable;
5254
import javax.annotation.processing.Generated;
5355
import org.assertj.core.api.InstanceOfAssertFactories;
5456
import org.junit.jupiter.api.Test;
5557
import org.junit.jupiter.api.extension.ExtendWith;
58+
import org.junit.jupiter.api.extension.ExtensionContext;
5659
import org.junit.jupiter.params.ParameterizedTest;
60+
import org.junit.jupiter.params.provider.Arguments;
61+
import org.junit.jupiter.params.provider.ArgumentsProvider;
62+
import org.junit.jupiter.params.provider.ArgumentsSource;
5763
import org.junit.jupiter.params.provider.ValueSource;
5864
import org.mockito.junit.jupiter.MockitoExtension;
5965

@@ -78,6 +84,9 @@ private sealed interface EndpointReturnBaseType permits ExpectedReturnValue, Err
7884
@Generated("by conjure-java")
7985
private sealed interface EndpointBinaryReturnBaseType permits BinaryReturnValue, ErrorReturnValue {}
8086

87+
@Generated("by conjure-java")
88+
private sealed interface EndpointOptionalBinaryReturnBaseType permits OptionalBinaryReturnValue, ErrorReturnValue {}
89+
8190
@Generated("by conjure-java")
8291
record ExpectedReturnValue(String value) implements EndpointReturnBaseType {
8392
@JsonCreator
@@ -93,6 +102,13 @@ record BinaryReturnValue(@MustBeClosed InputStream value) implements EndpointBin
93102
}
94103
}
95104

105+
@Generated("by conjure-java")
106+
record OptionalBinaryReturnValue(Optional<InputStream> value) implements EndpointOptionalBinaryReturnBaseType {
107+
public OptionalBinaryReturnValue {
108+
Preconditions.checkArgumentNotNull(value, "value cannot be null");
109+
}
110+
}
111+
96112
@Generated("by conjure-java")
97113
record ComplexArg(int foo, String bar) {}
98114

@@ -106,7 +122,8 @@ record ErrorForEndpointArgs(
106122
static final class ErrorReturnValue extends EndpointError<ErrorForEndpointArgs>
107123
implements EndpointErrorsConjureBodySerDeTest.EndpointReturnBaseType,
108124
EndpointErrorsConjureBodySerDeTest.EmptyBodyEndpointReturnBaseType,
109-
EndpointErrorsConjureBodySerDeTest.EndpointBinaryReturnBaseType {
125+
EndpointErrorsConjureBodySerDeTest.EndpointBinaryReturnBaseType,
126+
EndpointErrorsConjureBodySerDeTest.EndpointOptionalBinaryReturnBaseType {
110127
@JsonCreator
111128
ErrorReturnValue(
112129
@JsonProperty("errorCode") String errorCode,
@@ -235,12 +252,13 @@ public void testDeserializeExpectedValue() {
235252
assertThat(value).isEqualTo(new ExpectedReturnValue(expectedString));
236253
}
237254

238-
@Test
239-
public void testDeserializeBinaryValue() {
255+
@ParameterizedTest
256+
@ArgumentsSource(BinaryBodyArgumentsProvider.class)
257+
public void testDeserializeBinaryValue(byte[] binaryBody) {
240258
// Given
241-
byte[] is = new byte[] {1, 2, 3};
242-
TestResponse response =
243-
new TestResponse(is).contentType("application/octet-stream").code(200);
259+
TestResponse response = new TestResponse(binaryBody)
260+
.contentType("application/octet-stream")
261+
.code(200);
244262
// BodySerDe serializers = conjureBodySerDe("application/octet-stream", "text/plain");
245263

246264
BodySerDe serializers = new ConjureBodySerDe(
@@ -259,14 +277,84 @@ public void testDeserializeBinaryValue() {
259277
serializers.inputStreamDeserializer(deserializerArgs).deserialize(response);
260278
// Then
261279
assertThat(value).isInstanceOfSatisfying(BinaryReturnValue.class, binaryReturnValue -> {
262-
try {
263-
assertThat(binaryReturnValue.value.readAllBytes()).isEqualTo(is);
264-
} catch (IOException e) {
265-
throw new RuntimeException(e);
266-
}
280+
assertThat(EndpointErrorsConjureBodySerDeTest.readAllBytesUnchecked(binaryReturnValue::value))
281+
.isEqualTo(binaryBody);
282+
});
283+
}
284+
285+
@ParameterizedTest
286+
@ArgumentsSource(BinaryBodyArgumentsProvider.class)
287+
public void testDeserializeOptionalBinaryValuePresent(byte[] binaryBody) {
288+
// Given
289+
TestResponse response = new TestResponse(binaryBody)
290+
.contentType("application/octet-stream")
291+
.code(200);
292+
// BodySerDe serializers = conjureBodySerDe("application/octet-stream", "text/plain");
293+
294+
BodySerDe serializers = new ConjureBodySerDe(
295+
ImmutableList.of(WeightedEncoding.of(BinaryEncoding.INSTANCE)),
296+
Encodings.emptyContainerDeserializer(),
297+
DefaultConjureRuntime.DEFAULT_SERDE_CACHE_SPEC);
298+
299+
DeserializerArgs<EndpointOptionalBinaryReturnBaseType> deserializerArgs =
300+
DeserializerArgs.<EndpointOptionalBinaryReturnBaseType>builder()
301+
.baseType(new TypeMarker<>() {})
302+
.success(new TypeMarker<OptionalBinaryReturnValue>() {})
303+
.error("Default:FailedPrecondition", new TypeMarker<ErrorReturnValue>() {})
304+
.build();
305+
// When
306+
EndpointOptionalBinaryReturnBaseType value =
307+
serializers.optionalInputStreamDeserializer(deserializerArgs).deserialize(response);
308+
// Then
309+
assertThat(value).isInstanceOfSatisfying(OptionalBinaryReturnValue.class, optionalBinaryReturnValue -> {
310+
assertThat(optionalBinaryReturnValue.value()).isPresent();
311+
assertThat(EndpointErrorsConjureBodySerDeTest.readAllBytesUnchecked(optionalBinaryReturnValue.value()::get))
312+
.isEqualTo(binaryBody);
267313
});
268314
}
269315

316+
@Test
317+
public void testDeserializeOptionalBinaryValueError() throws JsonProcessingException {
318+
// Given
319+
TestEndpointError errorThrownByEndpoint =
320+
new TestEndpointError("value", "unsafeValue", new ComplexArg(1, "bar"), Optional.of(2), null);
321+
String responseBody =
322+
MAPPER.writeValueAsString(ConjureError.fromCheckedServiceException(errorThrownByEndpoint));
323+
324+
TestResponse response = TestResponse.withBody(responseBody)
325+
.contentType("application/json")
326+
.code(500);
327+
328+
BodySerDe serializers = new ConjureBodySerDe(
329+
ImmutableList.of(WeightedEncoding.of(BinaryEncoding.INSTANCE)),
330+
Encodings.emptyContainerDeserializer(),
331+
DefaultConjureRuntime.DEFAULT_SERDE_CACHE_SPEC);
332+
333+
DeserializerArgs<EndpointOptionalBinaryReturnBaseType> deserializerArgs =
334+
DeserializerArgs.<EndpointOptionalBinaryReturnBaseType>builder()
335+
.baseType(new TypeMarker<>() {})
336+
.success(new TypeMarker<OptionalBinaryReturnValue>() {})
337+
.error("Default:FailedPrecondition", new TypeMarker<ErrorReturnValue>() {})
338+
.build();
339+
// When
340+
EndpointOptionalBinaryReturnBaseType value =
341+
serializers.optionalInputStreamDeserializer(deserializerArgs).deserialize(response);
342+
// Then
343+
ErrorReturnValue expectedErrorForEndpoint = new ErrorReturnValue(
344+
ErrorType.FAILED_PRECONDITION.code().name(),
345+
ErrorType.FAILED_PRECONDITION.name(),
346+
errorThrownByEndpoint.getErrorInstanceId(),
347+
new ErrorForEndpointArgs("value", "unsafeValue", new ComplexArg(1, "bar"), Optional.of(2)));
348+
assertThat(value).isInstanceOf(ErrorReturnValue.class);
349+
assertThat(value)
350+
.extracting("errorCode", "errorName", "errorInstanceId", "args")
351+
.containsExactly(
352+
expectedErrorForEndpoint.errorCode,
353+
expectedErrorForEndpoint.errorName,
354+
expectedErrorForEndpoint.errorInstanceId,
355+
expectedErrorForEndpoint.args);
356+
}
357+
270358
@Test
271359
public void testDeserializeEmptyBody() {
272360
// Given
@@ -316,11 +404,18 @@ public void testDeserializeWithCustomEncoding() throws JsonProcessingException {
316404

317405
// Then
318406
assertThat(stubbingEncoding.getDeserializer(errorTypeMarker))
319-
.isInstanceOfSatisfying(ContentRecordingJsonDeserializer.class, deserializer -> {
320-
assertThat(deserializer.getDeserializedContent())
321-
.asInstanceOf(InstanceOfAssertFactories.LIST)
322-
.containsExactly(responseBody);
323-
});
407+
.isInstanceOfSatisfying(ContentRecordingJsonDeserializer.class, deserializer -> assertThat(
408+
deserializer.getDeserializedContent())
409+
.asInstanceOf(InstanceOfAssertFactories.LIST)
410+
.containsExactly(responseBody));
411+
}
412+
413+
private static byte[] readAllBytesUnchecked(Supplier<InputStream> stream) {
414+
try (InputStream is = stream.get()) {
415+
return is.readAllBytes();
416+
} catch (IOException e) {
417+
throw new RuntimeException(e);
418+
}
324419
}
325420

326421
private ConjureBodySerDe conjureBodySerDe(String... contentTypes) {
@@ -331,4 +426,11 @@ private ConjureBodySerDe conjureBodySerDe(String... contentTypes) {
331426
Encodings.emptyContainerDeserializer(),
332427
DefaultConjureRuntime.DEFAULT_SERDE_CACHE_SPEC);
333428
}
429+
430+
private static final class BinaryBodyArgumentsProvider implements ArgumentsProvider {
431+
@Override
432+
public Stream<? extends Arguments> provideArguments(ExtensionContext _context) {
433+
return Stream.of(Arguments.of((Object) new byte[] {1, 2, 3}), Arguments.of((Object) new byte[] {}));
434+
}
435+
}
334436
}

0 commit comments

Comments
 (0)