Skip to content

Commit

Permalink
Fix: sort document reference by long type id (#2257)
Browse files Browse the repository at this point in the history
  • Loading branch information
@milaGGL
milaGGL authored Dec 17, 2024
1 parent bef5668 commit 3fd0de9
Show file tree
Hide file tree
Showing 2 changed files with 165 additions and 8 deletions.
67 changes: 59 additions & 8 deletions dev/src/path.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,10 @@ abstract class Path<T> {
/**
* Compare the current path against another Path object.
*
* Compare the current path against another Path object. Paths are compared segment by segment,
* prioritizing numeric IDs (e.g., "__id123__") in numeric ascending order, followed by string
* segments in lexicographical order.
*
* @private
* @internal
* @param other The path to compare to.
Expand All @@ -158,20 +162,67 @@ abstract class Path<T> {
compareTo(other: Path<T>): number {
const len = Math.min(this.segments.length, other.segments.length);
for (let i = 0; i < len; i++) {
if (this.segments[i] < other.segments[i]) {
return -1;
}
if (this.segments[i] > other.segments[i]) {
return 1;
const comparison = this.compareSegments(
this.segments[i],
other.segments[i]
);
if (comparison !== 0) {
return comparison;
}
}
if (this.segments.length < other.segments.length) {
return Math.sign(this.segments.length - other.segments.length);
}

private compareSegments(lhs: string, rhs: string): number {
const isLhsNumeric = this.isNumericId(lhs);
const isRhsNumeric = this.isNumericId(rhs);

if (isLhsNumeric && !isRhsNumeric) {
// Only lhs is numeric
return -1;
} else if (!isLhsNumeric && isRhsNumeric) {
// Only rhs is numeric
return 1;
} else if (isLhsNumeric && isRhsNumeric) {
// both numeric
return this.compareNumbers(
this.extractNumericId(lhs),
this.extractNumericId(rhs)
);
} else {
// both non-numeric
return this.compareStrings(lhs, rhs);
}
}

// Checks if a segment is a numeric ID (starts with "__id" and ends with "__").
private isNumericId(segment: string): boolean {
return segment.startsWith('__id') && segment.endsWith('__');
}

// Extracts the long number from a numeric ID segment.
private extractNumericId(segment: string): bigint {
return BigInt(segment.substring(4, segment.length - 2));
}

private compareNumbers(lhs: bigint, rhs: bigint): number {
if (lhs < rhs) {
return -1;
} else if (lhs > rhs) {
return 1;
} else {
return 0;
}
if (this.segments.length > other.segments.length) {
}

private compareStrings(lhs: string, rhs: string): number {
if (lhs < rhs) {
return -1;
} else if (lhs > rhs) {
return 1;
} else {
return 0;
}
return 0;
}

/**
Expand Down
106 changes: 106 additions & 0 deletions dev/system-test/firestore.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3913,6 +3913,112 @@ describe('Query class', () => {
unsubscribe();
});

it('snapshot listener sorts query by DocumentId same way as server', async () => {
const batch = firestore.batch();
batch.set(randomCol.doc('A'), {a: 1});
batch.set(randomCol.doc('a'), {a: 1});
batch.set(randomCol.doc('Aa'), {a: 1});
batch.set(randomCol.doc('7'), {a: 1});
batch.set(randomCol.doc('12'), {a: 1});
batch.set(randomCol.doc('__id7__'), {a: 1});
batch.set(randomCol.doc('__id12__'), {a: 1});
batch.set(randomCol.doc('__id-2__'), {a: 1});
batch.set(randomCol.doc('__id1_'), {a: 1});
batch.set(randomCol.doc('_id1__'), {a: 1});
batch.set(randomCol.doc('__id'), {a: 1});
// largest long number
batch.set(randomCol.doc('__id9223372036854775807__'), {a: 1});
batch.set(randomCol.doc('__id9223372036854775806__'), {a: 1});
// smallest long number
batch.set(randomCol.doc('__id-9223372036854775808__'), {a: 1});
batch.set(randomCol.doc('__id-9223372036854775807__'), {a: 1});
await batch.commit();

const query = randomCol.orderBy(FieldPath.documentId());
const expectedDocs = [
'__id-9223372036854775808__',
'__id-9223372036854775807__',
'__id-2__',
'__id7__',
'__id12__',
'__id9223372036854775806__',
'__id9223372036854775807__',
'12',
'7',
'A',
'Aa',
'__id',
'__id1_',
'_id1__',
'a',
];

const getSnapshot = await query.get();
expect(getSnapshot.docs.map(d => d.id)).to.deep.equal(expectedDocs);

const unsubscribe = query.onSnapshot(snapshot =>
currentDeferred.resolve(snapshot)
);

const watchSnapshot = await waitForSnapshot();
// Compare the snapshot (including sort order) of a snapshot
snapshotsEqual(watchSnapshot, {
docs: getSnapshot.docs,
docChanges: getSnapshot.docChanges(),
});
unsubscribe();
});

it('snapshot listener sorts filtered query by DocumentId same way as server', async () => {
const batch = firestore.batch();
batch.set(randomCol.doc('A'), {a: 1});
batch.set(randomCol.doc('a'), {a: 1});
batch.set(randomCol.doc('Aa'), {a: 1});
batch.set(randomCol.doc('7'), {a: 1});
batch.set(randomCol.doc('12'), {a: 1});
batch.set(randomCol.doc('__id7__'), {a: 1});
batch.set(randomCol.doc('__id12__'), {a: 1});
batch.set(randomCol.doc('__id-2__'), {a: 1});
batch.set(randomCol.doc('__id1_'), {a: 1});
batch.set(randomCol.doc('_id1__'), {a: 1});
batch.set(randomCol.doc('__id'), {a: 1});
// largest long number
batch.set(randomCol.doc('__id9223372036854775807__'), {a: 1});
batch.set(randomCol.doc('__id9223372036854775806__'), {a: 1});
// smallest long number
batch.set(randomCol.doc('__id-9223372036854775808__'), {a: 1});
batch.set(randomCol.doc('__id-9223372036854775807__'), {a: 1});
await batch.commit();

const query = randomCol
.where(FieldPath.documentId(), '>', '__id7__')
.where(FieldPath.documentId(), '<=', 'A')
.orderBy(FieldPath.documentId());
const expectedDocs = [
'__id12__',
'__id9223372036854775806__',
'__id9223372036854775807__',
'12',
'7',
'A',
];

const getSnapshot = await query.get();
expect(getSnapshot.docs.map(d => d.id)).to.deep.equal(expectedDocs);

const unsubscribe = query.onSnapshot(snapshot =>
currentDeferred.resolve(snapshot)
);

const watchSnapshot = await waitForSnapshot();
// Compare the snapshot (including sort order) of a snapshot
snapshotsEqual(watchSnapshot, {
docs: getSnapshot.docs,
docChanges: getSnapshot.docChanges(),
});
unsubscribe();
});

it('SDK orders vector field same way as backend', async () => {
// We validate that the SDK orders the vector field the same way as the backend
// by comparing the sort order of vector fields from a Query.get() and
Expand Down

0 comments on commit 3fd0de9

Please sign in to comment.