From 95e702e50d40b9801fcd3d91a62edbd191b9747a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Sun, 28 Nov 2021 18:40:22 +0100 Subject: [PATCH] ability to abort on controllers instead of just signals, plus more tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Fredrik Adelöw --- packages/backend-common/api-report.md | 5 +- .../src/context/AbortContext.test.ts | 92 +++++++++++++++++++ .../src/context/AbortContext.ts | 87 +++++++++++++----- .../src/context/Contexts.test.ts | 89 ++++++++++++++++++ .../backend-common/src/context/Contexts.ts | 17 ++-- .../backend-common/src/context/RootContext.ts | 4 +- .../src/context/ValueContext.ts | 1 + packages/backend-common/src/context/types.ts | 2 + 8 files changed, 262 insertions(+), 35 deletions(-) create mode 100644 packages/backend-common/src/context/Contexts.test.ts diff --git a/packages/backend-common/api-report.md b/packages/backend-common/api-report.md index e77beef556..d63eeaf11b 100644 --- a/packages/backend-common/api-report.md +++ b/packages/backend-common/api-report.md @@ -6,6 +6,7 @@ /// /// +import { AbortController as AbortController_2 } from 'node-abort-controller'; import { AbortSignal as AbortSignal_2 } from 'node-abort-controller'; import { AwsS3Integration } from '@backstage/integration'; import { AzureIntegration } from '@backstage/integration'; @@ -148,7 +149,7 @@ export interface ContainerRunner { // @public export interface Context { - readonly abortSignal: AbortSignal; + readonly abortSignal: AbortSignal_2; readonly deadline: Date | undefined; use(...decorators: ContextDecorator[]): Context; value(key: string | symbol): T | undefined; @@ -160,7 +161,7 @@ export type ContextDecorator = (ctx: Context) => Context; // @public export class Contexts { static root(): Context; - static setAbort(signal: AbortSignal_2): ContextDecorator; + static setAbort(source: AbortController_2 | AbortSignal_2): ContextDecorator; static setTimeoutDuration(timeout: Duration): ContextDecorator; static setTimeoutMillis(timeout: number): ContextDecorator; static setValue( diff --git a/packages/backend-common/src/context/AbortContext.test.ts b/packages/backend-common/src/context/AbortContext.test.ts index fc7b366c23..aa01c2209a 100644 --- a/packages/backend-common/src/context/AbortContext.test.ts +++ b/packages/backend-common/src/context/AbortContext.test.ts @@ -160,6 +160,98 @@ describe('AbortContext', () => { }); }); + describe('forController', () => { + it('signals child when parent is aborted', () => { + const root = new RootContext(); + + const parentController = new AbortController(); + const parent = AbortContext.forController(root, parentController); + const parentListener = jest.fn(); + parent.abortSignal.addEventListener('abort', parentListener); + + const childController = new AbortController(); + const child = AbortContext.forController(parent, childController); + const childListener = jest.fn(); + child.abortSignal.addEventListener('abort', childListener); + + expect(parent.abortSignal.aborted).toBe(false); + expect(child.abortSignal.aborted).toBe(false); + expect(parentListener).toBeCalledTimes(0); + expect(childListener).toBeCalledTimes(0); + + parentController.abort(); + + expect(parent.abortSignal.aborted).toBe(true); + expect(child.abortSignal.aborted).toBe(true); + expect(parentListener).toBeCalledTimes(1); + expect(childListener).toBeCalledTimes(1); + }); + + it('does not signal parent when child is aborted', async () => { + const root = new RootContext(); + + const parentController = new AbortController(); + const parent = AbortContext.forController(root, parentController); + const parentListener = jest.fn(); + parent.abortSignal.addEventListener('abort', parentListener); + + const childController = new AbortController(); + const child = AbortContext.forController(parent, childController); + const childListener = jest.fn(); + child.abortSignal.addEventListener('abort', childListener); + + expect(parent.abortSignal.aborted).toBe(false); + expect(child.abortSignal.aborted).toBe(false); + expect(parentListener).toBeCalledTimes(0); + expect(childListener).toBeCalledTimes(0); + + childController.abort(); + + expect(parent.abortSignal.aborted).toBe(false); + expect(child.abortSignal.aborted).toBe(true); + expect(parentListener).toBeCalledTimes(0); + expect(childListener).toBeCalledTimes(1); + }); + + it('child carries over parent signal state if parent was already aborted', async () => { + const root = new RootContext(); + + const parentController = new AbortController(); + const parent = AbortContext.forController(root, parentController); + + parentController.abort(); + + const childController = new AbortController(); + const child = AbortContext.forController(parent, childController); + const childListener = jest.fn(); + child.abortSignal.addEventListener('abort', childListener); + + expect(parent.abortSignal.aborted).toBe(true); + expect(child.abortSignal.aborted).toBe(true); + expect(childListener).toBeCalledTimes(0); + + childController.abort(); + + expect(parent.abortSignal.aborted).toBe(true); + expect(child.abortSignal.aborted).toBe(true); + expect(childListener).toBeCalledTimes(0); + }); + + it('child carries over given signal state if it was already aborted', async () => { + const root = new RootContext(); + + const childController = new AbortController(); + childController.abort(); + + const child = AbortContext.forController(root, childController); + const childListener = jest.fn(); + child.abortSignal.addEventListener('abort', childListener); + + expect(child.abortSignal.aborted).toBe(true); + expect(childListener).toBeCalledTimes(0); + }); + }); + describe('forSignal', () => { it('signals child when parent is aborted', async () => { const root = new RootContext(); diff --git a/packages/backend-common/src/context/AbortContext.ts b/packages/backend-common/src/context/AbortContext.ts index a328241de2..78ad8646ed 100644 --- a/packages/backend-common/src/context/AbortContext.ts +++ b/packages/backend-common/src/context/AbortContext.ts @@ -14,7 +14,7 @@ * limitations under the License. */ -import { AbortSignal } from 'node-abort-controller'; +import { AbortController, AbortSignal } from 'node-abort-controller'; import { Context, ContextDecorator } from './types'; /** @@ -24,6 +24,10 @@ export class AbortContext implements Context { /** * Abort either when the parent aborts, or after the given timeout has * expired. + * + * @param ctx - The parent context + * @param timeout - A timeout value, in milliseconds + * @returns A new context */ static forTimeoutMillis(ctx: Context, timeout: number): Context { const desiredDeadline = new Date(Date.now() + timeout); @@ -40,43 +44,80 @@ export class AbortContext implements Context { } const controller = new AbortController(); - - const timeoutHandle = setTimeout(() => { - controller.abort(); - }, timeout); - - const abort = () => { - ctx.abortSignal.removeEventListener('abort', abort); - clearTimeout(timeoutHandle); - controller.abort(); - }; - + const timeoutHandle = setTimeout(abort, timeout); ctx.abortSignal.addEventListener('abort', abort); + function abort() { + ctx.abortSignal.removeEventListener('abort', abort); + clearTimeout(timeoutHandle!); + controller.abort(); + } + return new AbortContext(ctx, controller.signal, actualDeadline); } /** - * Abort either when the parent aborts, or when the given signal is triggered. + * Abort either when the parent aborts, or when the given controller is + * triggered. + * + * @remarks + * + * If you have access to the controller, this function is more efficient than + * {@link AbortContext#forSignal}. + * + * @param ctx - The parent context + * @param controller - An abort controller + * @returns A new context */ - static forSignal(ctx: Context, signal: AbortSignal): Context { - // If the parent context was already aborted, it is fine to reuse as-is + static forController(ctx: Context, controller: AbortController): Context { + // Already aborted context / signal are fine to reuse as-is if (ctx.abortSignal.aborted) { return ctx; + } else if (controller.signal.aborted) { + return new AbortContext(ctx, controller.signal, ctx.deadline); + } + + function abort() { + ctx.abortSignal.removeEventListener('abort', abort); + controller.abort(); + } + + ctx.abortSignal.addEventListener('abort', abort); + + return new AbortContext(ctx, controller.signal, ctx.deadline); + } + + /** + * Abort either when the parent aborts, or when the given signal is triggered. + * + * @remarks + * + * If you have access to the controller and not just the signal, + * {@link AbortContext#forController} is slightly more efficient to use. + * + * @param ctx - The parent context + * @param signal - An abort signal + * @returns A new context + */ + static forSignal(ctx: Context, signal: AbortSignal): Context { + // Already aborted context / signal are fine to reuse as-is + if (ctx.abortSignal.aborted) { + return ctx; + } else if (signal.aborted) { + return new AbortContext(ctx, signal, ctx.deadline); } const controller = new AbortController(); - const abort = controller.abort.bind(controller); - // If the incoming signal was already aborted, let's trigger the new one as - // well - if (signal.aborted) { - abort(); - } else { - ctx.abortSignal.addEventListener('abort', abort); - signal.addEventListener('abort', abort); + function abort() { + ctx.abortSignal.removeEventListener('abort', abort); + signal.removeEventListener('abort', abort); + controller.abort(); } + ctx.abortSignal.addEventListener('abort', abort); + signal.addEventListener('abort', abort); + return new AbortContext(ctx, controller.signal, ctx.deadline); } diff --git a/packages/backend-common/src/context/Contexts.test.ts b/packages/backend-common/src/context/Contexts.test.ts new file mode 100644 index 0000000000..6c05319a4f --- /dev/null +++ b/packages/backend-common/src/context/Contexts.test.ts @@ -0,0 +1,89 @@ +/* + * Copyright 2021 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { Duration } from 'luxon'; +import { AbortController } from 'node-abort-controller'; +import { Contexts } from './Contexts'; + +describe('Contexts', () => { + afterEach(() => { + jest.useRealTimers(); + }); + + describe('root', () => { + it('can create a root', () => { + const ctx = Contexts.root(); + expect(ctx.abortSignal).toBeDefined(); + expect(ctx.deadline).toBeUndefined(); + }); + }); + + describe('setAbort', () => { + it('works for controllers', () => { + const controller = new AbortController(); + const parent = Contexts.root(); + const child = parent.use(Contexts.setAbort(controller)); + expect(child.abortSignal.aborted).toBe(false); + controller.abort(); + expect(child.abortSignal.aborted).toBe(true); + }); + + it('works for signals', () => { + const controller = new AbortController(); + const parent = Contexts.root(); + const child = parent.use(Contexts.setAbort(controller.signal)); + expect(child.abortSignal.aborted).toBe(false); + controller.abort(); + expect(child.abortSignal.aborted).toBe(true); + }); + }); + + describe('setTimeoutDuration', () => { + it('works', () => { + jest.useFakeTimers(); + const parent = Contexts.root(); + const child = parent.use( + Contexts.setTimeoutDuration(Duration.fromMillis(200)), + ); + expect(child.abortSignal.aborted).toBe(false); + jest.advanceTimersByTime(100); + expect(child.abortSignal.aborted).toBe(false); + jest.advanceTimersByTime(101); + expect(child.abortSignal.aborted).toBe(true); + }); + }); + + describe('setTimeoutMillis', () => { + it('works', () => { + jest.useFakeTimers(); + const parent = Contexts.root(); + const child = parent.use(Contexts.setTimeoutMillis(200)); + expect(child.abortSignal.aborted).toBe(false); + jest.advanceTimersByTime(100); + expect(child.abortSignal.aborted).toBe(false); + jest.advanceTimersByTime(101); + expect(child.abortSignal.aborted).toBe(true); + }); + }); + + describe('setValue', () => { + it('works', () => { + const parent = Contexts.root(); + const child = parent.use(Contexts.setValue('k', 'v')); + expect(child.value('k')).toBe('v'); + }); + }); +}); diff --git a/packages/backend-common/src/context/Contexts.ts b/packages/backend-common/src/context/Contexts.ts index 775f6f730d..7bab26f05a 100644 --- a/packages/backend-common/src/context/Contexts.ts +++ b/packages/backend-common/src/context/Contexts.ts @@ -15,7 +15,7 @@ */ import { Duration } from 'luxon'; -import { AbortSignal } from 'node-abort-controller'; +import { AbortController, AbortSignal } from 'node-abort-controller'; import { AbortContext } from './AbortContext'; import { RootContext } from './RootContext'; import { Context, ContextDecorator } from './types'; @@ -42,21 +42,24 @@ export class Contexts { /** * Creates a derived context, which signals to abort operations either when - * any parent context signals, or when the given controller is aborted. + * any parent context signals, or when the given source is aborted. * * @remarks * * If the parent context was already aborted, then it is returned as-is. * - * If the given signal was already aborted, then a new already-aborted context + * If the given source was already aborted, then a new already-aborted context * is returned. * - * @param signal - An abort signal that you intend to perhaps trigger at some - * later point in time. + * @param source - An abort controller or signal that you intend to perhaps + * trigger at some later point in time. * @returns A decorator that can be passed to {@link Context.use} */ - static setAbort(signal: AbortSignal): ContextDecorator { - return ctx => AbortContext.forSignal(ctx, signal); + static setAbort(source: AbortController | AbortSignal): ContextDecorator { + return ctx => + 'aborted' in source + ? AbortContext.forSignal(ctx, source) + : AbortContext.forController(ctx, source); } /** diff --git a/packages/backend-common/src/context/RootContext.ts b/packages/backend-common/src/context/RootContext.ts index ae7c18a905..48d24514fa 100644 --- a/packages/backend-common/src/context/RootContext.ts +++ b/packages/backend-common/src/context/RootContext.ts @@ -17,13 +17,11 @@ import { AbortController } from 'node-abort-controller'; import { Context, ContextDecorator } from './types'; -const neverAborts = new AbortController().signal; - /** * An empty root context. */ export class RootContext implements Context { - readonly abortSignal = neverAborts; + readonly abortSignal = new AbortController().signal; readonly deadline = undefined; value(_key: string | symbol): T | undefined { diff --git a/packages/backend-common/src/context/ValueContext.ts b/packages/backend-common/src/context/ValueContext.ts index 1ed5e2dba5..368e73fa05 100644 --- a/packages/backend-common/src/context/ValueContext.ts +++ b/packages/backend-common/src/context/ValueContext.ts @@ -14,6 +14,7 @@ * limitations under the License. */ +import { AbortSignal } from 'node-abort-controller'; import { Context, ContextDecorator } from './types'; /** diff --git a/packages/backend-common/src/context/types.ts b/packages/backend-common/src/context/types.ts index 569a133c87..716a03308b 100644 --- a/packages/backend-common/src/context/types.ts +++ b/packages/backend-common/src/context/types.ts @@ -14,6 +14,8 @@ * limitations under the License. */ +import { AbortSignal } from 'node-abort-controller'; + /** * A function that accepts a context and produces a new, derived context from, * decorated with some specific behavior.