Skip to content

Commit 1548002

Browse files
Introducing UserDataReader/UserDataWriter (#1177)
1 parent 7178184 commit 1548002

File tree

12 files changed

+177
-145
lines changed

12 files changed

+177
-145
lines changed

firebase-firestore/ktx/src/test/java/com/google/firebase/firestore/testutil/TestUtil.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
package com.google.firebase.firestore.testutil;
1616

1717
import com.google.firebase.Timestamp;
18-
import com.google.firebase.firestore.UserDataConverter;
18+
import com.google.firebase.firestore.UserDataReader;
1919
import com.google.firebase.firestore.core.Query;
2020
import com.google.firebase.firestore.model.DatabaseId;
2121
import com.google.firebase.firestore.model.Document;
@@ -33,10 +33,10 @@ public class TestUtil {
3333

3434
public static FieldValue wrap(Object value) {
3535
DatabaseId databaseId = DatabaseId.forProject("project");
36-
UserDataConverter dataConverter = new UserDataConverter(databaseId);
36+
UserDataReader dataReader = new UserDataReader(databaseId);
3737
// HACK: We use parseQueryValue() since it accepts scalars as well as arrays / objects, and
3838
// our tests currently use wrap() pretty generically so we don't know the intent.
39-
return dataConverter.parseQueryValue(value);
39+
return dataReader.parseQueryValue(value);
4040
}
4141

4242
public static ObjectValue wrapObject(Map<String, Object> value) {

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

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -162,8 +162,8 @@ public Task<Void> set(@NonNull Object data, @NonNull SetOptions options) {
162162
checkNotNull(options, "Provided options must not be null.");
163163
ParsedSetData parsed =
164164
options.isMerge()
165-
? firestore.getDataConverter().parseMergeData(data, options.getFieldMask())
166-
: firestore.getDataConverter().parseSetData(data);
165+
? firestore.getUserDataReader().parseMergeData(data, options.getFieldMask())
166+
: firestore.getUserDataReader().parseSetData(data);
167167
return firestore
168168
.getClient()
169169
.write(parsed.toMutationList(key, Precondition.NONE))
@@ -180,7 +180,7 @@ public Task<Void> set(@NonNull Object data, @NonNull SetOptions options) {
180180
*/
181181
@NonNull
182182
public Task<Void> update(@NonNull Map<String, Object> data) {
183-
ParsedUpdateData parsedData = firestore.getDataConverter().parseUpdateData(data);
183+
ParsedUpdateData parsedData = firestore.getUserDataReader().parseUpdateData(data);
184184
return update(parsedData);
185185
}
186186

@@ -199,7 +199,7 @@ public Task<Void> update(
199199
@NonNull String field, @Nullable Object value, Object... moreFieldsAndValues) {
200200
ParsedUpdateData parsedData =
201201
firestore
202-
.getDataConverter()
202+
.getUserDataReader()
203203
.parseUpdateData(
204204
Util.collectUpdateArguments(
205205
/* fieldPathOffset= */ 1, field, value, moreFieldsAndValues));
@@ -220,7 +220,7 @@ public Task<Void> update(
220220
@NonNull FieldPath fieldPath, @Nullable Object value, Object... moreFieldsAndValues) {
221221
ParsedUpdateData parsedData =
222222
firestore
223-
.getDataConverter()
223+
.getUserDataReader()
224224
.parseUpdateData(
225225
Util.collectUpdateArguments(
226226
/* fieldPathOffset= */ 1, fieldPath, value, moreFieldsAndValues));

firebase-firestore/src/main/java/com/google/firebase/firestore/DocumentSnapshot.java

Lines changed: 17 additions & 110 deletions
Original file line numberDiff line numberDiff line change
@@ -19,21 +19,11 @@
1919
import androidx.annotation.NonNull;
2020
import androidx.annotation.Nullable;
2121
import com.google.firebase.Timestamp;
22-
import com.google.firebase.firestore.model.DatabaseId;
2322
import com.google.firebase.firestore.model.Document;
2423
import com.google.firebase.firestore.model.DocumentKey;
25-
import com.google.firebase.firestore.model.value.ArrayValue;
2624
import com.google.firebase.firestore.model.value.FieldValue;
27-
import com.google.firebase.firestore.model.value.ObjectValue;
28-
import com.google.firebase.firestore.model.value.ReferenceValue;
29-
import com.google.firebase.firestore.model.value.ServerTimestampValue;
30-
import com.google.firebase.firestore.model.value.TimestampValue;
3125
import com.google.firebase.firestore.util.CustomClassMapper;
32-
import com.google.firebase.firestore.util.Logger;
33-
import java.util.ArrayList;
3426
import java.util.Date;
35-
import java.util.HashMap;
36-
import java.util.List;
3727
import java.util.Map;
3828

3929
/**
@@ -78,18 +68,6 @@ public enum ServerTimestampBehavior {
7868
static final ServerTimestampBehavior DEFAULT = ServerTimestampBehavior.NONE;
7969
}
8070

81-
/** Holds settings that define field value deserialization options. */
82-
static class FieldValueOptions {
83-
final ServerTimestampBehavior serverTimestampBehavior;
84-
final boolean timestampsInSnapshotsEnabled;
85-
86-
private FieldValueOptions(
87-
ServerTimestampBehavior serverTimestampBehavior, boolean timestampsInSnapshotsEnabled) {
88-
this.serverTimestampBehavior = serverTimestampBehavior;
89-
this.timestampsInSnapshotsEnabled = timestampsInSnapshotsEnabled;
90-
}
91-
}
92-
9371
private final FirebaseFirestore firestore;
9472

9573
private final DocumentKey key;
@@ -166,13 +144,12 @@ public Map<String, Object> getData() {
166144
public Map<String, Object> getData(@NonNull ServerTimestampBehavior serverTimestampBehavior) {
167145
checkNotNull(
168146
serverTimestampBehavior, "Provided serverTimestampBehavior value must not be null.");
169-
return doc == null
170-
? null
171-
: convertObject(
172-
doc.getData(),
173-
new FieldValueOptions(
174-
serverTimestampBehavior,
175-
firestore.getFirestoreSettings().areTimestampsInSnapshotsEnabled()));
147+
UserDataWriter userDataWriter =
148+
new UserDataWriter(
149+
firestore,
150+
firestore.getFirestoreSettings().areTimestampsInSnapshotsEnabled(),
151+
serverTimestampBehavior);
152+
return doc == null ? null : userDataWriter.convertObject(doc.getData());
176153
}
177154

178155
/**
@@ -285,9 +262,8 @@ public Object get(
285262
serverTimestampBehavior, "Provided serverTimestampBehavior value must not be null.");
286263
return getInternal(
287264
fieldPath.getInternalPath(),
288-
new FieldValueOptions(
289-
serverTimestampBehavior,
290-
firestore.getFirestoreSettings().areTimestampsInSnapshotsEnabled()));
265+
serverTimestampBehavior,
266+
firestore.getFirestoreSettings().areTimestampsInSnapshotsEnabled());
291267
}
292268

293269
/**
@@ -438,8 +414,8 @@ public Date getDate(
438414
Object maybeDate =
439415
getInternal(
440416
FieldPath.fromDotSeparatedPath(field).getInternalPath(),
441-
new FieldValueOptions(
442-
serverTimestampBehavior, /*timestampsInSnapshotsEnabled=*/ false));
417+
serverTimestampBehavior,
418+
/* timestampsInSnapshots= */ false);
443419
return castTypedValue(maybeDate, field, Date.class);
444420
}
445421

@@ -479,7 +455,8 @@ public Timestamp getTimestamp(
479455
Object maybeTimestamp =
480456
getInternal(
481457
FieldPath.fromDotSeparatedPath(field).getInternalPath(),
482-
new FieldValueOptions(serverTimestampBehavior, /*timestampsInSnapshotsEnabled=*/ true));
458+
serverTimestampBehavior,
459+
/* timestampsInSnapshots= */ true);
483460
return castTypedValue(maybeTimestamp, field, Timestamp.class);
484461
}
485462

@@ -546,87 +523,17 @@ private <T> T castTypedValue(Object value, String field, Class<T> clazz) {
546523
return clazz.cast(value);
547524
}
548525

549-
@Nullable
550-
private Object convertValue(FieldValue value, FieldValueOptions options) {
551-
if (value instanceof ObjectValue) {
552-
return convertObject((ObjectValue) value, options);
553-
} else if (value instanceof ArrayValue) {
554-
return convertArray((ArrayValue) value, options);
555-
} else if (value instanceof ReferenceValue) {
556-
return convertReference((ReferenceValue) value);
557-
} else if (value instanceof TimestampValue) {
558-
return convertTimestamp((TimestampValue) value, options);
559-
} else if (value instanceof ServerTimestampValue) {
560-
return convertServerTimestamp((ServerTimestampValue) value, options);
561-
} else {
562-
return value.value();
563-
}
564-
}
565-
566-
private Object convertServerTimestamp(ServerTimestampValue value, FieldValueOptions options) {
567-
switch (options.serverTimestampBehavior) {
568-
case PREVIOUS:
569-
return value.getPreviousValue();
570-
case ESTIMATE:
571-
return value.getLocalWriteTime();
572-
default:
573-
return value.value();
574-
}
575-
}
576-
577-
private Object convertTimestamp(TimestampValue value, FieldValueOptions options) {
578-
Timestamp timestamp = value.value();
579-
if (options.timestampsInSnapshotsEnabled) {
580-
return timestamp;
581-
} else {
582-
return timestamp.toDate();
583-
}
584-
}
585-
586-
private Object convertReference(ReferenceValue value) {
587-
DocumentKey key = value.value();
588-
DatabaseId refDatabase = value.getDatabaseId();
589-
DatabaseId database = this.firestore.getDatabaseId();
590-
if (!refDatabase.equals(database)) {
591-
// TODO: Somehow support foreign references.
592-
Logger.warn(
593-
"DocumentSnapshot",
594-
"Document %s contains a document reference within a different database "
595-
+ "(%s/%s) which is not supported. It will be treated as a reference in "
596-
+ "the current database (%s/%s) instead.",
597-
key.getPath(),
598-
refDatabase.getProjectId(),
599-
refDatabase.getDatabaseId(),
600-
database.getProjectId(),
601-
database.getDatabaseId());
602-
}
603-
return new DocumentReference(key, firestore);
604-
}
605-
606-
private Map<String, Object> convertObject(ObjectValue objectValue, FieldValueOptions options) {
607-
Map<String, Object> result = new HashMap<>();
608-
for (Map.Entry<String, FieldValue> entry : objectValue.getInternalValue()) {
609-
result.put(entry.getKey(), convertValue(entry.getValue(), options));
610-
}
611-
return result;
612-
}
613-
614-
private List<Object> convertArray(ArrayValue arrayValue, FieldValueOptions options) {
615-
ArrayList<Object> result = new ArrayList<>(arrayValue.getInternalValue().size());
616-
for (FieldValue v : arrayValue.getInternalValue()) {
617-
result.add(convertValue(v, options));
618-
}
619-
return result;
620-
}
621-
622526
@Nullable
623527
private Object getInternal(
624528
@NonNull com.google.firebase.firestore.model.FieldPath fieldPath,
625-
@NonNull FieldValueOptions options) {
529+
@NonNull ServerTimestampBehavior serverTimestampBehavior,
530+
boolean timestampsInSnapshots) {
626531
if (doc != null) {
627532
FieldValue val = doc.getField(fieldPath);
628533
if (val != null) {
629-
return convertValue(val, options);
534+
UserDataWriter userDataWriter =
535+
new UserDataWriter(firestore, timestampsInSnapshots, serverTimestampBehavior);
536+
return userDataWriter.convertValue(val);
630537
}
631538
}
632539
return null;

firebase-firestore/src/main/java/com/google/firebase/firestore/FirebaseFirestore.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ public interface InstanceRegistry {
7474
private final CredentialsProvider credentialsProvider;
7575
private final AsyncQueue asyncQueue;
7676
private final FirebaseApp firebaseApp;
77-
private final UserDataConverter dataConverter;
77+
private final UserDataReader userDataReader;
7878
// When user requests to terminate, use this to notify `FirestoreMultiDbComponent` to deregister
7979
// this instance.
8080
private final InstanceRegistry instanceRegistry;
@@ -160,7 +160,7 @@ static FirebaseFirestore newInstance(
160160
@Nullable GrpcMetadataProvider metadataProvider) {
161161
this.context = checkNotNull(context);
162162
this.databaseId = checkNotNull(checkNotNull(databaseId));
163-
this.dataConverter = new UserDataConverter(databaseId);
163+
this.userDataReader = new UserDataReader(databaseId);
164164
this.persistenceKey = checkNotNull(persistenceKey);
165165
this.credentialsProvider = checkNotNull(credentialsProvider);
166166
this.asyncQueue = checkNotNull(asyncQueue);
@@ -577,8 +577,8 @@ DatabaseId getDatabaseId() {
577577
return databaseId;
578578
}
579579

580-
UserDataConverter getDataConverter() {
581-
return dataConverter;
580+
UserDataReader getUserDataReader() {
581+
return userDataReader;
582582
}
583583

584584
/**

firebase-firestore/src/main/java/com/google/firebase/firestore/Query.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -348,7 +348,7 @@ private Query whereHelper(@NonNull FieldPath fieldPath, Operator op, Object valu
348348
if (op == Operator.IN || op == Operator.ARRAY_CONTAINS_ANY) {
349349
validateDisjunctiveFilterElements(value, op);
350350
}
351-
fieldValue = firestore.getDataConverter().parseQueryValue(value, op == Operator.IN);
351+
fieldValue = firestore.getUserDataReader().parseQueryValue(value, op == Operator.IN);
352352
}
353353
Filter filter = FieldFilter.create(fieldPath.getInternalPath(), op, fieldValue);
354354
validateNewFilter(filter);
@@ -821,7 +821,7 @@ private Bound boundFromFields(String methodName, Object[] values, boolean before
821821
DocumentKey key = DocumentKey.fromPath(path);
822822
components.add(ReferenceValue.valueOf(firestore.getDatabaseId(), key));
823823
} else {
824-
FieldValue wrapped = firestore.getDataConverter().parseQueryValue(rawValue);
824+
FieldValue wrapped = firestore.getUserDataReader().parseQueryValue(rawValue);
825825
components.add(wrapped);
826826
}
827827
}

firebase-firestore/src/main/java/com/google/firebase/firestore/Transaction.java

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -86,8 +86,8 @@ public Transaction set(
8686
checkNotNull(options, "Provided options must not be null.");
8787
ParsedSetData parsed =
8888
options.isMerge()
89-
? firestore.getDataConverter().parseMergeData(data, options.getFieldMask())
90-
: firestore.getDataConverter().parseSetData(data);
89+
? firestore.getUserDataReader().parseMergeData(data, options.getFieldMask())
90+
: firestore.getUserDataReader().parseSetData(data);
9191
transaction.set(documentRef.getKey(), parsed);
9292
return this;
9393
}
@@ -104,7 +104,7 @@ public Transaction set(
104104
@NonNull
105105
public Transaction update(
106106
@NonNull DocumentReference documentRef, @NonNull Map<String, Object> data) {
107-
ParsedUpdateData parsedData = firestore.getDataConverter().parseUpdateData(data);
107+
ParsedUpdateData parsedData = firestore.getUserDataReader().parseUpdateData(data);
108108
return update(documentRef, parsedData);
109109
}
110110

@@ -127,7 +127,7 @@ public Transaction update(
127127
Object... moreFieldsAndValues) {
128128
ParsedUpdateData parsedData =
129129
firestore
130-
.getDataConverter()
130+
.getUserDataReader()
131131
.parseUpdateData(
132132
Util.collectUpdateArguments(
133133
/* fieldPathOffset= */ 1, field, value, moreFieldsAndValues));
@@ -152,7 +152,7 @@ public Transaction update(
152152
Object... moreFieldsAndValues) {
153153
ParsedUpdateData parsedData =
154154
firestore
155-
.getDataConverter()
155+
.getUserDataReader()
156156
.parseUpdateData(
157157
Util.collectUpdateArguments(
158158
/* fieldPathOffset= */ 1, fieldPath, value, moreFieldsAndValues));

firebase-firestore/src/main/java/com/google/firebase/firestore/UserDataConverter.java renamed to firebase-firestore/src/main/java/com/google/firebase/firestore/UserDataReader.java

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -65,11 +65,11 @@
6565
* @hide
6666
*/
6767
@RestrictTo(RestrictTo.Scope.LIBRARY_GROUP)
68-
public final class UserDataConverter {
68+
public final class UserDataReader {
6969

7070
private final DatabaseId databaseId;
7171

72-
public UserDataConverter(DatabaseId databaseId) {
72+
public UserDataReader(DatabaseId databaseId) {
7373
this.databaseId = databaseId;
7474
}
7575

@@ -398,7 +398,6 @@ private void parseSentinelFieldValue(
398398
* @return The parsed value, or {@code null} if the value was a FieldValue sentinel that should
399399
* not be included in the resulting parsed data.
400400
*/
401-
@Nullable
402401
private FieldValue parseScalarValue(Object input, ParseContext context) {
403402
if (input == null) {
404403
return NullValue.nullValue();

0 commit comments

Comments
 (0)