Skip to content

Refactor: Tighten Logger, encapsulate Response.Builder, add HttpStatus, fix races#3469

Open
saikat709 wants to merge 5 commits into
OpenFeign:masterfrom
saikat709:refactor/feign-cleanup
Open

Refactor: Tighten Logger, encapsulate Response.Builder, add HttpStatus, fix races#3469
saikat709 wants to merge 5 commits into
OpenFeign:masterfrom
saikat709:refactor/feign-cleanup

Conversation

@saikat709

Copy link
Copy Markdown

Adds a new top-level HttpStatus enum covering the 19 status codes that s 0
FeignException recognises and uses it to replace the literal 400,
401, 403, …, 504 magic numbers in the fourteen FeignClientException /
FeignServerException subclasses and in the clientErrorStatus /
serverErrorStatus switch statements. Also replaces the 204 / 205
literals in Logger.logAndRebufferResponse. Switches now switch on
HttpStatus constants, so the status-to-exception mapping reads off the
constant names directly.

Four follow-ups in the same files:

  • Logger.Level gets an atLeast(Level) helper. The four
    logLevel.ordinal() >= Level.X.ordinal() comparisons in logRequest,
    logAndRebufferResponse, and logIOException become
    logLevel.atLeast(Level.X). logAndRebufferResponse also loses its
    header loop and body rebuffering into logResponseHeaders(...) and
    rebufferBody(...) private helpers.
  • Response.Builder's seven fields are now private. The
    Response(Builder) constructor in the same file routes through
    package-private getters, restoring the immutability contract the ╮
    class javadoc already advertises.
  • Request.Options.getMethodOptions / setMethodOptions previously
    had a check-then-act race on the ConcurrentHashMap<String, Map<String, Options>> and used a plain HashMap for the inner map. They now use
    computeIfAbsent and a ConcurrentHashMap inner map.
    AsynchronousMethodHandler.CancellableFuture.inner is now an
    AtomicReference<CompletableFuture<T>> so retry callbacks no longer
    race with cancel(). Request.Body.data becomes final; the no-arg
    Body() constructor delegates to this(null). ╮
  • The deprecated RequestTemplate.resolve(Map, Map) overload
    (alreadyEncoded was ignored) is removed. All in-repo callers use
    the single-arg resolve(Map).

No public-API additions beyond HttpStatus and Logger.Level#atLeast.
japicmp is unchanged.

$ ./mvnw -pl core test ╮
... Tests run: 634, Failures: 0, Errors: 0, Skipped: 3

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant