From 36fa0c848622a5c4a53d6edfba80c6e6e31c3d12 Mon Sep 17 00:00:00 2001 From: stephenarosaj Date: Thu, 11 Jun 2026 11:13:20 -0700 Subject: [PATCH 01/16] add tests checking for new GQL output for [insert,upsert](many) --- .../data-connect-api-client-internal.spec.ts | 70 +++++++++++++++++++ 1 file changed, 70 insertions(+) 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 5951c29264..fd59ceeb46 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 @@ -774,6 +774,22 @@ describe('DataConnectApiClient CRUD helpers', () => { expect(err.cause).to.equal(expectedQueryError); } }); + + it('should call executeGraphql with variables and @allow directive for enums and other fields', async () => { + const data = { id: 'key1', name: 'Fred', status: 'ACTIVE' }; + const expectedMutation = ` + mutation($data: TestTable_Data! @allow(fields: "id name status")) { + testTable_insert(data: $data) + }`; + const expectedOptions = { + variables: { data } + }; + await apiClient.insert(tableName, data); + expect(executeGraphqlStub).to.have.been.calledOnceWithExactly( + normalizeGraphQLString(expectedMutation), + expectedOptions + ); + }); }); // --- INSERT MANY TESTS --- @@ -866,6 +882,25 @@ describe('DataConnectApiClient CRUD helpers', () => { expect(err.cause).to.equal(expectedQueryError); } }); + + it('should call executeGraphql with variables and @allow directive for enums and other fields', async () => { + const data = [ + { id: 'key1', name: 'Fred', status: 'ACTIVE' }, + { id: 'key2', name: 'Bob', tags: ['cool'] } + ]; + const expectedMutation = ` + mutation($data: [TestTable_Data! @allow(fields: "id name status tags")]!) { + testTable_insertMany(data: $data) + }`; + const expectedOptions = { + variables: { data } + }; + await apiClient.insertMany(tableName, data); + expect(executeGraphqlStub).to.have.been.calledOnceWithExactly( + normalizeGraphQLString(expectedMutation), + expectedOptions + ); + }); }); // --- UPSERT TESTS --- @@ -935,6 +970,22 @@ describe('DataConnectApiClient CRUD helpers', () => { expect(err.cause).to.equal(expectedQueryError); } }); + + it('should call executeGraphql with variables and @allow directive for enums and other fields', async () => { + const data = { id: 'key1', name: 'Fred', status: 'ACTIVE' }; + const expectedMutation = ` + mutation($data: TestTable_Data! @allow(fields: "id name status")) { + testTable_upsert(data: $data) + }`; + const expectedOptions = { + variables: { data } + }; + await apiClient.upsert(tableName, data); + expect(executeGraphqlStub).to.have.been.calledOnceWithExactly( + normalizeGraphQLString(expectedMutation), + expectedOptions + ); + }); }); // --- UPSERT MANY TESTS --- @@ -1024,6 +1075,25 @@ describe('DataConnectApiClient CRUD helpers', () => { expect(err.cause).to.equal(expectedQueryError); } }); + + it('should call executeGraphql with variables and @allow directive for enums and other fields', async () => { + const data = [ + { id: 'key1', name: 'Fred', status: 'ACTIVE' }, + { id: 'key2', name: 'Bob', tags: ['cool'] } + ]; + const expectedMutation = ` + mutation($data: [TestTable_Data! @allow(fields: "id name status tags")]!) { + testTable_upsertMany(data: $data) + }`; + const expectedOptions = { + variables: { data } + }; + await apiClient.upsertMany(tableName, data); + expect(executeGraphqlStub).to.have.been.calledOnceWithExactly( + normalizeGraphQLString(expectedMutation), + expectedOptions + ); + }); }); describe('String serialization', () => { From 1ed5b873702ca258017b9673ab2937b7bf211b0d Mon Sep 17 00:00:00 2001 From: stephenarosaj Date: Thu, 11 Jun 2026 11:27:28 -0700 Subject: [PATCH 02/16] change mutation functions to use @allow --- .../data-connect-api-client-internal.ts | 126 +++++++++--------- 1 file changed, 62 insertions(+), 64 deletions(-) diff --git a/src/data-connect/data-connect-api-client-internal.ts b/src/data-connect/data-connect-api-client-internal.ts index 8cb5ee289b..f4718a6a63 100644 --- a/src/data-connect/data-connect-api-client-internal.ts +++ b/src/data-connect/data-connect-api-client-internal.ts @@ -423,50 +423,6 @@ export class DataConnectApiClient { }); } - /** - * Converts JSON data into a GraphQL literal string. - * Handles nested objects, arrays, strings, numbers, and booleans. - * Ensures strings are properly escaped. - */ - private objectToString(data: unknown): string { - if (typeof data === 'string') { - return JSON.stringify(data); - } - 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'; - } - - // 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); - } - private formatTableName(tableName: string): string { // Format tableName: first character to lowercase if (tableName && tableName.length > 0) { @@ -515,11 +471,17 @@ export class DataConnectApiClient { } try { - tableName = this.formatTableName(tableName); - const gqlDataString = this.objectToString(data); - const mutation = `mutation { ${tableName}_insert(data: ${gqlDataString}) }`; - // Use internal executeGraphql - return this.executeGraphql(mutation).catch(this.handleBulkImportErrors); + const capitalizedTable = tableName.charAt(0).toUpperCase() + tableName.slice(1); + const formattedTableName = this.formatTableName(tableName); + + const keys = Object.keys(data) + .filter(key => (data as Record)[key] !== undefined) + .join(' '); + + const mutation = `mutation($data: ${capitalizedTable}_Data! @allow(fields: "${keys}")) { ${formattedTableName}_insert(data: $data) }`; + + return this.executeGraphql(mutation, { variables: { data } }) + .catch(this.handleBulkImportErrors); } catch (e: any) { throw new FirebaseDataConnectError({ code: DATA_CONNECT_ERROR_CODE_MAPPING.INTERNAL, @@ -550,11 +512,26 @@ export class DataConnectApiClient { } try { - tableName = this.formatTableName(tableName); - const gqlDataString = this.objectToString(data); - const mutation = `mutation { ${tableName}_insertMany(data: ${gqlDataString}) }`; - // Use internal executeGraphql - return this.executeGraphql(mutation).catch(this.handleBulkImportErrors); + const capitalizedTable = tableName.charAt(0).toUpperCase() + tableName.slice(1); + const formattedTableName = this.formatTableName(tableName); + + const allKeys = new Set(); + for (const element of data) { + if (validator.isNonNullObject(element)) { + const record = element as Record; + Object.keys(record).forEach(key => { + if (record[key] !== undefined) { + allKeys.add(key); + } + }); + } + } + const keys = Array.from(allKeys).join(' '); + + const mutation = `mutation($data: [${capitalizedTable}_Data! @allow(fields: "${keys}")]!) { ${formattedTableName}_insertMany(data: $data) }`; + + return this.executeGraphql(mutation, { variables: { data } }) + .catch(this.handleBulkImportErrors); } catch (e: any) { throw new FirebaseDataConnectError({ code: DATA_CONNECT_ERROR_CODE_MAPPING.INTERNAL, @@ -592,11 +569,17 @@ export class DataConnectApiClient { } try { - tableName = this.formatTableName(tableName); - const gqlDataString = this.objectToString(data); - const mutation = `mutation { ${tableName}_upsert(data: ${gqlDataString}) }`; - // Use internal executeGraphql - return this.executeGraphql(mutation).catch(this.handleBulkImportErrors); + const capitalizedTable = tableName.charAt(0).toUpperCase() + tableName.slice(1); + const formattedTableName = this.formatTableName(tableName); + + const keys = Object.keys(data) + .filter(key => (data as Record)[key] !== undefined) + .join(' '); + + const mutation = `mutation($data: ${capitalizedTable}_Data! @allow(fields: "${keys}")) { ${formattedTableName}_upsert(data: $data) }`; + + return this.executeGraphql(mutation, { variables: { data } }) + .catch(this.handleBulkImportErrors); } catch (e: any) { throw new FirebaseDataConnectError({ code: DATA_CONNECT_ERROR_CODE_MAPPING.INTERNAL, @@ -627,11 +610,26 @@ export class DataConnectApiClient { } try { - tableName = this.formatTableName(tableName); - const gqlDataString = this.objectToString(data); - const mutation = `mutation { ${tableName}_upsertMany(data: ${gqlDataString}) }`; - // Use internal executeGraphql - return this.executeGraphql(mutation).catch(this.handleBulkImportErrors); + const capitalizedTable = tableName.charAt(0).toUpperCase() + tableName.slice(1); + const formattedTableName = this.formatTableName(tableName); + + const allKeys = new Set(); + for (const element of data) { + if (validator.isNonNullObject(element)) { + const record = element as Record; + Object.keys(record).forEach(key => { + if (record[key] !== undefined) { + allKeys.add(key); + } + }); + } + } + const keys = Array.from(allKeys).join(' '); + + const mutation = `mutation($data: [${capitalizedTable}_Data! @allow(fields: "${keys}")]!) { ${formattedTableName}_upsertMany(data: $data) }`; + + return this.executeGraphql(mutation, { variables: { data } }) + .catch(this.handleBulkImportErrors); } catch (e: any) { throw new FirebaseDataConnectError({ code: DATA_CONNECT_ERROR_CODE_MAPPING.INTERNAL, From 4f6135e38c9bdb349df4e5b53e09d2c8a47eab24 Mon Sep 17 00:00:00 2001 From: stephenarosaj Date: Fri, 12 Jun 2026 10:13:08 -0700 Subject: [PATCH 03/16] factor out common code after @allow changes --- .../data-connect-api-client-internal.ts | 107 +++++++++--------- 1 file changed, 52 insertions(+), 55 deletions(-) diff --git a/src/data-connect/data-connect-api-client-internal.ts b/src/data-connect/data-connect-api-client-internal.ts index f4718a6a63..0fe2b74dc8 100644 --- a/src/data-connect/data-connect-api-client-internal.ts +++ b/src/data-connect/data-connect-api-client-internal.ts @@ -423,12 +423,47 @@ export class DataConnectApiClient { }); } - private formatTableName(tableName: string): string { - // Format tableName: first character to lowercase - if (tableName && tableName.length > 0) { - return tableName.charAt(0).toLowerCase() + tableName.slice(1); + /** + * Generates both capitalized and camel-cased variations of a table name. + * Capitalization matches the schema types, and camel-case matches mutations. + */ + private getTableNames(tableName: string): { capitalized: string; formatted: string } { + if (!tableName || tableName.length === 0) { + return { capitalized: tableName, formatted: tableName }; + } + const capitalized = tableName.charAt(0).toUpperCase() + tableName.slice(1); + const formatted = tableName.charAt(0).toLowerCase() + tableName.slice(1); + return { capitalized, formatted }; + } + + /** + * Extracts all defined property keys from an object as a space-separated string. + * Used to build the `@allow(fields: ...)` mutation directive for single operations. + */ + private getObjectKeys(data: Record | object): string { + return Object.keys(data) + .filter(key => (data as Record)[key] !== undefined) + .join(' '); + } + + /** + * Extracts the union of all defined property keys across an array of objects + * as a space-separated string. Used to build the `@allow(fields: ...)` mutation + * directive for bulk operations. + */ + private getArrayObjectsKeys(data: Array): string { + const allKeys = new Set(); + for (const element of data) { + if (validator.isNonNullObject(element)) { + const record = element as Record; + Object.keys(record).forEach(key => { + if (record[key] !== undefined) { + allKeys.add(key); + } + }); + } } - return tableName; + return Array.from(allKeys).join(' '); } private handleBulkImportErrors(err: FirebaseDataConnectError): never { @@ -471,14 +506,9 @@ export class DataConnectApiClient { } try { - const capitalizedTable = tableName.charAt(0).toUpperCase() + tableName.slice(1); - const formattedTableName = this.formatTableName(tableName); - - const keys = Object.keys(data) - .filter(key => (data as Record)[key] !== undefined) - .join(' '); - - const mutation = `mutation($data: ${capitalizedTable}_Data! @allow(fields: "${keys}")) { ${formattedTableName}_insert(data: $data) }`; + const { capitalized, formatted } = this.getTableNames(tableName); + const keys = this.getObjectKeys(data); + const mutation = `mutation($data: ${capitalized}_Data! @allow(fields: "${keys}")) { ${formatted}_insert(data: $data) }`; return this.executeGraphql(mutation, { variables: { data } }) .catch(this.handleBulkImportErrors); @@ -512,23 +542,9 @@ export class DataConnectApiClient { } try { - const capitalizedTable = tableName.charAt(0).toUpperCase() + tableName.slice(1); - const formattedTableName = this.formatTableName(tableName); - - const allKeys = new Set(); - for (const element of data) { - if (validator.isNonNullObject(element)) { - const record = element as Record; - Object.keys(record).forEach(key => { - if (record[key] !== undefined) { - allKeys.add(key); - } - }); - } - } - const keys = Array.from(allKeys).join(' '); - - const mutation = `mutation($data: [${capitalizedTable}_Data! @allow(fields: "${keys}")]!) { ${formattedTableName}_insertMany(data: $data) }`; + const { capitalized, formatted } = this.getTableNames(tableName); + const keys = this.getArrayObjectsKeys(data); + const mutation = `mutation($data: [${capitalized}_Data! @allow(fields: "${keys}")]!) { ${formatted}_insertMany(data: $data) }`; return this.executeGraphql(mutation, { variables: { data } }) .catch(this.handleBulkImportErrors); @@ -569,14 +585,9 @@ export class DataConnectApiClient { } try { - const capitalizedTable = tableName.charAt(0).toUpperCase() + tableName.slice(1); - const formattedTableName = this.formatTableName(tableName); - - const keys = Object.keys(data) - .filter(key => (data as Record)[key] !== undefined) - .join(' '); - - const mutation = `mutation($data: ${capitalizedTable}_Data! @allow(fields: "${keys}")) { ${formattedTableName}_upsert(data: $data) }`; + const { capitalized, formatted } = this.getTableNames(tableName); + const keys = this.getObjectKeys(data); + const mutation = `mutation($data: ${capitalized}_Data! @allow(fields: "${keys}")) { ${formatted}_upsert(data: $data) }`; return this.executeGraphql(mutation, { variables: { data } }) .catch(this.handleBulkImportErrors); @@ -610,23 +621,9 @@ export class DataConnectApiClient { } try { - const capitalizedTable = tableName.charAt(0).toUpperCase() + tableName.slice(1); - const formattedTableName = this.formatTableName(tableName); - - const allKeys = new Set(); - for (const element of data) { - if (validator.isNonNullObject(element)) { - const record = element as Record; - Object.keys(record).forEach(key => { - if (record[key] !== undefined) { - allKeys.add(key); - } - }); - } - } - const keys = Array.from(allKeys).join(' '); - - const mutation = `mutation($data: [${capitalizedTable}_Data! @allow(fields: "${keys}")]!) { ${formattedTableName}_upsertMany(data: $data) }`; + const { capitalized, formatted } = this.getTableNames(tableName); + const keys = this.getArrayObjectsKeys(data); + const mutation = `mutation($data: [${capitalized}_Data! @allow(fields: "${keys}")]!) { ${formatted}_upsertMany(data: $data) }`; return this.executeGraphql(mutation, { variables: { data } }) .catch(this.handleBulkImportErrors); From 8e12fc54ffbdef68f890106fff604ab99cf43531 Mon Sep 17 00:00:00 2001 From: stephenarosaj Date: Fri, 12 Jun 2026 10:13:24 -0700 Subject: [PATCH 04/16] update tests to expect @allow --- .../data-connect-api-client-internal.spec.ts | 225 ++++++++---------- 1 file changed, 104 insertions(+), 121 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 fd59ceeb46..38e21b1bcd 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 @@ -683,6 +683,10 @@ describe('DataConnectApiClient CRUD helpers', () => { .trim(); // Remove leading/trailing whitespace from the whole string }; + const capitalize = (str: string): string => { + return str.charAt(0).toUpperCase() + str.slice(1); + }; + beforeEach(() => { mockApp = mocks.appWithOptions(mockOptions); apiClient = new DataConnectApiClient(connectorConfig, mockApp); @@ -700,53 +704,45 @@ describe('DataConnectApiClient CRUD helpers', () => { // --- INSERT TESTS --- describe('insert()', () => { tableNames.forEach((tableName, index) => { - const expectedMutation = `mutation { ${formatedTableNames[index]}_insert(data: { name: "a" }) }`; + const capitalizedTable = capitalize(formatedTableNames[index]); + const expectedMutation = `mutation($data: ${capitalizedTable}_Data! @allow(fields: "name")) { ${formatedTableNames[index]}_insert(data: $data) }`; 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)); + await expect(executeGraphqlStub).to.have.been.calledOnceWithExactly( + normalizeGraphQLString(expectedMutation), + { variables: { data: { name: 'a' } } } + ); }); }); it('should call executeGraphql with the correct mutation for simple data', async () => { const simpleData = { name: 'test', value: 123 }; - const expectedMutation = ` - mutation { - ${formatedTableName}_insert(data: { - name: "test", - value: 123 - }) - }`; + const expectedMutation = `mutation($data: TestTable_Data! @allow(fields: "name value")) { ${formatedTableName}_insert(data: $data) }`; await apiClient.insert(tableName, simpleData); - expect(executeGraphqlStub).to.have.been.calledOnceWithExactly(normalizeGraphQLString(expectedMutation)); + expect(executeGraphqlStub).to.have.been.calledOnceWithExactly( + normalizeGraphQLString(expectedMutation), + { variables: { data: simpleData } } + ); }); 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\\' } }; - const expectedMutation = ` - mutation { - ${formatedTableName}_insert(data: { - id: "abc", active: true, scores: [10, 20], - info: { nested: "yes/no \\"quote\\" \\\\slash\\\\" } - }) - }`; + const expectedMutation = `mutation($data: TestTable_Data! @allow(fields: "id active scores info")) { ${formatedTableName}_insert(data: $data) }`; await apiClient.insert(tableName, complexData); - expect(executeGraphqlStub).to.have.been.calledOnceWithExactly(normalizeGraphQLString(expectedMutation)); + expect(executeGraphqlStub).to.have.been.calledOnceWithExactly( + normalizeGraphQLString(expectedMutation), + { variables: { data: complexData } } + ); }); it('should call executeGraphql with the correct mutation for undefined and null values', async () => { - const expectedMutation = ` - mutation { - ${formatedTableName}_insert(data: { - genre: "Action", - title: "Die Hard", - ratings: null, - director: {}, - extras: [1, null, "hello", null, { a: 1 }] - }) - }`; + const expectedMutation = `mutation($data: TestTable_Data! @allow(fields: "genre title ratings director extras")) { ${formatedTableName}_insert(data: $data) }`; await apiClient.insert(tableName, dataWithUndefined); - expect(executeGraphqlStub).to.have.been.calledOnceWithExactly(normalizeGraphQLString(expectedMutation)); + expect(executeGraphqlStub).to.have.been.calledOnceWithExactly( + normalizeGraphQLString(expectedMutation), + { variables: { data: dataWithUndefined } } + ); }); it('should throw FirebaseDataConnectError for invalid tableName', async () => { @@ -795,21 +791,26 @@ describe('DataConnectApiClient CRUD helpers', () => { // --- INSERT MANY TESTS --- describe('insertMany()', () => { tableNames.forEach((tableName, index) => { - const expectedMutation = `mutation { ${formatedTableNames[index]}_insertMany(data: [{ name: "a" }]) }`; + const capitalizedTable = capitalize(formatedTableNames[index]); + const expectedMutation = `mutation($data: [${capitalizedTable}_Data! @allow(fields: "name")]!) { ${formatedTableNames[index]}_insertMany(data: $data) }`; 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)); + await expect(executeGraphqlStub).to.have.been.calledOnceWithExactly( + normalizeGraphQLString(expectedMutation), + { variables: { data: [{ name: 'a' }] } } + ); }); }); it('should call executeGraphql with the correct mutation for simple data array', async () => { const simpleDataArray = [{ name: 'test1' }, { name: 'test2', value: 456 }]; - const expectedMutation = ` - mutation { - ${formatedTableName}_insertMany(data: [{ name: "test1" }, { name: "test2", value: 456 }]) }`; + const expectedMutation = `mutation($data: [TestTable_Data! @allow(fields: "name value")]!) { ${formatedTableName}_insertMany(data: $data) }`; await apiClient.insertMany(tableName, simpleDataArray); - expect(executeGraphqlStub).to.have.been.calledOnceWithExactly(normalizeGraphQLString(expectedMutation)); + expect(executeGraphqlStub).to.have.been.calledOnceWithExactly( + normalizeGraphQLString(expectedMutation), + { variables: { data: simpleDataArray } } + ); }); it('should call executeGraphql with the correct mutation for complex data array', async () => { @@ -817,39 +818,25 @@ describe('DataConnectApiClient CRUD helpers', () => { { id: 'a', active: true, info: { nested: 'n1 "quote"' } }, { id: 'b', scores: [1, 2], info: { nested: 'n2/\\' } } ]; - const expectedMutation = ` - mutation { - ${formatedTableName}_insertMany(data: - [{ id: "a", active: true, info: { nested: "n1 \\"quote\\"" } }, { id: "b", scores: [1, 2], - info: { nested: "n2/\\\\" } }]) }`; + const expectedMutation = `mutation($data: [TestTable_Data! @allow(fields: "id active info scores")]!) { ${formatedTableName}_insertMany(data: $data) }`; await apiClient.insertMany(tableName, complexDataArray); - expect(executeGraphqlStub).to.have.been.calledOnceWithExactly(normalizeGraphQLString(expectedMutation)); + expect(executeGraphqlStub).to.have.been.calledOnceWithExactly( + normalizeGraphQLString(expectedMutation), + { variables: { data: complexDataArray } } + ); }); 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 }] - }]) - }`; + ]; + const expectedMutation = `mutation($data: [TestTable_Data! @allow(fields: "genre title ratings director extras")]!) { ${formatedTableName}_insertMany(data: $data) }`; await apiClient.insertMany(tableName, dataArray); - expect(executeGraphqlStub).to.have.been.calledOnceWithExactly(normalizeGraphQLString(expectedMutation)); + expect(executeGraphqlStub).to.have.been.calledOnceWithExactly( + normalizeGraphQLString(expectedMutation), + { variables: { data: dataArray } } + ); }); it('should throw FirebaseDataConnectError for invalid tableName', async () => { @@ -906,43 +893,45 @@ describe('DataConnectApiClient CRUD helpers', () => { // --- UPSERT TESTS --- describe('upsert()', () => { tableNames.forEach((tableName, index) => { - const expectedMutation = `mutation { ${formatedTableNames[index]}_upsert(data: { name: "a" }) }`; + const capitalizedTable = capitalize(formatedTableNames[index]); + const expectedMutation = `mutation($data: ${capitalizedTable}_Data! @allow(fields: "name")) { ${formatedTableNames[index]}_upsert(data: $data) }`; 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)); + await expect(executeGraphqlStub).to.have.been.calledOnceWithExactly( + normalizeGraphQLString(expectedMutation), + { variables: { data: { name: 'a' } } } + ); }); }); 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" }) }`; + const expectedMutation = `mutation($data: TestTable_Data! @allow(fields: "id value")) { ${formatedTableName}_upsert(data: $data) }`; await apiClient.upsert(tableName, simpleData); - expect(executeGraphqlStub).to.have.been.calledOnceWithExactly(expectedMutation); + expect(executeGraphqlStub).to.have.been.calledOnceWithExactly( + normalizeGraphQLString(expectedMutation), + { variables: { data: simpleData } } + ); }); 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 expectedMutation = ` - mutation { ${formatedTableName}_upsert(data: - { id: "key2", active: false, items: [1, null], detail: { status: "done/\\\\" } }) }`; + const expectedMutation = `mutation($data: TestTable_Data! @allow(fields: "id active items detail")) { ${formatedTableName}_upsert(data: $data) }`; await apiClient.upsert(tableName, complexData); - expect(executeGraphqlStub).to.have.been.calledOnceWithExactly(normalizeGraphQLString(expectedMutation)); + expect(executeGraphqlStub).to.have.been.calledOnceWithExactly( + normalizeGraphQLString(expectedMutation), + { variables: { data: complexData } } + ); }); 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 }] - }) - }`; + const expectedMutation = `mutation($data: TestTable_Data! @allow(fields: "genre title ratings director extras")) { ${formatedTableName}_upsert(data: $data) }`; await apiClient.upsert(tableName, dataWithUndefined); - expect(executeGraphqlStub).to.have.been.calledOnceWithExactly(normalizeGraphQLString(expectedMutation)); + expect(executeGraphqlStub).to.have.been.calledOnceWithExactly( + normalizeGraphQLString(expectedMutation), + { variables: { data: dataWithUndefined } } + ); }); it('should throw FirebaseDataConnectError for invalid tableName', async () => { @@ -991,20 +980,26 @@ describe('DataConnectApiClient CRUD helpers', () => { // --- UPSERT MANY TESTS --- describe('upsertMany()', () => { tableNames.forEach((tableName, index) => { - const expectedMutation = `mutation { ${formatedTableNames[index]}_upsertMany(data: [{ name: "a" }]) }`; + const capitalizedTable = capitalize(formatedTableNames[index]); + const expectedMutation = `mutation($data: [${capitalizedTable}_Data! @allow(fields: "name")]!) { ${formatedTableNames[index]}_upsertMany(data: $data) }`; 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)); + await expect(executeGraphqlStub).to.have.been.calledOnceWithExactly( + normalizeGraphQLString(expectedMutation), + { variables: { data: [{ name: 'a' }] } } + ); }); }); it('should call executeGraphql with the correct mutation for simple data array', async () => { const simpleDataArray = [{ id: 'k1' }, { id: 'k2', value: 99 }]; - const expectedMutation = ` - mutation { ${formatedTableName}_upsertMany(data: [{ id: "k1" }, { id: "k2", value: 99 }]) }`; + const expectedMutation = `mutation($data: [TestTable_Data! @allow(fields: "id value")]!) { ${formatedTableName}_upsertMany(data: $data) }`; await apiClient.upsertMany(tableName, simpleDataArray); - expect(executeGraphqlStub).to.have.been.calledOnceWithExactly(normalizeGraphQLString(expectedMutation)); + expect(executeGraphqlStub).to.have.been.calledOnceWithExactly( + normalizeGraphQLString(expectedMutation), + { variables: { data: simpleDataArray } } + ); }); it('should call executeGraphql with the correct mutation for complex data array', async () => { @@ -1012,37 +1007,25 @@ describe('DataConnectApiClient CRUD helpers', () => { { id: 'x', active: true, info: { nested: 'n1/\\"x' } }, { id: 'y', scores: [null, 2] } ]; - const expectedMutation = ` - mutation { ${formatedTableName}_upsertMany(data: - [{ id: "x", active: true, info: { nested: "n1/\\\\\\"x" } }, { id: "y", scores: [null, 2] }]) }`; + const expectedMutation = `mutation($data: [TestTable_Data! @allow(fields: "id active info scores")]!) { ${formatedTableName}_upsertMany(data: $data) }`; await apiClient.upsertMany(tableName, complexDataArray); - expect(executeGraphqlStub).to.have.been.calledOnceWithExactly(normalizeGraphQLString(expectedMutation)); + expect(executeGraphqlStub).to.have.been.calledOnceWithExactly( + normalizeGraphQLString(expectedMutation), + { variables: { data: complexDataArray } } + ); }); 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 }] - }]) - }`; + ]; + const expectedMutation = `mutation($data: [TestTable_Data! @allow(fields: "genre title ratings director extras")]!) { ${formatedTableName}_upsertMany(data: $data) }`; await apiClient.upsertMany(tableName, dataArray); - expect(executeGraphqlStub).to.have.been.calledOnceWithExactly(normalizeGraphQLString(expectedMutation)); + expect(executeGraphqlStub).to.have.been.calledOnceWithExactly( + normalizeGraphQLString(expectedMutation), + { variables: { data: dataArray } } + ); }); it('should throw FirebaseDataConnectError for invalid tableName', async () => { @@ -1097,48 +1080,48 @@ describe('DataConnectApiClient CRUD helpers', () => { }); describe('String serialization', () => { - it('should correctly escape special characters in strings during insert', async () => { + it('should correctly handle special characters in strings during insert', async () => { const data = { content: 'Line 1\nLine 2', }; await apiClient.insert(tableName, data); - const callArgs = executeGraphqlStub.firstCall.args[0]; + const callOptions = executeGraphqlStub.firstCall.args[1]; - expect(callArgs).to.include(String.raw`content: "Line 1\nLine 2"`); + expect(callOptions.variables.data.content).to.equal('Line 1\nLine 2'); }); - it('should correctly escape backslash', async () => { + it('should correctly handle backslash', async () => { const data = { content: 'Backslash \\', }; await apiClient.insert(tableName, data); - const callArgs = executeGraphqlStub.firstCall.args[0]; + const callOptions = executeGraphqlStub.firstCall.args[1]; - expect(callArgs).to.include(String.raw`content: "Backslash \\"`); + expect(callOptions.variables.data.content).to.equal('Backslash \\'); }); - it('should correctly escape double quotes', async () => { + it('should correctly handle double quotes', async () => { const data = { content: 'Quote "test"', }; await apiClient.insert(tableName, data); - const callArgs = executeGraphqlStub.firstCall.args[0]; + const callOptions = executeGraphqlStub.firstCall.args[1]; - expect(callArgs).to.include(String.raw`content: "Quote \"test\""`); + expect(callOptions.variables.data.content).to.equal('Quote "test"'); }); - it('should correctly escape tab character', async () => { + it('should correctly handle tab character', async () => { const data = { content: 'Tab\tCharacter', }; await apiClient.insert(tableName, data); - const callArgs = executeGraphqlStub.firstCall.args[0]; + const callOptions = executeGraphqlStub.firstCall.args[1]; - expect(callArgs).to.include(String.raw`content: "Tab\tCharacter"`); + expect(callOptions.variables.data.content).to.equal('Tab\tCharacter'); }); it('should correctly handle emojis', async () => { @@ -1147,9 +1130,9 @@ describe('DataConnectApiClient CRUD helpers', () => { }; await apiClient.insert(tableName, data); - const callArgs = executeGraphqlStub.firstCall.args[0]; + const callOptions = executeGraphqlStub.firstCall.args[1]; - expect(callArgs).to.include('content: "Emoji 😊"'); + expect(callOptions.variables.data.content).to.equal('Emoji 😊'); }); }); }); From ad84ef5dd57c02b0bb241d4e8b597cbda277f36f Mon Sep 17 00:00:00 2001 From: stephenarosaj Date: Fri, 12 Jun 2026 13:11:19 -0700 Subject: [PATCH 05/16] update the @allow directive for list inputs --- .../data-connect-api-client-internal.ts | 4 ++-- .../data-connect-api-client-internal.spec.ts | 20 +++++++++---------- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/data-connect/data-connect-api-client-internal.ts b/src/data-connect/data-connect-api-client-internal.ts index 0fe2b74dc8..e534671561 100644 --- a/src/data-connect/data-connect-api-client-internal.ts +++ b/src/data-connect/data-connect-api-client-internal.ts @@ -544,7 +544,7 @@ export class DataConnectApiClient { try { const { capitalized, formatted } = this.getTableNames(tableName); const keys = this.getArrayObjectsKeys(data); - const mutation = `mutation($data: [${capitalized}_Data! @allow(fields: "${keys}")]!) { ${formatted}_insertMany(data: $data) }`; + const mutation = `mutation($data: [${capitalized}_Data!]! @allow(fields: "${keys}")) { ${formatted}_insertMany(data: $data) }`; return this.executeGraphql(mutation, { variables: { data } }) .catch(this.handleBulkImportErrors); @@ -623,7 +623,7 @@ export class DataConnectApiClient { try { const { capitalized, formatted } = this.getTableNames(tableName); const keys = this.getArrayObjectsKeys(data); - const mutation = `mutation($data: [${capitalized}_Data! @allow(fields: "${keys}")]!) { ${formatted}_upsertMany(data: $data) }`; + const mutation = `mutation($data: [${capitalized}_Data!]! @allow(fields: "${keys}")) { ${formatted}_upsertMany(data: $data) }`; return this.executeGraphql(mutation, { variables: { data } }) .catch(this.handleBulkImportErrors); 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 38e21b1bcd..e53131e7c6 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 @@ -792,7 +792,7 @@ describe('DataConnectApiClient CRUD helpers', () => { describe('insertMany()', () => { tableNames.forEach((tableName, index) => { const capitalizedTable = capitalize(formatedTableNames[index]); - const expectedMutation = `mutation($data: [${capitalizedTable}_Data! @allow(fields: "name")]!) { ${formatedTableNames[index]}_insertMany(data: $data) }`; + const expectedMutation = `mutation($data: [${capitalizedTable}_Data!]! @allow(fields: "name")) { ${formatedTableNames[index]}_insertMany(data: $data) }`; it(`should use the formatted tableName in the gql query: "${tableName}" as "${formatedTableNames[index]}"`, async () => { await apiClient.insertMany(tableName, [{ name: 'a' }]); @@ -805,7 +805,7 @@ describe('DataConnectApiClient CRUD helpers', () => { it('should call executeGraphql with the correct mutation for simple data array', async () => { const simpleDataArray = [{ name: 'test1' }, { name: 'test2', value: 456 }]; - const expectedMutation = `mutation($data: [TestTable_Data! @allow(fields: "name value")]!) { ${formatedTableName}_insertMany(data: $data) }`; + const expectedMutation = `mutation($data: [TestTable_Data!]! @allow(fields: "name value")) { ${formatedTableName}_insertMany(data: $data) }`; await apiClient.insertMany(tableName, simpleDataArray); expect(executeGraphqlStub).to.have.been.calledOnceWithExactly( normalizeGraphQLString(expectedMutation), @@ -818,7 +818,7 @@ describe('DataConnectApiClient CRUD helpers', () => { { id: 'a', active: true, info: { nested: 'n1 "quote"' } }, { id: 'b', scores: [1, 2], info: { nested: 'n2/\\' } } ]; - const expectedMutation = `mutation($data: [TestTable_Data! @allow(fields: "id active info scores")]!) { ${formatedTableName}_insertMany(data: $data) }`; + const expectedMutation = `mutation($data: [TestTable_Data!]! @allow(fields: "id active info scores")) { ${formatedTableName}_insertMany(data: $data) }`; await apiClient.insertMany(tableName, complexDataArray); expect(executeGraphqlStub).to.have.been.calledOnceWithExactly( normalizeGraphQLString(expectedMutation), @@ -831,7 +831,7 @@ describe('DataConnectApiClient CRUD helpers', () => { dataWithUndefined, dataWithUndefined ]; - const expectedMutation = `mutation($data: [TestTable_Data! @allow(fields: "genre title ratings director extras")]!) { ${formatedTableName}_insertMany(data: $data) }`; + const expectedMutation = `mutation($data: [TestTable_Data!]! @allow(fields: "genre title ratings director extras")) { ${formatedTableName}_insertMany(data: $data) }`; await apiClient.insertMany(tableName, dataArray); expect(executeGraphqlStub).to.have.been.calledOnceWithExactly( normalizeGraphQLString(expectedMutation), @@ -876,7 +876,7 @@ describe('DataConnectApiClient CRUD helpers', () => { { id: 'key2', name: 'Bob', tags: ['cool'] } ]; const expectedMutation = ` - mutation($data: [TestTable_Data! @allow(fields: "id name status tags")]!) { + mutation($data: [TestTable_Data!]! @allow(fields: "id name status tags")) { testTable_insertMany(data: $data) }`; const expectedOptions = { @@ -981,7 +981,7 @@ describe('DataConnectApiClient CRUD helpers', () => { describe('upsertMany()', () => { tableNames.forEach((tableName, index) => { const capitalizedTable = capitalize(formatedTableNames[index]); - const expectedMutation = `mutation($data: [${capitalizedTable}_Data! @allow(fields: "name")]!) { ${formatedTableNames[index]}_upsertMany(data: $data) }`; + const expectedMutation = `mutation($data: [${capitalizedTable}_Data!]! @allow(fields: "name")) { ${formatedTableNames[index]}_upsertMany(data: $data) }`; it(`should use the formatted tableName in the gql query: "${tableName}" as "${formatedTableNames[index]}"`, async () => { await apiClient.upsertMany(tableName, [{ name: 'a' }]); @@ -994,7 +994,7 @@ 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($data: [TestTable_Data! @allow(fields: "id value")]!) { ${formatedTableName}_upsertMany(data: $data) }`; + const expectedMutation = `mutation($data: [TestTable_Data!]! @allow(fields: "id value")) { ${formatedTableName}_upsertMany(data: $data) }`; await apiClient.upsertMany(tableName, simpleDataArray); expect(executeGraphqlStub).to.have.been.calledOnceWithExactly( normalizeGraphQLString(expectedMutation), @@ -1007,7 +1007,7 @@ describe('DataConnectApiClient CRUD helpers', () => { { id: 'x', active: true, info: { nested: 'n1/\\"x' } }, { id: 'y', scores: [null, 2] } ]; - const expectedMutation = `mutation($data: [TestTable_Data! @allow(fields: "id active info scores")]!) { ${formatedTableName}_upsertMany(data: $data) }`; + const expectedMutation = `mutation($data: [TestTable_Data!]! @allow(fields: "id active info scores")) { ${formatedTableName}_upsertMany(data: $data) }`; await apiClient.upsertMany(tableName, complexDataArray); expect(executeGraphqlStub).to.have.been.calledOnceWithExactly( normalizeGraphQLString(expectedMutation), @@ -1020,7 +1020,7 @@ describe('DataConnectApiClient CRUD helpers', () => { dataWithUndefined, dataWithUndefined ]; - const expectedMutation = `mutation($data: [TestTable_Data! @allow(fields: "genre title ratings director extras")]!) { ${formatedTableName}_upsertMany(data: $data) }`; + const expectedMutation = `mutation($data: [TestTable_Data!]! @allow(fields: "genre title ratings director extras")) { ${formatedTableName}_upsertMany(data: $data) }`; await apiClient.upsertMany(tableName, dataArray); expect(executeGraphqlStub).to.have.been.calledOnceWithExactly( normalizeGraphQLString(expectedMutation), @@ -1065,7 +1065,7 @@ describe('DataConnectApiClient CRUD helpers', () => { { id: 'key2', name: 'Bob', tags: ['cool'] } ]; const expectedMutation = ` - mutation($data: [TestTable_Data! @allow(fields: "id name status tags")]!) { + mutation($data: [TestTable_Data!]! @allow(fields: "id name status tags")) { testTable_upsertMany(data: $data) }`; const expectedOptions = { From c00855bfc2860f96a72213882b1cf3f34fd29b4c Mon Sep 17 00:00:00 2001 From: stephenarosaj Date: Mon, 15 Jun 2026 16:38:51 -0700 Subject: [PATCH 06/16] initial fix --- .gitignore | 5 +++- .../data-connect-api-client-internal.ts | 24 ++++++++++++------- src/data-connect/error.ts | 23 ++++++++++++++++++ .../data-connect-api-client-internal.spec.ts | 21 ++++++++++++++++ 4 files changed, 64 insertions(+), 9 deletions(-) diff --git a/.gitignore b/.gitignore index 141eeb7f06..dd780b4a90 100644 --- a/.gitignore +++ b/.gitignore @@ -22,4 +22,7 @@ firebase-admin-*.tgz docgen/markdown/ # Dataconnect integration test artifacts should not be checked in -test/integration/dataconnect/dataconnect/.dataconnect \ No newline at end of file +test/integration/dataconnect/dataconnect/.dataconnect +test/integration/dataconnect/dataconnect-debug.log +test/integration/dataconnect/firebase-debug.log +test/integration/dataconnect/pglite-debug.log diff --git a/src/data-connect/data-connect-api-client-internal.ts b/src/data-connect/data-connect-api-client-internal.ts index 8cb5ee289b..a874dc5cd6 100644 --- a/src/data-connect/data-connect-api-client-internal.ts +++ b/src/data-connect/data-connect-api-client-internal.ts @@ -21,7 +21,12 @@ import { HttpRequestConfig, HttpClient, RequestResponseError, AuthorizedHttpClient } from '../utils/api-request'; import { FirebaseError, toHttpResponse } from '../utils/error'; -import { FirebaseDataConnectError, DataConnectErrorCode, DATA_CONNECT_ERROR_CODE_MAPPING } from './error'; +import { + FirebaseDataConnectError, + DataConnectErrorCode, + DATA_CONNECT_ERROR_CODE_MAPPING, + GRPC_STATUS_CODE_TO_STRING +} from './error'; import * as utils from '../utils/index'; import * as validator from '../utils/validator'; import { ConnectorConfig, ExecuteGraphqlResponse, GraphqlOptions, OperationOptions } from './data-connect-api'; @@ -409,10 +414,17 @@ export class DataConnectApiClient { }); } - const error: ServerError = (response.data as ErrorResponse).error || {}; + const data = response.data as any; + const error: ServerError = (validator.isNonNullObject(data) && 'error' in data) ? data.error : data || {}; + + let status = error.status; + if (!status && validator.isNumber(error.code)) { + status = GRPC_STATUS_CODE_TO_STRING[error.code as number]; + } + let code: DataConnectErrorCode = DATA_CONNECT_ERROR_CODE_MAPPING.UNKNOWN; - if (error.status && error.status in DATA_CONNECT_ERROR_CODE_MAPPING) { - code = DATA_CONNECT_ERROR_CODE_MAPPING[error.status]; + if (status && status in DATA_CONNECT_ERROR_CODE_MAPPING) { + code = DATA_CONNECT_ERROR_CODE_MAPPING[status]; } const message = error.message || 'Unknown server error'; return new FirebaseDataConnectError({ @@ -670,10 +682,6 @@ export function useEmulator(): boolean { return !!emulatorHost(); } -interface ErrorResponse { - error?: ServerError; -} - interface ServerError { code?: number; message?: string; diff --git a/src/data-connect/error.ts b/src/data-connect/error.ts index 4c92aa48ab..f547856fae 100644 --- a/src/data-connect/error.ts +++ b/src/data-connect/error.ts @@ -69,3 +69,26 @@ export class FirebaseDataConnectError extends FirebaseError { }); } } + +/** + * Mappings from gRPC status codes to their string equivalents. + * @internal + */ +export const GRPC_STATUS_CODE_TO_STRING: Record = { + 1: 'CANCELLED', + 2: 'UNKNOWN', + 3: 'INVALID_ARGUMENT', + 4: 'DEADLINE_EXCEEDED', + 5: 'NOT_FOUND', + 6: 'ALREADY_EXISTS', + 7: 'PERMISSION_DENIED', + 8: 'RESOURCE_EXHAUSTED', + 9: 'FAILED_PRECONDITION', + 10: 'ABORTED', + 11: 'OUT_OF_RANGE', + 12: 'UNIMPLEMENTED', + 13: 'INTERNAL', + 14: 'UNAVAILABLE', + 15: 'DATA_LOSS', + 16: 'UNAUTHENTICATED', +}; 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 5951c29264..7595928f05 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 @@ -179,6 +179,27 @@ describe('DataConnectApiClient', () => { .and.have.property('cause', expected.cause); }); + it('should reject when a gRPC-to-HTTP transcoded error response is received', () => { + const grpcError = { + code: 7, + message: 'Permission denied', + }; + const mockErr = utils.errorFrom(grpcError, 403); + sandbox + .stub(HttpClient.prototype, 'send') + .rejects(mockErr); + const expected = new FirebaseDataConnectError({ + code: 'permission-denied', + message: 'Permission denied', + httpResponse: toHttpResponse(mockErr.response), + cause: mockErr + }); + return apiClient.executeGraphql('query', {}) + .should.eventually.be.rejected + .and.deep.include(expected) + .and.have.property('cause', expected.cause); + }); + it('should reject with unknown-error when error code is not present', () => { const mockErr = utils.errorFrom({}, 404); sandbox From 27da9b2515c7ce42cb9f804da87455c81310ea0c Mon Sep 17 00:00:00 2001 From: stephenarosaj Date: Tue, 16 Jun 2026 11:00:05 -0700 Subject: [PATCH 07/16] address reviewer comments --- src/data-connect/data-connect-api-client-internal.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/data-connect/data-connect-api-client-internal.ts b/src/data-connect/data-connect-api-client-internal.ts index a874dc5cd6..fe0c56b36c 100644 --- a/src/data-connect/data-connect-api-client-internal.ts +++ b/src/data-connect/data-connect-api-client-internal.ts @@ -415,8 +415,10 @@ export class DataConnectApiClient { } const data = response.data as any; - const error: ServerError = (validator.isNonNullObject(data) && 'error' in data) ? data.error : data || {}; - + const error: ServerError = (validator.isNonNullObject(data) && validator.isNonNullObject(data.error)) + ? data.error + : (validator.isNonNullObject(data) ? data : {}); + let status = error.status; if (!status && validator.isNumber(error.code)) { status = GRPC_STATUS_CODE_TO_STRING[error.code as number]; From 6fe0ea29c5e4422649101a2843e9f72362a36f87 Mon Sep 17 00:00:00 2001 From: stephenarosaj Date: Tue, 16 Jun 2026 15:07:40 -0700 Subject: [PATCH 08/16] fix style and normalize expected + actual query strings in tests --- package-lock.json | 1 - .../data-connect-api-client-internal.ts | 20 +- .../data-connect-api-client-internal.spec.ts | 200 +++++++++--------- 3 files changed, 118 insertions(+), 103 deletions(-) diff --git a/package-lock.json b/package-lock.json index bdf6f376a4..8a9491c591 100644 --- a/package-lock.json +++ b/package-lock.json @@ -12,7 +12,6 @@ "@fastify/busboy": "^3.0.0", "@firebase/database-compat": "^2.1.4", "@firebase/database-types": "^1.0.20", - "@google-cloud/storage": "7.21.0", "farmhash-modern": "^1.1.0", "fast-deep-equal": "^3.1.1", "google-auth-library": "^10.6.2", diff --git a/src/data-connect/data-connect-api-client-internal.ts b/src/data-connect/data-connect-api-client-internal.ts index 6c661ec515..2affeb3bb2 100644 --- a/src/data-connect/data-connect-api-client-internal.ts +++ b/src/data-connect/data-connect-api-client-internal.ts @@ -522,7 +522,10 @@ export class DataConnectApiClient { try { const { capitalized, formatted } = this.getTableNames(tableName); const keys = this.getObjectKeys(data); - const mutation = `mutation($data: ${capitalized}_Data! @allow(fields: "${keys}")) { ${formatted}_insert(data: $data) }`; + const mutation = + `mutation($data: ${capitalized}_Data! @allow(fields: "${keys}")) { + ${formatted}_insert(data: $data) + }`; return this.executeGraphql(mutation, { variables: { data } }) .catch(this.handleBulkImportErrors); @@ -558,7 +561,10 @@ export class DataConnectApiClient { try { const { capitalized, formatted } = this.getTableNames(tableName); const keys = this.getArrayObjectsKeys(data); - const mutation = `mutation($data: [${capitalized}_Data!]! @allow(fields: "${keys}")) { ${formatted}_insertMany(data: $data) }`; + const mutation = + `mutation($data: [${capitalized}_Data!]! @allow(fields: "${keys}")) { + ${formatted}_insertMany(data: $data) + }`; return this.executeGraphql(mutation, { variables: { data } }) .catch(this.handleBulkImportErrors); @@ -601,7 +607,10 @@ export class DataConnectApiClient { try { const { capitalized, formatted } = this.getTableNames(tableName); const keys = this.getObjectKeys(data); - const mutation = `mutation($data: ${capitalized}_Data! @allow(fields: "${keys}")) { ${formatted}_upsert(data: $data) }`; + const mutation = + `mutation($data: ${capitalized}_Data! @allow(fields: "${keys}")) { + ${formatted}_upsert(data: $data) + }`; return this.executeGraphql(mutation, { variables: { data } }) .catch(this.handleBulkImportErrors); @@ -637,7 +646,10 @@ export class DataConnectApiClient { try { const { capitalized, formatted } = this.getTableNames(tableName); const keys = this.getArrayObjectsKeys(data); - const mutation = `mutation($data: [${capitalized}_Data!]! @allow(fields: "${keys}")) { ${formatted}_upsertMany(data: $data) }`; + const mutation = + `mutation($data: [${capitalized}_Data!]! @allow(fields: "${keys}")) { + ${formatted}_upsertMany(data: $data) + }`; return this.executeGraphql(mutation, { variables: { data } }) .catch(this.handleBulkImportErrors); 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 c87c8fb832..3d347bc9c9 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 @@ -699,11 +699,26 @@ describe('DataConnectApiClient CRUD helpers', () => { // 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 + .replace(/\n/g, '') // Remove newlines + .replace(/\s+/g, ' ') // Replace multiple spaces with a single space .trim(); // Remove leading/trailing whitespace from the whole string }; + /** + * Helper function to normalize and validate the executeGraphql calls. Importantly, + * normalizes the actual input and the expected input to account for whitespace + * diffs. + */ + function expectNormalizedExecuteGraphqlCall( + expectedQuery: string, + expectedVariables: Record + ): void { + expect(executeGraphqlStub).to.have.been.calledOnce; + const call = executeGraphqlStub.getCall(0); + expect(normalizeGraphQLString(call.args[0])).to.equal(normalizeGraphQLString(expectedQuery)); + expect(call.args[1]).to.deep.equal(expectedVariables); + } + const capitalize = (str: string): string => { return str.charAt(0).toUpperCase() + str.slice(1); }; @@ -726,44 +741,44 @@ describe('DataConnectApiClient CRUD helpers', () => { describe('insert()', () => { tableNames.forEach((tableName, index) => { const capitalizedTable = capitalize(formatedTableNames[index]); - const expectedMutation = `mutation($data: ${capitalizedTable}_Data! @allow(fields: "name")) { ${formatedTableNames[index]}_insert(data: $data) }`; + const expectedMutation = + `mutation($data: ${capitalizedTable}_Data! @allow(fields: "name")) { + ${formatedTableNames[index]}_insert(data: $data) + }`; 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), - { variables: { data: { name: 'a' } } } - ); + expectNormalizedExecuteGraphqlCall(expectedMutation, { variables: { data: { name: 'a' } } }); }); }); it('should call executeGraphql with the correct mutation for simple data', async () => { const simpleData = { name: 'test', value: 123 }; - const expectedMutation = `mutation($data: TestTable_Data! @allow(fields: "name value")) { ${formatedTableName}_insert(data: $data) }`; + const expectedMutation = + `mutation($data: TestTable_Data! @allow(fields: "name value")) { + ${formatedTableName}_insert(data: $data) + }`; await apiClient.insert(tableName, simpleData); - expect(executeGraphqlStub).to.have.been.calledOnceWithExactly( - normalizeGraphQLString(expectedMutation), - { variables: { data: simpleData } } - ); + expectNormalizedExecuteGraphqlCall(expectedMutation, { variables: { data: simpleData } }); }); 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\\' } }; - const expectedMutation = `mutation($data: TestTable_Data! @allow(fields: "id active scores info")) { ${formatedTableName}_insert(data: $data) }`; + const expectedMutation = + `mutation($data: TestTable_Data! @allow(fields: "id active scores info")) { + ${formatedTableName}_insert(data: $data) + }`; await apiClient.insert(tableName, complexData); - expect(executeGraphqlStub).to.have.been.calledOnceWithExactly( - normalizeGraphQLString(expectedMutation), - { variables: { data: complexData } } - ); + expectNormalizedExecuteGraphqlCall(expectedMutation, { variables: { data: complexData } }); }); it('should call executeGraphql with the correct mutation for undefined and null values', async () => { - const expectedMutation = `mutation($data: TestTable_Data! @allow(fields: "genre title ratings director extras")) { ${formatedTableName}_insert(data: $data) }`; + const expectedMutation = + `mutation($data: TestTable_Data! @allow(fields: "genre title ratings director extras")) { + ${formatedTableName}_insert(data: $data) + }`; await apiClient.insert(tableName, dataWithUndefined); - expect(executeGraphqlStub).to.have.been.calledOnceWithExactly( - normalizeGraphQLString(expectedMutation), - { variables: { data: dataWithUndefined } } - ); + expectNormalizedExecuteGraphqlCall(expectedMutation, { variables: { data: dataWithUndefined } }); }); it('should throw FirebaseDataConnectError for invalid tableName', async () => { @@ -802,10 +817,7 @@ describe('DataConnectApiClient CRUD helpers', () => { variables: { data } }; await apiClient.insert(tableName, data); - expect(executeGraphqlStub).to.have.been.calledOnceWithExactly( - normalizeGraphQLString(expectedMutation), - expectedOptions - ); + expectNormalizedExecuteGraphqlCall(expectedMutation, expectedOptions); }); }); @@ -813,25 +825,25 @@ describe('DataConnectApiClient CRUD helpers', () => { describe('insertMany()', () => { tableNames.forEach((tableName, index) => { const capitalizedTable = capitalize(formatedTableNames[index]); - const expectedMutation = `mutation($data: [${capitalizedTable}_Data!]! @allow(fields: "name")) { ${formatedTableNames[index]}_insertMany(data: $data) }`; + const expectedMutation = ` + mutation($data: [${capitalizedTable}_Data!]! @allow(fields: "name")) { + ${formatedTableNames[index]}_insertMany(data: $data) + }`; 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), - { variables: { data: [{ name: 'a' }] } } - ); + expectNormalizedExecuteGraphqlCall(expectedMutation, { variables: { data: [{ name: 'a' }] } }); }); }); it('should call executeGraphql with the correct mutation for simple data array', async () => { const simpleDataArray = [{ name: 'test1' }, { name: 'test2', value: 456 }]; - const expectedMutation = `mutation($data: [TestTable_Data!]! @allow(fields: "name value")) { ${formatedTableName}_insertMany(data: $data) }`; + const expectedMutation = ` + mutation($data: [TestTable_Data!]! @allow(fields: "name value")) { + ${formatedTableName}_insertMany(data: $data) + }`; await apiClient.insertMany(tableName, simpleDataArray); - expect(executeGraphqlStub).to.have.been.calledOnceWithExactly( - normalizeGraphQLString(expectedMutation), - { variables: { data: simpleDataArray } } - ); + expectNormalizedExecuteGraphqlCall(expectedMutation, { variables: { data: simpleDataArray } }); }); it('should call executeGraphql with the correct mutation for complex data array', async () => { @@ -839,12 +851,12 @@ describe('DataConnectApiClient CRUD helpers', () => { { id: 'a', active: true, info: { nested: 'n1 "quote"' } }, { id: 'b', scores: [1, 2], info: { nested: 'n2/\\' } } ]; - const expectedMutation = `mutation($data: [TestTable_Data!]! @allow(fields: "id active info scores")) { ${formatedTableName}_insertMany(data: $data) }`; + const expectedMutation = ` + mutation($data: [TestTable_Data!]! @allow(fields: "id active info scores")) { + ${formatedTableName}_insertMany(data: $data) + }`; await apiClient.insertMany(tableName, complexDataArray); - expect(executeGraphqlStub).to.have.been.calledOnceWithExactly( - normalizeGraphQLString(expectedMutation), - { variables: { data: complexDataArray } } - ); + expectNormalizedExecuteGraphqlCall(expectedMutation, { variables: { data: complexDataArray } }); }); it('should call executeGraphql with the correct mutation for undefined and null', async () => { @@ -852,12 +864,12 @@ describe('DataConnectApiClient CRUD helpers', () => { dataWithUndefined, dataWithUndefined ]; - const expectedMutation = `mutation($data: [TestTable_Data!]! @allow(fields: "genre title ratings director extras")) { ${formatedTableName}_insertMany(data: $data) }`; + const expectedMutation = ` + mutation($data: [TestTable_Data!]! @allow(fields: "genre title ratings director extras")) { + ${formatedTableName}_insertMany(data: $data) + }`; await apiClient.insertMany(tableName, dataArray); - expect(executeGraphqlStub).to.have.been.calledOnceWithExactly( - normalizeGraphQLString(expectedMutation), - { variables: { data: dataArray } } - ); + expectNormalizedExecuteGraphqlCall(expectedMutation, { variables: { data: dataArray } }); }); it('should throw FirebaseDataConnectError for invalid tableName', async () => { @@ -904,10 +916,7 @@ describe('DataConnectApiClient CRUD helpers', () => { variables: { data } }; await apiClient.insertMany(tableName, data); - expect(executeGraphqlStub).to.have.been.calledOnceWithExactly( - normalizeGraphQLString(expectedMutation), - expectedOptions - ); + expectNormalizedExecuteGraphqlCall(expectedMutation, expectedOptions); }); }); @@ -915,44 +924,44 @@ describe('DataConnectApiClient CRUD helpers', () => { describe('upsert()', () => { tableNames.forEach((tableName, index) => { const capitalizedTable = capitalize(formatedTableNames[index]); - const expectedMutation = `mutation($data: ${capitalizedTable}_Data! @allow(fields: "name")) { ${formatedTableNames[index]}_upsert(data: $data) }`; + const expectedMutation = ` + mutation($data: ${capitalizedTable}_Data! @allow(fields: "name")) { + ${formatedTableNames[index]}_upsert(data: $data) + }`; 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), - { variables: { data: { name: 'a' } } } - ); + expectNormalizedExecuteGraphqlCall(expectedMutation, { variables: { data: { name: 'a' } } }); }); }); it('should call executeGraphql with the correct mutation for simple data', async () => { const simpleData = { id: 'key1', value: 'updated' }; - const expectedMutation = `mutation($data: TestTable_Data! @allow(fields: "id value")) { ${formatedTableName}_upsert(data: $data) }`; + const expectedMutation = ` + mutation($data: TestTable_Data! @allow(fields: "id value")) { + ${formatedTableName}_upsert(data: $data) + }`; await apiClient.upsert(tableName, simpleData); - expect(executeGraphqlStub).to.have.been.calledOnceWithExactly( - normalizeGraphQLString(expectedMutation), - { variables: { data: simpleData } } - ); + expectNormalizedExecuteGraphqlCall(expectedMutation, { variables: { data: simpleData } }); }); 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 expectedMutation = `mutation($data: TestTable_Data! @allow(fields: "id active items detail")) { ${formatedTableName}_upsert(data: $data) }`; + const expectedMutation = ` + mutation($data: TestTable_Data! @allow(fields: "id active items detail")) { + ${formatedTableName}_upsert(data: $data) + }`; await apiClient.upsert(tableName, complexData); - expect(executeGraphqlStub).to.have.been.calledOnceWithExactly( - normalizeGraphQLString(expectedMutation), - { variables: { data: complexData } } - ); + expectNormalizedExecuteGraphqlCall(expectedMutation, { variables: { data: complexData } }); }); it('should call executeGraphql with the correct mutation for undefined and null values', async () => { - const expectedMutation = `mutation($data: TestTable_Data! @allow(fields: "genre title ratings director extras")) { ${formatedTableName}_upsert(data: $data) }`; + const expectedMutation = ` + mutation($data: TestTable_Data! @allow(fields: "genre title ratings director extras")) { + ${formatedTableName}_upsert(data: $data) + }`; await apiClient.upsert(tableName, dataWithUndefined); - expect(executeGraphqlStub).to.have.been.calledOnceWithExactly( - normalizeGraphQLString(expectedMutation), - { variables: { data: dataWithUndefined } } - ); + expectNormalizedExecuteGraphqlCall(expectedMutation, { variables: { data: dataWithUndefined } }); }); it('should throw FirebaseDataConnectError for invalid tableName', async () => { @@ -991,10 +1000,7 @@ describe('DataConnectApiClient CRUD helpers', () => { variables: { data } }; await apiClient.upsert(tableName, data); - expect(executeGraphqlStub).to.have.been.calledOnceWithExactly( - normalizeGraphQLString(expectedMutation), - expectedOptions - ); + expectNormalizedExecuteGraphqlCall(expectedMutation, expectedOptions); }); }); @@ -1002,25 +1008,25 @@ describe('DataConnectApiClient CRUD helpers', () => { describe('upsertMany()', () => { tableNames.forEach((tableName, index) => { const capitalizedTable = capitalize(formatedTableNames[index]); - const expectedMutation = `mutation($data: [${capitalizedTable}_Data!]! @allow(fields: "name")) { ${formatedTableNames[index]}_upsertMany(data: $data) }`; + const expectedMutation = ` + mutation($data: [${capitalizedTable}_Data!]! @allow(fields: "name")) { + ${formatedTableNames[index]}_upsertMany(data: $data) + }`; 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), - { variables: { data: [{ name: 'a' }] } } - ); + expectNormalizedExecuteGraphqlCall(expectedMutation, { variables: { data: [{ name: 'a' }] } }); }); }); it('should call executeGraphql with the correct mutation for simple data array', async () => { const simpleDataArray = [{ id: 'k1' }, { id: 'k2', value: 99 }]; - const expectedMutation = `mutation($data: [TestTable_Data!]! @allow(fields: "id value")) { ${formatedTableName}_upsertMany(data: $data) }`; + const expectedMutation = ` + mutation($data: [TestTable_Data!]! @allow(fields: "id value")) { + ${formatedTableName}_upsertMany(data: $data) + }`; await apiClient.upsertMany(tableName, simpleDataArray); - expect(executeGraphqlStub).to.have.been.calledOnceWithExactly( - normalizeGraphQLString(expectedMutation), - { variables: { data: simpleDataArray } } - ); + expectNormalizedExecuteGraphqlCall(expectedMutation, { variables: { data: simpleDataArray } }); }); it('should call executeGraphql with the correct mutation for complex data array', async () => { @@ -1028,12 +1034,12 @@ describe('DataConnectApiClient CRUD helpers', () => { { id: 'x', active: true, info: { nested: 'n1/\\"x' } }, { id: 'y', scores: [null, 2] } ]; - const expectedMutation = `mutation($data: [TestTable_Data!]! @allow(fields: "id active info scores")) { ${formatedTableName}_upsertMany(data: $data) }`; + const expectedMutation = ` + mutation($data: [TestTable_Data!]! @allow(fields: "id active info scores")) { + ${formatedTableName}_upsertMany(data: $data) + }`; await apiClient.upsertMany(tableName, complexDataArray); - expect(executeGraphqlStub).to.have.been.calledOnceWithExactly( - normalizeGraphQLString(expectedMutation), - { variables: { data: complexDataArray } } - ); + expectNormalizedExecuteGraphqlCall(expectedMutation, { variables: { data: complexDataArray } }); }); it('should call executeGraphql with the correct mutation for undefined and null', async () => { @@ -1041,12 +1047,12 @@ describe('DataConnectApiClient CRUD helpers', () => { dataWithUndefined, dataWithUndefined ]; - const expectedMutation = `mutation($data: [TestTable_Data!]! @allow(fields: "genre title ratings director extras")) { ${formatedTableName}_upsertMany(data: $data) }`; + const expectedMutation = ` + mutation($data: [TestTable_Data!]! @allow(fields: "genre title ratings director extras")) { + ${formatedTableName}_upsertMany(data: $data) + }`; await apiClient.upsertMany(tableName, dataArray); - expect(executeGraphqlStub).to.have.been.calledOnceWithExactly( - normalizeGraphQLString(expectedMutation), - { variables: { data: dataArray } } - ); + expectNormalizedExecuteGraphqlCall(expectedMutation, { variables: { data: dataArray } }); }); it('should throw FirebaseDataConnectError for invalid tableName', async () => { @@ -1093,10 +1099,7 @@ describe('DataConnectApiClient CRUD helpers', () => { variables: { data } }; await apiClient.upsertMany(tableName, data); - expect(executeGraphqlStub).to.have.been.calledOnceWithExactly( - normalizeGraphQLString(expectedMutation), - expectedOptions - ); + expectNormalizedExecuteGraphqlCall(expectedMutation, expectedOptions); }); }); @@ -1157,3 +1160,4 @@ describe('DataConnectApiClient CRUD helpers', () => { }); }); }); + From 629eeb9f5caf419fdcbb50d1398faed7ee44567b Mon Sep 17 00:00:00 2001 From: stephenarosaj Date: Wed, 17 Jun 2026 11:11:42 -0700 Subject: [PATCH 09/16] add fdc to integration test documentation and update integration test artifact ignores --- .gitignore | 8 ++++++-- CONTRIBUTING.md | 16 +++++++++++++--- 2 files changed, 19 insertions(+), 5 deletions(-) diff --git a/.gitignore b/.gitignore index 32d4066dfc..e19489c327 100644 --- a/.gitignore +++ b/.gitignore @@ -21,6 +21,10 @@ firebase-admin-*.tgz docgen/markdown/ -# Dataconnect integration test artifacts should not be checked in +# Integration test artifacts should not be checked in +**/database-debug.log +**/firestore-debug.log test/integration/dataconnect/dataconnect/.dataconnect -test/integration/dataconnect/*.log +**/dataconnect-debug.log +**/pglite-debug.log + diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index b1c626c611..2b6e250d89 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -150,9 +150,19 @@ And then: 'npx mocha \"test/integration/{auth,database,firestore}.spec.ts\" --slow 5000 --timeout 20000 --require ts-node/register' ``` -Currently, only the Auth, Database, and Firestore test suites work. Some test -cases will be automatically skipped due to lack of emulator support. The section -below covers how to run the full test suite against an actual Firebase project. +Currently, only the Auth, Database, and Firestore test suites work. Some test cases +will be automatically skipped due to lack of emulator support. + +You can also run the Data Connect test suite against the emulators using the same command, +but with a config file specific to Data Connect emulator testing: + +```bash + firebase emulators:exec \ + --project fake-project-id --only dataconnect --config test/integration/dataconnect/firebase.json \ + 'npx mocha \"test/integration/data-connect.spec.ts\" --slow 5000 --timeout 20000 --require ts-node/register' +``` + +The section below covers how to run the full test suite against an actual Firebase project. #### Integration Tests with an actual Firebase project From 746a37c6677fa32981268f576863b9fde8302523 Mon Sep 17 00:00:00 2001 From: stephenarosaj Date: Wed, 17 Jun 2026 15:46:17 -0700 Subject: [PATCH 10/16] improve getTableNames variable naming for clarity --- .../data-connect-api-client-internal.ts | 34 +++++++++---------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/src/data-connect/data-connect-api-client-internal.ts b/src/data-connect/data-connect-api-client-internal.ts index 943c5754c7..2eed0d5dbf 100644 --- a/src/data-connect/data-connect-api-client-internal.ts +++ b/src/data-connect/data-connect-api-client-internal.ts @@ -441,13 +441,13 @@ export class DataConnectApiClient { * Generates both capitalized and camel-cased variations of a table name. * Capitalization matches the schema types, and camel-case matches mutations. */ - private getTableNames(tableName: string): { capitalized: string; formatted: string } { + private getTableNames(tableName: string): { capitalized: string; camelCase: string } { if (!tableName || tableName.length === 0) { - return { capitalized: tableName, formatted: tableName }; + return { capitalized: tableName, camelCase: tableName }; } const capitalized = tableName.charAt(0).toUpperCase() + tableName.slice(1); - const formatted = tableName.charAt(0).toLowerCase() + tableName.slice(1); - return { capitalized, formatted }; + const camelCase = tableName.charAt(0).toLowerCase() + tableName.slice(1); + return { capitalized, camelCase }; } /** @@ -481,7 +481,7 @@ export class DataConnectApiClient { } private handleBulkImportErrors(err: FirebaseDataConnectError): never { - if (err.code === `data-connect/${DATA_CONNECT_ERROR_CODE_MAPPING.QUERY_ERROR}`){ + if (err.code === `data-connect/${DATA_CONNECT_ERROR_CODE_MAPPING.QUERY_ERROR}`) { throw new FirebaseDataConnectError({ code: DATA_CONNECT_ERROR_CODE_MAPPING.QUERY_ERROR, message: `${err.message}. Make sure that your table name passed in matches the type name in your ` @@ -520,11 +520,11 @@ export class DataConnectApiClient { } try { - const { capitalized, formatted } = this.getTableNames(tableName); + const { capitalized, camelCase } = this.getTableNames(tableName); const keys = this.getObjectKeys(data); - const mutation = + const mutation = `mutation($data: ${capitalized}_Data! @allow(fields: "${keys}")) { - ${formatted}_insert(data: $data) + ${camelCase}_insert(data: $data) }`; return this.executeGraphql(mutation, { variables: { data } }) @@ -559,11 +559,11 @@ export class DataConnectApiClient { } try { - const { capitalized, formatted } = this.getTableNames(tableName); + const { capitalized, camelCase } = this.getTableNames(tableName); const keys = this.getArrayObjectsKeys(data); - const mutation = + const mutation = `mutation($data: [${capitalized}_Data!]! @allow(fields: "${keys}")) { - ${formatted}_insertMany(data: $data) + ${camelCase}_insertMany(data: $data) }`; return this.executeGraphql(mutation, { variables: { data } }) @@ -605,11 +605,11 @@ export class DataConnectApiClient { } try { - const { capitalized, formatted } = this.getTableNames(tableName); + const { capitalized, camelCase } = this.getTableNames(tableName); const keys = this.getObjectKeys(data); - const mutation = + const mutation = `mutation($data: ${capitalized}_Data! @allow(fields: "${keys}")) { - ${formatted}_upsert(data: $data) + ${camelCase}_upsert(data: $data) }`; return this.executeGraphql(mutation, { variables: { data } }) @@ -644,11 +644,11 @@ export class DataConnectApiClient { } try { - const { capitalized, formatted } = this.getTableNames(tableName); + const { capitalized, camelCase } = this.getTableNames(tableName); const keys = this.getArrayObjectsKeys(data); - const mutation = + const mutation = `mutation($data: [${capitalized}_Data!]! @allow(fields: "${keys}")) { - ${formatted}_upsertMany(data: $data) + ${camelCase}_upsertMany(data: $data) }`; return this.executeGraphql(mutation, { variables: { data } }) From 863e3b2d11e1439583d5f734781bd9b2b27819d3 Mon Sep 17 00:00:00 2001 From: stephenarosaj Date: Thu, 25 Jun 2026 14:20:37 -0700 Subject: [PATCH 11/16] add 10k limit and nested coalesced field keys for @allow --- .../data-connect-api-client-internal.ts | 85 +++++++++---- .../data-connect-api-client-internal.spec.ts | 118 ++++++++++++++++-- 2 files changed, 166 insertions(+), 37 deletions(-) diff --git a/src/data-connect/data-connect-api-client-internal.ts b/src/data-connect/data-connect-api-client-internal.ts index 2eed0d5dbf..b348b6856c 100644 --- a/src/data-connect/data-connect-api-client-internal.ts +++ b/src/data-connect/data-connect-api-client-internal.ts @@ -100,6 +100,10 @@ interface ConnectorsUrlParams extends ServicesUrlParams { connectorId: string; } +interface FieldNode { + children: Map; +} + /** * Class that facilitates sending requests to the Firebase Data Connect backend API. * @@ -451,33 +455,50 @@ export class DataConnectApiClient { } /** - * Extracts all defined property keys from an object as a space-separated string. - * Used to build the `@allow(fields: ...)` mutation directive for single operations. + * Extracts property keys from an object or array of objects as a space-separated string, + * including recursively nested object/array fields for the `@allow(fields: ...)` directive. + * Leverages a hierarchical tree to deduplicate and merge fields. */ - private getObjectKeys(data: Record | object): string { - return Object.keys(data) - .filter(key => (data as Record)[key] !== undefined) - .join(' '); + private getFieldsString(data: unknown): string { + const root: FieldNode = { children: new Map() }; + this.mergeFieldsIntoTree(data, root); + return this.serializeFieldNode(root); } - /** - * Extracts the union of all defined property keys across an array of objects - * as a space-separated string. Used to build the `@allow(fields: ...)` mutation - * directive for bulk operations. - */ - private getArrayObjectsKeys(data: Array): string { - const allKeys = new Set(); - for (const element of data) { - if (validator.isNonNullObject(element)) { - const record = element as Record; - Object.keys(record).forEach(key => { - if (record[key] !== undefined) { - allKeys.add(key); - } - }); + private mergeFieldsIntoTree(data: unknown, node: FieldNode): void { + if (validator.isArray(data)) { + for (const item of data) { + this.mergeFieldsIntoTree(item, node); + } + } else if (validator.isNonNullObject(data) && !(data instanceof Date)) { + const record = data as Record; + for (const [key, val] of Object.entries(record)) { + if (val === undefined) { + continue; + } + let childNode = node.children.get(key); + if (!childNode) { + childNode = { children: new Map() }; + node.children.set(key, childNode); + } + this.mergeFieldsIntoTree(val, childNode); } } - return Array.from(allKeys).join(' '); + } + + private serializeFieldNode(node: FieldNode): string { + const parts: string[] = []; + const sortedKeys = Array.from(node.children.keys()).sort((a, b) => a.localeCompare(b)); + for (const key of sortedKeys) { + const childNode = node.children.get(key)!; + if (childNode.children.size > 0) { + const nestedString = this.serializeFieldNode(childNode); + parts.push(`${key} { ${nestedString} }`); + } else { + parts.push(key); + } + } + return parts.join(' '); } private handleBulkImportErrors(err: FirebaseDataConnectError): never { @@ -521,7 +542,7 @@ export class DataConnectApiClient { try { const { capitalized, camelCase } = this.getTableNames(tableName); - const keys = this.getObjectKeys(data); + const keys = this.getFieldsString(data); const mutation = `mutation($data: ${capitalized}_Data! @allow(fields: "${keys}")) { ${camelCase}_insert(data: $data) @@ -557,10 +578,16 @@ export class DataConnectApiClient { message: '`data` must be a non-empty array for insertMany.', }); } + if (data.length > 10000) { + throw new FirebaseDataConnectError({ + code: DATA_CONNECT_ERROR_CODE_MAPPING.INVALID_ARGUMENT, + message: '`data` array exceeds the maximum limit of 10,000 items.' + }); + } try { const { capitalized, camelCase } = this.getTableNames(tableName); - const keys = this.getArrayObjectsKeys(data); + const keys = this.getFieldsString(data); const mutation = `mutation($data: [${capitalized}_Data!]! @allow(fields: "${keys}")) { ${camelCase}_insertMany(data: $data) @@ -606,7 +633,7 @@ export class DataConnectApiClient { try { const { capitalized, camelCase } = this.getTableNames(tableName); - const keys = this.getObjectKeys(data); + const keys = this.getFieldsString(data); const mutation = `mutation($data: ${capitalized}_Data! @allow(fields: "${keys}")) { ${camelCase}_upsert(data: $data) @@ -642,10 +669,16 @@ export class DataConnectApiClient { message: '`data` must be a non-empty array for upsertMany.' }); } + if (data.length > 10000) { + throw new FirebaseDataConnectError({ + code: DATA_CONNECT_ERROR_CODE_MAPPING.INVALID_ARGUMENT, + message: '`data` array exceeds the maximum limit of 10,000 items.' + }); + } try { const { capitalized, camelCase } = this.getTableNames(tableName); - const keys = this.getArrayObjectsKeys(data); + const keys = this.getFieldsString(data); const mutation = `mutation($data: [${capitalized}_Data!]! @allow(fields: "${keys}")) { ${camelCase}_upsertMany(data: $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 3d347bc9c9..ccab37035a 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 @@ -699,9 +699,10 @@ describe('DataConnectApiClient CRUD helpers', () => { // Helper function to normalize GraphQL strings const normalizeGraphQLString = (str: string): string => { return str - .replace(/\n/g, '') // Remove newlines - .replace(/\s+/g, ' ') // Replace multiple spaces with a single space - .trim(); // Remove leading/trailing whitespace from the whole string + .replace(/\s*\n\s*/g, ' ') // Replace newline and surrounding spaces with a single space + .replace(/\s+/g, ' ') // Collapse multiple spaces to a single space + .replace(/\s*([(){},:"'])\s*/g, '$1') // Remove all spaces surrounding structural characters + .trim(); // Remove leading/trailing whitespace from the whole string }; /** @@ -765,7 +766,7 @@ describe('DataConnectApiClient CRUD helpers', () => { 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\\' } }; const expectedMutation = - `mutation($data: TestTable_Data! @allow(fields: "id active scores info")) { + `mutation($data: TestTable_Data! @allow(fields: "active id info { nested } scores")) { ${formatedTableName}_insert(data: $data) }`; await apiClient.insert(tableName, complexData); @@ -774,7 +775,7 @@ describe('DataConnectApiClient CRUD helpers', () => { it('should call executeGraphql with the correct mutation for undefined and null values', async () => { const expectedMutation = - `mutation($data: TestTable_Data! @allow(fields: "genre title ratings director extras")) { + `mutation($data: TestTable_Data! @allow(fields: "director extras { a } genre ratings title")) { ${formatedTableName}_insert(data: $data) }`; await apiClient.insert(tableName, dataWithUndefined); @@ -819,6 +820,27 @@ describe('DataConnectApiClient CRUD helpers', () => { await apiClient.insert(tableName, data); expectNormalizedExecuteGraphqlCall(expectedMutation, expectedOptions); }); + + it('should recursively extract deep nested object/array fields in @allow directive', async () => { + const deepData = { + id: '123', + customerId: 'c1', + total: 100, + tags: ['a', 'b'], + products_on_order: [ + { id: 'p1', name: 'Product 1', price: 9.99, categories: [{ id: 'cat1', name: 'Category 1' }] } + ] + }; + const expectedMutation = ` + mutation( + $data: TestTable_Data! + @allow(fields: "customerId id products_on_order { categories { id name } id name price } tags total")) + { + testTable_insert(data: $data) + }`; + await apiClient.insert(tableName, deepData); + expectNormalizedExecuteGraphqlCall(expectedMutation, { variables: { data: deepData } }); + }); }); // --- INSERT MANY TESTS --- @@ -852,7 +874,7 @@ describe('DataConnectApiClient CRUD helpers', () => { { id: 'b', scores: [1, 2], info: { nested: 'n2/\\' } } ]; const expectedMutation = ` - mutation($data: [TestTable_Data!]! @allow(fields: "id active info scores")) { + mutation($data: [TestTable_Data!]! @allow(fields: "active id info { nested } scores")) { ${formatedTableName}_insertMany(data: $data) }`; await apiClient.insertMany(tableName, complexDataArray); @@ -865,7 +887,7 @@ describe('DataConnectApiClient CRUD helpers', () => { dataWithUndefined ]; const expectedMutation = ` - mutation($data: [TestTable_Data!]! @allow(fields: "genre title ratings director extras")) { + mutation($data: [TestTable_Data!]! @allow(fields: "director extras { a } genre ratings title")) { ${formatedTableName}_insertMany(data: $data) }`; await apiClient.insertMany(tableName, dataArray); @@ -892,6 +914,12 @@ describe('DataConnectApiClient CRUD helpers', () => { .to.be.rejectedWith(FirebaseDataConnectError, /`data` must be a non-empty array for insertMany./); }); + it('should throw FirebaseDataConnectError if the data array length exceeds 10,000', async () => { + const oversizedArray = new Array(10001).fill({ name: 'a' }); + await expect(apiClient.insertMany(tableName, oversizedArray)) + .to.be.rejectedWith(FirebaseDataConnectError, /`data` array exceeds the maximum limit of 10,000 items./); + }); + it('should amend the message for query errors', async () => { try { await apiClientQueryError.insertMany(tableName, [{ data: 1 }]); @@ -918,6 +946,68 @@ describe('DataConnectApiClient CRUD helpers', () => { await apiClient.insertMany(tableName, data); expectNormalizedExecuteGraphqlCall(expectedMutation, expectedOptions); }); + + // eslint-disable-next-line max-len + it('should coalesce different object shapes in a bulk array into a single union of fields in @allow directive', async () => { + const dataArray = [ + { + id: '1', + name: 'Item 1', + metadata: { + tags: ['new', 'sale'], + dimensions: { width: 10, height: 20 } + } + }, + { + id: '2', + price: 19.99, + metadata: { + dimensions: { depth: 5 }, + manufacturer: { name: 'M1', location: { country: 'US' } } + } + }, + { + id: '3', + name: 'Item 3', + metadata: { + tags: ['promo'], + manufacturer: { location: { city: 'SF' } } + } + } + ]; + + const expectedMutation = ` + mutation( + $data: [TestTable_Data!]! + @allow( + fields: " + id + metadata { + dimensions { + depth + height + width + } + manufacturer { + location { + city + country + } + name + } + tags + } + name + price + " + ) + ) { + ${formatedTableName}_insertMany(data: $data) + }`; + + await apiClient.insertMany(tableName, dataArray); + expectNormalizedExecuteGraphqlCall(expectedMutation, { variables: { data: dataArray } }); + }); }); // --- UPSERT TESTS --- @@ -948,7 +1038,7 @@ 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 expectedMutation = ` - mutation($data: TestTable_Data! @allow(fields: "id active items detail")) { + mutation($data: TestTable_Data! @allow(fields: "active detail { status } id items")) { ${formatedTableName}_upsert(data: $data) }`; await apiClient.upsert(tableName, complexData); @@ -957,7 +1047,7 @@ describe('DataConnectApiClient CRUD helpers', () => { it('should call executeGraphql with the correct mutation for undefined and null values', async () => { const expectedMutation = ` - mutation($data: TestTable_Data! @allow(fields: "genre title ratings director extras")) { + mutation($data: TestTable_Data! @allow(fields: "director extras { a } genre ratings title")) { ${formatedTableName}_upsert(data: $data) }`; await apiClient.upsert(tableName, dataWithUndefined); @@ -1035,7 +1125,7 @@ describe('DataConnectApiClient CRUD helpers', () => { { id: 'y', scores: [null, 2] } ]; const expectedMutation = ` - mutation($data: [TestTable_Data!]! @allow(fields: "id active info scores")) { + mutation($data: [TestTable_Data!]! @allow(fields: "active id info { nested } scores")) { ${formatedTableName}_upsertMany(data: $data) }`; await apiClient.upsertMany(tableName, complexDataArray); @@ -1048,7 +1138,7 @@ describe('DataConnectApiClient CRUD helpers', () => { dataWithUndefined ]; const expectedMutation = ` - mutation($data: [TestTable_Data!]! @allow(fields: "genre title ratings director extras")) { + mutation($data: [TestTable_Data!]! @allow(fields: "director extras { a } genre ratings title")) { ${formatedTableName}_upsertMany(data: $data) }`; await apiClient.upsertMany(tableName, dataArray); @@ -1075,6 +1165,12 @@ describe('DataConnectApiClient CRUD helpers', () => { .to.be.rejectedWith(FirebaseDataConnectError, /`data` must be a non-empty array for upsertMany./); }); + it('should throw FirebaseDataConnectError if the data array length exceeds 10,000', async () => { + const oversizedArray = new Array(10001).fill({ name: 'a' }); + await expect(apiClient.upsertMany(tableName, oversizedArray)) + .to.be.rejectedWith(FirebaseDataConnectError, /`data` array exceeds the maximum limit of 10,000 items./); + }); + it('should amend the message for query errors', async () => { try { await apiClientQueryError.upsertMany(tableName, [{ data: 1 }]); From f6eb83a7e2edaed80f839c4781c03ca417856d50 Mon Sep 17 00:00:00 2001 From: stephenarosaj Date: Thu, 25 Jun 2026 15:01:12 -0700 Subject: [PATCH 12/16] de-duplicate code in mutaiton CRUD functions, and refactor tests --- .../data-connect-api-client-internal.ts | 113 ++----- .../data-connect-api-client-internal.spec.ts | 299 ++++-------------- 2 files changed, 96 insertions(+), 316 deletions(-) diff --git a/src/data-connect/data-connect-api-client-internal.ts b/src/data-connect/data-connect-api-client-internal.ts index b348b6856c..3a19b9839a 100644 --- a/src/data-connect/data-connect-api-client-internal.ts +++ b/src/data-connect/data-connect-api-client-internal.ts @@ -520,43 +520,7 @@ export class DataConnectApiClient { tableName: string, data: Variables, ): Promise> { - if (!validator.isNonEmptyString(tableName)) { - throw new FirebaseDataConnectError({ - code: DATA_CONNECT_ERROR_CODE_MAPPING.INVALID_ARGUMENT, - message: '`tableName` must be a non-empty string.' - }); - } - if (validator.isArray(data)) { - throw new FirebaseDataConnectError({ - code: DATA_CONNECT_ERROR_CODE_MAPPING.INVALID_ARGUMENT, - message: '`data` must be an object, not an array, for single insert. For arrays, please use ' - + '`insertMany` function.' - }); - } - if (!validator.isNonNullObject(data)) { - throw new FirebaseDataConnectError({ - code: DATA_CONNECT_ERROR_CODE_MAPPING.INVALID_ARGUMENT, - message: '`data` must be a non-null object.' - }); - } - - try { - const { capitalized, camelCase } = this.getTableNames(tableName); - const keys = this.getFieldsString(data); - const mutation = - `mutation($data: ${capitalized}_Data! @allow(fields: "${keys}")) { - ${camelCase}_insert(data: $data) - }`; - - return this.executeGraphql(mutation, { variables: { data } }) - .catch(this.handleBulkImportErrors); - } catch (e: any) { - throw new FirebaseDataConnectError({ - code: DATA_CONNECT_ERROR_CODE_MAPPING.INTERNAL, - message: `Failed to construct insert mutation: ${e.message}`, - cause: e, - }); - } + return this.executeSingleMutation(tableName, data, 'insert'); } /** @@ -566,42 +530,7 @@ export class DataConnectApiClient { tableName: string, data: Variables, ): Promise> { - if (!validator.isNonEmptyString(tableName)) { - throw new FirebaseDataConnectError({ - code: DATA_CONNECT_ERROR_CODE_MAPPING.INVALID_ARGUMENT, - message: '`tableName` must be a non-empty string.' - }); - } - if (!validator.isNonEmptyArray(data)) { - throw new FirebaseDataConnectError({ - code: DATA_CONNECT_ERROR_CODE_MAPPING.INVALID_ARGUMENT, - message: '`data` must be a non-empty array for insertMany.', - }); - } - if (data.length > 10000) { - throw new FirebaseDataConnectError({ - code: DATA_CONNECT_ERROR_CODE_MAPPING.INVALID_ARGUMENT, - message: '`data` array exceeds the maximum limit of 10,000 items.' - }); - } - - try { - const { capitalized, camelCase } = this.getTableNames(tableName); - const keys = this.getFieldsString(data); - const mutation = - `mutation($data: [${capitalized}_Data!]! @allow(fields: "${keys}")) { - ${camelCase}_insertMany(data: $data) - }`; - - return this.executeGraphql(mutation, { variables: { data } }) - .catch(this.handleBulkImportErrors); - } catch (e: any) { - throw new FirebaseDataConnectError({ - code: DATA_CONNECT_ERROR_CODE_MAPPING.INTERNAL, - message: `Failed to construct insertMany mutation: ${e.message}`, - cause: e, - }); - } + return this.executeBulkMutation(tableName, data, 'insertMany'); } /** @@ -610,6 +539,24 @@ export class DataConnectApiClient { public async upsert( tableName: string, data: Variables, + ): Promise> { + return this.executeSingleMutation(tableName, data, 'upsert'); + } + + /** + * Insert multiple rows into the specified table, or update them if they already exist. + */ + public async upsertMany>( + tableName: string, + data: Variables, + ): Promise> { + return this.executeBulkMutation(tableName, data, 'upsertMany'); + } + + private async executeSingleMutation( + tableName: string, + data: Variables, + operationType: 'insert' | 'upsert' ): Promise> { if (!validator.isNonEmptyString(tableName)) { throw new FirebaseDataConnectError({ @@ -620,8 +567,8 @@ export class DataConnectApiClient { if (validator.isArray(data)) { throw new FirebaseDataConnectError({ code: DATA_CONNECT_ERROR_CODE_MAPPING.INVALID_ARGUMENT, - message: '`data` must be an object, not an array, for single upsert. For arrays, please use ' - + '`upsertMany` function.' + message: `\`data\` must be an object, not an array, for single ${operationType}.\ + For arrays, please use \`${operationType}Many\` function.` }); } if (!validator.isNonNullObject(data)) { @@ -636,7 +583,7 @@ export class DataConnectApiClient { const keys = this.getFieldsString(data); const mutation = `mutation($data: ${capitalized}_Data! @allow(fields: "${keys}")) { - ${camelCase}_upsert(data: $data) + ${camelCase}_${operationType}(data: $data) }`; return this.executeGraphql(mutation, { variables: { data } }) @@ -644,18 +591,16 @@ export class DataConnectApiClient { } catch (e: any) { throw new FirebaseDataConnectError({ code: DATA_CONNECT_ERROR_CODE_MAPPING.INTERNAL, - message: `Failed to construct upsert mutation: ${e.message}`, + message: `Failed to construct ${operationType} mutation: ${e.message}`, cause: e, }); } } - /** - * Insert multiple rows into the specified table, or update them if they already exist. - */ - public async upsertMany>( + private async executeBulkMutation>( tableName: string, data: Variables, + operationType: 'insertMany' | 'upsertMany' ): Promise> { if (!validator.isNonEmptyString(tableName)) { throw new FirebaseDataConnectError({ @@ -666,7 +611,7 @@ export class DataConnectApiClient { if (!validator.isNonEmptyArray(data)) { throw new FirebaseDataConnectError({ code: DATA_CONNECT_ERROR_CODE_MAPPING.INVALID_ARGUMENT, - message: '`data` must be a non-empty array for upsertMany.' + message: `\`data\` must be a non-empty array for ${operationType}.` }); } if (data.length > 10000) { @@ -681,7 +626,7 @@ export class DataConnectApiClient { const keys = this.getFieldsString(data); const mutation = `mutation($data: [${capitalized}_Data!]! @allow(fields: "${keys}")) { - ${camelCase}_upsertMany(data: $data) + ${camelCase}_${operationType}(data: $data) }`; return this.executeGraphql(mutation, { variables: { data } }) @@ -689,7 +634,7 @@ export class DataConnectApiClient { } catch (e: any) { throw new FirebaseDataConnectError({ code: DATA_CONNECT_ERROR_CODE_MAPPING.INTERNAL, - message: `Failed to construct upsertMany mutation: ${e.message}`, + message: `Failed to construct ${operationType} mutation: ${e.message}`, cause: e, }); } 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 ccab37035a..0d5c3a97fd 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 @@ -738,6 +738,72 @@ describe('DataConnectApiClient CRUD helpers', () => { return mockApp.delete(); }); + // --- GET FIELDS STRING TESTS --- + describe('getFieldsString()', () => { + it('should extract keys from a simple object sorted alphabetically', () => { + const data = { name: 'test', value: 123 }; + const fields = apiClient['getFieldsString'](data); + expect(fields).to.equal('name value'); + }); + + it('should recursively extract deep nested object fields sorted alphabetically', () => { + const data = { id: 'abc', active: true, scores: [10, 20], info: { nested: 'yes/no "quote" \\slash\\' } }; + const fields = apiClient['getFieldsString'](data); + expect(fields).to.equal('active id info { nested } scores'); + }); + + it('should recursively extract deep nested object/array fields in @allow directive format', () => { + const deepData = { + id: '123', + customerId: 'c1', + total: 100, + tags: ['a', 'b'], + products_on_order: [ + { id: 'p1', name: 'Product 1', price: 9.99, categories: [{ id: 'cat1', name: 'Category 1' }] } + ] + }; + const fields = apiClient['getFieldsString'](deepData); + expect(fields).to.equal('customerId id products_on_order { categories { id name } id name price } tags total'); + }); + + it('should skip undefined fields and handle nulls/empty objects', () => { + const fields = apiClient['getFieldsString'](dataWithUndefined); + expect(fields).to.equal('director extras { a } genre ratings title'); + }); + + it('should coalesce different object shapes in a bulk array into a single union of fields', () => { + const dataArray = [ + { + id: '1', + name: 'Item 1', + metadata: { + tags: ['new', 'sale'], + dimensions: { width: 10, height: 20 } + } + }, + { + id: '2', + price: 19.99, + metadata: { + dimensions: { depth: 5 }, + manufacturer: { name: 'M1', location: { country: 'US' } } + } + }, + { + id: '3', + name: 'Item 3', + metadata: { + tags: ['promo'], + manufacturer: { location: { city: 'SF' } } + } + } + ]; + const fields = apiClient['getFieldsString'](dataArray); + // eslint-disable-next-line max-len + expect(fields).to.equal('id metadata { dimensions { depth height width } manufacturer { location { city country } name } tags } name price'); + }); + }); + // --- INSERT TESTS --- describe('insert()', () => { tableNames.forEach((tableName, index) => { @@ -763,25 +829,6 @@ describe('DataConnectApiClient CRUD helpers', () => { expectNormalizedExecuteGraphqlCall(expectedMutation, { variables: { data: simpleData } }); }); - 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\\' } }; - const expectedMutation = - `mutation($data: TestTable_Data! @allow(fields: "active id info { nested } scores")) { - ${formatedTableName}_insert(data: $data) - }`; - await apiClient.insert(tableName, complexData); - expectNormalizedExecuteGraphqlCall(expectedMutation, { variables: { data: complexData } }); - }); - - it('should call executeGraphql with the correct mutation for undefined and null values', async () => { - const expectedMutation = - `mutation($data: TestTable_Data! @allow(fields: "director extras { a } genre ratings title")) { - ${formatedTableName}_insert(data: $data) - }`; - await apiClient.insert(tableName, dataWithUndefined); - expectNormalizedExecuteGraphqlCall(expectedMutation, { variables: { data: dataWithUndefined } }); - }); - 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./); @@ -796,7 +843,7 @@ 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 () => { try { await apiClientQueryError.insert(tableName, { data: 1 }); @@ -807,40 +854,6 @@ describe('DataConnectApiClient CRUD helpers', () => { expect(err.cause).to.equal(expectedQueryError); } }); - - it('should call executeGraphql with variables and @allow directive for enums and other fields', async () => { - const data = { id: 'key1', name: 'Fred', status: 'ACTIVE' }; - const expectedMutation = ` - mutation($data: TestTable_Data! @allow(fields: "id name status")) { - testTable_insert(data: $data) - }`; - const expectedOptions = { - variables: { data } - }; - await apiClient.insert(tableName, data); - expectNormalizedExecuteGraphqlCall(expectedMutation, expectedOptions); - }); - - it('should recursively extract deep nested object/array fields in @allow directive', async () => { - const deepData = { - id: '123', - customerId: 'c1', - total: 100, - tags: ['a', 'b'], - products_on_order: [ - { id: 'p1', name: 'Product 1', price: 9.99, categories: [{ id: 'cat1', name: 'Category 1' }] } - ] - }; - const expectedMutation = ` - mutation( - $data: TestTable_Data! - @allow(fields: "customerId id products_on_order { categories { id name } id name price } tags total")) - { - testTable_insert(data: $data) - }`; - await apiClient.insert(tableName, deepData); - expectNormalizedExecuteGraphqlCall(expectedMutation, { variables: { data: deepData } }); - }); }); // --- INSERT MANY TESTS --- @@ -868,32 +881,6 @@ describe('DataConnectApiClient CRUD helpers', () => { expectNormalizedExecuteGraphqlCall(expectedMutation, { variables: { data: simpleDataArray } }); }); - it('should call executeGraphql with the correct mutation for complex data array', async () => { - const complexDataArray = [ - { id: 'a', active: true, info: { nested: 'n1 "quote"' } }, - { id: 'b', scores: [1, 2], info: { nested: 'n2/\\' } } - ]; - const expectedMutation = ` - mutation($data: [TestTable_Data!]! @allow(fields: "active id info { nested } scores")) { - ${formatedTableName}_insertMany(data: $data) - }`; - await apiClient.insertMany(tableName, complexDataArray); - expectNormalizedExecuteGraphqlCall(expectedMutation, { variables: { data: complexDataArray } }); - }); - - it('should call executeGraphql with the correct mutation for undefined and null', async () => { - const dataArray = [ - dataWithUndefined, - dataWithUndefined - ]; - const expectedMutation = ` - mutation($data: [TestTable_Data!]! @allow(fields: "director extras { a } genre ratings title")) { - ${formatedTableName}_insertMany(data: $data) - }`; - await apiClient.insertMany(tableName, dataArray); - expectNormalizedExecuteGraphqlCall(expectedMutation, { variables: { data: dataArray } }); - }); - 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./); @@ -930,84 +917,6 @@ describe('DataConnectApiClient CRUD helpers', () => { expect(err.cause).to.equal(expectedQueryError); } }); - - it('should call executeGraphql with variables and @allow directive for enums and other fields', async () => { - const data = [ - { id: 'key1', name: 'Fred', status: 'ACTIVE' }, - { id: 'key2', name: 'Bob', tags: ['cool'] } - ]; - const expectedMutation = ` - mutation($data: [TestTable_Data!]! @allow(fields: "id name status tags")) { - testTable_insertMany(data: $data) - }`; - const expectedOptions = { - variables: { data } - }; - await apiClient.insertMany(tableName, data); - expectNormalizedExecuteGraphqlCall(expectedMutation, expectedOptions); - }); - - // eslint-disable-next-line max-len - it('should coalesce different object shapes in a bulk array into a single union of fields in @allow directive', async () => { - const dataArray = [ - { - id: '1', - name: 'Item 1', - metadata: { - tags: ['new', 'sale'], - dimensions: { width: 10, height: 20 } - } - }, - { - id: '2', - price: 19.99, - metadata: { - dimensions: { depth: 5 }, - manufacturer: { name: 'M1', location: { country: 'US' } } - } - }, - { - id: '3', - name: 'Item 3', - metadata: { - tags: ['promo'], - manufacturer: { location: { city: 'SF' } } - } - } - ]; - - const expectedMutation = ` - mutation( - $data: [TestTable_Data!]! - @allow( - fields: " - id - metadata { - dimensions { - depth - height - width - } - manufacturer { - location { - city - country - } - name - } - tags - } - name - price - " - ) - ) { - ${formatedTableName}_insertMany(data: $data) - }`; - - await apiClient.insertMany(tableName, dataArray); - expectNormalizedExecuteGraphqlCall(expectedMutation, { variables: { data: dataArray } }); - }); }); // --- UPSERT TESTS --- @@ -1035,25 +944,6 @@ describe('DataConnectApiClient CRUD helpers', () => { expectNormalizedExecuteGraphqlCall(expectedMutation, { variables: { data: simpleData } }); }); - 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 expectedMutation = ` - mutation($data: TestTable_Data! @allow(fields: "active detail { status } id items")) { - ${formatedTableName}_upsert(data: $data) - }`; - await apiClient.upsert(tableName, complexData); - expectNormalizedExecuteGraphqlCall(expectedMutation, { variables: { data: complexData } }); - }); - - it('should call executeGraphql with the correct mutation for undefined and null values', async () => { - const expectedMutation = ` - mutation($data: TestTable_Data! @allow(fields: "director extras { a } genre ratings title")) { - ${formatedTableName}_upsert(data: $data) - }`; - await apiClient.upsert(tableName, dataWithUndefined); - expectNormalizedExecuteGraphqlCall(expectedMutation, { variables: { data: dataWithUndefined } }); - }); - 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./); @@ -1079,19 +969,6 @@ describe('DataConnectApiClient CRUD helpers', () => { expect(err.cause).to.equal(expectedQueryError); } }); - - it('should call executeGraphql with variables and @allow directive for enums and other fields', async () => { - const data = { id: 'key1', name: 'Fred', status: 'ACTIVE' }; - const expectedMutation = ` - mutation($data: TestTable_Data! @allow(fields: "id name status")) { - testTable_upsert(data: $data) - }`; - const expectedOptions = { - variables: { data } - }; - await apiClient.upsert(tableName, data); - expectNormalizedExecuteGraphqlCall(expectedMutation, expectedOptions); - }); }); // --- UPSERT MANY TESTS --- @@ -1119,32 +996,6 @@ describe('DataConnectApiClient CRUD helpers', () => { expectNormalizedExecuteGraphqlCall(expectedMutation, { variables: { data: simpleDataArray } }); }); - 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] } - ]; - const expectedMutation = ` - mutation($data: [TestTable_Data!]! @allow(fields: "active id info { nested } scores")) { - ${formatedTableName}_upsertMany(data: $data) - }`; - await apiClient.upsertMany(tableName, complexDataArray); - expectNormalizedExecuteGraphqlCall(expectedMutation, { variables: { data: complexDataArray } }); - }); - - it('should call executeGraphql with the correct mutation for undefined and null', async () => { - const dataArray = [ - dataWithUndefined, - dataWithUndefined - ]; - const expectedMutation = ` - mutation($data: [TestTable_Data!]! @allow(fields: "director extras { a } genre ratings title")) { - ${formatedTableName}_upsertMany(data: $data) - }`; - await apiClient.upsertMany(tableName, dataArray); - expectNormalizedExecuteGraphqlCall(expectedMutation, { variables: { data: dataArray } }); - }); - it('should throw FirebaseDataConnectError for invalid tableName', async () => { expect(apiClient.upsertMany('', [{ data: 1 }])) .to.be.rejectedWith(FirebaseDataConnectError, /`tableName` must be a non-empty string./); @@ -1181,22 +1032,6 @@ describe('DataConnectApiClient CRUD helpers', () => { expect(err.cause).to.equal(expectedQueryError); } }); - - it('should call executeGraphql with variables and @allow directive for enums and other fields', async () => { - const data = [ - { id: 'key1', name: 'Fred', status: 'ACTIVE' }, - { id: 'key2', name: 'Bob', tags: ['cool'] } - ]; - const expectedMutation = ` - mutation($data: [TestTable_Data!]! @allow(fields: "id name status tags")) { - testTable_upsertMany(data: $data) - }`; - const expectedOptions = { - variables: { data } - }; - await apiClient.upsertMany(tableName, data); - expectNormalizedExecuteGraphqlCall(expectedMutation, expectedOptions); - }); }); describe('String serialization', () => { From 1451bbac2b772dd6af2578d63654ef3cf61f5537 Mon Sep 17 00:00:00 2001 From: stephenarosaj Date: Thu, 25 Jun 2026 15:02:52 -0700 Subject: [PATCH 13/16] update contributing --- CONTRIBUTING.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 2b6e250d89..78fb603021 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -154,7 +154,8 @@ Currently, only the Auth, Database, and Firestore test suites work. Some test ca will be automatically skipped due to lack of emulator support. You can also run the Data Connect test suite against the emulators using the same command, -but with a config file specific to Data Connect emulator testing: +but you must run only the dataconnect tests, using a config file specific to Data Connect +emulator testing: ```bash firebase emulators:exec \ From 6e91dff202daaf5ecda69d970af7558a6d176770 Mon Sep 17 00:00:00 2001 From: stephenarosaj Date: Thu, 25 Jun 2026 17:21:40 -0700 Subject: [PATCH 14/16] add maxCount to array bulk insert variables --- .../data-connect-api-client-internal.ts | 8 ++-- .../data-connect-api-client-internal.spec.ts | 37 ++++++++++++------- 2 files changed, 29 insertions(+), 16 deletions(-) diff --git a/src/data-connect/data-connect-api-client-internal.ts b/src/data-connect/data-connect-api-client-internal.ts index 3a19b9839a..e5fca90d40 100644 --- a/src/data-connect/data-connect-api-client-internal.ts +++ b/src/data-connect/data-connect-api-client-internal.ts @@ -67,6 +67,8 @@ const EXECUTE_GRAPH_QL_READ_ENDPOINT = 'executeGraphqlRead'; const IMPERSONATE_QUERY_ENDPOINT = 'impersonateQuery'; const IMPERSONATE_MUTATION_ENDPOINT = 'impersonateMutation'; +/** @internal The maximum number of items allowed in the @allow directive's maxCount argument. */ +export const ALLOW_DIRECTIVE_MAX_COUNT = 10_000; function getHeaders(isUsingGen: boolean): { [key: string]: string } { const headerValue = { @@ -614,10 +616,10 @@ export class DataConnectApiClient { message: `\`data\` must be a non-empty array for ${operationType}.` }); } - if (data.length > 10000) { + if (data.length > ALLOW_DIRECTIVE_MAX_COUNT) { throw new FirebaseDataConnectError({ code: DATA_CONNECT_ERROR_CODE_MAPPING.INVALID_ARGUMENT, - message: '`data` array exceeds the maximum limit of 10,000 items.' + message: `\`data\` array exceeds the maximum limit of ${ALLOW_DIRECTIVE_MAX_COUNT} items.` }); } @@ -625,7 +627,7 @@ export class DataConnectApiClient { const { capitalized, camelCase } = this.getTableNames(tableName); const keys = this.getFieldsString(data); const mutation = - `mutation($data: [${capitalized}_Data!]! @allow(fields: "${keys}")) { + `mutation($data: [${capitalized}_Data!]! @allow(fields: "${keys}", maxCount: ${ALLOW_DIRECTIVE_MAX_COUNT})) { ${camelCase}_${operationType}(data: $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 0d5c3a97fd..62c272982a 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,10 @@ import { } from '../../../src/utils/api-request'; import * as utils from '../utils'; import * as mocks from '../../resources/mocks'; -import { DataConnectApiClient } from '../../../src/data-connect/data-connect-api-client-internal'; +import { + ALLOW_DIRECTIVE_MAX_COUNT, + DataConnectApiClient +} from '../../../src/data-connect/data-connect-api-client-internal'; import { FirebaseDataConnectError, DATA_CONNECT_ERROR_CODE_MAPPING, @@ -64,8 +67,8 @@ describe('DataConnectApiClient', () => { }; 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.'; + + 'account credentials or set project ID as an app option. Alternatively, set the ' + + 'GOOGLE_CLOUD_PROJECT environment variable.'; const TEST_RESPONSE = { data: { @@ -861,7 +864,7 @@ describe('DataConnectApiClient CRUD helpers', () => { tableNames.forEach((tableName, index) => { const capitalizedTable = capitalize(formatedTableNames[index]); const expectedMutation = ` - mutation($data: [${capitalizedTable}_Data!]! @allow(fields: "name")) { + mutation($data: [${capitalizedTable}_Data!]! @allow(fields: "name", maxCount: ${ALLOW_DIRECTIVE_MAX_COUNT})) { ${formatedTableNames[index]}_insertMany(data: $data) }`; it(`should use the formatted tableName in the gql query: "${tableName}" as "${formatedTableNames[index]}"`, @@ -874,7 +877,7 @@ describe('DataConnectApiClient CRUD helpers', () => { it('should call executeGraphql with the correct mutation for simple data array', async () => { const simpleDataArray = [{ name: 'test1' }, { name: 'test2', value: 456 }]; const expectedMutation = ` - mutation($data: [TestTable_Data!]! @allow(fields: "name value")) { + mutation($data: [TestTable_Data!]! @allow(fields: "name value", maxCount: ${ALLOW_DIRECTIVE_MAX_COUNT})) { ${formatedTableName}_insertMany(data: $data) }`; await apiClient.insertMany(tableName, simpleDataArray); @@ -901,10 +904,14 @@ describe('DataConnectApiClient CRUD helpers', () => { .to.be.rejectedWith(FirebaseDataConnectError, /`data` must be a non-empty array for insertMany./); }); - it('should throw FirebaseDataConnectError if the data array length exceeds 10,000', async () => { - const oversizedArray = new Array(10001).fill({ name: 'a' }); + // eslint-disable-next-line max-len + it(`should throw FirebaseDataConnectError if the data array length exceeds ${ALLOW_DIRECTIVE_MAX_COUNT}`, async () => { + const oversizedArray = new Array(ALLOW_DIRECTIVE_MAX_COUNT + 1).fill({ name: 'a' }); await expect(apiClient.insertMany(tableName, oversizedArray)) - .to.be.rejectedWith(FirebaseDataConnectError, /`data` array exceeds the maximum limit of 10,000 items./); + .to.be.rejectedWith( + FirebaseDataConnectError, + new RegExp(`^\`data\` array exceeds the maximum limit of ${ALLOW_DIRECTIVE_MAX_COUNT} items.$`) + ); }); it('should amend the message for query errors', async () => { @@ -976,7 +983,7 @@ describe('DataConnectApiClient CRUD helpers', () => { tableNames.forEach((tableName, index) => { const capitalizedTable = capitalize(formatedTableNames[index]); const expectedMutation = ` - mutation($data: [${capitalizedTable}_Data!]! @allow(fields: "name")) { + mutation($data: [${capitalizedTable}_Data!]! @allow(fields: "name", maxCount: ${ALLOW_DIRECTIVE_MAX_COUNT})) { ${formatedTableNames[index]}_upsertMany(data: $data) }`; it(`should use the formatted tableName in the gql query: "${tableName}" as "${formatedTableNames[index]}"`, @@ -989,7 +996,7 @@ 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($data: [TestTable_Data!]! @allow(fields: "id value")) { + mutation($data: [TestTable_Data!]! @allow(fields: "id value", maxCount: ${ALLOW_DIRECTIVE_MAX_COUNT})) { ${formatedTableName}_upsertMany(data: $data) }`; await apiClient.upsertMany(tableName, simpleDataArray); @@ -1016,10 +1023,14 @@ describe('DataConnectApiClient CRUD helpers', () => { .to.be.rejectedWith(FirebaseDataConnectError, /`data` must be a non-empty array for upsertMany./); }); - it('should throw FirebaseDataConnectError if the data array length exceeds 10,000', async () => { - const oversizedArray = new Array(10001).fill({ name: 'a' }); + // eslint-disable-next-line max-len + it(`should throw FirebaseDataConnectError if the data array length exceeds ${ALLOW_DIRECTIVE_MAX_COUNT}`, async () => { + const oversizedArray = new Array(ALLOW_DIRECTIVE_MAX_COUNT + 1).fill({ name: 'a' }); await expect(apiClient.upsertMany(tableName, oversizedArray)) - .to.be.rejectedWith(FirebaseDataConnectError, /`data` array exceeds the maximum limit of 10,000 items./); + .to.be.rejectedWith( + FirebaseDataConnectError, + new RegExp(`^\`data\` array exceeds the maximum limit of ${ALLOW_DIRECTIVE_MAX_COUNT} items.$`) + ); }); it('should amend the message for query errors', async () => { From b350dfd8988b242784cf69658ff726dda146db4d Mon Sep 17 00:00:00 2001 From: stephenarosaj Date: Fri, 26 Jun 2026 11:11:49 -0700 Subject: [PATCH 15/16] limit @allow nesting to _on_ relational fields --- src/data-connect/data-connect-api-client-internal.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/data-connect/data-connect-api-client-internal.ts b/src/data-connect/data-connect-api-client-internal.ts index e5fca90d40..5bb26b00b2 100644 --- a/src/data-connect/data-connect-api-client-internal.ts +++ b/src/data-connect/data-connect-api-client-internal.ts @@ -483,7 +483,9 @@ export class DataConnectApiClient { childNode = { children: new Map() }; node.children.set(key, childNode); } - this.mergeFieldsIntoTree(val, childNode); + if (key.includes('_on_')) { + this.mergeFieldsIntoTree(val, childNode); + } } } } From 4f87c1287b896960d85892f0eafec46c58a8a558 Mon Sep 17 00:00:00 2001 From: stephenarosaj Date: Fri, 26 Jun 2026 12:00:22 -0700 Subject: [PATCH 16/16] update tests --- .../data-connect-api-client-internal.ts | 103 +++++----- .../data-connect-api-client-internal.spec.ts | 193 ++++++++++-------- 2 files changed, 165 insertions(+), 131 deletions(-) diff --git a/src/data-connect/data-connect-api-client-internal.ts b/src/data-connect/data-connect-api-client-internal.ts index 5bb26b00b2..3157cd423c 100644 --- a/src/data-connect/data-connect-api-client-internal.ts +++ b/src/data-connect/data-connect-api-client-internal.ts @@ -456,54 +456,7 @@ export class DataConnectApiClient { return { capitalized, camelCase }; } - /** - * Extracts property keys from an object or array of objects as a space-separated string, - * including recursively nested object/array fields for the `@allow(fields: ...)` directive. - * Leverages a hierarchical tree to deduplicate and merge fields. - */ - private getFieldsString(data: unknown): string { - const root: FieldNode = { children: new Map() }; - this.mergeFieldsIntoTree(data, root); - return this.serializeFieldNode(root); - } - private mergeFieldsIntoTree(data: unknown, node: FieldNode): void { - if (validator.isArray(data)) { - for (const item of data) { - this.mergeFieldsIntoTree(item, node); - } - } else if (validator.isNonNullObject(data) && !(data instanceof Date)) { - const record = data as Record; - for (const [key, val] of Object.entries(record)) { - if (val === undefined) { - continue; - } - let childNode = node.children.get(key); - if (!childNode) { - childNode = { children: new Map() }; - node.children.set(key, childNode); - } - if (key.includes('_on_')) { - this.mergeFieldsIntoTree(val, childNode); - } - } - } - } - - private serializeFieldNode(node: FieldNode): string { - const parts: string[] = []; - const sortedKeys = Array.from(node.children.keys()).sort((a, b) => a.localeCompare(b)); - for (const key of sortedKeys) { - const childNode = node.children.get(key)!; - if (childNode.children.size > 0) { - const nestedString = this.serializeFieldNode(childNode); - parts.push(`${key} { ${nestedString} }`); - } else { - parts.push(key); - } - } - return parts.join(' '); - } private handleBulkImportErrors(err: FirebaseDataConnectError): never { if (err.code === `data-connect/${DATA_CONNECT_ERROR_CODE_MAPPING.QUERY_ERROR}`) { @@ -584,7 +537,7 @@ export class DataConnectApiClient { try { const { capitalized, camelCase } = this.getTableNames(tableName); - const keys = this.getFieldsString(data); + const keys = getFieldsString(data); const mutation = `mutation($data: ${capitalized}_Data! @allow(fields: "${keys}")) { ${camelCase}_${operationType}(data: $data) @@ -627,7 +580,7 @@ export class DataConnectApiClient { try { const { capitalized, camelCase } = this.getTableNames(tableName); - const keys = this.getFieldsString(data); + const keys = getFieldsString(data); const mutation = `mutation($data: [${capitalized}_Data!]! @allow(fields: "${keys}", maxCount: ${ALLOW_DIRECTIVE_MAX_COUNT})) { ${camelCase}_${operationType}(data: $data) @@ -678,3 +631,55 @@ interface ServerError { message?: string; status?: string; } + +/** + * Extracts property keys from an object or array of objects as a space-separated string, + * including recursively nested object/array fields for the `@allow(fields: ...)` directive. + * Leverages a hierarchical tree to deduplicate and merge fields. + * + * @internal + */ +export function getFieldsString(data: unknown): string { + const root: FieldNode = { children: new Map() }; + mergeFieldsIntoTree(data, root); + return serializeFieldNode(root); +} + +function mergeFieldsIntoTree(data: unknown, node: FieldNode): void { + if (validator.isArray(data)) { + data.forEach((item) => mergeFieldsIntoTree(item, node)); + return; + } + if (!validator.isNonNullObject(data) || data instanceof Date) { + return; + } + const record = data as Record; + for (const [key, val] of Object.entries(record)) { + if (val === undefined) { + continue; + } + let childNode = node.children.get(key); + if (!childNode) { + childNode = { children: new Map() }; + node.children.set(key, childNode); + } + if (key.includes('_on_')) { + mergeFieldsIntoTree(val, childNode); + } + } +} + +function serializeFieldNode(node: FieldNode): string { + const parts: string[] = []; + const sortedKeys = Array.from(node.children.keys()).sort((a, b) => a.localeCompare(b)); + for (const key of sortedKeys) { + const childNode = node.children.get(key)!; + if (childNode.children.size > 0) { + const nestedString = serializeFieldNode(childNode); + parts.push(`${key} { ${nestedString} }`); + } else { + parts.push(key); + } + } + return parts.join(' '); +} 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 62c272982a..d8c1d242f8 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,9 +24,10 @@ import { } from '../../../src/utils/api-request'; import * as utils from '../utils'; import * as mocks from '../../resources/mocks'; -import { - ALLOW_DIRECTIVE_MAX_COUNT, - DataConnectApiClient +import { + ALLOW_DIRECTIVE_MAX_COUNT, + DataConnectApiClient, + getFieldsString } from '../../../src/data-connect/data-connect-api-client-internal'; import { FirebaseDataConnectError, @@ -294,7 +295,7 @@ describe('DataConnectApiClient', () => { describe('should reject with an appropriate error response on failure', () => { it('should reject when no operationName is provided', () => { - apiClient.executeQuery( '', undefined, unauthenticatedOptions) + apiClient.executeQuery('', undefined, unauthenticatedOptions) .should.eventually.be.rejectedWith('`name` must be a non-empty string.'); apiClient.executeQuery(undefined as unknown as string, undefined, unauthenticatedOptions) .should.eventually.be.rejectedWith('`name` must be a non-empty string.'); @@ -302,8 +303,8 @@ describe('DataConnectApiClient', () => { it('should reject when project id is not available', () => { clientWithoutProjectId.executeQuery( - 'unauthenticated query', - undefined, + 'unauthenticated query', + undefined, unauthenticatedOptions ).should.eventually.be.rejectedWith(noProjectId); }); @@ -395,8 +396,8 @@ describe('DataConnectApiClient', () => { .stub(HttpClient.prototype, 'send') .resolves(utils.responseFrom(TEST_RESPONSE, 200)); const resp = await apiClient.executeQuery( - 'unauthenticated query', - undefined, + 'unauthenticated query', + undefined, unauthenticatedOptions ); expect(resp.data.users).to.be.not.empty; @@ -407,8 +408,8 @@ describe('DataConnectApiClient', () => { method: 'POST', url: `https://firebasedataconnect.googleapis.com/v1/projects/test-project/locations/${connectorConfig.location}/services/${connectorConfig.serviceId}/connectors/${connectorConfig.connector}:impersonateQuery`, headers: EXPECTED_HEADERS, - data: { - operationName: 'unauthenticated query', + data: { + operationName: 'unauthenticated query', extensions: unauthenticatedOptions } }); @@ -419,8 +420,8 @@ describe('DataConnectApiClient', () => { .stub(HttpClient.prototype, 'send') .resolves(utils.responseFrom(TEST_RESPONSE, 200)); const resp = await apiClient.executeQuery( - 'authenticated query', - undefined, + 'authenticated query', + undefined, authenticatedOptions ); expect(resp.data.users).to.be.not.empty; @@ -431,8 +432,8 @@ describe('DataConnectApiClient', () => { method: 'POST', url: `https://firebasedataconnect.googleapis.com/v1/projects/test-project/locations/${connectorConfig.location}/services/${connectorConfig.serviceId}/connectors/${connectorConfig.connector}:impersonateQuery`, headers: EXPECTED_HEADERS, - data: { - operationName: 'authenticated query', + data: { + operationName: 'authenticated query', extensions: { impersonate: authenticatedOptions.impersonate } } }); @@ -445,25 +446,25 @@ describe('DataConnectApiClient', () => { .stub(HttpClient.prototype, 'send') .resolves(utils.responseFrom(TEST_RESPONSE, 200)); await apiClient.executeQuery( - 'unauthenticated query', - undefined, + 'unauthenticated query', + undefined, unauthenticatedOptions ); expect(stub).to.have.been.calledOnce.and.calledWith({ method: 'POST', url: `http://localhost:9399/v1/projects/test-project/locations/${connectorConfig.location}/services/${connectorConfig.serviceId}/connectors/${connectorConfig.connector}:impersonateQuery`, headers: EMULATOR_EXPECTED_HEADERS, - data: { - operationName: 'unauthenticated query', + data: { + operationName: 'unauthenticated query', extensions: unauthenticatedOptions } }); }); }); - const unauthenticatedOptions: OperationOptions = + const unauthenticatedOptions: OperationOptions = { impersonate: { unauthenticated: true } }; - const authenticatedOptions: OperationOptions = + const authenticatedOptions: OperationOptions = { impersonate: { authClaims: { sub: 'authenticated-UUID' } } }; describe('executeMutation', () => { @@ -576,8 +577,8 @@ describe('DataConnectApiClient', () => { method: 'POST', url: `https://firebasedataconnect.googleapis.com/v1/projects/test-project/locations/${connectorConfig.location}/services/${connectorConfig.serviceId}/connectors/${connectorConfig.connector}:impersonateMutation`, headers: EXPECTED_HEADERS, - data: { - operationName: 'unauthenticated mutation', + data: { + operationName: 'unauthenticated mutation', extensions: unauthenticatedOptions } }); @@ -598,8 +599,8 @@ describe('DataConnectApiClient', () => { method: 'POST', url: `https://firebasedataconnect.googleapis.com/v1/projects/test-project/locations/${connectorConfig.location}/services/${connectorConfig.serviceId}/connectors/${connectorConfig.connector}:impersonateMutation`, headers: EXPECTED_HEADERS, - data: { - operationName: 'authenticated mutation', + data: { + operationName: 'authenticated mutation', extensions: authenticatedOptions } }); @@ -616,8 +617,8 @@ describe('DataConnectApiClient', () => { method: 'POST', url: `http://localhost:9399/v1/projects/test-project/locations/${connectorConfig.location}/services/${connectorConfig.serviceId}/connectors/${connectorConfig.connector}:impersonateMutation`, headers: EMULATOR_EXPECTED_HEADERS, - data: { - operationName: 'unauthenticated mutation', + data: { + operationName: 'unauthenticated mutation', extensions: unauthenticatedOptions } }); @@ -743,67 +744,95 @@ describe('DataConnectApiClient CRUD helpers', () => { // --- GET FIELDS STRING TESTS --- describe('getFieldsString()', () => { - it('should extract keys from a simple object sorted alphabetically', () => { - const data = { name: 'test', value: 123 }; - const fields = apiClient['getFieldsString'](data); - expect(fields).to.equal('name value'); - }); + describe('single object', () => { + it('should extract keys from a simple object sorted alphabetically', () => { + const data = { name: 'test', value: 123 }; + const fields = getFieldsString(data); + expect(fields).to.equal('name value'); + }); - it('should recursively extract deep nested object fields sorted alphabetically', () => { - const data = { id: 'abc', active: true, scores: [10, 20], info: { nested: 'yes/no "quote" \\slash\\' } }; - const fields = apiClient['getFieldsString'](data); - expect(fields).to.equal('active id info { nested } scores'); - }); + it('should recursively extract nested object fields for objects with _on_ field names', () => { + const data = { + id: 'abc', + active: true, + scores: [10, 20], + info_on_test: { nested: 'yes/no "quote" \\slash\\' } + }; + const fields = getFieldsString(data); + expect(fields).to.equal('active id info_on_test { nested } scores'); + }); - it('should recursively extract deep nested object/array fields in @allow directive format', () => { - const deepData = { - id: '123', - customerId: 'c1', - total: 100, - tags: ['a', 'b'], - products_on_order: [ - { id: 'p1', name: 'Product 1', price: 9.99, categories: [{ id: 'cat1', name: 'Category 1' }] } - ] - }; - const fields = apiClient['getFieldsString'](deepData); - expect(fields).to.equal('customerId id products_on_order { categories { id name } id name price } tags total'); - }); + it('should recursively extract deep nested object/array fields for objects with _on_ field names', () => { + const deepData = { + id: '1', + tags_on_item: { name: 'Tag1', count: 1, colors_on_tag: { primary: 'red', secondary: 'red' } }, + }; + const fields = getFieldsString(deepData); + expect(fields).to.equal('id tags_on_item { colors_on_tag { primary secondary } count name }'); + }); - it('should skip undefined fields and handle nulls/empty objects', () => { - const fields = apiClient['getFieldsString'](dataWithUndefined); - expect(fields).to.equal('director extras { a } genre ratings title'); + it('should skip undefined fields and handle nulls/empty objects', () => { + const fields = getFieldsString(dataWithUndefined); + expect(fields).to.equal('director extras genre ratings title'); + }); }); + + describe('array of objects', () => { + it('should extract and coalesce keys from simple objects sorted alphabetically', () => { + const dataArray = [ + { name: 'test' }, + { value: 123 }, + { name: 'another', other: true } + ]; + const fields = getFieldsString(dataArray); + expect(fields).to.equal('name other value'); + }); - it('should coalesce different object shapes in a bulk array into a single union of fields', () => { - const dataArray = [ - { - id: '1', - name: 'Item 1', - metadata: { - tags: ['new', 'sale'], - dimensions: { width: 10, height: 20 } + it('should extract and coalesce different object shapes in a bulk array into a single union of fields', () => { + const dataArray = [ + { + id: '1', + name: 'Item 1', + }, + { + id: '2', + price: 19.99, + }, + { + id: '3', + name: 'Item 3', } - }, - { - id: '2', - price: 19.99, - metadata: { - dimensions: { depth: 5 }, - manufacturer: { name: 'M1', location: { country: 'US' } } - } - }, - { - id: '3', - name: 'Item 3', - metadata: { - tags: ['promo'], - manufacturer: { location: { city: 'SF' } } - } - } - ]; - const fields = apiClient['getFieldsString'](dataArray); - // eslint-disable-next-line max-len - expect(fields).to.equal('id metadata { dimensions { depth height width } manufacturer { location { city country } name } tags } name price'); + ]; + const fields = getFieldsString(dataArray); + expect(fields).to.equal('id name price'); + }); + + it('should recursively extract and coalesce nested object fields for objects with _on_ field names', () => { + const dataArray = [ + { id: 'abc', active: true, info_on_test: { nested: 'yes' } }, + { scores: [10, 20], info_on_test: { other: 123 } } + ]; + const fields = getFieldsString(dataArray); + expect(fields).to.equal('active id info_on_test { nested other } scores'); + }); + + it('should recursively coalesce deep nested fields for objects with _on_ names', () => { + const dataArray = [ + { id: '1', tags_on_item: { name: 'Tag1', colors_on_tag: { primary: 'red' } } }, + { tags_on_item: { count: 1, colors_on_tag: { secondary: 'blue' } } } + ]; + const fields = getFieldsString(dataArray); + expect(fields).to.equal('id tags_on_item { colors_on_tag { primary secondary } count name }'); + }); + + it('should skip undefined fields and handle nulls/empty objects across multiple objects in array', () => { + const dataArray = [ + dataWithUndefined, + { notes: 'actual note', releaseYear: 2024, genre: undefined } + ]; + const fields = getFieldsString(dataArray); + expect(fields).to.equal('director extras genre notes ratings releaseYear title'); + }); }); }); @@ -842,7 +871,7 @@ describe('DataConnectApiClient CRUD helpers', () => { .to.be.rejectedWith(FirebaseDataConnectError, /`data` must be a non-null object./); }); - it('should throw FirebaseDataConnectError for array data', async() => { + it('should throw FirebaseDataConnectError for array data', async () => { await expect(apiClient.insert(tableName, [])) .to.be.rejectedWith(FirebaseDataConnectError, /`data` must be an object, not an array, for single insert./); });