Skip to content

Commit 3d4cfed

Browse files
dconeybedconeybe
authored
Firestore: QueryTest.java: improve the test to actually validate the existence filter
1 parent 687e079 commit 3d4cfed

File tree

4 files changed

+349
-2
lines changed

4 files changed

+349
-2
lines changed

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

Lines changed: 44 additions & 2 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.ExistenceFilterMismatchListener;
4041
import com.google.firebase.firestore.testutil.EventAccumulator;
4142
import com.google.firebase.firestore.testutil.IntegrationTestUtil;
4243
import java.util.ArrayList;
@@ -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,33 @@ 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 testing
1081+
// hooks to "capture" the existence filter mismatches to verify them.
1082+
ExistenceFilterMismatchListener existenceFilterMismatchListener =
1083+
new ExistenceFilterMismatchListener();
1084+
QuerySnapshot snapshot2;
1085+
ExistenceFilterMismatchListener.ExistenceFilterMismatchInfo existenceFilterMismatchInfo;
1086+
try {
1087+
existenceFilterMismatchListener.startListening();
1088+
snapshot2 = waitFor(collection.get());
1089+
// TODO(b/270731363): Remove the "if" condition below once the Firestore Emulator is fixed
1090+
// to send an existence filter.
1091+
if (isRunningAgainstEmulator()) {
1092+
existenceFilterMismatchInfo = null;
1093+
} else {
1094+
existenceFilterMismatchInfo =
1095+
existenceFilterMismatchListener.getOrWaitForExistenceFilterMismatch(
1096+
/*timeoutMillis=*/ 5000);
1097+
}
1098+
} finally {
1099+
existenceFilterMismatchListener.stopListening();
1100+
}
10791101

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

11031145
@Test

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

