Skip to content

Commit

Permalink
Refactored Request Method Attribute on Requests
Browse files Browse the repository at this point in the history
* Added `HttpMethod` enum that represents the supported HTTP methods
replacing String handling.
* Deprecated `Request#method()` in favor of `Request#httpMethod()`
  • Loading branch information
kdavisk6 committed Jul 20, 2018
1 parent 806149c commit 47e071f
Show file tree
Hide file tree
Showing 8 changed files with 64 additions and 25 deletions.
4 changes: 2 additions & 2 deletions core/src/main/java/feign/Client.java
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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 =
Expand Down
6 changes: 3 additions & 3 deletions core/src/main/java/feign/FeignException.java
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand All @@ -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);
}
Expand Down
2 changes: 1 addition & 1 deletion core/src/main/java/feign/Logger.java
Original file line number Diff line number Diff line change
Expand Up @@ -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()) {
Expand Down
53 changes: 45 additions & 8 deletions core/src/main/java/feign/Request.java
Original file line number Diff line number Diff line change
Expand Up @@ -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. */
Expand Down Expand Up @@ -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');
Expand Down
2 changes: 1 addition & 1 deletion core/src/main/java/feign/Response.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
15 changes: 8 additions & 7 deletions core/src/main/java/feign/RetryableException.java
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
*/
package feign;

import feign.Request.HttpMethod;
import java.util.Date;

/**
Expand All @@ -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;
}

Expand All @@ -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;
}
}
2 changes: 1 addition & 1 deletion core/src/main/java/feign/codec/ErrorDecoder.java
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
5 changes: 3 additions & 2 deletions core/src/test/java/feign/FeignTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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());

Expand Down

0 comments on commit 47e071f

Please sign in to comment.