From 5d3f44dbe37c5ef91bb90e4ad6fbf9e71e68f2d3 Mon Sep 17 00:00:00 2001 From: Sebastian Schmidt Date: Thu, 23 Jan 2020 08:50:09 -0800 Subject: [PATCH 01/14] Adding Proto-based equality and comparison This will be used (and tested) in the follow-up CL that adds the FieldValue tpes. --- .../com/google/firebase/firestore/Blob.java | 14 +- .../firestore/local/SQLiteMutationQueue.java | 2 +- .../firestore/model/value/FieldValue.java | 20 +- .../firestore/util/ProtoValueUtil.java | 266 ++++++++++++++++++ .../google/firebase/firestore/util/Util.java | 21 +- 5 files changed, 294 insertions(+), 29 deletions(-) create mode 100644 firebase-firestore/src/main/java/com/google/firebase/firestore/util/ProtoValueUtil.java diff --git a/firebase-firestore/src/main/java/com/google/firebase/firestore/Blob.java b/firebase-firestore/src/main/java/com/google/firebase/firestore/Blob.java index 55a561dee32..2d182eef1b3 100644 --- a/firebase-firestore/src/main/java/com/google/firebase/firestore/Blob.java +++ b/firebase-firestore/src/main/java/com/google/firebase/firestore/Blob.java @@ -82,18 +82,6 @@ public int hashCode() { @Override public int compareTo(@NonNull Blob other) { - int size = Math.min(bytes.size(), other.bytes.size()); - for (int i = 0; i < size; i++) { - // Make sure the bytes are unsigned - int thisByte = bytes.byteAt(i) & 0xff; - int otherByte = other.bytes.byteAt(i) & 0xff; - if (thisByte < otherByte) { - return -1; - } else if (thisByte > otherByte) { - return 1; - } - // Byte values are equal, continue with comparison - } - return Util.compareIntegers(bytes.size(), other.bytes.size()); + return Util.compareByteString(bytes, other.bytes); } } diff --git a/firebase-firestore/src/main/java/com/google/firebase/firestore/local/SQLiteMutationQueue.java b/firebase-firestore/src/main/java/com/google/firebase/firestore/local/SQLiteMutationQueue.java index e906cf69003..48b304801e2 100644 --- a/firebase-firestore/src/main/java/com/google/firebase/firestore/local/SQLiteMutationQueue.java +++ b/firebase-firestore/src/main/java/com/google/firebase/firestore/local/SQLiteMutationQueue.java @@ -318,7 +318,7 @@ public List getAllMutationBatchesAffectingDocumentKeys( Collections.sort( result, (MutationBatch lhs, MutationBatch rhs) -> - Util.compareInts(lhs.getBatchId(), rhs.getBatchId())); + Util.compareIntegers(lhs.getBatchId(), rhs.getBatchId())); } return result; } diff --git a/firebase-firestore/src/main/java/com/google/firebase/firestore/model/value/FieldValue.java b/firebase-firestore/src/main/java/com/google/firebase/firestore/model/value/FieldValue.java index ae5485995a0..0ce1d37b8b9 100644 --- a/firebase-firestore/src/main/java/com/google/firebase/firestore/model/value/FieldValue.java +++ b/firebase-firestore/src/main/java/com/google/firebase/firestore/model/value/FieldValue.java @@ -42,17 +42,17 @@ */ public abstract class FieldValue implements Comparable { /** The order of types in Firestore; this order is defined by the backend. */ - static final int TYPE_ORDER_NULL = 0; + public static final int TYPE_ORDER_NULL = 0; - static final int TYPE_ORDER_BOOLEAN = 1; - static final int TYPE_ORDER_NUMBER = 2; - static final int TYPE_ORDER_TIMESTAMP = 3; - static final int TYPE_ORDER_STRING = 4; - static final int TYPE_ORDER_BLOB = 5; - static final int TYPE_ORDER_REFERENCE = 6; - static final int TYPE_ORDER_GEOPOINT = 7; - static final int TYPE_ORDER_ARRAY = 8; - static final int TYPE_ORDER_OBJECT = 9; + public static final int TYPE_ORDER_BOOLEAN = 1; + public static final int TYPE_ORDER_NUMBER = 2; + public static final int TYPE_ORDER_TIMESTAMP = 3; + public static final int TYPE_ORDER_STRING = 4; + public static final int TYPE_ORDER_BLOB = 5; + public static final int TYPE_ORDER_REFERENCE = 6; + public static final int TYPE_ORDER_GEOPOINT = 7; + public static final int TYPE_ORDER_ARRAY = 8; + public static final int TYPE_ORDER_OBJECT = 9; public abstract int typeOrder(); diff --git a/firebase-firestore/src/main/java/com/google/firebase/firestore/util/ProtoValueUtil.java b/firebase-firestore/src/main/java/com/google/firebase/firestore/util/ProtoValueUtil.java new file mode 100644 index 00000000000..001952622c2 --- /dev/null +++ b/firebase-firestore/src/main/java/com/google/firebase/firestore/util/ProtoValueUtil.java @@ -0,0 +1,266 @@ +// Copyright 2020 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.firebase.firestore.util; + +import static com.google.firebase.firestore.util.Assert.fail; +import static com.google.firebase.firestore.util.Assert.hardAssert; + +import androidx.annotation.Nullable; +import com.google.common.base.Splitter; +import com.google.firebase.firestore.model.value.FieldValue; +import com.google.firestore.v1.ArrayValue; +import com.google.firestore.v1.MapValue; +import com.google.firestore.v1.Value; +import java.util.Iterator; +import java.util.List; +import java.util.Map; +import java.util.TreeMap; + +public class ProtoValueUtil { + + public static int typeOrder(Value value) { + + switch (value.getValueTypeCase()) { + case NULL_VALUE: + return FieldValue.TYPE_ORDER_NULL; + case BOOLEAN_VALUE: + return FieldValue.TYPE_ORDER_BOOLEAN; + case INTEGER_VALUE: + return FieldValue.TYPE_ORDER_NUMBER; + case DOUBLE_VALUE: + return FieldValue.TYPE_ORDER_NUMBER; + case TIMESTAMP_VALUE: + return FieldValue.TYPE_ORDER_TIMESTAMP; + case STRING_VALUE: + return FieldValue.TYPE_ORDER_STRING; + case BYTES_VALUE: + return FieldValue.TYPE_ORDER_BLOB; + case REFERENCE_VALUE: + return FieldValue.TYPE_ORDER_REFERENCE; + case GEO_POINT_VALUE: + return FieldValue.TYPE_ORDER_GEOPOINT; + case ARRAY_VALUE: + return FieldValue.TYPE_ORDER_ARRAY; + case MAP_VALUE: + return FieldValue.TYPE_ORDER_OBJECT; + default: + throw fail("Invalid value type: " + value.getValueTypeCase()); + } + } + + /** Returns whether `value` is non-null and corresponds to the given type order. */ + public static boolean isType(@Nullable Value value, int typeOrder) { + return value != null && typeOrder(value) == typeOrder; + } + + public static boolean equals(Value left, Value right) { + int leftType = typeOrder(left); + int rightType = typeOrder(right); + if (leftType != rightType) { + return false; + } + + switch (leftType) { + case FieldValue.TYPE_ORDER_ARRAY: + return arrayEquals(left, right); + case FieldValue.TYPE_ORDER_NUMBER: + return numberEquals(left, right); + case FieldValue.TYPE_ORDER_OBJECT: + return objectEquals(left, right); + default: + return left.equals(right); + } + } + + private static boolean objectEquals(Value left, Value right) { + MapValue leftMap = left.getMapValue(); + MapValue rightMap = right.getMapValue(); + + if (leftMap.getFieldsCount() != rightMap.getFieldsCount()) { + return false; + } + + for (Map.Entry entry : leftMap.getFieldsMap().entrySet()) { + Value otherEntry = rightMap.getFieldsMap().get(entry.getKey()); + if (!entry.getValue().equals(otherEntry)) { + return false; + } + } + + return true; + } + + private static boolean numberEquals(Value left, Value right) { + if (left.getValueTypeCase() == Value.ValueTypeCase.INTEGER_VALUE + && right.getValueTypeCase() == Value.ValueTypeCase.INTEGER_VALUE) { + return left.equals(right); + } else if (left.getValueTypeCase() == Value.ValueTypeCase.DOUBLE_VALUE + && right.getValueTypeCase() == Value.ValueTypeCase.DOUBLE_VALUE) { + return Double.doubleToLongBits(left.getDoubleValue()) + == Double.doubleToLongBits(right.getDoubleValue()); + } + + return false; + } + + private static boolean arrayEquals(Value left, Value right) { + ArrayValue leftArray = left.getArrayValue(); + ArrayValue rightArray = right.getArrayValue(); + + if (leftArray.getValuesCount() != rightArray.getValuesCount()) { + return false; + } + + for (int i = 0; i < leftArray.getValuesCount(); ++i) { + if (!equals(leftArray.getValues(i), rightArray.getValues(i))) { + return false; + } + } + + return true; + } + + public static int compare(Value left, Value right) { + int leftType = typeOrder(left); + int rightType = typeOrder(right); + + if (leftType != rightType) { + return Util.compareIntegers(leftType, rightType); + } + + switch (leftType) { + case FieldValue.TYPE_ORDER_NULL: + return 0; + case FieldValue.TYPE_ORDER_BOOLEAN: + return Util.compareBooleans(left.getBooleanValue(), right.getBooleanValue()); + case FieldValue.TYPE_ORDER_NUMBER: + return compareNumbers(left, right); + case FieldValue.TYPE_ORDER_TIMESTAMP: + return compareTimestamps(left, right); + case FieldValue.TYPE_ORDER_STRING: + return left.getStringValue().compareTo(right.getStringValue()); + case FieldValue.TYPE_ORDER_BLOB: + return Util.compareByteString(left.getBytesValue(), right.getBytesValue()); + case FieldValue.TYPE_ORDER_REFERENCE: + return compareReferences(left, right); + case FieldValue.TYPE_ORDER_GEOPOINT: + return compareGeoPoints(left, right); + case FieldValue.TYPE_ORDER_ARRAY: + return compareArrays(left, right); + case FieldValue.TYPE_ORDER_OBJECT: + return compareMaps(left, right); + default: + throw fail("Invalid value type: " + leftType); + } + } + + private static int compareMaps(Value left, Value right) { + Iterator> iterator1 = + new TreeMap<>(left.getMapValue().getFieldsMap()).entrySet().iterator(); + Iterator> iterator2 = + new TreeMap<>(right.getMapValue().getFieldsMap()).entrySet().iterator(); + while (iterator1.hasNext() && iterator2.hasNext()) { + Map.Entry entry1 = iterator1.next(); + Map.Entry entry2 = iterator2.next(); + int keyCompare = entry1.getKey().compareTo(entry2.getKey()); + if (keyCompare != 0) { + return keyCompare; + } + int valueCompare = compare(entry1.getValue(), entry2.getValue()); + if (valueCompare != 0) { + return valueCompare; + } + } + + // Only equal if both iterators are exhausted. + return Util.compareBooleans(iterator1.hasNext(), iterator2.hasNext()); + } + + private static int compareArrays(Value left, Value right) { + int minLength = + Math.min(left.getArrayValue().getValuesCount(), right.getArrayValue().getValuesCount()); + for (int i = 0; i < minLength; i++) { + int cmp = compare(left.getArrayValue().getValues(i), right.getArrayValue().getValues(i)); + if (cmp != 0) { + return cmp; + } + } + return Util.compareIntegers( + left.getArrayValue().getValuesCount(), right.getArrayValue().getValuesCount()); + } + + private static int compareGeoPoints(Value left, Value right) { + int comparison = + Util.compareDoubles( + left.getGeoPointValue().getLatitude(), right.getGeoPointValue().getLatitude()); + if (comparison == 0) { + return Util.compareDoubles( + left.getGeoPointValue().getLongitude(), right.getGeoPointValue().getLongitude()); + } + return comparison; + } + + private static int compareReferences(Value left, Value right) { + List leftSegments = Splitter.on('/').splitToList(left.getReferenceValue()); + List rightSegments = Splitter.on('/').splitToList(right.getReferenceValue()); + int minLength = Math.min(leftSegments.size(), rightSegments.size()); + for (int i = 0; i < minLength; i++) { + int cmp = leftSegments.get(i).compareTo(rightSegments.get(i)); + if (cmp != 0) { + return cmp; + } + } + return Util.compareIntegers(leftSegments.size(), rightSegments.size()); + } + + private static int compareTimestamps(Value left, Value right) { + if (left.getTimestampValue().getSeconds() == right.getTimestampValue().getSeconds()) { + return Integer.signum( + left.getTimestampValue().getNanos() - right.getTimestampValue().getNanos()); + } + return Long.signum( + left.getTimestampValue().getSeconds() - right.getTimestampValue().getSeconds()); + } + + private static int compareNumbers(Value left, Value right) { + if (left.getValueTypeCase() == Value.ValueTypeCase.DOUBLE_VALUE) { + double thisDouble = left.getDoubleValue(); + if (right.getValueTypeCase() == Value.ValueTypeCase.DOUBLE_VALUE) { + return Util.compareDoubles(thisDouble, right.getDoubleValue()); + } else { + hardAssert( + right.getValueTypeCase() == Value.ValueTypeCase.INTEGER_VALUE, + "Unexpected value type: %s", + right); + return Util.compareMixed(thisDouble, right.getIntegerValue()); + } + } else { + hardAssert( + left.getValueTypeCase() == Value.ValueTypeCase.INTEGER_VALUE, + "Unexpected value type: %s", + left); + long thisLong = left.getIntegerValue(); + if (right.getValueTypeCase() == Value.ValueTypeCase.INTEGER_VALUE) { + return Util.compareLongs(thisLong, right.getIntegerValue()); + } else { + hardAssert( + right.getValueTypeCase() == Value.ValueTypeCase.DOUBLE_VALUE, + "Unexpected value type: %s", + right); + return -1 * Util.compareMixed(right.getDoubleValue(), thisLong); + } + } + } +} diff --git a/firebase-firestore/src/main/java/com/google/firebase/firestore/util/Util.java b/firebase-firestore/src/main/java/com/google/firebase/firestore/util/Util.java index 17fcdbb8692..6b1e5aaa988 100644 --- a/firebase-firestore/src/main/java/com/google/firebase/firestore/util/Util.java +++ b/firebase-firestore/src/main/java/com/google/firebase/firestore/util/Util.java @@ -86,11 +86,6 @@ public static int compareLongs(long i1, long i2) { return NumberComparisonHelper.compareLongs(i1, i2); } - /** Utility function to compare ints. See {@link #compareLongs} for rationale. */ - public static int compareInts(int i1, int i2) { - return NumberComparisonHelper.compareLongs(i1, i2); - } - /** Utility function to compare doubles (using Firestore semantics for NaN). */ public static int compareDoubles(double i1, double i2) { return NumberComparisonHelper.firestoreCompareDoubles(i1, i2); @@ -222,4 +217,20 @@ public static void crashMainThread(RuntimeException exception) { throw exception; }); } + + public static int compareByteString(ByteString left, ByteString right) { + int size = Math.min(left.size(), right.size()); + for (int i = 0; i < size; i++) { + // Make sure the bytes are unsigned + int thisByte = left.byteAt(i) & 0xff; + int otherByte = right.byteAt(i) & 0xff; + if (thisByte < otherByte) { + return -1; + } else if (thisByte > otherByte) { + return 1; + } + // Byte values are equal, continue with comparison + } + return Util.compareIntegers(left.size(), right.size()); + } } From 5d006f0d59f7fa8df29414201376a61e1b54a355 Mon Sep 17 00:00:00 2001 From: Sebastian Schmidt Date: Thu, 23 Jan 2020 08:57:17 -0800 Subject: [PATCH 02/14] Adding test utilities to create Value types This exposes a similar interface as wrap() and allows me to change the existing FieldValue test to use Value protos --- .../firebase/firestore/ValueHelper.java | 98 +++++++++++++++++++ 1 file changed, 98 insertions(+) create mode 100644 firebase-firestore/src/roboUtil/java/com/google/firebase/firestore/ValueHelper.java diff --git a/firebase-firestore/src/roboUtil/java/com/google/firebase/firestore/ValueHelper.java b/firebase-firestore/src/roboUtil/java/com/google/firebase/firestore/ValueHelper.java new file mode 100644 index 00000000000..23e3e6a2b67 --- /dev/null +++ b/firebase-firestore/src/roboUtil/java/com/google/firebase/firestore/ValueHelper.java @@ -0,0 +1,98 @@ +// Copyright 2020 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.firebase.firestore; + +import com.google.firebase.Timestamp; +import com.google.firebase.firestore.model.DatabaseId; +import com.google.firebase.firestore.model.DocumentKey; +import com.google.firestore.v1.ArrayValue; +import com.google.firestore.v1.Value; +import com.google.protobuf.NullValue; +import com.google.type.LatLng; +import java.util.List; + +/** Test helper to create Firestore Value protos from Java types. */ +public class ValueHelper { + + public static Value valueOf(Object o) { + if (o instanceof Value) { + return (Value) o; + } else if (o instanceof String) { + return (Value.newBuilder().setStringValue((String) o).build()); + } else if (o instanceof Integer) { + return (Value.newBuilder().setIntegerValue((long) (Integer) o).build()); + } else if (o instanceof Long) { + return (Value.newBuilder().setIntegerValue((Long) o).build()); + } else if (o instanceof Double) { + return (Value.newBuilder().setDoubleValue((Double) o).build()); + } else if (o instanceof Boolean) { + return (Value.newBuilder().setBooleanValue((Boolean) o).build()); + } else if (o instanceof Timestamp) { + Timestamp timestamp = (Timestamp) o; + return (Value.newBuilder() + .setTimestampValue( + com.google.protobuf.Timestamp.newBuilder() + .setSeconds(timestamp.getSeconds()) + .setNanos(timestamp.getNanoseconds()) + .build()) + .build()); + } else if (o instanceof GeoPoint) { + GeoPoint geoPoint = (GeoPoint) o; + return (Value.newBuilder() + .setGeoPointValue( + LatLng.newBuilder() + .setLatitude(geoPoint.getLatitude()) + .setLongitude(geoPoint.getLongitude()) + .build()) + .build()); + } else if (o instanceof Blob) { + return (Value.newBuilder().setBytesValue(((Blob) o).toByteString()).build()); + } else if (o instanceof DocumentReference) { + return (Value.newBuilder() + .setReferenceValue( + "projects/projectId/databases/(default)/documents/" + + ((DocumentReference) o).getPath()) + .build()); + } else if (o instanceof List) { + ArrayValue.Builder list = ArrayValue.newBuilder(); + for (Object element : (List) o) { + list.addValues(valueOf(element)); + } + return (Value.newBuilder().setArrayValue(list).build()); + } else if (o == null) { + return (Value.newBuilder().setNullValue(NullValue.NULL_VALUE).build()); + } + + throw new UnsupportedOperationException(); + } + + public static Value map(Object... entries) { + com.google.firestore.v1.MapValue.Builder builder = + com.google.firestore.v1.MapValue.newBuilder(); + for (int i = 0; i < entries.length; i += 2) { + builder.putFields((String) entries[i], valueOf(entries[i + 1])); + } + return Value.newBuilder().setMapValue(builder).build(); + } + + public static Value wrapRef(DatabaseId dbId, DocumentKey key) { + return Value.newBuilder() + .setReferenceValue( + String.format( + "projects/%s/databases/%s/documents/%s", + dbId.getProjectId(), dbId.getDatabaseId(), key.toString())) + .build(); + } +} From c315cdceb0a40023ddfcd0d966171b06c42e888d Mon Sep 17 00:00:00 2001 From: Sebastian Schmidt Date: Thu, 23 Jan 2020 11:14:44 -0800 Subject: [PATCH 03/14] Address feeback --- .../com/google/firebase/firestore/Blob.java | 2 +- .../ProtoValues.java} | 163 +++++++++--------- .../google/firebase/firestore/util/Util.java | 2 +- 3 files changed, 79 insertions(+), 88 deletions(-) rename firebase-firestore/src/main/java/com/google/firebase/firestore/{util/ProtoValueUtil.java => model/ProtoValues.java} (92%) diff --git a/firebase-firestore/src/main/java/com/google/firebase/firestore/Blob.java b/firebase-firestore/src/main/java/com/google/firebase/firestore/Blob.java index 2d182eef1b3..822fc4aabf9 100644 --- a/firebase-firestore/src/main/java/com/google/firebase/firestore/Blob.java +++ b/firebase-firestore/src/main/java/com/google/firebase/firestore/Blob.java @@ -82,6 +82,6 @@ public int hashCode() { @Override public int compareTo(@NonNull Blob other) { - return Util.compareByteString(bytes, other.bytes); + return Util.compareByteStrings(bytes, other.bytes); } } diff --git a/firebase-firestore/src/main/java/com/google/firebase/firestore/util/ProtoValueUtil.java b/firebase-firestore/src/main/java/com/google/firebase/firestore/model/ProtoValues.java similarity index 92% rename from firebase-firestore/src/main/java/com/google/firebase/firestore/util/ProtoValueUtil.java rename to firebase-firestore/src/main/java/com/google/firebase/firestore/model/ProtoValues.java index 001952622c2..daadfe84aa7 100644 --- a/firebase-firestore/src/main/java/com/google/firebase/firestore/util/ProtoValueUtil.java +++ b/firebase-firestore/src/main/java/com/google/firebase/firestore/model/ProtoValues.java @@ -12,14 +12,14 @@ // See the License for the specific language governing permissions and // limitations under the License. -package com.google.firebase.firestore.util; +package com.google.firebase.firestore.model; import static com.google.firebase.firestore.util.Assert.fail; -import static com.google.firebase.firestore.util.Assert.hardAssert; import androidx.annotation.Nullable; import com.google.common.base.Splitter; import com.google.firebase.firestore.model.value.FieldValue; +import com.google.firebase.firestore.util.Util; import com.google.firestore.v1.ArrayValue; import com.google.firestore.v1.MapValue; import com.google.firestore.v1.Value; @@ -28,7 +28,8 @@ import java.util.Map; import java.util.TreeMap; -public class ProtoValueUtil { +// TODO(mrschmidt): Make package-private +public class ProtoValues { public static int typeOrder(Value value) { @@ -73,10 +74,10 @@ public static boolean equals(Value left, Value right) { } switch (leftType) { - case FieldValue.TYPE_ORDER_ARRAY: - return arrayEquals(left, right); case FieldValue.TYPE_ORDER_NUMBER: return numberEquals(left, right); + case FieldValue.TYPE_ORDER_ARRAY: + return arrayEquals(left, right); case FieldValue.TYPE_ORDER_OBJECT: return objectEquals(left, right); default: @@ -84,24 +85,6 @@ public static boolean equals(Value left, Value right) { } } - private static boolean objectEquals(Value left, Value right) { - MapValue leftMap = left.getMapValue(); - MapValue rightMap = right.getMapValue(); - - if (leftMap.getFieldsCount() != rightMap.getFieldsCount()) { - return false; - } - - for (Map.Entry entry : leftMap.getFieldsMap().entrySet()) { - Value otherEntry = rightMap.getFieldsMap().get(entry.getKey()); - if (!entry.getValue().equals(otherEntry)) { - return false; - } - } - - return true; - } - private static boolean numberEquals(Value left, Value right) { if (left.getValueTypeCase() == Value.ValueTypeCase.INTEGER_VALUE && right.getValueTypeCase() == Value.ValueTypeCase.INTEGER_VALUE) { @@ -132,6 +115,24 @@ private static boolean arrayEquals(Value left, Value right) { return true; } + private static boolean objectEquals(Value left, Value right) { + MapValue leftMap = left.getMapValue(); + MapValue rightMap = right.getMapValue(); + + if (leftMap.getFieldsCount() != rightMap.getFieldsCount()) { + return false; + } + + for (Map.Entry entry : leftMap.getFieldsMap().entrySet()) { + Value otherEntry = rightMap.getFieldsMap().get(entry.getKey()); + if (!entry.getValue().equals(otherEntry)) { + return false; + } + } + + return true; + } + public static int compare(Value left, Value right) { int leftType = typeOrder(left); int rightType = typeOrder(right); @@ -152,7 +153,7 @@ public static int compare(Value left, Value right) { case FieldValue.TYPE_ORDER_STRING: return left.getStringValue().compareTo(right.getStringValue()); case FieldValue.TYPE_ORDER_BLOB: - return Util.compareByteString(left.getBytesValue(), right.getBytesValue()); + return Util.compareByteStrings(left.getBytesValue(), right.getBytesValue()); case FieldValue.TYPE_ORDER_REFERENCE: return compareReferences(left, right); case FieldValue.TYPE_ORDER_GEOPOINT: @@ -166,39 +167,46 @@ public static int compare(Value left, Value right) { } } - private static int compareMaps(Value left, Value right) { - Iterator> iterator1 = - new TreeMap<>(left.getMapValue().getFieldsMap()).entrySet().iterator(); - Iterator> iterator2 = - new TreeMap<>(right.getMapValue().getFieldsMap()).entrySet().iterator(); - while (iterator1.hasNext() && iterator2.hasNext()) { - Map.Entry entry1 = iterator1.next(); - Map.Entry entry2 = iterator2.next(); - int keyCompare = entry1.getKey().compareTo(entry2.getKey()); - if (keyCompare != 0) { - return keyCompare; + private static int compareNumbers(Value left, Value right) { + if (left.getValueTypeCase() == Value.ValueTypeCase.DOUBLE_VALUE) { + double thisDouble = left.getDoubleValue(); + if (right.getValueTypeCase() == Value.ValueTypeCase.DOUBLE_VALUE) { + return Util.compareDoubles(thisDouble, right.getDoubleValue()); + } else if (right.getValueTypeCase() == Value.ValueTypeCase.INTEGER_VALUE) { + return Util.compareMixed(thisDouble, right.getIntegerValue()); } - int valueCompare = compare(entry1.getValue(), entry2.getValue()); - if (valueCompare != 0) { - return valueCompare; + } else if (left.getValueTypeCase() == Value.ValueTypeCase.INTEGER_VALUE) { + long thisLong = left.getIntegerValue(); + if (right.getValueTypeCase() == Value.ValueTypeCase.INTEGER_VALUE) { + return Util.compareLongs(thisLong, right.getIntegerValue()); + } else if (right.getValueTypeCase() == Value.ValueTypeCase.DOUBLE_VALUE) { + return -1 * Util.compareMixed(right.getDoubleValue(), thisLong); } } - // Only equal if both iterators are exhausted. - return Util.compareBooleans(iterator1.hasNext(), iterator2.hasNext()); + throw fail("Unexpected values: %s vs %s", left, right); } - private static int compareArrays(Value left, Value right) { - int minLength = - Math.min(left.getArrayValue().getValuesCount(), right.getArrayValue().getValuesCount()); + private static int compareTimestamps(Value left, Value right) { + if (left.getTimestampValue().getSeconds() == right.getTimestampValue().getSeconds()) { + return Integer.signum( + left.getTimestampValue().getNanos() - right.getTimestampValue().getNanos()); + } + return Long.signum( + left.getTimestampValue().getSeconds() - right.getTimestampValue().getSeconds()); + } + + private static int compareReferences(Value left, Value right) { + List leftSegments = Splitter.on('/').splitToList(left.getReferenceValue()); + List rightSegments = Splitter.on('/').splitToList(right.getReferenceValue()); + int minLength = Math.min(leftSegments.size(), rightSegments.size()); for (int i = 0; i < minLength; i++) { - int cmp = compare(left.getArrayValue().getValues(i), right.getArrayValue().getValues(i)); + int cmp = leftSegments.get(i).compareTo(rightSegments.get(i)); if (cmp != 0) { return cmp; } } - return Util.compareIntegers( - left.getArrayValue().getValuesCount(), right.getArrayValue().getValuesCount()); + return Util.compareIntegers(leftSegments.size(), rightSegments.size()); } private static int compareGeoPoints(Value left, Value right) { @@ -212,55 +220,38 @@ private static int compareGeoPoints(Value left, Value right) { return comparison; } - private static int compareReferences(Value left, Value right) { - List leftSegments = Splitter.on('/').splitToList(left.getReferenceValue()); - List rightSegments = Splitter.on('/').splitToList(right.getReferenceValue()); - int minLength = Math.min(leftSegments.size(), rightSegments.size()); + private static int compareArrays(Value left, Value right) { + int minLength = + Math.min(left.getArrayValue().getValuesCount(), right.getArrayValue().getValuesCount()); for (int i = 0; i < minLength; i++) { - int cmp = leftSegments.get(i).compareTo(rightSegments.get(i)); + int cmp = compare(left.getArrayValue().getValues(i), right.getArrayValue().getValues(i)); if (cmp != 0) { return cmp; } } - return Util.compareIntegers(leftSegments.size(), rightSegments.size()); - } - - private static int compareTimestamps(Value left, Value right) { - if (left.getTimestampValue().getSeconds() == right.getTimestampValue().getSeconds()) { - return Integer.signum( - left.getTimestampValue().getNanos() - right.getTimestampValue().getNanos()); - } - return Long.signum( - left.getTimestampValue().getSeconds() - right.getTimestampValue().getSeconds()); + return Util.compareIntegers( + left.getArrayValue().getValuesCount(), right.getArrayValue().getValuesCount()); } - private static int compareNumbers(Value left, Value right) { - if (left.getValueTypeCase() == Value.ValueTypeCase.DOUBLE_VALUE) { - double thisDouble = left.getDoubleValue(); - if (right.getValueTypeCase() == Value.ValueTypeCase.DOUBLE_VALUE) { - return Util.compareDoubles(thisDouble, right.getDoubleValue()); - } else { - hardAssert( - right.getValueTypeCase() == Value.ValueTypeCase.INTEGER_VALUE, - "Unexpected value type: %s", - right); - return Util.compareMixed(thisDouble, right.getIntegerValue()); + private static int compareMaps(Value left, Value right) { + Iterator> iterator1 = + new TreeMap<>(left.getMapValue().getFieldsMap()).entrySet().iterator(); + Iterator> iterator2 = + new TreeMap<>(right.getMapValue().getFieldsMap()).entrySet().iterator(); + while (iterator1.hasNext() && iterator2.hasNext()) { + Map.Entry entry1 = iterator1.next(); + Map.Entry entry2 = iterator2.next(); + int keyCompare = entry1.getKey().compareTo(entry2.getKey()); + if (keyCompare != 0) { + return keyCompare; } - } else { - hardAssert( - left.getValueTypeCase() == Value.ValueTypeCase.INTEGER_VALUE, - "Unexpected value type: %s", - left); - long thisLong = left.getIntegerValue(); - if (right.getValueTypeCase() == Value.ValueTypeCase.INTEGER_VALUE) { - return Util.compareLongs(thisLong, right.getIntegerValue()); - } else { - hardAssert( - right.getValueTypeCase() == Value.ValueTypeCase.DOUBLE_VALUE, - "Unexpected value type: %s", - right); - return -1 * Util.compareMixed(right.getDoubleValue(), thisLong); + int valueCompare = compare(entry1.getValue(), entry2.getValue()); + if (valueCompare != 0) { + return valueCompare; } } + + // Only equal if both iterators are exhausted. + return Util.compareBooleans(iterator1.hasNext(), iterator2.hasNext()); } } diff --git a/firebase-firestore/src/main/java/com/google/firebase/firestore/util/Util.java b/firebase-firestore/src/main/java/com/google/firebase/firestore/util/Util.java index 6b1e5aaa988..14944321d52 100644 --- a/firebase-firestore/src/main/java/com/google/firebase/firestore/util/Util.java +++ b/firebase-firestore/src/main/java/com/google/firebase/firestore/util/Util.java @@ -218,7 +218,7 @@ public static void crashMainThread(RuntimeException exception) { }); } - public static int compareByteString(ByteString left, ByteString right) { + public static int compareByteStrings(ByteString left, ByteString right) { int size = Math.min(left.size(), right.size()); for (int i = 0; i < size; i++) { // Make sure the bytes are unsigned From a6f880ae8b4e4b11b91dfaeaec5b1ae86e40f12c Mon Sep 17 00:00:00 2001 From: Sebastian Schmidt Date: Thu, 23 Jan 2020 11:14:44 -0800 Subject: [PATCH 04/14] Address feeback --- .../com/google/firebase/firestore/Blob.java | 2 +- .../ProtoValues.java} | 163 +++++++++--------- .../google/firebase/firestore/util/Util.java | 2 +- 3 files changed, 79 insertions(+), 88 deletions(-) rename firebase-firestore/src/main/java/com/google/firebase/firestore/{util/ProtoValueUtil.java => model/ProtoValues.java} (92%) diff --git a/firebase-firestore/src/main/java/com/google/firebase/firestore/Blob.java b/firebase-firestore/src/main/java/com/google/firebase/firestore/Blob.java index 2d182eef1b3..822fc4aabf9 100644 --- a/firebase-firestore/src/main/java/com/google/firebase/firestore/Blob.java +++ b/firebase-firestore/src/main/java/com/google/firebase/firestore/Blob.java @@ -82,6 +82,6 @@ public int hashCode() { @Override public int compareTo(@NonNull Blob other) { - return Util.compareByteString(bytes, other.bytes); + return Util.compareByteStrings(bytes, other.bytes); } } diff --git a/firebase-firestore/src/main/java/com/google/firebase/firestore/util/ProtoValueUtil.java b/firebase-firestore/src/main/java/com/google/firebase/firestore/model/ProtoValues.java similarity index 92% rename from firebase-firestore/src/main/java/com/google/firebase/firestore/util/ProtoValueUtil.java rename to firebase-firestore/src/main/java/com/google/firebase/firestore/model/ProtoValues.java index 001952622c2..daadfe84aa7 100644 --- a/firebase-firestore/src/main/java/com/google/firebase/firestore/util/ProtoValueUtil.java +++ b/firebase-firestore/src/main/java/com/google/firebase/firestore/model/ProtoValues.java @@ -12,14 +12,14 @@ // See the License for the specific language governing permissions and // limitations under the License. -package com.google.firebase.firestore.util; +package com.google.firebase.firestore.model; import static com.google.firebase.firestore.util.Assert.fail; -import static com.google.firebase.firestore.util.Assert.hardAssert; import androidx.annotation.Nullable; import com.google.common.base.Splitter; import com.google.firebase.firestore.model.value.FieldValue; +import com.google.firebase.firestore.util.Util; import com.google.firestore.v1.ArrayValue; import com.google.firestore.v1.MapValue; import com.google.firestore.v1.Value; @@ -28,7 +28,8 @@ import java.util.Map; import java.util.TreeMap; -public class ProtoValueUtil { +// TODO(mrschmidt): Make package-private +public class ProtoValues { public static int typeOrder(Value value) { @@ -73,10 +74,10 @@ public static boolean equals(Value left, Value right) { } switch (leftType) { - case FieldValue.TYPE_ORDER_ARRAY: - return arrayEquals(left, right); case FieldValue.TYPE_ORDER_NUMBER: return numberEquals(left, right); + case FieldValue.TYPE_ORDER_ARRAY: + return arrayEquals(left, right); case FieldValue.TYPE_ORDER_OBJECT: return objectEquals(left, right); default: @@ -84,24 +85,6 @@ public static boolean equals(Value left, Value right) { } } - private static boolean objectEquals(Value left, Value right) { - MapValue leftMap = left.getMapValue(); - MapValue rightMap = right.getMapValue(); - - if (leftMap.getFieldsCount() != rightMap.getFieldsCount()) { - return false; - } - - for (Map.Entry entry : leftMap.getFieldsMap().entrySet()) { - Value otherEntry = rightMap.getFieldsMap().get(entry.getKey()); - if (!entry.getValue().equals(otherEntry)) { - return false; - } - } - - return true; - } - private static boolean numberEquals(Value left, Value right) { if (left.getValueTypeCase() == Value.ValueTypeCase.INTEGER_VALUE && right.getValueTypeCase() == Value.ValueTypeCase.INTEGER_VALUE) { @@ -132,6 +115,24 @@ private static boolean arrayEquals(Value left, Value right) { return true; } + private static boolean objectEquals(Value left, Value right) { + MapValue leftMap = left.getMapValue(); + MapValue rightMap = right.getMapValue(); + + if (leftMap.getFieldsCount() != rightMap.getFieldsCount()) { + return false; + } + + for (Map.Entry entry : leftMap.getFieldsMap().entrySet()) { + Value otherEntry = rightMap.getFieldsMap().get(entry.getKey()); + if (!entry.getValue().equals(otherEntry)) { + return false; + } + } + + return true; + } + public static int compare(Value left, Value right) { int leftType = typeOrder(left); int rightType = typeOrder(right); @@ -152,7 +153,7 @@ public static int compare(Value left, Value right) { case FieldValue.TYPE_ORDER_STRING: return left.getStringValue().compareTo(right.getStringValue()); case FieldValue.TYPE_ORDER_BLOB: - return Util.compareByteString(left.getBytesValue(), right.getBytesValue()); + return Util.compareByteStrings(left.getBytesValue(), right.getBytesValue()); case FieldValue.TYPE_ORDER_REFERENCE: return compareReferences(left, right); case FieldValue.TYPE_ORDER_GEOPOINT: @@ -166,39 +167,46 @@ public static int compare(Value left, Value right) { } } - private static int compareMaps(Value left, Value right) { - Iterator> iterator1 = - new TreeMap<>(left.getMapValue().getFieldsMap()).entrySet().iterator(); - Iterator> iterator2 = - new TreeMap<>(right.getMapValue().getFieldsMap()).entrySet().iterator(); - while (iterator1.hasNext() && iterator2.hasNext()) { - Map.Entry entry1 = iterator1.next(); - Map.Entry entry2 = iterator2.next(); - int keyCompare = entry1.getKey().compareTo(entry2.getKey()); - if (keyCompare != 0) { - return keyCompare; + private static int compareNumbers(Value left, Value right) { + if (left.getValueTypeCase() == Value.ValueTypeCase.DOUBLE_VALUE) { + double thisDouble = left.getDoubleValue(); + if (right.getValueTypeCase() == Value.ValueTypeCase.DOUBLE_VALUE) { + return Util.compareDoubles(thisDouble, right.getDoubleValue()); + } else if (right.getValueTypeCase() == Value.ValueTypeCase.INTEGER_VALUE) { + return Util.compareMixed(thisDouble, right.getIntegerValue()); } - int valueCompare = compare(entry1.getValue(), entry2.getValue()); - if (valueCompare != 0) { - return valueCompare; + } else if (left.getValueTypeCase() == Value.ValueTypeCase.INTEGER_VALUE) { + long thisLong = left.getIntegerValue(); + if (right.getValueTypeCase() == Value.ValueTypeCase.INTEGER_VALUE) { + return Util.compareLongs(thisLong, right.getIntegerValue()); + } else if (right.getValueTypeCase() == Value.ValueTypeCase.DOUBLE_VALUE) { + return -1 * Util.compareMixed(right.getDoubleValue(), thisLong); } } - // Only equal if both iterators are exhausted. - return Util.compareBooleans(iterator1.hasNext(), iterator2.hasNext()); + throw fail("Unexpected values: %s vs %s", left, right); } - private static int compareArrays(Value left, Value right) { - int minLength = - Math.min(left.getArrayValue().getValuesCount(), right.getArrayValue().getValuesCount()); + private static int compareTimestamps(Value left, Value right) { + if (left.getTimestampValue().getSeconds() == right.getTimestampValue().getSeconds()) { + return Integer.signum( + left.getTimestampValue().getNanos() - right.getTimestampValue().getNanos()); + } + return Long.signum( + left.getTimestampValue().getSeconds() - right.getTimestampValue().getSeconds()); + } + + private static int compareReferences(Value left, Value right) { + List leftSegments = Splitter.on('/').splitToList(left.getReferenceValue()); + List rightSegments = Splitter.on('/').splitToList(right.getReferenceValue()); + int minLength = Math.min(leftSegments.size(), rightSegments.size()); for (int i = 0; i < minLength; i++) { - int cmp = compare(left.getArrayValue().getValues(i), right.getArrayValue().getValues(i)); + int cmp = leftSegments.get(i).compareTo(rightSegments.get(i)); if (cmp != 0) { return cmp; } } - return Util.compareIntegers( - left.getArrayValue().getValuesCount(), right.getArrayValue().getValuesCount()); + return Util.compareIntegers(leftSegments.size(), rightSegments.size()); } private static int compareGeoPoints(Value left, Value right) { @@ -212,55 +220,38 @@ private static int compareGeoPoints(Value left, Value right) { return comparison; } - private static int compareReferences(Value left, Value right) { - List leftSegments = Splitter.on('/').splitToList(left.getReferenceValue()); - List rightSegments = Splitter.on('/').splitToList(right.getReferenceValue()); - int minLength = Math.min(leftSegments.size(), rightSegments.size()); + private static int compareArrays(Value left, Value right) { + int minLength = + Math.min(left.getArrayValue().getValuesCount(), right.getArrayValue().getValuesCount()); for (int i = 0; i < minLength; i++) { - int cmp = leftSegments.get(i).compareTo(rightSegments.get(i)); + int cmp = compare(left.getArrayValue().getValues(i), right.getArrayValue().getValues(i)); if (cmp != 0) { return cmp; } } - return Util.compareIntegers(leftSegments.size(), rightSegments.size()); - } - - private static int compareTimestamps(Value left, Value right) { - if (left.getTimestampValue().getSeconds() == right.getTimestampValue().getSeconds()) { - return Integer.signum( - left.getTimestampValue().getNanos() - right.getTimestampValue().getNanos()); - } - return Long.signum( - left.getTimestampValue().getSeconds() - right.getTimestampValue().getSeconds()); + return Util.compareIntegers( + left.getArrayValue().getValuesCount(), right.getArrayValue().getValuesCount()); } - private static int compareNumbers(Value left, Value right) { - if (left.getValueTypeCase() == Value.ValueTypeCase.DOUBLE_VALUE) { - double thisDouble = left.getDoubleValue(); - if (right.getValueTypeCase() == Value.ValueTypeCase.DOUBLE_VALUE) { - return Util.compareDoubles(thisDouble, right.getDoubleValue()); - } else { - hardAssert( - right.getValueTypeCase() == Value.ValueTypeCase.INTEGER_VALUE, - "Unexpected value type: %s", - right); - return Util.compareMixed(thisDouble, right.getIntegerValue()); + private static int compareMaps(Value left, Value right) { + Iterator> iterator1 = + new TreeMap<>(left.getMapValue().getFieldsMap()).entrySet().iterator(); + Iterator> iterator2 = + new TreeMap<>(right.getMapValue().getFieldsMap()).entrySet().iterator(); + while (iterator1.hasNext() && iterator2.hasNext()) { + Map.Entry entry1 = iterator1.next(); + Map.Entry entry2 = iterator2.next(); + int keyCompare = entry1.getKey().compareTo(entry2.getKey()); + if (keyCompare != 0) { + return keyCompare; } - } else { - hardAssert( - left.getValueTypeCase() == Value.ValueTypeCase.INTEGER_VALUE, - "Unexpected value type: %s", - left); - long thisLong = left.getIntegerValue(); - if (right.getValueTypeCase() == Value.ValueTypeCase.INTEGER_VALUE) { - return Util.compareLongs(thisLong, right.getIntegerValue()); - } else { - hardAssert( - right.getValueTypeCase() == Value.ValueTypeCase.DOUBLE_VALUE, - "Unexpected value type: %s", - right); - return -1 * Util.compareMixed(right.getDoubleValue(), thisLong); + int valueCompare = compare(entry1.getValue(), entry2.getValue()); + if (valueCompare != 0) { + return valueCompare; } } + + // Only equal if both iterators are exhausted. + return Util.compareBooleans(iterator1.hasNext(), iterator2.hasNext()); } } diff --git a/firebase-firestore/src/main/java/com/google/firebase/firestore/util/Util.java b/firebase-firestore/src/main/java/com/google/firebase/firestore/util/Util.java index 6b1e5aaa988..14944321d52 100644 --- a/firebase-firestore/src/main/java/com/google/firebase/firestore/util/Util.java +++ b/firebase-firestore/src/main/java/com/google/firebase/firestore/util/Util.java @@ -218,7 +218,7 @@ public static void crashMainThread(RuntimeException exception) { }); } - public static int compareByteString(ByteString left, ByteString right) { + public static int compareByteStrings(ByteString left, ByteString right) { int size = Math.min(left.size(), right.size()); for (int i = 0; i < size; i++) { // Make sure the bytes are unsigned From c8d1ce8f68fe2cca448d4965bf6ec5272b057c91 Mon Sep 17 00:00:00 2001 From: Sebastian Schmidt Date: Thu, 23 Jan 2020 11:53:14 -0800 Subject: [PATCH 05/14] Review feedback --- .../{ValueHelper.java => Values.java} | 67 +++++++++++-------- .../firestore/local/SQLiteSchemaTest.java | 2 +- 2 files changed, 39 insertions(+), 30 deletions(-) rename firebase-firestore/src/roboUtil/java/com/google/firebase/firestore/{ValueHelper.java => Values.java} (62%) diff --git a/firebase-firestore/src/roboUtil/java/com/google/firebase/firestore/ValueHelper.java b/firebase-firestore/src/roboUtil/java/com/google/firebase/firestore/Values.java similarity index 62% rename from firebase-firestore/src/roboUtil/java/com/google/firebase/firestore/ValueHelper.java rename to firebase-firestore/src/roboUtil/java/com/google/firebase/firestore/Values.java index 23e3e6a2b67..7aa94113963 100644 --- a/firebase-firestore/src/roboUtil/java/com/google/firebase/firestore/ValueHelper.java +++ b/firebase-firestore/src/roboUtil/java/com/google/firebase/firestore/Values.java @@ -22,62 +22,71 @@ import com.google.protobuf.NullValue; import com.google.type.LatLng; import java.util.List; +import java.util.Map; /** Test helper to create Firestore Value protos from Java types. */ -public class ValueHelper { +public class Values { + // TODO(mrschmidt): Move into UserDataConverter public static Value valueOf(Object o) { if (o instanceof Value) { return (Value) o; - } else if (o instanceof String) { - return (Value.newBuilder().setStringValue((String) o).build()); + } else if (o == null) { + return Value.newBuilder().setNullValue(NullValue.NULL_VALUE).build(); + } else if (o instanceof Boolean) { + return Value.newBuilder().setBooleanValue((Boolean) o).build(); } else if (o instanceof Integer) { - return (Value.newBuilder().setIntegerValue((long) (Integer) o).build()); + return Value.newBuilder().setIntegerValue((Integer) o).build(); } else if (o instanceof Long) { - return (Value.newBuilder().setIntegerValue((Long) o).build()); + return Value.newBuilder().setIntegerValue((Long) o).build(); } else if (o instanceof Double) { - return (Value.newBuilder().setDoubleValue((Double) o).build()); - } else if (o instanceof Boolean) { - return (Value.newBuilder().setBooleanValue((Boolean) o).build()); + return Value.newBuilder().setDoubleValue((Double) o).build(); } else if (o instanceof Timestamp) { Timestamp timestamp = (Timestamp) o; - return (Value.newBuilder() + return Value.newBuilder() .setTimestampValue( com.google.protobuf.Timestamp.newBuilder() .setSeconds(timestamp.getSeconds()) - .setNanos(timestamp.getNanoseconds()) - .build()) - .build()); + .setNanos(timestamp.getNanoseconds())) + .build(); + } else if (o instanceof String) { + return Value.newBuilder().setStringValue((String) o).build(); + } else if (o instanceof Blob) { + return Value.newBuilder().setBytesValue(((Blob) o).toByteString()).build(); + + } else if (o instanceof DocumentReference) { + return Value.newBuilder() + .setReferenceValue( + "projects/project/databases/(default)/documents/" + ((DocumentReference) o).getPath()) + .build(); } else if (o instanceof GeoPoint) { GeoPoint geoPoint = (GeoPoint) o; - return (Value.newBuilder() + return Value.newBuilder() .setGeoPointValue( LatLng.newBuilder() .setLatitude(geoPoint.getLatitude()) - .setLongitude(geoPoint.getLongitude()) - .build()) - .build()); - } else if (o instanceof Blob) { - return (Value.newBuilder().setBytesValue(((Blob) o).toByteString()).build()); - } else if (o instanceof DocumentReference) { - return (Value.newBuilder() - .setReferenceValue( - "projects/projectId/databases/(default)/documents/" - + ((DocumentReference) o).getPath()) - .build()); + .setLongitude(geoPoint.getLongitude())) + .build(); + } else if (o instanceof List) { ArrayValue.Builder list = ArrayValue.newBuilder(); for (Object element : (List) o) { list.addValues(valueOf(element)); } - return (Value.newBuilder().setArrayValue(list).build()); - } else if (o == null) { - return (Value.newBuilder().setNullValue(NullValue.NULL_VALUE).build()); + return Value.newBuilder().setArrayValue(list).build(); + } else if (o instanceof Map) { + com.google.firestore.v1.MapValue.Builder builder = + com.google.firestore.v1.MapValue.newBuilder(); + for (Map.Entry entry : ((Map) o).entrySet()) { + builder.putFields(entry.getKey(), valueOf(entry.getValue())); + } + return Value.newBuilder().setMapValue(builder).build(); } - throw new UnsupportedOperationException(); + throw new UnsupportedOperationException("Failed to serialize object: " + o); } + /** Creates a MapValue from a list of key/value arguments. */ public static Value map(Object... entries) { com.google.firestore.v1.MapValue.Builder builder = com.google.firestore.v1.MapValue.newBuilder(); @@ -87,7 +96,7 @@ public static Value map(Object... entries) { return Value.newBuilder().setMapValue(builder).build(); } - public static Value wrapRef(DatabaseId dbId, DocumentKey key) { + public static Value refValue(DatabaseId dbId, DocumentKey key) { return Value.newBuilder() .setReferenceValue( String.format( diff --git a/firebase-firestore/src/test/java/com/google/firebase/firestore/local/SQLiteSchemaTest.java b/firebase-firestore/src/test/java/com/google/firebase/firestore/local/SQLiteSchemaTest.java index 055d3f6961a..aa5a17d291d 100644 --- a/firebase-firestore/src/test/java/com/google/firebase/firestore/local/SQLiteSchemaTest.java +++ b/firebase-firestore/src/test/java/com/google/firebase/firestore/local/SQLiteSchemaTest.java @@ -268,7 +268,7 @@ private void addMutationBatch(SQLiteDatabase db, int batchId, String uid, String Write.newBuilder() .setUpdate( Document.newBuilder() - .setName("projects/projectId/databases/(default)/documents/" + doc))); + .setName("projects/project/databases/(default)/documents/" + doc))); } db.execSQL( From 074f88af805ba4e84c732fb25dff4683085e1cfd Mon Sep 17 00:00:00 2001 From: Sebastian Schmidt Date: Thu, 23 Jan 2020 14:08:02 -0800 Subject: [PATCH 06/14] Add Protobuf-backed FieldValue --- .../firebase/firestore/FirestoreTest.java | 20 ++ .../model/protovalue/ObjectValue.java | 266 ++++++++++++++++++ .../model/protovalue/PrimitiveValue.java | 149 ++++++++++ .../firestore/model/value/FieldValue.java | 15 + .../model/value/ServerTimestampValue.java | 5 +- .../firestore/model/FieldValueTest.java | 161 ++++++----- .../model/ObjectValueBuilderTest.java | 228 +++++++++++++++ 7 files changed, 767 insertions(+), 77 deletions(-) create mode 100644 firebase-firestore/src/main/java/com/google/firebase/firestore/model/protovalue/ObjectValue.java create mode 100644 firebase-firestore/src/main/java/com/google/firebase/firestore/model/protovalue/PrimitiveValue.java create mode 100644 firebase-firestore/src/test/java/com/google/firebase/firestore/model/ObjectValueBuilderTest.java diff --git a/firebase-firestore/src/androidTest/java/com/google/firebase/firestore/FirestoreTest.java b/firebase-firestore/src/androidTest/java/com/google/firebase/firestore/FirestoreTest.java index 405db082690..a38ee616b4d 100644 --- a/firebase-firestore/src/androidTest/java/com/google/firebase/firestore/FirestoreTest.java +++ b/firebase-firestore/src/androidTest/java/com/google/firebase/firestore/FirestoreTest.java @@ -169,6 +169,26 @@ public void testCanMergeEmptyObject() { listenerRegistration.remove(); } + @Test + public void testUpdateWithEmptyObjectReplacesAllFields() { + DocumentReference documentReference = testDocument(); + documentReference.set(map("a", "a")); + + waitFor(documentReference.update("a", Collections.emptyMap())); + DocumentSnapshot snapshot = waitFor(documentReference.get()); + assertEquals(map("a", Collections.emptyMap()), snapshot.getData()); + } + + @Test + public void testMergeWithEmptyObjectReplacesAllFields() { + DocumentReference documentReference = testDocument(); + documentReference.set(map("a", "a")); + + waitFor(documentReference.set(map("a", Collections.emptyMap()), SetOptions.merge())); + DocumentSnapshot snapshot = waitFor(documentReference.get()); + assertEquals(map("a", Collections.emptyMap()), snapshot.getData()); + } + @Test public void testCanDeleteFieldUsingMerge() { DocumentReference documentReference = testCollection("rooms").document("eros"); diff --git a/firebase-firestore/src/main/java/com/google/firebase/firestore/model/protovalue/ObjectValue.java b/firebase-firestore/src/main/java/com/google/firebase/firestore/model/protovalue/ObjectValue.java new file mode 100644 index 00000000000..230a65bef06 --- /dev/null +++ b/firebase-firestore/src/main/java/com/google/firebase/firestore/model/protovalue/ObjectValue.java @@ -0,0 +1,266 @@ +// Copyright 2020 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.firebase.firestore.model.protovalue; + +import static com.google.firebase.firestore.util.Assert.hardAssert; +import static com.google.firebase.firestore.util.ValueUtil.isType; + +import androidx.annotation.Nullable; +import com.google.firebase.firestore.model.FieldPath; +import com.google.firebase.firestore.model.mutation.FieldMask; +import com.google.firebase.firestore.model.value.FieldValue; +import com.google.firestore.v1.MapValue; +import com.google.firestore.v1.Value; +import java.util.HashSet; +import java.util.Iterator; +import java.util.Map; +import java.util.Set; +import java.util.SortedMap; +import java.util.TreeMap; + +public class ObjectValue extends PrimitiveValue { + private static final ObjectValue EMPTY_VALUE = + new ObjectValue( + com.google.firestore.v1.Value.newBuilder() + .setMapValue(com.google.firestore.v1.MapValue.getDefaultInstance()) + .build()); + + public static ObjectValue emptyObject() { + return EMPTY_VALUE; + } + + public ObjectValue(Value value) { + super(value); + hardAssert(isType(value, TYPE_ORDER_OBJECT), "ObjectValues must be backed by a MapValue"); + } + + /** + * Returns the value at the given path or null. + * + * @param fieldPath the path to search + * @return The value at the path or if there it doesn't exist. + */ + public @Nullable FieldValue get(FieldPath fieldPath) { + Value value = internalValue; + + for (int i = 0; i < fieldPath.length() - 1; ++i) { + value = value.getMapValue().getFieldsOrDefault(fieldPath.getSegment(i), null); + if (!isType(value, TYPE_ORDER_OBJECT)) { + return null; + } + } + + value = value.getMapValue().getFieldsOrDefault(fieldPath.getLastSegment(), null); + return value != null ? FieldValue.of(value) : null; + } + + /** Recursively extracts the FieldPaths that are set in this ObjectValue. */ + public FieldMask getFieldMask() { + return extractFieldMask(internalValue.getMapValue()); + } + + private FieldMask extractFieldMask(MapValue value) { + Set fields = new HashSet<>(); + for (Map.Entry entry : value.getFieldsMap().entrySet()) { + FieldPath currentPath = FieldPath.fromSingleSegment(entry.getKey()); + if (isType(entry.getValue(), TYPE_ORDER_OBJECT)) { + FieldMask nestedMask = extractFieldMask(entry.getValue().getMapValue()); + Set nestedFields = nestedMask.getMask(); + if (nestedFields.isEmpty()) { + // Preserve the empty map by adding it to the FieldMask. + fields.add(currentPath); + } else { + // For nested and non-empty ObjectValues, add the FieldPath of the leaf nodes. + for (FieldPath nestedPath : nestedFields) { + fields.add(currentPath.append(nestedPath)); + } + } + } else { + fields.add(currentPath); + } + } + return FieldMask.fromSet(fields); + } + + /** Creates a ObjectValue.Builder instance that is based on the current value. */ + public ObjectValue.Builder toBuilder() { + return new Builder(this); + } + + /** An ObjectValue.Builder provides APIs to set and delete fields from an ObjectValue. */ + public static class Builder { + + /** The existing data to mutate. */ + private ObjectValue baseObject; + + /** + * A list of FieldPath/Value pairs to apply to the base object. `null` values indicate field + * deletes. MapValues are expanded before they are stored in the overlay map, so that an entry + * exists for each leaf node. + */ + private SortedMap overlayMap; + + Builder(ObjectValue baseObject) { + this.baseObject = baseObject; + this.overlayMap = new TreeMap<>(); + } + + /** + * Sets the field to the provided value. + * + * @param path The field path to set. + * @param value The value to set. + * @return The current Builder instance. + */ + public Builder set(FieldPath path, Value value) { + hardAssert(!path.isEmpty(), "Cannot set field for empty path on ObjectValue"); + removeConflictingOverlays(path); + setOverlay(path, value); + return this; + } + + /** + * Removes the field at the specified path. If there is no field at the specified path nothing + * is changed. + * + * @param path The field path to remove + * @return The current Builder instance. + */ + public Builder delete(FieldPath path) { + hardAssert(!path.isEmpty(), "Cannot delete field for empty path on ObjectValue"); + removeConflictingOverlays(path); + setOverlay(path, null); + return this; + } + + /** Remove any existing overlays that would be replaced by setting `path` to a new value. */ + private void removeConflictingOverlays(FieldPath path) { + Iterator iterator = + overlayMap.subMap(path, createSuccessor(path)).keySet().iterator(); + while (iterator.hasNext()) { + iterator.next(); + iterator.remove(); + } + } + + /** + * Adds `value` to the overlay map at `path`. MapValues are recursively expanded into one + * overlay per leaf node. + */ + private void setOverlay(FieldPath path, @Nullable Value value) { + if (!isType(value, TYPE_ORDER_OBJECT) || value.getMapValue().getFieldsCount() == 0) { + overlayMap.put(path, value); + } else { + for (Map.Entry entry : value.getMapValue().getFieldsMap().entrySet()) { + setOverlay(path.append(entry.getKey()), entry.getValue()); + } + } + } + + /** Returns an ObjectValue with all mutations applied. */ + public ObjectValue build() { + if (overlayMap.isEmpty()) { + return baseObject; + } else { + MapValue.Builder result = baseObject.internalValue.getMapValue().toBuilder(); + applyOverlay(FieldPath.EMPTY_PATH, result); + return new ObjectValue(Value.newBuilder().setMapValue(result).build()); + } + } + + /** + * Applies any overlays from `overlayMap` that exist at `currentPath` to the `resultAtPath` map. + * Overlays are expanded recursively based on their location in the backing ObjectValue's + * subtree and are processed by nesting level. + * + *

Example: Overlays { 'a.b.c' : 'foo', 'a.b.d' : 'bar', 'a.e' : 'foobar' } + * + *

To apply these overlays, the methods first creates a MapValue.Builder for `a`. It then + * calls applyOverlay() with a current path of `a` and the newly created MapValue.Builder. In + * its second call, `applyOverlay` assigns `a.b` to a new MapBuilder and `a.e` to 'foobar'. The + * third call assigns `a.b.c` and `a.b.d` to the MapValue.Builder created in the second step. + * + *

The overall aim of this method is to minimize conversions between MapValues and their + * builders. + * + * @param currentPath The path at the current nesting level. Can be set toFieldValue.EMPTY_PATH + * to represent the root. + * @param resultAtPath A mutable copy of the existing data at the current nesting level. + * Overlays are applied to this argument. + * @return Whether any modifications were applied (in any part of the subtree under + * currentPath). + */ + private boolean applyOverlay(FieldPath currentPath, MapValue.Builder resultAtPath) { + // Extract the data that exists at or below the current path. Te extracted subtree is + // subdivided during each iteration. The iteration stops when the slice becomes empty. + SortedMap currentSlice = + currentPath.isEmpty() + ? overlayMap + : overlayMap.subMap(currentPath, createSuccessor(currentPath)); + + boolean modified = false; + + while (!currentSlice.isEmpty()) { + FieldPath fieldPath = currentSlice.firstKey(); + + if (fieldPath.length() == currentPath.length() + 1) { + // The key in the slice is a leaf node. We can apply the value directly. + String fieldName = fieldPath.getLastSegment(); + Value overlayValue = overlayMap.get(fieldPath); + if (overlayValue != null) { + resultAtPath.putFields(fieldName, overlayValue); + modified = true; + } else if (resultAtPath.containsFields(fieldName)) { + resultAtPath.removeFields(fieldName); + modified = true; + } + } else { + // Since we need a MapValue.Builder at each nesting level (e.g. to create the field for + // `a.b.c` we need to create a MapValue.Builder for `a` as well as `a.b`), we invoke + // applyOverlay() recursively with the next nesting level. + FieldPath nextSliceStart = fieldPath.keepFirst(currentPath.length() + 1); + @Nullable FieldValue existingValue = baseObject.get(nextSliceStart); + MapValue.Builder nextSliceBuilder = + existingValue instanceof ObjectValue + // If there is already data at the current path, base our modifications on top + // of the existing data. + ? ((ObjectValue) existingValue).internalValue.getMapValue().toBuilder() + : MapValue.newBuilder(); + modified = applyOverlay(nextSliceStart, nextSliceBuilder) || modified; + if (modified) { + // Only apply the result if a field has been modified. This avoids adding an empty + // map entry for deletes of non-existing fields. + resultAtPath.putFields( + nextSliceStart.getLastSegment(), + Value.newBuilder().setMapValue(nextSliceBuilder).build()); + } + } + + // Shrink the subtree to contain only values after the current field path. Note that we are + // still bound by the subtree created at the initial method invocation. The current loop + // exits when the subtree becomes empty. + currentSlice = currentSlice.tailMap(createSuccessor(fieldPath)); + } + + return modified; + } + + /** Create the first field path that is not part of the subtree created by `currentPath`. */ + private FieldPath createSuccessor(FieldPath currentPath) { + hardAssert(!currentPath.isEmpty(), "Can't create a successor for an empty path"); + return currentPath.popLast().append(currentPath.getLastSegment() + '0'); + } + } +} diff --git a/firebase-firestore/src/main/java/com/google/firebase/firestore/model/protovalue/PrimitiveValue.java b/firebase-firestore/src/main/java/com/google/firebase/firestore/model/protovalue/PrimitiveValue.java new file mode 100644 index 00000000000..05f3d396c55 --- /dev/null +++ b/firebase-firestore/src/main/java/com/google/firebase/firestore/model/protovalue/PrimitiveValue.java @@ -0,0 +1,149 @@ +// Copyright 2020 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.firebase.firestore.model.protovalue; + +import static com.google.firebase.firestore.util.Assert.fail; + +import androidx.annotation.NonNull; +import androidx.annotation.Nullable; +import com.google.firebase.Timestamp; +import com.google.firebase.firestore.Blob; +import com.google.firebase.firestore.GeoPoint; +import com.google.firebase.firestore.model.DocumentKey; +import com.google.firebase.firestore.model.ResourcePath; +import com.google.firebase.firestore.model.value.FieldValue; +import com.google.firebase.firestore.model.value.ServerTimestampValue; +import com.google.firebase.firestore.util.Assert; +import com.google.firebase.firestore.util.ValueUtil; +import com.google.firestore.v1.ArrayValue; +import com.google.firestore.v1.Value; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +public class PrimitiveValue extends FieldValue { + protected final Value internalValue; + + public PrimitiveValue(Value value) { + this.internalValue = value; + } + + @Override + public int typeOrder() { + return ValueUtil.typeOrder(internalValue); + } + + @Nullable + @Override + public Object value() { + return convertValue(internalValue); + } + + @Nullable + private Object convertValue(Value value) { + switch (value.getValueTypeCase()) { + case NULL_VALUE: + return null; + case BOOLEAN_VALUE: + return value.getBooleanValue(); + case INTEGER_VALUE: + return value.getIntegerValue(); + case DOUBLE_VALUE: + return value.getDoubleValue(); + case TIMESTAMP_VALUE: + return new Timestamp( + value.getTimestampValue().getSeconds(), value.getTimestampValue().getNanos()); + case STRING_VALUE: + return value.getStringValue(); + case BYTES_VALUE: + return Blob.fromByteString(value.getBytesValue()); + case REFERENCE_VALUE: + return convertReference(value.getReferenceValue()); + case GEO_POINT_VALUE: + return new GeoPoint( + value.getGeoPointValue().getLatitude(), value.getGeoPointValue().getLongitude()); + case ARRAY_VALUE: + return convertArray(value.getArrayValue()); + case MAP_VALUE: + return convertMap(value.getMapValue()); + default: + throw fail("Unknown value type: " + value.getValueTypeCase()); + } + } + + private Object convertReference(String value) { + // TODO(mrschmidt): Move `value()` and `convertValue()` to DocumentSnapshot, which would + // allow us to validate that the resource name points to the current project. + ResourcePath resourceName = ResourcePath.fromString(value); + Assert.hardAssert( + resourceName.length() > 4 && resourceName.getSegment(4).equals("documents"), + "Tried to deserialize invalid key %s", + resourceName); + return DocumentKey.fromPath(resourceName.popFirst(5)); + } + + private List convertArray(ArrayValue arrayValue) { + ArrayList result = new ArrayList<>(arrayValue.getValuesCount()); + for (Value v : arrayValue.getValuesList()) { + result.add(convertValue(v)); + } + return result; + } + + private Map convertMap(com.google.firestore.v1.MapValue mapValue) { + Map result = new HashMap<>(); + for (Map.Entry entry : mapValue.getFieldsMap().entrySet()) { + result.put(entry.getKey(), convertValue(entry.getValue())); + } + return result; + } + + @Override + public boolean equals(Object o) { + if (this == o) { + return true; + } + + if (o instanceof PrimitiveValue) { + PrimitiveValue value = (PrimitiveValue) o; + return ValueUtil.equals(this.internalValue, value.internalValue); + } + + return false; + } + + @Override + public int hashCode() { + return internalValue.hashCode(); + } + + @Override + public int compareTo(@NonNull FieldValue other) { + if (other instanceof PrimitiveValue) { + return ValueUtil.compare(this.internalValue, ((PrimitiveValue) other).internalValue); + } else if (ValueUtil.isType(this.internalValue, TYPE_ORDER_TIMESTAMP) + && other instanceof ServerTimestampValue) { + // TODO(mrschmidt): Handle timestamps directly in PrimitiveValue + return -1; + } else { + return defaultCompareTo(other); + } + } + + public Value toProto() { + return internalValue; + } +} diff --git a/firebase-firestore/src/main/java/com/google/firebase/firestore/model/value/FieldValue.java b/firebase-firestore/src/main/java/com/google/firebase/firestore/model/value/FieldValue.java index 0ce1d37b8b9..4feb7611a9f 100644 --- a/firebase-firestore/src/main/java/com/google/firebase/firestore/model/value/FieldValue.java +++ b/firebase-firestore/src/main/java/com/google/firebase/firestore/model/value/FieldValue.java @@ -18,7 +18,10 @@ import androidx.annotation.NonNull; import androidx.annotation.Nullable; +import com.google.firebase.firestore.model.protovalue.ObjectValue; +import com.google.firebase.firestore.model.protovalue.PrimitiveValue; import com.google.firebase.firestore.util.Util; +import com.google.firestore.v1.Value; /** * A field value represents a data type as stored by Firestore. @@ -41,6 +44,9 @@ * */ public abstract class FieldValue implements Comparable { + + // TODO(mrschmidt): Reduce visibility of these types once `protovalue` + // is merged into `value` package /** The order of types in Firestore; this order is defined by the backend. */ public static final int TYPE_ORDER_NULL = 0; @@ -54,6 +60,15 @@ public abstract class FieldValue implements Comparable { public static final int TYPE_ORDER_ARRAY = 8; public static final int TYPE_ORDER_OBJECT = 9; + /** Creates a new FieldValue based on the Protobuf Value. */ + public static FieldValue of(Value value) { + if (value.getValueTypeCase() == Value.ValueTypeCase.MAP_VALUE) { + return new ObjectValue(value); + } else { + return new PrimitiveValue(value); + } + } + public abstract int typeOrder(); /** diff --git a/firebase-firestore/src/main/java/com/google/firebase/firestore/model/value/ServerTimestampValue.java b/firebase-firestore/src/main/java/com/google/firebase/firestore/model/value/ServerTimestampValue.java index 81e68040d9e..73456fb4ba1 100644 --- a/firebase-firestore/src/main/java/com/google/firebase/firestore/model/value/ServerTimestampValue.java +++ b/firebase-firestore/src/main/java/com/google/firebase/firestore/model/value/ServerTimestampValue.java @@ -31,6 +31,9 @@ public final class ServerTimestampValue extends FieldValue { private final Timestamp localWriteTime; @Nullable private final FieldValue previousValue; + // TODO(mrschmidt): Represent ServerTimestamps as a PrimitiveType with a Map containing a private + // `__type__` field (or similar). + public ServerTimestampValue(Timestamp localWriteTime, @Nullable FieldValue previousValue) { this.localWriteTime = localWriteTime; this.previousValue = previousValue; @@ -86,7 +89,7 @@ public int hashCode() { public int compareTo(FieldValue o) { if (o instanceof ServerTimestampValue) { return localWriteTime.compareTo(((ServerTimestampValue) o).localWriteTime); - } else if (o instanceof TimestampValue) { + } else if (o.typeOrder() == TYPE_ORDER_TIMESTAMP) { // Server timestamps come after all concrete timestamps. return 1; } else { diff --git a/firebase-firestore/src/test/java/com/google/firebase/firestore/model/FieldValueTest.java b/firebase-firestore/src/test/java/com/google/firebase/firestore/model/FieldValueTest.java index 8423a883267..a0fff7e0c61 100644 --- a/firebase-firestore/src/test/java/com/google/firebase/firestore/model/FieldValueTest.java +++ b/firebase-firestore/src/test/java/com/google/firebase/firestore/model/FieldValueTest.java @@ -14,43 +14,35 @@ package com.google.firebase.firestore.model; +import static com.google.firebase.firestore.ValueUtil.map; +import static com.google.firebase.firestore.ValueUtil.valueOf; +import static com.google.firebase.firestore.ValueUtil.wrapRef; import static com.google.firebase.firestore.testutil.TestUtil.blob; import static com.google.firebase.firestore.testutil.TestUtil.dbId; import static com.google.firebase.firestore.testutil.TestUtil.field; import static com.google.firebase.firestore.testutil.TestUtil.fieldMask; import static com.google.firebase.firestore.testutil.TestUtil.key; -import static com.google.firebase.firestore.testutil.TestUtil.map; import static com.google.firebase.firestore.testutil.TestUtil.ref; -import static com.google.firebase.firestore.testutil.TestUtil.wrap; -import static com.google.firebase.firestore.testutil.TestUtil.wrapObject; +import static junit.framework.TestCase.assertTrue; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotEquals; import static org.junit.Assert.assertNull; -import static org.junit.Assert.assertTrue; import com.google.common.testing.EqualsTester; import com.google.firebase.Timestamp; import com.google.firebase.firestore.GeoPoint; +import com.google.firebase.firestore.ValueUtil; import com.google.firebase.firestore.model.mutation.FieldMask; -import com.google.firebase.firestore.model.value.BlobValue; -import com.google.firebase.firestore.model.value.BooleanValue; -import com.google.firebase.firestore.model.value.DoubleValue; +import com.google.firebase.firestore.model.protovalue.ObjectValue; +import com.google.firebase.firestore.model.protovalue.PrimitiveValue; import com.google.firebase.firestore.model.value.FieldValue; -import com.google.firebase.firestore.model.value.GeoPointValue; -import com.google.firebase.firestore.model.value.IntegerValue; -import com.google.firebase.firestore.model.value.NullValue; -import com.google.firebase.firestore.model.value.ObjectValue; -import com.google.firebase.firestore.model.value.ReferenceValue; import com.google.firebase.firestore.model.value.ServerTimestampValue; -import com.google.firebase.firestore.model.value.StringValue; -import com.google.firebase.firestore.model.value.TimestampValue; import com.google.firebase.firestore.testutil.ComparatorTester; +import com.google.firestore.v1.Value; import java.util.Arrays; import java.util.Calendar; import java.util.Date; -import java.util.Map; import java.util.TimeZone; -import java.util.TreeMap; import org.junit.Test; import org.junit.runner.RunWith; import org.robolectric.RobolectricTestRunner; @@ -74,10 +66,10 @@ public FieldValueTest() { @Test public void testExtractsFields() { - FieldValue val = wrapObject("foo", map("a", 1, "b", true, "c", "string")); - assertTrue(val instanceof ObjectValue); - ObjectValue obj = (ObjectValue) val; - assertTrue(obj.get(field("foo")) instanceof ObjectValue); + Value nestedValue = map("a", 1, "b", true, "c", "string"); + + ObjectValue obj = wrapObject("foo", nestedValue); + assertEquals(wrap(nestedValue), obj.get(field("foo"))); assertEquals(wrap(1), obj.get(field("foo.a"))); assertEquals(wrap(true), obj.get(field("foo.b"))); assertEquals(wrap("string"), obj.get(field("foo.c"))); @@ -89,7 +81,7 @@ public void testExtractsFields() { @Test public void testExtractsFieldMask() { - FieldValue val = + ObjectValue val = wrapObject( "a", "b", @@ -97,8 +89,7 @@ public void testExtractsFieldMask() { map("a", 1, "b", true, "c", "string", "nested", map("d", "e")), "emptymap", map()); - assertTrue(val instanceof ObjectValue); - FieldMask mask = ((ObjectValue) val).getFieldMask(); + FieldMask mask = val.getFieldMask(); assertEquals(fieldMask("a", "map.a", "map.b", "map.c", "map.nested.d", "emptymap"), mask); } @@ -115,7 +106,7 @@ public void testOverwritesExistingFields() { public void testAddsNewFields() { ObjectValue empty = ObjectValue.emptyObject(); ObjectValue mod = setField(empty, "a", wrap("mod")); - assertEquals(wrap(new TreeMap()), empty); + assertEquals(wrapObject(), empty); assertEquals(wrapObject("a", "mod"), mod); ObjectValue old = mod; @@ -127,8 +118,8 @@ public void testAddsNewFields() { @Test public void testAddsMultipleNewFields() { ObjectValue object = ObjectValue.emptyObject(); - object = object.toBuilder().set(field("a"), wrap("a")).build(); - object = object.toBuilder().set(field("b"), wrap("b")).set(field("c"), wrap("c")).build(); + object = object.toBuilder().set(field("a"), valueOf("a")).build(); + object = object.toBuilder().set(field("b"), valueOf("b")).set(field("c"), valueOf("c")).build(); assertEquals(wrapObject("a", "a", "b", "b", "c", "c"), object); } @@ -146,7 +137,7 @@ public void testImplicitlyCreatesObjects() { @Test public void testCanOverwritePrimitivesWithObjects() { ObjectValue old = wrapObject("a", map("b", "old")); - ObjectValue mod = setField(old, "a", wrapObject("b", "mod")); + ObjectValue mod = setField(old, "a", map("b", "mod")); assertNotEquals(old, mod); assertEquals(wrapObject("a", map("b", "old")), old); assertEquals(wrapObject("a", map("b", "mod")), mod); @@ -184,40 +175,40 @@ public void testDeletesHandleMissingKeys() { assertEquals(wrapObject("a", map("b", 1, "c", 2)), mod); mod = deleteField(old, "a.d"); - assertEquals(mod, old); + assertEquals(old, mod); assertEquals(wrapObject("a", map("b", 1, "c", 2)), mod); mod = deleteField(old, "a.b.c"); - assertEquals(mod, old); + assertEquals(old, mod); assertEquals(wrapObject("a", map("b", 1, "c", 2)), mod); } @Test public void testDeletesNestedKeys() { - Map orig = map("a", map("b", 1, "c", map("d", 2, "e", 3))); - ObjectValue old = wrapObject(orig); + Value orig = map("a", map("b", 1, "c", map("d", 2, "e", 3))); + ObjectValue old = (ObjectValue) FieldValue.of(orig); ObjectValue mod = deleteField(old, "a.c.d"); assertNotEquals(mod, old); - assertEquals(wrapObject(orig), old); + assertEquals(wrap(orig), old); - Map second = map("a", map("b", 1, "c", map("e", 3))); - assertEquals(wrapObject(second), mod); + Value second = map("a", map("b", 1, "c", map("e", 3))); + assertEquals(wrap(second), mod); old = mod; mod = deleteField(old, "a.c"); assertNotEquals(old, mod); - assertEquals(wrapObject(second), old); + assertEquals(wrap(second), old); - Map third = map("a", map("b", 1)); - assertEquals(wrapObject(third), mod); + Value third = map("a", map("b", 1)); + assertEquals(wrap(third), mod); old = mod; mod = deleteField(old, "a"); assertNotEquals(old, mod); - assertEquals(wrapObject(third), old); + assertEquals(wrap(third), old); assertEquals(ObjectValue.emptyObject(), mod); } @@ -233,46 +224,45 @@ public void testDeletesMultipleNewFields() { @Test public void testValueEquality() { new EqualsTester() - .addEqualityGroup(wrap(true), BooleanValue.valueOf(true)) - .addEqualityGroup(wrap(false), BooleanValue.valueOf(false)) - .addEqualityGroup(wrap(null), NullValue.nullValue()) + .addEqualityGroup(wrap(true), wrap(true)) + .addEqualityGroup(wrap(false), wrap(false)) + .addEqualityGroup(wrap(null), wrap(null)) .addEqualityGroup( - wrap(0.0 / 0.0), wrap(Double.longBitsToDouble(0x7ff8000000000000L)), DoubleValue.NaN) + wrap(0.0 / 0.0), wrap(Double.longBitsToDouble(0x7ff8000000000000L)), wrap(Double.NaN)) // -0.0 and 0.0 compareTo the same but are not equal. .addEqualityGroup(wrap(-0.0)) .addEqualityGroup(wrap(0.0)) - .addEqualityGroup(wrap(1), IntegerValue.valueOf(1L)) + .addEqualityGroup(wrap(1), wrap(1)) // Doubles and Longs aren't equal. - .addEqualityGroup(wrap(1.0), DoubleValue.valueOf(1.0)) - .addEqualityGroup(wrap(1.1), DoubleValue.valueOf(1.1)) - .addEqualityGroup(wrap(blob(0, 1, 2)), BlobValue.valueOf(blob(0, 1, 2))) + .addEqualityGroup(wrap(1.0), wrap(1.0)) + .addEqualityGroup(wrap(1.1), wrap(1.1)) + .addEqualityGroup(wrap(blob(0, 1, 2)), wrap(blob(0, 1, 2))) .addEqualityGroup(wrap(blob(0, 1))) - .addEqualityGroup(wrap("string"), StringValue.valueOf("string")) - .addEqualityGroup(StringValue.valueOf("strin")) + .addEqualityGroup(wrap("string"), wrap("string")) + .addEqualityGroup(wrap("strin")) // latin small letter e + combining acute accent - .addEqualityGroup(StringValue.valueOf("e\u0301b")) + .addEqualityGroup(wrap("e\u0301b")) // latin small letter e with acute accent - .addEqualityGroup(StringValue.valueOf("\u00e9a")) - .addEqualityGroup(wrap(date1), TimestampValue.valueOf(new Timestamp(date1))) - .addEqualityGroup(TimestampValue.valueOf(new Timestamp(date2))) + .addEqualityGroup(wrap("\u00e9a")) + .addEqualityGroup(wrap(new Timestamp(date1)), wrap(new Timestamp(date1))) + .addEqualityGroup(wrap(new Timestamp(date2))) // NOTE: ServerTimestampValues can't be parsed via wrap(). .addEqualityGroup( new ServerTimestampValue(new Timestamp(date1), null), new ServerTimestampValue(new Timestamp(date1), null)) .addEqualityGroup(new ServerTimestampValue(new Timestamp(date2), null)) - .addEqualityGroup(wrap(new GeoPoint(0, 1)), GeoPointValue.valueOf(new GeoPoint(0, 1))) - .addEqualityGroup(GeoPointValue.valueOf(new GeoPoint(1, 0))) - .addEqualityGroup( - wrap(ref("coll/doc1")), ReferenceValue.valueOf(dbId("project"), key("coll/doc1"))) - .addEqualityGroup(ReferenceValue.valueOf(dbId("project", "bar"), key("coll/doc2"))) - .addEqualityGroup(ReferenceValue.valueOf(dbId("project", "baz"), key("coll/doc2"))) + .addEqualityGroup(wrap(new GeoPoint(0, 1)), wrap(new GeoPoint(0, 1))) + .addEqualityGroup(wrap(new GeoPoint(1, 0))) + .addEqualityGroup(wrap(ref("coll/doc1")), wrap(ref("coll/doc1"))) + .addEqualityGroup(wrapRef(dbId("projectId", "bar"), key("coll/doc2"))) + .addEqualityGroup(wrapRef(dbId("projectId", "baz"), key("coll/doc2"))) .addEqualityGroup(wrap(Arrays.asList("foo", "bar")), wrap(Arrays.asList("foo", "bar"))) .addEqualityGroup(wrap(Arrays.asList("foo", "bar", "baz"))) .addEqualityGroup(wrap(Arrays.asList("foo"))) - .addEqualityGroup(wrapObject(map("bar", 1, "foo", 2)), wrapObject(map("foo", 2, "bar", 1))) - .addEqualityGroup(wrapObject(map("bar", 2, "foo", 1))) - .addEqualityGroup(wrapObject(map("bar", 1))) - .addEqualityGroup(wrapObject(map("foo", 1))) + .addEqualityGroup(wrapObject("bar", 1, "foo", 2), wrapObject("foo", 2, "bar", 1)) + .addEqualityGroup(wrapObject("bar", 2, "foo", 1)) + .addEqualityGroup(wrapObject("bar", 1)) + .addEqualityGroup(wrapObject("foo", 1)) .testEquals(); } @@ -311,8 +301,8 @@ public void testValueOrdering() { .addEqualityGroup(wrap(Double.POSITIVE_INFINITY)) // dates - .addEqualityGroup(wrap(date1)) - .addEqualityGroup(wrap(date2)) + .addEqualityGroup(wrap(new Timestamp(date1))) + .addEqualityGroup(wrap(new Timestamp(date2))) // server timestamps come after all concrete timestamps. // NOTE: server timestamps can't be parsed with wrap(). @@ -339,12 +329,12 @@ public void testValueOrdering() { .addEqualityGroup(wrap(blob(255))) // resource names - .addEqualityGroup(ReferenceValue.valueOf(dbId("p1", "d1"), key("c1/doc1"))) - .addEqualityGroup(ReferenceValue.valueOf(dbId("p1", "d1"), key("c1/doc2"))) - .addEqualityGroup(ReferenceValue.valueOf(dbId("p1", "d1"), key("c10/doc1"))) - .addEqualityGroup(ReferenceValue.valueOf(dbId("p1", "d1"), key("c2/doc1"))) - .addEqualityGroup(ReferenceValue.valueOf(dbId("p1", "d2"), key("c1/doc1"))) - .addEqualityGroup(ReferenceValue.valueOf(dbId("p2", "d1"), key("c1/doc1"))) + .addEqualityGroup(wrapRef(dbId("p1", "d1"), key("c1/doc1"))) + .addEqualityGroup(wrapRef(dbId("p1", "d1"), key("c1/doc2"))) + .addEqualityGroup(wrapRef(dbId("p1", "d1"), key("c10/doc1"))) + .addEqualityGroup(wrapRef(dbId("p1", "d1"), key("c2/doc1"))) + .addEqualityGroup(wrapRef(dbId("p1", "d2"), key("c1/doc1"))) + .addEqualityGroup(wrapRef(dbId("p2", "d1"), key("c1/doc1"))) // geo points .addEqualityGroup(wrap(new GeoPoint(-90, -180))) @@ -367,19 +357,38 @@ public void testValueOrdering() { .addEqualityGroup(wrap(Arrays.asList("foo", "0"))) // objects - .addEqualityGroup(wrapObject(map("bar", 0))) - .addEqualityGroup(wrapObject(map("bar", 0, "foo", 1))) - .addEqualityGroup(wrapObject(map("foo", 1))) - .addEqualityGroup(wrapObject(map("foo", 2))) - .addEqualityGroup(wrapObject(map("foo", "0"))) + .addEqualityGroup(wrapObject("bar", 0)) + .addEqualityGroup(wrapObject("bar", 0, "foo", 1)) + .addEqualityGroup(wrapObject("foo", 1)) + .addEqualityGroup(wrapObject("foo", 2)) + .addEqualityGroup(wrapObject("foo", "0")) .testCompare(); } - private ObjectValue setField(ObjectValue objectValue, String fieldPath, FieldValue value) { + private ObjectValue setField(ObjectValue objectValue, String fieldPath, PrimitiveValue value) { + return objectValue.toBuilder().set(field(fieldPath), value.toProto()).build(); + } + + private ObjectValue setField(ObjectValue objectValue, String fieldPath, Value value) { return objectValue.toBuilder().set(field(fieldPath), value).build(); } private ObjectValue deleteField(ObjectValue objectValue, String fieldPath) { return objectValue.toBuilder().delete(field(fieldPath)).build(); } + + // TODO(mrschmidt): Clean up the helpers and merge wrap() with TestUtil.wrap() + private ObjectValue wrapObject(Object... entries) { + FieldValue object = FieldValue.of(map(entries)); + assertTrue(object instanceof ObjectValue); + return (ObjectValue) object; + } + + private PrimitiveValue wrap(Object map) { + return (PrimitiveValue) FieldValue.of(ValueUtil.valueOf(map)); + } + + private PrimitiveValue wrapRef(DatabaseId dbId, DocumentKey key) { + return (PrimitiveValue) FieldValue.of(ValueUtil.wrapRef(dbId, key)); + } } diff --git a/firebase-firestore/src/test/java/com/google/firebase/firestore/model/ObjectValueBuilderTest.java b/firebase-firestore/src/test/java/com/google/firebase/firestore/model/ObjectValueBuilderTest.java new file mode 100644 index 00000000000..0ced5ae1737 --- /dev/null +++ b/firebase-firestore/src/test/java/com/google/firebase/firestore/model/ObjectValueBuilderTest.java @@ -0,0 +1,228 @@ +// Copyright 2020 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.firebase.firestore.model; + +import static com.google.firebase.firestore.ValueUtil.map; +import static com.google.firebase.firestore.testutil.TestUtil.field; +import static junit.framework.TestCase.assertEquals; +import static junit.framework.TestCase.assertTrue; + +import com.google.firebase.firestore.ValueUtil; +import com.google.firebase.firestore.model.protovalue.ObjectValue; +import com.google.firebase.firestore.model.value.FieldValue; +import com.google.firestore.v1.Value; +import java.util.Collections; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.robolectric.RobolectricTestRunner; +import org.robolectric.annotation.Config; + +@RunWith(RobolectricTestRunner.class) +@Config(manifest = Config.NONE) +public class ObjectValueBuilderTest { + private Value fooValue = ValueUtil.valueOf("foo"); + private Value barValue = ValueUtil.valueOf("bar"); + private Value emptyObject = ValueUtil.valueOf(Collections.emptyMap()); + + @Test + public void emptyBuilder() { + ObjectValue.Builder builder = ObjectValue.emptyObject().toBuilder(); + ObjectValue object = builder.build(); + assertEquals(wrapObject(), object); + } + + @Test + public void setSingleField() { + ObjectValue.Builder builder = ObjectValue.emptyObject().toBuilder(); + builder.set(field("foo"), fooValue); + ObjectValue object = builder.build(); + assertEquals(wrapObject("foo", fooValue), object); + } + + @Test + public void setEmptyObject() { + ObjectValue.Builder builder = ObjectValue.emptyObject().toBuilder(); + builder.set(field("foo"), emptyObject); + ObjectValue object = builder.build(); + assertEquals(wrapObject("foo", emptyObject), object); + } + + @Test + public void setMultipleFields() { + ObjectValue.Builder builder = ObjectValue.emptyObject().toBuilder(); + builder.set(field("foo"), fooValue); + builder.set(field("bar"), fooValue); + ObjectValue object = builder.build(); + assertEquals(wrapObject("foo", fooValue, "bar", fooValue), object); + } + + @Test + public void setSuccessorField() { + ObjectValue.Builder builder = ObjectValue.emptyObject().toBuilder(); + builder.set(field("a"), fooValue); + builder.set(field("a0"), fooValue); + ObjectValue object = builder.build(); + assertEquals(wrapObject("a", fooValue, "a0", fooValue), object); + } + + @Test + public void setNestedField() { + ObjectValue.Builder builder = ObjectValue.emptyObject().toBuilder(); + builder.set(field("a.b"), fooValue); + builder.set(field("c.d.e"), fooValue); + ObjectValue object = builder.build(); + assertEquals(wrapObject("a", map("b", fooValue), "c", map("d", map("e", fooValue))), object); + } + + @Test + public void setTwoFieldsInNestedObject() { + ObjectValue.Builder builder = ObjectValue.emptyObject().toBuilder(); + builder.set(field("a.b"), fooValue); + builder.set(field("a.c"), fooValue); + ObjectValue object = builder.build(); + assertEquals(wrapObject("a", map("b", fooValue, "c", fooValue)), object); + } + + @Test + public void setFieldInNestedObject() { + ObjectValue.Builder builder = ObjectValue.emptyObject().toBuilder(); + builder.set(field("a"), map("b", fooValue)); + builder.set(field("a.c"), fooValue); + ObjectValue object = builder.build(); + assertEquals(wrapObject("a", map("b", fooValue, "c", fooValue)), object); + } + + @Test + public void setNestedFieldMultipleTimes() { + ObjectValue.Builder builder = ObjectValue.emptyObject().toBuilder(); + builder.set(field("a.c"), fooValue); + builder.set(field("a"), map("b", fooValue)); + ObjectValue object = builder.build(); + assertEquals(wrapObject("a", map("b", fooValue)), object); + } + + @Test + public void setAndDeleteField() { + ObjectValue.Builder builder = ObjectValue.emptyObject().toBuilder(); + builder.set(field("foo"), fooValue); + builder.delete(field("foo")); + ObjectValue object = builder.build(); + assertEquals(wrapObject(), object); + } + + @Test + public void setAndDeleteNestedField() { + ObjectValue.Builder builder = ObjectValue.emptyObject().toBuilder(); + builder.set(field("a.b.c"), fooValue); + builder.set(field("a.b.d"), fooValue); + builder.set(field("f.g"), fooValue); + builder.set(field("h"), fooValue); + builder.delete(field("a.b.c")); + builder.delete(field("h")); + ObjectValue object = builder.build(); + assertEquals(wrapObject("a", map("b", map("d", fooValue)), "f", map("g", fooValue)), object); + } + + @Test + public void setSingleFieldInExistingObject() { + ObjectValue.Builder builder = wrapObject("a", fooValue).toBuilder(); + builder.set(field("b"), fooValue); + ObjectValue object = builder.build(); + assertEquals(wrapObject("a", fooValue, "b", fooValue), object); + } + + @Test + public void overwriteField() { + ObjectValue.Builder builder = wrapObject("a", fooValue).toBuilder(); + builder.set(field("a"), barValue); + ObjectValue object = builder.build(); + assertEquals(wrapObject("a", barValue), object); + } + + @Test + public void overwriteNestedFields() { + ObjectValue.Builder builder = + wrapObject("a", map("b", fooValue, "c", map("d", fooValue))).toBuilder(); + builder.set(field("a.b"), barValue); + builder.set(field("a.c.d"), barValue); + ObjectValue object = builder.build(); + assertEquals(wrapObject("a", map("b", barValue, "c", map("d", barValue))), object); + } + + @Test + public void overwriteDeeplyNestedField() { + ObjectValue.Builder builder = wrapObject("a", map("b", fooValue)).toBuilder(); + builder.set(field("a.b.c"), barValue); + ObjectValue object = builder.build(); + assertEquals(wrapObject("a", map("b", map("c", barValue))), object); + } + + @Test + public void mergeExistingObject() { + ObjectValue.Builder builder = wrapObject("a", map("b", fooValue)).toBuilder(); + builder.set(field("a.c"), fooValue); + ObjectValue object = builder.build(); + assertEquals(wrapObject("a", map("b", fooValue, "c", fooValue)), object); + } + + @Test + public void overwriteNestedObject() { + ObjectValue.Builder builder = + wrapObject("a", map("b", map("c", fooValue, "d", fooValue))).toBuilder(); + builder.set(field("a.b"), barValue); + ObjectValue object = builder.build(); + assertEquals(wrapObject("a", map("b", barValue)), object); + } + + @Test + public void deleteSingleField() { + ObjectValue.Builder builder = wrapObject("a", fooValue, "b", fooValue).toBuilder(); + builder.delete(field("a")); + ObjectValue object = builder.build(); + assertEquals(wrapObject("b", fooValue), object); + } + + @Test + public void deleteNestedObject() { + ObjectValue.Builder builder = + wrapObject("a", map("b", map("c", fooValue, "d", fooValue), "f", fooValue)).toBuilder(); + builder.delete(field("a.b")); + ObjectValue object = builder.build(); + assertEquals(wrapObject("a", map("f", fooValue)), object); + } + + @Test + public void deleteNonExistingField() { + ObjectValue.Builder builder = wrapObject("a", fooValue).toBuilder(); + builder.delete(field("b")); + ObjectValue object = builder.build(); + assertEquals(wrapObject("a", fooValue), object); + } + + @Test + public void deleteNonExistingNestedField() { + ObjectValue.Builder builder = wrapObject("a", map("b", fooValue)).toBuilder(); + builder.delete(field("a.b.c")); + ObjectValue object = builder.build(); + assertEquals(wrapObject("a", map("b", fooValue)), object); + } + + /** Creates a new ObjectValue based on key/value argument pairs. */ + private ObjectValue wrapObject(Object... entries) { + FieldValue object = FieldValue.of(map(entries)); + assertTrue(object instanceof ObjectValue); + return (ObjectValue) object; + } +} From 85e9fff50ba52fed1d64d01df56c1759eaaf6780 Mon Sep 17 00:00:00 2001 From: Sebastian Schmidt Date: Thu, 23 Jan 2020 16:27:46 -0800 Subject: [PATCH 07/14] Fix compile --- .../firestore/model/protovalue/ObjectValue.java | 4 ++-- .../firestore/model/protovalue/PrimitiveValue.java | 10 +++++----- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/firebase-firestore/src/main/java/com/google/firebase/firestore/model/protovalue/ObjectValue.java b/firebase-firestore/src/main/java/com/google/firebase/firestore/model/protovalue/ObjectValue.java index 230a65bef06..73cc52008ab 100644 --- a/firebase-firestore/src/main/java/com/google/firebase/firestore/model/protovalue/ObjectValue.java +++ b/firebase-firestore/src/main/java/com/google/firebase/firestore/model/protovalue/ObjectValue.java @@ -14,8 +14,8 @@ package com.google.firebase.firestore.model.protovalue; +import static com.google.firebase.firestore.model.ProtoValues.isType; import static com.google.firebase.firestore.util.Assert.hardAssert; -import static com.google.firebase.firestore.util.ValueUtil.isType; import androidx.annotation.Nullable; import com.google.firebase.firestore.model.FieldPath; @@ -185,7 +185,7 @@ public ObjectValue build() { * Overlays are expanded recursively based on their location in the backing ObjectValue's * subtree and are processed by nesting level. * - *

Example: Overlays { 'a.b.c' : 'foo', 'a.b.d' : 'bar', 'a.e' : 'foobar' } + *

Example: Overlays { 'a.b.c' : 'foo', 'a.b.d' : 'bar', 'a.e' : 'foobar' } * *

To apply these overlays, the methods first creates a MapValue.Builder for `a`. It then * calls applyOverlay() with a current path of `a` and the newly created MapValue.Builder. In diff --git a/firebase-firestore/src/main/java/com/google/firebase/firestore/model/protovalue/PrimitiveValue.java b/firebase-firestore/src/main/java/com/google/firebase/firestore/model/protovalue/PrimitiveValue.java index 05f3d396c55..4b52029a432 100644 --- a/firebase-firestore/src/main/java/com/google/firebase/firestore/model/protovalue/PrimitiveValue.java +++ b/firebase-firestore/src/main/java/com/google/firebase/firestore/model/protovalue/PrimitiveValue.java @@ -22,11 +22,11 @@ import com.google.firebase.firestore.Blob; import com.google.firebase.firestore.GeoPoint; import com.google.firebase.firestore.model.DocumentKey; +import com.google.firebase.firestore.model.ProtoValues; import com.google.firebase.firestore.model.ResourcePath; import com.google.firebase.firestore.model.value.FieldValue; import com.google.firebase.firestore.model.value.ServerTimestampValue; import com.google.firebase.firestore.util.Assert; -import com.google.firebase.firestore.util.ValueUtil; import com.google.firestore.v1.ArrayValue; import com.google.firestore.v1.Value; import java.util.ArrayList; @@ -43,7 +43,7 @@ public PrimitiveValue(Value value) { @Override public int typeOrder() { - return ValueUtil.typeOrder(internalValue); + return ProtoValues.typeOrder(internalValue); } @Nullable @@ -119,7 +119,7 @@ public boolean equals(Object o) { if (o instanceof PrimitiveValue) { PrimitiveValue value = (PrimitiveValue) o; - return ValueUtil.equals(this.internalValue, value.internalValue); + return ProtoValues.equals(this.internalValue, value.internalValue); } return false; @@ -133,8 +133,8 @@ public int hashCode() { @Override public int compareTo(@NonNull FieldValue other) { if (other instanceof PrimitiveValue) { - return ValueUtil.compare(this.internalValue, ((PrimitiveValue) other).internalValue); - } else if (ValueUtil.isType(this.internalValue, TYPE_ORDER_TIMESTAMP) + return ProtoValues.compare(this.internalValue, ((PrimitiveValue) other).internalValue); + } else if (ProtoValues.isType(this.internalValue, TYPE_ORDER_TIMESTAMP) && other instanceof ServerTimestampValue) { // TODO(mrschmidt): Handle timestamps directly in PrimitiveValue return -1; From 74f88959a003cf89a6623afd9658f06acb67d909 Mon Sep 17 00:00:00 2001 From: Sebastian Schmidt Date: Thu, 23 Jan 2020 16:41:49 -0800 Subject: [PATCH 08/14] More compile fixes --- .../firebase/firestore/model/FieldValueTest.java | 12 ++++++------ .../firestore/model/ObjectValueBuilderTest.java | 10 +++++----- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/firebase-firestore/src/test/java/com/google/firebase/firestore/model/FieldValueTest.java b/firebase-firestore/src/test/java/com/google/firebase/firestore/model/FieldValueTest.java index a0fff7e0c61..0fc977a510b 100644 --- a/firebase-firestore/src/test/java/com/google/firebase/firestore/model/FieldValueTest.java +++ b/firebase-firestore/src/test/java/com/google/firebase/firestore/model/FieldValueTest.java @@ -14,9 +14,9 @@ package com.google.firebase.firestore.model; -import static com.google.firebase.firestore.ValueUtil.map; -import static com.google.firebase.firestore.ValueUtil.valueOf; -import static com.google.firebase.firestore.ValueUtil.wrapRef; +import static com.google.firebase.firestore.Values.map; +import static com.google.firebase.firestore.Values.refValue; +import static com.google.firebase.firestore.Values.valueOf; import static com.google.firebase.firestore.testutil.TestUtil.blob; import static com.google.firebase.firestore.testutil.TestUtil.dbId; import static com.google.firebase.firestore.testutil.TestUtil.field; @@ -31,7 +31,7 @@ import com.google.common.testing.EqualsTester; import com.google.firebase.Timestamp; import com.google.firebase.firestore.GeoPoint; -import com.google.firebase.firestore.ValueUtil; +import com.google.firebase.firestore.Values; import com.google.firebase.firestore.model.mutation.FieldMask; import com.google.firebase.firestore.model.protovalue.ObjectValue; import com.google.firebase.firestore.model.protovalue.PrimitiveValue; @@ -385,10 +385,10 @@ private ObjectValue wrapObject(Object... entries) { } private PrimitiveValue wrap(Object map) { - return (PrimitiveValue) FieldValue.of(ValueUtil.valueOf(map)); + return (PrimitiveValue) FieldValue.of(valueOf(map)); } private PrimitiveValue wrapRef(DatabaseId dbId, DocumentKey key) { - return (PrimitiveValue) FieldValue.of(ValueUtil.wrapRef(dbId, key)); + return (PrimitiveValue) FieldValue.of(refValue(dbId, key)); } } diff --git a/firebase-firestore/src/test/java/com/google/firebase/firestore/model/ObjectValueBuilderTest.java b/firebase-firestore/src/test/java/com/google/firebase/firestore/model/ObjectValueBuilderTest.java index 0ced5ae1737..c68a4da50b7 100644 --- a/firebase-firestore/src/test/java/com/google/firebase/firestore/model/ObjectValueBuilderTest.java +++ b/firebase-firestore/src/test/java/com/google/firebase/firestore/model/ObjectValueBuilderTest.java @@ -14,12 +14,12 @@ package com.google.firebase.firestore.model; -import static com.google.firebase.firestore.ValueUtil.map; +import static com.google.firebase.firestore.Values.map; +import static com.google.firebase.firestore.Values.valueOf; import static com.google.firebase.firestore.testutil.TestUtil.field; import static junit.framework.TestCase.assertEquals; import static junit.framework.TestCase.assertTrue; -import com.google.firebase.firestore.ValueUtil; import com.google.firebase.firestore.model.protovalue.ObjectValue; import com.google.firebase.firestore.model.value.FieldValue; import com.google.firestore.v1.Value; @@ -32,9 +32,9 @@ @RunWith(RobolectricTestRunner.class) @Config(manifest = Config.NONE) public class ObjectValueBuilderTest { - private Value fooValue = ValueUtil.valueOf("foo"); - private Value barValue = ValueUtil.valueOf("bar"); - private Value emptyObject = ValueUtil.valueOf(Collections.emptyMap()); + private Value fooValue = valueOf("foo"); + private Value barValue = valueOf("bar"); + private Value emptyObject = valueOf(Collections.emptyMap()); @Test public void emptyBuilder() { From 618f903cc44c3d8e4dace9e9f24b6bd3b2758957 Mon Sep 17 00:00:00 2001 From: Sebastian Schmidt Date: Thu, 23 Jan 2020 17:14:23 -0800 Subject: [PATCH 09/14] Format --- .../java/com/google/firebase/firestore/model/FieldValueTest.java | 1 - 1 file changed, 1 deletion(-) diff --git a/firebase-firestore/src/test/java/com/google/firebase/firestore/model/FieldValueTest.java b/firebase-firestore/src/test/java/com/google/firebase/firestore/model/FieldValueTest.java index 0fc977a510b..a89e65c96e9 100644 --- a/firebase-firestore/src/test/java/com/google/firebase/firestore/model/FieldValueTest.java +++ b/firebase-firestore/src/test/java/com/google/firebase/firestore/model/FieldValueTest.java @@ -31,7 +31,6 @@ import com.google.common.testing.EqualsTester; import com.google.firebase.Timestamp; import com.google.firebase.firestore.GeoPoint; -import com.google.firebase.firestore.Values; import com.google.firebase.firestore.model.mutation.FieldMask; import com.google.firebase.firestore.model.protovalue.ObjectValue; import com.google.firebase.firestore.model.protovalue.PrimitiveValue; From c59910665915602fd659fec1b88947935cd8a011 Mon Sep 17 00:00:00 2001 From: Sebastian Schmidt Date: Thu, 23 Jan 2020 21:38:29 -0800 Subject: [PATCH 10/14] Remove duplicate file --- .../firebase/firestore/model/ProtoValues.java | 257 ------------------ 1 file changed, 257 deletions(-) delete mode 100644 firebase-firestore/src/main/java/com/google/firebase/firestore/model/ProtoValues.java diff --git a/firebase-firestore/src/main/java/com/google/firebase/firestore/model/ProtoValues.java b/firebase-firestore/src/main/java/com/google/firebase/firestore/model/ProtoValues.java deleted file mode 100644 index daadfe84aa7..00000000000 --- a/firebase-firestore/src/main/java/com/google/firebase/firestore/model/ProtoValues.java +++ /dev/null @@ -1,257 +0,0 @@ -// Copyright 2020 Google LLC -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package com.google.firebase.firestore.model; - -import static com.google.firebase.firestore.util.Assert.fail; - -import androidx.annotation.Nullable; -import com.google.common.base.Splitter; -import com.google.firebase.firestore.model.value.FieldValue; -import com.google.firebase.firestore.util.Util; -import com.google.firestore.v1.ArrayValue; -import com.google.firestore.v1.MapValue; -import com.google.firestore.v1.Value; -import java.util.Iterator; -import java.util.List; -import java.util.Map; -import java.util.TreeMap; - -// TODO(mrschmidt): Make package-private -public class ProtoValues { - - public static int typeOrder(Value value) { - - switch (value.getValueTypeCase()) { - case NULL_VALUE: - return FieldValue.TYPE_ORDER_NULL; - case BOOLEAN_VALUE: - return FieldValue.TYPE_ORDER_BOOLEAN; - case INTEGER_VALUE: - return FieldValue.TYPE_ORDER_NUMBER; - case DOUBLE_VALUE: - return FieldValue.TYPE_ORDER_NUMBER; - case TIMESTAMP_VALUE: - return FieldValue.TYPE_ORDER_TIMESTAMP; - case STRING_VALUE: - return FieldValue.TYPE_ORDER_STRING; - case BYTES_VALUE: - return FieldValue.TYPE_ORDER_BLOB; - case REFERENCE_VALUE: - return FieldValue.TYPE_ORDER_REFERENCE; - case GEO_POINT_VALUE: - return FieldValue.TYPE_ORDER_GEOPOINT; - case ARRAY_VALUE: - return FieldValue.TYPE_ORDER_ARRAY; - case MAP_VALUE: - return FieldValue.TYPE_ORDER_OBJECT; - default: - throw fail("Invalid value type: " + value.getValueTypeCase()); - } - } - - /** Returns whether `value` is non-null and corresponds to the given type order. */ - public static boolean isType(@Nullable Value value, int typeOrder) { - return value != null && typeOrder(value) == typeOrder; - } - - public static boolean equals(Value left, Value right) { - int leftType = typeOrder(left); - int rightType = typeOrder(right); - if (leftType != rightType) { - return false; - } - - switch (leftType) { - case FieldValue.TYPE_ORDER_NUMBER: - return numberEquals(left, right); - case FieldValue.TYPE_ORDER_ARRAY: - return arrayEquals(left, right); - case FieldValue.TYPE_ORDER_OBJECT: - return objectEquals(left, right); - default: - return left.equals(right); - } - } - - private static boolean numberEquals(Value left, Value right) { - if (left.getValueTypeCase() == Value.ValueTypeCase.INTEGER_VALUE - && right.getValueTypeCase() == Value.ValueTypeCase.INTEGER_VALUE) { - return left.equals(right); - } else if (left.getValueTypeCase() == Value.ValueTypeCase.DOUBLE_VALUE - && right.getValueTypeCase() == Value.ValueTypeCase.DOUBLE_VALUE) { - return Double.doubleToLongBits(left.getDoubleValue()) - == Double.doubleToLongBits(right.getDoubleValue()); - } - - return false; - } - - private static boolean arrayEquals(Value left, Value right) { - ArrayValue leftArray = left.getArrayValue(); - ArrayValue rightArray = right.getArrayValue(); - - if (leftArray.getValuesCount() != rightArray.getValuesCount()) { - return false; - } - - for (int i = 0; i < leftArray.getValuesCount(); ++i) { - if (!equals(leftArray.getValues(i), rightArray.getValues(i))) { - return false; - } - } - - return true; - } - - private static boolean objectEquals(Value left, Value right) { - MapValue leftMap = left.getMapValue(); - MapValue rightMap = right.getMapValue(); - - if (leftMap.getFieldsCount() != rightMap.getFieldsCount()) { - return false; - } - - for (Map.Entry entry : leftMap.getFieldsMap().entrySet()) { - Value otherEntry = rightMap.getFieldsMap().get(entry.getKey()); - if (!entry.getValue().equals(otherEntry)) { - return false; - } - } - - return true; - } - - public static int compare(Value left, Value right) { - int leftType = typeOrder(left); - int rightType = typeOrder(right); - - if (leftType != rightType) { - return Util.compareIntegers(leftType, rightType); - } - - switch (leftType) { - case FieldValue.TYPE_ORDER_NULL: - return 0; - case FieldValue.TYPE_ORDER_BOOLEAN: - return Util.compareBooleans(left.getBooleanValue(), right.getBooleanValue()); - case FieldValue.TYPE_ORDER_NUMBER: - return compareNumbers(left, right); - case FieldValue.TYPE_ORDER_TIMESTAMP: - return compareTimestamps(left, right); - case FieldValue.TYPE_ORDER_STRING: - return left.getStringValue().compareTo(right.getStringValue()); - case FieldValue.TYPE_ORDER_BLOB: - return Util.compareByteStrings(left.getBytesValue(), right.getBytesValue()); - case FieldValue.TYPE_ORDER_REFERENCE: - return compareReferences(left, right); - case FieldValue.TYPE_ORDER_GEOPOINT: - return compareGeoPoints(left, right); - case FieldValue.TYPE_ORDER_ARRAY: - return compareArrays(left, right); - case FieldValue.TYPE_ORDER_OBJECT: - return compareMaps(left, right); - default: - throw fail("Invalid value type: " + leftType); - } - } - - private static int compareNumbers(Value left, Value right) { - if (left.getValueTypeCase() == Value.ValueTypeCase.DOUBLE_VALUE) { - double thisDouble = left.getDoubleValue(); - if (right.getValueTypeCase() == Value.ValueTypeCase.DOUBLE_VALUE) { - return Util.compareDoubles(thisDouble, right.getDoubleValue()); - } else if (right.getValueTypeCase() == Value.ValueTypeCase.INTEGER_VALUE) { - return Util.compareMixed(thisDouble, right.getIntegerValue()); - } - } else if (left.getValueTypeCase() == Value.ValueTypeCase.INTEGER_VALUE) { - long thisLong = left.getIntegerValue(); - if (right.getValueTypeCase() == Value.ValueTypeCase.INTEGER_VALUE) { - return Util.compareLongs(thisLong, right.getIntegerValue()); - } else if (right.getValueTypeCase() == Value.ValueTypeCase.DOUBLE_VALUE) { - return -1 * Util.compareMixed(right.getDoubleValue(), thisLong); - } - } - - throw fail("Unexpected values: %s vs %s", left, right); - } - - private static int compareTimestamps(Value left, Value right) { - if (left.getTimestampValue().getSeconds() == right.getTimestampValue().getSeconds()) { - return Integer.signum( - left.getTimestampValue().getNanos() - right.getTimestampValue().getNanos()); - } - return Long.signum( - left.getTimestampValue().getSeconds() - right.getTimestampValue().getSeconds()); - } - - private static int compareReferences(Value left, Value right) { - List leftSegments = Splitter.on('/').splitToList(left.getReferenceValue()); - List rightSegments = Splitter.on('/').splitToList(right.getReferenceValue()); - int minLength = Math.min(leftSegments.size(), rightSegments.size()); - for (int i = 0; i < minLength; i++) { - int cmp = leftSegments.get(i).compareTo(rightSegments.get(i)); - if (cmp != 0) { - return cmp; - } - } - return Util.compareIntegers(leftSegments.size(), rightSegments.size()); - } - - private static int compareGeoPoints(Value left, Value right) { - int comparison = - Util.compareDoubles( - left.getGeoPointValue().getLatitude(), right.getGeoPointValue().getLatitude()); - if (comparison == 0) { - return Util.compareDoubles( - left.getGeoPointValue().getLongitude(), right.getGeoPointValue().getLongitude()); - } - return comparison; - } - - private static int compareArrays(Value left, Value right) { - int minLength = - Math.min(left.getArrayValue().getValuesCount(), right.getArrayValue().getValuesCount()); - for (int i = 0; i < minLength; i++) { - int cmp = compare(left.getArrayValue().getValues(i), right.getArrayValue().getValues(i)); - if (cmp != 0) { - return cmp; - } - } - return Util.compareIntegers( - left.getArrayValue().getValuesCount(), right.getArrayValue().getValuesCount()); - } - - private static int compareMaps(Value left, Value right) { - Iterator> iterator1 = - new TreeMap<>(left.getMapValue().getFieldsMap()).entrySet().iterator(); - Iterator> iterator2 = - new TreeMap<>(right.getMapValue().getFieldsMap()).entrySet().iterator(); - while (iterator1.hasNext() && iterator2.hasNext()) { - Map.Entry entry1 = iterator1.next(); - Map.Entry entry2 = iterator2.next(); - int keyCompare = entry1.getKey().compareTo(entry2.getKey()); - if (keyCompare != 0) { - return keyCompare; - } - int valueCompare = compare(entry1.getValue(), entry2.getValue()); - if (valueCompare != 0) { - return valueCompare; - } - } - - // Only equal if both iterators are exhausted. - return Util.compareBooleans(iterator1.hasNext(), iterator2.hasNext()); - } -} From 9354a50d365664965d5bdb50d6bbfe3330478bcc Mon Sep 17 00:00:00 2001 From: Sebastian Schmidt Date: Fri, 24 Jan 2020 09:02:13 -0800 Subject: [PATCH 11/14] Remove exceess newline --- .../src/roboUtil/java/com/google/firebase/firestore/Values.java | 1 - 1 file changed, 1 deletion(-) diff --git a/firebase-firestore/src/roboUtil/java/com/google/firebase/firestore/Values.java b/firebase-firestore/src/roboUtil/java/com/google/firebase/firestore/Values.java index 054813597a8..cb10ae4c20a 100644 --- a/firebase-firestore/src/roboUtil/java/com/google/firebase/firestore/Values.java +++ b/firebase-firestore/src/roboUtil/java/com/google/firebase/firestore/Values.java @@ -54,7 +54,6 @@ public static Value valueOf(Object o) { return Value.newBuilder().setStringValue((String) o).build(); } else if (o instanceof Blob) { return Value.newBuilder().setBytesValue(((Blob) o).toByteString()).build(); - } else if (o instanceof DocumentReference) { return Value.newBuilder() .setReferenceValue( From 2baaba1143ee179a10f278a541f7274b8cefa08f Mon Sep 17 00:00:00 2001 From: Sebastian Schmidt Date: Fri, 24 Jan 2020 09:18:38 -0800 Subject: [PATCH 12/14] Imports --- .../google/firebase/firestore/model/protovalue/ObjectValue.java | 2 +- .../firebase/firestore/model/protovalue/PrimitiveValue.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/firebase-firestore/src/main/java/com/google/firebase/firestore/model/protovalue/ObjectValue.java b/firebase-firestore/src/main/java/com/google/firebase/firestore/model/protovalue/ObjectValue.java index 73cc52008ab..7d760334770 100644 --- a/firebase-firestore/src/main/java/com/google/firebase/firestore/model/protovalue/ObjectValue.java +++ b/firebase-firestore/src/main/java/com/google/firebase/firestore/model/protovalue/ObjectValue.java @@ -14,7 +14,7 @@ package com.google.firebase.firestore.model.protovalue; -import static com.google.firebase.firestore.model.ProtoValues.isType; +import static com.google.firebase.firestore.model.value.ProtoValues.isType; import static com.google.firebase.firestore.util.Assert.hardAssert; import androidx.annotation.Nullable; diff --git a/firebase-firestore/src/main/java/com/google/firebase/firestore/model/protovalue/PrimitiveValue.java b/firebase-firestore/src/main/java/com/google/firebase/firestore/model/protovalue/PrimitiveValue.java index 4b52029a432..1dbde86906b 100644 --- a/firebase-firestore/src/main/java/com/google/firebase/firestore/model/protovalue/PrimitiveValue.java +++ b/firebase-firestore/src/main/java/com/google/firebase/firestore/model/protovalue/PrimitiveValue.java @@ -22,9 +22,9 @@ import com.google.firebase.firestore.Blob; import com.google.firebase.firestore.GeoPoint; import com.google.firebase.firestore.model.DocumentKey; -import com.google.firebase.firestore.model.ProtoValues; import com.google.firebase.firestore.model.ResourcePath; import com.google.firebase.firestore.model.value.FieldValue; +import com.google.firebase.firestore.model.value.ProtoValues; import com.google.firebase.firestore.model.value.ServerTimestampValue; import com.google.firebase.firestore.util.Assert; import com.google.firestore.v1.ArrayValue; From 7bf5e40e48c1f367e83e5d46fdc791dcb9dc2418 Mon Sep 17 00:00:00 2001 From: Sebastian Schmidt Date: Fri, 24 Jan 2020 11:03:56 -0800 Subject: [PATCH 13/14] Add package-info.java --- .../model/protovalue/package-info.java | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) create mode 100644 firebase-firestore/src/main/java/com/google/firebase/firestore/model/protovalue/package-info.java diff --git a/firebase-firestore/src/main/java/com/google/firebase/firestore/model/protovalue/package-info.java b/firebase-firestore/src/main/java/com/google/firebase/firestore/model/protovalue/package-info.java new file mode 100644 index 00000000000..8cce1a8e192 --- /dev/null +++ b/firebase-firestore/src/main/java/com/google/firebase/firestore/model/protovalue/package-info.java @@ -0,0 +1,19 @@ +// Copyright 2020 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +/** @hide */ +@RestrictTo(RestrictTo.Scope.LIBRARY_GROUP) +package com.google.firebase.firestore.model.protovalue; + +import androidx.annotation.RestrictTo; From 035970b6ce75f03b48449fa314c40688e3cefd2c Mon Sep 17 00:00:00 2001 From: Sebastian Schmidt Date: Tue, 28 Jan 2020 16:47:18 -0800 Subject: [PATCH 14/14] Review comments --- .../model/protovalue/ObjectValue.java | 201 ++++++++---------- .../model/protovalue/PrimitiveValue.java | 4 + .../firestore/model/FieldValueTest.java | 26 +-- .../model/ObjectValueBuilderTest.java | 89 ++++---- 4 files changed, 153 insertions(+), 167 deletions(-) diff --git a/firebase-firestore/src/main/java/com/google/firebase/firestore/model/protovalue/ObjectValue.java b/firebase-firestore/src/main/java/com/google/firebase/firestore/model/protovalue/ObjectValue.java index 7d760334770..08f4d9de0c3 100644 --- a/firebase-firestore/src/main/java/com/google/firebase/firestore/model/protovalue/ObjectValue.java +++ b/firebase-firestore/src/main/java/com/google/firebase/firestore/model/protovalue/ObjectValue.java @@ -23,12 +23,10 @@ import com.google.firebase.firestore.model.value.FieldValue; import com.google.firestore.v1.MapValue; import com.google.firestore.v1.Value; +import java.util.HashMap; import java.util.HashSet; -import java.util.Iterator; import java.util.Map; import java.util.Set; -import java.util.SortedMap; -import java.util.TreeMap; public class ObjectValue extends PrimitiveValue { private static final ObjectValue EMPTY_VALUE = @@ -37,13 +35,18 @@ public class ObjectValue extends PrimitiveValue { .setMapValue(com.google.firestore.v1.MapValue.getDefaultInstance()) .build()); + public ObjectValue(Value value) { + super(value); + hardAssert(isType(value, TYPE_ORDER_OBJECT), "ObjectValues must be backed by a MapValue"); + } + public static ObjectValue emptyObject() { return EMPTY_VALUE; } - public ObjectValue(Value value) { - super(value); - hardAssert(isType(value, TYPE_ORDER_OBJECT), "ObjectValues must be backed by a MapValue"); + /** Returns a new Builder instance that is based on an empty object. */ + public static ObjectValue.Builder newBuilder() { + return EMPTY_VALUE.toBuilder(); } /** @@ -53,17 +56,19 @@ public ObjectValue(Value value) { * @return The value at the path or if there it doesn't exist. */ public @Nullable FieldValue get(FieldPath fieldPath) { - Value value = internalValue; - - for (int i = 0; i < fieldPath.length() - 1; ++i) { - value = value.getMapValue().getFieldsOrDefault(fieldPath.getSegment(i), null); - if (!isType(value, TYPE_ORDER_OBJECT)) { - return null; + if (fieldPath.isEmpty()) { + return this; + } else { + Value value = internalValue; + for (int i = 0; i < fieldPath.length() - 1; ++i) { + value = value.getMapValue().getFieldsOrDefault(fieldPath.getSegment(i), null); + if (!isType(value, TYPE_ORDER_OBJECT)) { + return null; + } } + value = value.getMapValue().getFieldsOrDefault(fieldPath.getLastSegment(), null); + return value != null ? FieldValue.of(value) : null; } - - value = value.getMapValue().getFieldsOrDefault(fieldPath.getLastSegment(), null); - return value != null ? FieldValue.of(value) : null; } /** Recursively extracts the FieldPaths that are set in this ObjectValue. */ @@ -106,15 +111,15 @@ public static class Builder { private ObjectValue baseObject; /** - * A list of FieldPath/Value pairs to apply to the base object. `null` values indicate field - * deletes. MapValues are expanded before they are stored in the overlay map, so that an entry - * exists for each leaf node. + * A nested map that contains the accumulated changes in this builder. Values can either be + * `Value` protos, `Map` values (to represent additional nesting) or `null` (to + * represent field deletes). */ - private SortedMap overlayMap; + private Map overlayMap; Builder(ObjectValue baseObject) { this.baseObject = baseObject; - this.overlayMap = new TreeMap<>(); + this.overlayMap = new HashMap<>(); } /** @@ -126,7 +131,6 @@ public static class Builder { */ public Builder set(FieldPath path, Value value) { hardAssert(!path.isEmpty(), "Cannot set field for empty path on ObjectValue"); - removeConflictingOverlays(path); setOverlay(path, value); return this; } @@ -140,127 +144,94 @@ public Builder set(FieldPath path, Value value) { */ public Builder delete(FieldPath path) { hardAssert(!path.isEmpty(), "Cannot delete field for empty path on ObjectValue"); - removeConflictingOverlays(path); setOverlay(path, null); return this; } - /** Remove any existing overlays that would be replaced by setting `path` to a new value. */ - private void removeConflictingOverlays(FieldPath path) { - Iterator iterator = - overlayMap.subMap(path, createSuccessor(path)).keySet().iterator(); - while (iterator.hasNext()) { - iterator.next(); - iterator.remove(); - } - } - - /** - * Adds `value` to the overlay map at `path`. MapValues are recursively expanded into one - * overlay per leaf node. - */ + /** Adds `value` to the overlay map at `path`. Creates nested map entries if needed. */ private void setOverlay(FieldPath path, @Nullable Value value) { - if (!isType(value, TYPE_ORDER_OBJECT) || value.getMapValue().getFieldsCount() == 0) { - overlayMap.put(path, value); - } else { - for (Map.Entry entry : value.getMapValue().getFieldsMap().entrySet()) { - setOverlay(path.append(entry.getKey()), entry.getValue()); + Map currentLevel = overlayMap; + + for (int i = 0; i < path.length() - 1; ++i) { + String currentSegment = path.getSegment(i); + Object currentValue = currentLevel.get(currentSegment); + + if (currentValue instanceof Map) { + // Re-use a previously created map + currentLevel = (Map) currentValue; + } else if (currentValue instanceof Value + && ((Value) currentValue).getValueTypeCase() == Value.ValueTypeCase.MAP_VALUE) { + // Convert the existing Protobuf MapValue into a Java map + Map nextLevel = + new HashMap<>(((Value) currentValue).getMapValue().getFieldsMap()); + currentLevel.put(currentSegment, nextLevel); + currentLevel = nextLevel; + } else { + // Create an empty hash map to represent the current nesting level + Map nextLevel = new HashMap<>(); + currentLevel.put(currentSegment, nextLevel); + currentLevel = nextLevel; } } + + currentLevel.put(path.getLastSegment(), value); } /** Returns an ObjectValue with all mutations applied. */ public ObjectValue build() { - if (overlayMap.isEmpty()) { - return baseObject; + MapValue mergedResult = applyOverlay(FieldPath.EMPTY_PATH, overlayMap); + if (mergedResult != null) { + return new ObjectValue(Value.newBuilder().setMapValue(mergedResult).build()); } else { - MapValue.Builder result = baseObject.internalValue.getMapValue().toBuilder(); - applyOverlay(FieldPath.EMPTY_PATH, result); - return new ObjectValue(Value.newBuilder().setMapValue(result).build()); + return this.baseObject; } } /** - * Applies any overlays from `overlayMap` that exist at `currentPath` to the `resultAtPath` map. - * Overlays are expanded recursively based on their location in the backing ObjectValue's - * subtree and are processed by nesting level. - * - *

Example: Overlays { 'a.b.c' : 'foo', 'a.b.d' : 'bar', 'a.e' : 'foobar' } - * - *

To apply these overlays, the methods first creates a MapValue.Builder for `a`. It then - * calls applyOverlay() with a current path of `a` and the newly created MapValue.Builder. In - * its second call, `applyOverlay` assigns `a.b` to a new MapBuilder and `a.e` to 'foobar'. The - * third call assigns `a.b.c` and `a.b.d` to the MapValue.Builder created in the second step. - * - *

The overall aim of this method is to minimize conversions between MapValues and their - * builders. + * Applies any overlays from `currentOverlays` that exist at `currentPath` and returns the + * merged data at `currentPath` (or null if there were no changes). * * @param currentPath The path at the current nesting level. Can be set toFieldValue.EMPTY_PATH * to represent the root. - * @param resultAtPath A mutable copy of the existing data at the current nesting level. - * Overlays are applied to this argument. - * @return Whether any modifications were applied (in any part of the subtree under - * currentPath). + * @param currentOverlays The overlays at the current nesting level in the same format as + * `overlayMap`. + * @return The merged data at `currentPath` or null if no modifications were applied. */ - private boolean applyOverlay(FieldPath currentPath, MapValue.Builder resultAtPath) { - // Extract the data that exists at or below the current path. Te extracted subtree is - // subdivided during each iteration. The iteration stops when the slice becomes empty. - SortedMap currentSlice = - currentPath.isEmpty() - ? overlayMap - : overlayMap.subMap(currentPath, createSuccessor(currentPath)); - + private @Nullable MapValue applyOverlay( + FieldPath currentPath, Map currentOverlays) { boolean modified = false; - while (!currentSlice.isEmpty()) { - FieldPath fieldPath = currentSlice.firstKey(); - - if (fieldPath.length() == currentPath.length() + 1) { - // The key in the slice is a leaf node. We can apply the value directly. - String fieldName = fieldPath.getLastSegment(); - Value overlayValue = overlayMap.get(fieldPath); - if (overlayValue != null) { - resultAtPath.putFields(fieldName, overlayValue); - modified = true; - } else if (resultAtPath.containsFields(fieldName)) { - resultAtPath.removeFields(fieldName); + @Nullable FieldValue existingValue = baseObject.get(currentPath); + MapValue.Builder resultAtPath = + existingValue instanceof ObjectValue + // If there is already data at the current path, base our modifications on top + // of the existing data. + ? ((ObjectValue) existingValue).internalValue.getMapValue().toBuilder() + : MapValue.newBuilder(); + + for (Map.Entry entry : currentOverlays.entrySet()) { + String pathSegment = entry.getKey(); + Object value = entry.getValue(); + + if (value instanceof Map) { + @Nullable + MapValue nested = + applyOverlay(currentPath.append(pathSegment), (Map) value); + if (nested != null) { + resultAtPath.putFields(pathSegment, Value.newBuilder().setMapValue(nested).build()); modified = true; } - } else { - // Since we need a MapValue.Builder at each nesting level (e.g. to create the field for - // `a.b.c` we need to create a MapValue.Builder for `a` as well as `a.b`), we invoke - // applyOverlay() recursively with the next nesting level. - FieldPath nextSliceStart = fieldPath.keepFirst(currentPath.length() + 1); - @Nullable FieldValue existingValue = baseObject.get(nextSliceStart); - MapValue.Builder nextSliceBuilder = - existingValue instanceof ObjectValue - // If there is already data at the current path, base our modifications on top - // of the existing data. - ? ((ObjectValue) existingValue).internalValue.getMapValue().toBuilder() - : MapValue.newBuilder(); - modified = applyOverlay(nextSliceStart, nextSliceBuilder) || modified; - if (modified) { - // Only apply the result if a field has been modified. This avoids adding an empty - // map entry for deletes of non-existing fields. - resultAtPath.putFields( - nextSliceStart.getLastSegment(), - Value.newBuilder().setMapValue(nextSliceBuilder).build()); - } + } else if (value instanceof Value) { + resultAtPath.putFields(pathSegment, (Value) value); + modified = true; + } else if (resultAtPath.containsFields(pathSegment)) { + hardAssert(value == null, "Expected entry to be a Map, a Value or null"); + resultAtPath.removeFields(pathSegment); + modified = true; } - - // Shrink the subtree to contain only values after the current field path. Note that we are - // still bound by the subtree created at the initial method invocation. The current loop - // exits when the subtree becomes empty. - currentSlice = currentSlice.tailMap(createSuccessor(fieldPath)); } - return modified; - } - - /** Create the first field path that is not part of the subtree created by `currentPath`. */ - private FieldPath createSuccessor(FieldPath currentPath) { - hardAssert(!currentPath.isEmpty(), "Can't create a successor for an empty path"); - return currentPath.popLast().append(currentPath.getLastSegment() + '0'); + return modified ? resultAtPath.build() : null; } } } diff --git a/firebase-firestore/src/main/java/com/google/firebase/firestore/model/protovalue/PrimitiveValue.java b/firebase-firestore/src/main/java/com/google/firebase/firestore/model/protovalue/PrimitiveValue.java index 1dbde86906b..8baecfe0e3a 100644 --- a/firebase-firestore/src/main/java/com/google/firebase/firestore/model/protovalue/PrimitiveValue.java +++ b/firebase-firestore/src/main/java/com/google/firebase/firestore/model/protovalue/PrimitiveValue.java @@ -34,6 +34,10 @@ import java.util.List; import java.util.Map; +/** + * Represents a FieldValue that is backed by a single Firestore V1 Value proto and implements + * Firestore's Value semantics for ordering and equality. + */ public class PrimitiveValue extends FieldValue { protected final Value internalValue; diff --git a/firebase-firestore/src/test/java/com/google/firebase/firestore/model/FieldValueTest.java b/firebase-firestore/src/test/java/com/google/firebase/firestore/model/FieldValueTest.java index a89e65c96e9..fcc6e059fba 100644 --- a/firebase-firestore/src/test/java/com/google/firebase/firestore/model/FieldValueTest.java +++ b/firebase-firestore/src/test/java/com/google/firebase/firestore/model/FieldValueTest.java @@ -223,27 +223,29 @@ public void testDeletesMultipleNewFields() { @Test public void testValueEquality() { new EqualsTester() - .addEqualityGroup(wrap(true), wrap(true)) - .addEqualityGroup(wrap(false), wrap(false)) - .addEqualityGroup(wrap(null), wrap(null)) + .addEqualityGroup(wrap(true), wrap(valueOf(true))) + .addEqualityGroup(wrap(false), wrap(valueOf(false))) + .addEqualityGroup(wrap(null), wrap(valueOf(null))) .addEqualityGroup( - wrap(0.0 / 0.0), wrap(Double.longBitsToDouble(0x7ff8000000000000L)), wrap(Double.NaN)) + wrap(0.0 / 0.0), + wrap(Double.longBitsToDouble(0x7ff8000000000000L)), + wrap(valueOf(Double.NaN))) // -0.0 and 0.0 compareTo the same but are not equal. .addEqualityGroup(wrap(-0.0)) .addEqualityGroup(wrap(0.0)) - .addEqualityGroup(wrap(1), wrap(1)) + .addEqualityGroup(wrap(1), wrap(valueOf(1))) // Doubles and Longs aren't equal. - .addEqualityGroup(wrap(1.0), wrap(1.0)) - .addEqualityGroup(wrap(1.1), wrap(1.1)) - .addEqualityGroup(wrap(blob(0, 1, 2)), wrap(blob(0, 1, 2))) + .addEqualityGroup(wrap(1.0), wrap(valueOf(1.0))) + .addEqualityGroup(wrap(1.1), wrap(valueOf(1.1))) + .addEqualityGroup(wrap(blob(0, 1, 2)), wrap(valueOf(blob(0, 1, 2)))) .addEqualityGroup(wrap(blob(0, 1))) - .addEqualityGroup(wrap("string"), wrap("string")) + .addEqualityGroup(wrap("string"), wrap(valueOf("string"))) .addEqualityGroup(wrap("strin")) // latin small letter e + combining acute accent .addEqualityGroup(wrap("e\u0301b")) // latin small letter e with acute accent .addEqualityGroup(wrap("\u00e9a")) - .addEqualityGroup(wrap(new Timestamp(date1)), wrap(new Timestamp(date1))) + .addEqualityGroup(wrap(new Timestamp(date1)), wrap(valueOf(new Timestamp(date1)))) .addEqualityGroup(wrap(new Timestamp(date2))) // NOTE: ServerTimestampValues can't be parsed via wrap(). .addEqualityGroup( @@ -383,8 +385,8 @@ private ObjectValue wrapObject(Object... entries) { return (ObjectValue) object; } - private PrimitiveValue wrap(Object map) { - return (PrimitiveValue) FieldValue.of(valueOf(map)); + private PrimitiveValue wrap(Object value) { + return (PrimitiveValue) FieldValue.of(valueOf(value)); } private PrimitiveValue wrapRef(DatabaseId dbId, DocumentKey key) { diff --git a/firebase-firestore/src/test/java/com/google/firebase/firestore/model/ObjectValueBuilderTest.java b/firebase-firestore/src/test/java/com/google/firebase/firestore/model/ObjectValueBuilderTest.java index c68a4da50b7..e879110d45c 100644 --- a/firebase-firestore/src/test/java/com/google/firebase/firestore/model/ObjectValueBuilderTest.java +++ b/firebase-firestore/src/test/java/com/google/firebase/firestore/model/ObjectValueBuilderTest.java @@ -37,31 +37,31 @@ public class ObjectValueBuilderTest { private Value emptyObject = valueOf(Collections.emptyMap()); @Test - public void emptyBuilder() { - ObjectValue.Builder builder = ObjectValue.emptyObject().toBuilder(); + public void supportsEmptyBuilders() { + ObjectValue.Builder builder = ObjectValue.newBuilder(); ObjectValue object = builder.build(); - assertEquals(wrapObject(), object); + assertEquals(ObjectValue.emptyObject(), object); } @Test - public void setSingleField() { - ObjectValue.Builder builder = ObjectValue.emptyObject().toBuilder(); + public void setsSingleField() { + ObjectValue.Builder builder = ObjectValue.newBuilder(); builder.set(field("foo"), fooValue); ObjectValue object = builder.build(); assertEquals(wrapObject("foo", fooValue), object); } @Test - public void setEmptyObject() { - ObjectValue.Builder builder = ObjectValue.emptyObject().toBuilder(); + public void setsEmptyObject() { + ObjectValue.Builder builder = ObjectValue.newBuilder(); builder.set(field("foo"), emptyObject); ObjectValue object = builder.build(); assertEquals(wrapObject("foo", emptyObject), object); } @Test - public void setMultipleFields() { - ObjectValue.Builder builder = ObjectValue.emptyObject().toBuilder(); + public void setsMultipleFields() { + ObjectValue.Builder builder = ObjectValue.newBuilder(); builder.set(field("foo"), fooValue); builder.set(field("bar"), fooValue); ObjectValue object = builder.build(); @@ -69,17 +69,8 @@ public void setMultipleFields() { } @Test - public void setSuccessorField() { - ObjectValue.Builder builder = ObjectValue.emptyObject().toBuilder(); - builder.set(field("a"), fooValue); - builder.set(field("a0"), fooValue); - ObjectValue object = builder.build(); - assertEquals(wrapObject("a", fooValue, "a0", fooValue), object); - } - - @Test - public void setNestedField() { - ObjectValue.Builder builder = ObjectValue.emptyObject().toBuilder(); + public void setsNestedField() { + ObjectValue.Builder builder = ObjectValue.newBuilder(); builder.set(field("a.b"), fooValue); builder.set(field("c.d.e"), fooValue); ObjectValue object = builder.build(); @@ -87,8 +78,8 @@ public void setNestedField() { } @Test - public void setTwoFieldsInNestedObject() { - ObjectValue.Builder builder = ObjectValue.emptyObject().toBuilder(); + public void setsTwoFieldsInNestedObject() { + ObjectValue.Builder builder = ObjectValue.newBuilder(); builder.set(field("a.b"), fooValue); builder.set(field("a.c"), fooValue); ObjectValue object = builder.build(); @@ -96,8 +87,8 @@ public void setTwoFieldsInNestedObject() { } @Test - public void setFieldInNestedObject() { - ObjectValue.Builder builder = ObjectValue.emptyObject().toBuilder(); + public void setsFieldInNestedObject() { + ObjectValue.Builder builder = ObjectValue.newBuilder(); builder.set(field("a"), map("b", fooValue)); builder.set(field("a.c"), fooValue); ObjectValue object = builder.build(); @@ -105,8 +96,17 @@ public void setFieldInNestedObject() { } @Test - public void setNestedFieldMultipleTimes() { - ObjectValue.Builder builder = ObjectValue.emptyObject().toBuilder(); + public void setsDeeplyNestedFieldInNestedObject() { + ObjectValue.Builder builder = ObjectValue.newBuilder(); + builder.set(field("a.b.c.d.e.f"), fooValue); + ObjectValue object = builder.build(); + assertEquals( + wrapObject("a", map("b", map("c", map("d", map("e", map("f", fooValue)))))), object); + } + + @Test + public void setsNestedFieldMultipleTimes() { + ObjectValue.Builder builder = ObjectValue.newBuilder(); builder.set(field("a.c"), fooValue); builder.set(field("a"), map("b", fooValue)); ObjectValue object = builder.build(); @@ -114,8 +114,8 @@ public void setNestedFieldMultipleTimes() { } @Test - public void setAndDeleteField() { - ObjectValue.Builder builder = ObjectValue.emptyObject().toBuilder(); + public void setsAndDeletesField() { + ObjectValue.Builder builder = ObjectValue.newBuilder(); builder.set(field("foo"), fooValue); builder.delete(field("foo")); ObjectValue object = builder.build(); @@ -123,8 +123,8 @@ public void setAndDeleteField() { } @Test - public void setAndDeleteNestedField() { - ObjectValue.Builder builder = ObjectValue.emptyObject().toBuilder(); + public void setsAndDeletesNestedField() { + ObjectValue.Builder builder = ObjectValue.newBuilder(); builder.set(field("a.b.c"), fooValue); builder.set(field("a.b.d"), fooValue); builder.set(field("f.g"), fooValue); @@ -136,7 +136,7 @@ public void setAndDeleteNestedField() { } @Test - public void setSingleFieldInExistingObject() { + public void setsSingleFieldInExistingObject() { ObjectValue.Builder builder = wrapObject("a", fooValue).toBuilder(); builder.set(field("b"), fooValue); ObjectValue object = builder.build(); @@ -144,7 +144,7 @@ public void setSingleFieldInExistingObject() { } @Test - public void overwriteField() { + public void overwritesField() { ObjectValue.Builder builder = wrapObject("a", fooValue).toBuilder(); builder.set(field("a"), barValue); ObjectValue object = builder.build(); @@ -152,7 +152,7 @@ public void overwriteField() { } @Test - public void overwriteNestedFields() { + public void overwritesNestedFields() { ObjectValue.Builder builder = wrapObject("a", map("b", fooValue, "c", map("d", fooValue))).toBuilder(); builder.set(field("a.b"), barValue); @@ -162,7 +162,7 @@ public void overwriteNestedFields() { } @Test - public void overwriteDeeplyNestedField() { + public void overwritesDeeplyNestedField() { ObjectValue.Builder builder = wrapObject("a", map("b", fooValue)).toBuilder(); builder.set(field("a.b.c"), barValue); ObjectValue object = builder.build(); @@ -170,7 +170,7 @@ public void overwriteDeeplyNestedField() { } @Test - public void mergeExistingObject() { + public void mergesExistingObject() { ObjectValue.Builder builder = wrapObject("a", map("b", fooValue)).toBuilder(); builder.set(field("a.c"), fooValue); ObjectValue object = builder.build(); @@ -178,7 +178,7 @@ public void mergeExistingObject() { } @Test - public void overwriteNestedObject() { + public void overwritesNestedObject() { ObjectValue.Builder builder = wrapObject("a", map("b", map("c", fooValue, "d", fooValue))).toBuilder(); builder.set(field("a.b"), barValue); @@ -187,7 +187,16 @@ public void overwriteNestedObject() { } @Test - public void deleteSingleField() { + public void replacesNestedObject() { + Value singleValueObject = valueOf(map("c", barValue)); + ObjectValue.Builder builder = wrapObject("a", map("b", fooValue)).toBuilder(); + builder.set(field("a"), singleValueObject); + ObjectValue object = builder.build(); + assertEquals(wrapObject("a", map("c", barValue)), object); + } + + @Test + public void deletesSingleField() { ObjectValue.Builder builder = wrapObject("a", fooValue, "b", fooValue).toBuilder(); builder.delete(field("a")); ObjectValue object = builder.build(); @@ -195,7 +204,7 @@ public void deleteSingleField() { } @Test - public void deleteNestedObject() { + public void deletesNestedObject() { ObjectValue.Builder builder = wrapObject("a", map("b", map("c", fooValue, "d", fooValue), "f", fooValue)).toBuilder(); builder.delete(field("a.b")); @@ -204,7 +213,7 @@ public void deleteNestedObject() { } @Test - public void deleteNonExistingField() { + public void deletesNonExistingField() { ObjectValue.Builder builder = wrapObject("a", fooValue).toBuilder(); builder.delete(field("b")); ObjectValue object = builder.build(); @@ -212,7 +221,7 @@ public void deleteNonExistingField() { } @Test - public void deleteNonExistingNestedField() { + public void deletesNonExistingNestedField() { ObjectValue.Builder builder = wrapObject("a", map("b", fooValue)).toBuilder(); builder.delete(field("a.b.c")); ObjectValue object = builder.build();