Add a last sent date to heartbeat storage

Author: hsubox76

packages/app/src/heartbeatService.test.ts

@@ -99,38 +99,37 @@ describe('HeartbeatServiceImpl', () => {
      */
     it(`triggerHeartbeat() stores a heartbeat`, async () => {
       await heartbeatService.triggerHeartbeat();
-      expect(heartbeatService._heartbeatsCache?.length).to.equal(1);
-      const heartbeat1 = heartbeatService._heartbeatsCache?.[0];
+      expect(heartbeatService._heartbeatsCache?.heartbeats.length).to.equal(1);
+      const heartbeat1 = heartbeatService._heartbeatsCache?.heartbeats[0];
       expect(heartbeat1?.userAgent).to.equal(USER_AGENT_STRING_1);
       expect(heartbeat1?.date).to.equal('1970-01-01');
-      expect(writeStub).to.be.calledWith([heartbeat1]);
+      expect(writeStub).to.be.calledWith({ heartbeats: [heartbeat1] });
     });
     it(`triggerHeartbeat() doesn't store another heartbeat on the same day`, async () => {
-      expect(heartbeatService._heartbeatsCache?.length).to.equal(1);
+      expect(heartbeatService._heartbeatsCache?.heartbeats.length).to.equal(1);
       await heartbeatService.triggerHeartbeat();
-      expect(heartbeatService._heartbeatsCache?.length).to.equal(1);
+      expect(heartbeatService._heartbeatsCache?.heartbeats.length).to.equal(1);
     });
     it(`triggerHeartbeat() does store another heartbeat on a different day`, async () => {
-      expect(heartbeatService._heartbeatsCache?.length).to.equal(1);
+      expect(heartbeatService._heartbeatsCache?.heartbeats.length).to.equal(1);
       clock.tick(24 * 60 * 60 * 1000);
       await heartbeatService.triggerHeartbeat();
-      expect(heartbeatService._heartbeatsCache?.length).to.equal(2);
-      expect(heartbeatService._heartbeatsCache?.[1].date).to.equal(
+      expect(heartbeatService._heartbeatsCache?.heartbeats.length).to.equal(2);
+      expect(heartbeatService._heartbeatsCache?.heartbeats[1].date).to.equal(
         '1970-01-02'
       );
     });
     it(`triggerHeartbeat() stores another entry for a different user agent`, async () => {
       userAgentString = USER_AGENT_STRING_2;
-      expect(heartbeatService._heartbeatsCache?.length).to.equal(2);
+      expect(heartbeatService._heartbeatsCache?.heartbeats.length).to.equal(2);
       clock.tick(2 * 24 * 60 * 60 * 1000);
       await heartbeatService.triggerHeartbeat();
-      expect(heartbeatService._heartbeatsCache?.length).to.equal(3);
-      expect(heartbeatService._heartbeatsCache?.[2].date).to.equal(
+      expect(heartbeatService._heartbeatsCache?.heartbeats.length).to.equal(3);
+      expect(heartbeatService._heartbeatsCache?.heartbeats[2].date).to.equal(
         '1970-01-03'
       );
     });
     it('getHeartbeatHeaders() gets stored heartbeats and clears heartbeats', async () => {
-      const deleteStub = stub(heartbeatService._storage, 'deleteAll');
       const heartbeatHeaders = firebaseUtil.base64Decode(
         await heartbeatService.getHeartbeatsHeader()
       );
@@ -140,10 +139,13 @@ describe('HeartbeatServiceImpl', () => {
       expect(heartbeatHeaders).to.include('1970-01-02');
       expect(heartbeatHeaders).to.include('1970-01-03');
       expect(heartbeatHeaders).to.include(`"version":2`);
-      expect(heartbeatService._heartbeatsCache).to.equal(null);
+      expect(heartbeatService._heartbeatsCache?.heartbeats).to.be.empty;
+      expect(writeStub).to.be.calledWith({
+        lastSentHeartbeatDate: '1970-01-01',
+        heartbeats: []
+      });
       const emptyHeaders = await heartbeatService.getHeartbeatsHeader();
       expect(emptyHeaders).to.equal('');
-      expect(deleteStub).to.be.called;
     });
   });
   describe('If IndexedDB has entries', () => {
@@ -198,35 +200,43 @@ describe('HeartbeatServiceImpl', () => {
     it(`new heartbeat service reads from indexedDB cache`, async () => {
       const promiseResult = await heartbeatService._heartbeatsCachePromise;
       if (isIndexedDBAvailable()) {
-        expect(promiseResult).to.deep.equal(mockIndexedDBHeartbeats);
-        expect(heartbeatService._heartbeatsCache).to.deep.equal(
-          mockIndexedDBHeartbeats
-        );
+        expect(promiseResult).to.deep.equal({
+          heartbeats: mockIndexedDBHeartbeats
+        });
+        expect(heartbeatService._heartbeatsCache).to.deep.equal({
+          heartbeats: mockIndexedDBHeartbeats
+        });
       } else {
         // In Node or other no-indexed-db environments it will fail the
         // `canUseIndexedDb` check and return an empty array.
-        expect(promiseResult).to.deep.equal([]);
-        expect(heartbeatService._heartbeatsCache).to.deep.equal([]);
+        expect(promiseResult).to.deep.equal({
+          heartbeats: []
+        });
+        expect(heartbeatService._heartbeatsCache).to.deep.equal({
+          heartbeats: []
+        });
       }
     });
     it(`triggerHeartbeat() writes new heartbeats and retains old ones newer than 30 days`, async () => {
       userAgentString = USER_AGENT_STRING_2;
       clock.tick(3 * 24 * 60 * 60 * 1000);
       await heartbeatService.triggerHeartbeat();
       if (isIndexedDBAvailable()) {
-        expect(writeStub).to.be.calledWith([
-          // The first entry exceeds the 30 day retention limit.
-          mockIndexedDBHeartbeats[1],
-          { userAgent: USER_AGENT_STRING_2, date: '1970-01-04' }
-        ]);
+        expect(writeStub).to.be.calledWith({
+          heartbeats: [
+            // The first entry exceeds the 30 day retention limit.
+            mockIndexedDBHeartbeats[1],
+            { userAgent: USER_AGENT_STRING_2, date: '1970-01-04' }
+          ]
+        });
       } else {
-        expect(writeStub).to.be.calledWith([
-          { userAgent: USER_AGENT_STRING_2, date: '1970-01-04' }
-        ]);
+        expect(writeStub).to.be.calledWith({
+          heartbeats: [{ userAgent: USER_AGENT_STRING_2, date: '1970-01-04' }]
+        });
       }
     });
     it('getHeartbeatHeaders() gets stored heartbeats and clears heartbeats', async () => {
-      const deleteStub = stub(heartbeatService._storage, 'deleteAll');
+
       const heartbeatHeaders = firebaseUtil.base64Decode(
         await heartbeatService.getHeartbeatsHeader()
       );
@@ -237,10 +247,91 @@ describe('HeartbeatServiceImpl', () => {
       expect(heartbeatHeaders).to.include(USER_AGENT_STRING_2);
       expect(heartbeatHeaders).to.include('1970-01-04');
       expect(heartbeatHeaders).to.include(`"version":2`);
-      expect(heartbeatService._heartbeatsCache).to.equal(null);
+      expect(heartbeatService._heartbeatsCache?.heartbeats).to.be.empty;
+      expect(writeStub).to.be.calledWith({
+        lastSentHeartbeatDate: '1970-01-01',
+        heartbeats: []
+      });
       const emptyHeaders = await heartbeatService.getHeartbeatsHeader();
       expect(emptyHeaders).to.equal('');
-      expect(deleteStub).to.be.called;
+    });
+  });
+
+  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.
+      {
+        userAgent: 'old-user-agent',
+        date: '1969-12-01'
+      },
+      {
+        userAgent: 'old-user-agent',
+        date: '1969-12-31'
+      }
+    ];
+    before(() => {
+      const container = new ComponentContainer('heartbeatTestContainer');
+      container.addComponent(
+        new Component(
+          'app',
+          () =>
+            ({
+              options: { appId: 'an-app-id' },
+              name: 'an-app-name'
+            } as FirebaseApp),
+          ComponentType.VERSION
+        )
+      );
+      container.addComponent(
+        new Component(
+          'platform-logger',
+          () => ({ getPlatformInfoString: () => userAgentString }),
+          ComponentType.VERSION
+        )
+      );
+      stub(indexedDb, 'readHeartbeatsFromIndexedDB').resolves({
+        lastSentHeartbeatDate: '1970-01-01',
+        heartbeats: [...mockIndexedDBHeartbeats]
+      });
+      heartbeatService = new HeartbeatServiceImpl(container);
+    });
+    beforeEach(() => {
+      useFakeTimers();
+      writeStub = stub(heartbeatService._storage, 'overwrite');
+    });
+    it(`new heartbeat service reads from indexedDB cache`, async () => {
+      const promiseResult = await heartbeatService._heartbeatsCachePromise;
+      if (isIndexedDBAvailable()) {
+        expect(promiseResult).to.deep.equal({
+          lastSentHeartbeatDate: '1970-01-01',
+          heartbeats: mockIndexedDBHeartbeats
+        });
+        expect(heartbeatService._heartbeatsCache).to.deep.equal({
+          lastSentHeartbeatDate: '1970-01-01',
+          heartbeats: mockIndexedDBHeartbeats
+        });
+      } else {
+        // In Node or other no-indexed-db environments it will fail the
+        // `canUseIndexedDb` check and return an empty array.
+        expect(promiseResult).to.deep.equal({
+          heartbeats: []
+        });
+        expect(heartbeatService._heartbeatsCache).to.deep.equal({
+          heartbeats: []
+        });
+      }
+    });
+    it(`triggerHeartbeat() will skip storing new data`, async () => {
+      await heartbeatService.triggerHeartbeat();
+      expect(writeStub).to.not.be.called;
+      if (isIndexedDBAvailable()) {
+        expect(heartbeatService._heartbeatsCache?.heartbeats).to.deep.equal(
+          mockIndexedDBHeartbeats
+        );
+      }
     });
   });
 

packages/app/src/heartbeatService.ts

@@ -22,14 +22,14 @@ import {
   validateIndexedDBOpenable
 } from '@firebase/util';
 import {
-  deleteHeartbeatsFromIndexedDB,
   readHeartbeatsFromIndexedDB,
   writeHeartbeatsToIndexedDB
 } from './indexeddb';
 import { FirebaseApp } from './public-types';
 import {
   HeartbeatsByUserAgent,
   HeartbeatService,
+  HeartbeatsInIndexedDB,
   HeartbeatStorage,
   SingleDateHeartbeat
 } from './types';
@@ -54,15 +54,15 @@ export class HeartbeatServiceImpl implements HeartbeatService {
    * be kept in sync with indexedDB.
    * Leave public for easier testing.
    */
-  _heartbeatsCache: SingleDateHeartbeat[] | null = null;
+  _heartbeatsCache: HeartbeatsInIndexedDB | null = null;
 
   /**
    * the initialization promise for populating heartbeatCache.
    * If getHeartbeatsHeader() is called before the promise resolves
    * (hearbeatsCache == null), it should wait for this promise
    * Leave public for easier testing.
    */
-  _heartbeatsCachePromise: Promise<SingleDateHeartbeat[]>;
+  _heartbeatsCachePromise: Promise<HeartbeatsInIndexedDB>;
   constructor(private readonly container: ComponentContainer) {
     const app = this.container.getProvider('app').getImmediate();
     this._storage = new HeartbeatStorageImpl(app);
@@ -91,19 +91,21 @@ export class HeartbeatServiceImpl implements HeartbeatService {
     if (this._heartbeatsCache === null) {
       this._heartbeatsCache = await this._heartbeatsCachePromise;
     }
+    // Do not store a heartbeat if one is already stored for this day
+    // or if a header has already been sent today.
     if (
-      this._heartbeatsCache.some(
+      this._heartbeatsCache.lastSentHeartbeatDate === date ||
+      this._heartbeatsCache.heartbeats.some(
         singleDateHeartbeat => singleDateHeartbeat.date === date
       )
     ) {
-      // Do not store a heartbeat if one is already stored for this day.
       return;
     } else {
       // There is no entry for this date. Create one.
-      this._heartbeatsCache.push({ date, userAgent });
+      this._heartbeatsCache.heartbeats.push({ date, userAgent });
     }
     // Remove entries older than 30 days.
-    this._heartbeatsCache = this._heartbeatsCache.filter(
+    this._heartbeatsCache.heartbeats = this._heartbeatsCache.heartbeats.filter(
       singleDateHeartbeat => {
         const hbTimestamp = new Date(singleDateHeartbeat.date).valueOf();
         const now = Date.now();
@@ -123,28 +125,34 @@ export class HeartbeatServiceImpl implements HeartbeatService {
     if (this._heartbeatsCache === null) {
       await this._heartbeatsCachePromise;
     }
-    // If it's still null, it's been cleared and has not been repopulated.
-    if (this._heartbeatsCache === null) {
+    // If it's still null or the array is empty, there is no data to send.
+    if (
+      this._heartbeatsCache === null ||
+      this._heartbeatsCache.heartbeats.length === 0
+    ) {
       return '';
     }
+    const date = getUTCDateString();
     // Extract as many heartbeats from the cache as will fit under the size limit.
     const { heartbeatsToSend, unsentEntries } = extractHeartbeatsForHeader(
-      this._heartbeatsCache
+      this._heartbeatsCache.heartbeats
     );
     const headerString = base64Encode(
       JSON.stringify({ version: 2, heartbeats: heartbeatsToSend })
     );
+    // Store last sent date to prevent another being logged/sent for the same day.
+    this._heartbeatsCache.lastSentHeartbeatDate = date;
     if (unsentEntries.length > 0) {
       // Store any unsent entries if they exist.
-      this._heartbeatsCache = unsentEntries;
-      // This seems more likely than deleteAll (below) to lead to some odd state
+      this._heartbeatsCache.heartbeats = unsentEntries;
+      // This seems more likely than emptying the array (below) to lead to some odd state
       // since the cache isn't empty and this will be called again on the next request,
       // and is probably safest if we await it.
       await this._storage.overwrite(this._heartbeatsCache);
     } else {
-      this._heartbeatsCache = null;
+      this._heartbeatsCache.heartbeats = [];
       // Do not wait for this, to reduce latency.
-      void this._storage.deleteAll();
+      void this._storage.overwrite(this._heartbeatsCache);
     }
     return headerString;
   }
@@ -221,59 +229,48 @@ export class HeartbeatStorageImpl implements HeartbeatStorage {
   /**
    * Read all heartbeats.
    */
-  async read(): Promise<SingleDateHeartbeat[]> {
+  async read(): Promise<HeartbeatsInIndexedDB> {
     const canUseIndexedDB = await this._canUseIndexedDBPromise;
     if (!canUseIndexedDB) {
-      return [];
+      return { heartbeats: [] };
     } else {
       const idbHeartbeatObject = await readHeartbeatsFromIndexedDB(this.app);
-      return idbHeartbeatObject?.heartbeats || [];
+      return idbHeartbeatObject || { heartbeats: [] };
     }
   }
   // overwrite the storage with the provided heartbeats
-  async overwrite(heartbeats: SingleDateHeartbeat[]): Promise<void> {
-    const canUseIndexedDB = await this._canUseIndexedDBPromise;
-    if (!canUseIndexedDB) {
-      return;
-    } else {
-      return writeHeartbeatsToIndexedDB(this.app, { heartbeats });
-    }
-  }
-  // add heartbeats
-  async add(heartbeats: SingleDateHeartbeat[]): Promise<void> {
+  async overwrite(heartbeatsObject: HeartbeatsInIndexedDB): Promise<void> {
     const canUseIndexedDB = await this._canUseIndexedDBPromise;
     if (!canUseIndexedDB) {
       return;
     } else {
-      const existingHeartbeats = await this.read();
+      const existingHeartbeatsObject = await this.read();
       return writeHeartbeatsToIndexedDB(this.app, {
-        heartbeats: [...existingHeartbeats, ...heartbeats]
+        lastSentHeartbeatDate:
+          heartbeatsObject.lastSentHeartbeatDate ??
+          existingHeartbeatsObject.lastSentHeartbeatDate,
+        heartbeats: heartbeatsObject.heartbeats
       });
     }
   }
-  // delete heartbeats
-  async delete(heartbeats: SingleDateHeartbeat[]): Promise<void> {
+  // add heartbeats
+  async add(heartbeatsObject: HeartbeatsInIndexedDB): Promise<void> {
     const canUseIndexedDB = await this._canUseIndexedDBPromise;
     if (!canUseIndexedDB) {
       return;
     } else {
-      const existingHeartbeats = await this.read();
+      const existingHeartbeatsObject = await this.read();
       return writeHeartbeatsToIndexedDB(this.app, {
-        heartbeats: existingHeartbeats.filter(
-          existingHeartbeat => !heartbeats.includes(existingHeartbeat)
-        )
+        lastSentHeartbeatDate:
+          heartbeatsObject.lastSentHeartbeatDate ??
+          existingHeartbeatsObject.lastSentHeartbeatDate,
+        heartbeats: [
+          ...existingHeartbeatsObject.heartbeats,
+          ...heartbeatsObject.heartbeats
+        ]
       });
     }
   }
-  // delete all heartbeats
-  async deleteAll(): Promise<void> {
-    const canUseIndexedDB = await this._canUseIndexedDBPromise;
-    if (!canUseIndexedDB) {
-      return;
-    } else {
-      return deleteHeartbeatsFromIndexedDB(this.app);
-    }
-  }
 }
 
 /**