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()`

Author: kdavisk6

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<String> contentEncodingValues = request.headers().get(CONTENT_ENCODING);
       boolean gzipEncodedRequest =

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);
   }

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()) {

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<String, Collection<String>> 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<String, Collection<String>> headers,
+                               byte[] body,
+                               Charset charset) {
+    return new Request(httpMethod, url, headers, body, charset);
+
+  }
+
+  private final HttpMethod httpMethod;
   private final String url;
   private final Map<String, Collection<String>> headers;
   private final byte[] body;
   private final Charset charset;
 
-  Request(String method, String url, Map<String, Collection<String>> headers, byte[] body,
+  Request(HttpMethod method, String url, Map<String, Collection<String>> 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');

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;
     }
 

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;
   }
 }

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);
       }

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());
 

core/src/test/java/feign/client/DefaultClientTest.java

@@ -13,20 +13,23 @@
  */
 package feign.client;
 
-import feign.FeignException;
+import static org.hamcrest.core.Is.isA;
+import static org.junit.Assert.assertEquals;
+
 import java.io.IOException;
 import java.net.ProtocolException;
+
 import javax.net.ssl.HostnameVerifier;
 import javax.net.ssl.SSLSession;
+
 import org.junit.Test;
+
 import feign.Client;
 import feign.Feign;
 import feign.Feign.Builder;
 import feign.RetryableException;
 import okhttp3.mockwebserver.MockResponse;
 import okhttp3.mockwebserver.SocketPolicy;
-import static org.hamcrest.core.Is.isA;
-import static org.junit.Assert.assertEquals;
 
 /**
  * Tests client-specific behavior, such as ensuring Content-Length is sent when specified.