refactor: provider-registry:create(Container|Kubernetes)ProviderConnection (podman-desktop#11809)

* refactor: create*ProviderConnection
Signed-off-by: Philippe Martin <phmartin@redhat.com>

* fix: review
Signed-off-by: Philippe Martin <phmartin@redhat.com>

* refactor: create TaskConnectionUtils
Signed-off-by: Philippe Martin <phmartin@redhat.com>

* fix: review
Signed-off-by: Philippe Martin <phmartin@redhat.com>

Author: feloy

packages/main/src/plugin/index.spec.ts

@@ -23,11 +23,12 @@ import { tmpdir } from 'node:os';
 import type { PullEvent } from '@podman-desktop/api';
 import type { WebContents } from 'electron';
 import { app, BrowserWindow, clipboard, ipcMain, shell } from 'electron';
-import { beforeAll, beforeEach, expect, test, vi } from 'vitest';
+import { beforeAll, beforeEach, describe, expect, test, vi } from 'vitest';
 
 import { ExtensionLoader } from '/@/plugin/extension/extension-loader.js';
 import { Updater } from '/@/plugin/updater.js';
 import type { NotificationCardOptions } from '/@api/notification.js';
+import type { ProviderInfo } from '/@api/provider-info.js';
 
 import { securityRestrictionCurrentHandler } from '../security-restrictions-handler.js';
 import type { TrayMenu } from '../tray-menu.js';
@@ -37,9 +38,14 @@ import type { ConfigurationRegistry } from './configuration-registry.js';
 import { ContainerProviderRegistry } from './container-registry.js';
 import type { Directories } from './directories.js';
 import { Emitter } from './events/emitter.js';
+import type { LoggerWithEnd } from './index.js';
 import { PluginSystem } from './index.js';
 import type { MessageBox } from './message-box.js';
+import { NavigationManager } from './navigation/navigation-manager.js';
+import { ProviderRegistry } from './provider-registry.js';
+import { TaskImpl } from './tasks/task-impl.js';
 import { TaskManager } from './tasks/task-manager.js';
+import type { Task } from './tasks/tasks.js';
 import { Disposable } from './types/disposable.js';
 import { Deferred } from './util/deferred.js';
 import { HttpServer } from './webview/webview-registry.js';
@@ -413,3 +419,97 @@ test('ipcMain.handle returns caught error as objects message property if it is n
   const handleReturn = await handle(undefined, '1');
   expect(handleReturn.error).toEqual({ message: nonErrorInstance });
 });
