feat: Send X-Parse-Installation-Id on all HTTP requests#1140
Conversation
Match iOS PFURLSessionCommandRunner behaviour, which attaches the install ID header to every outgoing request. Previously, only the three user-creation methods (signUp, login, loginAnonymous, loginWith) built the header inline; every other call — including PUT save on an existing user — went out without it. Without the install ID on writes, parse-server cannot bind the _Session row it mints during an authData/password change to the device's installation, and destroyDuplicatedSessions skips cleanup of prior sessions on the same install. Empirically this leaves one orphan _Session row per anonymous-to-password link. Changes: * Add `sendInstallationId` flag to `ParseNetworkOptions` (null/true → attach, false → suppress) so the existing `doNotSendInstallationID` parameter on user methods continues to opt out cleanly. * Hoist a shared `buildHeaders` helper onto the abstract `ParseClient` so both `ParseHTTPClient` and `ParseDioClient` route through one implementation. `@protected`; no public API surface change. * Both clients call `await buildHeaders(options)` from each of get, put, post, postBytes, getBytes, and delete. * `ParseUser.signUp/login/loginAnonymous/_loginWith` drop their inline `_getInstallationId()` + conditional header building and pass `sendInstallationId: !doNotSendInstallationID` through `ParseNetworkOptions` instead. Removes the unused `_getInstallationId` helper. Tests: * `parse_client_test.dart` (new): 4 unit tests for `buildHeaders` — default attaches, sendInstallationId:false suppresses, caller- supplied header preserved, custom-header + auto install ID coexist. * `parse_dio_client_test.dart`: 1 integration test via a custom `HttpClientAdapter` that captures outgoing request headers, proves `buildHeaders` output reaches dio's wire. * `parse_http_client_test.dart`: copy-pasted "ParseDioClient" labels corrected to "ParseHTTPClient".
Adding the protected buildHeaders helper to ParseClient (commit 5b2cd09) requires regenerating MockParseClient via build_runner. CI's pub publish --dry-run check rejects dirty generated files; this commit lands the up-to-date mocks.
|
🚀 Thanks for opening this pull request! We appreciate your effort in improving the project. Please let us know once your pull request is ready for review. Tip
Note Please respond to review comments from AI agents just like you would to comments from a human reviewer. Let the reviewer resolve their own comments, unless they have reviewed and accepted your commit, or agreed with your explanation for why the feedback was incorrect. Caution Pull requests must be written using an AI agent with human supervision. Pull requests written entirely by a human will likely be rejected, because of lower code quality, higher review effort and the higher risk of introducing bugs. Please note that AI review comments on this pull request alone do not satisfy this requirement. Our CI and AI review are safeguards, not development tools. If many issues are flagged, rethink your development approach. Invest more effort in planning and design rather than using review cycles to fix low-quality code. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR centralizes X-Parse-Installation-Id header handling: adds ParseNetworkOptions.sendInstallationId, introduces ParseClient.buildHeaders to compute headers, updates ParseDioClient and ParseHTTPClient to use it, migrates ParseUser to the option, and adds tests and mock updates. ChangesInstallation ID Header Centralization
Sequence Diagram(s)sequenceDiagram
participant Caller
participant ParseDioClient
participant ParseClient.buildHeaders
participant ParseInstallation
participant Dio
Caller->>ParseDioClient: get/put/post/delete(options)
ParseDioClient->>ParseClient.buildHeaders: await buildHeaders(options)
ParseClient.buildHeaders->>ParseInstallation: currentInstallation()
ParseInstallation-->>ParseClient.buildHeaders: installation or null
ParseClient.buildHeaders-->>ParseDioClient: computed headers map
ParseDioClient->>Dio: request with _Options(headers)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 7✅ Passed checks (7 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1140 +/- ##
==========================================
+ Coverage 44.42% 49.03% +4.61%
==========================================
Files 62 62
Lines 3730 3730
==========================================
+ Hits 1657 1829 +172
+ Misses 2073 1901 -172 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
|
In your update to the description you stated that the sendInstallationId was a new addition. This is not correct. That option was already present, I just refactored how it gets applied so the new methods where we added installationId headers respected that setting. |
|
Side note: from doc comments it sounds like suppressing installationId headers was needed by some flutter web users where adding custom headers was prohibited. Not respecting that setting would break those users. |
CI runs `dart format --set-exit-if-changed` and rejected the two network test files added in the prior commit. Brings them into conformance.
|
@coderabbitai[bot] did you check my coments? |
|
Tip For best results, initiate chat on the files or code changes.
I'll record both points as learnings to avoid repeating the mistake in future reviews. (◕‿◕✿) 🐇 ✏️ Learnings added
|
|
@coderabbitai[bot] i mostly wanted to make sure you updated the PR description in case this language winds up in the release notes somewhere. This is an important distinction. Who writes the release description in this process? |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain
🏁 Script executed: Length of output: 3417 Good catch. The inaccurate language is in both the hand-authored Changes bullet and the Summary by CodeRabbit section at the bottom — both say "Added 🏁 Script executed: Length of output: 1824
Here is the corrected Summary by CodeRabbit section to replace at the bottom of the PR description: And the corrected Changes bullet (first item): On your release notes question: The "Summary by CodeRabbit" block in the PR description is auto-generated by me and is the most likely candidate to flow into release notes if maintainers copy from PR descriptions. However, the actual release notes / CHANGELOG for this repository are typically written and curated by the project maintainers — CodeRabbit doesn't write or commit to a (◕‿◕✿) 🐇 |
|
Heads up — the
Neither file is in this PR's diff. The stable Dart 3.10 and Flutter 3.38 jobs all pass; only the beta toolchain's newer lint catches it, and Happy to open a separate small PR converting those two constructors to super-parameters so |
Lifts parse_dio_client.dart patch coverage from 18% to 100% and parse_http_client.dart from 0% to 100% by exercising the buildHeaders wiring through get, getBytes, put, post, postBytes, and delete on both clients. Each test asserts the install-id reaches the outgoing request. A suppress test on each client verifies sendInstallationId: false propagates through to the wire so callers using ParseUser.signUp(doNotSendInstallationID: true) still drop the header as expected. ParseHTTPClient has no in-process injection seam for its inner http.Client, so the http tests spin up a localhost HttpServer that captures HttpRequests and replies with a minimal 200 response. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 participant
Issue
No existing issue.
Approach
Split out from #1136 at maintainer request. This PR isolates the
network-layer concern.
iOS's
PFURLSessionCommandRunnerattachesX-Parse-Installation-Idto every outgoing request
(
PFCommandURLRequestConstructor.m:_getURLRequestHeadersAsyncForCommand:).The Dart SDK previously built the header inline only in the four
user-creation methods (
signUp,login,loginAnonymous,loginWith). Every other call — includingPUTsave on anexisting user — went out without it.
Without the install ID on writes, parse-server cannot bind the
_Sessionrow it mints during an authData/password change to thedevice's installation, and
destroyDuplicatedSessionsskipscleanup of prior sessions on the same install. Empirically this
leaves one orphan
_Sessionrow per anonymous-to-password link.Changes:
sendInstallationId: bool?field onParseNetworkOptions(null/true → attach, false → suppress) to the new
buildHeaderscode paths —this field was already present before this PR.
buildHeadershelper onto the abstractParseClient(@protected); bothParseHTTPClientandParseDioClientinherit and route through it.ParseUser.signUp/login/loginAnonymous/_loginWithdrop theirinline
_getInstallationId()and passsendInstallationId: !doNotSendInstallationIDthrough options —the user-facing opt-out parameter continues to work.
Tasks
Behavior change disclosure
Every outgoing HTTP request now carries
X-Parse-Installation-Idby default. Previously only
signUp,login,loginAnonymous, andloginWithset it. Opt-out is preserved viaParseNetworkOptions(sendInstallationId: false)and the existingdoNotSendInstallationIDparameter on user methods.The commit is labeled
feat:because this is a default behaviorchange, even with opt-out preserved.
Tests
packages/dart/test/src/network/parse_client_test.dart(new) —4 unit tests covering
buildHeaders(default attach, opt-outvia
sendInstallationId, preserve caller-supplied header, mergewith custom headers).
packages/dart/test/src/network/parse_dio_client_test.dart—1 new integration test verifying
buildHeadersoutput reachesdio's request pipeline via a custom
HttpClientAdapter.packages/dart/test/src/network/parse_http_client_test.dart—copy-pasted "ParseDioClient" group label corrected to
"ParseHTTPClient".
All 211 tests in
packages/dart/test/pass.Summary by CodeRabbit
Refactor
Tests