From 29ddf5734196939c47b0267aedad86d948326bd3 Mon Sep 17 00:00:00 2001 From: Joachim Van Herwegen Date: Fri, 11 Jun 2021 15:38:02 +0200 Subject: [PATCH] feat: Add Finalizable interface --- src/init/ServerInitializer.ts | 15 +++++++++-- src/init/final/Finalizable.ts | 6 +++++ .../keyvalue/WrappedExpiringStorage.ts | 6 ++--- src/util/locking/RedisResourceLocker.ts | 12 ++++++--- test/unit/init/ServerInitializer.test.ts | 25 ++++++++++++++++--- .../keyvalue/WrappedExpiringStorage.test.ts | 2 +- 6 files changed, 52 insertions(+), 14 deletions(-) create mode 100644 src/init/final/Finalizable.ts diff --git a/src/init/ServerInitializer.ts b/src/init/ServerInitializer.ts index 6320dae61..40438dad2 100644 --- a/src/init/ServerInitializer.ts +++ b/src/init/ServerInitializer.ts @@ -1,13 +1,18 @@ +import type { Server } from 'http'; +import { promisify } from 'util'; import type { HttpServerFactory } from '../server/HttpServerFactory'; +import type { Finalizable } from './final/Finalizable'; import { Initializer } from './Initializer'; /** * Creates and starts an HTTP server. */ -export class ServerInitializer extends Initializer { +export class ServerInitializer extends Initializer implements Finalizable { private readonly serverFactory: HttpServerFactory; private readonly port: number; + private server?: Server; + public constructor(serverFactory: HttpServerFactory, port: number) { super(); this.serverFactory = serverFactory; @@ -15,6 +20,12 @@ export class ServerInitializer extends Initializer { } public async handle(): Promise { - this.serverFactory.startServer(this.port); + this.server = this.serverFactory.startServer(this.port); + } + + public async finalize(): Promise { + if (this.server) { + return promisify(this.server.close.bind(this.server))(); + } } } diff --git a/src/init/final/Finalizable.ts b/src/init/final/Finalizable.ts new file mode 100644 index 000000000..a99f75bf1 --- /dev/null +++ b/src/init/final/Finalizable.ts @@ -0,0 +1,6 @@ +/** + * Allows for cleaning up an object and stopping relevant loops when the application needs to be stopped. + */ +export interface Finalizable { + finalize: () => Promise; +} diff --git a/src/storage/keyvalue/WrappedExpiringStorage.ts b/src/storage/keyvalue/WrappedExpiringStorage.ts index 223cf67da..5fffdf48a 100644 --- a/src/storage/keyvalue/WrappedExpiringStorage.ts +++ b/src/storage/keyvalue/WrappedExpiringStorage.ts @@ -1,3 +1,4 @@ +import type { Finalizable } from '../../init/final/Finalizable'; import { getLoggerFor } from '../../logging/LogUtil'; import { InternalServerError } from '../../util/errors/InternalServerError'; import type { ExpiringStorage } from './ExpiringStorage'; @@ -11,9 +12,8 @@ export type Expires = { expires?: string; payload: T }; * Will delete expired entries when trying to get their value. * Has a timer that will delete all expired data every hour (default value). */ -export class WrappedExpiringStorage implements ExpiringStorage { +export class WrappedExpiringStorage implements ExpiringStorage, Finalizable { protected readonly logger = getLoggerFor(this); - private readonly source: KeyValueStorage>; private readonly timer: NodeJS.Timeout; @@ -118,7 +118,7 @@ export class WrappedExpiringStorage implements ExpiringStorage { clearInterval(this.timer); } } diff --git a/src/util/locking/RedisResourceLocker.ts b/src/util/locking/RedisResourceLocker.ts index 23715d13c..c34b48e37 100644 --- a/src/util/locking/RedisResourceLocker.ts +++ b/src/util/locking/RedisResourceLocker.ts @@ -3,6 +3,7 @@ import type { RedisClient } from 'redis'; import { createClient } from 'redis'; import type { Lock } from 'redlock'; import Redlock from 'redlock'; +import type { Finalizable } from '../../init/final/Finalizable'; import type { ResourceIdentifier } from '../../ldp/representation/ResourceIdentifier'; import { getLoggerFor } from '../../logging/LogUtil'; import { InternalServerError } from '../errors/InternalServerError'; @@ -37,7 +38,7 @@ const defaultRedlockConfig = { * in that sense it is kind of multithreaded. * - Redlock does not provide the ability to see which locks have expired */ -export class RedisResourceLocker implements ResourceLocker { +export class RedisResourceLocker implements ResourceLocker, Finalizable { protected readonly logger = getLoggerFor(this); private readonly redlock: Redlock; @@ -100,10 +101,13 @@ export class RedisResourceLocker implements ResourceLocker { public async finalize(): Promise { // This for loop is an extra failsafe, // this extra code won't slow down anything, this function will only be called to shut down in peace - for (const [ , { lock }] of this.lockMap.entries()) { - await this.release({ path: lock.resource }); + try { + for (const [ , { lock }] of this.lockMap.entries()) { + await this.release({ path: lock.resource }); + } + } finally { + await this.redlock.quit(); } - await this.redlock.quit(); } public async acquire(identifier: ResourceIdentifier): Promise { diff --git a/test/unit/init/ServerInitializer.test.ts b/test/unit/init/ServerInitializer.test.ts index 8eecc089a..3beb23baf 100644 --- a/test/unit/init/ServerInitializer.test.ts +++ b/test/unit/init/ServerInitializer.test.ts @@ -1,13 +1,19 @@ +import type { Server } from 'http'; import { ServerInitializer } from '../../../src/init/ServerInitializer'; import type { HttpServerFactory } from '../../../src/server/HttpServerFactory'; describe('ServerInitializer', (): void => { - const serverFactory: jest.Mocked = { - startServer: jest.fn(), - }; + let server: Server; + let serverFactory: jest.Mocked; let initializer: ServerInitializer; - beforeAll(async(): Promise => { + beforeEach(async(): Promise => { + server = { + close: jest.fn((fn: () => void): void => fn()), + } as any; + serverFactory = { + startServer: jest.fn().mockReturnValue(server), + }; initializer = new ServerInitializer(serverFactory, 3000); }); @@ -15,4 +21,15 @@ describe('ServerInitializer', (): void => { await initializer.handle(); expect(serverFactory.startServer).toHaveBeenCalledWith(3000); }); + + it('can stop the server.', async(): Promise => { + await initializer.handle(); + await expect(initializer.finalize()).resolves.toBeUndefined(); + expect(server.close).toHaveBeenCalledTimes(1); + }); + + it('only tries to stop the server if it was initialized.', async(): Promise => { + await expect(initializer.finalize()).resolves.toBeUndefined(); + expect(server.close).toHaveBeenCalledTimes(0); + }); }); diff --git a/test/unit/storage/keyvalue/WrappedExpiringStorage.test.ts b/test/unit/storage/keyvalue/WrappedExpiringStorage.test.ts index b1074d4da..729473d49 100644 --- a/test/unit/storage/keyvalue/WrappedExpiringStorage.test.ts +++ b/test/unit/storage/keyvalue/WrappedExpiringStorage.test.ts @@ -154,7 +154,7 @@ describe('A WrappedExpiringStorage', (): void => { yield* data; }); - expect(storage.finalize()).toBeUndefined(); + await expect(storage.finalize()).resolves.toBeUndefined(); // Make sure clearInterval was called with the interval timer expect(mockClear.mock.calls).toHaveLength(1);