Skip to content

Commit 9b2c2d3

Browse files
authored
Fix overlay recalculation bug with multiple batches (#3495)
1 parent ca4ca38 commit 9b2c2d3

File tree

3 files changed

+26
-1
lines changed

3 files changed

+26
-1
lines changed

firebase-firestore/CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@ by opting into a release at
1212
the shutdown from proceeding if a network connection was opened right before.
1313
- [changed] Queries are now send to the backend before the SDK starts local
1414
processing, which reduces overall Query latency.
15+
- [fixed] Fixed a NPE issue where mutations with multiple documents are not
16+
handled correctly during previous mutation acknowledgement(#3490).
1517

1618
# 24.0.1
1719
- [changed] Improved performance for databases that contain many document

firebase-firestore/src/main/java/com/google/firebase/firestore/local/LocalDocumentsView.java

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -165,8 +165,14 @@ private void recalculateAndSaveOverlays(Map<DocumentKey, MutableDocument> docs)
165165
// along the way.
166166
for (MutationBatch batch : batches) {
167167
for (DocumentKey key : batch.getKeys()) {
168+
MutableDocument baseDoc = docs.get(key);
169+
if (baseDoc == null) {
170+
// If this batch has documents not included in passed in `docs`, skip them.
171+
continue;
172+
}
173+
168174
FieldMask mask = masks.containsKey(key) ? masks.get(key) : FieldMask.EMPTY;
169-
mask = batch.applyToLocalView(docs.get(key), mask);
175+
mask = batch.applyToLocalView(baseDoc, mask);
170176
masks.put(key, mask);
171177
int batchId = batch.getBatchId();
172178
if (!documentsByBatchId.containsKey(batchId)) {

firebase-firestore/src/test/java/com/google/firebase/firestore/local/LocalStoreTestCase.java

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1599,4 +1599,21 @@ public void testOnlyPersistsUpdatesForDocumentsWhenVersionChanges() {
15991599
assertContains(doc("foo/bar", 1, map("val", "old")));
16001600
assertContains(doc("foo/baz", 2, map("val", "new")));
16011601
}
1602+
1603+
@Test
1604+
public void testCanHandleBatchAckWhenPendingBatchesHaveOtherDocs() {
1605+
// Prepare two batches, the first one will get rejected by the backend.
1606+
// When the first batch is rejected, overlay is recalculated with only the
1607+
// second batch, even though it has more documents than what is being rejected.
1608+
// See: https://github.com/firebase/firebase-android-sdk/issues/3490
1609+
writeMutation(patchMutation("foo/bar", map("foo", "bar")));
1610+
writeMutations(
1611+
asList(
1612+
setMutation("foo/bar", map("foo", "bar-set")),
1613+
setMutation("foo/another", map("foo", "another"))));
1614+
1615+
rejectMutation();
1616+
assertContains(doc("foo/bar", 0, map("foo", "bar-set")).setHasLocalMutations());
1617+
assertContains(doc("foo/another", 0, map("foo", "another")).setHasLocalMutations());
1618+
}
16021619
}

0 commit comments

Comments
 (0)