Skip to content

Commit 5eb6a01

Browse files
tom-andersentom-andersen
committed
Fixes / Refactor of Values
1 parent 45280be commit 5eb6a01

File tree

9 files changed

+48
-107
lines changed

9 files changed

+48
-107
lines changed

‎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/GeoPoint.java‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,10 @@
1414

1515
package com.google.firebase.firestore;
1616

17+
import static com.google.cloud.datastore.core.number.NumberComparisonHelper.firestoreCompareDoubles;
18+
1719
import androidx.annotation.NonNull;
1820
import androidx.annotation.Nullable;
19-
import com.google.firebase.firestore.util.Util;
2021

2122
/** Immutable class representing a {@code GeoPoint} in Cloud Firestore */
2223
public class GeoPoint implements Comparable<GeoPoint> {
@@ -52,9 +53,9 @@ public double getLongitude() {
5253

5354
@Override
5455
public int compareTo(@NonNull GeoPoint other) {
55-
int comparison = Util.compareDoubles(latitude, other.latitude);
56+
int comparison = firestoreCompareDoubles(latitude, other.latitude);
5657
if (comparison == 0) {
57-
return Util.compareDoubles(longitude, other.longitude);
58+
return firestoreCompareDoubles(longitude, other.longitude);
5859
} else {
5960
return comparison;
6061
}

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

Lines changed: 2 additions & 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,8 @@ 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 i1 = View.changeTypeOrder(o1);
304+
int typeComp = Integer.compare(i1, View.changeTypeOrder(o2));
305305
if (typeComp != 0) {
306306
return typeComp;
307307
}

‎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/MemoryMutationQueue.java‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@
2828
import com.google.firebase.firestore.model.mutation.Mutation;
2929
import com.google.firebase.firestore.model.mutation.MutationBatch;
3030
import com.google.firebase.firestore.remote.WriteStream;
31-
import com.google.firebase.firestore.util.Util;
3231
import com.google.protobuf.ByteString;
3332
import java.util.ArrayList;
3433
import java.util.Collections;
@@ -216,7 +215,7 @@ public List<MutationBatch> getAllMutationBatchesAffectingDocumentKey(DocumentKey
216215
public List<MutationBatch> getAllMutationBatchesAffectingDocumentKeys(
217216
Iterable<DocumentKey> documentKeys) {
218217
ImmutableSortedSet<Integer> uniqueBatchIDs =
219-
new ImmutableSortedSet<Integer>(emptyList(), Util.comparator());
218+
new ImmutableSortedSet<Integer>(emptyList(), Comparable::compareTo);
220219

221220
for (DocumentKey key : documentKeys) {
222221
DocumentReference start = new DocumentReference(key, 0);
@@ -255,7 +254,7 @@ public List<MutationBatch> getAllMutationBatchesAffectingQuery(Query query) {
255254

256255
// Find unique batchIDs referenced by all documents potentially matching the query.
257256
ImmutableSortedSet<Integer> uniqueBatchIDs =
258-
new ImmutableSortedSet<Integer>(emptyList(), Util.comparator());
257+
new ImmutableSortedSet<Integer>(emptyList(), Comparable::compareTo);
259258

260259
Iterator<DocumentReference> iterator = batchesByDocumentKey.iteratorFrom(start);
261260
while (iterator.hasNext()) {

‎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
@@ -107,7 +107,7 @@ public int compareTo(@NonNull B o) {
107107
}
108108
i++;
109109
}
110-
return Util.compareIntegers(myLength, theirLength);
110+
return Integer.compare(myLength, theirLength);
111111
}
112112

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

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

Lines changed: 32 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@
1313
// limitations under the License.
1414
package com.google.firebase.firestore.model
1515

16+
import com.google.cloud.datastore.core.number.NumberComparisonHelper.firestoreCompareDoubleWithLong
17+
import com.google.cloud.datastore.core.number.NumberComparisonHelper.firestoreCompareDoubles
1618
import com.google.firebase.firestore.Blob
1719
import com.google.firebase.firestore.DocumentReference
1820
import com.google.firebase.firestore.GeoPoint
@@ -28,7 +30,6 @@ import com.google.protobuf.ByteString
2830
import com.google.protobuf.NullValue
2931
import com.google.protobuf.Timestamp
3032
import com.google.type.LatLng
31-
import java.lang.Double.doubleToLongBits
3233
import java.util.Date
3334
import java.util.TreeMap
3435
import kotlin.math.min
@@ -135,17 +136,25 @@ internal object Values {
135136
}
136137
}
137138

138-
private fun numberEquals(left: Value, right: Value): Boolean {
139-
if (left.valueTypeCase != right.valueTypeCase) {
140-
return false
141-
}
142-
return when (left.valueTypeCase) {
143-
ValueTypeCase.INTEGER_VALUE -> left.integerValue == right.integerValue
139+
private fun numberEquals(left: Value, right: Value): Boolean =
140+
when (left.valueTypeCase) {
141+
ValueTypeCase.INTEGER_VALUE ->
142+
when (right.valueTypeCase) {
143+
ValueTypeCase.INTEGER_VALUE -> left.integerValue == right.integerValue
144+
ValueTypeCase.DOUBLE_VALUE ->
145+
firestoreCompareDoubleWithLong(right.doubleValue, left.integerValue) == 0
146+
else -> false
147+
}
144148
ValueTypeCase.DOUBLE_VALUE ->
145-
doubleToLongBits(left.doubleValue) == doubleToLongBits(right.doubleValue)
149+
when (right.valueTypeCase) {
150+
ValueTypeCase.INTEGER_VALUE ->
151+
firestoreCompareDoubleWithLong(left.doubleValue, right.integerValue) == 0
152+
ValueTypeCase.DOUBLE_VALUE ->
153+
firestoreCompareDoubles(left.doubleValue, right.doubleValue) == 0
154+
else -> false
155+
}
146156
else -> false
147157
}
148-
}
149158

150159
private fun arrayEquals(left: Value, right: Value): Boolean {
151160
val leftArray = left.arrayValue
@@ -199,13 +208,13 @@ internal object Values {
199208
val rightType = typeOrder(right)
200209

201210
if (leftType != rightType) {
202-
return Util.compareIntegers(leftType, rightType)
211+
return leftType.compareTo(rightType)
203212
}
204213

205214
return when (leftType) {
206215
TYPE_ORDER_NULL,
207216
TYPE_ORDER_MAX_VALUE -> 0
208-
TYPE_ORDER_BOOLEAN -> Util.compareBooleans(left.booleanValue, right.booleanValue)
217+
TYPE_ORDER_BOOLEAN -> left.booleanValue.compareTo(right.booleanValue)
209218
TYPE_ORDER_NUMBER -> compareNumbers(left, right)
210219
TYPE_ORDER_TIMESTAMP -> compareTimestamps(left.timestampValue, right.timestampValue)
211220
TYPE_ORDER_SERVER_TIMESTAMP ->
@@ -269,27 +278,27 @@ internal object Values {
269278
private fun compareNumbers(left: Value, right: Value): Int {
270279
if (left.hasDoubleValue()) {
271280
if (right.hasDoubleValue()) {
272-
return Util.compareDoubles(left.doubleValue, right.doubleValue)
281+
return firestoreCompareDoubles(left.doubleValue, right.doubleValue)
273282
} else if (right.hasIntegerValue()) {
274-
return Util.compareMixed(left.doubleValue, right.integerValue)
283+
return firestoreCompareDoubleWithLong(left.doubleValue, right.integerValue)
275284
}
276285
} else if (left.hasIntegerValue()) {
277286
if (right.hasIntegerValue()) {
278-
return Util.compareLongs(left.integerValue, right.integerValue)
287+
return java.lang.Long.compare(left.integerValue, right.integerValue)
279288
} else if (right.hasDoubleValue()) {
280-
return -1 * Util.compareMixed(right.doubleValue, left.integerValue)
289+
return -1 * firestoreCompareDoubleWithLong(right.doubleValue, left.integerValue)
281290
}
282291
}
283292

284293
throw Assert.fail("Unexpected values: %s vs %s", left, right)
285294
}
286295

287296
private fun compareTimestamps(left: Timestamp, right: Timestamp): Int {
288-
val cmp = Util.compareLongs(left.seconds, right.seconds)
297+
val cmp = left.seconds.compareTo(right.seconds)
289298
if (cmp != 0) {
290299
return cmp
291300
}
292-
return Util.compareIntegers(left.nanos, right.nanos)
301+
return left.nanos.compareTo(right.nanos)
293302
}
294303

295304
private fun compareReferences(leftPath: String, rightPath: String): Int {
@@ -303,13 +312,13 @@ internal object Values {
303312
return cmp
304313
}
305314
}
306-
return Util.compareIntegers(leftSegments.size, rightSegments.size)
315+
return leftSegments.size.compareTo(rightSegments.size)
307316
}
308317

309318
private fun compareGeoPoints(left: LatLng, right: LatLng): Int {
310-
val comparison = Util.compareDoubles(left.latitude, right.latitude)
319+
val comparison = firestoreCompareDoubles(left.latitude, right.latitude)
311320
if (comparison == 0) {
312-
return Util.compareDoubles(left.longitude, right.longitude)
321+
return firestoreCompareDoubles(left.longitude, right.longitude)
313322
}
314323
return comparison
315324
}
@@ -322,7 +331,7 @@ internal object Values {
322331
return cmp
323332
}
324333
}
325-
return Util.compareIntegers(left.valuesCount, right.valuesCount)
334+
return left.valuesCount.compareTo(right.valuesCount)
326335
}
327336

328337
private fun compareMaps(left: MapValue, right: MapValue): Int {
@@ -342,7 +351,7 @@ internal object Values {
342351
}
343352

344353
// Only equal if both iterators are exhausted.
345-
return Util.compareBooleans(iterator1.hasNext(), iterator2.hasNext())
354+
return iterator1.hasNext().compareTo(iterator2.hasNext())
346355
}
347356

348357
private fun compareVectors(left: MapValue, right: MapValue): Int {
@@ -353,8 +362,7 @@ internal object Values {
353362
val leftArrayValue = leftMap[VECTOR_MAP_VECTORS_KEY]!!.arrayValue
354363
val rightArrayValue = rightMap[VECTOR_MAP_VECTORS_KEY]!!.arrayValue
355364

356-
val lengthCompare =
357-
Util.compareIntegers(leftArrayValue.valuesCount, rightArrayValue.valuesCount)
365+
val lengthCompare = leftArrayValue.valuesCount.compareTo(rightArrayValue.valuesCount)
358366
if (lengthCompare != 0) {
359367
return lengthCompare
360368
}

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

Lines changed: 3 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@
1919
import android.os.Looper;
2020
import androidx.annotation.Nullable;
2121
import com.google.android.gms.tasks.Continuation;
22-
import com.google.cloud.datastore.core.number.NumberComparisonHelper;
2322
import com.google.firebase.firestore.FieldPath;
2423
import com.google.firebase.firestore.FirebaseFirestoreException;
2524
import com.google.firebase.firestore.FirebaseFirestoreException.Code;
@@ -57,63 +56,13 @@ public static String autoId() {
5756
return builder.toString();
5857
}
5958

60-
/**
61-
* Utility function to compare booleans. Note that we can't use Boolean.compare because it's only
62-
* available after Android 19.
63-
*/
64-
public static int compareBooleans(boolean b1, boolean b2) {
65-
if (b1 == b2) {
66-
return 0;
67-
} else if (b1) {
68-
return 1;
69-
} else {
70-
return -1;
71-
}
72-
}
73-
74-
/**
75-
* Utility function to compare integers. Note that we can't use Integer.compare because it's only
76-
* available after Android 19.
77-
*/
78-
public static int compareIntegers(int i1, int i2) {
79-
if (i1 < i2) {
80-
return -1;
81-
} else if (i1 > i2) {
82-
return 1;
83-
} else {
84-
return 0;
85-
}
86-
}
87-
8859
/** Compare strings in UTF-8 encoded byte order */
8960
public static int compareUtf8Strings(String left, String right) {
9061
ByteString leftBytes = ByteString.copyFromUtf8(left);
9162
ByteString rightBytes = ByteString.copyFromUtf8(right);
9263
return compareByteStrings(leftBytes, rightBytes);
9364
}
9465

95-
/**
96-
* Utility function to compare longs. Note that we can't use Long.compare because it's only
97-
* available after Android 19.
98-
*/
99-
public static int compareLongs(long i1, long i2) {
100-
return NumberComparisonHelper.compareLongs(i1, i2);
101-
}
102-
103-
/** Utility function to compare doubles (using Firestore semantics for NaN). */
104-
public static int compareDoubles(double i1, double i2) {
105-
return NumberComparisonHelper.firestoreCompareDoubles(i1, i2);
106-
}
107-
108-
/** Compares a double and a long (using Firestore semantics for NaN). */
109-
public static int compareMixed(double doubleValue, long longValue) {
110-
return NumberComparisonHelper.firestoreCompareDoubleWithLong(doubleValue, longValue);
111-
}
112-
113-
public static <T extends Comparable<T>> Comparator<T> comparator() {
114-
return Comparable::compareTo;
115-
}
116-
11766
public static FirebaseFirestoreException exceptionFromStatus(Status error) {
11867
StatusException statusException = error.asException();
11968
return new FirebaseFirestoreException(
@@ -136,15 +85,6 @@ private static Exception convertStatusException(Exception e) {
13685
}
13786
}
13887

139-
/** Turns a Throwable into an exception, converting it from a StatusException if necessary. */
140-
public static Exception convertThrowableToException(Throwable t) {
141-
if (t instanceof Exception) {
142-
return Util.convertStatusException((Exception) t);
143-
} else {
144-
return new Exception(t);
145-
}
146-
}
147-
14888
private static final Continuation<Void, Void> VOID_ERROR_TRANSFORMER =
14989
task -> {
15090
if (task.isSuccessful()) {
@@ -237,7 +177,7 @@ public static int compareByteArrays(byte[] left, byte[] right) {
237177
}
238178
// Byte values are equal, continue with comparison
239179
}
240-
return Util.compareIntegers(left.length, right.length);
180+
return Integer.compare(left.length, right.length);
241181
}
242182

243183
public static int compareByteStrings(ByteString left, ByteString right) {
@@ -253,7 +193,8 @@ public static int compareByteStrings(ByteString left, ByteString right) {
253193
}
254194
// Byte values are equal, continue with comparison
255195
}
256-
return Util.compareIntegers(left.size(), right.size());
196+
int i1 = left.size();
197+
return Integer.compare(i1, right.size());
257198
}
258199

259200
public static StringBuilder repeatSequence(

0 commit comments

Comments
 (0)