From 8d48d0559b9fd3a71bc9e9b6e90f7f134138d434 Mon Sep 17 00:00:00 2001 From: Ruslan Lopatin Date: Wed, 24 Jun 2020 12:11:59 +0600 Subject: [PATCH] Optimize async operations (#11) * Reduce the promise awaits * Avoid explicit Promise construction * Split processing agent implementations --- src/core/request-processor.ts | 121 ++++++++++++++++++++------------- src/http/http-listener.spec.ts | 51 ++++++++------ 2 files changed, 105 insertions(+), 67 deletions(-) diff --git a/src/core/request-processor.ts b/src/core/request-processor.ts index 8fa080c..b2fbb3e 100644 --- a/src/core/request-processor.ts +++ b/src/core/request-processor.ts @@ -80,7 +80,7 @@ export function requestProcessor( */ abstract class RequestProcessorAgent { - abstract readonly context: Promise>; + abstract readonly context: RequestContext; protected constructor(readonly config: RequestProcessor.Config) { } @@ -90,15 +90,15 @@ abstract class RequestProcessorAgent { modification?: RequestModification | RequestModifier, ): Promise { - let context: Promise>; + let context: RequestContext; if (modification) { - context = new ModifiedRequestProcessorAgent(this, modification).context; + context = (await nextRequestProcessorAgent(this, modification)).context; } else { - context = this.context as Promise>; + context = this.context as RequestContext; } - return this.config.next(handler, await context); + return this.config.next(handler, context); } abstract modify( @@ -118,15 +118,15 @@ abstract class RequestProcessorAgent { */ class RootRequestProcessorAgent extends RequestProcessorAgent { - readonly context: Promise>; + readonly context: RequestContext; constructor(config: RequestProcessor.Config, means: TMeans) { super(config); - this.context = Promise.resolve({ + this.context = { ...means, next: this.next.bind(this), modifiedBy: this.modifiedBy, - }); + }; } modifiedBy(): undefined { @@ -145,10 +145,27 @@ class RootRequestProcessorAgent extends RequestProcessorAgent +async function nextRequestProcessorAgent( + prev: RequestProcessorAgent, + modification: RequestModification | RequestModifier, +): Promise> { + if (isRequestModifier(modification)) { + return new ModifierRequestProcessingAgent( + prev, + modification, + await prev.modify(await modification.modification(prev.context)), + ); + } + return new ModificationRequestProcessorAgent(prev, await prev.modify(modification)); +} + +/** + * @internal + */ +class ModificationRequestProcessorAgent extends RequestProcessorAgent { - readonly context: Promise>; + readonly context: RequestContext; readonly modify: ( this: void, @@ -165,51 +182,63 @@ class ModifiedRequestProcessorAgent modification: RequestModification | RequestModifier, ) { super(prev.config); + this.modify = prev.modify as this['modify']; + this.modifiedBy = prev.modifiedBy as this['modifiedBy']; + this.context = { + ...prev.context, + ...modification, + next: this.next.bind(this), + modifiedBy: this.modifiedBy, + } as RequestContext; + } - let modify = prev.modify as ( - this: void, - modification: RequestModification, - ) => Promise>; - - let modPromise: Promise>; +} - if (isRequestModifier(modification)) { +/** + * @internal + */ +class ModifierRequestProcessingAgent + extends RequestProcessorAgent { - const modifier = modification; + readonly context: RequestContext; - modPromise = prev.context - .then(ctx => modifier.modification(ctx)) - .then(mod => prev.modify(mod)); + readonly modify: ( + this: void, + modification: RequestModification, + ) => Promise>; - if (modifier.modifyNext) { - modify = async (mod: RequestModification) => prev.modify( - modifier.modifyNext!(await this.context, mod) as RequestModification, - ) as Promise>; - } + readonly modifiedBy: ( + this: void, + ref: RequestModifierRef, + ) => RequestContext | undefined; - this.modifiedBy = ( - ref: RequestModifierRef, - ) => (modifier[RequestModifier__symbol] === ref[RequestModifier__symbol] - ? this.context - : prev.modifiedBy(ref) - ) as RequestContext | undefined; + constructor( + prev: RequestProcessorAgent, + modifier: RequestModifier, + modification: RequestModification | RequestModifier, + ) { + super(prev.config); + if (modifier.modifyNext) { + this.modify = async (mod: RequestModification) => prev.modify( + modifier.modifyNext!(this.context, mod) as RequestModification, + ) as Promise>; } else { - modPromise = prev.modify(modification); - this.modifiedBy = prev.modifiedBy as ( - this: void, - ref: RequestModifierRef, - ) => RequestContext | undefined; + this.modify = prev.modify as this['modify']; } - - this.modify = modify; - this.context = Promise.all([prev.context, modPromise]) - .then(([prevContext, mod]) => ({ - ...prevContext, - ...mod, - next: this.next.bind(this), - modifiedBy: this.modifiedBy, - } as RequestContext)); + this.modifiedBy = ( + ref: RequestModifierRef, + ) => (modifier[RequestModifier__symbol] === ref[RequestModifier__symbol] + ? this.context + : prev.modifiedBy(ref) + ) as RequestContext | undefined; + + this.context = { + ...prev.context, + ...modification, + next: this.next.bind(this), + modifiedBy: this.modifiedBy, + } as RequestContext; } } diff --git a/src/http/http-listener.spec.ts b/src/http/http-listener.spec.ts index d4fdf46..929efb9 100644 --- a/src/http/http-listener.spec.ts +++ b/src/http/http-listener.spec.ts @@ -21,6 +21,15 @@ describe('httpListener', () => { server.listener.mockReset(); }); + let logErrorSpy: jest.SpyInstance; + + beforeEach(() => { + logErrorSpy = jest.spyOn(suppressedLog, 'error'); + }); + afterEach(() => { + logErrorSpy.mockRestore(); + }); + it('invokes handler', async () => { const handler = jest.fn(({ response }: RequestContext) => { @@ -146,13 +155,14 @@ describe('httpListener', () => { it('logs error and invokes provided error handler', async () => { const error = new Error('test'); - const log = suppressedLog; - const logErrorSpy = jest.spyOn(log, 'error'); const errorHandler = jest.fn(({ response, error }: RequestContext) => { response.end(`ERROR ${error.message}`); }); - server.listener.mockImplementation(httpListener({ log, errorHandler }, () => { throw error; })); + server.listener.mockImplementation(httpListener( + { log:suppressedLog, errorHandler }, + () => { throw error; }, + )); const response = await server.get('/test'); @@ -166,13 +176,14 @@ describe('httpListener', () => { it('logs HTTP error and invokes provided error handler', async () => { const error = new HttpError(404, { details: 'Never Found' }); - const log = suppressedLog; - const logErrorSpy = jest.spyOn(log, 'error'); const errorHandler = jest.fn(({ response, error }: RequestContext) => { response.end(`ERROR ${error.message} ${error.details}`); }); - server.listener.mockImplementation(httpListener({ log, errorHandler }, () => { throw error; })); + server.listener.mockImplementation(httpListener( + { log: suppressedLog, errorHandler }, + () => { throw error; }, + )); const response = await server.get('/test'); const body = await response.body(); @@ -187,10 +198,11 @@ describe('httpListener', () => { it('logs ERROR when there is no error handler', async () => { const error = new Error('test'); - const log = suppressedLog; - const logErrorSpy = jest.spyOn(log, 'error'); - server.listener.mockImplementation(httpListener({ log, errorHandler: false }, () => { throw error; })); + server.listener.mockImplementation(httpListener( + { log: suppressedLog, errorHandler: false }, + () => { throw error; }, + )); const response = await server.get('/test'); @@ -200,18 +212,14 @@ describe('httpListener', () => { it('logs ERROR when there is neither error, not default handler', async () => { const error = new Error('test'); - const log = suppressedLog; - const logErrorSpy = jest.spyOn(log, 'error'); const listener = httpListener( - { log, defaultHandler: false, errorHandler: false }, + { log: suppressedLog, defaultHandler: false, errorHandler: false }, () => { throw error; }, ); server.listener.mockImplementation((request, response) => { listener(request, response); - Promise.resolve().finally(() => { - response.end('NO RESPONSE'); - }); + response.end('NO RESPONSE'); }); await server.get('/test'); @@ -223,27 +231,28 @@ describe('httpListener', () => { it('logs unhandled errors', async () => { const error = new Error('test'); - const log = suppressedLog; - const logErrorSpy = jest.spyOn(log, 'error'); const errorHandler = jest.fn(() => { throw error; }); + const whenErrorLogged = new Promise(resolve => { + logErrorSpy.mockImplementation(resolve); + }); const listener = httpListener( - { log, errorHandler }, + { log: suppressedLog, errorHandler }, () => { throw error; }, ); server.listener.mockImplementation((request, response) => { listener(request, response); - Promise.resolve().finally(() => { - response.end('NO RESPONSE'); - }); + response.end('NO RESPONSE'); }); const response = await server.get('/test'); expect(await response.body()).toBe('NO RESPONSE'); + + await whenErrorLogged; expect(logErrorSpy).toHaveBeenCalledWith('[GET /test]', 'Unhandled error', error); });