diff --git a/packages/firestore/src/local/memory_remote_document_cache.ts b/packages/firestore/src/local/memory_remote_document_cache.ts index 2b145acdf9d..42a0010d4ac 100644 --- a/packages/firestore/src/local/memory_remote_document_cache.ts +++ b/packages/firestore/src/local/memory_remote_document_cache.ts @@ -47,6 +47,11 @@ interface MemoryRemoteDocumentCacheEntry { size: number; } +/** + * The smallest value representable by a 64-bit signed integer (long). + */ +const MIN_LONG_VALUE = '-9223372036854775808'; + type DocumentEntryMap = SortedMap; function documentEntryMap(): DocumentEntryMap { return new SortedMap( @@ -171,7 +176,12 @@ class MemoryRemoteDocumentCacheImpl implements MemoryRemoteDocumentCache { // Documents are ordered by key, so we can use a prefix scan to narrow down // the documents we need to match the query against. const collectionPath = query.path; - const prefix = new DocumentKey(collectionPath.child('')); + // Document keys are ordered first by numeric value ("__id__"), + // then lexicographically by string value. Start the iterator at the minimum + // possible Document key value. + const prefix = new DocumentKey( + collectionPath.child('__id' + MIN_LONG_VALUE + '__') + ); const iterator = this.docs.getIteratorFrom(prefix); while (iterator.hasNext()) { const { diff --git a/packages/firestore/src/model/path.ts b/packages/firestore/src/model/path.ts index 3b68a67c68f..64cb0376a0e 100644 --- a/packages/firestore/src/model/path.ts +++ b/packages/firestore/src/model/path.ts @@ -15,6 +15,8 @@ * limitations under the License. */ +import { Integer } from '@firebase/webchannel-wrapper/bloom-blob'; + import { debugAssert, fail } from '../util/assert'; import { Code, FirestoreError } from '../util/error'; @@ -163,28 +165,59 @@ abstract class BasePath> { return this.segments.slice(this.offset, this.limit()); } + /** + * Compare 2 paths segment by segment, prioritizing numeric IDs + * (e.g., "__id123__") in numeric ascending order, followed by string + * segments in lexicographical order. + */ static comparator>( p1: BasePath, p2: BasePath ): number { const len = Math.min(p1.length, p2.length); for (let i = 0; i < len; i++) { - const left = p1.get(i); - const right = p2.get(i); - if (left < right) { - return -1; - } - if (left > right) { - return 1; + const comparison = BasePath.compareSegments(p1.get(i), p2.get(i)); + if (comparison !== 0) { + return comparison; } } - if (p1.length < p2.length) { + return Math.sign(p1.length - p2.length); + } + + private static compareSegments(lhs: string, rhs: string): number { + const isLhsNumeric = BasePath.isNumericId(lhs); + const isRhsNumeric = BasePath.isNumericId(rhs); + + if (isLhsNumeric && !isRhsNumeric) { + // Only lhs is numeric return -1; - } - if (p1.length > p2.length) { + } else if (!isLhsNumeric && isRhsNumeric) { + // Only rhs is numeric return 1; + } else if (isLhsNumeric && isRhsNumeric) { + // both numeric + return BasePath.extractNumericId(lhs).compare( + BasePath.extractNumericId(rhs) + ); + } else { + // both non-numeric + if (lhs < rhs) { + return -1; + } + if (lhs > rhs) { + return 1; + } + return 0; } - return 0; + } + + // Checks if a segment is a numeric ID (starts with "__id" and ends with "__"). + private static isNumericId(segment: string): boolean { + return segment.startsWith('__id') && segment.endsWith('__'); + } + + private static extractNumericId(segment: string): Integer { + return Integer.fromString(segment.substring(4, segment.length - 2)); } } diff --git a/packages/firestore/test/integration/api/database.test.ts b/packages/firestore/test/integration/api/database.test.ts index 81dc7362a22..1cda49d9229 100644 --- a/packages/firestore/test/integration/api/database.test.ts +++ b/packages/firestore/test/integration/api/database.test.ts @@ -79,7 +79,8 @@ import { withTestDocAndInitialData, withNamedTestDbsOrSkipUnlessUsingEmulator, toDataArray, - checkOnlineAndOfflineResultsMatch + checkOnlineAndOfflineResultsMatch, + toIds } from '../util/helpers'; import { DEFAULT_SETTINGS, DEFAULT_PROJECT_ID } from '../util/settings'; @@ -2245,4 +2246,182 @@ apiDescribe('Database', persistence => { }); }); }); + + describe('sort documents by DocumentId', () => { + it('snapshot listener sorts query by DocumentId same way as get query', async () => { + const testDocs = { + 'A': { a: 1 }, + 'a': { a: 1 }, + 'Aa': { a: 1 }, + '7': { a: 1 }, + '12': { a: 1 }, + '__id7__': { a: 1 }, + '__id12__': { a: 1 }, + '__id-2__': { a: 1 }, + '_id1__': { a: 1 }, + '__id1_': { a: 1 }, + '__id': { a: 1 }, + // largest long numbers + '__id9223372036854775807__': { a: 1 }, + '__id9223372036854775806__': { a: 1 }, + // smallest long numbers + '__id-9223372036854775808__': { a: 1 }, + '__id-9223372036854775807__': { a: 1 } + }; + + return withTestCollection(persistence, testDocs, async collectionRef => { + const orderedQuery = query(collectionRef, orderBy(documentId())); + const expectedDocs = [ + '__id-9223372036854775808__', + '__id-9223372036854775807__', + '__id-2__', + '__id7__', + '__id12__', + '__id9223372036854775806__', + '__id9223372036854775807__', + '12', + '7', + 'A', + 'Aa', + '__id', + '__id1_', + '_id1__', + 'a' + ]; + + const getSnapshot = await getDocsFromServer(orderedQuery); + expect(toIds(getSnapshot)).to.deep.equal(expectedDocs); + + const storeEvent = new EventsAccumulator(); + const unsubscribe = onSnapshot(orderedQuery, storeEvent.storeEvent); + const watchSnapshot = await storeEvent.awaitEvent(); + expect(toIds(watchSnapshot)).to.deep.equal(expectedDocs); + + unsubscribe(); + }); + }); + + it('snapshot listener sorts filtered query by DocumentId same way as get query', async () => { + const testDocs = { + 'A': { a: 1 }, + 'a': { a: 1 }, + 'Aa': { a: 1 }, + '7': { a: 1 }, + '12': { a: 1 }, + '__id7__': { a: 1 }, + '__id12__': { a: 1 }, + '__id-2__': { a: 1 }, + '_id1__': { a: 1 }, + '__id1_': { a: 1 }, + '__id': { a: 1 }, + // largest long numbers + '__id9223372036854775807__': { a: 1 }, + '__id9223372036854775806__': { a: 1 }, + // smallest long numbers + '__id-9223372036854775808__': { a: 1 }, + '__id-9223372036854775807__': { a: 1 } + }; + + return withTestCollection(persistence, testDocs, async collectionRef => { + const filteredQuery = query( + collectionRef, + orderBy(documentId()), + where(documentId(), '>', '__id7__'), + where(documentId(), '<=', 'Aa') + ); + const expectedDocs = [ + '__id12__', + '__id9223372036854775806__', + '__id9223372036854775807__', + '12', + '7', + 'A', + 'Aa' + ]; + + const getSnapshot = await getDocsFromServer(filteredQuery); + expect(toIds(getSnapshot)).to.deep.equal(expectedDocs); + + const storeEvent = new EventsAccumulator(); + const unsubscribe = onSnapshot(filteredQuery, storeEvent.storeEvent); + const watchSnapshot = await storeEvent.awaitEvent(); + expect(toIds(watchSnapshot)).to.deep.equal(expectedDocs); + unsubscribe(); + }); + }); + + // eslint-disable-next-line no-restricted-properties + (persistence.gc === 'lru' ? describe : describe.skip)('offline', () => { + it('SDK orders query the same way online and offline', async () => { + const testDocs = { + 'A': { a: 1 }, + 'a': { a: 1 }, + 'Aa': { a: 1 }, + '7': { a: 1 }, + '12': { a: 1 }, + '__id7__': { a: 1 }, + '__id12__': { a: 1 }, + '__id-2__': { a: 1 }, + '_id1__': { a: 1 }, + '__id1_': { a: 1 }, + '__id': { a: 1 }, + // largest long numbers + '__id9223372036854775807__': { a: 1 }, + '__id9223372036854775806__': { a: 1 }, + // smallest long numbers + '__id-9223372036854775808__': { a: 1 }, + '__id-9223372036854775807__': { a: 1 } + }; + + return withTestCollection( + persistence, + testDocs, + async collectionRef => { + const orderedQuery = query(collectionRef, orderBy(documentId())); + let expectedDocs = [ + '__id-9223372036854775808__', + '__id-9223372036854775807__', + '__id-2__', + '__id7__', + '__id12__', + '__id9223372036854775806__', + '__id9223372036854775807__', + '12', + '7', + 'A', + 'Aa', + '__id', + '__id1_', + '_id1__', + 'a' + ]; + await checkOnlineAndOfflineResultsMatch( + orderedQuery, + ...expectedDocs + ); + + const filteredQuery = query( + collectionRef, + orderBy(documentId()), + where(documentId(), '>', '__id7__'), + where(documentId(), '<=', 'Aa') + ); + expectedDocs = [ + '__id12__', + '__id9223372036854775806__', + '__id9223372036854775807__', + '12', + '7', + 'A', + 'Aa' + ]; + await checkOnlineAndOfflineResultsMatch( + filteredQuery, + ...expectedDocs + ); + } + ); + }); + }); + }); });