Lines changed: 150 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,150 @@
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 android.os.SystemClock;
18+
import androidx.annotation.NonNull;
19+
import androidx.annotation.Nullable;
20+
import com.google.firebase.firestore.ListenerRegistration;
21+
import java.util.ArrayList;
22+
23+
/**
24+
* Provides a mechanism for tests to listen for existence filter mismatches in the Watch "listen"
25+
* stream.
26+
*/
27+
public final class ExistenceFilterMismatchListener {
28+
29+
private TestingHooksExistenceFilterMismatchListenerImpl listener;
30+
private ListenerRegistration listenerRegistration;
31+
32+
/**
33+
* Starts listening for existence filter mismatches.
34+
*
35+
* @throws IllegalStateException if this object is already started.
36+
* @see #stopListening
37+
*/
38+
public synchronized void startListening() {
39+
if (listener != null) {
40+
throw new IllegalStateException("already registered");
41+
}
42+
listener = new TestingHooksExistenceFilterMismatchListenerImpl();
43+
listenerRegistration = TestingHooks.getInstance().addExistenceFilterMismatchListener(listener);
44+
}
45+
46+
/**
47+
* Stops listening for existence filter mismatches.
48+
*
49+
* <p>If listening has not been started then this method does nothing.
50+
*
51+
* @see #startListening
52+
*/
53+
public synchronized void stopListening() {
54+
if (listenerRegistration != null) {
55+
listenerRegistration.remove();
56+
}
57+
listenerRegistration = null;
58+
listener = null;
59+
}
60+
61+
/**
62+
* Returns the oldest existence filter mismatch observed, waiting if none has yet been observed.
63+
*
64+
* <p>The oldest existence filter mismatch observed since the most recent successful invocation of
65+
* {@link #startListening} will be returned. A subsequent invocation of this method will return
66+
* the second-oldest existence filter mismatch observed, and so on. An invocation of {@link
67+
* #stopListening} followed by another invocation of {@link #startListening} will discard any
68+
* existence filter mismatches that occurred while previously started and will start observing
69+
* afresh.
70+
*
71+
* @param timeoutMillis the maximum amount of time, in milliseconds, to wait for an existence
72+
* filter mismatch to occur.
73+
* @return information about the existence filter mismatch that occurred.
74+
* @throws InterruptedException if waiting is interrupted.
75+
* @throws IllegalStateException if this object has not been started by {@link #startListening}.
76+
* @throws IllegalArgumentException if the given timeout is less than or equal to zero.
77+
*/
78+
@Nullable
79+
public ExistenceFilterMismatchInfo getOrWaitForExistenceFilterMismatch(long timeoutMillis)
80+
throws InterruptedException {
81+
if (timeoutMillis <= 0) {
82+
throw new IllegalArgumentException("invalid timeout: " + timeoutMillis);
83+
}
84+
85+
TestingHooksExistenceFilterMismatchListenerImpl registeredListener;
86+
synchronized (this) {
87+
registeredListener = listener;
88+
}
89+
90+
if (registeredListener == null) {
91+
throw new IllegalStateException(
92+
"must be registered before waiting for an existence filter mismatch");
93+
}
94+
95+
return registeredListener.getOrWaitForExistenceFilterMismatch(timeoutMillis);
96+
}
97+
98+
private static final class TestingHooksExistenceFilterMismatchListenerImpl
99+
implements TestingHooks.ExistenceFilterMismatchListener {
100+
101+
private final ArrayList<ExistenceFilterMismatchInfo> existenceFilterMismatches =
102+
new ArrayList<>();
103+
104+
@Override
105+
public synchronized void onExistenceFilterMismatch(
106+
@NonNull TestingHooks.ExistenceFilterMismatchInfo info) {
107+
existenceFilterMismatches.add(new ExistenceFilterMismatchInfo(info));
108+
notifyAll();
109+
}
110+
111+
@Nullable
112+
synchronized ExistenceFilterMismatchInfo getOrWaitForExistenceFilterMismatch(long timeoutMillis)
113+
throws InterruptedException {
114+
if (timeoutMillis <= 0) {
115+
throw new IllegalArgumentException("invalid timeout: " + timeoutMillis);
116+
}
117+
118+
long endTimeMillis = SystemClock.uptimeMillis() + timeoutMillis;
119+
while (true) {
120+
if (existenceFilterMismatches.size() > 0) {
121+
return existenceFilterMismatches.remove(0);
122+
}
123+
long currentWaitMillis = endTimeMillis - SystemClock.uptimeMillis();
124+
if (currentWaitMillis <= 0) {
125+
return null;
126+
}
127+
128+
wait(currentWaitMillis);
129+
}
130+
}
131+
}
132+
133+
/** @see TestingHooks.ExistenceFilterMismatchInfo */
134+
public static final class ExistenceFilterMismatchInfo {
135+
136+
private final TestingHooks.ExistenceFilterMismatchInfo info;
137+
138+
ExistenceFilterMismatchInfo(@NonNull TestingHooks.ExistenceFilterMismatchInfo info) {
139+
this.info = info;
140+
}
141+
142+
public int localCacheCount() {
143+
return info.localCacheCount();
144+
}
145+
146+
public int existenceFilterCount() {
147+
return info.existenceFilterCount();
148+
}
149+
}
150+
}

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

