Protobuf-backed FieldValues #1184
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -35,25 +35,19 @@ | |
| import com.google.firebase.firestore.model.mutation.FieldMask; | ||
| import com.google.firebase.firestore.model.mutation.NumericIncrementTransformOperation; | ||
| import com.google.firebase.firestore.model.mutation.ServerTimestampOperation; | ||
| import com.google.firebase.firestore.model.value.FieldValue; | ||
| import com.google.firebase.firestore.model.value.NumberValue; | ||
| import com.google.firebase.firestore.model.value.ObjectValue; | ||
| import com.google.firebase.firestore.util.Assert; | ||
| import com.google.firebase.firestore.util.CustomClassMapper; | ||
| 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 com.google.protobuf.NullValue; | ||
| import com.google.type.LatLng; | ||
| import java.util.ArrayList; | ||
| import java.util.Date; | ||
| import java.util.Iterator; | ||
| import java.util.List; | ||
| import java.util.Map; | ||
|
|
@@ -130,8 +124,7 @@ public ParsedUpdateData parseUpdateData(Map<String, Object> data) { | |
| context.addToFieldMask(fieldPath); | ||
| } else { | ||
| @Nullable | ||
| Value parsedValue = convertAndParseFieldData(fieldValue, context.childContext(fieldPath)); | ||
| if (parsedValue != null) { | ||
| context.addToFieldMask(fieldPath); | ||
| updateData.set(fieldPath, parsedValue); | ||
|
|
@@ -181,8 +174,7 @@ public ParsedUpdateData parseUpdateData(List<Object> fieldsAndValues) { | |
| // Add it to the field mask, but don't add anything to updateData. | ||
| context.addToFieldMask(parsedField); | ||
| } else { | ||
| Value parsedValue = convertAndParseFieldData(fieldValue, context.childContext(parsedField)); | ||
| if (parsedValue != null) { | ||
| context.addToFieldMask(parsedField); | ||
| updateData.set(parsedField, parsedValue); | ||
|
|
@@ -209,16 +201,16 @@ public FieldValue parseQueryValue(Object input, boolean allowArrays) { | |
| new ParseAccumulator( | ||
| allowArrays ? UserData.Source.ArrayArgument : UserData.Source.Argument); | ||
|
|
||
| @Nullable Value parsed = convertAndParseFieldData(input, accumulator.rootContext()); | ||
| hardAssert(parsed != null, "Parsed data should not be null."); | ||
| hardAssert( | ||
| accumulator.getFieldTransforms().isEmpty(), | ||
| "Field transforms should have been disallowed."); | ||
| return FieldValue.valueOf(parsed); | ||
| } | ||
|
|
||
| /** Converts a POJO to native types and then parses it into model types. */ | ||
| private Value convertAndParseFieldData(Object input, ParseContext context) { | ||
| Object converted = CustomClassMapper.convertToPlainJavaTypes(input); | ||
| return parseData(converted, context); | ||
| } | ||
|
|
@@ -239,7 +231,7 @@ private ObjectValue convertAndParseDocumentData(Object input, ParseContext conte | |
| } | ||
|
|
||
| Object converted = CustomClassMapper.convertToPlainJavaTypes(input); | ||
| FieldValue value = FieldValue.valueOf(parseData(converted, context)); | ||
|
|
||
| if (!(value instanceof ObjectValue)) { | ||
| throw new IllegalArgumentException(badDocReason + "of type: " + Util.typeName(input)); | ||
|
|
@@ -257,7 +249,7 @@ private ObjectValue convertAndParseDocumentData(Object input, ParseContext conte | |
| * not be included in the resulting parsed data. | ||
| */ | ||
| @Nullable | ||
| private Value parseData(Object input, ParseContext context) { | ||
| if (input instanceof Map) { | ||
| return parseMap((Map<?, ?>) input, context); | ||
|
|
||
|
|
@@ -291,43 +283,42 @@ private FieldValue parseData(Object input, ParseContext context) { | |
| } | ||
| } | ||
|
|
||
| private <K, V> Value parseMap(Map<K, V> map, ParseContext context) { | ||
| if (map.isEmpty()) { | ||
| if (context.getPath() != null && !context.getPath().isEmpty()) { | ||
| context.addToFieldMask(context.getPath()); | ||
| } | ||
| return Value.newBuilder().setMapValue(MapValue.getDefaultInstance()).build(); | ||
| } else { | ||
| MapValue.Builder mapBuilder = MapValue.newBuilder(); | ||
| for (Entry<K, V> entry : map.entrySet()) { | ||
| if (!(entry.getKey() instanceof String)) { | ||
| throw context.createError( | ||
| String.format("Non-String Map key (%s) is not allowed", entry.getValue())); | ||
| } | ||
| String key = (String) entry.getKey(); | ||
| @Nullable Value parsedValue = parseData(entry.getValue(), context.childContext(key)); | ||
| if (parsedValue != null) { | ||
| mapBuilder.putFields(key, parsedValue); | ||
| } | ||
| } | ||
| return Value.newBuilder().setMapValue(mapBuilder).build(); | ||
| } | ||
| } | ||
|
|
||
| private <T> Value parseList(List<T> list, ParseContext context) { | ||
| ArrayValue.Builder arrayBuilder = ArrayValue.newBuilder(); | ||
| int entryIndex = 0; | ||
| for (T entry : list) { | ||
| @Nullable Value parsedEntry = parseData(entry, context.childContext(entryIndex)); | ||
| if (parsedEntry == null) { | ||
| // Just include nulls in the array for fields being replaced with a sentinel. | ||
| parsedEntry = Value.newBuilder().setNullValue(NullValue.NULL_VALUE).build(); | ||
| } | ||
| arrayBuilder.addValues(parsedEntry); | ||
| entryIndex++; | ||
| } | ||
| return Value.newBuilder().setArrayValue(arrayBuilder).build(); | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -398,35 +389,37 @@ private void parseSentinelFieldValue( | |
| * @return The parsed value, or {@code null} if the value was a FieldValue sentinel that should | ||
| * not be included in the resulting parsed data. | ||
| */ | ||
| private Value parseScalarValue(Object input, ParseContext context) { | ||
|
There was a problem hiding this comment. This code essentially duplicates There was a problem hiding this comment. I added a TODO for now. I would like to remove the individual field value wrappers first. |
||
| if (input == null) { | ||
| return Value.newBuilder().setNullValue(NullValue.NULL_VALUE).build(); | ||
| } else if (input instanceof Integer) { | ||
| return Value.newBuilder().setIntegerValue((Integer) input).build(); | ||
| } else if (input instanceof Long) { | ||
| return Value.newBuilder().setIntegerValue((Long) input).build(); | ||
| } else if (input instanceof Float) { | ||
| return Value.newBuilder().setDoubleValue(((Float) input).doubleValue()).build(); | ||
| } else if (input instanceof Double) { | ||
| return Value.newBuilder().setDoubleValue((Double) input).build(); | ||
| } else if (input instanceof Boolean) { | ||
| return Value.newBuilder().setBooleanValue((Boolean) input).build(); | ||
| } else if (input instanceof String) { | ||
| return Value.newBuilder().setStringValue((String) input).build(); | ||
| } else if (input instanceof Date) { | ||
| Timestamp timestamp = new Timestamp((Date) input); | ||
| return parseTimestamp(timestamp); | ||
| } else if (input instanceof Timestamp) { | ||
| Timestamp timestamp = (Timestamp) input; | ||
| return parseTimestamp(timestamp); | ||
| } else if (input instanceof GeoPoint) { | ||
| GeoPoint geoPoint = (GeoPoint) input; | ||
| return Value.newBuilder() | ||
| .setGeoPointValue( | ||
| LatLng.newBuilder() | ||
| .setLatitude(geoPoint.getLatitude()) | ||
| .setLongitude(geoPoint.getLongitude())) | ||
| .build(); | ||
| } else if (input instanceof Blob) { | ||
| return Value.newBuilder().setBytesValue(((Blob) input).toByteString()).build(); | ||
| } else if (input instanceof DocumentReference) { | ||
| DocumentReference ref = (DocumentReference) input; | ||
| // TODO: Rework once pre-converter is ported to Android. | ||
|
|
@@ -442,14 +435,35 @@ private FieldValue parseScalarValue(Object input, ParseContext context) { | |
| databaseId.getDatabaseId())); | ||
| } | ||
| } | ||
| return Value.newBuilder() | ||
| .setReferenceValue( | ||
| String.format( | ||
| "projects/%s/databases/%s/documents/%s", | ||
| databaseId.getProjectId(), | ||
| databaseId.getDatabaseId(), | ||
| ((DocumentReference) input).getPath())) | ||
| .build(); | ||
| } else if (input.getClass().isArray()) { | ||
| throw context.createError("Arrays are not supported; use a List instead"); | ||
| } else { | ||
| throw context.createError("Unsupported type: " + Util.typeName(input)); | ||
| } | ||
| } | ||
|
|
||
| private Value parseTimestamp(Timestamp timestamp) { | ||
| // Firestore backend truncates precision down to microseconds. To ensure offline mode works | ||
| // the same with regards to truncation, perform the truncation immediately without waiting for | ||
| // the backend to do that. | ||
| int truncatedNanoseconds = timestamp.getNanoseconds() / 1000 * 1000; | ||
|
|
||
| return Value.newBuilder() | ||
| .setTimestampValue( | ||
| com.google.protobuf.Timestamp.newBuilder() | ||
| .setSeconds(timestamp.getSeconds()) | ||
| .setNanos(truncatedNanoseconds)) | ||
| .build(); | ||
| } | ||
|
|
||
| private List<FieldValue> parseArrayTransformElements(List<Object> elements) { | ||
| ParseAccumulator accumulator = new ParseAccumulator(UserData.Source.Argument); | ||
|
|
||
|
|
@@ -460,7 +474,7 @@ private List<FieldValue> parseArrayTransformElements(List<Object> elements) { | |
| // being unioned or removed are not considered writes since they cannot | ||
| // contain any FieldValue sentinels, etc. | ||
| ParseContext context = accumulator.rootContext(); | ||
| result.add(FieldValue.valueOf(convertAndParseFieldData(element, context.childContext(i)))); | ||
| } | ||
| return result; | ||
| } | ||
|
|
||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR somehow changed the timing of events so much that the
fromCache:falseevent is now delivered beforehasPendingWrites:false. I am not 100% convinced that this is not due to a bug, but if I look at the test logs everything looks sane.