fix: add proper jsdoc, remove extraneous logic from plugin + tests

- add test cases for getChain + executeChain to validate the processor execution
- remove extraneous function to create a loader and just use the constructor
- address review comments

Author: coopernetes

package-lock.json

@@ -96,6 +96,7 @@
     "mocha": "^10.2.0",
     "nyc": "^17.0.0",
     "prettier": "^3.0.0",
+    "sinon": "^19.0.2",
     "vite": "^4.4.2"
   },
   "optionalDependencies": {

package.json

@@ -8,6 +8,9 @@ const lpModule = import('load-plugin');
  * @return {boolean} - True if the object or any of its prototypes has the 'isGitProxyPlugin' property set to true, false otherwise.
  */
 function isCompatiblePlugin(obj, propertyName = 'isGitProxyPlugin') {
+  // loop through the prototype chain to check if the object is a ProxyPlugin
+  // valid plugin objects will have the appropriate property set to true
+  // if the prototype chain is exhausted, return false
   while (obj != null) {
     if (Object.prototype.hasOwnProperty.call(obj, propertyName) &&
       obj.isGitProxyPlugin &&
@@ -21,46 +24,45 @@ function isCompatiblePlugin(obj, propertyName = 'isGitProxyPlugin') {
 
 /**
  * @typedef PluginTypeResult
- * @property {ProxyPlugin[]} pushPlugins - List of push plugins
- * @property {ProxyPlugin[]} pullPlugins - List of pull plugins
+ * @property {PushActionPlugin[]} pushAction - List of push action plugins
+ * @property {PullActionPlugin[]} pullAction - List of pull action plugins
  */
 
 /**
  * Registers and loads plugins used by git-proxy
  */
 class PluginLoader {
-  /**
-   * @property {Promise} load - A Promise that begins loading plugins from a list of modules. Callers must run `await loader.load` to load plugins.
-   */
-  load;
-  /**
-   * This property is not used in production code. It is exposed for testing purposes.
-   * @property {Promise} ready - A Promise that resolves when all plugins have been loaded.
-   */
-  ready;
-  /**
-   * Initialize PluginLoader with candidates modules (node_modules or relative
-   * file paths).
-   * @param {Array.<string>} targets List of Node module package names or files to load.
-   */
   constructor(targets) {
+    /**
+     * List of Node module specifiers to load as plugins. It can be a relative path, an 
+     * absolute path, or a module name (which can include scoped packages like '@bar/baz').
+     * @type {string[]}
+     * @public
+     */
     this.targets = targets;
     /**
-     * @type {ProxyPlugin[]} List of loaded ProxyPlugins
+     * List of loaded PushActionPlugin objects.
+     * @type {PushActionPlugin[]}
      * @public
      */
     this.pushPlugins = [];
+    /**
+     * List of loaded PullActionPlugin objects.
+     * @type {PullActionPlugin[]}
+     * @public
+     */
     this.pullPlugins = [];
     if (this.targets.length === 0) {
       console.log('No plugins configured'); // TODO: log.debug()
-      this.ready = Promise.resolve();
-      this.load = () => Promise.resolve(); // Ensure this.load is always defined
-      return;
     }
-    this.load = this._loadPlugins();
   }
 
-  async _loadPlugins() {
+  /**
+   * Load all plugins specified in the `targets` property. This method must complete before a PluginLoader instance
+   * can be used to retrieve plugins.
+   * @return {Promise<void>} A Promise that resolves when all plugins have been loaded.
+   */
+  async load() {
     try {
       const modulePromises = this.targets.map(target =>
         this._loadPluginModule(target).catch(error => {
@@ -84,30 +86,31 @@ class PluginLoader {
       );
 
       const settledPluginTypeResults = await Promise.allSettled(pluginTypeResultPromises);
+      /**
+       * @type {PluginTypeResult[]} List of resolved PluginTypeResult objects
+       */
       const pluginTypeResults = settledPluginTypeResults
         .filter(result => result.status === 'fulfilled' && result.value !== null)
         .map(result => result.value);
 
       for (const result of pluginTypeResults) {
-        this.pushPlugins.push(...result.pushPlugins)
-        this.pullPlugins.push(...result.pullPlugins)
+        this.pushPlugins.push(...result.pushAction)
+        this.pullPlugins.push(...result.pullAction)
       }
 
       const combinedPlugins = [...this.pushPlugins, ...this.pullPlugins];
       combinedPlugins.forEach(plugin => {
         console.log(`Loaded plugin: ${plugin.constructor.name}`);
       });
-
-      this.ready = Promise.resolve();
     } catch (error) {
       console.error(`Error loading plugins: ${error}`);
-      this.ready = Promise.reject(error);
     }
   }
+
   /**
-   * Load a plugin module from either a file path or a Node module.
-   * @param {string} target
-   * @return {Module}
+   * Resolve & load a Node module from either a given specifier (file path, import specifier or package name) using load-plugin.
+   * @param {string} target The module specifier to load
+   * @return {Promise<Module>} A resolved & loaded Module
    */
   async _loadPluginModule(target) {
     const lp = await lpModule;
@@ -116,40 +119,39 @@ class PluginLoader {
   }
 
   /**
-   * Set a list of ProxyPlugin objects to this.plugins
-   * from the keys exported by the passed in module.
-   * @param {object} pluginModule
-   * @return {PluginTypeResult} - An object containing the loaded plugins classified by their type.
+   * Checks for known compatible plugin objects in a Module and returns them classified by their type.
+   * @param {Module} pluginModule The module to extract plugins from
+   * @return {Promise<PluginTypeResult>} An object containing the loaded plugins classified by their type.
    */
   async _getPluginObjects(pluginModule) {
     const plugins = {
-      pushPlugins: [],
-      pullPlugins: [],
+      pushAction: [],
+      pullAction: [],
     };
-    // handles the case where the `module.exports = new ProxyPlugin()` or `exports default new ProxyPlugin()`
-    if (isCompatiblePlugin(pluginModule)) {
-      if (isCompatiblePlugin(pluginModule, 'isGitProxyPushActionPlugin')) {
-        console.log('found push plugin', pluginModule.constructor.name);
-        plugins.pushPlugins.push(pluginModule);
-      } else if (isCompatiblePlugin(pluginModule, 'isGitProxyPullActionPlugin')) {
-        console.log('found pull plugin', pluginModule.constructor.name);
-        plugins.pullPlugins.push(pluginModule);
+
+    function handlePlugin(potentialModule) {
+      if (isCompatiblePlugin(potentialModule, 'isGitProxyPushActionPlugin')) {
+        console.log('found push plugin', potentialModule.constructor.name);
+        plugins.pushAction.push(potentialModule);
+      } else if (isCompatiblePlugin(potentialModule, 'isGitProxyPullActionPlugin')) {
+        console.log('found pull plugin', potentialModule.constructor.name);
+        plugins.pullAction.push(potentialModule);
       } else {
-        console.error(`Error: Object ${pluginModule.constructor.name} does not seem to be a compatible plugin type`);
+        console.error(`Error: Object ${potentialModule.constructor.name} does not seem to be a compatible plugin type`);
       }
+    }
+
+    // handles the default export case
+    // `module.exports = new ProxyPlugin()` in CJS or `exports default new ProxyPlugin()` in ESM
+    // the "module" is a single object that could be a plugin
+    if (isCompatiblePlugin(pluginModule)) {
+      handlePlugin(pluginModule)
     } else {
-      // iterate over the module.exports keys if multiple arbitrary objects are exported
+      // handle the typical case of a module which exports multiple objects
+      // module.exports = { x, y } (CJS) or multiple `export ...` statements (ESM)
       for (const key of Object.keys(pluginModule)) {
         if (isCompatiblePlugin(pluginModule[key])) {
-          if (isCompatiblePlugin(pluginModule[key], 'isGitProxyPushActionPlugin')) {
-            console.log('found push plugin', pluginModule[key].constructor.name);
-            plugins.pushPlugins.push(pluginModule[key]);
-          } else if (isCompatiblePlugin(pluginModule[key], 'isGitProxyPullActionPlugin')) {
-            console.log('found pull plugin', pluginModule[key].constructor.name);
-            plugins.pullPlugins.push(pluginModule[key]);
-          } else {
-            console.error(`Error: Object ${pluginModule.constructor.name} does not seem to be a compatible plugin type`);
-          }
+          handlePlugin(pluginModule[key]);
         }
       }
     }
@@ -217,21 +219,9 @@ class PullActionPlugin extends ProxyPlugin {
   }
 }
 
-/**
- * 
- * @param {Array<string>} targets A list of loadable targets for plugin modules. 
- * @return {PluginLoader}
- */
-const createLoader = async (targets) => {
-  const loadTargets = targets;
-  const loader = new PluginLoader(loadTargets);
-  return loader;
-};
-
 module.exports = {
-  createLoader,
   PluginLoader,
   PushActionPlugin,
   PullActionPlugin,
   isCompatiblePlugin,
-}
+}