From dc1276ce42d43829ee4bd60b87a2f3456d07868a Mon Sep 17 00:00:00 2001 From: Daniel La Rocque Date: Thu, 23 Jan 2025 15:07:00 -0500 Subject: [PATCH 1/6] Delete earliest heartbeats only once there are 30 heartbeats --- packages/app/src/heartbeatService.test.ts | 60 ++++++++++++++++++++--- packages/app/src/heartbeatService.ts | 46 +++++++++++++---- 2 files changed, 91 insertions(+), 15 deletions(-) diff --git a/packages/app/src/heartbeatService.test.ts b/packages/app/src/heartbeatService.test.ts index 95ac71ca42d..0f2157e13d8 100644 --- a/packages/app/src/heartbeatService.test.ts +++ b/packages/app/src/heartbeatService.test.ts @@ -20,14 +20,15 @@ import '../test/setup'; import { countBytes, HeartbeatServiceImpl, - extractHeartbeatsForHeader + extractHeartbeatsForHeader, + getEarliestHeartbeatIdx } 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 +174,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 +236,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 +259,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,6 +273,36 @@ describe('HeartbeatServiceImpl', () => { const emptyHeaders = await heartbeatService.getHeartbeatsHeader(); expect(emptyHeaders).to.equal(''); }); + it('triggerHeartbeat() removes the earliest heartbeat once it exceeds the max number of heartbeats', async () => { + // Trigger heartbeats until we reach the limit + const numHeartbeats = + heartbeatService._heartbeatsCache?.heartbeats.length!; + for (let i = numHeartbeats; i <= 30; i++) { + await heartbeatService.triggerHeartbeat(); + clock.tick(24 * 60 * 60 * 1000); + } + + expect(heartbeatService._heartbeatsCache?.heartbeats.length).to.equal(30); + const earliestHeartbeatDate = getEarliestHeartbeatIdx( + heartbeatService._heartbeatsCache?.heartbeats! + ); + const earliestHeartbeat = + heartbeatService._heartbeatsCache?.heartbeats[earliestHeartbeatDate]!; + await heartbeatService.triggerHeartbeat(); + expect(heartbeatService._heartbeatsCache?.heartbeats.length).to.equal(30); + expect( + heartbeatService._heartbeatsCache?.heartbeats.indexOf(earliestHeartbeat) + ).to.equal(-1); + }); + it('triggerHeartbeat() never exceeds 30 heartbeats', 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(30); + } + }); }); describe('If IndexedDB records that a header was sent today', () => { @@ -426,4 +456,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); + }); + }); }); diff --git a/packages/app/src/heartbeatService.ts b/packages/app/src/heartbeatService.ts index 83a2b63993e..29a732064fd 100644 --- a/packages/app/src/heartbeatService.ts +++ b/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; +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; +} From 2fda6564052713906091d0fb473a27bc8160f2a4 Mon Sep 17 00:00:00 2001 From: Daniel La Rocque Date: Thu, 23 Jan 2025 15:12:55 -0500 Subject: [PATCH 2/6] Add changeset --- .changeset/yellow-rice-kneel.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/yellow-rice-kneel.md diff --git a/.changeset/yellow-rice-kneel.md b/.changeset/yellow-rice-kneel.md new file mode 100644 index 00000000000..66bf4c41307 --- /dev/null +++ b/.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. From 2ae1a6647f223c85b4f82424887893ba0ed2b373 Mon Sep 17 00:00:00 2001 From: Daniel La Rocque Date: Thu, 23 Jan 2025 15:45:55 -0500 Subject: [PATCH 3/6] Use max heartbeat const in tests --- packages/app/src/heartbeatService.test.ts | 14 +++++++------- packages/app/src/heartbeatService.ts | 2 +- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/packages/app/src/heartbeatService.test.ts b/packages/app/src/heartbeatService.test.ts index 0f2157e13d8..a6f9110ff13 100644 --- a/packages/app/src/heartbeatService.test.ts +++ b/packages/app/src/heartbeatService.test.ts @@ -21,7 +21,8 @@ import { countBytes, HeartbeatServiceImpl, extractHeartbeatsForHeader, - getEarliestHeartbeatIdx + getEarliestHeartbeatIdx, + MAX_NUM_STORED_HEARTBEATS } from './heartbeatService'; import { Component, @@ -277,30 +278,30 @@ describe('HeartbeatServiceImpl', () => { // Trigger heartbeats until we reach the limit const numHeartbeats = heartbeatService._heartbeatsCache?.heartbeats.length!; - for (let i = numHeartbeats; i <= 30; i++) { + 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(30); + 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(30); + expect(heartbeatService._heartbeatsCache?.heartbeats.length).to.equal(MAX_NUM_STORED_HEARTBEATS); expect( heartbeatService._heartbeatsCache?.heartbeats.indexOf(earliestHeartbeat) ).to.equal(-1); }); - it('triggerHeartbeat() never exceeds 30 heartbeats', async () => { + it('triggerHeartbeat() never exceeds MAX_NUM_STORED_HEARTBEATS heartbeats', 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(30); + ).to.be.lessThanOrEqual(MAX_NUM_STORED_HEARTBEATS); } }); }); @@ -310,7 +311,6 @@ describe('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' diff --git a/packages/app/src/heartbeatService.ts b/packages/app/src/heartbeatService.ts index 29a732064fd..ad602484d1f 100644 --- a/packages/app/src/heartbeatService.ts +++ b/packages/app/src/heartbeatService.ts @@ -36,7 +36,7 @@ import { import { logger } from './logger'; const MAX_HEADER_BYTES = 1024; -const MAX_NUM_STORED_HEARTBEATS = 30; +export const MAX_NUM_STORED_HEARTBEATS = 30; export class HeartbeatServiceImpl implements HeartbeatService { /** From fb96954244ca97b22893c1d3faf65b6e6284f671 Mon Sep 17 00:00:00 2001 From: Daniel La Rocque Date: Thu, 23 Jan 2025 15:47:16 -0500 Subject: [PATCH 4/6] Fix test name in all caps --- packages/app/src/heartbeatService.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/app/src/heartbeatService.test.ts b/packages/app/src/heartbeatService.test.ts index a6f9110ff13..46d636ae2e8 100644 --- a/packages/app/src/heartbeatService.test.ts +++ b/packages/app/src/heartbeatService.test.ts @@ -295,7 +295,7 @@ describe('HeartbeatServiceImpl', () => { heartbeatService._heartbeatsCache?.heartbeats.indexOf(earliestHeartbeat) ).to.equal(-1); }); - it('triggerHeartbeat() never exceeds MAX_NUM_STORED_HEARTBEATS heartbeats', async () => { + it('triggerHeartbeat() never exceeds max heartbeats', async () => { for (let i = 0; i <= 50; i++) { await heartbeatService.triggerHeartbeat(); clock.tick(24 * 60 * 60 * 1000); From a26157988c7b442c5a1770f1e7e5ea0957256128 Mon Sep 17 00:00:00 2001 From: Daniel La Rocque Date: Thu, 23 Jan 2025 15:49:31 -0500 Subject: [PATCH 5/6] Fix test names --- packages/app/src/heartbeatService.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/app/src/heartbeatService.test.ts b/packages/app/src/heartbeatService.test.ts index 46d636ae2e8..0be8178340b 100644 --- a/packages/app/src/heartbeatService.test.ts +++ b/packages/app/src/heartbeatService.test.ts @@ -274,7 +274,7 @@ describe('HeartbeatServiceImpl', () => { const emptyHeaders = await heartbeatService.getHeartbeatsHeader(); expect(emptyHeaders).to.equal(''); }); - it('triggerHeartbeat() removes the earliest heartbeat once it exceeds the max number of heartbeats', async () => { + 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!; @@ -295,7 +295,7 @@ describe('HeartbeatServiceImpl', () => { heartbeatService._heartbeatsCache?.heartbeats.indexOf(earliestHeartbeat) ).to.equal(-1); }); - it('triggerHeartbeat() never exceeds max heartbeats', async () => { + 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); From 1df988765ba1e6c49d3183a75dbf08a9c20a83e6 Mon Sep 17 00:00:00 2001 From: Daniel La Rocque Date: Thu, 23 Jan 2025 15:52:34 -0500 Subject: [PATCH 6/6] Formatting --- packages/app/src/heartbeatService.test.ts | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/packages/app/src/heartbeatService.test.ts b/packages/app/src/heartbeatService.test.ts index 0be8178340b..57f97ec7468 100644 --- a/packages/app/src/heartbeatService.test.ts +++ b/packages/app/src/heartbeatService.test.ts @@ -283,14 +283,18 @@ describe('HeartbeatServiceImpl', () => { clock.tick(24 * 60 * 60 * 1000); } - expect(heartbeatService._heartbeatsCache?.heartbeats.length).to.equal(MAX_NUM_STORED_HEARTBEATS); + 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.length).to.equal( + MAX_NUM_STORED_HEARTBEATS + ); expect( heartbeatService._heartbeatsCache?.heartbeats.indexOf(earliestHeartbeat) ).to.equal(-1);