Discard earliest heartbeat once there are 30 heartbeats (#8724)

* Delete earliest heartbeats only once there are 30 heartbeats

* Add changeset

* Use max heartbeat const in tests

* Fix test name in all caps

* Fix test names

* Formatting

Author: dlarocque

.changeset/yellow-rice-kneel.md

@@ -0,0 +1,5 @@
+---
+'@firebase/app': patch
+---
+
+Discard the earliest heartbeat once a limit of 30 heartbeats in storage has been hit.

packages/app/src/heartbeatService.test.ts

@@ -20,14 +20,16 @@ import '../test/setup';
 import {
   countBytes,
   HeartbeatServiceImpl,
-  extractHeartbeatsForHeader
+  extractHeartbeatsForHeader,
+  getEarliestHeartbeatIdx,
+  MAX_NUM_STORED_HEARTBEATS
 } from './heartbeatService';
 import {
   Component,
   ComponentType,
   ComponentContainer
 } from '@firebase/component';
-import { PlatformLoggerService } from './types';
+import { PlatformLoggerService, SingleDateHeartbeat } from './types';
 import { FirebaseApp } from './public-types';
 import * as firebaseUtil from '@firebase/util';
 import { SinonStub, stub, useFakeTimers } from 'sinon';
@@ -173,7 +175,6 @@ describe('HeartbeatServiceImpl', () => {
     let writeStub: SinonStub;
     let userAgentString = USER_AGENT_STRING_1;
     const mockIndexedDBHeartbeats = [
-      // Chosen so one will exceed 30 day limit and one will not.
       {
         agent: 'old-user-agent',
         date: '1969-12-01'
@@ -236,15 +237,14 @@ describe('HeartbeatServiceImpl', () => {
         });
       }
     });
-    it(`triggerHeartbeat() writes new heartbeats and retains old ones newer than 30 days`, async () => {
+    it(`triggerHeartbeat() writes new heartbeats and retains old ones`, async () => {
       userAgentString = USER_AGENT_STRING_2;
       clock.tick(3 * 24 * 60 * 60 * 1000);
       await heartbeatService.triggerHeartbeat();
       if (firebaseUtil.isIndexedDBAvailable()) {
         expect(writeStub).to.be.calledWith({
           heartbeats: [
-            // The first entry exceeds the 30 day retention limit.
-            mockIndexedDBHeartbeats[1],
+            ...mockIndexedDBHeartbeats,
             { agent: USER_AGENT_STRING_2, date: '1970-01-04' }
           ]
         });
@@ -260,6 +260,7 @@ describe('HeartbeatServiceImpl', () => {
       );
       if (firebaseUtil.isIndexedDBAvailable()) {
         expect(heartbeatHeaders).to.include('old-user-agent');
+        expect(heartbeatHeaders).to.include('1969-12-01');
         expect(heartbeatHeaders).to.include('1969-12-31');
       }
       expect(heartbeatHeaders).to.include(USER_AGENT_STRING_2);
@@ -273,14 +274,47 @@ describe('HeartbeatServiceImpl', () => {
       const emptyHeaders = await heartbeatService.getHeartbeatsHeader();
       expect(emptyHeaders).to.equal('');
     });
+    it('triggerHeartbeat() removes the earliest heartbeat once the max number of heartbeats is exceeded', async () => {
+      // Trigger heartbeats until we reach the limit
+      const numHeartbeats =
+        heartbeatService._heartbeatsCache?.heartbeats.length!;
+      for (let i = numHeartbeats; i <= MAX_NUM_STORED_HEARTBEATS; i++) {
+        await heartbeatService.triggerHeartbeat();
+        clock.tick(24 * 60 * 60 * 1000);
+      }
+
+      expect(heartbeatService._heartbeatsCache?.heartbeats.length).to.equal(
+        MAX_NUM_STORED_HEARTBEATS
+      );
+      const earliestHeartbeatDate = getEarliestHeartbeatIdx(
+        heartbeatService._heartbeatsCache?.heartbeats!
+      );
+      const earliestHeartbeat =
+        heartbeatService._heartbeatsCache?.heartbeats[earliestHeartbeatDate]!;
+      await heartbeatService.triggerHeartbeat();
+      expect(heartbeatService._heartbeatsCache?.heartbeats.length).to.equal(
+        MAX_NUM_STORED_HEARTBEATS
+      );
+      expect(
+        heartbeatService._heartbeatsCache?.heartbeats.indexOf(earliestHeartbeat)
+      ).to.equal(-1);
+    });
+    it('triggerHeartbeat() never causes the heartbeat count to exceed the max', async () => {
+      for (let i = 0; i <= 50; i++) {
+        await heartbeatService.triggerHeartbeat();
+        clock.tick(24 * 60 * 60 * 1000);
+        expect(
+          heartbeatService._heartbeatsCache?.heartbeats.length
+        ).to.be.lessThanOrEqual(MAX_NUM_STORED_HEARTBEATS);
+      }
+    });
   });
 
   describe('If IndexedDB records that a header was sent today', () => {
     let heartbeatService: HeartbeatServiceImpl;
     let writeStub: SinonStub;
     const userAgentString = USER_AGENT_STRING_1;
     const mockIndexedDBHeartbeats = [
-      // Chosen so one will exceed 30 day limit and one will not.
       {
         agent: 'old-user-agent',
         date: '1969-12-01'
@@ -426,4 +460,22 @@ describe('HeartbeatServiceImpl', () => {
       );
     });
   });
+
+  describe('getEarliestHeartbeatIdx()', () => {
+    it('returns -1 if the heartbeats array is empty', () => {
+      const heartbeats: SingleDateHeartbeat[] = [];
+      const idx = getEarliestHeartbeatIdx(heartbeats);
+      expect(idx).to.equal(-1);
+    });
+
+    it('returns the index of the earliest date', () => {
+      const heartbeats = [
+        { agent: generateUserAgentString(2), date: '2022-01-02' },
+        { agent: generateUserAgentString(1), date: '2022-01-01' },
+        { agent: generateUserAgentString(3), date: '2022-01-03' }
+      ];
+      const idx = getEarliestHeartbeatIdx(heartbeats);
+      expect(idx).to.equal(1);
+    });
+  });
 });

packages/app/src/heartbeatService.ts

@@ -36,8 +36,7 @@ import {
 import { logger } from './logger';
 
 const MAX_HEADER_BYTES = 1024;
-// 30 days
-const STORED_HEARTBEAT_RETENTION_MAX_MILLIS = 30 * 24 * 60 * 60 * 1000;
+export const MAX_NUM_STORED_HEARTBEATS = 30;
 
 export class HeartbeatServiceImpl implements HeartbeatService {
   /**
@@ -109,14 +108,19 @@ export class HeartbeatServiceImpl implements HeartbeatService {
       } else {
         // There is no entry for this date. Create one.
         this._heartbeatsCache.heartbeats.push({ date, agent });
+
+        // If the number of stored heartbeats exceeds the maximum number of stored heartbeats, remove the heartbeat with the earliest date.
+        // Since this is executed each time a heartbeat is pushed, the limit can only be exceeded by one, so only one needs to be removed.
+        if (
+          this._heartbeatsCache.heartbeats.length > MAX_NUM_STORED_HEARTBEATS
+        ) {
+          const earliestHeartbeatIdx = getEarliestHeartbeatIdx(
+            this._heartbeatsCache.heartbeats
+          );
+          this._heartbeatsCache.heartbeats.splice(earliestHeartbeatIdx, 1);
+        }
       }
-      // Remove entries older than 30 days.
-      this._heartbeatsCache.heartbeats =
-        this._heartbeatsCache.heartbeats.filter(singleDateHeartbeat => {
-          const hbTimestamp = new Date(singleDateHeartbeat.date).valueOf();
-          const now = Date.now();
-          return now - hbTimestamp <= STORED_HEARTBEAT_RETENTION_MAX_MILLIS;
-        });
+
       return this._storage.overwrite(this._heartbeatsCache);
     } catch (e) {
       logger.warn(e);
@@ -303,3 +307,27 @@ export function countBytes(heartbeatsCache: HeartbeatsByUserAgent[]): number {
     JSON.stringify({ version: 2, heartbeats: heartbeatsCache })
   ).length;
 }
+
+/**
+ * Returns the index of the heartbeat with the earliest date.
+ * If the heartbeats array is empty, -1 is returned.
+ */
+export function getEarliestHeartbeatIdx(
+  heartbeats: SingleDateHeartbeat[]
+): number {
+  if (heartbeats.length === 0) {
+    return -1;
+  }
+
+  let earliestHeartbeatIdx = 0;
+  let earliestHeartbeatDate = heartbeats[0].date;
+
+  for (let i = 1; i < heartbeats.length; i++) {
+    if (heartbeats[i].date < earliestHeartbeatDate) {
+      earliestHeartbeatDate = heartbeats[i].date;
+      earliestHeartbeatIdx = i;
+    }
+  }
+
+  return earliestHeartbeatIdx;
+}