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

firebase · Jan 15, 2025 · a1cbf93 · a1cbf93
1 parent b5dbd0a
commit a1cbf93
Show file tree

Hide file tree

Showing 5 changed files with 217 additions and 4 deletions.
diff --git a/firebase-firestore/CHANGELOG.md b/firebase-firestore/CHANGELOG.md
@@ -1,5 +1,5 @@
 # Unreleased
-
+* [fixed] Fixed a server and sdk mismatch in unicode string sorting. [#6615](//github.com/firebase/firebase-android-sdk/pull/6615)
 
 # 25.1.1
 * [changed] Update Firestore proto definitions. [#6369](//github.com/firebase/firebase-android-sdk/pull/6369)

diff --git a/firebase-firestore/src/androidTest/java/com/google/firebase/firestore/FirestoreTest.java b/firebase-firestore/src/androidTest/java/com/google/firebase/firestore/FirestoreTest.java
@@ -1653,4 +1653,210 @@ public void sdkOrdersQueryByDocumentIdTheSameWayOnlineAndOffline() {
     // Run query with snapshot listener
     checkOnlineAndOfflineResultsMatch(orderedQuery, expectedDocIds.toArray(new String[0]));
   }
+
+  @Test
+  public void snapshotListenerSortsUnicodeStringsAsServer() {
+    Map<String, Map<String, Object>> testDocs =
+        map(
+            "a", map("value", "Łukasiewicz"),
+            "b", map("value", "Sierpiński"),
+            "c", map("value", "岩澤"),
+            "d", map("value", "🄟"),
+            "e", map("value", "Ｐ"),
+            "f", map("value", "︒"),
+            "g", map("value", "🐵"));
+
+    CollectionReference colRef = testCollectionWithDocs(testDocs);
+    Query orderedQuery = colRef.orderBy("value");
+    List<String> expectedDocIds = Arrays.asList("b", "a", "c", "f", "e", "d", "g");
+
+    QuerySnapshot getSnapshot = waitFor(orderedQuery.get());
+    List<String> getSnapshotDocIds =
+        getSnapshot.getDocuments().stream().map(ds -> ds.getId()).collect(Collectors.toList());
+
+    EventAccumulator<QuerySnapshot> eventAccumulator = new EventAccumulator<QuerySnapshot>();
+    ListenerRegistration registration =
+        orderedQuery.addSnapshotListener(eventAccumulator.listener());
+
+    List<String> watchSnapshotDocIds = new ArrayList<>();
+    try {
+      QuerySnapshot watchSnapshot = eventAccumulator.await();
+      watchSnapshotDocIds =
+          watchSnapshot.getDocuments().stream()
+              .map(documentSnapshot -> documentSnapshot.getId())
+              .collect(Collectors.toList());
+    } finally {
+      registration.remove();
+    }
+
+    assertTrue(getSnapshotDocIds.equals(expectedDocIds));
+    assertTrue(watchSnapshotDocIds.equals(expectedDocIds));
+
+    checkOnlineAndOfflineResultsMatch(orderedQuery, expectedDocIds.toArray(new String[0]));
+  }
+
+  @Test
+  public void snapshotListenerSortsUnicodeStringsInArrayAsServer() {
+    Map<String, Map<String, Object>> testDocs =
+        map(
+            "a", map("value", Arrays.asList("Łukasiewicz")),
+            "b", map("value", Arrays.asList("Sierpiński")),
+            "c", map("value", Arrays.asList("岩澤")),
+            "d", map("value", Arrays.asList("🄟")),
+            "e", map("value", Arrays.asList("Ｐ")),
+            "f", map("value", Arrays.asList("︒")),
+            "g", map("value", Arrays.asList("🐵")));
+
+    CollectionReference colRef = testCollectionWithDocs(testDocs);
+    Query orderedQuery = colRef.orderBy("value");
+    List<String> expectedDocIds = Arrays.asList("b", "a", "c", "f", "e", "d", "g");
+
+    QuerySnapshot getSnapshot = waitFor(orderedQuery.get());
+    List<String> getSnapshotDocIds =
+        getSnapshot.getDocuments().stream().map(ds -> ds.getId()).collect(Collectors.toList());
+
+    EventAccumulator<QuerySnapshot> eventAccumulator = new EventAccumulator<QuerySnapshot>();
+    ListenerRegistration registration =
+        orderedQuery.addSnapshotListener(eventAccumulator.listener());
+
+    List<String> watchSnapshotDocIds = new ArrayList<>();
+    try {
+      QuerySnapshot watchSnapshot = eventAccumulator.await();
+      watchSnapshotDocIds =
+          watchSnapshot.getDocuments().stream()
+              .map(documentSnapshot -> documentSnapshot.getId())
+              .collect(Collectors.toList());
+    } finally {
+      registration.remove();
+    }
+
+    assertTrue(getSnapshotDocIds.equals(expectedDocIds));
+    assertTrue(watchSnapshotDocIds.equals(expectedDocIds));
+
+    checkOnlineAndOfflineResultsMatch(orderedQuery, expectedDocIds.toArray(new String[0]));
+  }
+
+  @Test
+  public void snapshotListenerSortsUnicodeStringsInMapAsServer() {
+    Map<String, Map<String, Object>> testDocs =
+        map(
+            "a", map("value", map("foo", "Łukasiewicz")),
+            "b", map("value", map("foo", "Sierpiński")),
+            "c", map("value", map("foo", "岩澤")),
+            "d", map("value", map("foo", "🄟")),
+            "e", map("value", map("foo", "Ｐ")),
+            "f", map("value", map("foo", "︒")),
+            "g", map("value", map("foo", "🐵")));
+
+    CollectionReference colRef = testCollectionWithDocs(testDocs);
+    Query orderedQuery = colRef.orderBy("value");
+    List<String> expectedDocIds = Arrays.asList("b", "a", "c", "f", "e", "d", "g");
+
+    QuerySnapshot getSnapshot = waitFor(orderedQuery.get());
+    List<String> getSnapshotDocIds =
+        getSnapshot.getDocuments().stream().map(ds -> ds.getId()).collect(Collectors.toList());
+
+    EventAccumulator<QuerySnapshot> eventAccumulator = new EventAccumulator<QuerySnapshot>();
+    ListenerRegistration registration =
+        orderedQuery.addSnapshotListener(eventAccumulator.listener());
+
+    List<String> watchSnapshotDocIds = new ArrayList<>();
+    try {
+      QuerySnapshot watchSnapshot = eventAccumulator.await();
+      watchSnapshotDocIds =
+          watchSnapshot.getDocuments().stream()
+              .map(documentSnapshot -> documentSnapshot.getId())
+              .collect(Collectors.toList());
+    } finally {
+      registration.remove();
+    }
+
+    assertTrue(getSnapshotDocIds.equals(expectedDocIds));
+    assertTrue(watchSnapshotDocIds.equals(expectedDocIds));
+
+    checkOnlineAndOfflineResultsMatch(orderedQuery, expectedDocIds.toArray(new String[0]));
+  }
+
+  @Test
+  public void snapshotListenerSortsUnicodeStringsInMapKeyAsServer() {
+    Map<String, Map<String, Object>> testDocs =
+        map(
+            "a", map("value", map("Łukasiewicz", "foo")),
+            "b", map("value", map("Sierpiński", "foo")),
+            "c", map("value", map("岩澤", "foo")),
+            "d", map("value", map("🄟", "foo")),
+            "e", map("value", map("Ｐ", "foo")),
+            "f", map("value", map("︒", "foo")),
+            "g", map("value", map("🐵", "foo")));
+
+    CollectionReference colRef = testCollectionWithDocs(testDocs);
+    Query orderedQuery = colRef.orderBy("value");
+    List<String> expectedDocIds = Arrays.asList("b", "a", "c", "f", "e", "d", "g");
+
+    QuerySnapshot getSnapshot = waitFor(orderedQuery.get());
+    List<String> getSnapshotDocIds =
+        getSnapshot.getDocuments().stream().map(ds -> ds.getId()).collect(Collectors.toList());
+
+    EventAccumulator<QuerySnapshot> eventAccumulator = new EventAccumulator<QuerySnapshot>();
+    ListenerRegistration registration =
+        orderedQuery.addSnapshotListener(eventAccumulator.listener());
+
+    List<String> watchSnapshotDocIds = new ArrayList<>();
+    try {
+      QuerySnapshot watchSnapshot = eventAccumulator.await();
+      watchSnapshotDocIds =
+          watchSnapshot.getDocuments().stream()
+              .map(documentSnapshot -> documentSnapshot.getId())
+              .collect(Collectors.toList());
+    } finally {
+      registration.remove();
+    }
+
+    assertTrue(getSnapshotDocIds.equals(expectedDocIds));
+    assertTrue(watchSnapshotDocIds.equals(expectedDocIds));
+
+    checkOnlineAndOfflineResultsMatch(orderedQuery, expectedDocIds.toArray(new String[0]));
+  }
+
+  @Test
+  public void snapshotListenerSortsUnicodeStringsInDocumentKeyAsServer() {
+    Map<String, Map<String, Object>> testDocs =
+        map(
+            "Łukasiewicz", map("value", "foo"),
+            "Sierpiński", map("value", "foo"),
+            "岩澤", map("value", "foo"),
+            "🄟", map("value", "foo"),
+            "Ｐ", map("value", "foo"),
+            "︒", map("value", "foo"),
+            "🐵", map("value", "foo"));
+
+    CollectionReference colRef = testCollectionWithDocs(testDocs);
+    Query orderedQuery = colRef.orderBy(FieldPath.documentId());
+    List<String> expectedDocIds =
+        Arrays.asList("Sierpiński", "Łukasiewicz", "岩澤", "︒", "Ｐ", "🄟", "🐵");
+
+    QuerySnapshot getSnapshot = waitFor(orderedQuery.get());
+    List<String> getSnapshotDocIds =
+        getSnapshot.getDocuments().stream().map(ds -> ds.getId()).collect(Collectors.toList());
+
+    EventAccumulator<QuerySnapshot> eventAccumulator = new EventAccumulator<QuerySnapshot>();
+    ListenerRegistration registration =
+        orderedQuery.addSnapshotListener(eventAccumulator.listener());
+
+    List<String> watchSnapshotDocIds = new ArrayList<>();
+    try {
+      QuerySnapshot watchSnapshot = eventAccumulator.await();
+      watchSnapshotDocIds =
+          watchSnapshot.getDocuments().stream()
+              .map(documentSnapshot -> documentSnapshot.getId())
+              .collect(Collectors.toList());
+    } finally {
+      registration.remove();
+    }
+
+    assertTrue(getSnapshotDocIds.equals(expectedDocIds));
+    assertTrue(watchSnapshotDocIds.equals(expectedDocIds));
+
+    checkOnlineAndOfflineResultsMatch(orderedQuery, expectedDocIds.toArray(new String[0]));
+  }
 }
diff --git a/firebase-firestore/src/main/java/com/google/firebase/firestore/model/BasePath.java b/firebase-firestore/src/main/java/com/google/firebase/firestore/model/BasePath.java
@@ -114,7 +114,7 @@ private static int compareSegments(String lhs, String rhs) {
     } else if (isLhsNumeric && isRhsNumeric) { // both numeric
       return Long.compare(extractNumericId(lhs), extractNumericId(rhs));
     } else { // both string
-      return lhs.compareTo(rhs);
+      return Util.compareUtf8Strings(lhs, rhs);
     }
   }
 

