Skip to content

Commit 05fe38b

Browse files
dconeybedconeybe
authored
Merge 41e84f3 into 4d1cc39 
2 parents 4d1cc39 + 41e84f3 commit 05fe38b

File tree

5 files changed

+182
-125
lines changed

5 files changed

+182
-125
lines changed

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

Lines changed: 93 additions & 98 deletions
Original file line numberDiff line numberDiff line change
@@ -1220,109 +1220,104 @@ public void bloomFilterShouldCorrectlyEncodeComplexUnicodeCharacters() throws Ex
12201220
testDocs.put(docId, map("foo", 42));
12211221
}
12221222

1223-
// Each iteration of the "while" loop below runs a single iteration of the test. The test will
1224-
// be run multiple times only if a bloom filter false positive occurs.
1225-
int attemptNumber = 0;
1226-
while (true) {
1227-
attemptNumber++;
1228-
1229-
// Create the documents whose names contain complex Unicode characters in a new collection.
1230-
CollectionReference collection = testCollectionWithDocs(testDocs);
1231-
1232-
// Run a query to populate the local cache with documents that have names with complex Unicode
1233-
// characters.
1234-
List<DocumentReference> createdDocuments = new ArrayList<>();
1235-
{
1236-
QuerySnapshot querySnapshot1 = waitFor(collection.get());
1237-
for (DocumentSnapshot documentSnapshot : querySnapshot1.getDocuments()) {
1238-
createdDocuments.add(documentSnapshot.getReference());
1239-
}
1240-
HashSet<String> createdDocumentIds = new HashSet<>();
1241-
for (DocumentSnapshot documentSnapshot : querySnapshot1.getDocuments()) {
1242-
createdDocumentIds.add(documentSnapshot.getId());
1243-
}
1244-
assertWithMessage("createdDocumentIds")
1245-
.that(createdDocumentIds)
1246-
.containsExactlyElementsIn(testDocIds);
1247-
}
1248-
1249-
// Delete one of the documents so that the next call to getDocs() will
1250-
// experience an existence filter mismatch. Do this deletion in a
1251-
// transaction, rather than using deleteDoc(), to avoid affecting the
1252-
// local cache.
1253-
waitFor(
1254-
collection
1255-
.getFirestore()
1256-
.runTransaction(
1257-
transaction -> {
1258-
DocumentReference documentToDelete = collection.document("DocumentToDelete");
1259-
DocumentSnapshot documentToDeleteSnapshot = transaction.get(documentToDelete);
1260-
assertWithMessage("documentToDeleteSnapshot.exists()")
1261-
.that(documentToDeleteSnapshot.exists())
1262-
.isTrue();
1263-
transaction.delete(documentToDelete);
1264-
return null;
1265-
}));
1266-
1267-
// Wait for 10 seconds, during which Watch will stop tracking the query and will send an
1268-
// existence filter rather than "delete" events when the query is resumed.
1269-
Thread.sleep(10000);
1270-
1271-
// Resume the query and save the resulting snapshot for verification. Use some internal
1272-
// testing hooks to "capture" the existence filter mismatches.
1273-
AtomicReference<QuerySnapshot> querySnapshot2Ref = new AtomicReference<>();
1274-
ArrayList<ExistenceFilterMismatchInfo> existenceFilterMismatches =
1275-
captureExistenceFilterMismatches(
1276-
() -> {
1277-
QuerySnapshot querySnapshot = waitFor(collection.get());
1278-
querySnapshot2Ref.set(querySnapshot);
1279-
});
1280-
QuerySnapshot querySnapshot2 = querySnapshot2Ref.get();
1223+
// Create the documents whose names contain complex Unicode characters in a new collection.
1224+
CollectionReference collection = testCollectionWithDocs(testDocs);
12811225

1282-
// Verify that the snapshot from the resumed query contains the expected documents; that is,
1283-
// that it contains the documents whose names contain complex Unicode characters and _not_ the
1284-
// document that was deleted.
1285-
HashSet<String> querySnapshot2DocumentIds = new HashSet<>();
1286-
for (DocumentSnapshot documentSnapshot : querySnapshot2.getDocuments()) {
1287-
querySnapshot2DocumentIds.add(documentSnapshot.getId());
1226+
// Run a query to populate the local cache with documents that have names with complex Unicode
1227+
// characters.
1228+
List<DocumentReference> createdDocuments = new ArrayList<>();
1229+
{
1230+
QuerySnapshot querySnapshot1 = waitFor(collection.get());
1231+
for (DocumentSnapshot documentSnapshot : querySnapshot1.getDocuments()) {
1232+
createdDocuments.add(documentSnapshot.getReference());
12881233
}
1289-
HashSet<String> querySnapshot2ExpectedDocumentIds = new HashSet<>(testDocIds);
1290-
querySnapshot2ExpectedDocumentIds.remove("DocumentToDelete");
1291-
assertWithMessage("querySnapshot2DocumentIds")
1292-
.that(querySnapshot2DocumentIds)
1293-
.containsExactlyElementsIn(querySnapshot2ExpectedDocumentIds);
1294-
1295-
// Verify that Watch sent an existence filter with the correct counts.
1296-
assertWithMessage("Watch should have sent exactly 1 existence filter")
1297-
.that(existenceFilterMismatches)
1298-
.hasSize(1);
1299-
ExistenceFilterMismatchInfo existenceFilterMismatchInfo = existenceFilterMismatches.get(0);
1300-
assertWithMessage("localCacheCount")
1301-
.that(existenceFilterMismatchInfo.localCacheCount())
1302-
.isEqualTo(testDocIds.size());
1303-
assertWithMessage("existenceFilterCount")
1304-
.that(existenceFilterMismatchInfo.existenceFilterCount())
1305-
.isEqualTo(testDocIds.size() - 1);
1306-
1307-
// Verify that Watch sent a valid bloom filter.
1308-
ExistenceFilterBloomFilterInfo bloomFilter = existenceFilterMismatchInfo.bloomFilter();
1309-
assertWithMessage("The bloom filter specified in the existence filter")
1310-
.that(bloomFilter)
1311-
.isNotNull();
1312-
1313-
// Verify that the bloom filter was successfully used to avert a full requery. If a false
1314-
// positive occurred, which is statistically rare, but technically possible, then retry the
1315-
// entire test.
1316-
if (attemptNumber == 1 && !bloomFilter.applied()) {
1317-
continue;
1234+
HashSet<String> createdDocumentIds = new HashSet<>();
1235+
for (DocumentSnapshot documentSnapshot : querySnapshot1.getDocuments()) {
1236+
createdDocumentIds.add(documentSnapshot.getId());
13181237
}
1238+
assertWithMessage("createdDocumentIds")
1239+
.that(createdDocumentIds)
1240+
.containsExactlyElementsIn(testDocIds);
1241+
}
13191242

1320-
assertWithMessage("bloom filter successfully applied with attemptNumber=" + attemptNumber)
1321-
.that(bloomFilter.applied())
1243+
// Delete one of the documents so that the next call to getDocs() will experience an existence
1244+
// filter mismatch. Do this deletion in a transaction, rather than using deleteDoc(), to avoid
1245+
// affecting the local cache.
1246+
DocumentReference documentToDelete = collection.document("DocumentToDelete");
1247+
waitFor(
1248+
collection
1249+
.getFirestore()
1250+
.runTransaction(
1251+
transaction -> {
1252+
DocumentSnapshot documentToDeleteSnapshot = transaction.get(documentToDelete);
1253+
assertWithMessage("documentToDeleteSnapshot.exists()")
1254+
.that(documentToDeleteSnapshot.exists())
1255+
.isTrue();
1256+
transaction.delete(documentToDelete);
1257+
return null;
1258+
}));
1259+
1260+
// Wait for 10 seconds, during which Watch will stop tracking the query and will send an
1261+
// existence filter rather than "delete" events when the query is resumed.
1262+
Thread.sleep(10000);
1263+
1264+
// Resume the query and save the resulting snapshot for verification. Use some internal testing
1265+
// hooks to "capture" the existence filter mismatches.
1266+
AtomicReference<QuerySnapshot> querySnapshot2Ref = new AtomicReference<>();
1267+
ArrayList<ExistenceFilterMismatchInfo> existenceFilterMismatches =
1268+
captureExistenceFilterMismatches(
1269+
() -> {
1270+
QuerySnapshot querySnapshot = waitFor(collection.get());
1271+
querySnapshot2Ref.set(querySnapshot);
1272+
});
1273+
QuerySnapshot querySnapshot2 = querySnapshot2Ref.get();
1274+
1275+
// Verify that the snapshot from the resumed query contains the expected documents; that is,
1276+
// that it contains the documents whose names contain complex Unicode characters and _not_ the
1277+
// document that was deleted.
1278+
HashSet<String> querySnapshot2DocumentIds = new HashSet<>();
1279+
for (DocumentSnapshot documentSnapshot : querySnapshot2.getDocuments()) {
1280+
querySnapshot2DocumentIds.add(documentSnapshot.getId());
1281+
}
1282+
HashSet<String> querySnapshot2ExpectedDocumentIds = new HashSet<>(testDocIds);
1283+
querySnapshot2ExpectedDocumentIds.remove("DocumentToDelete");
1284+
assertWithMessage("querySnapshot2DocumentIds")
1285+
.that(querySnapshot2DocumentIds)
1286+
.containsExactlyElementsIn(querySnapshot2ExpectedDocumentIds);
1287+
1288+
// Verify that Watch sent an existence filter with the correct counts.
1289+
assertWithMessage("Watch should have sent exactly 1 existence filter")
1290+
.that(existenceFilterMismatches)
1291+
.hasSize(1);
1292+
ExistenceFilterMismatchInfo existenceFilterMismatchInfo = existenceFilterMismatches.get(0);
1293+
assertWithMessage("localCacheCount")
1294+
.that(existenceFilterMismatchInfo.localCacheCount())
1295+
.isEqualTo(testDocIds.size());
1296+
assertWithMessage("existenceFilterCount")
1297+
.that(existenceFilterMismatchInfo.existenceFilterCount())
1298+
.isEqualTo(testDocIds.size() - 1);
1299+
1300+
// Verify that Watch sent a valid bloom filter.
1301+
ExistenceFilterBloomFilterInfo bloomFilter = existenceFilterMismatchInfo.bloomFilter();
1302+
assertWithMessage("The bloom filter specified in the existence filter")
1303+
.that(bloomFilter)
1304+
.isNotNull();
1305+
1306+
// The bloom filter application should statistically be successful almost every time; the _only_
1307+
// time when it would _not_ be successful is if there is a false positive when testing for
1308+
// 'DocumentToDelete' in the bloom filter. So verify that the bloom filter application is
1309+
// successful, unless there was a false positive.
1310+
boolean isFalsePositive = bloomFilter.mightContain(documentToDelete);
1311+
assertWithMessage("bloomFilter.applied()")
1312+
.that(bloomFilter.applied())
1313+
.isEqualTo(!isFalsePositive);
1314+
1315+
// Verify that the bloom filter contains the document paths with complex Unicode characters.
1316+
for (DocumentSnapshot documentSnapshot : querySnapshot2.getDocuments()) {
1317+
DocumentReference documentReference = documentSnapshot.getReference();
1318+
assertWithMessage("bloomFilter.mightContain() for " + documentReference.getPath())
1319+
.that(bloomFilter.mightContain(documentReference))
13221320
.isTrue();
1323-
1324-
// Break out of the test loop now that the test passes.
1325-
break;
13261321
}
13271322
}
13281323

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

Lines changed: 30 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616

1717
import androidx.annotation.NonNull;
1818
import androidx.annotation.Nullable;
19+
import com.google.firebase.firestore.DocumentReference;
1920
import com.google.firebase.firestore.ListenerRegistration;
2021
import java.util.ArrayList;
2122

@@ -84,17 +85,27 @@ public int existenceFilterCount() {
8485
@Nullable
8586
public ExistenceFilterBloomFilterInfo bloomFilter() {
8687
TestingHooks.ExistenceFilterBloomFilterInfo bloomFilterInfo = info.bloomFilter();
87-
return bloomFilterInfo == null ? null : new ExistenceFilterBloomFilterInfo(bloomFilterInfo);
88+
return bloomFilterInfo == null
89+
? null
90+
: new ExistenceFilterBloomFilterInfo(
91+
bloomFilterInfo, info.projectId(), info.databaseId());
8892
}
8993
}
9094

9195
/** @see TestingHooks.ExistenceFilterBloomFilterInfo */
9296
public static final class ExistenceFilterBloomFilterInfo {
9397

9498
private final TestingHooks.ExistenceFilterBloomFilterInfo info;
99+
private final String projectId;
100+
private final String databaseId;
95101

96-
ExistenceFilterBloomFilterInfo(@NonNull TestingHooks.ExistenceFilterBloomFilterInfo info) {
102+
ExistenceFilterBloomFilterInfo(
103+
@NonNull TestingHooks.ExistenceFilterBloomFilterInfo info,
104+
String projectId,
105+
String databaseId) {
97106
this.info = info;
107+
this.projectId = projectId;
108+
this.databaseId = databaseId;
98109
}
99110

100111
public boolean applied() {
@@ -112,5 +123,22 @@ public int bitmapLength() {
112123
public int padding() {
113124
return info.padding();
114125
}
126+
127+
public boolean mightContain(DocumentReference documentReference) {
128+
BloomFilter bloomFilter = info.bloomFilter();
129+
if (bloomFilter == null) {
130+
return false;
131+
}
132+
return bloomFilter.mightContain(getBloomFilterEntryFor(documentReference));
133+
}
134+
135+
private String getBloomFilterEntryFor(DocumentReference documentReference) {
136+
return "projects/"
137+
+ projectId
138+
+ "/databases/"
139+
+ databaseId
140+
+ "/documents/"
141+
+ documentReference.getPath();
142+
}
115143
}
116144
}

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

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@
1616

1717
import android.util.Base64;
1818
import androidx.annotation.NonNull;
19-
import androidx.annotation.VisibleForTesting;
2019
import com.google.protobuf.ByteString;
2120
import java.nio.charset.StandardCharsets;
2221
import java.security.MessageDigest;
@@ -93,7 +92,6 @@ public BloomFilterCreateException(String message) {
9392
}
9493
}
9594

96-
@VisibleForTesting
9795
int getBitCount() {
9896
return this.bitCount;
9997
}

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

Lines changed: 33 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import androidx.annotation.VisibleForTesting;
2323
import com.google.auto.value.AutoValue;
2424
import com.google.firebase.firestore.ListenerRegistration;
25+
import com.google.firebase.firestore.model.DatabaseId;
2526
import com.google.firestore.v1.BloomFilter;
2627
import java.util.concurrent.CopyOnWriteArrayList;
2728
import java.util.concurrent.atomic.AtomicReference;
@@ -128,9 +129,11 @@ abstract static class ExistenceFilterMismatchInfo {
128129
static ExistenceFilterMismatchInfo create(
129130
int localCacheCount,
130131
int existenceFilterCount,
132+
String projectId,
133+
String databaseId,
131134
@Nullable ExistenceFilterBloomFilterInfo bloomFilter) {
132135
return new AutoValue_TestingHooks_ExistenceFilterMismatchInfo(
133-
localCacheCount, existenceFilterCount, bloomFilter);
136+
localCacheCount, existenceFilterCount, projectId, databaseId, bloomFilter);
134137
}
135138

136139
/** Returns the number of documents that matched the query in the local cache. */
@@ -142,6 +145,12 @@ static ExistenceFilterMismatchInfo create(
142145
*/
143146
abstract int existenceFilterCount();
144147

148+
/** The projectId used when checking documents for membership in the bloom filter. */
149+
abstract String projectId();
150+
151+
/** The databaseId used when checking documents for membership in the bloom filter. */
152+
abstract String databaseId();
153+
145154
/**
146155
* Returns information about the bloom filter provided by Watch in the ExistenceFilter message's
147156
* `unchangedNames` field. A `null` return value means that Watch did _not_ provide a bloom
@@ -155,23 +164,37 @@ static ExistenceFilterMismatchInfo create(
155164
* with the values taken from the given arguments.
156165
*/
157166
static ExistenceFilterMismatchInfo from(
158-
boolean bloomFilterApplied, int localCacheCount, ExistenceFilter existenceFilter) {
167+
int localCacheCount,
168+
ExistenceFilter existenceFilter,
169+
DatabaseId databaseId,
170+
@Nullable com.google.firebase.firestore.remote.BloomFilter bloomFilter,
171+
boolean bloomFilterApplied) {
159172
return create(
160173
localCacheCount,
161174
existenceFilter.getCount(),
162-
ExistenceFilterBloomFilterInfo.from(bloomFilterApplied, existenceFilter));
175+
databaseId.getProjectId(),
176+
databaseId.getDatabaseId(),
177+
ExistenceFilterBloomFilterInfo.from(bloomFilter, bloomFilterApplied, existenceFilter));
163178
}
164179
}
165180

166181
@AutoValue
167182
abstract static class ExistenceFilterBloomFilterInfo {
168183

169184
static ExistenceFilterBloomFilterInfo create(
170-
boolean applied, int hashCount, int bitmapLength, int padding) {
185+
@Nullable com.google.firebase.firestore.remote.BloomFilter bloomFilter,
186+
boolean applied,
187+
int hashCount,
188+
int bitmapLength,
189+
int padding) {
171190
return new AutoValue_TestingHooks_ExistenceFilterBloomFilterInfo(
172-
applied, hashCount, bitmapLength, padding);
191+
bloomFilter, applied, hashCount, bitmapLength, padding);
173192
}
174193

194+
/** The BloomFilter created from the existence filter; may be null if creating it failed. */
195+
@Nullable
196+
abstract com.google.firebase.firestore.remote.BloomFilter bloomFilter();
197+
175198
/**
176199
* Returns whether a full requery was averted by using the bloom filter. If false, then
177200
* something happened, such as a false positive, to prevent using the bloom filter to avoid a
@@ -188,13 +211,17 @@ static ExistenceFilterBloomFilterInfo create(
188211
/** Returns the number of bits of padding in the last byte of the bloom filter. */
189212
abstract int padding();
190213

214+
@Nullable
191215
static ExistenceFilterBloomFilterInfo from(
192-
boolean bloomFilterApplied, ExistenceFilter existenceFilter) {
216+
@Nullable com.google.firebase.firestore.remote.BloomFilter bloomFilter,
217+
boolean bloomFilterApplied,
218+
ExistenceFilter existenceFilter) {
193219
BloomFilter unchangedNames = existenceFilter.getUnchangedNames();
194220
if (unchangedNames == null) {
195221
return null;
196222
}
197223
return create(
224+
bloomFilter,
198225
bloomFilterApplied,
199226
unchangedNames.getHashCount(),
200227
unchangedNames.getBits().getBitmap().size(),

0 commit comments

Comments
 (0)