Skip to content

Firestore: query.test.ts: use LRUGC in bloom filter tests #7401

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 14 commits into from
Jul 4, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 10 additions & 11 deletions packages/firestore/test/integration/api/query.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@ import {
enableNetwork,
endAt,
endBefore,
memoryLocalCache,
memoryLruGarbageCollector,
Firestore,
GeoPoint,
getDocs,
Expand All @@ -46,6 +48,7 @@ import {
onSnapshot,
or,
orderBy,
persistentLocalCache,
Query,
query,
QuerySnapshot,
Expand Down Expand Up @@ -2128,16 +2131,6 @@ apiDescribe('Queries', (persistence: boolean) => {
);
}

// Skip the verification of the existence filter mismatch when persistence
// is disabled because without persistence there is no resume token
// specified in the subsequent call to getDocs(), and, therefore, Watch
// will _not_ send an existence filter.
// TODO(b/272754156) Re-write this test using a snapshot listener instead
// of calls to getDocs() and remove this check for disabled persistence.
if (!persistence) {
return 'passed';
}

// Skip the verification of the existence filter mismatch when testing
// against the Firestore emulator because the Firestore emulator fails to
// to send an existence filter at all.
Expand Down Expand Up @@ -2193,12 +2186,18 @@ apiDescribe('Queries', (persistence: boolean) => {
return 'passed';
};

// Use LRU memory cache so that the resume token will not be deleted between
// calls to `getDocs()`.
const localCache = persistence
? persistentLocalCache()
: memoryLocalCache({ garbageCollector: memoryLruGarbageCollector() });

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While running locally against Prod, it is not using bloom filter:
Screenshot 2023-06-28 at 3 13 53 PM

// Run the test
let attemptNumber = 0;
while (true) {
attemptNumber++;
const iterationResult = await withTestCollection(
persistence,
localCache,
testDocs,
runTestIteration
);
Expand Down
34 changes: 20 additions & 14 deletions packages/firestore/test/integration/util/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,12 @@ import {
DocumentData,
DocumentReference,
Firestore,
MemoryLocalCache,
memoryLocalCache,
memoryLruGarbageCollector,
newTestApp,
newTestFirestore,
PersistentLocalCache,
persistentLocalCache,
PrivateSettings,
QuerySnapshot,
Expand Down Expand Up @@ -137,7 +139,7 @@ export function toIds(docSet: QuerySnapshot): string[] {
}

export function withTestDb(
persistence: boolean,
persistence: boolean | PersistentLocalCache | MemoryLocalCache,
fn: (db: Firestore) => Promise<void>
): Promise<void> {
return withTestDbs(persistence, 1, ([db]) => {
Expand All @@ -160,7 +162,7 @@ export function withEnsuredEagerGcTestDb(
}

export function withEnsuredLruGcTestDb(
persistence: boolean,
persistence: boolean | PersistentLocalCache | MemoryLocalCache,
fn: (db: Firestore) => Promise<void>
): Promise<void> {
const newSettings = { ...DEFAULT_SETTINGS };
Expand Down Expand Up @@ -188,7 +190,7 @@ export function withEnsuredLruGcTestDb(

/** Runs provided fn with a db for an alternate project id. */
export function withAlternateTestDb(
persistence: boolean,
persistence: boolean | PersistentLocalCache | MemoryLocalCache,
fn: (db: Firestore) => Promise<void>
): Promise<void> {
return withTestDbsSettings(
Expand All @@ -203,7 +205,7 @@ export function withAlternateTestDb(
}

export function withTestDbs(
persistence: boolean,
persistence: boolean | PersistentLocalCache | MemoryLocalCache,
numDbs: number,
fn: (db: Firestore[]) => Promise<void>
): Promise<void> {
Expand All @@ -216,7 +218,7 @@ export function withTestDbs(
);
}
export async function withTestDbsSettings<T>(
persistence: boolean,
persistence: boolean | PersistentLocalCache | MemoryLocalCache,
projectId: string,
settings: PrivateSettings,
numDbs: number,
Expand Down Expand Up @@ -250,7 +252,7 @@ export async function withTestDbsSettings<T>(
}

export async function withNamedTestDbsOrSkipUnlessUsingEmulator(
persistence: boolean,
persistence: boolean | PersistentLocalCache | MemoryLocalCache,
dbNames: string[],
fn: (db: Firestore[]) => Promise<void>
): Promise<void> {
Expand All @@ -265,8 +267,12 @@ export async function withNamedTestDbsOrSkipUnlessUsingEmulator(
const dbs: Firestore[] = [];
for (const dbName of dbNames) {
const newSettings = { ...DEFAULT_SETTINGS };
if (persistence) {
newSettings.localCache = persistentLocalCache();
if (typeof persistence === 'boolean') {
if (persistence) {
newSettings.localCache = persistentLocalCache();
}
} else {
newSettings.localCache = persistence;
}
const db = newTestFirestore(app, newSettings, dbName);
dbs.push(db);
Expand All @@ -285,7 +291,7 @@ export async function withNamedTestDbsOrSkipUnlessUsingEmulator(
}

export function withTestDoc(
persistence: boolean,
persistence: boolean | PersistentLocalCache | MemoryLocalCache,
fn: (doc: DocumentReference, db: Firestore) => Promise<void>
): Promise<void> {
return withTestDb(persistence, db => {
Expand All @@ -294,7 +300,7 @@ export function withTestDoc(
}

export function withTestDocAndSettings(
persistence: boolean,
persistence: boolean | PersistentLocalCache | MemoryLocalCache,
settings: PrivateSettings,
fn: (doc: DocumentReference) => Promise<void>
): Promise<void> {
Expand All @@ -315,7 +321,7 @@ export function withTestDocAndSettings(
// `withTestDoc(..., docRef => { setDoc(docRef, initialData) ...});` that
// otherwise is quite common.
export function withTestDocAndInitialData(
persistence: boolean,
persistence: boolean | PersistentLocalCache | MemoryLocalCache,
initialData: DocumentData | null,
fn: (doc: DocumentReference, db: Firestore) => Promise<void>
): Promise<void> {
Expand All @@ -330,15 +336,15 @@ export function withTestDocAndInitialData(
}

export function withTestCollection<T>(
persistence: boolean,
persistence: boolean | PersistentLocalCache | MemoryLocalCache,
docs: { [key: string]: DocumentData },
fn: (collection: CollectionReference, db: Firestore) => Promise<T>
): Promise<T> {
return withTestCollectionSettings(persistence, DEFAULT_SETTINGS, docs, fn);
}

export function withEmptyTestCollection(
persistence: boolean,
persistence: boolean | PersistentLocalCache | MemoryLocalCache,
fn: (collection: CollectionReference, db: Firestore) => Promise<void>
): Promise<void> {
return withTestCollection(persistence, {}, fn);
Expand All @@ -347,7 +353,7 @@ export function withEmptyTestCollection(
// TODO(mikelehen): Once we wipe the database between tests, we can probably
// return the same collection every time.
export function withTestCollectionSettings<T>(
persistence: boolean,
persistence: boolean | PersistentLocalCache | MemoryLocalCache,
settings: PrivateSettings,
docs: { [key: string]: DocumentData },
fn: (collection: CollectionReference, db: Firestore) => Promise<T>
Expand Down