Skip to content

Commit 7de7220

Browse files
milaGGLmilaGGL
committed
add comments
1 parent 65981c3 commit 7de7220

File tree

6 files changed

+97
-52
lines changed

6 files changed

+97
-52
lines changed

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -655,5 +655,9 @@ export function dbKeyComparator(l: DocumentKey, r: DocumentKey): number {
655655
return cmp;
656656
}
657657

658+
// TODO(b/329441702): Document IDs should be sorted by UTF-8 encoded byte
659+
// order, but IndexedDB sorts strings lexicographically. Document ID
660+
// comparison here still relies on primitive comparison to avoid mismatches
661+
// observed in snapshot listeners with Unicode characters in documentIds
658662
return primitiveComparator(left[left.length - 1], right[right.length - 1]);
659663
}

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

Lines changed: 0 additions & 39 deletions
This file was deleted.

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

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,7 @@ import { Integer } from '@firebase/webchannel-wrapper/bloom-blob';
1919

2020
import { debugAssert, fail } from '../util/assert';
2121
import { Code, FirestoreError } from '../util/error';
22-
import { primitiveComparator } from '../util/misc';
23-
24-
import { compareUtf8Strings } from './byteComparator';
22+
import { primitiveComparator, compareUtf8Strings } from '../util/misc';
2523

2624
export const DOCUMENT_KEY_NAME = '__name__';
2725

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

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,11 +25,14 @@ import {
2525
Value
2626
} from '../protos/firestore_proto_api';
2727
import { fail } from '../util/assert';
28-
import { arrayEquals, primitiveComparator } from '../util/misc';
28+
import {
29+
arrayEquals,
30+
primitiveComparator,
31+
compareUtf8Strings
32+
} from '../util/misc';
2933
import { forEach, objectSize } from '../util/obj';
3034
import { isNegativeZero } from '../util/types';
3135

32-
import { compareBlobs, compareUtf8Strings } from './byteComparator';
3336
import { DocumentKey } from './document_key';
3437
import {
3538
normalizeByteString,
@@ -338,6 +341,15 @@ function compareGeoPoints(left: LatLng, right: LatLng): number {
338341
);
339342
}
340343

344+
function compareBlobs(
345+
left: string | Uint8Array,
346+
right: string | Uint8Array
347+
): number {
348+
const leftBytes = normalizeByteString(left);
349+
const rightBytes = normalizeByteString(right);
350+
return leftBytes.compareTo(rightBytes);
351+
}
352+
341353
function compareArrays(left: ArrayValue, right: ArrayValue): number {
342354
const leftArray = left.values || [];
343355
const rightArray = right.values || [];

‎packages/firestore/src/util/misc.ts‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,22 @@ export interface Equatable<T> {
7474
isEqual(other: T): boolean;
7575
}
7676

77+
/** Compare strings in UTF-8 encoded byte order */
78+
export function compareUtf8Strings(left: string, right: string): number {
79+
// Convert the string to UTF-8 encoded bytes
80+
const encodedLeft = new TextEncoder().encode(left);
81+
const encodedRight = new TextEncoder().encode(right);
82+
83+
for (let i = 0; i < Math.min(encodedLeft.length, encodedRight.length); i++) {
84+
const comparison = primitiveComparator(encodedLeft[i], encodedRight[i]);
85+
if (comparison !== 0) {
86+
return comparison;
87+
}
88+
}
89+
90+
return primitiveComparator(encodedLeft.length, encodedRight.length);
91+
}
92+
7793
export interface Iterable<V> {
7894
forEach: (cb: (v: V) => void) => void;
7995
}

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

Lines changed: 62 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2425,8 +2425,8 @@ apiDescribe('Database', persistence => {
24252425
});
24262426
});
24272427

