diff --git a/core/src/main/java/feign/AsynchronousMethodHandler.java b/core/src/main/java/feign/AsynchronousMethodHandler.java index 4ffc663c6..95ea8509f 100644 --- a/core/src/main/java/feign/AsynchronousMethodHandler.java +++ b/core/src/main/java/feign/AsynchronousMethodHandler.java @@ -29,6 +29,7 @@ import java.util.concurrent.CompletableFuture; import java.util.concurrent.CompletionException; import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicReference; import java.util.function.BiConsumer; import java.util.stream.Stream; @@ -114,18 +115,19 @@ private CompletableFuture executeAndDecode( } private static class CancellableFuture extends CompletableFuture { - private CompletableFuture inner = null; + private final AtomicReference> inner = new AtomicReference<>(); public void setInner(CompletableFuture value) { - inner = value; - inner.whenComplete(pipeTo(this)); + inner.set(value); + value.whenComplete(pipeTo(this)); } @Override public boolean cancel(boolean mayInterruptIfRunning) { final boolean result = super.cancel(mayInterruptIfRunning); - if (inner != null) { - inner.cancel(mayInterruptIfRunning); + CompletableFuture current = inner.get(); + if (current != null) { + current.cancel(mayInterruptIfRunning); } return result; } diff --git a/core/src/main/java/feign/FeignException.java b/core/src/main/java/feign/FeignException.java index 0779598c6..3c96f6aaa 100644 --- a/core/src/main/java/feign/FeignException.java +++ b/core/src/main/java/feign/FeignException.java @@ -226,73 +226,73 @@ private static FeignException errorStatus( Request request, byte[] body, Map> headers) { - if (isClientError(status)) { + if (status >= 400 && status < 500) { return clientErrorStatus(status, message, request, body, headers); } - if (isServerError(status)) { + if (status >= 500 && status <= 599) { return serverErrorStatus(status, message, request, body, headers); } return new FeignException(status, message, request, body, headers); } - private static boolean isClientError(int status) { - return status >= 400 && status < 500; - } - private static FeignClientException clientErrorStatus( int status, String message, Request request, byte[] body, Map> headers) { - switch (status) { - case 400: + HttpStatus httpStatus = HttpStatus.from(status); + if (httpStatus == null) { + return new FeignClientException(status, message, request, body, headers); + } + switch (httpStatus) { + case BAD_REQUEST: return new BadRequest(message, request, body, headers); - case 401: + case UNAUTHORIZED: return new Unauthorized(message, request, body, headers); - case 403: + case FORBIDDEN: return new Forbidden(message, request, body, headers); - case 404: + case NOT_FOUND: return new NotFound(message, request, body, headers); - case 405: + case METHOD_NOT_ALLOWED: return new MethodNotAllowed(message, request, body, headers); - case 406: + case NOT_ACCEPTABLE: return new NotAcceptable(message, request, body, headers); - case 409: + case CONFLICT: return new Conflict(message, request, body, headers); - case 410: + case GONE: return new Gone(message, request, body, headers); - case 415: + case UNSUPPORTED_MEDIA_TYPE: return new UnsupportedMediaType(message, request, body, headers); - case 429: + case TOO_MANY_REQUESTS: return new TooManyRequests(message, request, body, headers); - case 422: + case UNPROCESSABLE_ENTITY: return new UnprocessableEntity(message, request, body, headers); default: return new FeignClientException(status, message, request, body, headers); } } - private static boolean isServerError(int status) { - return status >= 500 && status <= 599; - } - private static FeignServerException serverErrorStatus( int status, String message, Request request, byte[] body, Map> headers) { - switch (status) { - case 500: + HttpStatus httpStatus = HttpStatus.from(status); + if (httpStatus == null) { + return new FeignServerException(status, message, request, body, headers); + } + switch (httpStatus) { + case INTERNAL_SERVER_ERROR: return new InternalServerError(message, request, body, headers); - case 501: + case NOT_IMPLEMENTED: return new NotImplemented(message, request, body, headers); - case 502: + case BAD_GATEWAY: return new BadGateway(message, request, body, headers); - case 503: + case SERVICE_UNAVAILABLE: return new ServiceUnavailable(message, request, body, headers); - case 504: + case GATEWAY_TIMEOUT: return new GatewayTimeout(message, request, body, headers); default: return new FeignServerException(status, message, request, body, headers); @@ -324,77 +324,77 @@ public FeignClientException( public static class BadRequest extends FeignClientException { public BadRequest( String message, Request request, byte[] body, Map> headers) { - super(400, message, request, body, headers); + super(HttpStatus.BAD_REQUEST.code(), message, request, body, headers); } } public static class Unauthorized extends FeignClientException { public Unauthorized( String message, Request request, byte[] body, Map> headers) { - super(401, message, request, body, headers); + super(HttpStatus.UNAUTHORIZED.code(), message, request, body, headers); } } public static class Forbidden extends FeignClientException { public Forbidden( String message, Request request, byte[] body, Map> headers) { - super(403, message, request, body, headers); + super(HttpStatus.FORBIDDEN.code(), message, request, body, headers); } } public static class NotFound extends FeignClientException { public NotFound( String message, Request request, byte[] body, Map> headers) { - super(404, message, request, body, headers); + super(HttpStatus.NOT_FOUND.code(), message, request, body, headers); } } public static class MethodNotAllowed extends FeignClientException { public MethodNotAllowed( String message, Request request, byte[] body, Map> headers) { - super(405, message, request, body, headers); + super(HttpStatus.METHOD_NOT_ALLOWED.code(), message, request, body, headers); } } public static class NotAcceptable extends FeignClientException { public NotAcceptable( String message, Request request, byte[] body, Map> headers) { - super(406, message, request, body, headers); + super(HttpStatus.NOT_ACCEPTABLE.code(), message, request, body, headers); } } public static class Conflict extends FeignClientException { public Conflict( String message, Request request, byte[] body, Map> headers) { - super(409, message, request, body, headers); + super(HttpStatus.CONFLICT.code(), message, request, body, headers); } } public static class Gone extends FeignClientException { public Gone( String message, Request request, byte[] body, Map> headers) { - super(410, message, request, body, headers); + super(HttpStatus.GONE.code(), message, request, body, headers); } } public static class UnsupportedMediaType extends FeignClientException { public UnsupportedMediaType( String message, Request request, byte[] body, Map> headers) { - super(415, message, request, body, headers); + super(HttpStatus.UNSUPPORTED_MEDIA_TYPE.code(), message, request, body, headers); } } public static class TooManyRequests extends FeignClientException { public TooManyRequests( String message, Request request, byte[] body, Map> headers) { - super(429, message, request, body, headers); + super(HttpStatus.TOO_MANY_REQUESTS.code(), message, request, body, headers); } } public static class UnprocessableEntity extends FeignClientException { public UnprocessableEntity( String message, Request request, byte[] body, Map> headers) { - super(422, message, request, body, headers); + super(HttpStatus.UNPROCESSABLE_ENTITY.code(), message, request, body, headers); } } @@ -412,35 +412,35 @@ public FeignServerException( public static class InternalServerError extends FeignServerException { public InternalServerError( String message, Request request, byte[] body, Map> headers) { - super(500, message, request, body, headers); + super(HttpStatus.INTERNAL_SERVER_ERROR.code(), message, request, body, headers); } } public static class NotImplemented extends FeignServerException { public NotImplemented( String message, Request request, byte[] body, Map> headers) { - super(501, message, request, body, headers); + super(HttpStatus.NOT_IMPLEMENTED.code(), message, request, body, headers); } } public static class BadGateway extends FeignServerException { public BadGateway( String message, Request request, byte[] body, Map> headers) { - super(502, message, request, body, headers); + super(HttpStatus.BAD_GATEWAY.code(), message, request, body, headers); } } public static class ServiceUnavailable extends FeignServerException { public ServiceUnavailable( String message, Request request, byte[] body, Map> headers) { - super(503, message, request, body, headers); + super(HttpStatus.SERVICE_UNAVAILABLE.code(), message, request, body, headers); } } public static class GatewayTimeout extends FeignServerException { public GatewayTimeout( String message, Request request, byte[] body, Map> headers) { - super(504, message, request, body, headers); + super(HttpStatus.GATEWAY_TIMEOUT.code(), message, request, body, headers); } } diff --git a/core/src/main/java/feign/HttpStatus.java b/core/src/main/java/feign/HttpStatus.java new file mode 100644 index 000000000..529f203f4 --- /dev/null +++ b/core/src/main/java/feign/HttpStatus.java @@ -0,0 +1,76 @@ +/* + * Copyright © 2012 The Feign Authors (feign@commonhaus.dev) + * + * 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; + +/** + * Enumerates the HTTP status codes recognised by Feign's exception hierarchy. Using named constants + * instead of bare integers removes magic numbers from call sites and keeps the status-to-exception + * mapping in a single place. + */ +public enum HttpStatus { + OK(200), + NO_CONTENT(204), + RESET_CONTENT(205), + BAD_REQUEST(400), + UNAUTHORIZED(401), + FORBIDDEN(403), + NOT_FOUND(404), + METHOD_NOT_ALLOWED(405), + NOT_ACCEPTABLE(406), + CONFLICT(409), + GONE(410), + UNSUPPORTED_MEDIA_TYPE(415), + UNPROCESSABLE_ENTITY(422), + TOO_MANY_REQUESTS(429), + INTERNAL_SERVER_ERROR(500), + NOT_IMPLEMENTED(501), + BAD_GATEWAY(502), + SERVICE_UNAVAILABLE(503), + GATEWAY_TIMEOUT(504); + + private final int code; + + HttpStatus(int code) { + this.code = code; + } + + public int code() { + return code; + } + + /** + * Resolves an integer status code to the matching {@link HttpStatus} constant, or {@code null} if + * the code is not enumerated here. + */ + public static HttpStatus from(int code) { + for (HttpStatus status : values()) { + if (status.code == code) { + return status; + } + } + return null; + } + + /** {@code true} when the status indicates a 4xx client error. */ + public boolean isClientError() { + return code >= 400 && code < 500; + } + + /** {@code true} when the status indicates a 5xx server error. */ + public boolean isServerError() { + return code >= 500 && code <= 599; + } +} diff --git a/core/src/main/java/feign/Logger.java b/core/src/main/java/feign/Logger.java index a460251a6..b4f5c88db 100644 --- a/core/src/main/java/feign/Logger.java +++ b/core/src/main/java/feign/Logger.java @@ -68,7 +68,7 @@ protected boolean shouldLogResponseHeader(String header) { protected void logRequest(String configKey, Level logLevel, Request request) { String protocolVersion = resolveProtocolVersion(request.protocolVersion()); log(configKey, "---> %s %s %s", request.httpMethod().name(), request.url(), protocolVersion); - if (logLevel.ordinal() >= Level.HEADERS.ordinal()) { + if (logLevel.atLeast(Level.HEADERS)) { for (String field : request.headers().keySet()) { if (shouldLogRequestHeader(field)) { @@ -81,7 +81,7 @@ protected void logRequest(String configKey, Level logLevel, Request request) { int bodyLength = 0; if (request.body() != null) { bodyLength = request.length(); - if (logLevel.ordinal() >= Level.FULL.ordinal()) { + if (logLevel.atLeast(Level.FULL)) { String bodyText = request.charset() != null ? new String(request.body(), request.charset()) : null; log(configKey, ""); // CRLF @@ -105,27 +105,22 @@ protected Response logAndRebufferResponse( : ""; int status = response.status(); log(configKey, "<--- %s %s%s (%sms)", protocolVersion, status, reason, elapsedTime); - if (logLevel.ordinal() >= Level.HEADERS.ordinal()) { + if (logLevel.atLeast(Level.HEADERS)) { - for (String field : response.headers().keySet()) { - if (shouldLogResponseHeader(field)) { - for (String value : valuesOrEmpty(response.headers(), field)) { - log(configKey, "%s: %s", field, value); - } - } - } + logResponseHeaders(configKey, logLevel, response); int bodyLength = 0; - if (response.body() != null && !(status == 204 || status == 205)) { + if (response.body() != null + && status != HttpStatus.NO_CONTENT.code() + && status != HttpStatus.RESET_CONTENT.code()) { // HTTP 204 No Content "...response MUST NOT include a message-body" // HTTP 205 Reset Content "...response MUST NOT include an entity" - if (logLevel.ordinal() >= Level.FULL.ordinal()) { + if (logLevel.atLeast(Level.FULL)) { log(configKey, ""); // CRLF } - byte[] bodyData = Util.toByteArray(response.body().asInputStream()); - ensureClosed(response.body()); + byte[] bodyData = rebufferBody(response); bodyLength = bodyData.length; - if (logLevel.ordinal() >= Level.FULL.ordinal() && bodyLength > 0) { + if (logLevel.atLeast(Level.FULL) && bodyLength > 0) { log(configKey, "%s", decodeOrDefault(bodyData, UTF_8, "Binary data")); } log(configKey, "<--- END HTTP (%s-byte body)", bodyLength); @@ -137,6 +132,22 @@ protected Response logAndRebufferResponse( return response; } + private void logResponseHeaders(String configKey, Level logLevel, Response response) { + for (String field : response.headers().keySet()) { + if (shouldLogResponseHeader(field)) { + for (String value : valuesOrEmpty(response.headers(), field)) { + log(configKey, "%s: %s", field, value); + } + } + } + } + + private byte[] rebufferBody(Response response) throws IOException { + byte[] bodyData = Util.toByteArray(response.body().asInputStream()); + ensureClosed(response.body()); + return bodyData; + } + protected IOException logIOException( String configKey, Level logLevel, IOException ioe, long elapsedTime) { log( @@ -145,7 +156,7 @@ protected IOException logIOException( ioe.getClass().getSimpleName(), ioe.getMessage(), elapsedTime); - if (logLevel.ordinal() >= Level.FULL.ordinal()) { + if (logLevel.atLeast(Level.FULL)) { StringWriter sw = new StringWriter(); ioe.printStackTrace(new PrintWriter(sw)); log(configKey, "%s", sw.toString()); @@ -170,7 +181,12 @@ public enum Level { /** Log the basic information along with request and response headers. */ HEADERS, /** Log the headers, body, and metadata for both requests and responses. */ - FULL + FULL; + + /** Returns {@code true} if this level is at least as verbose as {@code other}. */ + public boolean atLeast(Level other) { + return ordinal() >= other.ordinal(); + } } /** Logs to System.err. */ diff --git a/core/src/main/java/feign/Request.java b/core/src/main/java/feign/Request.java index a3627a164..10979f36e 100644 --- a/core/src/main/java/feign/Request.java +++ b/core/src/main/java/feign/Request.java @@ -26,7 +26,6 @@ import java.util.Arrays; import java.util.Collection; import java.util.Collections; -import java.util.HashMap; import java.util.Map; import java.util.Optional; import java.util.concurrent.ConcurrentHashMap; @@ -335,9 +334,12 @@ public static class Options { */ @Experimental public Options getMethodOptions(String methodName) { - Map methodOptions = - threadToMethodOptions.getOrDefault(getThreadIdentifier(), new HashMap<>()); - return methodOptions.getOrDefault(methodName, this); + Map methodOptions = threadToMethodOptions.get(getThreadIdentifier()); + if (methodOptions == null) { + return this; + } + Options options = methodOptions.get(methodName); + return options != null ? options : this; } /** @@ -349,10 +351,9 @@ public Options getMethodOptions(String methodName) { @Experimental public void setMethodOptions(String methodName, Options options) { String threadIdentifier = getThreadIdentifier(); - Map methodOptions = - threadToMethodOptions.getOrDefault(threadIdentifier, new HashMap<>()); - threadToMethodOptions.put(threadIdentifier, methodOptions); - methodOptions.put(methodName, options); + threadToMethodOptions + .computeIfAbsent(threadIdentifier, key -> new ConcurrentHashMap<>()) + .put(methodName, options); } /** @@ -517,10 +518,10 @@ public static class Body implements Serializable { private transient Charset encoding; - private byte[] data; + private final byte[] data; private Body() { - super(); + this(null); } private Body(byte[] data) { diff --git a/core/src/main/java/feign/RequestTemplate.java b/core/src/main/java/feign/RequestTemplate.java index 6f376c4ae..dc687f847 100644 --- a/core/src/main/java/feign/RequestTemplate.java +++ b/core/src/main/java/feign/RequestTemplate.java @@ -261,22 +261,6 @@ public RequestTemplate resolve(Map variables) { return resolved; } - /** - * Resolves all expressions, using the variables provided. Values not present in the {@code - * alreadyEncoded} map are pct-encoded. - * - * @param unencoded variable values to substitute. - * @param alreadyEncoded variable names. - * @return a resolved Request Template - * @deprecated use {@link RequestTemplate#resolve(Map)}. Values already encoded are recognized as - * such and skipped. - */ - @SuppressWarnings("unused") - @Deprecated - RequestTemplate resolve(Map unencoded, Map alreadyEncoded) { - return this.resolve(unencoded); - } - /** * Creates a {@link Request} from this template. The template must be resolved before calling this * method, or an {@link IllegalStateException} will be thrown. diff --git a/core/src/main/java/feign/Response.java b/core/src/main/java/feign/Response.java index c1b175ced..e2732adfd 100644 --- a/core/src/main/java/feign/Response.java +++ b/core/src/main/java/feign/Response.java @@ -36,13 +36,13 @@ public final class Response implements Closeable { private final ProtocolVersion protocolVersion; private Response(Builder builder) { - checkState(builder.request != null, "original request is required"); - this.status = builder.status; - this.request = builder.request; - this.reason = builder.reason; // nullable - this.headers = caseInsensitiveCopyOf(builder.headers); - this.body = builder.body; // nullable - this.protocolVersion = builder.protocolVersion; + checkState(builder.request() != null, "original request is required"); + this.status = builder.status(); + this.request = builder.request(); + this.reason = builder.reason(); // nullable + this.headers = caseInsensitiveCopyOf(builder.headers()); + this.body = builder.body(); // nullable + this.protocolVersion = builder.protocolVersion(); } public Builder toBuilder() { @@ -55,11 +55,11 @@ public static Builder builder() { public static final class Builder { private static final ProtocolVersion DEFAULT_PROTOCOL_VERSION = ProtocolVersion.HTTP_1_1; - int status; - String reason; - Map> headers; - Body body; - Request request; + private int status; + private String reason; + private Map> headers; + private Body body; + private Request request; private RequestTemplate requestTemplate; private ProtocolVersion protocolVersion = DEFAULT_PROTOCOL_VERSION; @@ -74,6 +74,30 @@ public static final class Builder { this.protocolVersion = source.protocolVersion; } + int status() { + return status; + } + + String reason() { + return reason; + } + + Map> headers() { + return headers; + } + + Body body() { + return body; + } + + Request request() { + return request; + } + + ProtocolVersion protocolVersion() { + return protocolVersion; + } + /** * @see Response#status */