Merge pull request #9660 from backstage/blam/deprecations/1

deprecations: Some type deprecations and some small breaking changes to the some parts of the `scaffolder-backend`
2022-02-18 14:46:32 +01:00
parent f7d624c56f 3c1c6f55b9
commit 6912dd0a09
10 changed files with 207 additions and 91 deletions
@@ -0,0 +1,19 @@
+---
+'@backstage/plugin-scaffolder-backend': minor
+---
+
+- **BREAKING** - `DatabaseTaskStore()` constructor is now removed. Please use the `DatabaseTaskStore.create()` method instead.
+
+- **BREAKING** - `TaskStore.createTask()` method now only takes one argument of type `TaskStoreCreateTaskOptions` which encapsulates the `spec` and `secrets`
+
+```diff
+- TaskStore.createTask(spec, secrets)
+ TaskStore.createTask({ spec, secrets})
+```
+
+- **BREAKING** - `TaskBroker.dispatch()` method now only takes one argument of type `TaskBrokerDispatchOptions` which encapsulates the `spec` and `secrets`
+
+```diff
+- TaskBroker.dispatch(spec, secrets)
+ TaskBroker.dispatch({ spec, secrets})
+```
@@ -0,0 +1,7 @@
+---
+'@backstage/plugin-scaffolder-backend': patch
+---
+
+- **DEPRECATED** - `Status` has been deprecated in favour of `TaskStatus`
+- **DEPRECATED** - `CompletedTaskState` has been deprecated in favour of `TaskCompletionState`
+- **DEPRECATED** - `DispatchResult` has been deprecated in favour of `TaskBrokerDispatchResult`
@@ -50,8 +50,8 @@ export type ActionContext<Input extends JsonObject> = {
  templateInfo?: TemplateInfo;
 };

-// @public
-export type CompletedTaskState = 'failed' | 'completed';
+// @public @deprecated
+export type CompletedTaskState = TaskCompletionState;

 // Warning: (ae-missing-release-tag) "createBuiltinActions" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal)
 //
@@ -315,7 +315,6 @@ export type CreateWorkerOptions = {

 // @public
 export class DatabaseTaskStore implements TaskStore {
-  constructor(options: DatabaseTaskStoreOptions);
  // (undocumented)
  claimTask(): Promise<SerializedTask | undefined>;
  // (undocumented)
@@ -325,7 +324,7 @@ export class DatabaseTaskStore implements TaskStore {
    eventBody,
  }: {
    taskId: string;
-    status: Status;
+    status: TaskStatus;
    eventBody: JsonObject;
  }): Promise<void>;
  // Warning: (ae-forgotten-export) The symbol "DatabaseTaskStoreOptions" needs to be exported by the entry point index.d.ts
@@ -334,11 +333,8 @@ export class DatabaseTaskStore implements TaskStore {
  static create(options: DatabaseTaskStoreOptions): Promise<DatabaseTaskStore>;
  // (undocumented)
  createTask(
-    spec: TaskSpec,
-    secrets?: TaskSecrets,
-  ): Promise<{
-    taskId: string;
-  }>;
+    options: TaskStoreCreateTaskOptions,
+  ): Promise<TaskStoreCreateTaskResult>;
  // (undocumented)
  emitLogEvent({ taskId, body }: TaskStoreEmitOptions): Promise<void>;
  // (undocumented)
@@ -357,10 +353,8 @@ export class DatabaseTaskStore implements TaskStore {
  }>;
 }

-// @public
-export type DispatchResult = {
-  taskId: string;
-};
+// @public @deprecated
+export type DispatchResult = TaskBrokerDispatchResult;

 // Warning: (ae-missing-release-tag) "fetchContents" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal)
 //
