Skip to content

Commit a6f880a

Browse files
schmidt-sebastianschmidt-sebastian
committed
Address feeback
1 parent 5d3f44d commit a6f880a

File tree

3 files changed

+79
-88
lines changed

3 files changed

+79
-88
lines changed

‎firebase-firestore/src/main/java/com/google/firebase/firestore/Blob.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,6 @@ public int hashCode() {
8282

8383
@Override
8484
public int compareTo(@NonNull Blob other) {
85-
return Util.compareByteString(bytes, other.bytes);
85+
return Util.compareByteStrings(bytes, other.bytes);
8686
}
8787
}

‎firebase-firestore/src/main/java/com/google/firebase/firestore/util/ProtoValueUtil.java renamed to ‎firebase-firestore/src/main/java/com/google/firebase/firestore/model/ProtoValues.java

Lines changed: 77 additions & 86 deletions
Original file line numberDiff line numberDiff line change
@@ -12,14 +12,14 @@
1212
// See the License for the specific language governing permissions and
1313
// limitations under the License.
1414

15-
package com.google.firebase.firestore.util;
15+
package com.google.firebase.firestore.model;
1616

1717
import static com.google.firebase.firestore.util.Assert.fail;
18-
import static com.google.firebase.firestore.util.Assert.hardAssert;
1918

2019
import androidx.annotation.Nullable;
2120
import com.google.common.base.Splitter;
2221
import com.google.firebase.firestore.model.value.FieldValue;
22+
import com.google.firebase.firestore.util.Util;
2323
import com.google.firestore.v1.ArrayValue;
2424
import com.google.firestore.v1.MapValue;
2525
import com.google.firestore.v1.Value;
@@ -28,7 +28,8 @@
2828
import java.util.Map;
2929
import java.util.TreeMap;
3030

