Skip to content

Commit

Permalink
refactor(plugin): use recursively dfs to stable sort result & add tes…
Browse files Browse the repository at this point in the history
…t case
  • Loading branch information
noahziheng committed Aug 28, 2023
1 parent a06f8e5 commit 993f8f0
Show file tree
Hide file tree
Showing 3 changed files with 61 additions and 165 deletions.
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@artus/core",
"version": "2.0.5",
"version": "2.1.0",
"description": "Core package of Artus",
"main": "./lib/index.js",
"types": "./lib/index.d.ts",
Expand Down
85 changes: 46 additions & 39 deletions src/plugin/common.ts
Original file line number Diff line number Diff line change
@@ -1,79 +1,86 @@
import path from 'path';
import compatibleRequire from '../utils/compatible_require';
import { PluginType } from './types';
import { LoggerType } from '../logger';
import path from "path";
import compatibleRequire from "../utils/compatible_require";
import { PluginType } from "./types";
import { LoggerType } from "../logger";

export function sortPlugins(pluginInstanceMap: Map<string, PluginType>, logger: LoggerType) {
export function sortPlugins(
pluginInstanceMap: Map<string, PluginType>,
logger: LoggerType,
): PluginType[] {
const sortedPlugins: PluginType[] = [];
const visited: Record<string, boolean> = {};

while (sortedPlugins.length < pluginInstanceMap.size) {
let added = false;
const visit = (pluginName: string, depChain: string[] = []) => {
if (depChain.includes(pluginName)) {
throw new Error(
`Circular dependency found in plugins: ${depChain.join(", ")}`,
);
}

for (const [pluginName, plugin] of pluginInstanceMap) {
if (visited[pluginName]) {
continue;
}
if (visited[pluginName]) return;

visited[pluginName] = true;

let depsSatisfied = true;
const plugin = pluginInstanceMap.get(pluginName);
if (plugin) {
for (const dep of plugin.metadata.dependencies ?? []) {
const depPlugin = pluginInstanceMap.get(dep.name);
if (!depPlugin || !depPlugin.enable) {
if (dep.optional) {
logger?.warn(`Plugin ${plugin.name} need have optional dependency: ${dep.name}.`);
logger?.warn(
`Plugin ${plugin.name} need have optional dependency: ${dep.name}.`,
);
} else {
throw new Error(`Plugin ${plugin.name} need have dependency: ${dep.name}.`);
throw new Error(
`Plugin ${plugin.name} need have dependency: ${dep.name}.`,
);
}
} else if (!visited[dep.name]) { // Plugin exist and enabled, need check visited
depsSatisfied = false;
} else {
// Plugin exist and enabled, need visit
visit(dep.name, depChain.concat(pluginName));
}
}

if (depsSatisfied) {
sortedPlugins.push(plugin);
visited[plugin.name] = true;
added = true;
}
sortedPlugins.push(plugin);
}
};

if (!added) {
const cyclePluginNames: string[] = [];
const sortedPluginSet = new Set(sortedPlugins.map(p => p.name));
for (const pluginName of pluginInstanceMap.keys()) {
if (!sortedPluginSet.has(pluginName)) {
cyclePluginNames.push(pluginName);
}
}
throw new Error(`Circular dependency found in plugins: ${cyclePluginNames.join(', ')}`);
}
for (const pluginName of pluginInstanceMap.keys()) {
visit(pluginName);
}

return sortedPlugins;
}

// A util function of get package path for plugin
export function getPackagePath(packageName: string, paths: string[] = []): string {
export function getPackagePath(
packageName: string,
paths: string[] = [],
): string {
const opts = {
paths: paths.concat(__dirname),
};
return path.dirname(require.resolve(packageName, opts));
}

export async function getInlinePackageEntryPath(packagePath: string): Promise<string> {
export async function getInlinePackageEntryPath(
packagePath: string,
): Promise<string> {
const pkgJson = await compatibleRequire(`${packagePath}/package.json`);
let entryFilePath = '';
let entryFilePath = "";
if (pkgJson.exports) {
if (Array.isArray(pkgJson.exports)) {
throw new Error(`inline package multi exports is not supported`);
} else if (typeof pkgJson.exports === 'string') {
} else if (typeof pkgJson.exports === "string") {
entryFilePath = pkgJson.exports;
} else if (pkgJson.exports?.['.']) {
entryFilePath = pkgJson.exports['.'];
} else if (pkgJson.exports?.["."]) {
entryFilePath = pkgJson.exports["."];
}
}
if (!entryFilePath && pkgJson.main) {
entryFilePath = pkgJson.main;
}
// will use package root path if no entry file found
return entryFilePath ? path.resolve(packagePath, entryFilePath, '..') : packagePath;
return entryFilePath
? path.resolve(packagePath, entryFilePath, "..")
: packagePath;
}
139 changes: 14 additions & 125 deletions test/plugin.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,105 +11,57 @@ describe('test/plugin.test.ts', () => {
'plugin-a': {
enable: true,
path: path.resolve(__dirname, `${pluginPrefix}/plugin_a`),
manifest: {
pluginMeta: {
path: path.resolve(__dirname, `${pluginPrefix}/plugin_a/meta.js`),
extname: '.js',
filename: 'meta.js',
},
},
},
'plugin-b': {
enable: true,
path: path.resolve(__dirname, `${pluginPrefix}/plugin_b`),
manifest: {
pluginMeta: {
path: path.resolve(__dirname, `${pluginPrefix}/plugin_b/meta.js`),
extname: '.js',
filename: 'meta.js',
},
},
},
'plugin-c': {
enable: true,
path: path.resolve(__dirname, `${pluginPrefix}/plugin_c`),
manifest: {
pluginMeta: {
path: path.resolve(__dirname, `${pluginPrefix}/plugin_c/meta.js`),
extname: '.js',
filename: 'meta.js',
},
},
},
'plugin-d': {
enable: true,
path: path.resolve(__dirname, `${pluginPrefix}/plugin_d`),
},
};
const pluginList = await PluginFactory.createFromConfig(mockPluginConfig);
expect(pluginList.length).toEqual(3);
expect(pluginList.length).toEqual(4);
pluginList.forEach(plugin => {
expect(plugin).toBeInstanceOf(Plugin);
expect(plugin.enable).toBeTruthy();
});
expect(pluginList.map(plugin => plugin.name)).toStrictEqual(['plugin-c', 'plugin-b', 'plugin-a']);
expect(pluginList.map(plugin => plugin.name)).toStrictEqual(['plugin-c', 'plugin-b', 'plugin-a', 'plugin-d']);
});

it('should not load plugin with wrong order', async () => {
const mockPluginConfig = {
'plugin-a': {
enable: true,
path: path.resolve(__dirname, `${pluginPrefix}/plugin_a`),
manifest: {
pluginMeta: {
path: path.resolve(__dirname, `${pluginPrefix}/plugin_a/meta.js`),
extname: '.js',
filename: 'meta.js',
},
},
},
'plugin-b': {
enable: true,
path: path.resolve(__dirname, `${pluginPrefix}/plugin_b`),
manifest: {
pluginMeta: {
path: path.resolve(__dirname, `${pluginPrefix}/plugin_b/meta.js`),
extname: '.js',
filename: 'meta.js',
},
},
},
'plugin-c': {
enable: true,
path: path.resolve(__dirname, `${pluginPrefix}/plugin_c`),
manifest: {
pluginMeta: {
path: path.resolve(__dirname, `${pluginPrefix}/plugin_c/meta.js`),
extname: '.js',
filename: 'meta.js',
},
},
},
'plugin-d': {
enable: true,
path: path.resolve(__dirname, `${pluginPrefix}/plugin_d`),
},
'plugin-wrong-a': {
enable: true,
path: path.resolve(__dirname, `${pluginPrefix}/plugin_wrong_a`),
manifest: {
pluginMeta: {
path: path.resolve(__dirname, `${pluginPrefix}/plugin_wrong_a/meta.js`),
extname: '.js',
filename: 'meta.js',
},
},
},
'plugin-wrong-b': {
enable: true,
path: path.resolve(__dirname, `${pluginPrefix}/plugin_wrong_b`),
manifest: {
pluginMeta: {
path: path.resolve(__dirname, `${pluginPrefix}/plugin_wrong_b/meta.js`),
extname: '.js',
filename: 'meta.js',
},
},
},
};
expect(async () => {
await expect(async () => {
await PluginFactory.createFromConfig(mockPluginConfig);
}).rejects.toThrowError(new Error(`Circular dependency found in plugins: plugin-wrong-a, plugin-wrong-b`));
});
Expand All @@ -119,13 +71,6 @@ describe('test/plugin.test.ts', () => {
'plugin-a': {
enable: true,
path: path.resolve(__dirname, `${pluginPrefix}/plugin_a`),
manifest: {
pluginMeta: {
path: path.resolve(__dirname, `${pluginPrefix}/plugin_a/meta.js`),
extname: '.js',
filename: 'meta.js',
},
},
},
};
expect(async () => {
Expand All @@ -138,38 +83,17 @@ describe('test/plugin.test.ts', () => {
'plugin-a': {
enable: true,
path: path.resolve(__dirname, `${pluginPrefix}/plugin_a`),
manifest: {
pluginMeta: {
path: path.resolve(__dirname, `${pluginPrefix}/plugin_a/meta.js`),
extname: '.js',
filename: 'meta.js',
},
},
},
'plugin-b': {
enable: false,
path: path.resolve(__dirname, `${pluginPrefix}/plugin_b`),
manifest: {
pluginMeta: {
path: path.resolve(__dirname, `${pluginPrefix}/plugin_b/meta.js`),
extname: '.js',
filename: 'meta.js',
},
},
},
'plugin-c': {
enable: true,
path: path.resolve(__dirname, `${pluginPrefix}/plugin_c`),
manifest: {
pluginMeta: {
path: path.resolve(__dirname, `${pluginPrefix}/plugin_c/meta.js`),
extname: '.js',
filename: 'meta.js',
},
},
},
};
expect(async () => {
await expect(async () => {
await PluginFactory.createFromConfig(mockPluginConfig);
}).rejects.toThrowError(new Error(`Plugin plugin-a need have dependency: plugin-b.`));
});
Expand All @@ -179,13 +103,6 @@ describe('test/plugin.test.ts', () => {
'plugin-d': {
enable: true,
path: path.resolve(__dirname, `${pluginPrefix}/plugin_d`),
manifest: {
pluginMeta: {
path: path.resolve(__dirname, `${pluginPrefix}/plugin_d/meta.js`),
extname: '.js',
filename: 'meta.js',
},
},
},
};

Expand All @@ -209,27 +126,13 @@ describe('test/plugin.test.ts', () => {

it('should not throw if optional dependence disabled', async () => {
const mockPluginConfig = {
'plugin-c': {
'plugin-b': {
enable: false,
path: path.resolve(__dirname, `${pluginPrefix}/plugin_c`),
manifest: {
pluginMeta: {
path: path.resolve(__dirname, `${pluginPrefix}/plugin_c/meta.js`),
extname: '.js',
filename: 'meta.js',
},
},
path: path.resolve(__dirname, `${pluginPrefix}/plugin_b`),
},
'plugin-d': {
enable: true,
path: path.resolve(__dirname, `${pluginPrefix}/plugin_d`),
manifest: {
pluginMeta: {
path: path.resolve(__dirname, `${pluginPrefix}/plugin_d/meta.js`),
extname: '.js',
filename: 'meta.js',
},
},
},
};

Expand All @@ -256,24 +159,10 @@ describe('test/plugin.test.ts', () => {
'plugin-d': {
enable: true,
path: path.resolve(__dirname, `${pluginPrefix}/plugin_d`),
manifest: {
pluginMeta: {
path: path.resolve(__dirname, `${pluginPrefix}/plugin_d/meta.js`),
extname: '.js',
filename: 'meta.js',
},
},
},
'plugin-c': {
enable: true,
path: path.resolve(__dirname, `${pluginPrefix}/plugin_c`),
manifest: {
pluginMeta: {
path: path.resolve(__dirname, `${pluginPrefix}/plugin_c/meta.js`),
extname: '.js',
filename: 'meta.js',
},
},
},
};

Expand Down

0 comments on commit 993f8f0

Please sign in to comment.