Skip to content

Commit

Permalink
fix: add proper jsdoc, remove extraneous logic from plugin + tests
Browse files Browse the repository at this point in the history
- 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
  • Loading branch information
coopernetes committed Oct 21, 2024
1 parent f9667d6 commit a7b799c
Show file tree
Hide file tree
Showing 9 changed files with 498 additions and 129 deletions.
140 changes: 140 additions & 0 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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": {
Expand Down
126 changes: 58 additions & 68 deletions src/plugin.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 &&
Expand All @@ -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 => {
Expand All @@ -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}`);

Check warning on line 106 in src/plugin.js

View check run for this annotation

Codecov / codecov/patch

src/plugin.js#L106

Added line #L106 was not covered by tests
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;
Expand All @@ -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`);

Check warning on line 140 in src/plugin.js

View check run for this annotation

Codecov / codecov/patch

src/plugin.js#L140

Added line #L140 was not covered by tests
}
}

// 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]);
}
}
}
Expand Down Expand Up @@ -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,
}
}
Loading

0 comments on commit a7b799c

Please sign in to comment.