Skip to content

Commit 558d1f5

Browse files
milaGGLmilaGGL
authored
Merge 61c69b3 into dc48e69 
2 parents dc48e69 + 61c69b3 commit 558d1f5

File tree

8 files changed

+454
-59
lines changed

8 files changed

+454
-59
lines changed

‎.github/workflows/ci_tests.yml

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,22 @@ jobs:
121121
with:
122122
credentials_json: ${{ secrets.GCP_SERVICE_ACCOUNT }}
123123
- uses: google-github-actions/setup-gcloud@v0
124+
# create composite indexes with Terraform
125+
- name: Setup Terraform
126+
if: contains(matrix.module, ':firebase-firestore')
127+
uses: hashicorp/setup-terraform@v2
128+
- name: Terraform Init
129+
if: contains(matrix.module, ':firebase-firestore')
130+
run: |
131+
cd firebase-firestore
132+
terraform init
133+
continue-on-error: true
134+
- name: Terraform Apply
135+
if: github.event_name == 'pull_request' && contains(matrix.module, ':firebase-firestore')
136+
run: |
137+
cd firebase-firestore
138+
terraform apply -var-file=../google-services.json -auto-approve
139+
continue-on-error: true
124140
- name: ${{ matrix.module }} Integ Tests
125141
env:
126142
FIREBASE_CI: 1

