diff --git a/google-cloud-firestore/src/main/java/com/google/cloud/firestore/Order.java b/google-cloud-firestore/src/main/java/com/google/cloud/firestore/Order.java index 65a92a1be..28434920a 100644 --- a/google-cloud-firestore/src/main/java/com/google/cloud/firestore/Order.java +++ b/google-cloud-firestore/src/main/java/com/google/cloud/firestore/Order.java @@ -250,31 +250,60 @@ private int compareVectors(Value left, Value right) { } private int compareNumbers(Value left, Value right) { + // NaN is smaller than any other numbers + if (isNaN(left)) { + return isNaN(right) ? 0 : -1; + } else if (isNaN(right)) { + return 1; + } + if (left.getValueTypeCase() == ValueTypeCase.DOUBLE_VALUE) { if (right.getValueTypeCase() == ValueTypeCase.DOUBLE_VALUE) { return compareDoubles(left.getDoubleValue(), right.getDoubleValue()); } else { - return compareDoubles(left.getDoubleValue(), right.getIntegerValue()); + return compareDoubleAndLong(left.getDoubleValue(), right.getIntegerValue()); } } else { if (right.getValueTypeCase() == ValueTypeCase.INTEGER_VALUE) { return Long.compare(left.getIntegerValue(), right.getIntegerValue()); } else { - return compareDoubles(left.getIntegerValue(), right.getDoubleValue()); + return -compareDoubleAndLong(right.getDoubleValue(), left.getIntegerValue()); } } } + private boolean isNaN(Value value) { + return value.hasDoubleValue() && Double.isNaN(value.getDoubleValue()); + } + private int compareDoubles(double left, double right) { - // Firestore orders NaNs before all other numbers and treats -0.0, 0.0 and +0.0 as equal. - if (Double.isNaN(left)) { - return Double.isNaN(right) ? 0 : -1; - } + // Firestore treats -0.0, 0.0 and +0.0 as equal. + return Double.compare(left == -0.0 ? 0 : left, right == -0.0 ? 0 : right); + } - if (Double.isNaN(right)) { - return 1; - } + /** + * The maximum integer absolute number that can be represented as a double without loss of + * precision. This is 2^53 because double-precision floating point numbers have 53 bits + * significand precision (52 explicit bit + 1 hidden bit). + */ + private static final long MAX_INTEGER_TO_DOUBLE_PRECISION = 1L << 53; - return Double.compare(left == -0.0 ? 0 : left, right == -0.0 ? 0 : right); + private int compareDoubleAndLong(double doubleValue, long longValue) { + if (Math.abs(longValue) <= MAX_INTEGER_TO_DOUBLE_PRECISION) { + // Enough precision to compare as double, the cast will not be lossy. + return compareDoubles(doubleValue, (double) longValue); + } else if (doubleValue < ((double) Long.MAX_VALUE) + && doubleValue >= ((double) Long.MIN_VALUE)) { + // The above condition captures all doubles that belong to [min long, max long] inclusive. + // Java long to double conversion rounds-to-nearest, so Long.MAX_VALUE casts to 2^63, hence + // the use of "<" operator. + // The cast to long below may be lossy, but only for absolute values < 2^52 so the loss of + // precision does not affect the comparison, as longValue is outside that range. + return Long.compare((long) doubleValue, longValue); + } else { + // doubleValue is outside the representable range for longs, so always smaller if negative, + // and always greater otherwise. + return doubleValue < 0 ? -1 : 1; + } } } diff --git a/google-cloud-firestore/src/test/java/com/google/cloud/firestore/OrderTest.java b/google-cloud-firestore/src/test/java/com/google/cloud/firestore/OrderTest.java index 7a43f331a..c4993b444 100644 --- a/google-cloud-firestore/src/test/java/com/google/cloud/firestore/OrderTest.java +++ b/google-cloud-firestore/src/test/java/com/google/cloud/firestore/OrderTest.java @@ -32,7 +32,7 @@ public class OrderTest { @Test public void verifyOrder() { - Value[][] groups = new Value[65][]; + Value[][] groups = new Value[67][]; groups[0] = new Value[] {nullValue()}; @@ -42,83 +42,85 @@ public void verifyOrder() { // numbers groups[3] = new Value[] {doubleValue(Double.NaN), doubleValue(Double.NaN)}; groups[4] = new Value[] {doubleValue(Double.NEGATIVE_INFINITY)}; - groups[5] = new Value[] {intValue((long) Integer.MIN_VALUE - 1)}; - groups[6] = new Value[] {intValue(Integer.MIN_VALUE)}; - groups[7] = new Value[] {doubleValue(-1.1)}; + groups[5] = new Value[] {doubleValue((double) Long.MIN_VALUE - 100)}; + groups[6] = new Value[] {intValue((long) Integer.MIN_VALUE - 1)}; + groups[7] = new Value[] {intValue(Integer.MIN_VALUE)}; + groups[8] = new Value[] {doubleValue(-1.1)}; // Integers and Doubles order the same. - groups[8] = new Value[] {intValue(-1), doubleValue(-1.0)}; - groups[9] = new Value[] {doubleValue(-Double.MIN_VALUE)}; + groups[9] = new Value[] {intValue(-1), doubleValue(-1.0)}; + groups[10] = new Value[] {doubleValue(-Double.MIN_VALUE)}; // zeros all compare the same. - groups[10] = new Value[] {intValue(0), doubleValue(-0.0), doubleValue(0.0), doubleValue(+0.0)}; - groups[11] = new Value[] {doubleValue(Double.MIN_VALUE)}; - groups[12] = new Value[] {intValue(1), doubleValue(1.0)}; - groups[13] = new Value[] {doubleValue(1.1)}; - groups[14] = new Value[] {intValue(Integer.MAX_VALUE)}; - groups[15] = new Value[] {intValue((long) Integer.MAX_VALUE + 1)}; - groups[16] = new Value[] {doubleValue(Double.POSITIVE_INFINITY)}; - - groups[17] = new Value[] {timestampValue(123, 0)}; - groups[18] = new Value[] {timestampValue(123, 123)}; - groups[19] = new Value[] {timestampValue(345, 0)}; + groups[11] = new Value[] {intValue(0), doubleValue(-0.0), doubleValue(0.0), doubleValue(+0.0)}; + groups[12] = new Value[] {doubleValue(Double.MIN_VALUE)}; + groups[13] = new Value[] {intValue(1), doubleValue(1.0)}; + groups[14] = new Value[] {doubleValue(1.1)}; + groups[15] = new Value[] {intValue(Integer.MAX_VALUE)}; + groups[16] = new Value[] {intValue((long) Integer.MAX_VALUE + 1)}; + groups[17] = new Value[] {doubleValue(((double) Long.MAX_VALUE) + 100)}; + groups[18] = new Value[] {doubleValue(Double.POSITIVE_INFINITY)}; + + groups[19] = new Value[] {timestampValue(123, 0)}; + groups[20] = new Value[] {timestampValue(123, 123)}; + groups[21] = new Value[] {timestampValue(345, 0)}; // strings - groups[20] = new Value[] {stringValue("")}; - groups[21] = new Value[] {stringValue("\u0000\ud7ff\ue000\uffff")}; - groups[22] = new Value[] {stringValue("(╯°□°）╯︵ ┻━┻")}; - groups[23] = new Value[] {stringValue("a")}; - groups[24] = new Value[] {stringValue("abc def")}; + groups[22] = new Value[] {stringValue("")}; + groups[23] = new Value[] {stringValue("\u0000\ud7ff\ue000\uffff")}; + groups[24] = new Value[] {stringValue("(╯°□°）╯︵ ┻━┻")}; + groups[25] = new Value[] {stringValue("a")}; + groups[26] = new Value[] {stringValue("abc def")}; // latin small letter e + combining acute accent + latin small letter b - groups[25] = new Value[] {stringValue("e\u0301b")}; - groups[26] = new Value[] {stringValue("æ")}; + groups[27] = new Value[] {stringValue("e\u0301b")}; + groups[28] = new Value[] {stringValue("æ")}; // latin small letter e with acute accent + latin small letter a - groups[27] = new Value[] {stringValue("\u00e9a")}; + groups[29] = new Value[] {stringValue("\u00e9a")}; // blobs - groups[28] = new Value[] {blobValue(new byte[] {})}; - groups[29] = new Value[] {blobValue(new byte[] {0})}; - groups[30] = new Value[] {blobValue(new byte[] {0, 1, 2, 3, 4})}; - groups[31] = new Value[] {blobValue(new byte[] {0, 1, 2, 4, 3})}; - groups[32] = new Value[] {blobValue(new byte[] {127})}; + groups[30] = new Value[] {blobValue(new byte[] {})}; + groups[31] = new Value[] {blobValue(new byte[] {0})}; + groups[32] = new Value[] {blobValue(new byte[] {0, 1, 2, 3, 4})}; + groups[33] = new Value[] {blobValue(new byte[] {0, 1, 2, 4, 3})}; + groups[34] = new Value[] {blobValue(new byte[] {127})}; // resource names - groups[33] = new Value[] {referenceValue("projects/p1/databases/d1/documents/c1/doc1")}; - groups[34] = new Value[] {referenceValue("projects/p1/databases/d1/documents/c1/doc2")}; - groups[35] = new Value[] {referenceValue("projects/p1/databases/d1/documents/c1/doc2/c2/doc1")}; - groups[36] = new Value[] {referenceValue("projects/p1/databases/d1/documents/c1/doc2/c2/doc2")}; - groups[37] = new Value[] {referenceValue("projects/p1/databases/d1/documents/c10/doc1")}; - groups[38] = new Value[] {referenceValue("projects/p1/databases/d1/documents/c2/doc1")}; - groups[39] = new Value[] {referenceValue("projects/p2/databases/d2/documents/c1/doc1")}; - groups[40] = new Value[] {referenceValue("projects/p2/databases/d2/documents/c1-/doc1")}; - groups[41] = new Value[] {referenceValue("projects/p2/databases/d3/documents/c1-/doc1")}; + groups[35] = new Value[] {referenceValue("projects/p1/databases/d1/documents/c1/doc1")}; + groups[36] = new Value[] {referenceValue("projects/p1/databases/d1/documents/c1/doc2")}; + groups[37] = new Value[] {referenceValue("projects/p1/databases/d1/documents/c1/doc2/c2/doc1")}; + groups[38] = new Value[] {referenceValue("projects/p1/databases/d1/documents/c1/doc2/c2/doc2")}; + groups[39] = new Value[] {referenceValue("projects/p1/databases/d1/documents/c10/doc1")}; + groups[40] = new Value[] {referenceValue("projects/p1/databases/d1/documents/c2/doc1")}; + groups[41] = new Value[] {referenceValue("projects/p2/databases/d2/documents/c1/doc1")}; + groups[42] = new Value[] {referenceValue("projects/p2/databases/d2/documents/c1-/doc1")}; + groups[43] = new Value[] {referenceValue("projects/p2/databases/d3/documents/c1-/doc1")}; // geo points - groups[42] = new Value[] {geoPointValue(-90, -180)}; - groups[43] = new Value[] {geoPointValue(-90, 0)}; - groups[44] = new Value[] {geoPointValue(-90, 180)}; - groups[45] = new Value[] {geoPointValue(0, -180)}; - groups[46] = new Value[] {geoPointValue(0, 0)}; - groups[47] = new Value[] {geoPointValue(0, 180)}; - groups[48] = new Value[] {geoPointValue(1, -180)}; - groups[49] = new Value[] {geoPointValue(1, 0)}; - groups[50] = new Value[] {geoPointValue(1, 180)}; - groups[51] = new Value[] {geoPointValue(90, -180)}; - groups[52] = new Value[] {geoPointValue(90, 0)}; - groups[53] = new Value[] {geoPointValue(90, 180)}; + groups[44] = new Value[] {geoPointValue(-90, -180)}; + groups[45] = new Value[] {geoPointValue(-90, 0)}; + groups[46] = new Value[] {geoPointValue(-90, 180)}; + groups[47] = new Value[] {geoPointValue(0, -180)}; + groups[48] = new Value[] {geoPointValue(0, 0)}; + groups[49] = new Value[] {geoPointValue(0, 180)}; + groups[50] = new Value[] {geoPointValue(1, -180)}; + groups[51] = new Value[] {geoPointValue(1, 0)}; + groups[52] = new Value[] {geoPointValue(1, 180)}; + groups[53] = new Value[] {geoPointValue(90, -180)}; + groups[54] = new Value[] {geoPointValue(90, 0)}; + groups[55] = new Value[] {geoPointValue(90, 180)}; // arrays - groups[54] = new Value[] {arrayValue()}; - groups[55] = new Value[] {arrayValue(stringValue("bar"))}; - groups[56] = new Value[] {arrayValue(stringValue("foo"))}; - groups[57] = new Value[] {arrayValue(stringValue("foo"), intValue(0))}; - groups[58] = new Value[] {arrayValue(stringValue("foo"), intValue(1))}; - groups[59] = new Value[] {arrayValue(stringValue("foo"), stringValue("0"))}; + groups[56] = new Value[] {arrayValue()}; + groups[57] = new Value[] {arrayValue(stringValue("bar"))}; + groups[58] = new Value[] {arrayValue(stringValue("foo"))}; + groups[59] = new Value[] {arrayValue(stringValue("foo"), intValue(0))}; + groups[60] = new Value[] {arrayValue(stringValue("foo"), intValue(1))}; + groups[61] = new Value[] {arrayValue(stringValue("foo"), stringValue("0"))}; // objects - groups[60] = new Value[] {objectValue("bar", intValue(0))}; - groups[61] = new Value[] {objectValue("bar", intValue(0), "foo", intValue(1))}; - groups[62] = new Value[] {objectValue("bar", intValue(1))}; - groups[63] = new Value[] {objectValue("bar", intValue(2))}; - groups[64] = new Value[] {objectValue("bar", stringValue("0"))}; + groups[62] = new Value[] {objectValue("bar", intValue(0))}; + groups[63] = new Value[] {objectValue("bar", intValue(0), "foo", intValue(1))}; + groups[64] = new Value[] {objectValue("bar", intValue(1))}; + groups[65] = new Value[] {objectValue("bar", intValue(2))}; + groups[66] = new Value[] {objectValue("bar", stringValue("0"))}; for (int left = 0; left < groups.length; left++) { for (int right = 0; right < groups.length; right++) { diff --git a/google-cloud-firestore/src/test/java/com/google/cloud/firestore/it/ITQueryTest.java b/google-cloud-firestore/src/test/java/com/google/cloud/firestore/it/ITQueryTest.java index 6125d24f4..2381d0439 100644 --- a/google-cloud-firestore/src/test/java/com/google/cloud/firestore/it/ITQueryTest.java +++ b/google-cloud-firestore/src/test/java/com/google/cloud/firestore/it/ITQueryTest.java @@ -28,12 +28,14 @@ import com.google.cloud.firestore.*; import com.google.cloud.firestore.Query.Direction; import java.time.Duration; +import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; import java.util.Iterator; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; +import java.util.concurrent.CountDownLatch; import java.util.concurrent.ExecutionException; import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeoutException; @@ -1113,4 +1115,44 @@ public void testAggregateQueryProfile() throws Exception { assertThat(stats.getResultsReturned()).isEqualTo(1); assertThat(stats.getExecutionDuration()).isGreaterThan(Duration.ZERO); } + + @Test + public void snapshotListenerSortsNumbersSameWayAsServer() throws Exception { + CollectionReference col = createEmptyCollection(); + firestore + .batch() + .set(col.document("intMin"), map("value", Long.MIN_VALUE)) + .set(col.document("doubleMin"), map("value", ((double) Long.MIN_VALUE) - 100)) + .set(col.document("intMax"), map("value", Long.MAX_VALUE)) + .set(col.document("doubleMax"), map("value", ((double) Long.MAX_VALUE) + 100)) + .set(col.document("NaN"), map("value", Double.NaN)) + .set(col.document("integerMax"), map("value", (long) Integer.MAX_VALUE)) + .set(col.document("integerMin"), map("value", (long) Integer.MIN_VALUE)) + .set(col.document("negativeInfinity"), map("value", Double.NEGATIVE_INFINITY)) + .set(col.document("positiveInfinity"), map("value", Double.POSITIVE_INFINITY)) + .commit() + .get(); + + Query query = col.orderBy("value", Direction.ASCENDING); + + QuerySnapshot snapshot = query.get().get(); + List queryOrder = + snapshot.getDocuments().stream().map(doc -> doc.getId()).collect(Collectors.toList()); + + CountDownLatch latch = new CountDownLatch(1); + List 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, listenerOrder); // Assert order in the SDK + } }