Skip to content

Commit 0fca629

Browse files
committed
Firestore: QueryTest.java: improve the test that resumes a query with existence filter to actually validate the existence filter.
This is a port of firebase/firebase-js-sdk#7149, and builds upon the test added in #4799.
1 parent c51a44f commit 0fca629

File tree

4 files changed

+534
-52
lines changed

4 files changed

+534
-52
lines changed

firebase-firestore/src/androidTest/java/com/google/firebase/firestore/QueryTest.java

Lines changed: 134 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@
3737
import com.google.android.gms.tasks.Task;
3838
import com.google.common.collect.Lists;
3939
import com.google.firebase.firestore.Query.Direction;
40+
import com.google.firebase.firestore.remote.WatchChangeAggregatorTestingHooksAccessor;
4041
import com.google.firebase.firestore.testutil.EventAccumulator;
4142
import com.google.firebase.firestore.testutil.IntegrationTestUtil;
4243
import java.util.ArrayList;
@@ -1034,69 +1035,150 @@ public void testMultipleUpdatesWhileOffline() {
10341035
}
10351036

10361037
@Test
1037-
public void resumingAQueryShouldUseExistenceFilterToDetectDeletes() throws Exception {
1038+
public void resumingAQueryShouldUseBloomFilterToAvoidFullRequery() throws Exception {
10381039
// Prepare the names and contents of the 100 documents to create.
10391040
Map<String, Map<String, Object>> testData = new HashMap<>();
10401041
for (int i = 0; i < 100; i++) {
10411042
testData.put("doc" + (1000 + i), map("key", 42));
10421043
}
10431044

1044-
// Create 100 documents in a new collection.
1045-
CollectionReference collection = testCollectionWithDocs(testData);
1046-
1047-
// Run a query to populate the local cache with the 100 documents and a resume token.
1048-
List<DocumentReference> createdDocuments = new ArrayList<>();
1049-
{
1050-
QuerySnapshot querySnapshot = waitFor(collection.get());
1051-
assertWithMessage("querySnapshot1").that(querySnapshot.size()).isEqualTo(100);
1052-
for (DocumentSnapshot documentSnapshot : querySnapshot.getDocuments()) {
1053-
createdDocuments.add(documentSnapshot.getReference());
1045+
// Each iteration of the "while" loop below runs a single iteration of the test. The test will
1046+
// be run multiple times only if a bloom filter false positive occurs.
1047+
int attemptNumber = 0;
1048+
while (true) {
1049+
attemptNumber++;
1050+
1051+
// Create 100 documents in a new collection.
1052+
CollectionReference collection = testCollectionWithDocs(testData);
1053+
1054+
// Run a query to populate the local cache with the 100 documents and a resume token.
1055+
List<DocumentReference> createdDocuments = new ArrayList<>();
1056+
{
1057+
QuerySnapshot querySnapshot = waitFor(collection.get());
1058+
assertWithMessage("querySnapshot1").that(querySnapshot.size()).isEqualTo(100);
1059+
for (DocumentSnapshot documentSnapshot : querySnapshot.getDocuments()) {
1060+
createdDocuments.add(documentSnapshot.getReference());
1061+
}
10541062
}
1055-
}
10561063

1057-
// Delete 50 of the 100 documents. Do this in a transaction, rather than
1058-
// DocumentReference.delete(), to avoid affecting the local cache.
1059-
HashSet<String> deletedDocumentIds = new HashSet<>();
1060-
waitFor(
1061-
collection
1062-
.getFirestore()
1063-
.runTransaction(
1064-
transaction -> {
1065-
for (int i = 0; i < createdDocuments.size(); i += 2) {
1066-
DocumentReference documentToDelete = createdDocuments.get(i);
1067-
transaction.delete(documentToDelete);
1068-
deletedDocumentIds.add(documentToDelete.getId());
1069-
}
1070-
return null;
1071-
}));
1072-
1073-
// Wait for 10 seconds, during which Watch will stop tracking the query and will send an
1074-
// existence filter rather than "delete" events when the query is resumed.
1075-
Thread.sleep(10000);
1076-
1077-
// Resume the query and save the resulting snapshot for verification.
1078-
QuerySnapshot snapshot2 = waitFor(collection.get());
1079-
1080-
// Verify that the snapshot from the resumed query contains the expected documents; that is,
1081-
// that it contains the 50 documents that were _not_ deleted.
1082-
// TODO(b/270731363): Remove the "if" condition below once the Firestore Emulator is fixed to
1083-
// send an existence filter. At the time of writing, the Firestore emulator fails to send an
1084-
// existence filter, resulting in the client including the deleted documents in the snapshot
1085-
// of the resumed query.
1086-
if (!(isRunningAgainstEmulator() && snapshot2.size() == 100)) {
1087-
HashSet<String> actualDocumentIds = new HashSet<>();
1088-
for (DocumentSnapshot documentSnapshot : snapshot2.getDocuments()) {
1089-
actualDocumentIds.add(documentSnapshot.getId());
1064+
// Delete 50 of the 100 documents. Do this in a transaction, rather than
1065+
// DocumentReference.delete(), to avoid affecting the local cache.
1066+
HashSet<String> deletedDocumentIds = new HashSet<>();
1067+
waitFor(
1068+
collection
1069+
.getFirestore()
1070+
.runTransaction(
1071+
transaction -> {
1072+
for (int i = 0; i < createdDocuments.size(); i += 2) {
1073+
DocumentReference documentToDelete = createdDocuments.get(i);
1074+
transaction.delete(documentToDelete);
1075+
deletedDocumentIds.add(documentToDelete.getId());
1076+
}
1077+
return null;
1078+
}));
1079+
1080+
// Wait for 10 seconds, during which Watch will stop tracking the query and will send an
1081+
// existence filter rather than "delete" events when the query is resumed.
1082+
Thread.sleep(10000);
1083+
1084+
// Resume the query and save the resulting snapshot for verification. Use some internal
1085+
// testing hooks to "capture" the existence filter mismatches to verify that Watch sent a
1086+
// bloom filter, and it was used to avert a full requery.
1087+
QuerySnapshot snapshot2;
1088+
WatchChangeAggregatorTestingHooksAccessor.ExistenceFilterMismatchInfo
1089+
existenceFilterMismatchInfo;
1090+
WatchChangeAggregatorTestingHooksAccessor.ExistenceFilterMismatchAccumulator
1091+
existenceFilterMismatchAccumulator =
1092+
new WatchChangeAggregatorTestingHooksAccessor.ExistenceFilterMismatchAccumulator();
1093+
existenceFilterMismatchAccumulator.register();
1094+
try {
1095+
snapshot2 = waitFor(collection.get());
1096+
// TODO(b/270731363): Remove the "if" condition below once the Firestore Emulator is fixed
1097+
// to send an existence filter.
1098+
if (isRunningAgainstEmulator()) {
1099+
existenceFilterMismatchInfo = null;
1100+
} else {
1101+
existenceFilterMismatchInfo =
1102+
existenceFilterMismatchAccumulator.waitForExistenceFilterMismatch(
1103+
/*timeoutMillis=*/ 5000);
1104+
}
1105+
} finally {
1106+
existenceFilterMismatchAccumulator.unregister();
10901107
}
1091-
HashSet<String> expectedDocumentIds = new HashSet<>();
1092-
for (DocumentReference documentRef : createdDocuments) {
1093-
if (!deletedDocumentIds.contains(documentRef.getId())) {
1094-
expectedDocumentIds.add(documentRef.getId());
1108+
1109+
// Verify that the snapshot from the resumed query contains the expected documents; that is,
1110+
// that it contains the 50 documents that were _not_ deleted.
1111+
// TODO(b/270731363): Remove the "if" condition below once the Firestore Emulator is fixed to
1112+
// send an existence filter. At the time of writing, the Firestore emulator fails to send an
1113+
// existence filter, resulting in the client including the deleted documents in the snapshot
1114+
// of the resumed query.
1115+
if (!(isRunningAgainstEmulator() && snapshot2.size() == 100)) {
1116+
HashSet<String> actualDocumentIds = new HashSet<>();
1117+
for (DocumentSnapshot documentSnapshot : snapshot2.getDocuments()) {
1118+
actualDocumentIds.add(documentSnapshot.getId());
1119+
}
1120+
HashSet<String> expectedDocumentIds = new HashSet<>();
1121+
for (DocumentReference documentRef : createdDocuments) {
1122+
if (!deletedDocumentIds.contains(documentRef.getId())) {
1123+
expectedDocumentIds.add(documentRef.getId());
1124+
}
10951125
}
1126+
assertWithMessage("snapshot2.docs")
1127+
.that(actualDocumentIds)
1128+
.containsExactlyElementsIn(expectedDocumentIds);
10961129
}
1097-
assertWithMessage("snapshot2.docs")
1098-
.that(actualDocumentIds)
1099-
.containsExactlyElementsIn(expectedDocumentIds);
1130+
1131+
// Skip the verification of the existence filter mismatch when testing against the Firestore
1132+
// emulator because the Firestore emulator does not include the `unchanged_names` bloom filter
1133+
// when it sends ExistenceFilter messages. Some day the emulator _may_ implement this logic,
1134+
// at which time this short-circuit can be removed.
1135+
if (isRunningAgainstEmulator()) {
1136+
return;
1137+
}
1138+
1139+
// Verify that Watch sent an existence filter with the correct counts when the query was
1140+
// resumed.
1141+
assertWithMessage("Watch should have sent an existence filter")
1142+
.that(existenceFilterMismatchInfo)
1143+
.isNotNull();
1144+
assertWithMessage("localCacheCount")
1145+
.that(existenceFilterMismatchInfo.localCacheCount())
1146+
.isEqualTo(100);
1147+
assertWithMessage("existenceFilterCount")
1148+
.that(existenceFilterMismatchInfo.existenceFilterCount())
1149+
.isEqualTo(50);
1150+
1151+
// Skip the verification of the bloom filter when testing against production because the bloom
1152+
// filter is only implemented in nightly.
1153+
// TODO(b/271949433) Remove this "if" block once the bloom filter logic is deployed to
1154+
// production.
1155+
if (IntegrationTestUtil.getTargetBackend() != IntegrationTestUtil.TargetBackend.NIGHTLY) {
1156+
return;
1157+
}
1158+
1159+
// Verify that Watch sent a valid bloom filter.
1160+
WatchChangeAggregatorTestingHooksAccessor.ExistenceFilterBloomFilterInfo bloomFilter =
1161+
existenceFilterMismatchInfo.bloomFilter();
1162+
assertWithMessage("The bloom filter specified in the existence filter")
1163+
.that(bloomFilter)
1164+
.isNotNull();
1165+
assertWithMessage("hashCount").that(bloomFilter.hashCount()).isGreaterThan(0);
1166+
assertWithMessage("bitmapLength").that(bloomFilter.bitmapLength()).isGreaterThan(0);
1167+
assertWithMessage("padding").that(bloomFilter.padding()).isGreaterThan(0);
1168+
assertWithMessage("padding").that(bloomFilter.padding()).isLessThan(8);
1169+
1170+
// Verify that the bloom filter was successfully used to avert a full requery. If a false
1171+
// positive occurred then retry the entire test. Although statistically rare, false positives
1172+
// are expected to happen occasionally. When a false positive _does_ happen, just retry the
1173+
// test with a different set of documents. If that retry _also_ experiences a false positive,
1174+
// then fail the test because that is so improbable that something must have gone wrong.
1175+
if (attemptNumber == 1 && !bloomFilter.applied()) {
1176+
continue;
1177+
}
1178+
1179+
assertWithMessage("bloom filter successfully applied with attemptNumber=" + attemptNumber)
1180+
.that(bloomFilter.applied())
1181+
.isTrue();
11001182
}
11011183
}
11021184

0 commit comments

Comments
 (0)