‎.gitignore

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,3 +13,9 @@ smoke-test-logs/
1313
smoke-tests/build-debug-headGit-smoke-test
1414
smoke-tests/firehorn.log
1515
macrobenchmark-output.json
16+
17+
# generated Terraform docs
18+
.terraform/*
19+
.terraform.lock.hcl
20+
*.tfstate
21+
*.tfstate.*

‎firebase-firestore/firestore_index_config.tf

Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,104 @@
1+
locals {
2+
indexes = {
3+
index1 = [
4+
{
5+
field_path = "testId"
6+
order = "ASCENDING"
7+
},
8+
{
9+
field_path = "a"
10+
order = "ASCENDING"
11+
},
12+
]
13+
index2 = [
14+
{
15+
field_path = "testId"
16+
order = "ASCENDING"
17+
},
18+
{
19+
field_path = "b"
20+
order = "ASCENDING"
21+
},
22+
]
23+
index3 = [
24+
{
25+
field_path = "testId"
26+
order = "ASCENDING"
27+
},
28+
{
29+
field_path = "b"
30+
order = "DESCENDING"
31+
},
32+
]
33+
index4 = [
34+
{
35+
field_path = "a"
36+
order = "ASCENDING"
37+
},
38+
{
39+
field_path = "testId"
40+
order = "ASCENDING"
41+
},
42+
{
43+
field_path = "b"
44+
order = "ASCENDING"
45+
},
46+
]
47+
index5 = [
48+
{
49+
field_path = "a"
50+
order = "ASCENDING"
51+
},
52+
{
53+
field_path = "testId"
54+
order = "ASCENDING"
55+
},
56+
{
57+
field_path = "b"
58+
order = "DESCENDING"
59+
},
60+
]
61+
index6 = [
62+
{
63+
field_path = "a"
64+
order = "ASCENDING"
65+
},
66+
{
67+
field_path = "testId"
68+
order = "ASCENDING"
69+
},
70+
{
71+
field_path = "a"
72+
order = "DESCENDING"
73+
},
74+
]
75+
index7 = [
76+
{
77+
field_path = "b"
78+
order = "ASCENDING"
79+
},
80+
{
81+
field_path = "testId"
82+
order = "ASCENDING"
83+
},
84+
{
85+
field_path = "a"
86+
order = "ASCENDING"
87+
},
88+
]
89+
index8 = [
90+
{
91+
field_path = "b"
92+
order = "ASCENDING"
93+
},
94+
{
95+
field_path = "testId"
96+
order = "ASCENDING"
97+
},
98+
{
99+
field_path = "a"
100+
order = "DESCENDING"
101+
},
102+
]
103+
}
104+
}

‎firebase-firestore/main.tf

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
variable "project_info" {}
2+
3+
provider "google" {
4+
project = var.project_info.project_id
5+
}
6+
7+
resource "google_firestore_index" "default-db-index" {
8+
collection = "composite-index-test-collection"
9+
10+
for_each = local.indexes
11+
dynamic "fields" {
12+
for_each = distinct(flatten([for k, v in local.indexes : [
13+
for i in each.value : {
14+
field_path = i.field_path
15+
order = i.order
16+
}]]))
17+
content {
18+
field_path = lookup(fields.value, "field_path", null)
19+
order = lookup(fields.value, "order", null)
20+
}
21+
}
22+
23+
}
24+
25+
resource "google_firestore_index" "named-db-index" {
26+
collection = "composite-index-test-collection"
27+
database = "test-db"
28+
29+
for_each = local.indexes
30+
dynamic "fields" {
31+
for_each = distinct(flatten([for k, v in local.indexes : [
32+
for i in each.value : {
33+
field_path = i.field_path
34+
order = i.order
35+
}]]))
36+
content {
37+
field_path = lookup(fields.value, "field_path", null)
38+
order = lookup(fields.value, "order", null)
39+
}
40+
}
41+
}

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

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
// Copyright 2023 Google LLC
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
package com.google.firebase.firestore;
15+
16+
import static com.google.firebase.firestore.Filter.equalTo;
17+
import static com.google.firebase.firestore.Filter.greaterThan;
18+
import static com.google.firebase.firestore.Filter.or;
19+
import static com.google.firebase.firestore.testutil.TestUtil.map;
20+
21+
import androidx.test.ext.junit.runners.AndroidJUnit4;
22+
import com.google.firebase.firestore.testutil.CompositeIndexTestHelper;
23+
import com.google.firebase.firestore.testutil.IntegrationTestUtil;
24+
import java.util.Map;
25+
import org.junit.After;
26+
import org.junit.Test;
27+
import org.junit.runner.RunWith;
28+
29+
/*
30+
* Guidance for Creating Tests:
31+
* ----------------------------
32+
* When creating tests that require composite indexes, it is recommended to utilize the
33+
* "CompositeIndexTestHelper" class. This utility class provides methods for creating
34+
* and setting test documents and running queries with ease, ensuring proper data
35+
* isolation and query construction.
36+
*
37+
* Please remember to update the main index configuration file (firestore_index_config.tf)
38+
* with any new composite indexes needed for the tests. This ensures synchronization with
39+
* other testing environments, including CI. You can generate the required index link by
40+
* clicking on the Firebase console link in the error message while running tests locally.
41+
*/
42+
@RunWith(AndroidJUnit4.class)
43+
public class CompositeIndexQueryTest {
44+
45+
@After
46+
public void tearDown() {
47+
IntegrationTestUtil.tearDown();
48+
}
49+
50+
@Test
51+
public void testOrQueriesWithCompositeIndexes() {
52+
CompositeIndexTestHelper testHelper = new CompositeIndexTestHelper();
53+
Map<String, Map<String, Object>> testDocs =
54+
map(
55+
"doc1", map("a", 1, "b", 0),
56+
"doc2", map("a", 2, "b", 1),
57+
"doc3", map("a", 3, "b", 2),
58+
"doc4", map("a", 1, "b", 3),
59+
"doc5", map("a", 1, "b", 1));
60+
CollectionReference collection = testHelper.withTestDocs(testDocs);
61+
62+
Query query = collection.where(or(greaterThan("a", 2), equalTo("b", 1)));
63+
// with one inequality: a>2 || b==1.
64+
testHelper.assertOnlineAndOfflineResultsMatch(testHelper.query(query), "doc5", "doc2", "doc3");
65+
66+
// Test with limits (implicit order by ASC): (a==1) || (b > 0) LIMIT 2
67+
query = collection.where(or(equalTo("a", 1), greaterThan("b", 0))).limit(2);
68+
testHelper.assertOnlineAndOfflineResultsMatch(testHelper.query(query), "doc1", "doc2");
69+
70+
// Test with limits (explicit order by): (a==1) || (b > 0) LIMIT_TO_LAST 2
71+
// Note: The public query API does not allow implicit ordering when limitToLast is used.
72+
query = collection.where(or(equalTo("a", 1), greaterThan("b", 0))).limitToLast(2).orderBy("b");
73+
testHelper.assertOnlineAndOfflineResultsMatch(testHelper.query(query), "doc3", "doc4");
74+
75+
// Test with limits (explicit order by ASC): (a==2) || (b == 1) ORDER BY a LIMIT 1
76+
query = collection.where(or(equalTo("a", 2), equalTo("b", 1))).limit(1).orderBy("a");
77+
testHelper.assertOnlineAndOfflineResultsMatch(testHelper.query(query), "doc5");
78+
79+
// Test with limits (explicit order by DESC): (a==2) || (b == 1) ORDER BY a LIMIT_TO_LAST 1
80+
query = collection.where(or(equalTo("a", 2), equalTo("b", 1))).limitToLast(1).orderBy("a");
81+
testHelper.assertOnlineAndOfflineResultsMatch(testHelper.query(query), "doc2");
82+
}
83+
}

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

