Skip to content

Commit

Permalink
Make initializeApp idempotent (#5207)
Browse files Browse the repository at this point in the history
* Add deepEqual function

* make initializeApp idempotent

* make initializeApp in app-compat idempotent

* Create fair-readers-drum.md

* address comments
  • Loading branch information
Feiyang1 authored Jul 28, 2021
1 parent 4bc015c commit a3cbe71
Show file tree
Hide file tree
Showing 9 changed files with 453 additions and 15 deletions.
5 changes: 5 additions & 0 deletions .changeset/fair-readers-drum.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@firebase/util": minor
---

Added deepEqual for comparing objects
7 changes: 7 additions & 0 deletions packages-exp/app-compat/src/firebaseNamespaceCore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,8 @@ export function createFirebaseNamespaceCore(

/**
* Create a new App instance (name must be unique).
*
* This function is idempotent. It can be called more than once and return the same instance using the same options and config.
*/
function initializeAppCompat(
options: FirebaseOptions,
Expand All @@ -118,6 +120,11 @@ export function createFirebaseNamespaceCore(
options,
rawConfig
) as _FirebaseAppExp;

if (contains(apps, app.name)) {
return apps[app.name];
}

const appCompat = new firebaseAppImpl(app, namespace);
apps[app.name] = appCompat;
return appCompat;
Expand Down
58 changes: 52 additions & 6 deletions packages-exp/app-compat/test/firebaseAppCompat.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -346,15 +346,61 @@ function firebaseAppTests(
expect(firebase.apps.length).to.eq(2);
});

it('duplicate DEFAULT initialize is an error.', () => {
firebase.initializeApp({});
expect(() => firebase.initializeApp({})).throws(/\[DEFAULT\].*exists/i);
it('initializeApp can be called more than once and returns the same instance if the options and config are the same', () => {
const app = firebase.initializeApp(
{
apiKey: 'test1'
},
{ automaticDataCollectionEnabled: true }
);
expect(
firebase.initializeApp(
{
apiKey: 'test1'
},
{ automaticDataCollectionEnabled: true }
)
).to.equal(app);
});

it('duplicate DEFAULT initialize with different options is an error.', () => {
firebase.initializeApp({ apiKey: 'key1' });
expect(() => firebase.initializeApp({ apiKey: 'key2' })).throws(
/\[DEFAULT\].*exists/i
);
});

it('duplicate named App initialize with different options is an error.', () => {
firebase.initializeApp({ apiKey: 'key1', appId: 'id' }, 'abc');
expect(() => firebase.initializeApp({ apiKey: 'key1' }, 'abc')).throws(
/'abc'.*exists/i
);
});

it('duplicate named App initialize is an error.', () => {
firebase.initializeApp({}, 'abc');
it('duplicate DEFAULT initialize with different config is an error.', () => {
firebase.initializeApp(
{ apiKey: 'key1' },
{ automaticDataCollectionEnabled: true }
);
expect(() =>
firebase.initializeApp(
{ apiKey: 'key1' },
{ automaticDataCollectionEnabled: false }
)
).throws(/\[DEFAULT\].*exists/i);
});

expect(() => firebase.initializeApp({}, 'abc')).throws(/'abc'.*exists/i);
it('duplicate named App initialize with different config is an error.', () => {
firebase.initializeApp(
{ apiKey: 'key1' },
{ name: 'abc', automaticDataCollectionEnabled: true }
);
expect(() =>
firebase.initializeApp(
{ apiKey: 'key1' },
{ name: 'abc', automaticDataCollectionEnabled: false }
)
).throws(/'abc'.*exists/i);
});

it('automaticDataCollectionEnabled is `false` by default', () => {
Expand Down
82 changes: 76 additions & 6 deletions packages-exp/app-exp/src/api.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,15 +74,85 @@ describe('API tests', () => {
expect(app2.name).to.equal(appName);
});

it('throws when creating duplicate DEDAULT Apps', () => {
initializeApp({});
expect(() => initializeApp({})).throws(/\[DEFAULT\].*exists/i);
it('can be called more than once and returns the same instance if the options and config are the same', () => {
const app = initializeApp(
{
apiKey: 'test1'
},
{ automaticDataCollectionEnabled: true }
);
expect(
initializeApp(
{
apiKey: 'test1'
},
{ automaticDataCollectionEnabled: true }
)
).to.equal(app);
});

it('throws when creating duplicate DEDAULT Apps with different options', () => {
initializeApp({
apiKey: 'test1'
});
expect(() =>
initializeApp({
apiKey: 'test2'
})
).throws(/\[DEFAULT\].*exists/i);
});

it('throws when creating duplicate named Apps with different options', () => {
const appName = 'MyApp';
initializeApp(
{
apiKey: 'test1'
},
appName
);
expect(() =>
initializeApp(
{
apiKey: 'test2'
},
appName
)
).throws(/'MyApp'.*exists/i);
});

it('throws when creating duplicate DEDAULT Apps with different config values', () => {
initializeApp(
{
apiKey: 'test1'
},
{ automaticDataCollectionEnabled: true }
);
expect(() =>
initializeApp(
{
apiKey: 'test1'
},
{ automaticDataCollectionEnabled: false }
)
).throws(/\[DEFAULT\].*exists/i);
});

it('throws when creating duplicate named Apps', () => {
it('throws when creating duplicate named Apps with different config values', () => {
const appName = 'MyApp';
initializeApp({}, appName);
expect(() => initializeApp({}, appName)).throws(/'MyApp'.*exists/i);
initializeApp(
{
apiKey: 'test1'
},
{ name: appName, automaticDataCollectionEnabled: true }
);
expect(() =>
initializeApp(
{
apiKey: 'test1'
},
{ name: appName, automaticDataCollectionEnabled: false }
)
).throws(/'MyApp'.*exists/i);
});

it('takes an object as the second parameter to create named App', () => {
Expand Down
14 changes: 12 additions & 2 deletions packages-exp/app-exp/src/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ import {
LogOptions,
setUserLogHandler
} from '@firebase/logger';
import { deepEqual } from '@firebase/util';

/**
* The current SDK version.
Expand Down Expand Up @@ -129,8 +130,17 @@ export function initializeApp(
});
}

if (_apps.has(name)) {
throw ERROR_FACTORY.create(AppError.DUPLICATE_APP, { appName: name });
const existingApp = _apps.get(name) as FirebaseAppImpl;
if (existingApp) {
// return the existing app if options and config deep equal the ones in the existing app.
if (
deepEqual(options, existingApp.options) &&
deepEqual(config, existingApp.config)
) {
return existingApp;
} else {
throw ERROR_FACTORY.create(AppError.DUPLICATE_APP, { appName: name });
}
}

const container = new ComponentContainer(name);
Expand Down
3 changes: 2 additions & 1 deletion packages-exp/app-exp/src/errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,8 @@ const ERRORS: ErrorMap<AppError> = {
"No Firebase App '{$appName}' has been created - " +
'call Firebase App.initializeApp()',
[AppError.BAD_APP_NAME]: "Illegal App name: '{$appName}",
[AppError.DUPLICATE_APP]: "Firebase App named '{$appName}' already exists",
[AppError.DUPLICATE_APP]:
"Firebase App named '{$appName}' already exists with different options or config",
[AppError.APP_DELETED]: "Firebase App named '{$appName}' already deleted",
[AppError.INVALID_APP_ARGUMENT]:
'firebase.{$appName}() takes either no argument or a ' +
Expand Down
13 changes: 13 additions & 0 deletions packages-exp/app-exp/src/firebaseApp.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,13 @@ import { ERROR_FACTORY, AppError } from './errors';
export class FirebaseAppImpl implements FirebaseApp {
private readonly _options: FirebaseOptions;
private readonly _name: string;
/**
* Original config values passed in as a constructor parameter.
* It is only used to compare with another config object to support idempotent initializeApp().
*
* Updating automaticDataCollectionEnabled on the App instance will not change its value in _config.
*/
private readonly _config: Required<FirebaseAppSettings>;
private _automaticDataCollectionEnabled: boolean;
private _isDeleted = false;
private readonly _container: ComponentContainer;
Expand All @@ -40,6 +47,7 @@ export class FirebaseAppImpl implements FirebaseApp {
container: ComponentContainer
) {
this._options = { ...options };
this._config = { ...config };
this._name = config.name;
this._automaticDataCollectionEnabled =
config.automaticDataCollectionEnabled;
Expand Down Expand Up @@ -69,6 +77,11 @@ export class FirebaseAppImpl implements FirebaseApp {
return this._options;
}

get config(): Required<FirebaseAppSettings> {
this.checkDestroyed();
return this._config;
}

get container(): ComponentContainer {
return this._container;
}
Expand Down
38 changes: 38 additions & 0 deletions packages/util/src/obj.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,3 +52,41 @@ export function map<K extends string, V, U>(
}
return res as { [key in K]: U };
}

/**
* Deep equal two objects. Support Arrays and Objects.
*/
export function deepEqual(a: object, b: object): boolean {
if (a === b) {
return true;
}

const aKeys = Object.keys(a);
const bKeys = Object.keys(b);
for (const k of aKeys) {
if (!bKeys.includes(k)) {
return false;
}

const aProp = (a as Record<string, unknown>)[k];
const bProp = (b as Record<string, unknown>)[k];
if (isObject(aProp) && isObject(bProp)) {
if (!deepEqual(aProp, bProp)) {
return false;
}
} else if (aProp !== bProp) {
return false;
}
}

for (const k of bKeys) {
if (!aKeys.includes(k)) {
return false;
}
}
return true;
}

function isObject(thing: unknown): thing is object {
return thing !== null && typeof thing === 'object';
}
Loading

0 comments on commit a3cbe71

Please sign in to comment.