Skip to content

Commit 8283a55

Browse files
authored
Fix composite index testing build bug (#7682)
1 parent d2e4774 commit 8283a55

File tree

9 files changed

+41
-19
lines changed

9 files changed

+41
-19
lines changed

.github/workflows/test-changed-firestore-integration.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ jobs:
2929
if: github.event_name == 'pull_request'
3030
run: |
3131
cd packages/firestore
32-
terraform apply -var-file=../../config/project.json -auto-approve
32+
terraform apply -var-file=../../config/project.json -auto-approve &> /dev/null
3333
continue-on-error: true
3434
- name: Set up Node (16)
3535
uses: actions/setup-node@v3

.gitignore

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,4 +97,4 @@ toc/
9797
.terraform/*
9898
.terraform.lock.hcl
9999
*.tfstate
100-
*.tfstate.*
100+
*.tfstate.*

integration/firestore/gulpfile.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ function copyTests() {
4242
.src(
4343
[
4444
testBase + '/integration/api/*.ts',
45+
testBase + '/integration/util/composite_index_test_helper.ts',
4546
testBase + '/integration/util/events_accumulator.ts',
4647
testBase + '/integration/util/helpers.ts',
4748
testBase + '/integration/util/settings.ts',

packages/firestore/src/api.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -225,6 +225,7 @@ export { FieldPath as _FieldPath } from './model/path';
225225
export type { ResourcePath as _ResourcePath } from './model/path';
226226
export { ByteString as _ByteString } from './util/byte_string';
227227
export { logWarn as _logWarn } from './util/log';
228+
export { AutoId as _AutoId } from './util/misc';
228229
export type {
229230
AuthTokenFactory,
230231
FirstPartyCredentialsSettings

packages/firestore/src/util/misc.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,12 @@ export interface Indexable {
2424
[k: string]: unknown;
2525
}
2626

27+
/**
28+
* A utility class for generating unique alphanumeric IDs of a specified length.
29+
*
30+
* @internal
31+
* Exported internally for testing purposes.
32+
*/
2733
export class AutoId {
2834
static newId(): string {
2935
// Alphanumeric characters

packages/firestore/test/integration/api/README.md

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,3 +9,21 @@ put in test/integration/api_internal instead.
99
The line "import * as firebaseExport from '../util/firebase_export';" is
1010
replaced via the gulpfile in 'integration/firestore' and should not be
1111
modified.
12+
13+
14+
## Testing composite index query against production
15+
16+
### Setting Up the Environment:
17+
1. Create a `project.json` file in the `firebase-js-sdk/config` directory. This file should contain your target Firebase project's configuration.
18+
2. If not already logged in, authenticate with your Google Cloud Platform (GCP) account using `gcloud auth application-default login`. You can check your logged-in accounts by running `gcloud auth list`.
19+
3. Navigate to the `firebase-js-sdk/packages/firestore` directory, run:
20+
```
21+
terraform init
22+
terraform apply -var-file=../../config/project.json -auto-approve
23+
```
24+
Note: If the index creation encounters issues, such as concurrent operations, consider running the index creation process again. Error messages indicating that indexes have already been created can be safely disregarded.
25+
26+
27+
### Adding new composite index query tests
28+
1. To create a new composite index for local development, click on the provided link in the test error message, which will direct you to the Firebase Console.
29+
2. Add the newly created composite index to the `firestore_index_config.tf` file. The "__name__" field is not required to be explicitly added to the file, as the index creation will auto complete it on behalf.

packages/firestore/test/integration/api/composite_index_query.test.ts

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -33,12 +33,10 @@ import { apiDescribe } from '../util/helpers';
3333
* and setting test documents and running queries with ease, ensuring proper data
3434
* isolation and query construction.
3535
*
36-
* Please remember to update the main index configuration file (firestore_index_config.tf)
37-
* with any new composite indexes needed for the tests. This ensures synchronization with
38-
* other testing environments, including CI. You can generate the required index link by
39-
* clicking on the Firebase console link in the error message while running tests locally.
36+
* To get started, please refer to the instructions provided in the README file. This will guide you
37+
* through setting up your local testing environment and updating the Terraform configuration with
38+
* any new composite indexes required for your testing scenarios.
4039
*/
41-
4240
apiDescribe('Composite Index Queries', persistence => {
4341
// OR Query tests only run when the SDK's local cache is configured to use
4442
// LRU garbage collection (rather than eager garbage collection) because

packages/firestore/test/integration/util/composite_index_test_helper.ts

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -15,10 +15,6 @@
1515
* limitations under the License.
1616
*/
1717

18-
import { and } from '../../../src/lite-api/query';
19-
import { AutoId } from '../../../src/util/misc';
20-
import { field } from '../../util/helpers';
21-
2218
import {
2319
query as internalQuery,
2420
CollectionReference,
@@ -41,7 +37,10 @@ import {
4137
getDocs as getDocuments,
4238
QuerySnapshot,
4339
deleteDoc as deleteDocument,
44-
doc
40+
doc,
41+
and,
42+
_AutoId,
43+
_FieldPath
4544
} from './firebase_export';
4645
import {
4746
batchCommitDocsToCollection,
@@ -72,7 +71,7 @@ export class CompositeIndexTestHelper {
7271
// Creates a new instance of the CompositeIndexTestHelper class, with a unique test
7372
// identifier for data isolation.
7473
constructor() {
75-
this.testId = 'test-id-' + AutoId.newId();
74+
this.testId = 'test-id-' + _AutoId.newId();
7675
}
7776

7877
// Runs a test with specified documents in the COMPOSITE_INDEX_TEST_COLLECTION.
@@ -113,8 +112,8 @@ export class CompositeIndexTestHelper {
113112

114113
// Remove test-specific fields from a document, including the testId and expiration date.
115114
private removeTestSpecificFieldsFromDoc(doc: DocumentData): void {
116-
doc._document?.data?.delete(field(this.TTL_FIELD));
117-
doc._document?.data?.delete(field(this.TEST_ID_FIELD));
115+
doc._document?.data?.delete(new _FieldPath([this.TEST_ID_FIELD]));
116+
doc._document?.data?.delete(new _FieldPath([this.TEST_ID_FIELD]));
118117
}
119118

120119
// Helper method to hash document keys and add test-specific fields for the provided documents.

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

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,6 @@
1818
import { isIndexedDBAvailable } from '@firebase/util';
1919
import { expect } from 'chai';
2020

21-
import { AutoId } from '../../../src/util/misc';
22-
2321
import {
2422
clearIndexedDbPersistence,
2523
collection,
@@ -45,7 +43,8 @@ import {
4543
writeBatch,
4644
Query,
4745
getDocsFromServer,
48-
getDocsFromCache
46+
getDocsFromCache,
47+
_AutoId
4948
} from './firebase_export';
5049
import {
5150
ALT_PROJECT_ID,
@@ -448,7 +447,7 @@ export function withTestCollectionSettings<T>(
448447
docs: { [key: string]: DocumentData },
449448
fn: (collection: CollectionReference, db: Firestore) => Promise<T>
450449
): Promise<T> {
451-
const collectionId = AutoId.newId();
450+
const collectionId = _AutoId.newId();
452451
return batchCommitDocsToCollection(
453452
persistence,
454453
settings,

0 commit comments

Comments
 (0)