Skip to content

Commit 3893dcb

Browse files
schmidt-sebastianschmidt-sebastian
authored
Simplify Document (#1179)
1 parent 9354294 commit 3893dcb

File tree

11 files changed

+33
-125
lines changed

11 files changed

+33
-125
lines changed

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -64,21 +64,21 @@ public static SnapshotVersion version(long versionMicros) {
6464

6565
public static Document doc(String key, long version, Map<String, Object> data) {
6666
return new Document(
67-
key(key), version(version), Document.DocumentState.SYNCED, wrapObject(data));
67+
key(key), version(version), wrapObject(data), Document.DocumentState.SYNCED);
6868
}
6969

7070
public static Document doc(DocumentKey key, long version, Map<String, Object> data) {
71-
return new Document(key, version(version), Document.DocumentState.SYNCED, wrapObject(data));
71+
return new Document(key, version(version), wrapObject(data), Document.DocumentState.SYNCED);
7272
}
7373

7474
public static Document doc(
7575
String key, long version, ObjectValue data, Document.DocumentState documentState) {
76-
return new Document(key(key), version(version), documentState, data);
76+
return new Document(key(key), version(version), data, documentState);
7777
}
7878

7979
public static Document doc(
8080
String key, long version, Map<String, Object> data, Document.DocumentState documentState) {
81-
return new Document(key(key), version(version), documentState, wrapObject(data));
81+
return new Document(key(key), version(version), wrapObject(data), documentState);
8282
}
8383

8484
public static DocumentSet docSet(Comparator<Document> comparator, Document... documents) {

‎firebase-firestore/src/main/java/com/google/firebase/firestore/local/LocalSerializer.java

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -55,12 +55,7 @@ com.google.firebase.firestore.proto.MaybeDocument encodeMaybeDocument(MaybeDocum
5555
builder.setHasCommittedMutations(noDocument.hasCommittedMutations());
5656
} else if (document instanceof Document) {
5757
Document existingDocument = (Document) document;
58-
// Use the memoized encoded form if it exists.
59-
if (existingDocument.getProto() != null) {
60-
builder.setDocument(existingDocument.getProto());
61-
} else {
62-
builder.setDocument(encodeDocument(existingDocument));
63-
}
58+
builder.setDocument(encodeDocument(existingDocument));
6459
builder.setHasCommittedMutations(existingDocument.hasCommittedMutations());
6560
} else if (document instanceof UnknownDocument) {
6661
builder.setUnknownDocument(encodeUnknownDocument((UnknownDocument) document));
@@ -112,15 +107,15 @@ private com.google.firestore.v1.Document encodeDocument(Document document) {
112107
private Document decodeDocument(
113108
com.google.firestore.v1.Document document, boolean hasCommittedMutations) {
114109
DocumentKey key = rpcSerializer.decodeKey(document.getName());
110+
ObjectValue value = rpcSerializer.decodeFields(document.getFieldsMap());
115111
SnapshotVersion version = rpcSerializer.decodeVersion(document.getUpdateTime());
116112
return new Document(
117113
key,
118114
version,
115+
value,
119116
hasCommittedMutations
120117
? Document.DocumentState.COMMITTED_MUTATIONS
121-
: Document.DocumentState.SYNCED,
122-
document,
123-
rpcSerializer::decodeValue);
118+
: Document.DocumentState.SYNCED);
124119
}
125120

126121
/** Encodes a NoDocument value to the equivalent proto. */

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

Lines changed: 6 additions & 88 deletions
Original file line numberDiff line numberDiff line change
@@ -14,17 +14,11 @@
1414

1515
package com.google.firebase.firestore.model;
1616

17-
import static com.google.firebase.firestore.util.Assert.hardAssert;
18-
1917
import androidx.annotation.NonNull;
2018
import androidx.annotation.Nullable;
21-
import com.google.common.base.Function;
2219
import com.google.firebase.firestore.model.value.FieldValue;
2320
import com.google.firebase.firestore.model.value.ObjectValue;
24-
import com.google.firestore.v1.Value;
2521
import java.util.Comparator;
26-
import java.util.Map;
27-
import java.util.concurrent.ConcurrentHashMap;
2822

2923
/**
3024
* Represents a document in Firestore with a key, version, data and whether the data has local
@@ -51,101 +45,25 @@ public static Comparator<Document> keyComparator() {
5145
}
5246

5347
private final DocumentState documentState;
54-
private @Nullable final com.google.firestore.v1.Document proto;
55-
private @Nullable final Function<Value, FieldValue> converter;
56-
private @Nullable ObjectValue objectValue;
57-
58-
/** A cache for FieldValues that have already been deserialized in `getField()`. */
59-
private @Nullable Map<FieldPath, FieldValue> fieldValueCache;
48+
private ObjectValue objectValue;
6049

6150
public Document(
6251
DocumentKey key,
6352
SnapshotVersion version,
64-
DocumentState documentState,
65-
ObjectValue objectValue) {
53+
ObjectValue objectValue,
54+
DocumentState documentState) {
6655
super(key, version);
6756
this.documentState = documentState;
6857
this.objectValue = objectValue;
69-
this.proto = null;
70-
this.converter = null;
71-
}
72-
73-
public Document(
74-
DocumentKey key,
75-
SnapshotVersion version,
76-
DocumentState documentState,
77-
com.google.firestore.v1.Document proto,
78-
Function<com.google.firestore.v1.Value, FieldValue> converter) {
79-
super(key, version);
80-
this.documentState = documentState;
81-
this.proto = proto;
82-
this.converter = converter;
83-
}
84-
85-
/**
86-
* Memoized serialized form of the document for optimization purposes (avoids repeated
87-
* serialization). Might be null.
88-
*/
89-
public @Nullable com.google.firestore.v1.Document getProto() {
90-
return proto;
9158
}
9259

9360
@NonNull
9461
public ObjectValue getData() {
95-
if (objectValue == null) {
96-
hardAssert(proto != null && converter != null, "Expected proto and converter to be non-null");
97-
98-
ObjectValue.Builder result = ObjectValue.newBuilder();
99-
for (Map.Entry<String, com.google.firestore.v1.Value> entry :
100-
proto.getFieldsMap().entrySet()) {
101-
FieldPath path = FieldPath.fromSingleSegment(entry.getKey());
102-
FieldValue value = converter.apply(entry.getValue());
103-
result.set(path, value);
104-
}
105-
objectValue = result.build();
106-
107-
// Once objectValue is computed, values inside the fieldValueCache are no longer accessed.
108-
fieldValueCache = null;
109-
}
110-
11162
return objectValue;
11263
}
11364

11465
public @Nullable FieldValue getField(FieldPath path) {
115-
if (objectValue != null) {
116-
return objectValue.get(path);
117-
} else {
118-
hardAssert(proto != null && converter != null, "Expected proto and converter to be non-null");
119-
120-
Map<FieldPath, FieldValue> fieldValueCache = this.fieldValueCache;
121-
if (fieldValueCache == null) {
122-
// TODO(b/136090445): Remove the cache when `getField` is no longer called during Query
123-
// ordering.
124-
fieldValueCache = new ConcurrentHashMap<>();
125-
this.fieldValueCache = fieldValueCache;
126-
}
127-
128-
FieldValue fieldValue = fieldValueCache.get(path);
129-
if (fieldValue == null) {
130-
// Instead of deserializing the full Document proto, we only deserialize the value at
131-
// the requested field path. This speeds up Query execution as query filters can discard
132-
// documents based on a single field.
133-
Value protoValue = proto.getFieldsMap().get(path.getFirstSegment());
134-
for (int i = 1; protoValue != null && i < path.length(); ++i) {
135-
if (protoValue.getValueTypeCase() != Value.ValueTypeCase.MAP_VALUE) {
136-
return null;
137-
}
138-
protoValue = protoValue.getMapValue().getFieldsMap().get(path.getSegment(i));
139-
}
140-
141-
if (protoValue != null) {
142-
fieldValue = converter.apply(protoValue);
143-
fieldValueCache.put(path, fieldValue);
144-
}
145-
}
146-
147-
return fieldValue;
148-
}
66+
return objectValue.get(path);
14967
}
15068

15169
public @Nullable Object getFieldValue(FieldPath path) {
@@ -180,15 +98,15 @@ public boolean equals(Object o) {
18098
return getVersion().equals(document.getVersion())
18199
&& getKey().equals(document.getKey())
182100
&& documentState.equals(document.documentState)
183-
&& getData().equals(document.getData());
101+
&& objectValue.equals(document.objectValue);
184102
}
185103

186104
@Override
187105
public int hashCode() {
188-
// Note: We deliberately decided to omit `getData()` since its computation is expensive.
189106
int result = getKey().hashCode();
190107
result = 31 * result + getVersion().hashCode();
191108
result = 31 * result + documentState.hashCode();
109+
result = 31 * result + objectValue.hashCode();
192110
return result;
193111
}
194112

‎firebase-firestore/src/main/java/com/google/firebase/firestore/model/mutation/PatchMutation.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ public MaybeDocument applyToRemoteDocument(
112112

113113
SnapshotVersion version = mutationResult.getVersion();
114114
ObjectValue newData = patchDocument(maybeDoc);
115-
return new Document(getKey(), version, Document.DocumentState.COMMITTED_MUTATIONS, newData);
115+
return new Document(getKey(), version, newData, Document.DocumentState.COMMITTED_MUTATIONS);
116116
}
117117

118118
@Nullable
@@ -127,7 +127,7 @@ public MaybeDocument applyToLocalView(
127127

128128
SnapshotVersion version = getPostMutationVersion(maybeDoc);
129129
ObjectValue newData = patchDocument(maybeDoc);
130-
return new Document(getKey(), version, Document.DocumentState.LOCAL_MUTATIONS, newData);
130+
return new Document(getKey(), version, newData, Document.DocumentState.LOCAL_MUTATIONS);
131131
}
132132

133133
@Nullable

‎firebase-firestore/src/main/java/com/google/firebase/firestore/model/mutation/SetMutation.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ public MaybeDocument applyToRemoteDocument(
7272
// accepted the mutation so the precondition must have held.
7373

7474
SnapshotVersion version = mutationResult.getVersion();
75-
return new Document(getKey(), version, Document.DocumentState.COMMITTED_MUTATIONS, value);
75+
return new Document(getKey(), version, value, Document.DocumentState.COMMITTED_MUTATIONS);
7676
}
7777

7878
@Nullable
@@ -86,7 +86,7 @@ public MaybeDocument applyToLocalView(
8686
}
8787

8888
SnapshotVersion version = getPostMutationVersion(maybeDoc);
89-
return new Document(getKey(), version, Document.DocumentState.LOCAL_MUTATIONS, value);
89+
return new Document(getKey(), version, value, Document.DocumentState.LOCAL_MUTATIONS);
9090
}
9191

9292
/** Returns the object value to use when setting the document. */

‎firebase-firestore/src/main/java/com/google/firebase/firestore/model/mutation/TransformMutation.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@ public MaybeDocument applyToRemoteDocument(
101101
serverTransformResults(doc, mutationResult.getTransformResults());
102102
ObjectValue newData = transformObject(doc.getData(), transformResults);
103103
return new Document(
104-
getKey(), mutationResult.getVersion(), Document.DocumentState.COMMITTED_MUTATIONS, newData);
104+
getKey(), mutationResult.getVersion(), newData, Document.DocumentState.COMMITTED_MUTATIONS);
105105
}
106106

107107
@Nullable
@@ -118,7 +118,7 @@ public MaybeDocument applyToLocalView(
118118
List<FieldValue> transformResults = localTransformResults(localWriteTime, maybeDoc, baseDoc);
119119
ObjectValue newData = transformObject(doc.getData(), transformResults);
120120
return new Document(
121-
getKey(), doc.getVersion(), Document.DocumentState.LOCAL_MUTATIONS, newData);
121+
getKey(), doc.getVersion(), newData, Document.DocumentState.LOCAL_MUTATIONS);
122122
}
123123

124124
@Nullable

‎firebase-firestore/src/main/java/com/google/firebase/firestore/remote/RemoteSerializer.java

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -405,11 +405,11 @@ private Document decodeFoundDocument(BatchGetDocumentsResponse response) {
405405
response.getResultCase().equals(ResultCase.FOUND),
406406
"Tried to deserialize a found document from a missing document.");
407407
DocumentKey key = decodeKey(response.getFound().getName());
408+
ObjectValue value = decodeFields(response.getFound().getFieldsMap());
408409
SnapshotVersion version = decodeVersion(response.getFound().getUpdateTime());
409410
hardAssert(
410411
!version.equals(SnapshotVersion.NONE), "Got a document response with no snapshot version");
411-
return new Document(
412-
key, version, Document.DocumentState.SYNCED, response.getFound(), this::decodeValue);
412+
return new Document(key, version, value, Document.DocumentState.SYNCED);
413413
}
414414

