Skip to content

Commit

Permalink
[ServerApp] Mangle the FirebaseServerApp name (#7993)
Browse files Browse the repository at this point in the history 
Mangle the name of the ServerAap based on the hash of the parameters passed in.
In addition, return the same instance of app when the same parameters are used.
  • Loading branch information
@DellaBitta
DellaBitta authored Jan 30, 2024
1 parent 562d619 commit abc9d87
Show file tree
Hide file tree
Showing 4 changed files with 89 additions and 44 deletions.
73 changes: 65 additions & 8 deletions packages/app/src/api.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ import { createTestComponent } from '../test/util';
import { Component, ComponentType } from '@firebase/component';
import { Logger } from '@firebase/logger';
import { FirebaseAppImpl } from './firebaseApp';
import { FirebaseServerAppImpl } from './firebaseServerApp';
import { isBrowser } from '@firebase/util';

declare module '@firebase/component' {
Expand Down Expand Up @@ -184,7 +185,7 @@ describe('API tests', () => {
});

describe('initializeServerApp', () => {
it('creates FirebaseServerApp with options', () => {
it('creates FirebaseServerApp fails in browsers.', () => {
if (isBrowser()) {
const options = {
apiKey: 'APIKEY'
Expand All @@ -196,7 +197,7 @@ describe('API tests', () => {
}
});

it('creates FirebaseServerApp with options', () => {
it('creates FirebaseServerApp with options', async () => {
if (isBrowser()) {
// FirebaseServerApp isn't supported for execution in browser enviornments.
return;
Expand All @@ -211,9 +212,10 @@ describe('API tests', () => {
const app = initializeServerApp(options, serverAppSettings);
expect(app).to.not.equal(null);
expect(app.automaticDataCollectionEnabled).to.be.false;
await deleteApp(app);
});

it('creates FirebaseServerApp with automaticDataCollectionEnabled', () => {
it('creates FirebaseServerApp with automaticDataCollectionEnabled', async () => {
if (isBrowser()) {
// FirebaseServerApp isn't supported for execution in browser enviornments.
return;
Expand All @@ -230,9 +232,10 @@ describe('API tests', () => {
const app = initializeServerApp(options, serverAppSettings);
expect(app).to.not.equal(null);
expect(app.automaticDataCollectionEnabled).to.be.true;
await deleteApp(app);
});

it('creates FirebaseServerApp with releaseOnDeref', () => {
it('creates FirebaseServerApp with releaseOnDeref', async () => {
if (isBrowser()) {
// FirebaseServerApp isn't supported for execution in browser enviornments.
return;
Expand All @@ -247,9 +250,10 @@ describe('API tests', () => {
const app = initializeServerApp(options, serverAppSettings);
expect(app).to.not.equal(null);
expect(app.automaticDataCollectionEnabled).to.be.false;
await deleteApp(app);
});

it('creates FirebaseServerApp with FirebaseApp', () => {
it('creates FirebaseServerApp with FirebaseApp', async () => {
if (isBrowser()) {
// FirebaseServerApp isn't supported for execution in browser enviornments.
return;
Expand All @@ -266,12 +270,65 @@ describe('API tests', () => {
automaticDataCollectionEnabled: false
};

const serverApp = initializeServerApp(standardApp, serverAppSettings);
expect(serverApp).to.not.equal(null);
expect(serverApp.options.apiKey).to.equal('test1');
const app = initializeServerApp(standardApp, serverAppSettings);
expect(app).to.not.equal(null);
expect(app.options.apiKey).to.equal('test1');
await deleteApp(app);
});
});

it('create similar FirebaseServerApps does not return the same object', async () => {
if (isBrowser()) {
// FirebaseServerApp isn't supported for execution in browser enviornments.
return;
}

const options = { apiKey: 'APIKEY' };
const serverAppSettingsOne: FirebaseServerAppSettings = {
automaticDataCollectionEnabled: false,
releaseOnDeref: options
};

const serverAppSettingsTwo: FirebaseServerAppSettings = {
automaticDataCollectionEnabled: false
};

const appOne = initializeServerApp(options, serverAppSettingsOne);
expect(appOne).to.not.equal(null);
expect(appOne.automaticDataCollectionEnabled).to.be.false;
const appTwo = initializeServerApp(options, serverAppSettingsTwo);
expect(appTwo).to.not.equal(null);
expect(appTwo).to.not.equal(appOne);
await deleteApp(appOne);
await deleteApp(appTwo);
});

it('create duplicate FirebaseServerApps returns the same object', async () => {
if (isBrowser()) {
// FirebaseServerApp isn't supported for execution in browser enviornments.
return;
}

const options = { apiKey: 'APIKEY' };
const serverAppSettings: FirebaseServerAppSettings = {
automaticDataCollectionEnabled: false,
releaseOnDeref: options
};

const appOne = initializeServerApp(options, serverAppSettings);
expect(appOne).to.not.equal(null);
expect(appOne.automaticDataCollectionEnabled).to.be.false;
const appTwo = initializeServerApp(options, serverAppSettings);
expect(appTwo).to.not.equal(null);
expect(appTwo).to.equal(appOne);
await deleteApp(appOne);

// TODO: When Reference Counting works, update test. The following line should be false
// until and the app should be deleted a second time.
expect((appOne as FirebaseServerAppImpl).isDeleted).to.be.true;
// await deleteApp(appTwo);
});

describe('getApp', () => {
it('retrieves DEFAULT App', () => {
const app = initializeApp({});
Expand Down
31 changes: 17 additions & 14 deletions packages/app/src/api.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -235,22 +235,30 @@ export function initializeServerApp(
throw ERROR_FACTORY.create(AppError.INVALID_SERVER_APP_ENVIRONMENT);
}

const serverAppSettings: FirebaseServerAppSettings = {
automaticDataCollectionEnabled: false,
..._serverAppConfig
};

let appOptions: FirebaseOptions;
if (_isFirebaseApp(_options)) {
appOptions = _options.options;
} else {
appOptions = _options;
}

// Mangle the ap name based on a hash of the FirebaseServerAppSettings, and FirebaseOptions
// objects and the authIdToken, if provided.
const nameObj = {
authIdToken: _serverAppConfig?.authIdToken,
_serverAppConfig,
...appOptions
};
const hashCode = (s: string): number => {
return [...s].reduce(
(hash, c) => (Math.imul(31, hash) + c.charCodeAt(0)) | 0,
0
);
};

const serverAppSettings: FirebaseServerAppSettings = {
automaticDataCollectionEnabled: false,
..._serverAppConfig
};

if (serverAppSettings.releaseOnDeref !== undefined) {
if (typeof FinalizationRegistry === 'undefined') {
Expand All @@ -261,14 +269,6 @@ export function initializeServerApp(
}
}

// TODO: move this into util.js.
const hashCode = (s: string): number => {
return [...s].reduce(
(hash, c) => (Math.imul(31, hash) + c.charCodeAt(0)) | 0,
0
);
};

const nameString = '' + hashCode(JSON.stringify(nameObj));
const existingApp = _serverApps.get(nameString) as FirebaseServerApp;
if (existingApp) {
Expand All @@ -286,9 +286,12 @@ export function initializeServerApp(
const newApp = new FirebaseServerAppImpl(
appOptions,
serverAppSettings,
nameString,
container
);

_serverApps.set(nameString, newApp);

return newApp;
}

Expand Down
13 changes: 5 additions & 8 deletions packages/app/src/firebaseServerApp.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ describe('FirebaseServerApp', () => {
const firebaseServerAppImpl = new FirebaseServerAppImpl(
options,
serverAppSettings,
'testName',
new ComponentContainer('test')
);

Expand All @@ -55,6 +56,7 @@ describe('FirebaseServerApp', () => {
const firebaseServerAppImpl = new FirebaseServerAppImpl(
options,
serverAppSettings,
'testName',
new ComponentContainer('test')
);

Expand All @@ -75,6 +77,7 @@ describe('FirebaseServerApp', () => {
const firebaseServerAppImpl = new FirebaseServerAppImpl(
options,
serverAppSettings,
'testName',
new ComponentContainer('test')
);

Expand All @@ -96,6 +99,7 @@ describe('FirebaseServerApp', () => {
const app = new FirebaseServerAppImpl(
options,
serverAppSettings,
'testName',
new ComponentContainer('test')
);

Expand All @@ -122,6 +126,7 @@ describe('FirebaseServerApp', () => {
const app = new FirebaseServerAppImpl(
options,
serverAppSettings,
'testName',
new ComponentContainer('test')
);

Expand All @@ -131,13 +136,5 @@ describe('FirebaseServerApp', () => {
expect(() => app.authIdTokenVerified()).throws(
'Firebase Server App has been deleted'
);

expect(() => app.appCheckTokenVerified()).throws(
'Firebase Server App has been deleted'
);

expect(() => app.installationTokenVerified()).throws(
'Firebase Server App has been deleted'
);
});
});
16 changes: 2 additions & 14 deletions packages/app/src/firebaseServerApp.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ export class FirebaseServerAppImpl
constructor(
options: FirebaseOptions | FirebaseAppImpl,
serverConfig: FirebaseServerAppSettings,
name: string,
container: ComponentContainer
) {
// Build configuration parameters for the FirebaseAppImpl base class.
Expand All @@ -46,7 +47,7 @@ export class FirebaseServerAppImpl

// Create the FirebaseAppSettings object for the FirebaseAppImp constructor.
const config: Required<FirebaseAppSettings> = {
name: '',
name,
automaticDataCollectionEnabled
};

Expand All @@ -55,7 +56,6 @@ export class FirebaseServerAppImpl
super(options as FirebaseOptions, config, container);
} else {
const appImpl: FirebaseAppImpl = options as FirebaseAppImpl;

super(appImpl.options, config, container);
}

Expand Down Expand Up @@ -94,18 +94,6 @@ export class FirebaseServerAppImpl
return Promise.resolve();
}

appCheckTokenVerified(): Promise<void> {
this.checkDestroyed();
// TODO
return Promise.resolve();
}

installationTokenVerified(): Promise<void> {
this.checkDestroyed();
// TODO
return Promise.resolve();
}

/**
* This function will throw an Error if the App has already been deleted -
* use before performing API actions on the App.
Expand Down

0 comments on commit abc9d87

Please sign in to comment.