[ServerApp] RefCount FirebaseServerApps (#8000)

DellaBitta · web-flow · commit 6df26bb6aad9 · 2024-02-02T10:40:55.000-05:00
Update the `FirebaseServerApp` creation to return the same object if an existing object exists with the same configuration. However, the `deleteOnDeref` field is ignored when detecting duplicate apps, since that object reference could vary across multiple SSR rendering passes.

The hope is that a `FirebaseServerApp` instance awaiting deletion from a the `deleteOnDeref` feature maybe be reused if another SSR pass occurs in rapid succession, there by speeding up the SSR code.
diff --git a/packages/app/src/api.test.ts b/packages/app/src/api.test.ts
@@ -213,6 +213,7 @@ describe('API tests', () => {
       expect(app).to.not.equal(null);
       expect(app.automaticDataCollectionEnabled).to.be.false;
       await deleteApp(app);
+      expect((app as FirebaseServerAppImpl).isDeleted).to.be.true;
     });
 
     it('creates FirebaseServerApp with automaticDataCollectionEnabled', async () => {
@@ -233,6 +234,7 @@ describe('API tests', () => {
       expect(app).to.not.equal(null);
       expect(app.automaticDataCollectionEnabled).to.be.true;
       await deleteApp(app);
+      expect((app as FirebaseServerAppImpl).isDeleted).to.be.true;
     });
 
     it('creates FirebaseServerApp with releaseOnDeref', async () => {
@@ -251,6 +253,7 @@ describe('API tests', () => {
       expect(app).to.not.equal(null);
       expect(app.automaticDataCollectionEnabled).to.be.false;
       await deleteApp(app);
+      expect((app as FirebaseServerAppImpl).isDeleted).to.be.true;
     });
 
     it('creates FirebaseServerApp with FirebaseApp', async () => {
@@ -274,6 +277,7 @@ describe('API tests', () => {
       expect(app).to.not.equal(null);
       expect(app.options.apiKey).to.equal('test1');
       await deleteApp(app);
+      expect((app as FirebaseServerAppImpl).isDeleted).to.be.true;
     });
   });
 
@@ -285,8 +289,7 @@ describe('API tests', () => {
 
     const options = { apiKey: 'APIKEY' };
     const serverAppSettingsOne: FirebaseServerAppSettings = {
-      automaticDataCollectionEnabled: false,
-      releaseOnDeref: options
+      automaticDataCollectionEnabled: true
     };
 
     const serverAppSettingsTwo: FirebaseServerAppSettings = {
@@ -295,12 +298,42 @@ describe('API tests', () => {
 
     const appOne = initializeServerApp(options, serverAppSettingsOne);
     expect(appOne).to.not.equal(null);
-    expect(appOne.automaticDataCollectionEnabled).to.be.false;
+    expect(appOne.automaticDataCollectionEnabled).to.be.true;
     const appTwo = initializeServerApp(options, serverAppSettingsTwo);
     expect(appTwo).to.not.equal(null);
+    expect(appTwo.automaticDataCollectionEnabled).to.be.false;
     expect(appTwo).to.not.equal(appOne);
     await deleteApp(appOne);
     await deleteApp(appTwo);
+    expect((appOne as FirebaseServerAppImpl).isDeleted).to.be.true;
+    expect((appTwo as FirebaseServerAppImpl).isDeleted).to.be.true;
+  });
+
+  it('create FirebaseServerApps with varying deleteOnDeref, and they still return same object ', async () => {
+    if (isBrowser()) {
+      // FirebaseServerApp isn't supported for execution in browser enviornments.
+      return;
+    }
+
+    const options = { apiKey: 'APIKEY' };
+    const serverAppSettingsOne: FirebaseServerAppSettings = {
+      automaticDataCollectionEnabled: false
+    };
+
+    const serverAppSettingsTwo: FirebaseServerAppSettings = {
+      automaticDataCollectionEnabled: false,
+      releaseOnDeref: options
+    };
+
+    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.automaticDataCollectionEnabled).to.be.false;
+    expect(appTwo).to.equal(appOne);
+    await deleteApp(appOne);
+    await deleteApp(appTwo);
   });
 
   it('create duplicate FirebaseServerApps returns the same object', async () => {
@@ -322,11 +355,40 @@ describe('API tests', () => {
     expect(appTwo).to.not.equal(null);
     expect(appTwo).to.equal(appOne);
     await deleteApp(appOne);
+    await deleteApp(appTwo);
+  });
 
-    // TODO: When Reference Counting works, update test. The following line should be false
-    // until and the app should be deleted a second time.
+  it('deleting FirebaseServerApps is ref counted', 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 as FirebaseServerAppImpl).refCount).to.equal(1);
+
+    const appTwo = initializeServerApp(options, serverAppSettings);
+    expect(appTwo).to.equal(appOne);
+    expect((appOne as FirebaseServerAppImpl).refCount).to.equal(2);
+    expect((appTwo as FirebaseServerAppImpl).refCount).to.equal(2);
+
+    await deleteApp(appOne);
+    expect((appOne as FirebaseServerAppImpl).refCount).to.equal(1);
+    expect((appTwo as FirebaseServerAppImpl).refCount).to.equal(1);
+    expect((appOne as FirebaseServerAppImpl).isDeleted).to.be.false;
+    expect((appTwo as FirebaseServerAppImpl).isDeleted).to.be.false;
+
+    await deleteApp(appTwo);
+    expect((appOne as FirebaseServerAppImpl).refCount).to.equal(0);
+    expect((appTwo as FirebaseServerAppImpl).refCount).to.equal(0);
     expect((appOne as FirebaseServerAppImpl).isDeleted).to.be.true;
-    // await deleteApp(appTwo);
+    expect((appTwo as FirebaseServerAppImpl).isDeleted).to.be.true;
   });
 
   describe('getApp', () => {
diff --git a/packages/app/src/api.ts b/packages/app/src/api.ts
@@ -235,32 +235,37 @@ export function initializeServerApp(
     throw ERROR_FACTORY.create(AppError.INVALID_SERVER_APP_ENVIRONMENT);
   }
 
+  if (_serverAppConfig.automaticDataCollectionEnabled === undefined) {
+    _serverAppConfig.automaticDataCollectionEnabled = false;
+  }
+
   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.
+  // Build an app name based on a hash of the configuration options.
   const nameObj = {
-    _serverAppConfig,
+    ..._serverAppConfig,
     ...appOptions
   };
+
+  // However, Do not mangle the name based on releaseOnDeref, since it will vary between the
+  // construction of FirebaseServerApp instances. For example, if the object is the request headers.
+  if (nameObj.releaseOnDeref !== undefined) {
+    delete nameObj.releaseOnDeref;
+  }
+
   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 (_serverAppConfig.releaseOnDeref !== undefined) {
     if (typeof FinalizationRegistry === 'undefined') {
       throw ERROR_FACTORY.create(
         AppError.FINALIZATION_REGISTRY_NOT_SUPPORTED,
@@ -272,9 +277,9 @@ export function initializeServerApp(
   const nameString = '' + hashCode(JSON.stringify(nameObj));
   const existingApp = _serverApps.get(nameString) as FirebaseServerApp;
   if (existingApp) {
-    // TODO:
-    //   1: Register a new reference to finalization registry.
-    //   2: Incrememnt reference count.
+    (existingApp as FirebaseServerAppImpl).incRefCount(
+      _serverAppConfig.releaseOnDeref
+    );
     return existingApp;
   }
 
@@ -285,7 +290,7 @@ export function initializeServerApp(
 
   const newApp = new FirebaseServerAppImpl(
     appOptions,
-    serverAppSettings,
+    _serverAppConfig,
     nameString,
     container
   );
@@ -362,16 +367,20 @@ export function getApps(): FirebaseApp[] {
  * @public
  */
 export async function deleteApp(app: FirebaseApp): Promise<void> {
-  let foundApp = false;
+  let cleanupProviders = false;
   const name = app.name;
   if (_apps.has(name)) {
-    foundApp = true;
+    cleanupProviders = true;
     _apps.delete(name);
   } else if (_serverApps.has(name)) {
-    foundApp = true;
-    _serverApps.delete(name);
+    const firebaseServerApp = app as FirebaseServerAppImpl;
+    if (firebaseServerApp.decRefCount() <= 0) {
+      _serverApps.delete(name);
+      cleanupProviders = true;
+    }
   }
-  if (foundApp) {
+
+  if (cleanupProviders) {
     await Promise.all(
       (app as FirebaseAppImpl).container
         .getProviders()
diff --git a/packages/app/src/firebaseServerApp.ts b/packages/app/src/firebaseServerApp.ts
@@ -32,6 +32,7 @@ export class FirebaseServerAppImpl
 {
   private readonly _serverConfig: FirebaseServerAppSettings;
   private _finalizationRegistry: FinalizationRegistry<object>;
+  private _refCount: number;
 
   constructor(
     options: FirebaseOptions | FirebaseAppImpl,
@@ -65,22 +66,48 @@ export class FirebaseServerAppImpl
       ...serverConfig
     };
 
-    this._finalizationRegistry = new FinalizationRegistry(
-      this.automaticCleanup
-    );
+    this._finalizationRegistry = new FinalizationRegistry(() => {
+      this.automaticCleanup();
+    });
 
-    if (this._serverConfig.releaseOnDeref !== undefined) {
-      this._finalizationRegistry.register(
-        this._serverConfig.releaseOnDeref,
-        this
-      );
-      this._serverConfig.releaseOnDeref = undefined; // Don't keep a strong reference to the object.
+    this._refCount = 0;
+    this.incRefCount(this._serverConfig.releaseOnDeref);
+
+    // Do not retain a hard reference to the dref object, otherwise the FinalizationRegisry
+    // will never trigger.
+    this._serverConfig.releaseOnDeref = undefined;
+    serverConfig.releaseOnDeref = undefined;
+  }
+
+  get refCount(): number {
+    return this._refCount;
+  }
+
+  // Increment the reference count of this server app. If an object is provided, register it
+  // with the finalization registry.
+  incRefCount(obj: object | undefined): void {
+    if (this.isDeleted) {
+      return;
+    }
+    this._refCount++;
+    if (obj !== undefined) {
+      this._finalizationRegistry.register(obj, this);
+    }
+  }
+
+  // Decrement the reference count.
+  decRefCount(): number {
+    if (this.isDeleted) {
+      return 0;
     }
+    return --this._refCount;
   }
 
-  private automaticCleanup(serverApp: FirebaseServerAppImpl): void {
-    // TODO: implement reference counting.
-    void deleteApp(serverApp);
+  // Invoked by the FinalizationRegistry callback to note that this app should go through its
+  // reference counts and delete itself if no reference count remain. The coordinating logic that
+  // handles this is in deleteApp(...).
+  private automaticCleanup(): void {
+    void deleteApp(this);
   }
 
   get settings(): FirebaseServerAppSettings {
