From dbc4c20bb5d5dd8f7c805a7ac8fcdc40e297edd0 Mon Sep 17 00:00:00 2001 From: David Geary Date: Fri, 11 Oct 2024 17:55:25 +0100 Subject: [PATCH] Fix `LocalStorageAppender` removing too many entries on initialization --- .../appenders/localstorage-appender.spec.ts | 30 ++++++++++++++++++- .../lib/appenders/localstorage-appender.ts | 11 +++---- 2 files changed, 35 insertions(+), 6 deletions(-) diff --git a/projects/log4ngx/src/lib/appenders/localstorage-appender.spec.ts b/projects/log4ngx/src/lib/appenders/localstorage-appender.spec.ts index 4421a51..9482d6d 100644 --- a/projects/log4ngx/src/lib/appenders/localstorage-appender.spec.ts +++ b/projects/log4ngx/src/lib/appenders/localstorage-appender.spec.ts @@ -133,7 +133,35 @@ describe('LocalStorageAppender', () => { expect(logEntries).toBe(message1 + appender.logEntryDelimiter + message2); }); - it('should remove log entries when max days is exceeded', () => { + it('should only remove log entries when > max days exists at initialization', () => { + jasmine.clock().withMock(() => { + const yesterdayTimestamp: number = Date.now(); + let appender: LocalStorageAppender = new LocalStorageAppender(); + appender.initialize({ ...APPENDER_CONFIG, + maxDays: MAX_DAYS + 1 + }); + + /* Add the max number of entries *prior* to today, so when we reinitialize the appender + for today with reduced `maxDays`, we're left with the correct number of entries. + */ + for (let i: number = appender.maxDays - 1; i >= 0; i--) { + jasmine.clock().mockDate(new Date(yesterdayTimestamp - (i * MILLISECS_PER_DAY))); + + const message: string = Random.getString(RANDOM_MESSAGE_LENGTH); + const loggingEvent: LoggingEvent = new LoggingEvent(Level.debug, '', message); + appender.append(loggingEvent); + } + + expect(localStorage.length).toBe(appender.maxDays); + + /* Now reduce the max days and reinitialize */ + appender = new LocalStorageAppender(); + appender.initialize(APPENDER_CONFIG); + expect(localStorage.length).toBe(appender.maxDays); + }); + }); + + it('should remove log entries when max days is exceeded on appending new entries', () => { jasmine.clock().withMock(() => { const todayTimestamp: number = Date.now(); const appender: LocalStorageAppender = new LocalStorageAppender(); diff --git a/projects/log4ngx/src/lib/appenders/localstorage-appender.ts b/projects/log4ngx/src/lib/appenders/localstorage-appender.ts index db26bcc..900fd20 100644 --- a/projects/log4ngx/src/lib/appenders/localstorage-appender.ts +++ b/projects/log4ngx/src/lib/appenders/localstorage-appender.ts @@ -57,7 +57,7 @@ export class LocalStorageAppender extends Appender { this._localStorage = this.getLocalStorage(); if (this._localStorage !== undefined) { - this.getCurrentLogEntries(this._localStorage); /* Really just to initialize _currentKey and _currentLogEntries */ + this.getCurrentLogEntries(this._localStorage, false); /* Really just to initialize _currentKey and _currentLogEntries */ } else { // eslint-disable-next-line no-console -- there's not much else we can do console.error('LOG4NGX: LocalStorage is not available; calls to log via LocalStorageAppender will be ignored'); @@ -68,7 +68,7 @@ export class LocalStorageAppender extends Appender { if (this._localStorage !== undefined) { const localStorage: Storage = this._localStorage; const message: string = this.renderLoggingEvent(loggingEvent); - let currentLogEntries: string = this.getCurrentLogEntries(localStorage); + let currentLogEntries: string = this.getCurrentLogEntries(localStorage, true); if (currentLogEntries.length === 0) { currentLogEntries = message; @@ -120,13 +120,14 @@ export class LocalStorageAppender extends Appender { } /** Gets the current log entries for today, performing any necessary housekeeping on old entries */ - private getCurrentLogEntries(localStorage: Storage): string { + private getCurrentLogEntries(localStorage: Storage, appending: boolean): string { const key: string = this._keyPrefix + new Date().setHours(0, 0, 0, 0) .toString(); if (key !== this._currentKey) { - /* We're about to add a new key, so need to make sure we have `maxDays`-1 entries at most */ - this.removeOldLogKeys(this._maxDays - 1); + /* If we're about to add a new key, so need to make sure we have `maxDays`-1 entries at most */ + this.removeOldLogKeys(appending ? this._maxDays - 1 + : this._maxDays); /* We'll reset `_currentLogEntries` just in case there are already log entries for today from a previous session.