FIX: sort strings in UTF-8 encoded byte order (#2275)

Author: milaGGL

dev/src/order.ts

@@ -214,7 +214,7 @@ function compareObjects(left: ApiMapValue, right: ApiMapValue): number {
   leftKeys.sort();
   rightKeys.sort();
   for (let i = 0; i < leftKeys.length && i < rightKeys.length; i++) {
-    const keyComparison = primitiveComparator(leftKeys[i], rightKeys[i]);
+    const keyComparison = compareUtf8Strings(leftKeys[i], rightKeys[i]);
     if (keyComparison !== 0) {
       return keyComparison;
     }
@@ -248,6 +248,21 @@ function compareVectors(left: ApiMapValue, right: ApiMapValue): number {
   return compareArrays(leftArray, rightArray);
 }
 
+function stringToUtf8Bytes(str: string): Uint8Array {
+  return new TextEncoder().encode(str);
+}
+
+/*!
+ * Compare strings in UTF-8 encoded byte order
+ * @private
+ * @internal
+ */
+export function compareUtf8Strings(left: string, right: string): number {
+  const leftBytes = stringToUtf8Bytes(left);
+  const rightBytes = stringToUtf8Bytes(right);
+  return compareBlobs(Buffer.from(leftBytes), Buffer.from(rightBytes));
+}
+
 /*!
  * @private
  * @internal
@@ -269,7 +284,7 @@ export function compare(left: api.IValue, right: api.IValue): number {
     case TypeOrder.BOOLEAN:
       return primitiveComparator(left.booleanValue!, right.booleanValue!);
     case TypeOrder.STRING:
-      return primitiveComparator(left.stringValue!, right.stringValue!);
+      return compareUtf8Strings(left.stringValue!, right.stringValue!);
     case TypeOrder.NUMBER:
       return compareNumberProtos(left, right);
     case TypeOrder.TIMESTAMP:

dev/src/path.ts

@@ -17,6 +17,7 @@
 import * as firestore from '@google-cloud/firestore';
 
 import {google} from '../protos/firestore_v1_proto_api';
+import {compareUtf8Strings, primitiveComparator} from './order';
 
 import {isObject} from './util';
 import {
@@ -170,7 +171,7 @@ abstract class Path<T> {
         return comparison;
       }
     }
-    return Math.sign(this.segments.length - other.segments.length);
+    return primitiveComparator(this.segments.length, other.segments.length);
   }
 
   private compareSegments(lhs: string, rhs: string): number {
@@ -191,7 +192,7 @@ abstract class Path<T> {
       );
     } else {
       // both non-numeric
-      return this.compareStrings(lhs, rhs);
+      return compareUtf8Strings(lhs, rhs);
     }
   }
 
@@ -215,16 +216,6 @@ abstract class Path<T> {
     }
   }
 
-  private compareStrings(lhs: string, rhs: string): number {
-    if (lhs < rhs) {
-      return -1;
-    } else if (lhs > rhs) {
-      return 1;
-    } else {
-      return 0;
-    }
-  }
-
   /**
    * Returns a copy of the underlying segments.
    *

dev/system-test/firestore.ts

@@ -4084,6 +4084,156 @@ describe('Query class', () => {
         docChanges: expectedChanges,
       });
     });
+
+    describe('sort unicode strings', () => {
+      it('snapshot listener sorts unicode strings same as server', async () => {
+        const collection = await testCollectionWithDocs({
+          a: {value: 'Łukasiewicz'},
+          b: {value: 'Sierpiński'},
+          c: {value: '岩澤'},
+          d: {value: '🄟'},
+          e: {value: 'Ｐ'},
+          f: {value: '︒'},
+          g: {value: '🐵'},
+        });
+
+        const query = collection.orderBy('value');
+        const expectedDocs = ['b', 'a', 'c', 'f', 'e', 'd', 'g'];
+
+        const getSnapshot = await query.get();
+        expect(getSnapshot.docs.map(d => d.id)).to.deep.equal(expectedDocs);
+
+        const unsubscribe = query.onSnapshot(snapshot =>
+          currentDeferred.resolve(snapshot)
+        );
+        const watchSnapshot = await waitForSnapshot();
+        snapshotsEqual(watchSnapshot, {
+          docs: getSnapshot.docs,
+          docChanges: getSnapshot.docChanges(),
+        });
+        unsubscribe();
+      });
+
+      it('snapshot listener sorts unicode strings in array same as server', async () => {
+        const collection = await testCollectionWithDocs({
+          a: {value: ['Łukasiewicz']},
+          b: {value: ['Sierpiński']},
+          c: {value: ['岩澤']},
+          d: {value: ['🄟']},
+          e: {value: ['Ｐ']},
+          f: {value: ['︒']},
+          g: {value: ['🐵']},
+        });
+
+        const query = collection.orderBy('value');
+        const expectedDocs = ['b', 'a', 'c', 'f', 'e', 'd', 'g'];
+
+        const getSnapshot = await query.get();
+        expect(getSnapshot.docs.map(d => d.id)).to.deep.equal(expectedDocs);
+
+        const unsubscribe = query.onSnapshot(snapshot =>
+          currentDeferred.resolve(snapshot)
+        );
+        const watchSnapshot = await waitForSnapshot();
+        snapshotsEqual(watchSnapshot, {
+          docs: getSnapshot.docs,
+          docChanges: getSnapshot.docChanges(),
+        });
+        unsubscribe();
+      });
+
+      it('snapshot listener sorts unicode strings in map same as server', async () => {
+        const collection = await testCollectionWithDocs({
+          a: {value: {foo: 'Łukasiewicz'}},
+          b: {value: {foo: 'Sierpiński'}},
+          c: {value: {foo: '岩澤'}},
+          d: {value: {foo: '🄟'}},
+          e: {value: {foo: 'Ｐ'}},
+          f: {value: {foo: '︒'}},
+          g: {value: {foo: '🐵'}},
+        });
+
+        const query = collection.orderBy('value');
+        const expectedDocs = ['b', 'a', 'c', 'f', 'e', 'd', 'g'];
+
+        const getSnapshot = await query.get();
+        expect(getSnapshot.docs.map(d => d.id)).to.deep.equal(expectedDocs);
+
+        const unsubscribe = query.onSnapshot(snapshot =>
+          currentDeferred.resolve(snapshot)
+        );
+        const watchSnapshot = await waitForSnapshot();
+        snapshotsEqual(watchSnapshot, {
+          docs: getSnapshot.docs,
+          docChanges: getSnapshot.docChanges(),
+        });
+        unsubscribe();
+      });
+
+      it('snapshot listener sorts unicode strings in map key same as server', async () => {
+        const collection = await testCollectionWithDocs({
+          a: {value: {Łukasiewicz: true}},
+          b: {value: {Sierpiński: true}},
+          c: {value: {岩澤: true}},
+          d: {value: {'🄟': true}},
+          e: {value: {Ｐ: true}},
+          f: {value: {'︒': true}},
+          g: {value: {'🐵': true}},
+        });
+
+        const query = collection.orderBy('value');
+        const expectedDocs = ['b', 'a', 'c', 'f', 'e', 'd', 'g'];
+
+        const getSnapshot = await query.get();
+        expect(getSnapshot.docs.map(d => d.id)).to.deep.equal(expectedDocs);
+
+        const unsubscribe = query.onSnapshot(snapshot =>
+          currentDeferred.resolve(snapshot)
+        );
+        const watchSnapshot = await waitForSnapshot();
+        snapshotsEqual(watchSnapshot, {
+          docs: getSnapshot.docs,
+          docChanges: getSnapshot.docChanges(),
+        });
+        unsubscribe();
+      });
+
+      it('snapshot listener sorts unicode strings in document key same as server', async () => {
+        const collection = await testCollectionWithDocs({
+          Łukasiewicz: {value: true},
+          Sierpiński: {value: true},
+          岩澤: {value: true},
+          '🄟': {value: true},
+          Ｐ: {value: true},
+          '︒': {value: true},
+          '🐵': {value: true},
+        });
+
+        const query = collection.orderBy(FieldPath.documentId());
+        const expectedDocs = [
+          'Sierpiński',
+          'Łukasiewicz',
+          '岩澤',
+          '︒',
+          'Ｐ',
+          '🄟',
+          '🐵',
+        ];
+
+        const getSnapshot = await query.get();
+        expect(getSnapshot.docs.map(d => d.id)).to.deep.equal(expectedDocs);
+
+        const unsubscribe = query.onSnapshot(snapshot =>
+          currentDeferred.resolve(snapshot)
+        );
+        const watchSnapshot = await waitForSnapshot();
+        snapshotsEqual(watchSnapshot, {
+          docs: getSnapshot.docs,
+          docChanges: getSnapshot.docChanges(),
+        });
+        unsubscribe();
+      });
+    });
   });
 
   (process.env.FIRESTORE_EMULATOR_HOST === undefined