Skip to content

Commit 4d6b79e

Browse files
authored
Internal implementation of multiDb (#1949)
* Internal implementation of multiDb * Fixes from PR review * Fixes from PR review
1 parent d551107 commit 4d6b79e

File tree

5 files changed

+81
-35
lines changed

5 files changed

+81
-35
lines changed

etc/firebase-admin.firestore.api.md

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,10 +83,13 @@ export { FirestoreDataConverter }
8383

8484
export { GeoPoint }
8585

86+
// @public
87+
export function getFirestore(): Firestore;
88+
8689
// Warning: (ae-forgotten-export) The symbol "App" needs to be exported by the entry point index.d.ts
8790
//
8891
// @public
89-
export function getFirestore(app?: App): Firestore;
92+
export function getFirestore(app: App): Firestore;
9093

9194
export { GrpcStatus }
9295

src/firestore/firestore-internal.ts

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -25,14 +25,22 @@ import { App } from '../app';
2525

2626
export class FirestoreService {
2727

28-
private appInternal: App;
29-
private firestoreClient: Firestore;
28+
private readonly appInternal: App;
29+
private readonly databases: Map<string, Firestore> = new Map();
3030

3131
constructor(app: App) {
32-
this.firestoreClient = initFirestore(app);
3332
this.appInternal = app;
3433
}
3534

35+
getDatabase(databaseId: string): Firestore {
36+
let database = this.databases.get(databaseId);
37+
if (database === undefined) {
38+
database = initFirestore(this.app, databaseId);
39+
this.databases.set(databaseId, database);
40+
}
41+
return database;
42+
}
43+
3644
/**
3745
* Returns the app associated with this Storage instance.
3846
*
@@ -41,10 +49,6 @@ export class FirestoreService {
4149
get app(): App {
4250
return this.appInternal;
4351
}
44-
45-
get client(): Firestore {
46-
return this.firestoreClient;
47-
}
4852
}
4953

5054
export function getFirestoreOptions(app: App): Settings {
@@ -85,8 +89,9 @@ export function getFirestoreOptions(app: App): Settings {
8589
});
8690
}
8791

88-
function initFirestore(app: App): Firestore {
92+
function initFirestore(app: App, databaseId: string): Firestore {
8993
const options = getFirestoreOptions(app);
94+
options.databaseId = databaseId;
9095
let firestoreDatabase: typeof Firestore;
9196
try {
9297
// Lazy-load the Firestore implementation here, which in turns loads gRPC.

src/firestore/index.ts

Lines changed: 38 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import { Firestore } from '@google-cloud/firestore';
2424
import { App, getApp } from '../app';
2525
import { FirebaseApp } from '../app/firebase-app';
2626
import { FirestoreService } from './firestore-internal';
27+
import { DEFAULT_DATABASE_ID } from '@google-cloud/firestore/build/src/path';
2728

2829
export {
2930
AddPrefixToKeys,
@@ -71,7 +72,7 @@ export {
7172

7273
/**
7374
* Gets the {@link https://googleapis.dev/nodejs/firestore/latest/Firestore.html | Firestore}
74-
* service for the default app or a given app.
75+
* service for the default app.
7576
*
7677
* `getFirestore()` can be called with no arguments to access the default
7778
* app's `Firestore` service or as `getFirestore(app)` to access the
@@ -82,6 +83,20 @@ export {
8283
* // Get the Firestore service for the default app
8384
* const defaultFirestore = getFirestore();
8485
* ```
86+
87+
* @returns The default {@link https://googleapis.dev/nodejs/firestore/latest/Firestore.html | Firestore}
88+
* service if no app is provided or the `Firestore` service associated with the
89+
* provided app.
90+
*/
91+
export function getFirestore(): Firestore;
92+
93+
/**
94+
* Gets the {@link https://googleapis.dev/nodejs/firestore/latest/Firestore.html | Firestore}
95+
* service for the given app.
96+
*
97+
* `getFirestore()` can be called with no arguments to access the default
98+
* app's `Firestore` service or as `getFirestore(app)` to access the
99+
* `Firestore` service associated with a specific app.
85100
*
86101
* @example
87102
* ```javascript
@@ -96,13 +111,30 @@ export {
96111
* service if no app is provided or the `Firestore` service associated with the
97112
* provided app.
98113
*/
99-
export function getFirestore(app?: App): Firestore {
100-
if (typeof app === 'undefined') {
101-
app = getApp();
102-
}
114+
export function getFirestore(app: App): Firestore;
115+
116+
/**
117+
* @param databaseId
118+
* @internal
119+
*/
120+
export function getFirestore(databaseId: string): Firestore;
121+
122+
/**
123+
* @param app
124+
* @param databaseId
125+
* @internal
126+
*/
127+
export function getFirestore(app: App, databaseId: string): Firestore;
103128

129+
export function getFirestore(
130+
appOrDatabaseId?: App | string,
131+
optionalDatabaseId?: string
132+
): Firestore {
133+
const app: App = typeof appOrDatabaseId === 'object' ? appOrDatabaseId : getApp();
134+
const databaseId =
135+
(typeof appOrDatabaseId === 'string' ? appOrDatabaseId : optionalDatabaseId) || DEFAULT_DATABASE_ID;
104136
const firebaseApp: FirebaseApp = app as FirebaseApp;
105137
const firestoreService = firebaseApp.getOrInitService(
106138
'firestore', (app) => new FirestoreService(app));
107-
return firestoreService.client;
139+
return firestoreService.getDatabase(databaseId);
108140
}

test/unit/firestore/firestore.spec.ts

Lines changed: 9 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import {
2626
ComputeEngineCredential, RefreshTokenCredential
2727
} from '../../../src/app/credential-internal';
2828
import { FirestoreService, getFirestoreOptions } from '../../../src/firestore/firestore-internal';
29+
import { DEFAULT_DATABASE_ID } from '@google-cloud/firestore/build/src/path';
2930

3031
describe('Firestore', () => {
3132
let mockApp: FirebaseApp;
@@ -98,23 +99,25 @@ describe('Firestore', () => {
9899
it(`should throw given invalid app: ${ JSON.stringify(invalidApp) }`, () => {
99100
expect(() => {
100101
const firestoreAny: any = FirestoreService;
101-
return new firestoreAny(invalidApp);
102+
const firestoreService: FirestoreService = new firestoreAny(invalidApp);
103+
return firestoreService.getDatabase(DEFAULT_DATABASE_ID)
102104
}).to.throw('First argument passed to admin.firestore() must be a valid Firebase app instance.');
103105
});
104106
});
105107

106108
it('should throw given no app', () => {
107109
expect(() => {
108110
const firestoreAny: any = FirestoreService;
109-
return new firestoreAny();
111+
const firestoreService: FirestoreService = new firestoreAny();
112+
return firestoreService.getDatabase(DEFAULT_DATABASE_ID)
110113
}).to.throw('First argument passed to admin.firestore() must be a valid Firebase app instance.');
111114
});
112115

113116
it('should throw given an invalid credential with project ID', () => {
114117
// Project ID is read from the environment variable, but the credential is unsupported.
115118
process.env.GOOGLE_CLOUD_PROJECT = 'project_id';
116119
expect(() => {
117-
return new FirestoreService(mockCredentialApp);
120+
return new FirestoreService(mockCredentialApp).getDatabase(DEFAULT_DATABASE_ID);
118121
}).to.throw(invalidCredError);
119122
});
120123

@@ -123,13 +126,13 @@ describe('Firestore', () => {
123126
delete process.env.GOOGLE_CLOUD_PROJECT;
124127
delete process.env.GCLOUD_PROJECT;
125128
expect(() => {
126-
return new FirestoreService(mockCredentialApp);
129+
return new FirestoreService(mockCredentialApp).getDatabase(DEFAULT_DATABASE_ID);
127130
}).to.throw(invalidCredError);
128131
});
129132

130133
it('should not throw given a valid app', () => {
131134
expect(() => {
132-
return new FirestoreService(mockApp);
135+
return new FirestoreService(mockApp).getDatabase(DEFAULT_DATABASE_ID);
133136
}).not.to.throw();
134137
});
135138

@@ -139,7 +142,7 @@ describe('Firestore', () => {
139142
delete process.env.GOOGLE_CLOUD_PROJECT;
140143
delete process.env.GCLOUD_PROJECT;
141144
expect(() => {
142-
return new FirestoreService(config.app);
145+
return new FirestoreService(config.app).getDatabase(DEFAULT_DATABASE_ID);
143146
}).not.to.throw();
144147
});
145148
});
@@ -158,19 +161,6 @@ describe('Firestore', () => {
158161
});
159162
});
160163

161-
describe('client', () => {
162-
it('returns the client from the constructor', () => {
163-
// We expect referential equality here
164-
expect(firestore.client).to.not.be.null;
165-
});
166-
167-
it('is read-only', () => {
168-
expect(() => {
169-
firestore.client = mockApp;
170-
}).to.throw('Cannot set property client of #<FirestoreService> which has only a getter');
171-
});
172-
});
173-
174164
describe('options.projectId', () => {
175165
it('should return a string when project ID is present in credential', () => {
176166
const options = getFirestoreOptions(mockApp);

test/unit/firestore/index.spec.ts

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import * as chaiAsPromised from 'chai-as-promised';
2424
import * as mocks from '../../resources/mocks';
2525
import { App } from '../../../src/app/index';
2626
import { getFirestore, Firestore } from '../../../src/firestore/index';
27+
import { DEFAULT_DATABASE_ID } from '@google-cloud/firestore/build/src/path';
2728

2829
chai.should();
2930
chai.use(sinonChai);
@@ -66,8 +67,23 @@ describe('Firestore', () => {
6667

6768
it('should return the same instance for a given app instance', () => {
6869
const db1: Firestore = getFirestore(mockApp);
69-
const db2: Firestore = getFirestore(mockApp);
70+
const db2: Firestore = getFirestore(mockApp, DEFAULT_DATABASE_ID);
7071
expect(db1).to.equal(db2);
7172
});
73+
74+
it('should return the same instance for a given app instance and databaseId', () => {
75+
const db1: Firestore = getFirestore(mockApp, 'db');
76+
const db2: Firestore = getFirestore(mockApp, 'db');
77+
expect(db1).to.equal(db2);
78+
});
79+
80+
it('should return the different instance for given same app instance, but different databaseId', () => {
81+
const db0: Firestore = getFirestore(mockApp, DEFAULT_DATABASE_ID);
82+
const db1: Firestore = getFirestore(mockApp, 'db1');
83+
const db2: Firestore = getFirestore(mockApp, 'db2');
84+
expect(db0).to.not.equal(db1);
85+
expect(db0).to.not.equal(db2);
86+
expect(db1).to.not.equal(db2);
87+
});
7288
});
7389
});

0 commit comments

Comments
 (0)