Skip to content

Commit 32bf021

Browse files
milaGGLmilaGGL
authored
Fix: sort document reference by long type id (#8673)
1 parent 46c91bc commit 32bf021

File tree

3 files changed

+235
-13
lines changed

3 files changed

+235
-13
lines changed

‎packages/firestore/src/local/memory_remote_document_cache.ts

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,11 @@ interface MemoryRemoteDocumentCacheEntry {
4747
size: number;
4848
}
4949

50+
/**
51+
* The smallest value representable by a 64-bit signed integer (long).
52+
*/
53+
const MIN_LONG_VALUE = '-9223372036854775808';
54+
5055
type DocumentEntryMap = SortedMap<DocumentKey, MemoryRemoteDocumentCacheEntry>;
5156
function documentEntryMap(): DocumentEntryMap {
5257
return new SortedMap<DocumentKey, MemoryRemoteDocumentCacheEntry>(
@@ -171,7 +176,12 @@ class MemoryRemoteDocumentCacheImpl implements MemoryRemoteDocumentCache {
171176
// Documents are ordered by key, so we can use a prefix scan to narrow down
172177
// the documents we need to match the query against.
173178
const collectionPath = query.path;
174-
const prefix = new DocumentKey(collectionPath.child(''));
179+
// Document keys are ordered first by numeric value ("__id<Long>__"),
180+
// then lexicographically by string value. Start the iterator at the minimum
181+
// possible Document key value.
182+
const prefix = new DocumentKey(
183+
collectionPath.child('__id' + MIN_LONG_VALUE + '__')
184+
);
175185
const iterator = this.docs.getIteratorFrom(prefix);
176186
while (iterator.hasNext()) {
177187
const {

‎packages/firestore/src/model/path.ts

Lines changed: 44 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@
1515
* limitations under the License.
1616
*/
1717

18+
import { Integer } from '@firebase/webchannel-wrapper/bloom-blob';
19+
1820
import { debugAssert, fail } from '../util/assert';
1921
import { Code, FirestoreError } from '../util/error';
2022

@@ -163,28 +165,59 @@ abstract class BasePath<B extends BasePath<B>> {
163165
return this.segments.slice(this.offset, this.limit());
164166
}
165167

168+
/**
169+
* Compare 2 paths segment by segment, prioritizing numeric IDs
170+
* (e.g., "__id123__") in numeric ascending order, followed by string
171+
* segments in lexicographical order.
172+
*/
166173
static comparator<T extends BasePath<T>>(
167174
p1: BasePath<T>,
168175
p2: BasePath<T>
169176
): number {
170177
const len = Math.min(p1.length, p2.length);
171178
for (let i = 0; i < len; i++) {
172-
const left = p1.get(i);
173-
const right = p2.get(i);
174-
if (left < right) {
175-
return -1;
176-
}
177-
if (left > right) {
178-
return 1;
179+
const comparison = BasePath.compareSegments(p1.get(i), p2.get(i));
180+
if (comparison !== 0) {
181+
return comparison;
179182
}
180183
}
181-
if (p1.length < p2.length) {
184+
return Math.sign(p1.length - p2.length);
185+
}
186+
187+
private static compareSegments(lhs: string, rhs: string): number {
188+
const isLhsNumeric = BasePath.isNumericId(lhs);
189+
const isRhsNumeric = BasePath.isNumericId(rhs);
190+
191+
if (isLhsNumeric && !isRhsNumeric) {
192+
// Only lhs is numeric
182193
return -1;
183-
}
184-
if (p1.length > p2.length) {
194+
} else if (!isLhsNumeric && isRhsNumeric) {
195+
// Only rhs is numeric
185196
return 1;
197+
} else if (isLhsNumeric && isRhsNumeric) {
198+
// both numeric
199+
return BasePath.extractNumericId(lhs).compare(
200+
BasePath.extractNumericId(rhs)
201+
);
202+
} else {
203+
// both non-numeric
204+
if (lhs < rhs) {
205+
return -1;
206+
}
207+
if (lhs > rhs) {
208+
return 1;
209+
}
210+
return 0;
186211
}
187-
return 0;
212+
}
213+
214+
// Checks if a segment is a numeric ID (starts with "__id" and ends with "__").
215+
private static isNumericId(segment: string): boolean {
216+
return segment.startsWith('__id') && segment.endsWith('__');
217+
}
218+
219+
private static extractNumericId(segment: string): Integer {
220+
return Integer.fromString(segment.substring(4, segment.length - 2));
188221
}
189222
}
190223

‎packages/firestore/test/integration/api/database.test.ts

Lines changed: 180 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,8 @@ import {
7979
withTestDocAndInitialData,
8080
withNamedTestDbsOrSkipUnlessUsingEmulator,
8181
toDataArray,
82-
checkOnlineAndOfflineResultsMatch
82+
checkOnlineAndOfflineResultsMatch,
83+
toIds
8384
} from '../util/helpers';
8485
import { DEFAULT_SETTINGS, DEFAULT_PROJECT_ID } from '../util/settings';
8586

@@ -2245,4 +2246,182 @@ apiDescribe('Database', persistence => {
22452246
});
22462247
});
22472248
});
2249+
2250+
describe('sort documents by DocumentId', () => {
2251+
it('snapshot listener sorts query by DocumentId same way as get query', async () => {
2252+
const testDocs = {
2253+
'A': { a: 1 },
2254+
'a': { a: 1 },
2255+
'Aa': { a: 1 },
2256+
'7': { a: 1 },
2257+
'12': { a: 1 },
2258+
'__id7__': { a: 1 },
2259+
'__id12__': { a: 1 },
2260+
'__id-2__': { a: 1 },
2261+
'_id1__': { a: 1 },
2262+
'__id1_': { a: 1 },
2263+
'__id': { a: 1 },
2264+
// largest long numbers
2265+
'__id9223372036854775807__': { a: 1 },
2266+
'__id9223372036854775806__': { a: 1 },
2267+
// smallest long numbers
2268+
'__id-9223372036854775808__': { a: 1 },
2269+
'__id-9223372036854775807__': { a: 1 }
2270+
};
2271+
2272+
return withTestCollection(persistence, testDocs, async collectionRef => {
2273+
const orderedQuery = query(collectionRef, orderBy(documentId()));
2274+
const expectedDocs = [
2275+
'__id-9223372036854775808__',
2276+
'__id-9223372036854775807__',
2277+
'__id-2__',
2278+
'__id7__',
2279+
'__id12__',
2280+
'__id9223372036854775806__',
2281+
'__id9223372036854775807__',
2282+
'12',
2283+
'7',
2284+
'A',
2285+
'Aa',
2286+
'__id',
2287+
'__id1_',
2288+
'_id1__',
2289+
'a'
2290+
];
2291+
2292+
const getSnapshot = await getDocsFromServer(orderedQuery);
2293+
expect(toIds(getSnapshot)).to.deep.equal(expectedDocs);
2294+
2295+
const storeEvent = new EventsAccumulator<QuerySnapshot>();
2296+
const unsubscribe = onSnapshot(orderedQuery, storeEvent.storeEvent);
2297+
const watchSnapshot = await storeEvent.awaitEvent();
2298+
expect(toIds(watchSnapshot)).to.deep.equal(expectedDocs);
2299+
2300+
unsubscribe();
2301+
});
2302+
});
2303+
2304+
it('snapshot listener sorts filtered query by DocumentId same way as get query', async () => {
2305+
const testDocs = {
2306+
'A': { a: 1 },
2307+
'a': { a: 1 },
2308+
'Aa': { a: 1 },
2309+
'7': { a: 1 },
2310+
'12': { a: 1 },
2311+
'__id7__': { a: 1 },
2312+
'__id12__': { a: 1 },
2313+
'__id-2__': { a: 1 },
2314+
'_id1__': { a: 1 },
2315+
'__id1_': { a: 1 },
2316+
'__id': { a: 1 },
2317+
// largest long numbers
2318+
'__id9223372036854775807__': { a: 1 },
2319+
'__id9223372036854775806__': { a: 1 },
2320+
// smallest long numbers
2321+
'__id-9223372036854775808__': { a: 1 },
2322+
'__id-9223372036854775807__': { a: 1 }
2323+
};
2324+
2325+
return withTestCollection(persistence, testDocs, async collectionRef => {
2326+
const filteredQuery = query(
2327+
collectionRef,
2328+
orderBy(documentId()),
2329+
where(documentId(), '>', '__id7__'),
2330+
where(documentId(), '<=', 'Aa')
2331+
);
2332+
const expectedDocs = [
2333+
'__id12__',
2334+
'__id9223372036854775806__',
2335+
'__id9223372036854775807__',
2336+
'12',
2337+
'7',
2338+
'A',
2339+
'Aa'
2340+
];
2341+
2342+
const getSnapshot = await getDocsFromServer(filteredQuery);
2343+
expect(toIds(getSnapshot)).to.deep.equal(expectedDocs);
2344+
2345+
const storeEvent = new EventsAccumulator<QuerySnapshot>();
2346+
const unsubscribe = onSnapshot(filteredQuery, storeEvent.storeEvent);
2347+
const watchSnapshot = await storeEvent.awaitEvent();
2348+
expect(toIds(watchSnapshot)).to.deep.equal(expectedDocs);
2349+
unsubscribe();
2350+
});
2351+
});
2352+
2353+
// eslint-disable-next-line no-restricted-properties
2354+
(persistence.gc === 'lru' ? describe : describe.skip)('offline', () => {
2355+
it('SDK orders query the same way online and offline', async () => {
2356+
const testDocs = {
2357+
'A': { a: 1 },
2358+
'a': { a: 1 },
2359+
'Aa': { a: 1 },
2360+
'7': { a: 1 },
2361+
'12': { a: 1 },
2362+
'__id7__': { a: 1 },
2363+
'__id12__': { a: 1 },
2364+
'__id-2__': { a: 1 },
2365+
'_id1__': { a: 1 },
2366+
'__id1_': { a: 1 },
2367+
'__id': { a: 1 },
2368+
// largest long numbers
2369+
'__id9223372036854775807__': { a: 1 },
2370+
'__id9223372036854775806__': { a: 1 },
2371+
// smallest long numbers
2372+
'__id-9223372036854775808__': { a: 1 },
2373+
'__id-9223372036854775807__': { a: 1 }
2374+
};
2375+
2376+
return withTestCollection(
2377+
persistence,
2378+
testDocs,
2379+
async collectionRef => {
2380+
const orderedQuery = query(collectionRef, orderBy(documentId()));
2381+
let expectedDocs = [
2382+
'__id-9223372036854775808__',
2383+
'__id-9223372036854775807__',
2384+
'__id-2__',
2385+
'__id7__',
2386+
'__id12__',
2387+
'__id9223372036854775806__',
2388+
'__id9223372036854775807__',
2389+
'12',
2390+
'7',
2391+
'A',
2392+
'Aa',
2393+
'__id',
2394+
'__id1_',
2395+
'_id1__',
2396+
'a'
2397+
];
2398+
await checkOnlineAndOfflineResultsMatch(
2399+
orderedQuery,
2400+
...expectedDocs
2401+
);
2402+
2403+
const filteredQuery = query(
2404+
collectionRef,
2405+
orderBy(documentId()),
2406+
where(documentId(), '>', '__id7__'),
2407+
where(documentId(), '<=', 'Aa')
2408+
);
2409+
expectedDocs = [
2410+
'__id12__',
2411+
'__id9223372036854775806__',
2412+
'__id9223372036854775807__',
2413+
'12',
2414+
'7',
2415+
'A',
2416+
'Aa'
2417+
];
2418+
await checkOnlineAndOfflineResultsMatch(
2419+
filteredQuery,
2420+
...expectedDocs
2421+
);
2422+
}
2423+
);
2424+
});
2425+
});
2426+
});
22482427
});

0 commit comments

Comments
 (0)