From 4f2f577f5fec69bd26db79e693ade91d92c7c0b5 Mon Sep 17 00:00:00 2001 From: Jeremy Press Date: Mon, 23 Apr 2018 13:21:39 -0700 Subject: [PATCH] Chore: Refactor Timer to allow a reset of multiple tags (#778) --- src/lib/Preview.js | 15 +++++++-------- src/lib/Timer.js | 20 ++++++++++++-------- src/lib/__tests__/Preview-test.js | 4 ++-- src/lib/__tests__/Timer-test.js | 22 ++++++++++++++++++++++ 4 files changed, 43 insertions(+), 18 deletions(-) diff --git a/src/lib/Preview.js b/src/lib/Preview.js index 56e0ec236..2f2a84ded 100644 --- a/src/lib/Preview.js +++ b/src/lib/Preview.js @@ -1474,22 +1474,21 @@ class Preview extends EventEmitter { */ emitLoadMetrics() { if (!this.file || !this.file.id) { - Timer.reset(); return; } - // Do nothing if there is nothing worth logging. const infoTag = Timer.createTag(this.file.id, LOAD_METRIC.fileInfoTime); + const convertTag = Timer.createTag(this.file.id, LOAD_METRIC.convertTime); + const downloadTag = Timer.createTag(this.file.id, LOAD_METRIC.downloadResponseTime); + const fullLoadTag = Timer.createTag(this.file.id, LOAD_METRIC.fullDocumentLoadTime); + + // Do nothing if there is nothing worth logging. const infoTime = Timer.get(infoTag) || {}; if (!infoTime.elapsed) { - Timer.reset(); + Timer.reset([infoTag, convertTag, downloadTag, fullLoadTag]); return; } - const convertTag = Timer.createTag(this.file.id, LOAD_METRIC.convertTime); - const downloadTag = Timer.createTag(this.file.id, LOAD_METRIC.downloadResponseTime); - const fullLoadTag = Timer.createTag(this.file.id, LOAD_METRIC.fullDocumentLoadTime); - const timerList = [ infoTime, Timer.get(convertTag) || {}, @@ -1511,7 +1510,7 @@ class Preview extends EventEmitter { this.emit(PREVIEW_METRIC, event); - Timer.reset(); + Timer.reset([infoTag, convertTag, downloadTag, fullLoadTag]); } /** diff --git a/src/lib/Timer.js b/src/lib/Timer.js index ac8362499..e15006752 100644 --- a/src/lib/Timer.js +++ b/src/lib/Timer.js @@ -63,14 +63,20 @@ class Timer { * Resets the values in a certain time structure, if it exists. * * @public - * @param {string} [tag] - If provided, will reset a specific time structure associated with the tag. - * If empty, resets everything. + * @param {string|string[]} [tagOrTags] - If provided, will reset a specific time structure associated with + * the tag or array of tags. If empty, resets all tags. + * * @return {void} */ - reset(tag) { - if (tag) { - const time = this.get(tag); + reset(tagOrTags) { + if (!tagOrTags) { + Object.keys(this.times).forEach(this.reset.bind(this)); + return; + } + const tagArray = typeof tagOrTags === 'string' ? [tagOrTags] : tagOrTags; + tagArray.forEach((tag) => { + const time = this.get(tag); // If nothing exists, there's no reason to reset it if (!time) { return; @@ -79,9 +85,7 @@ class Timer { time.start = undefined; time.end = undefined; time.elapsed = undefined; - } else { - Object.keys(this.times).forEach(this.reset.bind(this)); - } + }); } /** diff --git a/src/lib/__tests__/Preview-test.js b/src/lib/__tests__/Preview-test.js index 280f1e276..0efdd6164 100644 --- a/src/lib/__tests__/Preview-test.js +++ b/src/lib/__tests__/Preview-test.js @@ -2222,12 +2222,12 @@ describe('lib/Preview', () => { Timer.reset(); }); - it('should reset the Timer and escape early if no file or file id', () => { + it('should do nothingescape early if no file or file id', () => { sandbox.stub(Timer, 'reset'); sandbox.stub(preview, 'emit'); preview.file = undefined; preview.emitLoadMetrics(); - expect(Timer.reset).to.be.called; + expect(Timer.reset).to.not.be.called; expect(preview.emit).to.not.be.called; }); diff --git a/src/lib/__tests__/Timer-test.js b/src/lib/__tests__/Timer-test.js index 943f1b257..929c97818 100644 --- a/src/lib/__tests__/Timer-test.js +++ b/src/lib/__tests__/Timer-test.js @@ -118,6 +118,28 @@ describe('lib/Timer', () => { expect(Timer.get(other).end).to.not.exist; expect(Timer.get(other).elapsed).to.not.exist; }); + + it('should reset multiple tags given an array of tags', () => { + Timer.start(tag); + Timer.stop(tag); + const other = 'other_tag'; + Timer.start(other); + Timer.stop(other); + const another = 'another_tag'; + Timer.start(another); + Timer.stop(another); + Timer.reset([tag, other]); + + expect(Timer.get(tag).start).to.not.exist; + expect(Timer.get(tag).end).to.not.exist; + expect(Timer.get(tag).elapsed).to.not.exist; + expect(Timer.get(other).start).to.not.exist; + expect(Timer.get(other).end).to.not.exist; + expect(Timer.get(other).elapsed).to.not.exist; + expect(Timer.get(another).start).to.exist; + expect(Timer.get(another).end).to.exist; + expect(Timer.get(another).elapsed).to.exist; + }); }); describe('createTag()', () => {