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

[saikat709]

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

[velo]

Thanks for the PR — the Builder encapsulation, deprecated-cleanup, and concurrency fixes are welcome. A few things are blocking merge as-is:

Tests. The PR adds public API (HttpStatus.from()/isClientError()/isServerError(), Logger.Level.atLeast()) and claims two race fixes (CancellableFuture, Options.setMethodOptions), but no test files are added or modified. Each of those needs coverage — especially the race fixes, which should have a test demonstrating the scenario.

CancellableFuture is a partial fix. If cancel() wins the race ahead of setInner(), the inner future is still left running — there's no isCancelled() re-check after inner.set(...). Worth closing that window while you're in there.

HttpStatus.from() cost. It linearly scans a fresh values() clone on every 4xx/5xx exception creation, replacing a branch-free int switch. Consider a static lookup array/map indexed by code.

On HttpStatus itself: committing feign-core to a public status enum is a real API decision, so let's make it pull its weight rather than being an internal lookup detour. One concrete way: overload FeignException.FeignClientException/FeignServerException (and the errorStatus path) to accept HttpStatus alongside the existing int constructors, so users can actually consume the type.

Alternatively, splitting the Builder/race/deprecated-removal half into its own PR (with tests) would let that land quickly while the HttpStatus design settles.

[velo]

Thanks for the contribution. There's real value in here, but I can't merge it in this shape — please split it up and add tests.

Please split

The PR bundles five unrelated changes:

1. CancellableFuture.inner → AtomicReference (race fix)
2. Request.Options thread-to-method-options map (race fix)
3. Response.Builder field encapsulation
4. Removal of the deprecated RequestTemplate.resolve(Map, Map)
5. A brand-new public feign.HttpStatus enum + Logger.Level#atLeast

Each has a different risk profile and a different reviewer question. Bundled together they have to be accepted or rejected as a unit, which isn't going to happen.

No tests

207 added lines, zero test files. The two concurrency fixes (1 and 2) are the most valuable part of this PR and they are also the most testable — setMethodOptions/getMethodOptions under concurrent access, and CancellableFuture#cancel racing setInner. Please land those first, with tests that fail against the current code.

On HttpStatus

This adds a new public type to feign core, which is a permanent API commitment. As written it enumerates only the 19 codes the exception hierarchy happens to use, and from() returns null for everything else — so users who reach for it will find it half-complete and null-returning. If we want an HTTP status enum in core it needs to be a deliberate design decision (complete set, Optional or a throwing lookup, isRedirect/isSuccess, etc.), not a by-product of removing magic numbers from a switch. Separate PR, please.

Note that the FeignException switch also got slower along the way: HttpStatus.from(status) is a linear scan over values() (which allocates a fresh array each call) on every error response, replacing a constant-time tableswitch on the int.

On the deprecated RequestTemplate.resolve(Map, Map) removal

Deprecated-API removal is being handled as its own batch on the 14.x line. Please drop it from this PR.

Smaller notes

• Level#atLeast is a nice readability win and I'd take it — but it's public API on a public enum, so it wants a line of test coverage.
• The logResponseHeaders / rebufferBody extractions in Logger are fine and behaviour-preserving.

[saikat709]

@velo — thanks for the thorough review, really appreciate the detailed notes.

Agreed on all points. Here's what we'll do:

Splitting into two focused PRs:

PR 1 — Concurrency fixes (CancellableFuture + Options.setMethodOptions): Will add the missing isCancelled() re-check after inner.set(value) to close the window you flagged, and add proper concurrent-access tests for both fixes.

PR 2 — Logger + Response.Builder refactor: Level.atLeast(), the logResponseHeaders/rebufferBody extractions, and Response.Builder field encapsulation — with test coverage for atLeast(). Will also revert the 204/205 literals back from HttpStatus references since HttpStatus is being deferred.

On RequestTemplate.resolve(Map, Map): Will restore the deprecated overload — understood it belongs in the 14.x deprecation batch.

On HttpStatus: Agreed it needs to be a deliberate design decision rather than a by-product of magic-number removal. Will drop it from these PRs and revisit in a separate, properly scoped PR with a complete status set and non-null-returning lookup.

Will close this PR once the two replacements are up. Thanks again.

[saikat709]

Closing this PR to split the changes into two focused, well-tested PRs per feedback:

• Fix concurrency races in CancellableFuture and Options setMethodOptions #3552 — Fixes concurrency races in CancellableFuture and Options.setMethodOptions (with tests)
• Refactor Logger level checks and encapsulate Response.Builder fields #3553 — Refactors Logger level checks and encapsulates Response.Builder fields (with tests)

The HttpStatus enum addition and the removal of deprecated RequestTemplate.resolve have been dropped from these PRs.

@velo — Both PRs are ready for review when you have a moment!