fix: Refactor task and handler classes

src/commands/base.ts

@@ -2,7 +2,6 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
-import paths from 'path';
 import {MissingArgumentError, SoloError} from '../core/errors.js';
 import {ShellRunner} from '../core/shell_runner.js';
 import {type LeaseManager} from '../core/lease/lease_manager.js';
@@ -21,10 +20,7 @@ import path from 'path';
 import * as constants from '../core/constants.js';
 import fs from 'fs';
 import {Task} from '../core/task.js';
-
-export interface CommandHandlers {
-  parent: BaseCommand;
-}
+import {getConfig} from '../core/config_builder.js';
 
 export abstract class BaseCommand extends ShellRunner {
   protected readonly helm: Helm;
@@ -58,32 +54,6 @@ export abstract class BaseCommand extends ShellRunner {
     this.remoteConfigManager = opts.remoteConfigManager;
   }
 
-  async prepareChartPath(chartDir: string, chartRepo: string, chartReleaseName: string) {
-    if (!chartRepo) throw new MissingArgumentError('chart repo name is required');
-    if (!chartReleaseName) throw new MissingArgumentError('chart release name is required');
-
-    if (chartDir) {
-      const chartPath = path.join(chartDir, chartReleaseName);
-      await this.helm.dependency('update', chartPath);
-      return chartPath;
-    }
-
-    return `${chartRepo}/${chartReleaseName}`;
-  }
-
-  prepareValuesFiles(valuesFile: string) {
-    let valuesArg = '';
-    if (valuesFile) {
-      const valuesFiles = valuesFile.split(',');
-      valuesFiles.forEach(vf => {
-        const vfp = paths.resolve(vf);
-        valuesArg += ` --values ${vfp}`;
-      });
-    }
-
-    return valuesArg;
-  }
-
   getConfigManager(): ConfigManager {
     return this.configManager;
   }
@@ -98,71 +68,7 @@ export abstract class BaseCommand extends ShellRunner {
    * getUnusedConfigs() to get an array of unused properties.
    */
   getConfig(configName: string, flags: CommandFlag[], extraProperties: string[] = []): object {
-    const configManager = this.configManager;
-
-    // build the dynamic class that will keep track of which properties are used
-    const NewConfigClass = class {
-      private usedConfigs: Map<string, number>;
-      constructor() {
-        // the map to keep track of which properties are used
-        this.usedConfigs = new Map();
-
-        // add the flags as properties to this class
-        flags?.forEach(flag => {
-          // @ts-ignore
-          this[`_${flag.constName}`] = configManager.getFlag(flag);
-          Object.defineProperty(this, flag.constName, {
-            get() {
-              this.usedConfigs.set(flag.constName, this.usedConfigs.get(flag.constName) + 1 || 1);
-              return this[`_${flag.constName}`];
-            },
-          });
-        });
-
-        // add the extra properties as properties to this class
-        extraProperties?.forEach(name => {
-          // @ts-ignore
-          this[`_${name}`] = '';
-          Object.defineProperty(this, name, {
-            get() {
-              this.usedConfigs.set(name, this.usedConfigs.get(name) + 1 || 1);
-              return this[`_${name}`];
-            },
-            set(value) {
-              this[`_${name}`] = value;
-            },
-          });
-        });
-      }
-
-      /** Get the list of unused configurations that were not accessed */
-      getUnusedConfigs() {
-        const unusedConfigs: string[] = [];
-
-        // add the flag constName to the unusedConfigs array if it was not accessed
-        flags?.forEach(flag => {
-          if (!this.usedConfigs.has(flag.constName)) {
-            unusedConfigs.push(flag.constName);
-          }
-        });
-
-        // add the extra properties to the unusedConfigs array if it was not accessed
-        extraProperties?.forEach(item => {
-          if (!this.usedConfigs.has(item)) {
-            unusedConfigs.push(item);
-          }
-        });
-        return unusedConfigs;
-      }
-    };
-
-    const newConfigInstance = new NewConfigClass();
-
-    // add the new instance to the configMaps so that it can be used to get the
-    // unused configurations using the configName from the BaseCommand
-    this._configMaps.set(configName, newConfigInstance);
-
-    return newConfigInstance;
+    return getConfig(this.configManager, this._configMaps, configName, flags, extraProperties);
   }
 
   getLeaseManager(): LeaseManager {
@@ -191,38 +97,18 @@ export abstract class BaseCommand extends ShellRunner {
 
   abstract close(): Promise<void>;
 
-  commandActionBuilder(actionTasks: any, options: any, errorString: string, lease: Lease | null) {
-    return async function (argv: any, commandDef: CommandHandlers) {
-      const tasks = new Listr([...actionTasks], options);
-
-      try {
-        await tasks.run();
-      } catch (e: Error | any) {
-        commandDef.parent.logger.error(`${errorString}: ${e.message}`, e);
-        throw new SoloError(`${errorString}: ${e.message}`, e);
-      } finally {
-        const promises = [];
-
-        promises.push(commandDef.parent.close());
-
-        if (lease) promises.push(lease.release());
-        await Promise.all(promises);
-      }
-    };
-  }
-
   /**
    * Setup home directories
    * @param dirs a list of directories that need to be created in sequence
    */
-  setupHomeDirectory(
+  public setupHomeDirectory(
     dirs: string[] = [
       constants.SOLO_HOME_DIR,
       constants.SOLO_LOGS_DIR,
       constants.SOLO_CACHE_DIR,
       constants.SOLO_VALUES_DIR,
     ],
-  ) {
+  ): string[] {
     const self = this;
 
     try {
@@ -233,14 +119,14 @@ export abstract class BaseCommand extends ShellRunner {
         self.logger.debug(`OK: setup directory: ${dirPath}`);
       });
     } catch (e: Error | any) {
-      this.logger.error(e);
+      self.logger.error(e);
       throw new SoloError(`failed to create directory: ${e.message}`, e);
     }
 
     return dirs;
   }
 
-  setupHomeDirectoryTask() {
+  public setupHomeDirectoryTask(): Task {
     return new Task('Setup home directory', async () => {
       this.setupHomeDirectory();
     });

src/commands/cluster/configs.ts

@@ -23,8 +23,7 @@ export const connectConfigBuilder = async function (argv, ctx, task) {
 };
 
 export const setupConfigBuilder = async function (argv, ctx, task) {
-  const parent = this.parent;
-  const configManager = parent.getConfigManager();
+  const configManager = this.configManager;
   configManager.update(argv);
   flags.disablePrompts([flags.chartDirectory]);
 
@@ -47,11 +46,12 @@ export const setupConfigBuilder = async function (argv, ctx, task) {
     soloChartVersion: configManager.getFlag(flags.soloChartVersion) as string,
   } as ClusterSetupConfigClass;
 
-  parent.logger.debug('Prepare ctx.config', {config: ctx.config, argv});
+  this.logger.debug('Prepare ctx.config', {config: ctx.config, argv});
 
-  ctx.isChartInstalled = await parent
-    .getChartManager()
-    .isChartInstalled(ctx.config.clusterSetupNamespace, constants.SOLO_CLUSTER_SETUP_CHART);
+  ctx.isChartInstalled = await this.chartManager.isChartInstalled(
+    ctx.config.clusterSetupNamespace,
+    constants.SOLO_CLUSTER_SETUP_CHART,
+  );
 
   return ctx.config;
 };
@@ -70,16 +70,17 @@ export const resetConfigBuilder = async function (argv, ctx, task) {
     }
   }
 
-  this.parent.getConfigManager().update(argv);
+  this.configManager.update(argv);
 
   ctx.config = {
-    clusterName: this.parent.getConfigManager().getFlag(flags.clusterName) as string,
-    clusterSetupNamespace: this.parent.getConfigManager().getFlag(flags.clusterSetupNamespace) as string,
+    clusterName: this.configManager.getFlag(flags.clusterName) as string,
+    clusterSetupNamespace: this.configManager.getFlag(flags.clusterSetupNamespace) as string,
   } as ClusterResetConfigClass;
 
-  ctx.isChartInstalled = await this.parent
-    .getChartManager()
-    .isChartInstalled(ctx.config.clusterSetupNamespace, constants.SOLO_CLUSTER_SETUP_CHART);
+  ctx.isChartInstalled = await this.chartManager.isChartInstalled(
+    ctx.config.clusterSetupNamespace,
+    constants.SOLO_CLUSTER_SETUP_CHART,
+  );
   if (!ctx.isChartInstalled) {
     throw new SoloError('No chart found for the cluster');
   }

src/commands/cluster/handlers.ts

@@ -1,39 +1,50 @@
 /**
  * SPDX-License-Identifier: Apache-2.0
  */
-import {type BaseCommand, type CommandHandlers} from '../base.js';
-import {type ClusterCommandTasks} from './tasks.js';
+import {ClusterCommandTasks} from './tasks.js';
 import * as helpers from '../../core/helpers.js';
 import * as constants from '../../core/constants.js';
 import * as ContextFlags from './flags.js';
 import {ListrRemoteConfig} from '../../core/config/remote/listr_config_tasks.js';
-import {type RemoteConfigManager} from '../../core/config/remote/remote_config_manager.js';
+import {RemoteConfigManager} from '../../core/config/remote/remote_config_manager.js';
 import {connectConfigBuilder, resetConfigBuilder, setupConfigBuilder} from './configs.js';
 import {SoloError} from '../../core/errors.js';
-
-export class ClusterCommandHandlers implements CommandHandlers {
-  readonly parent: BaseCommand;
-  readonly tasks: ClusterCommandTasks;
-  public readonly remoteConfigManager: RemoteConfigManager;
-  private getConfig: any;
-
-  constructor(parent: BaseCommand, tasks: ClusterCommandTasks, remoteConfigManager: RemoteConfigManager) {
-    this.parent = parent;
-    this.tasks = tasks;
-    this.remoteConfigManager = remoteConfigManager;
-    this.getConfig = parent.getConfig.bind(parent);
+import {inject, injectable} from 'tsyringe-neo';
+import {patchInject} from '../../core/container_helper.js';
+import {K8Client} from '../../core/kube/k8_client.js';
+import {type K8} from '../../core/kube/k8.js';
+import {CommandHandler} from '../../core/command_handler.js';
+import {LocalConfig} from '../../core/config/local_config.js';
+import {ChartManager} from '../../core/chart_manager.js';
+
+@injectable()
+export class ClusterCommandHandlers extends CommandHandler {
+  constructor(
+    @inject(ClusterCommandTasks) private readonly tasks: ClusterCommandTasks,
+    @inject(RemoteConfigManager) private readonly remoteConfigManager: RemoteConfigManager,
+    @inject(LocalConfig) private readonly localConfig: LocalConfig,
+    @inject(K8Client) private readonly k8: K8,
+    @inject(ChartManager) private readonly chartManager: ChartManager,
+  ) {
+    super();
+
+    this.tasks = patchInject(tasks, ClusterCommandTasks, this.constructor.name);
+    this.remoteConfigManager = patchInject(remoteConfigManager, RemoteConfigManager, this.constructor.name);
+    this.k8 = patchInject(k8, K8Client, this.constructor.name);
+    this.localConfig = patchInject(localConfig, LocalConfig, this.constructor.name);
+    this.chartManager = patchInject(chartManager, ChartManager, this.constructor.name);
   }
 
   async connect(argv: any) {
     argv = helpers.addFlagsToArgv(argv, ContextFlags.USE_FLAGS);
 
-    const action = this.parent.commandActionBuilder(
+    const action = this.commandActionBuilder(
       [
         this.tasks.initialize(argv, connectConfigBuilder.bind(this)),
-        this.parent.setupHomeDirectoryTask(),
-        this.parent.getLocalConfig().promptLocalConfigTask(this.parent.getK8()),
+        this.setupHomeDirectoryTask(),
+        this.localConfig.promptLocalConfigTask(this.k8),
         this.tasks.selectContext(),
-        ListrRemoteConfig.loadRemoteConfig(this.parent, argv),
+        ListrRemoteConfig.loadRemoteConfig(this.remoteConfigManager, argv),
         this.tasks.readClustersFromRemoteConfig(argv),
         this.tasks.updateLocalConfig(),
       ],
@@ -52,7 +63,7 @@ export class ClusterCommandHandlers implements CommandHandlers {
   async list(argv: any) {
     argv = helpers.addFlagsToArgv(argv, ContextFlags.USE_FLAGS);
 
-    const action = this.parent.commandActionBuilder(
+    const action = this.commandActionBuilder(
       [this.tasks.showClusterList()],
       {
         concurrent: false,
@@ -69,7 +80,7 @@ export class ClusterCommandHandlers implements CommandHandlers {
   async info(argv: any) {
     argv = helpers.addFlagsToArgv(argv, ContextFlags.USE_FLAGS);
 
-    const action = this.parent.commandActionBuilder(
+    const action = this.commandActionBuilder(
       [this.tasks.getClusterInfo()],
       {
         concurrent: false,
@@ -86,7 +97,7 @@ export class ClusterCommandHandlers implements CommandHandlers {
   async setup(argv: any) {
     argv = helpers.addFlagsToArgv(argv, ContextFlags.USE_FLAGS);
 
-    const action = this.parent.commandActionBuilder(
+    const action = this.commandActionBuilder(
       [
         this.tasks.initialize(argv, setupConfigBuilder.bind(this)),
         this.tasks.prepareChartValues(argv),
@@ -112,7 +123,7 @@ export class ClusterCommandHandlers implements CommandHandlers {
   async reset(argv: any) {
     argv = helpers.addFlagsToArgv(argv, ContextFlags.USE_FLAGS);
 
-    const action = this.parent.commandActionBuilder(
+    const action = this.commandActionBuilder(
       [
         this.tasks.initialize(argv, resetConfigBuilder.bind(this)),
         this.tasks.acquireNewLease(argv),

src/commands/cluster/index.ts

@@ -6,9 +6,9 @@ import * as ContextFlags from './flags.js';
 import {YargsCommand} from '../../core/yargs_command.js';
 import {BaseCommand} from './../base.js';
 import {type Opts} from '../../types/command_types.js';
-import {ClusterCommandTasks} from './tasks.js';
 import {ClusterCommandHandlers} from './handlers.js';
 import {DEFAULT_FLAGS, RESET_FLAGS, SETUP_FLAGS} from './flags.js';
+import {patchInject} from '../../core/container_helper.js';
 
 /**
  * Defines the core functionalities of 'node' command
@@ -19,7 +19,7 @@ export class ClusterCommand extends BaseCommand {
   constructor(opts: Opts) {
     super(opts);
 
-    this.handlers = new ClusterCommandHandlers(this, new ClusterCommandTasks(this, this.k8), this.remoteConfigManager);
+    this.handlers = patchInject(null, ClusterCommandHandlers, this.constructor.name);
   }
 
   getCommandDefinition() {