Skip to content

Commit 3d6c9ab

Browse files
dconeybedconeybe
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 3d6c9ab

File tree

4 files changed

+335
-3
lines changed

4 files changed

+335
-3
lines changed

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

Lines changed: 48 additions & 3 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,7 +1035,7 @@ 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++) {
@@ -1053,6 +1054,7 @@ public void resumingAQueryShouldUseExistenceFilterToDetectDeletes() throws Excep
10531054
createdDocuments.add(documentSnapshot.getReference());
10541055
}
10551056
}
1057+
assertWithMessage("createdDocuments").that(createdDocuments).hasSize(100);
10561058

10571059
// Delete 50 of the 100 documents. Do this in a transaction, rather than
10581060
// DocumentReference.delete(), to avoid affecting the local cache.
@@ -1069,13 +1071,36 @@ public void resumingAQueryShouldUseExistenceFilterToDetectDeletes() throws Excep
10691071
}
10701072
return null;
10711073
}));
1074+
assertWithMessage("deletedDocumentIds").that(deletedDocumentIds).hasSize(50);
10721075

10731076
// Wait for 10 seconds, during which Watch will stop tracking the query and will send an
10741077
// existence filter rather than "delete" events when the query is resumed.
10751078
Thread.sleep(10000);
10761079

1077-
// Resume the query and save the resulting snapshot for verification.
1078-
QuerySnapshot snapshot2 = waitFor(collection.get());
1080+
// Resume the query and save the resulting snapshot for verification. Use some internal
1081+
// testing hooks to "capture" the existence filter mismatches to verify that Watch sent a
1082+
// bloom filter, and it was used to avert a full requery.
1083+
QuerySnapshot snapshot2;
1084+
WatchChangeAggregatorTestingHooksAccessor.ExistenceFilterMismatchInfo
1085+
existenceFilterMismatchInfo;
1086+
WatchChangeAggregatorTestingHooksAccessor.ExistenceFilterMismatchAccumulator
1087+
existenceFilterMismatchAccumulator =
1088+
new WatchChangeAggregatorTestingHooksAccessor.ExistenceFilterMismatchAccumulator();
1089+
existenceFilterMismatchAccumulator.register();
1090+
try {
1091+
snapshot2 = waitFor(collection.get());
1092+
// TODO(b/270731363): Remove the "if" condition below once the Firestore Emulator is fixed
1093+
// to send an existence filter.
1094+
if (isRunningAgainstEmulator()) {
1095+
existenceFilterMismatchInfo = null;
1096+
} else {
1097+
existenceFilterMismatchInfo =
1098+
existenceFilterMismatchAccumulator.waitForExistenceFilterMismatch(
1099+
/*timeoutMillis=*/ 5000);
1100+
}
1101+
} finally {
1102+
existenceFilterMismatchAccumulator.unregister();
1103+
}
10791104

10801105
// Verify that the snapshot from the resumed query contains the expected documents; that is,
10811106
// that it contains the 50 documents that were _not_ deleted.
@@ -1098,6 +1123,26 @@ public void resumingAQueryShouldUseExistenceFilterToDetectDeletes() throws Excep
10981123
.that(actualDocumentIds)
10991124
.containsExactlyElementsIn(expectedDocumentIds);
11001125
}
1126+
1127+
// Skip the verification of the existence filter mismatch when testing against the Firestore
1128+
// emulator because the Firestore emulator fails to to send an existence filter at all.
1129+
// TODO(b/270731363): Enable the verification of the existence filter mismatch once the
1130+
// Firestore emulator is fixed to send an existence filter.
1131+
if (isRunningAgainstEmulator()) {
1132+
return;
1133+
}
1134+
1135+
// Verify that Watch sent an existence filter with the correct counts when the query was
1136+
// resumed.
1137+
assertWithMessage("Watch should have sent an existence filter")
1138+
.that(existenceFilterMismatchInfo)
1139+
.isNotNull();
1140+
assertWithMessage("localCacheCount")
1141+
.that(existenceFilterMismatchInfo.localCacheCount())
1142+
.isEqualTo(100);
1143+
assertWithMessage("existenceFilterCount")
1144+
.that(existenceFilterMismatchInfo.existenceFilterCount())
1145+
.isEqualTo(50);
11011146
}
11021147

