Skip to content

Commit 863e3b2

Browse files
stephenarosajstephenarosaj
committed
add 10k limit and nested coalesced field keys for @Allow 
1 parent 746a37c commit 863e3b2

2 files changed

Lines changed: 166 additions & 37 deletions

File tree

‎src/data-connect/data-connect-api-client-internal.ts‎

Lines changed: 59 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,10 @@ interface ConnectorsUrlParams extends ServicesUrlParams {
100100
connectorId: string;
101101
}
102102

103+
interface FieldNode {
104+
children: Map<string, FieldNode>;
105+
}
106+
103107
/**
104108
* Class that facilitates sending requests to the Firebase Data Connect backend API.
105109
*
@@ -451,33 +455,50 @@ export class DataConnectApiClient {
451455
}
452456

453457
/**
454-
* Extracts all defined property keys from an object as a space-separated string.
455-
* Used to build the `@allow(fields: ...)` mutation directive for single operations.
458+
* Extracts property keys from an object or array of objects as a space-separated string,
459+
* including recursively nested object/array fields for the `@allow(fields: ...)` directive.
460+
* Leverages a hierarchical tree to deduplicate and merge fields.
456461
*/
457-
private getObjectKeys(data: Record<string, unknown> | object): string {
458-
return Object.keys(data)
459-
.filter(key => (data as Record<string, unknown>)[key] !== undefined)
460-
.join(' ');
462+
private getFieldsString(data: unknown): string {
463+
const root: FieldNode = { children: new Map() };
464+
this.mergeFieldsIntoTree(data, root);
465+
return this.serializeFieldNode(root);
461466
}
462467

463-
/**
464-
* Extracts the union of all defined property keys across an array of objects
465-
* as a space-separated string. Used to build the `@allow(fields: ...)` mutation
466-
* directive for bulk operations.
467-
*/
468-
private getArrayObjectsKeys(data: Array<unknown>): string {
469-
const allKeys = new Set<string>();
470-
for (const element of data) {
471-
if (validator.isNonNullObject(element)) {
472-
const record = element as Record<string, unknown>;
473-
Object.keys(record).forEach(key => {
474-
if (record[key] !== undefined) {
475-
allKeys.add(key);
476-
}
477-
});
468+
private mergeFieldsIntoTree(data: unknown, node: FieldNode): void {
469+
if (validator.isArray(data)) {
470+
for (const item of data) {
471+
this.mergeFieldsIntoTree(item, node);
472+
}
473+
} else if (validator.isNonNullObject(data) && !(data instanceof Date)) {
474+
const record = data as Record<string, unknown>;
475+
for (const [key, val] of Object.entries(record)) {
476+
if (val === undefined) {
477+
continue;
478+
}
479+
let childNode = node.children.get(key);
480+
if (!childNode) {
481+
childNode = { children: new Map() };
482+
node.children.set(key, childNode);
483+
}
484+
this.mergeFieldsIntoTree(val, childNode);
478485
}
479486
}
480-
return Array.from(allKeys).join(' ');
487+
}
488+
489+
private serializeFieldNode(node: FieldNode): string {
490+
const parts: string[] = [];
491+
const sortedKeys = Array.from(node.children.keys()).sort((a, b) => a.localeCompare(b));
492+
for (const key of sortedKeys) {
493+
const childNode = node.children.get(key)!;
494+
if (childNode.children.size > 0) {
495+
const nestedString = this.serializeFieldNode(childNode);
496+
parts.push(`${key} { ${nestedString} }`);
497+
} else {
498+
parts.push(key);
499+
}
500+
}
501+
return parts.join(' ');
481502
}
482503

483504
private handleBulkImportErrors(err: FirebaseDataConnectError): never {
@@ -521,7 +542,7 @@ export class DataConnectApiClient {
521542

522543
try {
523544
const { capitalized, camelCase } = this.getTableNames(tableName);
524-
const keys = this.getObjectKeys(data);
545+
const keys = this.getFieldsString(data);
525546
const mutation =
526547
`mutation($data: ${capitalized}_Data! @allow(fields: "${keys}")) {
527548
${camelCase}_insert(data: $data)
@@ -557,10 +578,16 @@ export class DataConnectApiClient {
557578
message: '`data` must be a non-empty array for insertMany.',
558579
});
559580
}
581+
if (data.length > 10000) {
582+
throw new FirebaseDataConnectError({
583+
code: DATA_CONNECT_ERROR_CODE_MAPPING.INVALID_ARGUMENT,
584+
message: '`data` array exceeds the maximum limit of 10,000 items.'
585+
});
586+
}
560587

561588
try {
562589
const { capitalized, camelCase } = this.getTableNames(tableName);
563-
const keys = this.getArrayObjectsKeys(data);
590+
const keys = this.getFieldsString(data);
564591
const mutation =
565592
`mutation($data: [${capitalized}_Data!]! @allow(fields: "${keys}")) {
566593
${camelCase}_insertMany(data: $data)
@@ -606,7 +633,7 @@ export class DataConnectApiClient {
606633

607634
try {
608635
const { capitalized, camelCase } = this.getTableNames(tableName);
609-
const keys = this.getObjectKeys(data);
636+
const keys = this.getFieldsString(data);
610637
const mutation =
611638
`mutation($data: ${capitalized}_Data! @allow(fields: "${keys}")) {
612639
${camelCase}_upsert(data: $data)
@@ -642,10 +669,16 @@ export class DataConnectApiClient {
642669
message: '`data` must be a non-empty array for upsertMany.'
643670
});
644671
}
672+
if (data.length > 10000) {
673+
throw new FirebaseDataConnectError({
674+
code: DATA_CONNECT_ERROR_CODE_MAPPING.INVALID_ARGUMENT,
675+
message: '`data` array exceeds the maximum limit of 10,000 items.'
676+
});
677+
}
645678

646679
try {
647680
const { capitalized, camelCase } = this.getTableNames(tableName);
648-
const keys = this.getArrayObjectsKeys(data);
681+
const keys = this.getFieldsString(data);
649682
const mutation =
650683
`mutation($data: [${capitalized}_Data!]! @allow(fields: "${keys}")) {
651684
${camelCase}_upsertMany(data: $data)

‎test/unit/data-connect/data-connect-api-client-internal.spec.ts‎

Lines changed: 107 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -699,9 +699,10 @@ describe('DataConnectApiClient CRUD helpers', () => {
699699
// Helper function to normalize GraphQL strings
700700
const normalizeGraphQLString = (str: string): string => {
701701
return str
702-
.replace(/\n/g, '') // Remove newlines
703-
.replace(/\s+/g, ' ') // Replace multiple spaces with a single space
704-
.trim(); // Remove leading/trailing whitespace from the whole string
702+
.replace(/\s*\n\s*/g, ' ') // Replace newline and surrounding spaces with a single space
703+
.replace(/\s+/g, ' ') // Collapse multiple spaces to a single space
704+
.replace(/\s*([(){},:"'])\s*/g, '$1') // Remove all spaces surrounding structural characters
705+
.trim(); // Remove leading/trailing whitespace from the whole string
705706
};
706707

707708
/**
@@ -765,7 +766,7 @@ describe('DataConnectApiClient CRUD helpers', () => {
765766
it('should call executeGraphql with the correct mutation for complex data', async () => {
766767
const complexData = { id: 'abc', active: true, scores: [10, 20], info: { nested: 'yes/no "quote" \\slash\\' } };
767768
const expectedMutation =
768-
`mutation($data: TestTable_Data! @allow(fields: "id active scores info")) {
769+
`mutation($data: TestTable_Data! @allow(fields: "active id info { nested } scores")) {
769770
${formatedTableName}_insert(data: $data)
770771
}`;
771772
await apiClient.insert(tableName, complexData);
@@ -774,7 +775,7 @@ describe('DataConnectApiClient CRUD helpers', () => {
774775

775776
it('should call executeGraphql with the correct mutation for undefined and null values', async () => {
776777
const expectedMutation =
777-
`mutation($data: TestTable_Data! @allow(fields: "genre title ratings director extras")) {
778+
`mutation($data: TestTable_Data! @allow(fields: "director extras { a } genre ratings title")) {
778779
${formatedTableName}_insert(data: $data)
779780
}`;
780781
await apiClient.insert(tableName, dataWithUndefined);
@@ -819,6 +820,27 @@ describe('DataConnectApiClient CRUD helpers', () => {
819820
await apiClient.insert(tableName, data);
820821
expectNormalizedExecuteGraphqlCall(expectedMutation, expectedOptions);
821822
});
823+
824+
it('should recursively extract deep nested object/array fields in @allow directive', async () => {
825+
const deepData = {
826+
id: '123',
827+
customerId: 'c1',
828+
total: 100,
829+
tags: ['a', 'b'],
830+
products_on_order: [
831+
{ id: 'p1', name: 'Product 1', price: 9.99, categories: [{ id: 'cat1', name: 'Category 1' }] }
832+
]
833+
};
834+
const expectedMutation = `
835+
mutation(
836+
$data: TestTable_Data!
837+
@allow(fields: "customerId id products_on_order { categories { id name } id name price } tags total"))
838+
{
839+
testTable_insert(data: $data)
840+
}`;
841+
await apiClient.insert(tableName, deepData);
842+
expectNormalizedExecuteGraphqlCall(expectedMutation, { variables: { data: deepData } });
843+
});
822844
});
823845

824846
// --- INSERT MANY TESTS ---
@@ -852,7 +874,7 @@ describe('DataConnectApiClient CRUD helpers', () => {
852874
{ id: 'b', scores: [1, 2], info: { nested: 'n2/\\' } }
853875
];
854876
const expectedMutation = `
855-
mutation($data: [TestTable_Data!]! @allow(fields: "id active info scores")) {
877+
mutation($data: [TestTable_Data!]! @allow(fields: "active id info { nested } scores")) {
856878
${formatedTableName}_insertMany(data: $data)
857879
}`;
858880
await apiClient.insertMany(tableName, complexDataArray);
@@ -865,7 +887,7 @@ describe('DataConnectApiClient CRUD helpers', () => {
865887
dataWithUndefined
866888
];
867889
const expectedMutation = `
868-
mutation($data: [TestTable_Data!]! @allow(fields: "genre title ratings director extras")) {
890+
mutation($data: [TestTable_Data!]! @allow(fields: "director extras { a } genre ratings title")) {
869891
${formatedTableName}_insertMany(data: $data)
870892
}`;
871893
await apiClient.insertMany(tableName, dataArray);
@@ -892,6 +914,12 @@ describe('DataConnectApiClient CRUD helpers', () => {
892914
.to.be.rejectedWith(FirebaseDataConnectError, /`data` must be a non-empty array for insertMany./);
893915
});
894916

917+
it('should throw FirebaseDataConnectError if the data array length exceeds 10,000', async () => {
918+
const oversizedArray = new Array(10001).fill({ name: 'a' });
919+
await expect(apiClient.insertMany(tableName, oversizedArray))
920+
.to.be.rejectedWith(FirebaseDataConnectError, /`data` array exceeds the maximum limit of 10,000 items./);
921+
});
922+
895923
it('should amend the message for query errors', async () => {
896924
try {
897925
await apiClientQueryError.insertMany(tableName, [{ data: 1 }]);
@@ -918,6 +946,68 @@ describe('DataConnectApiClient CRUD helpers', () => {
918946
await apiClient.insertMany(tableName, data);
919947
expectNormalizedExecuteGraphqlCall(expectedMutation, expectedOptions);
920948
});
949+
950+
// eslint-disable-next-line max-len
951+
it('should coalesce different object shapes in a bulk array into a single union of fields in @allow directive', async () => {
952+
const dataArray = [
953+
{
954+
id: '1',
955+
name: 'Item 1',
956+
metadata: {
957+
tags: ['new', 'sale'],
958+
dimensions: { width: 10, height: 20 }
959+
}
960+
},
961+
{
962+
id: '2',
963+
price: 19.99,
964+
metadata: {
965+
dimensions: { depth: 5 },
966+
manufacturer: { name: 'M1', location: { country: 'US' } }
967+
}
968+
},
969+
{
970+
id: '3',
971+
name: 'Item 3',
972+
metadata: {
973+
tags: ['promo'],
974+
manufacturer: { location: { city: 'SF' } }
975+
}
976+
}
977+
];
978+
979+
const expectedMutation = `
980+
mutation(
981+
$data: [TestTable_Data!]!
982+
@allow(
983+
fields: "
984+
id
985+
metadata {
986+
dimensions {
987+
depth
988+
height
989+
width
990+
}
991+
manufacturer {
992+
location {
993+
city
994+
country
995+
}
996+
name
997+
}
998+
tags
999+
}
1000+
name
1001+
price
1002+
"
1003+
)
1004+
) {
1005+
${formatedTableName}_insertMany(data: $data)
1006+
}`;
1007+
1008+
await apiClient.insertMany(tableName, dataArray);
1009+
expectNormalizedExecuteGraphqlCall(expectedMutation, { variables: { data: dataArray } });
1010+
});
9211011
});
9221012

9231013
// --- UPSERT TESTS ---
@@ -948,7 +1038,7 @@ describe('DataConnectApiClient CRUD helpers', () => {
9481038
it('should call executeGraphql with the correct mutation for complex data', async () => {
9491039
const complexData = { id: 'key2', active: false, items: [1, null], detail: { status: 'done/\\' } };
9501040
const expectedMutation = `
951-
mutation($data: TestTable_Data! @allow(fields: "id active items detail")) {
1041+
mutation($data: TestTable_Data! @allow(fields: "active detail { status } id items")) {
9521042
${formatedTableName}_upsert(data: $data)
9531043
}`;
9541044
await apiClient.upsert(tableName, complexData);
@@ -957,7 +1047,7 @@ describe('DataConnectApiClient CRUD helpers', () => {
9571047

9581048
it('should call executeGraphql with the correct mutation for undefined and null values', async () => {
9591049
const expectedMutation = `
960-
mutation($data: TestTable_Data! @allow(fields: "genre title ratings director extras")) {
1050+
mutation($data: TestTable_Data! @allow(fields: "director extras { a } genre ratings title")) {
9611051
${formatedTableName}_upsert(data: $data)
9621052
}`;
9631053
await apiClient.upsert(tableName, dataWithUndefined);
@@ -1035,7 +1125,7 @@ describe('DataConnectApiClient CRUD helpers', () => {
10351125
{ id: 'y', scores: [null, 2] }
10361126
];
10371127
const expectedMutation = `
1038-
mutation($data: [TestTable_Data!]! @allow(fields: "id active info scores")) {
1128+
mutation($data: [TestTable_Data!]! @allow(fields: "active id info { nested } scores")) {
10391129
${formatedTableName}_upsertMany(data: $data)
10401130
}`;
10411131
await apiClient.upsertMany(tableName, complexDataArray);
@@ -1048,7 +1138,7 @@ describe('DataConnectApiClient CRUD helpers', () => {
10481138
dataWithUndefined
10491139
];
10501140
const expectedMutation = `
1051-
mutation($data: [TestTable_Data!]! @allow(fields: "genre title ratings director extras")) {
1141+
mutation($data: [TestTable_Data!]! @allow(fields: "director extras { a } genre ratings title")) {
10521142
${formatedTableName}_upsertMany(data: $data)
10531143
}`;
10541144
await apiClient.upsertMany(tableName, dataArray);
@@ -1075,6 +1165,12 @@ describe('DataConnectApiClient CRUD helpers', () => {
10751165
.to.be.rejectedWith(FirebaseDataConnectError, /`data` must be a non-empty array for upsertMany./);
10761166
});
10771167

1168+
it('should throw FirebaseDataConnectError if the data array length exceeds 10,000', async () => {
1169+
const oversizedArray = new Array(10001).fill({ name: 'a' });
1170+
await expect(apiClient.upsertMany(tableName, oversizedArray))
1171+
.to.be.rejectedWith(FirebaseDataConnectError, /`data` array exceeds the maximum limit of 10,000 items./);
1172+
});
1173+
10781174
it('should amend the message for query errors', async () => {
10791175
try {
10801176
await apiClientQueryError.upsertMany(tableName, [{ data: 1 }]);

0 commit comments

Comments
 (0)