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(core): Refactor nodes loading (no-changelog) #7283

Merged
merged 14 commits into from
Oct 9, 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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 0 additions & 1 deletion .dockerignore
Original file line number Diff line number Diff line change
Expand Up @@ -11,4 +11,3 @@ packages/**/.turbo
.github
*.tsbuildinfo
packages/cli/dist/**/e2e.*
packages/cli/dist/ReloadNodesAndCredentials.*
3 changes: 1 addition & 2 deletions packages/cli/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,7 @@
"templates",
"dist",
"oclif.manifest.json",
"!dist/**/e2e.*",
"!dist/ReloadNodesAndCredentials.*"
"!dist/**/e2e.*"
netroy marked this conversation as resolved.
Show resolved Hide resolved
],
"devDependencies": {
"@apidevtools/swagger-cli": "4.0.0",
Expand Down
28 changes: 9 additions & 19 deletions packages/cli/src/CredentialTypes.ts
Original file line number Diff line number Diff line change
@@ -1,32 +1,31 @@
import { Service } from 'typedi';
import { loadClassInIsolation } from 'n8n-core';
import type { ICredentialType, ICredentialTypes, LoadedClass } from 'n8n-workflow';
import { Service } from 'typedi';
import { RESPONSE_ERROR_MESSAGES } from './constants';
import { LoadNodesAndCredentials } from './LoadNodesAndCredentials';
import { RESPONSE_ERROR_MESSAGES } from '@/constants';
import { LoadNodesAndCredentials } from '@/LoadNodesAndCredentials';

@Service()
export class CredentialTypes implements ICredentialTypes {
constructor(private nodesAndCredentials: LoadNodesAndCredentials) {
nodesAndCredentials.credentialTypes = this;
}
constructor(private loadNodesAndCredentials: LoadNodesAndCredentials) {}

recognizes(type: string) {
return type in this.knownCredentials || type in this.loadedCredentials;
const { loadedCredentials, knownCredentials } = this.loadNodesAndCredentials;
return type in knownCredentials || type in loadedCredentials;
}

getByName(credentialType: string): ICredentialType {
return this.getCredential(credentialType).type;
}

getNodeTypesToTestWith(type: string): string[] {
return this.knownCredentials[type]?.nodesToTestWith ?? [];
return this.loadNodesAndCredentials.knownCredentials[type]?.nodesToTestWith ?? [];
}

/**
* Returns all parent types of the given credential type
*/
getParentTypes(typeName: string): string[] {
const extendsArr = this.knownCredentials[typeName]?.extends ?? [];
const extendsArr = this.loadNodesAndCredentials.knownCredentials[typeName]?.extends ?? [];
if (extendsArr.length) {
extendsArr.forEach((type) => {
extendsArr.push(...this.getParentTypes(type));
Expand All @@ -36,12 +35,11 @@ export class CredentialTypes implements ICredentialTypes {
}

private getCredential(type: string): LoadedClass<ICredentialType> {
const loadedCredentials = this.loadedCredentials;
const { loadedCredentials, knownCredentials } = this.loadNodesAndCredentials;
if (type in loadedCredentials) {
return loadedCredentials[type];
}

const knownCredentials = this.knownCredentials;
if (type in knownCredentials) {
const { className, sourcePath } = knownCredentials[type];
const loaded: ICredentialType = loadClassInIsolation(sourcePath, className);
Expand All @@ -50,12 +48,4 @@ export class CredentialTypes implements ICredentialTypes {
}
throw new Error(`${RESPONSE_ERROR_MESSAGES.NO_CREDENTIAL}: ${type}`);
}

private get loadedCredentials() {
return this.nodesAndCredentials.loaded.credentials;
}

private get knownCredentials() {
return this.nodesAndCredentials.known.credentials;
}
}
18 changes: 9 additions & 9 deletions packages/cli/src/CredentialsHelper.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
/* eslint-disable @typescript-eslint/no-unsafe-argument */

/* eslint-disable @typescript-eslint/no-unsafe-member-access */
/* eslint-disable @typescript-eslint/no-unsafe-assignment */
/* eslint-disable @typescript-eslint/no-unsafe-return */
Expand Down Expand Up @@ -89,13 +88,11 @@ const mockNodeTypes: INodeTypes = {
};

export class CredentialsHelper extends ICredentialsHelper {
constructor(
encryptionKey: string,
private credentialTypes = Container.get(CredentialTypes),
private nodeTypes = Container.get(NodeTypes),
) {
super(encryptionKey);
}
private credentialTypes = Container.get(CredentialTypes);

private nodeTypes = Container.get(NodeTypes);

private credentialsOverwrites = Container.get(CredentialsOverwrites);

/**
* Add the required authentication information to the request
Expand Down Expand Up @@ -388,7 +385,10 @@ export class CredentialsHelper extends ICredentialsHelper {
const credentialsProperties = this.getCredentialsProperties(type);

// Load and apply the credentials overwrites if any exist
const dataWithOverwrites = CredentialsOverwrites().applyOverwrite(type, decryptedDataOriginal);
const dataWithOverwrites = this.credentialsOverwrites.applyOverwrite(
type,
decryptedDataOriginal,
);

// Add the default credential values
let decryptedData = NodeHelpers.getNodeParameters(
Expand Down
28 changes: 7 additions & 21 deletions packages/cli/src/CredentialsOverwrites.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,17 @@
import config from '@/config';
import { Service } from 'typedi';
import type { ICredentialDataDecryptedObject } from 'n8n-workflow';
import { deepCopy, LoggerProxy as Logger, jsonParse, ICredentialTypes } from 'n8n-workflow';
import { deepCopy, LoggerProxy as Logger, jsonParse } from 'n8n-workflow';
import config from '@/config';
import type { ICredentialsOverwrite } from '@/Interfaces';
import { CredentialTypes } from '@/CredentialTypes';

class CredentialsOverwritesClass {
@Service()
export class CredentialsOverwrites {
private overwriteData: ICredentialsOverwrite = {};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For understanding: What's the difference between the overwrites that happen in CredentialsOverwrites vs. the overwrites that happen in the FrontendService? Why do we need both and for them to be separate?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The overwrites in FrontendService are only used in the frontend, and that code should not run on worker or webhooks. That said, we could still move that code into a method on CredentialsOverwrites, if that looks cleaner.
let me know.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Skimming through them they look very similar and with no context or method documentation I do not find the difference clear. Running low on time so I'll leave this be for now.


private resolvedTypes: string[] = [];

constructor(private credentialTypes: ICredentialTypes) {
constructor(private credentialTypes: CredentialTypes) {
const data = config.getEnv('credentials.overwrite.data');
const overwriteData = jsonParse<ICredentialsOverwrite>(data, {
errorMessage: 'The credentials-overwrite is not valid JSON.',
Expand Down Expand Up @@ -96,20 +99,3 @@ class CredentialsOverwritesClass {
return this.overwriteData;
}
}

let credentialsOverwritesInstance: CredentialsOverwritesClass | undefined;

// eslint-disable-next-line @typescript-eslint/naming-convention
export function CredentialsOverwrites(
credentialTypes?: ICredentialTypes,
): CredentialsOverwritesClass {
if (!credentialsOverwritesInstance) {
if (credentialTypes) {
credentialsOverwritesInstance = new CredentialsOverwritesClass(credentialTypes);
} else {
throw new Error('CredentialsOverwrites not initialized yet');
}
}

return credentialsOverwritesInstance;
}