ability to abort on controllers instead of just signals, plus more tests

Signed-off-by: Fredrik Adelöw <freben@gmail.com>

packages/backend-common/api-report.md

@@ -6,6 +6,7 @@
 /// <reference types="node" />
 /// <reference types="webpack-env" />
 
+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<T = unknown>(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(

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();

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);
   }
 

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');
+    });
+  });
+});

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);
   }
 
   /**

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<T = unknown>(_key: string | symbol): T | undefined {

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';
 
 /**

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.