Merge OData query params at boot time instead of via middleware#2
Closed
isama92 wants to merge 1 commit intosimplesquid:mainfrom
Closed
Merge OData query params at boot time instead of via middleware#2isama92 wants to merge 1 commit intosimplesquid:mainfrom
isama92 wants to merge 1 commit intosimplesquid:mainfrom
Conversation
The `HasODataQuery` trait previously merged OData query parameters into the PendingRequest via a request middleware registered at `PipeOrder::LAST`. However, Saloon's `DetermineMockResponse` middleware runs at normal priority (before LAST), so when using `Saloon::fake()` with callbacks that inspect `$pendingRequest->query()->all()` for URL matching, the OData params haven't been merged yet and the query appears empty. This moves the merge to happen directly during boot, which runs during PendingRequest construction before any middleware pipeline execution. All existing functionality (connector-level version resolution, `#[DefaultODataQuery]` attributes, constructor-time filter setup) is preserved since it all happens before or during boot. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Member
|
Thanks for flagging this @isama92! Merged the fix into main with an additional test for same-key overrides. Released in v0.1.1. |
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
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.
Problem
The
HasODataQuerytrait merges OData query parameters into the PendingRequest via a request middleware registered atPipeOrder::LAST. However, Saloon'sDetermineMockResponsemiddleware runs at normal priority (before LAST). When usingSaloon::fake()with a callback that inspects$pendingRequest->query()->all()to match URLs, the OData params haven't been merged yet — so the query appears empty and mock matching fails.Fix
Move the merge from a
PipeOrder::LASTmiddleware to a direct$pendingRequest->query()->merge()call during boot. Boot runs during PendingRequest construction (before the middleware pipeline), so the params are available by the timeDetermineMockResponseresolves the mock.All existing functionality is preserved:
#[UsesODataVersion]resolution#[DefaultODataQuery]attributes$this->odataQuery()->filter(...)All 120 existing tests pass.