From 37d7d85d9d1c78196107e7b342766ea7bf4f86d0 Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Thu, 24 Apr 2025 18:39:17 +0000 Subject: [PATCH 01/11] refactor(data-connect): Move CRUD helper implementation to client Refactors the previously added DataConnect CRUD helper methods (`insert`, `insertMany`, `upsert`, `upsertMany`) based on feedback. The core implementation logic (input validation, data serialization using `objectToString`, GraphQL mutation string construction, and calling `executeGraphql`) has been moved from the `DataConnect` class (`data-connect.ts`) to the `DataConnectApiClient` class (`data-connect-api-client-internal.ts`). The methods in the `DataConnect` class now act as simple pass-through delegates to the corresponding methods on the internal client instance. Unit tests have been updated accordingly: - Tests in `index.spec.ts` now verify the delegation from `DataConnect` to `DataConnectApiClient`. - New tests have been added to `data-connect-api-client-internal.spec.ts` to cover the implementation details within `DataConnectApiClient`. This change improves separation of concerns, keeping the public API surface (`DataConnect`) clean and concentrating the implementation details within the internal client. --- .../data-connect-api-client-internal.ts | 136 ++++++++++++ src/data-connect/data-connect.ts | 63 ++++++ .../data-connect-api-client-internal.spec.ts | 193 ++++++++++++++++++ test/unit/data-connect/index.spec.ts | 71 +++++++ 4 files changed, 463 insertions(+) diff --git a/src/data-connect/data-connect-api-client-internal.ts b/src/data-connect/data-connect-api-client-internal.ts index b6082b91f5..3e13827183 100644 --- a/src/data-connect/data-connect-api-client-internal.ts +++ b/src/data-connect/data-connect-api-client-internal.ts @@ -25,6 +25,36 @@ import * as utils from '../utils/index'; import * as validator from '../utils/validator'; import { ConnectorConfig, ExecuteGraphqlResponse, GraphqlOptions } from './data-connect-api'; +/** + * Converts a JavaScript value into a GraphQL literal string. + * Handles nested objects, arrays, strings, numbers, and booleans. + * Ensures strings are properly escaped. + */ +function objectToString(data: any): string { + if (typeof data !== 'object' || data === null) { + if (typeof data === 'string') { + // Properly escape double quotes and backslashes within strings + const escapedString = data.replace(/\/g, '\\').replace(/"/g, '\"'); + return `"${escapedString}"`; + } + // Handle numbers, booleans, null directly + return String(data); + } + + if (Array.isArray(data)) { + const elements = data.map(item => objectToString(item)).join(', '); + return `[${elements}]`; + } + + // Handle plain objects + const entries = Object.entries(data).map(([key, value]) => { + // GraphQL object keys are typically unquoted identifiers + return `${key}: ${objectToString(value)}`; + }); + + return `{ ${entries.join(', ')} }`; +} + const API_VERSION = 'v1alpha'; /** The Firebase Data Connect backend base URL format. */ @@ -198,6 +228,112 @@ export class DataConnectApiClient { const message = error.message || `Unknown server error: ${response.text}`; return new FirebaseDataConnectError(code, message); } + + /** + * Insert a single row into the specified table. + * (Implementation moved from DataConnect class) + */ + public insert( + tableName: string, + data: object, + ): Promise> { + if (!validator.isNonEmptyString(tableName)) { + throw new FirebaseDataConnectError('invalid-argument', '`tableName` must be a non-empty string.'); + } + if (!validator.isNonNullObject(data)) { + throw new FirebaseDataConnectError('invalid-argument', '`data` must be a non-null object.'); + } + if (Array.isArray(data)) { + throw new FirebaseDataConnectError('invalid-argument', '`data` must be an object, not an array, for single insert.'); + } + + try { + const gqlDataString = objectToString(data); + const mutation = `mutation { ${tableName}_insert(data: ${gqlDataString}) }`; + // Use internal executeGraphql + return this.executeGraphql(mutation); + } catch (e: any) { + throw new FirebaseDataConnectError('internal-error', `Failed to construct insert mutation: ${e.message}`); + } + } + + /** + * Insert multiple rows into the specified table. + * (Implementation moved from DataConnect class) + */ + public insertMany( + tableName: string, + data: object[], + ): Promise> { + if (!validator.isNonEmptyString(tableName)) { + throw new FirebaseDataConnectError('invalid-argument', '`tableName` must be a non-empty string.'); + } + if (!validator.isNonEmptyArray(data)) { + throw new FirebaseDataConnectError('invalid-argument', '`data` must be a non-empty array for insertMany.'); + } + + try { + const gqlDataString = objectToString(data); + const mutation = `mutation { ${tableName}_insertMany(data: ${gqlDataString}) }`; + // Use internal executeGraphql + return this.executeGraphql(mutation); + } catch (e: any) { + throw new FirebaseDataConnectError('internal-error', `Failed to construct insertMany mutation: ${e.message}`); + } + } + + /** + * Insert a single row into the specified table, or update it if it already exists. + * (Implementation moved from DataConnect class) + */ + public upsert( + tableName: string, + data: object, + ): Promise> { + if (!validator.isNonEmptyString(tableName)) { + throw new FirebaseDataConnectError('invalid-argument', '`tableName` must be a non-empty string.'); + } + if (!validator.isNonNullObject(data)) { + throw new FirebaseDataConnectError('invalid-argument', '`data` must be a non-null object.'); + } + if (Array.isArray(data)) { + throw new FirebaseDataConnectError('invalid-argument', '`data` must be an object, not an array, for single upsert.'); + } + + try { + const gqlDataString = objectToString(data); + const mutation = `mutation { ${tableName}_upsert(data: ${gqlDataString}) }`; + // Use internal executeGraphql + return this.executeGraphql(mutation); + } catch (e: any) { + throw new FirebaseDataConnectError('internal-error', `Failed to construct upsert mutation: ${e.message}`); + } + } + + /** + * Insert multiple rows into the specified table, or update them if they already exist. + * (Implementation moved from DataConnect class) + */ + public upsertMany( + tableName: string, + data: object[], + ): Promise> { + if (!validator.isNonEmptyString(tableName)) { + throw new FirebaseDataConnectError('invalid-argument', '`tableName` must be a non-empty string.'); + } + if (!validator.isNonEmptyArray(data)) { + throw new FirebaseDataConnectError('invalid-argument', '`data` must be a non-empty array for upsertMany.'); + } + + try { + const gqlDataString = objectToString(data); + const mutation = `mutation { ${tableName}_upsertMany(data: ${gqlDataString}) }`; + // Use internal executeGraphql + return this.executeGraphql(mutation); + } catch (e: any) { + throw new FirebaseDataConnectError('internal-error', `Failed to construct upsertMany mutation: ${e.message}`); + } + } } /** diff --git a/src/data-connect/data-connect.ts b/src/data-connect/data-connect.ts index dbded00042..d4158859a7 100644 --- a/src/data-connect/data-connect.ts +++ b/src/data-connect/data-connect.ts @@ -17,6 +17,7 @@ import { App } from '../app'; import { DataConnectApiClient } from './data-connect-api-client-internal'; +// FirebaseDataConnectError, objectToString, and validator are no longer needed here import { ConnectorConfig, @@ -103,4 +104,66 @@ export class DataConnect { ): Promise> { return this.client.executeGraphqlRead(query, options); } + + /** + * Insert a single row into the specified table. + * + * @param tableName - The name of the table to insert data into. + * @param data - The data object to insert. The keys should correspond to the column names. + * @returns A promise that fulfills with a `ExecuteGraphqlResponse`. + * @beta + */ + public insert( + tableName: string, + data: object, + ): Promise> { + return this.client.insert(tableName, data); + } + + /** + * Insert multiple rows into the specified table. + * + * @param tableName - The name of the table to insert data into. + * @param data - An array of data objects to insert. Each object's keys should correspond to the column names. + * @returns A promise that fulfills with a `ExecuteGraphqlResponse`. + * @beta + */ + public insertMany( + tableName: string, + data: object[], + ): Promise> { + return this.client.insertMany(tableName, data); + } + + /** + * Insert a single row into the specified table, or update it if it already exists. + * The specific behavior (insert or update) depends on the underlying database and schema configuration. + * + * @param tableName - The name of the table to upsert data into. + * @param data - The data object to upsert. The keys should correspond to the column names. + * @returns A promise that fulfills with a `ExecuteGraphqlResponse`. + * @beta + */ + public upsert( + tableName: string, + data: object, + ): Promise> { + return this.client.upsert(tableName, data); + } + + /** + * Insert multiple rows into the specified table, or update them if they already exist. + * The specific behavior (insert or update) depends on the underlying database and schema configuration. + * + * @param tableName - The name of the table to upsert data into. + * @param data - An array of data objects to upsert. Each object's keys should correspond to the column names. + * @returns A promise that fulfills with a `ExecuteGraphqlResponse`. + * @beta + */ + public upsertMany( + tableName: string, + data: object[], + ): Promise> { + return this.client.upsertMany(tableName, data); + } } diff --git a/test/unit/data-connect/data-connect-api-client-internal.spec.ts b/test/unit/data-connect/data-connect-api-client-internal.spec.ts index 9787a2d7c9..dc1e18372d 100644 --- a/test/unit/data-connect/data-connect-api-client-internal.spec.ts +++ b/test/unit/data-connect/data-connect-api-client-internal.spec.ts @@ -225,3 +225,196 @@ describe('DataConnectApiClient', () => { }); }); }); + +// New test suite for the CRUD helpers moved into the ApiClient +describe('DataConnectApiClient CRUD helpers', () => { + let mockApp: FirebaseApp; + let apiClient: DataConnectApiClient; + let executeGraphqlStub: sinon.SinonStub; + + const connectorConfig: ConnectorConfig = { + location: 'us-west1', + serviceId: 'my-crud-service', + }; + + const mockOptions = { + credential: new mocks.MockCredential(), + projectId: 'test-project-crud', + }; + + const testTableName = 'TestTable'; + + beforeEach(() => { + mockApp = mocks.appWithOptions(mockOptions); + apiClient = new DataConnectApiClient(connectorConfig, mockApp); + // Stub the instance's executeGraphql method + executeGraphqlStub = sinon.stub(apiClient, 'executeGraphql').resolves({ data: {} }); + }); + + afterEach(() => { + sinon.restore(); + return mockApp.delete(); + }); + + // --- INSERT TESTS --- + describe('insert()', () => { + it('should call executeGraphql with the correct mutation for simple data', async () => { + const simpleData = { name: 'test', value: 123 }; + const expectedMutation = `mutation { ${testTableName}_insert(data: { name: "test", value: 123 }) }`; + await apiClient.insert(testTableName, simpleData); + expect(executeGraphqlStub).to.have.been.calledOnceWithExactly(expectedMutation); + }); + + it('should call executeGraphql with the correct mutation for complex data', async () => { + const complexData = { id: 'abc', active: true, scores: [10, 20], info: { nested: "yes/no \"quote\" \\slash\\" } }; + // Note: Need to match the specific escaping from objectToString: / -> \\, " -> \" + const expectedMutation = `mutation { ${testTableName}_insert(data: { id: "abc", active: true, scores: [10, 20], info: { nested: "yes/no \\"quote\\" \\\\slash\\\\" } }) }`; + await apiClient.insert(testTableName, complexData); + expect(executeGraphqlStub).to.have.been.calledOnceWithExactly(expectedMutation); + }); + + it('should throw FirebaseDataConnectError for invalid tableName', () => { + expect(() => apiClient.insert('', { data: 1 })) + .to.throw(FirebaseDataConnectError, /`tableName` must be a non-empty string./) + .with.property('code', 'data-connect/invalid-argument'); + }); + + it('should throw FirebaseDataConnectError for null data', () => { + expect(() => apiClient.insert(testTableName, null as any)) + .to.throw(FirebaseDataConnectError, /`data` must be a non-null object./) + .with.property('code', 'data-connect/invalid-argument'); + }); + + it('should throw FirebaseDataConnectError for array data', () => { + expect(() => apiClient.insert(testTableName, [])) + .to.throw(FirebaseDataConnectError, /`data` must be an object, not an array, for single insert./) + .with.property('code', 'data-connect/invalid-argument'); + }); + }); + + // --- INSERT MANY TESTS --- + describe('insertMany()', () => { + it('should call executeGraphql with the correct mutation for simple data array', async () => { + const simpleDataArray = [{ name: 'test1' }, { name: 'test2', value: 456 }]; + const expectedMutation = `mutation { ${testTableName}_insertMany(data: [{ name: "test1" }, { name: "test2", value: 456 }]) }`; + await apiClient.insertMany(testTableName, simpleDataArray); + expect(executeGraphqlStub).to.have.been.calledOnceWithExactly(expectedMutation); + }); + + it('should call executeGraphql with the correct mutation for complex data array', async () => { + const complexDataArray = [ + { id: 'a', active: true, info: { nested: 'n1' } }, + { id: 'b', scores: [1, 2], info: { nested: "n2/\\" } } + ]; + // Note: Matching specific escaping: / -> \\, " -> \" + const expectedMutation = `mutation { ${testTableName}_insertMany(data: [{ id: "a", active: true, info: { nested: "n1" } }, { id: "b", scores: [1, 2], info: { nested: "n2/\\\\" } }]) }`; + await apiClient.insertMany(testTableName, complexDataArray); + expect(executeGraphqlStub).to.have.been.calledOnceWithExactly(expectedMutation); + }); + + it('should throw FirebaseDataConnectError for invalid tableName', () => { + expect(() => apiClient.insertMany('', [{ data: 1 }])) + .to.throw(FirebaseDataConnectError, /`tableName` must be a non-empty string./) + .with.property('code', 'data-connect/invalid-argument'); + }); + + it('should throw FirebaseDataConnectError for null data', () => { + expect(() => apiClient.insertMany(testTableName, null as any)) + .to.throw(FirebaseDataConnectError, /`data` must be a non-empty array for insertMany./) + .with.property('code', 'data-connect/invalid-argument'); + }); + + it('should throw FirebaseDataConnectError for empty array data', () => { + expect(() => apiClient.insertMany(testTableName, [])) + .to.throw(FirebaseDataConnectError, /`data` must be a non-empty array for insertMany./) + .with.property('code', 'data-connect/invalid-argument'); + }); + + it('should throw FirebaseDataConnectError for non-array data', () => { + expect(() => apiClient.insertMany(testTableName, { data: 1 } as any)) + .to.throw(FirebaseDataConnectError, /`data` must be a non-empty array for insertMany./) + .with.property('code', 'data-connect/invalid-argument'); + }); + }); + + // --- UPSERT TESTS --- + describe('upsert()', () => { + it('should call executeGraphql with the correct mutation for simple data', async () => { + const simpleData = { id: 'key1', value: 'updated' }; + const expectedMutation = `mutation { ${testTableName}_upsert(data: { id: "key1", value: "updated" }) }`; + await apiClient.upsert(testTableName, simpleData); + expect(executeGraphqlStub).to.have.been.calledOnceWithExactly(expectedMutation); + }); + + it('should call executeGraphql with the correct mutation for complex data', async () => { + const complexData = { id: 'key2', active: false, items: [1, null], detail: { status: "done/\\" } }; + // Note: Matching specific escaping: / -> \\, " -> \" + const expectedMutation = `mutation { ${testTableName}_upsert(data: { id: "key2", active: false, items: [1, null], detail: { status: "done/\\\\" } }) }`; + await apiClient.upsert(testTableName, complexData); + expect(executeGraphqlStub).to.have.been.calledOnceWithExactly(expectedMutation); + }); + + it('should throw FirebaseDataConnectError for invalid tableName', () => { + expect(() => apiClient.upsert('', { data: 1 })) + .to.throw(FirebaseDataConnectError, /`tableName` must be a non-empty string./) + .with.property('code', 'data-connect/invalid-argument'); + }); + + it('should throw FirebaseDataConnectError for null data', () => { + expect(() => apiClient.upsert(testTableName, null as any)) + .to.throw(FirebaseDataConnectError, /`data` must be a non-null object./) + .with.property('code', 'data-connect/invalid-argument'); + }); + + it('should throw FirebaseDataConnectError for array data', () => { + expect(() => apiClient.upsert(testTableName, [{ data: 1 }])) + .to.throw(FirebaseDataConnectError, /`data` must be an object, not an array, for single upsert./) + .with.property('code', 'data-connect/invalid-argument'); + }); + }); + + // --- UPSERT MANY TESTS --- + describe('upsertMany()', () => { + it('should call executeGraphql with the correct mutation for simple data array', async () => { + const simpleDataArray = [{ id: 'k1' }, { id: 'k2', value: 99 }]; + const expectedMutation = `mutation { ${testTableName}_upsertMany(data: [{ id: "k1" }, { id: "k2", value: 99 }]) }`; + await apiClient.upsertMany(testTableName, simpleDataArray); + expect(executeGraphqlStub).to.have.been.calledOnceWithExactly(expectedMutation); + }); + + it('should call executeGraphql with the correct mutation for complex data array', async () => { + const complexDataArray = [ + { id: 'x', active: true, info: { nested: 'n1/\\"x' } }, + { id: 'y', scores: [null, 2] } + ]; + // Note: Matching specific escaping: / -> \\, " -> \" + const expectedMutation = `mutation { ${testTableName}_upsertMany(data: [{ id: "x", active: true, info: { nested: "n1/\\\\\\"x" } }, { id: "y", scores: [null, 2] }]) }`; + await apiClient.upsertMany(testTableName, complexDataArray); + expect(executeGraphqlStub).to.have.been.calledOnceWithExactly(expectedMutation); + }); + + it('should throw FirebaseDataConnectError for invalid tableName', () => { + expect(() => apiClient.upsertMany('', [{ data: 1 }])) + .to.throw(FirebaseDataConnectError, /`tableName` must be a non-empty string./) + .with.property('code', 'data-connect/invalid-argument'); + }); + + it('should throw FirebaseDataConnectError for null data', () => { + expect(() => apiClient.upsertMany(testTableName, null as any)) + .to.throw(FirebaseDataConnectError, /`data` must be a non-empty array for upsertMany./) + .with.property('code', 'data-connect/invalid-argument'); + }); + + it('should throw FirebaseDataConnectError for empty array data', () => { + expect(() => apiClient.upsertMany(testTableName, [])) + .to.throw(FirebaseDataConnectError, /`data` must be a non-empty array for upsertMany./) + .with.property('code', 'data-connect/invalid-argument'); + }); + + it('should throw FirebaseDataConnectError for non-array data', () => { + expect(() => apiClient.upsertMany(testTableName, { data: 1 } as any)) + .to.throw(FirebaseDataConnectError, /`data` must be a non-empty array for upsertMany./) + .with.property('code', 'data-connect/invalid-argument'); + }); + }); +}); diff --git a/test/unit/data-connect/index.spec.ts b/test/unit/data-connect/index.spec.ts index 228cafbf84..8c8a7984c0 100644 --- a/test/unit/data-connect/index.spec.ts +++ b/test/unit/data-connect/index.spec.ts @@ -18,12 +18,14 @@ 'use strict'; import * as chai from 'chai'; +import * as sinon from 'sinon'; import * as sinonChai from 'sinon-chai'; import * as chaiAsPromised from 'chai-as-promised'; import * as mocks from '../../resources/mocks'; import { App } from '../../../src/app/index'; import { getDataConnect, DataConnect } from '../../../src/data-connect/index'; +import { DataConnectApiClient, FirebaseDataConnectError } from '../../../src/data-connect/data-connect-api-client-internal'; chai.should(); chai.use(sinonChai); @@ -84,3 +86,72 @@ describe('DataConnect', () => { }); }); }); + +describe('DataConnect CRUD helpers delegation', () => { // Renamed suite for clarity + let mockApp: App; + let dataConnect: DataConnect; + // Stubs for the client methods + let clientInsertStub: sinon.SinonStub; + let clientInsertManyStub: sinon.SinonStub; + let clientUpsertStub: sinon.SinonStub; + let clientUpsertManyStub: sinon.SinonStub; + + const connectorConfig = { + location: 'us-west1', + serviceId: 'my-crud-service', + }; + + const testTableName = 'TestTable'; + + beforeEach(() => { + mockApp = mocks.app(); + // Instantiate DataConnect normally + dataConnect = getDataConnect(connectorConfig, mockApp); + + // Stub the DataConnectApiClient prototype methods + clientInsertStub = sinon.stub(DataConnectApiClient.prototype, 'insert').resolves({ data: {} }); + clientInsertManyStub = sinon.stub(DataConnectApiClient.prototype, 'insertMany').resolves({ data: {} }); + clientUpsertStub = sinon.stub(DataConnectApiClient.prototype, 'upsert').resolves({ data: {} }); + clientUpsertManyStub = sinon.stub(DataConnectApiClient.prototype, 'upsertMany').resolves({ data: {} }); + }); + + afterEach(() => { + sinon.restore(); + }); + + // --- INSERT TESTS --- + describe('insert()', () => { + it('should delegate insert call to the client', async () => { + const simpleData = { name: 'test', value: 123 }; + await dataConnect.insert(testTableName, simpleData); + expect(clientInsertStub).to.have.been.calledOnceWithExactly(testTableName, simpleData); + }); + }); + + // --- INSERT MANY TESTS --- + describe('insertMany()', () => { + it('should delegate insertMany call to the client', async () => { + const simpleDataArray = [{ name: 'test1' }, { name: 'test2' }]; + await dataConnect.insertMany(testTableName, simpleDataArray); + expect(clientInsertManyStub).to.have.been.calledOnceWithExactly(testTableName, simpleDataArray); + }); + }); + + // --- UPSERT TESTS --- + describe('upsert()', () => { + it('should delegate upsert call to the client', async () => { + const simpleData = { id: 'key1', value: 'updated' }; + await dataConnect.upsert(testTableName, simpleData); + expect(clientUpsertStub).to.have.been.calledOnceWithExactly(testTableName, simpleData); + }); + }); + + // --- UPSERT MANY TESTS --- + describe('upsertMany()', () => { + it('should delegate upsertMany call to the client', async () => { + const simpleDataArray = [{ id: 'k1' }, { id: 'k2' }]; + await dataConnect.upsertMany(testTableName, simpleDataArray); + expect(clientUpsertManyStub).to.have.been.calledOnceWithExactly(testTableName, simpleDataArray); + }); + }); +}); From 3cac10279faaef93bd16b9e531b67004e9eba432 Mon Sep 17 00:00:00 2001 From: Lahiru Maramba Date: Fri, 25 Apr 2025 15:33:18 -0400 Subject: [PATCH 02/11] fix lint issues and function signatures --- .../data-connect-api-client-internal.ts | 40 +++++++------- src/data-connect/data-connect.ts | 40 +++++++------- .../data-connect-api-client-internal.spec.ts | 53 ++++++++++++------- test/unit/data-connect/index.spec.ts | 2 +- 4 files changed, 77 insertions(+), 58 deletions(-) diff --git a/src/data-connect/data-connect-api-client-internal.ts b/src/data-connect/data-connect-api-client-internal.ts index 3e13827183..c55cea5171 100644 --- a/src/data-connect/data-connect-api-client-internal.ts +++ b/src/data-connect/data-connect-api-client-internal.ts @@ -34,7 +34,7 @@ function objectToString(data: any): string { if (typeof data !== 'object' || data === null) { if (typeof data === 'string') { // Properly escape double quotes and backslashes within strings - const escapedString = data.replace(/\/g, '\\').replace(/"/g, '\"'); + const escapedString = data.replace(/\//g, '\\').replace(/,"/g, '"'); return `"${escapedString}"`; } // Handle numbers, booleans, null directly @@ -233,10 +233,10 @@ export class DataConnectApiClient { * Insert a single row into the specified table. * (Implementation moved from DataConnect class) */ - public insert( + public async insert( tableName: string, - data: object, - ): Promise> { + data: Variables, + ): Promise> { if (!validator.isNonEmptyString(tableName)) { throw new FirebaseDataConnectError('invalid-argument', '`tableName` must be a non-empty string.'); } @@ -244,14 +244,15 @@ export class DataConnectApiClient { throw new FirebaseDataConnectError('invalid-argument', '`data` must be a non-null object.'); } if (Array.isArray(data)) { - throw new FirebaseDataConnectError('invalid-argument', '`data` must be an object, not an array, for single insert.'); + throw new FirebaseDataConnectError( + 'invalid-argument', '`data` must be an object, not an array, for single insert.'); } try { const gqlDataString = objectToString(data); const mutation = `mutation { ${tableName}_insert(data: ${gqlDataString}) }`; // Use internal executeGraphql - return this.executeGraphql(mutation); + return this.executeGraphql(mutation); } catch (e: any) { throw new FirebaseDataConnectError('internal-error', `Failed to construct insert mutation: ${e.message}`); } @@ -261,10 +262,10 @@ export class DataConnectApiClient { * Insert multiple rows into the specified table. * (Implementation moved from DataConnect class) */ - public insertMany( + public async insertMany>( tableName: string, - data: object[], - ): Promise> { + data: Variables, + ): Promise> { if (!validator.isNonEmptyString(tableName)) { throw new FirebaseDataConnectError('invalid-argument', '`tableName` must be a non-empty string.'); } @@ -276,7 +277,7 @@ export class DataConnectApiClient { const gqlDataString = objectToString(data); const mutation = `mutation { ${tableName}_insertMany(data: ${gqlDataString}) }`; // Use internal executeGraphql - return this.executeGraphql(mutation); + return this.executeGraphql(mutation); } catch (e: any) { throw new FirebaseDataConnectError('internal-error', `Failed to construct insertMany mutation: ${e.message}`); } @@ -286,10 +287,10 @@ export class DataConnectApiClient { * Insert a single row into the specified table, or update it if it already exists. * (Implementation moved from DataConnect class) */ - public upsert( + public async upsert( tableName: string, - data: object, - ): Promise> { + data: Variables, + ): Promise> { if (!validator.isNonEmptyString(tableName)) { throw new FirebaseDataConnectError('invalid-argument', '`tableName` must be a non-empty string.'); } @@ -297,14 +298,15 @@ export class DataConnectApiClient { throw new FirebaseDataConnectError('invalid-argument', '`data` must be a non-null object.'); } if (Array.isArray(data)) { - throw new FirebaseDataConnectError('invalid-argument', '`data` must be an object, not an array, for single upsert.'); + throw new FirebaseDataConnectError( + 'invalid-argument', '`data` must be an object, not an array, for single upsert.'); } try { const gqlDataString = objectToString(data); const mutation = `mutation { ${tableName}_upsert(data: ${gqlDataString}) }`; // Use internal executeGraphql - return this.executeGraphql(mutation); + return this.executeGraphql(mutation); } catch (e: any) { throw new FirebaseDataConnectError('internal-error', `Failed to construct upsert mutation: ${e.message}`); } @@ -314,10 +316,10 @@ export class DataConnectApiClient { * Insert multiple rows into the specified table, or update them if they already exist. * (Implementation moved from DataConnect class) */ - public upsertMany( + public async upsertMany>( tableName: string, - data: object[], - ): Promise> { + data: Variables, + ): Promise> { if (!validator.isNonEmptyString(tableName)) { throw new FirebaseDataConnectError('invalid-argument', '`tableName` must be a non-empty string.'); } @@ -329,7 +331,7 @@ export class DataConnectApiClient { const gqlDataString = objectToString(data); const mutation = `mutation { ${tableName}_upsertMany(data: ${gqlDataString}) }`; // Use internal executeGraphql - return this.executeGraphql(mutation); + return this.executeGraphql(mutation); } catch (e: any) { throw new FirebaseDataConnectError('internal-error', `Failed to construct upsertMany mutation: ${e.message}`); } diff --git a/src/data-connect/data-connect.ts b/src/data-connect/data-connect.ts index d4158859a7..afd6120d20 100644 --- a/src/data-connect/data-connect.ts +++ b/src/data-connect/data-connect.ts @@ -109,30 +109,30 @@ export class DataConnect { * Insert a single row into the specified table. * * @param tableName - The name of the table to insert data into. - * @param data - The data object to insert. The keys should correspond to the column names. + * @param variables - The data object to insert. The keys should correspond to the column names. * @returns A promise that fulfills with a `ExecuteGraphqlResponse`. * @beta */ - public insert( + public insert( tableName: string, - data: object, - ): Promise> { - return this.client.insert(tableName, data); + variables: Variables, + ): Promise> { + return this.client.insert(tableName, variables); } /** * Insert multiple rows into the specified table. * * @param tableName - The name of the table to insert data into. - * @param data - An array of data objects to insert. Each object's keys should correspond to the column names. + * @param variables - An array of data objects to insert. Each object's keys should correspond to the column names. * @returns A promise that fulfills with a `ExecuteGraphqlResponse`. * @beta */ - public insertMany( + public insertMany>( tableName: string, - data: object[], - ): Promise> { - return this.client.insertMany(tableName, data); + variables: Variables, + ): Promise> { + return this.client.insertMany(tableName, variables); } /** @@ -140,15 +140,15 @@ export class DataConnect { * The specific behavior (insert or update) depends on the underlying database and schema configuration. * * @param tableName - The name of the table to upsert data into. - * @param data - The data object to upsert. The keys should correspond to the column names. + * @param variables - The data object to upsert. The keys should correspond to the column names. * @returns A promise that fulfills with a `ExecuteGraphqlResponse`. * @beta */ - public upsert( + public upsert( tableName: string, - data: object, - ): Promise> { - return this.client.upsert(tableName, data); + variables: Variables, + ): Promise> { + return this.client.upsert(tableName, variables); } /** @@ -156,14 +156,14 @@ export class DataConnect { * The specific behavior (insert or update) depends on the underlying database and schema configuration. * * @param tableName - The name of the table to upsert data into. - * @param data - An array of data objects to upsert. Each object's keys should correspond to the column names. + * @param variables - An array of data objects to upsert. Each object's keys should correspond to the column names. * @returns A promise that fulfills with a `ExecuteGraphqlResponse`. * @beta */ - public upsertMany( + public upsertMany>( tableName: string, - data: object[], - ): Promise> { - return this.client.upsertMany(tableName, data); + variables: Variables, + ): Promise> { + return this.client.upsertMany(tableName, variables); } } diff --git a/test/unit/data-connect/data-connect-api-client-internal.spec.ts b/test/unit/data-connect/data-connect-api-client-internal.spec.ts index dc1e18372d..89d8835589 100644 --- a/test/unit/data-connect/data-connect-api-client-internal.spec.ts +++ b/test/unit/data-connect/data-connect-api-client-internal.spec.ts @@ -2,14 +2,14 @@ * @license * Copyright 2024 Google Inc. * - * Licensed under the Apache License, Version 2.0 (the "License"); + * 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, + * 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. @@ -260,15 +260,21 @@ describe('DataConnectApiClient CRUD helpers', () => { describe('insert()', () => { it('should call executeGraphql with the correct mutation for simple data', async () => { const simpleData = { name: 'test', value: 123 }; - const expectedMutation = `mutation { ${testTableName}_insert(data: { name: "test", value: 123 }) }`; + const expectedMutation = `mutation { ${testTableName}_insert(data: { name: 'test', value: 123 }) }`; await apiClient.insert(testTableName, simpleData); expect(executeGraphqlStub).to.have.been.calledOnceWithExactly(expectedMutation); }); it('should call executeGraphql with the correct mutation for complex data', async () => { - const complexData = { id: 'abc', active: true, scores: [10, 20], info: { nested: "yes/no \"quote\" \\slash\\" } }; - // Note: Need to match the specific escaping from objectToString: / -> \\, " -> \" - const expectedMutation = `mutation { ${testTableName}_insert(data: { id: "abc", active: true, scores: [10, 20], info: { nested: "yes/no \\"quote\\" \\\\slash\\\\" } }) }`; + const complexData = { id: 'abc', active: true, scores: [10, 20], info: { nested: 'yes/no "quote" \\slash\\' } }; + // Note: Need to match the specific escaping from objectToString: / -> \\, ' -> \' + const expectedMutation = ` + mutation { + ${testTableName}_insert(data: { + id: 'abc', active: true, scores: [10, 20], + info: { nested: 'yes/no \\'quote\\' \\\\slash\\\\' } + }) + }`; await apiClient.insert(testTableName, complexData); expect(executeGraphqlStub).to.have.been.calledOnceWithExactly(expectedMutation); }); @@ -296,7 +302,9 @@ describe('DataConnectApiClient CRUD helpers', () => { describe('insertMany()', () => { it('should call executeGraphql with the correct mutation for simple data array', async () => { const simpleDataArray = [{ name: 'test1' }, { name: 'test2', value: 456 }]; - const expectedMutation = `mutation { ${testTableName}_insertMany(data: [{ name: "test1" }, { name: "test2", value: 456 }]) }`; + const expectedMutation = ` + mutation { + ${testTableName}_insertMany(data: [{ name: 'test1' }, { name: 'test2', value: 456 }]) }`; await apiClient.insertMany(testTableName, simpleDataArray); expect(executeGraphqlStub).to.have.been.calledOnceWithExactly(expectedMutation); }); @@ -304,10 +312,14 @@ describe('DataConnectApiClient CRUD helpers', () => { it('should call executeGraphql with the correct mutation for complex data array', async () => { const complexDataArray = [ { id: 'a', active: true, info: { nested: 'n1' } }, - { id: 'b', scores: [1, 2], info: { nested: "n2/\\" } } + { id: 'b', scores: [1, 2], info: { nested: 'n2/\\' } } ]; - // Note: Matching specific escaping: / -> \\, " -> \" - const expectedMutation = `mutation { ${testTableName}_insertMany(data: [{ id: "a", active: true, info: { nested: "n1" } }, { id: "b", scores: [1, 2], info: { nested: "n2/\\\\" } }]) }`; + // Note: Matching specific escaping: / -> \\, ' -> \' + const expectedMutation = ` + mutation { + ${testTableName}_insertMany(data: + [{ id: 'a', active: true, info: { nested: 'n1' } }, { id: 'b', scores: [1, 2], + info: { nested: 'n2/\\\\' } }]) }`; await apiClient.insertMany(testTableName, complexDataArray); expect(executeGraphqlStub).to.have.been.calledOnceWithExactly(expectedMutation); }); @@ -330,7 +342,7 @@ describe('DataConnectApiClient CRUD helpers', () => { .with.property('code', 'data-connect/invalid-argument'); }); - it('should throw FirebaseDataConnectError for non-array data', () => { + it('should throw FirebaseDataConnectError for non-array data', () => { expect(() => apiClient.insertMany(testTableName, { data: 1 } as any)) .to.throw(FirebaseDataConnectError, /`data` must be a non-empty array for insertMany./) .with.property('code', 'data-connect/invalid-argument'); @@ -341,15 +353,17 @@ describe('DataConnectApiClient CRUD helpers', () => { describe('upsert()', () => { it('should call executeGraphql with the correct mutation for simple data', async () => { const simpleData = { id: 'key1', value: 'updated' }; - const expectedMutation = `mutation { ${testTableName}_upsert(data: { id: "key1", value: "updated" }) }`; + const expectedMutation = `mutation { ${testTableName}_upsert(data: { id: 'key1', value: 'updated' }) }`; await apiClient.upsert(testTableName, simpleData); expect(executeGraphqlStub).to.have.been.calledOnceWithExactly(expectedMutation); }); it('should call executeGraphql with the correct mutation for complex data', async () => { - const complexData = { id: 'key2', active: false, items: [1, null], detail: { status: "done/\\" } }; - // Note: Matching specific escaping: / -> \\, " -> \" - const expectedMutation = `mutation { ${testTableName}_upsert(data: { id: "key2", active: false, items: [1, null], detail: { status: "done/\\\\" } }) }`; + const complexData = { id: 'key2', active: false, items: [1, null], detail: { status: 'done/\\' } }; + // Note: Matching specific escaping: / -> \\, ' -> \' + const expectedMutation = ` + mutation { ${testTableName}_upsert(data: + { id: 'key2', active: false, items: [1, null], detail: { status: 'done/\\\\' } }) }`; await apiClient.upsert(testTableName, complexData); expect(executeGraphqlStub).to.have.been.calledOnceWithExactly(expectedMutation); }); @@ -377,7 +391,8 @@ describe('DataConnectApiClient CRUD helpers', () => { describe('upsertMany()', () => { it('should call executeGraphql with the correct mutation for simple data array', async () => { const simpleDataArray = [{ id: 'k1' }, { id: 'k2', value: 99 }]; - const expectedMutation = `mutation { ${testTableName}_upsertMany(data: [{ id: "k1" }, { id: "k2", value: 99 }]) }`; + const expectedMutation = ` + mutation { ${testTableName}_upsertMany(data: [{ id: 'k1' }, { id: 'k2', value: 99 }]) }`; await apiClient.upsertMany(testTableName, simpleDataArray); expect(executeGraphqlStub).to.have.been.calledOnceWithExactly(expectedMutation); }); @@ -387,8 +402,10 @@ describe('DataConnectApiClient CRUD helpers', () => { { id: 'x', active: true, info: { nested: 'n1/\\"x' } }, { id: 'y', scores: [null, 2] } ]; - // Note: Matching specific escaping: / -> \\, " -> \" - const expectedMutation = `mutation { ${testTableName}_upsertMany(data: [{ id: "x", active: true, info: { nested: "n1/\\\\\\"x" } }, { id: "y", scores: [null, 2] }]) }`; + // Note: Matching specific escaping: / -> \\, ' -> \' + const expectedMutation = ` + mutation { ${testTableName}_upsertMany(data: + [{ id: 'x', active: true, info: { nested: 'n1/\\\\\\'x' } }, { id: 'y', scores: [null, 2] }]) }`; await apiClient.upsertMany(testTableName, complexDataArray); expect(executeGraphqlStub).to.have.been.calledOnceWithExactly(expectedMutation); }); diff --git a/test/unit/data-connect/index.spec.ts b/test/unit/data-connect/index.spec.ts index 8c8a7984c0..3f8c888967 100644 --- a/test/unit/data-connect/index.spec.ts +++ b/test/unit/data-connect/index.spec.ts @@ -25,7 +25,7 @@ import * as chaiAsPromised from 'chai-as-promised'; import * as mocks from '../../resources/mocks'; import { App } from '../../../src/app/index'; import { getDataConnect, DataConnect } from '../../../src/data-connect/index'; -import { DataConnectApiClient, FirebaseDataConnectError } from '../../../src/data-connect/data-connect-api-client-internal'; +import { DataConnectApiClient } from '../../../src/data-connect/data-connect-api-client-internal'; chai.should(); chai.use(sinonChai); From 777e30f19f0737a83bcfc38bf7abb8e7ead3ab15 Mon Sep 17 00:00:00 2001 From: Lahiru Maramba Date: Fri, 25 Apr 2025 15:45:04 -0400 Subject: [PATCH 03/11] update api changes --- etc/firebase-admin.data-connect.api.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/etc/firebase-admin.data-connect.api.md b/etc/firebase-admin.data-connect.api.md index 3650368734..2284d14b65 100644 --- a/etc/firebase-admin.data-connect.api.md +++ b/etc/firebase-admin.data-connect.api.md @@ -29,6 +29,14 @@ export class DataConnect { executeGraphql(query: string, options?: GraphqlOptions): Promise>; // @beta executeGraphqlRead(query: string, options?: GraphqlOptions): Promise>; + // @beta + insert(tableName: string, variables: Variables): Promise>; + // @beta + insertMany>(tableName: string, variables: Variables): Promise>; + // @beta + upsert(tableName: string, variables: Variables): Promise>; + // @beta + upsertMany>(tableName: string, variables: Variables): Promise>; } // @public From 4361fb8b00c19a600448c8d1aed7b0d491333408 Mon Sep 17 00:00:00 2001 From: Lahiru Maramba Date: Mon, 5 May 2025 13:17:42 -0400 Subject: [PATCH 04/11] clean up the new api --- .../data-connect-api-client-internal.ts | 76 +++++++++---------- src/data-connect/data-connect.ts | 37 +++++---- 2 files changed, 53 insertions(+), 60 deletions(-) diff --git a/src/data-connect/data-connect-api-client-internal.ts b/src/data-connect/data-connect-api-client-internal.ts index c55cea5171..c87fe7af78 100644 --- a/src/data-connect/data-connect-api-client-internal.ts +++ b/src/data-connect/data-connect-api-client-internal.ts @@ -25,36 +25,6 @@ import * as utils from '../utils/index'; import * as validator from '../utils/validator'; import { ConnectorConfig, ExecuteGraphqlResponse, GraphqlOptions } from './data-connect-api'; -/** - * Converts a JavaScript value into a GraphQL literal string. - * Handles nested objects, arrays, strings, numbers, and booleans. - * Ensures strings are properly escaped. - */ -function objectToString(data: any): string { - if (typeof data !== 'object' || data === null) { - if (typeof data === 'string') { - // Properly escape double quotes and backslashes within strings - const escapedString = data.replace(/\//g, '\\').replace(/,"/g, '"'); - return `"${escapedString}"`; - } - // Handle numbers, booleans, null directly - return String(data); - } - - if (Array.isArray(data)) { - const elements = data.map(item => objectToString(item)).join(', '); - return `[${elements}]`; - } - - // Handle plain objects - const entries = Object.entries(data).map(([key, value]) => { - // GraphQL object keys are typically unquoted identifiers - return `${key}: ${objectToString(value)}`; - }); - - return `{ ${entries.join(', ')} }`; -} - const API_VERSION = 'v1alpha'; /** The Firebase Data Connect backend base URL format. */ @@ -229,9 +199,38 @@ export class DataConnectApiClient { return new FirebaseDataConnectError(code, message); } + /** + * Converts JSON data into a GraphQL literal string. + * Handles nested objects, arrays, strings, numbers, and booleans. + * Ensures strings are properly escaped. + */ + private objectToString(data: any): string { + if (typeof data !== 'object' || data === null) { + if (typeof data === 'string') { + // Properly escape double quotes and backslashes within strings + const escapedString = data.replace(/\//g, '\\').replace(/,"/g, '"'); + return `"${escapedString}"`; + } + // Handle numbers, booleans, null directly + return String(data); + } + + if (validator.isArray(data)) { + const elements = data.map(item => this.objectToString(item)).join(', '); + return `[${elements}]`; + } + + // Handle plain objects + const entries = Object.entries(data).map(([key, value]) => { + // GraphQL object keys are typically unquoted identifiers + return `${key}: ${this.objectToString(value)}`; + }); + + return `{ ${entries.join(', ')} }`; + } + /** * Insert a single row into the specified table. - * (Implementation moved from DataConnect class) */ public async insert( tableName: string, @@ -243,13 +242,13 @@ export class DataConnectApiClient { if (!validator.isNonNullObject(data)) { throw new FirebaseDataConnectError('invalid-argument', '`data` must be a non-null object.'); } - if (Array.isArray(data)) { + if (validator.isArray(data)) { throw new FirebaseDataConnectError( 'invalid-argument', '`data` must be an object, not an array, for single insert.'); } try { - const gqlDataString = objectToString(data); + const gqlDataString = this.objectToString(data); const mutation = `mutation { ${tableName}_insert(data: ${gqlDataString}) }`; // Use internal executeGraphql return this.executeGraphql(mutation); @@ -260,7 +259,6 @@ export class DataConnectApiClient { /** * Insert multiple rows into the specified table. - * (Implementation moved from DataConnect class) */ public async insertMany>( tableName: string, @@ -274,7 +272,7 @@ export class DataConnectApiClient { } try { - const gqlDataString = objectToString(data); + const gqlDataString = this.objectToString(data); const mutation = `mutation { ${tableName}_insertMany(data: ${gqlDataString}) }`; // Use internal executeGraphql return this.executeGraphql(mutation); @@ -285,7 +283,6 @@ export class DataConnectApiClient { /** * Insert a single row into the specified table, or update it if it already exists. - * (Implementation moved from DataConnect class) */ public async upsert( tableName: string, @@ -297,13 +294,13 @@ export class DataConnectApiClient { if (!validator.isNonNullObject(data)) { throw new FirebaseDataConnectError('invalid-argument', '`data` must be a non-null object.'); } - if (Array.isArray(data)) { + if (validator.isArray(data)) { throw new FirebaseDataConnectError( 'invalid-argument', '`data` must be an object, not an array, for single upsert.'); } try { - const gqlDataString = objectToString(data); + const gqlDataString = this.objectToString(data); const mutation = `mutation { ${tableName}_upsert(data: ${gqlDataString}) }`; // Use internal executeGraphql return this.executeGraphql(mutation); @@ -314,7 +311,6 @@ export class DataConnectApiClient { /** * Insert multiple rows into the specified table, or update them if they already exist. - * (Implementation moved from DataConnect class) */ public async upsertMany>( tableName: string, @@ -328,7 +324,7 @@ export class DataConnectApiClient { } try { - const gqlDataString = objectToString(data); + const gqlDataString = this.objectToString(data); const mutation = `mutation { ${tableName}_upsertMany(data: ${gqlDataString}) }`; // Use internal executeGraphql return this.executeGraphql(mutation); diff --git a/src/data-connect/data-connect.ts b/src/data-connect/data-connect.ts index afd6120d20..671bfe4b73 100644 --- a/src/data-connect/data-connect.ts +++ b/src/data-connect/data-connect.ts @@ -17,7 +17,6 @@ import { App } from '../app'; import { DataConnectApiClient } from './data-connect-api-client-internal'; -// FirebaseDataConnectError, objectToString, and validator are no longer needed here import { ConnectorConfig, @@ -47,10 +46,10 @@ export class DataConnectService { } /** - * Returns the app associated with this `DataConnectService` instance. - * - * @returns The app associated with this `DataConnectService` instance. - */ + * Returns the app associated with this `DataConnectService` instance. + * + * @returns The app associated with this `DataConnectService` instance. + */ get app(): App { return this.appInternal; } @@ -64,24 +63,24 @@ export class DataConnect { private readonly client: DataConnectApiClient; /** - * @param connectorConfig - The connector configuration. - * @param app - The app for this `DataConnect` service. - * @constructor - * @internal - */ + * @param connectorConfig - The connector configuration. + * @param app - The app for this `DataConnect` service. + * @constructor + * @internal + */ constructor(readonly connectorConfig: ConnectorConfig, readonly app: App) { this.client = new DataConnectApiClient(connectorConfig, app); } /** - * Execute an arbitrary GraphQL query or mutation - * - * @param query - The GraphQL query or mutation. - * @param options - Optional {@link GraphqlOptions} when executing a GraphQL query or mutation. - * - * @returns A promise that fulfills with a `ExecuteGraphqlResponse`. - * @beta - */ + * Execute an arbitrary GraphQL query or mutation + * + * @param query - The GraphQL query or mutation. + * @param options - Optional {@link GraphqlOptions} when executing a GraphQL query or mutation. + * + * @returns A promise that fulfills with a `ExecuteGraphqlResponse`. + * @beta + */ public executeGraphql( query: string, options?: GraphqlOptions, @@ -137,7 +136,6 @@ export class DataConnect { /** * Insert a single row into the specified table, or update it if it already exists. - * The specific behavior (insert or update) depends on the underlying database and schema configuration. * * @param tableName - The name of the table to upsert data into. * @param variables - The data object to upsert. The keys should correspond to the column names. @@ -153,7 +151,6 @@ export class DataConnect { /** * Insert multiple rows into the specified table, or update them if they already exist. - * The specific behavior (insert or update) depends on the underlying database and schema configuration. * * @param tableName - The name of the table to upsert data into. * @param variables - An array of data objects to upsert. Each object's keys should correspond to the column names. From 2f62f08d972466eb0af38cce1db6411d0237f32d Mon Sep 17 00:00:00 2001 From: Lahiru Maramba Date: Mon, 5 May 2025 21:14:02 -0400 Subject: [PATCH 05/11] clean up unit tests --- .../data-connect-api-client-internal.ts | 71 ++++--- .../data-connect-api-client-internal.spec.ts | 185 ++++++++++-------- test/unit/index.spec.ts | 4 + 3 files changed, 155 insertions(+), 105 deletions(-) diff --git a/src/data-connect/data-connect-api-client-internal.ts b/src/data-connect/data-connect-api-client-internal.ts index c87fe7af78..f7e8de3e96 100644 --- a/src/data-connect/data-connect-api-client-internal.ts +++ b/src/data-connect/data-connect-api-client-internal.ts @@ -199,34 +199,51 @@ export class DataConnectApiClient { return new FirebaseDataConnectError(code, message); } - /** - * Converts JSON data into a GraphQL literal string. - * Handles nested objects, arrays, strings, numbers, and booleans. - * Ensures strings are properly escaped. - */ - private objectToString(data: any): string { - if (typeof data !== 'object' || data === null) { - if (typeof data === 'string') { - // Properly escape double quotes and backslashes within strings - const escapedString = data.replace(/\//g, '\\').replace(/,"/g, '"'); - return `"${escapedString}"`; - } - // Handle numbers, booleans, null directly + /** + * Converts JSON data into a GraphQL literal string. + * Handles nested objects, arrays, strings, numbers, and booleans. + * Ensures strings are properly escaped. + */ + private objectToString(data: any): string { + if (typeof data === 'string') { + const escapedString = data + .replace(/\\/g, '\\\\') // Replace \ with \\ + .replace(/"/g, '\\"'); // Replace " with \" + return `"${escapedString}"`; + } + if (typeof data === 'number' || typeof data === 'boolean' || data === null) { return String(data); } - if (validator.isArray(data)) { const elements = data.map(item => this.objectToString(item)).join(', '); return `[${elements}]`; } + if (typeof data === 'object' && data !== null) { + // Filter out properties where the value is undefined BEFORE mapping + const kvPairs = Object.entries(data) + .filter(([, val]) => val !== undefined) + .map(([key, val]) => { + // GraphQL object keys are typically unquoted. + return `${key}: ${this.objectToString(val)}`; + }); + + if (kvPairs.length === 0) { + return '{}'; // Represent an object with no defined properties as {} + } + return `{ ${kvPairs.join(', ')} }`; + } + + // If value is undefined (and not an object property, which is handled above, + // e.g., if objectToString(undefined) is called directly or for an array element) + // it should be represented as 'null'. + if (typeof data === 'undefined') { + return 'null'; + } - // Handle plain objects - const entries = Object.entries(data).map(([key, value]) => { - // GraphQL object keys are typically unquoted identifiers - return `${key}: ${this.objectToString(value)}`; - }); - - return `{ ${entries.join(', ')} }`; + // Fallback for any other types (e.g., Symbol, BigInt - though less common in GQL contexts) + // Consider how these should be handled or if an error should be thrown. + // For now, simple string conversion. + return String(data); } /** @@ -239,13 +256,13 @@ export class DataConnectApiClient { if (!validator.isNonEmptyString(tableName)) { throw new FirebaseDataConnectError('invalid-argument', '`tableName` must be a non-empty string.'); } - if (!validator.isNonNullObject(data)) { - throw new FirebaseDataConnectError('invalid-argument', '`data` must be a non-null object.'); - } if (validator.isArray(data)) { throw new FirebaseDataConnectError( 'invalid-argument', '`data` must be an object, not an array, for single insert.'); } + if (!validator.isNonNullObject(data)) { + throw new FirebaseDataConnectError('invalid-argument', '`data` must be a non-null object.'); + } try { const gqlDataString = this.objectToString(data); @@ -291,13 +308,13 @@ export class DataConnectApiClient { if (!validator.isNonEmptyString(tableName)) { throw new FirebaseDataConnectError('invalid-argument', '`tableName` must be a non-empty string.'); } - if (!validator.isNonNullObject(data)) { - throw new FirebaseDataConnectError('invalid-argument', '`data` must be a non-null object.'); - } if (validator.isArray(data)) { throw new FirebaseDataConnectError( 'invalid-argument', '`data` must be an object, not an array, for single upsert.'); } + if (!validator.isNonNullObject(data)) { + throw new FirebaseDataConnectError('invalid-argument', '`data` must be a non-null object.'); + } try { const gqlDataString = this.objectToString(data); diff --git a/test/unit/data-connect/data-connect-api-client-internal.spec.ts b/test/unit/data-connect/data-connect-api-client-internal.spec.ts index 89d8835589..86e5758363 100644 --- a/test/unit/data-connect/data-connect-api-client-internal.spec.ts +++ b/test/unit/data-connect/data-connect-api-client-internal.spec.ts @@ -2,14 +2,14 @@ * @license * Copyright 2024 Google Inc. * - * Licensed under the Apache License, Version 2.0 (the 'License'); + * 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, + * 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. @@ -46,6 +46,12 @@ describe('DataConnectApiClient', () => { 'X-Goog-Api-Client': getMetricsHeader(), }; + const EMULATOR_EXPECTED_HEADERS = { + 'Authorization': 'Bearer owner', + 'X-Firebase-Client': `fire-admin-node/${getSdkVersion()}`, + 'X-Goog-Api-Client': getMetricsHeader(), + }; + const noProjectId = 'Failed to determine project ID. Initialize the SDK with service ' + 'account credentials or set project ID as an app option. Alternatively, set the ' + 'GOOGLE_CLOUD_PROJECT environment variable.'; @@ -218,7 +224,7 @@ describe('DataConnectApiClient', () => { expect(stub).to.have.been.calledOnce.and.calledWith({ method: 'POST', url: `http://localhost:9399/v1alpha/projects/test-project/locations/${connectorConfig.location}/services/${connectorConfig.serviceId}:executeGraphql`, - headers: EXPECTED_HEADERS, + headers: EMULATOR_EXPECTED_HEADERS, data: { query: 'query' } }); }); @@ -226,7 +232,6 @@ describe('DataConnectApiClient', () => { }); }); -// New test suite for the CRUD helpers moved into the ApiClient describe('DataConnectApiClient CRUD helpers', () => { let mockApp: FirebaseApp; let apiClient: DataConnectApiClient; @@ -244,6 +249,14 @@ describe('DataConnectApiClient CRUD helpers', () => { const testTableName = 'TestTable'; + // Helper function to normalize GraphQL strings + const normalizeGraphQLString = (str: string): string => { + return str + .replace(/\s*\n\s*/g, '\n') // Remove leading/trailing whitespace around newlines + .replace(/\s+/g, ' ') // Replace multiple spaces with a single space + .trim(); // Remove leading/trailing whitespace from the whole string + }; + beforeEach(() => { mockApp = mocks.appWithOptions(mockOptions); apiClient = new DataConnectApiClient(connectorConfig, mockApp); @@ -260,41 +273,70 @@ describe('DataConnectApiClient CRUD helpers', () => { describe('insert()', () => { it('should call executeGraphql with the correct mutation for simple data', async () => { const simpleData = { name: 'test', value: 123 }; - const expectedMutation = `mutation { ${testTableName}_insert(data: { name: 'test', value: 123 }) }`; + const expectedMutation = ` + mutation { + ${testTableName}_insert(data: { + name: "test", + value: 123 + }) + }`; await apiClient.insert(testTableName, simpleData); - expect(executeGraphqlStub).to.have.been.calledOnceWithExactly(expectedMutation); + expect(executeGraphqlStub).to.have.been.calledOnceWithExactly(normalizeGraphQLString(expectedMutation)); }); it('should call executeGraphql with the correct mutation for complex data', async () => { const complexData = { id: 'abc', active: true, scores: [10, 20], info: { nested: 'yes/no "quote" \\slash\\' } }; - // Note: Need to match the specific escaping from objectToString: / -> \\, ' -> \' const expectedMutation = ` mutation { ${testTableName}_insert(data: { - id: 'abc', active: true, scores: [10, 20], - info: { nested: 'yes/no \\'quote\\' \\\\slash\\\\' } + id: "abc", active: true, scores: [10, 20], + info: { nested: "yes/no \\"quote\\" \\\\slash\\\\" } }) }`; await apiClient.insert(testTableName, complexData); - expect(executeGraphqlStub).to.have.been.calledOnceWithExactly(expectedMutation); + expect(executeGraphqlStub).to.have.been.calledOnceWithExactly(normalizeGraphQLString(expectedMutation)); + }); + + it('should call executeGraphql with the correct mutation for undefined and null values', async () => { + const dataWithUndefined = { + genre: 'Action', + title: 'Die Hard', + ratings: null, + director: { + name: undefined, + age: undefined + }, + notes: undefined, + releaseYear: undefined, + extras: [1, undefined, 'hello', undefined, { a: 1, b: undefined }] + }; + const expectedMutation = ` + mutation { + ${testTableName}_insert(data: { + genre: "Action", + title: "Die Hard", + ratings: null, + director: {}, + extras: [1, null, "hello", null, { a: 1 }] + }) + }`; + await apiClient.insert(testTableName, dataWithUndefined); + expect(executeGraphqlStub).to.have.been.calledOnceWithExactly(normalizeGraphQLString(expectedMutation)); }); - it('should throw FirebaseDataConnectError for invalid tableName', () => { - expect(() => apiClient.insert('', { data: 1 })) - .to.throw(FirebaseDataConnectError, /`tableName` must be a non-empty string./) - .with.property('code', 'data-connect/invalid-argument'); + it('should throw FirebaseDataConnectError for invalid tableName', async () => { + await expect(apiClient.insert('', { data: 1 })) + .to.be.rejectedWith(FirebaseDataConnectError, /`tableName` must be a non-empty string./); }); - it('should throw FirebaseDataConnectError for null data', () => { - expect(() => apiClient.insert(testTableName, null as any)) - .to.throw(FirebaseDataConnectError, /`data` must be a non-null object./) - .with.property('code', 'data-connect/invalid-argument'); + it('should throw FirebaseDataConnectError for null data', async () => { + await expect(apiClient.insert(testTableName, null as any)) + .to.be.rejectedWith(FirebaseDataConnectError, /`data` must be a non-null object./); }); - it('should throw FirebaseDataConnectError for array data', () => { - expect(() => apiClient.insert(testTableName, [])) - .to.throw(FirebaseDataConnectError, /`data` must be an object, not an array, for single insert./) - .with.property('code', 'data-connect/invalid-argument'); + it('should throw FirebaseDataConnectError for array data', async() => { + await expect(apiClient.insert(testTableName, [])) + .to.be.rejectedWith(FirebaseDataConnectError, /`data` must be an object, not an array, for single insert./); }); }); @@ -304,48 +346,43 @@ describe('DataConnectApiClient CRUD helpers', () => { const simpleDataArray = [{ name: 'test1' }, { name: 'test2', value: 456 }]; const expectedMutation = ` mutation { - ${testTableName}_insertMany(data: [{ name: 'test1' }, { name: 'test2', value: 456 }]) }`; + ${testTableName}_insertMany(data: [{ name: "test1" }, { name: "test2", value: 456 }]) }`; await apiClient.insertMany(testTableName, simpleDataArray); - expect(executeGraphqlStub).to.have.been.calledOnceWithExactly(expectedMutation); + expect(executeGraphqlStub).to.have.been.calledOnceWithExactly(normalizeGraphQLString(expectedMutation)); }); it('should call executeGraphql with the correct mutation for complex data array', async () => { const complexDataArray = [ - { id: 'a', active: true, info: { nested: 'n1' } }, + { id: 'a', active: true, info: { nested: 'n1 "quote"' } }, { id: 'b', scores: [1, 2], info: { nested: 'n2/\\' } } ]; - // Note: Matching specific escaping: / -> \\, ' -> \' const expectedMutation = ` mutation { ${testTableName}_insertMany(data: - [{ id: 'a', active: true, info: { nested: 'n1' } }, { id: 'b', scores: [1, 2], - info: { nested: 'n2/\\\\' } }]) }`; + [{ id: "a", active: true, info: { nested: "n1 \\"quote\\"" } }, { id: "b", scores: [1, 2], + info: { nested: "n2/\\\\" } }]) }`; await apiClient.insertMany(testTableName, complexDataArray); - expect(executeGraphqlStub).to.have.been.calledOnceWithExactly(expectedMutation); + expect(executeGraphqlStub).to.have.been.calledOnceWithExactly(normalizeGraphQLString(expectedMutation)); }); - it('should throw FirebaseDataConnectError for invalid tableName', () => { - expect(() => apiClient.insertMany('', [{ data: 1 }])) - .to.throw(FirebaseDataConnectError, /`tableName` must be a non-empty string./) - .with.property('code', 'data-connect/invalid-argument'); + it('should throw FirebaseDataConnectError for invalid tableName', async () => { + await expect(apiClient.insertMany('', [{ data: 1 }])) + .to.be.rejectedWith(FirebaseDataConnectError, /`tableName` must be a non-empty string./); }); it('should throw FirebaseDataConnectError for null data', () => { - expect(() => apiClient.insertMany(testTableName, null as any)) - .to.throw(FirebaseDataConnectError, /`data` must be a non-empty array for insertMany./) - .with.property('code', 'data-connect/invalid-argument'); + expect(apiClient.insertMany(testTableName, null as any)) + .to.be.rejectedWith(FirebaseDataConnectError, /`data` must be a non-empty array for insertMany./); }); it('should throw FirebaseDataConnectError for empty array data', () => { - expect(() => apiClient.insertMany(testTableName, [])) - .to.throw(FirebaseDataConnectError, /`data` must be a non-empty array for insertMany./) - .with.property('code', 'data-connect/invalid-argument'); + expect(apiClient.insertMany(testTableName, [])) + .to.be.rejectedWith(FirebaseDataConnectError, /`data` must be a non-empty array for insertMany./); }); it('should throw FirebaseDataConnectError for non-array data', () => { - expect(() => apiClient.insertMany(testTableName, { data: 1 } as any)) - .to.throw(FirebaseDataConnectError, /`data` must be a non-empty array for insertMany./) - .with.property('code', 'data-connect/invalid-argument'); + expect(apiClient.insertMany(testTableName, { data: 1 } as any)) + .to.be.rejectedWith(FirebaseDataConnectError, /`data` must be a non-empty array for insertMany./); }); }); @@ -353,7 +390,7 @@ describe('DataConnectApiClient CRUD helpers', () => { describe('upsert()', () => { it('should call executeGraphql with the correct mutation for simple data', async () => { const simpleData = { id: 'key1', value: 'updated' }; - const expectedMutation = `mutation { ${testTableName}_upsert(data: { id: 'key1', value: 'updated' }) }`; + const expectedMutation = `mutation { ${testTableName}_upsert(data: { id: "key1", value: "updated" }) }`; await apiClient.upsert(testTableName, simpleData); expect(executeGraphqlStub).to.have.been.calledOnceWithExactly(expectedMutation); }); @@ -363,27 +400,24 @@ describe('DataConnectApiClient CRUD helpers', () => { // Note: Matching specific escaping: / -> \\, ' -> \' const expectedMutation = ` mutation { ${testTableName}_upsert(data: - { id: 'key2', active: false, items: [1, null], detail: { status: 'done/\\\\' } }) }`; + { id: "key2", active: false, items: [1, null], detail: { status: "done/\\\\" } }) }`; await apiClient.upsert(testTableName, complexData); - expect(executeGraphqlStub).to.have.been.calledOnceWithExactly(expectedMutation); + expect(executeGraphqlStub).to.have.been.calledOnceWithExactly(normalizeGraphQLString(expectedMutation)); }); - it('should throw FirebaseDataConnectError for invalid tableName', () => { - expect(() => apiClient.upsert('', { data: 1 })) - .to.throw(FirebaseDataConnectError, /`tableName` must be a non-empty string./) - .with.property('code', 'data-connect/invalid-argument'); + it('should throw FirebaseDataConnectError for invalid tableName', async () => { + await expect(apiClient.upsert('', { data: 1 })) + .to.be.rejectedWith(FirebaseDataConnectError, /`tableName` must be a non-empty string./); }); - it('should throw FirebaseDataConnectError for null data', () => { - expect(() => apiClient.upsert(testTableName, null as any)) - .to.throw(FirebaseDataConnectError, /`data` must be a non-null object./) - .with.property('code', 'data-connect/invalid-argument'); + it('should throw FirebaseDataConnectError for null data', async () => { + await expect(apiClient.upsert(testTableName, null as any)) + .to.be.rejectedWith(FirebaseDataConnectError, /`data` must be a non-null object./); }); - it('should throw FirebaseDataConnectError for array data', () => { - expect(() => apiClient.upsert(testTableName, [{ data: 1 }])) - .to.throw(FirebaseDataConnectError, /`data` must be an object, not an array, for single upsert./) - .with.property('code', 'data-connect/invalid-argument'); + it('should throw FirebaseDataConnectError for array data', async () => { + await expect(apiClient.upsert(testTableName, [{ data: 1 }])) + .to.be.rejectedWith(FirebaseDataConnectError, /`data` must be an object, not an array, for single upsert./); }); }); @@ -392,9 +426,9 @@ describe('DataConnectApiClient CRUD helpers', () => { it('should call executeGraphql with the correct mutation for simple data array', async () => { const simpleDataArray = [{ id: 'k1' }, { id: 'k2', value: 99 }]; const expectedMutation = ` - mutation { ${testTableName}_upsertMany(data: [{ id: 'k1' }, { id: 'k2', value: 99 }]) }`; + mutation { ${testTableName}_upsertMany(data: [{ id: "k1" }, { id: "k2", value: 99 }]) }`; await apiClient.upsertMany(testTableName, simpleDataArray); - expect(executeGraphqlStub).to.have.been.calledOnceWithExactly(expectedMutation); + expect(executeGraphqlStub).to.have.been.calledOnceWithExactly(normalizeGraphQLString(expectedMutation)); }); it('should call executeGraphql with the correct mutation for complex data array', async () => { @@ -402,36 +436,31 @@ describe('DataConnectApiClient CRUD helpers', () => { { id: 'x', active: true, info: { nested: 'n1/\\"x' } }, { id: 'y', scores: [null, 2] } ]; - // Note: Matching specific escaping: / -> \\, ' -> \' const expectedMutation = ` mutation { ${testTableName}_upsertMany(data: - [{ id: 'x', active: true, info: { nested: 'n1/\\\\\\'x' } }, { id: 'y', scores: [null, 2] }]) }`; + [{ id: "x", active: true, info: { nested: "n1/\\\\\\"x" } }, { id: "y", scores: [null, 2] }]) }`; await apiClient.upsertMany(testTableName, complexDataArray); - expect(executeGraphqlStub).to.have.been.calledOnceWithExactly(expectedMutation); + expect(executeGraphqlStub).to.have.been.calledOnceWithExactly(normalizeGraphQLString(expectedMutation)); }); - it('should throw FirebaseDataConnectError for invalid tableName', () => { - expect(() => apiClient.upsertMany('', [{ data: 1 }])) - .to.throw(FirebaseDataConnectError, /`tableName` must be a non-empty string./) - .with.property('code', 'data-connect/invalid-argument'); + it('should throw FirebaseDataConnectError for invalid tableName', async () => { + expect(apiClient.upsertMany('', [{ data: 1 }])) + .to.be.rejectedWith(FirebaseDataConnectError, /`tableName` must be a non-empty string./); }); - it('should throw FirebaseDataConnectError for null data', () => { - expect(() => apiClient.upsertMany(testTableName, null as any)) - .to.throw(FirebaseDataConnectError, /`data` must be a non-empty array for upsertMany./) - .with.property('code', 'data-connect/invalid-argument'); + it('should throw FirebaseDataConnectError for null data', async () => { + expect(apiClient.upsertMany(testTableName, null as any)) + .to.be.rejectedWith(FirebaseDataConnectError, /`data` must be a non-empty array for upsertMany./); }); - it('should throw FirebaseDataConnectError for empty array data', () => { - expect(() => apiClient.upsertMany(testTableName, [])) - .to.throw(FirebaseDataConnectError, /`data` must be a non-empty array for upsertMany./) - .with.property('code', 'data-connect/invalid-argument'); + it('should throw FirebaseDataConnectError for empty array data', async () => { + expect(apiClient.upsertMany(testTableName, [])) + .to.be.rejectedWith(FirebaseDataConnectError, /`data` must be a non-empty array for upsertMany./); }); - it('should throw FirebaseDataConnectError for non-array data', () => { - expect(() => apiClient.upsertMany(testTableName, { data: 1 } as any)) - .to.throw(FirebaseDataConnectError, /`data` must be a non-empty array for upsertMany./) - .with.property('code', 'data-connect/invalid-argument'); + it('should throw FirebaseDataConnectError for non-array data', async () => { + await expect(apiClient.upsertMany(testTableName, { data: 1 } as any)) + .to.be.rejectedWith(FirebaseDataConnectError, /`data` must be a non-empty array for upsertMany./); }); }); }); diff --git a/test/unit/index.spec.ts b/test/unit/index.spec.ts index 53847f6d62..f6bfb1b39a 100644 --- a/test/unit/index.spec.ts +++ b/test/unit/index.spec.ts @@ -117,3 +117,7 @@ import './functions/functions-api-client-internal.spec'; // Extensions import './extensions/extensions.spec'; import './extensions/extensions-api-client-internal.spec'; + +// Data Connect +import './data-connect/index.spec'; +import './data-connect/data-connect-api-client-internal.spec'; From 21473eb3798d8b3dd0b7bf924898239eecd47ee5 Mon Sep 17 00:00:00 2001 From: Lahiru Maramba Date: Mon, 5 May 2025 21:46:00 -0400 Subject: [PATCH 06/11] format tableName --- .../data-connect-api-client-internal.ts | 12 ++++ .../data-connect-api-client-internal.spec.ts | 59 ++++++++++--------- test/unit/data-connect/index.spec.ts | 4 +- 3 files changed, 44 insertions(+), 31 deletions(-) diff --git a/src/data-connect/data-connect-api-client-internal.ts b/src/data-connect/data-connect-api-client-internal.ts index f7e8de3e96..483753e132 100644 --- a/src/data-connect/data-connect-api-client-internal.ts +++ b/src/data-connect/data-connect-api-client-internal.ts @@ -246,6 +246,14 @@ export class DataConnectApiClient { return String(data); } + private formatTableName(tableName: string): string { + // Format tableName: first character to lowercase + if (tableName && tableName.length > 0) { + return tableName.charAt(0).toLowerCase() + tableName.slice(1); + } + return tableName; + } + /** * Insert a single row into the specified table. */ @@ -264,6 +272,7 @@ export class DataConnectApiClient { throw new FirebaseDataConnectError('invalid-argument', '`data` must be a non-null object.'); } + tableName = this.formatTableName(tableName); try { const gqlDataString = this.objectToString(data); const mutation = `mutation { ${tableName}_insert(data: ${gqlDataString}) }`; @@ -288,6 +297,7 @@ export class DataConnectApiClient { throw new FirebaseDataConnectError('invalid-argument', '`data` must be a non-empty array for insertMany.'); } + tableName = this.formatTableName(tableName); try { const gqlDataString = this.objectToString(data); const mutation = `mutation { ${tableName}_insertMany(data: ${gqlDataString}) }`; @@ -316,6 +326,7 @@ export class DataConnectApiClient { throw new FirebaseDataConnectError('invalid-argument', '`data` must be a non-null object.'); } + tableName = this.formatTableName(tableName); try { const gqlDataString = this.objectToString(data); const mutation = `mutation { ${tableName}_upsert(data: ${gqlDataString}) }`; @@ -340,6 +351,7 @@ export class DataConnectApiClient { throw new FirebaseDataConnectError('invalid-argument', '`data` must be a non-empty array for upsertMany.'); } + tableName = this.formatTableName(tableName); try { const gqlDataString = this.objectToString(data); const mutation = `mutation { ${tableName}_upsertMany(data: ${gqlDataString}) }`; diff --git a/test/unit/data-connect/data-connect-api-client-internal.spec.ts b/test/unit/data-connect/data-connect-api-client-internal.spec.ts index 86e5758363..405aede0c4 100644 --- a/test/unit/data-connect/data-connect-api-client-internal.spec.ts +++ b/test/unit/data-connect/data-connect-api-client-internal.spec.ts @@ -247,7 +247,8 @@ describe('DataConnectApiClient CRUD helpers', () => { projectId: 'test-project-crud', }; - const testTableName = 'TestTable'; + const tableName = 'TestTable'; + const formatedTableName = 'testTable'; // Helper function to normalize GraphQL strings const normalizeGraphQLString = (str: string): string => { @@ -275,12 +276,12 @@ describe('DataConnectApiClient CRUD helpers', () => { const simpleData = { name: 'test', value: 123 }; const expectedMutation = ` mutation { - ${testTableName}_insert(data: { + ${formatedTableName}_insert(data: { name: "test", value: 123 }) }`; - await apiClient.insert(testTableName, simpleData); + await apiClient.insert(tableName, simpleData); expect(executeGraphqlStub).to.have.been.calledOnceWithExactly(normalizeGraphQLString(expectedMutation)); }); @@ -288,12 +289,12 @@ describe('DataConnectApiClient CRUD helpers', () => { const complexData = { id: 'abc', active: true, scores: [10, 20], info: { nested: 'yes/no "quote" \\slash\\' } }; const expectedMutation = ` mutation { - ${testTableName}_insert(data: { + ${formatedTableName}_insert(data: { id: "abc", active: true, scores: [10, 20], info: { nested: "yes/no \\"quote\\" \\\\slash\\\\" } }) }`; - await apiClient.insert(testTableName, complexData); + await apiClient.insert(tableName, complexData); expect(executeGraphqlStub).to.have.been.calledOnceWithExactly(normalizeGraphQLString(expectedMutation)); }); @@ -312,7 +313,7 @@ describe('DataConnectApiClient CRUD helpers', () => { }; const expectedMutation = ` mutation { - ${testTableName}_insert(data: { + ${formatedTableName}_insert(data: { genre: "Action", title: "Die Hard", ratings: null, @@ -320,7 +321,7 @@ describe('DataConnectApiClient CRUD helpers', () => { extras: [1, null, "hello", null, { a: 1 }] }) }`; - await apiClient.insert(testTableName, dataWithUndefined); + await apiClient.insert(tableName, dataWithUndefined); expect(executeGraphqlStub).to.have.been.calledOnceWithExactly(normalizeGraphQLString(expectedMutation)); }); @@ -330,12 +331,12 @@ describe('DataConnectApiClient CRUD helpers', () => { }); it('should throw FirebaseDataConnectError for null data', async () => { - await expect(apiClient.insert(testTableName, null as any)) + await expect(apiClient.insert(tableName, null as any)) .to.be.rejectedWith(FirebaseDataConnectError, /`data` must be a non-null object./); }); it('should throw FirebaseDataConnectError for array data', async() => { - await expect(apiClient.insert(testTableName, [])) + await expect(apiClient.insert(tableName, [])) .to.be.rejectedWith(FirebaseDataConnectError, /`data` must be an object, not an array, for single insert./); }); }); @@ -346,8 +347,8 @@ describe('DataConnectApiClient CRUD helpers', () => { const simpleDataArray = [{ name: 'test1' }, { name: 'test2', value: 456 }]; const expectedMutation = ` mutation { - ${testTableName}_insertMany(data: [{ name: "test1" }, { name: "test2", value: 456 }]) }`; - await apiClient.insertMany(testTableName, simpleDataArray); + ${formatedTableName}_insertMany(data: [{ name: "test1" }, { name: "test2", value: 456 }]) }`; + await apiClient.insertMany(tableName, simpleDataArray); expect(executeGraphqlStub).to.have.been.calledOnceWithExactly(normalizeGraphQLString(expectedMutation)); }); @@ -358,10 +359,10 @@ describe('DataConnectApiClient CRUD helpers', () => { ]; const expectedMutation = ` mutation { - ${testTableName}_insertMany(data: + ${formatedTableName}_insertMany(data: [{ id: "a", active: true, info: { nested: "n1 \\"quote\\"" } }, { id: "b", scores: [1, 2], info: { nested: "n2/\\\\" } }]) }`; - await apiClient.insertMany(testTableName, complexDataArray); + await apiClient.insertMany(tableName, complexDataArray); expect(executeGraphqlStub).to.have.been.calledOnceWithExactly(normalizeGraphQLString(expectedMutation)); }); @@ -371,17 +372,17 @@ describe('DataConnectApiClient CRUD helpers', () => { }); it('should throw FirebaseDataConnectError for null data', () => { - expect(apiClient.insertMany(testTableName, null as any)) + expect(apiClient.insertMany(tableName, null as any)) .to.be.rejectedWith(FirebaseDataConnectError, /`data` must be a non-empty array for insertMany./); }); it('should throw FirebaseDataConnectError for empty array data', () => { - expect(apiClient.insertMany(testTableName, [])) + expect(apiClient.insertMany(tableName, [])) .to.be.rejectedWith(FirebaseDataConnectError, /`data` must be a non-empty array for insertMany./); }); it('should throw FirebaseDataConnectError for non-array data', () => { - expect(apiClient.insertMany(testTableName, { data: 1 } as any)) + expect(apiClient.insertMany(tableName, { data: 1 } as any)) .to.be.rejectedWith(FirebaseDataConnectError, /`data` must be a non-empty array for insertMany./); }); }); @@ -390,8 +391,8 @@ describe('DataConnectApiClient CRUD helpers', () => { describe('upsert()', () => { it('should call executeGraphql with the correct mutation for simple data', async () => { const simpleData = { id: 'key1', value: 'updated' }; - const expectedMutation = `mutation { ${testTableName}_upsert(data: { id: "key1", value: "updated" }) }`; - await apiClient.upsert(testTableName, simpleData); + const expectedMutation = `mutation { ${formatedTableName}_upsert(data: { id: "key1", value: "updated" }) }`; + await apiClient.upsert(tableName, simpleData); expect(executeGraphqlStub).to.have.been.calledOnceWithExactly(expectedMutation); }); @@ -399,9 +400,9 @@ describe('DataConnectApiClient CRUD helpers', () => { const complexData = { id: 'key2', active: false, items: [1, null], detail: { status: 'done/\\' } }; // Note: Matching specific escaping: / -> \\, ' -> \' const expectedMutation = ` - mutation { ${testTableName}_upsert(data: + mutation { ${formatedTableName}_upsert(data: { id: "key2", active: false, items: [1, null], detail: { status: "done/\\\\" } }) }`; - await apiClient.upsert(testTableName, complexData); + await apiClient.upsert(tableName, complexData); expect(executeGraphqlStub).to.have.been.calledOnceWithExactly(normalizeGraphQLString(expectedMutation)); }); @@ -411,12 +412,12 @@ describe('DataConnectApiClient CRUD helpers', () => { }); it('should throw FirebaseDataConnectError for null data', async () => { - await expect(apiClient.upsert(testTableName, null as any)) + await expect(apiClient.upsert(tableName, null as any)) .to.be.rejectedWith(FirebaseDataConnectError, /`data` must be a non-null object./); }); it('should throw FirebaseDataConnectError for array data', async () => { - await expect(apiClient.upsert(testTableName, [{ data: 1 }])) + await expect(apiClient.upsert(tableName, [{ data: 1 }])) .to.be.rejectedWith(FirebaseDataConnectError, /`data` must be an object, not an array, for single upsert./); }); }); @@ -426,8 +427,8 @@ describe('DataConnectApiClient CRUD helpers', () => { it('should call executeGraphql with the correct mutation for simple data array', async () => { const simpleDataArray = [{ id: 'k1' }, { id: 'k2', value: 99 }]; const expectedMutation = ` - mutation { ${testTableName}_upsertMany(data: [{ id: "k1" }, { id: "k2", value: 99 }]) }`; - await apiClient.upsertMany(testTableName, simpleDataArray); + mutation { ${formatedTableName}_upsertMany(data: [{ id: "k1" }, { id: "k2", value: 99 }]) }`; + await apiClient.upsertMany(tableName, simpleDataArray); expect(executeGraphqlStub).to.have.been.calledOnceWithExactly(normalizeGraphQLString(expectedMutation)); }); @@ -437,9 +438,9 @@ describe('DataConnectApiClient CRUD helpers', () => { { id: 'y', scores: [null, 2] } ]; const expectedMutation = ` - mutation { ${testTableName}_upsertMany(data: + mutation { ${formatedTableName}_upsertMany(data: [{ id: "x", active: true, info: { nested: "n1/\\\\\\"x" } }, { id: "y", scores: [null, 2] }]) }`; - await apiClient.upsertMany(testTableName, complexDataArray); + await apiClient.upsertMany(tableName, complexDataArray); expect(executeGraphqlStub).to.have.been.calledOnceWithExactly(normalizeGraphQLString(expectedMutation)); }); @@ -449,17 +450,17 @@ describe('DataConnectApiClient CRUD helpers', () => { }); it('should throw FirebaseDataConnectError for null data', async () => { - expect(apiClient.upsertMany(testTableName, null as any)) + expect(apiClient.upsertMany(tableName, null as any)) .to.be.rejectedWith(FirebaseDataConnectError, /`data` must be a non-empty array for upsertMany./); }); it('should throw FirebaseDataConnectError for empty array data', async () => { - expect(apiClient.upsertMany(testTableName, [])) + expect(apiClient.upsertMany(tableName, [])) .to.be.rejectedWith(FirebaseDataConnectError, /`data` must be a non-empty array for upsertMany./); }); it('should throw FirebaseDataConnectError for non-array data', async () => { - await expect(apiClient.upsertMany(testTableName, { data: 1 } as any)) + await expect(apiClient.upsertMany(tableName, { data: 1 } as any)) .to.be.rejectedWith(FirebaseDataConnectError, /`data` must be a non-empty array for upsertMany./); }); }); diff --git a/test/unit/data-connect/index.spec.ts b/test/unit/data-connect/index.spec.ts index 3f8c888967..b71670544b 100644 --- a/test/unit/data-connect/index.spec.ts +++ b/test/unit/data-connect/index.spec.ts @@ -87,7 +87,7 @@ describe('DataConnect', () => { }); }); -describe('DataConnect CRUD helpers delegation', () => { // Renamed suite for clarity +describe('DataConnect CRUD helpers delegation', () => { let mockApp: App; let dataConnect: DataConnect; // Stubs for the client methods @@ -105,7 +105,7 @@ describe('DataConnect CRUD helpers delegation', () => { // Renamed suite for cla beforeEach(() => { mockApp = mocks.app(); - // Instantiate DataConnect normally + dataConnect = getDataConnect(connectorConfig, mockApp); // Stub the DataConnectApiClient prototype methods From 4d6dea626ca323f3ab1d1e94170d7c17d5e9fcf3 Mon Sep 17 00:00:00 2001 From: Lahiru Maramba Date: Tue, 6 May 2025 10:57:55 -0400 Subject: [PATCH 07/11] add tests for formatted tableName --- .../data-connect-api-client-internal.spec.ts | 132 ++++++++++++++++-- 1 file changed, 119 insertions(+), 13 deletions(-) diff --git a/test/unit/data-connect/data-connect-api-client-internal.spec.ts b/test/unit/data-connect/data-connect-api-client-internal.spec.ts index 405aede0c4..a8f01eb479 100644 --- a/test/unit/data-connect/data-connect-api-client-internal.spec.ts +++ b/test/unit/data-connect/data-connect-api-client-internal.spec.ts @@ -250,6 +250,22 @@ describe('DataConnectApiClient CRUD helpers', () => { const tableName = 'TestTable'; const formatedTableName = 'testTable'; + const dataWithUndefined = { + genre: 'Action', + title: 'Die Hard', + ratings: null, + director: { + name: undefined, + age: undefined + }, + notes: undefined, + releaseYear: undefined, + extras: [1, undefined, 'hello', undefined, { a: 1, b: undefined }] + }; + + const tableNames = ['movie', 'Movie', 'MOVIE', 'toybox', 'toyBox', 'ToyBox', 'TOYBOX']; + const formatedTableNames = ['movie', 'movie', 'mOVIE', 'toybox', 'toyBox', 'toyBox', 'tOYBOX']; + // Helper function to normalize GraphQL strings const normalizeGraphQLString = (str: string): string => { return str @@ -272,6 +288,15 @@ describe('DataConnectApiClient CRUD helpers', () => { // --- INSERT TESTS --- describe('insert()', () => { + tableNames.forEach((tableName, index) => { + const expectedMutation = `mutation { ${formatedTableNames[index]}_insert(data: { name: "a" }) }`; + it(`should use the formatted tableName in the gql query: "${tableName}" as "${formatedTableNames[index]}"`, + async () => { + await apiClient.insert(tableName, { name: 'a' }); + await expect(executeGraphqlStub).to.have.been.calledOnceWithExactly(normalizeGraphQLString(expectedMutation)); + }); + }); + it('should call executeGraphql with the correct mutation for simple data', async () => { const simpleData = { name: 'test', value: 123 }; const expectedMutation = ` @@ -299,18 +324,6 @@ describe('DataConnectApiClient CRUD helpers', () => { }); it('should call executeGraphql with the correct mutation for undefined and null values', async () => { - const dataWithUndefined = { - genre: 'Action', - title: 'Die Hard', - ratings: null, - director: { - name: undefined, - age: undefined - }, - notes: undefined, - releaseYear: undefined, - extras: [1, undefined, 'hello', undefined, { a: 1, b: undefined }] - }; const expectedMutation = ` mutation { ${formatedTableName}_insert(data: { @@ -343,6 +356,15 @@ describe('DataConnectApiClient CRUD helpers', () => { // --- INSERT MANY TESTS --- describe('insertMany()', () => { + tableNames.forEach((tableName, index) => { + const expectedMutation = `mutation { ${formatedTableNames[index]}_insertMany(data: [{ name: "a" }]) }`; + it(`should use the formatted tableName in the gql query: "${tableName}" as "${formatedTableNames[index]}"`, + async () => { + await apiClient.insertMany(tableName, [{ name: 'a' }]); + await expect(executeGraphqlStub).to.have.been.calledOnceWithExactly(normalizeGraphQLString(expectedMutation)); + }); + }); + it('should call executeGraphql with the correct mutation for simple data array', async () => { const simpleDataArray = [{ name: 'test1' }, { name: 'test2', value: 456 }]; const expectedMutation = ` @@ -366,6 +388,32 @@ describe('DataConnectApiClient CRUD helpers', () => { expect(executeGraphqlStub).to.have.been.calledOnceWithExactly(normalizeGraphQLString(expectedMutation)); }); + it('should call executeGraphql with the correct mutation for undefined and null', async () => { + const dataArray = [ + dataWithUndefined, + dataWithUndefined + ] + const expectedMutation = ` + mutation { + ${formatedTableName}_insertMany(data: [{ + genre: "Action", + title: "Die Hard", + ratings: null, + director: {}, + extras: [1, null, "hello", null, { a: 1 }] + }, + { + genre: "Action", + title: "Die Hard", + ratings: null, + director: {}, + extras: [1, null, "hello", null, { a: 1 }] + }]) + }`; + await apiClient.insertMany(tableName, dataArray); + expect(executeGraphqlStub).to.have.been.calledOnceWithExactly(normalizeGraphQLString(expectedMutation)); + }); + it('should throw FirebaseDataConnectError for invalid tableName', async () => { await expect(apiClient.insertMany('', [{ data: 1 }])) .to.be.rejectedWith(FirebaseDataConnectError, /`tableName` must be a non-empty string./); @@ -389,6 +437,15 @@ describe('DataConnectApiClient CRUD helpers', () => { // --- UPSERT TESTS --- describe('upsert()', () => { + tableNames.forEach((tableName, index) => { + const expectedMutation = `mutation { ${formatedTableNames[index]}_upsert(data: { name: "a" }) }`; + it(`should use the formatted tableName in the gql query: "${tableName}" as "${formatedTableNames[index]}"`, + async () => { + await apiClient.upsert(tableName, { name: 'a' }); + await expect(executeGraphqlStub).to.have.been.calledOnceWithExactly(normalizeGraphQLString(expectedMutation)); + }); + }); + it('should call executeGraphql with the correct mutation for simple data', async () => { const simpleData = { id: 'key1', value: 'updated' }; const expectedMutation = `mutation { ${formatedTableName}_upsert(data: { id: "key1", value: "updated" }) }`; @@ -398,7 +455,6 @@ describe('DataConnectApiClient CRUD helpers', () => { it('should call executeGraphql with the correct mutation for complex data', async () => { const complexData = { id: 'key2', active: false, items: [1, null], detail: { status: 'done/\\' } }; - // Note: Matching specific escaping: / -> \\, ' -> \' const expectedMutation = ` mutation { ${formatedTableName}_upsert(data: { id: "key2", active: false, items: [1, null], detail: { status: "done/\\\\" } }) }`; @@ -406,6 +462,21 @@ describe('DataConnectApiClient CRUD helpers', () => { expect(executeGraphqlStub).to.have.been.calledOnceWithExactly(normalizeGraphQLString(expectedMutation)); }); + it('should call executeGraphql with the correct mutation for undefined and null values', async () => { + const expectedMutation = ` + mutation { + ${formatedTableName}_upsert(data: { + genre: "Action", + title: "Die Hard", + ratings: null, + director: {}, + extras: [1, null, "hello", null, { a: 1 }] + }) + }`; + await apiClient.upsert(tableName, dataWithUndefined); + expect(executeGraphqlStub).to.have.been.calledOnceWithExactly(normalizeGraphQLString(expectedMutation)); + }); + it('should throw FirebaseDataConnectError for invalid tableName', async () => { await expect(apiClient.upsert('', { data: 1 })) .to.be.rejectedWith(FirebaseDataConnectError, /`tableName` must be a non-empty string./); @@ -424,6 +495,15 @@ describe('DataConnectApiClient CRUD helpers', () => { // --- UPSERT MANY TESTS --- describe('upsertMany()', () => { + tableNames.forEach((tableName, index) => { + const expectedMutation = `mutation { ${formatedTableNames[index]}_upsertMany(data: [{ name: "a" }]) }`; + it(`should use the formatted tableName in the gql query: "${tableName}" as "${formatedTableNames[index]}"`, + async () => { + await apiClient.upsertMany(tableName, [{ name: 'a' }]); + await expect(executeGraphqlStub).to.have.been.calledOnceWithExactly(normalizeGraphQLString(expectedMutation)); + }); + }); + it('should call executeGraphql with the correct mutation for simple data array', async () => { const simpleDataArray = [{ id: 'k1' }, { id: 'k2', value: 99 }]; const expectedMutation = ` @@ -444,6 +524,32 @@ describe('DataConnectApiClient CRUD helpers', () => { expect(executeGraphqlStub).to.have.been.calledOnceWithExactly(normalizeGraphQLString(expectedMutation)); }); + it('should call executeGraphql with the correct mutation for undefined and null', async () => { + const dataArray = [ + dataWithUndefined, + dataWithUndefined + ] + const expectedMutation = ` + mutation { + ${formatedTableName}_upsertMany(data: [{ + genre: "Action", + title: "Die Hard", + ratings: null, + director: {}, + extras: [1, null, "hello", null, { a: 1 }] + }, + { + genre: "Action", + title: "Die Hard", + ratings: null, + director: {}, + extras: [1, null, "hello", null, { a: 1 }] + }]) + }`; + await apiClient.upsertMany(tableName, dataArray); + expect(executeGraphqlStub).to.have.been.calledOnceWithExactly(normalizeGraphQLString(expectedMutation)); + }); + it('should throw FirebaseDataConnectError for invalid tableName', async () => { expect(apiClient.upsertMany('', [{ data: 1 }])) .to.be.rejectedWith(FirebaseDataConnectError, /`tableName` must be a non-empty string./); From d6e9b6a5b8f84d0c0969ff22ae5be7a983bdb2dd Mon Sep 17 00:00:00 2001 From: Lahiru Maramba Date: Tue, 6 May 2025 12:40:04 -0400 Subject: [PATCH 08/11] Amend the error message with tableName checks for query errors --- .../data-connect-api-client-internal.ts | 78 +++++++++++++------ .../data-connect-api-client-internal.spec.ts | 34 +++++++- 2 files changed, 89 insertions(+), 23 deletions(-) diff --git a/src/data-connect/data-connect-api-client-internal.ts b/src/data-connect/data-connect-api-client-internal.ts index 483753e132..c99f244de2 100644 --- a/src/data-connect/data-connect-api-client-internal.ts +++ b/src/data-connect/data-connect-api-client-internal.ts @@ -254,6 +254,15 @@ export class DataConnectApiClient { return tableName; } + private handleBulkImportErrors(err: FirebaseDataConnectError): never { + if (err.code === `data-connect/${DATA_CONNECT_ERROR_CODE_MAPPING.QUERY_ERROR}`){ + throw new FirebaseDataConnectError( + DATA_CONNECT_ERROR_CODE_MAPPING.QUERY_ERROR, + `${err.message}. Make sure that your table name passed in matches the type name in your GraphQL schema file.`); + } + throw err; + } + /** * Insert a single row into the specified table. */ @@ -262,24 +271,31 @@ export class DataConnectApiClient { data: Variables, ): Promise> { if (!validator.isNonEmptyString(tableName)) { - throw new FirebaseDataConnectError('invalid-argument', '`tableName` must be a non-empty string.'); + throw new FirebaseDataConnectError( + DATA_CONNECT_ERROR_CODE_MAPPING.INVALID_ARGUMENT, + '`tableName` must be a non-empty string.'); } if (validator.isArray(data)) { throw new FirebaseDataConnectError( - 'invalid-argument', '`data` must be an object, not an array, for single insert.'); + DATA_CONNECT_ERROR_CODE_MAPPING.INVALID_ARGUMENT, + '`data` must be an object, not an array, for single insert.'); } if (!validator.isNonNullObject(data)) { - throw new FirebaseDataConnectError('invalid-argument', '`data` must be a non-null object.'); + throw new FirebaseDataConnectError( + DATA_CONNECT_ERROR_CODE_MAPPING.INVALID_ARGUMENT, + '`data` must be a non-null object.'); } - tableName = this.formatTableName(tableName); try { + tableName = this.formatTableName(tableName); const gqlDataString = this.objectToString(data); const mutation = `mutation { ${tableName}_insert(data: ${gqlDataString}) }`; // Use internal executeGraphql - return this.executeGraphql(mutation); + return this.executeGraphql(mutation).catch(this.handleBulkImportErrors); } catch (e: any) { - throw new FirebaseDataConnectError('internal-error', `Failed to construct insert mutation: ${e.message}`); + throw new FirebaseDataConnectError( + DATA_CONNECT_ERROR_CODE_MAPPING.INTERNAL, + `Failed to construct insert mutation: ${e.message}`); } } @@ -291,20 +307,25 @@ export class DataConnectApiClient { data: Variables, ): Promise> { if (!validator.isNonEmptyString(tableName)) { - throw new FirebaseDataConnectError('invalid-argument', '`tableName` must be a non-empty string.'); + throw new FirebaseDataConnectError( + DATA_CONNECT_ERROR_CODE_MAPPING.INVALID_ARGUMENT, + '`tableName` must be a non-empty string.'); } if (!validator.isNonEmptyArray(data)) { - throw new FirebaseDataConnectError('invalid-argument', '`data` must be a non-empty array for insertMany.'); + throw new FirebaseDataConnectError( + DATA_CONNECT_ERROR_CODE_MAPPING.INVALID_ARGUMENT, + '`data` must be a non-empty array for insertMany.'); } - tableName = this.formatTableName(tableName); try { + tableName = this.formatTableName(tableName); const gqlDataString = this.objectToString(data); const mutation = `mutation { ${tableName}_insertMany(data: ${gqlDataString}) }`; // Use internal executeGraphql - return this.executeGraphql(mutation); + return this.executeGraphql(mutation).catch(this.handleBulkImportErrors); } catch (e: any) { - throw new FirebaseDataConnectError('internal-error', `Failed to construct insertMany mutation: ${e.message}`); + throw new FirebaseDataConnectError(DATA_CONNECT_ERROR_CODE_MAPPING.INTERNAL, + `Failed to construct insertMany mutation: ${e.message}`); } } @@ -316,24 +337,31 @@ export class DataConnectApiClient { data: Variables, ): Promise> { if (!validator.isNonEmptyString(tableName)) { - throw new FirebaseDataConnectError('invalid-argument', '`tableName` must be a non-empty string.'); + throw new FirebaseDataConnectError( + DATA_CONNECT_ERROR_CODE_MAPPING.INVALID_ARGUMENT, + '`tableName` must be a non-empty string.'); } if (validator.isArray(data)) { throw new FirebaseDataConnectError( - 'invalid-argument', '`data` must be an object, not an array, for single upsert.'); + DATA_CONNECT_ERROR_CODE_MAPPING.INVALID_ARGUMENT, + '`data` must be an object, not an array, for single upsert.'); } if (!validator.isNonNullObject(data)) { - throw new FirebaseDataConnectError('invalid-argument', '`data` must be a non-null object.'); + throw new FirebaseDataConnectError( + DATA_CONNECT_ERROR_CODE_MAPPING.INVALID_ARGUMENT, + '`data` must be a non-null object.'); } - tableName = this.formatTableName(tableName); try { + tableName = this.formatTableName(tableName); const gqlDataString = this.objectToString(data); const mutation = `mutation { ${tableName}_upsert(data: ${gqlDataString}) }`; // Use internal executeGraphql - return this.executeGraphql(mutation); + return this.executeGraphql(mutation).catch(this.handleBulkImportErrors); } catch (e: any) { - throw new FirebaseDataConnectError('internal-error', `Failed to construct upsert mutation: ${e.message}`); + throw new FirebaseDataConnectError( + DATA_CONNECT_ERROR_CODE_MAPPING.INTERNAL, + `Failed to construct upsert mutation: ${e.message}`); } } @@ -345,20 +373,26 @@ export class DataConnectApiClient { data: Variables, ): Promise> { if (!validator.isNonEmptyString(tableName)) { - throw new FirebaseDataConnectError('invalid-argument', '`tableName` must be a non-empty string.'); + throw new FirebaseDataConnectError( + DATA_CONNECT_ERROR_CODE_MAPPING.INVALID_ARGUMENT, + '`tableName` must be a non-empty string.'); } if (!validator.isNonEmptyArray(data)) { - throw new FirebaseDataConnectError('invalid-argument', '`data` must be a non-empty array for upsertMany.'); + throw new FirebaseDataConnectError( + DATA_CONNECT_ERROR_CODE_MAPPING.INVALID_ARGUMENT, + '`data` must be a non-empty array for upsertMany.'); } - tableName = this.formatTableName(tableName); try { + tableName = this.formatTableName(tableName); const gqlDataString = this.objectToString(data); const mutation = `mutation { ${tableName}_upsertMany(data: ${gqlDataString}) }`; // Use internal executeGraphql - return this.executeGraphql(mutation); + return this.executeGraphql(mutation).catch(this.handleBulkImportErrors); } catch (e: any) { - throw new FirebaseDataConnectError('internal-error', `Failed to construct upsertMany mutation: ${e.message}`); + throw new FirebaseDataConnectError( + DATA_CONNECT_ERROR_CODE_MAPPING.INTERNAL, + `Failed to construct upsertMany mutation: ${e.message}`); } } } diff --git a/test/unit/data-connect/data-connect-api-client-internal.spec.ts b/test/unit/data-connect/data-connect-api-client-internal.spec.ts index a8f01eb479..aa6474046d 100644 --- a/test/unit/data-connect/data-connect-api-client-internal.spec.ts +++ b/test/unit/data-connect/data-connect-api-client-internal.spec.ts @@ -24,7 +24,7 @@ import { } from '../../../src/utils/api-request'; import * as utils from '../utils'; import * as mocks from '../../resources/mocks'; -import { DataConnectApiClient, FirebaseDataConnectError } +import { DATA_CONNECT_ERROR_CODE_MAPPING, DataConnectApiClient, FirebaseDataConnectError } from '../../../src/data-connect/data-connect-api-client-internal'; import { FirebaseApp } from '../../../src/app/firebase-app'; import { ConnectorConfig } from '../../../src/data-connect'; @@ -235,6 +235,7 @@ describe('DataConnectApiClient', () => { describe('DataConnectApiClient CRUD helpers', () => { let mockApp: FirebaseApp; let apiClient: DataConnectApiClient; + let apiClientQueryError: DataConnectApiClient; let executeGraphqlStub: sinon.SinonStub; const connectorConfig: ConnectorConfig = { @@ -266,6 +267,15 @@ describe('DataConnectApiClient CRUD helpers', () => { const tableNames = ['movie', 'Movie', 'MOVIE', 'toybox', 'toyBox', 'ToyBox', 'TOYBOX']; const formatedTableNames = ['movie', 'movie', 'mOVIE', 'toybox', 'toyBox', 'toyBox', 'tOYBOX']; + const serverErrorString = 'Server error response'; + const additionalErrorMessageForBulkImport = + 'Make sure that your table name passed in matches the type name in your GraphQL schema file.'; + + const expectedQueryError = new FirebaseDataConnectError( + DATA_CONNECT_ERROR_CODE_MAPPING.QUERY_ERROR, + serverErrorString + ); + // Helper function to normalize GraphQL strings const normalizeGraphQLString = (str: string): string => { return str @@ -277,8 +287,10 @@ describe('DataConnectApiClient CRUD helpers', () => { beforeEach(() => { mockApp = mocks.appWithOptions(mockOptions); apiClient = new DataConnectApiClient(connectorConfig, mockApp); + apiClientQueryError = new DataConnectApiClient(connectorConfig, mockApp); // Stub the instance's executeGraphql method executeGraphqlStub = sinon.stub(apiClient, 'executeGraphql').resolves({ data: {} }); + sinon.stub(apiClientQueryError, 'executeGraphql').rejects(expectedQueryError); }); afterEach(() => { @@ -352,6 +364,11 @@ describe('DataConnectApiClient CRUD helpers', () => { await expect(apiClient.insert(tableName, [])) .to.be.rejectedWith(FirebaseDataConnectError, /`data` must be an object, not an array, for single insert./); }); + + it('should amend the message for query errors', async () => { + await expect(apiClientQueryError.insert(tableName, { data: 1 })) + .to.be.rejectedWith(FirebaseDataConnectError, `${serverErrorString}. ${additionalErrorMessageForBulkImport}`); + }); }); // --- INSERT MANY TESTS --- @@ -433,6 +450,11 @@ describe('DataConnectApiClient CRUD helpers', () => { expect(apiClient.insertMany(tableName, { data: 1 } as any)) .to.be.rejectedWith(FirebaseDataConnectError, /`data` must be a non-empty array for insertMany./); }); + + it('should amend the message for query errors', async () => { + await expect(apiClientQueryError.insertMany(tableName, [{ data: 1 }])) + .to.be.rejectedWith(FirebaseDataConnectError, `${serverErrorString}. ${additionalErrorMessageForBulkImport}`); + }); }); // --- UPSERT TESTS --- @@ -491,6 +513,11 @@ describe('DataConnectApiClient CRUD helpers', () => { await expect(apiClient.upsert(tableName, [{ data: 1 }])) .to.be.rejectedWith(FirebaseDataConnectError, /`data` must be an object, not an array, for single upsert./); }); + + it('should amend the message for query errors', async () => { + await expect(apiClientQueryError.upsert(tableName, { data: 1 })) + .to.be.rejectedWith(FirebaseDataConnectError, `${serverErrorString}. ${additionalErrorMessageForBulkImport}`); + }); }); // --- UPSERT MANY TESTS --- @@ -569,5 +596,10 @@ describe('DataConnectApiClient CRUD helpers', () => { await expect(apiClient.upsertMany(tableName, { data: 1 } as any)) .to.be.rejectedWith(FirebaseDataConnectError, /`data` must be a non-empty array for upsertMany./); }); + + it('should amend the message for query errors', async () => { + await expect(apiClientQueryError.upsertMany(tableName, [{ data: 1 }])) + .to.be.rejectedWith(FirebaseDataConnectError, `${serverErrorString}. ${additionalErrorMessageForBulkImport}`); + }); }); }); From 18a0645218e63ea4542df688e3c1071527aa71dd Mon Sep 17 00:00:00 2001 From: Lahiru Maramba Date: Wed, 7 May 2025 16:51:07 -0400 Subject: [PATCH 09/11] address PR feedback --- src/data-connect/data-connect-api-client-internal.ts | 6 +++--- .../data-connect/data-connect-api-client-internal.spec.ts | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/data-connect/data-connect-api-client-internal.ts b/src/data-connect/data-connect-api-client-internal.ts index c99f244de2..e12005b8bb 100644 --- a/src/data-connect/data-connect-api-client-internal.ts +++ b/src/data-connect/data-connect-api-client-internal.ts @@ -204,7 +204,7 @@ export class DataConnectApiClient { * Handles nested objects, arrays, strings, numbers, and booleans. * Ensures strings are properly escaped. */ - private objectToString(data: any): string { + private objectToString(data: unknown): string { if (typeof data === 'string') { const escapedString = data .replace(/\\/g, '\\\\') // Replace \ with \\ @@ -278,7 +278,7 @@ export class DataConnectApiClient { if (validator.isArray(data)) { throw new FirebaseDataConnectError( DATA_CONNECT_ERROR_CODE_MAPPING.INVALID_ARGUMENT, - '`data` must be an object, not an array, for single insert.'); + '`data` must be an object, not an array, for single insert. For arrays, please use `insertMany` function.'); } if (!validator.isNonNullObject(data)) { throw new FirebaseDataConnectError( @@ -344,7 +344,7 @@ export class DataConnectApiClient { if (validator.isArray(data)) { throw new FirebaseDataConnectError( DATA_CONNECT_ERROR_CODE_MAPPING.INVALID_ARGUMENT, - '`data` must be an object, not an array, for single upsert.'); + '`data` must be an object, not an array, for single upsert. For arrays, please use `upsertMany` function.'); } if (!validator.isNonNullObject(data)) { throw new FirebaseDataConnectError( diff --git a/test/unit/data-connect/data-connect-api-client-internal.spec.ts b/test/unit/data-connect/data-connect-api-client-internal.spec.ts index aa6474046d..a5798703e5 100644 --- a/test/unit/data-connect/data-connect-api-client-internal.spec.ts +++ b/test/unit/data-connect/data-connect-api-client-internal.spec.ts @@ -264,8 +264,8 @@ describe('DataConnectApiClient CRUD helpers', () => { extras: [1, undefined, 'hello', undefined, { a: 1, b: undefined }] }; - const tableNames = ['movie', 'Movie', 'MOVIE', 'toybox', 'toyBox', 'ToyBox', 'TOYBOX']; - const formatedTableNames = ['movie', 'movie', 'mOVIE', 'toybox', 'toyBox', 'toyBox', 'tOYBOX']; + const tableNames = ['movie', 'Movie', 'MOVIE', 'mOvIE', 'toybox', 'toyBox', 'toyBOX', 'ToyBox', 'TOYBOX']; + const formatedTableNames = ['movie', 'movie', 'mOVIE', 'mOvIE', 'toybox', 'toyBox', 'toyBOX', 'toyBox', 'tOYBOX']; const serverErrorString = 'Server error response'; const additionalErrorMessageForBulkImport = From ecd291557f8b17eab4df3bc4806134f69bc4708a Mon Sep 17 00:00:00 2001 From: Lahiru Maramba Date: Thu, 8 May 2025 13:31:08 -0400 Subject: [PATCH 10/11] Throw for sparse arrays --- .../data-connect-api-client-internal.ts | 5 ++ .../data-connect-api-client-internal.spec.ts | 67 ++++++++++++++++--- 2 files changed, 61 insertions(+), 11 deletions(-) diff --git a/src/data-connect/data-connect-api-client-internal.ts b/src/data-connect/data-connect-api-client-internal.ts index e12005b8bb..1e649fff62 100644 --- a/src/data-connect/data-connect-api-client-internal.ts +++ b/src/data-connect/data-connect-api-client-internal.ts @@ -215,6 +215,11 @@ export class DataConnectApiClient { return String(data); } if (validator.isArray(data)) { + if (data.includes(null) || data.includes(undefined)) { + throw new FirebaseDataConnectError( + DATA_CONNECT_ERROR_CODE_MAPPING.INVALID_ARGUMENT, + 'Array elements cannot contain `null` or `undefined` values.'); + } const elements = data.map(item => this.objectToString(item)).join(', '); return `[${elements}]`; } diff --git a/test/unit/data-connect/data-connect-api-client-internal.spec.ts b/test/unit/data-connect/data-connect-api-client-internal.spec.ts index a5798703e5..267c060d34 100644 --- a/test/unit/data-connect/data-connect-api-client-internal.spec.ts +++ b/test/unit/data-connect/data-connect-api-client-internal.spec.ts @@ -261,9 +261,14 @@ describe('DataConnectApiClient CRUD helpers', () => { }, notes: undefined, releaseYear: undefined, - extras: [1, undefined, 'hello', undefined, { a: 1, b: undefined }] + extras: [1, 'hello', { a: 1, b: undefined }] }; + const dataWithSparseArray = { + genre: 'Action', + extras: [1, undefined, 'hello', undefined, { a: 1, b: undefined }] + } + const tableNames = ['movie', 'Movie', 'MOVIE', 'mOvIE', 'toybox', 'toyBox', 'toyBOX', 'ToyBox', 'TOYBOX']; const formatedTableNames = ['movie', 'movie', 'mOVIE', 'mOvIE', 'toybox', 'toyBox', 'toyBOX', 'toyBox', 'tOYBOX']; @@ -343,7 +348,7 @@ describe('DataConnectApiClient CRUD helpers', () => { title: "Die Hard", ratings: null, director: {}, - extras: [1, null, "hello", null, { a: 1 }] + extras: [1, "hello", { a: 1 }] }) }`; await apiClient.insert(tableName, dataWithUndefined); @@ -369,6 +374,16 @@ describe('DataConnectApiClient CRUD helpers', () => { await expect(apiClientQueryError.insert(tableName, { data: 1 })) .to.be.rejectedWith(FirebaseDataConnectError, `${serverErrorString}. ${additionalErrorMessageForBulkImport}`); }); + + [ + { a: [null] }, { a: [undefined] }, { a: [1, null, 2] }, { a: [1, undefined, 2] }, + { a: ['a', null, 'b', undefined, 'd'] }, dataWithSparseArray + ].forEach((data) => { + it('should throw FirebaseDataConnectError for arrays with null or undefined elements', async () => { + await expect(apiClient.insert(tableName, data)) + .to.be.rejectedWith(FirebaseDataConnectError, /Array elements cannot contain `null` or `undefined` values./); + }); + }); }); // --- INSERT MANY TESTS --- @@ -417,14 +432,14 @@ describe('DataConnectApiClient CRUD helpers', () => { title: "Die Hard", ratings: null, director: {}, - extras: [1, null, "hello", null, { a: 1 }] + extras: [1, "hello", { a: 1 }] }, { genre: "Action", title: "Die Hard", ratings: null, director: {}, - extras: [1, null, "hello", null, { a: 1 }] + extras: [1, "hello", { a: 1 }] }]) }`; await apiClient.insertMany(tableName, dataArray); @@ -455,6 +470,16 @@ describe('DataConnectApiClient CRUD helpers', () => { await expect(apiClientQueryError.insertMany(tableName, [{ data: 1 }])) .to.be.rejectedWith(FirebaseDataConnectError, `${serverErrorString}. ${additionalErrorMessageForBulkImport}`); }); + + [ + [null], [undefined], [1, null, 2], [1, undefined, 2], + ['a', null, 'b', undefined, 'd'], [dataWithSparseArray] + ].forEach((data) => { + it('should throw FirebaseDataConnectError for arrays with null or undefined elements', async () => { + await expect(apiClient.insertMany(tableName, data)) + .to.be.rejectedWith(FirebaseDataConnectError, /Array elements cannot contain `null` or `undefined` values./); + }); + }); }); // --- UPSERT TESTS --- @@ -476,10 +501,10 @@ describe('DataConnectApiClient CRUD helpers', () => { }); it('should call executeGraphql with the correct mutation for complex data', async () => { - const complexData = { id: 'key2', active: false, items: [1, null], detail: { status: 'done/\\' } }; + const complexData = { id: 'key2', active: false, items: [1, 2], detail: { status: 'done/\\' } }; const expectedMutation = ` mutation { ${formatedTableName}_upsert(data: - { id: "key2", active: false, items: [1, null], detail: { status: "done/\\\\" } }) }`; + { id: "key2", active: false, items: [1, 2], detail: { status: "done/\\\\" } }) }`; await apiClient.upsert(tableName, complexData); expect(executeGraphqlStub).to.have.been.calledOnceWithExactly(normalizeGraphQLString(expectedMutation)); }); @@ -492,7 +517,7 @@ describe('DataConnectApiClient CRUD helpers', () => { title: "Die Hard", ratings: null, director: {}, - extras: [1, null, "hello", null, { a: 1 }] + extras: [1, "hello", { a: 1 }] }) }`; await apiClient.upsert(tableName, dataWithUndefined); @@ -518,6 +543,16 @@ describe('DataConnectApiClient CRUD helpers', () => { await expect(apiClientQueryError.upsert(tableName, { data: 1 })) .to.be.rejectedWith(FirebaseDataConnectError, `${serverErrorString}. ${additionalErrorMessageForBulkImport}`); }); + + [ + { a: [null] }, { a: [undefined] }, { a: [1, null, 2] }, { a: [1, undefined, 2] }, + { a: ['a', null, 'b', undefined, 'd'] }, dataWithSparseArray + ].forEach((data) => { + it('should throw FirebaseDataConnectError for arrays with null or undefined elements', async () => { + await expect(apiClient.upsert(tableName, data)) + .to.be.rejectedWith(FirebaseDataConnectError, /Array elements cannot contain `null` or `undefined` values./); + }); + }); }); // --- UPSERT MANY TESTS --- @@ -542,11 +577,11 @@ describe('DataConnectApiClient CRUD helpers', () => { it('should call executeGraphql with the correct mutation for complex data array', async () => { const complexDataArray = [ { id: 'x', active: true, info: { nested: 'n1/\\"x' } }, - { id: 'y', scores: [null, 2] } + { id: 'y', scores: [1, 2] } ]; const expectedMutation = ` mutation { ${formatedTableName}_upsertMany(data: - [{ id: "x", active: true, info: { nested: "n1/\\\\\\"x" } }, { id: "y", scores: [null, 2] }]) }`; + [{ id: "x", active: true, info: { nested: "n1/\\\\\\"x" } }, { id: "y", scores: [1, 2] }]) }`; await apiClient.upsertMany(tableName, complexDataArray); expect(executeGraphqlStub).to.have.been.calledOnceWithExactly(normalizeGraphQLString(expectedMutation)); }); @@ -563,14 +598,14 @@ describe('DataConnectApiClient CRUD helpers', () => { title: "Die Hard", ratings: null, director: {}, - extras: [1, null, "hello", null, { a: 1 }] + extras: [1, "hello", { a: 1 }] }, { genre: "Action", title: "Die Hard", ratings: null, director: {}, - extras: [1, null, "hello", null, { a: 1 }] + extras: [1, "hello", { a: 1 }] }]) }`; await apiClient.upsertMany(tableName, dataArray); @@ -601,5 +636,15 @@ describe('DataConnectApiClient CRUD helpers', () => { await expect(apiClientQueryError.upsertMany(tableName, [{ data: 1 }])) .to.be.rejectedWith(FirebaseDataConnectError, `${serverErrorString}. ${additionalErrorMessageForBulkImport}`); }); + + [ + [null], [undefined], [1, null, 2], [1, undefined, 2], + ['a', null, 'b', undefined, 'd'], [dataWithSparseArray] + ].forEach((data) => { + it('should throw FirebaseDataConnectError for arrays with null or undefined elements', async () => { + await expect(apiClient.upsertMany(tableName, data)) + .to.be.rejectedWith(FirebaseDataConnectError, /Array elements cannot contain `null` or `undefined` values./); + }); + }); }); }); From 56d7e889e5299b9091e63d0cee34e632c7227f6c Mon Sep 17 00:00:00 2001 From: Lahiru Maramba Date: Fri, 9 May 2025 11:01:03 -0400 Subject: [PATCH 11/11] Revert "Throw for sparse arrays" This reverts commit ecd291557f8b17eab4df3bc4806134f69bc4708a. --- .../data-connect-api-client-internal.ts | 5 -- .../data-connect-api-client-internal.spec.ts | 67 +++---------------- 2 files changed, 11 insertions(+), 61 deletions(-) diff --git a/src/data-connect/data-connect-api-client-internal.ts b/src/data-connect/data-connect-api-client-internal.ts index 1e649fff62..e12005b8bb 100644 --- a/src/data-connect/data-connect-api-client-internal.ts +++ b/src/data-connect/data-connect-api-client-internal.ts @@ -215,11 +215,6 @@ export class DataConnectApiClient { return String(data); } if (validator.isArray(data)) { - if (data.includes(null) || data.includes(undefined)) { - throw new FirebaseDataConnectError( - DATA_CONNECT_ERROR_CODE_MAPPING.INVALID_ARGUMENT, - 'Array elements cannot contain `null` or `undefined` values.'); - } const elements = data.map(item => this.objectToString(item)).join(', '); return `[${elements}]`; } diff --git a/test/unit/data-connect/data-connect-api-client-internal.spec.ts b/test/unit/data-connect/data-connect-api-client-internal.spec.ts index 267c060d34..a5798703e5 100644 --- a/test/unit/data-connect/data-connect-api-client-internal.spec.ts +++ b/test/unit/data-connect/data-connect-api-client-internal.spec.ts @@ -261,13 +261,8 @@ describe('DataConnectApiClient CRUD helpers', () => { }, notes: undefined, releaseYear: undefined, - extras: [1, 'hello', { a: 1, b: undefined }] - }; - - const dataWithSparseArray = { - genre: 'Action', extras: [1, undefined, 'hello', undefined, { a: 1, b: undefined }] - } + }; const tableNames = ['movie', 'Movie', 'MOVIE', 'mOvIE', 'toybox', 'toyBox', 'toyBOX', 'ToyBox', 'TOYBOX']; const formatedTableNames = ['movie', 'movie', 'mOVIE', 'mOvIE', 'toybox', 'toyBox', 'toyBOX', 'toyBox', 'tOYBOX']; @@ -348,7 +343,7 @@ describe('DataConnectApiClient CRUD helpers', () => { title: "Die Hard", ratings: null, director: {}, - extras: [1, "hello", { a: 1 }] + extras: [1, null, "hello", null, { a: 1 }] }) }`; await apiClient.insert(tableName, dataWithUndefined); @@ -374,16 +369,6 @@ describe('DataConnectApiClient CRUD helpers', () => { await expect(apiClientQueryError.insert(tableName, { data: 1 })) .to.be.rejectedWith(FirebaseDataConnectError, `${serverErrorString}. ${additionalErrorMessageForBulkImport}`); }); - - [ - { a: [null] }, { a: [undefined] }, { a: [1, null, 2] }, { a: [1, undefined, 2] }, - { a: ['a', null, 'b', undefined, 'd'] }, dataWithSparseArray - ].forEach((data) => { - it('should throw FirebaseDataConnectError for arrays with null or undefined elements', async () => { - await expect(apiClient.insert(tableName, data)) - .to.be.rejectedWith(FirebaseDataConnectError, /Array elements cannot contain `null` or `undefined` values./); - }); - }); }); // --- INSERT MANY TESTS --- @@ -432,14 +417,14 @@ describe('DataConnectApiClient CRUD helpers', () => { title: "Die Hard", ratings: null, director: {}, - extras: [1, "hello", { a: 1 }] + extras: [1, null, "hello", null, { a: 1 }] }, { genre: "Action", title: "Die Hard", ratings: null, director: {}, - extras: [1, "hello", { a: 1 }] + extras: [1, null, "hello", null, { a: 1 }] }]) }`; await apiClient.insertMany(tableName, dataArray); @@ -470,16 +455,6 @@ describe('DataConnectApiClient CRUD helpers', () => { await expect(apiClientQueryError.insertMany(tableName, [{ data: 1 }])) .to.be.rejectedWith(FirebaseDataConnectError, `${serverErrorString}. ${additionalErrorMessageForBulkImport}`); }); - - [ - [null], [undefined], [1, null, 2], [1, undefined, 2], - ['a', null, 'b', undefined, 'd'], [dataWithSparseArray] - ].forEach((data) => { - it('should throw FirebaseDataConnectError for arrays with null or undefined elements', async () => { - await expect(apiClient.insertMany(tableName, data)) - .to.be.rejectedWith(FirebaseDataConnectError, /Array elements cannot contain `null` or `undefined` values./); - }); - }); }); // --- UPSERT TESTS --- @@ -501,10 +476,10 @@ describe('DataConnectApiClient CRUD helpers', () => { }); it('should call executeGraphql with the correct mutation for complex data', async () => { - const complexData = { id: 'key2', active: false, items: [1, 2], detail: { status: 'done/\\' } }; + const complexData = { id: 'key2', active: false, items: [1, null], detail: { status: 'done/\\' } }; const expectedMutation = ` mutation { ${formatedTableName}_upsert(data: - { id: "key2", active: false, items: [1, 2], detail: { status: "done/\\\\" } }) }`; + { id: "key2", active: false, items: [1, null], detail: { status: "done/\\\\" } }) }`; await apiClient.upsert(tableName, complexData); expect(executeGraphqlStub).to.have.been.calledOnceWithExactly(normalizeGraphQLString(expectedMutation)); }); @@ -517,7 +492,7 @@ describe('DataConnectApiClient CRUD helpers', () => { title: "Die Hard", ratings: null, director: {}, - extras: [1, "hello", { a: 1 }] + extras: [1, null, "hello", null, { a: 1 }] }) }`; await apiClient.upsert(tableName, dataWithUndefined); @@ -543,16 +518,6 @@ describe('DataConnectApiClient CRUD helpers', () => { await expect(apiClientQueryError.upsert(tableName, { data: 1 })) .to.be.rejectedWith(FirebaseDataConnectError, `${serverErrorString}. ${additionalErrorMessageForBulkImport}`); }); - - [ - { a: [null] }, { a: [undefined] }, { a: [1, null, 2] }, { a: [1, undefined, 2] }, - { a: ['a', null, 'b', undefined, 'd'] }, dataWithSparseArray - ].forEach((data) => { - it('should throw FirebaseDataConnectError for arrays with null or undefined elements', async () => { - await expect(apiClient.upsert(tableName, data)) - .to.be.rejectedWith(FirebaseDataConnectError, /Array elements cannot contain `null` or `undefined` values./); - }); - }); }); // --- UPSERT MANY TESTS --- @@ -577,11 +542,11 @@ describe('DataConnectApiClient CRUD helpers', () => { it('should call executeGraphql with the correct mutation for complex data array', async () => { const complexDataArray = [ { id: 'x', active: true, info: { nested: 'n1/\\"x' } }, - { id: 'y', scores: [1, 2] } + { id: 'y', scores: [null, 2] } ]; const expectedMutation = ` mutation { ${formatedTableName}_upsertMany(data: - [{ id: "x", active: true, info: { nested: "n1/\\\\\\"x" } }, { id: "y", scores: [1, 2] }]) }`; + [{ id: "x", active: true, info: { nested: "n1/\\\\\\"x" } }, { id: "y", scores: [null, 2] }]) }`; await apiClient.upsertMany(tableName, complexDataArray); expect(executeGraphqlStub).to.have.been.calledOnceWithExactly(normalizeGraphQLString(expectedMutation)); }); @@ -598,14 +563,14 @@ describe('DataConnectApiClient CRUD helpers', () => { title: "Die Hard", ratings: null, director: {}, - extras: [1, "hello", { a: 1 }] + extras: [1, null, "hello", null, { a: 1 }] }, { genre: "Action", title: "Die Hard", ratings: null, director: {}, - extras: [1, "hello", { a: 1 }] + extras: [1, null, "hello", null, { a: 1 }] }]) }`; await apiClient.upsertMany(tableName, dataArray); @@ -636,15 +601,5 @@ describe('DataConnectApiClient CRUD helpers', () => { await expect(apiClientQueryError.upsertMany(tableName, [{ data: 1 }])) .to.be.rejectedWith(FirebaseDataConnectError, `${serverErrorString}. ${additionalErrorMessageForBulkImport}`); }); - - [ - [null], [undefined], [1, null, 2], [1, undefined, 2], - ['a', null, 'b', undefined, 'd'], [dataWithSparseArray] - ].forEach((data) => { - it('should throw FirebaseDataConnectError for arrays with null or undefined elements', async () => { - await expect(apiClient.upsertMany(tableName, data)) - .to.be.rejectedWith(FirebaseDataConnectError, /Array elements cannot contain `null` or `undefined` values./); - }); - }); }); });