Skip to content

Commit

Permalink
Add x-goog-request-params to request header. (#7440)
Browse files Browse the repository at this point in the history
* Add `x-goog-request-params` to request header.

* Create eighty-roses-appear.md

* Pretty

* Fix

* Pretty

* Pretty

* Pretty

* Failing CI fix (maybe)

* Fix test

* Fix test

* Complete solution

* Complete solution

* Update testing emulator to 1.18.1 (#7445)

* Attempt to make backward compatible with emulator.

* Pretty

* Pretty

* Improve changeset message.

* Encode values on 'x-goog-request-params' header.

* Try slightly older version  of emulator will pass auth test.

* Revert emulator change

---------

Co-authored-by: Mark Duckworth <1124037+MarkDuckworth@users.noreply.github.com>
  • Loading branch information
tom-andersen and MarkDuckworth authored Jul 17, 2023
1 parent 52b17b4 commit c5518c8
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 11 deletions.
5 changes: 5 additions & 0 deletions .changeset/eighty-roses-appear.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@firebase/firestore": patch
---

Fixed issue where count and lite API queries did not work with named databases.
27 changes: 18 additions & 9 deletions packages/firestore/src/remote/rest_connection.ts
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 @@ -54,7 +58,8 @@ function getGoogApiClientValue(): string {
export abstract class RestConnection implements Connection {
protected readonly databaseId: DatabaseId;
protected readonly baseUrl: string;
private readonly databaseRoot: string;
private readonly databasePath: string;
private readonly requestParams: string;

get shouldResourcePathBeIncludedInRequest(): boolean {
// Both `invokeRPC()` and `invokeStreamingRPC()` use their `path` arguments to determine
Expand All @@ -65,13 +70,14 @@ export abstract class RestConnection implements Connection {
constructor(private readonly databaseInfo: DatabaseInfo) {
this.databaseId = databaseInfo.databaseId;
const proto = databaseInfo.ssl ? 'https' : 'http';
const projectId = encodeURIComponent(this.databaseId.projectId);
const databaseId = encodeURIComponent(this.databaseId.database);
this.baseUrl = proto + '://' + databaseInfo.host;
this.databaseRoot =
'projects/' +
this.databaseId.projectId +
'/databases/' +
this.databaseId.database +
'/documents';
this.databasePath = `projects/${projectId}/databases/${databaseId}`;
this.requestParams =
this.databaseId.database === DEFAULT_DATABASE_NAME
? `project_id=${projectId}`
: `project_id=${projectId}&database_id=${databaseId}`;
}

invokeRPC<Req, Resp>(
Expand All @@ -85,7 +91,10 @@ 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 = {
'google-cloud-resource-prefix': this.databasePath,
'x-goog-request-params': this.requestParams
};
this.modifyHeadersForRequest(headers, authToken, appCheckToken);

return this.performRPCRequest<Req, Resp>(rpcName, url, headers, req).then(
Expand Down
8 changes: 6 additions & 2 deletions packages/firestore/test/unit/remote/rest_connection.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,9 @@ 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',
'google-cloud-resource-prefix': 'projects/testproject/databases/(default)'
});
});

Expand All @@ -111,7 +113,9 @@ 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',
'google-cloud-resource-prefix': 'projects/testproject/databases/(default)'
// Note: AppCheck token should not exist here.
});
});
Expand Down

0 comments on commit c5518c8

Please sign in to comment.