Skip to content

Migrate firestore to component framework #2329

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

Merged
merged 5 commits into from
Nov 7, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions packages/firestore/index.node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,11 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/

import firebase from '@firebase/app';
import { FirebaseNamespace } from '@firebase/app-types';
import * as types from '@firebase/firestore-types';
import { configureForFirebase } from './src/platform/config';
import './src/platform_node/node_init';
import { FirebaseNamespace } from '@firebase/app-types';

export function registerFirestore(instance: FirebaseNamespace): void {
configureForFirebase(instance);
Expand Down
3 changes: 1 addition & 2 deletions packages/firestore/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,11 @@
*/

import firebase from '@firebase/app';
import { FirebaseNamespace } from '@firebase/app-types';
import { Firestore } from './src/api/database';
import { configureForFirebase } from './src/platform/config';
import './src/platform_browser/browser_init';

import * as types from '@firebase/firestore-types';
import { FirebaseNamespace } from '@firebase/app-types';

export function registerFirestore(instance: FirebaseNamespace): void {
configureForFirebase(instance);
Expand Down
1 change: 1 addition & 0 deletions packages/firestore/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
"@firebase/webchannel-wrapper": "0.2.29",
"@grpc/proto-loader": "^0.5.0",
"@firebase/util": "0.2.31",
"@firebase/component": "0.1.0",
"grpc": "1.24.1",
"tslib": "1.10.0"
},
Expand Down
81 changes: 52 additions & 29 deletions packages/firestore/src/api/credentials.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,12 @@
* limitations under the License.
*/

import { FirebaseApp } from '@firebase/app-types';
import { _FirebaseApp } from '@firebase/app-types/private';
import { User } from '../auth/user';
import { assert } from '../util/assert';
import { Code, FirestoreError } from '../util/error';
import { FirebaseAuthInternal } from '@firebase/auth-interop-types';
import { Provider } from '@firebase/component';

// TODO(mikelehen): This should be split into multiple files and probably
// moved to an auth/ folder to match other platforms.
Expand Down Expand Up @@ -148,7 +149,9 @@ export class FirebaseCredentialsProvider implements CredentialsProvider {

private forceRefresh = false;

constructor(private readonly app: FirebaseApp) {
private auth: FirebaseAuthInternal | null;

constructor(authProvider: Provider<FirebaseAuthInternal>) {
this.tokenListener = () => {
this.tokenCounter++;
this.currentUser = this.getUser();
Expand All @@ -160,10 +163,26 @@ export class FirebaseCredentialsProvider implements CredentialsProvider {

this.tokenCounter = 0;

// Will fire at least once where we set this.currentUser
(this.app as _FirebaseApp).INTERNAL.addAuthTokenListener(
this.tokenListener
);
this.auth = authProvider.getImmediate({ optional: true });

if (this.auth) {
this.auth.addAuthTokenListener(this.tokenListener!);
} else {
// if auth is not available, invoke tokenListener once with null token
this.tokenListener(null);
authProvider.get().then(
auth => {
this.auth = auth;
if (this.tokenListener) {
// tokenListener can be removed by removeChangeListener()
this.auth.addAuthTokenListener(this.tokenListener);
}
},
() => {
/* this.authProvider.get() never rejects */
}
);
}
}

getToken(): Promise<Token | null> {
Expand All @@ -178,29 +197,32 @@ export class FirebaseCredentialsProvider implements CredentialsProvider {
const initialTokenCounter = this.tokenCounter;
const forceRefresh = this.forceRefresh;
this.forceRefresh = false;
return (this.app as _FirebaseApp).INTERNAL.getToken(forceRefresh).then(
tokenData => {
// Cancel the request since the token changed while the request was
// outstanding so the response is potentially for a previous user (which
// user, we can't be sure).
if (this.tokenCounter !== initialTokenCounter) {
throw new FirestoreError(
Code.ABORTED,
'getToken aborted due to token change.'

if (!this.auth) {
return Promise.resolve(null);
}

return this.auth.getToken(forceRefresh).then(tokenData => {
// Cancel the request since the token changed while the request was
// outstanding so the response is potentially for a previous user (which
// user, we can't be sure).
if (this.tokenCounter !== initialTokenCounter) {
throw new FirestoreError(
Code.ABORTED,
'getToken aborted due to token change.'
);
} else {
if (tokenData) {
assert(
typeof tokenData.accessToken === 'string',
'Invalid tokenData returned from getToken():' + tokenData
);
return new OAuthToken(tokenData.accessToken, this.currentUser);
} else {
if (tokenData) {
assert(
typeof tokenData.accessToken === 'string',
'Invalid tokenData returned from getToken():' + tokenData
);
return new OAuthToken(tokenData.accessToken, this.currentUser);
} else {
return null;
}
return null;
}
}
);
});
}

invalidateToken(): void {
Expand All @@ -223,9 +245,10 @@ export class FirebaseCredentialsProvider implements CredentialsProvider {
this.changeListener !== null,
'removeChangeListener() called when no listener registered'
);
(this.app as _FirebaseApp).INTERNAL.removeAuthTokenListener(
this.tokenListener!
);

if (this.auth) {
this.auth.removeAuthTokenListener(this.tokenListener!);
}
this.tokenListener = null;
this.changeListener = null;
}
Expand All @@ -235,7 +258,7 @@ export class FirebaseCredentialsProvider implements CredentialsProvider {
// This method should only be called in the AuthTokenListener callback
// to guarantee to get the actual user.
private getUser(): User {
const currentUid = (this.app as _FirebaseApp).INTERNAL.getUid();
const currentUid = this.auth && this.auth.getUid();
assert(
currentUid === null || typeof currentUid === 'string',
'Received invalid UID: ' + currentUid
Expand Down
9 changes: 7 additions & 2 deletions packages/firestore/src/api/database.ts
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,8 @@ import {
fieldPathFromArgument,
UserDataConverter
} from './user_data_converter';
import { FirebaseAuthInternal } from '@firebase/auth-interop-types';
import { Provider } from '@firebase/component';

// settings() defaults:
const DEFAULT_HOST = 'firestore.googleapis.com';
Expand Down Expand Up @@ -307,15 +309,18 @@ export class Firestore implements firestore.FirebaseFirestore, FirebaseService {

readonly _dataConverter: UserDataConverter;

constructor(databaseIdOrApp: FirestoreDatabase | FirebaseApp) {
constructor(
databaseIdOrApp: FirestoreDatabase | FirebaseApp,
authProvider: Provider<FirebaseAuthInternal>
) {
if (typeof (databaseIdOrApp as FirebaseApp).options === 'object') {
// This is very likely a Firebase app object
// TODO(b/34177605): Can we somehow use instanceof?
const app = databaseIdOrApp as FirebaseApp;
this._firebaseApp = app;
this._databaseId = Firestore.databaseIdFromApp(app);
this._persistenceKey = app.name;
this._credentials = new FirebaseCredentialsProvider(app);
this._credentials = new FirebaseCredentialsProvider(authProvider);
} else {
const external = databaseIdOrApp as FirestoreDatabase;
if (!external.projectId) {
Expand Down
16 changes: 11 additions & 5 deletions packages/firestore/src/platform/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
* limitations under the License.
*/

import { FirebaseApp, FirebaseNamespace } from '@firebase/app-types';
import { FirebaseNamespace } from '@firebase/app-types';
import { _FirebaseNamespace } from '@firebase/app-types/private';
import { PublicBlob } from '../api/blob';
import {
Expand All @@ -36,6 +36,7 @@ import { PublicFieldValue } from '../api/field_value';
import { GeoPoint } from '../api/geo_point';
import { Timestamp } from '../api/timestamp';
import { shallowCopy } from '../util/obj';
import { Component, ComponentType } from '@firebase/component';

const firestoreNamespace = {
Firestore: PublicFirestore,
Expand All @@ -60,10 +61,15 @@ const firestoreNamespace = {
* Configures Firestore as part of the Firebase SDK by calling registerService.
*/
export function configureForFirebase(firebase: FirebaseNamespace): void {
(firebase as _FirebaseNamespace).INTERNAL.registerService(
'firestore',
(app: FirebaseApp) => new Firestore(app),
shallowCopy(firestoreNamespace)
(firebase as _FirebaseNamespace).INTERNAL.registerComponent(
new Component(
'firestore',
container => {
const app = container.getProvider('app').getImmediate()!;
return new Firestore(app, container.getProvider('auth-internal'));
},
ComponentType.PUBLIC
).setServiceProps(shallowCopy(firestoreNamespace))
);
}

Expand Down
12 changes: 8 additions & 4 deletions packages/firestore/test/util/api_helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,14 +37,18 @@ import { Document } from '../../src/model/document';
import { DocumentSet } from '../../src/model/document_set';
import { JsonObject } from '../../src/model/field_value';
import { doc, key, path as pathFrom } from './helpers';
import { Provider, ComponentContainer } from '@firebase/component';

/**
* A mock Firestore. Will not work for integration test.
*/
export const FIRESTORE = new Firestore({
projectId: 'projectid',
database: 'database'
});
export const FIRESTORE = new Firestore(
{
projectId: 'projectid',
database: 'database'
},
new Provider('auth-interop', new ComponentContainer('default'))
);

export function firestore(): Firestore {
return FIRESTORE;
Expand Down