Skip to content

Commit 1b0a574

Browse files
dconeybedconeybe
authored
Firestore: use Integer.compare() and Long.compare() instead Firestore bespoke implementations (#7109)
1 parent 84c4d81 commit 1b0a574

File tree

9 files changed

+34
-49
lines changed

9 files changed

+34
-49
lines changed

‎firebase-firestore/CHANGELOG.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,9 @@
22
* [fixed] Further improved performance of UTF-8 string ordering logic,
33
which had degraded in v25.1.2 and received some improvements in v25.1.3.
44
[#7053](//github.com/firebase/firebase-android-sdk/issues/7053)
5+
* [changed] Use the `compare()` methods defined in standard `Integer`, `Long`, and `Character`
6+
classes instead of Firestore's bespoke implementations.
7+
[#7109](//github.com/firebase/firebase-android-sdk/pull/7109)
58

69

710
# 25.1.4

‎firebase-firestore/src/main/java/com/google/cloud/datastore/core/number/NumberComparisonHelper.java

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ public static int firestoreCompareDoubleWithLong(double doubleValue, long longVa
5050
}
5151

5252
long doubleAsLong = (long) doubleValue;
53-
int cmp = compareLongs(doubleAsLong, longValue);
53+
int cmp = Long.compare(doubleAsLong, longValue);
5454
if (cmp != 0) {
5555
return cmp;
5656
}
@@ -60,11 +60,6 @@ public static int firestoreCompareDoubleWithLong(double doubleValue, long longVa
6060
return firestoreCompareDoubles(doubleValue, longAsDouble);
6161
}
6262

63-
/** Compares longs. */
64-
public static int compareLongs(long leftLong, long rightLong) {
65-
return Long.compare(leftLong, rightLong);
66-
}
67-
6863
/**
6964
* Compares doubles with Firestore query semantics: NaN precedes all other numbers and equals
7065
* itself, all zeroes are equal.

‎firebase-firestore/src/main/java/com/google/firebase/firestore/core/View.java

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@
1717
import static com.google.firebase.firestore.core.Query.LimitType.LIMIT_TO_FIRST;
1818
import static com.google.firebase.firestore.core.Query.LimitType.LIMIT_TO_LAST;
1919
import static com.google.firebase.firestore.util.Assert.hardAssert;
20-
import static com.google.firebase.firestore.util.Util.compareIntegers;
2120

2221
import androidx.annotation.Nullable;
2322
import com.google.firebase.database.collection.ImmutableSortedMap;
@@ -301,7 +300,7 @@ public ViewChange applyChanges(
301300
Collections.sort(
302301
viewChanges,
303302
(DocumentViewChange o1, DocumentViewChange o2) -> {
304-
int typeComp = compareIntegers(View.changeTypeOrder(o1), View.changeTypeOrder(o2));
303+
int typeComp = Integer.compare(View.changeTypeOrder(o1), View.changeTypeOrder(o2));
305304
if (typeComp != 0) {
306305
return typeComp;
307306
}

‎firebase-firestore/src/main/java/com/google/firebase/firestore/local/DocumentReference.java

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,6 @@
1414

1515
package com.google.firebase.firestore.local;
1616

17-
import static com.google.firebase.firestore.util.Util.compareIntegers;
18-
1917
import com.google.firebase.firestore.model.DocumentKey;
2018
import java.util.Comparator;
2119

@@ -60,12 +58,12 @@ int getId() {
6058
return keyComp;
6159
}
6260

63-
return compareIntegers(o1.targetOrBatchId, o2.targetOrBatchId);
61+
return Integer.compare(o1.targetOrBatchId, o2.targetOrBatchId);
6462
};
6563

6664
static final Comparator<DocumentReference> BY_TARGET =
6765
(o1, o2) -> {
68-
int targetComp = compareIntegers(o1.targetOrBatchId, o2.targetOrBatchId);
66+
int targetComp = Integer.compare(o1.targetOrBatchId, o2.targetOrBatchId);
6967

7068
if (targetComp != 0) {
7169
return targetComp;

‎firebase-firestore/src/main/java/com/google/firebase/firestore/local/SQLiteMutationQueue.java

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@
3030
import com.google.firebase.firestore.model.mutation.MutationBatch;
3131
import com.google.firebase.firestore.remote.WriteStream;
3232
import com.google.firebase.firestore.util.Consumer;
33-
import com.google.firebase.firestore.util.Util;
3433
import com.google.protobuf.ByteString;
3534
import com.google.protobuf.InvalidProtocolBufferException;
3635
import com.google.protobuf.MessageLite;
@@ -324,7 +323,7 @@ public List<MutationBatch> getAllMutationBatchesAffectingDocumentKeys(
324323
Collections.sort(
325324
result,
326325
(MutationBatch lhs, MutationBatch rhs) ->
327-
Util.compareIntegers(lhs.getBatchId(), rhs.getBatchId()));
326+
Integer.compare(lhs.getBatchId(), rhs.getBatchId()));
328327
}
329328
return result;
330329
}

‎firebase-firestore/src/main/java/com/google/firebase/firestore/model/BasePath.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ public int compareTo(@NonNull B o) {
100100
}
101101
i++;
102102
}
103-
return Util.compareIntegers(myLength, theirLength);
103+
return Integer.compare(myLength, theirLength);
104104
}
105105

106106
private static int compareSegments(String lhs, String rhs) {

‎firebase-firestore/src/main/java/com/google/firebase/firestore/model/Values.java

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -214,7 +214,7 @@ public static int compare(Value left, Value right) {
214214
int rightType = typeOrder(right);
215215

216216
if (leftType != rightType) {
217-
return Util.compareIntegers(leftType, rightType);
217+
return Integer.compare(leftType, rightType);
218218
}
219219

220220
switch (leftType) {
@@ -291,7 +291,7 @@ private static int compareNumbers(Value left, Value right) {
291291
} else if (left.getValueTypeCase() == Value.ValueTypeCase.INTEGER_VALUE) {
292292
long leftLong = left.getIntegerValue();
293293
if (right.getValueTypeCase() == Value.ValueTypeCase.INTEGER_VALUE) {
294-
return Util.compareLongs(leftLong, right.getIntegerValue());
294+
return Long.compare(leftLong, right.getIntegerValue());
295295
} else if (right.getValueTypeCase() == Value.ValueTypeCase.DOUBLE_VALUE) {
296296
return -1 * Util.compareMixed(right.getDoubleValue(), leftLong);
297297
}
@@ -301,11 +301,11 @@ private static int compareNumbers(Value left, Value right) {
301301
}
302302

303303
private static int compareTimestamps(Timestamp left, Timestamp right) {
304-
int cmp = Util.compareLongs(left.getSeconds(), right.getSeconds());
304+
int cmp = Long.compare(left.getSeconds(), right.getSeconds());
305305
if (cmp != 0) {
306306
return cmp;
307307
}
308-
return Util.compareIntegers(left.getNanos(), right.getNanos());
308+
return Integer.compare(left.getNanos(), right.getNanos());
309309
}
310310

311311
private static int compareReferences(String leftPath, String rightPath) {
@@ -319,7 +319,7 @@ private static int compareReferences(String leftPath, String rightPath) {
319319
return cmp;
320320
}
321321
}
322-
return Util.compareIntegers(leftSegments.length, rightSegments.length);
322+
return Integer.compare(leftSegments.length, rightSegments.length);
323323
}
324324

325325
private static int compareGeoPoints(LatLng left, LatLng right) {
@@ -338,7 +338,7 @@ private static int compareArrays(ArrayValue left, ArrayValue right) {
338338
return cmp;
339339
}
340340
}
341-
return Util.compareIntegers(left.getValuesCount(), right.getValuesCount());
341+
return Integer.compare(left.getValuesCount(), right.getValuesCount());
342342
}
343343

344344
private static int compareMaps(MapValue left, MapValue right) {
@@ -372,7 +372,7 @@ private static int compareVectors(MapValue left, MapValue right) {
372372
ArrayValue rightArrayValue = rightMap.get(Values.VECTOR_MAP_VECTORS_KEY).getArrayValue();
373373

374374
int lengthCompare =
375-
Util.compareIntegers(leftArrayValue.getValuesCount(), rightArrayValue.getValuesCount());
375+
Integer.compare(leftArrayValue.getValuesCount(), rightArrayValue.getValuesCount());
376376
if (lengthCompare != 0) {
377377
return lengthCompare;
378378
}

‎firebase-firestore/src/main/java/com/google/firebase/firestore/util/Util.java

Lines changed: 3 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -73,20 +73,6 @@ public static int compareBooleans(boolean b1, boolean b2) {
7373
}
7474
}
7575

76-
/**
77-
* Utility function to compare integers. Note that we can't use Integer.compare because it's only
78-
* available after Android 19.
79-
*/
80-
public static int compareIntegers(int i1, int i2) {
81-
if (i1 < i2) {
82-
return -1;
83-
} else if (i1 > i2) {
84-
return 1;
85-
} else {
86-
return 0;
87-
}
88-
}
89-
9076
/** Compare strings in UTF-8 encoded byte order */
9177
public static int compareUtf8Strings(String left, String right) {
9278
// noinspection StringEquality
@@ -119,7 +105,7 @@ public static int compareUtf8Strings(String left, String right) {
119105
final char rightChar = right.charAt(i);
120106
if (leftChar != rightChar) {
121107
return (isSurrogate(leftChar) == isSurrogate(rightChar))
122-
? Util.compareIntegers(leftChar, rightChar)
108+
? Character.compare(leftChar, rightChar)
123109
: isSurrogate(leftChar) ? 1 : -1;
124110
}
125111
}
@@ -129,14 +115,6 @@ public static int compareUtf8Strings(String left, String right) {
129115
return Integer.compare(left.length(), right.length());
130116
}
131117

132-
/**
133-
* Utility function to compare longs. Note that we can't use Long.compare because it's only
134-
* available after Android 19.
135-
*/
136-
public static int compareLongs(long i1, long i2) {
137-
return NumberComparisonHelper.compareLongs(i1, i2);
138-
}
139-
140118
/** Utility function to compare doubles (using Firestore semantics for NaN). */
141119
public static int compareDoubles(double i1, double i2) {
142120
return NumberComparisonHelper.firestoreCompareDoubles(i1, i2);
@@ -274,7 +252,7 @@ public static int compareByteArrays(byte[] left, byte[] right) {
274252
}
275253
// Byte values are equal, continue with comparison
276254
}
277-
return Util.compareIntegers(left.length, right.length);
255+
return Integer.compare(left.length, right.length);
278256
}
279257

280258
public static int compareByteStrings(ByteString left, ByteString right) {
@@ -290,7 +268,7 @@ public static int compareByteStrings(ByteString left, ByteString right) {
290268
}
291269
// Byte values are equal, continue with comparison
292270
}
293-
return Util.compareIntegers(left.size(), right.size());
271+
return Integer.compare(left.size(), right.size());
294272
}
295273

296274
public static StringBuilder repeatSequence(

‎firebase-firestore/src/test/java/com/google/firebase/firestore/util/UtilTest.java

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,7 @@ public void compareUtf8StringsShouldReturnCorrectValue() {
108108
ByteString b2 = ByteString.copyFromUtf8(s2);
109109
int expected = Util.compareByteStrings(b1, b2);
110110

111-
if (actual == expected) {
111+
if (Integer.signum(actual) == Integer.signum(expected)) {
112112
passCount++;
113113
} else {
114114
errors.add(
@@ -117,9 +117,12 @@ public void compareUtf8StringsShouldReturnCorrectValue() {
117117
+ "\", s2=\""
118118
+ s2
119119
+ "\") returned "
120+
+ signName(actual)
121+
+ " ("
120122
+ actual
123+
+ ")"
121124
+ ", but expected "
122-
+ expected
125+
+ signName(expected)
123126
+ " (i="
124127
+ i
125128
+ ", s1.length="
@@ -142,6 +145,16 @@ public void compareUtf8StringsShouldReturnCorrectValue() {
142145
}
143146
}
144147

148+
private static String signName(int value) {
149+
if (value < 0) {
150+
return "a negative value";
151+
} else if (value > 0) {
152+
return "a positive value";
153+
} else {
154+
return "0";
155+
}
156+
}
157+
145158
private static class StringPairGenerator {
146159

147160
private final StringGenerator stringGenerator;

0 commit comments

Comments
 (0)