11031148
@Test

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

Lines changed: 173 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,173 @@
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 android.os.SystemClock;
20+
import androidx.annotation.AnyThread;
21+
import androidx.annotation.NonNull;
22+
import androidx.annotation.Nullable;
23+
import com.google.firebase.firestore.ListenerRegistration;
24+
import java.util.ArrayList;
25+
26+
/**
27+
* Provides access to the {@link WatchChangeAggregatorTestingHooks} class and its methods.
28+
*
29+
* <p>The {@link WatchChangeAggregatorTestingHooks} class has default visibility, and, therefore, is
30+
* only visible to other classes declared in the same package. This class effectively "re-exports"
31+
* the functionality from {@link WatchChangeAggregatorTestingHooks} in a class with {@code public}
32+
* visibility so that tests written in other packages can access its functionality.
33+
*/
34+
public final class WatchChangeAggregatorTestingHooksAccessor {
35+
36+
private WatchChangeAggregatorTestingHooksAccessor() {}
37+
38+
/** @see WatchChangeAggregatorTestingHooks#addExistenceFilterMismatchListener */
39+
public static ListenerRegistration addExistenceFilterMismatchListener(
40+
@NonNull ExistenceFilterMismatchListener listener) {
41+
checkNotNull(listener, "a null listener is not allowed");
42+
return WatchChangeAggregatorTestingHooks.addExistenceFilterMismatchListener(
43+
new ExistenceFilterMismatchListenerWrapper(listener));
44+
}
45+
46+
/** @see WatchChangeAggregatorTestingHooks.ExistenceFilterMismatchListener */
47+
public interface ExistenceFilterMismatchListener {
48+
@AnyThread
49+
void onExistenceFilterMismatch(ExistenceFilterMismatchInfo info);
50+
}
51+
52+
/** @see WatchChangeAggregatorTestingHooks.ExistenceFilterMismatchInfo */
53+
public interface ExistenceFilterMismatchInfo {
54+
int localCacheCount();
55+
56+
int existenceFilterCount();
57+
}
58+
59+
private static final class ExistenceFilterMismatchInfoImpl
60+
implements ExistenceFilterMismatchInfo {
61+
62+
private final WatchChangeAggregatorTestingHooks.ExistenceFilterMismatchInfo info;
63+
64+
ExistenceFilterMismatchInfoImpl(
65+
@NonNull WatchChangeAggregatorTestingHooks.ExistenceFilterMismatchInfo info) {
66+
this.info = info;
67+
}
68+
69+
@Override
70+
public int localCacheCount() {
71+
return info.localCacheCount();
72+
}
73+
74+
@Override
75+
public int existenceFilterCount() {
76+
return info.existenceFilterCount();
77+
}
78+
}
79+
80+
private static final class ExistenceFilterMismatchListenerWrapper
81+
implements WatchChangeAggregatorTestingHooks.ExistenceFilterMismatchListener {
82+
83+
private final ExistenceFilterMismatchListener wrappedListener;
84+
85+
ExistenceFilterMismatchListenerWrapper(
86+
@NonNull ExistenceFilterMismatchListener listenerToWrap) {
87+
this.wrappedListener = listenerToWrap;
88+
}
89+
90+
@Override
91+
public void onExistenceFilterMismatch(
92+
WatchChangeAggregatorTestingHooks.ExistenceFilterMismatchInfo info) {
93+
this.wrappedListener.onExistenceFilterMismatch(new ExistenceFilterMismatchInfoImpl(info));
94+
}
95+
}
96+
97+
public static final class ExistenceFilterMismatchAccumulator {
98+
99+
private ExistenceFilterMismatchListenerImpl listener;
100+
private ListenerRegistration listenerRegistration = null;
101+
102+
/** Registers the accumulator to begin listening for existence filter mismatches. */
103+
public synchronized void register() {
104+
if (listener != null) {
105+
throw new IllegalStateException("already registered");
106+
}
107+
listener = new ExistenceFilterMismatchListenerImpl();
108+
listenerRegistration =
109+
WatchChangeAggregatorTestingHooksAccessor.addExistenceFilterMismatchListener(listener);
110+
}
111+
112+
/** Unregisters the accumulator from listening for existence filter mismatches. */
113+
public synchronized void unregister() {
114+
if (listener == null) {
115+
return;
116+
}
117+
listenerRegistration.remove();
118+
listenerRegistration = null;
119+
listener = null;
120+
}
121+
122+
@Nullable
123+
public WatchChangeAggregatorTestingHooksAccessor.ExistenceFilterMismatchInfo
124+
waitForExistenceFilterMismatch(long timeoutMillis) throws InterruptedException {
125+
ExistenceFilterMismatchListenerImpl capturedListener;
126+
synchronized (this) {
127+
capturedListener = listener;
128+
}
129+
if (capturedListener == null) {
130+
throw new IllegalStateException(
131+
"must be registered before waiting for an existence filter mismatch");
132+
}
133+
return capturedListener.waitForExistenceFilterMismatch(timeoutMillis);
134+
}
135+
136+
private static final class ExistenceFilterMismatchListenerImpl
137+
implements WatchChangeAggregatorTestingHooksAccessor.ExistenceFilterMismatchListener {
138+
139+
private final ArrayList<ExistenceFilterMismatchInfo> existenceFilterMismatches =
140+
new ArrayList<>();
141+
142+
@Override
143+
public void onExistenceFilterMismatch(
144+
WatchChangeAggregatorTestingHooksAccessor.ExistenceFilterMismatchInfo info) {
145+
synchronized (existenceFilterMismatches) {
146+
existenceFilterMismatches.add(info);
147+
existenceFilterMismatches.notifyAll();
148+
}
149+
}
150+
151+
@Nullable
152+
WatchChangeAggregatorTestingHooksAccessor.ExistenceFilterMismatchInfo
153+
waitForExistenceFilterMismatch(long timeoutMillis) throws InterruptedException {
154+
if (timeoutMillis <= 0) {
155+
throw new IllegalArgumentException("invalid timeout: " + timeoutMillis);
156+
}
157+
synchronized (existenceFilterMismatches) {
158+
long endTimeMillis = SystemClock.uptimeMillis() + timeoutMillis;
159+
while (true) {
160+
if (existenceFilterMismatches.size() > 0) {
161+
return existenceFilterMismatches.remove(0);
162+
}
163+
long currentWaitMillis = endTimeMillis - SystemClock.uptimeMillis();
164+
if (currentWaitMillis <= 0) {
165+
return null;
166+
}
167+
existenceFilterMismatches.wait(currentWaitMillis);
168+
}
169+
}
170+
}
171+
}
172+
}
173+
}

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -202,6 +202,10 @@ public void handleExistenceFilter(ExistenceFilterWatchChange watchChange) {
202202
// `isFromCache:true`.
203203
resetTarget(targetId);
204204
pendingTargetResets.add(targetId);
205+
206+
WatchChangeAggregatorTestingHooks.notifyOnExistenceFilterMismatch(
207+
WatchChangeAggregatorTestingHooks.ExistenceFilterMismatchInfo.from(
208+
(int) currentSize, watchChange.getExistenceFilter()));
205209
}
206210
}
207211
}

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

