Skip to content

Commit 705a3ce

Browse files
dconeybedconeybe
committed
Write most of QueryTest test, except for bloom filter validation logic
1 parent 378ddee commit 705a3ce

File tree

2 files changed

+300
-33
lines changed

2 files changed

+300
-33
lines changed

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

Lines changed: 158 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414

1515
package com.google.firebase.firestore;
1616

17+
import static com.google.common.truth.Truth.assertWithMessage;
1718
import static com.google.firebase.firestore.testutil.IntegrationTestUtil.isRunningAgainstEmulator;
1819
import static com.google.firebase.firestore.testutil.IntegrationTestUtil.nullList;
1920
import static com.google.firebase.firestore.testutil.IntegrationTestUtil.querySnapshotToIds;
@@ -32,14 +33,18 @@
3233
import static org.junit.Assert.assertTrue;
3334
import static org.junit.Assume.assumeFalse;
3435

36+
import android.os.SystemClock;
37+
3538
import androidx.test.ext.junit.runners.AndroidJUnit4;
3639
import com.google.android.gms.tasks.Task;
3740
import com.google.common.collect.Lists;
3841
import com.google.firebase.firestore.Query.Direction;
42+
import com.google.firebase.firestore.remote.WatchChangeAggregatorTestingHooksAccessor;
3943
import com.google.firebase.firestore.testutil.EventAccumulator;
4044
import com.google.firebase.firestore.testutil.IntegrationTestUtil;
4145
import java.util.ArrayList;
4246
import java.util.HashMap;
47+
import java.util.HashSet;
4348
import java.util.LinkedHashMap;
4449
import java.util.List;
4550
import java.util.Map;
@@ -1033,43 +1038,163 @@ public void testMultipleUpdatesWhileOffline() {
10331038
}
10341039

