FIX: sort strings in UTF-8 encoded byte order

docs-devsite/index.md

@@ -28,4 +28,3 @@ https://github.com/firebase/firebase-js-sdk
 |  [@firebase/remote-config](./remote-config.md#remote-config_package) | The Firebase Remote Config Web SDK. This SDK does not work in a Node.js environment. |
 |  [@firebase/storage](./storage.md#storage_package) | Cloud Storage for Firebase |
 |  [@firebase/vertexai](./vertexai.md#vertexai_package) | The Vertex AI in Firebase Web SDK. |
-

packages/firestore/src/local/indexeddb_remote_document_cache.ts

@@ -655,5 +655,9 @@ export function dbKeyComparator(l: DocumentKey, r: DocumentKey): number {
     return cmp;
   }
 
+  // TODO(b/329441702): Document IDs should be sorted by UTF-8 encoded byte
+  // order, but IndexedDB sorts strings lexicographically. Document ID
+  // comparison here still relies on primitive comparison to avoid mismatches
+  // observed in snapshot listeners with Unicode characters in documentIds
   return primitiveComparator(left[left.length - 1], right[right.length - 1]);
 }

packages/firestore/src/model/path.ts

@@ -19,6 +19,7 @@ import { Integer } from '@firebase/webchannel-wrapper/bloom-blob';
 
 import { debugAssert, fail } from '../util/assert';
 import { Code, FirestoreError } from '../util/error';
+import { primitiveComparator, compareUtf8Strings } from '../util/misc';
 
 export const DOCUMENT_KEY_NAME = '__name__';
 
@@ -181,7 +182,7 @@ abstract class BasePath<B extends BasePath<B>> {
         return comparison;
       }
     }
-    return Math.sign(p1.length - p2.length);
+    return primitiveComparator(p1.length, p2.length);
   }
 
   private static compareSegments(lhs: string, rhs: string): number {
@@ -201,13 +202,7 @@ abstract class BasePath<B extends BasePath<B>> {
       );
     } else {
       // both non-numeric
-      if (lhs < rhs) {
-        return -1;
-      }
-      if (lhs > rhs) {
-        return 1;
-      }
-      return 0;
+      return compareUtf8Strings(lhs, rhs);
     }
   }
 

packages/firestore/src/model/values.ts

@@ -25,7 +25,11 @@ import {
   Value
 } from '../protos/firestore_proto_api';
 import { fail } from '../util/assert';
-import { arrayEquals, primitiveComparator } from '../util/misc';
+import {
+  arrayEquals,
+  primitiveComparator,
+  compareUtf8Strings
+} from '../util/misc';
 import { forEach, objectSize } from '../util/obj';
 import { isNegativeZero } from '../util/types';
 
@@ -251,7 +255,7 @@ export function valueCompare(left: Value, right: Value): number {
         getLocalWriteTime(right)
       );
     case TypeOrder.StringValue:
-      return primitiveComparator(left.stringValue!, right.stringValue!);
+      return compareUtf8Strings(left.stringValue!, right.stringValue!);
     case TypeOrder.BlobValue:
       return compareBlobs(left.bytesValue!, right.bytesValue!);
     case TypeOrder.RefValue:
@@ -400,7 +404,7 @@ function compareMaps(left: MapValue, right: MapValue): number {
   rightKeys.sort();
 
   for (let i = 0; i < leftKeys.length && i < rightKeys.length; ++i) {
-    const keyCompare = primitiveComparator(leftKeys[i], rightKeys[i]);
+    const keyCompare = compareUtf8Strings(leftKeys[i], rightKeys[i]);
     if (keyCompare !== 0) {
       return keyCompare;
     }

packages/firestore/src/util/misc.ts

@@ -74,6 +74,22 @@ export interface Equatable<T> {
   isEqual(other: T): boolean;
 }
 
+/** Compare strings in UTF-8 encoded byte order */
+export function compareUtf8Strings(left: string, right: string): number {
+  // Convert the string to UTF-8 encoded bytes
+  const encodedLeft = new TextEncoder().encode(left);
+  const encodedRight = new TextEncoder().encode(right);
+
+  for (let i = 0; i < Math.min(encodedLeft.length, encodedRight.length); i++) {
+    const comparison = primitiveComparator(encodedLeft[i], encodedRight[i]);
+    if (comparison !== 0) {
+      return comparison;
+    }
+  }
+
+  return primitiveComparator(encodedLeft.length, encodedRight.length);
+}
+
 export interface Iterable<V> {
   forEach: (cb: (v: V) => void) => void;
 }

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

@@ -2424,4 +2424,199 @@ apiDescribe('Database', persistence => {
       });
     });
   });