Lines changed: 110 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,110 @@
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.VisibleForTesting;
22+
import com.google.auto.value.AutoValue;
23+
import com.google.firebase.firestore.ListenerRegistration;
24+
import com.google.firebase.firestore.util.Executors;
25+
import java.util.HashMap;
26+
import java.util.Map;
27+
28+
final class WatchChangeAggregatorTestingHooks {
29+
30+
private WatchChangeAggregatorTestingHooks() {}
31+
32+
private static final Map<Object, ExistenceFilterMismatchListener>
33+
existenceFilterMismatchListeners = new HashMap<>();
34+
35+
/**
36+
* Notifies all registered {@link ExistenceFilterMismatchListener}` listeners registered via
37+
* {@link #addExistenceFilterMismatchListener}.
38+
*
39+
* @param info Information about the existence filter mismatch to deliver to the listeners.
40+
*/
41+
static void notifyOnExistenceFilterMismatch(ExistenceFilterMismatchInfo info) {
42+
synchronized (existenceFilterMismatchListeners) {
43+
for (ExistenceFilterMismatchListener listener : existenceFilterMismatchListeners.values()) {
44+
Executors.BACKGROUND_EXECUTOR.execute(() -> listener.onExistenceFilterMismatch(info));
45+
}
46+
}
47+
}
48+
49+
/**
50+
* Registers a {@link ExistenceFilterMismatchListener} to be notified when an existence filter
51+
* mismatch occurs in the Watch listen stream.
52+
*
53+
* <p>The relative order in which callbacks are notified is unspecified; do not rely on any
54+
* particular ordering. If a given callback is registered multiple times then it will be notified
55+
* multiple times, once per registration.
56+
*
57+
* <p>The thread on which the callback occurs is unspecified; listeners should perform their work
58+
* as quickly as possible and return to avoid blocking any critical work. In particular, the
59+
* listener callbacks should <em>not</em> block or perform long-running operations. Listener
60+
* callbacks can occur concurrently with other callbacks on the same and other listeners.
61+
*
62+
* @param listener the listener to register.
63+
* @return an object that unregisters the given listener via its {@link
64+
* ListenerRegistration#remove} method; only the first unregistration request does anything;
65+
* all subsequent requests do nothing.
66+
*/
67+
@VisibleForTesting
68+
static ListenerRegistration addExistenceFilterMismatchListener(
69+
@NonNull ExistenceFilterMismatchListener listener) {
70+
checkNotNull(listener, "a null listener is not allowed");
71+
72+
Object listenerId = new Object();
73+
synchronized (existenceFilterMismatchListeners) {
74+
existenceFilterMismatchListeners.put(listenerId, listener);
75+
}
76+
77+
return () -> {
78+
synchronized (existenceFilterMismatchListeners) {
79+
existenceFilterMismatchListeners.remove(listenerId);
80+
}
81+
};
82+
}
83+
84+
interface ExistenceFilterMismatchListener {
85+
@AnyThread
86+
void onExistenceFilterMismatch(ExistenceFilterMismatchInfo info);
87+
}
88+
89+
@AutoValue
90+
abstract static class ExistenceFilterMismatchInfo {
91+
92+
static ExistenceFilterMismatchInfo create(int localCacheCount, int existenceFilterCount) {
93+
return new AutoValue_WatchChangeAggregatorTestingHooks_ExistenceFilterMismatchInfo(
94+
localCacheCount, existenceFilterCount);
95+
}
96+
97+
/** Returns the number of documents that matched the query in the local cache. */
98+
abstract int localCacheCount();
99+
100+
/**
101+
* Returns the number of documents that matched the query on the server, as specified in the
102+
* ExistenceFilter message's `count` field.
103+
*/
104+
abstract int existenceFilterCount();
105+
106+
static ExistenceFilterMismatchInfo from(int localCacheCount, ExistenceFilter existenceFilter) {
107+
return create(localCacheCount, existenceFilter.getCount());
108+
}
109+
}
110+
}

0 commit comments

Comments
 (0)