From dafae52afda0b653a763b071960ee87010a63aa1 Mon Sep 17 00:00:00 2001 From: Daniel La Rocque Date: Mon, 27 Jan 2025 17:40:14 -0600 Subject: [PATCH] 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 --- .changeset/yellow-rice-kneel.md | 5 ++ packages/app/src/heartbeatService.test.ts | 66 ++++++++++++++++++++--- packages/app/src/heartbeatService.ts | 46 ++++++++++++---- 3 files changed, 101 insertions(+), 16 deletions(-) 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. diff --git a/packages/app/src/heartbeatService.test.ts b/packages/app/src/heartbeatService.test.ts index 95ac71ca42d..57f97ec7468 100644 --- a/packages/app/src/heartbeatService.test.ts +++ b/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,6 +274,40 @@ 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', () => { @@ -280,7 +315,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' @@ -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); + }); + }); }); diff --git a/packages/app/src/heartbeatService.ts b/packages/app/src/heartbeatService.ts index 83a2b63993e..ad602484d1f 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; +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; +}