From 162dfbca8f489ee44db679e091b38b96905fd710 Mon Sep 17 00:00:00 2001 From: Patrick Koenig Date: Thu, 21 Nov 2019 14:18:55 -0800 Subject: [PATCH 1/4] Upgrade Feign --- .palantir/revapi.yml | 33 +++++++ .../verification/SingleParamServicesTest.java | 9 +- .../src/test/resources/ignored-test-cases.yml | 6 +- conjure-java-jaxrs-client/build.gradle | 14 +-- .../feignimpl/AbstractDelegatingContract.java | 6 +- .../feignimpl/GuavaEmptyOptionalExpander.java | 3 + .../feignimpl/GuavaOptionalAwareContract.java | 27 +++--- .../Java8EmptyOptionalDoubleExpander.java | 3 + .../feignimpl/Java8EmptyOptionalExpander.java | 3 + .../Java8EmptyOptionalIntExpander.java | 3 + .../Java8EmptyOptionalLongExpander.java | 3 + .../feignimpl/Java8OptionalAwareContract.java | 83 ++++++----------- .../JaxRsClientGuavaOptionalHandlingTest.java | 6 +- .../JaxRsClientJava8OptionalHandlingTest.java | 12 +-- .../conjure/java/client/jaxrs/TracerTest.java | 6 +- .../feignimpl/EmptyContainerDecoderTest.java | 32 +++++-- .../InputStreamDelegateDecoderTest.java | 32 ++++++- .../feignimpl/NeverReturnNullDecoderTest.java | 44 +++++++-- .../jaxrs/feignimpl/QosErrorDecoderTest.java | 44 ++++++--- .../feignimpl/TextDelegateDecoderTest.java | 89 +++++++++++++++---- keystores/build.gradle | 12 +-- versions.lock | 17 ++-- versions.props | 2 +- 23 files changed, 329 insertions(+), 160 deletions(-) diff --git a/.palantir/revapi.yml b/.palantir/revapi.yml index 333dbb087..9b3352add 100644 --- a/.palantir/revapi.yml +++ b/.palantir/revapi.yml @@ -6,3 +6,36 @@ acceptedBreaks: old: null new: "method com.codahale.metrics.Timer com.palantir.conjure.java.okhttp.HostMetrics::getQos()" justification: "ABI-safe change to rarely used class, low impact" + 4.43.0: + com.palantir.conjure.java.runtime:okhttp-clients: [] + com.palantir.conjure.java.runtime:refresh-utils: [] + com.palantir.conjure.java.runtime:jetty-http2-agent: [] + com.palantir.conjure.java.runtime:keystores: [] + com.palantir.conjure.java.runtime:pkcs1-reader-bouncy-castle: [] + com.palantir.conjure.java.runtime:conjure-java-jackson-serialization: [] + com.palantir.conjure.java.runtime:conjure-scala-jaxrs-client: [] + com.palantir.conjure.java.runtime:conjure-java-jaxrs-client: + - code: "java.method.removed" + old: "method java.util.List com.palantir.conjure.java.client.jaxrs.feignimpl.AbstractDelegatingContract::parseAndValidatateMetadata(java.lang.Class)\ + \ @ com.palantir.conjure.java.client.jaxrs.feignimpl.PathTemplateHeaderEnrichmentContract" + new: null + justification: "{Feign method was renamed}" + - code: "java.method.removed" + old: "method java.util.List com.palantir.conjure.java.client.jaxrs.feignimpl.AbstractDelegatingContract::parseAndValidatateMetadata(java.lang.Class)\ + \ @ com.palantir.conjure.java.client.jaxrs.feignimpl.Java8OptionalAwareContract" + new: null + justification: "{Feign method was renamed}" + - code: "java.method.removed" + old: "method java.util.List com.palantir.conjure.java.client.jaxrs.feignimpl.AbstractDelegatingContract::parseAndValidatateMetadata(java.lang.Class)\ + \ @ com.palantir.conjure.java.client.jaxrs.feignimpl.GuavaOptionalAwareContract" + new: null + justification: "{Feign method was renamed}" + - code: "java.method.removed" + old: "method java.util.List com.palantir.conjure.java.client.jaxrs.feignimpl.AbstractDelegatingContract::parseAndValidatateMetadata(java.lang.Class)\ + \ @ com.palantir.conjure.java.client.jaxrs.feignimpl.SlashEncodingContract" + new: null + justification: "{Feign method was renamed}" + com.palantir.conjure.java.runtime:conjure-java-retrofit2-client: [] + com.palantir.conjure.java.runtime:conjure-java-jersey-server: [] + com.palantir.conjure.java.runtime:pkcs1-reader-sun: [] + com.palantir.conjure.java.runtime:client-config: [] diff --git a/conjure-java-client-verifier/src/test/java/com/palantir/verification/SingleParamServicesTest.java b/conjure-java-client-verifier/src/test/java/com/palantir/verification/SingleParamServicesTest.java index 6bf45de61..d673a7a52 100644 --- a/conjure-java-client-verifier/src/test/java/com/palantir/verification/SingleParamServicesTest.java +++ b/conjure-java-client-verifier/src/test/java/com/palantir/verification/SingleParamServicesTest.java @@ -21,6 +21,7 @@ import com.palantir.conjure.java.api.errors.RemoteException; import com.palantir.conjure.java.serialization.ObjectMappers; import com.palantir.conjure.verification.server.EndpointName; +import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; import java.lang.reflect.Type; import java.util.ArrayList; @@ -129,8 +130,12 @@ public void runTestCase() throws Exception { } log.info("Successfully post param to endpoint {} and index {}", endpointName, index); - } catch (RemoteException e) { - log.error("Caught exception with params: {}", e.getError().parameters(), e); + } catch (InvocationTargetException e) { + Throwable targetException = e.getTargetException(); + if (targetException instanceof RemoteException) { + RemoteException remoteException = (RemoteException) targetException; + log.error("Caught exception with params: {}", remoteException.getError().parameters(), e); + } throw e; } } diff --git a/conjure-java-client-verifier/src/test/resources/ignored-test-cases.yml b/conjure-java-client-verifier/src/test/resources/ignored-test-cases.yml index c34e4fbe7..495e47107 100644 --- a/conjure-java-client-verifier/src/test/resources/ignored-test-cases.yml +++ b/conjure-java-client-verifier/src/test/resources/ignored-test-cases.yml @@ -77,7 +77,11 @@ client: - '{"10": true, "10e0": false}' - '{"10": true, "10.0": false}' - singleHeaderService: {} + singleHeaderService: + headerString: + - '""' + headerAliasString: + - '""' singlePathParamService: pathParamString: diff --git a/conjure-java-jaxrs-client/build.gradle b/conjure-java-jaxrs-client/build.gradle index 9ec7cc60d..6e6fb0918 100644 --- a/conjure-java-jaxrs-client/build.gradle +++ b/conjure-java-jaxrs-client/build.gradle @@ -8,26 +8,26 @@ dependencies { api "com.google.code.findbugs:jsr305" api "javax.ws.rs:javax.ws.rs-api" // TODO(dsanduleac): Should be implementation, but can't because we expose feign.TextDelegateEncoder - api "com.netflix.feign:feign-core" + api "io.github.openfeign:feign-core" implementation project(":conjure-java-jackson-serialization") implementation project(":keystores") implementation "com.google.guava:guava" implementation "com.github.ben-manes.caffeine:caffeine" - implementation "com.netflix.feign:feign-jackson" - implementation("com.netflix.feign:feign-jaxrs") { + implementation "com.palantir.tracing:tracing-okhttp3" + implementation "io.github.openfeign:feign-jackson" + implementation("io.github.openfeign:feign-jaxrs") { // the shipped version clashes with the newer javax.ws.rs:javax.ws.rs-api used by (e.g.) dropwizard exclude group: "javax.ws.rs", module: "jsr311-api" } - implementation "com.netflix.feign:feign-okhttp" - implementation "com.netflix.feign:feign-slf4j" - implementation "com.palantir.tracing:tracing-okhttp3" + implementation "io.github.openfeign:feign-okhttp" + implementation "io.github.openfeign:feign-slf4j" implementation "org.slf4j:slf4j-api" testImplementation project(":conjure-java-retrofit2-client") testImplementation project(":conjure-java-jersey-server") testImplementation project(':keystores') - testImplementation "com.netflix.feign:feign-jackson" + testImplementation "io.github.openfeign:feign-jackson" testImplementation "com.squareup.okhttp3:mockwebserver" testImplementation("io.dropwizard:dropwizard-testing") { exclude module: 'metrics-core' } testImplementation "com.palantir.tracing:tracing-test-utils" diff --git a/conjure-java-jaxrs-client/src/main/java/com/palantir/conjure/java/client/jaxrs/feignimpl/AbstractDelegatingContract.java b/conjure-java-jaxrs-client/src/main/java/com/palantir/conjure/java/client/jaxrs/feignimpl/AbstractDelegatingContract.java index 86dd283f2..3d409796b 100644 --- a/conjure-java-jaxrs-client/src/main/java/com/palantir/conjure/java/client/jaxrs/feignimpl/AbstractDelegatingContract.java +++ b/conjure-java-jaxrs-client/src/main/java/com/palantir/conjure/java/client/jaxrs/feignimpl/AbstractDelegatingContract.java @@ -26,7 +26,7 @@ /** * Base class that provides the structure for a delegating {@link Contract}. - * Delegates the initial {@link #parseAndValidatateMetadata(Class)} call to + * Delegates the initial {@link #parseAndValidateMetadata(Class)} call to * the wrapped Contract and then calls {@link #processMetadata(Class, Method, MethodMetadata)} * on all of the methods that have metadata from the initial call. */ @@ -39,8 +39,8 @@ abstract class AbstractDelegatingContract implements Contract { } @Override - public final List parseAndValidatateMetadata(Class targetType) { - List mdList = delegate.parseAndValidatateMetadata(targetType); + public final List parseAndValidateMetadata(Class targetType) { + List mdList = delegate.parseAndValidateMetadata(targetType); Map methodMetadataByConfigKey = new LinkedHashMap(); for (MethodMetadata md : mdList) { diff --git a/conjure-java-jaxrs-client/src/main/java/com/palantir/conjure/java/client/jaxrs/feignimpl/GuavaEmptyOptionalExpander.java b/conjure-java-jaxrs-client/src/main/java/com/palantir/conjure/java/client/jaxrs/feignimpl/GuavaEmptyOptionalExpander.java index c54be9907..892d113a0 100644 --- a/conjure-java-jaxrs-client/src/main/java/com/palantir/conjure/java/client/jaxrs/feignimpl/GuavaEmptyOptionalExpander.java +++ b/conjure-java-jaxrs-client/src/main/java/com/palantir/conjure/java/client/jaxrs/feignimpl/GuavaEmptyOptionalExpander.java @@ -23,7 +23,10 @@ /** * Expands Optional by using the empty string for {@link com.google.common.base.Optional#absent()} and * the {@link Object#toString()} of the value otherwise. + * + * @deprecated no longer used */ +@Deprecated public final class GuavaEmptyOptionalExpander implements Expander { @Override diff --git a/conjure-java-jaxrs-client/src/main/java/com/palantir/conjure/java/client/jaxrs/feignimpl/GuavaOptionalAwareContract.java b/conjure-java-jaxrs-client/src/main/java/com/palantir/conjure/java/client/jaxrs/feignimpl/GuavaOptionalAwareContract.java index b2caeeb78..c73fe07af 100644 --- a/conjure-java-jaxrs-client/src/main/java/com/palantir/conjure/java/client/jaxrs/feignimpl/GuavaOptionalAwareContract.java +++ b/conjure-java-jaxrs-client/src/main/java/com/palantir/conjure/java/client/jaxrs/feignimpl/GuavaOptionalAwareContract.java @@ -16,9 +16,6 @@ package com.palantir.conjure.java.client.jaxrs.feignimpl; -import com.google.common.base.Function; -import com.google.common.collect.FluentIterable; -import com.google.common.collect.Lists; import feign.Contract; import feign.MethodMetadata; import java.lang.annotation.Annotation; @@ -28,9 +25,9 @@ import javax.ws.rs.QueryParam; /** - * Decorates a {@link Contract} and uses {@link GuavaNullOptionalExpander} for any {@link QueryParam} parameters, - * {@link GuavaEmptyOptionalExpander} for any {@link HeaderParam} parameters, and throws a {@link RuntimeException} - * at first encounter of an {@link com.google.common.base.Optional} typed {@link PathParam}. + * Decorates a {@link Contract} and uses {@link GuavaNullOptionalExpander} for any {@link QueryParam} parametersor + * {@link HeaderParam} parameters, and throws a {@link RuntimeException} at the first encounter of an + * {@link com.google.common.base.Optional} typed {@link PathParam}. *

* {@link PathParam}s require a value, and so we explicitly disallow use with {@link com.google.common.base.Optional}. */ @@ -46,14 +43,16 @@ protected void processMetadata(Class targetType, Method method, MethodMetadat Annotation[][] annotations = method.getParameterAnnotations(); for (int i = 0; i < parameterTypes.length; i++) { Class cls = parameterTypes[i]; - if (cls.equals(com.google.common.base.Optional.class)) { - FluentIterable> paramAnnotations = - FluentIterable.from(Lists.newArrayList(annotations[i])).transform(EXTRACT_CLASS); - if (paramAnnotations.contains(HeaderParam.class)) { - metadata.indexToExpanderClass().put(i, GuavaEmptyOptionalExpander.class); - } else if (paramAnnotations.contains(QueryParam.class)) { + if (!cls.equals(com.google.common.base.Optional.class)) { + continue; + } + + for (Annotation annotation : annotations[i]) { + Class annotationType = annotation.annotationType(); + if (annotationType.equals(HeaderParam.class) + || annotationType.equals(QueryParam.class)) { metadata.indexToExpanderClass().put(i, GuavaNullOptionalExpander.class); - } else if (paramAnnotations.contains(PathParam.class)) { + } else if (annotationType.equals(PathParam.class)) { throw new RuntimeException(String.format( "Cannot use Guava Optionals with PathParams. (Class: %s, Method: %s, Param: arg%d)", targetType.getName(), @@ -63,6 +62,4 @@ protected void processMetadata(Class targetType, Method method, MethodMetadat } } } - - private static final Function> EXTRACT_CLASS = input -> input.annotationType(); } diff --git a/conjure-java-jaxrs-client/src/main/java/com/palantir/conjure/java/client/jaxrs/feignimpl/Java8EmptyOptionalDoubleExpander.java b/conjure-java-jaxrs-client/src/main/java/com/palantir/conjure/java/client/jaxrs/feignimpl/Java8EmptyOptionalDoubleExpander.java index ba209afba..a594ad358 100644 --- a/conjure-java-jaxrs-client/src/main/java/com/palantir/conjure/java/client/jaxrs/feignimpl/Java8EmptyOptionalDoubleExpander.java +++ b/conjure-java-jaxrs-client/src/main/java/com/palantir/conjure/java/client/jaxrs/feignimpl/Java8EmptyOptionalDoubleExpander.java @@ -24,7 +24,10 @@ /** * Expands OptionalDouble by using the empty string for {@link OptionalDouble#empty()} and the * {@link Double#toString()} of the value otherwise. + * + * @deprecated no longer used */ +@Deprecated public final class Java8EmptyOptionalDoubleExpander implements Expander { @Override diff --git a/conjure-java-jaxrs-client/src/main/java/com/palantir/conjure/java/client/jaxrs/feignimpl/Java8EmptyOptionalExpander.java b/conjure-java-jaxrs-client/src/main/java/com/palantir/conjure/java/client/jaxrs/feignimpl/Java8EmptyOptionalExpander.java index 334983712..d7850d518 100644 --- a/conjure-java-jaxrs-client/src/main/java/com/palantir/conjure/java/client/jaxrs/feignimpl/Java8EmptyOptionalExpander.java +++ b/conjure-java-jaxrs-client/src/main/java/com/palantir/conjure/java/client/jaxrs/feignimpl/Java8EmptyOptionalExpander.java @@ -24,7 +24,10 @@ /** * Expands Optional by using the empty string for {@link Optional#empty()} and the {@link Object#toString()} of * the value otherwise. + * + * @deprecated no longer used */ +@Deprecated public final class Java8EmptyOptionalExpander implements Expander { @Override diff --git a/conjure-java-jaxrs-client/src/main/java/com/palantir/conjure/java/client/jaxrs/feignimpl/Java8EmptyOptionalIntExpander.java b/conjure-java-jaxrs-client/src/main/java/com/palantir/conjure/java/client/jaxrs/feignimpl/Java8EmptyOptionalIntExpander.java index 63e5043c8..56b9b9d3e 100644 --- a/conjure-java-jaxrs-client/src/main/java/com/palantir/conjure/java/client/jaxrs/feignimpl/Java8EmptyOptionalIntExpander.java +++ b/conjure-java-jaxrs-client/src/main/java/com/palantir/conjure/java/client/jaxrs/feignimpl/Java8EmptyOptionalIntExpander.java @@ -24,7 +24,10 @@ /** * Expands OptionalInt by using the empty string for {@link OptionalInt#empty()} and the {@link Integer#toString()} of * the value otherwise. + * + * @deprecated no longer used */ +@Deprecated public final class Java8EmptyOptionalIntExpander implements Expander { @Override diff --git a/conjure-java-jaxrs-client/src/main/java/com/palantir/conjure/java/client/jaxrs/feignimpl/Java8EmptyOptionalLongExpander.java b/conjure-java-jaxrs-client/src/main/java/com/palantir/conjure/java/client/jaxrs/feignimpl/Java8EmptyOptionalLongExpander.java index 1ca30f647..3fd45d95f 100644 --- a/conjure-java-jaxrs-client/src/main/java/com/palantir/conjure/java/client/jaxrs/feignimpl/Java8EmptyOptionalLongExpander.java +++ b/conjure-java-jaxrs-client/src/main/java/com/palantir/conjure/java/client/jaxrs/feignimpl/Java8EmptyOptionalLongExpander.java @@ -24,7 +24,10 @@ /** * Expands OptionalLong by using the empty string for {@link OptionalLong#empty()} and the {@link Long#toString()} of * the value otherwise. + * + * @deprecated no longer used */ +@Deprecated public final class Java8EmptyOptionalLongExpander implements Expander { @Override diff --git a/conjure-java-jaxrs-client/src/main/java/com/palantir/conjure/java/client/jaxrs/feignimpl/Java8OptionalAwareContract.java b/conjure-java-jaxrs-client/src/main/java/com/palantir/conjure/java/client/jaxrs/feignimpl/Java8OptionalAwareContract.java index 678ce8d38..c5cdfcbb1 100644 --- a/conjure-java-jaxrs-client/src/main/java/com/palantir/conjure/java/client/jaxrs/feignimpl/Java8OptionalAwareContract.java +++ b/conjure-java-jaxrs-client/src/main/java/com/palantir/conjure/java/client/jaxrs/feignimpl/Java8OptionalAwareContract.java @@ -16,10 +16,7 @@ package com.palantir.conjure.java.client.jaxrs.feignimpl; -import com.google.common.base.Function; -import com.google.common.collect.FluentIterable; import com.google.common.collect.ImmutableList; -import com.google.common.collect.Lists; import feign.Contract; import feign.MethodMetadata; import feign.Param.Expander; @@ -35,23 +32,19 @@ import javax.ws.rs.QueryParam; /** - * Decorates a {@link Contract} and uses {@link Java8NullOptionalExpander} for any {@link QueryParam} parameters, - * {@link Java8EmptyOptionalExpander} for any {@link HeaderParam} parameters, and throws a {@link RuntimeException} at - * first encounter of an {@link Optional} typed {@link PathParam}. + * Decorates a {@link Contract} and uses {@link Java8NullOptionalExpander} for any {@link QueryParam} or + * {@link HeaderParam} parameters, and throws a {@link RuntimeException} at the first encounter of an {@link Optional} + * typed {@link PathParam}. *

* {@link PathParam}s require a value, and so we explicitly disallow use with {@link Optional}. */ public final class Java8OptionalAwareContract extends AbstractDelegatingContract { private static final List expanders = ImmutableList.of( - new ExpanderDef(Optional.class, Java8EmptyOptionalExpander.class, Java8NullOptionalExpander.class), - new ExpanderDef(OptionalInt.class, Java8EmptyOptionalIntExpander.class, Java8NullOptionalIntExpander.class), - new ExpanderDef(OptionalDouble.class, - Java8EmptyOptionalDoubleExpander.class, - Java8NullOptionalDoubleExpander.class), - new ExpanderDef(OptionalLong.class, - Java8EmptyOptionalLongExpander.class, - Java8NullOptionalLongExpander.class)); + new ExpanderDef(Optional.class, Java8NullOptionalExpander.class), + new ExpanderDef(OptionalInt.class, Java8NullOptionalIntExpander.class), + new ExpanderDef(OptionalDouble.class, Java8NullOptionalDoubleExpander.class), + new ExpanderDef(OptionalLong.class, Java8NullOptionalLongExpander.class)); public Java8OptionalAwareContract(Contract delegate) { super(delegate); @@ -64,59 +57,37 @@ protected void processMetadata(Class targetType, Method method, MethodMetadat for (int i = 0; i < parameterTypes.length; i++) { Class cls = parameterTypes[i]; for (ExpanderDef def : expanders) { - if (cls.equals(def.match)) { - FluentIterable> paramAnnotations = getAnnotations(annotations, i); - configureOptionalExpanders(targetType, - method, - metadata, - i, - paramAnnotations, - def.emptyExpanderClass, - def.nullExpanderClass); + if (!cls.equals(def.match)) { + continue; } - } - } - } - - private FluentIterable> getAnnotations(Annotation[][] annotations, int index) { - return FluentIterable.from(Lists.newArrayList(annotations[index])).transform(EXTRACT_CLASS); - } - private void configureOptionalExpanders( - Class targetType, - Method method, - MethodMetadata metadata, - int index, - FluentIterable> paramAnnotations, - Class emptyExpanderClass, - Class nullExpanderClass) { - if (paramAnnotations.contains(HeaderParam.class)) { - metadata.indexToExpanderClass().put(index, emptyExpanderClass); - } else if (paramAnnotations.contains(QueryParam.class)) { - metadata.indexToExpanderClass().put(index, nullExpanderClass); - } else if (paramAnnotations.contains(PathParam.class)) { - throw new RuntimeException(String.format( - "Cannot use Java8 Optionals with PathParams. (Class: %s, Method: %s, Param: arg%d)", - targetType.getName(), - method.getName(), - index)); + for (Annotation annotation : annotations[i]) { + Class annotationType = annotation.annotationType(); + if (annotationType.equals(HeaderParam.class) + || annotationType.equals(QueryParam.class)) { + metadata.indexToExpanderClass().put(i, def.expanderClass); + break; + } else if (annotationType.equals(PathParam.class)) { + throw new RuntimeException(String.format( + "Cannot use Java8 Optionals with PathParams. (Class: %s, Method: %s, Param: arg%d)", + targetType.getName(), + method.getName(), + i)); + } + } + } } } - private static final Function> EXTRACT_CLASS = input -> input.annotationType(); - private static final class ExpanderDef { private final Class match; - private final Class emptyExpanderClass; - private final Class nullExpanderClass; + private final Class expanderClass; ExpanderDef( Class match, - Class emptyExpanderClass, - Class nullExpanderClass) { + Class expanderClass) { this.match = match; - this.emptyExpanderClass = emptyExpanderClass; - this.nullExpanderClass = nullExpanderClass; + this.expanderClass = expanderClass; } } } diff --git a/conjure-java-jaxrs-client/src/test/java/com/palantir/conjure/java/client/jaxrs/JaxRsClientGuavaOptionalHandlingTest.java b/conjure-java-jaxrs-client/src/test/java/com/palantir/conjure/java/client/jaxrs/JaxRsClientGuavaOptionalHandlingTest.java index 31f7da9c0..1acfe94c0 100644 --- a/conjure-java-jaxrs-client/src/test/java/com/palantir/conjure/java/client/jaxrs/JaxRsClientGuavaOptionalHandlingTest.java +++ b/conjure-java-jaxrs-client/src/test/java/com/palantir/conjure/java/client/jaxrs/JaxRsClientGuavaOptionalHandlingTest.java @@ -105,7 +105,7 @@ public void testAbsentQuery() throws Exception { public void testEmptyStringQuery() throws Exception { proxy.query(com.google.common.base.Optional.of(""), "str2"); RecordedRequest takeRequest = server.takeRequest(); - assertThat(takeRequest.getRequestLine()).isEqualTo("GET /foo?opt=&req=str2 HTTP/1.1"); + assertThat(takeRequest.getRequestLine()).isEqualTo("GET /foo?opt&req=str2 HTTP/1.1"); } @Test @@ -119,7 +119,7 @@ public void testStringQuery() throws Exception { public void testAbsentHeader() throws Exception { proxy.header(com.google.common.base.Optional.absent(), "str2"); RecordedRequest takeRequest = server.takeRequest(); - assertThat(takeRequest.getHeader("opt")).isEqualTo(""); + assertThat(takeRequest.getHeader("opt")).isEqualTo(null); assertThat(takeRequest.getHeader("req")).isEqualTo("str2"); } @@ -127,7 +127,7 @@ public void testAbsentHeader() throws Exception { public void testEmptyStringHeader() throws Exception { proxy.header(com.google.common.base.Optional.of(""), "str2"); RecordedRequest takeRequest = server.takeRequest(); - assertThat(takeRequest.getHeader("opt")).isEqualTo(""); + assertThat(takeRequest.getHeader("opt")).isEqualTo(null); assertThat(takeRequest.getHeader("req")).isEqualTo("str2"); } diff --git a/conjure-java-jaxrs-client/src/test/java/com/palantir/conjure/java/client/jaxrs/JaxRsClientJava8OptionalHandlingTest.java b/conjure-java-jaxrs-client/src/test/java/com/palantir/conjure/java/client/jaxrs/JaxRsClientJava8OptionalHandlingTest.java index 5d689f32e..63e74cd04 100644 --- a/conjure-java-jaxrs-client/src/test/java/com/palantir/conjure/java/client/jaxrs/JaxRsClientJava8OptionalHandlingTest.java +++ b/conjure-java-jaxrs-client/src/test/java/com/palantir/conjure/java/client/jaxrs/JaxRsClientJava8OptionalHandlingTest.java @@ -134,7 +134,7 @@ public void testAbsentQuery() throws Exception { public void testEmptyStringQuery() throws Exception { proxy.query(Optional.of(""), "str2"); RecordedRequest takeRequest = server.takeRequest(); - assertThat(takeRequest.getRequestLine()).isEqualTo("GET /foo?opt=&req=str2 HTTP/1.1"); + assertThat(takeRequest.getRequestLine()).isEqualTo("GET /foo?opt&req=str2 HTTP/1.1"); } @Test @@ -148,7 +148,7 @@ public void testStringQuery() throws Exception { public void testAbsentHeader() throws Exception { proxy.header(Optional.empty(), "str2"); RecordedRequest takeRequest = server.takeRequest(); - assertThat(takeRequest.getHeader("opt")).isEqualTo(""); + assertThat(takeRequest.getHeader("opt")).isEqualTo(null); assertThat(takeRequest.getHeader("req")).isEqualTo("str2"); } @@ -156,7 +156,7 @@ public void testAbsentHeader() throws Exception { public void testEmptyStringHeader() throws Exception { proxy.header(Optional.of(""), "str2"); RecordedRequest takeRequest = server.takeRequest(); - assertThat(takeRequest.getHeader("opt")).isEqualTo(""); + assertThat(takeRequest.getHeader("opt")).isEqualTo(null); assertThat(takeRequest.getHeader("req")).isEqualTo("str2"); } @@ -195,7 +195,7 @@ public void testIntQuery() throws Exception { public void testAbsentIntHeader() throws Exception { proxy.headerInt(OptionalInt.empty(), "str2"); RecordedRequest takeRequest = server.takeRequest(); - assertThat(takeRequest.getHeader("opt")).isEqualTo(""); + assertThat(takeRequest.getHeader("opt")).isEqualTo(null); assertThat(takeRequest.getHeader("req")).isEqualTo("str2"); } @@ -225,7 +225,7 @@ public void testDoubleQuery() throws Exception { public void testAbsentDoubleHeader() throws Exception { proxy.headerDouble(OptionalDouble.empty(), "str2"); RecordedRequest takeRequest = server.takeRequest(); - assertThat(takeRequest.getHeader("opt")).isEqualTo(""); + assertThat(takeRequest.getHeader("opt")).isEqualTo(null); assertThat(takeRequest.getHeader("req")).isEqualTo("str2"); } @@ -255,7 +255,7 @@ public void testLongQuery() throws Exception { public void testAbsentLongHeader() throws Exception { proxy.headerLong(OptionalLong.empty(), "str2"); RecordedRequest takeRequest = server.takeRequest(); - assertThat(takeRequest.getHeader("opt")).isEqualTo(""); + assertThat(takeRequest.getHeader("opt")).isEqualTo(null); assertThat(takeRequest.getHeader("req")).isEqualTo("str2"); } diff --git a/conjure-java-jaxrs-client/src/test/java/com/palantir/conjure/java/client/jaxrs/TracerTest.java b/conjure-java-jaxrs-client/src/test/java/com/palantir/conjure/java/client/jaxrs/TracerTest.java index e23edee2c..d7ff721f7 100644 --- a/conjure-java-jaxrs-client/src/test/java/com/palantir/conjure/java/client/jaxrs/TracerTest.java +++ b/conjure-java-jaxrs-client/src/test/java/com/palantir/conjure/java/client/jaxrs/TracerTest.java @@ -91,7 +91,7 @@ public void testClientIsInstrumentedWithTracer() throws InterruptedException { } @Test - public void test503_eventually_works() throws InterruptedException { + public void test503_eventually_works() { server.enqueue(new MockResponse().setResponseCode(503)); server.enqueue(new MockResponse().setResponseCode(503)); server.enqueue(new MockResponse().setBody("\"foo\"")); @@ -101,7 +101,7 @@ public void test503_eventually_works() throws InterruptedException { } @Test - public void give_me_some_delays() throws InterruptedException { + public void give_me_some_delays() { server.enqueue(new MockResponse() .setHeadersDelay(100, TimeUnit.MILLISECONDS) .setHeader("Content-Type", "application/json") @@ -113,7 +113,7 @@ public void give_me_some_delays() throws InterruptedException { } @Test - public void test503_exhausting_retries() throws InterruptedException { + public void test503_exhausting_retries() { // Default is 4 retries, so doing 5 server.enqueue(new MockResponse().setResponseCode(503)); server.enqueue(new MockResponse().setResponseCode(503)); diff --git a/conjure-java-jaxrs-client/src/test/java/com/palantir/conjure/java/client/jaxrs/feignimpl/EmptyContainerDecoderTest.java b/conjure-java-jaxrs-client/src/test/java/com/palantir/conjure/java/client/jaxrs/feignimpl/EmptyContainerDecoderTest.java index a0f58f90b..60f58221c 100644 --- a/conjure-java-jaxrs-client/src/test/java/com/palantir/conjure/java/client/jaxrs/feignimpl/EmptyContainerDecoderTest.java +++ b/conjure-java-jaxrs-client/src/test/java/com/palantir/conjure/java/client/jaxrs/feignimpl/EmptyContainerDecoderTest.java @@ -31,11 +31,13 @@ import com.google.common.collect.ImmutableSet; import com.google.common.net.HttpHeaders; import com.palantir.conjure.java.serialization.ObjectMappers; +import feign.Request; +import feign.Request.Body; +import feign.Request.HttpMethod; import feign.Response; import feign.codec.Decoder; import java.io.IOException; import java.nio.charset.StandardCharsets; -import java.util.Collections; import java.util.List; import java.util.Map; import java.util.Objects; @@ -51,8 +53,18 @@ public class EmptyContainerDecoderTest { private static final ObjectMapper mapper = ObjectMappers.newClientObjectMapper(); - private static final Response HTTP_204 = - Response.create(204, "No Content", Collections.emptyMap(), new byte[] {}); + private static final Request REQUEST = Request.create( + HttpMethod.GET, + "", + ImmutableMap.of(), + Body.empty(), + null); + private static final Response HTTP_204 = Response.builder() + .request(REQUEST) + .status(204) + .reason("No Content") + .body("", StandardCharsets.UTF_8) + .build(); private final Decoder delegate = mock(Decoder.class); private final EmptyContainerDecoder emptyContainerDecoder = new EmptyContainerDecoder(mapper, delegate); @@ -60,13 +72,15 @@ public class EmptyContainerDecoderTest { @Test public void http_200_uses_delegate_decoder() throws IOException { when(delegate.decode(any(), eq(String.class))).thenReturn("text response"); - Response http200 = Response.create(200, - "OK", - ImmutableMap.of( + Response http200 = Response.builder() + .request(REQUEST) + .status(200) + .reason("OK") + .headers(ImmutableMap.of( HttpHeaders.CONTENT_TYPE, - ImmutableSet.of(MediaType.TEXT_PLAIN)), - "text response", - StandardCharsets.UTF_8); + ImmutableSet.of(MediaType.TEXT_PLAIN))) + .body("text response", StandardCharsets.UTF_8) + .build(); emptyContainerDecoder.decode(http200, String.class); verify(delegate, times(1)).decode(any(), any()); diff --git a/conjure-java-jaxrs-client/src/test/java/com/palantir/conjure/java/client/jaxrs/feignimpl/InputStreamDelegateDecoderTest.java b/conjure-java-jaxrs-client/src/test/java/com/palantir/conjure/java/client/jaxrs/feignimpl/InputStreamDelegateDecoderTest.java index 965774039..1ceead825 100644 --- a/conjure-java-jaxrs-client/src/test/java/com/palantir/conjure/java/client/jaxrs/feignimpl/InputStreamDelegateDecoderTest.java +++ b/conjure-java-jaxrs-client/src/test/java/com/palantir/conjure/java/client/jaxrs/feignimpl/InputStreamDelegateDecoderTest.java @@ -24,6 +24,9 @@ import com.palantir.conjure.java.client.jaxrs.JaxRsClient; import com.palantir.conjure.java.client.jaxrs.TestBase; import com.palantir.conjure.java.okhttp.HostMetricsRegistry; +import feign.Request; +import feign.Request.Body; +import feign.Request.HttpMethod; import feign.Response; import feign.Util; import feign.codec.Decoder; @@ -38,6 +41,14 @@ import org.mockito.Mockito; public final class InputStreamDelegateDecoderTest extends TestBase { + + private static final Request REQUEST = Request.create( + HttpMethod.GET, + "", + ImmutableMap.of(), + Body.empty(), + null); + @ClassRule public static final DropwizardAppRule APP = new DropwizardAppRule<>(GuavaTestServer.class, "src/test/resources/test-server.yml"); @@ -63,7 +74,12 @@ public void before() { public void testDecodesAsInputStream() throws Exception { String data = "data"; - Response response = Response.create(200, "OK", ImmutableMap.of(), data, StandardCharsets.UTF_8); + Response response = Response.builder() + .request(REQUEST) + .status(200) + .reason("OK") + .body(data, StandardCharsets.UTF_8) + .build(); InputStream decoded = (InputStream) inputStreamDelegateDecoder.decode(response, InputStream.class); @@ -75,7 +91,12 @@ public void testUsesDelegateWhenReturnTypeNotInputStream() throws Exception { String returned = "string"; when(delegate.decode(any(), any())).thenReturn(returned); - Response response = Response.create(200, "OK", ImmutableMap.of(), returned, StandardCharsets.UTF_8); + Response response = Response.builder() + .request(REQUEST) + .status(200) + .reason("OK") + .body(returned, StandardCharsets.UTF_8) + .build(); String decodedObject = (String) inputStreamDelegateDecoder.decode(response, String.class); assertThat(decodedObject).isEqualTo(returned); } @@ -83,7 +104,12 @@ public void testUsesDelegateWhenReturnTypeNotInputStream() throws Exception { @Test public void testSupportsNullBody() throws Exception { String data = ""; - Response response = Response.create(200, "OK", ImmutableMap.of(), (Response.Body) null); + Response response = Response.builder() + .request(REQUEST) + .status(200) + .reason("OK") + .body((Response.Body) null) + .build(); InputStream decoded = (InputStream) inputStreamDelegateDecoder.decode(response, InputStream.class); diff --git a/conjure-java-jaxrs-client/src/test/java/com/palantir/conjure/java/client/jaxrs/feignimpl/NeverReturnNullDecoderTest.java b/conjure-java-jaxrs-client/src/test/java/com/palantir/conjure/java/client/jaxrs/feignimpl/NeverReturnNullDecoderTest.java index 72bf131b5..3d6110521 100644 --- a/conjure-java-jaxrs-client/src/test/java/com/palantir/conjure/java/client/jaxrs/feignimpl/NeverReturnNullDecoderTest.java +++ b/conjure-java-jaxrs-client/src/test/java/com/palantir/conjure/java/client/jaxrs/feignimpl/NeverReturnNullDecoderTest.java @@ -19,29 +19,41 @@ import static org.assertj.core.api.Assertions.assertThat; import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; import com.palantir.conjure.java.client.jaxrs.TestBase; import com.palantir.conjure.java.serialization.ObjectMappers; import com.palantir.logsafe.SafeArg; import com.palantir.logsafe.testing.Assertions; +import feign.Request; +import feign.Request.Body; +import feign.Request.HttpMethod; import feign.Response; import feign.codec.Decoder; import feign.jackson.JacksonDecoder; import java.nio.charset.StandardCharsets; -import java.util.Collection; -import java.util.HashMap; import java.util.List; -import java.util.Map; import org.junit.Test; public final class NeverReturnNullDecoderTest extends TestBase { - private final Map> headers = new HashMap<>(); + private static final Request REQUEST = Request.create( + HttpMethod.GET, + "", + ImmutableMap.of(), + Body.empty(), + null); + private final Decoder textDelegateDecoder = new NeverReturnNullDecoder( new JacksonDecoder(ObjectMappers.newClientObjectMapper())); @Test public void throws_nullpointerexception_when_body_is_null() { - Response response = Response.create(200, "OK", headers, null, StandardCharsets.UTF_8); + Response response = Response.builder() + .request(REQUEST) + .status(200) + .reason("OK") + .body(null, StandardCharsets.UTF_8) + .build(); Assertions.assertThatLoggableExceptionThrownBy(() -> textDelegateDecoder.decode(response, List.class)) .isInstanceOf(NullPointerException.class) @@ -51,7 +63,12 @@ public void throws_nullpointerexception_when_body_is_null() { @Test public void throws_nullpointerexception_when_body_is_string_null() { - Response response = Response.create(200, "OK", headers, "null", StandardCharsets.UTF_8); + Response response = Response.builder() + .request(REQUEST) + .status(200) + .reason("OK") + .body("null", StandardCharsets.UTF_8) + .build(); Assertions.assertThatLoggableExceptionThrownBy(() -> textDelegateDecoder.decode(response, List.class)) .isInstanceOf(NullPointerException.class) @@ -61,7 +78,12 @@ public void throws_nullpointerexception_when_body_is_string_null() { @Test public void throws_nullpointerexception_when_body_is_empty_string() { - Response response = Response.create(200, "OK", headers, "", StandardCharsets.UTF_8); + Response response = Response.builder() + .request(REQUEST) + .status(200) + .reason("OK") + .body("", StandardCharsets.UTF_8) + .build(); Assertions.assertThatLoggableExceptionThrownBy(() -> textDelegateDecoder.decode(response, List.class)) .isInstanceOf(NullPointerException.class) @@ -71,7 +93,13 @@ public void throws_nullpointerexception_when_body_is_empty_string() { @Test public void works_fine_when_body_is_not_null() throws Exception { - Response response = Response.create(200, "OK", headers, "[1, 2, 3]", StandardCharsets.UTF_8); + Response response = Response.builder() + .request(REQUEST) + .status(200) + .reason("OK") + .body("[1, 2, 3]", StandardCharsets.UTF_8) + .build(); + Object decodedObject = textDelegateDecoder.decode(response, List.class); assertThat(decodedObject).isEqualTo(ImmutableList.of(1, 2, 3)); } diff --git a/conjure-java-jaxrs-client/src/test/java/com/palantir/conjure/java/client/jaxrs/feignimpl/QosErrorDecoderTest.java b/conjure-java-jaxrs-client/src/test/java/com/palantir/conjure/java/client/jaxrs/feignimpl/QosErrorDecoderTest.java index bc58641ad..245cd899c 100644 --- a/conjure-java-jaxrs-client/src/test/java/com/palantir/conjure/java/client/jaxrs/feignimpl/QosErrorDecoderTest.java +++ b/conjure-java-jaxrs-client/src/test/java/com/palantir/conjure/java/client/jaxrs/feignimpl/QosErrorDecoderTest.java @@ -21,17 +21,25 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.palantir.conjure.java.api.errors.QosException; +import feign.Request; +import feign.Request.Body; +import feign.Request.HttpMethod; import feign.Response; import feign.codec.ErrorDecoder; +import java.nio.charset.StandardCharsets; import java.time.Duration; -import java.util.Collection; -import java.util.Map; import javax.ws.rs.core.HttpHeaders; import org.junit.Before; import org.junit.Test; public final class QosErrorDecoderTest { + private static final Request REQUEST = Request.create( + HttpMethod.GET, + "", + ImmutableMap.of(), + Body.empty(), + null); private static final String methodKey = "method"; private QosErrorDecoder decoder; @@ -43,8 +51,13 @@ public void before() { @Test public void http_429_throw_qos_throttle() { - Map> headers = ImmutableMap.of(); - Response response = Response.create(429, "too many requests", headers, new byte[0]); + Response response = Response.builder() + .request(REQUEST) + .status(429) + .reason("too many requests") + .body("", StandardCharsets.UTF_8) + .build(); + assertThat(decoder.decode(methodKey, response)) .isInstanceOfSatisfying( QosException.Throttle.class, @@ -53,10 +66,16 @@ public void http_429_throw_qos_throttle() { @Test public void http_429_throw_qos_throttle_with_retry_after() { - Map> headers = ImmutableMap.of( - HttpHeaders.RETRY_AFTER, - ImmutableList.of("5")); - Response response = Response.create(429, "too many requests", headers, new byte[0]); + Response response = Response.builder() + .request(REQUEST) + .status(429) + .reason("too many requests") + .headers(ImmutableMap.of( + HttpHeaders.RETRY_AFTER, + ImmutableList.of("5"))) + .body("", StandardCharsets.UTF_8) + .build(); + assertThat(decoder.decode(methodKey, response)) .isInstanceOfSatisfying( QosException.Throttle.class, @@ -65,8 +84,13 @@ public void http_429_throw_qos_throttle_with_retry_after() { @Test public void http_503_throw_qos_unavailable() { - Map> headers = ImmutableMap.of(); - Response response = Response.create(503, "too many requests", headers, new byte[0]); + Response response = Response.builder() + .request(REQUEST) + .status(503) + .reason("too many requests") + .body("", StandardCharsets.UTF_8) + .build(); + assertThat(decoder.decode(methodKey, response)) .isInstanceOf(QosException.Unavailable.class); } diff --git a/conjure-java-jaxrs-client/src/test/java/com/palantir/conjure/java/client/jaxrs/feignimpl/TextDelegateDecoderTest.java b/conjure-java-jaxrs-client/src/test/java/com/palantir/conjure/java/client/jaxrs/feignimpl/TextDelegateDecoderTest.java index b21331572..cc35fa108 100644 --- a/conjure-java-jaxrs-client/src/test/java/com/palantir/conjure/java/client/jaxrs/feignimpl/TextDelegateDecoderTest.java +++ b/conjure-java-jaxrs-client/src/test/java/com/palantir/conjure/java/client/jaxrs/feignimpl/TextDelegateDecoderTest.java @@ -24,21 +24,22 @@ import static org.mockito.Mockito.verifyNoMoreInteractions; import static org.mockito.Mockito.when; +import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.common.net.HttpHeaders; import com.palantir.conjure.java.client.jaxrs.JaxRsClient; import com.palantir.conjure.java.client.jaxrs.TestBase; import com.palantir.conjure.java.okhttp.HostMetricsRegistry; import feign.FeignException; +import feign.Request; +import feign.Request.Body; +import feign.Request.HttpMethod; import feign.Response; import feign.codec.DecodeException; import feign.codec.Decoder; import io.dropwizard.Configuration; import io.dropwizard.testing.junit.DropwizardAppRule; import java.nio.charset.StandardCharsets; -import java.util.Collection; -import java.util.HashMap; -import java.util.Map; import javax.ws.rs.core.MediaType; import org.junit.Before; import org.junit.ClassRule; @@ -47,6 +48,13 @@ import org.junit.rules.ExpectedException; public final class TextDelegateDecoderTest extends TestBase { + + private static final Request REQUEST = Request.create( + HttpMethod.GET, + "", + ImmutableMap.of(), + Body.empty(), + null); private static final String DELEGATE_RESPONSE = "delegate response"; @ClassRule @@ -57,14 +65,12 @@ public final class TextDelegateDecoderTest extends TestBase { public ExpectedException expectedException = ExpectedException.none(); private GuavaTestServer.TestService service; - private Map> headers; private Decoder delegate; private Decoder textDelegateDecoder; @Before public void before() { delegate = mock(Decoder.class); - headers = new HashMap<>(); textDelegateDecoder = new TextDelegateDecoder(delegate); String endpointUri = "http://localhost:" + APP.getLocalPort(); @@ -77,8 +83,15 @@ public void before() { @Test public void testUsesStringDecoderWithTextPlain() throws Exception { - headers.put(HttpHeaders.CONTENT_TYPE, ImmutableSet.of(MediaType.TEXT_PLAIN)); - Response response = Response.create(200, "OK", headers, "text response", StandardCharsets.UTF_8); + Response response = Response.builder() + .request(REQUEST) + .status(200) + .reason("OK") + .headers(ImmutableMap.of( + HttpHeaders.CONTENT_TYPE, + ImmutableSet.of(MediaType.TEXT_PLAIN))) + .body("text response", StandardCharsets.UTF_8) + .build(); Object decodedObject = textDelegateDecoder.decode(response, String.class); assertThat("text response").isEqualTo(decodedObject); @@ -95,8 +108,15 @@ public void testCannotReturnStringWithMediaTypeJson() { @Test public void testUsesStringDecoderWithTextPlainAndCharset() throws Exception { - headers.put(HttpHeaders.CONTENT_TYPE, ImmutableSet.of(MediaType.TEXT_PLAIN + "; charset=utf-8")); - Response response = Response.create(200, "OK", headers, "text response", StandardCharsets.UTF_8); + Response response = Response.builder() + .request(REQUEST) + .status(200) + .reason("OK") + .headers(ImmutableMap.of( + HttpHeaders.CONTENT_TYPE, + ImmutableSet.of(MediaType.TEXT_PLAIN + "; charset=utf-8"))) + .body("text response", StandardCharsets.UTF_8) + .build(); Object decodedObject = textDelegateDecoder.decode(response, String.class); @@ -106,8 +126,15 @@ public void testUsesStringDecoderWithTextPlainAndCharset() throws Exception { @Test public void testUsesStringDecoderWithTextPlainWithWeirdHeaderCapitalization() throws Exception { - headers.put("content-TYPE", ImmutableSet.of(MediaType.TEXT_PLAIN)); - Response response = Response.create(200, "OK", headers, "text response", StandardCharsets.UTF_8); + Response response = Response.builder() + .request(REQUEST) + .status(200) + .reason("OK") + .headers(ImmutableMap.of( + "content-TYPE", + ImmutableSet.of(MediaType.TEXT_PLAIN))) + .body("text response", StandardCharsets.UTF_8) + .build(); Object decodedObject = textDelegateDecoder.decode(response, String.class); assertThat("text response").isEqualTo(decodedObject); @@ -116,8 +143,15 @@ public void testUsesStringDecoderWithTextPlainWithWeirdHeaderCapitalization() th @Test public void testReturnsEmptyStringForNullResponseBodyWithTextPlain() throws Exception { - headers.put(HttpHeaders.CONTENT_TYPE, ImmutableSet.of(MediaType.TEXT_PLAIN)); - Response response = Response.create(200, "OK", headers, null, StandardCharsets.UTF_8); + Response response = Response.builder() + .request(REQUEST) + .status(200) + .reason("OK") + .headers(ImmutableMap.of( + HttpHeaders.CONTENT_TYPE, + ImmutableSet.of(MediaType.TEXT_PLAIN))) + .body(null, StandardCharsets.UTF_8) + .build(); Object decodedObject = textDelegateDecoder.decode(response, String.class); assertThat(decodedObject).isEqualTo(""); @@ -127,7 +161,12 @@ public void testReturnsEmptyStringForNullResponseBodyWithTextPlain() throws Exce @Test public void testUsesDelegateWithNoHeader() throws Exception { when(delegate.decode(any(), any())).thenReturn(DELEGATE_RESPONSE); - Response response = Response.create(200, "OK", headers, new byte[0]); + Response response = Response.builder() + .request(REQUEST) + .status(200) + .reason("OK") + .body("", StandardCharsets.UTF_8) + .build(); Object decodedObject = textDelegateDecoder.decode(response, String.class); assertThat(DELEGATE_RESPONSE).isEqualTo(decodedObject); @@ -135,9 +174,16 @@ public void testUsesDelegateWithNoHeader() throws Exception { @Test public void testUsesDelegateWithComplexHeader() throws Exception { - headers.put(HttpHeaders.CONTENT_TYPE, ImmutableSet.of(MediaType.TEXT_PLAIN, MediaType.APPLICATION_JSON)); when(delegate.decode(any(), any())).thenReturn(DELEGATE_RESPONSE); - Response response = Response.create(200, "OK", headers, new byte[0]); + Response response = Response.builder() + .request(REQUEST) + .status(200) + .reason("OK") + .headers(ImmutableMap.of( + HttpHeaders.CONTENT_TYPE, + ImmutableSet.of(MediaType.TEXT_PLAIN, MediaType.APPLICATION_JSON))) + .body("", StandardCharsets.UTF_8) + .build(); Object decodedObject = textDelegateDecoder.decode(response, String.class); assertThat(DELEGATE_RESPONSE).isEqualTo(decodedObject); @@ -145,9 +191,16 @@ public void testUsesDelegateWithComplexHeader() throws Exception { @Test public void testUsesDelegateWithNonTextContentType() throws Exception { - headers.put(HttpHeaders.CONTENT_TYPE, ImmutableSet.of(MediaType.APPLICATION_JSON)); when(delegate.decode(any(), any())).thenReturn(DELEGATE_RESPONSE); - Response response = Response.create(200, "OK", headers, new byte[0]); + Response response = Response.builder() + .request(REQUEST) + .status(200) + .reason("OK") + .headers(ImmutableMap.of( + HttpHeaders.CONTENT_TYPE, + ImmutableSet.of(MediaType.APPLICATION_JSON))) + .body("", StandardCharsets.UTF_8) + .build(); Object decodedObject = textDelegateDecoder.decode(response, String.class); assertThat(DELEGATE_RESPONSE).isEqualTo(decodedObject); diff --git a/keystores/build.gradle b/keystores/build.gradle index e7451cd91..957f9c8fd 100644 --- a/keystores/build.gradle +++ b/keystores/build.gradle @@ -10,18 +10,18 @@ dependencies { implementation "com.palantir.safe-logging:preconditions" testImplementation project(":conjure-java-jackson-serialization") - testImplementation("com.netflix.feign:feign-jaxrs") { + testImplementation("io.github.openfeign:feign-jaxrs") { // the shipped version clashes with the newer javax.ws.rs:javax.ws.rs-api used by (e.g.) dropwizard exclude group: "javax.ws.rs", module: "jsr311-api" } - testImplementation("com.netflix.feign:feign-core") - testImplementation "com.netflix.feign:feign-okhttp" testImplementation "com.squareup.okhttp3:okhttp" - testImplementation "org.hamcrest:hamcrest-all" testImplementation("io.dropwizard:dropwizard-core") { exclude module: 'metrics-core' } testImplementation("io.dropwizard:dropwizard-testing") { exclude module: 'metrics-core' } testImplementation "io.dropwizard.metrics:metrics-core" + testImplementation "io.github.openfeign:feign-core" + testImplementation "io.github.openfeign:feign-okhttp" testImplementation "junit:junit" + testImplementation "org.hamcrest:hamcrest-all" annotationProcessor "org.immutables:value" compileOnly 'org.immutables:value::annotations' @@ -45,7 +45,7 @@ task testSun(type: Test) { [tasks.testBouncyCastle, tasks.testSun].each { it.dependsOn(generateCerts) } -private void configureTestTask(Test test, String name) { +private void configureTestTask(test, name) { test.classpath = test.classpath + project(":pkcs1-reader-${name}").sourceSets.main.getOutput() + project(":pkcs1-reader-${name}").sourceSets.main.runtimeClasspath @@ -53,6 +53,6 @@ private void configureTestTask(Test test, String name) { test.reports.html.destination file("${project.test.reports.html.getDestination().absolutePath}-${name}") project.plugins.withType(JacocoPlugin) { - ((JacocoTaskExtension) test.jacoco).destinationFile = project.file("${project.getBuildDir()}/jacoco/${name}.exec") + test.jacoco.destinationFile = project.file("${project.getBuildDir()}/jacoco/${name}.exec") } } diff --git a/versions.lock b/versions.lock index f63a09f06..a182aa3cb 100644 --- a/versions.lock +++ b/versions.lock @@ -2,7 +2,7 @@ com.fasterxml:classmate:1.3.1 (1 constraints: a40e4f58) com.fasterxml.jackson.core:jackson-annotations:2.10.1 (9 constraints: 5da6fb11) com.fasterxml.jackson.core:jackson-core:2.10.1 (14 constraints: e4293053) -com.fasterxml.jackson.core:jackson-databind:2.10.1 (19 constraints: a061ef90) +com.fasterxml.jackson.core:jackson-databind:2.10.1 (19 constraints: de63cdae) com.fasterxml.jackson.dataformat:jackson-dataformat-cbor:2.10.1 (2 constraints: 941c47c9) com.fasterxml.jackson.datatype:jackson-datatype-guava:2.10.1 (3 constraints: 6a20e231) com.fasterxml.jackson.datatype:jackson-datatype-jdk8:2.10.1 (3 constraints: 6a20e231) @@ -22,11 +22,6 @@ com.google.guava:listenablefuture:9999.0-empty-to-avoid-conflict-with-guava (1 c com.google.j2objc:j2objc-annotations:1.3 (1 constraints: b809eda0) com.jcraft:jzlib:1.1.3 (1 constraints: 0705f635) com.netflix.concurrency-limits:concurrency-limits-core:0.2.2 (1 constraints: 0605f335) -com.netflix.feign:feign-core:8.18.0 (5 constraints: 693ac3a3) -com.netflix.feign:feign-jackson:8.18.0 (1 constraints: 43056b3b) -com.netflix.feign:feign-jaxrs:8.18.0 (1 constraints: 43056b3b) -com.netflix.feign:feign-okhttp:8.18.0 (1 constraints: 43056b3b) -com.netflix.feign:feign-slf4j:8.18.0 (1 constraints: 43056b3b) com.palantir.conjure.java.api:errors:2.9.0 (2 constraints: ff14ccb7) com.palantir.conjure.java.api:service-config:2.9.0 (1 constraints: 0d051036) com.palantir.conjure.java.api:ssl-config:2.9.0 (2 constraints: a1179a5f) @@ -39,12 +34,17 @@ com.palantir.tracing:tracing-jersey:3.4.1 (1 constraints: 0a050736) com.palantir.tracing:tracing-okhttp3:3.4.1 (1 constraints: 0a050736) com.palantir.tritium:tritium-registry:0.11.2 (1 constraints: 3605283b) com.squareup.okhttp3:logging-interceptor:3.13.1 (1 constraints: 3a053f3b) -com.squareup.okhttp3:okhttp:3.13.1 (6 constraints: 8459d8f9) +com.squareup.okhttp3:okhttp:3.13.1 (6 constraints: 5c5af6f0) com.squareup.okio:okio:1.17.2 (1 constraints: 850cc309) com.squareup.retrofit2:converter-jackson:2.4.0 (1 constraints: 08050136) com.squareup.retrofit2:retrofit:2.4.0 (2 constraints: 4b1d8704) com.thoughtworks.paranamer:paranamer:2.8 (1 constraints: 0c1627c6) io.dropwizard.metrics:metrics-core:3.2.6 (2 constraints: e31c7ecf) +io.github.openfeign:feign-core:10.6.0 (5 constraints: 873d6faf) +io.github.openfeign:feign-jackson:10.6.0 (1 constraints: 3905393b) +io.github.openfeign:feign-jaxrs:10.6.0 (1 constraints: 3905393b) +io.github.openfeign:feign-okhttp:10.6.0 (1 constraints: 3905393b) +io.github.openfeign:feign-slf4j:10.6.0 (1 constraints: 3905393b) jakarta.activation:jakarta.activation-api:1.2.1 (2 constraints: b928fbbd) jakarta.xml.bind:jakarta.xml.bind-api:2.3.2 (1 constraints: 30198ba6) javax.annotation:javax.annotation-api:1.3.2 (3 constraints: 1a310f90) @@ -73,9 +73,8 @@ org.hibernate:hibernate-validator:5.4.2.Final (2 constraints: f727b721) org.immutables:value:2.8.2 (1 constraints: 0e050f36) org.javassist:javassist:3.22.0-GA (2 constraints: f61d2fc1) org.jboss.logging:jboss-logging:3.3.0.Final (1 constraints: bd10c4b6) -org.jvnet:animal-sniffer-annotation:1.0 (1 constraints: f20b95eb) org.scala-lang:scala-library:2.11.12 (1 constraints: 3516632c) -org.slf4j:slf4j-api:1.7.26 (16 constraints: 04e0db13) +org.slf4j:slf4j-api:1.7.26 (16 constraints: d8e01e73) [Test dependencies] ch.qos.logback:logback-access:1.2.3 (1 constraints: b41148e2) diff --git a/versions.props b/versions.props index 898f6947f..360de2fb7 100644 --- a/versions.props +++ b/versions.props @@ -6,7 +6,6 @@ com.google.code.findbugs:jsr305 = 3.0.2 com.google.guava:guava = 23.6.1-jre com.jcraft:jzlib = 1.1.3 com.netflix.concurrency-limits:* = 0.2.2 -com.netflix.feign:feign-*= 8.18.0 com.palantir.conjure.java:conjure-java = 4.8.1 com.palantir.conjure.java:conjure-lib = 4.8.1 com.palantir.conjure.java.api:* = 2.9.0 @@ -15,6 +14,7 @@ com.palantir.safe-logging:* = 1.12.0 com.palantir.tracing:* = 3.4.1 com.palantir.tritium:tritium-registry = 0.11.2 io.dropwizard:dropwizard-* = 1.3.7 +io.github.openfeign:feign-*= 10.6.0 io.reactivex.rxjava2:rxjava = 2.2.14 javax.ws.rs:javax.ws.rs-api = 2.1 junit:junit = 4.12 From 02600edf1145ad2bda1a324d139dc3d55d4dda7e Mon Sep 17 00:00:00 2001 From: Patrick Koenig Date: Thu, 21 Nov 2019 22:18:55 +0000 Subject: [PATCH 2/4] Add generated changelog entries --- changelog/@unreleased/pr-1337.v2.yml | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 changelog/@unreleased/pr-1337.v2.yml diff --git a/changelog/@unreleased/pr-1337.v2.yml b/changelog/@unreleased/pr-1337.v2.yml new file mode 100644 index 000000000..abf9b4e8a --- /dev/null +++ b/changelog/@unreleased/pr-1337.v2.yml @@ -0,0 +1,6 @@ +type: improvement +improvement: + description: Feign has been upgraded to 10.6.0. This fixes the serialization of + absent optional header parameters. + links: + - https://github.com/palantir/conjure-java-runtime/pull/1337 From 7ef0e0aca875df43f55c59c6018600d0f5715b50 Mon Sep 17 00:00:00 2001 From: Patrick Koenig Date: Mon, 25 Nov 2019 22:48:42 -0800 Subject: [PATCH 3/4] Fix tracing --- .../feignimpl/PathTemplateHeaderEnrichmentContract.java | 5 ++--- .../com/palantir/conjure/java/client/jaxrs/TracerTest.java | 2 +- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/conjure-java-jaxrs-client/src/main/java/com/palantir/conjure/java/client/jaxrs/feignimpl/PathTemplateHeaderEnrichmentContract.java b/conjure-java-jaxrs-client/src/main/java/com/palantir/conjure/java/client/jaxrs/feignimpl/PathTemplateHeaderEnrichmentContract.java index 25c6460fa..318663314 100644 --- a/conjure-java-jaxrs-client/src/main/java/com/palantir/conjure/java/client/jaxrs/feignimpl/PathTemplateHeaderEnrichmentContract.java +++ b/conjure-java-jaxrs-client/src/main/java/com/palantir/conjure/java/client/jaxrs/feignimpl/PathTemplateHeaderEnrichmentContract.java @@ -34,8 +34,7 @@ protected void processMetadata(Class _targetType, Method _method, MethodMetad metadata.template().method() + " " + metadata.template() .url() - // escape from feign string interpolation - // See RequestTemplate.expand - .replace("{", "{{")); + .replace("{", "<") + .replace("}", ">")); } } diff --git a/conjure-java-jaxrs-client/src/test/java/com/palantir/conjure/java/client/jaxrs/TracerTest.java b/conjure-java-jaxrs-client/src/test/java/com/palantir/conjure/java/client/jaxrs/TracerTest.java index d7ff721f7..a3fa1e701 100644 --- a/conjure-java-jaxrs-client/src/test/java/com/palantir/conjure/java/client/jaxrs/TracerTest.java +++ b/conjure-java-jaxrs-client/src/test/java/com/palantir/conjure/java/client/jaxrs/TracerTest.java @@ -78,7 +78,7 @@ public void testClientIsInstrumentedWithTracer() throws InterruptedException { Tracer.unsubscribe(TracerTest.class.getName()); assertThat(observedSpans).containsExactlyInAnyOrder( - Maps.immutableEntry(SpanType.LOCAL, "OkHttp: GET /{param}"), + Maps.immutableEntry(SpanType.LOCAL, "OkHttp: GET /"), Maps.immutableEntry(SpanType.LOCAL, "OkHttp: attempt 0"), Maps.immutableEntry(SpanType.LOCAL, "OkHttp: client-side-concurrency-limiter 0/10"), Maps.immutableEntry(SpanType.LOCAL, "OkHttp: dispatcher"), From 2c799046e2b3a4f1b725c852013be22327350a72 Mon Sep 17 00:00:00 2001 From: Dan Fox Date: Tue, 26 Nov 2019 17:20:10 +0000 Subject: [PATCH 4/4] Add generated changelog entries --- changelog/@unreleased/pr-1337.v2.yml | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/changelog/@unreleased/pr-1337.v2.yml b/changelog/@unreleased/pr-1337.v2.yml index abf9b4e8a..ade65415b 100644 --- a/changelog/@unreleased/pr-1337.v2.yml +++ b/changelog/@unreleased/pr-1337.v2.yml @@ -1,6 +1,7 @@ type: improvement improvement: - description: Feign has been upgraded to 10.6.0. This fixes the serialization of - absent optional header parameters. + description: Feign has been upgraded to 10.6.0. This involved a maven group change + from `com.netflix.feign` -> `io.github.openfeign`. This fixes the serialization + of absent optional header parameters. links: - https://github.com/palantir/conjure-java-runtime/pull/1337