From ba47ce79519e950b6a2d5f210ce266796052131a Mon Sep 17 00:00:00 2001 From: Ruben Verborgh Date: Tue, 8 Dec 2020 19:18:08 +0000 Subject: [PATCH] change: Refactor AllVoidCompositeHandler into SequenceHandler. --- config/presets/http.json | 4 +- config/presets/init.json | 4 +- config/presets/ldp/response-writer.json | 4 +- config/presets/middleware.json | 4 +- src/index.ts | 2 +- src/util/AllVoidCompositeHandler.ts | 24 ------- src/util/AsyncHandler.ts | 8 +-- src/util/SequenceHandler.ts | 32 ++++++++++ test/configs/Util.ts | 5 +- .../unit/util/AllVoidCompositeHandler.test.ts | 30 --------- test/unit/util/SequenceHandler.test.ts | 64 +++++++++++++++++++ 11 files changed, 112 insertions(+), 69 deletions(-) delete mode 100644 src/util/AllVoidCompositeHandler.ts create mode 100644 src/util/SequenceHandler.ts delete mode 100644 test/unit/util/AllVoidCompositeHandler.test.ts create mode 100644 test/unit/util/SequenceHandler.test.ts diff --git a/config/presets/http.json b/config/presets/http.json index 108128927..40f02dc1e 100644 --- a/config/presets/http.json +++ b/config/presets/http.json @@ -20,8 +20,8 @@ }, { "@id": "urn:solid-server:default:HttpHandler", - "@type": "AllVoidCompositeHandler", - "AllVoidCompositeHandler:_handlers": [ + "@type": "SequenceHandler", + "SequenceHandler:_handlers": [ { "@id": "urn:solid-server:default:Middleware" }, diff --git a/config/presets/init.json b/config/presets/init.json index a29fa7f92..a7aff9a01 100644 --- a/config/presets/init.json +++ b/config/presets/init.json @@ -3,8 +3,8 @@ "@graph": [ { "@id": "urn:solid-server:default:Initializer", - "@type": "AllVoidCompositeHandler", - "AllVoidCompositeHandler:_handlers": [ + "@type": "SequenceHandler", + "SequenceHandler:_handlers": [ { "@type": "LoggerInitializer", "LoggerInitializer:_loggerFactory": { diff --git a/config/presets/ldp/response-writer.json b/config/presets/ldp/response-writer.json index 778e23658..a92c1342a 100644 --- a/config/presets/ldp/response-writer.json +++ b/config/presets/ldp/response-writer.json @@ -3,8 +3,8 @@ "@graph": [ { "@id": "urn:solid-server:default:MetadataSerializer", - "@type": "AllVoidCompositeHandler", - "AllVoidCompositeHandler:_handlers": [ + "@type": "SequenceHandler", + "SequenceHandler:_handlers": [ { "@type": "MappedMetadataWriter", "MappedMetadataWriter:_headerMap": [ diff --git a/config/presets/middleware.json b/config/presets/middleware.json index 564319ad6..134a37874 100644 --- a/config/presets/middleware.json +++ b/config/presets/middleware.json @@ -3,8 +3,8 @@ "@graph": [ { "@id": "urn:solid-server:default:Middleware", - "@type": "AllVoidCompositeHandler", - "AllVoidCompositeHandler:_handlers": [ + "@type": "SequenceHandler", + "SequenceHandler:_handlers": [ { "@type": "CorsHandler", "CorsHandler:_options_methods": [ diff --git a/src/index.ts b/src/index.ts index 6d145f8d6..0987f4d13 100644 --- a/src/index.ts +++ b/src/index.ts @@ -181,10 +181,10 @@ export * from './util/locking/SingleThreadedResourceLocker'; export * from './util/locking/WrappedExpiringResourceLocker'; // Util -export * from './util/AllVoidCompositeHandler'; export * from './util/AsyncHandler'; export * from './util/FirstCompositeHandler'; export * from './util/HeaderUtil'; export * from './util/PathUtil'; export * from './util/QuadUtil'; +export * from './util/SequenceHandler'; export * from './util/StreamUtil'; diff --git a/src/util/AllVoidCompositeHandler.ts b/src/util/AllVoidCompositeHandler.ts deleted file mode 100644 index b7442ca94..000000000 --- a/src/util/AllVoidCompositeHandler.ts +++ /dev/null @@ -1,24 +0,0 @@ -import { AsyncHandler } from './AsyncHandler'; - -/** - * A composite handler that runs all of its handlers independent of their result. - * The `canHandle` check of this handler will always succeed. - */ -export class AllVoidCompositeHandler extends AsyncHandler { - private readonly handlers: AsyncHandler[]; - - public constructor(handlers: AsyncHandler[]) { - super(); - this.handlers = handlers; - } - - public async handle(input: TIn): Promise { - for (const handler of this.handlers) { - try { - await handler.handleSafe(input); - } catch { - // Ignore errors - } - } - } -} diff --git a/src/util/AsyncHandler.ts b/src/util/AsyncHandler.ts index 3658565a6..f94567322 100644 --- a/src/util/AsyncHandler.ts +++ b/src/util/AsyncHandler.ts @@ -1,7 +1,7 @@ /** * Simple interface for classes that can potentially handle a specific kind of data asynchronously. */ -export abstract class AsyncHandler { +export abstract class AsyncHandler { /** * Checks if the input data can be handled by this class. * Throws an error if it can't handle the data. @@ -10,7 +10,7 @@ export abstract class AsyncHandler { * @returns A promise resolving if this input can be handled, rejecting with an Error if not. */ // eslint-disable-next-line @typescript-eslint/no-unused-vars - public async canHandle(input: TInput): Promise { + public async canHandle(input: TIn): Promise { // Support any input by default } @@ -20,7 +20,7 @@ export abstract class AsyncHandler { * * @returns A promise resolving when the handling is finished. Return value depends on the given type. */ - public abstract handle(input: TInput): Promise; + public abstract handle(input: TIn): Promise; /** * Helper function that first runs the canHandle function followed by the handle function. @@ -30,7 +30,7 @@ export abstract class AsyncHandler { * * @returns The result of the handle function of the handler. */ - public async handleSafe(data: TInput): Promise { + public async handleSafe(data: TIn): Promise { await this.canHandle(data); return this.handle(data); diff --git a/src/util/SequenceHandler.ts b/src/util/SequenceHandler.ts new file mode 100644 index 000000000..f84678cb9 --- /dev/null +++ b/src/util/SequenceHandler.ts @@ -0,0 +1,32 @@ +import { AsyncHandler } from './AsyncHandler'; + +/** + * A composite handler that will try to run all supporting handlers sequentially + * and return the value of the last supported handler. + * The `canHandle` check of this handler will always succeed. + */ +export class SequenceHandler extends AsyncHandler { + private readonly handlers: AsyncHandler[]; + + public constructor(handlers: AsyncHandler[]) { + super(); + this.handlers = [ ...handlers ]; + } + + public async handle(input: TIn): Promise { + let result: TOut | undefined; + for (const handler of this.handlers) { + let supported: boolean; + try { + await handler.canHandle(input); + supported = true; + } catch { + supported = false; + } + if (supported) { + result = await handler.handle(input); + } + } + return result; + } +} diff --git a/test/configs/Util.ts b/test/configs/Util.ts index 8f530bdb1..b8720e3dc 100644 --- a/test/configs/Util.ts +++ b/test/configs/Util.ts @@ -13,7 +13,7 @@ import type { OperationHandler, } from '../../src/index'; import { - AcceptPreferenceParser, AllVoidCompositeHandler, + AcceptPreferenceParser, BasicMetadataExtractor, BasicRequestParser, BasicResponseWriter, @@ -35,6 +35,7 @@ import { PutOperationHandler, RawBodyParser, RepresentationConvertingStore, + SequenceHandler, SingleThreadedResourceLocker, SlugParser, SparqlUpdatePatchHandler, @@ -117,7 +118,7 @@ export const getOperationHandler = (store: ResourceStore): OperationHandler => { }; export const getResponseWriter = (): ResponseWriter => { - const serializer = new AllVoidCompositeHandler([ + const serializer = new SequenceHandler([ new MappedMetadataWriter({ [CONTENT_TYPE]: 'content-type', [HTTP.location]: 'location', diff --git a/test/unit/util/AllVoidCompositeHandler.test.ts b/test/unit/util/AllVoidCompositeHandler.test.ts deleted file mode 100644 index aec3a9beb..000000000 --- a/test/unit/util/AllVoidCompositeHandler.test.ts +++ /dev/null @@ -1,30 +0,0 @@ -import { AllVoidCompositeHandler } from '../../../src/util/AllVoidCompositeHandler'; -import type { AsyncHandler } from '../../../src/util/AsyncHandler'; - -describe('An AllVoidCompositeHandler', (): void => { - let handler1: AsyncHandler; - let handler2: AsyncHandler; - let composite: AllVoidCompositeHandler; - - beforeEach(async(): Promise => { - handler1 = { handleSafe: jest.fn() } as any; - handler2 = { handleSafe: jest.fn() } as any; - - composite = new AllVoidCompositeHandler([ handler1, handler2 ]); - }); - - it('can handle all input.', async(): Promise => { - await expect(composite.canHandle('test')).resolves.toBeUndefined(); - }); - - it('runs all handlers without caring about their result.', async(): Promise => { - handler1.handleSafe = jest.fn(async(): Promise => { - throw new Error('error'); - }); - await expect(composite.handleSafe('test')).resolves.toBeUndefined(); - expect(handler1.handleSafe).toHaveBeenCalledTimes(1); - expect(handler1.handleSafe).toHaveBeenLastCalledWith('test'); - expect(handler2.handleSafe).toHaveBeenCalledTimes(1); - expect(handler2.handleSafe).toHaveBeenLastCalledWith('test'); - }); -}); diff --git a/test/unit/util/SequenceHandler.test.ts b/test/unit/util/SequenceHandler.test.ts new file mode 100644 index 000000000..8c6be9959 --- /dev/null +++ b/test/unit/util/SequenceHandler.test.ts @@ -0,0 +1,64 @@ +import type { AsyncHandler } from '../../../src/util/AsyncHandler'; +import { SequenceHandler } from '../../../src/util/SequenceHandler'; + +describe('A SequenceHandler', (): void => { + const handlers: jest.Mocked>[] = [ + { + canHandle: jest.fn(), + handle: jest.fn().mockResolvedValue('0'), + } as any, + { + canHandle: jest.fn().mockRejectedValue(new Error('not supported')), + handle: jest.fn().mockRejectedValue(new Error('should not be called')), + } as any, + { + canHandle: jest.fn(), + handle: jest.fn().mockResolvedValue('2'), + } as any, + ]; + let composite: SequenceHandler; + + beforeEach(async(): Promise => { + composite = new SequenceHandler(handlers); + }); + + it('can handle all input.', async(): Promise => { + await expect(composite.canHandle('test')).resolves.toBeUndefined(); + }); + + it('runs all supported handlers.', async(): Promise => { + await composite.handleSafe('test'); + + expect(handlers[0].canHandle).toHaveBeenCalledTimes(1); + expect(handlers[0].canHandle).toHaveBeenLastCalledWith('test'); + expect(handlers[0].handle).toHaveBeenCalledTimes(1); + expect(handlers[0].handle).toHaveBeenLastCalledWith('test'); + + expect(handlers[1].canHandle).toHaveBeenCalledTimes(1); + expect(handlers[1].canHandle).toHaveBeenLastCalledWith('test'); + expect(handlers[1].handle).toHaveBeenCalledTimes(0); + + expect(handlers[2].canHandle).toHaveBeenCalledTimes(1); + expect(handlers[2].canHandle).toHaveBeenLastCalledWith('test'); + expect(handlers[2].handle).toHaveBeenCalledTimes(1); + expect(handlers[2].handle).toHaveBeenLastCalledWith('test'); + }); + + it('returns the result of the last supported handler.', async(): Promise => { + await expect(composite.handleSafe('test')).resolves.toBe('2'); + + handlers[2].canHandle.mockRejectedValueOnce(new Error('not supported')); + await expect(composite.handleSafe('test')).resolves.toBe('0'); + }); + + it('returns undefined if no handler is supported.', async(): Promise => { + handlers[0].canHandle.mockRejectedValueOnce(new Error('not supported')); + handlers[2].canHandle.mockRejectedValueOnce(new Error('not supported')); + await expect(composite.handleSafe('test')).resolves.toBeUndefined(); + }); + + it('errors if a handler errors.', async(): Promise => { + handlers[2].handle.mockRejectedValueOnce(new Error('failure')); + await expect(composite.handleSafe('test')).rejects.toThrow('failure'); + }); +});