Lines changed: 1 addition & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
import static com.google.firebase.firestore.Filter.notInArray;
3030
import static com.google.firebase.firestore.Filter.or;
3131
import static com.google.firebase.firestore.remote.TestingHooksUtil.captureExistenceFilterMismatches;
32+
import static com.google.firebase.firestore.testutil.IntegrationTestUtil.checkOnlineAndOfflineResultsMatch;
3233
import static com.google.firebase.firestore.testutil.IntegrationTestUtil.isRunningAgainstEmulator;
3334
import static com.google.firebase.firestore.testutil.IntegrationTestUtil.nullList;
3435
import static com.google.firebase.firestore.testutil.IntegrationTestUtil.querySnapshotToIds;
@@ -78,25 +79,6 @@ public void tearDown() {
7879
IntegrationTestUtil.tearDown();
7980
}
8081

81-
/**
82-
* Checks that running the query while online (against the backend/emulator) results in the same
83-
* documents as running the query while offline. If `expectedDocs` is provided, it also checks
84-
* that both online and offline query result is equal to the expected documents.
85-
*
86-
* @param query The query to check
87-
* @param expectedDocs Ordered list of document keys that are expected to match the query
88-
*/
89-
public void checkOnlineAndOfflineResultsMatch(Query query, String... expectedDocs) {
90-
QuerySnapshot docsFromServer = waitFor(query.get(Source.SERVER));
91-
QuerySnapshot docsFromCache = waitFor(query.get(Source.CACHE));
92-
93-
assertEquals(querySnapshotToIds(docsFromServer), querySnapshotToIds(docsFromCache));
94-
List<String> expected = asList(expectedDocs);
95-
if (!expected.isEmpty()) {
96-
assertEquals(expected, querySnapshotToIds(docsFromCache));
97-
}
98-
}
99-
10082
@Test
10183
public void testLimitQueries() {
10284
CollectionReference collection =
@@ -1522,46 +1504,6 @@ public void testOrQueries() {
15221504
collection.where(or(equalTo("a", 2), equalTo("b", 1))).limit(1), "doc2");
15231505
}
15241506

1525-
@Test
1526-
public void testOrQueriesWithCompositeIndexes() {
1527-
assumeTrue(
1528-
"Skip this test if running against production because it results in a "
1529-
+ "'missing index' error. The Firestore Emulator, however, does serve these "
1530-
+ " queries.",
1531-
isRunningAgainstEmulator());
1532-
Map<String, Map<String, Object>> testDocs =
1533-
map(
1534-
"doc1", map("a", 1, "b", 0),
1535-
"doc2", map("a", 2, "b", 1),
1536-
"doc3", map("a", 3, "b", 2),
1537-
"doc4", map("a", 1, "b", 3),
1538-
"doc5", map("a", 1, "b", 1));
1539-
CollectionReference collection = testCollectionWithDocs(testDocs);
1540-
1541-
// with one inequality: a>2 || b==1.
1542-
checkOnlineAndOfflineResultsMatch(
1543-
collection.where(or(greaterThan("a", 2), equalTo("b", 1))), "doc5", "doc2", "doc3");
1544-
1545-
// Test with limits (implicit order by ASC): (a==1) || (b > 0) LIMIT 2
1546-
checkOnlineAndOfflineResultsMatch(
1547-
collection.where(or(equalTo("a", 1), greaterThan("b", 0))).limit(2), "doc1", "doc2");
1548-
1549-
// Test with limits (explicit order by): (a==1) || (b > 0) LIMIT_TO_LAST 2
1550-
// Note: The public query API does not allow implicit ordering when limitToLast is used.
1551-
checkOnlineAndOfflineResultsMatch(
1552-
collection.where(or(equalTo("a", 1), greaterThan("b", 0))).limitToLast(2).orderBy("b"),
1553-
"doc3",
1554-
"doc4");
1555-
1556-
// Test with limits (explicit order by ASC): (a==2) || (b == 1) ORDER BY a LIMIT 1
1557-
checkOnlineAndOfflineResultsMatch(
1558-
collection.where(or(equalTo("a", 2), equalTo("b", 1))).limit(1).orderBy("a"), "doc5");
1559-
1560-
// Test with limits (explicit order by DESC): (a==2) || (b == 1) ORDER BY a LIMIT_TO_LAST 1
1561-
checkOnlineAndOfflineResultsMatch(
1562-
collection.where(or(equalTo("a", 2), equalTo("b", 1))).limitToLast(1).orderBy("a"), "doc2");
1563-
}
1564-
15651507
@Test
15661508
public void testOrQueriesWithIn() {
15671509
Map<String, Map<String, Object>> testDocs =

0 commit comments

Comments
 (0)