-
Notifications
You must be signed in to change notification settings - Fork 390
feat(fs): preferRest app option for Firestore #1901
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 4 commits
44d2779
b8f6fa1
34453df
624c951
25ee0c3
d9de877
4026721
d534af9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -23,24 +23,46 @@ import * as validator from '../utils/validator'; | |
import * as utils from '../utils/index'; | ||
import { App } from '../app'; | ||
|
||
/** | ||
* Settings to pass to the Firestore constructor. | ||
* | ||
* @public | ||
*/ | ||
export interface FirestoreSettings { | ||
/** | ||
* Use HTTP/1.1 REST transport where possible. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It may be worth clarifying the behavior of this setting.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done! |
||
* | ||
* @defaultValue `undefined` | ||
*/ | ||
preferRest?: boolean; | ||
} | ||
|
||
export class FirestoreService { | ||
|
||
private readonly appInternal: App; | ||
private readonly databases: Map<string, Firestore> = new Map(); | ||
private readonly firestoreSettings: FirestoreSettings; | ||
|
||
constructor(app: App) { | ||
constructor(app: App, firestoreSettings?: FirestoreSettings) { | ||
this.appInternal = app; | ||
this.firestoreSettings = firestoreSettings ?? {}; | ||
} | ||
|
||
getDatabase(databaseId: string): Firestore { | ||
let database = this.databases.get(databaseId); | ||
if (database === undefined) { | ||
database = initFirestore(this.app, databaseId); | ||
database = initFirestore(this.app, databaseId, this.firestoreSettings); | ||
this.databases.set(databaseId, database); | ||
} | ||
return database; | ||
} | ||
|
||
checkIfSameSettings(firestoreSettings: FirestoreSettings): boolean { | ||
// If we start passing more settings to Firestore constructor, | ||
// replace this with deep equality check. | ||
return (firestoreSettings.preferRest === this.firestoreSettings.preferRest); | ||
} | ||
|
||
/** | ||
* Returns the app associated with this Storage instance. | ||
* | ||
|
@@ -51,7 +73,7 @@ export class FirestoreService { | |
} | ||
} | ||
|
||
export function getFirestoreOptions(app: App): Settings { | ||
export function getFirestoreOptions(app: App, firestoreSettings?: FirestoreSettings): Settings { | ||
if (!validator.isNonNullObject(app) || !('options' in app)) { | ||
throw new FirebaseFirestoreError({ | ||
code: 'invalid-argument', | ||
|
@@ -63,6 +85,7 @@ export function getFirestoreOptions(app: App): Settings { | |
const credential = app.options.credential; | ||
// eslint-disable-next-line @typescript-eslint/no-var-requires | ||
const { version: firebaseVersion } = require('../../package.json'); | ||
const preferRest = firestoreSettings?.preferRest; | ||
lahirumaramba marked this conversation as resolved.
Show resolved
Hide resolved
|
||
if (credential instanceof ServiceAccountCredential) { | ||
return { | ||
credentials: { | ||
|
@@ -73,12 +96,15 @@ export function getFirestoreOptions(app: App): Settings { | |
// guaranteed to be available. | ||
projectId: projectId!, | ||
firebaseVersion, | ||
preferRest, | ||
}; | ||
} else if (isApplicationDefault(app.options.credential)) { | ||
// Try to use the Google application default credentials. | ||
// If an explicit project ID is not available, let Firestore client discover one from the | ||
// environment. This prevents the users from having to set GOOGLE_CLOUD_PROJECT in GCP runtimes. | ||
return validator.isNonEmptyString(projectId) ? { projectId, firebaseVersion } : { firebaseVersion }; | ||
return validator.isNonEmptyString(projectId) | ||
? { projectId, firebaseVersion, preferRest } | ||
: { firebaseVersion, preferRest }; | ||
} | ||
|
||
throw new FirebaseFirestoreError({ | ||
|
@@ -89,8 +115,8 @@ export function getFirestoreOptions(app: App): Settings { | |
}); | ||
} | ||
|
||
function initFirestore(app: App, databaseId: string): Firestore { | ||
const options = getFirestoreOptions(app); | ||
function initFirestore(app: App, databaseId: string, firestoreSettings?: FirestoreSettings): Firestore { | ||
const options = getFirestoreOptions(app, firestoreSettings); | ||
options.databaseId = databaseId; | ||
let firestoreDatabase: typeof Firestore; | ||
try { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -23,8 +23,9 @@ | |
import { Firestore } from '@google-cloud/firestore'; | ||
import { App, getApp } from '../app'; | ||
import { FirebaseApp } from '../app/firebase-app'; | ||
import { FirestoreService } from './firestore-internal'; | ||
import { FirestoreService, FirestoreSettings } from './firestore-internal'; | ||
import { DEFAULT_DATABASE_ID } from '@google-cloud/firestore/build/src/path'; | ||
import { FirebaseFirestoreError } from '../utils/error'; | ||
|
||
export { | ||
AddPrefixToKeys, | ||
|
@@ -71,6 +72,8 @@ export { | |
setLogFunction, | ||
} from '@google-cloud/firestore'; | ||
|
||
export { FirestoreSettings }; | ||
|
||
/** | ||
* Gets the {@link https://googleapis.dev/nodejs/firestore/latest/Firestore.html | Firestore} | ||
* service for the default app. | ||
|
@@ -139,3 +142,58 @@ export function getFirestore( | |
'firestore', (app) => new FirestoreService(app)); | ||
return firestoreService.getDatabase(databaseId); | ||
} | ||
|
||
/** | ||
* Gets the {@link https://googleapis.dev/nodejs/firestore/latest/Firestore.html | Firestore} | ||
* service for the given app, passing extra parameters to its constructor. | ||
* | ||
* @example | ||
* ```javascript | ||
* // Get the Firestore service for a specific app, require HTTP/1.1 REST transport | ||
* const otherFirestore = initializeFirestore(app, {preferRest: true}); | ||
* ``` | ||
* | ||
* @param App - whose `Firestore` service to | ||
alexander-fenster marked this conversation as resolved.
Show resolved
Hide resolved
|
||
* return. If not provided, the default `Firestore` service will be returned. | ||
* | ||
* @param settings - Settings object to be passed to the constructor. | ||
* | ||
* @returns The `Firestore` service associated with the provided app. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should this be There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done! |
||
*/ | ||
export function initializeFirestore(app: App, settings?: FirestoreSettings): Firestore; | ||
|
||
/** | ||
* @param app | ||
* @param settings | ||
* @param databaseId | ||
* @internal | ||
*/ | ||
export function initializeFirestore( | ||
app: App, | ||
settings: FirestoreSettings, | ||
databaseId: string | ||
): Firestore; | ||
lahirumaramba marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
export function initializeFirestore( | ||
app: App, | ||
settings?: FirestoreSettings, | ||
databaseId?: string | ||
): Firestore { | ||
settings ??= {}; | ||
databaseId ??= DEFAULT_DATABASE_ID; | ||
const firebaseApp: FirebaseApp = app as FirebaseApp; | ||
const firestoreService = firebaseApp.getOrInitService( | ||
'firestore', (app) => new FirestoreService(app, settings)); | ||
|
||
if (!firestoreService.checkIfSameSettings(settings)) { | ||
throw new FirebaseFirestoreError({ | ||
code: 'failed-precondition', | ||
message: 'initializeFirestore() has already been called with ' + | ||
'different options. To avoid this error, call initializeFirestore() with the ' + | ||
'same options as when it was originally called, or call getFirestore() to return the' + | ||
' already initialized instance.' | ||
}); | ||
} | ||
|
||
return firestoreService.getDatabase(databaseId); | ||
} |
Uh oh!
There was an error while loading. Please reload this page.