415415
private NoDocument decodeMissingDocument(BatchGetDocumentsResponse response) {
@@ -1066,13 +1066,8 @@ public WatchChange decodeWatchChange(ListenResponse protoChange) {
10661066
SnapshotVersion version = decodeVersion(docChange.getDocument().getUpdateTime());
10671067
hardAssert(
10681068
!version.equals(SnapshotVersion.NONE), "Got a document change without an update time");
1069-
Document document =
1070-
new Document(
1071-
key,
1072-
version,
1073-
Document.DocumentState.SYNCED,
1074-
docChange.getDocument(),
1075-
this::decodeValue);
1069+
ObjectValue data = decodeFields(docChange.getDocument().getFieldsMap());
1070+
Document document = new Document(key, version, data, Document.DocumentState.SYNCED);
10761071
watchChange = new WatchChange.DocumentChange(added, removed, document.getKey(), document);
10771072
break;
10781073
case DOCUMENT_DELETE:

‎firebase-firestore/src/test/java/com/google/firebase/firestore/local/LruGarbageCollectorTestCase.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -578,7 +578,7 @@ public void testRemoveTargetsThenGC() {
578578
() -> {
579579
SnapshotVersion newVersion = version(3);
580580
Document doc =
581-
new Document(middleDocToUpdate, newVersion, Document.DocumentState.SYNCED, testValue);
581+
new Document(middleDocToUpdate, newVersion, testValue, Document.DocumentState.SYNCED);
582582
documentCache.add(doc, newVersion);
583583
updateTargetInTransaction(middleTarget);
584584
});

‎firebase-firestore/src/test/java/com/google/firebase/firestore/model/DocumentTest.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ public class DocumentTest {
3939
public void testInstantiation() {
4040
Document document =
4141
new Document(
42-
key("messages/first"), version(1), Document.DocumentState.SYNCED, wrapObject("a", 1));
42+
key("messages/first"), version(1), wrapObject("a", 1), Document.DocumentState.SYNCED);
4343

4444
assertEquals(key("messages/first"), document.getKey());
4545
assertEquals(version(1), document.getVersion());
@@ -56,7 +56,7 @@ public void testExtractFields() {
5656
"owner",
5757
map("name", "Jonny", "title", "scallywag"));
5858
Document document =
59-
new Document(key("rooms/eros"), version(1), Document.DocumentState.SYNCED, data);
59+
new Document(key("rooms/eros"), version(1), data, Document.DocumentState.SYNCED);
6060

6161
assertEquals("Discuss all the project related stuff", document.getFieldValue(field("desc")));
6262
assertEquals("scallywag", document.getFieldValue(field("owner.title")));

‎firebase-firestore/src/test/java/com/google/firebase/firestore/model/MutationTest.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -169,8 +169,8 @@ public void testAppliesLocalServerTimestampTransformsToDocuments() {
169169
new Document(
170170
key("collection/key"),
171171
version(0),
172-
Document.DocumentState.LOCAL_MUTATIONS,
173-
expectedData);
172+
expectedData,
173+
Document.DocumentState.LOCAL_MUTATIONS);
174174
assertEquals(expectedDoc, transformedDoc);
175175
}
176176

‎firebase-firestore/src/testUtil/java/com/google/firebase/firestore/testutil/TestUtil.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -179,21 +179,21 @@ public static SnapshotVersion version(long versionMicros) {
179179

180180
public static Document doc(String key, long version, Map<String, Object> data) {
181181
return new Document(
182-
key(key), version(version), Document.DocumentState.SYNCED, wrapObject(data));
182+
key(key), version(version), wrapObject(data), Document.DocumentState.SYNCED);
183183
}
184184

185185
public static Document doc(DocumentKey key, long version, Map<String, Object> data) {
186-
return new Document(key, version(version), Document.DocumentState.SYNCED, wrapObject(data));
186+
return new Document(key, version(version), wrapObject(data), Document.DocumentState.SYNCED);
187187
}
188188

189189
public static Document doc(
190190
String key, long version, ObjectValue data, Document.DocumentState documentState) {
191-
return new Document(key(key), version(version), documentState, data);
191+
return new Document(key(key), version(version), data, documentState);
192192
}
193193

194194
public static Document doc(
195195
String key, long version, Map<String, Object> data, Document.DocumentState documentState) {
196-
return new Document(key(key), version(version), documentState, wrapObject(data));
196+
return new Document(key(key), version(version), wrapObject(data), documentState);
197197
}
198198

199199
public static NoDocument deletedDoc(String key, long version) {

0 commit comments

Comments
 (0)