31-
public class ProtoValueUtil {
31+
// TODO(mrschmidt): Make package-private
32+
public class ProtoValues {
3233

3334
public static int typeOrder(Value value) {
3435

@@ -73,35 +74,17 @@ public static boolean equals(Value left, Value right) {
7374
}
7475

7576
switch (leftType) {
76-
case FieldValue.TYPE_ORDER_ARRAY:
77-
return arrayEquals(left, right);
7877
case FieldValue.TYPE_ORDER_NUMBER:
7978
return numberEquals(left, right);
79+
case FieldValue.TYPE_ORDER_ARRAY:
80+
return arrayEquals(left, right);
8081
case FieldValue.TYPE_ORDER_OBJECT:
8182
return objectEquals(left, right);
8283
default:
8384
return left.equals(right);
8485
}
8586
}
8687

87-
private static boolean objectEquals(Value left, Value right) {
88-
MapValue leftMap = left.getMapValue();
89-
MapValue rightMap = right.getMapValue();
90-
91-
if (leftMap.getFieldsCount() != rightMap.getFieldsCount()) {
92-
return false;
93-
}
94-
95-
for (Map.Entry<String, Value> entry : leftMap.getFieldsMap().entrySet()) {
96-
Value otherEntry = rightMap.getFieldsMap().get(entry.getKey());
97-
if (!entry.getValue().equals(otherEntry)) {
98-
return false;
99-
}
100-
}
101-
102-
return true;
103-
}
104-
10588
private static boolean numberEquals(Value left, Value right) {
10689
if (left.getValueTypeCase() == Value.ValueTypeCase.INTEGER_VALUE
10790
&& right.getValueTypeCase() == Value.ValueTypeCase.INTEGER_VALUE) {
@@ -132,6 +115,24 @@ private static boolean arrayEquals(Value left, Value right) {
132115
return true;
133116
}
134117

118+
private static boolean objectEquals(Value left, Value right) {
119+
MapValue leftMap = left.getMapValue();
120+
MapValue rightMap = right.getMapValue();
121+
122+
if (leftMap.getFieldsCount() != rightMap.getFieldsCount()) {
123+
return false;
124+
}
125+
126+
for (Map.Entry<String, Value> entry : leftMap.getFieldsMap().entrySet()) {
127+
Value otherEntry = rightMap.getFieldsMap().get(entry.getKey());
128+
if (!entry.getValue().equals(otherEntry)) {
129+
return false;
130+
}
131+
}
132+
133+
return true;
134+
}
135+
135136
public static int compare(Value left, Value right) {
136137
int leftType = typeOrder(left);
137138
int rightType = typeOrder(right);
@@ -152,7 +153,7 @@ public static int compare(Value left, Value right) {
152153
case FieldValue.TYPE_ORDER_STRING:
153154
return left.getStringValue().compareTo(right.getStringValue());
154155
case FieldValue.TYPE_ORDER_BLOB:
155-
return Util.compareByteString(left.getBytesValue(), right.getBytesValue());
156+
return Util.compareByteStrings(left.getBytesValue(), right.getBytesValue());
156157
case FieldValue.TYPE_ORDER_REFERENCE:
157158
return compareReferences(left, right);
158159
case FieldValue.TYPE_ORDER_GEOPOINT:
@@ -166,39 +167,46 @@ public static int compare(Value left, Value right) {
166167
}
167168
}
168169

169-
private static int compareMaps(Value left, Value right) {
170-
Iterator<Map.Entry<String, Value>> iterator1 =
171-
new TreeMap<>(left.getMapValue().getFieldsMap()).entrySet().iterator();
172-
Iterator<Map.Entry<String, Value>> iterator2 =
173-
new TreeMap<>(right.getMapValue().getFieldsMap()).entrySet().iterator();
174-
while (iterator1.hasNext() && iterator2.hasNext()) {
175-
Map.Entry<String, Value> entry1 = iterator1.next();
176-
Map.Entry<String, Value> entry2 = iterator2.next();
177-
int keyCompare = entry1.getKey().compareTo(entry2.getKey());
178-
if (keyCompare != 0) {
179-
return keyCompare;
170+
private static int compareNumbers(Value left, Value right) {
171+
if (left.getValueTypeCase() == Value.ValueTypeCase.DOUBLE_VALUE) {
172+
double thisDouble = left.getDoubleValue();
173+
if (right.getValueTypeCase() == Value.ValueTypeCase.DOUBLE_VALUE) {
174+
return Util.compareDoubles(thisDouble, right.getDoubleValue());
175+
} else if (right.getValueTypeCase() == Value.ValueTypeCase.INTEGER_VALUE) {
176+
return Util.compareMixed(thisDouble, right.getIntegerValue());
180177
}
181-
int valueCompare = compare(entry1.getValue(), entry2.getValue());
182-
if (valueCompare != 0) {
183-
return valueCompare;
178+
} else if (left.getValueTypeCase() == Value.ValueTypeCase.INTEGER_VALUE) {
179+
long thisLong = left.getIntegerValue();
180+
if (right.getValueTypeCase() == Value.ValueTypeCase.INTEGER_VALUE) {
181+
return Util.compareLongs(thisLong, right.getIntegerValue());
182+
} else if (right.getValueTypeCase() == Value.ValueTypeCase.DOUBLE_VALUE) {
183+
return -1 * Util.compareMixed(right.getDoubleValue(), thisLong);
184184
}
185185
}
186186

187-
// Only equal if both iterators are exhausted.
188-
return Util.compareBooleans(iterator1.hasNext(), iterator2.hasNext());
187+
throw fail("Unexpected values: %s vs %s", left, right);
189188
}
190189

191-
private static int compareArrays(Value left, Value right) {
192-
int minLength =
193-
Math.min(left.getArrayValue().getValuesCount(), right.getArrayValue().getValuesCount());
190+
private static int compareTimestamps(Value left, Value right) {
191+
if (left.getTimestampValue().getSeconds() == right.getTimestampValue().getSeconds()) {
192+
return Integer.signum(
193+
left.getTimestampValue().getNanos() - right.getTimestampValue().getNanos());
194+
}
195+
return Long.signum(
196+
left.getTimestampValue().getSeconds() - right.getTimestampValue().getSeconds());
197+
}
198+
199+
private static int compareReferences(Value left, Value right) {
200+
List<String> leftSegments = Splitter.on('/').splitToList(left.getReferenceValue());
201+
List<String> rightSegments = Splitter.on('/').splitToList(right.getReferenceValue());
202+
int minLength = Math.min(leftSegments.size(), rightSegments.size());
194203
for (int i = 0; i < minLength; i++) {
195-
int cmp = compare(left.getArrayValue().getValues(i), right.getArrayValue().getValues(i));
204+
int cmp = leftSegments.get(i).compareTo(rightSegments.get(i));
196205
if (cmp != 0) {
197206
return cmp;
198207
}
199208
}
200-
return Util.compareIntegers(
201-
left.getArrayValue().getValuesCount(), right.getArrayValue().getValuesCount());
209+
return Util.compareIntegers(leftSegments.size(), rightSegments.size());
202210
}
203211

204212
private static int compareGeoPoints(Value left, Value right) {
@@ -212,55 +220,38 @@ private static int compareGeoPoints(Value left, Value right) {
212220
return comparison;
213221
}
214222

215-
private static int compareReferences(Value left, Value right) {
216-
List<String> leftSegments = Splitter.on('/').splitToList(left.getReferenceValue());
217-
List<String> rightSegments = Splitter.on('/').splitToList(right.getReferenceValue());
218-
int minLength = Math.min(leftSegments.size(), rightSegments.size());
223+
private static int compareArrays(Value left, Value right) {
224+
int minLength =
225+
Math.min(left.getArrayValue().getValuesCount(), right.getArrayValue().getValuesCount());
219226
for (int i = 0; i < minLength; i++) {
220-
int cmp = leftSegments.get(i).compareTo(rightSegments.get(i));
227+
int cmp = compare(left.getArrayValue().getValues(i), right.getArrayValue().getValues(i));
221228
if (cmp != 0) {
222229
return cmp;
223230
}
224231
}
225-
return Util.compareIntegers(leftSegments.size(), rightSegments.size());
226-
}
227-
228-
private static int compareTimestamps(Value left, Value right) {
229-
if (left.getTimestampValue().getSeconds() == right.getTimestampValue().getSeconds()) {
230-
return Integer.signum(
231-
left.getTimestampValue().getNanos() - right.getTimestampValue().getNanos());
232-
}
233-
return Long.signum(
234-
left.getTimestampValue().getSeconds() - right.getTimestampValue().getSeconds());
232+
return Util.compareIntegers(
233+
left.getArrayValue().getValuesCount(), right.getArrayValue().getValuesCount());
235234
}
236235

237-
private static int compareNumbers(Value left, Value right) {
238-
if (left.getValueTypeCase() == Value.ValueTypeCase.DOUBLE_VALUE) {
239-
double thisDouble = left.getDoubleValue();
240-
if (right.getValueTypeCase() == Value.ValueTypeCase.DOUBLE_VALUE) {
241-
return Util.compareDoubles(thisDouble, right.getDoubleValue());
242-
} else {
243-
hardAssert(
244-
right.getValueTypeCase() == Value.ValueTypeCase.INTEGER_VALUE,
245-
"Unexpected value type: %s",
246-
right);
247-
return Util.compareMixed(thisDouble, right.getIntegerValue());
236+
private static int compareMaps(Value left, Value right) {
237+
Iterator<Map.Entry<String, Value>> iterator1 =
238+
new TreeMap<>(left.getMapValue().getFieldsMap()).entrySet().iterator();
239+
Iterator<Map.Entry<String, Value>> iterator2 =
240+
new TreeMap<>(right.getMapValue().getFieldsMap()).entrySet().iterator();
241+
while (iterator1.hasNext() && iterator2.hasNext()) {
242+
Map.Entry<String, Value> entry1 = iterator1.next();
243+
Map.Entry<String, Value> entry2 = iterator2.next();
244+
int keyCompare = entry1.getKey().compareTo(entry2.getKey());
245+
if (keyCompare != 0) {
246+
return keyCompare;
248247
}
249-
} else {
250-
hardAssert(
251-
left.getValueTypeCase() == Value.ValueTypeCase.INTEGER_VALUE,
252-
"Unexpected value type: %s",
253-
left);
254-
long thisLong = left.getIntegerValue();
255-
if (right.getValueTypeCase() == Value.ValueTypeCase.INTEGER_VALUE) {
256-
return Util.compareLongs(thisLong, right.getIntegerValue());
257-
} else {
258-
hardAssert(
259-
right.getValueTypeCase() == Value.ValueTypeCase.DOUBLE_VALUE,
260-
"Unexpected value type: %s",
261-
right);
262-
return -1 * Util.compareMixed(right.getDoubleValue(), thisLong);
248+
int valueCompare = compare(entry1.getValue(), entry2.getValue());
249+
if (valueCompare != 0) {
250+
return valueCompare;
263251
}
264252
}
253+
254+
// Only equal if both iterators are exhausted.
255+
return Util.compareBooleans(iterator1.hasNext(), iterator2.hasNext());
265256
}
266257
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -218,7 +218,7 @@ public static void crashMainThread(RuntimeException exception) {
218218
});
219219
}
220220

221-
public static int compareByteString(ByteString left, ByteString right) {
221+
public static int compareByteStrings(ByteString left, ByteString right) {
222222
int size = Math.min(left.size(), right.size());
223223
for (int i = 0; i < size; i++) {
224224
// Make sure the bytes are unsigned

0 commit comments

Comments
 (0)