Skip to content

Commit 4f793f0

Browse files
schmidt-sebastianschmidt-sebastian
committed
Use only one Document class
1 parent d60bb85 commit 4f793f0

File tree

11 files changed

+126
-117
lines changed

11 files changed

+126
-117
lines changed

‎firebase-firestore/CHANGELOG.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
# Unreleased
2-
- [changed] Improved performance for queries that only return a subset of the
3-
data in a collection.
2+
- [changed] Improved performance for queries with filters that only return a
3+
small subset of the documents in a collection.
44
- [changed] Instead of failing silently, Firestore now crashes the client app
55
if it fails to load SSL Ciphers. To avoid these crashes, you must bundle
66
Conscrypt to support non-GMSCore devices on Android KitKat or JellyBean (see

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

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -111,12 +111,16 @@ private com.google.firestore.v1.Document encodeDocument(Document document) {
111111
/** Decodes a Document proto to the equivalent model. */
112112
private Document decodeDocument(
113113
com.google.firestore.v1.Document document, boolean hasCommittedMutations) {
114-
return Document.fromProto(
115-
this.rpcSerializer,
116-
document,
114+
DocumentKey key = rpcSerializer.decodeKey(document.getName());
115+
SnapshotVersion version = rpcSerializer.decodeVersion(document.getUpdateTime());
116+
return new Document(
117+
key,
118+
version,
117119
hasCommittedMutations
118120
? Document.DocumentState.COMMITTED_MUTATIONS
119-
: Document.DocumentState.SYNCED);
121+
: Document.DocumentState.SYNCED,
122+
document,
123+
rpcSerializer::decodeValue);
120124
}
121125

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

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

Lines changed: 78 additions & 80 deletions
Original file line numberDiff line numberDiff line change
@@ -16,18 +16,21 @@
1616

1717
import static com.google.firebase.firestore.util.Assert.hardAssert;
1818

19+
import com.google.common.base.Function;
1920
import com.google.firebase.firestore.model.value.FieldValue;
2021
import com.google.firebase.firestore.model.value.ObjectValue;
21-
import com.google.firebase.firestore.remote.RemoteSerializer;
2222
import com.google.firestore.v1.Value;
2323
import java.util.Comparator;
24+
import java.util.Map;
25+
import java.util.concurrent.ConcurrentHashMap;
26+
import javax.annotation.Nonnull;
2427
import javax.annotation.Nullable;
2528

2629
/**
2730
* Represents a document in Firestore with a key, version, data and whether the data has local
2831
* mutations applied to it.
2932
*/
30-
public abstract class Document extends MaybeDocument {
33+
public final class Document extends MaybeDocument {
3134

3235
/** Describes the `hasPendingWrites` state of a document. */
3336
public enum DocumentState {
@@ -47,101 +50,96 @@ public static Comparator<Document> keyComparator() {
4750
return KEY_COMPARATOR;
4851
}
4952

50-
/** Creates a new Document from an existing ObjectValue. */
51-
public static Document fromObjectValue(
52-
DocumentKey key, SnapshotVersion version, ObjectValue data, DocumentState documentState) {
53-
return new Document(key, version, documentState) {
54-
@Nullable
55-
@Override
56-
public com.google.firestore.v1.Document getProto() {
57-
return null;
58-
}
53+
/** A cache for FieldValues that have already been deserialized in `getField()`. */
54+
private final Map<FieldPath, FieldValue> fieldValueCache = new ConcurrentHashMap<>();
5955

60-
@Override
61-
public ObjectValue getData() {
62-
return data;
63-
}
56+
private final DocumentState documentState;
57+
private @Nullable final com.google.firestore.v1.Document proto;
58+
private @Nullable final Function<Value, FieldValue> converter;
59+
private ObjectValue objectValue;
60+
61+
public Document(
62+
DocumentKey key,
63+
SnapshotVersion version,
64+
DocumentState documentState,
65+
ObjectValue objectValue) {
66+
super(key, version);
67+
this.documentState = documentState;
68+
this.objectValue = objectValue;
69+
this.proto = null;
70+
this.converter = null;
71+
}
6472

65-
@Nullable
66-
@Override
67-
public FieldValue getField(FieldPath path) {
68-
return data.get(path);
69-
}
70-
};
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;
7183
}
7284

7385
/**
74-
* Creates a new Document from a Proto representation.
75-
*
76-
* <p>The Proto is only converted to an ObjectValue if the consumer calls `getData()`.
86+
* Memoized serialized form of the document for optimization purposes (avoids repeated
87+
* serialization). Might be null.
7788
*/
78-
public static Document fromProto(
79-
RemoteSerializer serializer,
80-
com.google.firestore.v1.Document proto,
81-
DocumentState documentState) {
82-
DocumentKey key = serializer.decodeKey(proto.getName());
83-
SnapshotVersion version = serializer.decodeVersion(proto.getUpdateTime());
89+
public @Nullable com.google.firestore.v1.Document getProto() {
90+
return proto;
91+
}
8492

85-
hardAssert(
86-
!version.equals(SnapshotVersion.NONE), "Found a document Proto with no snapshot version");
93+
@Nonnull
94+
public ObjectValue getData() {
95+
if (objectValue == null) {
96+
hardAssert(proto != null && converter != null, "Expected proto and converter to be non-null");
97+
98+
ObjectValue result = ObjectValue.emptyObject();
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 = result.set(path, value);
104+
}
105+
objectValue = result;
87106

88-
return new Document(key, version, documentState) {
89-
private ObjectValue memoizedData = null;
107+
// Once objectValue is computed, values inside the fieldValueCache are no longer accessed.
108+
fieldValueCache.clear();
109+
}
90110

91-
@Override
92-
public com.google.firestore.v1.Document getProto() {
93-
return proto;
94-
}
111+
return objectValue;
112+
}
95113

96-
@Override
97-
public ObjectValue getData() {
98-
if (memoizedData != null) {
99-
return memoizedData;
100-
} else {
101-
memoizedData = serializer.decodeFields(proto.getFieldsMap());
102-
return memoizedData;
114+
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+
FieldValue fieldValue = fieldValueCache.get(path);
121+
if (fieldValue == null) {
122+
// Instead of deserializing the full Document proto, we only deserialize the value at
123+
// the requested field path. This speeds up Query execution as query filters can discard
124+
// documents based on a single field.
125+
Value protoValue = proto.getFieldsMap().get(path.getFirstSegment());
126+
for (int i = 1; protoValue != null && i < path.length(); ++i) {
127+
if (protoValue.getValueTypeCase() != Value.ValueTypeCase.MAP_VALUE) {
128+
return null;
129+
}
130+
protoValue = protoValue.getMapValue().getFieldsMap().get(path.getSegment(i));
103131
}
104-
}
105132

106-
@Nullable
107-
@Override
108-
public FieldValue getField(FieldPath path) {
109-
if (memoizedData != null) {
110-
return memoizedData.get(path);
111-
} else {
112-
// Instead of deserializing the full Document proto, we only deserialize the value at
113-
// the requested field path. This speeds up Query execution as query filters can discard
114-
// documents based on a single field.
115-
Value value = proto.getFieldsMap().get(path.getFirstSegment());
116-
for (int i = 1; value != null && i < path.length(); ++i) {
117-
if (value.getValueTypeCase() != Value.ValueTypeCase.MAP_VALUE) {
118-
return null;
119-
}
120-
value = value.getMapValue().getFieldsMap().get(path.getSegment(i));
121-
}
122-
return value != null ? serializer.decodeValue(value) : null;
133+
if (protoValue != null) {
134+
fieldValue = converter.apply(protoValue);
135+
fieldValueCache.put(path, fieldValue);
123136
}
124137
}
125-
};
126-
}
127138

128-
private final DocumentState documentState;
129-
130-
private Document(DocumentKey key, SnapshotVersion version, DocumentState documentState) {
131-
super(key, version);
132-
this.documentState = documentState;
139+
return fieldValue;
140+
}
133141
}
134142

135-
/**
136-
* Memoized serialized form of the document for optimization purposes (avoids repeated
137-
* serialization). Might be null.
138-
*/
139-
public abstract @Nullable com.google.firestore.v1.Document getProto();
140-
141-
public abstract ObjectValue getData();
142-
143-
public abstract @Nullable FieldValue getField(FieldPath path);
144-
145143
public @Nullable Object getFieldValue(FieldPath path) {
146144
FieldValue value = getField(path);
147145
return (value == null) ? null : value.value();

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

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

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

119118
@Nullable
@@ -128,8 +127,7 @@ public MaybeDocument applyToLocalView(
128127

129128
SnapshotVersion version = getPostMutationVersion(maybeDoc);
130129
ObjectValue newData = patchDocument(maybeDoc);
131-
return Document.fromObjectValue(
132-
getKey(), version, newData, Document.DocumentState.LOCAL_MUTATIONS);
130+
return new Document(getKey(), version, Document.DocumentState.LOCAL_MUTATIONS, newData);
133131
}
134132

135133
@Nullable

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

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

7474
SnapshotVersion version = mutationResult.getVersion();
75-
return Document.fromObjectValue(
76-
getKey(), version, value, Document.DocumentState.COMMITTED_MUTATIONS);
75+
return new Document(getKey(), version, Document.DocumentState.COMMITTED_MUTATIONS, value);
7776
}
7877

7978
@Nullable
@@ -87,8 +86,7 @@ public MaybeDocument applyToLocalView(
8786
}
8887

8988
SnapshotVersion version = getPostMutationVersion(maybeDoc);
90-
return Document.fromObjectValue(
91-
getKey(), version, value, Document.DocumentState.LOCAL_MUTATIONS);
89+
return new Document(getKey(), version, Document.DocumentState.LOCAL_MUTATIONS, value);
9290
}
9391

9492
@Nullable

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -102,8 +102,8 @@ public MaybeDocument applyToRemoteDocument(
102102
List<FieldValue> transformResults =
103103
serverTransformResults(doc, mutationResult.getTransformResults());
104104
ObjectValue newData = transformObject(doc.getData(), transformResults);
105-
return Document.fromObjectValue(
106-
getKey(), mutationResult.getVersion(), newData, Document.DocumentState.COMMITTED_MUTATIONS);
105+
return new Document(
106+
getKey(), mutationResult.getVersion(), Document.DocumentState.COMMITTED_MUTATIONS, newData);
107107
}
108108

109109
@Nullable
@@ -119,8 +119,8 @@ public MaybeDocument applyToLocalView(
119119
Document doc = requireDocument(maybeDoc);
120120
List<FieldValue> transformResults = localTransformResults(localWriteTime, baseDoc);
121121
ObjectValue newData = transformObject(doc.getData(), transformResults);
122-
return Document.fromObjectValue(
123-
getKey(), doc.getVersion(), newData, Document.DocumentState.LOCAL_MUTATIONS);
122+
return new Document(
123+
getKey(), doc.getVersion(), Document.DocumentState.LOCAL_MUTATIONS, newData);
124124
}
125125

126126
@Override

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

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -406,7 +406,12 @@ private Document decodeFoundDocument(BatchGetDocumentsResponse response) {
406406
Assert.hardAssert(
407407
response.getResultCase().equals(ResultCase.FOUND),
408408
"Tried to deserialize a found document from a missing document.");
409-
return Document.fromProto(this, response.getFound(), Document.DocumentState.SYNCED);
409+
DocumentKey key = decodeKey(response.getFound().getName());
410+
SnapshotVersion version = decodeVersion(response.getFound().getUpdateTime());
411+
hardAssert(
412+
!version.equals(SnapshotVersion.NONE), "Got a document response with no snapshot version");
413+
return new Document(
414+
key, version, Document.DocumentState.SYNCED, response.getFound(), this::decodeValue);
410415
}
411416

412417
private NoDocument decodeMissingDocument(BatchGetDocumentsResponse response) {
@@ -1046,16 +1051,25 @@ public WatchChange decodeWatchChange(ListenResponse protoChange) {
10461051
DocumentChange docChange = protoChange.getDocumentChange();
10471052
List<Integer> added = docChange.getTargetIdsList();
10481053
List<Integer> removed = docChange.getRemovedTargetIdsList();
1054+
DocumentKey key = decodeKey(docChange.getDocument().getName());
1055+
SnapshotVersion version = decodeVersion(docChange.getDocument().getUpdateTime());
1056+
hardAssert(
1057+
!version.equals(SnapshotVersion.NONE), "Got a document change without an update time");
10491058
Document document =
1050-
Document.fromProto(this, docChange.getDocument(), Document.DocumentState.SYNCED);
1059+
new Document(
1060+
key,
1061+
version,
1062+
Document.DocumentState.SYNCED,
1063+
docChange.getDocument(),
1064+
this::decodeValue);
10511065
watchChange = new WatchChange.DocumentChange(added, removed, document.getKey(), document);
10521066
break;
10531067
case DOCUMENT_DELETE:
10541068
DocumentDelete docDelete = protoChange.getDocumentDelete();
10551069
removed = docDelete.getRemovedTargetIdsList();
1056-
DocumentKey key = decodeKey(docDelete.getDocument());
1070+
key = decodeKey(docDelete.getDocument());
10571071
// Note that version might be unset in which case we use SnapshotVersion.NONE
1058-
SnapshotVersion version = decodeVersion(docDelete.getReadTime());
1072+
version = decodeVersion(docDelete.getReadTime());
10591073
NoDocument doc = new NoDocument(key, version, /*hasCommittedMutations=*/ false);
10601074
watchChange =
10611075
new WatchChange.DocumentChange(Collections.emptyList(), removed, doc.getKey(), doc);

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -553,8 +553,7 @@ public void testRemoveTargetsThenGC() {
553553
() -> {
554554
SnapshotVersion newVersion = version(3);
555555
Document doc =
556-
Document.fromObjectValue(
557-
middleDocToUpdate, newVersion, testValue, Document.DocumentState.SYNCED);
556+
new Document(middleDocToUpdate, newVersion, Document.DocumentState.SYNCED, testValue);
558557
documentCache.add(doc);
559558
updateTargetInTransaction(middleTarget);
560559
});

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

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -38,8 +38,8 @@ public class DocumentTest {
3838
@Test
3939
public void testInstantiation() {
4040
Document document =
41-
Document.fromObjectValue(
42-
key("messages/first"), version(1), wrapObject("a", 1), Document.DocumentState.SYNCED);
41+
new Document(
42+
key("messages/first"), version(1), Document.DocumentState.SYNCED, wrapObject("a", 1));
4343

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

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

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -161,11 +161,11 @@ public void testAppliesLocalServerTimestampTransformsToDocuments() {
161161
new ServerTimestampValue(timestamp, StringValue.valueOf("bar-value")));
162162

163163
Document expectedDoc =
164-
Document.fromObjectValue(
164+
new Document(
165165
key("collection/key"),
166166
version(0),
167-
expectedData,
168-
Document.DocumentState.LOCAL_MUTATIONS);
167+
Document.DocumentState.LOCAL_MUTATIONS,
168+
expectedData);
169169
assertEquals(expectedDoc, transformedDoc);
170170
}
171171

0 commit comments

Comments
 (0)