diff --git a/core/src/main/java/feign/Request.java b/core/src/main/java/feign/Request.java index b778347ff..39fd80441 100644 --- a/core/src/main/java/feign/Request.java +++ b/core/src/main/java/feign/Request.java @@ -20,6 +20,7 @@ import java.net.HttpURLConnection; import java.nio.charset.Charset; import java.util.*; +import java.util.concurrent.TimeUnit; /** An immutable request to an http server. */ public final class Request { @@ -38,13 +39,17 @@ private Body(byte[] data, Charset encoding, BodyTemplate bodyTemplate) { } public Request.Body expand(Map variables) { - if (bodyTemplate == null) return this; + if (bodyTemplate == null) { + return this; + } return encoded(bodyTemplate.expand(variables).getBytes(encoding), encoding); } public List getVariables() { - if (bodyTemplate == null) return Collections.emptyList(); + if (bodyTemplate == null) { + return Collections.emptyList(); + } return bodyTemplate.getVariables(); } @@ -70,12 +75,16 @@ public String bodyTemplate() { } public String asString() { - return encoding != null && data != null ? new String(data, encoding) : "Binary data"; + return !isBinary() ? new String(data, encoding) : "Binary data"; } public static Body empty() { return new Request.Body(null, null, null); } + + public boolean isBinary() { + return encoding == null || data == null; + } } public enum HttpMethod { @@ -96,6 +105,7 @@ public enum HttpMethod { * * @deprecated {@link #create(HttpMethod, String, Map, byte[], Charset)} */ + @Deprecated public static Request create( String method, String url, @@ -103,7 +113,7 @@ public static Request create( byte[] body, Charset charset) { checkNotNull(method, "httpMethod of %s", method); - HttpMethod httpMethod = HttpMethod.valueOf(method.toUpperCase()); + final HttpMethod httpMethod = HttpMethod.valueOf(method.toUpperCase()); return create(httpMethod, url, headers, body, charset); } @@ -158,6 +168,7 @@ public static Request create( * @return the HttpMethod string * @deprecated @see {@link #httpMethod()} */ + @Deprecated public String method() { return httpMethod.name(); } @@ -188,6 +199,7 @@ public Map> headers() { * * @deprecated use {@link #requestBody()} instead */ + @Deprecated public Charset charset() { return body.encoding; } @@ -199,6 +211,7 @@ public Charset charset() { * @see #charset() * @deprecated use {@link #requestBody()} instead */ + @Deprecated public byte[] body() { return body.data; } @@ -209,10 +222,10 @@ public Body requestBody() { @Override public String toString() { - StringBuilder builder = new StringBuilder(); + final StringBuilder builder = new StringBuilder(); builder.append(httpMethod).append(' ').append(url).append(" HTTP/1.1\n"); - for (String field : headers.keySet()) { - for (String value : valuesOrEmpty(headers, field)) { + for (final String field : headers.keySet()) { + for (final String value : valuesOrEmpty(headers, field)) { builder.append(field).append(": ").append(value).append('\n'); } } @@ -228,22 +241,43 @@ public String toString() { */ public static class Options { - private final int connectTimeoutMillis; - private final int readTimeoutMillis; + private final long connectTimeout; + private final TimeUnit connectTimeoutUnit; + private final long readTimeout; + private final TimeUnit readTimeoutUnit; private final boolean followRedirects; - public Options(int connectTimeoutMillis, int readTimeoutMillis, boolean followRedirects) { - this.connectTimeoutMillis = connectTimeoutMillis; - this.readTimeoutMillis = readTimeoutMillis; + public Options( + long connectTimeout, + TimeUnit connectTimeoutUnit, + long readTimeout, + TimeUnit readTimeoutUnit, + boolean followRedirects) { + super(); + this.connectTimeout = connectTimeout; + this.connectTimeoutUnit = connectTimeoutUnit; + this.readTimeout = readTimeout; + this.readTimeoutUnit = readTimeoutUnit; this.followRedirects = followRedirects; } + @Deprecated + public Options(int connectTimeoutMillis, int readTimeoutMillis, boolean followRedirects) { + this( + connectTimeoutMillis, + TimeUnit.MILLISECONDS, + readTimeoutMillis, + TimeUnit.MILLISECONDS, + followRedirects); + } + + @Deprecated public Options(int connectTimeoutMillis, int readTimeoutMillis) { this(connectTimeoutMillis, readTimeoutMillis, true); } public Options() { - this(10 * 1000, 60 * 1000); + this(10, TimeUnit.SECONDS, 60, TimeUnit.SECONDS, true); } /** @@ -251,8 +285,9 @@ public Options() { * * @see java.net.HttpURLConnection#getConnectTimeout() */ + @Deprecated public int connectTimeoutMillis() { - return connectTimeoutMillis; + return (int) connectTimeoutUnit.toMillis(connectTimeout); } /** @@ -260,8 +295,9 @@ public int connectTimeoutMillis() { * * @see java.net.HttpURLConnection#getReadTimeout() */ + @Deprecated public int readTimeoutMillis() { - return readTimeoutMillis; + return (int) readTimeoutUnit.toMillis(readTimeout); } /** @@ -272,5 +308,21 @@ public int readTimeoutMillis() { public boolean isFollowRedirects() { return followRedirects; } + + public long connectTimeout() { + return connectTimeout; + } + + public TimeUnit connectTimeoutUnit() { + return connectTimeoutUnit; + } + + public long readTimeout() { + return readTimeout; + } + + public TimeUnit readTimeoutUnit() { + return readTimeoutUnit; + } } } diff --git a/hc5/README.md b/hc5/README.md new file mode 100644 index 000000000..bf65df948 --- /dev/null +++ b/hc5/README.md @@ -0,0 +1,12 @@ +Apache Http Compoments 5 +======================== + +This module directs Feign's http requests to Apache's [HttpClient 5](https://hc.apache.org/httpcomponents-client-5.0.x/index.html). + +To use HttpClient with Feign, add the `feign-hc5` module to your classpath. Then, configure Feign to use the `ApacheHttp5Client`: + +```java +GitHub github = Feign.builder() + .client(new ApacheHttp5Client()) + .target(GitHub.class, "https://api.github.com"); +``` diff --git a/hc5/pom.xml b/hc5/pom.xml new file mode 100644 index 000000000..d3d3b8397 --- /dev/null +++ b/hc5/pom.xml @@ -0,0 +1,65 @@ + + + + 4.0.0 + + + io.github.openfeign + parent + 10.4.1-SNAPSHOT + + + feign-hc5 + Feign Apache Http Client 5 + Feign Apache HttpComponents Client 5 + + + ${project.basedir}/.. + + + + + ${project.groupId} + feign-core + + + + org.apache.httpcomponents.client5 + httpclient5 + 5.0-beta5 + + + + ${project.groupId} + feign-core + test-jar + test + + + + ${project.groupId} + feign-jaxrs2 + test + + + + com.squareup.okhttp3 + mockwebserver + test + + + diff --git a/hc5/src/main/java/feign/hc5/ApacheHttp5Client.java b/hc5/src/main/java/feign/hc5/ApacheHttp5Client.java new file mode 100644 index 000000000..646b0b1f0 --- /dev/null +++ b/hc5/src/main/java/feign/hc5/ApacheHttp5Client.java @@ -0,0 +1,235 @@ +/** + * Copyright 2012-2019 The Feign Authors + * + *

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 feign.hc5; + +import static feign.Util.UTF_8; + +import feign.*; +import feign.Request.Body; +import java.io.*; +import java.net.URI; +import java.net.URISyntaxException; +import java.nio.charset.Charset; +import java.util.*; +import org.apache.hc.client5.http.classic.HttpClient; +import org.apache.hc.client5.http.config.Configurable; +import org.apache.hc.client5.http.config.RequestConfig; +import org.apache.hc.client5.http.impl.classic.HttpClientBuilder; +import org.apache.hc.client5.http.protocol.HttpClientContext; +import org.apache.hc.core5.http.*; +import org.apache.hc.core5.http.io.entity.ByteArrayEntity; +import org.apache.hc.core5.http.io.entity.EntityUtils; +import org.apache.hc.core5.http.io.entity.StringEntity; +import org.apache.hc.core5.http.io.support.ClassicRequestBuilder; +import org.apache.hc.core5.net.URIBuilder; +import org.apache.hc.core5.net.URLEncodedUtils; + +/** + * This module directs Feign's http requests to Apache's + * HttpClient 5. Ex. + * + * 

+ * GitHub github = Feign.builder().client(new ApacheHttp5Client()).target(GitHub.class,
+ * "https://api.github.com");
+ */
+/*
+ */
+public final class ApacheHttp5Client implements Client {
+  private static final String ACCEPT_HEADER_NAME = "Accept";
+
+  private final HttpClient client;
+
+  public ApacheHttp5Client() {
+    this(HttpClientBuilder.create().build());
+  }
+
+  public ApacheHttp5Client(HttpClient client) {
+    this.client = client;
+  }
+
+  @Override
+  public Response execute(Request request, Request.Options options) throws IOException {
+    ClassicHttpRequest httpUriRequest;
+    try {
+      httpUriRequest = toClassicHttpRequest(request, options);
+    } catch (final URISyntaxException e) {
+      throw new IOException("URL '" + request.url() + "' couldn't be parsed into a URI", e);
+    }
+    final HttpHost target = HttpHost.create(URI.create(request.url()));
+    final HttpClientContext context = configureTimeouts(options);
+
+    final ClassicHttpResponse httpResponse =
+        (ClassicHttpResponse) client.execute(target, httpUriRequest, context);
+    return toFeignResponse(httpResponse, request);
+  }
+
+  protected HttpClientContext configureTimeouts(Request.Options options) {
+    final HttpClientContext context = new HttpClientContext();
+    // per request timeouts
+    final RequestConfig requestConfig =
+        (client instanceof Configurable
+                ? RequestConfig.copy(((Configurable) client).getConfig())
+                : RequestConfig.custom())
+            .setConnectTimeout(options.connectTimeout(), options.connectTimeoutUnit())
+            .setResponseTimeout(options.readTimeout(), options.readTimeoutUnit())
+            .build();
+    context.setRequestConfig(requestConfig);
+    return context;
+  }
+
+  ClassicHttpRequest toClassicHttpRequest(Request request, Request.Options options)
+      throws URISyntaxException {
+    final ClassicRequestBuilder requestBuilder =
+        ClassicRequestBuilder.create(request.httpMethod().name());
+
+    final URI uri = new URIBuilder(request.url()).build();
+
+    requestBuilder.setUri(uri.getScheme() + "://" + uri.getAuthority() + uri.getRawPath());
+
+    // request query params
+    final List queryParams = URLEncodedUtils.parse(uri, requestBuilder.getCharset());
+    for (final NameValuePair queryParam : queryParams) {
+      requestBuilder.addParameter(queryParam);
+    }
+
+    // request headers
+    boolean hasAcceptHeader = false;
+    for (final Map.Entry> headerEntry : request.headers().entrySet()) {
+      final String headerName = headerEntry.getKey();
+      if (headerName.equalsIgnoreCase(ACCEPT_HEADER_NAME)) {
+        hasAcceptHeader = true;
+      }
+
+      if (headerName.equalsIgnoreCase(Util.CONTENT_LENGTH)) {
+        // The 'Content-Length' header is always set by the Apache client and it
+        // doesn't like us to set it as well.
+        continue;
+      }
+
+      for (final String headerValue : headerEntry.getValue()) {
+        requestBuilder.addHeader(headerName, headerValue);
+      }
+    }
+    // some servers choke on the default accept string, so we'll set it to anything
+    if (!hasAcceptHeader) {
+      requestBuilder.addHeader(ACCEPT_HEADER_NAME, "*/*");
+    }
+
+    // request body
+    final Body requestBody = request.requestBody();
+    if (requestBody.asBytes() != null) {
+      HttpEntity entity;
+      if (requestBody.isBinary()) {
+        entity = new ByteArrayEntity(requestBody.asBytes(), null);
+      } else {
+        final ContentType contentType = getContentType(request);
+        entity = new StringEntity(requestBody.asString(), contentType);
+      }
+
+      requestBuilder.setEntity(entity);
+    } else {
+      requestBuilder.setEntity(new ByteArrayEntity(new byte[0], null));
+    }
+
+    final ClassicHttpRequest classicRequest = requestBuilder.build();
+
+    return classicRequest;
+  }
+
+  private ContentType getContentType(Request request) {
+    ContentType contentType = null;
+    for (final Map.Entry> entry : request.headers().entrySet()) {
+      if (entry.getKey().equalsIgnoreCase("Content-Type")) {
+        final Collection values = entry.getValue();
+        if (values != null && !values.isEmpty()) {
+          contentType = ContentType.parse(values.iterator().next());
+          if (contentType.getCharset() == null) {
+            contentType = contentType.withCharset(request.charset());
+          }
+          break;
+        }
+      }
+    }
+    return contentType;
+  }
+
+  Response toFeignResponse(ClassicHttpResponse httpResponse, Request request) throws IOException {
+    final int statusCode = httpResponse.getCode();
+
+    final String reason = httpResponse.getReasonPhrase();
+
+    final Map> headers = new HashMap>();
+    for (final Header header : httpResponse.getHeaders()) {
+      final String name = header.getName();
+      final String value = header.getValue();
+
+      Collection headerValues = headers.get(name);
+      if (headerValues == null) {
+        headerValues = new ArrayList();
+        headers.put(name, headerValues);
+      }
+      headerValues.add(value);
+    }
+
+    return Response.builder()
+        .status(statusCode)
+        .reason(reason)
+        .headers(headers)
+        .request(request)
+        .body(toFeignBody(httpResponse))
+        .build();
+  }
+
+  Response.Body toFeignBody(ClassicHttpResponse httpResponse) {
+    final HttpEntity entity = httpResponse.getEntity();
+    if (entity == null) {
+      return null;
+    }
+    return new Response.Body() {
+
+      @Override
+      public Integer length() {
+        return entity.getContentLength() >= 0 && entity.getContentLength() <= Integer.MAX_VALUE
+            ? (int) entity.getContentLength()
+            : null;
+      }
+
+      @Override
+      public boolean isRepeatable() {
+        return entity.isRepeatable();
+      }
+
+      @Override
+      public InputStream asInputStream() throws IOException {
+        return entity.getContent();
+      }
+
+      @Override
+      public Reader asReader() throws IOException {
+        return new InputStreamReader(asInputStream(), UTF_8);
+      }
+
+      @Override
+      public Reader asReader(Charset charset) throws IOException {
+        Util.checkNotNull(charset, "charset should not be null");
+        return new InputStreamReader(asInputStream(), charset);
+      }
+
+      @Override
+      public void close() throws IOException {
+        EntityUtils.consume(entity);
+      }
+    };
+  }
+}
diff --git a/hc5/src/test/java/feign/hc5/ApacheHttp5ClientTest.java b/hc5/src/test/java/feign/hc5/ApacheHttp5ClientTest.java
new file mode 100644
index 000000000..830643c7e
--- /dev/null
+++ b/hc5/src/test/java/feign/hc5/ApacheHttp5ClientTest.java
@@ -0,0 +1,79 @@
+/**
+ * Copyright 2012-2019 The Feign Authors
+ *
+ * 
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 feign.hc5;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assume.assumeTrue;
+
+import feign.Feign;
+import feign.Feign.Builder;
+import feign.client.AbstractClientTest;
+import feign.jaxrs.JAXRSContract;
+import java.nio.charset.StandardCharsets;
+import javax.ws.rs.PUT;
+import javax.ws.rs.Path;
+import javax.ws.rs.QueryParam;
+import okhttp3.mockwebserver.MockResponse;
+import okhttp3.mockwebserver.RecordedRequest;
+import org.apache.hc.client5.http.classic.HttpClient;
+import org.apache.hc.client5.http.impl.classic.HttpClientBuilder;
+import org.junit.Test;
+
+/** Tests client-specific behavior, such as ensuring Content-Length is sent when specified. */
+public class ApacheHttp5ClientTest extends AbstractClientTest {
+
+  @Override
+  public Builder newBuilder() {
+    return Feign.builder().client(new ApacheHttp5Client());
+  }
+
+  @Test
+  public void queryParamsAreRespectedWhenBodyIsEmpty() throws InterruptedException {
+    final HttpClient httpClient = HttpClientBuilder.create().build();
+    final JaxRsTestInterface testInterface =
+        Feign.builder()
+            .contract(new JAXRSContract())
+            .client(new ApacheHttp5Client(httpClient))
+            .target(JaxRsTestInterface.class, "http://localhost:" + server.getPort());
+
+    server.enqueue(new MockResponse().setBody("foo"));
+    server.enqueue(new MockResponse().setBody("foo"));
+
+    assertEquals("foo", testInterface.withBody("foo", "bar"));
+    final RecordedRequest request1 = server.takeRequest();
+    assertEquals("/withBody?foo=foo", request1.getPath());
+    assertEquals("bar", request1.getBody().readString(StandardCharsets.UTF_8));
+
+    assertEquals("foo", testInterface.withoutBody("foo"));
+    final RecordedRequest request2 = server.takeRequest();
+    assertEquals("/withoutBody?foo=foo", request2.getPath());
+    assertEquals("", request2.getBody().readString(StandardCharsets.UTF_8));
+  }
+
+  @Override
+  public void testVeryLongResponseNullLength() {
+    assumeTrue("HC5 client seems to hang with response size equalto Long.MAX", false);
+  }
+
+  @Path("/")
+  public interface JaxRsTestInterface {
+    @PUT
+    @Path("/withBody")
+    String withBody(@QueryParam("foo") String foo, String bar);
+
+    @PUT
+    @Path("/withoutBody")
+    String withoutBody(@QueryParam("foo") String foo);
+  }
+}
diff --git a/jaxrs2/pom.xml b/jaxrs2/pom.xml
index cdc31e072..2507359d4 100644
--- a/jaxrs2/pom.xml
+++ b/jaxrs2/pom.xml
@@ -57,7 +57,6 @@
       javax.ws.rs
       javax.ws.rs-api
       2.1
-      provided
     
 
     
diff --git a/pom.xml b/pom.xml
index 60c242447..3b8d9a054 100644
--- a/pom.xml
+++ b/pom.xml
@@ -29,6 +29,7 @@
     core
     gson
     httpclient
+    hc5
     hystrix
     jackson
     jackson-jaxb