fix(admin): attach usage-tracking headers to Firestore requests#308
fix(admin): attach usage-tracking headers to Firestore requests#308demolaf wants to merge 7 commits into
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request ensures that Firestore requests carry the SDK's usage-tracking headers (X-Firebase-Client and X-Goog-Api-Client) by extracting them into a shared utility and applying them to both FirebaseUserAgentClient and Firestore settings. The review feedback highlights two important improvements: first, instead of only applying tracking headers when no custom headers exist, they should be merged with any user-provided headers to prevent them from being completely omitted; second, the shared headers should be defined as a final variable rather than a getter to avoid redundant map allocations and regex evaluations on every HTTP request.
Coverage Report✅ Coverage 75.53% meets 40% threshold Total Coverage: 75.53% Package Breakdown
Minimum threshold: 40% |
| request.headers['X-Firebase-Client'] = 'fire-admin-dart/$packageVersion'; | ||
| request.headers['X-Goog-Api-Client'] = | ||
| 'gl-dart/$dartVersion fire-admin/$packageVersion'; | ||
| request.headers.addAll(firebaseUserAgentHeaders); |
There was a problem hiding this comment.
Is this semantically different than what was there before?
| @@ -1,5 +1,6 @@ | |||
| ## 0.5.4-wip | |||
|
|
|||
| - Fixed `Firestore` requests not carrying the SDK's usage-tracking headers (`X-Firebase-Client`, `X-Goog-Api-Client`). | |||
There was a problem hiding this comment.
Is this meant to be wrapped at 80 columns.
2 participants
Firestorerequests never carried the SDK's usage-tracking headers (X-Firebase-Client,X-Goog-Api-Client) —FirestoreHttpClientbuilds its own HTTP client independently ofFirebaseApp.client/FirebaseUserAgentClient, so it was never wrapped.Uses the new
Settings.headersfrom #307 to attach the same headers, and extracts a sharedfirebaseUserAgentHeadershelper so the values aren't duplicated betweenFirebaseUserAgentClientandFirestore. Depends on #307 merging first.