refactor(core): Tear down internal hooks (no-changelog) (#10340)

This commit is contained in:
Iván Ovejero 2024-08-12 10:13:15 +02:00 committed by GitHub
parent 78984986a6
commit 6b52bebf52
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
23 changed files with 95 additions and 98 deletions

View file

@ -5,7 +5,6 @@ import type { ExternalSecretsSettings } from '@/Interfaces';
import { License } from '@/License';
import { ExternalSecretsManager } from '@/ExternalSecrets/ExternalSecretsManager.ee';
import { ExternalSecretsProviders } from '@/ExternalSecrets/ExternalSecretsProviders.ee';
import { InternalHooks } from '@/InternalHooks';
import { mockInstance } from '@test/mocking';
import {
DummyProvider,
@ -22,7 +21,6 @@ describe('External Secrets Manager', () => {
const mockProvidersInstance = new MockProviders();
const license = mockInstance(License);
const settingsRepo = mockInstance(SettingsRepository);
mockInstance(InternalHooks);
const cipher = Container.get(Cipher);
let providersMock: ExternalSecretsProviders;

View file

@ -1,10 +1,10 @@
import { Service } from 'typedi';
import type { User } from '@db/entities/User';
import { Telemetry } from '@/telemetry';
import { MessageEventBus } from './eventbus/MessageEventBus/MessageEventBus';
/**
* @deprecated Do not add to this class. To add log streaming or telemetry events, use
* @deprecated Do not add to this class. It will be removed once we remove
* further dep cycles. To add log streaming or telemetry events, use
* `EventService` to emit the event and then use the `LogStreamingEventRelay` or
* `TelemetryEventRelay` to forward them to the event bus or telemetry.
*/
@ -21,35 +21,4 @@ export class InternalHooks {
async init() {
await this.telemetry.init();
}
onUserInviteEmailClick(userInviteClickData: { inviter: User; invitee: User }) {
this.telemetry.track('User clicked invite link from email', {
user_id: userInviteClickData.invitee.id,
});
}
onUserPasswordResetEmailClick(userPasswordResetData: { user: User }) {
this.telemetry.track('User clicked password reset link from email', {
user_id: userPasswordResetData.user.id,
});
}
onUserTransactionalEmail(userTransactionalEmailData: {
user_id: string;
message_type:
| 'Reset password'
| 'New user invite'
| 'Resend invite'
| 'Workflow shared'
| 'Credentials shared';
public_api: boolean;
}) {
this.telemetry.track('Instance sent transactional email to user', userTransactionalEmailData);
}
onUserPasswordResetRequestClick(userPasswordResetData: { user: User }) {
this.telemetry.track('User requested password reset while logged out', {
user_id: userPasswordResetData.user.id,
});
}
}

View file

@ -8,7 +8,6 @@ import { GlobalConfig } from '@n8n/config';
import type { User } from '@db/entities/User';
import type { WorkflowEntity } from '@db/entities/WorkflowEntity';
import { UserRepository } from '@db/repositories/user.repository';
import { InternalHooks } from '@/InternalHooks';
import { Logger } from '@/Logger';
import { UrlService } from '@/services/url.service';
import { InternalServerError } from '@/errors/response-errors/internal-server.error';
@ -112,10 +111,10 @@ export class UserManagementMailer {
this.logger.info('Sent workflow shared email successfully', { sharerId: sharer.id });
Container.get(InternalHooks).onUserTransactionalEmail({
user_id: sharer.id,
message_type: 'Workflow shared',
public_api: false,
Container.get(EventService).emit('user-transactional-email-sent', {
userId: sharer.id,
messageType: 'Workflow shared',
publicApi: false,
});
return result;
@ -167,10 +166,10 @@ export class UserManagementMailer {
this.logger.info('Sent credentials shared email successfully', { sharerId: sharer.id });
Container.get(InternalHooks).onUserTransactionalEmail({
user_id: sharer.id,
message_type: 'Credentials shared',
public_api: false,
Container.get(EventService).emit('user-transactional-email-sent', {
userId: sharer.id,
messageType: 'Credentials shared',
publicApi: false,
});
return result;

View file

@ -15,8 +15,8 @@ import { ExternalHooks } from '@/ExternalHooks';
import { NodeTypes } from '@/NodeTypes';
import { LoadNodesAndCredentials } from '@/LoadNodesAndCredentials';
import type { N8nInstanceType } from '@/Interfaces';
import { InternalHooks } from '@/InternalHooks';
import { PostHogClient } from '@/posthog';
import { InternalHooks } from '@/InternalHooks';
import { License } from '@/License';
import { ExternalSecretsManager } from '@/ExternalSecrets/ExternalSecretsManager.ee';
import { initExpressionEvaluator } from '@/ExpressionEvaluator';

View file

@ -9,7 +9,6 @@ import { AUTH_COOKIE_NAME } from '@/constants';
import type { AuthenticatedRequest, MeRequest } from '@/requests';
import { UserService } from '@/services/user.service';
import { ExternalHooks } from '@/ExternalHooks';
import { InternalHooks } from '@/InternalHooks';
import { License } from '@/License';
import { BadRequestError } from '@/errors/response-errors/bad-request.error';
import { UserRepository } from '@/databases/repositories/user.repository';
@ -21,7 +20,6 @@ const browserId = 'test-browser-id';
describe('MeController', () => {
const externalHooks = mockInstance(ExternalHooks);
mockInstance(InternalHooks);
const eventService = mockInstance(EventService);
const userService = mockInstance(UserService);
const userRepository = mockInstance(UserRepository);

View file

@ -14,7 +14,6 @@ import {
isLdapCurrentAuthenticationMethod,
isSamlCurrentAuthenticationMethod,
} from '@/sso/ssoHelpers';
import { InternalHooks } from '../InternalHooks';
import { License } from '@/License';
import { UserService } from '@/services/user.service';
import { MfaService } from '@/Mfa/mfa.service';
@ -30,7 +29,6 @@ import { EventService } from '@/events/event.service';
export class AuthController {
constructor(
private readonly logger: Logger,
private readonly internalHooks: InternalHooks,
private readonly authService: AuthService,
private readonly mfaService: MfaService,
private readonly userService: UserService,
@ -179,7 +177,6 @@ export class AuthController {
throw new BadRequestError('Invalid request');
}
this.internalHooks.onUserInviteEmailClick({ inviter, invitee });
this.eventService.emit('user-invite-email-click', { inviter, invitee });
const { firstName, lastName } = inviter;

View file

@ -13,7 +13,6 @@ import { RESPONSE_ERROR_MESSAGES } from '@/constants';
import { MfaService } from '@/Mfa/mfa.service';
import { Logger } from '@/Logger';
import { ExternalHooks } from '@/ExternalHooks';
import { InternalHooks } from '@/InternalHooks';
import { UrlService } from '@/services/url.service';
import { InternalServerError } from '@/errors/response-errors/internal-server.error';
import { BadRequestError } from '@/errors/response-errors/bad-request.error';
@ -28,7 +27,6 @@ export class PasswordResetController {
constructor(
private readonly logger: Logger,
private readonly externalHooks: ExternalHooks,
private readonly internalHooks: InternalHooks,
private readonly mailer: UserManagementMailer,
private readonly authService: AuthService,
private readonly userService: UserService,
@ -131,13 +129,12 @@ export class PasswordResetController {
}
this.logger.info('Sent password reset email successfully', { userId: user.id, email });
this.internalHooks.onUserTransactionalEmail({
user_id: id,
message_type: 'Reset password',
public_api: false,
this.eventService.emit('user-transactional-email-sent', {
userId: id,
messageType: 'Reset password',
publicApi: false,
});
this.internalHooks.onUserPasswordResetRequestClick({ user });
this.eventService.emit('user-password-reset-request-click', { user });
}
@ -170,7 +167,6 @@ export class PasswordResetController {
}
this.logger.info('Reset-password token resolved successfully', { userId: user.id });
this.internalHooks.onUserPasswordResetEmailClick({ user });
this.eventService.emit('user-password-reset-email-click', { user });
}

View file

@ -207,6 +207,17 @@ export type RelayEventMap = {
user: UserLike;
};
'user-transactional-email-sent': {
userId: string;
messageType:
| 'Reset password'
| 'New user invite'
| 'Resend invite'
| 'Workflow shared'
| 'Credentials shared';
publicApi: boolean;
};
// #endregion
// #region Public API

View file

@ -95,6 +95,10 @@ export class TelemetryEventRelay extends EventRelay {
'user-submitted-personalization-survey': (event) =>
this.userSubmittedPersonalizationSurvey(event),
'email-failed': (event) => this.emailFailed(event),
'user-transactional-email-sent': (event) => this.userTransactionalEmailSent(event),
'user-invite-email-click': (event) => this.userInviteEmailClick(event),
'user-password-reset-email-click': (event) => this.userPasswordResetEmailClick(event),
'user-password-reset-request-click': (event) => this.userPasswordResetRequestClick(event),
});
}
@ -955,5 +959,41 @@ export class TelemetryEventRelay extends EventRelay {
});
}
private userTransactionalEmailSent({
userId,
messageType,
publicApi,
}: RelayEventMap['user-transactional-email-sent']) {
this.telemetry.track('User sent transactional email', {
user_id: userId,
message_type: messageType,
public_api: publicApi,
});
}
// #endregion
// #region Click
private userInviteEmailClick({ invitee }: RelayEventMap['user-invite-email-click']) {
this.telemetry.track('User clicked invite link from email', {
user_id: invitee.id,
});
}
private userPasswordResetEmailClick({ user }: RelayEventMap['user-password-reset-email-click']) {
this.telemetry.track('User clicked password reset link from email', {
user_id: user.id,
});
}
private userPasswordResetRequestClick({
user,
}: RelayEventMap['user-password-reset-request-click']) {
this.telemetry.track('User requested password reset while logged out', {
user_id: user.id,
});
}
// #endregion
}

View file

@ -13,7 +13,6 @@ import { OrchestrationService } from '@/services/orchestration.service';
import config from '@/config';
import { ExecutionRecoveryService } from '@/executions/execution-recovery.service';
import { ExecutionRepository } from '@/databases/repositories/execution.repository';
import { InternalHooks } from '@/InternalHooks';
import { Push } from '@/push';
import { ARTIFICIAL_TASK_DATA } from '@/constants';
import { NodeCrashedError } from '@/errors/node-crashed.error';
@ -26,7 +25,6 @@ import type { EventMessageTypes as EventMessage } from '@/eventbus/EventMessageC
describe('ExecutionRecoveryService', () => {
const push = mockInstance(Push);
mockInstance(InternalHooks);
const instanceSettings = new InstanceSettings();
let executionRecoveryService: ExecutionRecoveryService;

View file

@ -1,4 +1,4 @@
import { Container, Service } from 'typedi';
import { Service } from 'typedi';
import type { IUserSettings } from 'n8n-workflow';
import { ApplicationError, ErrorReporterProxy as ErrorReporter } from 'n8n-workflow';
@ -8,7 +8,6 @@ import type { Invitation, PublicUser } from '@/Interfaces';
import type { PostHogClient } from '@/posthog';
import { Logger } from '@/Logger';
import { UserManagementMailer } from '@/UserManagement/email';
import { InternalHooks } from '@/InternalHooks';
import { UrlService } from '@/services/url.service';
import type { UserRequest } from '@/requests';
import { InternalServerError } from '@/errors/response-errors/internal-server.error';
@ -144,10 +143,11 @@ export class UserService {
if (result.emailSent) {
invitedUser.user.emailSent = true;
delete invitedUser.user?.inviteAcceptUrl;
Container.get(InternalHooks).onUserTransactionalEmail({
user_id: id,
message_type: 'New user invite',
public_api: false,
this.eventService.emit('user-transactional-email-sent', {
userId: id,
messageType: 'New user invite',
publicApi: false,
});
}

View file

@ -1,6 +1,5 @@
import { nanoid } from 'nanoid';
import { InternalHooks } from '@/InternalHooks';
import { ImportCredentialsCommand } from '@/commands/import/credentials';
import { LoadNodesAndCredentials } from '@/LoadNodesAndCredentials';
@ -11,7 +10,6 @@ import { getAllCredentials, getAllSharedCredentials } from '../shared/db/credent
import { createMember, createOwner } from '../shared/db/users';
import { getPersonalProject } from '../shared/db/projects';
mockInstance(InternalHooks);
mockInstance(LoadNodesAndCredentials);
const command = setupTestCommand(ImportCredentialsCommand);

View file

@ -1,6 +1,5 @@
import { nanoid } from 'nanoid';
import { InternalHooks } from '@/InternalHooks';
import { ImportWorkflowsCommand } from '@/commands/import/workflow';
import { LoadNodesAndCredentials } from '@/LoadNodesAndCredentials';
@ -11,7 +10,6 @@ import { getAllSharedWorkflows, getAllWorkflows } from '../shared/db/workflows';
import { createMember, createOwner } from '../shared/db/users';
import { getPersonalProject } from '../shared/db/projects';
mockInstance(InternalHooks);
mockInstance(LoadNodesAndCredentials);
const command = setupTestCommand(ImportWorkflowsCommand);

View file

@ -4,7 +4,6 @@ import { EntityNotFoundError } from '@n8n/typeorm';
import { Reset } from '@/commands/ldap/reset';
import { LoadNodesAndCredentials } from '@/LoadNodesAndCredentials';
import { InternalHooks } from '@/InternalHooks';
import { WorkflowRepository } from '@db/repositories/workflow.repository';
import { CredentialsRepository } from '@db/repositories/credentials.repository';
import { SharedWorkflowRepository } from '@db/repositories/sharedWorkflow.repository';
@ -26,7 +25,6 @@ import { createTeamProject, findProject, getPersonalProject } from '../../shared
mockInstance(Telemetry);
mockInstance(Push);
mockInstance(InternalHooks);
mockInstance(LoadNodesAndCredentials);
const command = setupTestCommand(Reset);

View file

@ -1,4 +1,3 @@
import { InternalHooks } from '@/InternalHooks';
import { License } from '@/License';
import { LoadNodesAndCredentials } from '@/LoadNodesAndCredentials';
import { ClearLicenseCommand } from '@/commands/license/clear';
@ -6,7 +5,6 @@ import { ClearLicenseCommand } from '@/commands/license/clear';
import { setupTestCommand } from '@test-integration/utils/testCommand';
import { mockInstance } from '../../shared/mocking';
mockInstance(InternalHooks);
mockInstance(LoadNodesAndCredentials);
const license = mockInstance(License);
const command = setupTestCommand(ClearLicenseCommand);

View file

@ -1,7 +1,6 @@
import { Container } from 'typedi';
import { Reset } from '@/commands/user-management/reset';
import { InternalHooks } from '@/InternalHooks';
import { LoadNodesAndCredentials } from '@/LoadNodesAndCredentials';
import { NodeTypes } from '@/NodeTypes';
import { SharedWorkflowRepository } from '@db/repositories/sharedWorkflow.repository';
@ -20,7 +19,6 @@ import { getPersonalProject } from '../shared/db/projects';
import { encryptCredentialData, saveCredential } from '../shared/db/credentials';
import { randomCredentialPayload } from '../shared/random';
mockInstance(InternalHooks);
mockInstance(LoadNodesAndCredentials);
mockInstance(NodeTypes);
const command = setupTestCommand(Reset);

View file

@ -1,4 +1,3 @@
import { InternalHooks } from '@/InternalHooks';
import { LoadNodesAndCredentials } from '@/LoadNodesAndCredentials';
import { UpdateWorkflowCommand } from '@/commands/update/workflow';
@ -7,7 +6,6 @@ import * as testDb from '../../shared/testDb';
import { createWorkflowWithTrigger, getAllWorkflows } from '../../shared/db/workflows';
import { mockInstance } from '../../../shared/mocking';
mockInstance(InternalHooks);
mockInstance(LoadNodesAndCredentials);
const command = setupTestCommand(UpdateWorkflowCommand);

View file

@ -5,7 +5,6 @@ import config from '@/config';
import { ExternalSecretsManager } from '@/ExternalSecrets/ExternalSecretsManager.ee';
import { MessageEventBus } from '@/eventbus/MessageEventBus/MessageEventBus';
import { LoadNodesAndCredentials } from '@/LoadNodesAndCredentials';
import { InternalHooks } from '@/InternalHooks';
import { OrchestrationHandlerWorkerService } from '@/services/orchestration/worker/orchestration.handler.worker.service';
import { OrchestrationWorkerService } from '@/services/orchestration/worker/orchestration.worker.service';
import { License } from '@/License';
@ -18,7 +17,6 @@ import { LogStreamingEventRelay } from '@/events/log-streaming-event-relay';
config.set('executions.mode', 'queue');
config.set('binaryDataManager.availableModes', 'filesystem');
mockInstance(InternalHooks);
mockInstance(LoadNodesAndCredentials);
const binaryDataService = mockInstance(BinaryDataService);
const externalHooks = mockInstance(ExternalHooks);

View file

@ -1,8 +1,6 @@
import { mocked } from 'jest-mock';
import Container from 'typedi';
import { Not } from '@n8n/typeorm';
import { InternalHooks } from '@/InternalHooks';
import { EventService } from '@/events/event.service';
import { ExternalHooks } from '@/ExternalHooks';
import { UserManagementMailer } from '@/UserManagement/email';
import { UserRepository } from '@/databases/repositories/user.repository';
@ -31,7 +29,7 @@ import { ProjectRelationRepository } from '@/databases/repositories/projectRelat
describe('InvitationController', () => {
const mailer = mockInstance(UserManagementMailer);
const externalHooks = mockInstance(ExternalHooks);
const internalHooks = mockInstance(InternalHooks);
const eventService = mockInstance(EventService);
const testServer = utils.setupTestServer({ endpointGroups: ['invitations'] });
@ -413,14 +411,24 @@ describe('InvitationController', () => {
expect(externalHookName).toBe('user.invited');
expect(externalHookArg?.[0]).toStrictEqual([newUserEmail]);
// internal hooks
const calls = mocked(internalHooks).onUserTransactionalEmail.mock.calls;
for (const [onUserTransactionalEmailArg] of calls) {
expect(onUserTransactionalEmailArg.user_id).toBeDefined();
expect(onUserTransactionalEmailArg.message_type).toBe('New user invite');
expect(onUserTransactionalEmailArg.public_api).toBe(false);
for (const [eventName, payload] of eventService.emit.mock.calls) {
if (eventName === 'user-invited') {
expect(payload).toEqual({
user: expect.objectContaining({ id: expect.any(String) }),
targetUserId: expect.arrayContaining([expect.any(String), expect.any(String)]),
publicApi: false,
emailSent: true,
inviteeRole: 'global:member',
});
} else if (eventName === 'user-transactional-email-sent') {
expect(payload).toEqual({
userId: expect.any(String),
messageType: 'New user invite',
publicApi: false,
});
} else {
fail(`Unexpected event name: ${eventName}`);
}
}
});

View file

@ -6,6 +6,9 @@ import type { BaseCommand } from '@/commands/BaseCommand';
import * as testDb from '../testDb';
import { TelemetryEventRelay } from '@/events/telemetry-event-relay';
import { mockInstance } from '@test/mocking';
import { InternalHooks } from '@/InternalHooks';
mockInstance(InternalHooks);
export const setupTestCommand = <T extends BaseCommand>(Command: Class<T>) => {
const config = mock<Config>();

View file

@ -14,7 +14,6 @@ import { PostHogClient } from '@/posthog';
import { Push } from '@/push';
import { License } from '@/License';
import { Logger } from '@/Logger';
import { InternalHooks } from '@/InternalHooks';
import { AuthService } from '@/auth/auth.service';
import type { APIRequest } from '@/requests';
@ -82,7 +81,6 @@ export const setupTestServer = ({
// Mock all telemetry and logging
mockInstance(Logger);
mockInstance(InternalHooks);
mockInstance(PostHogClient);
mockInstance(Push);

View file

@ -4,7 +4,6 @@ import type { INodeType, INodeTypeDescription, IWebhookFunctions } from 'n8n-wor
import { AbstractServer } from '@/AbstractServer';
import { ExternalHooks } from '@/ExternalHooks';
import { InternalHooks } from '@/InternalHooks';
import { NodeTypes } from '@/NodeTypes';
import { Push } from '@/push';
import type { WorkflowEntity } from '@db/entities/WorkflowEntity';
@ -21,7 +20,6 @@ mockInstance(Telemetry);
describe('Webhook API', () => {
mockInstance(ExternalHooks);
mockInstance(InternalHooks);
mockInstance(Push);
let agent: SuperAgentTest;

View file

@ -5,7 +5,6 @@ import { mock } from 'jest-mock-extended';
import { AbstractServer } from '@/AbstractServer';
import { ActiveWebhooks } from '@/webhooks/ActiveWebhooks';
import { ExternalHooks } from '@/ExternalHooks';
import { InternalHooks } from '@/InternalHooks';
import { TestWebhooks } from '@/webhooks/TestWebhooks';
import { WaitingWebhooks } from '@/webhooks/WaitingWebhooks';
import { WaitingForms } from '@/WaitingForms';
@ -19,7 +18,6 @@ let agent: SuperAgentTest;
describe('WebhookServer', () => {
mockInstance(ExternalHooks);
mockInstance(InternalHooks);
describe('CORS', () => {
const corsOrigin = 'https://example.com';