2428-
describe('Unicode strings', () => {
2429-
const expectedResult = ['b', 'a', 'c', 'f', 'e', 'd', 'g'];
2428+
describe('Sort unicode strings', () => {
2429+
const expectedDocs = ['b', 'a', 'c', 'f', 'e', 'd', 'g'];
24302430
it('snapshot listener sorts unicode strings the same as server', async () => {
24312431
const testDocs = {
24322432
'a': { value: 'Łukasiewicz' },
@@ -2442,14 +2442,16 @@ apiDescribe('Database', persistence => {
24422442
const orderedQuery = query(collectionRef, orderBy('value'));
24432443

24442444
const getSnapshot = await getDocsFromServer(orderedQuery);
2445-
expect(toIds(getSnapshot)).to.deep.equal(expectedResult);
2445+
expect(toIds(getSnapshot)).to.deep.equal(expectedDocs);
24462446

24472447
const storeEvent = new EventsAccumulator<QuerySnapshot>();
24482448
const unsubscribe = onSnapshot(orderedQuery, storeEvent.storeEvent);
24492449
const watchSnapshot = await storeEvent.awaitEvent();
24502450
expect(toIds(watchSnapshot)).to.deep.equal(toIds(getSnapshot));
24512451

24522452
unsubscribe();
2453+
2454+
await checkOnlineAndOfflineResultsMatch(orderedQuery, ...expectedDocs);
24532455
});
24542456
});
24552457

@@ -2468,14 +2470,16 @@ apiDescribe('Database', persistence => {
24682470
const orderedQuery = query(collectionRef, orderBy('value'));
24692471

24702472
const getSnapshot = await getDocsFromServer(orderedQuery);
2471-
expect(toIds(getSnapshot)).to.deep.equal(expectedResult);
2473+
expect(toIds(getSnapshot)).to.deep.equal(expectedDocs);
24722474

24732475
const storeEvent = new EventsAccumulator<QuerySnapshot>();
24742476
const unsubscribe = onSnapshot(orderedQuery, storeEvent.storeEvent);
24752477
const watchSnapshot = await storeEvent.awaitEvent();
24762478
expect(toIds(watchSnapshot)).to.deep.equal(toIds(getSnapshot));
24772479

24782480
unsubscribe();
2481+
2482+
await checkOnlineAndOfflineResultsMatch(orderedQuery, ...expectedDocs);
24792483
});
24802484
});
24812485

@@ -2494,14 +2498,16 @@ apiDescribe('Database', persistence => {
24942498
const orderedQuery = query(collectionRef, orderBy('value'));
24952499

24962500
const getSnapshot = await getDocsFromServer(orderedQuery);
2497-
expect(toIds(getSnapshot)).to.deep.equal(expectedResult);
2501+
expect(toIds(getSnapshot)).to.deep.equal(expectedDocs);
24982502

24992503
const storeEvent = new EventsAccumulator<QuerySnapshot>();
25002504
const unsubscribe = onSnapshot(orderedQuery, storeEvent.storeEvent);
25012505
const watchSnapshot = await storeEvent.awaitEvent();
25022506
expect(toIds(watchSnapshot)).to.deep.equal(toIds(getSnapshot));
25032507

25042508
unsubscribe();
2509+
2510+
await checkOnlineAndOfflineResultsMatch(orderedQuery, ...expectedDocs);
25052511
});
25062512
});
25072513

@@ -2520,14 +2526,16 @@ apiDescribe('Database', persistence => {
25202526
const orderedQuery = query(collectionRef, orderBy('value'));
25212527

25222528
const getSnapshot = await getDocsFromServer(orderedQuery);
2523-
expect(toIds(getSnapshot)).to.deep.equal(expectedResult);
2529+
expect(toIds(getSnapshot)).to.deep.equal(expectedDocs);
25242530

25252531
const storeEvent = new EventsAccumulator<QuerySnapshot>();
25262532
const unsubscribe = onSnapshot(orderedQuery, storeEvent.storeEvent);
25272533
const watchSnapshot = await storeEvent.awaitEvent();
25282534
expect(toIds(watchSnapshot)).to.deep.equal(toIds(getSnapshot));
25292535

25302536
unsubscribe();
2537+
2538+
await checkOnlineAndOfflineResultsMatch(orderedQuery, ...expectedDocs);
25312539
});
25322540
});
25332541

@@ -2546,23 +2554,69 @@ apiDescribe('Database', persistence => {
25462554
const orderedQuery = query(collectionRef, orderBy(documentId()));
25472555

25482556
const getSnapshot = await getDocsFromServer(orderedQuery);
2549-
expect(toIds(getSnapshot)).to.deep.equal([
2557+
const expectedDocs = [
25502558
'Sierpiński',
25512559
'Łukasiewicz',
25522560
'岩澤',
25532561
'︒',
25542562
'Ｐ',
25552563
'🄟',
25562564
'🐵'
2557-
]);
2565+
];
2566+
expect(toIds(getSnapshot)).to.deep.equal(expectedDocs);
25582567

25592568
const storeEvent = new EventsAccumulator<QuerySnapshot>();
25602569
const unsubscribe = onSnapshot(orderedQuery, storeEvent.storeEvent);
25612570
const watchSnapshot = await storeEvent.awaitEvent();
25622571
expect(toIds(watchSnapshot)).to.deep.equal(toIds(getSnapshot));
25632572

25642573
unsubscribe();
2574+
2575+
await checkOnlineAndOfflineResultsMatch(orderedQuery, ...expectedDocs);
25652576
});
25662577
});
2578+
2579+
// eslint-disable-next-line no-restricted-properties
2580+
(persistence.storage === 'indexeddb' ? it.skip : it)(
2581+
'snapshot listener sorts unicode strings in document key the same as server with persistence',
2582+
async () => {
2583+
const testDocs = {
2584+
'Łukasiewicz': { value: true },
2585+
'Sierpiński': { value: true },
2586+
'岩澤': { value: true },
2587+
'🄟': { value: true },
2588+
'Ｐ': { value: true },
2589+
'︒': { value: true },
2590+
'🐵': { value: true }
2591+
};
2592+
2593+
return withTestCollection(
2594+
persistence,
2595+
testDocs,
2596+
async collectionRef => {
2597+
const orderedQuery = query(collectionRef, orderBy('value'));
2598+
2599+
const getSnapshot = await getDocsFromServer(orderedQuery);
2600+
expect(toIds(getSnapshot)).to.deep.equal([
2601+
'Sierpiński',
2602+
'Łukasiewicz',
2603+
'岩澤',
2604+
'︒',
2605+
'Ｐ',
2606+
'🄟',
2607+
'🐵'
2608+
]);
2609+
2610+
const storeEvent = new EventsAccumulator<QuerySnapshot>();
2611+
const unsubscribe = onSnapshot(orderedQuery, storeEvent.storeEvent);
2612+
const watchSnapshot = await storeEvent.awaitEvent();
2613+
// TODO: IndexedDB sorts string lexicographically, and misses the document with ID '🄟','🐵'
2614+
expect(toIds(watchSnapshot)).to.deep.equal(toIds(getSnapshot));
2615+
2616+
unsubscribe();
2617+
}
2618+
);
2619+
}
2620+
);
25672621
});
25682622
});

0 commit comments

Comments
 (0)