From 47e071f6de4c64e069c2cedbbbd92b5fb131f75e Mon Sep 17 00:00:00 2001 From: Kevin Davis Date: Fri, 20 Jul 2018 12:20:42 -0400 Subject: [PATCH] Refactored Request Method Attribute on Requests * Added `HttpMethod` enum that represents the supported HTTP methods replacing String handling. * Deprecated `Request#method()` in favor of `Request#httpMethod()` --- core/src/main/java/feign/Client.java | 4 +- core/src/main/java/feign/FeignException.java | 6 +-- core/src/main/java/feign/Logger.java | 2 +- core/src/main/java/feign/Request.java | 53 ++++++++++++++++--- core/src/main/java/feign/Response.java | 2 +- .../main/java/feign/RetryableException.java | 15 +++--- .../main/java/feign/codec/ErrorDecoder.java | 2 +- core/src/test/java/feign/FeignTest.java | 5 +- 8 files changed, 64 insertions(+), 25 deletions(-) diff --git a/core/src/main/java/feign/Client.java b/core/src/main/java/feign/Client.java index 6bb292891d..4317114868 100644 --- a/core/src/main/java/feign/Client.java +++ b/core/src/main/java/feign/Client.java @@ -65,7 +65,7 @@ public Default(SSLSocketFactory sslContextFactory, HostnameVerifier hostnameVeri @Override public Response execute(Request request, Options options) throws IOException { HttpURLConnection connection = convertAndSend(request, options); - return convertResponse(connection, request).toBuilder().request(request).build(); + return convertResponse(connection, request); } HttpURLConnection convertAndSend(Request request, Options options) throws IOException { @@ -84,7 +84,7 @@ HttpURLConnection convertAndSend(Request request, Options options) throws IOExce connection.setReadTimeout(options.readTimeoutMillis()); connection.setAllowUserInteraction(false); connection.setInstanceFollowRedirects(options.isFollowRedirects()); - connection.setRequestMethod(request.method()); + connection.setRequestMethod(request.httpMethod().name()); Collection contentEncodingValues = request.headers().get(CONTENT_ENCODING); boolean gzipEncodedRequest = diff --git a/core/src/main/java/feign/FeignException.java b/core/src/main/java/feign/FeignException.java index 513b2f433d..b4b39e62a8 100644 --- a/core/src/main/java/feign/FeignException.java +++ b/core/src/main/java/feign/FeignException.java @@ -43,7 +43,7 @@ public int status() { static FeignException errorReading(Request request, Response ignored, IOException cause) { return new FeignException( - format("%s reading %s %s", cause.getMessage(), request.method(), request.url()), + format("%s reading %s %s", cause.getMessage(), request.httpMethod().name(), request.url()), cause); } @@ -61,8 +61,8 @@ public static FeignException errorStatus(String methodKey, Response response) { static FeignException errorExecuting(Request request, IOException cause) { return new RetryableException( - format("%s executing %s %s", cause.getMessage(), request.method(), request.url()), - request.method(), + format("%s executing %s %s", cause.getMessage(), request.httpMethod(), request.url()), + request.httpMethod(), cause, null); } diff --git a/core/src/main/java/feign/Logger.java b/core/src/main/java/feign/Logger.java index e34dd551c6..dd4f99e1d0 100644 --- a/core/src/main/java/feign/Logger.java +++ b/core/src/main/java/feign/Logger.java @@ -44,7 +44,7 @@ protected static String methodTag(String configKey) { protected abstract void log(String configKey, String format, Object... args); protected void logRequest(String configKey, Level logLevel, Request request) { - log(configKey, "---> %s %s HTTP/1.1", request.method(), request.url()); + log(configKey, "---> %s %s HTTP/1.1", request.httpMethod().name(), request.url()); if (logLevel.ordinal() >= Level.HEADERS.ordinal()) { for (String field : request.headers().keySet()) { diff --git a/core/src/main/java/feign/Request.java b/core/src/main/java/feign/Request.java index 294aa17ddc..e3a37e9197 100644 --- a/core/src/main/java/feign/Request.java +++ b/core/src/main/java/feign/Request.java @@ -25,40 +25,77 @@ */ public final class Request { - public enum Methods { + public enum HttpMethod { GET, HEAD, POST, PUT, DELETE, CONNECT, OPTIONS, TRACE, PATCH } /** * No parameters can be null except {@code body} and {@code charset}. All parameters must be * effectively immutable, via safe copies, not mutating or otherwise. + * + * @deprecated {@link #create(HttpMethod, String, Map, byte[], Charset)} */ public static Request create(String method, String url, Map> headers, byte[] body, Charset charset) { - return new Request(method, url, headers, body, charset); + checkNotNull(method, "httpMethod of %s", method); + HttpMethod httpMethod = HttpMethod.valueOf(method.toUpperCase()); + return create(httpMethod, url, headers, body, charset); } - private final Methods method; + /** + * Builds a Request. All parameters must be effectively immutable, via safe copies. + * + * @param httpMethod for the request. + * @param url for the request. + * @param headers to include. + * @param body of the request, can be {@literal null} + * @param charset of the request, can be {@literal null} + * @return a Request + */ + public static Request create(HttpMethod httpMethod, + String url, + Map> headers, + byte[] body, + Charset charset) { + return new Request(httpMethod, url, headers, body, charset); + + } + + private final HttpMethod httpMethod; private final String url; private final Map> headers; private final byte[] body; private final Charset charset; - Request(String method, String url, Map> headers, byte[] body, + Request(HttpMethod method, String url, Map> headers, byte[] body, Charset charset) { - this.method = Methods.valueOf(checkNotNull(method, "method of %s", url)); + this.httpMethod = checkNotNull(method, "httpMethod of %s", method.name()); this.url = checkNotNull(url, "url"); this.headers = checkNotNull(headers, "headers of %s %s", method, url); this.body = body; // nullable this.charset = charset; // nullable } - /* Method to invoke on the server. */ + /** + * Http Method for this request. + * + * @return the HttpMethod string + * @deprecated @see {@link #httpMethod()} + */ public String method() { - return method.name(); + return httpMethod.name(); + } + + /** + * Http Method for the request. + * + * @return the HttpMethod. + */ + public HttpMethod httpMethod() { + return this.httpMethod; } /* Fully resolved URL including query. */ @@ -93,7 +130,7 @@ public byte[] body() { @Override public String toString() { StringBuilder builder = new StringBuilder(); - builder.append(method).append(' ').append(url).append(" HTTP/1.1\n"); + builder.append(httpMethod).append(' ').append(url).append(" HTTP/1.1\n"); for (String field : headers.keySet()) { for (String value : valuesOrEmpty(headers, field)) { builder.append(field).append(": ").append(value).append('\n'); diff --git a/core/src/main/java/feign/Response.java b/core/src/main/java/feign/Response.java index 195156006d..4b0545aa03 100644 --- a/core/src/main/java/feign/Response.java +++ b/core/src/main/java/feign/Response.java @@ -128,7 +128,7 @@ public Builder request(Request request) { /* don't keep the body, we don't want to tie up memory on large requests */ this.request = Request.create( - request.method(), request.url(), request.headers(), null, request.charset()); + request.httpMethod(), request.url(), request.headers(), null, request.charset()); return this; } diff --git a/core/src/main/java/feign/RetryableException.java b/core/src/main/java/feign/RetryableException.java index 5e04087fe0..cc2e1b702b 100644 --- a/core/src/main/java/feign/RetryableException.java +++ b/core/src/main/java/feign/RetryableException.java @@ -13,6 +13,7 @@ */ package feign; +import feign.Request.HttpMethod; import java.util.Date; /** @@ -24,23 +25,23 @@ public class RetryableException extends FeignException { private static final long serialVersionUID = 1L; private final Long retryAfter; - private final String method; + private final HttpMethod httpMethod; /** * @param retryAfter usually corresponds to the {@link feign.Util#RETRY_AFTER} header. */ - public RetryableException(String message, String method, Throwable cause, Date retryAfter) { + public RetryableException(String message, HttpMethod httpMethod, Throwable cause, Date retryAfter) { super(message, cause); - this.method = method; + this.httpMethod = httpMethod; this.retryAfter = retryAfter != null ? retryAfter.getTime() : null; } /** * @param retryAfter usually corresponds to the {@link feign.Util#RETRY_AFTER} header. */ - public RetryableException(String message, String method, Date retryAfter) { + public RetryableException(String message, HttpMethod httpMethod, Date retryAfter) { super(message); - this.method = method; + this.httpMethod = httpMethod; this.retryAfter = retryAfter != null ? retryAfter.getTime() : null; } @@ -52,7 +53,7 @@ public Date retryAfter() { return retryAfter != null ? new Date(retryAfter) : null; } - public String method() { - return this.method; + public HttpMethod method() { + return this.httpMethod; } } diff --git a/core/src/main/java/feign/codec/ErrorDecoder.java b/core/src/main/java/feign/codec/ErrorDecoder.java index 841b5bc16a..2da7aefba2 100644 --- a/core/src/main/java/feign/codec/ErrorDecoder.java +++ b/core/src/main/java/feign/codec/ErrorDecoder.java @@ -95,7 +95,7 @@ public Exception decode(String methodKey, Response response) { if (retryAfter != null) { return new RetryableException( exception.getMessage(), - response.request().method(), + response.request().httpMethod(), exception, retryAfter); } diff --git a/core/src/test/java/feign/FeignTest.java b/core/src/test/java/feign/FeignTest.java index 25cbfe2171..e3cc35b4bd 100644 --- a/core/src/test/java/feign/FeignTest.java +++ b/core/src/test/java/feign/FeignTest.java @@ -15,6 +15,7 @@ import com.google.gson.Gson; import com.google.gson.reflect.TypeToken; +import feign.Request.HttpMethod; import okhttp3.mockwebserver.MockResponse; import okhttp3.mockwebserver.SocketPolicy; import okhttp3.mockwebserver.MockWebServer; @@ -483,7 +484,7 @@ public void retryableExceptionInDecoder() throws Exception { public Object decode(Response response, Type type) throws IOException { String string = super.decode(response, type).toString(); if ("retry!".equals(string)) { - throw new RetryableException(string, "post", null); + throw new RetryableException(string, HttpMethod.POST, null); } return string; } @@ -524,7 +525,7 @@ public void ensureRetryerClonesItself() { .errorDecoder(new ErrorDecoder() { @Override public Exception decode(String methodKey, Response response) { - return new RetryableException("play it again sam!", "post", null); + return new RetryableException("play it again sam!", HttpMethod.POST, null); } }).target(TestInterface.class, "http://localhost:" + server.getPort());