diff --git a/src/firestore/firestore-internal.ts b/src/firestore/firestore-internal.ts index e366c10318..148d61e9f2 100644 --- a/src/firestore/firestore-internal.ts +++ b/src/firestore/firestore-internal.ts @@ -53,35 +53,43 @@ export class FirestoreService { this.appInternal = app; } - getDatabase(databaseId: string, settings?: FirestoreSettings): Firestore { - settings ??= {}; + initializeDatabase(databaseId: string, settings: FirestoreSettings): Firestore { + const existingInstance = this.databases.get(databaseId); + if (existingInstance) { + const initialSettings = this.firestoreSettings.get(databaseId) ?? {}; + if (this.checkIfSameSettings(settings, initialSettings)) { + return existingInstance; + } + 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.' + }); + } + const newInstance = initFirestore(this.app, databaseId, settings); + this.databases.set(databaseId, newInstance); + this.firestoreSettings.set(databaseId, settings); + return newInstance; + } + + getDatabase(databaseId: string): Firestore { let database = this.databases.get(databaseId); if (database === undefined) { - database = initFirestore(this.app, databaseId, settings); + database = initFirestore(this.app, databaseId, {}); this.databases.set(databaseId, database); - this.firestoreSettings.set(databaseId, settings); - } else { - if (!this.checkIfSameSettings(databaseId, 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.' - }); - } + this.firestoreSettings.set(databaseId, {}); } return database; } - private checkIfSameSettings(databaseId: string, firestoreSettings: FirestoreSettings): boolean { + private checkIfSameSettings(settingsA: FirestoreSettings, settingsB: FirestoreSettings): boolean { + const a = settingsA ?? {}; + const b = settingsB ?? {}; // If we start passing more settings to Firestore constructor, // replace this with deep equality check. - const existingSettings = this.firestoreSettings.get(databaseId); - if (!existingSettings) { - return true; - } - return (existingSettings.preferRest === firestoreSettings.preferRest); + return (a.preferRest === b.preferRest); } /** diff --git a/src/firestore/index.ts b/src/firestore/index.ts index 3a13deca63..661d9fe9bc 100644 --- a/src/firestore/index.ts +++ b/src/firestore/index.ts @@ -184,5 +184,5 @@ export function initializeFirestore( const firestoreService = firebaseApp.getOrInitService( 'firestore', (app) => new FirestoreService(app)); - return firestoreService.getDatabase(databaseId, settings); + return firestoreService.initializeDatabase(databaseId, settings); } diff --git a/test/unit/firestore/index.spec.ts b/test/unit/firestore/index.spec.ts index fc829318ca..11c5c24936 100644 --- a/test/unit/firestore/index.spec.ts +++ b/test/unit/firestore/index.spec.ts @@ -33,7 +33,8 @@ chai.use(chaiAsPromised); const expect = chai.expect; describe('Firestore', () => { - let mockApp: App; + let mockAppOne: App; + let mockAppTwo: App; let mockCredentialApp: App; const noProjectIdError = 'Failed to initialize Google Cloud Firestore client with the ' @@ -41,7 +42,8 @@ describe('Firestore', () => { + 'application default credentials to use Cloud Firestore API.'; beforeEach(() => { - mockApp = mocks.app(); + mockAppOne = mocks.app(); + mockAppTwo = mocks.app(); mockCredentialApp = mocks.mockCredentialApp(); }); @@ -61,26 +63,26 @@ describe('Firestore', () => { it('should not throw given a valid app', () => { expect(() => { - return getFirestore(mockApp); + return getFirestore(mockAppOne); }).not.to.throw(); }); it('should return the same instance for a given app instance', () => { - const db1: Firestore = getFirestore(mockApp); - const db2: Firestore = getFirestore(mockApp, DEFAULT_DATABASE_ID); + const db1: Firestore = getFirestore(mockAppOne); + const db2: Firestore = getFirestore(mockAppOne, DEFAULT_DATABASE_ID); expect(db1).to.equal(db2); }); it('should return the same instance for a given app instance and databaseId', () => { - const db1: Firestore = getFirestore(mockApp, 'db'); - const db2: Firestore = getFirestore(mockApp, 'db'); + const db1: Firestore = getFirestore(mockAppOne, 'db'); + const db2: Firestore = getFirestore(mockAppOne, 'db'); expect(db1).to.equal(db2); }); it('should return the different instance for given same app instance, but different databaseId', () => { - const db0: Firestore = getFirestore(mockApp, DEFAULT_DATABASE_ID); - const db1: Firestore = getFirestore(mockApp, 'db1'); - const db2: Firestore = getFirestore(mockApp, 'db2'); + const db0: Firestore = getFirestore(mockAppOne, DEFAULT_DATABASE_ID); + const db1: Firestore = getFirestore(mockAppOne, 'db1'); + const db2: Firestore = getFirestore(mockAppOne, 'db2'); expect(db0).to.not.equal(db1); expect(db0).to.not.equal(db2); expect(db1).to.not.equal(db2); @@ -97,41 +99,65 @@ describe('Firestore', () => { it('should not throw given a valid app', () => { expect(() => { - return initializeFirestore(mockApp); + return initializeFirestore(mockAppOne); }).not.to.throw(); }); it('should return the same instance for a given app instance', () => { - const db1: Firestore = initializeFirestore(mockApp); - const db2: Firestore = initializeFirestore(mockApp, {}, DEFAULT_DATABASE_ID); + const db1: Firestore = initializeFirestore(mockAppOne); + const db2: Firestore = initializeFirestore(mockAppOne, {}, DEFAULT_DATABASE_ID); + + const db3: Firestore = initializeFirestore(mockAppTwo, { preferRest: true }); + const db4: Firestore = initializeFirestore(mockAppTwo, { preferRest: true }, DEFAULT_DATABASE_ID); + expect(db1).to.equal(db2); + expect(db3).to.equal(db4); }); it('should return the same instance for a given app instance and databaseId', () => { - const db1: Firestore = initializeFirestore(mockApp, {}, 'db'); - const db2: Firestore = initializeFirestore(mockApp, {}, 'db'); + const db1: Firestore = initializeFirestore(mockAppOne, {}, 'db'); + const db2: Firestore = initializeFirestore(mockAppOne, {}, 'db'); + + const db3: Firestore = initializeFirestore(mockAppTwo, { preferRest: true }, 'db'); + const db4: Firestore = initializeFirestore(mockAppTwo, { preferRest: true }, 'db'); + expect(db1).to.equal(db2); + expect(db3).to.equal(db4); }); - it('should return the different instance for given same app instance, but different databaseId', () => { - const db0: Firestore = initializeFirestore(mockApp, {}, DEFAULT_DATABASE_ID); - const db1: Firestore = initializeFirestore(mockApp, {}, 'db1'); - const db2: Firestore = initializeFirestore(mockApp, {}, 'db2'); + it('should return a different instance for given same app instance, but different databaseId', () => { + const db0: Firestore = initializeFirestore(mockAppOne, {}, DEFAULT_DATABASE_ID); + const db1: Firestore = initializeFirestore(mockAppOne, {}, 'db1'); + const db2: Firestore = initializeFirestore(mockAppOne, {}, 'db2'); + + const db3: Firestore = initializeFirestore(mockAppTwo, { preferRest: true }, DEFAULT_DATABASE_ID); + const db4: Firestore = initializeFirestore(mockAppTwo, { preferRest: true }, 'db1'); + const db5: Firestore = initializeFirestore(mockAppTwo, { preferRest: true }, 'db2'); + expect(db0).to.not.equal(db1); expect(db0).to.not.equal(db2); expect(db1).to.not.equal(db2); + + expect(db3).to.not.equal(db4); + expect(db3).to.not.equal(db5); + expect(db4).to.not.equal(db5); }); it('getFirestore should return the same instance as initializeFirestore returned earlier', () => { - const db1: Firestore = initializeFirestore(mockApp, {}, 'db'); - const db2: Firestore = getFirestore(mockApp, 'db'); + const db1: Firestore = initializeFirestore(mockAppOne, {}, 'db'); + const db2: Firestore = getFirestore(mockAppOne, 'db'); + + const db3: Firestore = initializeFirestore(mockAppTwo, { preferRest: true }); + const db4: Firestore = getFirestore(mockAppTwo); + expect(db1).to.equal(db2); + expect(db3).to.equal(db4); }); it('initializeFirestore should not allow create an instance with different settings', () => { - initializeFirestore(mockApp, {}, 'db'); + initializeFirestore(mockAppTwo, {}, 'db'); expect(() => { - return initializeFirestore(mockApp, { preferRest: true }, 'db'); + return initializeFirestore(mockAppTwo, { preferRest: true }, 'db'); }).to.throw(/has already been called with different options/); }); });