From bc90c396cdda21a5ba507570758d1ff6a86d7938 Mon Sep 17 00:00:00 2001 From: Maneesh Tewani Date: Fri, 6 Sep 2024 17:40:13 -0700 Subject: [PATCH 1/5] Sent gmpid via headers --- config/mocharc.node.js | 2 +- packages/data-connect/src/api/DataConnect.ts | 1 + packages/data-connect/src/network/fetch.ts | 5 ++ .../src/network/transport/index.ts | 1 + .../src/network/transport/rest.ts | 3 + packages/data-connect/test/unit/fetch.test.ts | 4 +- packages/data-connect/test/unit/gmpid.test.ts | 80 +++++++++++++++++++ .../data-connect/test/unit/queries.test.ts | 6 +- 8 files changed, 96 insertions(+), 6 deletions(-) create mode 100644 packages/data-connect/test/unit/gmpid.test.ts diff --git a/config/mocharc.node.js b/config/mocharc.node.js index e493fce4d52..219256f9370 100644 --- a/config/mocharc.node.js +++ b/config/mocharc.node.js @@ -25,7 +25,7 @@ const config = { require: 'ts-node/register', timeout: 5000, - retries: 5, + // retries: 5, exit: true }; diff --git a/packages/data-connect/src/api/DataConnect.ts b/packages/data-connect/src/api/DataConnect.ts index 9393b9ef4e4..27ab83660fd 100644 --- a/packages/data-connect/src/api/DataConnect.ts +++ b/packages/data-connect/src/api/DataConnect.ts @@ -160,6 +160,7 @@ export class DataConnect { this._transport = new this._transportClass( this.dataConnectOptions, this.app.options.apiKey, + this.app.options.appId, this._authTokenProvider, this._appCheckTokenProvider, undefined, diff --git a/packages/data-connect/src/network/fetch.ts b/packages/data-connect/src/network/fetch.ts index fbd930977ab..95196a4e2cc 100644 --- a/packages/data-connect/src/network/fetch.ts +++ b/packages/data-connect/src/network/fetch.ts @@ -34,6 +34,7 @@ export function dcFetch( url: string, body: U, { signal }: AbortController, + appId: string | null, accessToken: string | null, appCheckToken: string | null, _isUsingGen: boolean @@ -48,6 +49,10 @@ export function dcFetch( if (accessToken) { headers['X-Firebase-Auth-Token'] = accessToken; } + console.log(appId); + if(appId) { + headers['x-firebase-gmpid'] = appId; + } if (appCheckToken) { headers['X-Firebase-AppCheck'] = appCheckToken; } diff --git a/packages/data-connect/src/network/transport/index.ts b/packages/data-connect/src/network/transport/index.ts index 8ccbc88f435..5518faa0f95 100644 --- a/packages/data-connect/src/network/transport/index.ts +++ b/packages/data-connect/src/network/transport/index.ts @@ -45,6 +45,7 @@ export interface CancellableOperation extends PromiseLike<{ data: T }> { export type TransportClass = new ( options: DataConnectOptions, apiKey?: string, + appId?: string, authProvider?: AuthTokenProvider, appCheckProvider?: AppCheckTokenProvider, transportOptions?: TransportOptions, diff --git a/packages/data-connect/src/network/transport/rest.ts b/packages/data-connect/src/network/transport/rest.ts index aaaf22abd64..85847868c5d 100644 --- a/packages/data-connect/src/network/transport/rest.ts +++ b/packages/data-connect/src/network/transport/rest.ts @@ -39,6 +39,7 @@ export class RESTTransport implements DataConnectTransport { constructor( options: DataConnectOptions, private apiKey?: string | undefined, + private appId?: string, private authProvider?: AuthTokenProvider | undefined, private appCheckProvider?: AppCheckTokenProvider | undefined, transportOptions?: TransportOptions | undefined, @@ -175,6 +176,7 @@ export class RESTTransport implements DataConnectTransport { variables: body } as unknown as U, // TODO(mtewani): This is a patch, fix this. abortController, + this.appId, this._accessToken, this._appCheckToken, this._isUsingGen @@ -203,6 +205,7 @@ export class RESTTransport implements DataConnectTransport { variables: body } as unknown as U, abortController, + this.appId, this._accessToken, this._appCheckToken, this._isUsingGen diff --git a/packages/data-connect/test/unit/fetch.test.ts b/packages/data-connect/test/unit/fetch.test.ts index f0d7f38ee8c..22c0693ff0e 100644 --- a/packages/data-connect/test/unit/fetch.test.ts +++ b/packages/data-connect/test/unit/fetch.test.ts @@ -40,7 +40,7 @@ describe('fetch', () => { message }); await expect( - dcFetch('http://localhost', {}, {} as AbortController, null, null, false) + dcFetch('http://localhost', {}, {} as AbortController, null, null, null, false) ).to.eventually.be.rejectedWith(message); }); it('should throw a stringified message when the server responds with an error without a message property in the body', async () => { @@ -51,7 +51,7 @@ describe('fetch', () => { }; mockFetch(json); await expect( - dcFetch('http://localhost', {}, {} as AbortController, null, null, false) + dcFetch('http://localhost', {}, {} as AbortController, null, null, null, false) ).to.eventually.be.rejectedWith(JSON.stringify(json)); }); }); diff --git a/packages/data-connect/test/unit/gmpid.test.ts b/packages/data-connect/test/unit/gmpid.test.ts new file mode 100644 index 00000000000..f452c2948bb --- /dev/null +++ b/packages/data-connect/test/unit/gmpid.test.ts @@ -0,0 +1,80 @@ +/** + * @license + * Copyright 2024 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { deleteApp, initializeApp, FirebaseApp } from '@firebase/app'; +import { expect, use } from 'chai'; +import * as sinon from 'sinon'; +import sinonChai from 'sinon-chai'; + +import { DataConnect, executeQuery, getDataConnect, queryRef } from '../../src'; +import { initializeFetch } from '../../src/network/fetch'; + +use(sinonChai); +const json = { + message: 'unauthorized' +}; +const fakeFetchImpl = sinon.stub().returns( + Promise.resolve({ + json: () => { + return Promise.resolve(json); + }, + status: 401 + } as Response) +); + +describe('GMPID Tests', () => { + let dc: DataConnect; + let app: FirebaseApp; + let APPID = 'MYAPPID'; + beforeEach(() => { + initializeFetch(fakeFetchImpl); + app = initializeApp({ projectId: 'p',appId: APPID }, 'fdsasdf'); // TODO(mtewani): Replace with util function + dc = getDataConnect(app, { connector: 'c', location: 'l', service: 's' }); + }); + afterEach(async () => { + await dc._delete(); + await deleteApp(app); + }); + it('should send a request with the corresponding gmpid if using the app id is specified', async () => { + // @ts-ignore + await executeQuery(queryRef(dc, '')).catch(() => {}); + expect(fakeFetchImpl).to.be.calledWithMatch( + 'https://firebasedataconnect.googleapis.com/v1alpha/projects/p/locations/l/services/s/connectors/c:executeQuery', + { + headers: { + ['x-firebase-gmpid']: APPID + } + } + ); + }); + it('should send a request with no gmpid if using the app id is not specified', async () => { + const app2 = initializeApp({ projectId: 'p' }, 'def'); // TODO(mtewani): Replace with util function + const dc2 = getDataConnect(app2, { connector: 'c', location: 'l', service: 's' }); + // @ts-ignore + await executeQuery(queryRef(dc2, '')).catch(() => {}); + expect(fakeFetchImpl).to.be.calledWithMatch( + 'https://firebasedataconnect.googleapis.com/v1alpha/projects/p/locations/l/services/s/connectors/c:executeQuery', + { + headers: { + ['x-firebase-gmpid']: APPID + } + } + ); + dc2._delete(); + deleteApp(app2); + }); +}); diff --git a/packages/data-connect/test/unit/queries.test.ts b/packages/data-connect/test/unit/queries.test.ts index 444601bd5ea..68bd96268a6 100644 --- a/packages/data-connect/test/unit/queries.test.ts +++ b/packages/data-connect/test/unit/queries.test.ts @@ -68,7 +68,7 @@ describe('Queries', () => { it('[QUERY] should retry auth whenever the fetcher returns with unauthorized', async () => { initializeFetch(fakeFetchImpl); const authProvider = new FakeAuthProvider(); - const rt = new RESTTransport(options, undefined, authProvider); + const rt = new RESTTransport(options, undefined, undefined, authProvider); await expect(rt.invokeQuery('test', null)).to.eventually.be.rejectedWith( json.message ); @@ -77,7 +77,7 @@ describe('Queries', () => { it('[MUTATION] should retry auth whenever the fetcher returns with unauthorized', async () => { initializeFetch(fakeFetchImpl); const authProvider = new FakeAuthProvider(); - const rt = new RESTTransport(options, undefined, authProvider); + const rt = new RESTTransport(options, undefined, undefined, authProvider); await expect(rt.invokeMutation('test', null)).to.eventually.be.rejectedWith( json.message ); @@ -86,7 +86,7 @@ describe('Queries', () => { it("should not retry auth whenever the fetcher returns with unauthorized and the token doesn't change", async () => { initializeFetch(fakeFetchImpl); const authProvider = new FakeAuthProvider(); - const rt = new RESTTransport(options, undefined, authProvider); + const rt = new RESTTransport(options, undefined, undefined, authProvider); rt._setLastToken('initial token'); await expect( rt.invokeQuery('test', null) as Promise From efabed990e7749f4d5793d687e0b48fcabe8b3de Mon Sep 17 00:00:00 2001 From: Maneesh Tewani Date: Fri, 6 Sep 2024 21:21:27 -0700 Subject: [PATCH 2/5] Removed console.log --- packages/data-connect/src/network/fetch.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/data-connect/src/network/fetch.ts b/packages/data-connect/src/network/fetch.ts index 95196a4e2cc..7b4d3ad33f1 100644 --- a/packages/data-connect/src/network/fetch.ts +++ b/packages/data-connect/src/network/fetch.ts @@ -49,7 +49,6 @@ export function dcFetch( if (accessToken) { headers['X-Firebase-Auth-Token'] = accessToken; } - console.log(appId); if(appId) { headers['x-firebase-gmpid'] = appId; } From 961cf54728c65663a3a8e75fef691e320a3d6ecf Mon Sep 17 00:00:00 2001 From: Maneesh Tewani Date: Fri, 6 Sep 2024 21:21:48 -0700 Subject: [PATCH 3/5] Removed unnecessary file write in seeder --- packages/data-connect/test/emulatorSeeder.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/data-connect/test/emulatorSeeder.ts b/packages/data-connect/test/emulatorSeeder.ts index df7071a5868..36cdf691169 100644 --- a/packages/data-connect/test/emulatorSeeder.ts +++ b/packages/data-connect/test/emulatorSeeder.ts @@ -82,7 +82,6 @@ export async function setupQueries( connection_string: 'postgresql://postgres:secretpassword@localhost:5432/postgres?sslmode=disable' }; - fs.writeFileSync('./emulator.json', JSON.stringify(toWrite)); return fetch(`http://localhost:${EMULATOR_PORT}/setupSchema`, { method: 'POST', body: JSON.stringify(toWrite) From da31e421e7bea7247d9e5fc07f42160db96902e7 Mon Sep 17 00:00:00 2001 From: Maneesh Tewani Date: Thu, 19 Sep 2024 23:29:41 +0100 Subject: [PATCH 4/5] Updated tests --- packages/data-connect/src/network/fetch.ts | 2 +- packages/data-connect/test/unit/fetch.test.ts | 20 +++++++++++++++++-- packages/data-connect/test/unit/gmpid.test.ts | 14 ++++++++----- 3 files changed, 28 insertions(+), 8 deletions(-) diff --git a/packages/data-connect/src/network/fetch.ts b/packages/data-connect/src/network/fetch.ts index 7b4d3ad33f1..928b9f873cf 100644 --- a/packages/data-connect/src/network/fetch.ts +++ b/packages/data-connect/src/network/fetch.ts @@ -49,7 +49,7 @@ export function dcFetch( if (accessToken) { headers['X-Firebase-Auth-Token'] = accessToken; } - if(appId) { + if (appId) { headers['x-firebase-gmpid'] = appId; } if (appCheckToken) { diff --git a/packages/data-connect/test/unit/fetch.test.ts b/packages/data-connect/test/unit/fetch.test.ts index 22c0693ff0e..a50ac188724 100644 --- a/packages/data-connect/test/unit/fetch.test.ts +++ b/packages/data-connect/test/unit/fetch.test.ts @@ -40,7 +40,15 @@ describe('fetch', () => { message }); await expect( - dcFetch('http://localhost', {}, {} as AbortController, null, null, null, false) + dcFetch( + 'http://localhost', + {}, + {} as AbortController, + null, + null, + null, + false + ) ).to.eventually.be.rejectedWith(message); }); it('should throw a stringified message when the server responds with an error without a message property in the body', async () => { @@ -51,7 +59,15 @@ describe('fetch', () => { }; mockFetch(json); await expect( - dcFetch('http://localhost', {}, {} as AbortController, null, null, null, false) + dcFetch( + 'http://localhost', + {}, + {} as AbortController, + null, + null, + null, + false + ) ).to.eventually.be.rejectedWith(JSON.stringify(json)); }); }); diff --git a/packages/data-connect/test/unit/gmpid.test.ts b/packages/data-connect/test/unit/gmpid.test.ts index f452c2948bb..c6679ca242d 100644 --- a/packages/data-connect/test/unit/gmpid.test.ts +++ b/packages/data-connect/test/unit/gmpid.test.ts @@ -39,10 +39,10 @@ const fakeFetchImpl = sinon.stub().returns( describe('GMPID Tests', () => { let dc: DataConnect; let app: FirebaseApp; - let APPID = 'MYAPPID'; + const APPID = 'MYAPPID'; beforeEach(() => { initializeFetch(fakeFetchImpl); - app = initializeApp({ projectId: 'p',appId: APPID }, 'fdsasdf'); // TODO(mtewani): Replace with util function + app = initializeApp({ projectId: 'p', appId: APPID }, 'fdsasdf'); // TODO(mtewani): Replace with util function dc = getDataConnect(app, { connector: 'c', location: 'l', service: 's' }); }); afterEach(async () => { @@ -63,7 +63,11 @@ describe('GMPID Tests', () => { }); it('should send a request with no gmpid if using the app id is not specified', async () => { const app2 = initializeApp({ projectId: 'p' }, 'def'); // TODO(mtewani): Replace with util function - const dc2 = getDataConnect(app2, { connector: 'c', location: 'l', service: 's' }); + const dc2 = getDataConnect(app2, { + connector: 'c', + location: 'l', + service: 's' + }); // @ts-ignore await executeQuery(queryRef(dc2, '')).catch(() => {}); expect(fakeFetchImpl).to.be.calledWithMatch( @@ -74,7 +78,7 @@ describe('GMPID Tests', () => { } } ); - dc2._delete(); - deleteApp(app2); + await dc2._delete(); + await deleteApp(app2); }); }); From 9e5396871012b8c943b7716215b9dad5de49fa5d Mon Sep 17 00:00:00 2001 From: Maneesh Tewani Date: Thu, 19 Sep 2024 23:51:36 +0100 Subject: [PATCH 5/5] Removed retries line --- config/mocharc.node.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/mocharc.node.js b/config/mocharc.node.js index 219256f9370..e493fce4d52 100644 --- a/config/mocharc.node.js +++ b/config/mocharc.node.js @@ -25,7 +25,7 @@ const config = { require: 'ts-node/register', timeout: 5000, - // retries: 5, + retries: 5, exit: true };