From 4229932a3ac75c2532da4e495e96b779fc5b6c92 Mon Sep 17 00:00:00 2001 From: Joachim Van Herwegen Date: Thu, 28 May 2020 10:55:29 +0200 Subject: [PATCH] feat: add CompositeAsyncHandler to support multiple handlers --- jest.config.js | 16 ++-- package-lock.json | 24 ++++-- package.json | 4 +- src/authentication/CredentialsExtractor.ts | 2 +- src/authorization/Authorizer.ts | 2 +- src/ldp/http/RequestParser.ts | 2 +- src/ldp/operations/OperationHandler.ts | 2 +- src/ldp/permissions/PermissionsExtractor.ts | 2 +- src/server/HttpHandler.ts | 2 +- src/util/AsyncHandler.ts | 26 +++++- src/util/CompositeAsyncHandler.ts | 86 ++++++++++++++++++++ test/unit/util/AsyncHandler.test.ts | 30 +++++++ test/unit/util/CompositeAsyncHandler.test.ts | 76 +++++++++++++++++ test/util/StaticAsyncHandler.ts | 24 ++++++ 14 files changed, 275 insertions(+), 23 deletions(-) create mode 100644 src/util/CompositeAsyncHandler.ts create mode 100644 test/unit/util/AsyncHandler.test.ts create mode 100644 test/unit/util/CompositeAsyncHandler.test.ts create mode 100644 test/util/StaticAsyncHandler.ts diff --git a/jest.config.js b/jest.config.js index 98aa9b990..345d28032 100644 --- a/jest.config.js +++ b/jest.config.js @@ -7,18 +7,22 @@ module.exports = { "transform": { "^.+\\.ts$": "ts-jest" }, - "testRegex": "/test/.*\\.ts$", + "testRegex": "/test/.*\\.test\\.ts$", "moduleFileExtensions": [ "ts", "js" ], "testEnvironment": "node", "collectCoverage": true, - // either we don't build the test files (but then eslint needs a separate tsconfig) or we do this - "testPathIgnorePatterns": [ - ".*\\.d\\.ts" - ], "coveragePathIgnorePatterns": [ "/node_modules/" - ] + ], + "coverageThreshold": { + "./src": { + "branches": 100, + "functions": 100, + "lines": 100, + "statements": 100 + } + } }; diff --git a/package-lock.json b/package-lock.json index e22fe56fa..ed93f0967 100644 --- a/package-lock.json +++ b/package-lock.json @@ -5798,9 +5798,9 @@ } }, "ts-jest": { - "version": "25.5.1", - "resolved": "https://registry.npmjs.org/ts-jest/-/ts-jest-25.5.1.tgz", - "integrity": "sha512-kHEUlZMK8fn8vkxDjwbHlxXRB9dHYpyzqKIGDNxbzs+Rz+ssNDSDNusEK8Fk/sDd4xE6iKoQLfFkFVaskmTJyw==", + "version": "26.0.0", + "resolved": "https://registry.npmjs.org/ts-jest/-/ts-jest-26.0.0.tgz", + "integrity": "sha512-eBpWH65mGgzobuw7UZy+uPP9lwu+tPp60o324ASRX4Ijg8UC5dl2zcge4kkmqr2Zeuk9FwIjvCTOPuNMEyGWWw==", "dev": true, "requires": { "bs-logger": "0.x", @@ -5810,9 +5810,23 @@ "lodash.memoize": "4.x", "make-error": "1.x", "micromatch": "4.x", - "mkdirp": "0.x", - "semver": "6.x", + "mkdirp": "1.x", + "semver": "7.x", "yargs-parser": "18.x" + }, + "dependencies": { + "mkdirp": { + "version": "1.0.4", + "resolved": "https://registry.npmjs.org/mkdirp/-/mkdirp-1.0.4.tgz", + "integrity": "sha512-vVqVZQyf3WLx2Shd0qJ9xuvqgAyKPLAiqITEtqW0oIUjzo3PePDd6fW9iFz30ef7Ysp/oiWqbhszeGWW2T6Gzw==", + "dev": true + }, + "semver": { + "version": "7.3.2", + "resolved": "https://registry.npmjs.org/semver/-/semver-7.3.2.tgz", + "integrity": "sha512-OrOb32TeeambH6UrhtShmF7CRDqhL6/5XpPNp2DuRH6+9QLw/orhp72j87v8Qa1ScDkvrrBNpZcDejAirJmfXQ==", + "dev": true + } } }, "tslib": { diff --git a/package.json b/package.json index d836fa165..43b3f2f49 100644 --- a/package.json +++ b/package.json @@ -11,7 +11,7 @@ }, "husky": { "hooks": { - "pre-commit": "npm run lint && npm run build" + "pre-commit": "npm run build && npm run lint && npm run test" } }, "files": [ @@ -34,7 +34,7 @@ "eslint-plugin-tsdoc": "^0.2.4", "husky": "^4.2.5", "jest": "^26.0.1", - "ts-jest": "^25.5.1", + "ts-jest": "^26.0.0", "typescript": "^3.9.2" } } diff --git a/src/authentication/CredentialsExtractor.ts b/src/authentication/CredentialsExtractor.ts index aca9a40f8..28c1ef271 100644 --- a/src/authentication/CredentialsExtractor.ts +++ b/src/authentication/CredentialsExtractor.ts @@ -6,4 +6,4 @@ import { HttpRequest } from '../server/HttpRequest'; * Responsible for extracting credentials from an incoming request. * Will return `null` if no credentials were found. */ -export type CredentialsExtractor = AsyncHandler; +export abstract class CredentialsExtractor extends AsyncHandler {} diff --git a/src/authorization/Authorizer.ts b/src/authorization/Authorizer.ts index ae2932d5d..c70c6f384 100644 --- a/src/authorization/Authorizer.ts +++ b/src/authorization/Authorizer.ts @@ -7,7 +7,7 @@ import { ResourceIdentifier } from '../ldp/representation/ResourceIdentifier'; * Verifies if the given credentials have access to the given permissions on the given resource. * An {@link Error} with the necessary explanation will be thrown when permissions are not granted. */ -export type Authorizer = AsyncHandler; +export abstract class Authorizer extends AsyncHandler {} export interface AuthorizerArgs { /** diff --git a/src/ldp/http/RequestParser.ts b/src/ldp/http/RequestParser.ts index 8c664fc43..90c82926a 100644 --- a/src/ldp/http/RequestParser.ts +++ b/src/ldp/http/RequestParser.ts @@ -5,4 +5,4 @@ import { Operation } from '../operations/Operation'; /** * Converts an incoming HttpRequest to an Operation. */ -export type RequestParser = AsyncHandler; +export abstract class RequestParser extends AsyncHandler {} diff --git a/src/ldp/operations/OperationHandler.ts b/src/ldp/operations/OperationHandler.ts index 81e33f5c7..405be2f10 100644 --- a/src/ldp/operations/OperationHandler.ts +++ b/src/ldp/operations/OperationHandler.ts @@ -4,4 +4,4 @@ import { Operation } from './Operation'; /** * Handler for a specific operation type. */ -export type OperationHandler = AsyncHandler; +export abstract class OperationHandler extends AsyncHandler {} diff --git a/src/ldp/permissions/PermissionsExtractor.ts b/src/ldp/permissions/PermissionsExtractor.ts index eee29296b..76df3d2a2 100644 --- a/src/ldp/permissions/PermissionsExtractor.ts +++ b/src/ldp/permissions/PermissionsExtractor.ts @@ -5,4 +5,4 @@ import { PermissionSet } from './PermissionSet'; /** * Verifies which permissions are requested on a given {@link Operation}. */ -export type PermissionsExtractor = AsyncHandler; +export abstract class PermissionsExtractor extends AsyncHandler {} diff --git a/src/server/HttpHandler.ts b/src/server/HttpHandler.ts index ab1e2a294..36acef1f5 100644 --- a/src/server/HttpHandler.ts +++ b/src/server/HttpHandler.ts @@ -5,4 +5,4 @@ import { HttpResponse } from './HttpResponse'; /** * An HTTP request handler. */ -export type HttpHandler = AsyncHandler<{ request: HttpRequest; response: HttpResponse }>; +export abstract class HttpHandler extends AsyncHandler<{ request: HttpRequest; response: HttpResponse }> {} diff --git a/src/util/AsyncHandler.ts b/src/util/AsyncHandler.ts index 6c064a01e..f21212a2b 100644 --- a/src/util/AsyncHandler.ts +++ b/src/util/AsyncHandler.ts @@ -1,17 +1,35 @@ /** * Simple interface for classes that can potentially handle a specific kind of data asynchronously. */ -export interface 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. * @param input - Input data that would be handled potentially. - * @returns A promise resolving to if this input can be handled. + * + * @returns A promise resolving if this input can be handled, rejecting with an Error if not. */ - canHandle: (input: TInput) => Promise; + public abstract canHandle (input: TInput): Promise; + /** * Handles the given input. This should only be done if the {@link canHandle} function returned `true`. * @param input - Input data that needs to be handled. + * * @returns A promise resolving when the handling is finished. Return value depends on the given type. */ - handle: (input: TInput) => Promise; + public abstract handle (input: TInput): Promise; + + /** + * Helper function that first runs the canHandle function followed by the handle function. + * Throws the error of the {@link canHandle} function if the data can't be handled, + * or returns the result of the {@link handle} function otherwise. + * @param data - The data to handle. + * + * @returns The result of the handle function of the handler. + */ + public async handleSafe (data: TInput): Promise { + await this.canHandle(data); + + return this.handle(data); + } } diff --git a/src/util/CompositeAsyncHandler.ts b/src/util/CompositeAsyncHandler.ts new file mode 100644 index 000000000..ee24919ea --- /dev/null +++ b/src/util/CompositeAsyncHandler.ts @@ -0,0 +1,86 @@ +import { AsyncHandler } from './AsyncHandler'; +import { UnsupportedHttpError } from './errors/UnsupportedHttpError'; + +/** + * Handler that combines several other handlers, + * thereby allowing other classes that depend on a single handler to still use multiple. + */ +export class CompositeAsyncHandler implements AsyncHandler { + private readonly handlers: AsyncHandler[]; + + /** + * Creates a new CompositeAsyncHandler that stores the given handlers. + * @param handlers - Handlers over which it will run. + */ + public constructor (handlers: AsyncHandler[]) { + this.handlers = handlers; + } + + /** + * Checks if any of the stored handlers can handle the given input. + * @param input - The data that would need to be handled. + * + * @returns A promise resolving if at least 1 handler supports to input, or rejecting if none do. + */ + public async canHandle (input: TIn): Promise { + await this.findHandler(input); + } + + /** + * Finds a handler that supports the given input and then lets it handle the given data. + * @param input - The data that needs to be handled. + * + * @returns A promise corresponding to the handle call of a handler that supports the input. + * It rejects if no handlers support the given data. + */ + public async handle (input: TIn): Promise { + let handler: AsyncHandler; + + try { + handler = await this.findHandler(input); + } catch (error) { + throw new Error('All handlers failed. This might be the consequence of calling handle before canHandle.'); + } + + return handler.handle(input); + } + + /** + * Identical to {@link AsyncHandler.handleSafe} but optimized for composite by only needing 1 canHandle call on members. + * @param input - The input data. + * + * @returns A promise corresponding to the handle call of a handler that supports the input. + * It rejects if no handlers support the given data. + */ + public async handleSafe (input: TIn): Promise { + const handler = await this.findHandler(input); + + return handler.handle(input); + } + + /** + * Finds a handler that can handle the given input data. + * Otherwise an error gets thrown. + * + * @param input - The input data. + * + * @returns A promise resolving to a handler that supports the data or otherwise rejecting. + */ + private async findHandler (input: TIn): Promise> { + const errors: Error[] = []; + + for (const handler of this.handlers) { + try { + await handler.canHandle(input); + + return handler; + } catch (error) { + errors.push(error); + } + } + + const joined = errors.map((error: Error): string => error.message).join(', '); + + throw new UnsupportedHttpError(`No handler supports the given input: [${joined}].`); + } +} diff --git a/test/unit/util/AsyncHandler.test.ts b/test/unit/util/AsyncHandler.test.ts new file mode 100644 index 000000000..51513d87e --- /dev/null +++ b/test/unit/util/AsyncHandler.test.ts @@ -0,0 +1,30 @@ +import { AsyncHandler } from '../../../src/util/AsyncHandler'; +import { StaticAsyncHandler } from '../../util/StaticAsyncHandler'; + +describe('An AsyncHandler', (): void => { + it('calls canHandle and handle when handleSafe is called.', async (): Promise => { + const handlerTrue: AsyncHandler = new StaticAsyncHandler(true, null); + const canHandleFn = jest.fn(async (input: any): Promise => input); + const handleFn = jest.fn(async (input: any): Promise => input); + + handlerTrue.canHandle = canHandleFn; + handlerTrue.handle = handleFn; + await expect(handlerTrue.handleSafe('test')).resolves.toBe('test'); + expect(canHandleFn).toHaveBeenCalledTimes(1); + expect(handleFn).toHaveBeenCalledTimes(1); + }); + + it('does not call handle when canHandle errors during a handleSafe call.', async (): Promise => { + const handlerFalse: AsyncHandler = new StaticAsyncHandler(false, null); + const canHandleFn = jest.fn(async (): Promise => { + throw new Error('test'); + }); + const handleFn = jest.fn(async (input: any): Promise => input); + + handlerFalse.canHandle = canHandleFn; + handlerFalse.handle = handleFn; + await expect(handlerFalse.handleSafe('test')).rejects.toThrow(Error); + expect(canHandleFn).toHaveBeenCalledTimes(1); + expect(handleFn).toHaveBeenCalledTimes(0); + }); +}); diff --git a/test/unit/util/CompositeAsyncHandler.test.ts b/test/unit/util/CompositeAsyncHandler.test.ts new file mode 100644 index 000000000..f72dd8f53 --- /dev/null +++ b/test/unit/util/CompositeAsyncHandler.test.ts @@ -0,0 +1,76 @@ +import { AsyncHandler } from '../../../src/util/AsyncHandler'; +import { CompositeAsyncHandler } from '../../../src/util/CompositeAsyncHandler'; +import { StaticAsyncHandler } from '../../util/StaticAsyncHandler'; + +describe('A CompositeAsyncHandler', (): void => { + describe('with no handlers', (): void => { + it('can never handle data.', async (): Promise => { + const handler = new CompositeAsyncHandler([]); + + await expect(handler.canHandle(null)).rejects.toThrow(Error); + }); + + it('errors if its handle function is called.', async (): Promise => { + const handler = new CompositeAsyncHandler([]); + + await expect(handler.handle(null)).rejects.toThrow(Error); + }); + }); + + describe('with multiple handlers', (): void => { + let handlerTrue: AsyncHandler; + let handlerFalse: AsyncHandler; + let canHandleFn: jest.Mock, [any]>; + let handleFn: jest.Mock, [any]>; + + beforeEach(async (): Promise => { + handlerTrue = new StaticAsyncHandler(true, null); + handlerFalse = new StaticAsyncHandler(false, null); + + canHandleFn = jest.fn(async (input: any): Promise => input); + handleFn = jest.fn(async (input: any): Promise => input); + handlerTrue.canHandle = canHandleFn; + handlerTrue.handle = handleFn; + }); + + it('can handle data if a handler supports it.', async (): Promise => { + const handler = new CompositeAsyncHandler([ handlerFalse, handlerTrue ]); + + await expect(handler.canHandle(null)).resolves.toBeUndefined(); + }); + + it('can not handle data if no handler supports it.', async (): Promise => { + const handler = new CompositeAsyncHandler([ handlerFalse, handlerFalse ]); + + await expect(handler.canHandle(null)).rejects.toThrow('[Not supported., Not supported.]'); + }); + + it('handles data if a handler supports it.', async (): Promise => { + const handler = new CompositeAsyncHandler([ handlerFalse, handlerTrue ]); + + await expect(handler.handle('test')).resolves.toEqual('test'); + expect(canHandleFn).toHaveBeenCalledTimes(1); + expect(handleFn).toHaveBeenCalledTimes(1); + }); + + it('errors if the handle function is called but no handler supports the data.', async (): Promise => { + const handler = new CompositeAsyncHandler([ handlerFalse, handlerFalse ]); + + await expect(handler.handle('test')).rejects.toThrow('All handlers failed.'); + }); + + it('only calls the canHandle function once of its handlers when handleSafe is called.', async (): Promise => { + const handler = new CompositeAsyncHandler([ handlerFalse, handlerTrue ]); + + await expect(handler.handleSafe('test')).resolves.toEqual('test'); + expect(canHandleFn).toHaveBeenCalledTimes(1); + expect(handleFn).toHaveBeenCalledTimes(1); + }); + + it('throws the same error as canHandle when calling handleSafe if no handler supports the data.', async (): Promise => { + const handler = new CompositeAsyncHandler([ handlerFalse, handlerFalse ]); + + await expect(handler.handleSafe(null)).rejects.toThrow('[Not supported., Not supported.]'); + }); + }); +}); diff --git a/test/util/StaticAsyncHandler.ts b/test/util/StaticAsyncHandler.ts new file mode 100644 index 000000000..617507f35 --- /dev/null +++ b/test/util/StaticAsyncHandler.ts @@ -0,0 +1,24 @@ +import { AsyncHandler } from '../../src/util/AsyncHandler'; + +export class StaticAsyncHandler extends AsyncHandler { + private readonly canHandleStatic: boolean; + + private readonly handleStatic: TOut; + + public constructor (canHandleStatic: boolean, handleStatic: TOut) { + super(); + this.canHandleStatic = canHandleStatic; + this.handleStatic = handleStatic; + } + + public async canHandle (): Promise { + if (this.canHandleStatic) { + return; + } + throw new Error('Not supported.'); + } + + public async handle (): Promise { + return this.handleStatic; + } +}