Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add x-goog-request-params to request header. #7440

Merged
merged 22 commits into from
Jul 17, 2023
Merged

Add x-goog-request-params to request header. #7440

Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
adc145c
Add `x-goog-request-params` to request header.
tom-andersen Jul 11, 2023
ca5694e
Create eighty-roses-appear.md
tom-andersen Jul 11, 2023
19afe7b
Pretty
tom-andersen Jul 11, 2023
23d05f9
Fix
tom-andersen Jul 11, 2023
04fdf00
Pretty
tom-andersen Jul 11, 2023
924dfc6
Pretty
tom-andersen Jul 11, 2023
1b79205
Pretty
tom-andersen Jul 11, 2023
3fed99b
Failing CI fix (maybe)
tom-andersen Jul 11, 2023
8958fa3
Fix test
tom-andersen Jul 11, 2023
0034772
Fix test
tom-andersen Jul 11, 2023
d625eb5
Complete solution
tom-andersen Jul 11, 2023
b60b73d
Complete solution
tom-andersen Jul 11, 2023
ea0a70e
Update testing emulator to 1.18.1 (#7445)
MarkDuckworth Jul 12, 2023
8a31353
Attempt to make backward compatible with emulator.
tom-andersen Jul 12, 2023
6eb76d3
Pretty
tom-andersen Jul 12, 2023
f26c712
Pretty
tom-andersen Jul 12, 2023
c2e17fc
Merge branch 'master' of https://github.com/firebase/firebase-js-sdk …
tom-andersen Jul 13, 2023
614e23d
Improve changeset message.
tom-andersen Jul 13, 2023
0457b63
Encode values on 'x-goog-request-params' header.
tom-andersen Jul 13, 2023
513e0bb
Merge remote-tracking branch 'origin/master' into tomandersen/x-goog-…
tom-andersen Jul 17, 2023
4e616b8
Try slightly older version of emulator will pass auth test.
tom-andersen Jul 17, 2023
a61a8e6
Revert emulator change
tom-andersen Jul 17, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/eighty-roses-appear.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@firebase/firestore": patch
---

Add `x-goog-request-params` to REST request header.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you think about release notes stating what is fixed in terms of what the customer understands?

For example this could be "Fixed issue where count and lite API queries did not work with named databases."

13 changes: 11 additions & 2 deletions packages/firestore/src/remote/rest_connection.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,11 @@

import { SDK_VERSION } from '../../src/core/version';
import { Token } from '../api/credentials';
import { DatabaseId, DatabaseInfo } from '../core/database_info';
import {
DatabaseId,
DatabaseInfo,
DEFAULT_DATABASE_NAME
} from '../core/database_info';
import { debugAssert } from '../util/assert';
import { generateUniqueDebugId } from '../util/debug_uid';
import { FirestoreError } from '../util/error';
Expand Down Expand Up @@ -85,7 +89,12 @@ export abstract class RestConnection implements Connection {
const url = this.makeUrl(rpcName, path);
logDebug(LOG_TAG, `Sending RPC '${rpcName}' ${streamId}:`, url, req);

const headers = {};
const headers: StringMap = {
'x-goog-request-params':
this.databaseId.database === DEFAULT_DATABASE_NAME
? `project_id=${this.databaseId.projectId}`
: `project_id=${this.databaseId.projectId}&database_id=${this.databaseId.database}`
dconeybe marked this conversation as resolved.
Show resolved Hide resolved
};
this.modifyHeadersForRequest(headers, authToken, appCheckToken);

return this.performRPCRequest<Req, Resp>(rpcName, url, headers, req).then(
Expand Down
6 changes: 4 additions & 2 deletions packages/firestore/test/unit/remote/rest_connection.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,8 @@ describe('RestConnection', () => {
'Content-Type': 'text/plain',
'X-Firebase-GMPID': 'test-app-id',
'X-Goog-Api-Client': `gl-js/ fire/${SDK_VERSION}`,
'x-firebase-appcheck': 'some-app-check-token'
'x-firebase-appcheck': 'some-app-check-token',
'x-goog-request-params': 'project_id=testproject'
});
});

Expand All @@ -111,7 +112,8 @@ describe('RestConnection', () => {
expect(connection.lastHeaders).to.deep.equal({
'Content-Type': 'text/plain',
'X-Firebase-GMPID': 'test-app-id',
'X-Goog-Api-Client': `gl-js/ fire/${SDK_VERSION}`
'X-Goog-Api-Client': `gl-js/ fire/${SDK_VERSION}`,
'x-goog-request-params': 'project_id=testproject'
// Note: AppCheck token should not exist here.
});
});
Expand Down
4 changes: 2 additions & 2 deletions scripts/emulator-testing/emulators/firestore-emulator.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,11 @@ export class FirestoreEmulator extends Emulator {

constructor(port: number, projectId = 'test-emulator') {
super(
'cloud-firestore-emulator-v1.14.4.jar',
'cloud-firestore-emulator-v1.18.1.jar',
// Use locked version of emulator for test to be deterministic.
// The latest version can be found from firestore emulator doc:
// https://firebase.google.com/docs/firestore/security/test-rules-emulator
'https://storage.googleapis.com/firebase-preview-drop/emulator/cloud-firestore-emulator-v1.14.4.jar',
'https://storage.googleapis.com/firebase-preview-drop/emulator/cloud-firestore-emulator-v1.18.1.jar',
port
);
this.projectId = projectId;
Expand Down