Skip to content

Commit 3db08f1

Browse files
schmidt-sebastianschmidt-sebastian
authored
Stripping Empty Write Requests (googleapis#2739)
1 parent 99e510d commit 3db08f1

12 files changed

Lines changed: 499 additions & 241 deletions

File tree

‎google-cloud-firestore/src/main/java/com/google/cloud/firestore/DocumentMask.java‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,8 @@
2525

2626
/** A DocumentMask contains the field paths affected by an update. */
2727
final class DocumentMask {
28+
static final DocumentMask EMPTY_MASK = new DocumentMask(new TreeSet<FieldPath>());
29+
2830
private final SortedSet<FieldPath> fieldPaths; // Sorted for testing.
2931

3032
DocumentMask(Collection<FieldPath> fieldPaths) {
@@ -46,7 +48,11 @@ private static List<FieldPath> extractFromMap(Map<String, Object> values, FieldP
4648
for (Map.Entry<String, Object> entry : values.entrySet()) {
4749
Object value = entry.getValue();
4850
FieldPath childPath = path.append(FieldPath.of(entry.getKey()));
49-
if (value instanceof Map) {
51+
if (entry.getValue() == FieldValue.SERVER_TIMESTAMP_SENTINEL) {
52+
// Ignore
53+
} else if (entry.getValue() == FieldValue.DELETE_SENTINEL) {
54+
fieldPaths.add(childPath);
55+
} else if (value instanceof Map) {
5056
fieldPaths.addAll(extractFromMap((Map<String, Object>) value, childPath));
5157
} else {
5258
// We don't need to special case arrays here as we don't support partial array updates.
@@ -65,4 +71,8 @@ com.google.firestore.v1beta1.DocumentMask toPb() {
6571
}
6672
return updateMask.build();
6773
}
74+
75+
boolean isEmpty() {
76+
return fieldPaths.isEmpty();
77+
}
6878
}

‎google-cloud-firestore/src/main/java/com/google/cloud/firestore/DocumentSnapshot.java‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
import com.google.common.base.Preconditions;
2121
import com.google.firestore.v1beta1.Document;
2222
import com.google.firestore.v1beta1.Value;
23+
import com.google.firestore.v1beta1.Write;
2324
import com.google.protobuf.Timestamp;
2425
import java.util.ArrayList;
2526
import java.util.Date;
@@ -193,6 +194,11 @@ public boolean exists() {
193194
return fields != null;
194195
}
195196

197+
/** Checks whether this DocumentSnapshot contains any fields. */
198+
boolean isEmpty() {
199+
return fields == null || fields.isEmpty();
200+
}
201+
196202
/**
197203
* Returns the fields of the document as a Map. Field values will be converted to their native
198204
* Java representation.
@@ -395,12 +401,13 @@ public DocumentReference getReference() {
395401
return docRef;
396402
}
397403

398-
Document.Builder toPb() {
404+
Write.Builder toPb() {
399405
Preconditions.checkState(exists(), "Can't call toDocument() on a document that doesn't exist");
400-
Document.Builder document = Document.newBuilder();
406+
Write.Builder write = Write.newBuilder();
407+
Document.Builder document = write.getUpdateBuilder();
401408
document.setName(docRef.getName());
402409
document.putAllFields(fields);
403-
return document;
410+
return write;
404411
}
405412

406413
@Override

‎google-cloud-firestore/src/main/java/com/google/cloud/firestore/DocumentTransform.java‎

Lines changed: 26 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -19,25 +19,30 @@
1919
import com.google.firestore.v1beta1.DocumentTransform.FieldTransform;
2020
import com.google.firestore.v1beta1.Write;
2121
import java.util.ArrayList;
22+
import java.util.Collections;
2223
import java.util.List;
2324
import java.util.Map;
25+
import java.util.Set;
26+
import java.util.SortedMap;
27+
import java.util.TreeMap;
2428

2529
/**
2630
* A DocumentTransform contains pending server-side transforms and their corresponding field paths.
2731
*/
2832
final class DocumentTransform {
2933

3034
private DocumentReference documentReference;
31-
private final List<FieldTransform> transforms;
35+
private final SortedMap<FieldPath, FieldTransform> transforms; // Sorted for testing.
3236

33-
private DocumentTransform(DocumentReference documentReference, List<FieldTransform> transforms) {
37+
private DocumentTransform(
38+
DocumentReference documentReference, SortedMap<FieldPath, FieldTransform> transforms) {
3439
this.documentReference = documentReference;
3540
this.transforms = transforms;
3641
}
3742

3843
static DocumentTransform fromFieldPathMap(
3944
DocumentReference documentReference, Map<FieldPath, Object> values) {
40-
List<FieldTransform> transforms = new ArrayList<>();
45+
SortedMap<FieldPath, FieldTransform> transforms = new TreeMap<>();
4146

4247
for (Map.Entry<FieldPath, Object> entry : values.entrySet()) {
4348
FieldPath path = entry.getKey();
@@ -46,23 +51,23 @@ static DocumentTransform fromFieldPathMap(
4651
FieldTransform.Builder fieldTransform = FieldTransform.newBuilder();
4752
fieldTransform.setFieldPath(path.getEncodedPath());
4853
fieldTransform.setSetToServerValue(FieldTransform.ServerValue.REQUEST_TIME);
49-
transforms.add(fieldTransform.build());
54+
transforms.put(path, fieldTransform.build());
5055
} else if (value instanceof Map) {
51-
transforms.addAll(
56+
transforms.putAll(
5257
extractFromMap((Map<String, Object>) value, path, /* allowTransforms= */ true));
5358
} else if (entry.getValue() instanceof List) {
5459
// DocumentTransforms cannot be contained in Lists. We still process the List for
5560
// validation.
56-
extractFromArray((List<Object>) entry.getValue(), path);
61+
validateArray((List<Object>) entry.getValue(), path);
5762
}
5863
}
5964

6065
return new DocumentTransform(documentReference, transforms);
6166
}
6267

63-
private static List<FieldTransform> extractFromMap(
68+
private static SortedMap<FieldPath, FieldTransform> extractFromMap(
6469
Map<String, Object> values, FieldPath path, boolean allowTransforms) {
65-
List<FieldTransform> transforms = new ArrayList<>();
70+
SortedMap<FieldPath, FieldTransform> transforms = new TreeMap<>();
6671

6772
for (Map.Entry<String, Object> entry : values.entrySet()) {
6873
Object value = entry.getValue();
@@ -72,51 +77,51 @@ private static List<FieldTransform> extractFromMap(
7277
FieldTransform.Builder fieldTransform = FieldTransform.newBuilder();
7378
fieldTransform.setFieldPath(path.getEncodedPath());
7479
fieldTransform.setSetToServerValue(FieldTransform.ServerValue.REQUEST_TIME);
75-
transforms.add(fieldTransform.build());
80+
transforms.put(path, fieldTransform.build());
7681
} else {
7782
throw FirestoreException.invalidState(
7883
"Server timestamps are not supported as Array values.");
7984
}
8085
} else if (value instanceof Map) {
81-
transforms.addAll(extractFromMap((Map<String, Object>) value, path, allowTransforms));
86+
transforms.putAll(extractFromMap((Map<String, Object>) value, path, allowTransforms));
8287
} else if (entry.getValue() instanceof List) {
83-
extractFromArray((List<Object>) entry.getValue(), path);
88+
validateArray((List<Object>) entry.getValue(), path);
8489
}
8590
path = path.getParent();
8691
}
8792

8893
return transforms;
8994
}
9095

91-
private static List<FieldTransform> extractFromArray(List<Object> values, FieldPath path) {
92-
List<FieldTransform> transforms = new ArrayList<>();
93-
96+
private static void validateArray(List<Object> values, FieldPath path) {
9497
for (int i = 0; i < values.size(); ++i) {
9598
Object value = values.get(i);
9699
path = path.append(FieldPath.of(Integer.toString(i)));
97100
if (value == FieldValue.SERVER_TIMESTAMP_SENTINEL) {
98101
throw FirestoreException.invalidState(
99102
"Server timestamps are not supported as Array values.");
100103
} else if (value instanceof Map) {
101-
transforms.addAll(extractFromMap((Map<String, Object>) value, path, false));
104+
extractFromMap((Map<String, Object>) value, path, false);
102105
} else if (value instanceof List) {
103-
extractFromArray((List<Object>) value, path);
106+
validateArray((List<Object>) value, path);
104107
}
105108
path = path.getParent();
106109
}
107-
108-
return transforms;
109110
}
110111

111112
boolean isEmpty() {
112113
return transforms.isEmpty();
113114
}
114115

115-
Write toPb() {
116+
Set<FieldPath> getFields() {
117+
return Collections.unmodifiableSet(transforms.keySet());
118+
}
119+
120+
Write.Builder toPb() {
116121
Write.Builder write = Write.newBuilder();
117122
com.google.firestore.v1beta1.DocumentTransform.Builder transform = write.getTransformBuilder();
118-
transform.addAllFieldTransforms(transforms);
123+
transform.addAllFieldTransforms(transforms.values());
119124
transform.setDocument(documentReference.getName());
120-
return write.build();
125+
return write;
121126
}
122127
}

‎google-cloud-firestore/src/main/java/com/google/cloud/firestore/Query.java‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,9 +42,7 @@
4242
import java.util.ArrayList;
4343
import java.util.Comparator;
4444
import java.util.Iterator;
45-
import java.util.LinkedHashMap;
4645
import java.util.List;
47-
import java.util.Map.Entry;
4846
import java.util.Objects;
4947
import java.util.concurrent.Executor;
5048
import javax.annotation.Nonnull;

0 commit comments

Comments
 (0)