diff --git a/packages/firestore/src/local/memory_remote_document_cache.ts b/packages/firestore/src/local/memory_remote_document_cache.ts index 180718e5ada..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,9 +176,11 @@ 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; - // DocumentId can be numeric ("__id__") or a plain string. Numeric IDs ordered before strings, sorted numerically. + // 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' + Number.MIN_SAFE_INTEGER + '__') + collectionPath.child('__id' + MIN_LONG_VALUE + '__') ); const iterator = this.docs.getIteratorFrom(prefix); while (iterator.hasNext()) { diff --git a/packages/firestore/src/model/path.ts b/packages/firestore/src/model/path.ts index b6573ee1a4a..2a50564adbb 100644 --- a/packages/firestore/src/model/path.ts +++ b/packages/firestore/src/model/path.ts @@ -17,6 +17,7 @@ import { debugAssert, fail } from '../util/assert'; import { Code, FirestoreError } from '../util/error'; +import { Integer } from '@firebase/webchannel-wrapper/bloom-blob'; export const DOCUMENT_KEY_NAME = '__name__'; @@ -194,8 +195,8 @@ abstract class BasePath> { return 1; } else if (isLhsNumeric && isRhsNumeric) { // both numeric - return Math.sign( - BasePath.extractNumericId(lhs) - BasePath.extractNumericId(rhs) + return BasePath.extractNumericId(lhs).compare( + BasePath.extractNumericId(rhs) ); } else { // both non-numeric @@ -214,8 +215,8 @@ abstract class BasePath> { return segment.startsWith('__id') && segment.endsWith('__'); } - private static extractNumericId(segment: string): number { - return parseInt(segment.substring(4, segment.length - 2), 10); + 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 882c05b1ecc..3cf00d05d37 100644 --- a/packages/firestore/test/integration/api/database.test.ts +++ b/packages/firestore/test/integration/api/database.test.ts @@ -2259,15 +2259,25 @@ apiDescribe('Database', persistence => { __id12__: { a: 1 }, '__id-2__': { a: 1 }, '_id1__': { a: 1 }, - '__id1_': { a: 1 } + '__id1_': { a: 1 }, + // max safe integer +1, +2 + '__id9007199254740992__': { a: 1 }, + '__id9007199254740993__': { a: 2 }, + // smallest long numbers + '__id-9223372036854775808__': { a: 3 }, + '__id-9223372036854775807__': { a: 4 } }; return withTestCollection(persistence, testDocs, async collectionRef => { const orderedQuery = query(collectionRef, orderBy(documentId())); const expectedDocs = [ + '__id-9223372036854775808__', + '__id-9223372036854775807__', '__id-2__', '__id7__', '__id12__', + '__id9007199254740992__', + '__id9007199254740993__', '12', '7', 'A', @@ -2284,6 +2294,7 @@ apiDescribe('Database', persistence => { const unsubscribe = onSnapshot(orderedQuery, storeEvent.storeEvent); const watchSnapshot = await storeEvent.awaitEvent(); expect(toIds(watchSnapshot)).to.deep.equal(expectedDocs); + unsubscribe(); }); }); @@ -2299,7 +2310,13 @@ apiDescribe('Database', persistence => { __id12__: { a: 1 }, '__id-2__': { a: 1 }, '_id1__': { a: 1 }, - '__id1_': { a: 1 } + '__id1_': { a: 1 }, + // max safe integer +1, +2 + '__id9007199254740992__': { a: 1 }, + '__id9007199254740993__': { a: 2 }, + // smallest long numbers + '__id-9223372036854775808__': { a: 3 }, + '__id-9223372036854775807__': { a: 4 } }; return withTestCollection(persistence, testDocs, async collectionRef => { @@ -2309,7 +2326,15 @@ apiDescribe('Database', persistence => { where(documentId(), '>', '__id7__'), where(documentId(), '<=', 'Aa') ); - const expectedDocs = ['__id12__', '12', '7', 'A', 'Aa']; + const expectedDocs = [ + '__id12__', + '__id9007199254740992__', + '__id9007199254740993__', + '12', + '7', + 'A', + 'Aa' + ]; const getSnapshot = await getDocsFromServer(filteredQuery); expect(toIds(getSnapshot)).to.deep.equal(expectedDocs); @@ -2335,18 +2360,28 @@ apiDescribe('Database', persistence => { __id12__: { a: 1 }, '__id-2__': { a: 1 }, '_id1__': { a: 1 }, - '__id1_': { a: 1 } + '__id1_': { a: 1 }, + // max safe integer +1, +2 + '__id9007199254740992__': { a: 1 }, + '__id9007199254740993__': { a: 2 }, + // smallest long numbers + '__id-9223372036854775808__': { a: 3 }, + '__id-9223372036854775807__': { a: 4 } }; return withTestCollection( - persistence, + persistence.toLruGc(), testDocs, async collectionRef => { const orderedQuery = query(collectionRef, orderBy(documentId())); let expectedDocs = [ + '__id-9223372036854775808__', + '__id-9223372036854775807__', '__id-2__', '__id7__', '__id12__', + '__id9007199254740992__', + '__id9007199254740993__', '12', '7', 'A', @@ -2366,7 +2401,15 @@ apiDescribe('Database', persistence => { where(documentId(), '>', '__id7__'), where(documentId(), '<=', 'Aa') ); - expectedDocs = ['__id12__', '12', '7', 'A', 'Aa']; + expectedDocs = [ + '__id12__', + '__id9007199254740992__', + '__id9007199254740993__', + '12', + '7', + 'A', + 'Aa' + ]; await checkOnlineAndOfflineResultsMatch( filteredQuery, ...expectedDocs