Lines changed: 150 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,150 @@
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.concurrent.CopyOnWriteArrayList;
26+
import java.util.concurrent.atomic.AtomicReference;
27+
28+
/**
29+
* Manages "testing hooks", hooks into the internals of the SDK to verify internal state and events
30+
* during integration tests.
31+
*
32+
* <p>Do not use this class except for testing purposes.
33+
*/
34+
@VisibleForTesting
35+
final class TestingHooks {
36+
37+
private static final TestingHooks instance = new TestingHooks();
38+
39+
// Use CopyOnWriteArrayList to store the listeners so that we don't need to worry about
40+
// synchronizing adds, removes, and traversals.
41+
private final CopyOnWriteArrayList<AtomicReference<ExistenceFilterMismatchListener>>
42+
existenceFilterMismatchListeners = new CopyOnWriteArrayList<>();
43+
44+
private TestingHooks() {}
45+
46+
/** Returns the singleton instance of this class. */
47+
@NonNull
48+
static TestingHooks getInstance() {
49+
return instance;
50+
}
51+
52+
/**
53+
* Asynchronously notifies all registered {@link ExistenceFilterMismatchListener}` listeners
54+
* registered via {@link #addExistenceFilterMismatchListener}.
55+
*
56+
* @param info Information about the existence filter mismatch to deliver to the listeners.
57+
*/
58+
void notifyOnExistenceFilterMismatch(@NonNull ExistenceFilterMismatchInfo info) {
59+
for (AtomicReference<ExistenceFilterMismatchListener> listenerRef :
60+
existenceFilterMismatchListeners) {
61+
Executors.BACKGROUND_EXECUTOR.execute(
62+
() -> {
63+
ExistenceFilterMismatchListener listener = listenerRef.get();
64+
if (listener != null) {
65+
listener.onExistenceFilterMismatch(info);
66+
}
67+
});
68+
}
69+
}
70+
71+
/**
72+
* Registers a {@link ExistenceFilterMismatchListener} to be notified when an existence filter
73+
* mismatch occurs in the Watch listen stream.
74+
*
75+
* <p>The relative order in which callbacks are notified is unspecified; do not rely on any
76+
* particular ordering. If a given callback is registered multiple times then it will be notified
77+
* multiple times, once per registration.
78+
*
79+
* <p>The thread on which the callback occurs is unspecified; listeners should perform their work
80+
* as quickly as possible and return to avoid blocking any critical work. In particular, the
81+
* listener callbacks should <em>not</em> block or perform long-running operations. Listener
82+
* callbacks can occur concurrently with other callbacks on the same and other listeners.
83+
*
84+
* @param listener the listener to register.
85+
* @return an object that unregisters the given listener via its {@link
86+
* ListenerRegistration#remove} method; only the first unregistration request does anything;
87+
* all subsequent requests do nothing.
88+
*/
89+
ListenerRegistration addExistenceFilterMismatchListener(
90+
@NonNull ExistenceFilterMismatchListener listener) {
91+
checkNotNull(listener, "a null listener is not allowed");
92+
93+
AtomicReference<ExistenceFilterMismatchListener> listenerRef = new AtomicReference<>(listener);
94+
existenceFilterMismatchListeners.add(listenerRef);
95+
96+
return () -> {
97+
listenerRef.set(null);
98+
existenceFilterMismatchListeners.remove(listenerRef);
99+
};
100+
}
101+
102+
/**
103+
* Implementations of this interface can be registered with {@link
104+
* #addExistenceFilterMismatchListener}.
105+
*/
106+
interface ExistenceFilterMismatchListener {
107+
108+
/**
109+
* Invoked when an existence filter mismatch occurs.
110+
*
111+
* @param info information about the existence filter mismatch.
112+
*/
113+
@AnyThread
114+
void onExistenceFilterMismatch(@NonNull ExistenceFilterMismatchInfo info);
115+
}
116+
117+
/**
118+
* Information about an existence filter mismatch, as specified to listeners registered with
119+
* {@link #addExistenceFilterMismatchListener}.
120+
*/
121+
@AutoValue
122+
abstract static class ExistenceFilterMismatchInfo {
123+
124+
/**
125+
* Creates and returns a new instance of {@link ExistenceFilterMismatchInfo} with the given
126+
* values.
127+
*/
128+
static ExistenceFilterMismatchInfo create(int localCacheCount, int existenceFilterCount) {
129+
return new AutoValue_TestingHooks_ExistenceFilterMismatchInfo(
130+
localCacheCount, existenceFilterCount);
131+
}
132+
133+
/** Returns the number of documents that matched the query in the local cache. */
134+
abstract int localCacheCount();
135+
136+
/**
137+
* Returns the number of documents that matched the query on the server, as specified in the
138+
* ExistenceFilter message's `count` field.
139+
*/
140+
abstract int existenceFilterCount();
141+
142+
/**
143+
* Convenience method to create and return a new instance of {@link ExistenceFilterMismatchInfo}
144+
* with the values taken from the given arguments.
145+
*/
146+
static ExistenceFilterMismatchInfo from(int localCacheCount, ExistenceFilter existenceFilter) {
147+
return create(localCacheCount, existenceFilter.getCount());
148+
}
149+
}
150+
}

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -202,6 +202,11 @@ public void handleExistenceFilter(ExistenceFilterWatchChange watchChange) {
202202
// `isFromCache:true`.
203203
resetTarget(targetId);
204204
pendingTargetResets.add(targetId);
205+
206+
TestingHooks.getInstance()
207+
.notifyOnExistenceFilterMismatch(
208+
TestingHooks.ExistenceFilterMismatchInfo.from(
209+
(int) currentSize, watchChange.getExistenceFilter()));
205210
}
206211
}
207212
}

0 commit comments

Comments
 (0)