+
+describe.each<{
+  handler: string;
+  methodName: 'createContainerProviderConnection' | 'createKubernetesProviderConnection';
+}>([
+  {
+    handler: 'provider-registry:createContainerProviderConnection',
+    methodName: 'createContainerProviderConnection',
+  },
+  {
+    handler: 'provider-registry:createKubernetesProviderConnection',
+    methodName: 'createKubernetesProviderConnection',
+  },
+])('$handler', async ({ handler, methodName }) => {
+  let originalTask: Task;
+
+  beforeEach(() => {
+    originalTask = {
+      status: 'in-progress',
+      error: '',
+    } as unknown as Task;
+    vi.spyOn(TaskManager.prototype, 'createTask').mockReturnValue(originalTask);
+    vi.spyOn(NavigationManager.prototype, 'navigateToProviderTask');
+    vi.spyOn(ProviderRegistry.prototype, 'getProviderInfo').mockReturnValue({
+      name: 'provider1',
+    } as ProviderInfo);
+  });
+
+  test('createTask is called', async () => {
+    await pluginSystem.initExtensions(new Emitter<ConfigurationRegistry>());
+    const handle = handlers.get(handler);
+    expect(handle).not.equal(undefined);
+    await handle(undefined, 'internal1', { key1: 'value1', key2: 42 }, 'logger1', 'token1', 'task1');
+    expect(TaskManager.prototype.createTask).toHaveBeenCalledOnce();
+    const params = vi.mocked(TaskManager.prototype.createTask).mock.calls[0]?.[0];
+    if (!params) {
+      // this is already expected
+      throw new Error('param should be defined');
+    }
+    expect(params.title).toEqual(`Creating provider1 provider`);
+    expect(params.action?.name).toEqual(`Open task`);
+
+    // check that action.execute passed to createTask is calling navigateToProviderTask
+    const execute = params.action?.execute;
+    expect(execute).toBeDefined();
+    if (!execute) {
+      throw new Error('execute should be defined');
+    }
+    execute(new TaskImpl('task1id', 'task1name'));
+    expect(NavigationManager.prototype.navigateToProviderTask).toHaveBeenCalledOnce();
+    expect(NavigationManager.prototype.navigateToProviderTask).toHaveBeenCalledWith('internal1', 'task1');
+  });
+
+  test(`${methodName} is called and is resolved`, async () => {
+    vi.spyOn(ProviderRegistry.prototype, methodName).mockResolvedValue();
+    const onEndMock = vi.fn();
+    const errorMock = vi.fn();
+    vi.spyOn(pluginSystem, 'getLogHandler').mockReturnValue({
+      onEnd: onEndMock,
+      error: errorMock,
+    } as unknown as LoggerWithEnd);
+    await pluginSystem.initExtensions(new Emitter<ConfigurationRegistry>());
+    const handle = handlers.get(handler);
+    expect(handle).not.equal(undefined);
+    const result = await handle(undefined, 'internal1', { key1: 'value1', key2: 42 }, 'logger1', 'token1', 'task1');
+    expect(result).toEqual({ result: undefined });
+    expect(onEndMock).toHaveBeenCalled();
+    expect(errorMock).not.toHaveBeenCalled();
+    expect(originalTask.status).toEqual('success');
+    expect(originalTask.error).toEqual('');
+  });
+
+  test(`${methodName} is called and is rejected`, async () => {
+    const rejectError = new Error('an error');
+    vi.spyOn(ProviderRegistry.prototype, methodName).mockRejectedValue(rejectError);
+    const onEndMock = vi.fn();
+    const errorMock = vi.fn();
+    vi.spyOn(pluginSystem, 'getLogHandler').mockReturnValue({
+      onEnd: onEndMock,
+      error: errorMock,
+    } as unknown as LoggerWithEnd);
+    await pluginSystem.initExtensions(new Emitter<ConfigurationRegistry>());
+    const handle = handlers.get(handler);
+    expect(handle).not.equal(undefined);
+    const result = await handle(undefined, 'internal1', { key1: 'value1', key2: 42 }, 'logger1', 'token1', 'task1');
+    expect(result).toEqual({
+      error: rejectError,
+    });
+    expect(onEndMock).toHaveBeenCalled();
+    expect(errorMock).toHaveBeenCalledWith(rejectError);
+    expect(originalTask.status).toEqual('in-progress');
+    expect(originalTask.error).toEqual('Something went wrong while trying to create provider: Error: an error');
+  });
+});

packages/main/src/plugin/index.ts

@@ -198,6 +198,7 @@ import type { IDisposable } from './types/disposable.js';
 import type { Deferred } from './util/deferred.js';
 import { Exec } from './util/exec.js';
 import { getFreePort, getFreePortRange, isFreePort } from './util/port.js';
+import { TaskConnectionUtils } from './util/task-connection-utils.js';
 import { ViewRegistry } from './view-registry.js';
 import { WebviewRegistry } from './webview/webview-registry.js';
 import { WelcomeInit } from './welcome/welcome-init.js';
@@ -2320,6 +2321,12 @@ export class PluginSystem {
       },
     );
 
