From 4a4005bd4613e0cd6147cf0b789be40b14b70584 Mon Sep 17 00:00:00 2001 From: Hitesh Shrestha Date: Tue, 20 Aug 2024 16:42:47 +0545 Subject: [PATCH 1/4] Fix: return response from callback --- src/impl/domain.ts | 2 +- src/index.ts | 2 +- test/domain.test.ts | 115 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 117 insertions(+), 2 deletions(-) diff --git a/src/impl/domain.ts b/src/impl/domain.ts index dfee2f5..6c131f3 100644 --- a/src/impl/domain.ts +++ b/src/impl/domain.ts @@ -28,7 +28,7 @@ export function initialize(callback: (err?: any) => void, params?: AsyncStorePar d[STORE_KEY] = Object.create(null); d[ID_KEY] = randomUUID(); - d.run(callback); + return d.run(callback); } /** diff --git a/src/index.ts b/src/index.ts index 5d1a15f..d31a2a4 100644 --- a/src/index.ts +++ b/src/index.ts @@ -111,7 +111,7 @@ export function initialize(adapter: AsyncStoreAdapter = AsyncStoreAdapter.DOMAIN return (callback: (err?: any) => void, params?: AsyncStoreParams) => { initializedAdapter = adapter; - instance.initialize(callback, params); + return instance.initialize(callback, params); }; } diff --git a/test/domain.test.ts b/test/domain.test.ts index 5c1547b..8b4173a 100644 --- a/test/domain.test.ts +++ b/test/domain.test.ts @@ -839,5 +839,120 @@ describe('store: [adapter=DOMAIN]', () => { globalStore.initialize(adapter)(callback); }); }); + + it("should work even if the store is initialized under active domain w/o affecting the existing domain's attributes.", (done) => { + // Existing domain. + const d = domain.create() as any; + + d.existingData = 'Hello world'; + + const callback = () => { + Promise.resolve().then(first).then(second).then(third).then(done).catch(done); + }; + + const first = () => { + globalStore.set({ foo: 'foo' }); + }; + + const second = () => { + // Store should still have the data set. + expect(globalStore.get('foo')).to.equal('foo'); + + // And the existing data in the domain before our store + // was initialized should still be there. + expect((process.domain as any).existingData).to.equal('Hello world'); + }; + + const third = () => { + // Ensure the same existing domain is used instead of creating a new one. + expect(process.domain).to.equal(d); + }; + + d.run(() => { + // Ensure data in the existing domain is available at this point. + expect((process.domain as any).existingData).to.equal('Hello world'); + + globalStore.initialize(adapter)(callback); + }); + }); + it("should work even if the store is initialized under active domain w/o affecting the existing domain's attributes.", (done) => { + // Existing domain. + const d = domain.create() as any; + + d.existingData = 'Hello world'; + + const callback = () => { + Promise.resolve().then(first).then(second).then(third).then(done).catch(done); + }; + + const first = () => { + globalStore.set({ foo: 'foo' }); + }; + + const second = () => { + // Store should still have the data set. + expect(globalStore.get('foo')).to.equal('foo'); + + // And the existing data in the domain before our store + // was initialized should still be there. + expect((process.domain as any).existingData).to.equal('Hello world'); + }; + + const third = () => { + // Ensure the same existing domain is used instead of creating a new one. + expect(process.domain).to.equal(d); + }; + + d.run(() => { + // Ensure data in the existing domain is available at this point. + expect((process.domain as any).existingData).to.equal('Hello world'); + + globalStore.initialize(adapter)(callback); + }); + }); + + it('should return the same response from the callback function.', async () => { + const callback = async () => { + globalStore.set({ foo: 'foo' }); + + return Promise.resolve('Hello world'); + }; + + const response = await globalStore.initialize(adapter)(callback); + + expect(response).to.equal('Hello world'); + }); + }); + + describe('Error Handling:', () => { + it('should bubble up the promise rejection from the callback.', async () => { + const callback = async () => { + globalStore.set({ foo: 'foo' }); + + return Promise.reject('Hello world'); + }; + + try { + await globalStore.initialize(adapter)(callback); + } catch (e) { + expect(e).to.equal('Hello world'); + } + }); + + it('should bubble up the error thrown from the callback.', async () => { + const callback = async () => { + globalStore.set({ foo: 'foo' }); + + throw new Error('Hello world'); + }; + + try { + await globalStore.initialize(adapter)(callback); + } catch (e) { + if (e instanceof Error) { + expect(e.message).to.equal('Hello world'); + } + } + }); }); }); From c642fa45a2f1140ea042cab64a784d1c514fdce7 Mon Sep 17 00:00:00 2001 From: Hitesh Shrestha Date: Wed, 21 Aug 2024 14:06:18 +0545 Subject: [PATCH 2/4] Test update and resolution for PR comments --- src/index.ts | 2 +- test/domain.test.ts | 101 ++++++++++++-------------------------------- 2 files changed, 27 insertions(+), 76 deletions(-) diff --git a/src/index.ts b/src/index.ts index d31a2a4..1f96e71 100644 --- a/src/index.ts +++ b/src/index.ts @@ -99,7 +99,7 @@ export function initializeFastifyPlugin(adapter: AsyncStoreAdapter = AsyncStoreA * Initialize the async store based on the adapter provided. * * @param {AsyncStoreAdapter} [adapter=AsyncStoreAdapter.DOMAIN] - * @returns {(params: AsyncStoreParams) => void} + * @returns {(params: AsyncStoreParams) => any} */ export function initialize(adapter: AsyncStoreAdapter = AsyncStoreAdapter.DOMAIN) { if (isInitialized()) { diff --git a/test/domain.test.ts b/test/domain.test.ts index 8b4173a..eb98369 100644 --- a/test/domain.test.ts +++ b/test/domain.test.ts @@ -840,119 +840,70 @@ describe('store: [adapter=DOMAIN]', () => { }); }); - it("should work even if the store is initialized under active domain w/o affecting the existing domain's attributes.", (done) => { - // Existing domain. - const d = domain.create() as any; - - d.existingData = 'Hello world'; - - const callback = () => { - Promise.resolve().then(first).then(second).then(third).then(done).catch(done); - }; - - const first = () => { + it('should return the response from async callback function.', async () => { + const callback = async () => { globalStore.set({ foo: 'foo' }); - }; - - const second = () => { - // Store should still have the data set. - expect(globalStore.get('foo')).to.equal('foo'); - - // And the existing data in the domain before our store - // was initialized should still be there. - expect((process.domain as any).existingData).to.equal('Hello world'); - }; - const third = () => { - // Ensure the same existing domain is used instead of creating a new one. - expect(process.domain).to.equal(d); - }; + functionAccessingStore(); + const response = await asyncTask(); - d.run(() => { - // Ensure data in the existing domain is available at this point. - expect((process.domain as any).existingData).to.equal('Hello world'); - - globalStore.initialize(adapter)(callback); - }); - }); - it("should work even if the store is initialized under active domain w/o affecting the existing domain's attributes.", (done) => { - // Existing domain. - const d = domain.create() as any; - - d.existingData = 'Hello world'; - - const callback = () => { - Promise.resolve().then(first).then(second).then(third).then(done).catch(done); + return response; }; - const first = () => { - globalStore.set({ foo: 'foo' }); - }; - - const second = () => { - // Store should still have the data set. + const functionAccessingStore = () => { expect(globalStore.get('foo')).to.equal('foo'); - - // And the existing data in the domain before our store - // was initialized should still be there. - expect((process.domain as any).existingData).to.equal('Hello world'); }; - const third = () => { - // Ensure the same existing domain is used instead of creating a new one. - expect(process.domain).to.equal(d); - }; - - d.run(() => { - // Ensure data in the existing domain is available at this point. - expect((process.domain as any).existingData).to.equal('Hello world'); - - globalStore.initialize(adapter)(callback); - }); - }); - - it('should return the same response from the callback function.', async () => { - const callback = async () => { - globalStore.set({ foo: 'foo' }); - - return Promise.resolve('Hello world'); + const asyncTask = () => { + return new Promise((resolve) => { + setTimeout(() => { + resolve(globalStore.get('foo')); + }, 1); + }); }; const response = await globalStore.initialize(adapter)(callback); - - expect(response).to.equal('Hello world'); + expect(response).to.equal('foo'); }); }); describe('Error Handling:', () => { it('should bubble up the promise rejection from the callback.', async () => { - const callback = async () => { + const callback = () => { globalStore.set({ foo: 'foo' }); - return Promise.reject('Hello world'); + return new Promise((resolve, rejects) => { + setTimeout(() => { + rejects('Hello world'); + }, 1); + }); }; try { await globalStore.initialize(adapter)(callback); + expect.fail('Should not reach here.'); } catch (e) { expect(e).to.equal('Hello world'); } }); - it('should bubble up the error thrown from the callback.', async () => { - const callback = async () => { + it('should bubble up the error thrown from the callback.', (done) => { + const callback = () => { globalStore.set({ foo: 'foo' }); throw new Error('Hello world'); }; try { - await globalStore.initialize(adapter)(callback); + globalStore.initialize(adapter)(callback); + expect.fail('Should not reach here.'); } catch (e) { if (e instanceof Error) { expect(e.message).to.equal('Hello world'); } } + + done(); }); }); }); From be02235b549d9487ce83aa16a68698a35529d47f Mon Sep 17 00:00:00 2001 From: Hitesh Shrestha Date: Wed, 21 Aug 2024 14:26:45 +0545 Subject: [PATCH 3/4] Update doc and interface --- src/AsyncStore.ts | 2 +- src/index.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/AsyncStore.ts b/src/AsyncStore.ts index b3c5e13..a385e82 100644 --- a/src/AsyncStore.ts +++ b/src/AsyncStore.ts @@ -4,7 +4,7 @@ import AsyncStoreParams from './AsyncStoreParams'; * Async Store implementation contract. */ interface AsyncStore { - initialize: (callback: (err?: any) => void, params?: AsyncStoreParams) => void; + initialize: (callback: (err?: any) => void, params?: AsyncStoreParams) => any; set: (properties: any) => void; get: (key: string) => any; getAll: () => any; diff --git a/src/index.ts b/src/index.ts index 1f96e71..1ba3276 100644 --- a/src/index.ts +++ b/src/index.ts @@ -99,7 +99,7 @@ export function initializeFastifyPlugin(adapter: AsyncStoreAdapter = AsyncStoreA * Initialize the async store based on the adapter provided. * * @param {AsyncStoreAdapter} [adapter=AsyncStoreAdapter.DOMAIN] - * @returns {(params: AsyncStoreParams) => any} + * @returns {(callback: (err?: any) => void, params?: AsyncStoreParams) => any } */ export function initialize(adapter: AsyncStoreAdapter = AsyncStoreAdapter.DOMAIN) { if (isInitialized()) { From cd44aeceabb12f1f9c37733929ba3621bb618c36 Mon Sep 17 00:00:00 2001 From: Hitesh Shrestha Date: Wed, 21 Aug 2024 15:54:49 +0545 Subject: [PATCH 4/4] Add a test to verify response from non-async callback function --- test/domain.test.ts | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/test/domain.test.ts b/test/domain.test.ts index eb98369..231e1e1 100644 --- a/test/domain.test.ts +++ b/test/domain.test.ts @@ -840,6 +840,23 @@ describe('store: [adapter=DOMAIN]', () => { }); }); + it('should return the response from callback function.', (done) => { + const callback = () => { + globalStore.set({ foo: 'foo' }); + + return functionAccessingStore(); + }; + + const functionAccessingStore = () => { + return globalStore.get('foo'); + }; + + const response = globalStore.initialize(adapter)(callback); + expect(response).to.equal('foo'); + + done(); + }); + it('should return the response from async callback function.', async () => { const callback = async () => { globalStore.set({ foo: 'foo' }); @@ -872,9 +889,9 @@ describe('store: [adapter=DOMAIN]', () => { const callback = () => { globalStore.set({ foo: 'foo' }); - return new Promise((resolve, rejects) => { + return new Promise((resolve, reject) => { setTimeout(() => { - rejects('Hello world'); + reject('Hello world'); }, 1); }); };