Skip to content

Commit 46d562d

Browse files
feat(fs): preferRest app option for Firestore (#1901)
* feat: preferRest option for Firestore * test: fix unit test * docs: pr feedback: update comment * fix: PR feedback * docs: PR feedback
1 parent 85cec55 commit 46d562d

File tree

6 files changed

+178
-10
lines changed

6 files changed

+178
-10
lines changed

etc/firebase-admin.firestore.api.md

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,11 @@ export { Firestore }
8282

8383
export { FirestoreDataConverter }
8484

85+
// @public
86+
export interface FirestoreSettings {
87+
preferRest?: boolean;
88+
}
89+
8590
export { GeoPoint }
8691

8792
// @public
@@ -94,6 +99,9 @@ export function getFirestore(app: App): Firestore;
9499

95100
export { GrpcStatus }
96101

102+
// @public
103+
export function initializeFirestore(app: App, settings?: FirestoreSettings): Firestore;
104+
97105
export { NestedUpdateFields }
98106

99107
export { OrderByDirection }

src/firestore/firestore-internal.ts

Lines changed: 53 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -23,24 +23,67 @@ import * as validator from '../utils/validator';
2323
import * as utils from '../utils/index';
2424
import { App } from '../app';
2525

26+
/**
27+
* Settings to pass to the Firestore constructor.
28+
*
29+
* @public
30+
*/
31+
export interface FirestoreSettings {
32+
/**
33+
* Use HTTP/1.1 REST transport where possible.
34+
*
35+
* `preferRest` will force the use of HTTP/1.1 REST transport until a method
36+
* that requires gRPC is called. When a method requires gRPC, this Firestore
37+
* client will load dependent gRPC libraries and then use gRPC transport for
38+
* all communication from that point forward. Currently the only operation
39+
* that requires gRPC is creating a snapshot listener using `onSnapshot()`.
40+
*
41+
* @defaultValue `undefined`
42+
*/
43+
preferRest?: boolean;
44+
}
45+
2646
export class FirestoreService {
2747

2848
private readonly appInternal: App;
2949
private readonly databases: Map<string, Firestore> = new Map();
50+
private readonly firestoreSettings: Map<string, FirestoreSettings> = new Map();
3051

3152
constructor(app: App) {
3253
this.appInternal = app;
3354
}
3455

35-
getDatabase(databaseId: string): Firestore {
56+
getDatabase(databaseId: string, settings?: FirestoreSettings): Firestore {
57+
settings ??= {};
3658
let database = this.databases.get(databaseId);
3759
if (database === undefined) {
38-
database = initFirestore(this.app, databaseId);
60+
database = initFirestore(this.app, databaseId, settings);
3961
this.databases.set(databaseId, database);
62+
this.firestoreSettings.set(databaseId, settings);
63+
} else {
64+
if (!this.checkIfSameSettings(databaseId, settings)) {
65+
throw new FirebaseFirestoreError({
66+
code: 'failed-precondition',
67+
message: 'initializeFirestore() has already been called with ' +
68+
'different options. To avoid this error, call initializeFirestore() with the ' +
69+
'same options as when it was originally called, or call getFirestore() to return the' +
70+
' already initialized instance.'
71+
});
72+
}
4073
}
4174
return database;
4275
}
4376

77+
private checkIfSameSettings(databaseId: string, firestoreSettings: FirestoreSettings): boolean {
78+
// If we start passing more settings to Firestore constructor,
79+
// replace this with deep equality check.
80+
const existingSettings = this.firestoreSettings.get(databaseId);
81+
if (!existingSettings) {
82+
return true;
83+
}
84+
return (existingSettings.preferRest === firestoreSettings.preferRest);
85+
}
86+
4487
/**
4588
* Returns the app associated with this Storage instance.
4689
*
@@ -51,7 +94,7 @@ export class FirestoreService {
5194
}
5295
}
5396

54-
export function getFirestoreOptions(app: App): Settings {
97+
export function getFirestoreOptions(app: App, firestoreSettings?: FirestoreSettings): Settings {
5598
if (!validator.isNonNullObject(app) || !('options' in app)) {
5699
throw new FirebaseFirestoreError({
57100
code: 'invalid-argument',
@@ -63,6 +106,7 @@ export function getFirestoreOptions(app: App): Settings {
63106
const credential = app.options.credential;
64107
// eslint-disable-next-line @typescript-eslint/no-var-requires
65108
const { version: firebaseVersion } = require('../../package.json');
109+
const preferRest = firestoreSettings?.preferRest;
66110
if (credential instanceof ServiceAccountCredential) {
67111
return {
68112
credentials: {
@@ -73,12 +117,15 @@ export function getFirestoreOptions(app: App): Settings {
73117
// guaranteed to be available.
74118
projectId: projectId!,
75119
firebaseVersion,
120+
preferRest,
76121
};
77122
} else if (isApplicationDefault(app.options.credential)) {
78123
// Try to use the Google application default credentials.
79124
// If an explicit project ID is not available, let Firestore client discover one from the
80125
// environment. This prevents the users from having to set GOOGLE_CLOUD_PROJECT in GCP runtimes.
81-
return validator.isNonEmptyString(projectId) ? { projectId, firebaseVersion } : { firebaseVersion };
126+
return validator.isNonEmptyString(projectId)
127+
? { projectId, firebaseVersion, preferRest }
128+
: { firebaseVersion, preferRest };
82129
}
83130

84131
throw new FirebaseFirestoreError({
@@ -89,8 +136,8 @@ export function getFirestoreOptions(app: App): Settings {
89136
});
90137
}
91138

92-
function initFirestore(app: App, databaseId: string): Firestore {
93-
const options = getFirestoreOptions(app);
139+
function initFirestore(app: App, databaseId: string, firestoreSettings?: FirestoreSettings): Firestore {
140+
const options = getFirestoreOptions(app, firestoreSettings);
94141
options.databaseId = databaseId;
95142
let firestoreDatabase: typeof Firestore;
96143
try {

src/firestore/index.ts

Lines changed: 49 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@
2323
import { Firestore } from '@google-cloud/firestore';
2424
import { App, getApp } from '../app';
2525
import { FirebaseApp } from '../app/firebase-app';
26-
import { FirestoreService } from './firestore-internal';
26+
import { FirestoreService, FirestoreSettings } from './firestore-internal';
2727
import { DEFAULT_DATABASE_ID } from '@google-cloud/firestore/build/src/path';
2828

2929
export {
@@ -71,6 +71,8 @@ export {
7171
setLogFunction,
7272
} from '@google-cloud/firestore';
7373

74+
export { FirestoreSettings };
75+
7476
/**
7577
* Gets the {@link https://googleapis.dev/nodejs/firestore/latest/Firestore.html | Firestore}
7678
* service for the default app.
@@ -105,7 +107,7 @@ export function getFirestore(): Firestore;
105107
* const otherFirestore = getFirestore(app);
106108
* ```
107109
*
108-
* @param App - whose `Firestore` service to
110+
* @param App - which `Firestore` service to
109111
* return. If not provided, the default `Firestore` service will be returned.
110112
*
111113
* @returns The default {@link https://googleapis.dev/nodejs/firestore/latest/Firestore.html | Firestore}
@@ -139,3 +141,48 @@ export function getFirestore(
139141
'firestore', (app) => new FirestoreService(app));
140142
return firestoreService.getDatabase(databaseId);
141143
}
144+
145+
/**
146+
* Gets the {@link https://googleapis.dev/nodejs/firestore/latest/Firestore.html | Firestore}
147+
* service for the given app, passing extra parameters to its constructor.
148+
*
149+
* @example
150+
* ```javascript
151+
* // Get the Firestore service for a specific app, require HTTP/1.1 REST transport
152+
* const otherFirestore = initializeFirestore(app, {preferRest: true});
153+
* ```
154+
*
155+
* @param App - which `Firestore` service to
156+
* return. If not provided, the default `Firestore` service will be returned.
157+
*
158+
* @param settings - Settings object to be passed to the constructor.
159+
*
160+
* @returns The `Firestore` service associated with the provided app and settings.
161+
*/
162+
export function initializeFirestore(app: App, settings?: FirestoreSettings): Firestore;
163+
164+
/**
165+
* @param app
166+
* @param settings
167+
* @param databaseId
168+
* @internal
169+
*/
170+
export function initializeFirestore(
171+
app: App,
172+
settings: FirestoreSettings,
173+
databaseId: string
174+
): Firestore;
175+
176+
export function initializeFirestore(
177+
app: App,
178+
settings?: FirestoreSettings,
179+
databaseId?: string
180+
): Firestore {
181+
settings ??= {};
182+
databaseId ??= DEFAULT_DATABASE_ID;
183+
const firebaseApp: FirebaseApp = app as FirebaseApp;
184+
const firestoreService = firebaseApp.getOrInitService(
185+
'firestore', (app) => new FirestoreService(app));
186+
187+
return firestoreService.getDatabase(databaseId, settings);
188+
}

test/integration/firestore.spec.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ import { clone } from 'lodash';
2020
import * as admin from '../../lib/index';
2121
import {
2222
DocumentReference, DocumentSnapshot, FieldValue, Firestore, FirestoreDataConverter,
23-
QueryDocumentSnapshot, Timestamp, getFirestore, setLogFunction,
23+
QueryDocumentSnapshot, Timestamp, getFirestore, initializeFirestore, setLogFunction,
2424
} from '../../lib/firestore/index';
2525

2626
chai.should();
@@ -47,6 +47,11 @@ describe('admin.firestore', () => {
4747
expect(firestore).to.not.be.undefined;
4848
});
4949

50+
it('initializeFirestore returns a Firestore client', () => {
51+
const firestore: Firestore = initializeFirestore(admin.app());
52+
expect(firestore).to.not.be.undefined;
53+
});
54+
5055
it('admin.firestore() returns a Firestore client', () => {
5156
const firestore: admin.firestore.Firestore = admin.firestore();
5257
expect(firestore).to.not.be.undefined;

test/unit/firestore/firestore.spec.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -200,4 +200,16 @@ describe('Firestore', () => {
200200
});
201201
});
202202
});
203+
204+
describe('options.preferRest', () => {
205+
it('should not enable preferRest by default', () => {
206+
const options = getFirestoreOptions(mockApp);
207+
expect(options.preferRest).to.be.undefined;
208+
});
209+
210+
it('should enable preferRest if provided', () => {
211+
const options = getFirestoreOptions(mockApp, { preferRest: true });
212+
expect(options.preferRest).to.be.true;
213+
});
214+
});
203215
});

test/unit/firestore/index.spec.ts

Lines changed: 50 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ import * as chaiAsPromised from 'chai-as-promised';
2323

2424
import * as mocks from '../../resources/mocks';
2525
import { App } from '../../../src/app/index';
26-
import { getFirestore, Firestore } from '../../../src/firestore/index';
26+
import { getFirestore, initializeFirestore, Firestore } from '../../../src/firestore/index';
2727
import { DEFAULT_DATABASE_ID } from '@google-cloud/firestore/build/src/path';
2828

2929
chai.should();
@@ -86,4 +86,53 @@ describe('Firestore', () => {
8686
expect(db1).to.not.equal(db2);
8787
});
8888
});
89+
90+
describe('initializeFirestore()', () => {
91+
it('should reject given an invalid credential without project ID', () => {
92+
// Project ID not set in the environment.
93+
delete process.env.GOOGLE_CLOUD_PROJECT;
94+
delete process.env.GCLOUD_PROJECT;
95+
expect(() => initializeFirestore(mockCredentialApp)).to.throw(noProjectIdError);
96+
});
97+
98+
it('should not throw given a valid app', () => {
99+
expect(() => {
100+
return initializeFirestore(mockApp);
101+
}).not.to.throw();
102+
});
103+
104+
it('should return the same instance for a given app instance', () => {
105+
const db1: Firestore = initializeFirestore(mockApp);
106+
const db2: Firestore = initializeFirestore(mockApp, {}, DEFAULT_DATABASE_ID);
107+
expect(db1).to.equal(db2);
108+
});
109+
110+
it('should return the same instance for a given app instance and databaseId', () => {
111+
const db1: Firestore = initializeFirestore(mockApp, {}, 'db');
112+
const db2: Firestore = initializeFirestore(mockApp, {}, 'db');
113+
expect(db1).to.equal(db2);
114+
});
115+
116+
it('should return the different instance for given same app instance, but different databaseId', () => {
117+
const db0: Firestore = initializeFirestore(mockApp, {}, DEFAULT_DATABASE_ID);
118+
const db1: Firestore = initializeFirestore(mockApp, {}, 'db1');
119+
const db2: Firestore = initializeFirestore(mockApp, {}, 'db2');
120+
expect(db0).to.not.equal(db1);
121+
expect(db0).to.not.equal(db2);
122+
expect(db1).to.not.equal(db2);
123+
});
124+
125+
it('getFirestore should return the same instance as initializeFirestore returned earlier', () => {
126+
const db1: Firestore = initializeFirestore(mockApp, {}, 'db');
127+
const db2: Firestore = getFirestore(mockApp, 'db');
128+
expect(db1).to.equal(db2);
129+
});
130+
131+
it('initializeFirestore should not allow create an instance with different settings', () => {
132+
initializeFirestore(mockApp, {}, 'db');
133+
expect(() => {
134+
return initializeFirestore(mockApp, { preferRest: true }, 'db');
135+
}).to.throw(/has already been called with different options/);
136+
});
137+
});
89138
});

0 commit comments

Comments
 (0)