Skip to content

Commit

Permalink
chore: introduce cancellable field in the tasks (podman-desktop#10065)
Browse files Browse the repository at this point in the history
* chore: introduce cancellable field in the tasks

if cancellable attribute is provided, it registers
the token in the registry so frontend could ask
to cancel this specific token

This PR is not providing any frontend code to cancel the task

related to podman-desktop#9092
Signed-off-by: Florent Benoit <[email protected]>
  • Loading branch information
benoitf authored Nov 25, 2024
1 parent ef90803 commit f471c3e
Show file tree
Hide file tree
Showing 16 changed files with 228 additions and 6 deletions.
2 changes: 2 additions & 0 deletions packages/api/src/taskInfo.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,4 +35,6 @@ export interface TaskInfo {
error?: string;
progress?: number;
action?: string;
cancellable: boolean;
cancellationTokenSourceId?: number;
}
2 changes: 1 addition & 1 deletion packages/main/src/plugin/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -674,7 +674,7 @@ export class PluginSystem {
apiSender,
trayMenuRegistry,
messageBox,
new ProgressImpl(taskManager, navigationManager),
new ProgressImpl(taskManager, navigationManager, cancellationTokenRegistry),
statusBarRegistry,
kubernetesClient,
fileSystemMonitoring,
Expand Down
64 changes: 63 additions & 1 deletion packages/main/src/plugin/tasks/progress-impl.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ import type { NavigationManager } from '/@/plugin/navigation/navigation-manager.
import type { TaskAction } from '/@/plugin/tasks/tasks.js';
import type { TaskState, TaskStatus } from '/@api/taskInfo.js';

import { CancellationTokenSource } from '../cancellation-token.js';
import type { CancellationTokenRegistry } from '../cancellation-token-registry.js';
import { ProgressImpl, ProgressLocation } from './progress-impl.js';
import { TaskImpl } from './task-impl.js';
import type { TaskManager } from './task-manager.js';
Expand All @@ -37,6 +39,11 @@ const navigationManager = {
navigateToRoute: vi.fn(),
} as unknown as NavigationManager;

const cancellationTokenRegistry = {
createCancellationTokenSource: vi.fn(),
getCancellationTokenSource: vi.fn(),
} as unknown as CancellationTokenRegistry;

class TestTaskImpl extends TaskImpl {
constructor(id: string, name: string, state: TaskState, status: TaskStatus) {
super(id, name);
Expand All @@ -48,7 +55,7 @@ class TestTaskImpl extends TaskImpl {
let progress: ProgressImpl;
beforeEach(() => {
vi.clearAllMocks();
progress = new ProgressImpl(taskManager, navigationManager);
progress = new ProgressImpl(taskManager, navigationManager, cancellationTokenRegistry);
});

test('Should create a task and report update', async () => {
Expand Down Expand Up @@ -151,3 +158,58 @@ test('Should create a task with a navigation action', async () => {
// ensure the arguments and routeId is properly used
expect(navigationManager.navigateToRoute).toHaveBeenCalledWith('dummy-route-id', 'hello', 'world');
});

test('Should create a cancellable task with a source id if cancellable option provided ', async () => {
const dummyTask = new TestTaskImpl('test-task-id', 'test-title', 'running', 'in-progress');
vi.mocked(taskManager.createTask).mockReturnValue(dummyTask);

const tokenSourceId = 1234;
// get id for the token source
vi.mocked(cancellationTokenRegistry.createCancellationTokenSource).mockReturnValue(tokenSourceId);

//get the token source
vi.mocked(cancellationTokenRegistry.getCancellationTokenSource).mockReturnValue(new CancellationTokenSource());

await progress.withProgress(
{ location: ProgressLocation.TASK_WIDGET, title: 'My task', cancellable: true },
async progress => {
progress.report({ increment: 50 });
},
);

// grab the options passed to createTask
const options = vi.mocked(taskManager.createTask).mock.calls[0]?.[0];
expect(options).toBeDefined();
// expect callable has been set
expect(options?.cancellable).toBeTruthy();
// expect the token source id to be set
expect(options?.cancellationTokenSourceId).toBe(tokenSourceId);

// check that the token source was created
expect(cancellationTokenRegistry.createCancellationTokenSource).toHaveBeenCalled();
expect(cancellationTokenRegistry.getCancellationTokenSource).toHaveBeenCalled();
});

test('Should not provide cancellable and source id if cancellable option is omitted ', async () => {
const dummyTask = new TestTaskImpl('test-task-id', 'test-title', 'running', 'in-progress');
vi.mocked(taskManager.createTask).mockReturnValue(dummyTask);

await progress.withProgress(
{ location: ProgressLocation.TASK_WIDGET, title: 'My task', cancellable: false },
async progress => {
progress.report({ increment: 50 });
},
);

// grab the options passed to createTask
const options = vi.mocked(taskManager.createTask).mock.calls[0]?.[0];
expect(options).toBeDefined();
// expect callable not provided
expect(options?.cancellable).toBeFalsy();
// expect the token source id not being set
expect(options?.cancellationTokenSourceId).toBeUndefined();

// check that the cancellationTokenRegistry was never called
expect(cancellationTokenRegistry.createCancellationTokenSource).not.toHaveBeenCalled();
expect(cancellationTokenRegistry.getCancellationTokenSource).not.toHaveBeenCalled();
});
25 changes: 24 additions & 1 deletion packages/main/src/plugin/tasks/progress-impl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import type { NavigationManager } from '/@/plugin/navigation/navigation-manager.
import type { TaskAction } from '/@/plugin/tasks/tasks.js';

import { CancellationTokenImpl } from '../cancellation-token.js';
import type { CancellationTokenRegistry } from '../cancellation-token-registry.js';
import type { TaskManager } from './task-manager.js';

export enum ProgressLocation {
Expand All @@ -40,6 +41,7 @@ export class ProgressImpl {
constructor(
private taskManager: TaskManager,
private navigationManager: NavigationManager,
private cancellationTokenRegistry: CancellationTokenRegistry,
) {}

/**
Expand Down Expand Up @@ -107,8 +109,29 @@ export class ProgressImpl {
token: extensionApi.CancellationToken,
) => Promise<R>,
): Promise<R> {
const isCancellable = options.cancellable ?? false;
let cancellationToken: extensionApi.CancellationToken;
let cancellationTokenSourceId: number | undefined;

// if cancellable, register the token source and provides the source id to the task so frontend can cancel the task
if (isCancellable) {
cancellationTokenSourceId = this.cancellationTokenRegistry.createCancellationTokenSource();
const cancellationTokenSource =
this.cancellationTokenRegistry.getCancellationTokenSource(cancellationTokenSourceId);
// no token, error
if (!cancellationTokenSource) {
throw new Error('Failed to create CancellationTokenSource');
}
cancellationToken = cancellationTokenSource.token;
} else {
cancellationToken = new CancellationTokenImpl();
}

const t = this.taskManager.createTask({
title: options.title,
// if the task is cancellable, we set the token source id
cancellable: isCancellable,
cancellationTokenSourceId,
action: this.getTaskAction(options),
});

Expand All @@ -123,7 +146,7 @@ export class ProgressImpl {
}
},
},
new CancellationTokenImpl(),
cancellationToken,
)
.then(value => {
// Middleware to capture the success of the task
Expand Down
28 changes: 28 additions & 0 deletions packages/main/src/plugin/tasks/task-impl.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,34 @@ describe('update field should send an update event', () => {
}),
});
});

test('cancellable', () => {
const onUpdateListenerMock = vi.fn<(e: TaskUpdateEvent) => void>();
const task = new TaskImpl('test-id', 'Test name');
task.onUpdate(onUpdateListenerMock);

task.cancellable = true;
expect(onUpdateListenerMock).toHaveBeenCalledWith({
action: 'update',
task: expect.objectContaining({
cancellable: true,
}),
});
});

test('cancellationTokenSourceId', () => {
const onUpdateListenerMock = vi.fn<(e: TaskUpdateEvent) => void>();
const task = new TaskImpl('test-id', 'Test name');
task.onUpdate(onUpdateListenerMock);

task.cancellationTokenSourceId = 123;
expect(onUpdateListenerMock).toHaveBeenCalledWith({
action: 'update',
task: expect.objectContaining({
cancellationTokenSourceId: 123,
}),
});
});
});

test('dispose should send a delete TaskUpdateEvent', () => {
Expand Down
20 changes: 20 additions & 0 deletions packages/main/src/plugin/tasks/task-impl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ export class TaskImpl implements Task {
protected mState: TaskState;
protected mStatus: TaskStatus;
protected mName: string;
protected mCancellable = false;
protected mCancellationTokenSourceId: number | undefined;

constructor(
public readonly id: string,
Expand Down Expand Up @@ -114,6 +116,24 @@ export class TaskImpl implements Task {
this.emitter?.fire({ action: action, task: this });
}

set cancellable(value: boolean) {
this.mCancellable = value;
this.notify();
}

get cancellable(): boolean {
return this.mCancellable;
}

set cancellationTokenSourceId(value: number) {
this.mCancellationTokenSourceId = value;
this.notify();
}

get cancellationTokenSourceId(): number | undefined {
return this.mCancellationTokenSourceId;
}

dispose(): void {
this.notify('delete');
this.emitter?.dispose();
Expand Down
36 changes: 36 additions & 0 deletions packages/main/src/plugin/tasks/task-manager.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -298,6 +298,42 @@ test('clear tasks should clear task not in running state', async () => {
);
});

test('create task being cancellable', async () => {
const taskManager = new TaskManager(apiSender, statusBarRegistry, commandRegistry, configurationRegistry);
const task = taskManager.createTask({ cancellable: true, cancellationTokenSourceId: 1 });
expect(task.id).equal('task-1');
expect(task.name).equal('Task 1');
expect(task.state).equal('running');
expect(task.cancellable).toBeTruthy();
expect(task.cancellationTokenSourceId).toEqual(1);
expect(apiSenderSendMock).toBeCalledWith(
'task-created',
expect.objectContaining({
id: task.id,
name: task.name,
state: task.state,
cancellable: true,
cancellationTokenSourceId: 1,
}),
);
});

test('create task being cancellable throw error if missing cancellationTokenSourceId', async () => {
const taskManager = new TaskManager(apiSender, statusBarRegistry, commandRegistry, configurationRegistry);

expect(() => taskManager.createTask({ cancellable: true })).toThrow(
'cancellable task requires a cancellationTokenSourceId',
);
});

test('create task having cancellationTokenSourceId without being cancellable throw error', async () => {
const taskManager = new TaskManager(apiSender, statusBarRegistry, commandRegistry, configurationRegistry);

expect(() => taskManager.createTask({ cancellationTokenSourceId: 4 })).toThrow(
'cancellationTokenSourceId is only allowed for cancellable tasks',
);
});

describe('execute', () => {
test('execute should throw an error if the task does not exist', async () => {
const taskManager = new TaskManager(apiSender, statusBarRegistry, commandRegistry, configurationRegistry);
Expand Down
21 changes: 20 additions & 1 deletion packages/main/src/plugin/tasks/task-manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -113,11 +113,26 @@ export class TaskManager {
this.setStatusBarEntry(true);
}

public createTask(options?: { title?: string; action?: TaskAction }): Task {
public createTask(options?: {
title?: string;
action?: TaskAction;
cancellable?: boolean;
cancellationTokenSourceId?: number;
}): Task {
this.taskId++;

const task = new TaskImpl(`task-${this.taskId}`, options?.title ? options.title : `Task ${this.taskId}`);
task.action = options?.action;
task.cancellable = options?.cancellable ?? false;
if (task.cancellable && !options?.cancellationTokenSourceId) {
throw new Error('cancellable task requires a cancellationTokenSourceId');
}
if (options?.cancellationTokenSourceId && !task.cancellable) {
throw new Error('cancellationTokenSourceId is only allowed for cancellable tasks');
}
if (options?.cancellationTokenSourceId) {
task.cancellationTokenSourceId = options?.cancellationTokenSourceId;
}
this.registerTask(task);
return task;
}
Expand Down Expand Up @@ -162,6 +177,8 @@ export class TaskManager {
body: task.body ?? '',
state: 'completed',
error: undefined,
cancellable: task.cancellable,
cancellationTokenSourceId: task.cancellationTokenSourceId,
};
}

Expand All @@ -174,6 +191,8 @@ export class TaskManager {
progress: task.progress,
error: task.error,
action: task.action?.name,
cancellable: task.cancellable,
cancellationTokenSourceId: task.cancellationTokenSourceId,
};
}

Expand Down
2 changes: 2 additions & 0 deletions packages/main/src/plugin/tasks/tasks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,5 +39,7 @@ export interface Task extends Disposable {
error?: string;
progress?: number;
action?: TaskAction;
cancellable: boolean;
cancellationTokenSourceId?: number;
readonly onUpdate: Event<TaskUpdateEvent>;
}
1 change: 1 addition & 0 deletions packages/renderer/src/lib/statusbar/StatusBar.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ beforeEach(() => {
status: 'in-progress',
started: 0,
id: 'dummy-task',
cancellable: false,
},
]);
});
Expand Down
6 changes: 6 additions & 0 deletions packages/renderer/src/lib/statusbar/TaskIndicator.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ test('clicking on task should send task manager toggle event', async () => {
status: 'in-progress',
started: 0,
id: 'dummy-task',
cancellable: false,
},
]);

Expand All @@ -68,6 +69,7 @@ test('one task running should display it', async () => {
status: 'in-progress',
started: 0,
id: 'dummy-task',
cancellable: false,
},
]);

Expand All @@ -85,13 +87,15 @@ test('multiple tasks running should display them', async () => {
status: 'in-progress',
started: 0,
id: 'foo-task',
cancellable: false,
},
{
name: 'Bar Task',
state: 'running',
status: 'in-progress',
started: 0,
id: 'foo-task',
cancellable: false,
},
]);

Expand All @@ -110,6 +114,7 @@ test('task with defined progress value should display it', async () => {
started: 0,
id: 'dummy-task',
progress: 50,
cancellable: false,
},
]);

Expand All @@ -127,6 +132,7 @@ test('task with undefined progress value should show indeterminate progress', as
started: 0,
id: 'dummy-task',
progress: undefined, // indeterminate
cancellable: false,
},
]);

Expand Down
Loading

0 comments on commit f471c3e

Please sign in to comment.