Refine API Client architecture and request lifecycles - #11
Closed
binaryfire wants to merge 20 commits into
Closed
Conversation
Make API client instances reusable integration prototypes that create a fresh pending request for each operation. Separate API middleware from the underlying HTTP middleware surface, resolve middleware through Pipeline and the container, and use decorated forwarding so fluent and value-returning HTTP methods remain truthful. Install one retry-aware bridge, keep context and active request state operation-local, preserve configured-query omission semantics, reject unsupported async and custom-client paths before transport, and clear transient state on every exit.
Give API request middleware one coherent model for structured and raw data. Keep logical data, encoded bytes, media type, and framing synchronized after every mutation. Preserve GET and HEAD query ownership, dot-notation removal, seekable PSR streams, transfer encoding, custom JSON media types, and exact content lengths. Use throwing JSON encoding, reject unrepresentable conversions at the API boundary, and preserve wrapper attributes and decoded state during reconstruction.
Reconstruct API responses without losing cookies, transfer metadata, decoder state, decoding flags, or exception settings. Invalidate decoded state when middleware replaces the response body and define an explicit array conversion contract for array, empty, JSON null, malformed, scalar, and custom-decoder results. Keep raw response access available, forward response methods and macros through resources, preserve caller JSON flags, and make resource construction and serialization fully typed.
Keep exact logical request data until supported middleware or callbacks replace the prepared PSR body, then invalidate it before later callbacks, recorders, fakes, and API middleware observe the request. Gate stream-identity tracking behind configured Guzzle middleware and keep its internal option hidden from public callbacks and downstream handlers. Correct media-type parsing, empty and malformed JSON handling, recursive query normalization, numeric query keys, single-key removal, static subtype returns, raw-body option ownership, and transient request initialization.
Use the structured request option only while it remains the authoritative caller payload. Allow raw bodies and bodies replaced by supported request hooks to flow through Telescope\u0027s existing bounded PSR-body parser, while preserving the structured path for exact PHP payload values. Add coverage for raw JSON body capture without duplicating HTTP client body-ownership logic in the watcher.
Move the time-limit test helper reset into tearDown so PHPUnit restores process-local state even when an assertion or callback throws. Keep the test-owned cleanup at its owning boundary and avoid adding another framework-static cleanup registration.
Add max-level PHPStan fixtures for client and pending-request chains, all resource terminals, decorated value forwarding, resource narrowing, request mutation, and Arrayable contracts. Pin the concrete resource annotation and runtime property pairing so both direct client calls and explicitly created pending requests retain their integration-specific result type.
Declare the Guzzle, PSR-7, promises, and PSR HTTP interfaces imported directly by API Client, HTTP, and Support. Remove API Client\u0027s obsolete Engine dependency and keep root constraints authoritative for monorepo development. Add bounded metadata tests that compare split constraints with the root manifest instead of copying version literals into test expectations.
Declare direct Guzzle, PSR HTTP, Flysystem, URI, and Monolog owners in the framework split packages that import them. Advertise Faker and external search clients only where they enable concrete factory or testing features, and document the client packages required by the external search test traits. Extend package metadata coverage without pinning change-prone version literals.
Declare each direct Guzzle, promises, or PSR HTTP dependency used by Inertia, Socialite, and Telescope. Remove the unused API Client dependency from Reverb so its split manifest reflects the code it actually ships. Cover every corrected relationship with focused metadata tests and keep package ordering consistent with repository conventions.
Document API Client as an integration-focused sister to the HTTP client using Laravel-style prose and examples. Cover reusable client classes, optional typed configuration, pending requests, context, resources, errors, request and response middleware, structured and raw mutation, transport customization, fakes, and middleware lifetimes. Keep the package README minimal and add the guide to the Boost documentation index.
Record the final API Client architecture, public API decisions, lifecycle ownership, shared HTTP and Telescope boundaries, dependency corrections, testing strategy, and performance constraints. Mark every durable API Client and cross-package finding revalidated, close the package checklist entry, and add the signed-off ledger assessment. Retain the anti-overengineering rules and rejected alternatives needed to prevent future maintenance from reintroducing worker-shared state, callback growth, duplicate caches, or compatibility machinery.
…ctness-ergonomics
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis change revises the API Client lifecycle, middleware, request and response conversion, shared HTTP body handling, package dependencies, documentation, audit records, and regression coverage. ChangesAPI Client overhaul
Estimated code review effort: 5 (Critical) | ~90 minutes Sequence Diagram(s)sequenceDiagram
participant ApiClient
participant PendingRequest
participant HttpPendingRequest
participant ApiResponse
participant ApiResource
ApiClient->>PendingRequest: createPendingRequest()
PendingRequest->>HttpPendingRequest: send API request
HttpPendingRequest-->>PendingRequest: HTTP response
PendingRequest->>ApiResponse: createFrom(response)
ApiResponse-->>PendingRequest: converted response
PendingRequest->>ApiResource: make(response, request)
ApiResource-->>ApiClient: typed resource
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Collaborator
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
Greptile SummaryThe PR rebuilds API Client around operation-local pending requests and middleware while tightening shared HTTP request-state handling, Telescope payload reconstruction, package metadata, typing, tests, and documentation.
Confidence Score: 5/5The PR appears safe to merge because no blocking failure remains in the eligible follow-up review scope. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| src/api-client/src/ApiClient.php | Reworks the client into a boot-stable integration definition that creates and configures a fresh pending request for each operation. |
| src/api-client/src/PendingRequest.php | Adds operation-local middleware, retry-aware request bridging, context propagation, synchronous transport boundaries, and unconditional transient-state cleanup. |
| src/api-client/src/ApiRequest.php | Introduces coherent structured-data and raw-body mutation with media-type, framing, and stream synchronization. |
| src/api-client/src/ApiResponse.php | Preserves HTTP response metadata while adding API resource conversion and decoded-state invalidation after body replacement. |
| src/http/src/Client/PendingRequest.php | Refines query normalization, logical payload ownership, middleware body tracking, and request preparation across transport, fake, and recorder paths. |
| src/http/src/Client/Request.php | Extends request metadata so logical structured data remains available only while authoritative. |
| src/telescope/src/Watchers/ClientRequestWatcher.php | Uses authoritative structured metadata when available and otherwise parses the bounded PSR request body. |
| composer.json | Declares directly imported Guzzle promises and PSR-7 packages at the repository root. |
| src/api-client/composer.json | Updates split-package dependency ownership for the rebuilt API Client. |
| types/ApiClient/ApiClient.php | Updates the static-analysis surface to match the refined fluent client and typed-resource API. |
Reviews (3): Last reviewed commit: "docs(plans): record bodyless form behavi..." | Re-trigger Greptile
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (4)
tests/Broadcasting/PackageMetadataTest.php (1)
41-41: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winGuard manifest keys before comparing constraints. All three loops index
$composer['require'][$dependency]and$rootComposer['require'][$dependency]directly. If a split manifest omits a declared dependency, PHP raises an undefined-array-key error instead of a readable assertion failure. That hides the exact manifest defect these tests exist to detect.
tests/Broadcasting/PackageMetadataTest.php#L41-L41: addassertArrayHasKey($dependency, $composer['require'])inside the loop before theassertSameon line 42, so a missingguzzlehttp/guzzledeclaration reports the package name.tests/Contracts/PackageMetadataTest.php#L36-L36: add the sameassertArrayHasKeyguard inside the loop that now includespsr/http-message.tests/Foundation/PackageMetadataTest.php#L47-L54: add the sameassertArrayHasKeyguard inside the new loop that comparesguzzlehttp/guzzle,league/flysystem,league/uri, andmonolog/monolog.♻️ Proposed guard, shown for the Broadcasting loop
foreach (['guzzlehttp/guzzle', 'psr/log', 'symfony/http-kernel'] as $dependency) { + $this->assertArrayHasKey($dependency, $composer['require']); $this->assertSame($rootComposer['require'][$dependency], $composer['require'][$dependency]); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/Broadcasting/PackageMetadataTest.php` at line 41, Guard each dependency lookup with assertArrayHasKey before comparing constraints: in tests/Broadcasting/PackageMetadataTest.php lines 41-41, tests/Contracts/PackageMetadataTest.php lines 36-36, and tests/Foundation/PackageMetadataTest.php lines 47-54, add the guard inside the respective dependency loops for $composer['require']; preserve the existing assertions and dependency lists so missing declarations produce a readable package-specific failure.src/http/src/Client/PendingRequest.php (2)
740-740: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider normalizing the
head()query too.
get()now routes the query throughnormalizeQuery(), buthead()still forwards the raw value at Line 756. AStringableorArrayablequery passed tohead()therefore reaches Guzzle unnormalized. The declared parameter type ofhead()is narrower (array|string|null), so this is a consistency gap rather than a defect.♻️ Optional alignment for `head()`
- public function head(string $url, array|string|null $query = null): PromiseInterface|Response + public function head(string $url, Arrayable|array|JsonSerializable|string|null $query = null): PromiseInterface|Response { return $this->send( 'HEAD', $url, func_num_args() === 1 ? [] : [ - 'query' => $query, + 'query' => $this->normalizeQuery($query), ] ); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/http/src/Client/PendingRequest.php` at line 740, Update the head() method to pass its query argument through normalizeQuery(), matching the get() path and ensuring supported query values are normalized before reaching Guzzle.
1479-1509: 🗄️ Data Integrity & Integration | 🔵 Trivial | 💤 Low valueBody comparison uses object identity, so in-place stream writes stay undetected.
$request->getBody() !== $preparedBodycomparesStreamInterfaceinstances. PSR-7 mutation throughwithBody()creates a new stream, so the common cases are covered. A middleware that writes into the existing stream (for example$request->getBody()->write(...)) keeps the same instance, sohypervel_dataremains attached and then describes a body that no longer exists.This is an accepted trade-off if in-place mutation is out of scope. If you want to close it, record the stream size and pointer alongside the instance, or document the limitation next to
PREPARED_BODY_OPTION.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/http/src/Client/PendingRequest.php` around lines 1479 - 1509, Update buildPreparedBodyHandler and buildBeforeSendingHandler to detect in-place stream mutations by storing and comparing the prepared body’s relevant stream state, such as instance, size, and pointer position, before deciding whether to unset hypervel_data. Preserve the existing behavior for unchanged bodies and continue removing PREPARED_BODY_OPTION before invoking the handler.src/telescope/src/Watchers/ClientRequestWatcher.php (1)
154-158: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueThe documented behavior records a payload that can differ from the transmitted body.
The comment is accurate for this code path.
recordRequest()captures$optionsbefore the handler stack runs, so the unset ofhypervel_dataperformed byHypervel\Http\Client\PendingRequest::buildBeforeSendingHandler()does not reach this snapshot. As a result, Telescope shows the caller's original structured payload even when a before-sending callback replaced the body.Consider stating that consequence in the comment, so operators reading a Telescope entry know the payload reflects the caller's intent and not always the bytes on the wire.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/telescope/src/Watchers/ClientRequestWatcher.php` around lines 154 - 158, Update the documentation comment near recordRequest() to explicitly state that hypervel_data may reflect the caller’s original structured payload and can differ from the transmitted request body when before-sending callbacks replace it; retain the existing distinction for raw and third-party traffic parsed from the PSR-7 body.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/api-client/src/ApiResponse.php`:
- Around line 173-176: Update ApiResponse::offsetGet() to explicitly handle
offsets that are absent from the array, choosing the intended contract: return
null without emitting a PHP warning or throw a typed exception consistent with
the API. Preserve the existing toArray() error behavior and ensure
ApiResource::offsetGet() and ApiResource::__get() receive the defined result.
In `@src/api-client/src/PendingRequest.php`:
- Around line 303-376: The sendRequest/prepareClient flow must stop relying on
shared $this->activeRequest state, which is unsafe for concurrent reuse and can
be cleared before a resumed operation reads it. Pass the ApiRequest produced by
the beforeSending bridge through the request lifecycle or another
operation-local mechanism, then use that value to build the response resource
and remove the shared-state cleanup dependency; alternatively enforce and guard
a single-operation-only contract if the lifecycle cannot be made
operation-local.
In `@src/boost/docs/api-client.md`:
- Around line 252-258: Update the `query` method example to call `query` on the
already defined `$github` variable instead of `$client`, preserving the existing
request path and structured data.
In `@tests/ApiClient/ApiResponseTest.php`:
- Around line 129-137: Restore the original
HttpResponse::$defaultJsonDecodingFlags value around
testConfiguredThrowingJsonFlagsRemainObservable, using setUp/tearDown or a
try/finally that executes despite the expected JsonException. Ensure subsequent
ApiResponseTest cases, including testCreateFromPreservesHttpResponseState, see
the prior flag configuration.
---
Nitpick comments:
In `@src/http/src/Client/PendingRequest.php`:
- Line 740: Update the head() method to pass its query argument through
normalizeQuery(), matching the get() path and ensuring supported query values
are normalized before reaching Guzzle.
- Around line 1479-1509: Update buildPreparedBodyHandler and
buildBeforeSendingHandler to detect in-place stream mutations by storing and
comparing the prepared body’s relevant stream state, such as instance, size, and
pointer position, before deciding whether to unset hypervel_data. Preserve the
existing behavior for unchanged bodies and continue removing
PREPARED_BODY_OPTION before invoking the handler.
In `@src/telescope/src/Watchers/ClientRequestWatcher.php`:
- Around line 154-158: Update the documentation comment near recordRequest() to
explicitly state that hypervel_data may reflect the caller’s original structured
payload and can differ from the transmitted request body when before-sending
callbacks replace it; retain the existing distinction for raw and third-party
traffic parsed from the PSR-7 body.
In `@tests/Broadcasting/PackageMetadataTest.php`:
- Line 41: Guard each dependency lookup with assertArrayHasKey before comparing
constraints: in tests/Broadcasting/PackageMetadataTest.php lines 41-41,
tests/Contracts/PackageMetadataTest.php lines 36-36, and
tests/Foundation/PackageMetadataTest.php lines 47-54, add the guard inside the
respective dependency loops for $composer['require']; preserve the existing
assertions and dependency lists so missing declarations produce a readable
package-specific failure.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 0fe0f9a3-7534-4a51-ab4a-20dcb82a544b
📒 Files selected for processing (49)
composer.jsondocs/plans/2026-07-12-0900-framework-coroutine-state-lifecycle-audit.mddocs/plans/2026-07-12-0915-framework-coroutine-state-lifecycle-audit-ledger.mddocs/plans/2026-08-09-2105-api-client-correctness-laravel-ergonomics-and-lifecycles.mdsrc/api-client/README.mdsrc/api-client/composer.jsonsrc/api-client/src/ApiClient.phpsrc/api-client/src/ApiRequest.phpsrc/api-client/src/ApiResource.phpsrc/api-client/src/ApiResponse.phpsrc/api-client/src/Concerns/HasContext.phpsrc/api-client/src/Exceptions/InvalidResourceDataException.phpsrc/api-client/src/PendingRequest.phpsrc/boost/docs/api-client.mdsrc/boost/docs/documentation.mdsrc/boost/docs/testing.mdsrc/broadcasting/composer.jsonsrc/contracts/composer.jsonsrc/database/composer.jsonsrc/foundation/composer.jsonsrc/http/composer.jsonsrc/http/src/Client/PendingRequest.phpsrc/http/src/Client/Request.phpsrc/inertia/composer.jsonsrc/reverb/composer.jsonsrc/socialite/composer.jsonsrc/support/composer.jsonsrc/telescope/composer.jsonsrc/telescope/src/Watchers/ClientRequestWatcher.phptests/ApiClient/ApiClientTest.phptests/ApiClient/ApiRequestTest.phptests/ApiClient/ApiResourceTest.phptests/ApiClient/ApiResponseTest.phptests/ApiClient/PackageMetadataTest.phptests/ApiClient/PendingRequestTest.phptests/Broadcasting/PackageMetadataTest.phptests/Contracts/PackageMetadataTest.phptests/Database/PackageMetadataTest.phptests/Foundation/PackageMetadataTest.phptests/Http/HttpClientTest.phptests/Http/PackageMetadataTest.phptests/Inertia/PackageMetadataTest.phptests/Reverb/PackageMetadataTest.phptests/Socialite/PackageMetadataTest.phptests/Support/PackageMetadataTest.phptests/Telescope/PackageMetadataTest.phptests/Telescope/Watchers/ClientRequestWatcherTest.phptests/Testing/PHPUnit/TimeLimitTest.phptypes/ApiClient/ApiClient.php
💤 Files with no reviewable changes (1)
- src/reverb/composer.json
Normalize nested Stringable, JsonSerializable, and Arrayable values consistently for query, form, and multipart requests while keeping final wire options and logical request data under their correct owners. Preserve multipart-configured GET and HEAD query payloads, align HEAD with GET across HTTP and API Client types, and recover URL-embedded HEAD query data for assertions, fakes, and recorders. Add focused runtime and static-analysis coverage for valid nested values, rejected result shapes, logical and wire agreement, and the public exception contracts.
Keep array conversion strict for invalid response bodies while making property and array access return null when a successfully decoded resource does not contain the requested key. Cover direct response offsets and resource property access so absent fields follow familiar Laravel data-access behavior without hiding malformed or scalar response failures.
Require root and split dependency keys to exist before comparing their constraints so a missing entry cannot silently satisfy a null-to-null assertion. Apply the same direct presence contract across every affected package metadata test while retaining existing broader split checks and avoiding copied version literals or helper machinery.
Fix the query and raw-response examples, define the pending request concurrency boundary, and document null reads for fields missing from valid resource arrays. Clarify that Telescope's structured request payload comes from the original Guzzle options, so later before-sending body replacements can change transport bytes without rewriting the recorded caller intent.
Update the final design for recursive structured request normalization, split logical and wire ownership, multipart query preservation, GET and HEAD URL-query observation, and guarded metadata comparisons. Keep the completion and performance criteria aligned with the implemented code and the focused regressions that protect each corrected boundary.
Collaborator
Author
|
@coderabbitai Review follow-up is complete. Accepted and fixed:
Two suggestions did not result in runtime code:
The full repository formatter, static analysis, parallel test suite, Testbench suite, and dogfood package checks are green. The follow-up also received a complete final review after the last GET/HEAD correction. |
|
Tip For best results, initiate chat on the files or code changes.
The accepted findings are addressed. The stated The global after-test subscriber remains the authoritative owner for HTTP response static-state cleanup. A duplicate reset in an API Client test is not required. 🐇 ✅ ✏️ Learnings added
If you found this review helpful, would you consider giving us a shout-out on X? Thank you for using CodeRabbit! You are interacting with an AI system. |
Collaborator
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/http/src/Client/PendingRequest.php`:
- Around line 1212-1217: Update parseHttpOptions() handling of the form_params
key to allow null values for bodyless asForm() requests: skip the option or
normalize null to an empty array before the array validation. Preserve
validation and normalization for non-null form payloads so asForm()->get(),
asForm()->head(), and empty asForm()->delete() proceed without throwing.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 635f22a0-353d-4149-b1e0-ffeacd939b61
📒 Files selected for processing (32)
docs/plans/2026-08-09-2105-api-client-correctness-laravel-ergonomics-and-lifecycles.mdsrc/api-client/src/ApiResponse.phpsrc/api-client/src/PendingRequest.phpsrc/boost/docs/api-client.mdsrc/http/src/Client/PendingRequest.phpsrc/support/src/Facades/Http.phpsrc/telescope/src/Watchers/ClientRequestWatcher.phptests/ApiClient/ApiResourceTest.phptests/ApiClient/ApiResponseTest.phptests/ApiClient/PackageMetadataTest.phptests/ApiClient/PendingRequestTest.phptests/Broadcasting/PackageMetadataTest.phptests/Concurrency/PackageMetadataTest.phptests/Contracts/PackageMetadataTest.phptests/Di/PackageMetadataTest.phptests/Foundation/PackageMetadataTest.phptests/Http/HttpClientTest.phptests/Http/PackageMetadataTest.phptests/Inertia/PackageMetadataTest.phptests/Mail/PackageMetadataTest.phptests/Notifications/PackageMetadataTest.phptests/Passkeys/PackageMetadataTest.phptests/Permission/PackageMetadataTest.phptests/Process/PackageMetadataTest.phptests/Routing/PackageMetadataTest.phptests/Scout/PackageMetadataTest.phptests/Socialite/PackageMetadataTest.phptests/Support/PackageMetadataTest.phptests/Telescope/PackageMetadataTest.phptests/Testing/PackageMetadataTest.phptests/View/PackageMetadataTest.phptypes/ApiClient/ApiClient.php
🚧 Files skipped from review as they are similar to previous changes (13)
- tests/Contracts/PackageMetadataTest.php
- tests/Broadcasting/PackageMetadataTest.php
- tests/Socialite/PackageMetadataTest.php
- src/telescope/src/Watchers/ClientRequestWatcher.php
- tests/Inertia/PackageMetadataTest.php
- tests/Support/PackageMetadataTest.php
- src/api-client/src/ApiResponse.php
- tests/Http/PackageMetadataTest.php
- tests/Foundation/PackageMetadataTest.php
- tests/Telescope/PackageMetadataTest.php
- src/api-client/src/PendingRequest.php
- docs/plans/2026-08-09-2105-api-client-correctness-laravel-ergonomics-and-lifecycles.md
- tests/ApiClient/ApiResponseTest.php
Treat a directly null form_params option as the framework-planted absence of a payload while continuing to reject caller-supplied structured values that normalize to null or another non-array shape. Restore bodyless GET, queried GET, HEAD, and empty DELETE behavior for the HTTP client and API Client integrations configured with asForm, with focused regressions at both public surfaces.
Document the distinction between a directly null form option planted for a bodyless request and caller-supplied structured form data that resolves to null. Extend the verification matrix to cover bodyless form-configured HTTP and API Client read and delete operations.
Collaborator
Author
|
@coderabbitai The additional form-request finding is resolved. The root cause was the new strict form validator treating a null body-format option planted by the framework as caller-supplied data. The fix preserves that direct null unchanged, which restores the previous Guzzle behavior without manufacturing a request body. Structured values that resolve to null or another non-array shape remain invalid. Coverage now includes bodyless GET, GET with query data, HEAD, and empty DELETE at the HTTP client boundary and through an API Client whose integration configuration applies asForm. The null-versus-normalized-null distinction is also pinned so a later cleanup cannot move the guard below structured normalization. The correction passed focused formatting, static analysis, and HTTP/API Client tests, followed by final review sign-off. |
|
Tip For best results, initiate chat on the files or code changes.
The direct framework-provided 🐇 ✅ ✏️ Learnings added
You are interacting with an AI system. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR rebuilds API Client around reusable integration definitions and fresh pending operations. The public surface now follows the same fluent conventions as the HTTP client while adding the API-specific pieces integrations need: typed resources, operation context, and request and response middleware.
The change also fixes the shared HTTP request-state defects exposed by the API Client work, makes split-package dependency ownership accurate, and adds first-party documentation.
API Client design
API client classes now hold only boot-stable integration defaults. Each call creates a fresh pending request, applies the client defaults through
configurePendingRequest, and keeps all mutable request state local to that operation.The new surface:
withResourcenarrowing.Typed data objects remain available for client configuration, but they are optional constructor dependencies rather than a base-class requirement.
Request and response correctness
API request mutation now has one coherent owner for structured data, raw bodies, media types, and framing. JSON and form mutations keep logical data, encoded bytes, content type, and content length synchronized. Raw and multipart bodies fail at the conversion boundary instead of producing stale or invalid request state. Replacement streams are seekable and preserve transfer-encoding rules.
API response reconstruction preserves cookies, transfer metadata, decoder state, flags, and exception settings. Replacing a response body invalidates the old decoded value. Resource array conversion now distinguishes arrays, empty bodies, JSON null, malformed JSON, scalar JSON, and custom decoders while leaving raw response access available.
The shared HTTP client changes:
JsonSerializablequery values recursively;Telescope now uses structured request metadata only while it remains authoritative and falls back to its bounded PSR-body parser for raw or replaced bodies.
Explicit transport boundaries
API Client remains synchronous.
async(true)is rejected before dispatch instead of performing work that cannot produce a valid API resource. Replacing the complete Guzzle client is also rejected because it bypasses the required API bridge.setHandlerremains the supported low-level transport seam.HTTP errors remain non-throwing by default, matching the HTTP client. Applications may opt into exception behavior with
throw.Package ownership
The split manifests now declare the Guzzle, PSR HTTP, Flysystem, URI, CommonMark, Monolog, and promises packages imported directly by their source. Optional search clients and Faker are suggestions only where they enable documented features. Reverb no longer declares an unused API Client dependency.
Metadata tests compare split constraints with the root manifest instead of copying version literals that drift during routine dependency updates.
Performance
The design removes worker-shared middleware mutation, package-local middleware caching, option staging, callback growth, and stale-state repair paths. Operation context uses PHP copy-on-write arrays. Stateless unbound middleware continues to use normal worker-lifetime container reuse. Prepared-body tracking exists only when Guzzle middleware is configured and compares stream identity without parsing the body.
No request path gains locks, serialization, polling, extra network calls, unbounded caches, or retained request graphs.
Documentation and validation
A new Laravel-style API Client guide covers integration classes, typed configuration, pending requests, context, resources, errors, middleware, body mutation, transport customization, and HTTP fakes. The package README points to this single documentation source.
Focused package and shared-owner suites, strict Composer validation, formatting, static analysis, the complete parallel suite, Testbench package mode, documentation examples, and final diff checks are green.
For more details, see:
docs/plans/2026-08-09-2105-api-client-correctness-laravel-ergonomics-and-lifecycles.md.Summary by CodeRabbit