Skip to content

Commit a7d13b8

Browse files
dconeybedconeybe
authored
Merge f010340 into d41ee2f 
2 parents d41ee2f + f010340 commit a7d13b8

File tree

2 files changed

+128
-124
lines changed

2 files changed

+128
-124
lines changed

‎packages/firestore/test/integration/api/query.test.ts

Lines changed: 108 additions & 124 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@ import {
2626
Bytes,
2727
collection,
2828
collectionGroup,
29-
CollectionReference,
3029
deleteDoc,
3130
disableNetwork,
3231
doc,
@@ -36,7 +35,6 @@ import {
3635
enableNetwork,
3736
endAt,
3837
endBefore,
39-
Firestore,
4038
GeoPoint,
4139
getDocs,
4240
getDocsFromCache,
@@ -60,10 +58,12 @@ import {
6058
} from '../util/firebase_export';
6159
import {
6260
apiDescribe,
61+
RetryError,
6362
toChangesArray,
6463
toDataArray,
6564
toIds,
6665
withEmptyTestCollection,
66+
withRetry,
6767
withTestCollection,
6868
withTestDb
6969
} from '../util/helpers';
@@ -2082,134 +2082,118 @@ apiDescribe('Queries', persistence => {
20822082
testDocs['doc' + (1000 + i)] = { key: 42 };
20832083
}
20842084

2085-
// The function that runs a single iteration of the test.
2086-
// Below this definition, there is a "while" loop that calls this function
2087-
// potentially multiple times.
2088-
const runTestIteration = async (
2089-
coll: CollectionReference,
2090-
db: Firestore
2091-
): Promise<'retry' | 'passed'> => {
2092-
// Run a query to populate the local cache with the 100 documents and a
2093-
// resume token.
2094-
const snapshot1 = await getDocs(coll);
2095-
expect(snapshot1.size, 'snapshot1.size').to.equal(100);
2096-
const createdDocuments = snapshot1.docs.map(snapshot => snapshot.ref);
2097-
2098-
// Delete 50 of the 100 documents. Do this in a transaction, rather than
2099-
// deleteDoc(), to avoid affecting the local cache.
2100-
const deletedDocumentIds = new Set<string>();
2101-
await runTransaction(db, async txn => {
2102-
for (let i = 0; i < createdDocuments.length; i += 2) {
2103-
const documentToDelete = createdDocuments[i];
2104-
txn.delete(documentToDelete);
2105-
deletedDocumentIds.add(documentToDelete.id);
2106-
}
2107-
});
2085+
// Ensure that the local cache is configured to use LRU garbage
2086+
// collection (rather than eager garbage collection) so that the resume
2087+
// token and document data does not get prematurely evicted.
2088+
const lruPersistence = persistence.toLruGc();
21082089

2109-
// Wait for 10 seconds, during which Watch will stop tracking the query
2110-
// and will send an existence filter rather than "delete" events when the
2111-
// query is resumed.
2112-
await new Promise(resolve => setTimeout(resolve, 10000));
2113-
2114-
// Resume the query and save the resulting snapshot for verification.
2115-
// Use some internal testing hooks to "capture" the existence filter
2116-
// mismatches to verify that Watch sent a bloom filter, and it was used to
2117-
// avert a full requery.
2118-
const [existenceFilterMismatches, snapshot2] =
2119-
await captureExistenceFilterMismatches(() => getDocs(coll));
2120-
2121-
// Verify that the snapshot from the resumed query contains the expected
2122-
// documents; that is, that it contains the 50 documents that were _not_
2123-
// deleted.
2124-
// TODO(b/270731363): Remove the "if" condition below once the
2125-
// Firestore Emulator is fixed to send an existence filter. At the time of
2126-
// writing, the Firestore emulator fails to send an existence filter,
2127-
// resulting in the client including the deleted documents in the snapshot
2128-
// of the resumed query.
2129-
if (!(USE_EMULATOR && snapshot2.size === 100)) {
2130-
const actualDocumentIds = snapshot2.docs
2131-
.map(documentSnapshot => documentSnapshot.ref.id)
2132-
.sort();
2133-
const expectedDocumentIds = createdDocuments
2134-
.filter(documentRef => !deletedDocumentIds.has(documentRef.id))
2135-
.map(documentRef => documentRef.id)
2136-
.sort();
2137-
expect(actualDocumentIds, 'snapshot2.docs').to.deep.equal(
2138-
expectedDocumentIds
2139-
);
2140-
}
2090+
return withRetry(async attemptNumber => {
2091+
return withTestCollection(lruPersistence, testDocs, async (coll, db) => {
2092+
// Run a query to populate the local cache with the 100 documents and a
2093+
// resume token.
2094+
const snapshot1 = await getDocs(coll);
2095+
expect(snapshot1.size, 'snapshot1.size').to.equal(100);
2096+
const createdDocuments = snapshot1.docs.map(snapshot => snapshot.ref);
2097+
2098+
// Delete 50 of the 100 documents. Do this in a transaction, rather than
2099+
// deleteDoc(), to avoid affecting the local cache.
2100+
const deletedDocumentIds = new Set<string>();
2101+
await runTransaction(db, async txn => {
2102+
for (let i = 0; i < createdDocuments.length; i += 2) {
2103+
const documentToDelete = createdDocuments[i];
2104+
txn.delete(documentToDelete);
2105+
deletedDocumentIds.add(documentToDelete.id);
2106+
}
2107+
});
21412108

2142-
// Skip the verification of the existence filter mismatch when testing
2143-
// against the Firestore emulator because the Firestore emulator fails to
2144-
// to send an existence filter at all.
2145-
// TODO(b/270731363): Enable the verification of the existence filter
2146-
// mismatch once the Firestore emulator is fixed to send an existence
2147-
// filter.
2148-
if (USE_EMULATOR) {
2149-
return 'passed';
2150-
}
2109+
// Wait for 10 seconds, during which Watch will stop tracking the query
2110+
// and will send an existence filter rather than "delete" events when
2111+
// the query is resumed.
2112+
await new Promise(resolve => setTimeout(resolve, 10000));
2113+
2114+
// Resume the query and save the resulting snapshot for verification.
2115+
// Use some internal testing hooks to "capture" the existence filter
2116+
// mismatches to verify that Watch sent a bloom filter, and it was used
2117+
// to avert a full requery.
2118+
const [existenceFilterMismatches, snapshot2] =
2119+
await captureExistenceFilterMismatches(() => getDocs(coll));
2120+
2121+
// Verify that the snapshot from the resumed query contains the expected
2122+
// documents; that is, that it contains the 50 documents that were _not_
2123+
// deleted.
2124+
// TODO(b/270731363): Remove the "if" condition below once the
2125+
// Firestore Emulator is fixed to send an existence filter. At the time
2126+
// of writing, the Firestore emulator fails to send an existence filter,
2127+
// resulting in the client including the deleted documents in the
2128+
// snapshot of the resumed query.
2129+
if (!(USE_EMULATOR && snapshot2.size === 100)) {
2130+
const actualDocumentIds = snapshot2.docs
2131+
.map(documentSnapshot => documentSnapshot.ref.id)
2132+
.sort();
2133+
const expectedDocumentIds = createdDocuments
2134+
.filter(documentRef => !deletedDocumentIds.has(documentRef.id))
2135+
.map(documentRef => documentRef.id)
2136+
.sort();
2137+
expect(actualDocumentIds, 'snapshot2.docs').to.deep.equal(
2138+
expectedDocumentIds
2139+
);
2140+
}
21512141

2152-
// Verify that Watch sent an existence filter with the correct counts when
2153-
// the query was resumed.
2154-
expect(
2155-
existenceFilterMismatches,
2156-
'existenceFilterMismatches'
2157-
).to.have.length(1);
2158-
const { localCacheCount, existenceFilterCount, bloomFilter } =
2159-
existenceFilterMismatches[0];
2160-
expect(localCacheCount, 'localCacheCount').to.equal(100);
2161-
expect(existenceFilterCount, 'existenceFilterCount').to.equal(50);
2162-
2163-
// Verify that Watch sent a valid bloom filter.
2164-
if (!bloomFilter) {
2165-
expect.fail(
2166-
'The existence filter should have specified a bloom filter in its ' +
2167-
'`unchanged_names` field.'
2168-
);
2169-
throw new Error('should never get here');
2170-
}
2142+
// Skip the verification of the existence filter mismatch when testing
2143+
// against the Firestore emulator because the Firestore emulator fails
2144+
// to to send an existence filter at all.
2145+
// TODO(b/270731363): Enable the verification of the existence filter
2146+
// mismatch once the Firestore emulator is fixed to send an existence
2147+
// filter.
2148+
if (USE_EMULATOR) {
2149+
return;
2150+
}
21712151

2172-
expect(bloomFilter.hashCount, 'bloomFilter.hashCount').to.be.above(0);
2173-
expect(bloomFilter.bitmapLength, 'bloomFilter.bitmapLength').to.be.above(
2174-
0
2175-
);
2176-
expect(bloomFilter.padding, 'bloomFilterPadding').to.be.above(0);
2177-
expect(bloomFilter.padding, 'bloomFilterPadding').to.be.below(8);
2178-
2179-
// Verify that the bloom filter was successfully used to avert a full
2180-
// requery. If a false positive occurred then retry the entire test.
2181-
// Although statistically rare, false positives are expected to happen
2182-
// occasionally. When a false positive _does_ happen, just retry the test
2183-
// with a different set of documents. If that retry _also_ experiences a
2184-
// false positive, then fail the test because that is so improbable that
2185-
// something must have gone wrong.
2186-
if (attemptNumber === 1 && !bloomFilter.applied) {
2187-
return 'retry';
2188-
}
2189-
expect(
2190-
bloomFilter.applied,
2191-
`bloomFilter.applied with attemptNumber=${attemptNumber}`
2192-
).to.be.true;
2152+
// Verify that Watch sent an existence filter with the correct counts
2153+
// when the query was resumed.
2154+
expect(
2155+
existenceFilterMismatches,
2156+
'existenceFilterMismatches'
2157+
).to.have.length(1);
2158+
const { localCacheCount, existenceFilterCount, bloomFilter } =
2159+
existenceFilterMismatches[0];
2160+
expect(localCacheCount, 'localCacheCount').to.equal(100);
2161+
expect(existenceFilterCount, 'existenceFilterCount').to.equal(50);
2162+
2163+
// Verify that Watch sent a valid bloom filter.
2164+
if (!bloomFilter) {
2165+
expect.fail(
2166+
'The existence filter should have specified a bloom filter in its ' +
2167+
'`unchanged_names` field.'
2168+
);
2169+
throw new Error('should never get here');
2170+
}
21932171

2194-
return 'passed';
2195-
};
2172+
expect(bloomFilter.hashCount, 'bloomFilter.hashCount').to.be.above(0);
2173+
expect(
2174+
bloomFilter.bitmapLength,
2175+
'bloomFilter.bitmapLength'
2176+
).to.be.above(0);
2177+
expect(bloomFilter.padding, 'bloomFilterPadding').to.be.above(0);
2178+
expect(bloomFilter.padding, 'bloomFilterPadding').to.be.below(8);
2179+
2180+
// Verify that the bloom filter was successfully used to avert a full
2181+
// requery. If a false positive occurred then retry the entire test.
2182+
// Although statistically rare, false positives are expected to happen
2183+
// occasionally. When a false positive _does_ happen, just retry the
2184+
// test with a different set of documents. If that retry _also_
2185+
// experiences a false positive, then fail the test because that is so
2186+
// improbable that something must have gone wrong.
2187+
if (attemptNumber === 1 && !bloomFilter.applied) {
2188+
throw new RetryError();
2189+
}
21962190

2197-
// Run the test
2198-
let attemptNumber = 0;
2199-
while (true) {
2200-
attemptNumber++;
2201-
const iterationResult = await withTestCollection(
2202-
// Ensure that the local cache is configured to use LRU garbage
2203-
// collection (rather than eager garbage collection) so that the resume
2204-
// token and document data does not get prematurely evicted.
2205-
persistence.toLruGc(),
2206-
testDocs,
2207-
runTestIteration
2208-
);
2209-
if (iterationResult === 'passed') {
2210-
break;
2211-
}
2212-
}
2191+
expect(
2192+
bloomFilter.applied,
2193+
`bloomFilter.applied with attemptNumber=${attemptNumber}`
2194+
).to.be.true;
2195+
});
2196+
});
22132197
}).timeout('90s');
22142198
});
22152199

‎packages/firestore/test/integration/util/helpers.ts

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -384,6 +384,26 @@ export function withTestDocAndInitialData(
384384
});
385385
}
386386

387+
export class RetryError extends Error {
388+
readonly name = 'FirestoreIntegrationTestRetryError';
389+
}
390+
391+
export async function withRetry<T>(
392+
fn: (attemptNumber: number) => Promise<T>
393+
): Promise<T> {
394+
let attemptNumber = 0;
395+
while (true) {
396+
attemptNumber++;
397+
try {
398+
return await fn(attemptNumber);
399+
} catch (error) {
400+
if (!(error instanceof RetryError)) {
401+
throw error;
402+
}
403+
}
404+
}
405+
}
406+
387407
export function withTestCollection<T>(
388408
persistence: PersistenceMode,
389409
docs: { [key: string]: DocumentData },

0 commit comments

Comments
 (0)