Refactor Logger level checks and encapsulate Response.Builder fields - #3553
Refactor Logger level checks and encapsulate Response.Builder fields#3553saikat709 wants to merge 1 commit into
Conversation
- 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)
velo
left a comment
There was a problem hiding this comment.
The Level.atLeast() extraction is a good, well-named improvement over the scattered ordinal() comparisons, and shortening logAndRebufferResponse by pulling out logResponseHeaders/rebufferBody is a reasonable move. A few things need fixing before merge:
-
The six new Response.Builder getters aren't needed. Builder is a static nested class of Response, so the enclosing class already has access to its private fields directly — I verified this compiles fine (mvn -pl core compile) with the getters removed and the constructor reading builder.status/builder.request/etc. directly. The getters add a second, wider access surface (package-visible, no benefit) and collide in name with the existing fluent setters (status() getter vs status(int) setter). Please drop them and keep direct field access from the constructor — the private modifier change alone already achieves the stated goal.
-
logResponseHeaders(String, Level, Response) takes a logLevel parameter that's never used — the caller already gates the call behind logLevel.atLeast(Level.HEADERS). Please drop the unused parameter.
-
One spot wasn't converted to the new abstraction:
logLevel.compareTo(Level.NONE) > 0should belogLevel.atLeast(Level.BASIC), consistent with the rest of the refactor. -
LoggerMethodsTest.java currently fails the project's own formatting check (git-code-format-maven-plugin / verify-formatting) — there's a stray trailing-whitespace-only blank line before one of the new @test methods. This will fail CI as-is; please run the formatter.
Happy to merge once these are addressed.
Refactors logging level comparisons and encapsulates
Response.Builderfields for better API hygiene.Summary of Changes
Logger.Level#atLeast: AddsatLeast(Level other)helper toLogger.Leveland replacesordinal()comparisons acrossLogger.java.logAndRebufferResponseintologResponseHeadersandrebufferBodyprivate helpers.Response.Builderencapsulation: MakesBuilderfields private (status,reason,headers,body,request).Tests Added
LoggerMethodsTest: Adds tests forLogger.Level.atLeast()covering equal, more verbose, and less verbose level comparisons.$ ./mvnw test -pl core -Dtest=LoggerMethodsTest,LoggerTest,LoggerRebufferTest -Dtoolchain.skip=true
Tests run: 11, Failures: 0, Errors: 0, Skipped: 0