+
+  describe('Sort unicode strings', () => {
+    const expectedDocs = ['b', 'a', 'c', 'f', 'e', 'd', 'g'];
+    it('snapshot listener sorts unicode strings the same as server', async () => {
+      const testDocs = {
+        'a': { value: 'Łukasiewicz' },
+        'b': { value: 'Sierpiński' },
+        'c': { value: '岩澤' },
+        'd': { value: '🄟' },
+        'e': { value: 'Ｐ' },
+        'f': { value: '︒' },
+        'g': { value: '🐵' }
+      };
+
+      return withTestCollection(persistence, testDocs, async collectionRef => {
+        const orderedQuery = query(collectionRef, orderBy('value'));
+
+        const getSnapshot = await getDocsFromServer(orderedQuery);
+        expect(toIds(getSnapshot)).to.deep.equal(expectedDocs);
+
+        const storeEvent = new EventsAccumulator<QuerySnapshot>();
+        const unsubscribe = onSnapshot(orderedQuery, storeEvent.storeEvent);
+        const watchSnapshot = await storeEvent.awaitEvent();
+        expect(toIds(watchSnapshot)).to.deep.equal(toIds(getSnapshot));
+
+        unsubscribe();
+
+        await checkOnlineAndOfflineResultsMatch(orderedQuery, ...expectedDocs);
+      });
+    });
+
+    it('snapshot listener sorts unicode strings in array the same  as server', async () => {
+      const testDocs = {
+        'a': { value: ['Łukasiewicz'] },
+        'b': { value: ['Sierpiński'] },
+        'c': { value: ['岩澤'] },
+        'd': { value: ['🄟'] },
+        'e': { value: ['Ｐ'] },
+        'f': { value: ['︒'] },
+        'g': { value: ['🐵'] }
+      };
+
+      return withTestCollection(persistence, testDocs, async collectionRef => {
+        const orderedQuery = query(collectionRef, orderBy('value'));
+
+        const getSnapshot = await getDocsFromServer(orderedQuery);
+        expect(toIds(getSnapshot)).to.deep.equal(expectedDocs);
+
+        const storeEvent = new EventsAccumulator<QuerySnapshot>();
+        const unsubscribe = onSnapshot(orderedQuery, storeEvent.storeEvent);
+        const watchSnapshot = await storeEvent.awaitEvent();
+        expect(toIds(watchSnapshot)).to.deep.equal(toIds(getSnapshot));
+
+        unsubscribe();
+
+        await checkOnlineAndOfflineResultsMatch(orderedQuery, ...expectedDocs);
+      });
+    });
+
+    it('snapshot listener sorts unicode strings in map the same as server', async () => {
+      const testDocs = {
+        'a': { value: { foo: 'Łukasiewicz' } },
+        'b': { value: { foo: 'Sierpiński' } },
+        'c': { value: { foo: '岩澤' } },
+        'd': { value: { foo: '🄟' } },
+        'e': { value: { foo: 'Ｐ' } },
+        'f': { value: { foo: '︒' } },
+        'g': { value: { foo: '🐵' } }
+      };
+
+      return withTestCollection(persistence, testDocs, async collectionRef => {
+        const orderedQuery = query(collectionRef, orderBy('value'));
+
+        const getSnapshot = await getDocsFromServer(orderedQuery);
+        expect(toIds(getSnapshot)).to.deep.equal(expectedDocs);
+
+        const storeEvent = new EventsAccumulator<QuerySnapshot>();
+        const unsubscribe = onSnapshot(orderedQuery, storeEvent.storeEvent);
+        const watchSnapshot = await storeEvent.awaitEvent();
+        expect(toIds(watchSnapshot)).to.deep.equal(toIds(getSnapshot));
+
+        unsubscribe();
+
+        await checkOnlineAndOfflineResultsMatch(orderedQuery, ...expectedDocs);
+      });
+    });
+
+    it('snapshot listener sorts unicode strings in map key the same as server', async () => {
+      const testDocs = {
+        'a': { value: { 'Łukasiewicz': true } },
+        'b': { value: { 'Sierpiński': true } },
+        'c': { value: { '岩澤': true } },
+        'd': { value: { '🄟': true } },
+        'e': { value: { 'Ｐ': true } },
+        'f': { value: { '︒': true } },
+        'g': { value: { '🐵': true } }
+      };
+
+      return withTestCollection(persistence, testDocs, async collectionRef => {
+        const orderedQuery = query(collectionRef, orderBy('value'));
+
+        const getSnapshot = await getDocsFromServer(orderedQuery);
+        expect(toIds(getSnapshot)).to.deep.equal(expectedDocs);
+
+        const storeEvent = new EventsAccumulator<QuerySnapshot>();
+        const unsubscribe = onSnapshot(orderedQuery, storeEvent.storeEvent);
+        const watchSnapshot = await storeEvent.awaitEvent();
+        expect(toIds(watchSnapshot)).to.deep.equal(toIds(getSnapshot));
+
+        unsubscribe();
+
+        await checkOnlineAndOfflineResultsMatch(orderedQuery, ...expectedDocs);
+      });
+    });
+
+    it('snapshot listener sorts unicode strings in document key the same as server', async () => {
+      const testDocs = {
+        'Łukasiewicz': { value: true },
+        'Sierpiński': { value: true },
+        '岩澤': { value: true },
+        '🄟': { value: true },
+        'Ｐ': { value: true },
+        '︒': { value: true },
+        '🐵': { value: true }
+      };
+
+      return withTestCollection(persistence, testDocs, async collectionRef => {
+        const orderedQuery = query(collectionRef, orderBy(documentId()));
+
+        const getSnapshot = await getDocsFromServer(orderedQuery);
+        const expectedDocs = [
+          'Sierpiński',
+          'Łukasiewicz',
+          '岩澤',
+          '︒',
+          'Ｐ',
+          '🄟',
+          '🐵'
+        ];
+        expect(toIds(getSnapshot)).to.deep.equal(expectedDocs);
+
+        const storeEvent = new EventsAccumulator<QuerySnapshot>();
+        const unsubscribe = onSnapshot(orderedQuery, storeEvent.storeEvent);
+        const watchSnapshot = await storeEvent.awaitEvent();
+        expect(toIds(watchSnapshot)).to.deep.equal(toIds(getSnapshot));
+
+        unsubscribe();
+
+        await checkOnlineAndOfflineResultsMatch(orderedQuery, ...expectedDocs);
+      });
+    });
+
+    // eslint-disable-next-line no-restricted-properties
+    (persistence.storage === 'indexeddb' ? it.skip : it)(
+      'snapshot listener sorts unicode strings in document key the same as server with persistence',
+      async () => {
+        const testDocs = {
+          'Łukasiewicz': { value: true },
+          'Sierpiński': { value: true },
+          '岩澤': { value: true },
+          '🄟': { value: true },
+          'Ｐ': { value: true },
+          '︒': { value: true },
+          '🐵': { value: true }
+        };
+
+        return withTestCollection(
+          persistence,
+          testDocs,
+          async collectionRef => {
+            const orderedQuery = query(collectionRef, orderBy('value'));
+
+            const getSnapshot = await getDocsFromServer(orderedQuery);
+            expect(toIds(getSnapshot)).to.deep.equal([
+              'Sierpiński',
+              'Łukasiewicz',
+              '岩澤',
+              '︒',
+              'Ｐ',
+              '🄟',
+              '🐵'
+            ]);
+
+            const storeEvent = new EventsAccumulator<QuerySnapshot>();
+            const unsubscribe = onSnapshot(orderedQuery, storeEvent.storeEvent);
+            const watchSnapshot = await storeEvent.awaitEvent();
+            // TODO: IndexedDB sorts string lexicographically, and misses the document with ID '🄟','🐵'
+            expect(toIds(watchSnapshot)).to.deep.equal(toIds(getSnapshot));
+
+            unsubscribe();
+          }
+        );
+      }
+    );
+  });
 });