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

ci: Validate load options methods in nodes-base #5862

Merged
merged 8 commits into from
Apr 12, 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
51 changes: 47 additions & 4 deletions packages/core/bin/generate-ui-types
Original file line number Diff line number Diff line change
Expand Up @@ -9,25 +9,68 @@ LoggerProxy.init({
warn: console.warn.bind(console),
});

function findReferencedMethods(obj, refs = {}, latestName = '') {
for (const key in obj) {
if (key === 'name' && 'group' in obj) {
latestName = obj[key];
}

if (typeof obj[key] === 'object') {
findReferencedMethods(obj[key], refs, latestName);
}

if (key === 'loadOptionsMethod') {
refs[latestName] = refs[latestName]
? [...new Set([...refs[latestName], obj[key]])]
: [obj[key]];
}
}

return refs;
}

(async () => {
const loader = new PackageDirectoryLoader(packageDir);
await loader.loadAll();
await loader.loadAll({ withLoadOptionsMethods: true });

const credentialTypes = Object.values(loader.credentialTypes).map((data) => data.type);

const nodeTypes = Object.values(loader.nodeTypes)
const loaderNodeTypes = Object.values(loader.nodeTypes);

const definedMethods = loaderNodeTypes.reduce((acc, cur) => {
NodeHelpers.getVersionedNodeTypeAll(cur.type).forEach((type) => {
const methods = type.description?.__loadOptionsMethods;

if (!methods) return;

const { name } = type.description;

acc[name] = acc[name] ? acc[name].push(methods) : methods;
});

return acc;
}, {});

const nodeTypes = loaderNodeTypes
.map((data) => {
const nodeType = NodeHelpers.getVersionedNodeType(data.type);
NodeHelpers.applySpecialNodeParameters(nodeType);
return data.type;
})
.flatMap((nodeData) => {
const allNodeTypes = NodeHelpers.getVersionedNodeTypeAll(nodeData);
return allNodeTypes.map((element) => element.description);
return NodeHelpers.getVersionedNodeTypeAll(nodeData).map((item) => {
const { __loadOptionsMethods, ...rest } = item.description;

return rest;
});
});

const referencedMethods = findReferencedMethods(nodeTypes);

await Promise.all([
writeJSON('types/credentials.json', credentialTypes),
writeJSON('types/nodes.json', nodeTypes),
writeJSON('methods/defined.json', definedMethods),
writeJSON('methods/referenced.json', referencedMethods),
]);
})();
14 changes: 13 additions & 1 deletion packages/core/src/DirectoryLoader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@ export abstract class DirectoryLoader {

types: Types = { nodes: [], credentials: [] };

withLoadOptionsMethods = false; // only for validation during build

constructor(
readonly directory: string,
protected readonly excludeNodes: string[] = [],
Expand Down Expand Up @@ -103,6 +105,7 @@ export abstract class DirectoryLoader {
const currentVersionNode = tempNode.nodeVersions[tempNode.currentVersion];
this.addCodex({ node: currentVersionNode, filePath, isCustom });
nodeVersion = tempNode.currentVersion;
if (this.withLoadOptionsMethods) this.addLoadOptionsMethods(currentVersionNode);

if (currentVersionNode.hasOwnProperty('executeSingle')) {
Logger.warn(
Expand All @@ -111,6 +114,7 @@ export abstract class DirectoryLoader {
);
}
} else {
if (this.withLoadOptionsMethods) this.addLoadOptionsMethods(tempNode);
// Short renaming to avoid type issues

nodeVersion = Array.isArray(tempNode.description.version)
Expand Down Expand Up @@ -244,6 +248,12 @@ export abstract class DirectoryLoader {
}
}

private addLoadOptionsMethods(node: INodeType) {
if (node?.methods?.loadOptions) {
node.description.__loadOptionsMethods = Object.keys(node.methods.loadOptions);
}
}

private fixIconPath(
obj: INodeTypeDescription | INodeTypeBaseDescription | ICredentialType,
filePath: string,
Expand Down Expand Up @@ -296,7 +306,9 @@ export class PackageDirectoryLoader extends DirectoryLoader {
this.packageName = this.packageJson.name;
}

override async loadAll() {
override async loadAll(options = { withLoadOptionsMethods: false }) {
this.withLoadOptionsMethods = options.withLoadOptionsMethods;

await this.readPackageJson();

const { n8n } = this.packageJson;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -508,9 +508,6 @@ export const contactFields: INodeProperties[] = [
name: 'lead_source_id',
type: 'options',
default: '',
typeOptions: {
loadOptionsMethod: 'getLeadSources',
},
description:
'ID of the source where contact came from. Choose from the list, or specify an ID using an <a href="https://docs.n8n.io/code-examples/expressions/">expression</a>.',
},
Expand Down Expand Up @@ -580,9 +577,6 @@ export const contactFields: INodeProperties[] = [
name: 'subscription_status',
type: 'options',
default: '',
typeOptions: {
loadOptionsMethod: 'getSubscriptionStatuses',
},
description:
'Status of subscription that the contact is in. Choose from the list, or specify an ID using an <a href="https://docs.n8n.io/code-examples/expressions/">expression</a>.',
},
Expand All @@ -591,9 +585,6 @@ export const contactFields: INodeProperties[] = [
name: 'subscription_types',
type: 'options',
default: '',
typeOptions: {
loadOptionsMethod: 'getSubscriptionTypes',
},
description:
'Type of subscription that the contact is in. Choose from the list, or specify an ID using an <a href="https://docs.n8n.io/code-examples/expressions/">expression</a>.',
},
Expand Down
2 changes: 1 addition & 1 deletion packages/nodes-base/nodes/Paddle/PaymentDescription.ts
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ export const paymentFields: INodeProperties[] = [
name: 'paymentId',
type: 'options',
typeOptions: {
loadOptionsMethod: 'getpayment',
loadOptionsMethod: 'getPayments',
},
default: '',
required: true,
Expand Down
2 changes: 1 addition & 1 deletion packages/nodes-base/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
"build:translations": "gulp build:translations",
"build:metadata": "pnpm n8n-generate-known && pnpm n8n-generate-ui-types",
"format": "prettier --write . --ignore-path ../../.prettierignore",
"lint": "eslint --quiet nodes credentials",
"lint": "eslint --quiet nodes credentials; node ./scripts/validate-load-options-methods.js",
"lintfix": "eslint nodes credentials --fix",
"watch": "tsc-watch -p tsconfig.build.json --onSuccess \"pnpm n8n-generate-ui-types\"",
"test": "jest"
Expand Down
43 changes: 43 additions & 0 deletions packages/nodes-base/scripts/validate-load-options-methods.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
let referencedMethods;
let definedMethods;

try {
referencedMethods = require('../dist/methods/referenced.json');
definedMethods = require('../dist/methods/defined.json');
} catch (error) {
console.error(
'Failed to find methods to validate. Please run `npm run n8n-generate-ui-types` first.',
);
process.exit(1);
}

const compareMethods = (base, other) => {
const result = [];

for (const [nodeName, methods] of Object.entries(base)) {
if (nodeName in other) {
const found = methods.filter((item) => !other[nodeName].includes(item));

if (found.length > 0) result.push({ [nodeName]: found });
}
}

return result;
};

const referencedButUndefined = compareMethods(referencedMethods, definedMethods);

if (referencedButUndefined.length > 0) {
console.error('ERROR: The following load options methods are referenced but undefined.');
console.error('Please fix or remove the references or define the methods.');
console.error(referencedButUndefined);
process.exit(1);
}

const definedButUnused = compareMethods(definedMethods, referencedMethods);

if (definedButUnused.length > 0) {
console.warn('Warning: The following load options methods are defined but unused.');
console.warn('Please consider using or removing the methods.');
console.warn(definedButUnused);
}
1 change: 1 addition & 0 deletions packages/workflow/src/Interfaces.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1430,6 +1430,7 @@ export interface INodeTypeDescription extends INodeTypeBaseDescription {
};
};
actions?: INodeActionTypeDescription[];
__loadOptionsMethods?: string[]; // only for validation during build
}

export interface INodeHookDescription {
Expand Down