Merge pull request #34 from pnepywoda/pnepywoda/feigncontract

FeignClients now handle Optional in PathParam and QueryParam

http-clients/src/main/java/com/palantir/remoting/http/FeignClients.java

@@ -40,7 +40,7 @@ private FeignClients() {}
  */
  public static FeignClientFactory standard() {
  return FeignClientFactory.of(
- new JAXRSContract(),
+ new GuavaOptionalAwareContract(new JAXRSContract()),
  new JacksonEncoder(ObjectMappers.guavaJdk7()),
  new OptionalAwareDecoder(new TextDelegateDecoder(new JacksonDecoder(ObjectMappers.guavaJdk7()))),
  SerializableErrorErrorDecoder.INSTANCE,
@@ -52,7 +52,7 @@ public static FeignClientFactory standard() {
  */
  public static FeignClientFactory vanilla() {
  return FeignClientFactory.of(
- new JAXRSContract(),
+ new GuavaOptionalAwareContract(new JAXRSContract()),
  new JacksonEncoder(ObjectMappers.vanilla()),
  new OptionalAwareDecoder(new TextDelegateDecoder(new JacksonDecoder(ObjectMappers.vanilla()))),
  SerializableErrorErrorDecoder.INSTANCE,
@@ -64,7 +64,7 @@ public static FeignClientFactory vanilla() {
  */
  public static FeignClientFactory withMapper(ObjectMapper mapper) {
  return FeignClientFactory.of(
- new JAXRSContract(),
+ new GuavaOptionalAwareContract(new JAXRSContract()),
  new JacksonEncoder(mapper),
  new OptionalAwareDecoder(new TextDelegateDecoder(new JacksonDecoder(mapper))),
  SerializableErrorErrorDecoder.INSTANCE,

http-clients/src/main/java/com/palantir/remoting/http/GuavaOptionalAwareContract.java

@@ -0,0 +1,66 @@
+/*
+ * 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 feign.Contract;
+import feign.Feign;
+import feign.MethodMetadata;
+import java.lang.reflect.Method;
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Map;
+
+/**
+ * Decorates a Contract and uses {@link GuavaOptionalExpander} for any HeaderParam, PathParam or QueryParam parameters.
+ */
+public final class GuavaOptionalAwareContract implements Contract {
+
+ private final Contract delegate;
+
+ public GuavaOptionalAwareContract(Contract delegate) {
+ this.delegate = delegate;
+ }
+
+ @Override
+ public List<MethodMetadata> parseAndValidatateMetadata(Class<?> targetType) {
+ List<MethodMetadata> mdList = delegate.parseAndValidatateMetadata(targetType);
+ Map<String, MethodMetadata> methodMetadataByConfigKey = new LinkedHashMap<String, MethodMetadata>();
+ for (MethodMetadata md : mdList) {
+ methodMetadataByConfigKey.put(md.configKey(), md);
+ }
+
+ for (Method method : targetType.getMethods()) {
+ if (method.getDeclaringClass() == Object.class) {
+ continue;
+ }
+ String configKey = Feign.configKey(targetType, method);
+ MethodMetadata md = methodMetadataByConfigKey.get(configKey);
+ if (md != null) {
+ Class<?>[] parameterTypes = method.getParameterTypes();
+ for (int i = 0; i < parameterTypes.length; i++) {
+ Class<?> cls = parameterTypes[i];
+ if (cls.equals(Optional.class)) {
+ md.indexToExpanderClass().put(i, GuavaOptionalExpander.class);
+ }
+ }
+ }
+ }
+ return mdList;
+ }
+
+}

http-clients/src/main/java/com/palantir/remoting/http/GuavaOptionalExpander.java

@@ -0,0 +1,36 @@
+/*
+ * 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 null for Optional.absent and the toString() of the value otherwise.
+ */
+public final class GuavaOptionalExpander 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()) : null;
+ }
+
+}

http-clients/src/test/java/com/palantir/remoting/http/GuavaOptionalAwareContractTest.java

@@ -0,0 +1,137 @@
+/*
+ * 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 static org.hamcrest.Matchers.is;
+import static org.junit.Assert.assertThat;
+
+import com.google.common.base.Optional;
+import com.google.common.collect.ImmutableSet;
+import com.squareup.okhttp.mockwebserver.MockResponse;
+import com.squareup.okhttp.mockwebserver.MockWebServer;
+import com.squareup.okhttp.mockwebserver.RecordedRequest;
+import javax.net.ssl.SSLSocketFactory;
+import javax.ws.rs.GET;
+import javax.ws.rs.HeaderParam;
+import javax.ws.rs.Path;
+import javax.ws.rs.PathParam;
+import javax.ws.rs.QueryParam;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+
+public final class GuavaOptionalAwareContractTest {
+
+ @Rule
+ public final MockWebServer server = new MockWebServer();
+
+ private FakeoInterface proxy;
+
+ @Before
+ public void before() {
+ proxy = FeignClients.standard().createProxy(Optional.<SSLSocketFactory>absent(),
+ ImmutableSet.of("http://localhost:" + server.getPort()),
+ FakeoInterface.class);
+ server.enqueue(new MockResponse().setBody("\"foo\""));
+ }
+
+ @Path("/")
+ public interface FakeoInterface {
+ @GET
+ @Path("{opt}/foo/{req}")
+ String path(@PathParam("opt") Optional<String> opt, @PathParam("req") String req);
+
+ @GET
+ @Path("foo")
+ String query(@QueryParam("opt") Optional<String> opt, @QueryParam("req") String req);
+
+ @GET
+ @Path("foo")
+ String header(@HeaderParam("opt") Optional<String> opt, @HeaderParam("req") String req);
+ }
+
+ @Test
+ public void testAbsentPath() throws Exception {
+ proxy.path(Optional.<String>absent(), "str2");
+ RecordedRequest takeRequest = server.takeRequest();
+ // note that this differs from feign's default behavior of leaving the template
+ // in the path. The default would be "/{opt}/foo"
+ // This is because feign expands null values to their parameter name if they
+ // have no Expander and the value is null
+ assertThat(takeRequest.getPath(), is("/null/foo/str2"));
+ }
+
+ @Test
+ public void testEmptyStringPath() throws Exception {
+ proxy.path(Optional.<String>of(""), "str2");
+ RecordedRequest takeRequest = server.takeRequest();
+ assertThat(takeRequest.getPath(), is("//foo/str2"));
+ }
+
+ @Test
+ public void testStringPath() throws Exception {
+ proxy.path(Optional.<String>of("str"), "str2");
+ RecordedRequest takeRequest = server.takeRequest();
+ assertThat(takeRequest.getPath(), is("/str/foo/str2"));
+ }
+
+ @Test
+ public void testAbsentQuery() throws Exception {
+ proxy.query(Optional.<String>absent(), "str2");
+ RecordedRequest takeRequest = server.takeRequest();
+ assertThat(takeRequest.getRequestLine(), is("GET /foo?req=str2 HTTP/1.1"));
+ }
+
+ @Test
+ public void testEmptyStringQuery() throws Exception {
+ proxy.query(Optional.<String>of(""), "str2");
+ RecordedRequest takeRequest = server.takeRequest();
+ assertThat(takeRequest.getRequestLine(), is("GET /foo?opt=&req=str2 HTTP/1.1"));
+ }
+
+ @Test
+ public void testStringQuery() throws Exception {
+ proxy.query(Optional.<String>of("str"), "str2");
+ RecordedRequest takeRequest = server.takeRequest();
+ assertThat(takeRequest.getRequestLine(), is("GET /foo?opt=str&req=str2 HTTP/1.1"));
+ }
+
+ @Test
+ public void testAbsentHeader() throws Exception {
+ proxy.header(Optional.<String>absent(), "str2");
+ RecordedRequest takeRequest = server.takeRequest();
+ assertThat(takeRequest.getHeader("opt"), is("{opt}"));
+ assertThat(takeRequest.getHeader("req"), is("str2"));
+ }
+
+ @Test
+ public void testEmptyStringHeader() throws Exception {
+ proxy.header(Optional.<String>of(""), "str2");
+ RecordedRequest takeRequest = server.takeRequest();
+ assertThat(takeRequest.getHeader("opt"), is(""));
+ assertThat(takeRequest.getHeader("req"), is("str2"));
+ }
+
+ @Test
+ public void testStringHeader() throws Exception {
+ proxy.header(Optional.<String>of("str"), "str2");
+ RecordedRequest takeRequest = server.takeRequest();
+ assertThat(takeRequest.getHeader("opt"), is("str"));
+ assertThat(takeRequest.getHeader("req"), is("str2"));
+ }
+
+}