+    const taskConnectionUtils = new TaskConnectionUtils({
+      getLogHandler: this.getLogHandler.bind(this),
+      cancellationTokenRegistry,
+      taskManager,
+    });
+
     this.ipcHandle(
       'provider-registry:createContainerProviderConnection',
       async (
@@ -2330,40 +2337,16 @@ export class PluginSystem {
         tokenId: number | undefined,
         taskId: number | undefined,
       ): Promise<void> => {
-        const logger = this.getLogHandler('provider-registry:taskConnection-onData', loggerId);
-        let token;
-        if (tokenId) {
-          const tokenSource = cancellationTokenRegistry.getCancellationTokenSource(tokenId);
-          token = tokenSource?.token;
-        }
-
         const providerName = providerRegistry.getProviderInfo(internalProviderId)?.name;
-        const task = taskManager.createTask({
+        return taskConnectionUtils.withTask({
+          loggerId,
+          tokenId,
           title: `Creating ${providerName ?? 'Container'} provider`,
-          action: {
-            name: 'Open task',
-            execute: () => {
-              navigationManager
-                .navigateToProviderTask(internalProviderId, taskId)
-                .catch((err: unknown) => console.error(err));
-            },
-          },
+          navigateToTask: () => navigationManager.navigateToProviderTask(internalProviderId, taskId),
+          execute: (logger: LoggerWithEnd, token?: containerDesktopAPI.CancellationToken) =>
+            providerRegistry.createContainerProviderConnection(internalProviderId, params, logger, token),
+          executeErrorMsg: (err: unknown) => `Something went wrong while trying to create provider: ${err}`,
         });
-
-        return providerRegistry
-          .createContainerProviderConnection(internalProviderId, params, logger, token)
-          .then(result => {
-            task.status = 'success';
-            return result;
-          })
-          .catch((err: unknown) => {
-            task.error = `Something went wrong while trying to create provider: ${err}`;
-            logger.error(err);
-            throw err;
-          })
-          .finally(() => {
-            logger.onEnd();
-          });
       },
     );
 
@@ -2388,40 +2371,16 @@ export class PluginSystem {
         tokenId: number | undefined,
         taskId: number | undefined,
       ): Promise<void> => {
-        const logger = this.getLogHandler('provider-registry:taskConnection-onData', loggerId);
-        let token;
-        if (tokenId) {
-          const tokenSource = cancellationTokenRegistry.getCancellationTokenSource(tokenId);
-          token = tokenSource?.token;
-        }
-
         const providerName = providerRegistry.getProviderInfo(internalProviderId)?.name;
-        const task = taskManager.createTask({
+        return taskConnectionUtils.withTask({
+          loggerId,
+          tokenId,
           title: `Creating ${providerName ?? 'Kubernetes'} provider`,
-          action: {
-            name: 'Open task',
-            execute: () => {
-              navigationManager
-                .navigateToProviderTask(internalProviderId, taskId)
-                .catch((err: unknown) => console.error(err));
-            },
-          },
+          navigateToTask: () => navigationManager.navigateToProviderTask(internalProviderId, taskId),
+          execute: (logger: LoggerWithEnd, token?: containerDesktopAPI.CancellationToken) =>
+            providerRegistry.createKubernetesProviderConnection(internalProviderId, params, logger, token),
+          executeErrorMsg: (err: unknown) => `Something went wrong while trying to create provider: ${err}`,
         });
-
-        return providerRegistry
-          .createKubernetesProviderConnection(internalProviderId, params, logger, token)
-          .then(result => {
-            task.status = 'success';
-            return result;
-          })
-          .catch((err: unknown) => {
-            task.error = `Something went wrong while trying to create provider: ${err}`;
-            logger.error(err);
-            throw err;
-          })
-          .finally(() => {
-            logger.onEnd();
-          });
       },
     );
 

packages/main/src/plugin/util/task-connection-utils.spec.ts

@@ -0,0 +1,141 @@
+/**********************************************************************
+ * Copyright (C) 2025 Red Hat, Inc.
+ *
+ * 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.
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ ***********************************************************************/
+
+import { beforeEach, expect, test, vi } from 'vitest';
+
+import { CancellationTokenSource } from '../cancellation-token.js';
+import type { CancellationTokenRegistry } from '../cancellation-token-registry.js';
+import type { LoggerWithEnd } from '../index.js';
+import { TaskImpl } from '../tasks/task-impl.js';
+import type { TaskManager } from '../tasks/task-manager.js';
+import type { Task } from '../tasks/tasks.js';
+import type { TaskOptions } from './task-connection-utils.js';
+import { TaskConnectionUtils } from './task-connection-utils.js';
+
+let originalTask: Task;
+let taskConnectionUtils: TaskConnectionUtils;
+
+const getLogHandlerResult = {
+  onEnd: vi.fn(),
+  error: vi.fn(),
+} as unknown as LoggerWithEnd;
+
+const cancellationTokenRegistry = {
+  getCancellationTokenSource: vi.fn(),
+} as unknown as CancellationTokenRegistry;
+
+const taskManager = {
+  createTask: vi.fn(),
+} as unknown as TaskManager;
+
+beforeEach(() => {
+  vi.resetAllMocks();
+  vi.restoreAllMocks();
+
+  originalTask = {
+    status: 'in-progress',
+    error: '',
+  } as unknown as Task;
+
+  const options = {
+    getLogHandler: vi.fn(),
+    cancellationTokenRegistry,
+    taskManager,
+  };
+
+  vi.mocked(options.getLogHandler).mockReturnValue(getLogHandlerResult);
+  vi.mocked(taskManager.createTask).mockReturnValue(originalTask);
+  vi.mocked(cancellationTokenRegistry.getCancellationTokenSource).mockReturnValue(new CancellationTokenSource());
+
+  taskConnectionUtils = new TaskConnectionUtils(options);
+});
+
+test('createTask is called', async () => {
+  const options: TaskOptions = {
+    loggerId: 'logger1',
+    tokenId: 42,
+    title: 'a title',
+    navigateToTask: vi.fn(),
+    execute: vi.fn(),
+    executeErrorMsg: vi.fn(),
+  };
+  vi.mocked(options.execute).mockResolvedValue();
+  vi.mocked(options.navigateToTask).mockResolvedValue();
+
+  await taskConnectionUtils.withTask(options);
+
+  expect(taskManager.createTask).toHaveBeenCalledOnce();
+  const createTaskParams = vi.mocked(taskManager.createTask).mock.calls[0]?.[0];
+  if (!createTaskParams) {
+    throw new Error('param should be defined');
+  }
+  expect(createTaskParams.title).toEqual('a title');
+  expect(createTaskParams.action?.name).toEqual(`Open task`);
+
+  // check that action.execute passed to createTask is calling options.navigateToTask
+  const execute = createTaskParams.action?.execute;
+  expect(execute).toBeDefined();
+  if (!execute) {
+    throw new Error('execute should be defined');
+  }
+  execute(new TaskImpl('id', 'name'));
+  expect(options.navigateToTask).toHaveBeenCalled();
+});
+
+test(`options.execute is called and is resolved`, async () => {
+  const options: TaskOptions = {
+    loggerId: 'logger1',
+    tokenId: 42,
+    title: 'a title',
+    navigateToTask: vi.fn(),
+    execute: vi.fn(),
+    executeErrorMsg: vi.fn(),
+  };
+  vi.mocked(options.execute).mockResolvedValue();
+  vi.mocked(options.navigateToTask).mockResolvedValue();
+
+  const result = await taskConnectionUtils.withTask(options);
+
+  expect(result).toBeUndefined();
+  expect(getLogHandlerResult.onEnd).toHaveBeenCalled();
+  expect(getLogHandlerResult.error).not.toHaveBeenCalled();
+  expect(originalTask.status).toEqual('success');
+  expect(originalTask.error).toEqual('');
+});
+
+test(`options.execute is called and is rejected`, async () => {
+  const rejectError = new Error('an error');
+
+  const options: TaskOptions = {
+    loggerId: 'logger1',
+    tokenId: 42,
+    title: 'a title',
+    navigateToTask: vi.fn(),
+    execute: vi.fn(),
+    executeErrorMsg: vi.fn(),
+  };
+  vi.mocked(options.execute).mockRejectedValue(rejectError);
+  vi.mocked(options.navigateToTask).mockResolvedValue();
+  vi.mocked(options.executeErrorMsg).mockReturnValue('an error msg');
+  await expect(() => taskConnectionUtils.withTask(options)).rejects.toThrow();
+
+  expect(getLogHandlerResult.onEnd).toHaveBeenCalled();
+  expect(getLogHandlerResult.error).toHaveBeenCalledWith(rejectError);
+  expect(originalTask.status).toEqual('in-progress');
+  expect(originalTask.error).toEqual('an error msg');
+});