From 9bdf87013a68f9ddd359dab679d7845931df2ae2 Mon Sep 17 00:00:00 2001 From: Mark Elliot Date: Mon, 8 Feb 2016 22:25:24 -0800 Subject: [PATCH] Absent optional headers become the empty string --- .../http/GuavaEmptyOptionalExpander.java | 37 +++++++++++++++++++ ...er.java => GuavaNullOptionalExpander.java} | 5 ++- .../http/GuavaOptionalAwareContract.java | 21 ++++++++++- .../http/GuavaOptionalAwareContractTest.java | 2 +- 4 files changed, 60 insertions(+), 5 deletions(-) create mode 100644 http-clients/src/main/java/com/palantir/remoting/http/GuavaEmptyOptionalExpander.java rename http-clients/src/main/java/com/palantir/remoting/http/{GuavaOptionalExpander.java => GuavaNullOptionalExpander.java} (85%) diff --git a/http-clients/src/main/java/com/palantir/remoting/http/GuavaEmptyOptionalExpander.java b/http-clients/src/main/java/com/palantir/remoting/http/GuavaEmptyOptionalExpander.java new file mode 100644 index 000000000..7d674d858 --- /dev/null +++ b/http-clients/src/main/java/com/palantir/remoting/http/GuavaEmptyOptionalExpander.java @@ -0,0 +1,37 @@ +/* + * Copyright 2016 Palantir Technologies, Inc. All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.palantir.remoting.http; + +import com.google.common.base.Optional; +import com.google.common.base.Preconditions; +import feign.Param.Expander; +import java.util.Objects; + +/** + * Expands Optional by using the empty string for {@link Optional#absent()} and the {@link Object#toString()} of + * the value otherwise. + */ +public final class GuavaEmptyOptionalExpander implements Expander { + + @Override + public String expand(Object value) { + Preconditions.checkArgument(value instanceof Optional, "Value must be an Optional. Was: %s", value.getClass()); + Optional optional = (Optional) value; + return optional.isPresent() ? Objects.toString(optional.get()) : ""; + } + +} diff --git a/http-clients/src/main/java/com/palantir/remoting/http/GuavaOptionalExpander.java b/http-clients/src/main/java/com/palantir/remoting/http/GuavaNullOptionalExpander.java similarity index 85% rename from http-clients/src/main/java/com/palantir/remoting/http/GuavaOptionalExpander.java rename to http-clients/src/main/java/com/palantir/remoting/http/GuavaNullOptionalExpander.java index 9bc76ac21..c70312a20 100644 --- a/http-clients/src/main/java/com/palantir/remoting/http/GuavaOptionalExpander.java +++ b/http-clients/src/main/java/com/palantir/remoting/http/GuavaNullOptionalExpander.java @@ -22,9 +22,10 @@ import java.util.Objects; /** - * Expands Optional by using null for Optional.absent and the toString() of the value otherwise. + * Expands Optional by using null for {@link Optional#absent()} and the {@link Object#toString()} of the + * value otherwise. */ -public final class GuavaOptionalExpander implements Expander { +public final class GuavaNullOptionalExpander implements Expander { @Override public String expand(Object value) { diff --git a/http-clients/src/main/java/com/palantir/remoting/http/GuavaOptionalAwareContract.java b/http-clients/src/main/java/com/palantir/remoting/http/GuavaOptionalAwareContract.java index 8920ba6c2..18a95e7a7 100644 --- a/http-clients/src/main/java/com/palantir/remoting/http/GuavaOptionalAwareContract.java +++ b/http-clients/src/main/java/com/palantir/remoting/http/GuavaOptionalAwareContract.java @@ -16,17 +16,22 @@ package com.palantir.remoting.http; +import com.google.common.base.Function; import com.google.common.base.Optional; +import com.google.common.collect.FluentIterable; import feign.Contract; import feign.Feign; import feign.MethodMetadata; +import java.lang.annotation.Annotation; import java.lang.reflect.Method; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; +import javax.ws.rs.HeaderParam; /** - * Decorates a Contract and uses {@link GuavaOptionalExpander} for any HeaderParam, PathParam or QueryParam parameters. + * Decorates a Contract and uses {@link GuavaNullOptionalExpander} for any PathParam or QueryParam parameters + * and {@link GuavaEmptyOptionalExpander} for any {@link HeaderParam} parameters. */ public final class GuavaOptionalAwareContract implements Contract { @@ -52,10 +57,15 @@ public List parseAndValidatateMetadata(Class targetType) { MethodMetadata md = methodMetadataByConfigKey.get(configKey); if (md != null) { Class[] parameterTypes = method.getParameterTypes(); + Annotation[][] annotations = method.getParameterAnnotations(); for (int i = 0; i < parameterTypes.length; i++) { Class cls = parameterTypes[i]; if (cls.equals(Optional.class)) { - md.indexToExpanderClass().put(i, GuavaOptionalExpander.class); + if (FluentIterable.of(annotations[i]).transform(EXTRACT_CLASS).contains(HeaderParam.class)) { + md.indexToExpanderClass().put(i, GuavaEmptyOptionalExpander.class); + } else { + md.indexToExpanderClass().put(i, GuavaNullOptionalExpander.class); + } } } } @@ -63,4 +73,11 @@ public List parseAndValidatateMetadata(Class targetType) { return mdList; } + private static final Function> EXTRACT_CLASS = new Function>() { + @Override + public Class apply(Annotation input) { + return input.annotationType(); + } + }; + } diff --git a/http-clients/src/test/java/com/palantir/remoting/http/GuavaOptionalAwareContractTest.java b/http-clients/src/test/java/com/palantir/remoting/http/GuavaOptionalAwareContractTest.java index c3a423286..bb4b46926 100644 --- a/http-clients/src/test/java/com/palantir/remoting/http/GuavaOptionalAwareContractTest.java +++ b/http-clients/src/test/java/com/palantir/remoting/http/GuavaOptionalAwareContractTest.java @@ -114,7 +114,7 @@ public void testStringQuery() throws Exception { public void testAbsentHeader() throws Exception { proxy.header(Optional.absent(), "str2"); RecordedRequest takeRequest = server.takeRequest(); - assertThat(takeRequest.getHeader("opt"), is("{opt}")); + assertThat(takeRequest.getHeader("opt"), is("")); assertThat(takeRequest.getHeader("req"), is("str2")); }