From 1c429094e047be657ea4723ee576be853a176593 Mon Sep 17 00:00:00 2001 From: milaGGL <107142260+milaGGL@users.noreply.github.com> Date: Thu, 14 Sep 2023 17:13:52 -0400 Subject: [PATCH 01/50] testing out the changes --- .../api/composite_index_query.test.ts | 53 +++++++ .../util/composite_index_test_helper.ts | 134 ++++++++++++++++++ .../test/integration/util/helpers.ts | 19 ++- .../test/integration/util/settings.ts | 3 + 4 files changed, 208 insertions(+), 1 deletion(-) create mode 100644 packages/firestore/test/integration/api/composite_index_query.test.ts create mode 100644 packages/firestore/test/integration/util/composite_index_test_helper.ts diff --git a/packages/firestore/test/integration/api/composite_index_query.test.ts b/packages/firestore/test/integration/api/composite_index_query.test.ts new file mode 100644 index 00000000000..857aeec9c0e --- /dev/null +++ b/packages/firestore/test/integration/api/composite_index_query.test.ts @@ -0,0 +1,53 @@ +/** + * @license + * Copyright 2017 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { expect } from 'chai'; + +import { apiDescribe, toIds } from '../util/helpers'; +import { where, getDocs, orderBy } from '../util/firebase_export'; +import { CompositeIndexTestHelper } from '../util/composite_index_test_helper'; + +apiDescribe.skip('Queries', persistence => { + describe('query with OrderBy fields', () => { + it('can query by field and use order by', () => { + const testDocs = { + doc1: { key: 'a', sort: 3 }, + doc2: { key: 'b', sort: 2 }, + doc3: { key: 'c', sort: 1 } + }; + const testHelper = new CompositeIndexTestHelper(); + return testHelper.withTestDocs(persistence, testDocs, async coll => { + const filteredQuery = testHelper.query(coll, where('key', '!=', 'c')); + const result = await getDocs(filteredQuery); + expect(toIds(result)).to.deep.equal(['doc1', 'doc2']); + }); + }); + + it('can query by field and use order by', () => { + const testDocs = { + doc1: { key: 'a', sort: 1 }, + doc2: { key: 'b', sort: 2, v: 1 } + }; + const testHelper = new CompositeIndexTestHelper(); + return testHelper.withTestDocs(persistence, testDocs, async coll => { + const filteredQuery = testHelper.query(coll, where('key', '!=', 'd')); + const result = await getDocs(filteredQuery); + expect(toIds(result)).to.deep.equal(['doc1', 'doc2']); + }); + }); + }); +}); diff --git a/packages/firestore/test/integration/util/composite_index_test_helper.ts b/packages/firestore/test/integration/util/composite_index_test_helper.ts new file mode 100644 index 00000000000..27d499fa7d5 --- /dev/null +++ b/packages/firestore/test/integration/util/composite_index_test_helper.ts @@ -0,0 +1,134 @@ +import { AutoId } from '../../../src/util/misc'; +import { + batchCommitDocsToCollection, + PERSISTENCE_MODE_UNSPECIFIED, + PersistenceMode, +} from './helpers'; +import { + query, + CollectionReference, + DocumentData, + Firestore, + Query, + QueryConstraint, + where, +} from '../../../src'; +import { + COMPOSITE_INDEX_TEST_COLLECTION, + DEFAULT_SETTINGS +} from './settings'; + +export class CompositeIndexTestHelper { + private readonly testId: string = 'test-id-' + AutoId.newId(); + + async withTestDocs( + persistence: PersistenceMode | typeof PERSISTENCE_MODE_UNSPECIFIED, + docs: { [key: string]: DocumentData }, + fn: (collection: CollectionReference, db: Firestore) => Promise + ): Promise { + this.addTestId(docs); + return batchCommitDocsToCollection( + persistence, + DEFAULT_SETTINGS, + docs, + COMPOSITE_INDEX_TEST_COLLECTION, + fn + ); + + // return withTestDbsSettings( + // persistence, + // DEFAULT_PROJECT_ID, + // DEFAULT_SETTINGS, + // 2, + // ([testDb, setupDb]) => { + // const testCollection = collection( + // testDb, + // COMPOSITE_INDEX_TEST_COLLECTION + // ); + // + // return Promise.all( + // batchCommitDocsToCollection( + // setupDb, + // COMPOSITE_INDEX_TEST_COLLECTION, + // docs + // ) + // ).then(() => fn(testCollection, testDb)); + // } + // ); + } + + // Add test-id to docs created under a specific test. + private addTestId(docs: { [key: string]: DocumentData }): void { + for (const key in docs) { + if (docs.hasOwnProperty(key)) { + docs[key]['test-id'] = this.testId; + } + } + } + + // add filter on test id + query( + query_: Query, + ...queryConstraints: QueryConstraint[] + ): Query { + return query( + query_, + where('test-id', '==', this.testId), + ...queryConstraints + ); + } + + // add doc with test id + // set doc with test-id +} + +// export function batchCommitDocsToCollection(collectionId: string){ +// return withTestDbsSettings( +// persistence, +// DEFAULT_PROJECT_ID, +// settings, +// 2, +// ([testDb, setupDb]) => { +// // Abuse .doc() to get a random ID. +// const testCollection = collection(testDb, collectionId); +// +// return Promise.all( +// batchCommitDocs(setupDb, collectionId, docs) +// ).then(() => fn(testCollection, testDb)); +// } +// ); +// +// } +// +// export function batchCommitDocs( +// db: Firestore, +// collectionId: string, +// docs: { [key: string]: DocumentData } +// ): Array> { +// const collectionRef = collection(db, collectionId); +// +// const writeBatchCommits: Array> = []; +// let writeBatch_: WriteBatch | null = null; +// +// let writeBatchSize = 0; +// for (const key of Object.keys(docs)) { +// if (writeBatch_ === null) { +// writeBatch_ = writeBatch(db); +// } +// writeBatch_.set(doc(collectionRef, key), docs[key]); +// writeBatchSize++; +// +// // Write batches are capped at 500 writes. Use 400 just to be safe. +// if (writeBatchSize === 400) { +// writeBatchCommits.push(writeBatch_.commit()); +// writeBatch_ = null; +// writeBatchSize = 0; +// } +// } +// +// if (writeBatch_ !== null) { +// writeBatchCommits.push(writeBatch_.commit()); +// } +// +// return writeBatchCommits; +// } diff --git a/packages/firestore/test/integration/util/helpers.ts b/packages/firestore/test/integration/util/helpers.ts index 9f97ec6bf79..4216bf7924d 100644 --- a/packages/firestore/test/integration/util/helpers.ts +++ b/packages/firestore/test/integration/util/helpers.ts @@ -48,6 +48,7 @@ import { TARGET_DB_ID, USE_EMULATOR } from './settings'; +import { AutoId } from '../../../src/util/misc'; /* eslint-disable no-restricted-globals */ @@ -441,6 +442,23 @@ export function withTestCollectionSettings( settings: PrivateSettings, docs: { [key: string]: DocumentData }, fn: (collection: CollectionReference, db: Firestore) => Promise +): Promise { + const collectionId = AutoId.newId(); + return batchCommitDocsToCollection( + persistence, + settings, + docs, + collectionId, + fn + ); +} + +export function batchCommitDocsToCollection( + persistence: PersistenceMode | typeof PERSISTENCE_MODE_UNSPECIFIED, + settings: PrivateSettings, + docs: { [key: string]: DocumentData }, + collectionId: string, + fn: (collection: CollectionReference, db: Firestore) => Promise ): Promise { return withTestDbsSettings( persistence, @@ -449,7 +467,6 @@ export function withTestCollectionSettings( 2, ([testDb, setupDb]) => { // Abuse .doc() to get a random ID. - const collectionId = 'test-collection-' + doc(collection(testDb, 'x')).id; const testCollection = collection(testDb, collectionId); const setupCollection = collection(setupDb, collectionId); diff --git a/packages/firestore/test/integration/util/settings.ts b/packages/firestore/test/integration/util/settings.ts index e57c896d977..65318363cd6 100644 --- a/packages/firestore/test/integration/util/settings.ts +++ b/packages/firestore/test/integration/util/settings.ts @@ -116,3 +116,6 @@ export const DEFAULT_PROJECT_ID = USE_EMULATOR ? process.env.FIRESTORE_EMULATOR_PROJECT_ID || 'test-emulator' : PROJECT_CONFIG.projectId; export const ALT_PROJECT_ID = 'test-db2'; + +export const COMPOSITE_INDEX_TEST_COLLECTION = + 'composite-index-test-collection'; From 2e8e4436f312abeddfd911c8e5cdf55e9c4196a2 Mon Sep 17 00:00:00 2001 From: milaGGL <107142260+milaGGL@users.noreply.github.com> Date: Fri, 15 Sep 2023 14:03:18 -0400 Subject: [PATCH 02/50] format --- .../api/composite_index_query.test.ts | 53 ++++-- .../util/composite_index_test_helper.ts | 163 ++++++++---------- .../test/integration/util/helpers.ts | 4 +- 3 files changed, 118 insertions(+), 102 deletions(-) diff --git a/packages/firestore/test/integration/api/composite_index_query.test.ts b/packages/firestore/test/integration/api/composite_index_query.test.ts index 857aeec9c0e..f65419e7713 100644 --- a/packages/firestore/test/integration/api/composite_index_query.test.ts +++ b/packages/firestore/test/integration/api/composite_index_query.test.ts @@ -17,37 +17,68 @@ import { expect } from 'chai'; -import { apiDescribe, toIds } from '../util/helpers'; -import { where, getDocs, orderBy } from '../util/firebase_export'; import { CompositeIndexTestHelper } from '../util/composite_index_test_helper'; +import { where, getDocs, orderBy, getDoc, doc } from '../util/firebase_export'; +import { apiDescribe, toDataArray, toIds } from '../util/helpers'; -apiDescribe.skip('Queries', persistence => { +apiDescribe('Queries', persistence => { describe('query with OrderBy fields', () => { - it('can query by field and use order by', () => { + it('can run composite index query', () => { const testDocs = { doc1: { key: 'a', sort: 3 }, - doc2: { key: 'b', sort: 2 }, - doc3: { key: 'c', sort: 1 } + doc2: { key: 'a', sort: 2 }, + doc3: { key: 'b', sort: 1 } }; const testHelper = new CompositeIndexTestHelper(); return testHelper.withTestDocs(persistence, testDocs, async coll => { - const filteredQuery = testHelper.query(coll, where('key', '!=', 'c')); + const filteredQuery = testHelper.query( + coll, + where('key', '==', 'a'), + orderBy('sort') + ); const result = await getDocs(filteredQuery); - expect(toIds(result)).to.deep.equal(['doc1', 'doc2']); + expect(toIds(result)).to.deep.equal(['doc2', 'doc1']); }); }); - it('can query by field and use order by', () => { + it('data isolation is working', () => { const testDocs = { doc1: { key: 'a', sort: 1 }, - doc2: { key: 'b', sort: 2, v: 1 } + doc2: { key: 'a', sort: 2 } }; const testHelper = new CompositeIndexTestHelper(); return testHelper.withTestDocs(persistence, testDocs, async coll => { - const filteredQuery = testHelper.query(coll, where('key', '!=', 'd')); + const filteredQuery = testHelper.query( + coll, + where('key', '==', 'a'), + orderBy('sort') + ); const result = await getDocs(filteredQuery); expect(toIds(result)).to.deep.equal(['doc1', 'doc2']); }); }); + + it('can do addDoc and setDoc', () => { + const testHelper = new CompositeIndexTestHelper(); + return testHelper.withEmptyCollection(persistence, async coll => { + const docRef1 = await testHelper.addDoc(coll, { key: 'a', sort: 1 }); + const docRef2 = doc(coll, 'setDoc'); + await testHelper.setDoc(docRef2, { key: 'a', sort: 2 }); + + const docSnap1 = await getDoc(docRef1); + const docSnap2 = await getDoc(docRef2); + + const filteredQuery = testHelper.query( + coll, + where('key', '==', 'a'), + orderBy('sort') + ); + const result = await getDocs(filteredQuery); + expect(toDataArray(result)).to.deep.equal([ + docSnap1.data(), + docSnap2.data() + ]); + }); + }); }); }); diff --git a/packages/firestore/test/integration/util/composite_index_test_helper.ts b/packages/firestore/test/integration/util/composite_index_test_helper.ts index 27d499fa7d5..19a568012d8 100644 --- a/packages/firestore/test/integration/util/composite_index_test_helper.ts +++ b/packages/firestore/test/integration/util/composite_index_test_helper.ts @@ -1,9 +1,22 @@ +/** + * @license + * Copyright 2023 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + import { AutoId } from '../../../src/util/misc'; -import { - batchCommitDocsToCollection, - PERSISTENCE_MODE_UNSPECIFIED, - PersistenceMode, -} from './helpers'; + import { query, CollectionReference, @@ -12,21 +25,33 @@ import { Query, QueryConstraint, where, -} from '../../../src'; + WithFieldValue, + DocumentReference, + addDoc as addDocument, + setDoc as setDocument +} from './firebase_export'; import { - COMPOSITE_INDEX_TEST_COLLECTION, - DEFAULT_SETTINGS -} from './settings'; + batchCommitDocsToCollection, + PERSISTENCE_MODE_UNSPECIFIED, + PersistenceMode +} from './helpers'; +import { COMPOSITE_INDEX_TEST_COLLECTION, DEFAULT_SETTINGS } from './settings'; export class CompositeIndexTestHelper { - private readonly testId: string = 'test-id-' + AutoId.newId(); + private readonly testId: string; + private readonly TEST_ID_FIELD: string = 'testId'; + + constructor() { + // Initialize the testId when an instance of the class is created. + this.testId = 'test-id-' + AutoId.newId(); + } async withTestDocs( persistence: PersistenceMode | typeof PERSISTENCE_MODE_UNSPECIFIED, docs: { [key: string]: DocumentData }, fn: (collection: CollectionReference, db: Firestore) => Promise ): Promise { - this.addTestId(docs); + this.addTestIdToDocs(docs); return batchCommitDocsToCollection( persistence, DEFAULT_SETTINGS, @@ -34,38 +59,32 @@ export class CompositeIndexTestHelper { COMPOSITE_INDEX_TEST_COLLECTION, fn ); - - // return withTestDbsSettings( - // persistence, - // DEFAULT_PROJECT_ID, - // DEFAULT_SETTINGS, - // 2, - // ([testDb, setupDb]) => { - // const testCollection = collection( - // testDb, - // COMPOSITE_INDEX_TEST_COLLECTION - // ); - // - // return Promise.all( - // batchCommitDocsToCollection( - // setupDb, - // COMPOSITE_INDEX_TEST_COLLECTION, - // docs - // ) - // ).then(() => fn(testCollection, testDb)); - // } - // ); + } + async withEmptyCollection( + persistence: PersistenceMode | typeof PERSISTENCE_MODE_UNSPECIFIED, + fn: (collection: CollectionReference, db: Firestore) => Promise + ): Promise { + return batchCommitDocsToCollection( + persistence, + DEFAULT_SETTINGS, + {}, + COMPOSITE_INDEX_TEST_COLLECTION, + fn + ); } // Add test-id to docs created under a specific test. - private addTestId(docs: { [key: string]: DocumentData }): void { - for (const key in docs) { - if (docs.hasOwnProperty(key)) { - docs[key]['test-id'] = this.testId; - } + // Docs are modified instead of deep copy for convenience of equality check in tests. + private addTestIdToDocs(docs: { [key: string]: DocumentData }): void { + for (const doc of Object.values(docs)) { + doc[this.TEST_ID_FIELD] = this.testId; } } + private addTestIdToDoc(doc: DocumentData): void { + doc[this.TEST_ID_FIELD] = this.testId; + } + // add filter on test id query( query_: Query, @@ -73,62 +92,28 @@ export class CompositeIndexTestHelper { ): Query { return query( query_, - where('test-id', '==', this.testId), + where(this.TEST_ID_FIELD, '==', this.testId), ...queryConstraints ); } // add doc with test id + addDoc( + reference: CollectionReference, + data: WithFieldValue + ): Promise> { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + (data as Record)[this.TEST_ID_FIELD] = this.testId; + return addDocument(reference, data); + } + // set doc with test-id + setDoc( + reference: DocumentReference, + data: WithFieldValue + ): Promise { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + (data as Record)[this.TEST_ID_FIELD] = this.testId; + return setDocument(reference, data); + } } - -// export function batchCommitDocsToCollection(collectionId: string){ -// return withTestDbsSettings( -// persistence, -// DEFAULT_PROJECT_ID, -// settings, -// 2, -// ([testDb, setupDb]) => { -// // Abuse .doc() to get a random ID. -// const testCollection = collection(testDb, collectionId); -// -// return Promise.all( -// batchCommitDocs(setupDb, collectionId, docs) -// ).then(() => fn(testCollection, testDb)); -// } -// ); -// -// } -// -// export function batchCommitDocs( -// db: Firestore, -// collectionId: string, -// docs: { [key: string]: DocumentData } -// ): Array> { -// const collectionRef = collection(db, collectionId); -// -// const writeBatchCommits: Array> = []; -// let writeBatch_: WriteBatch | null = null; -// -// let writeBatchSize = 0; -// for (const key of Object.keys(docs)) { -// if (writeBatch_ === null) { -// writeBatch_ = writeBatch(db); -// } -// writeBatch_.set(doc(collectionRef, key), docs[key]); -// writeBatchSize++; -// -// // Write batches are capped at 500 writes. Use 400 just to be safe. -// if (writeBatchSize === 400) { -// writeBatchCommits.push(writeBatch_.commit()); -// writeBatch_ = null; -// writeBatchSize = 0; -// } -// } -// -// if (writeBatch_ !== null) { -// writeBatchCommits.push(writeBatch_.commit()); -// } -// -// return writeBatchCommits; -// } diff --git a/packages/firestore/test/integration/util/helpers.ts b/packages/firestore/test/integration/util/helpers.ts index 4216bf7924d..7976b3f8da3 100644 --- a/packages/firestore/test/integration/util/helpers.ts +++ b/packages/firestore/test/integration/util/helpers.ts @@ -17,6 +17,8 @@ import { isIndexedDBAvailable } from '@firebase/util'; +import { AutoId } from '../../../src/util/misc'; + import { clearIndexedDbPersistence, collection, @@ -48,7 +50,6 @@ import { TARGET_DB_ID, USE_EMULATOR } from './settings'; -import { AutoId } from '../../../src/util/misc'; /* eslint-disable no-restricted-globals */ @@ -466,7 +467,6 @@ export function batchCommitDocsToCollection( settings, 2, ([testDb, setupDb]) => { - // Abuse .doc() to get a random ID. const testCollection = collection(testDb, collectionId); const setupCollection = collection(setupDb, collectionId); From 1e166e5255a1f3abd0a3fdcf04c5e3e87bb9e833 Mon Sep 17 00:00:00 2001 From: milaGGL <107142260+milaGGL@users.noreply.github.com> Date: Fri, 15 Sep 2023 14:08:14 -0400 Subject: [PATCH 03/50] Update composite_index_query.test.ts --- .../test/integration/api/composite_index_query.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/firestore/test/integration/api/composite_index_query.test.ts b/packages/firestore/test/integration/api/composite_index_query.test.ts index f65419e7713..77949cc3d7e 100644 --- a/packages/firestore/test/integration/api/composite_index_query.test.ts +++ b/packages/firestore/test/integration/api/composite_index_query.test.ts @@ -1,6 +1,6 @@ /** * @license - * Copyright 2017 Google LLC + * Copyright 2023 Google LLC * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. From 85933f6aa14445f0c8b574f4fa861b5d0afaf759 Mon Sep 17 00:00:00 2001 From: milaGGL <107142260+milaGGL@users.noreply.github.com> Date: Fri, 15 Sep 2023 16:37:32 -0400 Subject: [PATCH 04/50] try github workflow --- .../test-changed-firestore-integration.yml | 18 ++++++++++++++ packages/firestore/composite_index.tf | 24 +++++++++++++++++++ .../api/composite_index_query.test.ts | 3 ++- .../util/composite_index_test_helper.ts | 6 ++--- 4 files changed, 47 insertions(+), 4 deletions(-) create mode 100644 packages/firestore/composite_index.tf diff --git a/.github/workflows/test-changed-firestore-integration.yml b/.github/workflows/test-changed-firestore-integration.yml index c45a706d80d..b792df247af 100644 --- a/.github/workflows/test-changed-firestore-integration.yml +++ b/.github/workflows/test-changed-firestore-integration.yml @@ -13,6 +13,24 @@ jobs: with: # This makes Actions fetch all Git history so run-changed script can diff properly. fetch-depth: 0 + - name: Setup Terraform + uses: hashicorp/setup-terraform@v2 + - name: Terraform Init + id: init + run: terraform init + continue-on-error: true + - name: Terraform Validate + id: validate + run: terraform validate -no-color + continue-on-error: true + - name: Terraform Plan + id: plan + if: github.event_name == 'pull_request' + run: terraform plan -no-color -input=false + continue-on-error: true + - name: Terraform Apply + if: github.ref == 'refs/heads/master' && github.event_name == 'push' + run: terraform apply -auto-approve -input=false - name: Set up Node (16) uses: actions/setup-node@v3 with: diff --git a/packages/firestore/composite_index.tf b/packages/firestore/composite_index.tf new file mode 100644 index 00000000000..8b0295e04ce --- /dev/null +++ b/packages/firestore/composite_index.tf @@ -0,0 +1,24 @@ +locals { + data = jsondecode(file("../../config/project.json")) +} + +resource "google_firestore_index" "my-index" { + project = local.data.projectId + collection = "composite-index-test-collection" + + fields { + field_path = "key" + order = "ASCENDING" + } + + fields { + field_path = "testId" + order = "DESCENDING" + } + + fields { + field_path = "sort" + order = "DESCENDING" + } + +} diff --git a/packages/firestore/test/integration/api/composite_index_query.test.ts b/packages/firestore/test/integration/api/composite_index_query.test.ts index 77949cc3d7e..a86ba4aaa9b 100644 --- a/packages/firestore/test/integration/api/composite_index_query.test.ts +++ b/packages/firestore/test/integration/api/composite_index_query.test.ts @@ -22,7 +22,8 @@ import { where, getDocs, orderBy, getDoc, doc } from '../util/firebase_export'; import { apiDescribe, toDataArray, toIds } from '../util/helpers'; apiDescribe('Queries', persistence => { - describe('query with OrderBy fields', () => { + // eslint-disable-next-line no-restricted-properties + describe.only('query with OrderBy fields', () => { it('can run composite index query', () => { const testDocs = { doc1: { key: 'a', sort: 3 }, diff --git a/packages/firestore/test/integration/util/composite_index_test_helper.ts b/packages/firestore/test/integration/util/composite_index_test_helper.ts index 19a568012d8..c66c41454b8 100644 --- a/packages/firestore/test/integration/util/composite_index_test_helper.ts +++ b/packages/firestore/test/integration/util/composite_index_test_helper.ts @@ -108,9 +108,9 @@ export class CompositeIndexTestHelper { } // set doc with test-id - setDoc( - reference: DocumentReference, - data: WithFieldValue + setDoc( + reference: DocumentReference, + data: WithFieldValue ): Promise { // eslint-disable-next-line @typescript-eslint/no-explicit-any (data as Record)[this.TEST_ID_FIELD] = this.testId; From b073d8fbb777692d56a9753f089c2d1301b51282 Mon Sep 17 00:00:00 2001 From: milaGGL <107142260+milaGGL@users.noreply.github.com> Date: Mon, 18 Sep 2023 09:40:40 -0400 Subject: [PATCH 05/50] move .tf file up --- packages/firestore/composite_index.tf => composite_index.tf | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) rename packages/firestore/composite_index.tf => composite_index.tf (86%) diff --git a/packages/firestore/composite_index.tf b/composite_index.tf similarity index 86% rename from packages/firestore/composite_index.tf rename to composite_index.tf index 8b0295e04ce..e38a9531f77 100644 --- a/packages/firestore/composite_index.tf +++ b/composite_index.tf @@ -1,5 +1,5 @@ locals { - data = jsondecode(file("../../config/project.json")) + data = jsondecode(file("./config/project.json")) } resource "google_firestore_index" "my-index" { From b8f4bee32e84d61939f962a7c183c348237e02e4 Mon Sep 17 00:00:00 2001 From: milaGGL <107142260+milaGGL@users.noreply.github.com> Date: Mon, 18 Sep 2023 10:56:49 -0400 Subject: [PATCH 06/50] copy project.json --- .../test-changed-firestore-integration.yml | 5 ++-- composite_index.tf | 23 ++++++++++++++++++- 2 files changed, 25 insertions(+), 3 deletions(-) diff --git a/.github/workflows/test-changed-firestore-integration.yml b/.github/workflows/test-changed-firestore-integration.yml index b792df247af..fba90048bfb 100644 --- a/.github/workflows/test-changed-firestore-integration.yml +++ b/.github/workflows/test-changed-firestore-integration.yml @@ -21,7 +21,9 @@ jobs: continue-on-error: true - name: Terraform Validate id: validate - run: terraform validate -no-color + run: | + cp config/ci.config.json config/project.json + terraform validate -no-color continue-on-error: true - name: Terraform Plan id: plan @@ -29,7 +31,6 @@ jobs: run: terraform plan -no-color -input=false continue-on-error: true - name: Terraform Apply - if: github.ref == 'refs/heads/master' && github.event_name == 'push' run: terraform apply -auto-approve -input=false - name: Set up Node (16) uses: actions/setup-node@v3 diff --git a/composite_index.tf b/composite_index.tf index e38a9531f77..7622c3a066c 100644 --- a/composite_index.tf +++ b/composite_index.tf @@ -1,5 +1,5 @@ locals { - data = jsondecode(file("./config/project.json")) + data = jsondecode(file("config/project.json")) } resource "google_firestore_index" "my-index" { @@ -22,3 +22,24 @@ resource "google_firestore_index" "my-index" { } } +resource "google_firestore_index" "my-index" { + project = local.data.projectId + collection = "composite-index-test-collection" + database = "test-db" + + fields { + field_path = "key" + order = "ASCENDING" + } + + fields { + field_path = "testId" + order = "DESCENDING" + } + + fields { + field_path = "sort" + order = "DESCENDING" + } + +} \ No newline at end of file From 4d75c751bc22d6b3b05ec7b2f68d35d7c671bc78 Mon Sep 17 00:00:00 2001 From: milaGGL <107142260+milaGGL@users.noreply.github.com> Date: Mon, 18 Sep 2023 10:58:12 -0400 Subject: [PATCH 07/50] Update composite_index.tf --- composite_index.tf | 22 ---------------------- 1 file changed, 22 deletions(-) diff --git a/composite_index.tf b/composite_index.tf index 7622c3a066c..d07e870ced3 100644 --- a/composite_index.tf +++ b/composite_index.tf @@ -20,26 +20,4 @@ resource "google_firestore_index" "my-index" { field_path = "sort" order = "DESCENDING" } - -} -resource "google_firestore_index" "my-index" { - project = local.data.projectId - collection = "composite-index-test-collection" - database = "test-db" - - fields { - field_path = "key" - order = "ASCENDING" - } - - fields { - field_path = "testId" - order = "DESCENDING" - } - - fields { - field_path = "sort" - order = "DESCENDING" - } - } \ No newline at end of file From e0d3a11a0e5427bbdb7e60c6895e9a1539939761 Mon Sep 17 00:00:00 2001 From: milaGGL <107142260+milaGGL@users.noreply.github.com> Date: Mon, 18 Sep 2023 11:05:53 -0400 Subject: [PATCH 08/50] Update composite_index.tf --- composite_index.tf | 2 ++ 1 file changed, 2 insertions(+) diff --git a/composite_index.tf b/composite_index.tf index d07e870ced3..c91e3f10183 100644 --- a/composite_index.tf +++ b/composite_index.tf @@ -3,6 +3,8 @@ locals { } resource "google_firestore_index" "my-index" { + provider = google + project = local.data.projectId collection = "composite-index-test-collection" From b869a528a988d7d9a731de8fc60a660f5288e6a6 Mon Sep 17 00:00:00 2001 From: milaGGL <107142260+milaGGL@users.noreply.github.com> Date: Mon, 18 Sep 2023 11:09:43 -0400 Subject: [PATCH 09/50] Update composite_index.tf --- composite_index.tf | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/composite_index.tf b/composite_index.tf index c91e3f10183..05146df50d3 100644 --- a/composite_index.tf +++ b/composite_index.tf @@ -2,9 +2,12 @@ locals { data = jsondecode(file("config/project.json")) } -resource "google_firestore_index" "my-index" { - provider = google +provider "google" { + project = local.data.projectId + region = "us-central1" +} +resource "google_firestore_index" "my-index" { project = local.data.projectId collection = "composite-index-test-collection" From 6f3b9839f55309225c1212a35adcb4d5d44bbab6 Mon Sep 17 00:00:00 2001 From: milaGGL <107142260+milaGGL@users.noreply.github.com> Date: Mon, 18 Sep 2023 12:56:29 -0400 Subject: [PATCH 10/50] add env: --- .../test-changed-firestore-integration.yml | 20 ++++++------- composite_index.tf | 29 +++++++++++++++---- 2 files changed, 34 insertions(+), 15 deletions(-) diff --git a/.github/workflows/test-changed-firestore-integration.yml b/.github/workflows/test-changed-firestore-integration.yml index fba90048bfb..93fc248585b 100644 --- a/.github/workflows/test-changed-firestore-integration.yml +++ b/.github/workflows/test-changed-firestore-integration.yml @@ -13,25 +13,25 @@ jobs: with: # This makes Actions fetch all Git history so run-changed script can diff properly. fetch-depth: 0 + + # create composite indexes with Terraform - name: Setup Terraform uses: hashicorp/setup-terraform@v2 - name: Terraform Init - id: init run: terraform init continue-on-error: true - - name: Terraform Validate - id: validate - run: | - cp config/ci.config.json config/project.json - terraform validate -no-color - continue-on-error: true - name: Terraform Plan - id: plan if: github.event_name == 'pull_request' - run: terraform plan -no-color -input=false + run: | + cp config/ci.config.json config/project.json + terraform plan + env: + GOOGLE_APPLICATION_CREDENTIALS: ${{secrets.FIREBASE_CLI_TOKEN}} continue-on-error: true - name: Terraform Apply - run: terraform apply -auto-approve -input=false + run: terraform apply -auto-approve + continue-on-error: true + - name: Set up Node (16) uses: actions/setup-node@v3 with: diff --git a/composite_index.tf b/composite_index.tf index 05146df50d3..358ee7916d7 100644 --- a/composite_index.tf +++ b/composite_index.tf @@ -2,14 +2,33 @@ locals { data = jsondecode(file("config/project.json")) } -provider "google" { - project = local.data.projectId - region = "us-central1" + provider "google" { + project = local.data.projectId + region = "us-central1" + } + +resource "google_firestore_index" "default-db-index" { + collection = "composite-index-test-collection" + + fields { + field_path = "key" + order = "ASCENDING" + } + + fields { + field_path = "testId" + order = "DESCENDING" + } + + fields { + field_path = "sort" + order = "DESCENDING" + } } -resource "google_firestore_index" "my-index" { - project = local.data.projectId +resource "google_firestore_index" "named-db-index" { collection = "composite-index-test-collection" + database = "test-db" fields { field_path = "key" From f26a87e8326c6b08215b1e6bd433f32ff8f3b7cf Mon Sep 17 00:00:00 2001 From: milaGGL <107142260+milaGGL@users.noreply.github.com> Date: Mon, 18 Sep 2023 13:18:06 -0400 Subject: [PATCH 11/50] Update test-changed-firestore-integration.yml --- .github/workflows/test-changed-firestore-integration.yml | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/.github/workflows/test-changed-firestore-integration.yml b/.github/workflows/test-changed-firestore-integration.yml index 93fc248585b..5e4ade3ef6b 100644 --- a/.github/workflows/test-changed-firestore-integration.yml +++ b/.github/workflows/test-changed-firestore-integration.yml @@ -13,7 +13,6 @@ jobs: with: # This makes Actions fetch all Git history so run-changed script can diff properly. fetch-depth: 0 - # create composite indexes with Terraform - name: Setup Terraform uses: hashicorp/setup-terraform@v2 @@ -26,7 +25,7 @@ jobs: cp config/ci.config.json config/project.json terraform plan env: - GOOGLE_APPLICATION_CREDENTIALS: ${{secrets.FIREBASE_CLI_TOKEN}} + GOOGLE_APPLICATION_CREDENTIALS: '${{ secrets.GCP_SA_KEY }}' continue-on-error: true - name: Terraform Apply run: terraform apply -auto-approve From c62c8aa58866a4147fc1f4651324633ad725e2d7 Mon Sep 17 00:00:00 2001 From: milaGGL <107142260+milaGGL@users.noreply.github.com> Date: Mon, 18 Sep 2023 13:19:40 -0400 Subject: [PATCH 12/50] Update test-changed-firestore-integration.yml --- .github/workflows/test-changed-firestore-integration.yml | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/.github/workflows/test-changed-firestore-integration.yml b/.github/workflows/test-changed-firestore-integration.yml index 5e4ade3ef6b..ea13c554488 100644 --- a/.github/workflows/test-changed-firestore-integration.yml +++ b/.github/workflows/test-changed-firestore-integration.yml @@ -13,6 +13,9 @@ jobs: with: # This makes Actions fetch all Git history so run-changed script can diff properly. fetch-depth: 0 + - uses: 'google-github-actions/auth@v0' + with: + credentials_json: '${{ secrets.GCP_SA_KEY }}' # create composite indexes with Terraform - name: Setup Terraform uses: hashicorp/setup-terraform@v2 @@ -24,8 +27,6 @@ jobs: run: | cp config/ci.config.json config/project.json terraform plan - env: - GOOGLE_APPLICATION_CREDENTIALS: '${{ secrets.GCP_SA_KEY }}' continue-on-error: true - name: Terraform Apply run: terraform apply -auto-approve From 9e44a4c8c51e6e895780c19f166f8b91a30880ec Mon Sep 17 00:00:00 2001 From: milaGGL <107142260+milaGGL@users.noreply.github.com> Date: Mon, 18 Sep 2023 13:33:08 -0400 Subject: [PATCH 13/50] Update composite_index.tf --- composite_index.tf | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/composite_index.tf b/composite_index.tf index 358ee7916d7..7efdf6eb9b4 100644 --- a/composite_index.tf +++ b/composite_index.tf @@ -7,6 +7,18 @@ locals { region = "us-central1" } + resource "time_sleep" "wait_60_seconds" { + depends_on = [local.data] + create_duration = "60s" +} + +resource "google_project_service" "firestore" { + project = local.data.projectId + service = "firestore.googleapis.com" + # Needed for CI tests for permissions to propagate, should not be needed for actual usage + depends_on = [time_sleep.wait_60_seconds] +} + resource "google_firestore_index" "default-db-index" { collection = "composite-index-test-collection" From 3b192f09437c7665b0e96539c76b612e1b7b1ed4 Mon Sep 17 00:00:00 2001 From: milaGGL <107142260+milaGGL@users.noreply.github.com> Date: Mon, 18 Sep 2023 13:50:34 -0400 Subject: [PATCH 14/50] Update composite_index.tf --- composite_index.tf | 8 -------- 1 file changed, 8 deletions(-) diff --git a/composite_index.tf b/composite_index.tf index 7efdf6eb9b4..4e110772904 100644 --- a/composite_index.tf +++ b/composite_index.tf @@ -4,19 +4,11 @@ locals { provider "google" { project = local.data.projectId - region = "us-central1" } - resource "time_sleep" "wait_60_seconds" { - depends_on = [local.data] - create_duration = "60s" -} - resource "google_project_service" "firestore" { project = local.data.projectId service = "firestore.googleapis.com" - # Needed for CI tests for permissions to propagate, should not be needed for actual usage - depends_on = [time_sleep.wait_60_seconds] } resource "google_firestore_index" "default-db-index" { From 987800fc691018a218b007de67ffe20ca7bb41ea Mon Sep 17 00:00:00 2001 From: milaGGL <107142260+milaGGL@users.noreply.github.com> Date: Mon, 18 Sep 2023 14:11:41 -0400 Subject: [PATCH 15/50] try FIREBASE_CLI_TOKEN --- .github/workflows/test-changed-firestore-integration.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/test-changed-firestore-integration.yml b/.github/workflows/test-changed-firestore-integration.yml index ea13c554488..cd113d5e33f 100644 --- a/.github/workflows/test-changed-firestore-integration.yml +++ b/.github/workflows/test-changed-firestore-integration.yml @@ -15,7 +15,7 @@ jobs: fetch-depth: 0 - uses: 'google-github-actions/auth@v0' with: - credentials_json: '${{ secrets.GCP_SA_KEY }}' + credentials_json: '${{ secrets.FIREBASE_CLI_TOKEN }}' # create composite indexes with Terraform - name: Setup Terraform uses: hashicorp/setup-terraform@v2 From 27ad48369f00b12407400c01fe8aec4dfde1fb30 Mon Sep 17 00:00:00 2001 From: milaGGL <107142260+milaGGL@users.noreply.github.com> Date: Mon, 18 Sep 2023 14:12:30 -0400 Subject: [PATCH 16/50] Revert "try FIREBASE_CLI_TOKEN" This reverts commit 987800fc691018a218b007de67ffe20ca7bb41ea. --- .github/workflows/test-changed-firestore-integration.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/test-changed-firestore-integration.yml b/.github/workflows/test-changed-firestore-integration.yml index cd113d5e33f..ea13c554488 100644 --- a/.github/workflows/test-changed-firestore-integration.yml +++ b/.github/workflows/test-changed-firestore-integration.yml @@ -15,7 +15,7 @@ jobs: fetch-depth: 0 - uses: 'google-github-actions/auth@v0' with: - credentials_json: '${{ secrets.FIREBASE_CLI_TOKEN }}' + credentials_json: '${{ secrets.GCP_SA_KEY }}' # create composite indexes with Terraform - name: Setup Terraform uses: hashicorp/setup-terraform@v2 From f42e295ebd7aafe4beaa856408f4a00a2909672e Mon Sep 17 00:00:00 2001 From: milaGGL <107142260+milaGGL@users.noreply.github.com> Date: Mon, 18 Sep 2023 14:31:05 -0400 Subject: [PATCH 17/50] Update test-changed-firestore-integration.yml --- .github/workflows/test-changed-firestore-integration.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/test-changed-firestore-integration.yml b/.github/workflows/test-changed-firestore-integration.yml index ea13c554488..84528e28db0 100644 --- a/.github/workflows/test-changed-firestore-integration.yml +++ b/.github/workflows/test-changed-firestore-integration.yml @@ -25,6 +25,7 @@ jobs: - name: Terraform Plan if: github.event_name == 'pull_request' run: | + echo "${{secrets.GCP_SA_KEY}}" cp config/ci.config.json config/project.json terraform plan continue-on-error: true From a913d74cffa2bd274348b645ec5ca5eda782d84e Mon Sep 17 00:00:00 2001 From: milaGGL <107142260+milaGGL@users.noreply.github.com> Date: Mon, 18 Sep 2023 14:32:25 -0400 Subject: [PATCH 18/50] Revert "Update test-changed-firestore-integration.yml" This reverts commit f42e295ebd7aafe4beaa856408f4a00a2909672e. --- .github/workflows/test-changed-firestore-integration.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/.github/workflows/test-changed-firestore-integration.yml b/.github/workflows/test-changed-firestore-integration.yml index 84528e28db0..ea13c554488 100644 --- a/.github/workflows/test-changed-firestore-integration.yml +++ b/.github/workflows/test-changed-firestore-integration.yml @@ -25,7 +25,6 @@ jobs: - name: Terraform Plan if: github.event_name == 'pull_request' run: | - echo "${{secrets.GCP_SA_KEY}}" cp config/ci.config.json config/project.json terraform plan continue-on-error: true From 75a33da6627b1b1bf58d7aa9800b1a0afd096ff8 Mon Sep 17 00:00:00 2001 From: milaGGL <107142260+milaGGL@users.noreply.github.com> Date: Mon, 18 Sep 2023 14:50:46 -0400 Subject: [PATCH 19/50] remove services --- composite_index.tf | 5 ----- 1 file changed, 5 deletions(-) diff --git a/composite_index.tf b/composite_index.tf index 4e110772904..b6c3707039e 100644 --- a/composite_index.tf +++ b/composite_index.tf @@ -6,11 +6,6 @@ locals { project = local.data.projectId } -resource "google_project_service" "firestore" { - project = local.data.projectId - service = "firestore.googleapis.com" -} - resource "google_firestore_index" "default-db-index" { collection = "composite-index-test-collection" From 5e4464c1ec747900dc4bee5cccf47052b4e7c36a Mon Sep 17 00:00:00 2001 From: milaGGL <107142260+milaGGL@users.noreply.github.com> Date: Mon, 18 Sep 2023 15:08:47 -0400 Subject: [PATCH 20/50] Update test-changed-firestore-integration.yml --- .github/workflows/test-changed-firestore-integration.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/test-changed-firestore-integration.yml b/.github/workflows/test-changed-firestore-integration.yml index ea13c554488..8302c899b49 100644 --- a/.github/workflows/test-changed-firestore-integration.yml +++ b/.github/workflows/test-changed-firestore-integration.yml @@ -15,7 +15,7 @@ jobs: fetch-depth: 0 - uses: 'google-github-actions/auth@v0' with: - credentials_json: '${{ secrets.GCP_SA_KEY }}' + credentials_json: '${{ secrets.JSSDK_ACTIONS_SA_KEY }}' # create composite indexes with Terraform - name: Setup Terraform uses: hashicorp/setup-terraform@v2 From b6bdc65020bd1f6d41286b6df385d5e691fb10e8 Mon Sep 17 00:00:00 2001 From: milaGGL <107142260+milaGGL@users.noreply.github.com> Date: Mon, 18 Sep 2023 17:30:53 -0400 Subject: [PATCH 21/50] Update test-changed-firestore.yml --- .github/workflows/test-changed-firestore.yml | 36 ++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/.github/workflows/test-changed-firestore.yml b/.github/workflows/test-changed-firestore.yml index f8cc135380e..4006ca9aa28 100644 --- a/.github/workflows/test-changed-firestore.yml +++ b/.github/workflows/test-changed-firestore.yml @@ -98,6 +98,24 @@ jobs: needs: build if: ${{ needs.build.outputs.changed == 'true'}} steps: + - uses: 'google-github-actions/auth@v0' + with: + credentials_json: '${{ secrets.JSSDK_ACTIONS_SA_KEY }}' + # create composite indexes with Terraform + - name: Setup Terraform + uses: hashicorp/setup-terraform@v2 + - name: Terraform Init + run: terraform init + continue-on-error: true + - name: Terraform Plan + if: github.event_name == 'pull_request' + run: | + cp config/ci.config.json config/project.json + terraform plan + continue-on-error: true + - name: Terraform Apply + run: terraform apply -auto-approve + continue-on-error: true - name: Set up Node (16) uses: actions/setup-node@v3 with: @@ -163,6 +181,24 @@ jobs: needs: build if: ${{ needs.build.outputs.changed == 'true'}} steps: + - uses: 'google-github-actions/auth@v0' + with: + credentials_json: '${{ secrets.JSSDK_ACTIONS_SA_KEY }}' + # create composite indexes with Terraform + - name: Setup Terraform + uses: hashicorp/setup-terraform@v2 + - name: Terraform Init + run: terraform init + continue-on-error: true + - name: Terraform Plan + if: github.event_name == 'pull_request' + run: | + cp config/ci.config.json config/project.json + terraform plan + continue-on-error: true + - name: Terraform Apply + run: terraform apply -auto-approve + continue-on-error: true - name: install Firefox stable run: | sudo apt-get update From 7035d6c82c2ee167643c5e03b3648987842a884e Mon Sep 17 00:00:00 2001 From: milaGGL <107142260+milaGGL@users.noreply.github.com> Date: Mon, 18 Sep 2023 18:17:07 -0400 Subject: [PATCH 22/50] Update test-changed-firestore.yml --- .github/workflows/test-changed-firestore.yml | 63 ++++++++++---------- 1 file changed, 33 insertions(+), 30 deletions(-) diff --git a/.github/workflows/test-changed-firestore.yml b/.github/workflows/test-changed-firestore.yml index 4006ca9aa28..20c02a0378d 100644 --- a/.github/workflows/test-changed-firestore.yml +++ b/.github/workflows/test-changed-firestore.yml @@ -98,28 +98,29 @@ jobs: needs: build if: ${{ needs.build.outputs.changed == 'true'}} steps: - - uses: 'google-github-actions/auth@v0' - with: - credentials_json: '${{ secrets.JSSDK_ACTIONS_SA_KEY }}' - # create composite indexes with Terraform - - name: Setup Terraform - uses: hashicorp/setup-terraform@v2 - - name: Terraform Init - run: terraform init - continue-on-error: true - - name: Terraform Plan - if: github.event_name == 'pull_request' - run: | - cp config/ci.config.json config/project.json - terraform plan - continue-on-error: true - - name: Terraform Apply - run: terraform apply -auto-approve - continue-on-error: true - name: Set up Node (16) uses: actions/setup-node@v3 with: node-version: 16.x + - uses: 'google-github-actions/auth@v0' + with: + credentials_json: '${{ secrets.JSSDK_ACTIONS_SA_KEY }}' + # create composite indexes with Terraform + - name: Setup Terraform + uses: hashicorp/setup-terraform@v2 + - name: Terraform Init + run: terraform init + continue-on-error: true + - name: Terraform Plan + if: github.event_name == 'pull_request' + run: | + cp config/ci.config.json config/project.json + terraform plan + continue-on-error: true + - name: Terraform Apply + run: terraform apply -auto-approve + continue-on-error: true + - name: install Chrome stable run: | sudo apt-get update @@ -181,9 +182,20 @@ jobs: needs: build if: ${{ needs.build.outputs.changed == 'true'}} steps: + - name: install Firefox stable + run: | + sudo apt-get update + sudo apt-get install firefox + - name: Download build archive + uses: actions/download-artifact@v3 + with: + name: build.tar.gz + - name: Unzip build artifact + run: tar xf build.tar.gz + - uses: 'google-github-actions/auth@v0' - with: - credentials_json: '${{ secrets.JSSDK_ACTIONS_SA_KEY }}' + with: + credentials_json: '${{ secrets.JSSDK_ACTIONS_SA_KEY }}' # create composite indexes with Terraform - name: Setup Terraform uses: hashicorp/setup-terraform@v2 @@ -199,16 +211,7 @@ jobs: - name: Terraform Apply run: terraform apply -auto-approve continue-on-error: true - - name: install Firefox stable - run: | - sudo apt-get update - sudo apt-get install firefox - - name: Download build archive - uses: actions/download-artifact@v3 - with: - name: build.tar.gz - - name: Unzip build artifact - run: tar xf build.tar.gz + - name: Set up Node (16) uses: actions/setup-node@v3 with: From 80c95eb6bd23b3483367eaf2781e5e0a52bb839a Mon Sep 17 00:00:00 2001 From: milaGGL <107142260+milaGGL@users.noreply.github.com> Date: Mon, 18 Sep 2023 19:02:00 -0400 Subject: [PATCH 23/50] check project --- composite_index.tf | 46 ------------------- firestore_index_config.tf | 33 +++++++++++++ main.tf | 39 ++++++++++++++++ .../api/composite_index_query.test.ts | 1 + 4 files changed, 73 insertions(+), 46 deletions(-) delete mode 100644 composite_index.tf create mode 100644 firestore_index_config.tf create mode 100644 main.tf diff --git a/composite_index.tf b/composite_index.tf deleted file mode 100644 index b6c3707039e..00000000000 --- a/composite_index.tf +++ /dev/null @@ -1,46 +0,0 @@ -locals { - data = jsondecode(file("config/project.json")) -} - - provider "google" { - project = local.data.projectId - } - -resource "google_firestore_index" "default-db-index" { - collection = "composite-index-test-collection" - - fields { - field_path = "key" - order = "ASCENDING" - } - - fields { - field_path = "testId" - order = "DESCENDING" - } - - fields { - field_path = "sort" - order = "DESCENDING" - } -} - -resource "google_firestore_index" "named-db-index" { - collection = "composite-index-test-collection" - database = "test-db" - - fields { - field_path = "key" - order = "ASCENDING" - } - - fields { - field_path = "testId" - order = "DESCENDING" - } - - fields { - field_path = "sort" - order = "DESCENDING" - } -} \ No newline at end of file diff --git a/firestore_index_config.tf b/firestore_index_config.tf new file mode 100644 index 00000000000..d9e9e9da57b --- /dev/null +++ b/firestore_index_config.tf @@ -0,0 +1,33 @@ +locals { + project = jsondecode(file("config/project.json")) + indexes = { + index1 = [ + { + field_path = "key" + order = "ASCENDING" + }, + { + field_path = "testId" + order = "ASCENDING" + }, + { + field_path = "sort" + order = "ASCENDING" + }, + ], + index2 = [ + { + field_path = "key" + order = "ASCENDING" + }, + { + field_path = "testId" + order = "DESCENDING" + }, + { + field_path = "sort" + order = "DESCENDING" + }, + ], + } +} diff --git a/main.tf b/main.tf new file mode 100644 index 00000000000..051260bfcad --- /dev/null +++ b/main.tf @@ -0,0 +1,39 @@ +provider "google" { + project = local.project.projectId +} + +resource "google_firestore_index" "default-db-index" { + collection = "composite-index-test-collection" + + for_each = local.indexes + dynamic "fields" { + for_each = distinct(flatten([for k, v in local.indexes : [ + for i in each.value : { + field_path = i.field_path + order = i.order + }]])) + content { + field_path = lookup(fields.value, "field_path", null) + order = lookup(fields.value, "order", null) + } + } + +} + +resource "google_firestore_index" "named-db-index" { + collection = "composite-index-test-collection" + database = "test-db" + + for_each = local.indexes + dynamic "fields" { + for_each = distinct(flatten([for k, v in local.indexes : [ + for i in each.value : { + field_path = i.field_path + order = i.order + }]])) + content { + field_path = lookup(fields.value, "field_path", null) + order = lookup(fields.value, "order", null) + } + } +} diff --git a/packages/firestore/test/integration/api/composite_index_query.test.ts b/packages/firestore/test/integration/api/composite_index_query.test.ts index a86ba4aaa9b..ac9ce8c8238 100644 --- a/packages/firestore/test/integration/api/composite_index_query.test.ts +++ b/packages/firestore/test/integration/api/composite_index_query.test.ts @@ -32,6 +32,7 @@ apiDescribe('Queries', persistence => { }; const testHelper = new CompositeIndexTestHelper(); return testHelper.withTestDocs(persistence, testDocs, async coll => { + console.log("=========="+JSON.stringify(coll.firestore)) const filteredQuery = testHelper.query( coll, where('key', '==', 'a'), From c32b3d98b47ec0fd188f4a0874fb13631a1498ec Mon Sep 17 00:00:00 2001 From: milaGGL <107142260+milaGGL@users.noreply.github.com> Date: Mon, 18 Sep 2023 19:28:39 -0400 Subject: [PATCH 24/50] remove unnecessary terraform run --- .github/workflows/test-changed-firestore.yml | 39 ------------------- .../api/composite_index_query.test.ts | 1 - 2 files changed, 40 deletions(-) diff --git a/.github/workflows/test-changed-firestore.yml b/.github/workflows/test-changed-firestore.yml index 20c02a0378d..f8cc135380e 100644 --- a/.github/workflows/test-changed-firestore.yml +++ b/.github/workflows/test-changed-firestore.yml @@ -102,25 +102,6 @@ jobs: uses: actions/setup-node@v3 with: node-version: 16.x - - uses: 'google-github-actions/auth@v0' - with: - credentials_json: '${{ secrets.JSSDK_ACTIONS_SA_KEY }}' - # create composite indexes with Terraform - - name: Setup Terraform - uses: hashicorp/setup-terraform@v2 - - name: Terraform Init - run: terraform init - continue-on-error: true - - name: Terraform Plan - if: github.event_name == 'pull_request' - run: | - cp config/ci.config.json config/project.json - terraform plan - continue-on-error: true - - name: Terraform Apply - run: terraform apply -auto-approve - continue-on-error: true - - name: install Chrome stable run: | sudo apt-get update @@ -192,26 +173,6 @@ jobs: name: build.tar.gz - name: Unzip build artifact run: tar xf build.tar.gz - - - uses: 'google-github-actions/auth@v0' - with: - credentials_json: '${{ secrets.JSSDK_ACTIONS_SA_KEY }}' - # create composite indexes with Terraform - - name: Setup Terraform - uses: hashicorp/setup-terraform@v2 - - name: Terraform Init - run: terraform init - continue-on-error: true - - name: Terraform Plan - if: github.event_name == 'pull_request' - run: | - cp config/ci.config.json config/project.json - terraform plan - continue-on-error: true - - name: Terraform Apply - run: terraform apply -auto-approve - continue-on-error: true - - name: Set up Node (16) uses: actions/setup-node@v3 with: diff --git a/packages/firestore/test/integration/api/composite_index_query.test.ts b/packages/firestore/test/integration/api/composite_index_query.test.ts index ac9ce8c8238..a86ba4aaa9b 100644 --- a/packages/firestore/test/integration/api/composite_index_query.test.ts +++ b/packages/firestore/test/integration/api/composite_index_query.test.ts @@ -32,7 +32,6 @@ apiDescribe('Queries', persistence => { }; const testHelper = new CompositeIndexTestHelper(); return testHelper.withTestDocs(persistence, testDocs, async coll => { - console.log("=========="+JSON.stringify(coll.firestore)) const filteredQuery = testHelper.query( coll, where('key', '==', 'a'), From aed8c2f7ddd7f17686bdecc75d97b560c0bcc3f1 Mon Sep 17 00:00:00 2001 From: milaGGL <107142260+milaGGL@users.noreply.github.com> Date: Mon, 18 Sep 2023 23:27:39 -0400 Subject: [PATCH 25/50] run all integration tests --- .../test/integration/api/composite_index_query.test.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/firestore/test/integration/api/composite_index_query.test.ts b/packages/firestore/test/integration/api/composite_index_query.test.ts index a86ba4aaa9b..77949cc3d7e 100644 --- a/packages/firestore/test/integration/api/composite_index_query.test.ts +++ b/packages/firestore/test/integration/api/composite_index_query.test.ts @@ -22,8 +22,7 @@ import { where, getDocs, orderBy, getDoc, doc } from '../util/firebase_export'; import { apiDescribe, toDataArray, toIds } from '../util/helpers'; apiDescribe('Queries', persistence => { - // eslint-disable-next-line no-restricted-properties - describe.only('query with OrderBy fields', () => { + describe('query with OrderBy fields', () => { it('can run composite index query', () => { const testDocs = { doc1: { key: 'a', sort: 3 }, From 4531fdb7cefd0abca724ae052f215abd3b947989 Mon Sep 17 00:00:00 2001 From: milaGGL <107142260+milaGGL@users.noreply.github.com> Date: Tue, 19 Sep 2023 11:16:40 -0400 Subject: [PATCH 26/50] skip MIEQ --- packages/firestore/test/integration/api/query.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/firestore/test/integration/api/query.test.ts b/packages/firestore/test/integration/api/query.test.ts index 3a0e7c1c2e8..03d46f70d23 100644 --- a/packages/firestore/test/integration/api/query.test.ts +++ b/packages/firestore/test/integration/api/query.test.ts @@ -1347,7 +1347,7 @@ apiDescribe('Queries', persistence => { }); // eslint-disable-next-line no-restricted-properties - (USE_EMULATOR ? describe : describe.skip)('Multiple Inequality', () => { + describe.skip('Multiple Inequality', () => { it('can use multiple inequality filters', async () => { const testDocs = { doc1: { key: 'a', sort: 0, v: 0 }, From a78eebe480f1a721274d3e3a02815688f6b7a037 Mon Sep 17 00:00:00 2001 From: milaGGL <107142260+milaGGL@users.noreply.github.com> Date: Tue, 19 Sep 2023 13:15:11 -0400 Subject: [PATCH 27/50] Update query.test.ts --- packages/firestore/test/integration/api/query.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/firestore/test/integration/api/query.test.ts b/packages/firestore/test/integration/api/query.test.ts index 3a0e7c1c2e8..28a2a61979c 100644 --- a/packages/firestore/test/integration/api/query.test.ts +++ b/packages/firestore/test/integration/api/query.test.ts @@ -1347,7 +1347,7 @@ apiDescribe('Queries', persistence => { }); // eslint-disable-next-line no-restricted-properties - (USE_EMULATOR ? describe : describe.skip)('Multiple Inequality', () => { + (USE_EMULATOR ? describe.only : describe.skip)('Multiple Inequality', () => { it('can use multiple inequality filters', async () => { const testDocs = { doc1: { key: 'a', sort: 0, v: 0 }, From 37e37706ae61c3dbc3982d5f11c0c44cc1a02230 Mon Sep 17 00:00:00 2001 From: milaGGL <107142260+milaGGL@users.noreply.github.com> Date: Tue, 19 Sep 2023 14:51:06 -0400 Subject: [PATCH 28/50] Update test-changed-firestore.yml --- .github/workflows/test-changed-firestore.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.github/workflows/test-changed-firestore.yml b/.github/workflows/test-changed-firestore.yml index f8cc135380e..705eadca5b8 100644 --- a/.github/workflows/test-changed-firestore.yml +++ b/.github/workflows/test-changed-firestore.yml @@ -118,6 +118,8 @@ jobs: run: cp config/ci.config.json config/project.json - name: Run tests run: cd packages/firestore && yarn run ${{ matrix.test-name }} + env: + EXPERIMENTAL_MODE: true compat-test-firefox: name: Test Firestore Compatible on Firefox @@ -185,6 +187,7 @@ jobs: run: cd packages/firestore && xvfb-run yarn run ${{ matrix.test-name }} env: BROWSERS: 'Firefox' + EXPERIMENTAL_MODE: true # A job that fails if any required job in the test matrix fails, # to be used as a required check for merging. From f37eeaf28dafd49126ca5999e187f7344ca7fd0b Mon Sep 17 00:00:00 2001 From: milaGGL <107142260+milaGGL@users.noreply.github.com> Date: Tue, 19 Sep 2023 15:26:05 -0400 Subject: [PATCH 29/50] try run export EXPERIMENTAL_MODE=true --- .github/workflows/test-changed-firestore.yml | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/.github/workflows/test-changed-firestore.yml b/.github/workflows/test-changed-firestore.yml index 705eadca5b8..b0e59e8e5ca 100644 --- a/.github/workflows/test-changed-firestore.yml +++ b/.github/workflows/test-changed-firestore.yml @@ -115,11 +115,11 @@ jobs: - name: Bump Node memory limit run: echo "NODE_OPTIONS=--max_old_space_size=4096" >> $GITHUB_ENV - name: Test setup and yarn install - run: cp config/ci.config.json config/project.json + run: | + export EXPERIMENTAL_MODE=true + cp config/ci.config.json config/project.json - name: Run tests run: cd packages/firestore && yarn run ${{ matrix.test-name }} - env: - EXPERIMENTAL_MODE: true compat-test-firefox: name: Test Firestore Compatible on Firefox @@ -182,12 +182,13 @@ jobs: - name: Bump Node memory limit run: echo "NODE_OPTIONS=--max_old_space_size=4096" >> $GITHUB_ENV - name: Test setup and yarn install - run: cp config/ci.config.json config/project.json + run: | + export EXPERIMENTAL_MODE=true + cp config/ci.config.json config/project.json - name: Run tests run: cd packages/firestore && xvfb-run yarn run ${{ matrix.test-name }} env: BROWSERS: 'Firefox' - EXPERIMENTAL_MODE: true # A job that fails if any required job in the test matrix fails, # to be used as a required check for merging. From 75ba83666d75669316635f3f3675c51f71ac0d79 Mon Sep 17 00:00:00 2001 From: milaGGL <107142260+milaGGL@users.noreply.github.com> Date: Tue, 19 Sep 2023 15:34:58 -0400 Subject: [PATCH 30/50] add or query tests to composite index tests --- .../test-changed-firestore-integration.yml | 8 +- firestore_index_config.tf | 95 +++++++- .../api/composite_index_query.test.ts | 225 +++++++++++++++++- .../test/integration/api/query.test.ts | 28 +-- .../util/composite_index_test_helper.ts | 23 +- .../test/integration/util/helpers.ts | 28 ++- 6 files changed, 363 insertions(+), 44 deletions(-) diff --git a/.github/workflows/test-changed-firestore-integration.yml b/.github/workflows/test-changed-firestore-integration.yml index 8302c899b49..1b13773c02d 100644 --- a/.github/workflows/test-changed-firestore-integration.yml +++ b/.github/workflows/test-changed-firestore-integration.yml @@ -22,16 +22,12 @@ jobs: - name: Terraform Init run: terraform init continue-on-error: true - - name: Terraform Plan + - name: Terraform Apply if: github.event_name == 'pull_request' run: | cp config/ci.config.json config/project.json - terraform plan - continue-on-error: true - - name: Terraform Apply - run: terraform apply -auto-approve + terraform apply -auto-approve continue-on-error: true - - name: Set up Node (16) uses: actions/setup-node@v3 with: diff --git a/firestore_index_config.tf b/firestore_index_config.tf index d9e9e9da57b..6e3e9ea2fa6 100644 --- a/firestore_index_config.tf +++ b/firestore_index_config.tf @@ -14,20 +14,107 @@ locals { field_path = "sort" order = "ASCENDING" }, - ], + ] index2 = [ { - field_path = "key" + field_path = "b" + order = "ASCENDING" + }, + { + field_path = "testId" + order = "ASCENDING" + }, + { + field_path = "a" + order = "ASCENDING" + }, + ] + index9 = [ + { + field_path = "b" order = "ASCENDING" }, { field_path = "testId" + order = "ASCENDING" + }, + { + field_path = "a" order = "DESCENDING" }, + ] + index3 = [ { - field_path = "sort" + field_path = "a" + order = "ASCENDING" + }, + { + field_path = "testId" + order = "ASCENDING" + }, + { + field_path = "b" + order = "DESCENDING" + }, + ] + index4 = [ + { + field_path = "a" + order = "ASCENDING" + }, + { + field_path = "testId" + order = "ASCENDING" + }, + { + field_path = "b" + order = "ASCENDING" + }, + ] + index5 = [ + { + field_path = "a" + order = "ASCENDING" + }, + { + field_path = "testId" + order = "ASCENDING" + }, + { + field_path = "a" + order = "DESCENDING" + }, + ] + index6 = [ + { + field_path = "testId" + order = "ASCENDING" + }, + { + field_path = "a" + order = "ASCENDING" + }, + ] + index7 = [ + { + field_path = "testId" + order = "ASCENDING" + }, + { + field_path = "b" + order = "ASCENDING" + }, + ] + index8 = [ + { + field_path = "testId" + order = "ASCENDING" + }, + { + field_path = "b" order = "DESCENDING" }, - ], + ] + } } diff --git a/packages/firestore/test/integration/api/composite_index_query.test.ts b/packages/firestore/test/integration/api/composite_index_query.test.ts index 77949cc3d7e..9cecdab5ebe 100644 --- a/packages/firestore/test/integration/api/composite_index_query.test.ts +++ b/packages/firestore/test/integration/api/composite_index_query.test.ts @@ -18,11 +18,27 @@ import { expect } from 'chai'; import { CompositeIndexTestHelper } from '../util/composite_index_test_helper'; -import { where, getDocs, orderBy, getDoc, doc } from '../util/firebase_export'; -import { apiDescribe, toDataArray, toIds } from '../util/helpers'; +import { + where, + getDocs, + orderBy, + getDoc, + doc, + and, + limit, + limitToLast, + or +} from '../util/firebase_export'; +import { + apiDescribe, + checkOnlineAndOfflineResultsMatch, + toDataArray, + toIds +} from '../util/helpers'; apiDescribe('Queries', persistence => { - describe('query with OrderBy fields', () => { + // TODO(Mila): Remove these tests once helper is validated + describe('testing composite index helper', () => { it('can run composite index query', () => { const testDocs = { doc1: { key: 'a', sort: 3 }, @@ -81,4 +97,207 @@ apiDescribe('Queries', persistence => { }); }); }); + + // OR Query tests only run when the SDK's local cache is configured to use + // LRU garbage collection (rather than eager garbage collection) because + // they validate that the result from server and cache match. Additionally, + // these tests must be skipped if running against production because it + // results in a 'missing index' error. The Firestore Emulator, however, does + // serve these queries. + // eslint-disable-next-line no-restricted-properties + (persistence.gc === 'lru' ? describe.only : describe.skip)( + 'OR Queries', + () => { + it('can use query overloads', () => { + const testDocs = { + doc1: { a: 1, b: 0 }, + doc2: { a: 2, b: 1 }, + doc3: { a: 3, b: 2 }, + doc4: { a: 1, b: 3 }, + doc5: { a: 1, b: 1 } + }; + const testHelper = new CompositeIndexTestHelper(); + return testHelper.withTestDocs(persistence, testDocs, async coll => { + // a == 1, limit 2, b - desc + await checkOnlineAndOfflineResultsMatch( + testHelper.query( + coll, + where('a', '==', 1), + limit(2), + orderBy('b', 'desc') + ), + 'doc4', + 'doc5' + ); + }); + }); + + it('can use or queries', () => { + const testDocs = { + doc1: { a: 1, b: 0 }, + doc2: { a: 2, b: 1 }, + doc3: { a: 3, b: 2 }, + doc4: { a: 1, b: 3 }, + doc5: { a: 1, b: 1 } + }; + const testHelper = new CompositeIndexTestHelper(); + return testHelper.withTestDocs(persistence, testDocs, async coll => { + // with one inequality: a>2 || b==1. + await checkOnlineAndOfflineResultsMatch( + testHelper.compositeQuery( + coll, + or(where('a', '>', 2), where('b', '==', 1)) + ), + 'doc5', + 'doc2', + 'doc3' + ); + + // Test with limits (implicit order by ASC): (a==1) || (b > 0) LIMIT 2 + await checkOnlineAndOfflineResultsMatch( + testHelper.compositeQuery( + coll, + or(where('a', '==', 1), where('b', '>', 0)), + limit(2) + ), + 'doc1', + 'doc2' + ); + + // Test with limits (explicit order by): (a==1) || (b > 0) LIMIT_TO_LAST 2 + // Note: The public query API does not allow implicit ordering when limitToLast is used. + await checkOnlineAndOfflineResultsMatch( + testHelper.compositeQuery( + coll, + or(where('a', '==', 1), where('b', '>', 0)), + limitToLast(2), + orderBy('b') + ), + 'doc3', + 'doc4' + ); + + // Test with limits (explicit order by ASC): (a==2) || (b == 1) ORDER BY a LIMIT 1 + await checkOnlineAndOfflineResultsMatch( + testHelper.compositeQuery( + coll, + or(where('a', '==', 2), where('b', '==', 1)), + limit(1), + orderBy('a') + ), + 'doc5' + ); + + // Test with limits (explicit order by DESC): (a==2) || (b == 1) ORDER BY a LIMIT_TO_LAST 1 + await checkOnlineAndOfflineResultsMatch( + testHelper.compositeQuery( + coll, + or(where('a', '==', 2), where('b', '==', 1)), + limitToLast(1), + orderBy('a') + ), + 'doc2' + ); + }); + }); + + // eslint-disable-next-line no-restricted-properties + it('supports multiple in ops', () => { + const testDocs = { + doc1: { a: 1, b: 0 }, + doc2: { b: 1 }, + doc3: { a: 3, b: 2 }, + doc4: { a: 1, b: 3 }, + doc5: { a: 1 }, + doc6: { a: 2 } + }; + const testHelper = new CompositeIndexTestHelper(); + return testHelper.withTestDocs(persistence, testDocs, async coll => { + // Two IN operations on different fields with disjunction. + await checkOnlineAndOfflineResultsMatch( + testHelper.compositeQuery( + coll, + or(where('a', 'in', [2, 3]), where('b', 'in', [0, 2])), + orderBy('a') + ), + 'doc1', + 'doc6', + 'doc3' + ); + + // Two IN operations on different fields with conjunction. + await checkOnlineAndOfflineResultsMatch( + testHelper.compositeQuery( + coll, + and(where('a', 'in', [2, 3]), where('b', 'in', [0, 2])), + orderBy('a') + ), + 'doc3' + ); + + // Two IN operations on the same field. + // a IN [1,2,3] && a IN [0,1,4] should result in "a==1". + await checkOnlineAndOfflineResultsMatch( + testHelper.compositeQuery( + coll, + and(where('a', 'in', [1, 2, 3]), where('a', 'in', [0, 1, 4])) + ), + 'doc1', + 'doc4', + 'doc5' + ); + + // a IN [2,3] && a IN [0,1,4] is never true and so the result should be an + // empty set. + await checkOnlineAndOfflineResultsMatch( + testHelper.compositeQuery( + coll, + and(where('a', 'in', [2, 3]), where('a', 'in', [0, 1, 4])) + ) + ); + + // a IN [0,3] || a IN [0,2] should union them (similar to: a IN [0,2,3]). + await checkOnlineAndOfflineResultsMatch( + testHelper.compositeQuery( + coll, + or(where('a', 'in', [0, 3]), where('a', 'in', [0, 2])) + ), + 'doc3', + 'doc6' + ); + + // Nested composite filter on the same field. + await checkOnlineAndOfflineResultsMatch( + testHelper.compositeQuery( + coll, + and( + where('a', 'in', [1, 3]), + or( + where('a', 'in', [0, 2]), + and(where('b', '>=', 1), where('a', 'in', [1, 3])) + ) + ) + ), + 'doc3', + 'doc4' + ); + + // Nested composite filter on the different fields. + await checkOnlineAndOfflineResultsMatch( + testHelper.compositeQuery( + coll, + and( + where('b', 'in', [0, 3]), + or( + where('b', 'in', [1]), + and(where('b', 'in', [2, 3]), where('a', 'in', [1, 3])) + ) + ) + ), + 'doc4' + ); + }); + }); + } + ); }); diff --git a/packages/firestore/test/integration/api/query.test.ts b/packages/firestore/test/integration/api/query.test.ts index 03d46f70d23..416f9f5121b 100644 --- a/packages/firestore/test/integration/api/query.test.ts +++ b/packages/firestore/test/integration/api/query.test.ts @@ -43,14 +43,11 @@ import { getAggregateFromServer, getCountFromServer, getDocs, - getDocsFromCache, - getDocsFromServer, limit, limitToLast, onSnapshot, or, orderBy, - Query, query, QuerySnapshot, setDoc, @@ -72,7 +69,8 @@ import { withEmptyTestCollection, withRetry, withTestCollection, - withTestDb + withTestDb, + checkOnlineAndOfflineResultsMatch } from '../util/helpers'; import { USE_EMULATOR } from '../util/settings'; import { captureExistenceFilterMismatches } from '../util/testing_hooks_util'; @@ -3094,25 +3092,3 @@ function verifyDocumentChange( expect(change.oldIndex).to.equal(oldIndex); expect(change.newIndex).to.equal(newIndex); } - -/** - * Checks that running the query while online (against the backend/emulator) results in the same - * documents as running the query while offline. If `expectedDocs` is provided, it also checks - * that both online and offline query result is equal to the expected documents. - * - * @param query The query to check - * @param expectedDocs Ordered list of document keys that are expected to match the query - */ -async function checkOnlineAndOfflineResultsMatch( - query: Query, - ...expectedDocs: string[] -): Promise { - const docsFromServer = await getDocsFromServer(query); - - if (expectedDocs.length !== 0) { - expect(expectedDocs).to.deep.equal(toIds(docsFromServer)); - } - - const docsFromCache = await getDocsFromCache(query); - expect(toIds(docsFromServer)).to.deep.equal(toIds(docsFromCache)); -} diff --git a/packages/firestore/test/integration/util/composite_index_test_helper.ts b/packages/firestore/test/integration/util/composite_index_test_helper.ts index c66c41454b8..52f1e551729 100644 --- a/packages/firestore/test/integration/util/composite_index_test_helper.ts +++ b/packages/firestore/test/integration/util/composite_index_test_helper.ts @@ -15,10 +15,11 @@ * limitations under the License. */ +import { and } from '../../../src/lite-api/query'; import { AutoId } from '../../../src/util/misc'; import { - query, + query as _query, CollectionReference, DocumentData, Firestore, @@ -28,7 +29,9 @@ import { WithFieldValue, DocumentReference, addDoc as addDocument, - setDoc as setDocument + setDoc as setDocument, + QueryCompositeFilterConstraint, + QueryNonFilterConstraint } from './firebase_export'; import { batchCommitDocsToCollection, @@ -85,18 +88,30 @@ export class CompositeIndexTestHelper { doc[this.TEST_ID_FIELD] = this.testId; } - // add filter on test id + //add filter on test id query( query_: Query, ...queryConstraints: QueryConstraint[] ): Query { - return query( + return _query( query_, where(this.TEST_ID_FIELD, '==', this.testId), ...queryConstraints ); } + compositeQuery( + query_: Query, + compositeFilter: QueryCompositeFilterConstraint, + ...queryConstraints: QueryNonFilterConstraint[] + ): Query { + return _query( + query_, + and(where(this.TEST_ID_FIELD, '==', this.testId), compositeFilter), + ...queryConstraints + ); + } + // add doc with test id addDoc( reference: CollectionReference, diff --git a/packages/firestore/test/integration/util/helpers.ts b/packages/firestore/test/integration/util/helpers.ts index 7976b3f8da3..6c46eb51a4c 100644 --- a/packages/firestore/test/integration/util/helpers.ts +++ b/packages/firestore/test/integration/util/helpers.ts @@ -16,6 +16,7 @@ */ import { isIndexedDBAvailable } from '@firebase/util'; +import { expect } from 'chai'; import { AutoId } from '../../../src/util/misc'; @@ -41,7 +42,10 @@ import { SnapshotListenOptions, terminate, WriteBatch, - writeBatch + writeBatch, + Query, + getDocsFromServer, + getDocsFromCache } from './firebase_export'; import { ALT_PROJECT_ID, @@ -534,3 +538,25 @@ export function partitionedTestDocs(partitions: { return testDocs; } + +/** + * Checks that running the query while online (against the backend/emulator) results in the same + * documents as running the query while offline. If `expectedDocs` is provided, it also checks + * that both online and offline query result is equal to the expected documents. + * + * @param query The query to check + * @param expectedDocs Ordered list of document keys that are expected to match the query + */ +export async function checkOnlineAndOfflineResultsMatch( + query: Query, + ...expectedDocs: string[] +): Promise { + const docsFromServer = await getDocsFromServer(query); + + if (expectedDocs.length !== 0) { + expect(expectedDocs).to.deep.equal(toIds(docsFromServer)); + } + + const docsFromCache = await getDocsFromCache(query); + expect(toIds(docsFromServer)).to.deep.equal(toIds(docsFromCache)); +} From ff6b0a972bf61a8f001661f5fd22d467a6452516 Mon Sep 17 00:00:00 2001 From: milaGGL <107142260+milaGGL@users.noreply.github.com> Date: Tue, 19 Sep 2023 16:11:23 -0400 Subject: [PATCH 31/50] Revert "try run export EXPERIMENTAL_MODE=true" This reverts commit f37eeaf28dafd49126ca5999e187f7344ca7fd0b. --- .github/workflows/test-changed-firestore.yml | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/.github/workflows/test-changed-firestore.yml b/.github/workflows/test-changed-firestore.yml index b0e59e8e5ca..705eadca5b8 100644 --- a/.github/workflows/test-changed-firestore.yml +++ b/.github/workflows/test-changed-firestore.yml @@ -115,11 +115,11 @@ jobs: - name: Bump Node memory limit run: echo "NODE_OPTIONS=--max_old_space_size=4096" >> $GITHUB_ENV - name: Test setup and yarn install - run: | - export EXPERIMENTAL_MODE=true - cp config/ci.config.json config/project.json + run: cp config/ci.config.json config/project.json - name: Run tests run: cd packages/firestore && yarn run ${{ matrix.test-name }} + env: + EXPERIMENTAL_MODE: true compat-test-firefox: name: Test Firestore Compatible on Firefox @@ -182,13 +182,12 @@ jobs: - name: Bump Node memory limit run: echo "NODE_OPTIONS=--max_old_space_size=4096" >> $GITHUB_ENV - name: Test setup and yarn install - run: | - export EXPERIMENTAL_MODE=true - cp config/ci.config.json config/project.json + run: cp config/ci.config.json config/project.json - name: Run tests run: cd packages/firestore && xvfb-run yarn run ${{ matrix.test-name }} env: BROWSERS: 'Firefox' + EXPERIMENTAL_MODE: true # A job that fails if any required job in the test matrix fails, # to be used as a required check for merging. From d0affc9ffc40d11e1d373ac3b5e7f31686d29416 Mon Sep 17 00:00:00 2001 From: milaGGL <107142260+milaGGL@users.noreply.github.com> Date: Tue, 19 Sep 2023 16:39:12 -0400 Subject: [PATCH 32/50] upgrade cloud firestore emulator version --- scripts/emulator-testing/emulators/firestore-emulator.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/scripts/emulator-testing/emulators/firestore-emulator.ts b/scripts/emulator-testing/emulators/firestore-emulator.ts index 8e30d2cfe99..53dcb713d55 100644 --- a/scripts/emulator-testing/emulators/firestore-emulator.ts +++ b/scripts/emulator-testing/emulators/firestore-emulator.ts @@ -22,11 +22,11 @@ export class FirestoreEmulator extends Emulator { constructor(port: number, projectId = 'test-emulator') { super( - 'cloud-firestore-emulator-v1.14.4.jar', + 'cloud-firestore-emulator-v1.18.1.jar', // Use locked version of emulator for test to be deterministic. // The latest version can be found from firestore emulator doc: // https://firebase.google.com/docs/firestore/security/test-rules-emulator - 'https://storage.googleapis.com/firebase-preview-drop/emulator/cloud-firestore-emulator-v1.14.4.jar', + 'https://storage.googleapis.com/firebase-preview-drop/emulator/cloud-firestore-emulator-v1.18.1.jar', port ); this.projectId = projectId; From 035b560002732dc181e9cb08b6e1bd633a0d89ae Mon Sep 17 00:00:00 2001 From: milaGGL <107142260+milaGGL@users.noreply.github.com> Date: Tue, 19 Sep 2023 17:17:00 -0400 Subject: [PATCH 33/50] Update query.test.ts --- packages/firestore/test/integration/api/query.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/firestore/test/integration/api/query.test.ts b/packages/firestore/test/integration/api/query.test.ts index 28a2a61979c..3a0e7c1c2e8 100644 --- a/packages/firestore/test/integration/api/query.test.ts +++ b/packages/firestore/test/integration/api/query.test.ts @@ -1347,7 +1347,7 @@ apiDescribe('Queries', persistence => { }); // eslint-disable-next-line no-restricted-properties - (USE_EMULATOR ? describe.only : describe.skip)('Multiple Inequality', () => { + (USE_EMULATOR ? describe : describe.skip)('Multiple Inequality', () => { it('can use multiple inequality filters', async () => { const testDocs = { doc1: { key: 'a', sort: 0, v: 0 }, From c7fd47855da2a7b84c2541dd58fbeeaf6301da8b Mon Sep 17 00:00:00 2001 From: milaGGL <107142260+milaGGL@users.noreply.github.com> Date: Tue, 19 Sep 2023 17:38:40 -0400 Subject: [PATCH 34/50] format --- .gitignore | 8 +- firestore_index_config.tf | 43 +- .../api/composite_index_query.test.ts | 410 +++++++----------- .../test/integration/api/query.test.ts | 2 +- 4 files changed, 190 insertions(+), 273 deletions(-) diff --git a/.gitignore b/.gitignore index 89ef5a65fa6..6fa0085c80d 100644 --- a/.gitignore +++ b/.gitignore @@ -91,4 +91,10 @@ tsdoc-metadata.json # generated html docs docs-rut/ docs/ -toc/ \ No newline at end of file +toc/ + +# generated Terraform docs +.terraform/* +.terraform.lock.hcl +*.tfstate +*.tfstate.* \ No newline at end of file diff --git a/firestore_index_config.tf b/firestore_index_config.tf index 6e3e9ea2fa6..7949e965c0a 100644 --- a/firestore_index_config.tf +++ b/firestore_index_config.tf @@ -2,20 +2,6 @@ locals { project = jsondecode(file("config/project.json")) indexes = { index1 = [ - { - field_path = "key" - order = "ASCENDING" - }, - { - field_path = "testId" - order = "ASCENDING" - }, - { - field_path = "sort" - order = "ASCENDING" - }, - ] - index2 = [ { field_path = "b" order = "ASCENDING" @@ -29,9 +15,9 @@ locals { order = "ASCENDING" }, ] - index9 = [ + index2 = [ { - field_path = "b" + field_path = "a" order = "ASCENDING" }, { @@ -39,7 +25,7 @@ locals { order = "ASCENDING" }, { - field_path = "a" + field_path = "b" order = "DESCENDING" }, ] @@ -54,7 +40,7 @@ locals { }, { field_path = "b" - order = "DESCENDING" + order = "ASCENDING" }, ] index4 = [ @@ -67,22 +53,18 @@ locals { order = "ASCENDING" }, { - field_path = "b" - order = "ASCENDING" + field_path = "a" + order = "DESCENDING" }, ] index5 = [ - { - field_path = "a" - order = "ASCENDING" - }, { field_path = "testId" order = "ASCENDING" }, { field_path = "a" - order = "DESCENDING" + order = "ASCENDING" }, ] index6 = [ @@ -91,7 +73,7 @@ locals { order = "ASCENDING" }, { - field_path = "a" + field_path = "b" order = "ASCENDING" }, ] @@ -102,19 +84,22 @@ locals { }, { field_path = "b" - order = "ASCENDING" + order = "DESCENDING" }, ] index8 = [ + { + field_path = "b" + order = "ASCENDING" + }, { field_path = "testId" order = "ASCENDING" }, { - field_path = "b" + field_path = "a" order = "DESCENDING" }, ] - } } diff --git a/packages/firestore/test/integration/api/composite_index_query.test.ts b/packages/firestore/test/integration/api/composite_index_query.test.ts index 9cecdab5ebe..bf0e58d57b7 100644 --- a/packages/firestore/test/integration/api/composite_index_query.test.ts +++ b/packages/firestore/test/integration/api/composite_index_query.test.ts @@ -15,15 +15,10 @@ * limitations under the License. */ -import { expect } from 'chai'; - import { CompositeIndexTestHelper } from '../util/composite_index_test_helper'; import { where, - getDocs, orderBy, - getDoc, - doc, and, limit, limitToLast, @@ -31,273 +26,204 @@ import { } from '../util/firebase_export'; import { apiDescribe, - checkOnlineAndOfflineResultsMatch, - toDataArray, - toIds + checkOnlineAndOfflineResultsMatch } from '../util/helpers'; apiDescribe('Queries', persistence => { - // TODO(Mila): Remove these tests once helper is validated - describe('testing composite index helper', () => { - it('can run composite index query', () => { + // OR Query tests only run when the SDK's local cache is configured to use + // LRU garbage collection (rather than eager garbage collection) because + // they validate that the result from server and cache match. + // eslint-disable-next-line no-restricted-properties + (persistence.gc === 'lru' ? describe : describe.skip)('OR Queries', () => { + it('can use query overloads', () => { const testDocs = { - doc1: { key: 'a', sort: 3 }, - doc2: { key: 'a', sort: 2 }, - doc3: { key: 'b', sort: 1 } + doc1: { a: 1, b: 0 }, + doc2: { a: 2, b: 1 }, + doc3: { a: 3, b: 2 }, + doc4: { a: 1, b: 3 }, + doc5: { a: 1, b: 1 } }; const testHelper = new CompositeIndexTestHelper(); return testHelper.withTestDocs(persistence, testDocs, async coll => { - const filteredQuery = testHelper.query( - coll, - where('key', '==', 'a'), - orderBy('sort') + // a == 1, limit 2, b - desc + await checkOnlineAndOfflineResultsMatch( + testHelper.query( + coll, + where('a', '==', 1), + limit(2), + orderBy('b', 'desc') + ), + 'doc4', + 'doc5' ); - const result = await getDocs(filteredQuery); - expect(toIds(result)).to.deep.equal(['doc2', 'doc1']); }); }); - it('data isolation is working', () => { + it('can use or queries', () => { const testDocs = { - doc1: { key: 'a', sort: 1 }, - doc2: { key: 'a', sort: 2 } + doc1: { a: 1, b: 0 }, + doc2: { a: 2, b: 1 }, + doc3: { a: 3, b: 2 }, + doc4: { a: 1, b: 3 }, + doc5: { a: 1, b: 1 } }; const testHelper = new CompositeIndexTestHelper(); return testHelper.withTestDocs(persistence, testDocs, async coll => { - const filteredQuery = testHelper.query( - coll, - where('key', '==', 'a'), - orderBy('sort') + // with one inequality: a>2 || b==1. + await checkOnlineAndOfflineResultsMatch( + testHelper.compositeQuery( + coll, + or(where('a', '>', 2), where('b', '==', 1)) + ), + 'doc5', + 'doc2', + 'doc3' ); - const result = await getDocs(filteredQuery); - expect(toIds(result)).to.deep.equal(['doc1', 'doc2']); - }); - }); - - it('can do addDoc and setDoc', () => { - const testHelper = new CompositeIndexTestHelper(); - return testHelper.withEmptyCollection(persistence, async coll => { - const docRef1 = await testHelper.addDoc(coll, { key: 'a', sort: 1 }); - const docRef2 = doc(coll, 'setDoc'); - await testHelper.setDoc(docRef2, { key: 'a', sort: 2 }); - const docSnap1 = await getDoc(docRef1); - const docSnap2 = await getDoc(docRef2); - - const filteredQuery = testHelper.query( - coll, - where('key', '==', 'a'), - orderBy('sort') + // Test with limits (implicit order by ASC): (a==1) || (b > 0) LIMIT 2 + await checkOnlineAndOfflineResultsMatch( + testHelper.compositeQuery( + coll, + or(where('a', '==', 1), where('b', '>', 0)), + limit(2) + ), + 'doc1', + 'doc2' ); - const result = await getDocs(filteredQuery); - expect(toDataArray(result)).to.deep.equal([ - docSnap1.data(), - docSnap2.data() - ]); - }); - }); - }); - - // OR Query tests only run when the SDK's local cache is configured to use - // LRU garbage collection (rather than eager garbage collection) because - // they validate that the result from server and cache match. Additionally, - // these tests must be skipped if running against production because it - // results in a 'missing index' error. The Firestore Emulator, however, does - // serve these queries. - // eslint-disable-next-line no-restricted-properties - (persistence.gc === 'lru' ? describe.only : describe.skip)( - 'OR Queries', - () => { - it('can use query overloads', () => { - const testDocs = { - doc1: { a: 1, b: 0 }, - doc2: { a: 2, b: 1 }, - doc3: { a: 3, b: 2 }, - doc4: { a: 1, b: 3 }, - doc5: { a: 1, b: 1 } - }; - const testHelper = new CompositeIndexTestHelper(); - return testHelper.withTestDocs(persistence, testDocs, async coll => { - // a == 1, limit 2, b - desc - await checkOnlineAndOfflineResultsMatch( - testHelper.query( - coll, - where('a', '==', 1), - limit(2), - orderBy('b', 'desc') - ), - 'doc4', - 'doc5' - ); - }); - }); - - it('can use or queries', () => { - const testDocs = { - doc1: { a: 1, b: 0 }, - doc2: { a: 2, b: 1 }, - doc3: { a: 3, b: 2 }, - doc4: { a: 1, b: 3 }, - doc5: { a: 1, b: 1 } - }; - const testHelper = new CompositeIndexTestHelper(); - return testHelper.withTestDocs(persistence, testDocs, async coll => { - // with one inequality: a>2 || b==1. - await checkOnlineAndOfflineResultsMatch( - testHelper.compositeQuery( - coll, - or(where('a', '>', 2), where('b', '==', 1)) - ), - 'doc5', - 'doc2', - 'doc3' - ); - // Test with limits (implicit order by ASC): (a==1) || (b > 0) LIMIT 2 - await checkOnlineAndOfflineResultsMatch( - testHelper.compositeQuery( - coll, - or(where('a', '==', 1), where('b', '>', 0)), - limit(2) - ), - 'doc1', - 'doc2' - ); - - // Test with limits (explicit order by): (a==1) || (b > 0) LIMIT_TO_LAST 2 - // Note: The public query API does not allow implicit ordering when limitToLast is used. - await checkOnlineAndOfflineResultsMatch( - testHelper.compositeQuery( - coll, - or(where('a', '==', 1), where('b', '>', 0)), - limitToLast(2), - orderBy('b') - ), - 'doc3', - 'doc4' - ); + // Test with limits (explicit order by): (a==1) || (b > 0) LIMIT_TO_LAST 2 + // Note: The public query API does not allow implicit ordering when limitToLast is used. + await checkOnlineAndOfflineResultsMatch( + testHelper.compositeQuery( + coll, + or(where('a', '==', 1), where('b', '>', 0)), + limitToLast(2), + orderBy('b') + ), + 'doc3', + 'doc4' + ); - // Test with limits (explicit order by ASC): (a==2) || (b == 1) ORDER BY a LIMIT 1 - await checkOnlineAndOfflineResultsMatch( - testHelper.compositeQuery( - coll, - or(where('a', '==', 2), where('b', '==', 1)), - limit(1), - orderBy('a') - ), - 'doc5' - ); + // Test with limits (explicit order by ASC): (a==2) || (b == 1) ORDER BY a LIMIT 1 + await checkOnlineAndOfflineResultsMatch( + testHelper.compositeQuery( + coll, + or(where('a', '==', 2), where('b', '==', 1)), + limit(1), + orderBy('a') + ), + 'doc5' + ); - // Test with limits (explicit order by DESC): (a==2) || (b == 1) ORDER BY a LIMIT_TO_LAST 1 - await checkOnlineAndOfflineResultsMatch( - testHelper.compositeQuery( - coll, - or(where('a', '==', 2), where('b', '==', 1)), - limitToLast(1), - orderBy('a') - ), - 'doc2' - ); - }); + // Test with limits (explicit order by DESC): (a==2) || (b == 1) ORDER BY a LIMIT_TO_LAST 1 + await checkOnlineAndOfflineResultsMatch( + testHelper.compositeQuery( + coll, + or(where('a', '==', 2), where('b', '==', 1)), + limitToLast(1), + orderBy('a') + ), + 'doc2' + ); }); + }); - // eslint-disable-next-line no-restricted-properties - it('supports multiple in ops', () => { - const testDocs = { - doc1: { a: 1, b: 0 }, - doc2: { b: 1 }, - doc3: { a: 3, b: 2 }, - doc4: { a: 1, b: 3 }, - doc5: { a: 1 }, - doc6: { a: 2 } - }; - const testHelper = new CompositeIndexTestHelper(); - return testHelper.withTestDocs(persistence, testDocs, async coll => { - // Two IN operations on different fields with disjunction. - await checkOnlineAndOfflineResultsMatch( - testHelper.compositeQuery( - coll, - or(where('a', 'in', [2, 3]), where('b', 'in', [0, 2])), - orderBy('a') - ), - 'doc1', - 'doc6', - 'doc3' - ); + // eslint-disable-next-line no-restricted-properties + it('supports multiple in ops', () => { + const testDocs = { + doc1: { a: 1, b: 0 }, + doc2: { b: 1 }, + doc3: { a: 3, b: 2 }, + doc4: { a: 1, b: 3 }, + doc5: { a: 1 }, + doc6: { a: 2 } + }; + const testHelper = new CompositeIndexTestHelper(); + return testHelper.withTestDocs(persistence, testDocs, async coll => { + // Two IN operations on different fields with disjunction. + await checkOnlineAndOfflineResultsMatch( + testHelper.compositeQuery( + coll, + or(where('a', 'in', [2, 3]), where('b', 'in', [0, 2])), + orderBy('a') + ), + 'doc1', + 'doc6', + 'doc3' + ); - // Two IN operations on different fields with conjunction. - await checkOnlineAndOfflineResultsMatch( - testHelper.compositeQuery( - coll, - and(where('a', 'in', [2, 3]), where('b', 'in', [0, 2])), - orderBy('a') - ), - 'doc3' - ); + // Two IN operations on different fields with conjunction. + await checkOnlineAndOfflineResultsMatch( + testHelper.compositeQuery( + coll, + and(where('a', 'in', [2, 3]), where('b', 'in', [0, 2])), + orderBy('a') + ), + 'doc3' + ); - // Two IN operations on the same field. - // a IN [1,2,3] && a IN [0,1,4] should result in "a==1". - await checkOnlineAndOfflineResultsMatch( - testHelper.compositeQuery( - coll, - and(where('a', 'in', [1, 2, 3]), where('a', 'in', [0, 1, 4])) - ), - 'doc1', - 'doc4', - 'doc5' - ); + // Two IN operations on the same field. + // a IN [1,2,3] && a IN [0,1,4] should result in "a==1". + await checkOnlineAndOfflineResultsMatch( + testHelper.compositeQuery( + coll, + and(where('a', 'in', [1, 2, 3]), where('a', 'in', [0, 1, 4])) + ), + 'doc1', + 'doc4', + 'doc5' + ); - // a IN [2,3] && a IN [0,1,4] is never true and so the result should be an - // empty set. - await checkOnlineAndOfflineResultsMatch( - testHelper.compositeQuery( - coll, - and(where('a', 'in', [2, 3]), where('a', 'in', [0, 1, 4])) - ) - ); + // a IN [2,3] && a IN [0,1,4] is never true and so the result should be an + // empty set. + await checkOnlineAndOfflineResultsMatch( + testHelper.compositeQuery( + coll, + and(where('a', 'in', [2, 3]), where('a', 'in', [0, 1, 4])) + ) + ); - // a IN [0,3] || a IN [0,2] should union them (similar to: a IN [0,2,3]). - await checkOnlineAndOfflineResultsMatch( - testHelper.compositeQuery( - coll, - or(where('a', 'in', [0, 3]), where('a', 'in', [0, 2])) - ), - 'doc3', - 'doc6' - ); + // a IN [0,3] || a IN [0,2] should union them (similar to: a IN [0,2,3]). + await checkOnlineAndOfflineResultsMatch( + testHelper.compositeQuery( + coll, + or(where('a', 'in', [0, 3]), where('a', 'in', [0, 2])) + ), + 'doc3', + 'doc6' + ); - // Nested composite filter on the same field. - await checkOnlineAndOfflineResultsMatch( - testHelper.compositeQuery( - coll, - and( - where('a', 'in', [1, 3]), - or( - where('a', 'in', [0, 2]), - and(where('b', '>=', 1), where('a', 'in', [1, 3])) - ) + // Nested composite filter on the same field. + await checkOnlineAndOfflineResultsMatch( + testHelper.compositeQuery( + coll, + and( + where('a', 'in', [1, 3]), + or( + where('a', 'in', [0, 2]), + and(where('b', '>=', 1), where('a', 'in', [1, 3])) ) - ), - 'doc3', - 'doc4' - ); + ) + ), + 'doc3', + 'doc4' + ); - // Nested composite filter on the different fields. - await checkOnlineAndOfflineResultsMatch( - testHelper.compositeQuery( - coll, - and( - where('b', 'in', [0, 3]), - or( - where('b', 'in', [1]), - and(where('b', 'in', [2, 3]), where('a', 'in', [1, 3])) - ) + // Nested composite filter on the different fields. + await checkOnlineAndOfflineResultsMatch( + testHelper.compositeQuery( + coll, + and( + where('b', 'in', [0, 3]), + or( + where('b', 'in', [1]), + and(where('b', 'in', [2, 3]), where('a', 'in', [1, 3])) ) - ), - 'doc4' - ); - }); + ) + ), + 'doc4' + ); }); - } - ); + }); + }); }); diff --git a/packages/firestore/test/integration/api/query.test.ts b/packages/firestore/test/integration/api/query.test.ts index 416f9f5121b..33767e85667 100644 --- a/packages/firestore/test/integration/api/query.test.ts +++ b/packages/firestore/test/integration/api/query.test.ts @@ -1345,7 +1345,7 @@ apiDescribe('Queries', persistence => { }); // eslint-disable-next-line no-restricted-properties - describe.skip('Multiple Inequality', () => { + (USE_EMULATOR ? describe : describe.skip)('Multiple Inequality', () => { it('can use multiple inequality filters', async () => { const testDocs = { doc1: { key: 'a', sort: 0, v: 0 }, From 5c3e9dd7a76bd8960f3a3929f8f61a5972634cdd Mon Sep 17 00:00:00 2001 From: milaGGL <107142260+milaGGL@users.noreply.github.com> Date: Wed, 20 Sep 2023 12:38:14 -0400 Subject: [PATCH 35/50] format --- .../api/composite_index_query.test.ts | 13 ++++++ .../util/composite_index_test_helper.ts | 41 +++++++++++-------- 2 files changed, 38 insertions(+), 16 deletions(-) diff --git a/packages/firestore/test/integration/api/composite_index_query.test.ts b/packages/firestore/test/integration/api/composite_index_query.test.ts index bf0e58d57b7..150456bd8a3 100644 --- a/packages/firestore/test/integration/api/composite_index_query.test.ts +++ b/packages/firestore/test/integration/api/composite_index_query.test.ts @@ -29,6 +29,19 @@ import { checkOnlineAndOfflineResultsMatch } from '../util/helpers'; +/* + * Guidance for Creating Tests: + * ---------------------------- + * When creating tests that require composite indexes, it is recommended to utilize the + * "CompositeIndexTestHelper" class. This utility class provides methods for creating + * and setting test documents and running queries with ease, ensuring proper data + * isolation and query construction. + * + * Please remember to update the main index configuration file (firestore_index_config.tf) + * with any new composite indexes required by the test cases to maintain synchronization + * of other testing environments, including CI. + */ + apiDescribe('Queries', persistence => { // OR Query tests only run when the SDK's local cache is configured to use // LRU garbage collection (rather than eager garbage collection) because diff --git a/packages/firestore/test/integration/util/composite_index_test_helper.ts b/packages/firestore/test/integration/util/composite_index_test_helper.ts index 52f1e551729..cbb09a832c7 100644 --- a/packages/firestore/test/integration/util/composite_index_test_helper.ts +++ b/packages/firestore/test/integration/util/composite_index_test_helper.ts @@ -19,7 +19,7 @@ import { and } from '../../../src/lite-api/query'; import { AutoId } from '../../../src/util/misc'; import { - query as _query, + query as internalQuery, CollectionReference, DocumentData, Firestore, @@ -40,6 +40,15 @@ import { } from './helpers'; import { COMPOSITE_INDEX_TEST_COLLECTION, DEFAULT_SETTINGS } from './settings'; +/** + * This helper class is designed to facilitate integration testing of Firestore + * queries that require composite indexes within a controlled testing environment. + * + * Key Features: + * - Runs tests against the dedicated test collection with predefined composite indexes. + * - Automatically associates a test ID with documents for data isolation. + * - Constructs Firestore queries with test ID filters. + */ export class CompositeIndexTestHelper { private readonly testId: string; private readonly TEST_ID_FIELD: string = 'testId'; @@ -76,43 +85,43 @@ export class CompositeIndexTestHelper { ); } - // Add test-id to docs created under a specific test. - // Docs are modified instead of deep copy for convenience of equality check in tests. - private addTestIdToDocs(docs: { [key: string]: DocumentData }): void { - for (const doc of Object.values(docs)) { - doc[this.TEST_ID_FIELD] = this.testId; - } - } - - private addTestIdToDoc(doc: DocumentData): void { - doc[this.TEST_ID_FIELD] = this.testId; + // Add testId to documents created under a specific test. + private addTestIdToDocs(docs: { [key: string]: DocumentData }): { + [key: string]: DocumentData; + } { + return Object.keys(docs).reduce((result, key) => { + const doc = { ...docs[key], [this.TEST_ID_FIELD]: this.testId }; + result[key] = doc; + return result; + }, {} as { [key: string]: DocumentData }); } - //add filter on test id + // Adds a filter on test id for a query. query( query_: Query, ...queryConstraints: QueryConstraint[] ): Query { - return _query( + return internalQuery( query_, where(this.TEST_ID_FIELD, '==', this.testId), ...queryConstraints ); } + // Adds a filter on test id for a composite query, compositeQuery( query_: Query, compositeFilter: QueryCompositeFilterConstraint, ...queryConstraints: QueryNonFilterConstraint[] ): Query { - return _query( + return internalQuery( query_, and(where(this.TEST_ID_FIELD, '==', this.testId), compositeFilter), ...queryConstraints ); } - // add doc with test id + // Add a document with test id. addDoc( reference: CollectionReference, data: WithFieldValue @@ -122,7 +131,7 @@ export class CompositeIndexTestHelper { return addDocument(reference, data); } - // set doc with test-id + // Set a document with test id. setDoc( reference: DocumentReference, data: WithFieldValue From 6e3514c5fb0b3bda79ba65c6869423bc7e895d0e Mon Sep 17 00:00:00 2001 From: milaGGL <107142260+milaGGL@users.noreply.github.com> Date: Wed, 20 Sep 2023 14:01:17 -0400 Subject: [PATCH 36/50] Update composite_index_test_helper.ts --- .../test/integration/util/composite_index_test_helper.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/firestore/test/integration/util/composite_index_test_helper.ts b/packages/firestore/test/integration/util/composite_index_test_helper.ts index cbb09a832c7..503dc315035 100644 --- a/packages/firestore/test/integration/util/composite_index_test_helper.ts +++ b/packages/firestore/test/integration/util/composite_index_test_helper.ts @@ -63,11 +63,10 @@ export class CompositeIndexTestHelper { docs: { [key: string]: DocumentData }, fn: (collection: CollectionReference, db: Firestore) => Promise ): Promise { - this.addTestIdToDocs(docs); return batchCommitDocsToCollection( persistence, DEFAULT_SETTINGS, - docs, + this.addTestIdToDocs(docs), COMPOSITE_INDEX_TEST_COLLECTION, fn ); From 3ac112c8f9be9620212869f4519b43abd96e476f Mon Sep 17 00:00:00 2001 From: milaGGL <107142260+milaGGL@users.noreply.github.com> Date: Wed, 20 Sep 2023 15:30:16 -0400 Subject: [PATCH 37/50] hash document keys --- .../api/composite_index_query.test.ts | 361 +++++++++--------- .../util/composite_index_test_helper.ts | 31 +- 2 files changed, 208 insertions(+), 184 deletions(-) diff --git a/packages/firestore/test/integration/api/composite_index_query.test.ts b/packages/firestore/test/integration/api/composite_index_query.test.ts index 150456bd8a3..f422f435f1a 100644 --- a/packages/firestore/test/integration/api/composite_index_query.test.ts +++ b/packages/firestore/test/integration/api/composite_index_query.test.ts @@ -24,10 +24,7 @@ import { limitToLast, or } from '../util/firebase_export'; -import { - apiDescribe, - checkOnlineAndOfflineResultsMatch -} from '../util/helpers'; +import { apiDescribe } from '../util/helpers'; /* * Guidance for Creating Tests: @@ -39,7 +36,8 @@ import { * * Please remember to update the main index configuration file (firestore_index_config.tf) * with any new composite indexes required by the test cases to maintain synchronization - * of other testing environments, including CI. + * of other testing environments, including CI. The required composite index can be generated by + * clicking on the link to Firebase console in error message while running the test locally. */ apiDescribe('Queries', persistence => { @@ -47,196 +45,199 @@ apiDescribe('Queries', persistence => { // LRU garbage collection (rather than eager garbage collection) because // they validate that the result from server and cache match. // eslint-disable-next-line no-restricted-properties - (persistence.gc === 'lru' ? describe : describe.skip)('OR Queries', () => { - it('can use query overloads', () => { - const testDocs = { - doc1: { a: 1, b: 0 }, - doc2: { a: 2, b: 1 }, - doc3: { a: 3, b: 2 }, - doc4: { a: 1, b: 3 }, - doc5: { a: 1, b: 1 } - }; - const testHelper = new CompositeIndexTestHelper(); - return testHelper.withTestDocs(persistence, testDocs, async coll => { - // a == 1, limit 2, b - desc - await checkOnlineAndOfflineResultsMatch( - testHelper.query( - coll, - where('a', '==', 1), - limit(2), - orderBy('b', 'desc') - ), - 'doc4', - 'doc5' - ); + (persistence.gc === 'lru' ? describe.only : describe.skip)( + 'OR Queries', + () => { + it('can use query overloads', () => { + const testDocs = { + doc1: { a: 1, b: 0 }, + doc2: { a: 2, b: 1 }, + doc3: { a: 3, b: 2 }, + doc4: { a: 1, b: 3 }, + doc5: { a: 1, b: 1 } + }; + const testHelper = new CompositeIndexTestHelper(); + return testHelper.withTestDocs(persistence, testDocs, async coll => { + // a == 1, limit 2, b - desc + await testHelper.checkOnlineAndOfflineResultsMatch( + testHelper.query( + coll, + where('a', '==', 1), + limit(2), + orderBy('b', 'desc') + ), + 'doc4', + 'doc5' + ); + }); }); - }); - it('can use or queries', () => { - const testDocs = { - doc1: { a: 1, b: 0 }, - doc2: { a: 2, b: 1 }, - doc3: { a: 3, b: 2 }, - doc4: { a: 1, b: 3 }, - doc5: { a: 1, b: 1 } - }; - const testHelper = new CompositeIndexTestHelper(); - return testHelper.withTestDocs(persistence, testDocs, async coll => { - // with one inequality: a>2 || b==1. - await checkOnlineAndOfflineResultsMatch( - testHelper.compositeQuery( - coll, - or(where('a', '>', 2), where('b', '==', 1)) - ), - 'doc5', - 'doc2', - 'doc3' - ); + it('can use or queries', () => { + const testDocs = { + doc1: { a: 1, b: 0 }, + doc2: { a: 2, b: 1 }, + doc3: { a: 3, b: 2 }, + doc4: { a: 1, b: 3 }, + doc5: { a: 1, b: 1 } + }; + const testHelper = new CompositeIndexTestHelper(); + return testHelper.withTestDocs(persistence, testDocs, async coll => { + // with one inequality: a>2 || b==1. + await testHelper.checkOnlineAndOfflineResultsMatch( + testHelper.compositeQuery( + coll, + or(where('a', '>', 2), where('b', '==', 1)) + ), + 'doc5', + 'doc2', + 'doc3' + ); - // Test with limits (implicit order by ASC): (a==1) || (b > 0) LIMIT 2 - await checkOnlineAndOfflineResultsMatch( - testHelper.compositeQuery( - coll, - or(where('a', '==', 1), where('b', '>', 0)), - limit(2) - ), - 'doc1', - 'doc2' - ); + // Test with limits (implicit order by ASC): (a==1) || (b > 0) LIMIT 2 + await testHelper.checkOnlineAndOfflineResultsMatch( + testHelper.compositeQuery( + coll, + or(where('a', '==', 1), where('b', '>', 0)), + limit(2) + ), + 'doc1', + 'doc2' + ); - // Test with limits (explicit order by): (a==1) || (b > 0) LIMIT_TO_LAST 2 - // Note: The public query API does not allow implicit ordering when limitToLast is used. - await checkOnlineAndOfflineResultsMatch( - testHelper.compositeQuery( - coll, - or(where('a', '==', 1), where('b', '>', 0)), - limitToLast(2), - orderBy('b') - ), - 'doc3', - 'doc4' - ); + // Test with limits (explicit order by): (a==1) || (b > 0) LIMIT_TO_LAST 2 + // Note: The public query API does not allow implicit ordering when limitToLast is used. + await testHelper.checkOnlineAndOfflineResultsMatch( + testHelper.compositeQuery( + coll, + or(where('a', '==', 1), where('b', '>', 0)), + limitToLast(2), + orderBy('b') + ), + 'doc3', + 'doc4' + ); - // Test with limits (explicit order by ASC): (a==2) || (b == 1) ORDER BY a LIMIT 1 - await checkOnlineAndOfflineResultsMatch( - testHelper.compositeQuery( - coll, - or(where('a', '==', 2), where('b', '==', 1)), - limit(1), - orderBy('a') - ), - 'doc5' - ); + // Test with limits (explicit order by ASC): (a==2) || (b == 1) ORDER BY a LIMIT 1 + await testHelper.checkOnlineAndOfflineResultsMatch( + testHelper.compositeQuery( + coll, + or(where('a', '==', 2), where('b', '==', 1)), + limit(1), + orderBy('a') + ), + 'doc5' + ); - // Test with limits (explicit order by DESC): (a==2) || (b == 1) ORDER BY a LIMIT_TO_LAST 1 - await checkOnlineAndOfflineResultsMatch( - testHelper.compositeQuery( - coll, - or(where('a', '==', 2), where('b', '==', 1)), - limitToLast(1), - orderBy('a') - ), - 'doc2' - ); + // Test with limits (explicit order by DESC): (a==2) || (b == 1) ORDER BY a LIMIT_TO_LAST 1 + await testHelper.checkOnlineAndOfflineResultsMatch( + testHelper.compositeQuery( + coll, + or(where('a', '==', 2), where('b', '==', 1)), + limitToLast(1), + orderBy('a') + ), + 'doc2' + ); + }); }); - }); - // eslint-disable-next-line no-restricted-properties - it('supports multiple in ops', () => { - const testDocs = { - doc1: { a: 1, b: 0 }, - doc2: { b: 1 }, - doc3: { a: 3, b: 2 }, - doc4: { a: 1, b: 3 }, - doc5: { a: 1 }, - doc6: { a: 2 } - }; - const testHelper = new CompositeIndexTestHelper(); - return testHelper.withTestDocs(persistence, testDocs, async coll => { - // Two IN operations on different fields with disjunction. - await checkOnlineAndOfflineResultsMatch( - testHelper.compositeQuery( - coll, - or(where('a', 'in', [2, 3]), where('b', 'in', [0, 2])), - orderBy('a') - ), - 'doc1', - 'doc6', - 'doc3' - ); + // eslint-disable-next-line no-restricted-properties + it('supports multiple in ops', () => { + const testDocs = { + doc1: { a: 1, b: 0 }, + doc2: { b: 1 }, + doc3: { a: 3, b: 2 }, + doc4: { a: 1, b: 3 }, + doc5: { a: 1 }, + doc6: { a: 2 } + }; + const testHelper = new CompositeIndexTestHelper(); + return testHelper.withTestDocs(persistence, testDocs, async coll => { + // Two IN operations on different fields with disjunction. + await testHelper.checkOnlineAndOfflineResultsMatch( + testHelper.compositeQuery( + coll, + or(where('a', 'in', [2, 3]), where('b', 'in', [0, 2])), + orderBy('a') + ), + 'doc1', + 'doc6', + 'doc3' + ); - // Two IN operations on different fields with conjunction. - await checkOnlineAndOfflineResultsMatch( - testHelper.compositeQuery( - coll, - and(where('a', 'in', [2, 3]), where('b', 'in', [0, 2])), - orderBy('a') - ), - 'doc3' - ); + // Two IN operations on different fields with conjunction. + await testHelper.checkOnlineAndOfflineResultsMatch( + testHelper.compositeQuery( + coll, + and(where('a', 'in', [2, 3]), where('b', 'in', [0, 2])), + orderBy('a') + ), + 'doc3' + ); - // Two IN operations on the same field. - // a IN [1,2,3] && a IN [0,1,4] should result in "a==1". - await checkOnlineAndOfflineResultsMatch( - testHelper.compositeQuery( - coll, - and(where('a', 'in', [1, 2, 3]), where('a', 'in', [0, 1, 4])) - ), - 'doc1', - 'doc4', - 'doc5' - ); + // Two IN operations on the same field. + // a IN [1,2,3] && a IN [0,1,4] should result in "a==1". + await testHelper.checkOnlineAndOfflineResultsMatch( + testHelper.compositeQuery( + coll, + and(where('a', 'in', [1, 2, 3]), where('a', 'in', [0, 1, 4])) + ), + 'doc1', + 'doc4', + 'doc5' + ); - // a IN [2,3] && a IN [0,1,4] is never true and so the result should be an - // empty set. - await checkOnlineAndOfflineResultsMatch( - testHelper.compositeQuery( - coll, - and(where('a', 'in', [2, 3]), where('a', 'in', [0, 1, 4])) - ) - ); + // a IN [2,3] && a IN [0,1,4] is never true and so the result should be an + // empty set. + await testHelper.checkOnlineAndOfflineResultsMatch( + testHelper.compositeQuery( + coll, + and(where('a', 'in', [2, 3]), where('a', 'in', [0, 1, 4])) + ) + ); - // a IN [0,3] || a IN [0,2] should union them (similar to: a IN [0,2,3]). - await checkOnlineAndOfflineResultsMatch( - testHelper.compositeQuery( - coll, - or(where('a', 'in', [0, 3]), where('a', 'in', [0, 2])) - ), - 'doc3', - 'doc6' - ); + // a IN [0,3] || a IN [0,2] should union them (similar to: a IN [0,2,3]). + await testHelper.checkOnlineAndOfflineResultsMatch( + testHelper.compositeQuery( + coll, + or(where('a', 'in', [0, 3]), where('a', 'in', [0, 2])) + ), + 'doc3', + 'doc6' + ); - // Nested composite filter on the same field. - await checkOnlineAndOfflineResultsMatch( - testHelper.compositeQuery( - coll, - and( - where('a', 'in', [1, 3]), - or( - where('a', 'in', [0, 2]), - and(where('b', '>=', 1), where('a', 'in', [1, 3])) + // Nested composite filter on the same field. + await testHelper.checkOnlineAndOfflineResultsMatch( + testHelper.compositeQuery( + coll, + and( + where('a', 'in', [1, 3]), + or( + where('a', 'in', [0, 2]), + and(where('b', '>=', 1), where('a', 'in', [1, 3])) + ) ) - ) - ), - 'doc3', - 'doc4' - ); + ), + 'doc3', + 'doc4' + ); - // Nested composite filter on the different fields. - await checkOnlineAndOfflineResultsMatch( - testHelper.compositeQuery( - coll, - and( - where('b', 'in', [0, 3]), - or( - where('b', 'in', [1]), - and(where('b', 'in', [2, 3]), where('a', 'in', [1, 3])) + // Nested composite filter on the different fields. + await testHelper.checkOnlineAndOfflineResultsMatch( + testHelper.compositeQuery( + coll, + and( + where('b', 'in', [0, 3]), + or( + where('b', 'in', [1]), + and(where('b', 'in', [2, 3]), where('a', 'in', [1, 3])) + ) ) - ) - ), - 'doc4' - ); + ), + 'doc4' + ); + }); }); - }); - }); + } + ); }); diff --git a/packages/firestore/test/integration/util/composite_index_test_helper.ts b/packages/firestore/test/integration/util/composite_index_test_helper.ts index 503dc315035..075a74d372b 100644 --- a/packages/firestore/test/integration/util/composite_index_test_helper.ts +++ b/packages/firestore/test/integration/util/composite_index_test_helper.ts @@ -35,6 +35,7 @@ import { } from './firebase_export'; import { batchCommitDocsToCollection, + checkOnlineAndOfflineResultsMatch, PERSISTENCE_MODE_UNSPECIFIED, PersistenceMode } from './helpers'; @@ -58,6 +59,7 @@ export class CompositeIndexTestHelper { this.testId = 'test-id-' + AutoId.newId(); } + // Runs a test with specified documents in the COMPOSITE_INDEX_TEST_COLLECTION. async withTestDocs( persistence: PersistenceMode | typeof PERSISTENCE_MODE_UNSPECIFIED, docs: { [key: string]: DocumentData }, @@ -66,11 +68,13 @@ export class CompositeIndexTestHelper { return batchCommitDocsToCollection( persistence, DEFAULT_SETTINGS, - this.addTestIdToDocs(docs), + this.hashDocs(docs), COMPOSITE_INDEX_TEST_COLLECTION, fn ); } + + // Runs a test without pre-created documents in the COMPOSITE_INDEX_TEST_COLLECTION. async withEmptyCollection( persistence: PersistenceMode | typeof PERSISTENCE_MODE_UNSPECIFIED, fn: (collection: CollectionReference, db: Firestore) => Promise @@ -84,17 +88,36 @@ export class CompositeIndexTestHelper { ); } - // Add testId to documents created under a specific test. - private addTestIdToDocs(docs: { [key: string]: DocumentData }): { + // Hash the document key and add testId to documents created under a specific test to support data + // isolation in parallel testing. + private hashDocs(docs: { [key: string]: DocumentData }): { [key: string]: DocumentData; } { return Object.keys(docs).reduce((result, key) => { const doc = { ...docs[key], [this.TEST_ID_FIELD]: this.testId }; - result[key] = doc; + result[key + '-' + this.testId] = doc; return result; }, {} as { [key: string]: DocumentData }); } + // Hash the document keys with testId. + toHashedIds(docs: string[]): string[] { + return docs.map(docId => docId + '-' + this.testId); + } + + // Checks that running the query while online (against the backend/emulator) results in the same + // as running it while offline. The expected document Ids are hashed to match the actual document + // IDs created by the test helper. + async checkOnlineAndOfflineResultsMatch( + query: Query, + ...expectedDocs: string[] + ): Promise { + return checkOnlineAndOfflineResultsMatch( + query, + ...this.toHashedIds(expectedDocs) + ); + } + // Adds a filter on test id for a query. query( query_: Query, From d9488d1717abf290d8b0685251930ae3ddc421bb Mon Sep 17 00:00:00 2001 From: milaGGL <107142260+milaGGL@users.noreply.github.com> Date: Fri, 22 Sep 2023 11:58:17 -0400 Subject: [PATCH 38/50] resolve comments --- .../api/composite_index_query.test.ts | 354 ++++++------ .../test/integration/api/query.test.ts | 518 ++++++------------ .../util/composite_index_test_helper.ts | 46 +- 3 files changed, 352 insertions(+), 566 deletions(-) diff --git a/packages/firestore/test/integration/api/composite_index_query.test.ts b/packages/firestore/test/integration/api/composite_index_query.test.ts index f422f435f1a..9a5093b830f 100644 --- a/packages/firestore/test/integration/api/composite_index_query.test.ts +++ b/packages/firestore/test/integration/api/composite_index_query.test.ts @@ -40,204 +40,200 @@ import { apiDescribe } from '../util/helpers'; * clicking on the link to Firebase console in error message while running the test locally. */ -apiDescribe('Queries', persistence => { +apiDescribe('Composite Index Queries', persistence => { // OR Query tests only run when the SDK's local cache is configured to use // LRU garbage collection (rather than eager garbage collection) because // they validate that the result from server and cache match. // eslint-disable-next-line no-restricted-properties - (persistence.gc === 'lru' ? describe.only : describe.skip)( - 'OR Queries', - () => { - it('can use query overloads', () => { - const testDocs = { - doc1: { a: 1, b: 0 }, - doc2: { a: 2, b: 1 }, - doc3: { a: 3, b: 2 }, - doc4: { a: 1, b: 3 }, - doc5: { a: 1, b: 1 } - }; - const testHelper = new CompositeIndexTestHelper(); - return testHelper.withTestDocs(persistence, testDocs, async coll => { - // a == 1, limit 2, b - desc - await testHelper.checkOnlineAndOfflineResultsMatch( - testHelper.query( - coll, - where('a', '==', 1), - limit(2), - orderBy('b', 'desc') - ), - 'doc4', - 'doc5' - ); - }); + (persistence.gc === 'lru' ? describe : describe.skip)('OR Queries', () => { + it('can use query overloads', () => { + const testDocs = { + doc1: { a: 1, b: 0 }, + doc2: { a: 2, b: 1 }, + doc3: { a: 3, b: 2 }, + doc4: { a: 1, b: 3 }, + doc5: { a: 1, b: 1 } + }; + const testHelper = new CompositeIndexTestHelper(); + return testHelper.withTestDocs(persistence, testDocs, async coll => { + // a == 1, limit 2, b - desc + await testHelper.checkOnlineAndOfflineResultsMatch( + testHelper.query( + coll, + where('a', '==', 1), + limit(2), + orderBy('b', 'desc') + ), + 'doc4', + 'doc5' + ); }); + }); - it('can use or queries', () => { - const testDocs = { - doc1: { a: 1, b: 0 }, - doc2: { a: 2, b: 1 }, - doc3: { a: 3, b: 2 }, - doc4: { a: 1, b: 3 }, - doc5: { a: 1, b: 1 } - }; - const testHelper = new CompositeIndexTestHelper(); - return testHelper.withTestDocs(persistence, testDocs, async coll => { - // with one inequality: a>2 || b==1. - await testHelper.checkOnlineAndOfflineResultsMatch( - testHelper.compositeQuery( - coll, - or(where('a', '>', 2), where('b', '==', 1)) - ), - 'doc5', - 'doc2', - 'doc3' - ); + it('can use or queries', () => { + const testDocs = { + doc1: { a: 1, b: 0 }, + doc2: { a: 2, b: 1 }, + doc3: { a: 3, b: 2 }, + doc4: { a: 1, b: 3 }, + doc5: { a: 1, b: 1 } + }; + const testHelper = new CompositeIndexTestHelper(); + return testHelper.withTestDocs(persistence, testDocs, async coll => { + // with one inequality: a>2 || b==1. + await testHelper.checkOnlineAndOfflineResultsMatch( + testHelper.compositeQuery( + coll, + or(where('a', '>', 2), where('b', '==', 1)) + ), + 'doc5', + 'doc2', + 'doc3' + ); - // Test with limits (implicit order by ASC): (a==1) || (b > 0) LIMIT 2 - await testHelper.checkOnlineAndOfflineResultsMatch( - testHelper.compositeQuery( - coll, - or(where('a', '==', 1), where('b', '>', 0)), - limit(2) - ), - 'doc1', - 'doc2' - ); + // Test with limits (implicit order by ASC): (a==1) || (b > 0) LIMIT 2 + await testHelper.checkOnlineAndOfflineResultsMatch( + testHelper.compositeQuery( + coll, + or(where('a', '==', 1), where('b', '>', 0)), + limit(2) + ), + 'doc1', + 'doc2' + ); - // Test with limits (explicit order by): (a==1) || (b > 0) LIMIT_TO_LAST 2 - // Note: The public query API does not allow implicit ordering when limitToLast is used. - await testHelper.checkOnlineAndOfflineResultsMatch( - testHelper.compositeQuery( - coll, - or(where('a', '==', 1), where('b', '>', 0)), - limitToLast(2), - orderBy('b') - ), - 'doc3', - 'doc4' - ); + // Test with limits (explicit order by): (a==1) || (b > 0) LIMIT_TO_LAST 2 + // Note: The public query API does not allow implicit ordering when limitToLast is used. + await testHelper.checkOnlineAndOfflineResultsMatch( + testHelper.compositeQuery( + coll, + or(where('a', '==', 1), where('b', '>', 0)), + limitToLast(2), + orderBy('b') + ), + 'doc3', + 'doc4' + ); - // Test with limits (explicit order by ASC): (a==2) || (b == 1) ORDER BY a LIMIT 1 - await testHelper.checkOnlineAndOfflineResultsMatch( - testHelper.compositeQuery( - coll, - or(where('a', '==', 2), where('b', '==', 1)), - limit(1), - orderBy('a') - ), - 'doc5' - ); + // Test with limits (explicit order by ASC): (a==2) || (b == 1) ORDER BY a LIMIT 1 + await testHelper.checkOnlineAndOfflineResultsMatch( + testHelper.compositeQuery( + coll, + or(where('a', '==', 2), where('b', '==', 1)), + limit(1), + orderBy('a') + ), + 'doc5' + ); - // Test with limits (explicit order by DESC): (a==2) || (b == 1) ORDER BY a LIMIT_TO_LAST 1 - await testHelper.checkOnlineAndOfflineResultsMatch( - testHelper.compositeQuery( - coll, - or(where('a', '==', 2), where('b', '==', 1)), - limitToLast(1), - orderBy('a') - ), - 'doc2' - ); - }); + // Test with limits (explicit order by DESC): (a==2) || (b == 1) ORDER BY a LIMIT_TO_LAST 1 + await testHelper.checkOnlineAndOfflineResultsMatch( + testHelper.compositeQuery( + coll, + or(where('a', '==', 2), where('b', '==', 1)), + limitToLast(1), + orderBy('a') + ), + 'doc2' + ); }); + }); - // eslint-disable-next-line no-restricted-properties - it('supports multiple in ops', () => { - const testDocs = { - doc1: { a: 1, b: 0 }, - doc2: { b: 1 }, - doc3: { a: 3, b: 2 }, - doc4: { a: 1, b: 3 }, - doc5: { a: 1 }, - doc6: { a: 2 } - }; - const testHelper = new CompositeIndexTestHelper(); - return testHelper.withTestDocs(persistence, testDocs, async coll => { - // Two IN operations on different fields with disjunction. - await testHelper.checkOnlineAndOfflineResultsMatch( - testHelper.compositeQuery( - coll, - or(where('a', 'in', [2, 3]), where('b', 'in', [0, 2])), - orderBy('a') - ), - 'doc1', - 'doc6', - 'doc3' - ); + it('supports multiple in ops', () => { + const testDocs = { + doc1: { a: 1, b: 0 }, + doc2: { b: 1 }, + doc3: { a: 3, b: 2 }, + doc4: { a: 1, b: 3 }, + doc5: { a: 1 }, + doc6: { a: 2 } + }; + const testHelper = new CompositeIndexTestHelper(); + return testHelper.withTestDocs(persistence, testDocs, async coll => { + // Two IN operations on different fields with disjunction. + await testHelper.checkOnlineAndOfflineResultsMatch( + testHelper.compositeQuery( + coll, + or(where('a', 'in', [2, 3]), where('b', 'in', [0, 2])), + orderBy('a') + ), + 'doc1', + 'doc6', + 'doc3' + ); - // Two IN operations on different fields with conjunction. - await testHelper.checkOnlineAndOfflineResultsMatch( - testHelper.compositeQuery( - coll, - and(where('a', 'in', [2, 3]), where('b', 'in', [0, 2])), - orderBy('a') - ), - 'doc3' - ); + // Two IN operations on different fields with conjunction. + await testHelper.checkOnlineAndOfflineResultsMatch( + testHelper.compositeQuery( + coll, + and(where('a', 'in', [2, 3]), where('b', 'in', [0, 2])), + orderBy('a') + ), + 'doc3' + ); - // Two IN operations on the same field. - // a IN [1,2,3] && a IN [0,1,4] should result in "a==1". - await testHelper.checkOnlineAndOfflineResultsMatch( - testHelper.compositeQuery( - coll, - and(where('a', 'in', [1, 2, 3]), where('a', 'in', [0, 1, 4])) - ), - 'doc1', - 'doc4', - 'doc5' - ); + // Two IN operations on the same field. + // a IN [1,2,3] && a IN [0,1,4] should result in "a==1". + await testHelper.checkOnlineAndOfflineResultsMatch( + testHelper.compositeQuery( + coll, + and(where('a', 'in', [1, 2, 3]), where('a', 'in', [0, 1, 4])) + ), + 'doc1', + 'doc4', + 'doc5' + ); - // a IN [2,3] && a IN [0,1,4] is never true and so the result should be an - // empty set. - await testHelper.checkOnlineAndOfflineResultsMatch( - testHelper.compositeQuery( - coll, - and(where('a', 'in', [2, 3]), where('a', 'in', [0, 1, 4])) - ) - ); + // a IN [2,3] && a IN [0,1,4] is never true and so the result should be an + // empty set. + await testHelper.checkOnlineAndOfflineResultsMatch( + testHelper.compositeQuery( + coll, + and(where('a', 'in', [2, 3]), where('a', 'in', [0, 1, 4])) + ) + ); - // a IN [0,3] || a IN [0,2] should union them (similar to: a IN [0,2,3]). - await testHelper.checkOnlineAndOfflineResultsMatch( - testHelper.compositeQuery( - coll, - or(where('a', 'in', [0, 3]), where('a', 'in', [0, 2])) - ), - 'doc3', - 'doc6' - ); + // a IN [0,3] || a IN [0,2] should union them (similar to: a IN [0,2,3]). + await testHelper.checkOnlineAndOfflineResultsMatch( + testHelper.compositeQuery( + coll, + or(where('a', 'in', [0, 3]), where('a', 'in', [0, 2])) + ), + 'doc3', + 'doc6' + ); - // Nested composite filter on the same field. - await testHelper.checkOnlineAndOfflineResultsMatch( - testHelper.compositeQuery( - coll, - and( - where('a', 'in', [1, 3]), - or( - where('a', 'in', [0, 2]), - and(where('b', '>=', 1), where('a', 'in', [1, 3])) - ) + // Nested composite filter on the same field. + await testHelper.checkOnlineAndOfflineResultsMatch( + testHelper.compositeQuery( + coll, + and( + where('a', 'in', [1, 3]), + or( + where('a', 'in', [0, 2]), + and(where('b', '>=', 1), where('a', 'in', [1, 3])) ) - ), - 'doc3', - 'doc4' - ); + ) + ), + 'doc3', + 'doc4' + ); - // Nested composite filter on the different fields. - await testHelper.checkOnlineAndOfflineResultsMatch( - testHelper.compositeQuery( - coll, - and( - where('b', 'in', [0, 3]), - or( - where('b', 'in', [1]), - and(where('b', 'in', [2, 3]), where('a', 'in', [1, 3])) - ) + // Nested composite filter on the different fields. + await testHelper.checkOnlineAndOfflineResultsMatch( + testHelper.compositeQuery( + coll, + and( + where('b', 'in', [0, 3]), + or( + where('b', 'in', [1]), + and(where('b', 'in', [2, 3]), where('a', 'in', [1, 3])) ) - ), - 'doc4' - ); - }); + ) + ), + 'doc4' + ); }); - } - ); + }); + }); }); diff --git a/packages/firestore/test/integration/api/query.test.ts b/packages/firestore/test/integration/api/query.test.ts index 33767e85667..9d2430e9401 100644 --- a/packages/firestore/test/integration/api/query.test.ts +++ b/packages/firestore/test/integration/api/query.test.ts @@ -2230,389 +2230,183 @@ apiDescribe('Queries', persistence => { // OR Query tests only run when the SDK's local cache is configured to use // LRU garbage collection (rather than eager garbage collection) because - // they validate that the result from server and cache match. Additionally, - // these tests must be skipped if running against production because it - // results in a 'missing index' error. The Firestore Emulator, however, does - // serve these queries. + // they validate that the result from server and cache match. // eslint-disable-next-line no-restricted-properties - (persistence.gc === 'lru' && USE_EMULATOR ? describe : describe.skip)( - 'OR Queries', - () => { - it('can use query overloads', () => { - const testDocs = { - doc1: { a: 1, b: 0 }, - doc2: { a: 2, b: 1 }, - doc3: { a: 3, b: 2 }, - doc4: { a: 1, b: 3 }, - doc5: { a: 1, b: 1 } - }; - - return withTestCollection(persistence, testDocs, async coll => { - // a == 1, limit 2, b - desc - await checkOnlineAndOfflineResultsMatch( - query(coll, where('a', '==', 1), limit(2), orderBy('b', 'desc')), - 'doc4', - 'doc5' - ); - }); - }); - - it('can use or queries', () => { - const testDocs = { - doc1: { a: 1, b: 0 }, - doc2: { a: 2, b: 1 }, - doc3: { a: 3, b: 2 }, - doc4: { a: 1, b: 3 }, - doc5: { a: 1, b: 1 } - }; - - return withTestCollection(persistence, testDocs, async coll => { - // with one inequality: a>2 || b==1. - await checkOnlineAndOfflineResultsMatch( - query(coll, or(where('a', '>', 2), where('b', '==', 1))), - 'doc5', - 'doc2', - 'doc3' - ); - - // Test with limits (implicit order by ASC): (a==1) || (b > 0) LIMIT 2 - await checkOnlineAndOfflineResultsMatch( - query(coll, or(where('a', '==', 1), where('b', '>', 0)), limit(2)), - 'doc1', - 'doc2' - ); - - // Test with limits (explicit order by): (a==1) || (b > 0) LIMIT_TO_LAST 2 - // Note: The public query API does not allow implicit ordering when limitToLast is used. - await checkOnlineAndOfflineResultsMatch( - query( - coll, - or(where('a', '==', 1), where('b', '>', 0)), - limitToLast(2), - orderBy('b') - ), - 'doc3', - 'doc4' - ); - - // Test with limits (explicit order by ASC): (a==2) || (b == 1) ORDER BY a LIMIT 1 - await checkOnlineAndOfflineResultsMatch( - query( - coll, - or(where('a', '==', 2), where('b', '==', 1)), - limit(1), - orderBy('a') - ), - 'doc5' - ); - - // Test with limits (explicit order by DESC): (a==2) || (b == 1) ORDER BY a LIMIT_TO_LAST 1 - await checkOnlineAndOfflineResultsMatch( - query( - coll, - or(where('a', '==', 2), where('b', '==', 1)), - limitToLast(1), - orderBy('a') - ), - 'doc2' - ); - }); - }); - - it('can use or queries with not-in', () => { - const testDocs = { - doc1: { a: 1, b: 0 }, - doc2: { b: 1 }, - doc3: { a: 3, b: 2 }, - doc4: { a: 1, b: 3 }, - doc5: { a: 1 }, - doc6: { a: 2 } - }; - - return withTestCollection(persistence, testDocs, async coll => { - // a==2 || b not-in [2,3] - // Has implicit orderBy b. - await checkOnlineAndOfflineResultsMatch( - query(coll, or(where('a', '==', 2), where('b', 'not-in', [2, 3]))), - 'doc1', - 'doc2' - ); - }); - }); - - // eslint-disable-next-line no-restricted-properties - it('supports order by equality', () => { - const testDocs = { - doc1: { a: 1, b: [0] }, - doc2: { b: [1] }, - doc3: { a: 3, b: [2, 7], c: 10 }, - doc4: { a: 1, b: [3, 7] }, - doc5: { a: 1 }, - doc6: { a: 2, c: 20 } - }; + (persistence.gc === 'lru' ? describe : describe.skip)('OR Queries', () => { + // eslint-disable-next-line no-restricted-properties + it('supports order by equality', () => { + const testDocs = { + doc1: { a: 1, b: [0] }, + doc2: { b: [1] }, + doc3: { a: 3, b: [2, 7], c: 10 }, + doc4: { a: 1, b: [3, 7] }, + doc5: { a: 1 }, + doc6: { a: 2, c: 20 } + }; - return withTestCollection(persistence, testDocs, async coll => { - await checkOnlineAndOfflineResultsMatch( - query(coll, where('a', '==', 1), orderBy('a')), - 'doc1', - 'doc4', - 'doc5' - ); + return withTestCollection(persistence, testDocs, async coll => { + await checkOnlineAndOfflineResultsMatch( + query(coll, where('a', '==', 1), orderBy('a')), + 'doc1', + 'doc4', + 'doc5' + ); - await checkOnlineAndOfflineResultsMatch( - query(coll, where('a', 'in', [2, 3]), orderBy('a')), - 'doc6', - 'doc3' - ); - }); + await checkOnlineAndOfflineResultsMatch( + query(coll, where('a', 'in', [2, 3]), orderBy('a')), + 'doc6', + 'doc3' + ); }); + }); - // eslint-disable-next-line no-restricted-properties - it('supports multiple in ops', () => { - const testDocs = { - doc1: { a: 1, b: 0 }, - doc2: { b: 1 }, - doc3: { a: 3, b: 2 }, - doc4: { a: 1, b: 3 }, - doc5: { a: 1 }, - doc6: { a: 2 } - }; - - return withTestCollection(persistence, testDocs, async coll => { - // Two IN operations on different fields with disjunction. - await checkOnlineAndOfflineResultsMatch( - query( - coll, - or(where('a', 'in', [2, 3]), where('b', 'in', [0, 2])), - orderBy('a') - ), - 'doc1', - 'doc6', - 'doc3' - ); - - // Two IN operations on different fields with conjunction. - await checkOnlineAndOfflineResultsMatch( - query( - coll, - and(where('a', 'in', [2, 3]), where('b', 'in', [0, 2])), - orderBy('a') - ), - 'doc3' - ); - - // Two IN operations on the same field. - // a IN [1,2,3] && a IN [0,1,4] should result in "a==1". - await checkOnlineAndOfflineResultsMatch( - query( - coll, - and(where('a', 'in', [1, 2, 3]), where('a', 'in', [0, 1, 4])) - ), - 'doc1', - 'doc4', - 'doc5' - ); + // eslint-disable-next-line no-restricted-properties + it('supports using in with array contains any', () => { + const testDocs = { + doc1: { a: 1, b: [0] }, + doc2: { b: [1] }, + doc3: { a: 3, b: [2, 7], c: 10 }, + doc4: { a: 1, b: [3, 7] }, + doc5: { a: 1 }, + doc6: { a: 2, c: 20 } + }; - // a IN [2,3] && a IN [0,1,4] is never true and so the result should be an - // empty set. - await checkOnlineAndOfflineResultsMatch( - query( - coll, - and(where('a', 'in', [2, 3]), where('a', 'in', [0, 1, 4])) + return withTestCollection(persistence, testDocs, async coll => { + await checkOnlineAndOfflineResultsMatch( + query( + coll, + or( + where('a', 'in', [2, 3]), + where('b', 'array-contains-any', [0, 7]) ) - ); - - // a IN [0,3] || a IN [0,2] should union them (similar to: a IN [0,2,3]). - await checkOnlineAndOfflineResultsMatch( - query(coll, or(where('a', 'in', [0, 3]), where('a', 'in', [0, 2]))), - 'doc3', - 'doc6' - ); - - // Nested composite filter on the same field. - await checkOnlineAndOfflineResultsMatch( - query( - coll, - and( - where('a', 'in', [1, 3]), - or( - where('a', 'in', [0, 2]), - and(where('b', '>=', 1), where('a', 'in', [1, 3])) - ) - ) - ), - 'doc3', - 'doc4' - ); - - // Nested composite filter on the different fields. - await checkOnlineAndOfflineResultsMatch( - query( - coll, - and( - where('b', 'in', [0, 3]), - or( - where('b', 'in', [1]), - and(where('b', 'in', [2, 3]), where('a', 'in', [1, 3])) - ) - ) - ), - 'doc4' - ); - }); - }); - - // eslint-disable-next-line no-restricted-properties - it('supports using in with array contains any', () => { - const testDocs = { - doc1: { a: 1, b: [0] }, - doc2: { b: [1] }, - doc3: { a: 3, b: [2, 7], c: 10 }, - doc4: { a: 1, b: [3, 7] }, - doc5: { a: 1 }, - doc6: { a: 2, c: 20 } - }; - - return withTestCollection(persistence, testDocs, async coll => { - await checkOnlineAndOfflineResultsMatch( - query( - coll, - or( - where('a', 'in', [2, 3]), - where('b', 'array-contains-any', [0, 7]) - ) - ), - 'doc1', - 'doc3', - 'doc4', - 'doc6' - ); + ), + 'doc1', + 'doc3', + 'doc4', + 'doc6' + ); - await checkOnlineAndOfflineResultsMatch( - query( - coll, - and( - where('a', 'in', [2, 3]), - where('b', 'array-contains-any', [0, 7]) - ) - ), - 'doc3' - ); + await checkOnlineAndOfflineResultsMatch( + query( + coll, + and( + where('a', 'in', [2, 3]), + where('b', 'array-contains-any', [0, 7]) + ) + ), + 'doc3' + ); - await checkOnlineAndOfflineResultsMatch( - query( - coll, - or( - and(where('a', 'in', [2, 3]), where('c', '==', 10)), - where('b', 'array-contains-any', [0, 7]) - ) - ), - 'doc1', - 'doc3', - 'doc4' - ); + await checkOnlineAndOfflineResultsMatch( + query( + coll, + or( + and(where('a', 'in', [2, 3]), where('c', '==', 10)), + where('b', 'array-contains-any', [0, 7]) + ) + ), + 'doc1', + 'doc3', + 'doc4' + ); - await checkOnlineAndOfflineResultsMatch( - query( - coll, - and( - where('a', 'in', [2, 3]), - or( - where('b', 'array-contains-any', [0, 7]), - where('c', '==', 20) - ) - ) - ), - 'doc3', - 'doc6' - ); - }); + await checkOnlineAndOfflineResultsMatch( + query( + coll, + and( + where('a', 'in', [2, 3]), + or(where('b', 'array-contains-any', [0, 7]), where('c', '==', 20)) + ) + ), + 'doc3', + 'doc6' + ); }); + }); - // eslint-disable-next-line no-restricted-properties - it('supports using in with array contains', () => { - const testDocs = { - doc1: { a: 1, b: [0] }, - doc2: { b: [1] }, - doc3: { a: 3, b: [2, 7] }, - doc4: { a: 1, b: [3, 7] }, - doc5: { a: 1 }, - doc6: { a: 2 } - }; + // eslint-disable-next-line no-restricted-properties + it('supports using in with array contains', () => { + const testDocs = { + doc1: { a: 1, b: [0] }, + doc2: { b: [1] }, + doc3: { a: 3, b: [2, 7] }, + doc4: { a: 1, b: [3, 7] }, + doc5: { a: 1 }, + doc6: { a: 2 } + }; - return withTestCollection(persistence, testDocs, async coll => { - await checkOnlineAndOfflineResultsMatch( - query( - coll, - or(where('a', 'in', [2, 3]), where('b', 'array-contains', 3)) - ), - 'doc3', - 'doc4', - 'doc6' - ); + return withTestCollection(persistence, testDocs, async coll => { + await checkOnlineAndOfflineResultsMatch( + query( + coll, + or(where('a', 'in', [2, 3]), where('b', 'array-contains', 3)) + ), + 'doc3', + 'doc4', + 'doc6' + ); - await checkOnlineAndOfflineResultsMatch( - query( - coll, - and(where('a', 'in', [2, 3]), where('b', 'array-contains', 7)) - ), - 'doc3' - ); + await checkOnlineAndOfflineResultsMatch( + query( + coll, + and(where('a', 'in', [2, 3]), where('b', 'array-contains', 7)) + ), + 'doc3' + ); - await checkOnlineAndOfflineResultsMatch( - query( - coll, - or( - where('a', 'in', [2, 3]), - and(where('b', 'array-contains', 3), where('a', '==', 1)) - ) - ), - 'doc3', - 'doc4', - 'doc6' - ); + await checkOnlineAndOfflineResultsMatch( + query( + coll, + or( + where('a', 'in', [2, 3]), + and(where('b', 'array-contains', 3), where('a', '==', 1)) + ) + ), + 'doc3', + 'doc4', + 'doc6' + ); - await checkOnlineAndOfflineResultsMatch( - query( - coll, - and( - where('a', 'in', [2, 3]), - or(where('b', 'array-contains', 7), where('a', '==', 1)) - ) - ), - 'doc3' - ); - }); + await checkOnlineAndOfflineResultsMatch( + query( + coll, + and( + where('a', 'in', [2, 3]), + or(where('b', 'array-contains', 7), where('a', '==', 1)) + ) + ), + 'doc3' + ); }); + }); - // eslint-disable-next-line no-restricted-properties - it('supports order by equality', () => { - const testDocs = { - doc1: { a: 1, b: [0] }, - doc2: { b: [1] }, - doc3: { a: 3, b: [2, 7], c: 10 }, - doc4: { a: 1, b: [3, 7] }, - doc5: { a: 1 }, - doc6: { a: 2, c: 20 } - }; + // eslint-disable-next-line no-restricted-properties + it('supports order by equality', () => { + const testDocs = { + doc1: { a: 1, b: [0] }, + doc2: { b: [1] }, + doc3: { a: 3, b: [2, 7], c: 10 }, + doc4: { a: 1, b: [3, 7] }, + doc5: { a: 1 }, + doc6: { a: 2, c: 20 } + }; - return withTestCollection(persistence, testDocs, async coll => { - await checkOnlineAndOfflineResultsMatch( - query(coll, where('a', '==', 1), orderBy('a')), - 'doc1', - 'doc4', - 'doc5' - ); + return withTestCollection(persistence, testDocs, async coll => { + await checkOnlineAndOfflineResultsMatch( + query(coll, where('a', '==', 1), orderBy('a')), + 'doc1', + 'doc4', + 'doc5' + ); - await checkOnlineAndOfflineResultsMatch( - query(coll, where('a', 'in', [2, 3]), orderBy('a')), - 'doc6', - 'doc3' - ); - }); + await checkOnlineAndOfflineResultsMatch( + query(coll, where('a', 'in', [2, 3]), orderBy('a')), + 'doc6', + 'doc3' + ); }); - } - ); + }); + }); // Reproduces https://github.com/firebase/firebase-js-sdk/issues/5873 describe('Caching empty results', () => { diff --git a/packages/firestore/test/integration/util/composite_index_test_helper.ts b/packages/firestore/test/integration/util/composite_index_test_helper.ts index 075a74d372b..3bf6a4a1b4f 100644 --- a/packages/firestore/test/integration/util/composite_index_test_helper.ts +++ b/packages/firestore/test/integration/util/composite_index_test_helper.ts @@ -68,41 +68,37 @@ export class CompositeIndexTestHelper { return batchCommitDocsToCollection( persistence, DEFAULT_SETTINGS, - this.hashDocs(docs), + this.hashDocIdsAndAddTestIdField(docs), COMPOSITE_INDEX_TEST_COLLECTION, fn ); } - // Runs a test without pre-created documents in the COMPOSITE_INDEX_TEST_COLLECTION. - async withEmptyCollection( - persistence: PersistenceMode | typeof PERSISTENCE_MODE_UNSPECIFIED, - fn: (collection: CollectionReference, db: Firestore) => Promise - ): Promise { - return batchCommitDocsToCollection( - persistence, - DEFAULT_SETTINGS, - {}, - COMPOSITE_INDEX_TEST_COLLECTION, - fn - ); + // Hash the document key with testId. + toHashedId(docId: string): string { + return docId + '-' + this.testId; + } + + toHashedIds(docs: string[]): string[] { + return docs.map(docId => this.toHashedId(docId)); + } + + addTestIdFieldToDoc(doc: DocumentData): DocumentData { + return { ...doc, [this.TEST_ID_FIELD]: this.testId }; } // Hash the document key and add testId to documents created under a specific test to support data // isolation in parallel testing. - private hashDocs(docs: { [key: string]: DocumentData }): { + private hashDocIdsAndAddTestIdField(docs: { [key: string]: DocumentData }): { [key: string]: DocumentData; } { - return Object.keys(docs).reduce((result, key) => { - const doc = { ...docs[key], [this.TEST_ID_FIELD]: this.testId }; - result[key + '-' + this.testId] = doc; - return result; - }, {} as { [key: string]: DocumentData }); - } - - // Hash the document keys with testId. - toHashedIds(docs: string[]): string[] { - return docs.map(docId => docId + '-' + this.testId); + const result: { [key: string]: DocumentData } = {}; + for (const key in docs) { + if (docs.hasOwnProperty(key)) { + result[this.toHashedId(key)] = this.addTestIdFieldToDoc(docs[key]); + } + } + return result; } // Checks that running the query while online (against the backend/emulator) results in the same @@ -130,7 +126,7 @@ export class CompositeIndexTestHelper { ); } - // Adds a filter on test id for a composite query, + // Adds a filter on test id for a composite query. compositeQuery( query_: Query, compositeFilter: QueryCompositeFilterConstraint, From 32b23a752cb0ea77d6e867bbd48510b1de6dd65a Mon Sep 17 00:00:00 2001 From: milaGGL <107142260+milaGGL@users.noreply.github.com> Date: Fri, 22 Sep 2023 12:58:56 -0400 Subject: [PATCH 39/50] pass in project.json as variable to terrafrom --- .github/workflows/test-changed-firestore-integration.yml | 2 +- firestore_index_config.tf | 1 - main.tf | 4 +++- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/.github/workflows/test-changed-firestore-integration.yml b/.github/workflows/test-changed-firestore-integration.yml index 1b13773c02d..48ce2955350 100644 --- a/.github/workflows/test-changed-firestore-integration.yml +++ b/.github/workflows/test-changed-firestore-integration.yml @@ -26,7 +26,7 @@ jobs: if: github.event_name == 'pull_request' run: | cp config/ci.config.json config/project.json - terraform apply -auto-approve + terraform apply -var-file=config/project.json -auto-approve continue-on-error: true - name: Set up Node (16) uses: actions/setup-node@v3 diff --git a/firestore_index_config.tf b/firestore_index_config.tf index 7949e965c0a..98ed132d62f 100644 --- a/firestore_index_config.tf +++ b/firestore_index_config.tf @@ -1,5 +1,4 @@ locals { - project = jsondecode(file("config/project.json")) indexes = { index1 = [ { diff --git a/main.tf b/main.tf index 051260bfcad..397c4d96ebd 100644 --- a/main.tf +++ b/main.tf @@ -1,5 +1,7 @@ +variable "projectId" {} + provider "google" { - project = local.project.projectId + project = var.projectId } resource "google_firestore_index" "default-db-index" { From 2b7ac37dd727c7376c95fc56f2e3488a28bffcf8 Mon Sep 17 00:00:00 2001 From: milaGGL <107142260+milaGGL@users.noreply.github.com> Date: Mon, 25 Sep 2023 12:35:26 -0400 Subject: [PATCH 40/50] Update composite_index_test_helper.ts --- .../util/composite_index_test_helper.ts | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/packages/firestore/test/integration/util/composite_index_test_helper.ts b/packages/firestore/test/integration/util/composite_index_test_helper.ts index 3bf6a4a1b4f..0665021defd 100644 --- a/packages/firestore/test/integration/util/composite_index_test_helper.ts +++ b/packages/firestore/test/integration/util/composite_index_test_helper.ts @@ -142,20 +142,18 @@ export class CompositeIndexTestHelper { // Add a document with test id. addDoc( reference: CollectionReference, - data: WithFieldValue + data: object ): Promise> { - // eslint-disable-next-line @typescript-eslint/no-explicit-any - (data as Record)[this.TEST_ID_FIELD] = this.testId; - return addDocument(reference, data); + const docWithTestId = this.addTestIdFieldToDoc(data) as WithFieldValue; + return addDocument(reference, docWithTestId); } // Set a document with test id. setDoc( reference: DocumentReference, - data: WithFieldValue + data: object ): Promise { - // eslint-disable-next-line @typescript-eslint/no-explicit-any - (data as Record)[this.TEST_ID_FIELD] = this.testId; - return setDocument(reference, data); + const docWithTestId = this.addTestIdFieldToDoc(data) as WithFieldValue; + return setDocument(reference, docWithTestId); } } From 0552cd12a2dd99b3178057f692dc6636eeec2c0f Mon Sep 17 00:00:00 2001 From: milaGGL <107142260+milaGGL@users.noreply.github.com> Date: Mon, 25 Sep 2023 15:16:59 -0400 Subject: [PATCH 41/50] Update composite_index_test_helper.ts --- .../util/composite_index_test_helper.ts | 36 +++++++++++++------ 1 file changed, 26 insertions(+), 10 deletions(-) diff --git a/packages/firestore/test/integration/util/composite_index_test_helper.ts b/packages/firestore/test/integration/util/composite_index_test_helper.ts index 0665021defd..e669cd8aebb 100644 --- a/packages/firestore/test/integration/util/composite_index_test_helper.ts +++ b/packages/firestore/test/integration/util/composite_index_test_helper.ts @@ -31,7 +31,8 @@ import { addDoc as addDocument, setDoc as setDocument, QueryCompositeFilterConstraint, - QueryNonFilterConstraint + QueryNonFilterConstraint, + Timestamp } from './firebase_export'; import { batchCommitDocsToCollection, @@ -68,7 +69,7 @@ export class CompositeIndexTestHelper { return batchCommitDocsToCollection( persistence, DEFAULT_SETTINGS, - this.hashDocIdsAndAddTestIdField(docs), + this.hashDocIdsAndAddCompositeTestFields(docs), COMPOSITE_INDEX_TEST_COLLECTION, fn ); @@ -83,19 +84,30 @@ export class CompositeIndexTestHelper { return docs.map(docId => this.toHashedId(docId)); } - addTestIdFieldToDoc(doc: DocumentData): DocumentData { - return { ...doc, [this.TEST_ID_FIELD]: this.testId }; + addCompositeTestFieldsToDoc(doc: DocumentData): DocumentData { + return { + ...doc, + [this.TEST_ID_FIELD]: this.testId, + expireAt: new Timestamp( // Expire test data after 24 hours + Timestamp.now().seconds + 24 * 60 * 60, + Timestamp.now().nanoseconds + ) + }; } // Hash the document key and add testId to documents created under a specific test to support data // isolation in parallel testing. - private hashDocIdsAndAddTestIdField(docs: { [key: string]: DocumentData }): { + private hashDocIdsAndAddCompositeTestFields(docs: { + [key: string]: DocumentData; + }): { [key: string]: DocumentData; } { const result: { [key: string]: DocumentData } = {}; for (const key in docs) { if (docs.hasOwnProperty(key)) { - result[this.toHashedId(key)] = this.addTestIdFieldToDoc(docs[key]); + result[this.toHashedId(key)] = this.addCompositeTestFieldsToDoc( + docs[key] + ); } } return result; @@ -139,21 +151,25 @@ export class CompositeIndexTestHelper { ); } - // Add a document with test id. + // Add a document with test id and expire date. addDoc( reference: CollectionReference, data: object ): Promise> { - const docWithTestId = this.addTestIdFieldToDoc(data) as WithFieldValue; + const docWithTestId = this.addCompositeTestFieldsToDoc( + data + ) as WithFieldValue; return addDocument(reference, docWithTestId); } - // Set a document with test id. + // Set a document with test id and expire date. setDoc( reference: DocumentReference, data: object ): Promise { - const docWithTestId = this.addTestIdFieldToDoc(data) as WithFieldValue; + const docWithTestId = this.addCompositeTestFieldsToDoc( + data + ) as WithFieldValue; return setDocument(reference, docWithTestId); } } From 1af8c3c1efcd85688164b07c7849c233fdb70030 Mon Sep 17 00:00:00 2001 From: milaGGL <107142260+milaGGL@users.noreply.github.com> Date: Wed, 27 Sep 2023 11:30:24 -0400 Subject: [PATCH 42/50] move the composite index tests into the dedicates file --- firestore_index_config.tf | 38 +-- .../api/composite_index_query.test.ts | 275 ++++++------------ .../test/integration/api/query.test.ts | 98 ------- .../util/composite_index_test_helper.ts | 33 +-- 4 files changed, 125 insertions(+), 319 deletions(-) diff --git a/firestore_index_config.tf b/firestore_index_config.tf index 98ed132d62f..7fafeae6fbf 100644 --- a/firestore_index_config.tf +++ b/firestore_index_config.tf @@ -1,10 +1,6 @@ locals { indexes = { index1 = [ - { - field_path = "b" - order = "ASCENDING" - }, { field_path = "testId" order = "ASCENDING" @@ -16,9 +12,15 @@ locals { ] index2 = [ { - field_path = "a" + field_path = "testId" order = "ASCENDING" }, + { + field_path = "b" + order = "ASCENDING" + }, + ] + index3 = [ { field_path = "testId" order = "ASCENDING" @@ -28,7 +30,7 @@ locals { order = "DESCENDING" }, ] - index3 = [ + index4 = [ { field_path = "a" order = "ASCENDING" @@ -42,7 +44,7 @@ locals { order = "ASCENDING" }, ] - index4 = [ + index5 = [ { field_path = "a" order = "ASCENDING" @@ -52,38 +54,36 @@ locals { order = "ASCENDING" }, { - field_path = "a" + field_path = "b" order = "DESCENDING" }, ] - index5 = [ - { - field_path = "testId" - order = "ASCENDING" - }, + index6 = [ { field_path = "a" order = "ASCENDING" }, - ] - index6 = [ { field_path = "testId" order = "ASCENDING" }, { - field_path = "b" - order = "ASCENDING" + field_path = "a" + order = "DESCENDING" }, ] index7 = [ + { + field_path = "b" + order = "ASCENDING" + }, { field_path = "testId" order = "ASCENDING" }, { - field_path = "b" - order = "DESCENDING" + field_path = "a" + order = "ASCENDING" }, ] index8 = [ diff --git a/packages/firestore/test/integration/api/composite_index_query.test.ts b/packages/firestore/test/integration/api/composite_index_query.test.ts index 9a5093b830f..8f3cf14a69c 100644 --- a/packages/firestore/test/integration/api/composite_index_query.test.ts +++ b/packages/firestore/test/integration/api/composite_index_query.test.ts @@ -19,7 +19,6 @@ import { CompositeIndexTestHelper } from '../util/composite_index_test_helper'; import { where, orderBy, - and, limit, limitToLast, or @@ -45,195 +44,101 @@ apiDescribe('Composite Index Queries', persistence => { // LRU garbage collection (rather than eager garbage collection) because // they validate that the result from server and cache match. // eslint-disable-next-line no-restricted-properties - (persistence.gc === 'lru' ? describe : describe.skip)('OR Queries', () => { - it('can use query overloads', () => { - const testDocs = { - doc1: { a: 1, b: 0 }, - doc2: { a: 2, b: 1 }, - doc3: { a: 3, b: 2 }, - doc4: { a: 1, b: 3 }, - doc5: { a: 1, b: 1 } - }; - const testHelper = new CompositeIndexTestHelper(); - return testHelper.withTestDocs(persistence, testDocs, async coll => { - // a == 1, limit 2, b - desc - await testHelper.checkOnlineAndOfflineResultsMatch( - testHelper.query( - coll, - where('a', '==', 1), - limit(2), - orderBy('b', 'desc') - ), - 'doc4', - 'doc5' - ); + (persistence.gc === 'lru' ? describe.only : describe.skip)( + 'OR Queries', + () => { + it('can use query overloads', () => { + const testDocs = { + doc1: { a: 1, b: 0 }, + doc2: { a: 2, b: 1 }, + doc3: { a: 3, b: 2 }, + doc4: { a: 1, b: 3 }, + doc5: { a: 1, b: 1 } + }; + const testHelper = new CompositeIndexTestHelper(); + return testHelper.withTestDocs(persistence, testDocs, async coll => { + // a == 1, limit 2, b - desc + await testHelper.assertOnlineAndOfflineResultsMatch( + testHelper.query( + coll, + where('a', '==', 1), + limit(2), + orderBy('b', 'desc') + ), + 'doc4', + 'doc5' + ); + }); }); - }); - it('can use or queries', () => { - const testDocs = { - doc1: { a: 1, b: 0 }, - doc2: { a: 2, b: 1 }, - doc3: { a: 3, b: 2 }, - doc4: { a: 1, b: 3 }, - doc5: { a: 1, b: 1 } - }; - const testHelper = new CompositeIndexTestHelper(); - return testHelper.withTestDocs(persistence, testDocs, async coll => { - // with one inequality: a>2 || b==1. - await testHelper.checkOnlineAndOfflineResultsMatch( - testHelper.compositeQuery( - coll, - or(where('a', '>', 2), where('b', '==', 1)) - ), - 'doc5', - 'doc2', - 'doc3' - ); + it('can use or queries', () => { + const testDocs = { + doc1: { a: 1, b: 0 }, + doc2: { a: 2, b: 1 }, + doc3: { a: 3, b: 2 }, + doc4: { a: 1, b: 3 }, + doc5: { a: 1, b: 1 } + }; + const testHelper = new CompositeIndexTestHelper(); + return testHelper.withTestDocs(persistence, testDocs, async coll => { + // with one inequality: a>2 || b==1. + await testHelper.assertOnlineAndOfflineResultsMatch( + testHelper.compositeQuery( + coll, + or(where('a', '>', 2), where('b', '==', 1)) + ), + 'doc5', + 'doc2', + 'doc3' + ); - // Test with limits (implicit order by ASC): (a==1) || (b > 0) LIMIT 2 - await testHelper.checkOnlineAndOfflineResultsMatch( - testHelper.compositeQuery( - coll, - or(where('a', '==', 1), where('b', '>', 0)), - limit(2) - ), - 'doc1', - 'doc2' - ); + // Test with limits (implicit order by ASC): (a==1) || (b > 0) LIMIT 2 + await testHelper.assertOnlineAndOfflineResultsMatch( + testHelper.compositeQuery( + coll, + or(where('a', '==', 1), where('b', '>', 0)), + limit(2) + ), + 'doc1', + 'doc2' + ); - // Test with limits (explicit order by): (a==1) || (b > 0) LIMIT_TO_LAST 2 - // Note: The public query API does not allow implicit ordering when limitToLast is used. - await testHelper.checkOnlineAndOfflineResultsMatch( - testHelper.compositeQuery( - coll, - or(where('a', '==', 1), where('b', '>', 0)), - limitToLast(2), - orderBy('b') - ), - 'doc3', - 'doc4' - ); + // Test with limits (explicit order by): (a==1) || (b > 0) LIMIT_TO_LAST 2 + // Note: The public query API does not allow implicit ordering when limitToLast is used. + await testHelper.assertOnlineAndOfflineResultsMatch( + testHelper.compositeQuery( + coll, + or(where('a', '==', 1), where('b', '>', 0)), + limitToLast(2), + orderBy('b') + ), + 'doc3', + 'doc4' + ); - // Test with limits (explicit order by ASC): (a==2) || (b == 1) ORDER BY a LIMIT 1 - await testHelper.checkOnlineAndOfflineResultsMatch( - testHelper.compositeQuery( - coll, - or(where('a', '==', 2), where('b', '==', 1)), - limit(1), - orderBy('a') - ), - 'doc5' - ); + // Test with limits (explicit order by ASC): (a==2) || (b == 1) ORDER BY a LIMIT 1 + await testHelper.assertOnlineAndOfflineResultsMatch( + testHelper.compositeQuery( + coll, + or(where('a', '==', 2), where('b', '==', 1)), + limit(1), + orderBy('a') + ), + 'doc5' + ); - // Test with limits (explicit order by DESC): (a==2) || (b == 1) ORDER BY a LIMIT_TO_LAST 1 - await testHelper.checkOnlineAndOfflineResultsMatch( - testHelper.compositeQuery( - coll, - or(where('a', '==', 2), where('b', '==', 1)), - limitToLast(1), - orderBy('a') - ), - 'doc2' - ); + // Test with limits (explicit order by DESC): (a==2) || (b == 1) ORDER BY a LIMIT_TO_LAST 1 + await testHelper.assertOnlineAndOfflineResultsMatch( + testHelper.compositeQuery( + coll, + or(where('a', '==', 2), where('b', '==', 1)), + limitToLast(1), + orderBy('a') + ), + 'doc2' + ); + }); }); - }); - - it('supports multiple in ops', () => { - const testDocs = { - doc1: { a: 1, b: 0 }, - doc2: { b: 1 }, - doc3: { a: 3, b: 2 }, - doc4: { a: 1, b: 3 }, - doc5: { a: 1 }, - doc6: { a: 2 } - }; - const testHelper = new CompositeIndexTestHelper(); - return testHelper.withTestDocs(persistence, testDocs, async coll => { - // Two IN operations on different fields with disjunction. - await testHelper.checkOnlineAndOfflineResultsMatch( - testHelper.compositeQuery( - coll, - or(where('a', 'in', [2, 3]), where('b', 'in', [0, 2])), - orderBy('a') - ), - 'doc1', - 'doc6', - 'doc3' - ); - - // Two IN operations on different fields with conjunction. - await testHelper.checkOnlineAndOfflineResultsMatch( - testHelper.compositeQuery( - coll, - and(where('a', 'in', [2, 3]), where('b', 'in', [0, 2])), - orderBy('a') - ), - 'doc3' - ); - - // Two IN operations on the same field. - // a IN [1,2,3] && a IN [0,1,4] should result in "a==1". - await testHelper.checkOnlineAndOfflineResultsMatch( - testHelper.compositeQuery( - coll, - and(where('a', 'in', [1, 2, 3]), where('a', 'in', [0, 1, 4])) - ), - 'doc1', - 'doc4', - 'doc5' - ); - - // a IN [2,3] && a IN [0,1,4] is never true and so the result should be an - // empty set. - await testHelper.checkOnlineAndOfflineResultsMatch( - testHelper.compositeQuery( - coll, - and(where('a', 'in', [2, 3]), where('a', 'in', [0, 1, 4])) - ) - ); - - // a IN [0,3] || a IN [0,2] should union them (similar to: a IN [0,2,3]). - await testHelper.checkOnlineAndOfflineResultsMatch( - testHelper.compositeQuery( - coll, - or(where('a', 'in', [0, 3]), where('a', 'in', [0, 2])) - ), - 'doc3', - 'doc6' - ); - - // Nested composite filter on the same field. - await testHelper.checkOnlineAndOfflineResultsMatch( - testHelper.compositeQuery( - coll, - and( - where('a', 'in', [1, 3]), - or( - where('a', 'in', [0, 2]), - and(where('b', '>=', 1), where('a', 'in', [1, 3])) - ) - ) - ), - 'doc3', - 'doc4' - ); - - // Nested composite filter on the different fields. - await testHelper.checkOnlineAndOfflineResultsMatch( - testHelper.compositeQuery( - coll, - and( - where('b', 'in', [0, 3]), - or( - where('b', 'in', [1]), - and(where('b', 'in', [2, 3]), where('a', 'in', [1, 3])) - ) - ) - ), - 'doc4' - ); - }); - }); - }); + } + ); }); diff --git a/packages/firestore/test/integration/api/query.test.ts b/packages/firestore/test/integration/api/query.test.ts index 35d20e35626..767853bebde 100644 --- a/packages/firestore/test/integration/api/query.test.ts +++ b/packages/firestore/test/integration/api/query.test.ts @@ -2327,104 +2327,6 @@ apiDescribe('Queries', persistence => { }); }); - // OR Query tests only run when the SDK's local cache is configured to use - // LRU garbage collection (rather than eager garbage collection) because - // they validate that the result from server and cache match. - // eslint-disable-next-line no-restricted-properties - (persistence.gc === 'lru' && USE_EMULATOR ? describe : describe.skip)( - 'OR Queries That Need Composite Indexes', - () => { - it('can use query overloads', () => { - const testDocs = { - doc1: { a: 1, b: 0 }, - doc2: { a: 2, b: 1 }, - doc3: { a: 3, b: 2 }, - doc4: { a: 1, b: 3 }, - doc5: { a: 1, b: 1 } - }; - - return withTestCollection(persistence, testDocs, async coll => { - await checkOnlineAndOfflineResultsMatch( - query(coll, where('a', '==', 1), orderBy('a')), - 'doc1', - 'doc4', - 'doc5' - ); - - await checkOnlineAndOfflineResultsMatch( - query(coll, where('a', 'in', [2, 3]), orderBy('a')), - 'doc6', - 'doc3' - ); - }); - }); - - // eslint-disable-next-line no-restricted-properties - it('supports using in with array contains any', () => { - const testDocs = { - doc1: { a: 1, b: [0] }, - doc2: { b: [1] }, - doc3: { a: 3, b: [2, 7], c: 10 }, - doc4: { a: 1, b: [3, 7] }, - doc5: { a: 1 }, - doc6: { a: 2, c: 20 } - }; - - return withTestCollection(persistence, testDocs, async coll => { - // with one inequality: a>2 || b==1. - await checkOnlineAndOfflineResultsMatch( - query(coll, or(where('a', '>', 2), where('b', '==', 1))), - 'doc5', - 'doc2', - 'doc3' - ); - - // Test with limits (implicit order by ASC): (a==1) || (b > 0) LIMIT 2 - await checkOnlineAndOfflineResultsMatch( - query(coll, or(where('a', '==', 1), where('b', '>', 0)), limit(2)), - 'doc1', - 'doc2' - ); - - // Test with limits (explicit order by): (a==1) || (b > 0) LIMIT_TO_LAST 2 - // Note: The public query API does not allow implicit ordering when limitToLast is used. - await checkOnlineAndOfflineResultsMatch( - query( - coll, - or(where('a', '==', 1), where('b', '>', 0)), - limitToLast(2), - orderBy('b') - ), - 'doc3', - 'doc4' - ); - - // Test with limits (explicit order by ASC): (a==2) || (b == 1) ORDER BY a LIMIT 1 - await checkOnlineAndOfflineResultsMatch( - query( - coll, - or(where('a', '==', 2), where('b', '==', 1)), - limit(1), - orderBy('a') - ), - 'doc5' - ); - - // Test with limits (explicit order by DESC): (a==2) || (b == 1) ORDER BY a LIMIT_TO_LAST 1 - await checkOnlineAndOfflineResultsMatch( - query( - coll, - or(where('a', '==', 2), where('b', '==', 1)), - limitToLast(1), - orderBy('a') - ), - 'doc2' - ); - }); - }); - } - ); - // Reproduces https://github.com/firebase/firebase-js-sdk/issues/5873 describe('Caching empty results', () => { it('can raise initial snapshot from cache, even if it is empty', () => { diff --git a/packages/firestore/test/integration/util/composite_index_test_helper.ts b/packages/firestore/test/integration/util/composite_index_test_helper.ts index e669cd8aebb..6e301cb3254 100644 --- a/packages/firestore/test/integration/util/composite_index_test_helper.ts +++ b/packages/firestore/test/integration/util/composite_index_test_helper.ts @@ -55,8 +55,9 @@ export class CompositeIndexTestHelper { private readonly testId: string; private readonly TEST_ID_FIELD: string = 'testId'; + // Creates a new instance of the CompositeIndexTestHelper class, with a unique test + // identifier for data isolation. constructor() { - // Initialize the testId when an instance of the class is created. this.testId = 'test-id-' + AutoId.newId(); } @@ -69,7 +70,7 @@ export class CompositeIndexTestHelper { return batchCommitDocsToCollection( persistence, DEFAULT_SETTINGS, - this.hashDocIdsAndAddCompositeTestFields(docs), + this.prepareTestDocuments(docs), COMPOSITE_INDEX_TEST_COLLECTION, fn ); @@ -84,7 +85,8 @@ export class CompositeIndexTestHelper { return docs.map(docId => this.toHashedId(docId)); } - addCompositeTestFieldsToDoc(doc: DocumentData): DocumentData { + // Adds test-specific fields to a document, including the testId and expiration date. + addTestSpecificFieldsToDoc(doc: DocumentData): DocumentData { return { ...doc, [this.TEST_ID_FIELD]: this.testId, @@ -95,17 +97,14 @@ export class CompositeIndexTestHelper { }; } - // Hash the document key and add testId to documents created under a specific test to support data - // isolation in parallel testing. - private hashDocIdsAndAddCompositeTestFields(docs: { - [key: string]: DocumentData; - }): { + // Helper method to hash document keys and add test-specific fields for the provided documents. + private prepareTestDocuments(docs: { [key: string]: DocumentData }): { [key: string]: DocumentData; } { const result: { [key: string]: DocumentData } = {}; for (const key in docs) { if (docs.hasOwnProperty(key)) { - result[this.toHashedId(key)] = this.addCompositeTestFieldsToDoc( + result[this.toHashedId(key)] = this.addTestSpecificFieldsToDoc( docs[key] ); } @@ -113,10 +112,10 @@ export class CompositeIndexTestHelper { return result; } - // Checks that running the query while online (against the backend/emulator) results in the same - // as running it while offline. The expected document Ids are hashed to match the actual document - // IDs created by the test helper. - async checkOnlineAndOfflineResultsMatch( + // Asserts that the result of running the query while online (against the backend/emulator) is + // the same as running it while offline. The expected document Ids are hashed to match the + // actual document IDs created by the test helper. + async assertOnlineAndOfflineResultsMatch( query: Query, ...expectedDocs: string[] ): Promise { @@ -151,23 +150,23 @@ export class CompositeIndexTestHelper { ); } - // Add a document with test id and expire date. + // Adds a document to a Firestore collection with test-specific fields. addDoc( reference: CollectionReference, data: object ): Promise> { - const docWithTestId = this.addCompositeTestFieldsToDoc( + const docWithTestId = this.addTestSpecificFieldsToDoc( data ) as WithFieldValue; return addDocument(reference, docWithTestId); } - // Set a document with test id and expire date. + // Sets a document in Firestore with test-specific fields. setDoc( reference: DocumentReference, data: object ): Promise { - const docWithTestId = this.addCompositeTestFieldsToDoc( + const docWithTestId = this.addTestSpecificFieldsToDoc( data ) as WithFieldValue; return setDocument(reference, docWithTestId); From 946c0ba2b7f095195712a1d22e7c987a1dc9ae29 Mon Sep 17 00:00:00 2001 From: milaGGL <107142260+milaGGL@users.noreply.github.com> Date: Wed, 27 Sep 2023 11:47:37 -0400 Subject: [PATCH 43/50] format --- .../api/composite_index_query.test.ts | 183 +++++++++--------- .../util/composite_index_test_helper.ts | 9 +- 2 files changed, 95 insertions(+), 97 deletions(-) diff --git a/packages/firestore/test/integration/api/composite_index_query.test.ts b/packages/firestore/test/integration/api/composite_index_query.test.ts index 8f3cf14a69c..ba7b74ad350 100644 --- a/packages/firestore/test/integration/api/composite_index_query.test.ts +++ b/packages/firestore/test/integration/api/composite_index_query.test.ts @@ -34,9 +34,9 @@ import { apiDescribe } from '../util/helpers'; * isolation and query construction. * * Please remember to update the main index configuration file (firestore_index_config.tf) - * with any new composite indexes required by the test cases to maintain synchronization - * of other testing environments, including CI. The required composite index can be generated by - * clicking on the link to Firebase console in error message while running the test locally. + * with any new composite indexes needed for the tests. This ensures synchronization with + * other testing environments, including CI. You can generate the required index link by + * clicking on the Firebase console link in the error message while running tests locally. */ apiDescribe('Composite Index Queries', persistence => { @@ -44,101 +44,98 @@ apiDescribe('Composite Index Queries', persistence => { // LRU garbage collection (rather than eager garbage collection) because // they validate that the result from server and cache match. // eslint-disable-next-line no-restricted-properties - (persistence.gc === 'lru' ? describe.only : describe.skip)( - 'OR Queries', - () => { - it('can use query overloads', () => { - const testDocs = { - doc1: { a: 1, b: 0 }, - doc2: { a: 2, b: 1 }, - doc3: { a: 3, b: 2 }, - doc4: { a: 1, b: 3 }, - doc5: { a: 1, b: 1 } - }; - const testHelper = new CompositeIndexTestHelper(); - return testHelper.withTestDocs(persistence, testDocs, async coll => { - // a == 1, limit 2, b - desc - await testHelper.assertOnlineAndOfflineResultsMatch( - testHelper.query( - coll, - where('a', '==', 1), - limit(2), - orderBy('b', 'desc') - ), - 'doc4', - 'doc5' - ); - }); + (persistence.gc === 'lru' ? describe : describe.skip)('OR Queries', () => { + it('can use query overloads', () => { + const testDocs = { + doc1: { a: 1, b: 0 }, + doc2: { a: 2, b: 1 }, + doc3: { a: 3, b: 2 }, + doc4: { a: 1, b: 3 }, + doc5: { a: 1, b: 1 } + }; + const testHelper = new CompositeIndexTestHelper(); + return testHelper.withTestDocs(persistence, testDocs, async coll => { + // a == 1, limit 2, b - desc + await testHelper.assertOnlineAndOfflineResultsMatch( + testHelper.query( + coll, + where('a', '==', 1), + limit(2), + orderBy('b', 'desc') + ), + 'doc4', + 'doc5' + ); }); + }); - it('can use or queries', () => { - const testDocs = { - doc1: { a: 1, b: 0 }, - doc2: { a: 2, b: 1 }, - doc3: { a: 3, b: 2 }, - doc4: { a: 1, b: 3 }, - doc5: { a: 1, b: 1 } - }; - const testHelper = new CompositeIndexTestHelper(); - return testHelper.withTestDocs(persistence, testDocs, async coll => { - // with one inequality: a>2 || b==1. - await testHelper.assertOnlineAndOfflineResultsMatch( - testHelper.compositeQuery( - coll, - or(where('a', '>', 2), where('b', '==', 1)) - ), - 'doc5', - 'doc2', - 'doc3' - ); + it('can use or queries', () => { + const testDocs = { + doc1: { a: 1, b: 0 }, + doc2: { a: 2, b: 1 }, + doc3: { a: 3, b: 2 }, + doc4: { a: 1, b: 3 }, + doc5: { a: 1, b: 1 } + }; + const testHelper = new CompositeIndexTestHelper(); + return testHelper.withTestDocs(persistence, testDocs, async coll => { + // with one inequality: a>2 || b==1. + await testHelper.assertOnlineAndOfflineResultsMatch( + testHelper.compositeQuery( + coll, + or(where('a', '>', 2), where('b', '==', 1)) + ), + 'doc5', + 'doc2', + 'doc3' + ); - // Test with limits (implicit order by ASC): (a==1) || (b > 0) LIMIT 2 - await testHelper.assertOnlineAndOfflineResultsMatch( - testHelper.compositeQuery( - coll, - or(where('a', '==', 1), where('b', '>', 0)), - limit(2) - ), - 'doc1', - 'doc2' - ); + // Test with limits (implicit order by ASC): (a==1) || (b > 0) LIMIT 2 + await testHelper.assertOnlineAndOfflineResultsMatch( + testHelper.compositeQuery( + coll, + or(where('a', '==', 1), where('b', '>', 0)), + limit(2) + ), + 'doc1', + 'doc2' + ); - // Test with limits (explicit order by): (a==1) || (b > 0) LIMIT_TO_LAST 2 - // Note: The public query API does not allow implicit ordering when limitToLast is used. - await testHelper.assertOnlineAndOfflineResultsMatch( - testHelper.compositeQuery( - coll, - or(where('a', '==', 1), where('b', '>', 0)), - limitToLast(2), - orderBy('b') - ), - 'doc3', - 'doc4' - ); + // Test with limits (explicit order by): (a==1) || (b > 0) LIMIT_TO_LAST 2 + // Note: The public query API does not allow implicit ordering when limitToLast is used. + await testHelper.assertOnlineAndOfflineResultsMatch( + testHelper.compositeQuery( + coll, + or(where('a', '==', 1), where('b', '>', 0)), + limitToLast(2), + orderBy('b') + ), + 'doc3', + 'doc4' + ); - // Test with limits (explicit order by ASC): (a==2) || (b == 1) ORDER BY a LIMIT 1 - await testHelper.assertOnlineAndOfflineResultsMatch( - testHelper.compositeQuery( - coll, - or(where('a', '==', 2), where('b', '==', 1)), - limit(1), - orderBy('a') - ), - 'doc5' - ); + // Test with limits (explicit order by ASC): (a==2) || (b == 1) ORDER BY a LIMIT 1 + await testHelper.assertOnlineAndOfflineResultsMatch( + testHelper.compositeQuery( + coll, + or(where('a', '==', 2), where('b', '==', 1)), + limit(1), + orderBy('a') + ), + 'doc5' + ); - // Test with limits (explicit order by DESC): (a==2) || (b == 1) ORDER BY a LIMIT_TO_LAST 1 - await testHelper.assertOnlineAndOfflineResultsMatch( - testHelper.compositeQuery( - coll, - or(where('a', '==', 2), where('b', '==', 1)), - limitToLast(1), - orderBy('a') - ), - 'doc2' - ); - }); + // Test with limits (explicit order by DESC): (a==2) || (b == 1) ORDER BY a LIMIT_TO_LAST 1 + await testHelper.assertOnlineAndOfflineResultsMatch( + testHelper.compositeQuery( + coll, + or(where('a', '==', 2), where('b', '==', 1)), + limitToLast(1), + orderBy('a') + ), + 'doc2' + ); }); - } - ); + }); + }); }); diff --git a/packages/firestore/test/integration/util/composite_index_test_helper.ts b/packages/firestore/test/integration/util/composite_index_test_helper.ts index 6e301cb3254..3a0ef7ecbbe 100644 --- a/packages/firestore/test/integration/util/composite_index_test_helper.ts +++ b/packages/firestore/test/integration/util/composite_index_test_helper.ts @@ -49,6 +49,7 @@ import { COMPOSITE_INDEX_TEST_COLLECTION, DEFAULT_SETTINGS } from './settings'; * Key Features: * - Runs tests against the dedicated test collection with predefined composite indexes. * - Automatically associates a test ID with documents for data isolation. + * - Utilizes TTL policy for automatic test data cleanup. * - Constructs Firestore queries with test ID filters. */ export class CompositeIndexTestHelper { @@ -155,10 +156,10 @@ export class CompositeIndexTestHelper { reference: CollectionReference, data: object ): Promise> { - const docWithTestId = this.addTestSpecificFieldsToDoc( + const processedData = this.addTestSpecificFieldsToDoc( data ) as WithFieldValue; - return addDocument(reference, docWithTestId); + return addDocument(reference, processedData); } // Sets a document in Firestore with test-specific fields. @@ -166,9 +167,9 @@ export class CompositeIndexTestHelper { reference: DocumentReference, data: object ): Promise { - const docWithTestId = this.addTestSpecificFieldsToDoc( + const processedData = this.addTestSpecificFieldsToDoc( data ) as WithFieldValue; - return setDocument(reference, docWithTestId); + return setDocument(reference, processedData); } } From 66821d84410b9236d65765e39521cf1f8e0e7f21 Mon Sep 17 00:00:00 2001 From: milaGGL <107142260+milaGGL@users.noreply.github.com> Date: Wed, 27 Sep 2023 11:49:12 -0400 Subject: [PATCH 44/50] Update composite_index_query.test.ts --- .../test/integration/api/composite_index_query.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/firestore/test/integration/api/composite_index_query.test.ts b/packages/firestore/test/integration/api/composite_index_query.test.ts index ba7b74ad350..4d7257d0fbd 100644 --- a/packages/firestore/test/integration/api/composite_index_query.test.ts +++ b/packages/firestore/test/integration/api/composite_index_query.test.ts @@ -34,7 +34,7 @@ import { apiDescribe } from '../util/helpers'; * isolation and query construction. * * Please remember to update the main index configuration file (firestore_index_config.tf) - * with any new composite indexes needed for the tests. This ensures synchronization with + * with any new composite indexes needed for the tests. This ensures synchronization with * other testing environments, including CI. You can generate the required index link by * clicking on the Firebase console link in the error message while running tests locally. */ From 719e51aa5dffad0d3484ed0da452903f8e85e0ca Mon Sep 17 00:00:00 2001 From: milaGGL <107142260+milaGGL@users.noreply.github.com> Date: Thu, 28 Sep 2023 10:32:38 -0400 Subject: [PATCH 45/50] remove duplicated cp config/ci.config.json config/project.json --- .github/workflows/test-changed-firestore-integration.yml | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/.github/workflows/test-changed-firestore-integration.yml b/.github/workflows/test-changed-firestore-integration.yml index 48ce2955350..98234683477 100644 --- a/.github/workflows/test-changed-firestore-integration.yml +++ b/.github/workflows/test-changed-firestore-integration.yml @@ -39,9 +39,7 @@ jobs: - name: Bump Node memory limit run: echo "NODE_OPTIONS=--max_old_space_size=4096" >> $GITHUB_ENV - name: Test setup and yarn install - run: | - cp config/ci.config.json config/project.json - yarn + run: yarn - name: build run: yarn build:changed firestore-integration - name: Run tests if firestore or its dependencies has changed From aa01d4a08a7b4f7254657fdfa35a5480e257db6e Mon Sep 17 00:00:00 2001 From: milaGGL <107142260+milaGGL@users.noreply.github.com> Date: Mon, 2 Oct 2023 14:42:59 -0400 Subject: [PATCH 46/50] move terraform into firestore --- .../test-changed-firestore-integration.yml | 9 ++-- .../firestore/firestore_index_config.tf | 0 main.tf => packages/firestore/main.tf | 0 .../util/composite_index_test_helper.ts | 49 ++++++++++++++++++- 4 files changed, 53 insertions(+), 5 deletions(-) rename firestore_index_config.tf => packages/firestore/firestore_index_config.tf (100%) rename main.tf => packages/firestore/main.tf (100%) diff --git a/.github/workflows/test-changed-firestore-integration.yml b/.github/workflows/test-changed-firestore-integration.yml index 98234683477..f57d3c3ef57 100644 --- a/.github/workflows/test-changed-firestore-integration.yml +++ b/.github/workflows/test-changed-firestore-integration.yml @@ -20,13 +20,16 @@ jobs: - name: Setup Terraform uses: hashicorp/setup-terraform@v2 - name: Terraform Init - run: terraform init + run: | + cd packages/firestore + terraform init continue-on-error: true - name: Terraform Apply if: github.event_name == 'pull_request' run: | - cp config/ci.config.json config/project.json - terraform apply -var-file=config/project.json -auto-approve + cp ../../config/ci.config.json config/project.json + terraform apply -var-file=../../config/project.json -auto-approve + cd ../.. continue-on-error: true - name: Set up Node (16) uses: actions/setup-node@v3 diff --git a/firestore_index_config.tf b/packages/firestore/firestore_index_config.tf similarity index 100% rename from firestore_index_config.tf rename to packages/firestore/firestore_index_config.tf diff --git a/main.tf b/packages/firestore/main.tf similarity index 100% rename from main.tf rename to packages/firestore/main.tf diff --git a/packages/firestore/test/integration/util/composite_index_test_helper.ts b/packages/firestore/test/integration/util/composite_index_test_helper.ts index 3a0ef7ecbbe..b650578b911 100644 --- a/packages/firestore/test/integration/util/composite_index_test_helper.ts +++ b/packages/firestore/test/integration/util/composite_index_test_helper.ts @@ -17,6 +17,7 @@ import { and } from '../../../src/lite-api/query'; import { AutoId } from '../../../src/util/misc'; +import { field } from '../../util/helpers'; import { query as internalQuery, @@ -32,7 +33,13 @@ import { setDoc as setDocument, QueryCompositeFilterConstraint, QueryNonFilterConstraint, - Timestamp + Timestamp, + DocumentSnapshot, + getDoc as getDocument, + updateDoc as updateDocument, + UpdateData, + getDocs as getDocuments, + QuerySnapshot } from './firebase_export'; import { batchCommitDocsToCollection, @@ -55,6 +62,7 @@ import { COMPOSITE_INDEX_TEST_COLLECTION, DEFAULT_SETTINGS } from './settings'; export class CompositeIndexTestHelper { private readonly testId: string; private readonly TEST_ID_FIELD: string = 'testId'; + private readonly TTL_FIELD: string = 'expireAt'; // Creates a new instance of the CompositeIndexTestHelper class, with a unique test // identifier for data isolation. @@ -91,13 +99,20 @@ export class CompositeIndexTestHelper { return { ...doc, [this.TEST_ID_FIELD]: this.testId, - expireAt: new Timestamp( // Expire test data after 24 hours + [this.TTL_FIELD]: new Timestamp( // Expire test data after 24 hours Timestamp.now().seconds + 24 * 60 * 60, Timestamp.now().nanoseconds ) }; } + // Remove test-specific fields from a document, including the testId and expiration date. + removeTestSpecificFieldsFromDoc(doc: DocumentData): DocumentData { + doc._document?.data?.delete(field(this.TTL_FIELD)); + doc._document?.data?.delete(field(this.TEST_ID_FIELD)); + return doc; + } + // Helper method to hash document keys and add test-specific fields for the provided documents. private prepareTestDocuments(docs: { [key: string]: DocumentData }): { [key: string]: DocumentData; @@ -172,4 +187,34 @@ export class CompositeIndexTestHelper { ) as WithFieldValue; return setDocument(reference, processedData); } + + // This is is the same as making the update on the doc directly with merge=true. + updateDoc( + reference: DocumentReference, + data: UpdateData + ): Promise { + const processedData = this.addTestSpecificFieldsToDoc( + data + ) as UpdateData; + return updateDocument(reference, processedData); + } + + + async getDoc( + reference: DocumentReference + ): Promise> { + const docSnapshot = await getDocument(reference); + this.removeTestSpecificFieldsFromDoc(docSnapshot); + return docSnapshot; + } + + async getDocs( + query_: Query + ): Promise> { + const querySnapshot = await getDocuments(this.query(query_)); + querySnapshot.forEach(doc => { + this.removeTestSpecificFieldsFromDoc(doc); + }); + return querySnapshot; + } } From 364aa84f72f24cace2d03dd76b7f90ca98982b21 Mon Sep 17 00:00:00 2001 From: milaGGL <107142260+milaGGL@users.noreply.github.com> Date: Tue, 3 Oct 2023 11:37:45 -0400 Subject: [PATCH 47/50] add extra test helpers --- .../test-changed-firestore-integration.yml | 8 ++- .../util/composite_index_test_helper.ts | 67 ++++++++++--------- 2 files changed, 41 insertions(+), 34 deletions(-) diff --git a/.github/workflows/test-changed-firestore-integration.yml b/.github/workflows/test-changed-firestore-integration.yml index f57d3c3ef57..5c6791ea324 100644 --- a/.github/workflows/test-changed-firestore-integration.yml +++ b/.github/workflows/test-changed-firestore-integration.yml @@ -21,14 +21,18 @@ jobs: uses: hashicorp/setup-terraform@v2 - name: Terraform Init run: | + cp config/ci.config.json config/project.json + pwd + ls cd packages/firestore terraform init continue-on-error: true - name: Terraform Apply if: github.event_name == 'pull_request' run: | - cp ../../config/ci.config.json config/project.json - terraform apply -var-file=../../config/project.json -auto-approve + pwd + ls + terraform apply -var-file=config/project.json -auto-approve cd ../.. continue-on-error: true - name: Set up Node (16) diff --git a/packages/firestore/test/integration/util/composite_index_test_helper.ts b/packages/firestore/test/integration/util/composite_index_test_helper.ts index b650578b911..7c5fe88218f 100644 --- a/packages/firestore/test/integration/util/composite_index_test_helper.ts +++ b/packages/firestore/test/integration/util/composite_index_test_helper.ts @@ -39,7 +39,9 @@ import { updateDoc as updateDocument, UpdateData, getDocs as getDocuments, - QuerySnapshot + QuerySnapshot, + deleteDoc as deleteDocument, + doc } from './firebase_export'; import { batchCommitDocsToCollection, @@ -86,16 +88,16 @@ export class CompositeIndexTestHelper { } // Hash the document key with testId. - toHashedId(docId: string): string { + private toHashedId(docId: string): string { return docId + '-' + this.testId; } - toHashedIds(docs: string[]): string[] { + private toHashedIds(docs: string[]): string[] { return docs.map(docId => this.toHashedId(docId)); } // Adds test-specific fields to a document, including the testId and expiration date. - addTestSpecificFieldsToDoc(doc: DocumentData): DocumentData { + private addTestSpecificFieldsToDoc(doc: DocumentData): DocumentData { return { ...doc, [this.TEST_ID_FIELD]: this.testId, @@ -107,7 +109,7 @@ export class CompositeIndexTestHelper { } // Remove test-specific fields from a document, including the testId and expiration date. - removeTestSpecificFieldsFromDoc(doc: DocumentData): DocumentData { + private removeTestSpecificFieldsFromDoc(doc: DocumentData): DocumentData { doc._document?.data?.delete(field(this.TTL_FIELD)); doc._document?.data?.delete(field(this.TEST_ID_FIELD)); return doc; @@ -142,10 +144,7 @@ export class CompositeIndexTestHelper { } // Adds a filter on test id for a query. - query( - query_: Query, - ...queryConstraints: QueryConstraint[] - ): Query { + query(query_: Query, ...queryConstraints: QueryConstraint[]): Query { return internalQuery( query_, where(this.TEST_ID_FIELD, '==', this.testId), @@ -154,23 +153,33 @@ export class CompositeIndexTestHelper { } // Adds a filter on test id for a composite query. - compositeQuery( - query_: Query, + compositeQuery( + query_: Query, compositeFilter: QueryCompositeFilterConstraint, ...queryConstraints: QueryNonFilterConstraint[] - ): Query { + ): Query { return internalQuery( query_, and(where(this.TEST_ID_FIELD, '==', this.testId), compositeFilter), ...queryConstraints ); } + // Get document reference from a document key. + getDocRef( + coll: CollectionReference, + docId: string + ): DocumentReference { + if (!docId.includes('test-id-')) { + docId = this.toHashedId(docId); + } + return doc(coll, docId); + } // Adds a document to a Firestore collection with test-specific fields. - addDoc( - reference: CollectionReference, + addDoc( + reference: CollectionReference, data: object - ): Promise> { + ): Promise> { const processedData = this.addTestSpecificFieldsToDoc( data ) as WithFieldValue; @@ -178,39 +187,33 @@ export class CompositeIndexTestHelper { } // Sets a document in Firestore with test-specific fields. - setDoc( - reference: DocumentReference, - data: object - ): Promise { + setDoc(reference: DocumentReference, data: object): Promise { const processedData = this.addTestSpecificFieldsToDoc( data ) as WithFieldValue; return setDocument(reference, processedData); } - // This is is the same as making the update on the doc directly with merge=true. updateDoc( reference: DocumentReference, data: UpdateData ): Promise { - const processedData = this.addTestSpecificFieldsToDoc( - data - ) as UpdateData; - return updateDocument(reference, processedData); + return updateDocument(reference, data); + } + + deleteDoc(reference: DocumentReference): Promise { + return deleteDocument(reference); } - - async getDoc( - reference: DocumentReference - ): Promise> { - const docSnapshot = await getDocument(reference); + // Retrieves a single document from Firestore with test-specific fields removed. + async getDoc(docRef: DocumentReference): Promise> { + const docSnapshot = await getDocument(docRef); this.removeTestSpecificFieldsFromDoc(docSnapshot); return docSnapshot; } - async getDocs( - query_: Query - ): Promise> { + // Retrieves multiple documents from Firestore with test-specific fields removed. + async getDocs(query_: Query): Promise> { const querySnapshot = await getDocuments(this.query(query_)); querySnapshot.forEach(doc => { this.removeTestSpecificFieldsFromDoc(doc); From bd906a551566fa84efa30b7cbf8961e73e4fc635 Mon Sep 17 00:00:00 2001 From: milaGGL <107142260+milaGGL@users.noreply.github.com> Date: Tue, 3 Oct 2023 11:48:30 -0400 Subject: [PATCH 48/50] move the terraform into firestore --- .github/workflows/test-changed-firestore-integration.yml | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/.github/workflows/test-changed-firestore-integration.yml b/.github/workflows/test-changed-firestore-integration.yml index 5c6791ea324..10ae7866ce7 100644 --- a/.github/workflows/test-changed-firestore-integration.yml +++ b/.github/workflows/test-changed-firestore-integration.yml @@ -22,18 +22,14 @@ jobs: - name: Terraform Init run: | cp config/ci.config.json config/project.json - pwd - ls cd packages/firestore terraform init continue-on-error: true - name: Terraform Apply if: github.event_name == 'pull_request' run: | - pwd - ls - terraform apply -var-file=config/project.json -auto-approve - cd ../.. + cd packages/firestore + terraform apply -var-file=../../config/project.json -auto-approve continue-on-error: true - name: Set up Node (16) uses: actions/setup-node@v3 From ed3f5cfb0b488fc6ffbc6028dad68156f0bfdb9c Mon Sep 17 00:00:00 2001 From: milaGGL <107142260+milaGGL@users.noreply.github.com> Date: Tue, 3 Oct 2023 14:13:24 -0400 Subject: [PATCH 49/50] format --- .../util/composite_index_test_helper.ts | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/packages/firestore/test/integration/util/composite_index_test_helper.ts b/packages/firestore/test/integration/util/composite_index_test_helper.ts index 7c5fe88218f..688ea705554 100644 --- a/packages/firestore/test/integration/util/composite_index_test_helper.ts +++ b/packages/firestore/test/integration/util/composite_index_test_helper.ts @@ -52,14 +52,17 @@ import { import { COMPOSITE_INDEX_TEST_COLLECTION, DEFAULT_SETTINGS } from './settings'; /** - * This helper class is designed to facilitate integration testing of Firestore - * queries that require composite indexes within a controlled testing environment. + * This helper class is designed to facilitate integration testing of Firestore queries that + * require composite indexes within a controlled testing environment. * - * Key Features: - * - Runs tests against the dedicated test collection with predefined composite indexes. - * - Automatically associates a test ID with documents for data isolation. - * - Utilizes TTL policy for automatic test data cleanup. - * - Constructs Firestore queries with test ID filters. + *

Key Features: + * + *

    + *
  • Runs tests against the dedicated test collection with predefined composite indexes. + *
  • Automatically associates a test ID with documents for data isolation. + *
  • Utilizes TTL policy for automatic test data cleanup. + *
  • Constructs Firestore queries with test ID filters. + *
*/ export class CompositeIndexTestHelper { private readonly testId: string; From e78dff5d386d1f9ea12baf8a1a7077a6dbed6a25 Mon Sep 17 00:00:00 2001 From: milaGGL <107142260+milaGGL@users.noreply.github.com> Date: Tue, 3 Oct 2023 15:58:20 -0400 Subject: [PATCH 50/50] Update composite_index_test_helper.ts --- .../test/integration/util/composite_index_test_helper.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/firestore/test/integration/util/composite_index_test_helper.ts b/packages/firestore/test/integration/util/composite_index_test_helper.ts index 688ea705554..292f52fa09e 100644 --- a/packages/firestore/test/integration/util/composite_index_test_helper.ts +++ b/packages/firestore/test/integration/util/composite_index_test_helper.ts @@ -112,10 +112,9 @@ export class CompositeIndexTestHelper { } // Remove test-specific fields from a document, including the testId and expiration date. - private removeTestSpecificFieldsFromDoc(doc: DocumentData): DocumentData { + private removeTestSpecificFieldsFromDoc(doc: DocumentData): void { doc._document?.data?.delete(field(this.TTL_FIELD)); doc._document?.data?.delete(field(this.TEST_ID_FIELD)); - return doc; } // Helper method to hash document keys and add test-specific fields for the provided documents.