Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(loader): format plugin config in factory && support inline package #229

Merged
merged 3 commits into from
Feb 21, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion src/constant.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,6 @@ export const EXCEPTION_FILENAME = 'exception.json';
export const DEFAULT_LOADER_LIST_WITH_ORDER = [
'exception',
'exception-filter',
'plugin-config',
'plugin-meta',
'framework-config',
'package-json',
Expand Down
10 changes: 7 additions & 3 deletions src/loader/impl/config.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,14 @@
import * as path from 'path';
import { Container } from '@artus/injection';
import ConfigurationHandler from '../../configuration';
import { ArtusInjectEnum, PLUGIN_CONFIG_PATTERN, FRAMEWORK_PATTERN } from '../../constant';
import { ArtusInjectEnum, FRAMEWORK_PATTERN } from '../../constant';
import { DefineLoader } from '../decorator';
import { ManifestItem, Loader, LoaderFindOptions } from '../types';
import compatibleRequire from '../../utils/compatible_require';
import { isMatch } from '../../utils';
import { Application } from '../../types';
import { getConfigMetaFromFilename } from '../utils/config_file_meta';
import { PluginFactory } from '../../plugin';

@DefineLoader('config')
class ConfigLoader implements Loader {
Expand All @@ -28,7 +29,6 @@ class ConfigLoader implements Loader {
static async is(opts: LoaderFindOptions): Promise<boolean> {
return (
this.isConfigDir(opts) &&
!isMatch(opts.filename, PLUGIN_CONFIG_PATTERN) &&
!isMatch(opts.filename, FRAMEWORK_PATTERN)
);
}
Expand All @@ -41,7 +41,11 @@ class ConfigLoader implements Loader {
async load(item: ManifestItem) {
const { namespace, env } = getConfigMetaFromFilename(item.filename);
let configObj = await this.loadConfigFile(item);
if (namespace) {
if (namespace === 'plugin') {
configObj = {
plugin: await PluginFactory.formatPluginConfig(configObj, item),
};
} else if (namespace) {
configObj = {
[namespace]: configObj,
};
Expand Down
2 changes: 0 additions & 2 deletions src/loader/impl/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import ExceptionLoader from './exception';
import ExceptionFilterLoader from './exception_filter';
import LifecycleLoader from './lifecycle';
import PluginMetaLoader from './plugin_meta';
import PluginConfigLoader from './plugin_config';
import FrameworkConfigLoader from './framework_config';
import PackageLoader from './package';

Expand All @@ -15,7 +14,6 @@ export {
ExceptionFilterLoader,
LifecycleLoader,
PluginMetaLoader,
PluginConfigLoader,
FrameworkConfigLoader,
PackageLoader,
};
44 changes: 0 additions & 44 deletions src/loader/impl/plugin_config.ts

This file was deleted.

20 changes: 20 additions & 0 deletions src/plugin/common.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import path from 'path';
import compatibleRequire from '../utils/compatible_require';
import { PluginType } from './types';

// A utils function that toplogical sort plugins
Expand Down Expand Up @@ -38,3 +39,22 @@ export function getPackagePath(packageName: string, paths?: string[]): string {
const opts = paths ? { paths } : undefined;
return path.resolve(require.resolve(packageName, opts), '..');
}

export async function getInlinePackageEntryPath(packagePath: string): Promise<string> {
const pkgJson = await compatibleRequire(`${packagePath}/package.json`);
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') {
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;
}
28 changes: 27 additions & 1 deletion src/plugin/factory.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
import { topologicalSort } from './common';
import path from 'path';
import { ManifestItem } from '../loader';
import { getInlinePackageEntryPath, getPackagePath, topologicalSort } from './common';
import { Plugin } from './impl';
import { PluginConfigItem, PluginCreateOptions, PluginMap, PluginType } from './types';
import { exists } from '../utils/fs';

export class PluginFactory {
static async create(name: string, item: PluginConfigItem, opts?: PluginCreateOptions): Promise<PluginType> {
Expand Down Expand Up @@ -30,4 +33,27 @@ export class PluginFactory {
}
return pluginSortResult.map(name => pluginInstanceMap.get(name)!);
}

static async formatPluginConfig(config: Record<string, PluginConfigItem>, manifestItem?: ManifestItem): Promise<Record<string, PluginConfigItem>> {
const newConfig: Record<string, PluginConfigItem> = {};
const loaderState = manifestItem?.loaderState as { baseDir: string };
for (const pluginName of Object.keys(config)) {
const pluginConfigItem: PluginConfigItem = config[pluginName];
if (pluginConfigItem.package) {
// convert package to path when load plugin config
if (pluginConfigItem.path) {
throw new Error(
`Plugin ${pluginName} config can't have both package and path at ${manifestItem?.path ?? 'UNKNOWN_PATH'}`,
);
}
pluginConfigItem.path = getPackagePath(pluginConfigItem.package, [loaderState?.baseDir]);
delete pluginConfigItem.package;
} else if (pluginConfigItem.path && await exists(path.resolve(pluginConfigItem.path, 'package.json'))) {
// plugin path is a npm package, need resolve main file
pluginConfigItem.path = await getInlinePackageEntryPath(pluginConfigItem.path);
}
newConfig[pluginName] = pluginConfigItem;
}
return newConfig;
}
}
5 changes: 0 additions & 5 deletions src/scanner/scan.ts
Original file line number Diff line number Diff line change
Expand Up @@ -280,11 +280,6 @@ export class Scanner {
}
relative && items.forEach(item => (item.path = path.relative(appRoot, item.path)));
return items.filter(item => {
// remove PluginConfig to avoid re-merge on application running
if (item.loader === 'plugin-config') {
return false;
}

// remove other env config
if (item.loader === 'config' || item.loader === 'framework-config') {
const { env: filenameEnv } = getConfigMetaFromFilename(item.filename);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ export default class Hello {
async index(ctx: Context) {
const { params: { config } } = ctx.input;
return {
message: `get conifg succeed`,
message: `get config succeed`,
config,
};
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ export default class Hello {
async index(ctx: Context) {
const { params: { config } } = ctx.input;
return {
message: `get conifg succeed`,
message: `get config succeed`,
config,
};
}
Expand Down
Empty file.
3 changes: 3 additions & 0 deletions test/fixtures/plugins/plugin_with_entry_a/mock_lib/meta.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"name": "plugin-with-entry-a"
}
4 changes: 4 additions & 0 deletions test/fixtures/plugins/plugin_with_entry_a/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{
"name": "@artus/test-plugin-with-entry-a",
"main": "mock_lib/index.js"
}
Empty file.
3 changes: 3 additions & 0 deletions test/fixtures/plugins/plugin_with_entry_b/mock_lib/meta.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"name": "plugin-with-entry-b"
}
5 changes: 5 additions & 0 deletions test/fixtures/plugins/plugin_with_entry_b/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"name": "@artus/test-plugin-with-entry-b",
"exports": "./mock_lib/index.js",
"main": "fake/index.js"
}
Empty file.
3 changes: 3 additions & 0 deletions test/fixtures/plugins/plugin_with_entry_c/mock_lib/meta.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"name": "plugin-with-entry-c"
}
7 changes: 7 additions & 0 deletions test/fixtures/plugins/plugin_with_entry_c/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"name": "@artus/test-plugin-with-entry-c",
"exports": {
".": "./mock_lib/index.js"
},
"main": "fake/index.js"
}
Empty file.
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"name": "plugin-with-entry-wrong"
}
7 changes: 7 additions & 0 deletions test/fixtures/plugins/plugin_with_entry_wrong/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"name": "@artus/test-plugin-with-entry-wrong",
"exports": [
"./mock_lib/index.js",
"./mock_lib/index.json"
]
}
2 changes: 1 addition & 1 deletion test/framework.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ describe('test/framework.test.ts', () => {
// check config loaded succeed
const testResponseConfig = await axios.get(`http://127.0.0.1:${port}/config`);
assert(testResponseConfig.status === 200);
assert(testResponseConfig.data.message === 'get conifg succeed');
assert(testResponseConfig.data.message === 'get config succeed');

// check frameworke used as env
const testResponseName2 = await axios.get(`http://127.0.0.1:${port}/get_name2`);
Expand Down
35 changes: 35 additions & 0 deletions test/plugin.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -207,4 +207,39 @@ describe('test/app.test.ts', () => {
console.warn = originWarn;
});
});

describe('format plugin config', () => {
it('should format plugin with entry in package.json', async () => {
const mockPluginConfig = {
'plugin-with-entry-a': {
enable: true,
path: path.resolve(__dirname, `${pluginPrefix}/plugin_with_entry_a`),
},
'plugin-with-entry-b': {
enable: true,
path: path.resolve(__dirname, `${pluginPrefix}/plugin_with_entry_b`),
},
'plugin-with-entry-c': {
enable: true,
path: path.resolve(__dirname, `${pluginPrefix}/plugin_with_entry_c`),
},
};
const formattedPluginConfigList = await PluginFactory.formatPluginConfig(mockPluginConfig);
const pluginList = await PluginFactory.createFromConfig(formattedPluginConfigList);
expect(pluginList.length).toBe(3);
for (const pluginItem of pluginList) {
expect(pluginItem.importPath).toBe(path.resolve(__dirname, pluginPrefix, pluginItem.name.replaceAll('-', '_') , 'mock_lib'));
}
});

it('should throw error if multi exports entry in package.json', async () => {
const mockPluginConfig = {
'plugin-with-entry-wrong': {
enable: true,
path: path.resolve(__dirname, `${pluginPrefix}/plugin_with_entry_wrong`),
},
};
expect(() => PluginFactory.formatPluginConfig(mockPluginConfig)).rejects.toThrowError('inline package multi exports is not supported');
});
});
});
12 changes: 5 additions & 7 deletions test/scanner.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,21 +13,19 @@ describe('test/scanner.test.ts', () => {
expect(manifest).toBeDefined();
expect(manifest.items).toBeDefined();
// console.log('manifest', manifest);
expect(manifest.items.length).toBe(11);
expect(manifest.items.length).toBe(12);

expect(manifest.items.find(item => item.filename === 'not_to_be_scanned_file.ts')).toBeFalsy();

expect(manifest.items.filter(item => item.loader === 'plugin-config').length).toBe(0);
expect(manifest.items.filter(item => item.loader === 'plugin-meta').length).toBe(1);
expect(manifest.items.filter(item => item.loader === 'exception').length).toBe(1);
expect(manifest.items.filter(item => item.loader === 'exception-filter').length).toBe(1);
expect(manifest.items.filter(item => item.loader === 'lifecycle-hook-unit').length).toBe(2);
expect(manifest.items.filter(item => item.loader === 'config').length).toBe(1);
expect(manifest.items.filter(item => item.loader === 'config').length).toBe(2);
expect(manifest.items.filter(item => item.loader === 'module').length).toBe(4);

expect(manifest.items.filter(item => item.unitName === 'redis').length).toBe(2);
expect(manifest.items.filter(item => item.unitName === 'mysql').length).toBe(0);
expect(manifest.items.filter(item => item.source === 'app').length).toBe(9);
expect(manifest.items.filter(item => item.source === 'app').length).toBe(10);
expect(manifest.pluginConfig).toStrictEqual({
redis: { enable: true, path: path.join('src', 'redis_plugin') },
mysql: { enable: false, path: path.join('src', 'mysql_plugin') },
Expand All @@ -38,8 +36,8 @@ describe('test/scanner.test.ts', () => {
// console.log('devManifest', devManifest);
expect(devManifest).toBeDefined();
expect(devManifest.items).toBeDefined();
expect(devManifest.items.length).toBe(13);
expect(devManifest.items.filter(item => item.loader === 'config').length).toBe(2);
expect(devManifest.items.length).toBe(15);
expect(devManifest.items.filter(item => item.loader === 'config').length).toBe(4);
expect(devManifest.items.filter(item => item.loader === 'plugin-meta').length).toBe(2);
expect(devManifest.items.find(item => item.unitName === 'testDuplicate')).toBeDefined();
expect(devManifest.pluginConfig).toStrictEqual({
Expand Down
4 changes: 2 additions & 2 deletions test/specific.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ describe('test/specific.test.ts', () => {
// check config loaded succeed
const testResponseConfig = await axios.get(`http://127.0.0.1:${port}/config`);
assert(testResponseConfig.status === 200);
assert(testResponseConfig.data.message === 'get conifg succeed');
assert(testResponseConfig.data.message === 'get config succeed');

// check frameworke used as env
const testResponseName2 = await axios.get(`http://127.0.0.1:${port}/get_name2`);
Expand Down Expand Up @@ -111,7 +111,7 @@ describe('test/specific.test.ts', () => {
// check config loaded succeed
const testResponseConfig = await axios.get(`http://127.0.0.1:${port}/config`);
assert(testResponseConfig.status === 200);
assert(testResponseConfig.data.message === 'get conifg succeed');
assert(testResponseConfig.data.message === 'get config succeed');

// check frameworke used as env
const testResponseName2 = await axios.get(`http://127.0.0.1:${port}/get_name2`);
Expand Down