Skip to content

Commit

Permalink
fix: Removing FirebaseServiceInterface and FirebaseServiceInternalsIn…
Browse files Browse the repository at this point in the history 
…terface (#1128)

* fix: Removing FirebaseService and FirebaseServiceInterface internal APIs

* chore: Added unit tests for service caching behavior

* fix: Using equal instead of deep.equal for reference tests
  • Loading branch information
@hiranya911
hiranya911 authored Jan 11, 2021
1 parent b60191e commit 2f6da89
Show file tree
Hide file tree
Showing 19 changed files with 135 additions and 505 deletions.
21 changes: 1 addition & 20 deletions src/auth/auth.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ import {
AbstractAuthRequestHandler, AuthRequestHandler, TenantAwareAuthRequestHandler, useEmulator,
} from './auth-api-request';
import { AuthClientErrorCode, FirebaseAuthError, ErrorInfo } from '../utils/error';
import { FirebaseServiceInterface, FirebaseServiceInternalsInterface } from '../firebase-service';
import * as utils from '../utils/index';
import * as validator from '../utils/validator';
import { auth } from './index';
Expand Down Expand Up @@ -59,22 +58,6 @@ import BaseAuthInterface = auth.BaseAuth;
import AuthInterface = auth.Auth;
import TenantAwareAuthInterface = auth.TenantAwareAuth;

/**
* Internals of an Auth instance.
*/
class AuthInternals implements FirebaseServiceInternalsInterface {
/**
* Deletes the service and its associated resources.
*
* @return {Promise<()>} An empty Promise that will be fulfilled when the service is deleted.
*/
public delete(): Promise<void> {
// There are no resources to clean up
return Promise.resolve(undefined);
}
}


/**
* Base Auth class. Mainly used for user management APIs.
*/
Expand Down Expand Up @@ -820,10 +803,8 @@ export class TenantAwareAuth
* Auth service bound to the provided app.
* An Auth instance can have multiple tenants.
*/
export class Auth extends BaseAuth<AuthRequestHandler>
implements FirebaseServiceInterface, AuthInterface {
export class Auth extends BaseAuth<AuthRequestHandler> implements AuthInterface {

public INTERNAL: AuthInternals = new AuthInternals();
private readonly tenantManager_: TenantManager;
private readonly app_: FirebaseApp;

Expand Down
48 changes: 20 additions & 28 deletions src/database/database-internal.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ import * as path from 'path';

import { FirebaseApp } from '../firebase-app';
import { FirebaseDatabaseError, AppErrorCodes, FirebaseAppError } from '../utils/error';
import { FirebaseServiceInterface, FirebaseServiceInternalsInterface } from '../firebase-service';
import { Database as DatabaseImpl } from '@firebase/database';
import { database } from './index';

Expand All @@ -29,35 +28,14 @@ import { getSdkVersion } from '../utils/index';

import Database = database.Database;

/**
* Internals of a Database instance.
*/
class DatabaseInternals implements FirebaseServiceInternalsInterface {
export class DatabaseService {

private readonly appInternal: FirebaseApp;

public databases: {
private databases: {
[dbUrl: string]: Database;
} = {};

/**
* Deletes the service and its associated resources.
*
* @return {Promise<()>} An empty Promise that will be fulfilled when the service is deleted.
*/
public delete(): Promise<void> {
for (const dbUrl of Object.keys(this.databases)) {
const db: DatabaseImpl = ((this.databases[dbUrl] as any) as DatabaseImpl);
db.INTERNAL.delete();
}
return Promise.resolve(undefined);
}
}

export class DatabaseService implements FirebaseServiceInterface {

public readonly INTERNAL: DatabaseInternals = new DatabaseInternals();

private readonly appInternal: FirebaseApp;

constructor(app: FirebaseApp) {
if (!validator.isNonNullObject(app) || !('options' in app)) {
throw new FirebaseDatabaseError({
Expand All @@ -68,6 +46,20 @@ export class DatabaseService implements FirebaseServiceInterface {
this.appInternal = app;
}

/**
* @internal
*/
public delete(): Promise<void> {
const promises = [];
for (const dbUrl of Object.keys(this.databases)) {
const db: DatabaseImpl = ((this.databases[dbUrl] as any) as DatabaseImpl);
promises.push(db.INTERNAL.delete());
}
return Promise.all(promises).then(() => {
this.databases = {};
});
}

/**
* Returns the app associated with this DatabaseService instance.
*
Expand All @@ -86,7 +78,7 @@ export class DatabaseService implements FirebaseServiceInterface {
});
}

let db: Database = this.INTERNAL.databases[dbUrl];
let db: Database = this.databases[dbUrl];
if (typeof db === 'undefined') {
const rtdb = require('@firebase/database'); // eslint-disable-line @typescript-eslint/no-var-requires
db = rtdb.initStandalone(this.appInternal, dbUrl, getSdkVersion()).instance;
Expand All @@ -102,7 +94,7 @@ export class DatabaseService implements FirebaseServiceInterface {
return rulesClient.setRules(source);
};

this.INTERNAL.databases[dbUrl] = db;
this.databases[dbUrl] = db;
}
return db;
}
Expand Down
61 changes: 18 additions & 43 deletions src/firebase-app.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,7 @@ import { AppOptions, app } from './firebase-namespace-api';
import { credential, GoogleOAuthAccessToken } from './credential/index';
import { getApplicationDefault } from './credential/credential-internal';
import * as validator from './utils/validator';
import { deepCopy, deepExtend } from './utils/deep-copy';
import { FirebaseServiceInterface } from './firebase-service';
import { deepCopy } from './utils/deep-copy';
import { FirebaseNamespaceInternals } from './firebase-namespace';
import { AppErrorCodes, FirebaseAppError } from './utils/error';

Expand Down Expand Up @@ -238,7 +237,7 @@ export class FirebaseApp implements app.App {

private name_: string;
private options_: AppOptions;
private services_: {[name: string]: FirebaseServiceInterface} = {};
private services_: {[name: string]: unknown} = {};
private isDeleted_ = false;

constructor(options: AppOptions, name: string, private firebaseInternals_: FirebaseNamespaceInternals) {
Expand Down Expand Up @@ -268,11 +267,6 @@ export class FirebaseApp implements app.App {
);
}

Object.keys(firebaseInternals_.serviceFactories).forEach((serviceName) => {
// Defer calling createService() until the service is accessed
(this as {[key: string]: any})[serviceName] = this.getService_.bind(this, serviceName);
});

this.INTERNAL = new FirebaseAppInternals(credential);
}

Expand Down Expand Up @@ -428,51 +422,24 @@ export class FirebaseApp implements app.App {
this.INTERNAL.delete();

return Promise.all(Object.keys(this.services_).map((serviceName) => {
return this.services_[serviceName].INTERNAL.delete();
const service = this.services_[serviceName];
if (isStateful(service)) {
return service.delete();
}
return Promise.resolve();
})).then(() => {
this.services_ = {};
this.isDeleted_ = true;
});
}

private ensureService_<T extends FirebaseServiceInterface>(serviceName: string, initializer: () => T): T {
this.checkDestroyed_();

let service: T;
if (serviceName in this.services_) {
service = this.services_[serviceName] as T;
} else {
service = initializer();
this.services_[serviceName] = service;
}
return service;
}

/**
* Returns the service instance associated with this FirebaseApp instance (creating it on demand
* if needed). This is used for looking up monkeypatched service instances.
*
* @param serviceName The name of the service instance to return.
* @return The service instance with the provided name.
*/
private getService_(serviceName: string): FirebaseServiceInterface {
private ensureService_<T>(serviceName: string, initializer: () => T): T {
this.checkDestroyed_();

if (!(serviceName in this.services_)) {
this.services_[serviceName] = this.firebaseInternals_.serviceFactories[serviceName](
this,
this.extendApp_.bind(this),
);
this.services_[serviceName] = initializer();
}

return this.services_[serviceName];
}

/**
* Callback function used to extend an App instance at the time of service instance creation.
*/
private extendApp_(props: {[prop: string]: any}): void {
deepExtend(this, props);
return this.services_[serviceName] as T;
}

/**
Expand All @@ -487,3 +454,11 @@ export class FirebaseApp implements app.App {
}
}
}

interface StatefulFirebaseService {
delete(): Promise<void>;
}

function isStateful(service: any): service is StatefulFirebaseService {
return typeof service.delete === 'function';
}
86 changes: 2 additions & 84 deletions src/firebase-namespace.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,9 @@

import fs = require('fs');

import { deepExtend } from './utils/deep-copy';
import { AppErrorCodes, FirebaseAppError } from './utils/error';
import { AppOptions, app } from './firebase-namespace-api';
import { AppHook, FirebaseApp } from './firebase-app';
import { FirebaseServiceFactory, FirebaseServiceInterface } from './firebase-service';
import { FirebaseApp } from './firebase-app';
import { cert, refreshToken, applicationDefault } from './credential/credential';
import { getApplicationDefault } from './credential/credential-internal';

Expand Down Expand Up @@ -69,11 +67,8 @@ export interface FirebaseServiceNamespace <T> {
* Internals of a FirebaseNamespace instance.
*/
export class FirebaseNamespaceInternals {
public serviceFactories: {[serviceName: string]: FirebaseServiceFactory} = {};

private apps_: {[appName: string]: App} = {};
private appHooks_: {[service: string]: AppHook} = {};

constructor(public firebase_: {[key: string]: any}) {}

/**
Expand Down Expand Up @@ -118,11 +113,7 @@ export class FirebaseNamespaceInternals {
}

const app = new FirebaseApp(options, appName, this);

this.apps_[appName] = app;

this.callAppHooks_(app, 'create');

return app;
}

Expand Down Expand Up @@ -170,80 +161,7 @@ export class FirebaseNamespaceInternals {
}

const appToRemove = this.app(appName);
this.callAppHooks_(appToRemove, 'delete');
delete this.apps_[appName];
}

/**
* Registers a new service on this Firebase namespace.
*
* @param serviceName The name of the Firebase service to register.
* @param createService A factory method to generate an instance of the Firebase service.
* @param serviceProperties Optional properties to extend this Firebase namespace with.
* @param appHook Optional callback that handles app-related events like app creation and deletion.
* @return The Firebase service's namespace.
*/
public registerService(
serviceName: string,
createService: FirebaseServiceFactory,
serviceProperties?: object,
appHook?: AppHook): FirebaseServiceNamespace<FirebaseServiceInterface> {
let errorMessage;
if (typeof serviceName === 'undefined') {
errorMessage = 'No service name provided. Service name must be a non-empty string.';
} else if (typeof serviceName !== 'string' || serviceName === '') {
errorMessage = `Invalid service name "${serviceName}" provided. Service name must be a non-empty string.`;
} else if (serviceName in this.serviceFactories) {
errorMessage = `Firebase service named "${serviceName}" has already been registered.`;
}

if (typeof errorMessage !== 'undefined') {
throw new FirebaseAppError(
AppErrorCodes.INTERNAL_ERROR,
`INTERNAL ASSERT FAILED: ${errorMessage}`,
);
}

this.serviceFactories[serviceName] = createService;
if (appHook) {
this.appHooks_[serviceName] = appHook;
}

// The service namespace is an accessor function which takes a FirebaseApp instance
// or uses the default app if no FirebaseApp instance is provided
const serviceNamespace: FirebaseServiceNamespace<FirebaseServiceInterface> = (appArg?: App) => {
if (typeof appArg === 'undefined') {
appArg = this.app();
}

// Forward service instance lookup to the FirebaseApp
return (appArg as any)[serviceName]();
};

// ... and a container for service-level properties.
if (serviceProperties !== undefined) {
deepExtend(serviceNamespace, serviceProperties);
}

// Monkey-patch the service namespace onto the Firebase namespace
this.firebase_[serviceName] = serviceNamespace;

return serviceNamespace;
}

/**
* Calls the app hooks corresponding to the provided event name for each service within the
* provided App instance.
*
* @param app The App instance whose app hooks to call.
* @param eventName The event name representing which app hooks to call.
*/
private callAppHooks_(app: App, eventName: string): void {
Object.keys(this.serviceFactories).forEach((serviceName) => {
if (this.appHooks_[serviceName]) {
this.appHooks_[serviceName](eventName, app);
}
});
delete this.apps_[appToRemove.name];
}

/**
Expand Down
43 changes: 0 additions & 43 deletions src/firebase-service.ts
View file

This file was deleted.

Loading

0 comments on commit 2f6da89

Please sign in to comment.