Skip to content

Commit

Permalink
FIX: sort strings in UTF-8 encoded byte order (#1967)
Browse files Browse the repository at this point in the history
  • Loading branch information
@milaGGL
milaGGL authored Jan 15, 2025
1 parent 8cb4dc8 commit 4309639
Show file tree
Hide file tree
Showing 3 changed files with 212 additions and 5 deletions.
2 changes: 1 addition & 1 deletion google-cloud-firestore/src/main/java/com/google/cloud/firestore/BasePath.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ private 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 Order.compareUtf8Strings(lhs, rhs);
}
}

Expand Down
14 changes: 10 additions & 4 deletions google-cloud-firestore/src/main/java/com/google/cloud/firestore/Order.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ public int compare(@Nonnull Value left, @Nonnull Value right) {
case TIMESTAMP:
return compareTimestamps(left, right);
case STRING:
return compareStrings(left, right);
return compareUtf8Strings(left.getStringValue(), right.getStringValue());
case BLOB:
return compareBlobs(left, right);
case REF:
Expand All @@ -134,14 +134,20 @@ public int compare(@Nonnull Value left, @Nonnull Value right) {
}
}

private int compareStrings(Value left, Value right) {
return left.getStringValue().compareTo(right.getStringValue());
/** 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);
}

private int compareBlobs(Value left, Value right) {
ByteString leftBytes = left.getBytesValue();
ByteString rightBytes = right.getBytesValue();
return compareByteStrings(leftBytes, rightBytes);
}

private static int compareByteStrings(ByteString leftBytes, ByteString rightBytes) {
int size = Math.min(leftBytes.size(), rightBytes.size());
for (int i = 0; i < size; i++) {
// Make sure the bytes are unsigned
Expand Down Expand Up @@ -211,7 +217,7 @@ private int compareObjects(Value left, Value right) {
while (leftIterator.hasNext() && rightIterator.hasNext()) {
Entry<String, Value> leftEntry = leftIterator.next();
Entry<String, Value> rightEntry = rightIterator.next();
int keyCompare = leftEntry.getKey().compareTo(rightEntry.getKey());
int keyCompare = compareUtf8Strings(leftEntry.getKey(), rightEntry.getKey());
if (keyCompare != 0) {
return keyCompare;
}
Expand Down
201 changes: 201 additions & 0 deletions google-cloud-firestore/src/test/java/com/google/cloud/firestore/it/ITQueryTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1155,4 +1155,205 @@ public void snapshotListenerSortsNumbersSameWayAsServer() throws Exception {

assertEquals(queryOrder, listenerOrder); // Assert order in the SDK
}

@Test
public void snapshotListenerSortsUnicodeStringsSameWayAsServer() throws Exception {
CollectionReference col = createEmptyCollection();

firestore
.batch()
.set(col.document("a"), map("value", "Łukasiewicz"))
.set(col.document("b"), map("value", "Sierpiński"))
.set(col.document("c"), map("value", "岩澤"))
.set(col.document("d"), map("value", "🄟"))
.set(col.document("e"), map("value", "Ｐ"))
.set(col.document("f"), map("value", "︒"))
.set(col.document("g"), map("value", "🐵"))
.commit()
.get();

Query query = col.orderBy("value", Direction.ASCENDING);

QuerySnapshot snapshot = query.get().get();
List<String> queryOrder =
snapshot.getDocuments().stream().map(doc -> doc.getId()).collect(Collectors.toList());

CountDownLatch latch = new CountDownLatch(1);
List<String> listenerOrder = new ArrayList<>();
ListenerRegistration registration =
query.addSnapshotListener(
(value, error) -> {
listenerOrder.addAll(
value.getDocuments().stream()
.map(doc -> doc.getId())
.collect(Collectors.toList()));
latch.countDown();
});
latch.await();
registration.remove();

assertEquals(queryOrder, Arrays.asList("b", "a", "c", "f", "e", "d", "g"));
assertEquals(queryOrder, listenerOrder);
}

@Test
public void snapshotListenerSortsUnicodeStringsInArraySameWayAsServer() throws Exception {
CollectionReference col = createEmptyCollection();

firestore
.batch()
.set(col.document("a"), map("value", Arrays.asList("Łukasiewicz")))
.set(col.document("b"), map("value", Arrays.asList("Sierpiński")))
.set(col.document("c"), map("value", Arrays.asList("岩澤")))
.set(col.document("d"), map("value", Arrays.asList("🄟")))
.set(col.document("e"), map("value", Arrays.asList("Ｐ")))
.set(col.document("f"), map("value", Arrays.asList("︒")))
.set(col.document("g"), map("value", Arrays.asList("🐵")))
.commit()
.get();

Query query = col.orderBy("value", Direction.ASCENDING);

QuerySnapshot snapshot = query.get().get();
List<String> queryOrder =
snapshot.getDocuments().stream().map(doc -> doc.getId()).collect(Collectors.toList());

CountDownLatch latch = new CountDownLatch(1);
List<String> listenerOrder = new ArrayList<>();
ListenerRegistration registration =
query.addSnapshotListener(
(value, error) -> {
listenerOrder.addAll(
value.getDocuments().stream()
.map(doc -> doc.getId())
.collect(Collectors.toList()));
latch.countDown();
});
latch.await();
registration.remove();

assertEquals(queryOrder, Arrays.asList("b", "a", "c", "f", "e", "d", "g"));
assertEquals(queryOrder, listenerOrder);
}

@Test
public void snapshotListenerSortsUnicodeStringsInMapSameWayAsServer() throws Exception {
CollectionReference col = createEmptyCollection();

firestore
.batch()
.set(col.document("a"), map("value", map("foo", "Łukasiewicz")))
.set(col.document("b"), map("value", map("foo", "Sierpiński")))
.set(col.document("c"), map("value", map("foo", "岩澤")))
.set(col.document("d"), map("value", map("foo", "🄟")))
.set(col.document("e"), map("value", map("foo", "Ｐ")))
.set(col.document("f"), map("value", map("foo", "︒")))
.set(col.document("g"), map("value", map("foo", "🐵")))
.commit()
.get();

Query query = col.orderBy("value", Direction.ASCENDING);

QuerySnapshot snapshot = query.get().get();
List<String> queryOrder =
snapshot.getDocuments().stream().map(doc -> doc.getId()).collect(Collectors.toList());

CountDownLatch latch = new CountDownLatch(1);
List<String> listenerOrder = new ArrayList<>();
ListenerRegistration registration =
query.addSnapshotListener(
(value, error) -> {
listenerOrder.addAll(
value.getDocuments().stream()
.map(doc -> doc.getId())
.collect(Collectors.toList()));
latch.countDown();
});
latch.await();
registration.remove();

assertEquals(queryOrder, Arrays.asList("b", "a", "c", "f", "e", "d", "g"));
assertEquals(queryOrder, listenerOrder);
}

@Test
public void snapshotListenerSortsUnicodeStringsInMapKeySameWayAsServer() throws Exception {
CollectionReference col = createEmptyCollection();

firestore
.batch()
.set(col.document("a"), map("value", map("Łukasiewicz", "foo")))
.set(col.document("b"), map("value", map("Sierpiński", "foo")))
.set(col.document("c"), map("value", map("岩澤", "foo")))
.set(col.document("d"), map("value", map("🄟", "foo")))
.set(col.document("e"), map("value", map("Ｐ", "foo")))
.set(col.document("f"), map("value", map("︒", "foo")))
.set(col.document("g"), map("value", map("🐵", "foo")))
.commit()
.get();

Query query = col.orderBy("value", Direction.ASCENDING);

QuerySnapshot snapshot = query.get().get();
List<String> queryOrder =
snapshot.getDocuments().stream().map(doc -> doc.getId()).collect(Collectors.toList());

CountDownLatch latch = new CountDownLatch(1);
List<String> listenerOrder = new ArrayList<>();
ListenerRegistration registration =
query.addSnapshotListener(
(value, error) -> {
listenerOrder.addAll(
value.getDocuments().stream()
.map(doc -> doc.getId())
.collect(Collectors.toList()));
latch.countDown();
});
latch.await();
registration.remove();

assertEquals(queryOrder, Arrays.asList("b", "a", "c", "f", "e", "d", "g"));
assertEquals(queryOrder, listenerOrder);
}

@Test
public void snapshotListenerSortsUnicodeStringsInDocumentKeySameWayAsServer() throws Exception {
CollectionReference col = createEmptyCollection();

firestore
.batch()
.set(col.document("Łukasiewicz"), map("value", "foo"))
.set(col.document("Sierpiński"), map("value", "foo"))
.set(col.document("岩澤"), map("value", "foo"))
.set(col.document("🄟"), map("value", "foo"))
.set(col.document("Ｐ"), map("value", "foo"))
.set(col.document("︒"), map("value", "foo"))
.set(col.document("🐵"), map("value", "foo"))
.commit()
.get();

Query query = col.orderBy(FieldPath.documentId());

QuerySnapshot snapshot = query.get().get();
List<String> queryOrder =
snapshot.getDocuments().stream().map(doc -> doc.getId()).collect(Collectors.toList());

CountDownLatch latch = new CountDownLatch(1);
List<String> listenerOrder = new ArrayList<>();
ListenerRegistration registration =
query.addSnapshotListener(
(value, error) -> {
listenerOrder.addAll(
value.getDocuments().stream()
.map(doc -> doc.getId())
.collect(Collectors.toList()));
latch.countDown();
});
latch.await();
registration.remove();

assertEquals(
queryOrder, Arrays.asList("Sierpiński", "Łukasiewicz", "岩澤", "︒", "Ｐ", "🄟", "🐵"));
assertEquals(queryOrder, listenerOrder);
}
}

0 comments on commit 4309639

Please sign in to comment.