10351040
@Test
1036-
public void resumingQueryShouldRemoveDeletedDocumentsIndicatedByExistenceFilter()
1037-
throws InterruptedException {
1038-
assumeFalse(
1039-
"Skip this test when running against the Firestore emulator as there is a bug related to "
1040-
+ "sending existence filter in response: b/270731363.",
1041-
isRunningAgainstEmulator());
1042-
1041+
public void resumingAQueryShouldUseBloomFilterToAvoidFullRequery() throws Exception {
1042+
// Prepare the names and contents of the 100 documents to create.
10431043
Map<String, Map<String, Object>> testData = new HashMap<>();
1044-
for (int i = 1; i <= 100; i++) {
1045-
testData.put("doc" + i, map("key", i));
1044+
for (int i = 0; i < 100; i++) {
1045+
testData.put("doc" + (1000 + i), map("key", 42));
10461046
}
1047-
CollectionReference collection = testCollectionWithDocs(testData);
10481047

1049-
// Populate the cache and save the resume token.
1050-
QuerySnapshot snapshot1 = waitFor(collection.get());
1051-
assertEquals(snapshot1.size(), 100);
1052-
List<DocumentSnapshot> documents = snapshot1.getDocuments();
1048+
// Each iteration of the "while" loop below runs a single iteration of the test. The test will
1049+
// be run multiple times only if a bloom filter false positive occurs.
1050+
while (true) {
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+
}
1062+
}
1063+
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 existenceFilterMismatchInfo;
1089+
ExistenceFilterMismatchAccumulator existenceFilterMismatchAccumulator = new ExistenceFilterMismatchAccumulator();
1090+
existenceFilterMismatchAccumulator.register();
1091+
try {
1092+
snapshot2 = waitFor(collection.get());
1093+
existenceFilterMismatchInfo = existenceFilterMismatchAccumulator.waitForExistenceFilterMismatch(/*timeoutMillis=*/5000);
1094+
} finally {
1095+
existenceFilterMismatchAccumulator.unregister();
1096+
}
1097+
1098+
// Verify that the snapshot from the resumed query contains the expected documents; that is,
1099+
// that it contains the 50 documents that were _not_ deleted.
1100+
// TODO(b/270731363): Remove the "if" condition below once the Firestore Emulator is fixed to
1101+
// send an existence filter. At the time of writing, the Firestore emulator fails to send an
1102+
// existence filter, resulting in the client including the deleted documents in the snapshot
1103+
// of the resumed query.
1104+
if (!(isRunningAgainstEmulator() && snapshot2.size() == 100)) {
1105+
HashSet<String> actualDocumentIds = new HashSet<>();
1106+
for (DocumentSnapshot documentSnapshot : snapshot2.getDocuments()) {
1107+
actualDocumentIds.add(documentSnapshot.getId());
1108+
}
1109+
HashSet<String> expectedDocumentIds = new HashSet<>();
1110+
for (DocumentReference documentRef : createdDocuments) {
1111+
if (!deletedDocumentIds.contains(documentRef.getId())) {
1112+
expectedDocumentIds.add(documentRef.getId());
1113+
}
1114+
}
1115+
assertWithMessage("snapshot2.docs").that(actualDocumentIds).containsExactlyElementsIn(expectedDocumentIds);
1116+
}
1117+
1118+
// Skip the verification of the existence filter mismatch when testing against the Firestore
1119+
// emulator because the Firestore emulator does not include the `unchanged_names` bloom filter
1120+
// when it sends ExistenceFilter messages. Some day the emulator _may_ implement this logic,
1121+
// at which time this short-circuit can be removed.
1122+
if (isRunningAgainstEmulator()) {
1123+
return;
1124+
}
1125+
1126+
// Verify that Watch sent an existence filter with the correct counts when the query was
1127+
// resumed.
1128+
assertWithMessage("localCacheCount").that(existenceFilterMismatchInfo.localCacheCount()).isEqualTo(100);
1129+
assertWithMessage("existenceFilterCount").that(existenceFilterMismatchInfo.existenceFilterCount()).isEqualTo(50);
1130+
}
1131+
}
1132+
1133+
private static final class ExistenceFilterMismatchAccumulator {
1134+
1135+
private final ExistenceFilterMismatchListenerImpl listener = new ExistenceFilterMismatchListenerImpl();
1136+
private ListenerRegistration listenerRegistration = null;
1137+
1138+
void register() {
1139+
if (listenerRegistration != null) {
1140+
throw new IllegalStateException("already registered");
1141+
}
1142+
listenerRegistration = WatchChangeAggregatorTestingHooksAccessor.addExistenceFilterMismatchListener(listener);
1143+
}
1144+
1145+
void unregister() {
1146+
if (listenerRegistration == null) {
1147+
return;
1148+
}
1149+
listenerRegistration.remove();
1150+
listenerRegistration = null;
1151+
}
1152+
1153+
WatchChangeAggregatorTestingHooksAccessor.ExistenceFilterMismatchInfo waitForExistenceFilterMismatch(long timeoutMillis) throws InterruptedException {
1154+
if (listenerRegistration == null) {
1155+
throw new IllegalStateException("must be registered before waiting for an existence filter mismatch");
1156+
}
1157+
return listener.waitForExistenceFilterMismatch(timeoutMillis);
1158+
}
1159+
1160+
private final class ExistenceFilterMismatchListenerImpl implements WatchChangeAggregatorTestingHooksAccessor.ExistenceFilterMismatchListener {
1161+
1162+
private final ArrayList<WatchChangeAggregatorTestingHooksAccessor.ExistenceFilterMismatchInfo> existenceFilterMismatches = new ArrayList<>();
1163+
1164+
@Override
1165+
public void onExistenceFilterMismatch(WatchChangeAggregatorTestingHooksAccessor.ExistenceFilterMismatchInfo info) {
1166+
synchronized (existenceFilterMismatchesLock) {
1167+
existenceFilterMismatches.add(info);
1168+
existenceFilterMismatchesLock.notifyAll();
1169+
}
1170+
}
1171+
1172+
WatchChangeAggregatorTestingHooksAccessor.ExistenceFilterMismatchInfo waitForExistenceFilterMismatch(long timeoutMillis) throws InterruptedException {
1173+
if (timeoutMillis <= 0) {
1174+
throw new IllegalArgumentException("invalid timeout: " + timeoutMillis);
1175+
}
1176+
synchronized (existenceFilterMismatchesLock) {
1177+
long endTimeMillis = SystemClock.uptimeMillis() + timeoutMillis;
1178+
while (true) {
1179+
if (existenceFilterMismatches.size() > 0) {
1180+
return existenceFilterMismatches.remove(0);
1181+
}
1182+
long currentWaitMillis = endTimeMillis - SystemClock.uptimeMillis();
1183+
if (currentWaitMillis <= 0) {
1184+
throw new WaitForExistenceFilterMismatchTimeoutException("timeout (" + timeoutMillis + "ms) waiting for an existence filter mismatch");
1185+
}
1186+
existenceFilterMismatchesLock.wait(currentWaitMillis);
1187+
}
1188+
}
1189+
}
1190+
1191+
final class WaitForExistenceFilterMismatchTimeoutException extends RuntimeException {
1192+
WaitForExistenceFilterMismatchTimeoutException(String message) {
1193+
super(message);
1194+
}
1195+
}
1196+
}
10531197

1054-
// Delete 50 docs in transaction so that it doesn't affect local cache.
1055-
waitFor(
1056-
collection
1057-
.getFirestore()
1058-
.runTransaction(
1059-
transaction -> {
1060-
for (int i = 1; i <= 50; i++) {
1061-
DocumentReference docRef = documents.get(i).getReference();
1062-
transaction.delete(docRef);
1063-
}
1064-
return null;
1065-
}));
1066-
1067-
// Wait 10 seconds, during which Watch will stop tracking the query
1068-
// and will send an existence filter rather than "delete" events.
1069-
Thread.sleep(10000);
1070-
1071-
QuerySnapshot snapshot2 = waitFor(collection.get());
1072-
assertEquals(snapshot2.size(), 50);
10731198
}
10741199