diff --git a/firebase-firestore/src/main/java/com/google/firebase/firestore/model/Values.java b/firebase-firestore/src/main/java/com/google/firebase/firestore/model/Values.java
@@ -230,7 +230,7 @@ public static int compare(Value left, Value right) {
       case TYPE_ORDER_SERVER_TIMESTAMP:
         return compareTimestamps(getLocalWriteTime(left), getLocalWriteTime(right));
       case TYPE_ORDER_STRING:
-        return left.getStringValue().compareTo(right.getStringValue());
+        return Util.compareUtf8Strings(left.getStringValue(), right.getStringValue());
       case TYPE_ORDER_BLOB:
         return Util.compareByteStrings(left.getBytesValue(), right.getBytesValue());
       case TYPE_ORDER_REFERENCE:
@@ -349,7 +349,7 @@ private static int compareMaps(MapValue left, MapValue right) {
     while (iterator1.hasNext() && iterator2.hasNext()) {
       Map.Entry<String, Value> entry1 = iterator1.next();
       Map.Entry<String, Value> entry2 = iterator2.next();
-      int keyCompare = entry1.getKey().compareTo(entry2.getKey());
+      int keyCompare = Util.compareUtf8Strings(entry1.getKey(), entry2.getKey());
       if (keyCompare != 0) {
         return keyCompare;
       }

diff --git a/firebase-firestore/src/main/java/com/google/firebase/firestore/util/Util.java b/firebase-firestore/src/main/java/com/google/firebase/firestore/util/Util.java
@@ -85,6 +85,13 @@ public static int compareIntegers(int i1, int i2) {
     }
   }
 
+  /** Compare strings in UTF-8 encoded byte order */
+  public static int compareUtf8Strings(String left, String right) {
+    ByteString leftBytes = ByteString.copyFromUtf8(left);
+    ByteString rightBytes = ByteString.copyFromUtf8(right);
+    return compareByteStrings(leftBytes, rightBytes);
+  }
+
   /**
    * Utility function to compare longs. Note that we can't use Long.compare because it's only
    * available after Android 19.