From 387bf8c4e5b9f9d555856f7e76039ced27d9d60f Mon Sep 17 00:00:00 2001 From: Stephen Rosa Date: Mon, 14 Oct 2024 11:18:59 -0700 Subject: [PATCH 1/6] Add check for non-query refType in executeQuery argument --- packages/data-connect/src/core/QueryManager.ts | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/packages/data-connect/src/core/QueryManager.ts b/packages/data-connect/src/core/QueryManager.ts index c82e0fee903..8b7c59aea85 100644 --- a/packages/data-connect/src/core/QueryManager.ts +++ b/packages/data-connect/src/core/QueryManager.ts @@ -37,7 +37,7 @@ import { DataConnectTransport } from '../network'; import { encoderImpl } from '../util/encoder'; import { setIfNotExists } from '../util/map'; -import { DataConnectError } from './error'; +import { Code, DataConnectError } from './error'; interface TrackedQuery { ref: Omit, 'dataConnect'>; @@ -172,6 +172,12 @@ export class QueryManager { executeQuery( queryRef: QueryRef ): QueryPromise { + if (queryRef.refType !== QUERY_STR) { + throw new DataConnectError( + Code.INVALID_ARGUMENT, + `ExecuteQuery can only execute query operation` + ); + } const key = encoderImpl({ name: queryRef.name, variables: queryRef.variables, From 59d2e4f0627f109ec00d68cc1cf65d0abf183167 Mon Sep 17 00:00:00 2001 From: Stephen Rosa Date: Mon, 14 Oct 2024 18:48:03 -0700 Subject: [PATCH 2/6] Add unit test checking for non-query refType in executeQuery argument --- .../test/unit/QueryManager.test.ts | 94 +++++++++++++++++++ 1 file changed, 94 insertions(+) create mode 100644 packages/data-connect/test/unit/QueryManager.test.ts diff --git a/packages/data-connect/test/unit/QueryManager.test.ts b/packages/data-connect/test/unit/QueryManager.test.ts new file mode 100644 index 00000000000..d962e5b37a9 --- /dev/null +++ b/packages/data-connect/test/unit/QueryManager.test.ts @@ -0,0 +1,94 @@ +/** + * @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 { initializeApp } from '@firebase/app'; +import { FirebaseAuthTokenData } from '@firebase/auth-interop-types'; +import { expect } from 'chai'; +import * as chai from 'chai'; +import chaiAsPromised from 'chai-as-promised'; + +import { + DataConnectOptions, + getDataConnect, + MUTATION_STR, + QUERY_STR, + QueryRef +} from '../../src'; +import { Code, DataConnectError } from '../../src/core/error'; +import { + AuthTokenListener, + AuthTokenProvider +} from '../../src/core/FirebaseAuthProvider'; +import { QueryManager } from '../../src/core/QueryManager'; +import { RESTTransport } from '../../src/network/transport/rest'; +chai.use(chaiAsPromised); +const options: DataConnectOptions = { + connector: 'c', + location: 'l', + projectId: 'p', + service: 's' +}; +const INITIAL_TOKEN = 'initial token'; +class FakeAuthProvider implements AuthTokenProvider { + private token: string | null = INITIAL_TOKEN; + addTokenChangeListener(listener: AuthTokenListener): void {} + getToken(forceRefresh: boolean): Promise { + if (!forceRefresh) { + return Promise.resolve({ accessToken: this.token! }); + } + return Promise.resolve({ accessToken: 'testToken' }); + } + setToken(_token: string | null): void { + this.token = _token; + } +} + +describe('Query Manager Tests', () => { + it('should refuse to make requests to execute non-query operations', async () => { + const authProvider = new FakeAuthProvider(); + const rt = new RESTTransport(options, undefined, undefined, authProvider); + const qm = new QueryManager(rt); + const app = initializeApp({ projectId: 'p' }); + const dc = getDataConnect(app, { + connector: 'c', + location: 'l', + service: 's' + }); + + const mutationRef: QueryRef = { + name: 'm', + variables: 'v', + dataConnect: dc, + refType: MUTATION_STR as 'query' + }; + + const queryRef: QueryRef = { + name: 'm', + variables: 'v', + dataConnect: dc, + refType: QUERY_STR + }; + + const error = new DataConnectError( + Code.INVALID_ARGUMENT, + `ExecuteQuery can only execute query operation` + ); + + expect(() => qm.executeQuery(mutationRef)).to.throw(error.message); + expect(() => qm.executeQuery(queryRef)).to.not.throw(error.message); + }); +}); From e0a32a113354db3004d2cec5e89faf878485b137 Mon Sep 17 00:00:00 2001 From: Stephen Rosa Date: Tue, 15 Oct 2024 10:42:35 -0700 Subject: [PATCH 3/6] add changeset --- .changeset/beige-roses-cross.md | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 .changeset/beige-roses-cross.md diff --git a/.changeset/beige-roses-cross.md b/.changeset/beige-roses-cross.md new file mode 100644 index 00000000000..78a3356ff5c --- /dev/null +++ b/.changeset/beige-roses-cross.md @@ -0,0 +1,6 @@ +--- +'@firebase/data-connect': patch +--- + +- Add check for non-query refType in `executeQuery` that throws error on refType mismatch +- Add unit test to check that error is thrown when `executeQuery` receives non-query refType, and not thrown otherwise From 9bc852c9292513859dbdd4209d837c1a018fb52a Mon Sep 17 00:00:00 2001 From: Stephen Rosa Date: Tue, 15 Oct 2024 11:31:18 -0700 Subject: [PATCH 4/6] implement requested changes to new test --- .../test/unit/QueryManager.test.ts | 70 ++++++------------- 1 file changed, 21 insertions(+), 49 deletions(-) diff --git a/packages/data-connect/test/unit/QueryManager.test.ts b/packages/data-connect/test/unit/QueryManager.test.ts index d962e5b37a9..d911bd2b847 100644 --- a/packages/data-connect/test/unit/QueryManager.test.ts +++ b/packages/data-connect/test/unit/QueryManager.test.ts @@ -15,26 +15,20 @@ * limitations under the License. */ -import { initializeApp } from '@firebase/app'; -import { FirebaseAuthTokenData } from '@firebase/auth-interop-types'; +import { deleteApp, FirebaseApp, initializeApp } from '@firebase/app'; import { expect } from 'chai'; import * as chai from 'chai'; import chaiAsPromised from 'chai-as-promised'; import { + DataConnect, DataConnectOptions, + executeQuery, getDataConnect, - MUTATION_STR, - QUERY_STR, - QueryRef + mutationRef, + queryRef, } from '../../src'; import { Code, DataConnectError } from '../../src/core/error'; -import { - AuthTokenListener, - AuthTokenProvider -} from '../../src/core/FirebaseAuthProvider'; -import { QueryManager } from '../../src/core/QueryManager'; -import { RESTTransport } from '../../src/network/transport/rest'; chai.use(chaiAsPromised); const options: DataConnectOptions = { connector: 'c', @@ -42,53 +36,31 @@ const options: DataConnectOptions = { projectId: 'p', service: 's' }; -const INITIAL_TOKEN = 'initial token'; -class FakeAuthProvider implements AuthTokenProvider { - private token: string | null = INITIAL_TOKEN; - addTokenChangeListener(listener: AuthTokenListener): void {} - getToken(forceRefresh: boolean): Promise { - if (!forceRefresh) { - return Promise.resolve({ accessToken: this.token! }); - } - return Promise.resolve({ accessToken: 'testToken' }); - } - setToken(_token: string | null): void { - this.token = _token; - } -} describe('Query Manager Tests', () => { - it('should refuse to make requests to execute non-query operations', async () => { - const authProvider = new FakeAuthProvider(); - const rt = new RESTTransport(options, undefined, undefined, authProvider); - const qm = new QueryManager(rt); - const app = initializeApp({ projectId: 'p' }); - const dc = getDataConnect(app, { - connector: 'c', - location: 'l', - service: 's' - }); + let dc: DataConnect; + let app: FirebaseApp; - const mutationRef: QueryRef = { - name: 'm', - variables: 'v', - dataConnect: dc, - refType: MUTATION_STR as 'query' - }; + beforeEach(() => { + app = initializeApp({ projectId: 'p' }); + dc = getDataConnect(app, options); + }); + afterEach(async () => { + await dc._delete(); + await deleteApp(app); + }); - const queryRef: QueryRef = { - name: 'm', - variables: 'v', - dataConnect: dc, - refType: QUERY_STR - }; + it('should refuse to make requests to execute non-query operations', async () => { + const query = queryRef(dc, 'q'); + const mutation = mutationRef(dc, 'm'); const error = new DataConnectError( Code.INVALID_ARGUMENT, `ExecuteQuery can only execute query operation` ); - expect(() => qm.executeQuery(mutationRef)).to.throw(error.message); - expect(() => qm.executeQuery(queryRef)).to.not.throw(error.message); + // @ts-ignore + expect(() => executeQuery(mutation)).to.throw(error.message); + expect(() => executeQuery(query)).to.not.throw(error.message); }); }); From a0ecb6f141e91bf5cf32eeea61d69f607303ac90 Mon Sep 17 00:00:00 2001 From: Stephen Rosa Date: Tue, 15 Oct 2024 11:50:44 -0700 Subject: [PATCH 5/6] change app initialization parameters to prevent clobbering in 'utils.test.ts' test cases --- .../test/unit/QueryManager.test.ts | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/packages/data-connect/test/unit/QueryManager.test.ts b/packages/data-connect/test/unit/QueryManager.test.ts index d911bd2b847..9acc948d57c 100644 --- a/packages/data-connect/test/unit/QueryManager.test.ts +++ b/packages/data-connect/test/unit/QueryManager.test.ts @@ -22,28 +22,27 @@ import chaiAsPromised from 'chai-as-promised'; import { DataConnect, - DataConnectOptions, executeQuery, getDataConnect, mutationRef, - queryRef, + queryRef } from '../../src'; import { Code, DataConnectError } from '../../src/core/error'; chai.use(chaiAsPromised); -const options: DataConnectOptions = { - connector: 'c', - location: 'l', - projectId: 'p', - service: 's' -}; describe('Query Manager Tests', () => { let dc: DataConnect; let app: FirebaseApp; + const APPID = 'MYAPPID'; + const APPNAME = 'MYAPPNAME'; beforeEach(() => { - app = initializeApp({ projectId: 'p' }); - dc = getDataConnect(app, options); + app = initializeApp({ projectId: 'p', appId: APPID }, APPNAME); + dc = getDataConnect(app, { + connector: 'c', + location: 'l', + service: 's' + }); }); afterEach(async () => { await dc._delete(); From d4de37a8dce0b1ab4d191636ce6f45d9ad7716bd Mon Sep 17 00:00:00 2001 From: Stephen Rosa Date: Tue, 15 Oct 2024 14:15:00 -0700 Subject: [PATCH 6/6] update changelog --- .changeset/beige-roses-cross.md | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/.changeset/beige-roses-cross.md b/.changeset/beige-roses-cross.md index 78a3356ff5c..395abbb1909 100644 --- a/.changeset/beige-roses-cross.md +++ b/.changeset/beige-roses-cross.md @@ -2,5 +2,4 @@ '@firebase/data-connect': patch --- -- Add check for non-query refType in `executeQuery` that throws error on refType mismatch -- Add unit test to check that error is thrown when `executeQuery` receives non-query refType, and not thrown otherwise +- Throw error when calling `executeQuery` with mutations \ No newline at end of file