10751200
// TODO(orquery): Enable this test when prod supports OR queries.

‎firebase-firestore/src/androidTest/java/com/google/firebase/firestore/remote/WatchChangeAggregatorTestingHooksAccessor.java

Lines changed: 142 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,142 @@
1+
// Copyright 2023 Google LLC
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
package com.google.firebase.firestore.remote;
16+
17+
import static com.google.firebase.firestore.util.Preconditions.checkNotNull;
18+
19+
import androidx.annotation.AnyThread;
20+
import androidx.annotation.NonNull;
21+
import androidx.annotation.Nullable;
22+
23+
import com.google.firebase.firestore.ListenerRegistration;
24+
25+
/**
26+
* Provides access to the {@link WatchChangeAggregatorTestingHooks} class and its methods.
27+
*
28+
* The {@link WatchChangeAggregatorTestingHooks} class has default visibility, and, therefore, is
29+
* only visible to other classes declared in the same package. This class effectively "re-exports"
30+
* the functionality from {@link WatchChangeAggregatorTestingHooks} in a class with {@code public}
31+
* visibility so that tests written in other packages can access its functionality.
32+
*/
33+
public final class WatchChangeAggregatorTestingHooksAccessor {
34+
35+
private WatchChangeAggregatorTestingHooksAccessor() {}
36+
37+
/**
38+
* @see WatchChangeAggregatorTestingHooks#addExistenceFilterMismatchListener
39+
*/
40+
public static ListenerRegistration addExistenceFilterMismatchListener(
41+
@NonNull ExistenceFilterMismatchListener listener) {
42+
checkNotNull(listener, "a null listener is not allowed");
43+
return WatchChangeAggregatorTestingHooks.addExistenceFilterMismatchListener(new ExistenceFilterMismatchListenerWrapper(listener));
44+
}
45+
46+
/**
47+
* @see WatchChangeAggregatorTestingHooks.ExistenceFilterMismatchListener
48+
*/
49+
public interface ExistenceFilterMismatchListener {
50+
@AnyThread
51+
void onExistenceFilterMismatch(ExistenceFilterMismatchInfo info);
52+
}
53+
54+
/**
55+
* @see WatchChangeAggregatorTestingHooks.ExistenceFilterMismatchInfo
56+
*/
57+
public interface ExistenceFilterMismatchInfo {
58+
int localCacheCount();
59+
int existenceFilterCount();
60+
@Nullable ExistenceFilterBloomFilterInfo bloomFilter();
61+
}
62+
63+
/**
64+
* @see WatchChangeAggregatorTestingHooks.ExistenceFilterBloomFilterInfo
65+
*/
66+
public interface ExistenceFilterBloomFilterInfo {
67+
boolean applied();
68+
int hashCount();
69+
int bitmapLength();
70+
int padding();
71+
}
72+
73+
private static final class ExistenceFilterMismatchInfoImpl implements ExistenceFilterMismatchInfo {
74+
75+
private final WatchChangeAggregatorTestingHooks.ExistenceFilterMismatchInfo info;
76+
77+
ExistenceFilterMismatchInfoImpl(@NonNull WatchChangeAggregatorTestingHooks.ExistenceFilterMismatchInfo info) {
78+
this.info = info;
79+
}
80+
81+
@Override
82+
public int localCacheCount() {
83+
return info.localCacheCount();
84+
}
85+
86+
@Override
87+
public int existenceFilterCount() {
88+
return info.existenceFilterCount();
89+
}
90+
91+
@Nullable
92+
@Override
93+
public ExistenceFilterBloomFilterInfo bloomFilter() {
94+
WatchChangeAggregatorTestingHooks.ExistenceFilterBloomFilterInfo bloomFilterInfo = info.bloomFilter();
95+
return bloomFilterInfo == null ? null : new ExistenceFilterBloomFilterInfoImpl(bloomFilterInfo);
96+
}
97+
}
98+
99+
private static final class ExistenceFilterBloomFilterInfoImpl implements ExistenceFilterBloomFilterInfo {
100+
101+
private final WatchChangeAggregatorTestingHooks.ExistenceFilterBloomFilterInfo info;
102+
103+
ExistenceFilterBloomFilterInfoImpl(@NonNull WatchChangeAggregatorTestingHooks.ExistenceFilterBloomFilterInfo info) {
104+
this.info = info;
105+
}
106+
107+
@Override
108+
public boolean applied() {
109+
return info.applied();
110+
}
111+
112+
@Override
113+
public int hashCount() {
114+
return info.hashCount();
115+
}
116+
117+
@Override
118+
public int bitmapLength() {
119+
return info.bitmapLength();
120+
}
121+
122+
@Override
123+
public int padding() {
124+
return info.padding();
125+
}
126+
}
127+
128+
private static final class ExistenceFilterMismatchListenerWrapper implements WatchChangeAggregatorTestingHooks.ExistenceFilterMismatchListener {
129+
130+
private final ExistenceFilterMismatchListener wrappedListener;
131+
132+
ExistenceFilterMismatchListenerWrapper(@NonNull ExistenceFilterMismatchListener listenerToWrap) {
133+
this.wrappedListener = listenerToWrap;
134+
}
135+
136+
@Override
137+
public void onExistenceFilterMismatch(WatchChangeAggregatorTestingHooks.ExistenceFilterMismatchInfo info) {
138+
this.wrappedListener.onExistenceFilterMismatch(new ExistenceFilterMismatchInfoImpl(info));
139+
}
140+
}
141+
142+
}

0 commit comments

Comments
 (0)