diff --git a/.github/workflows/ci_tests.yml b/.github/workflows/ci_tests.yml index a0a3c5ef178..066b2c4ae1a 100644 --- a/.github/workflows/ci_tests.yml +++ b/.github/workflows/ci_tests.yml @@ -121,6 +121,22 @@ jobs: with: credentials_json: ${{ secrets.GCP_SERVICE_ACCOUNT }} - uses: google-github-actions/setup-gcloud@v0 + # create composite indexes with Terraform + - name: Setup Terraform + if: contains(matrix.module, ':firebase-firestore') + uses: hashicorp/setup-terraform@v2 + - name: Terraform Init + if: contains(matrix.module, ':firebase-firestore') + run: | + cd firebase-firestore + terraform init + continue-on-error: true + - name: Terraform Apply + if: github.event_name == 'pull_request' && contains(matrix.module, ':firebase-firestore') + run: | + cd firebase-firestore + terraform apply -var-file=../google-services.json -auto-approve + continue-on-error: true - name: ${{ matrix.module }} Integ Tests env: FIREBASE_CI: 1 diff --git a/.gitignore b/.gitignore index da3e77d46fe..d76e10d3c80 100644 --- a/.gitignore +++ b/.gitignore @@ -13,3 +13,9 @@ smoke-test-logs/ smoke-tests/build-debug-headGit-smoke-test smoke-tests/firehorn.log macrobenchmark-output.json + +# generated Terraform docs +.terraform/* +.terraform.lock.hcl +*.tfstate +*.tfstate.* \ No newline at end of file diff --git a/firebase-firestore/firestore_index_config.tf b/firebase-firestore/firestore_index_config.tf new file mode 100644 index 00000000000..7fafeae6fbf --- /dev/null +++ b/firebase-firestore/firestore_index_config.tf @@ -0,0 +1,104 @@ +locals { + indexes = { + index1 = [ + { + field_path = "testId" + order = "ASCENDING" + }, + { + field_path = "a" + order = "ASCENDING" + }, + ] + index2 = [ + { + field_path = "testId" + order = "ASCENDING" + }, + { + field_path = "b" + order = "ASCENDING" + }, + ] + index3 = [ + { + 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 = "b" + order = "DESCENDING" + }, + ] + index6 = [ + { + field_path = "a" + order = "ASCENDING" + }, + { + field_path = "testId" + order = "ASCENDING" + }, + { + field_path = "a" + order = "DESCENDING" + }, + ] + index7 = [ + { + field_path = "b" + order = "ASCENDING" + }, + { + field_path = "testId" + order = "ASCENDING" + }, + { + field_path = "a" + order = "ASCENDING" + }, + ] + index8 = [ + { + field_path = "b" + order = "ASCENDING" + }, + { + field_path = "testId" + order = "ASCENDING" + }, + { + field_path = "a" + order = "DESCENDING" + }, + ] + } +} diff --git a/firebase-firestore/main.tf b/firebase-firestore/main.tf new file mode 100644 index 00000000000..7812a6d0dc6 --- /dev/null +++ b/firebase-firestore/main.tf @@ -0,0 +1,41 @@ +variable "project_info" {} + +provider "google" { + project = var.project_info.project_id +} + +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/firebase-firestore/src/androidTest/java/com/google/firebase/firestore/CompositeIndexQueryTest.java b/firebase-firestore/src/androidTest/java/com/google/firebase/firestore/CompositeIndexQueryTest.java new file mode 100644 index 00000000000..368503aaebe --- /dev/null +++ b/firebase-firestore/src/androidTest/java/com/google/firebase/firestore/CompositeIndexQueryTest.java @@ -0,0 +1,83 @@ +// 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. +package com.google.firebase.firestore; + +import static com.google.firebase.firestore.Filter.equalTo; +import static com.google.firebase.firestore.Filter.greaterThan; +import static com.google.firebase.firestore.Filter.or; +import static com.google.firebase.firestore.testutil.TestUtil.map; + +import androidx.test.ext.junit.runners.AndroidJUnit4; +import com.google.firebase.firestore.testutil.CompositeIndexTestHelper; +import com.google.firebase.firestore.testutil.IntegrationTestUtil; +import java.util.Map; +import org.junit.After; +import org.junit.Test; +import org.junit.runner.RunWith; + +/* + * 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 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. + */ +@RunWith(AndroidJUnit4.class) +public class CompositeIndexQueryTest { + + @After + public void tearDown() { + IntegrationTestUtil.tearDown(); + } + + @Test + public void testOrQueriesWithCompositeIndexes() { + CompositeIndexTestHelper testHelper = new CompositeIndexTestHelper(); + Map> testDocs = + map( + "doc1", map("a", 1, "b", 0), + "doc2", map("a", 2, "b", 1), + "doc3", map("a", 3, "b", 2), + "doc4", map("a", 1, "b", 3), + "doc5", map("a", 1, "b", 1)); + CollectionReference collection = testHelper.withTestDocs(testDocs); + + Query query = collection.where(or(greaterThan("a", 2), equalTo("b", 1))); + // with one inequality: a>2 || b==1. + testHelper.assertOnlineAndOfflineResultsMatch(testHelper.query(query), "doc5", "doc2", "doc3"); + + // Test with limits (implicit order by ASC): (a==1) || (b > 0) LIMIT 2 + query = collection.where(or(equalTo("a", 1), greaterThan("b", 0))).limit(2); + testHelper.assertOnlineAndOfflineResultsMatch(testHelper.query(query), "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. + query = collection.where(or(equalTo("a", 1), greaterThan("b", 0))).limitToLast(2).orderBy("b"); + testHelper.assertOnlineAndOfflineResultsMatch(testHelper.query(query), "doc3", "doc4"); + + // Test with limits (explicit order by ASC): (a==2) || (b == 1) ORDER BY a LIMIT 1 + query = collection.where(or(equalTo("a", 2), equalTo("b", 1))).limit(1).orderBy("a"); + testHelper.assertOnlineAndOfflineResultsMatch(testHelper.query(query), "doc5"); + + // Test with limits (explicit order by DESC): (a==2) || (b == 1) ORDER BY a LIMIT_TO_LAST 1 + query = collection.where(or(equalTo("a", 2), equalTo("b", 1))).limitToLast(1).orderBy("a"); + testHelper.assertOnlineAndOfflineResultsMatch(testHelper.query(query), "doc2"); + } +} diff --git a/firebase-firestore/src/androidTest/java/com/google/firebase/firestore/QueryTest.java b/firebase-firestore/src/androidTest/java/com/google/firebase/firestore/QueryTest.java index 25781532473..48a6841c868 100644 --- a/firebase-firestore/src/androidTest/java/com/google/firebase/firestore/QueryTest.java +++ b/firebase-firestore/src/androidTest/java/com/google/firebase/firestore/QueryTest.java @@ -29,6 +29,7 @@ import static com.google.firebase.firestore.Filter.notInArray; import static com.google.firebase.firestore.Filter.or; import static com.google.firebase.firestore.remote.TestingHooksUtil.captureExistenceFilterMismatches; +import static com.google.firebase.firestore.testutil.IntegrationTestUtil.checkOnlineAndOfflineResultsMatch; import static com.google.firebase.firestore.testutil.IntegrationTestUtil.isRunningAgainstEmulator; import static com.google.firebase.firestore.testutil.IntegrationTestUtil.nullList; import static com.google.firebase.firestore.testutil.IntegrationTestUtil.querySnapshotToIds; @@ -78,25 +79,6 @@ public void tearDown() { IntegrationTestUtil.tearDown(); } - /** - * 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 - */ - public void checkOnlineAndOfflineResultsMatch(Query query, String... expectedDocs) { - QuerySnapshot docsFromServer = waitFor(query.get(Source.SERVER)); - QuerySnapshot docsFromCache = waitFor(query.get(Source.CACHE)); - - assertEquals(querySnapshotToIds(docsFromServer), querySnapshotToIds(docsFromCache)); - List expected = asList(expectedDocs); - if (!expected.isEmpty()) { - assertEquals(expected, querySnapshotToIds(docsFromCache)); - } - } - @Test public void testLimitQueries() { CollectionReference collection = @@ -1522,46 +1504,6 @@ public void testOrQueries() { collection.where(or(equalTo("a", 2), equalTo("b", 1))).limit(1), "doc2"); } - @Test - public void testOrQueriesWithCompositeIndexes() { - assumeTrue( - "Skip this test if running against production because it results in a " - + "'missing index' error. The Firestore Emulator, however, does serve these " - + " queries.", - isRunningAgainstEmulator()); - Map> testDocs = - map( - "doc1", map("a", 1, "b", 0), - "doc2", map("a", 2, "b", 1), - "doc3", map("a", 3, "b", 2), - "doc4", map("a", 1, "b", 3), - "doc5", map("a", 1, "b", 1)); - CollectionReference collection = testCollectionWithDocs(testDocs); - - // with one inequality: a>2 || b==1. - checkOnlineAndOfflineResultsMatch( - collection.where(or(greaterThan("a", 2), equalTo("b", 1))), "doc5", "doc2", "doc3"); - - // Test with limits (implicit order by ASC): (a==1) || (b > 0) LIMIT 2 - checkOnlineAndOfflineResultsMatch( - collection.where(or(equalTo("a", 1), greaterThan("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. - checkOnlineAndOfflineResultsMatch( - collection.where(or(equalTo("a", 1), greaterThan("b", 0))).limitToLast(2).orderBy("b"), - "doc3", - "doc4"); - - // Test with limits (explicit order by ASC): (a==2) || (b == 1) ORDER BY a LIMIT 1 - checkOnlineAndOfflineResultsMatch( - collection.where(or(equalTo("a", 2), equalTo("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 - checkOnlineAndOfflineResultsMatch( - collection.where(or(equalTo("a", 2), equalTo("b", 1))).limitToLast(1).orderBy("a"), "doc2"); - } - @Test public void testOrQueriesWithIn() { Map> testDocs = diff --git a/firebase-firestore/src/androidTest/java/com/google/firebase/firestore/testutil/CompositeIndexTestHelper.java b/firebase-firestore/src/androidTest/java/com/google/firebase/firestore/testutil/CompositeIndexTestHelper.java new file mode 100644 index 00000000000..333fce7d230 --- /dev/null +++ b/firebase-firestore/src/androidTest/java/com/google/firebase/firestore/testutil/CompositeIndexTestHelper.java @@ -0,0 +1,180 @@ +// 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. + +package com.google.firebase.firestore.testutil; + +import static com.google.firebase.firestore.testutil.IntegrationTestUtil.checkOnlineAndOfflineResultsMatch; +import static com.google.firebase.firestore.testutil.IntegrationTestUtil.testFirestore; +import static com.google.firebase.firestore.testutil.IntegrationTestUtil.waitFor; +import static com.google.firebase.firestore.testutil.IntegrationTestUtil.writeAllDocs; +import static com.google.firebase.firestore.util.Util.autoId; + +import androidx.annotation.NonNull; +import com.google.android.gms.tasks.Task; +import com.google.firebase.Timestamp; +import com.google.firebase.firestore.CollectionReference; +import com.google.firebase.firestore.DocumentReference; +import com.google.firebase.firestore.DocumentSnapshot; +import com.google.firebase.firestore.Query; +import com.google.firebase.firestore.QuerySnapshot; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +/** + * 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. + *
+ */ +public class CompositeIndexTestHelper { + private final String testId; + private static final String TEST_ID_FIELD = "testId"; + private static final String TTL_FIELD = "expireAt"; + private static final String COMPOSITE_INDEX_TEST_COLLECTION = "composite-index-test-collection"; + + // Creates a new instance of the CompositeIndexTestHelper class, with a unique test + // identifier for data isolation. + public CompositeIndexTestHelper() { + this.testId = "test-id-" + autoId(); + } + + // Runs a test with specified documents in the COMPOSITE_INDEX_TEST_COLLECTION. + @NonNull + public CollectionReference withTestDocs(@NonNull Map> docs) { + CollectionReference writer = testFirestore().collection(COMPOSITE_INDEX_TEST_COLLECTION); + writeAllDocs(writer, prepareTestDocuments(docs)); + CollectionReference reader = testFirestore().collection(writer.getPath()); + return reader; + } + + // Hash the document key with testId. + private String toHashedId(String docId) { + return docId + '-' + testId; + } + + private String[] toHashedIds(String[] docs) { + String[] hashedIds = new String[docs.length]; + for (int i = 0; i < docs.length; i++) { + hashedIds[i] = toHashedId(docs[i]); + } + return hashedIds; + } + + // Adds test-specific fields to a document, including the testId and expiration date. + private Map addTestSpecificFieldsToDoc(Map doc) { + Map updatedDoc = new HashMap<>(doc); + updatedDoc.put(TEST_ID_FIELD, testId); + updatedDoc.put( + TTL_FIELD, + new Timestamp( // Expire test data after 24 hours + Timestamp.now().getSeconds() + 24 * 60 * 60, Timestamp.now().getNanoseconds())); + return updatedDoc; + } + + // Remove test-specific fields from a document. + private Map removeTestSpecificFieldsFromDoc(Map doc) { + doc.remove(TTL_FIELD); + doc.remove(TEST_ID_FIELD); + return doc; + } + + // Helper method to hash document keys and add test-specific fields for the provided documents. + private Map> prepareTestDocuments( + Map> docs) { + Map> result = new HashMap<>(); + for (String key : docs.keySet()) { + Map doc = addTestSpecificFieldsToDoc(docs.get(key)); + result.put(toHashedId(key), doc); + } + return result; + } + + // 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. + @NonNull + public void assertOnlineAndOfflineResultsMatch( + @NonNull Query query, @NonNull String... expectedDocs) { + checkOnlineAndOfflineResultsMatch(query, toHashedIds(expectedDocs)); + } + + // Adds a filter on test id for a query. + @NonNull + public Query query(@NonNull Query query_) { + return query_.whereEqualTo(TEST_ID_FIELD, testId); + } + + // Get document reference from a document key. + @NonNull + public DocumentReference getDocRef( + @NonNull CollectionReference collection, @NonNull String docId) { + if (!docId.contains("test-id-")) { + docId = toHashedId(docId); + } + return collection.document(docId); + } + + // Adds a document to a Firestore collection with test-specific fields. + @NonNull + public Task addDoc( + @NonNull CollectionReference collection, @NonNull Map data) { + return collection.add(addTestSpecificFieldsToDoc(data)); + } + + // Sets a document in Firestore with test-specific fields. + @NonNull + public Task setDoc(@NonNull DocumentReference document, @NonNull Map data) { + return document.set(addTestSpecificFieldsToDoc(data)); + } + + @NonNull + public Task updateDoc( + @NonNull DocumentReference document, @NonNull Map data) { + return document.update(data); + } + + @NonNull + public Task deleteDoc(@NonNull DocumentReference document) { + return document.delete(); + } + + // Retrieve a single document from Firestore with test-specific fields removed. + // TODO(composite-index-testing) Return sanitized DocumentSnapshot instead of its data. + @NonNull + public Map getSanitizedDocumentData(@NonNull DocumentReference document) { + DocumentSnapshot docSnapshot = waitFor(document.get()); + return removeTestSpecificFieldsFromDoc(docSnapshot.getData()); + } + + // Retrieve multiple documents from Firestore with test-specific fields removed. + // TODO(composite-index-testing) Return sanitized QuerySnapshot instead of its data. + @NonNull + public List> getSanitizedQueryData(@NonNull Query query_) { + QuerySnapshot querySnapshot = waitFor(query(query_).get()); + List> res = new ArrayList<>(); + for (DocumentSnapshot doc : querySnapshot) { + res.add(removeTestSpecificFieldsFromDoc(doc.getData())); + } + return res; + } +} diff --git a/firebase-firestore/src/androidTest/java/com/google/firebase/firestore/testutil/IntegrationTestUtil.java b/firebase-firestore/src/androidTest/java/com/google/firebase/firestore/testutil/IntegrationTestUtil.java index 0e54459c8cb..0cde1711910 100644 --- a/firebase-firestore/src/androidTest/java/com/google/firebase/firestore/testutil/IntegrationTestUtil.java +++ b/firebase-firestore/src/androidTest/java/com/google/firebase/firestore/testutil/IntegrationTestUtil.java @@ -16,6 +16,8 @@ import static com.google.firebase.firestore.testutil.TestUtil.map; import static com.google.firebase.firestore.util.Util.autoId; +import static java.util.Arrays.asList; +import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNull; import android.content.Context; @@ -35,7 +37,9 @@ import com.google.firebase.firestore.FirebaseFirestoreSettings; import com.google.firebase.firestore.ListenerRegistration; import com.google.firebase.firestore.MetadataChanges; +import com.google.firebase.firestore.Query; import com.google.firebase.firestore.QuerySnapshot; +import com.google.firebase.firestore.Source; import com.google.firebase.firestore.WriteBatch; import com.google.firebase.firestore.auth.User; import com.google.firebase.firestore.core.DatabaseInfo; @@ -508,4 +512,23 @@ public static List nullList() { nullArray.add(null); return nullArray; } + + /** + * 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 + */ + public static void checkOnlineAndOfflineResultsMatch(Query query, String... expectedDocs) { + QuerySnapshot docsFromServer = waitFor(query.get(Source.SERVER)); + QuerySnapshot docsFromCache = waitFor(query.get(Source.CACHE)); + + assertEquals(querySnapshotToIds(docsFromServer), querySnapshotToIds(docsFromCache)); + List expected = asList(expectedDocs); + if (!expected.isEmpty()) { + assertEquals(expected, querySnapshotToIds(docsFromCache)); + } + } }