From 85c9afbb2ea59e51b10f4d0dad4cf3ae01e7da6f Mon Sep 17 00:00:00 2001 From: Jonathan Griggs Date: Fri, 16 Sep 2022 01:16:16 -0600 Subject: [PATCH 1/8] Add `error` property on result --- src/OmnesiacCache.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/OmnesiacCache.ts b/src/OmnesiacCache.ts index 1f9155a..2886020 100644 --- a/src/OmnesiacCache.ts +++ b/src/OmnesiacCache.ts @@ -2,6 +2,7 @@ export interface OmnesiacResult { ttl?: number; inFlight?: boolean; result?: T; + error?: Error | false; } export default class OmnesiacCache { From 6d6004a1a56cf8fa585e939237d68143b4478cfb Mon Sep 17 00:00:00 2001 From: Jonathan Griggs Date: Fri, 16 Sep 2022 01:16:41 -0600 Subject: [PATCH 2/8] Refactor default implementation --- src/index.ts | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/src/index.ts b/src/index.ts index d0bde25..5dcfec2 100644 --- a/src/index.ts +++ b/src/index.ts @@ -16,16 +16,24 @@ export = function Omnesiac any>( return async function(key: string, ...args: Parameters): Promise | void> { const val = cache.get(key); - if (!val) { - cache.set(key, { inFlight: true }); - const retVal = await fn(...args); - cache.set(key, { ttl, inFlight: false, result: retVal }); - return retVal; - } else if (val.inFlight) { + if (val && val.inFlight && blocking) { while (val.inFlight && blocking) { await wait(pollFrequencyMs); } + if (!val.error) { + return val.result; + } + } else if (val && val.inFlight && !blocking) { + // If you're not blocking, then by definition, you can't be dependent upon a return value. + return; + } + cache.set(key, { inFlight: true }); + try { + const retVal = await fn(...args); + cache.set(key, { ttl, inFlight: false, result: retVal }); + return retVal; + } catch (err) { + cache.set(key, { ttl, inFlight: false, error: err }); } - return val.result; }; }; From 22fa73ee42d34dd12b5287776b9280368d070ecf Mon Sep 17 00:00:00 2001 From: Jonathan Griggs Date: Fri, 16 Sep 2022 05:27:48 -0600 Subject: [PATCH 3/8] Refactor to handle errors --- src/index.ts | 27 +++++++++++++-------------- 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/src/index.ts b/src/index.ts index a0ee25d..a521153 100644 --- a/src/index.ts +++ b/src/index.ts @@ -16,24 +16,23 @@ export = function Omnesiac any>( return async function (key: string, ...args: Parameters): Promise | void> { const val = cache.get(key); - if (val && val.inFlight && blocking) { + if (val?.inFlight && blocking) { while (val.inFlight && blocking) { await wait(pollFrequencyMs); } - if (!val.error) { - return val.result; - } - } else if (val && val.inFlight && !blocking) { - // If you're not blocking, then by definition, you can't be dependent upon a return value. - return; } - cache.set(key, { inFlight: true }); - try { - const retVal = await fn(...args); - cache.set(key, { ttl, inFlight: false, result: retVal }); - return retVal; - } catch (err) { - cache.set(key, { ttl, inFlight: false, error: err }); + + if (!val || val.error) { + cache.set(key, { inFlight: true, error: false }); + try { + const retVal = await fn(...args); + cache.set(key, { ttl, inFlight: false, result: retVal, error: false }); + return retVal; + } catch (err) { + cache.set(key, { ttl: 0, inFlight: false, error: err as Error }); + throw err; + } } + return val.result; }; }; From 8e703cd1404c32f9f4f9b8a287d528e2baad1c0e Mon Sep 17 00:00:00 2001 From: Jonathan Griggs Date: Fri, 16 Sep 2022 07:41:26 -0600 Subject: [PATCH 4/8] Refactor tests to use sinon fake-timers; Cover exception handling --- src/index.spec.ts | 198 +++++++++++++++++++++++++++++++++++----------- 1 file changed, 151 insertions(+), 47 deletions(-) diff --git a/src/index.spec.ts b/src/index.spec.ts index 3408af6..a13b7df 100644 --- a/src/index.spec.ts +++ b/src/index.spec.ts @@ -1,35 +1,63 @@ import should = require('should'); import Omnesiac = require('./index'); import * as sinon from 'sinon'; - -function wait(ms: number, result?: unknown): Promise { - return new Promise((resolve) => { - setTimeout(() => resolve(result), ms); +import { before } from 'mocha'; + +function wait(ms: number, result?: unknown, error?: string): Promise { + return new Promise((resolve, reject) => { + setTimeout(() => { + if (error) reject(new Error(error)); + resolve(result); + }, ms); }); } interface AsyncTestResult { // eslint-disable-next-line @typescript-eslint/no-explicit-any result?: any; - time: number; + seq: number; +} + +class Counter { + private count = 0; + get next(): number { + return ++this.count; + } + reset() { + this.count = 0; + } } describe('Omnesiac', () => { + let clock: sinon.SinonFakeTimers; + const counter = new Counter(); + + before(() => { + clock = sinon.useFakeTimers(); + }); + + after(() => { + clock.restore(); + counter.reset(); + }); + describe('blocking = false', () => { it('should only process one request at a time, concurrent requests should not be blocked by in-flight', async () => { const fn = sinon.spy(wait); const omnesized = Omnesiac(fn, { blocking: false }); - const wrapper = async (): Promise => { - const result = await omnesized('key', 50, 'waited for 50'); - return { result, time: Date.now() }; + const wrapper = async (val: unknown = 'Unspecified Value'): Promise => { + const result = await omnesized('key', 50, val); + return { result, seq: counter.next }; }; - const [{ result: result1, time: time1 }, { result: result2, time: time2 }, { result: result3, time: time3 }] = - await Promise.all([wrapper(), wrapper(), wrapper()]); + const resultPromises = Promise.all([wrapper(), wrapper(), wrapper()]); + await clock.runAllAsync(); + const [{ result: result1, seq: seq1 }, { result: result2, seq: seq2 }, { result: result3, seq: seq3 }] = + await resultPromises; - time1.should.be.a.Number().greaterThan(time2); - time1.should.be.a.Number().greaterThan(time3); + seq1.should.be.a.Number().greaterThan(seq2); + seq1.should.be.a.Number().greaterThan(seq3); should(result1).be.ok(); should(result2).not.be.ok(); @@ -38,50 +66,76 @@ describe('Omnesiac', () => { fn.calledOnce.should.be.true; }); }); + describe('blocking = true', () => { it('should only process one request at a time, concurrent requests should block during in-flight and return result', async () => { const fn = sinon.spy(wait); const omnesized = Omnesiac(fn, { blocking: true }); - let counter = 1; - const wrapper = async (): Promise => { - const result = await omnesized('key', 100, counter++); - return { result, time: Date.now() }; + const wrapper = async (val: unknown = 'Unspecified Value'): Promise => { + const result = await omnesized('key', 100, val); + return { result, seq: counter.next }; }; - const [{ result: result1, time: time1 }, { result: result2, time: time2 }, { result: result3, time: time3 }] = - await Promise.all([wrapper(), wrapper(), wrapper()]); - - time1.should.be.a.Number().lessThanOrEqual(time2); - time1.should.be.a.Number().lessThanOrEqual(time3); - - should(result1).be.ok(); - should(result2).be.ok(); - should(result3).be.ok(); + const resultPromises = Promise.all([wrapper(1), wrapper(2), wrapper(3)]); + await clock.runAllAsync(); + const [{ result: result1, seq: seq1 }, { result: result2, seq: seq2 }, { result: result3, seq: seq3 }] = + await resultPromises; result1.should.be.a.Number().eql(1); result2.should.be.a.Number().eql(result1); result3.should.be.a.Number().eql(result1); + seq1.should.be.a.Number().lessThan(seq2); + seq2.should.be.a.Number().lessThan(seq3); + fn.calledOnce.should.be.true; }); + describe('Exceptions', () => { + it('should throw an exception', async () => { + const fn = sinon.spy(wait); + const omnesized = Omnesiac(fn, { blocking: true }); + + const wrapper = async (val: unknown = 'Unspecified Value', error?: string): Promise => { + const result = await omnesized('key', 100, val, error); + return { result, seq: counter.next }; + }; + + const pOne = wrapper(1, 'You Been Errored'); + const pTwo = wrapper(2); + const pThree = wrapper(3); + clock.runAll(); + await pOne.should.be.rejectedWith('You Been Errored'); + await clock.runAllAsync(); + await pTwo.should.not.be.rejected(); + await pThree.should.not.be.rejected(); + const { result: result2, seq: seq2 } = await pTwo; + const { result: result3, seq: seq3 } = await pThree; + + result2.should.be.a.Number().eql(2); + result3.should.be.a.Number().eql(result2); + + seq2.should.be.a.Number().greaterThan(1); + seq2.should.be.a.Number().lessThan(seq3); + + fn.calledOnce.should.be.true; + }); + }); }); + describe('ttl = ?', () => { it('should memoize the results of the function until the ttl has expired', async () => { const fn = sinon.spy(wait); - const omnesized = Omnesiac(fn, { blocking: true, ttl: 75 }); + const omnesized = Omnesiac(fn, { blocking: true, ttl: 200 }); - let counter = 1; - const wrapper = async (): Promise => { - const result = await omnesized('key', 50, counter++); - return { result, time: Date.now() }; + const wrapper = async (val: unknown = 'Unspecified Value'): Promise => { + const result = await omnesized('key', 100, val); + return { result, seq: counter.next }; }; - const [{ result: result1, time: time1 }, { result: result2, time: time2 }, { result: result3, time: time3 }] = - await Promise.all([wrapper(), wrapper(), wrapper()]); - - time1.should.be.a.Number().lessThanOrEqual(time2); - time1.should.be.a.Number().lessThanOrEqual(time3); + let resultPromises = Promise.all([wrapper(1), wrapper(2), wrapper(3)]); + await clock.tickAsync(100); + const [{ result: result1 }, { result: result2 }, { result: result3 }] = await resultPromises; should(result1).be.ok(); should(result2).be.ok(); @@ -93,26 +147,76 @@ describe('Omnesiac', () => { fn.callCount.should.be.a.Number().eql(1); - await wait(100); - - const [{ result: result4, time: time4 }, { result: result5, time: time5 }, { result: result6, time: time6 }] = - await Promise.all([wrapper(), wrapper(), wrapper()]); + await clock.tickAsync(50); - time4.should.be.a.Number().greaterThan(time1); - time4.should.be.a.Number().greaterThan(time2); - time4.should.be.a.Number().greaterThan(time3); - time4.should.be.a.Number().lessThanOrEqual(time5); - time4.should.be.a.Number().lessThanOrEqual(time6); + resultPromises = Promise.all([wrapper(4), wrapper(5), wrapper(6)]); + await clock.tickAsync(50); + const [{ result: result4 }, { result: result5 }, { result: result6 }] = await resultPromises; should(result4).be.ok(); should(result5).be.ok(); should(result6).be.ok(); - result4.should.be.a.Number().eql(4); - result5.should.be.a.Number().eql(result4); - result6.should.be.a.Number().eql(result4); + result4.should.be.a.Number().eql(result1); + result5.should.be.a.Number().eql(result1); + result6.should.be.a.Number().eql(result1); + + fn.callCount.should.be.a.Number().eql(1); + + await clock.tickAsync(100); + + resultPromises = Promise.all([wrapper(7), wrapper(8), wrapper(9)]); + await clock.runAllAsync(); + const [{ result: result7 }, { result: result8 }, { result: result9 }] = await resultPromises; + + should(result7).be.ok(); + should(result8).be.ok(); + should(result9).be.ok(); + + result7.should.be.a.Number().eql(7); + result8.should.be.a.Number().eql(result7); + result9.should.be.a.Number().eql(result7); fn.callCount.should.be.a.Number().eql(2); }); + it('should cache forever if ttl = 0 (default)', async () => { + const fn = sinon.spy(wait); + const omnesized = Omnesiac(fn, { blocking: true }); + + const wrapper = async (val: unknown = 'Unspecified Value'): Promise => { + const result = await omnesized('key', 100, val); + return { result, seq: counter.next }; + }; + + let resultPromises = Promise.all([wrapper(1), wrapper(2), wrapper(3)]); + await clock.tickAsync(100); + const [{ result: result1 }, { result: result2 }, { result: result3 }] = await resultPromises; + + should(result1).be.ok(); + should(result2).be.ok(); + should(result3).be.ok(); + + result1.should.be.a.Number().eql(1); + result2.should.be.a.Number().eql(result1); + result3.should.be.a.Number().eql(result1); + + fn.callCount.should.be.a.Number().eql(1); + + await clock.tickAsync(1e9); + + resultPromises = Promise.all([wrapper(4), wrapper(5), wrapper(6)]); + await clock.runAllAsync(); + const [{ result: result4 }, { result: result5 }, { result: result6 }] = await resultPromises; + + should(result4).be.ok(); + should(result5).be.ok(); + should(result6).be.ok(); + + result4.should.be.a.Number().eql(result1); + result5.should.be.a.Number().eql(result1); + result6.should.be.a.Number().eql(result1); + + fn.callCount.should.be.a.Number().eql(1); + }); }); }); From 27769042314916319c5bb8fa26fc923eb981ac33 Mon Sep 17 00:00:00 2001 From: Jonathan Griggs Date: Fri, 16 Sep 2022 07:46:02 -0600 Subject: [PATCH 5/8] Bump major version due to default exception-handling behavior --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 35828f4..abe2e79 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "omnesiac", - "version": "0.0.7-unpublished", + "version": "1.0.0", "description": "Mutually-Exclusive, Asynchronous Memoization", "main": "dist/index.js", "types": "dist/types/index.d.ts", From b19c11162b80282cc6c19a1fd16f3baf6ec49de8 Mon Sep 17 00:00:00 2001 From: Jonathan Griggs Date: Fri, 16 Sep 2022 09:13:38 -0600 Subject: [PATCH 6/8] Refactor OmnesiacCache.spec to use sinon fake-timers --- src/OmnesiacCache.spec.ts | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/src/OmnesiacCache.spec.ts b/src/OmnesiacCache.spec.ts index 294c215..310ed67 100644 --- a/src/OmnesiacCache.spec.ts +++ b/src/OmnesiacCache.spec.ts @@ -2,13 +2,16 @@ import should = require('should'); import OmnesiacCache from './OmnesiacCache'; import * as sinon from 'sinon'; -function wait(ms: number, result?: unknown): Promise { - return new Promise((resolve) => { - setTimeout(() => resolve(result), ms); +describe('OmnesiacCache', () => { + let clock: sinon.SinonFakeTimers; + + before(() => { + clock = sinon.useFakeTimers(); }); -} -describe('OmnesiacCache', () => { + after(() => { + clock.restore(); + }); describe('set()', () => { it('should cache a value, and remove after TTL has expired', async () => { const cache = new OmnesiacCache(); @@ -19,11 +22,11 @@ describe('OmnesiacCache', () => { cache.set(key, { ttl }); cache.set(otherKey, { ttl: 0 }); - await wait(ttl / 2); + await clock.tickAsync(ttl / 2); cache.get(key).should.be.an.Object().with.property('ttl').eql(ttl); removeSpy.calledOnceWith(key).should.be.false(); - await wait(ttl / 2 + 1); + await clock.tickAsync(ttl / 2); removeSpy.calledOnceWith(key).should.be.true(); should(cache.get(key)).not.be.ok(); @@ -39,7 +42,7 @@ describe('OmnesiacCache', () => { cache.set(key, { ttl }); cache.set(otherKey, { ttl: 0 }); - await wait(50); + await clock.tickAsync(50); cache.get(key).should.be.an.Object().with.property('ttl').eql(ttl); From 1cc0e1435c89b8b46ecdc490161142c0e3b25891 Mon Sep 17 00:00:00 2001 From: Jonathan Griggs Date: Fri, 16 Sep 2022 09:46:36 -0600 Subject: [PATCH 7/8] Let's get CI going with Github Actions --- .github/workflows/node.js.yml | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) create mode 100644 .github/workflows/node.js.yml diff --git a/.github/workflows/node.js.yml b/.github/workflows/node.js.yml new file mode 100644 index 0000000..21663ee --- /dev/null +++ b/.github/workflows/node.js.yml @@ -0,0 +1,29 @@ +# This workflow will do a clean installation of node dependencies, cache/restore them, build the source code and run tests across different versions of node +# For more information see: https://help.github.com/actions/language-and-framework-guides/using-nodejs-with-github-actions + +name: Node.js CI + +on: + push: + branches: ['main'] + pull_request: + branches: ['main'] + +jobs: + build: + runs-on: ubuntu-latest + + strategy: + matrix: + node-version: [14.x, 16.x, 18.x] + # See supported Node.js release schedule at https://nodejs.org/en/about/releases/ + + steps: + - uses: actions/checkout@v3 + - name: Use Node.js ${{ matrix.node-version }} + uses: actions/setup-node@v3 + with: + node-version: ${{ matrix.node-version }} + cache: 'yarn' + - run: yarn install + - run: yarn run coverage From 15e85533440e49a2d317ac93ff68522c98310dfe Mon Sep 17 00:00:00 2001 From: Jonathan Griggs Date: Fri, 16 Sep 2022 10:07:38 -0600 Subject: [PATCH 8/8] Get the branch name correct --- .github/workflows/node.js.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/node.js.yml b/.github/workflows/node.js.yml index 21663ee..98e16ad 100644 --- a/.github/workflows/node.js.yml +++ b/.github/workflows/node.js.yml @@ -5,9 +5,9 @@ name: Node.js CI on: push: - branches: ['main'] + branches: ['main', 'master'] pull_request: - branches: ['main'] + branches: ['main', 'master'] jobs: build: