Skip to content

Commit 67ebf87

Browse files
joehanTorbenWetter
andauthored
Correct handling of __name__ in firestore indexes (#8862)
* fix(firestore): smart filtering of __name__ fields in index listings Preserve __name__ fields with DESCENDING order while filtering out implicit ASCENDING ones to fix deployment conflicts. - Fixes duplicate index issues (#7629) - Fixes deployment conflicts (#8859) - Add comprehensive test coverage Fixes #7629, #8859 * refactor(firestore): extract index filtering logic to static method Refactored __name__ field filtering from listIndexes into static method FirestoreApi.processIndexes() to ensure tests verify production code rather than duplicating the implementation. * refactor(firestore): simplify processIndexes filter logic Simplified the filter expression in processIndexes() to a single boolean expression as suggested in PR feedback for better readability and conciseness. * Update CHANGELOG.md * Only filters __name__ when order matches order of last non name field and it is the last field * Super defensive --------- Co-authored-by: Torben Wetter <[email protected]>
1 parent fe8ae92 commit 67ebf87

File tree

3 files changed

+167
-1
lines changed

3 files changed

+167
-1
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
- Fixed issue where `__name__` fields with DESCENDING order were incorrectly filtered from index listings, causing duplicate index issues (#7629) and deployment conflicts (#8859). The fix now preserves `__name__` fields with explicit DESCENDING order while filtering out implicit ASCENDING `__name__` fields.

src/firestore/api.ts

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,31 @@ export class FirestoreApi {
1818
apiClient = new Client({ urlPrefix: firestoreOrigin(), apiVersion: "v1" });
1919
printer = new PrettyPrint();
2020

21+
/**
22+
* Process indexes by filtering out implicit __name__ fields with ASCENDING order.
23+
* Keeps explicit __name__ fields with DESCENDING order.
24+
* @param indexes Array of indexes to process
25+
* @returns Processed array of indexes with filtered fields
26+
*/
27+
public static processIndexes(indexes: types.Index[]): types.Index[] {
28+
return indexes.map((index: types.Index): types.Index => {
29+
// Per https://firebase.google.com/docs/firestore/query-data/index-overview#default_ordering_and_the_name_field
30+
// this matches the direction of the last non-name field in the index.
31+
let fields = index.fields;
32+
const lastField = index.fields?.[index.fields.length - 1];
33+
if (lastField?.fieldPath === "__name__") {
34+
const defaultDirection = index.fields?.[index.fields.length - 2]?.order;
35+
if (lastField?.order === defaultDirection) {
36+
fields = fields.slice(0, -1);
37+
}
38+
}
39+
return {
40+
...index,
41+
fields,
42+
};
43+
});
44+
}
45+
2146
/**
2247
* Deploy an index specification to the specified project.
2348
* @param options the CLI options.
@@ -183,7 +208,7 @@ export class FirestoreApi {
183208
return [];
184209
}
185210

186-
return indexes;
211+
return FirestoreApi.processIndexes(indexes);
187212
}
188213

189214
/**

src/firestore/indexes.spec.ts

Lines changed: 140 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -447,6 +447,146 @@ describe("IndexSpecMatching", () => {
447447
});
448448
});
449449

450+
describe("IndexListingWithNameFields", () => {
451+
it("should filter out __name__ fields with in the default order, when the default is ASCENDING", () => {
452+
const mockIndexes: API.Index[] = [
453+
{
454+
name: "/projects/myproject/databases/(default)/collectionGroups/collection/indexes/abc123",
455+
queryScope: API.QueryScope.COLLECTION,
456+
fields: [
457+
{ fieldPath: "foo", order: API.Order.ASCENDING },
458+
{ fieldPath: "__name__", order: API.Order.ASCENDING },
459+
],
460+
state: API.State.READY,
461+
},
462+
];
463+
464+
const result = FirestoreApi.processIndexes(mockIndexes);
465+
466+
expect(result[0].fields).to.have.length(1);
467+
expect(result[0].fields[0].fieldPath).to.equal("foo");
468+
});
469+
470+
it("should filter out __name__ fields with in the default order, when the default is DESCENDING", () => {
471+
const mockIndexes: API.Index[] = [
472+
{
473+
name: "/projects/myproject/databases/(default)/collectionGroups/collection/indexes/abc123",
474+
queryScope: API.QueryScope.COLLECTION,
475+
fields: [
476+
{ fieldPath: "foo", order: API.Order.DESCENDING },
477+
{ fieldPath: "__name__", order: API.Order.DESCENDING },
478+
],
479+
state: API.State.READY,
480+
},
481+
];
482+
483+
const result = FirestoreApi.processIndexes(mockIndexes);
484+
485+
expect(result[0].fields).to.have.length(1);
486+
expect(result[0].fields[0].fieldPath).to.equal("foo");
487+
});
488+
489+
it("should keep __name__ fields with DESCENDING order, when the default is ASCENDING", () => {
490+
const mockIndexes: API.Index[] = [
491+
{
492+
name: "/projects/myproject/databases/(default)/collectionGroups/collection/indexes/abc123",
493+
queryScope: API.QueryScope.COLLECTION,
494+
fields: [
495+
{ fieldPath: "foo", order: API.Order.ASCENDING },
496+
{ fieldPath: "__name__", order: API.Order.DESCENDING },
497+
],
498+
state: API.State.READY,
499+
},
500+
];
501+
502+
const result = FirestoreApi.processIndexes(mockIndexes);
503+
504+
expect(result[0].fields).to.have.length(2);
505+
expect(result[0].fields[0].fieldPath).to.equal("foo");
506+
expect(result[0].fields[1].fieldPath).to.equal("__name__");
507+
expect(result[0].fields[1].order).to.equal(API.Order.DESCENDING);
508+
});
509+
510+
it("should keep __name__ fields with ASCENDING order, when the default is DESCENDING", () => {
511+
const mockIndexes: API.Index[] = [
512+
{
513+
name: "/projects/myproject/databases/(default)/collectionGroups/collection/indexes/abc123",
514+
queryScope: API.QueryScope.COLLECTION,
515+
fields: [
516+
{ fieldPath: "foo", order: API.Order.DESCENDING },
517+
{ fieldPath: "__name__", order: API.Order.ASCENDING },
518+
],
519+
state: API.State.READY,
520+
},
521+
];
522+
523+
const result = FirestoreApi.processIndexes(mockIndexes);
524+
525+
expect(result[0].fields).to.have.length(2);
526+
expect(result[0].fields[0].fieldPath).to.equal("foo");
527+
expect(result[0].fields[1].fieldPath).to.equal("__name__");
528+
expect(result[0].fields[1].order).to.equal(API.Order.ASCENDING);
529+
});
530+
531+
it("should distinguish between indexes that differ only by __name__ order", () => {
532+
const mockIndexes: API.Index[] = [
533+
{
534+
name: "/projects/myproject/databases/(default)/collectionGroups/collection/indexes/abc123",
535+
queryScope: API.QueryScope.COLLECTION,
536+
fields: [
537+
{ fieldPath: "foo", order: API.Order.ASCENDING },
538+
{ fieldPath: "__name__", order: API.Order.ASCENDING },
539+
],
540+
state: API.State.READY,
541+
},
542+
{
543+
name: "/projects/myproject/databases/(default)/collectionGroups/collection/indexes/def456",
544+
queryScope: API.QueryScope.COLLECTION,
545+
fields: [
546+
{ fieldPath: "foo", order: API.Order.ASCENDING },
547+
{ fieldPath: "__name__", order: API.Order.DESCENDING },
548+
],
549+
state: API.State.READY,
550+
},
551+
];
552+
553+
const result = FirestoreApi.processIndexes(mockIndexes);
554+
555+
// First index should have __name__ field filtered out
556+
expect(result[0].fields).to.have.length(1);
557+
expect(result[0].fields[0].fieldPath).to.equal("foo");
558+
559+
// Second index should keep __name__ field with DESCENDING order
560+
expect(result[1].fields).to.have.length(2);
561+
expect(result[1].fields[0].fieldPath).to.equal("foo");
562+
expect(result[1].fields[1].fieldPath).to.equal("__name__");
563+
expect(result[1].fields[1].order).to.equal(API.Order.DESCENDING);
564+
565+
// The two processed indexes should be different (fixing the duplicate issue)
566+
expect(JSON.stringify(result[0].fields)).to.not.equal(JSON.stringify(result[1].fields));
567+
});
568+
569+
it("should handle indexes with no __name__ fields", () => {
570+
const mockIndexes: API.Index[] = [
571+
{
572+
name: "/projects/myproject/databases/(default)/collectionGroups/collection/indexes/abc123",
573+
queryScope: API.QueryScope.COLLECTION,
574+
fields: [
575+
{ fieldPath: "foo", order: API.Order.ASCENDING },
576+
{ fieldPath: "bar", arrayConfig: API.ArrayConfig.CONTAINS },
577+
],
578+
state: API.State.READY,
579+
},
580+
];
581+
582+
const result = FirestoreApi.processIndexes(mockIndexes);
583+
584+
expect(result[0].fields).to.have.length(2);
585+
expect(result[0].fields[0].fieldPath).to.equal("foo");
586+
expect(result[0].fields[1].fieldPath).to.equal("bar");
587+
});
588+
});
589+
450590
describe("IndexSorting", () => {
451591
it("should be able to handle empty arrays", () => {
452592
expect(([] as Spec.Index[]).sort(sort.compareSpecIndex)).to.eql([]);

0 commit comments

Comments
 (0)