@@ -449,7 +443,7 @@ export class ScaffolderEntitiesProcessor implements CatalogProcessor {
 export type SerializedTask = {
  id: string;
  spec: TaskSpec;
-  status: Status;
+  status: TaskStatus;
  createdAt: string;
  lastHeartbeatAt?: string;
  secrets?: TaskSecrets;
@@ -464,20 +458,17 @@ export type SerializedTaskEvent = {
  createdAt: string;
 };

-// @public
-export type Status =
-  | 'open'
-  | 'processing'
-  | 'failed'
-  | 'cancelled'
-  | 'completed';
+// @public @deprecated
+export type Status = TaskStatus;

 // @public
 export interface TaskBroker {
  // (undocumented)
  claim(): Promise<TaskContext>;
  // (undocumented)
-  dispatch(spec: TaskSpec, secrets?: TaskSecrets): Promise<DispatchResult>;
+  dispatch(
+    options: TaskBrokerDispatchOptions,
+  ): Promise<TaskBrokerDispatchResult>;
  // (undocumented)
  get(taskId: string): Promise<SerializedTask>;
  // (undocumented)
@@ -499,10 +490,24 @@ export interface TaskBroker {
  vacuumTasks(timeoutS: { timeoutS: number }): Promise<void>;
 }

+// @public
+export type TaskBrokerDispatchOptions = {
+  spec: TaskSpec;
+  secrets?: TaskSecrets;
+};
+
+// @public
+export type TaskBrokerDispatchResult = {
+  taskId: string;
+};
+
+// @public
+export type TaskCompletionState = 'failed' | 'completed';
+
 // @public
 export interface TaskContext {
  // (undocumented)
-  complete(result: CompletedTaskState, metadata?: JsonValue): Promise<void>;
+  complete(result: TaskCompletionState, metadata?: JsonValue): Promise<void>;
  // (undocumented)
  done: boolean;
  // (undocumented)
@@ -521,7 +526,7 @@ export type TaskEventType = 'completion' | 'log';
 // @public
 export class TaskManager implements TaskContext {
  // (undocumented)
-  complete(result: CompletedTaskState, metadata?: JsonObject): Promise<void>;
+  complete(result: TaskCompletionState, metadata?: JsonObject): Promise<void>;
  // (undocumented)
  static create(
    state: TaskState,
@@ -561,6 +566,14 @@ export interface TaskState {
  taskId: string;
 }

+// @public
+export type TaskStatus =
+  | 'open'
+  | 'processing'
+  | 'failed'
+  | 'cancelled'
+  | 'completed';
+
 // @public
 export interface TaskStore {
  // (undocumented)
@@ -568,16 +581,13 @@ export interface TaskStore {
  // (undocumented)
  completeTask(options: {
    taskId: string;
-    status: Status;
+    status: TaskStatus;
    eventBody: JsonObject;
  }): Promise<void>;
  // (undocumented)
  createTask(
-    task: TaskSpec,
-    secrets?: TaskSecrets,
-  ): Promise<{
-    taskId: string;
-  }>;
+    options: TaskStoreCreateTaskOptions,
+  ): Promise<TaskStoreCreateTaskResult>;
  // (undocumented)
  emitLogEvent({ taskId, body }: TaskStoreEmitOptions): Promise<void>;
  // (undocumented)
@@ -596,6 +606,17 @@ export interface TaskStore {
  }>;
 }

+// @public
+export type TaskStoreCreateTaskOptions = {
+  spec: TaskSpec;
+  secrets?: TaskSecrets;
+};
+
+// @public
+export type TaskStoreCreateTaskResult = {
+  taskId: string;
+};
+
 // @public
 export type TaskStoreEmitOptions = {
  taskId: string;
@@ -22,13 +22,13 @@ import { v4 as uuid } from 'uuid';
 import {
  SerializedTaskEvent,
  SerializedTask,
-  Status,
+  TaskStatus,
  TaskEventType,
-  TaskSecrets,
-  TaskSpec,
  TaskStore,
  TaskStoreEmitOptions,
  TaskStoreListEventsOptions,
+  TaskStoreCreateTaskOptions,
+  TaskStoreCreateTaskResult,
 } from './types';
 import { DateTime } from 'luxon';

@@ -40,7 +40,7 @@ const migrationsDir = resolvePackagePath(
 export type RawDbTaskRow = {
  id: string;
  spec: string;
-  status: Status;
+  status: TaskStatus;
  last_heartbeat_at?: string;
  created_at: string;
  secrets?: string | null;
@@ -80,7 +80,7 @@ export class DatabaseTaskStore implements TaskStore {
    return new DatabaseTaskStore(options);
  }

-  constructor(options: DatabaseTaskStoreOptions) {
+  private constructor(options: DatabaseTaskStoreOptions) {
    this.db = options.database;
  }

@@ -108,14 +108,13 @@ export class DatabaseTaskStore implements TaskStore {
  }

  async createTask(
-    spec: TaskSpec,
-    secrets?: TaskSecrets,
-  ): Promise<{ taskId: string }> {
+    options: TaskStoreCreateTaskOptions,
+  ): Promise<TaskStoreCreateTaskResult> {
    const taskId = uuid();
    await this.db<RawDbTaskRow>('tasks').insert({
      id: taskId,
-      spec: JSON.stringify(spec),
-      secrets: secrets ? JSON.stringify(secrets) : undefined,
+      spec: JSON.stringify(options.spec),
+      secrets: options.secrets ? JSON.stringify(options.secrets) : undefined,
      status: 'open',
    });
    return { taskId };
@@ -202,7 +201,7 @@ export class DatabaseTaskStore implements TaskStore {
    eventBody,
  }: {
    taskId: string;
-    status: Status;
+    status: TaskStatus;
    eventBody: JsonObject;
  }): Promise<void> {
    let oldStatus: string;
@@ -47,7 +47,7 @@ describe('StorageTaskBroker', () => {
  const logger = getVoidLogger();
  it('should claim a dispatched work item', async () => {
    const broker = new StorageTaskBroker(storage, logger);
-    await broker.dispatch({} as TaskSpec);
+    await broker.dispatch({ spec: {} as TaskSpec });
    await expect(broker.claim()).resolves.toEqual(expect.any(TaskManager));
  });

@@ -57,15 +57,15 @@ describe('StorageTaskBroker', () => {

    await expect(Promise.race([promise, 'waiting'])).resolves.toBe('waiting');

-    await broker.dispatch({} as TaskSpec);
+    await broker.dispatch({ spec: {} as TaskSpec });
    await expect(promise).resolves.toEqual(expect.any(TaskManager));
  });

  it('should dispatch multiple items and claim them in order', async () => {
    const broker = new StorageTaskBroker(storage, logger);
-    await broker.dispatch({ steps: [{ id: 'a' }] } as TaskSpec);
-    await broker.dispatch({ steps: [{ id: 'b' }] } as TaskSpec);
-    await broker.dispatch({ steps: [{ id: 'c' }] } as TaskSpec);
+    await broker.dispatch({ spec: { steps: [{ id: 'a' }] } as TaskSpec });
+    await broker.dispatch({ spec: { steps: [{ id: 'b' }] } as TaskSpec });
+    await broker.dispatch({ spec: { steps: [{ id: 'c' }] } as TaskSpec });

    const taskA = await broker.claim();
    const taskB = await broker.claim();
@@ -80,14 +80,14 @@ describe('StorageTaskBroker', () => {

  it('should store secrets', async () => {
    const broker = new StorageTaskBroker(storage, logger);
-    await broker.dispatch({} as TaskSpec, fakeSecrets);
+    await broker.dispatch({ spec: {} as TaskSpec, secrets: fakeSecrets });
    const task = await broker.claim();
    expect(task.secrets).toEqual(fakeSecrets);
  }, 10000);

  it('should complete a task', async () => {
    const broker = new StorageTaskBroker(storage, logger);
-    const dispatchResult = await broker.dispatch({} as TaskSpec);
+    const dispatchResult = await broker.dispatch({ spec: {} as TaskSpec });
    const task = await broker.claim();
    await task.complete('completed');
    const taskRow = await storage.getTask(dispatchResult.taskId);
@@ -96,7 +96,10 @@ describe('StorageTaskBroker', () => {

  it('should remove secrets after picking up a task', async () => {
    const broker = new StorageTaskBroker(storage, logger);
-    const dispatchResult = await broker.dispatch({} as TaskSpec, fakeSecrets);
+    const dispatchResult = await broker.dispatch({
+      spec: {} as TaskSpec,
+      secrets: fakeSecrets,
+    });
    await broker.claim();

    const taskRow = await storage.getTask(dispatchResult.taskId);
@@ -105,7 +108,7 @@ describe('StorageTaskBroker', () => {

  it('should fail a task', async () => {
    const broker = new StorageTaskBroker(storage, logger);
-    const dispatchResult = await broker.dispatch({} as TaskSpec);
+    const dispatchResult = await broker.dispatch({ spec: {} as TaskSpec });
    const task = await broker.claim();
    await task.complete('failed');
    const taskRow = await storage.getTask(dispatchResult.taskId);
@@ -116,7 +119,7 @@ describe('StorageTaskBroker', () => {
    const broker1 = new StorageTaskBroker(storage, logger);
    const broker2 = new StorageTaskBroker(storage, logger);

-    const { taskId } = await broker1.dispatch({} as TaskSpec);
+    const { taskId } = await broker1.dispatch({ spec: {} as TaskSpec });

    const logPromise = new Promise<SerializedTaskEvent[]>(resolve => {
      const observedEvents = new Array<SerializedTaskEvent>();
@@ -155,7 +158,7 @@ describe('StorageTaskBroker', () => {

  it('should heartbeat', async () => {
    const broker = new StorageTaskBroker(storage, logger);
-    const { taskId } = await broker.dispatch({} as TaskSpec);
+    const { taskId } = await broker.dispatch({ spec: {} as TaskSpec });
    const task = await broker.claim();

    const initialTask = await storage.getTask(taskId);
@@ -173,7 +176,7 @@ describe('StorageTaskBroker', () => {

  it('should be update the status to failed if heartbeat fails', async () => {
    const broker = new StorageTaskBroker(storage, logger);
-    const { taskId } = await broker.dispatch({} as TaskSpec);
+    const { taskId } = await broker.dispatch({ spec: {} as TaskSpec });
    const task = await broker.claim();

    jest
@@ -17,16 +17,16 @@ import { JsonObject } from '@backstage/types';
 import { assertError } from '@backstage/errors';
 import { Logger } from 'winston';
 import {
-  CompletedTaskState,
+  TaskCompletionState,
  TaskContext,
  TaskSecrets,
  TaskSpec,
  TaskStore,
  TaskBroker,
-  DispatchResult,
  SerializedTaskEvent,
  SerializedTask,
 } from './types';
+import { TaskBrokerDispatchOptions } from '.';

 /**
 * TaskManager
@@ -75,7 +75,7 @@ export class TaskManager implements TaskContext {
  }

  async complete(
-    result: CompletedTaskState,
+    result: TaskCompletionState,
    metadata?: JsonObject,
  ): Promise<void> {
    await this.storage.completeTask({
@@ -155,10 +155,9 @@ export class StorageTaskBroker implements TaskBroker {
  }

  async dispatch(
-    spec: TaskSpec,
-    secrets?: TaskSecrets,
-  ): Promise<DispatchResult> {
-    const taskRow = await this.storage.createTask(spec, secrets);
+    options: TaskBrokerDispatchOptions,
+  ): Promise<{ taskId: string }> {
+    const taskRow = await this.storage.createTask(options);
    this.signalDispatch();
    return {
      taskId: taskRow.taskId,
@@ -91,12 +91,14 @@ describe('TaskWorker', () => {
    });

    await broker.dispatch({
-      apiVersion: 'backstage.io/v1beta2',
-      steps: [{ id: 'test', name: 'test', action: 'not-found-action' }],
-      output: {
-        result: '{{ steps.test.output.testOutput }}',
+      spec: {
+        apiVersion: 'backstage.io/v1beta2',
+        steps: [{ id: 'test', name: 'test', action: 'not-found-action' }],
+        output: {
+          result: '{{ steps.test.output.testOutput }}',
+        },
+        values: {},
      },
-      values: {},
    });

    const task = await broker.claim();
@@ -122,12 +124,14 @@ describe('TaskWorker', () => {
    });

    await broker.dispatch({
-      apiVersion: 'scaffolder.backstage.io/v1beta3',
-      steps: [{ id: 'test', name: 'test', action: 'not-found-action' }],
-      output: {
-        result: '{{ steps.test.output.testOutput }}',
+      spec: {
+        apiVersion: 'scaffolder.backstage.io/v1beta3',
+        steps: [{ id: 'test', name: 'test', action: 'not-found-action' }],
+        output: {
+          result: '{{ steps.test.output.testOutput }}',
+        },
+        parameters: {},
      },
-      parameters: {},
    });

    const task = await broker.claim();
@@ -151,12 +155,14 @@ describe('TaskWorker', () => {
    });

    const { taskId } = await broker.dispatch({
-      apiVersion: 'scaffolder.backstage.io/v1beta3',
-      steps: [{ id: 'test', name: 'test', action: 'not-found-action' }],
-      output: {
-        result: '{{ steps.test.output.testOutput }}',
+      spec: {
+        apiVersion: 'scaffolder.backstage.io/v1beta3',
+        steps: [{ id: 'test', name: 'test', action: 'not-found-action' }],
+        output: {
+          result: '{{ steps.test.output.testOutput }}',
+        },
+        parameters: {},
      },
-      parameters: {},
    });

    const task = await broker.claim();
@@ -21,6 +21,7 @@ export type { CreateWorkerOptions } from './TaskWorker';
 export type {
  TaskSecrets,
  TaskSpec,
+  TaskCompletionState,
  CompletedTaskState,
  TaskStoreEmitOptions,
  TaskStoreListEventsOptions,
@@ -29,10 +30,15 @@ export type {
  TaskSpecV1beta2,
  TaskSpecV1beta3,
  Status,
+  TaskStatus,
  TaskEventType,
  TaskBroker,
  TaskContext,
  TaskStore,
  DispatchResult,
+  TaskBrokerDispatchResult,
+  TaskBrokerDispatchOptions,
+  TaskStoreCreateTaskOptions,
+  TaskStoreCreateTaskResult,
  TemplateMetadata,
 } from './types';
@@ -32,11 +32,11 @@ export type {
 };

 /**
- * Status
+ * The status of each step of the Task
 *
 * @public
 */
-export type Status =
+export type TaskStatus =
  | 'open'
  | 'processing'
  | 'failed'
@@ -44,11 +44,27 @@ export type Status =
  | 'completed';

 /**
- * CompletedTaskState
+ * The status of each step of the Task
+ *
+ * @public
+ * @deprecated use TaskStatus instead
+ */
+export type Status = TaskStatus;
+
+/**
+ * The state of a completed task.
 *
 * @public
 */
-export type CompletedTaskState = 'failed' | 'completed';
+export type TaskCompletionState = 'failed' | 'completed';
+
+/**
+ * The state of a completed task.
+ *
+ * @public
+ * @deprecated use TaskCompletionState instead
+ */
+export type CompletedTaskState = TaskCompletionState;

 /**
 * SerializedTask
@@ -58,7 +74,7 @@ export type CompletedTaskState = 'failed' | 'completed';
 export type SerializedTask = {
  id: string;
  spec: TaskSpec;
-  status: Status;
+  status: TaskStatus;
  createdAt: string;
  lastHeartbeatAt?: string;
  secrets?: TaskSecrets;
@@ -94,14 +110,33 @@ export type TaskSecrets = Record<string, string> & {
 };

 /**
- * DispatchResult
+ * The result of {@link TaskBroker.dispatch}
 *
 * @public
 */
-export type DispatchResult = {
+export type TaskBrokerDispatchResult = {
  taskId: string;
 };

+/**
+ * The options passed to {@link TaskBroker.dispatch}
+ * Currently a spec and optional secrets
+ *
+ * @public
+ */
+export type TaskBrokerDispatchOptions = {
+  spec: TaskSpec;
+  secrets?: TaskSecrets;
+};
+
+/**
+ * DispatchResult
+ *
+ * @public
+ * @deprecated use TaskBrokerDispatchResult instead
+ */
+export type DispatchResult = TaskBrokerDispatchResult;
+
 /**
 * Task
 *
@@ -112,7 +147,7 @@ export interface TaskContext {
  secrets?: TaskSecrets;
  done: boolean;
  emitLog(message: string, metadata?: JsonValue): Promise<void>;
-  complete(result: CompletedTaskState, metadata?: JsonValue): Promise<void>;
+  complete(result: TaskCompletionState, metadata?: JsonValue): Promise<void>;
  getWorkspaceName(): Promise<string>;
 }

@@ -123,7 +158,9 @@ export interface TaskContext {
 */
 export interface TaskBroker {
  claim(): Promise<TaskContext>;
-  dispatch(spec: TaskSpec, secrets?: TaskSecrets): Promise<DispatchResult>;
+  dispatch(
+    options: TaskBrokerDispatchOptions,
+  ): Promise<TaskBrokerDispatchResult>;
  vacuumTasks(timeoutS: { timeoutS: number }): Promise<void>;
  observe(
    options: {
@@ -158,6 +195,23 @@ export type TaskStoreListEventsOptions = {
  after?: number | undefined;
 };

+/**
+ * The options passed to {@link TaskStore.createTask}
+ * @public
+ */
+export type TaskStoreCreateTaskOptions = {
+  spec: TaskSpec;
+  secrets?: TaskSecrets;
+};
+
+/**
+ * The response from {@link TaskStore.createTask}
+ * @public
+ */
+export type TaskStoreCreateTaskResult = {
+  taskId: string;
+};
+
 /**
 * TaskStore
 *
@@ -165,14 +219,13 @@ export type TaskStoreListEventsOptions = {
 */
 export interface TaskStore {
  createTask(
-    task: TaskSpec,
-    secrets?: TaskSecrets,
-  ): Promise<{ taskId: string }>;
+    options: TaskStoreCreateTaskOptions,
+  ): Promise<TaskStoreCreateTaskResult>;
  getTask(taskId: string): Promise<SerializedTask>;
  claimTask(): Promise<SerializedTask | undefined>;
  completeTask(options: {
    taskId: string;
-    status: Status;
+    status: TaskStatus;
    eventBody: JsonObject;
  }): Promise<void>;
  heartbeatTask(taskId: string): Promise<void>;
@@ -266,9 +266,12 @@ export async function createRouter(
        );
      }

-      const result = await taskBroker.dispatch(taskSpec, {
-        ...req.body.secrets,
-        backstageToken: token,
+      const result = await taskBroker.dispatch({
+        spec: taskSpec,
+        secrets: {
+          ...req.body.secrets,
+          backstageToken: token,
+        },
      });

      res.status(201).json({ id: result.taskId });