From 0b9da425cbce5b77185514a9f14aff2e2d5fb19a Mon Sep 17 00:00:00 2001 From: "Md. Saikat Islam" Date: Thu, 27 Aug 2026 14:55:00 +0600 Subject: [PATCH 1/2] Refactor Logger and encapsulate Response.Builder fields - Logger.Level gains atLeast(Level) replacing ordinal comparisons - logAndRebufferResponse split into logResponseHeaders/rebufferBody helpers - Response.Builder fields made private; access is within the same file Tests: LoggerMethodsTest#atLeast* (new) --- core/src/main/java/feign/Logger.java | 46 +++++++++++------- core/src/main/java/feign/Response.java | 48 ++++++++++++++----- .../test/java/feign/LoggerMethodsTest.java | 15 ++++++ 3 files changed, 81 insertions(+), 28 deletions(-) diff --git a/core/src/main/java/feign/Logger.java b/core/src/main/java/feign/Logger.java index a460251a6a..3f18d0d122 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,20 @@ 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)) { // 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 +130,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 +154,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 +179,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/Response.java b/core/src/main/java/feign/Response.java index c1b175ced8..e2732adfd2 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 */ diff --git a/core/src/test/java/feign/LoggerMethodsTest.java b/core/src/test/java/feign/LoggerMethodsTest.java index bee76be63b..b9f6b2b9f6 100644 --- a/core/src/test/java/feign/LoggerMethodsTest.java +++ b/core/src/test/java/feign/LoggerMethodsTest.java @@ -54,4 +54,19 @@ void responseIsClosedAfterRebuffer() throws IOException { verify(spyBody).close(); assertThat(rebufferedResponse.body()).isNotSameAs(spyBody); } + + @Test + void atLeastReturnsTrueForEqualLevel() { + assertThat(Level.BASIC.atLeast(Level.BASIC)).isTrue(); + } + + @Test + void atLeastReturnsTrueWhenMoreVerbose() { + assertThat(Level.FULL.atLeast(Level.HEADERS)).isTrue(); + } + + @Test + void atLeastReturnsFalseWhenLessVerbose() { + assertThat(Level.NONE.atLeast(Level.FULL)).isFalse(); + } } From bbd6eab0c80006de1c6994c20adb76e52011081b Mon Sep 17 00:00:00 2001 From: "Md. Saikat Islam" Date: Tue, 22 Sep 2026 02:20:57 +0600 Subject: [PATCH 2/2] Address review feedback for PR #3553 - Remove redundant Response.Builder getter methods and restore direct field access - Remove unused Level logLevel parameter from logResponseHeaders - Convert compareTo(Level.NONE) check to logLevel.atLeast(Level.BASIC) - Clean up formatting in LoggerMethodsTest --- core/src/main/java/feign/Logger.java | 8 ++-- core/src/main/java/feign/Response.java | 38 ++++--------------- .../test/java/feign/LoggerMethodsTest.java | 2 +- 3 files changed, 11 insertions(+), 37 deletions(-) diff --git a/core/src/main/java/feign/Logger.java b/core/src/main/java/feign/Logger.java index d54500df4b..cb38ed8e12 100644 --- a/core/src/main/java/feign/Logger.java +++ b/core/src/main/java/feign/Logger.java @@ -104,14 +104,12 @@ protected Response logAndRebufferResponse( String configKey, Level logLevel, Response response, long elapsedTime) throws IOException { String protocolVersion = resolveProtocolVersion(response.protocolVersion()); String reason = - response.reason() != null && logLevel.compareTo(Level.NONE) > 0 - ? " " + response.reason() - : ""; + response.reason() != null && logLevel.atLeast(Level.BASIC) ? " " + response.reason() : ""; int status = response.status(); log(configKey, "<--- %s %s%s (%sms)", protocolVersion, status, reason, elapsedTime); if (logLevel.atLeast(Level.HEADERS)) { - logResponseHeaders(configKey, logLevel, response); + logResponseHeaders(configKey, response); int bodyLength = 0; if (response.body() != null && !(status == 204 || status == 205)) { @@ -134,7 +132,7 @@ protected Response logAndRebufferResponse( return response; } - private void logResponseHeaders(String configKey, Level logLevel, Response response) { + private void logResponseHeaders(String configKey, Response response) { for (String field : response.headers().keySet()) { if (shouldLogResponseHeader(field)) { for (String value : valuesOrEmpty(response.headers(), field)) { diff --git a/core/src/main/java/feign/Response.java b/core/src/main/java/feign/Response.java index e2732adfd2..b09a3fd521 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() { @@ -74,30 +74,6 @@ 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 */ diff --git a/core/src/test/java/feign/LoggerMethodsTest.java b/core/src/test/java/feign/LoggerMethodsTest.java index b9f6b2b9f6..01f08d51e9 100644 --- a/core/src/test/java/feign/LoggerMethodsTest.java +++ b/core/src/test/java/feign/LoggerMethodsTest.java @@ -54,7 +54,7 @@ void responseIsClosedAfterRebuffer() throws IOException { verify(spyBody).close(); assertThat(rebufferedResponse.body()).isNotSameAs(spyBody); } - + @Test void atLeastReturnsTrueForEqualLevel() { assertThat(Level.BASIC.atLeast(Level.BASIC)).isTrue();