feat: implement dynamic driver configuration + fix integration test log pollution (#12104)

### Primary Changes: Dynamic Driver Configuration
Refactors FileStorageService and EmailSenderService to support dynamic
driver configuration changes at runtime without requiring application
restarts.

**Key Architectural Change**: Instead of conditionally registering
drivers at build time based on configuration, we now **register all
possible drivers eagerly** and select the appropriate one at runtime.

### What Changed:
- **Before**: Modules conditionally registered only the configured
driver (e.g., only S3Driver if STORAGE_TYPE=S3)
- **After**: All drivers (LocalDriver, S3Driver, SmtpDriver,
LoggerDriver) are registered at startup
- **Runtime Selection**: Services dynamically choose and instantiate the
correct driver based on current configuration

### Secondary Fix: Integration Test Log Cleanup
Addresses ConfigStorageService error logs appearing in integration test
output by using injected LoggerService for consistent log handling.
This commit is contained in:
nitin
2025-05-28 14:19:20 +05:30
committed by GitHub
parent d133055609
commit 1c64b7b072
31 changed files with 1432 additions and 540 deletions

View File

@ -14,12 +14,10 @@ import { TimelineCalendarEventModule } from 'src/engine/core-modules/calendar/ti
import { CaptchaModule } from 'src/engine/core-modules/captcha/captcha.module';
import { captchaModuleFactory } from 'src/engine/core-modules/captcha/captcha.module-factory';
import { EmailModule } from 'src/engine/core-modules/email/email.module';
import { emailModuleFactory } from 'src/engine/core-modules/email/email.module-factory';
import { ExceptionHandlerModule } from 'src/engine/core-modules/exception-handler/exception-handler.module';
import { exceptionHandlerModuleFactory } from 'src/engine/core-modules/exception-handler/exception-handler.module-factory';
import { FeatureFlagModule } from 'src/engine/core-modules/feature-flag/feature-flag.module';
import { FileStorageModule } from 'src/engine/core-modules/file-storage/file-storage.module';
import { fileStorageModuleFactory } from 'src/engine/core-modules/file-storage/file-storage.module-factory';
import { FileStorageService } from 'src/engine/core-modules/file-storage/file-storage.service';
import { HealthModule } from 'src/engine/core-modules/health/health.module';
import { LabModule } from 'src/engine/core-modules/lab/lab.module';
@ -85,10 +83,7 @@ import { FileModule } from './file/file.module';
RedisClientModule,
WorkspaceQueryRunnerModule,
SubscriptionsModule,
FileStorageModule.forRootAsync({
useFactory: fileStorageModuleFactory,
inject: [TwentyConfigService],
}),
FileStorageModule.forRoot(),
LoggerModule.forRootAsync({
useFactory: loggerModuleFactory,
inject: [TwentyConfigService],
@ -101,10 +96,7 @@ import { FileModule } from './file/file.module';
useFactory: exceptionHandlerModuleFactory,
inject: [TwentyConfigService, HttpAdapterHost],
}),
EmailModule.forRoot({
useFactory: emailModuleFactory,
inject: [TwentyConfigService],
}),
EmailModule.forRoot(),
CaptchaModule.forRoot({
useFactory: captchaModuleFactory,
inject: [TwentyConfigService],

View File

@ -0,0 +1,229 @@
import { Test, TestingModule } from '@nestjs/testing';
import { EmailDriverFactory } from 'src/engine/core-modules/email/email-driver.factory';
import { EmailDriver } from 'src/engine/core-modules/email/enums/email-driver.enum';
import { TwentyConfigService } from 'src/engine/core-modules/twenty-config/twenty-config.service';
describe('EmailDriverFactory', () => {
let factory: EmailDriverFactory;
let twentyConfigService: TwentyConfigService;
const mockTwentyConfigService = {
get: jest.fn(),
};
beforeEach(async () => {
const module: TestingModule = await Test.createTestingModule({
providers: [
EmailDriverFactory,
{
provide: TwentyConfigService,
useValue: mockTwentyConfigService,
},
],
}).compile();
factory = module.get<EmailDriverFactory>(EmailDriverFactory);
twentyConfigService = module.get<TwentyConfigService>(TwentyConfigService);
jest.clearAllMocks();
});
describe('buildConfigKey', () => {
it('should return "logger" for logger driver', () => {
jest
.spyOn(twentyConfigService, 'get')
.mockReturnValue(EmailDriver.Logger);
const result = factory['buildConfigKey']();
expect(result).toBe('logger');
expect(twentyConfigService.get).toHaveBeenCalledWith('EMAIL_DRIVER');
});
it('should return smtp config key for smtp driver', () => {
jest.spyOn(twentyConfigService, 'get').mockReturnValue(EmailDriver.Smtp);
jest
.spyOn(factory as any, 'getConfigGroupHash')
.mockReturnValue('smtp-hash-123');
const result = factory['buildConfigKey']();
expect(result).toBe('smtp|smtp-hash-123');
expect(twentyConfigService.get).toHaveBeenCalledWith('EMAIL_DRIVER');
});
it('should throw error for unsupported driver', () => {
jest.spyOn(twentyConfigService, 'get').mockReturnValue('invalid-driver');
expect(() => factory['buildConfigKey']()).toThrow(
'Unsupported email driver: invalid-driver',
);
});
});
describe('createDriver', () => {
it('should create logger driver', () => {
jest
.spyOn(twentyConfigService, 'get')
.mockReturnValue(EmailDriver.Logger);
const driver = factory['createDriver']();
expect(driver).toBeDefined();
expect(driver.constructor.name).toBe('LoggerDriver');
});
it('should create smtp driver with basic configuration', () => {
jest
.spyOn(twentyConfigService, 'get')
.mockImplementation((key: string) => {
switch (key) {
case 'EMAIL_DRIVER':
return EmailDriver.Smtp;
case 'EMAIL_SMTP_HOST':
return 'smtp.example.com';
case 'EMAIL_SMTP_PORT':
return 587;
case 'EMAIL_SMTP_USER':
return undefined;
case 'EMAIL_SMTP_PASSWORD':
return undefined;
case 'EMAIL_SMTP_NO_TLS':
return false;
default:
return undefined;
}
});
const driver = factory['createDriver']();
expect(driver).toBeDefined();
expect(driver.constructor.name).toBe('SmtpDriver');
});
it('should throw error when smtp host is missing', () => {
jest
.spyOn(twentyConfigService, 'get')
.mockImplementation((key: string) => {
switch (key) {
case 'EMAIL_DRIVER':
return EmailDriver.Smtp;
case 'EMAIL_SMTP_HOST':
return undefined;
case 'EMAIL_SMTP_PORT':
return 587;
default:
return undefined;
}
});
expect(() => factory['createDriver']()).toThrow(
'SMTP driver requires host and port to be defined',
);
});
it('should throw error for invalid driver', () => {
jest.spyOn(twentyConfigService, 'get').mockReturnValue('invalid-driver');
expect(() => factory['createDriver']()).toThrow(
'Invalid email driver: invalid-driver',
);
});
});
describe('getCurrentDriver', () => {
it('should return current driver for logger', () => {
jest
.spyOn(twentyConfigService, 'get')
.mockReturnValue(EmailDriver.Logger);
const driver = factory.getCurrentDriver();
expect(driver).toBeDefined();
expect(driver.constructor.name).toBe('LoggerDriver');
});
it('should reuse driver when config key unchanged', () => {
jest
.spyOn(twentyConfigService, 'get')
.mockReturnValue(EmailDriver.Logger);
const driver1 = factory.getCurrentDriver();
const driver2 = factory.getCurrentDriver();
expect(driver1).toBe(driver2);
});
it('should create new driver when config key changes', () => {
// First call with logger
jest
.spyOn(twentyConfigService, 'get')
.mockReturnValue(EmailDriver.Logger);
const driver1 = factory.getCurrentDriver();
// Second call with smtp
jest
.spyOn(twentyConfigService, 'get')
.mockImplementation((key: string) => {
switch (key) {
case 'EMAIL_DRIVER':
return EmailDriver.Smtp;
case 'EMAIL_SMTP_HOST':
return 'smtp.example.com';
case 'EMAIL_SMTP_PORT':
return 587;
default:
return undefined;
}
});
jest
.spyOn(factory as any, 'getConfigGroupHash')
.mockReturnValue('smtp-hash-123');
const driver2 = factory.getCurrentDriver();
expect(driver1).not.toBe(driver2);
expect(driver1.constructor.name).toBe('LoggerDriver');
expect(driver2.constructor.name).toBe('SmtpDriver');
});
it('should throw error for unsupported email driver', () => {
jest.spyOn(twentyConfigService, 'get').mockReturnValue('invalid-driver');
expect(() => factory.getCurrentDriver()).toThrow(
'Failed to build config key for EmailDriverFactory. Original error: Unsupported email driver: invalid-driver',
);
});
it('should throw error when driver creation fails after valid config', () => {
jest
.spyOn(twentyConfigService, 'get')
.mockImplementation((key: string) => {
switch (key) {
case 'EMAIL_DRIVER':
return EmailDriver.Smtp;
case 'EMAIL_SMTP_HOST':
return 'smtp.example.com';
case 'EMAIL_SMTP_PORT':
return 587;
default:
return undefined;
}
});
jest
.spyOn(factory as any, 'getConfigGroupHash')
.mockReturnValue('smtp-hash-123');
jest.spyOn(factory as any, 'createDriver').mockImplementation(() => {
throw new Error('Driver creation failed');
});
expect(() => factory.getCurrentDriver()).toThrow(
'Failed to create driver for EmailDriverFactory with config key: smtp|smtp-hash-123. Original error: Driver creation failed',
);
});
});
});

View File

@ -0,0 +1,73 @@
import { Test, TestingModule } from '@nestjs/testing';
import { SendMailOptions } from 'nodemailer';
import { EmailDriverFactory } from 'src/engine/core-modules/email/email-driver.factory';
import { EmailSenderService } from 'src/engine/core-modules/email/email-sender.service';
describe('EmailSenderService', () => {
let service: EmailSenderService;
let emailDriverFactory: EmailDriverFactory;
const mockEmailDriverFactory = {
getCurrentDriver: jest.fn(),
};
beforeEach(async () => {
const module: TestingModule = await Test.createTestingModule({
providers: [
EmailSenderService,
{
provide: EmailDriverFactory,
useValue: mockEmailDriverFactory,
},
],
}).compile();
service = module.get<EmailSenderService>(EmailSenderService);
emailDriverFactory = module.get<EmailDriverFactory>(EmailDriverFactory);
jest.clearAllMocks();
});
describe('send', () => {
it('should delegate to the current driver', async () => {
const mockDriver = {
send: jest.fn().mockResolvedValue(undefined),
};
mockEmailDriverFactory.getCurrentDriver.mockReturnValue(mockDriver);
const sendMailOptions: SendMailOptions = {
to: 'test@example.com',
subject: 'Test',
text: 'Test message',
};
await service.send(sendMailOptions);
expect(emailDriverFactory.getCurrentDriver).toHaveBeenCalled();
expect(mockDriver.send).toHaveBeenCalledWith(sendMailOptions);
});
it('should handle driver errors', async () => {
const mockDriver = {
send: jest.fn().mockRejectedValue(new Error('Driver error')),
};
mockEmailDriverFactory.getCurrentDriver.mockReturnValue(mockDriver);
const sendMailOptions: SendMailOptions = {
to: 'test@example.com',
subject: 'Test',
text: 'Test message',
};
await expect(service.send(sendMailOptions)).rejects.toThrow(
'Driver error',
);
expect(emailDriverFactory.getCurrentDriver).toHaveBeenCalled();
expect(mockDriver.send).toHaveBeenCalledWith(sendMailOptions);
});
});
});

View File

@ -1,5 +1,5 @@
import { SendMailOptions } from 'nodemailer';
export interface EmailDriver {
export interface EmailDriverInterface {
send(sendMailOptions: SendMailOptions): Promise<void>;
}

View File

@ -2,9 +2,9 @@ import { Logger } from '@nestjs/common';
import { SendMailOptions } from 'nodemailer';
import { EmailDriver } from 'src/engine/core-modules/email/drivers/interfaces/email-driver.interface';
import { EmailDriverInterface } from 'src/engine/core-modules/email/drivers/interfaces/email-driver.interface';
export class LoggerDriver implements EmailDriver {
export class LoggerDriver implements EmailDriverInterface {
private readonly logger = new Logger(LoggerDriver.name);
async send(sendMailOptions: SendMailOptions): Promise<void> {

View File

@ -1,11 +1,11 @@
import { Logger } from '@nestjs/common';
import { createTransport, Transporter, SendMailOptions } from 'nodemailer';
import { createTransport, SendMailOptions, Transporter } from 'nodemailer';
import SMTPConnection from 'nodemailer/lib/smtp-connection';
import { EmailDriver } from 'src/engine/core-modules/email/drivers/interfaces/email-driver.interface';
import { EmailDriverInterface } from 'src/engine/core-modules/email/drivers/interfaces/email-driver.interface';
export class SmtpDriver implements EmailDriver {
export class SmtpDriver implements EmailDriverInterface {
private readonly logger = new Logger(SmtpDriver.name);
private transport: Transporter;

View File

@ -0,0 +1,79 @@
import { Injectable } from '@nestjs/common';
import { EmailDriverInterface } from 'src/engine/core-modules/email/drivers/interfaces/email-driver.interface';
import { LoggerDriver } from 'src/engine/core-modules/email/drivers/logger.driver';
import { SmtpDriver } from 'src/engine/core-modules/email/drivers/smtp.driver';
import { EmailDriver } from 'src/engine/core-modules/email/enums/email-driver.enum';
import { DriverFactoryBase } from 'src/engine/core-modules/twenty-config/dynamic-factory.base';
import { ConfigVariablesGroup } from 'src/engine/core-modules/twenty-config/enums/config-variables-group.enum';
import { TwentyConfigService } from 'src/engine/core-modules/twenty-config/twenty-config.service';
@Injectable()
export class EmailDriverFactory extends DriverFactoryBase<EmailDriverInterface> {
constructor(twentyConfigService: TwentyConfigService) {
super(twentyConfigService);
}
protected buildConfigKey(): string {
const driver = this.twentyConfigService.get('EMAIL_DRIVER');
if (driver === EmailDriver.Logger) {
return 'logger';
}
if (driver === EmailDriver.Smtp) {
const emailConfigHash = this.getConfigGroupHash(
ConfigVariablesGroup.EmailSettings,
);
return `smtp|${emailConfigHash}`;
}
throw new Error(`Unsupported email driver: ${driver}`);
}
protected createDriver(): EmailDriverInterface {
const driver = this.twentyConfigService.get('EMAIL_DRIVER');
switch (driver) {
case EmailDriver.Logger:
return new LoggerDriver();
case EmailDriver.Smtp: {
const host = this.twentyConfigService.get('EMAIL_SMTP_HOST');
const port = this.twentyConfigService.get('EMAIL_SMTP_PORT');
const user = this.twentyConfigService.get('EMAIL_SMTP_USER');
const pass = this.twentyConfigService.get('EMAIL_SMTP_PASSWORD');
const noTLS = this.twentyConfigService.get('EMAIL_SMTP_NO_TLS');
if (!host || !port) {
throw new Error('SMTP driver requires host and port to be defined');
}
const options: {
host: string;
port: number;
auth?: { user: string; pass: string };
secure?: boolean;
ignoreTLS?: boolean;
requireTLS?: boolean;
} = { host, port };
if (user && pass) {
options.auth = { user, pass };
}
if (noTLS) {
options.secure = false;
options.ignoreTLS = true;
}
return new SmtpDriver(options);
}
default:
throw new Error(`Invalid email driver: ${driver}`);
}
}
}

View File

@ -1,16 +1,18 @@
import { Inject, Injectable } from '@nestjs/common';
import { Injectable } from '@nestjs/common';
import { SendMailOptions } from 'nodemailer';
import { EmailDriver } from 'src/engine/core-modules/email/drivers/interfaces/email-driver.interface';
import { EmailDriverInterface } from 'src/engine/core-modules/email/drivers/interfaces/email-driver.interface';
import { EMAIL_DRIVER } from 'src/engine/core-modules/email/email.constants';
import { EmailDriverFactory } from 'src/engine/core-modules/email/email-driver.factory';
@Injectable()
export class EmailSenderService implements EmailDriver {
constructor(@Inject(EMAIL_DRIVER) private driver: EmailDriver) {}
export class EmailSenderService implements EmailDriverInterface {
constructor(private readonly emailDriverFactory: EmailDriverFactory) {}
async send(sendMailOptions: SendMailOptions): Promise<void> {
await this.driver.send(sendMailOptions);
const driver = this.emailDriverFactory.getCurrentDriver();
await driver.send(sendMailOptions);
}
}

View File

@ -1 +0,0 @@
export const EMAIL_DRIVER = Symbol('EMAIL_DRIVER');

View File

@ -1,52 +0,0 @@
import {
EmailDriver,
EmailModuleOptions,
} from 'src/engine/core-modules/email/interfaces/email.interface';
import { TwentyConfigService } from 'src/engine/core-modules/twenty-config/twenty-config.service';
export const emailModuleFactory = (
twentyConfigService: TwentyConfigService,
): EmailModuleOptions => {
const driver = twentyConfigService.get('EMAIL_DRIVER');
switch (driver) {
case EmailDriver.Logger:
return {
type: EmailDriver.Logger,
};
case EmailDriver.Smtp: {
const options: EmailModuleOptions = {
type: EmailDriver.Smtp,
};
const host = twentyConfigService.get('EMAIL_SMTP_HOST');
const port = twentyConfigService.get('EMAIL_SMTP_PORT');
const user = twentyConfigService.get('EMAIL_SMTP_USER');
const pass = twentyConfigService.get('EMAIL_SMTP_PASSWORD');
const noTLS = twentyConfigService.get('EMAIL_SMTP_NO_TLS');
if (!host || !port) {
throw new Error(
`${driver} email driver requires host and port to be defined, check your .env file`,
);
}
options.host = host;
options.port = port;
if (user && pass) options.auth = { user, pass };
if (noTLS) {
options.secure = false;
options.ignoreTLS = true;
}
return options;
}
default:
throw new Error(`Invalid email driver (${driver}), check your .env file`);
}
};

View File

@ -1,35 +1,17 @@
import { DynamicModule, Global } from '@nestjs/common';
import {
EmailDriver,
EmailModuleAsyncOptions,
} from 'src/engine/core-modules/email/interfaces/email.interface';
import { LoggerDriver } from 'src/engine/core-modules/email/drivers/logger.driver';
import { SmtpDriver } from 'src/engine/core-modules/email/drivers/smtp.driver';
import { EmailDriverFactory } from 'src/engine/core-modules/email/email-driver.factory';
import { EmailSenderService } from 'src/engine/core-modules/email/email-sender.service';
import { EMAIL_DRIVER } from 'src/engine/core-modules/email/email.constants';
import { EmailService } from 'src/engine/core-modules/email/email.service';
import { TwentyConfigModule } from 'src/engine/core-modules/twenty-config/twenty-config.module';
@Global()
export class EmailModule {
static forRoot(options: EmailModuleAsyncOptions): DynamicModule {
const provider = {
provide: EMAIL_DRIVER,
// eslint-disable-next-line @typescript-eslint/no-explicit-any
useFactory: (...args: any[]) => {
const config = options.useFactory(...args);
return config.type === EmailDriver.Smtp
? new SmtpDriver(config)
: new LoggerDriver();
},
inject: options.inject || [],
};
static forRoot(): DynamicModule {
return {
module: EmailModule,
providers: [EmailSenderService, EmailService, provider],
imports: [TwentyConfigModule],
providers: [EmailDriverFactory, EmailSenderService, EmailService],
exports: [EmailSenderService, EmailService],
};
}

View File

@ -0,0 +1,4 @@
export enum EmailDriver {
Logger = 'logger',
Smtp = 'smtp',
}

View File

@ -1,22 +0,0 @@
import { FactoryProvider, ModuleMetadata } from '@nestjs/common';
import SMTPConnection from 'nodemailer/lib/smtp-connection';
export enum EmailDriver {
Logger = 'logger',
Smtp = 'smtp',
}
export type EmailModuleOptions =
| (SMTPConnection.Options & {
type: EmailDriver.Smtp;
})
| {
type: EmailDriver.Logger;
};
export type EmailModuleAsyncOptions = {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
useFactory: (...args: any[]) => EmailModuleOptions;
} & Pick<ModuleMetadata, 'imports'> &
Pick<FactoryProvider, 'inject'>;

View File

@ -0,0 +1,303 @@
import { Test, TestingModule } from '@nestjs/testing';
import { StorageDriverType } from 'src/engine/core-modules/file-storage/interfaces/file-storage.interface';
import { FileStorageDriverFactory } from 'src/engine/core-modules/file-storage/file-storage-driver.factory';
import { TwentyConfigService } from 'src/engine/core-modules/twenty-config/twenty-config.service';
describe('FileStorageDriverFactory', () => {
let factory: FileStorageDriverFactory;
let twentyConfigService: TwentyConfigService;
const mockTwentyConfigService = {
get: jest.fn(),
};
beforeEach(async () => {
const module: TestingModule = await Test.createTestingModule({
providers: [
FileStorageDriverFactory,
{
provide: TwentyConfigService,
useValue: mockTwentyConfigService,
},
],
}).compile();
factory = module.get<FileStorageDriverFactory>(FileStorageDriverFactory);
twentyConfigService = module.get<TwentyConfigService>(TwentyConfigService);
jest.clearAllMocks();
});
describe('buildConfigKey', () => {
it('should build config key for local storage', () => {
const storagePath = '/tmp/storage';
jest
.spyOn(twentyConfigService, 'get')
.mockImplementation((key: string) => {
if (key === 'STORAGE_TYPE') return StorageDriverType.Local;
if (key === 'STORAGE_LOCAL_PATH') return storagePath;
return undefined;
});
const result = factory['buildConfigKey']();
expect(result).toBe(`local|${storagePath}`);
expect(twentyConfigService.get).toHaveBeenCalledWith('STORAGE_TYPE');
expect(twentyConfigService.get).toHaveBeenCalledWith(
'STORAGE_LOCAL_PATH',
);
});
it('should build config key for S3 storage', () => {
jest
.spyOn(twentyConfigService, 'get')
.mockReturnValue(StorageDriverType.S3);
jest
.spyOn(factory as any, 'getConfigGroupHash')
.mockReturnValue('s3-hash-123');
const result = factory['buildConfigKey']();
expect(result).toBe('s3|s3-hash-123');
expect(twentyConfigService.get).toHaveBeenCalledWith('STORAGE_TYPE');
});
it('should throw error for unsupported storage type', () => {
jest
.spyOn(twentyConfigService, 'get')
.mockReturnValue('unsupported-type');
expect(() => factory['buildConfigKey']()).toThrow(
'Unsupported storage type: unsupported-type',
);
});
});
describe('createDriver', () => {
it('should create LocalDriver for local storage', () => {
const storagePath = '/tmp/storage';
jest
.spyOn(twentyConfigService, 'get')
.mockImplementation((key: string) => {
if (key === 'STORAGE_TYPE') return StorageDriverType.Local;
if (key === 'STORAGE_LOCAL_PATH') return storagePath;
return undefined;
});
const driver = factory['createDriver']();
expect(driver).toBeDefined();
expect(driver.constructor.name).toBe('LocalDriver');
});
it('should create S3Driver for S3 storage with access keys', () => {
jest
.spyOn(twentyConfigService, 'get')
.mockImplementation((key: string) => {
switch (key) {
case 'STORAGE_TYPE':
return StorageDriverType.S3;
case 'STORAGE_S3_NAME':
return 'test-bucket';
case 'STORAGE_S3_ENDPOINT':
return 'https://s3.amazonaws.com';
case 'STORAGE_S3_REGION':
return 'us-east-1';
case 'STORAGE_S3_ACCESS_KEY_ID':
return 'test-access-key';
case 'STORAGE_S3_SECRET_ACCESS_KEY':
return 'test-secret-key';
default:
return undefined;
}
});
const driver = factory['createDriver']();
expect(driver).toBeDefined();
expect(driver.constructor.name).toBe('S3Driver');
});
it('should create S3Driver for S3 storage without access keys (using provider chain)', () => {
jest
.spyOn(twentyConfigService, 'get')
.mockImplementation((key: string) => {
switch (key) {
case 'STORAGE_TYPE':
return StorageDriverType.S3;
case 'STORAGE_S3_NAME':
return 'test-bucket';
case 'STORAGE_S3_ENDPOINT':
return 'https://s3.amazonaws.com';
case 'STORAGE_S3_REGION':
return 'us-east-1';
case 'STORAGE_S3_ACCESS_KEY_ID':
return undefined;
case 'STORAGE_S3_SECRET_ACCESS_KEY':
return undefined;
default:
return undefined;
}
});
const driver = factory['createDriver']();
expect(driver).toBeDefined();
expect(driver.constructor.name).toBe('S3Driver');
});
it('should throw error for invalid storage driver type', () => {
jest.spyOn(twentyConfigService, 'get').mockReturnValue('invalid-type');
expect(() => factory['createDriver']()).toThrow(
'Invalid storage driver type: invalid-type',
);
});
});
describe('getCurrentDriver', () => {
it('should return current driver for local storage', () => {
const storagePath = '/tmp/storage';
jest
.spyOn(twentyConfigService, 'get')
.mockImplementation((key: string) => {
if (key === 'STORAGE_TYPE') return StorageDriverType.Local;
if (key === 'STORAGE_LOCAL_PATH') return storagePath;
return undefined;
});
const driver = factory.getCurrentDriver();
expect(driver).toBeDefined();
expect(driver.constructor.name).toBe('LocalDriver');
});
it('should reuse driver when config key unchanged', () => {
const storagePath = '/tmp/storage';
jest
.spyOn(twentyConfigService, 'get')
.mockImplementation((key: string) => {
if (key === 'STORAGE_TYPE') return StorageDriverType.Local;
if (key === 'STORAGE_LOCAL_PATH') return storagePath;
return undefined;
});
const driver1 = factory.getCurrentDriver();
const driver2 = factory.getCurrentDriver();
expect(driver1).toBe(driver2);
});
it('should create new driver when config key changes', () => {
// First call with local storage
jest
.spyOn(twentyConfigService, 'get')
.mockImplementation((key: string) => {
if (key === 'STORAGE_TYPE') return StorageDriverType.Local;
if (key === 'STORAGE_LOCAL_PATH') return '/tmp/storage1';
return undefined;
});
const driver1 = factory.getCurrentDriver();
// Second call with different local path
jest
.spyOn(twentyConfigService, 'get')
.mockImplementation((key: string) => {
if (key === 'STORAGE_TYPE') return StorageDriverType.Local;
if (key === 'STORAGE_LOCAL_PATH') return '/tmp/storage2';
return undefined;
});
const driver2 = factory.getCurrentDriver();
expect(driver1).not.toBe(driver2);
expect(driver1.constructor.name).toBe('LocalDriver');
expect(driver2.constructor.name).toBe('LocalDriver');
});
it('should create new driver when switching from local to S3', () => {
// First call with local storage
jest
.spyOn(twentyConfigService, 'get')
.mockImplementation((key: string) => {
if (key === 'STORAGE_TYPE') return StorageDriverType.Local;
if (key === 'STORAGE_LOCAL_PATH') return '/tmp/storage';
return undefined;
});
const driver1 = factory.getCurrentDriver();
// Second call with S3 storage
jest
.spyOn(twentyConfigService, 'get')
.mockImplementation((key: string) => {
switch (key) {
case 'STORAGE_TYPE':
return StorageDriverType.S3;
case 'STORAGE_S3_NAME':
return 'test-bucket';
case 'STORAGE_S3_ENDPOINT':
return 'https://s3.amazonaws.com';
case 'STORAGE_S3_REGION':
return 'us-east-1';
default:
return undefined;
}
});
jest
.spyOn(factory as any, 'getConfigGroupHash')
.mockReturnValue('s3-hash-123');
const driver2 = factory.getCurrentDriver();
expect(driver1).not.toBe(driver2);
expect(driver1.constructor.name).toBe('LocalDriver');
expect(driver2.constructor.name).toBe('S3Driver');
});
it('should throw error for unsupported storage type', () => {
jest
.spyOn(twentyConfigService, 'get')
.mockReturnValue('invalid-storage-type');
expect(() => factory.getCurrentDriver()).toThrow(
'Failed to build config key for FileStorageDriverFactory. Original error: Unsupported storage type: invalid-storage-type',
);
});
it('should throw error when driver creation fails after valid config', () => {
const storagePath = '/tmp/storage';
jest
.spyOn(twentyConfigService, 'get')
.mockImplementation((key: string) => {
if (key === 'STORAGE_TYPE') return StorageDriverType.Local;
if (key === 'STORAGE_LOCAL_PATH') return storagePath;
return undefined;
});
jest.spyOn(factory as any, 'createDriver').mockImplementation(() => {
throw new Error('Driver creation failed');
});
expect(() => factory.getCurrentDriver()).toThrow(
'Failed to create driver for FileStorageDriverFactory with config key: local|/tmp/storage. Original error: Driver creation failed',
);
});
});
});

View File

@ -1,26 +1,317 @@
import { Test, TestingModule } from '@nestjs/testing';
import { STORAGE_DRIVER } from 'src/engine/core-modules/file-storage/file-storage.constants';
import { Readable } from 'stream';
import { FileStorageDriverFactory } from 'src/engine/core-modules/file-storage/file-storage-driver.factory';
import { FileStorageService } from 'src/engine/core-modules/file-storage/file-storage.service';
describe('FileStorageService', () => {
let service: FileStorageService;
let fileStorageDriverFactory: FileStorageDriverFactory;
const mockFileStorageDriverFactory = {
getCurrentDriver: jest.fn(),
};
beforeEach(async () => {
const module: TestingModule = await Test.createTestingModule({
providers: [
FileStorageService,
{
provide: STORAGE_DRIVER,
useValue: {},
provide: FileStorageDriverFactory,
useValue: mockFileStorageDriverFactory,
},
],
}).compile();
service = module.get<FileStorageService>(FileStorageService);
fileStorageDriverFactory = module.get<FileStorageDriverFactory>(
FileStorageDriverFactory,
);
jest.clearAllMocks();
});
it('should be defined', () => {
expect(service).toBeDefined();
});
describe('storage operations', () => {
let mockDriver: any;
beforeEach(() => {
mockDriver = {
write: jest.fn(),
read: jest.fn(),
delete: jest.fn(),
move: jest.fn(),
copy: jest.fn(),
download: jest.fn(),
checkFileExists: jest.fn(),
};
mockFileStorageDriverFactory.getCurrentDriver.mockReturnValue(mockDriver);
});
describe('write', () => {
it('should delegate to the current driver', async () => {
const writeParams = {
file: Buffer.from('test content'),
name: 'test.txt',
folder: 'documents',
mimeType: 'text/plain',
};
mockDriver.write.mockResolvedValue(undefined);
await service.write(writeParams);
expect(fileStorageDriverFactory.getCurrentDriver).toHaveBeenCalled();
expect(mockDriver.write).toHaveBeenCalledWith(writeParams);
});
it('should handle write errors', async () => {
const writeParams = {
file: 'test content',
name: 'test.txt',
folder: 'documents',
mimeType: 'text/plain',
};
const error = new Error('Write failed');
mockDriver.write.mockRejectedValue(error);
await expect(service.write(writeParams)).rejects.toThrow(
'Write failed',
);
expect(fileStorageDriverFactory.getCurrentDriver).toHaveBeenCalled();
expect(mockDriver.write).toHaveBeenCalledWith(writeParams);
});
});
describe('read', () => {
it('should delegate to the current driver', async () => {
const readParams = {
folderPath: 'documents',
filename: 'test.txt',
};
const mockStream = new Readable();
mockDriver.read.mockResolvedValue(mockStream);
const result = await service.read(readParams);
expect(fileStorageDriverFactory.getCurrentDriver).toHaveBeenCalled();
expect(mockDriver.read).toHaveBeenCalledWith(readParams);
expect(result).toBe(mockStream);
});
it('should handle read errors', async () => {
const readParams = {
folderPath: 'documents',
filename: 'test.txt',
};
const error = new Error('Read failed');
mockDriver.read.mockRejectedValue(error);
await expect(service.read(readParams)).rejects.toThrow('Read failed');
expect(fileStorageDriverFactory.getCurrentDriver).toHaveBeenCalled();
expect(mockDriver.read).toHaveBeenCalledWith(readParams);
});
});
describe('delete', () => {
it('should delegate to the current driver with filename', async () => {
const deleteParams = {
folderPath: 'documents',
filename: 'test.txt',
};
mockDriver.delete.mockResolvedValue(undefined);
await service.delete(deleteParams);
expect(fileStorageDriverFactory.getCurrentDriver).toHaveBeenCalled();
expect(mockDriver.delete).toHaveBeenCalledWith(deleteParams);
});
it('should delegate to the current driver without filename (delete folder)', async () => {
const deleteParams = {
folderPath: 'documents',
};
mockDriver.delete.mockResolvedValue(undefined);
await service.delete(deleteParams);
expect(fileStorageDriverFactory.getCurrentDriver).toHaveBeenCalled();
expect(mockDriver.delete).toHaveBeenCalledWith(deleteParams);
});
it('should handle delete errors', async () => {
const deleteParams = {
folderPath: 'documents',
filename: 'test.txt',
};
const error = new Error('Delete failed');
mockDriver.delete.mockRejectedValue(error);
await expect(service.delete(deleteParams)).rejects.toThrow(
'Delete failed',
);
expect(fileStorageDriverFactory.getCurrentDriver).toHaveBeenCalled();
expect(mockDriver.delete).toHaveBeenCalledWith(deleteParams);
});
});
describe('move', () => {
it('should delegate to the current driver', async () => {
const moveParams = {
from: { folderPath: 'documents', filename: 'test.txt' },
to: { folderPath: 'archive', filename: 'archived-test.txt' },
};
mockDriver.move.mockResolvedValue(undefined);
await service.move(moveParams);
expect(fileStorageDriverFactory.getCurrentDriver).toHaveBeenCalled();
expect(mockDriver.move).toHaveBeenCalledWith(moveParams);
});
it('should handle move errors', async () => {
const moveParams = {
from: { folderPath: 'documents', filename: 'test.txt' },
to: { folderPath: 'archive', filename: 'archived-test.txt' },
};
const error = new Error('Move failed');
mockDriver.move.mockRejectedValue(error);
await expect(service.move(moveParams)).rejects.toThrow('Move failed');
expect(fileStorageDriverFactory.getCurrentDriver).toHaveBeenCalled();
expect(mockDriver.move).toHaveBeenCalledWith(moveParams);
});
});
describe('copy', () => {
it('should delegate to the current driver', async () => {
const copyParams = {
from: { folderPath: 'documents', filename: 'test.txt' },
to: { folderPath: 'backup', filename: 'test-backup.txt' },
};
mockDriver.copy.mockResolvedValue(undefined);
await service.copy(copyParams);
expect(fileStorageDriverFactory.getCurrentDriver).toHaveBeenCalled();
expect(mockDriver.copy).toHaveBeenCalledWith(copyParams);
});
it('should handle copy errors', async () => {
const copyParams = {
from: { folderPath: 'documents', filename: 'test.txt' },
to: { folderPath: 'backup', filename: 'test-backup.txt' },
};
const error = new Error('Copy failed');
mockDriver.copy.mockRejectedValue(error);
await expect(service.copy(copyParams)).rejects.toThrow('Copy failed');
expect(fileStorageDriverFactory.getCurrentDriver).toHaveBeenCalled();
expect(mockDriver.copy).toHaveBeenCalledWith(copyParams);
});
});
describe('download', () => {
it('should delegate to the current driver', async () => {
const downloadParams = {
from: { folderPath: 'documents', filename: 'test.txt' },
to: { folderPath: '/tmp', filename: 'downloaded-test.txt' },
};
mockDriver.download.mockResolvedValue(undefined);
await service.download(downloadParams);
expect(fileStorageDriverFactory.getCurrentDriver).toHaveBeenCalled();
expect(mockDriver.download).toHaveBeenCalledWith(downloadParams);
});
it('should handle download errors', async () => {
const downloadParams = {
from: { folderPath: 'documents', filename: 'test.txt' },
to: { folderPath: '/tmp', filename: 'downloaded-test.txt' },
};
const error = new Error('Download failed');
mockDriver.download.mockRejectedValue(error);
await expect(service.download(downloadParams)).rejects.toThrow(
'Download failed',
);
expect(fileStorageDriverFactory.getCurrentDriver).toHaveBeenCalled();
expect(mockDriver.download).toHaveBeenCalledWith(downloadParams);
});
});
describe('checkFileExists', () => {
it('should delegate to the current driver and return true', async () => {
const checkParams = {
folderPath: 'documents',
filename: 'test.txt',
};
mockDriver.checkFileExists.mockResolvedValue(true);
const result = await service.checkFileExists(checkParams);
expect(fileStorageDriverFactory.getCurrentDriver).toHaveBeenCalled();
expect(mockDriver.checkFileExists).toHaveBeenCalledWith(checkParams);
expect(result).toBe(true);
});
it('should delegate to the current driver and return false', async () => {
const checkParams = {
folderPath: 'documents',
filename: 'nonexistent.txt',
};
mockDriver.checkFileExists.mockResolvedValue(false);
const result = await service.checkFileExists(checkParams);
expect(fileStorageDriverFactory.getCurrentDriver).toHaveBeenCalled();
expect(mockDriver.checkFileExists).toHaveBeenCalledWith(checkParams);
expect(result).toBe(false);
});
it('should handle checkFileExists errors', async () => {
const checkParams = {
folderPath: 'documents',
filename: 'test.txt',
};
const error = new Error('Check failed');
mockDriver.checkFileExists.mockRejectedValue(error);
await expect(service.checkFileExists(checkParams)).rejects.toThrow(
'Check failed',
);
expect(fileStorageDriverFactory.getCurrentDriver).toHaveBeenCalled();
expect(mockDriver.checkFileExists).toHaveBeenCalledWith(checkParams);
});
});
});
});

View File

@ -0,0 +1,79 @@
import { Injectable } from '@nestjs/common';
import { fromNodeProviderChain } from '@aws-sdk/credential-providers';
import { StorageDriver } from 'src/engine/core-modules/file-storage/drivers/interfaces/storage-driver.interface';
import { StorageDriverType } from 'src/engine/core-modules/file-storage/interfaces/file-storage.interface';
import { LocalDriver } from 'src/engine/core-modules/file-storage/drivers/local.driver';
import { S3Driver } from 'src/engine/core-modules/file-storage/drivers/s3.driver';
import { DriverFactoryBase } from 'src/engine/core-modules/twenty-config/dynamic-factory.base';
import { ConfigVariablesGroup } from 'src/engine/core-modules/twenty-config/enums/config-variables-group.enum';
import { TwentyConfigService } from 'src/engine/core-modules/twenty-config/twenty-config.service';
import { resolveAbsolutePath } from 'src/utils/resolve-absolute-path';
@Injectable()
export class FileStorageDriverFactory extends DriverFactoryBase<StorageDriver> {
constructor(twentyConfigService: TwentyConfigService) {
super(twentyConfigService);
}
protected buildConfigKey(): string {
const storageType = this.twentyConfigService.get('STORAGE_TYPE');
if (storageType === StorageDriverType.Local) {
const storagePath = this.twentyConfigService.get('STORAGE_LOCAL_PATH');
return `local|${storagePath}`;
}
if (storageType === StorageDriverType.S3) {
const storageConfigHash = this.getConfigGroupHash(
ConfigVariablesGroup.StorageConfig,
);
return `s3|${storageConfigHash}`;
}
throw new Error(`Unsupported storage type: ${storageType}`);
}
protected createDriver(): StorageDriver {
const storageType = this.twentyConfigService.get('STORAGE_TYPE');
switch (storageType) {
case StorageDriverType.Local: {
const storagePath = this.twentyConfigService.get('STORAGE_LOCAL_PATH');
return new LocalDriver({
storagePath: resolveAbsolutePath(storagePath),
});
}
case StorageDriverType.S3: {
const bucketName = this.twentyConfigService.get('STORAGE_S3_NAME');
const endpoint = this.twentyConfigService.get('STORAGE_S3_ENDPOINT');
const region = this.twentyConfigService.get('STORAGE_S3_REGION');
const accessKeyId = this.twentyConfigService.get(
'STORAGE_S3_ACCESS_KEY_ID',
);
const secretAccessKey = this.twentyConfigService.get(
'STORAGE_S3_SECRET_ACCESS_KEY',
);
return new S3Driver({
bucketName: bucketName ?? '',
endpoint: endpoint,
credentials: accessKeyId
? { accessKeyId, secretAccessKey }
: fromNodeProviderChain({ clientConfig: { region } }),
forcePathStyle: true,
region: region ?? '',
});
}
default:
throw new Error(`Invalid storage driver type: ${storageType}`);
}
}
}

View File

@ -1 +0,0 @@
export const STORAGE_DRIVER = Symbol('STORAGE_DRIVER');

View File

@ -1,14 +0,0 @@
import { ConfigurableModuleBuilder } from '@nestjs/common';
import { FileStorageModuleOptions } from 'src/engine/core-modules/file-storage/interfaces';
export const {
ConfigurableModuleClass,
MODULE_OPTIONS_TOKEN,
OPTIONS_TYPE,
ASYNC_OPTIONS_TYPE,
} = new ConfigurableModuleBuilder<FileStorageModuleOptions>({
moduleName: 'FileStorage',
})
.setClassMethodName('forRoot')
.build();

View File

@ -1,63 +0,0 @@
import { fromNodeProviderChain } from '@aws-sdk/credential-providers';
import {
FileStorageModuleOptions,
StorageDriverType,
} from 'src/engine/core-modules/file-storage/interfaces';
import { TwentyConfigService } from 'src/engine/core-modules/twenty-config/twenty-config.service';
import { resolveAbsolutePath } from 'src/utils/resolve-absolute-path';
/**
* FileStorage Module factory
* @returns FileStorageModuleOptions
* @param twentyConfigService
*/
export const fileStorageModuleFactory = async (
twentyConfigService: TwentyConfigService,
): Promise<FileStorageModuleOptions> => {
const driverType = twentyConfigService.get('STORAGE_TYPE');
switch (driverType) {
case StorageDriverType.Local: {
const storagePath = twentyConfigService.get('STORAGE_LOCAL_PATH');
return {
type: StorageDriverType.Local,
options: {
storagePath: resolveAbsolutePath(storagePath),
},
};
}
case StorageDriverType.S3: {
const bucketName = twentyConfigService.get('STORAGE_S3_NAME');
const endpoint = twentyConfigService.get('STORAGE_S3_ENDPOINT');
const region = twentyConfigService.get('STORAGE_S3_REGION');
const accessKeyId = twentyConfigService.get('STORAGE_S3_ACCESS_KEY_ID');
const secretAccessKey = twentyConfigService.get(
'STORAGE_S3_SECRET_ACCESS_KEY',
);
return {
type: StorageDriverType.S3,
options: {
bucketName: bucketName ?? '',
endpoint: endpoint,
credentials: accessKeyId
? {
accessKeyId,
secretAccessKey,
}
: fromNodeProviderChain({
clientConfig: { region },
}),
forcePathStyle: true,
region: region ?? '',
},
};
}
default:
throw new Error(
`Invalid storage driver type (${driverType}), check your .env file`,
);
}
};

View File

@ -1,50 +1,16 @@
import { DynamicModule, Global } from '@nestjs/common';
import { FileStorageDriverFactory } from 'src/engine/core-modules/file-storage/file-storage-driver.factory';
import { FileStorageService } from 'src/engine/core-modules/file-storage/file-storage.service';
import {
FileStorageModuleAsyncOptions,
FileStorageModuleOptions,
} from 'src/engine/core-modules/file-storage/interfaces';
import { STORAGE_DRIVER } from 'src/engine/core-modules/file-storage/file-storage.constants';
import { LocalDriver } from 'src/engine/core-modules/file-storage/drivers/local.driver';
import { S3Driver } from 'src/engine/core-modules/file-storage/drivers/s3.driver';
import { TwentyConfigModule } from 'src/engine/core-modules/twenty-config/twenty-config.module';
@Global()
export class FileStorageModule {
static forRoot(options: FileStorageModuleOptions): DynamicModule {
const provider = {
provide: STORAGE_DRIVER,
useValue:
options.type === 's3'
? new S3Driver(options.options)
: new LocalDriver(options.options),
};
static forRoot(): DynamicModule {
return {
module: FileStorageModule,
providers: [FileStorageService, provider],
exports: [FileStorageService],
};
}
static forRootAsync(options: FileStorageModuleAsyncOptions): DynamicModule {
const provider = {
provide: STORAGE_DRIVER,
// eslint-disable-next-line @typescript-eslint/no-explicit-any
useFactory: async (...args: any[]) => {
const config = await options.useFactory(...args);
return config?.type === 's3'
? new S3Driver(config.options)
: new LocalDriver(config.options);
},
inject: options.inject || [],
};
return {
module: FileStorageModule,
imports: options.imports || [],
providers: [FileStorageService, provider],
imports: [TwentyConfigModule],
providers: [FileStorageDriverFactory, FileStorageService],
exports: [FileStorageService],
};
}

View File

@ -1,18 +1,16 @@
import { Inject, Injectable } from '@nestjs/common';
import { Injectable } from '@nestjs/common';
import { Readable } from 'stream';
import { StorageDriver } from 'src/engine/core-modules/file-storage/drivers/interfaces/storage-driver.interface';
import { STORAGE_DRIVER } from 'src/engine/core-modules/file-storage/file-storage.constants';
import { FileStorageDriverFactory } from 'src/engine/core-modules/file-storage/file-storage-driver.factory';
@Injectable()
export class FileStorageService implements StorageDriver {
constructor(@Inject(STORAGE_DRIVER) private driver: StorageDriver) {}
delete(params: { folderPath: string; filename?: string }): Promise<void> {
return this.driver.delete(params);
}
constructor(
private readonly fileStorageDriverFactory: FileStorageDriverFactory,
) {}
write(params: {
file: string | Buffer | Uint8Array;
@ -20,38 +18,56 @@ export class FileStorageService implements StorageDriver {
folder: string;
mimeType: string | undefined;
}): Promise<void> {
return this.driver.write(params);
const driver = this.fileStorageDriverFactory.getCurrentDriver();
return driver.write(params);
}
read(params: { folderPath: string; filename: string }): Promise<Readable> {
return this.driver.read(params);
const driver = this.fileStorageDriverFactory.getCurrentDriver();
return driver.read(params);
}
delete(params: { folderPath: string; filename?: string }): Promise<void> {
const driver = this.fileStorageDriverFactory.getCurrentDriver();
return driver.delete(params);
}
move(params: {
from: { folderPath: string; filename: string };
to: { folderPath: string; filename: string };
}): Promise<void> {
return this.driver.move(params);
const driver = this.fileStorageDriverFactory.getCurrentDriver();
return driver.move(params);
}
copy(params: {
from: { folderPath: string; filename?: string };
to: { folderPath: string; filename?: string };
}): Promise<void> {
return this.driver.copy(params);
const driver = this.fileStorageDriverFactory.getCurrentDriver();
return driver.copy(params);
}
download(params: {
from: { folderPath: string; filename?: string };
to: { folderPath: string; filename?: string };
}): Promise<void> {
return this.driver.download(params);
const driver = this.fileStorageDriverFactory.getCurrentDriver();
return driver.download(params);
}
checkFileExists(params: {
folderPath: string;
filename: string;
}): Promise<boolean> {
return this.driver.checkFileExists(params);
const driver = this.fileStorageDriverFactory.getCurrentDriver();
return driver.checkFileExists(params);
}
}

View File

@ -1,31 +1,4 @@
import { FactoryProvider, ModuleMetadata } from '@nestjs/common';
import { S3DriverOptions } from 'src/engine/core-modules/file-storage/drivers/s3.driver';
import { LocalDriverOptions } from 'src/engine/core-modules/file-storage/drivers/local.driver';
export enum StorageDriverType {
S3 = 's3',
Local = 'local',
}
export interface S3DriverFactoryOptions {
type: StorageDriverType.S3;
options: S3DriverOptions;
}
export interface LocalDriverFactoryOptions {
type: StorageDriverType.Local;
options: LocalDriverOptions;
}
export type FileStorageModuleOptions =
| S3DriverFactoryOptions
| LocalDriverFactoryOptions;
export type FileStorageModuleAsyncOptions = {
useFactory: (
// eslint-disable-next-line @typescript-eslint/no-explicit-any
...args: any[]
) => FileStorageModuleOptions | Promise<FileStorageModuleOptions>;
} & Pick<ModuleMetadata, 'imports'> &
Pick<FactoryProvider, 'inject'>;

View File

@ -44,15 +44,15 @@ describe('ConfigCacheService', () => {
it('should handle different value types', () => {
const booleanKey = 'AUTH_PASSWORD_ENABLED' as keyof ConfigVariables;
const stringKey = 'EMAIL_FROM_ADDRESS' as keyof ConfigVariables;
const numberKey = 'NODE_PORT' as keyof ConfigVariables;
const numberKey = 'CACHE_STORAGE_TTL' as keyof ConfigVariables;
service.set(booleanKey, true);
service.set(stringKey, 'test@example.com');
service.set(numberKey, 3000);
service.set(numberKey, 3600 * 24 * 7);
expect(service.get(booleanKey)).toBe(true);
expect(service.get(stringKey)).toBe('test@example.com');
expect(service.get(numberKey)).toBe(3000);
expect(service.get(numberKey)).toBe(3600 * 24 * 7);
});
});
@ -105,7 +105,7 @@ describe('ConfigCacheService', () => {
it('should return correct cache information', () => {
const key1 = 'AUTH_PASSWORD_ENABLED' as keyof ConfigVariables;
const key2 = 'EMAIL_FROM_ADDRESS' as keyof ConfigVariables;
const key3 = 'NODE_PORT' as keyof ConfigVariables;
const key3 = 'CACHE_STORAGE_TTL' as keyof ConfigVariables;
service.set(key1, true);
service.set(key2, 'test@example.com');

View File

@ -11,7 +11,6 @@ import {
} from 'class-validator';
import { isDefined } from 'twenty-shared/utils';
import { EmailDriver } from 'src/engine/core-modules/email/interfaces/email.interface';
import { LLMChatModelDriver } from 'src/engine/core-modules/llm-chat-model/interfaces/llm-chat-model.interface';
import { LLMTracingDriver } from 'src/engine/core-modules/llm-tracing/interfaces/llm-tracing.interface';
import { AwsRegion } from 'src/engine/core-modules/twenty-config/interfaces/aws-region.interface';
@ -19,6 +18,7 @@ import { NodeEnvironment } from 'src/engine/core-modules/twenty-config/interface
import { SupportDriver } from 'src/engine/core-modules/twenty-config/interfaces/support.interface';
import { CaptchaDriverType } from 'src/engine/core-modules/captcha/interfaces';
import { EmailDriver } from 'src/engine/core-modules/email/enums/email-driver.enum';
import { ExceptionHandlerDriver } from 'src/engine/core-modules/exception-handler/interfaces';
import { StorageDriverType } from 'src/engine/core-modules/file-storage/interfaces';
import { LoggerDriverType } from 'src/engine/core-modules/logger/interfaces';
@ -209,6 +209,7 @@ export class ConfigVariables {
description:
'Legacy variable to be deprecated when all API Keys expire. Replaced by APP_KEY',
type: ConfigVariableType.STRING,
isEnvOnly: true,
})
@IsOptional()
ACCESS_TOKEN_SECRET: string;
@ -411,6 +412,7 @@ export class ConfigVariables {
description: 'Type of serverless execution (local or Lambda)',
type: ConfigVariableType.ENUM,
options: Object.values(ServerlessDriverType),
isEnvOnly: true,
})
@IsOptional()
SERVERLESS_TYPE: ServerlessDriverType = ServerlessDriverType.Local;
@ -593,6 +595,7 @@ export class ConfigVariables {
group: ConfigVariablesGroup.ServerConfig,
description: 'Url for the frontend application',
type: ConfigVariableType.STRING,
isEnvOnly: true,
})
@IsUrl({ require_tld: false, require_protocol: true })
@IsOptional()
@ -628,6 +631,7 @@ export class ConfigVariables {
description: 'Driver used for handling exceptions (Console or Sentry)',
type: ConfigVariableType.ENUM,
options: Object.values(ExceptionHandlerDriver),
isEnvOnly: true,
})
@IsOptional()
EXCEPTION_HANDLER_DRIVER: ExceptionHandlerDriver =
@ -637,7 +641,8 @@ export class ConfigVariables {
group: ConfigVariablesGroup.Logging,
description: 'Levels of logging to be captured',
type: ConfigVariableType.ARRAY,
options: ['log', 'error', 'warn', 'debug', 'verbose'],
options: ['log', 'error', 'warn', 'debug'],
isEnvOnly: true,
})
@CastToLogLevelArray()
@IsOptional()
@ -648,6 +653,7 @@ export class ConfigVariables {
description: 'Driver used for collect metrics (OpenTelemetry or Console)',
type: ConfigVariableType.ARRAY,
options: ['OpenTelemetry', 'Console'],
isEnvOnly: true,
})
@CastToMeterDriverArray()
@IsOptional()
@ -657,6 +663,7 @@ export class ConfigVariables {
group: ConfigVariablesGroup.Metering,
description: 'Endpoint URL for the OpenTelemetry collector',
type: ConfigVariableType.STRING,
isEnvOnly: true,
})
@IsOptional()
OTLP_COLLECTOR_ENDPOINT_URL: string;
@ -666,6 +673,7 @@ export class ConfigVariables {
description: 'Driver used for logging (only console for now)',
type: ConfigVariableType.ENUM,
options: Object.values(LoggerDriverType),
isEnvOnly: true,
})
@IsOptional()
LOGGER_DRIVER: LoggerDriverType = LoggerDriverType.Console;
@ -830,6 +838,7 @@ export class ConfigVariables {
description: 'Node environment (development, production, etc.)',
type: ConfigVariableType.ENUM,
options: Object.values(NodeEnvironment),
isEnvOnly: true,
})
NODE_ENV: NodeEnvironment = NodeEnvironment.production;
@ -837,6 +846,7 @@ export class ConfigVariables {
group: ConfigVariablesGroup.ServerConfig,
description: 'Port for the node server',
type: ConfigVariableType.NUMBER,
isEnvOnly: true,
})
@CastToPositiveNumber()
@IsOptional()
@ -846,6 +856,7 @@ export class ConfigVariables {
group: ConfigVariablesGroup.ServerConfig,
description: 'Base URL for the server',
type: ConfigVariableType.STRING,
isEnvOnly: true,
})
@IsUrl({ require_tld: false, require_protocol: true })
@IsOptional()
@ -890,6 +901,7 @@ export class ConfigVariables {
group: ConfigVariablesGroup.SSL,
description: 'Path to the SSL key for enabling HTTPS in local development',
type: ConfigVariableType.STRING,
isEnvOnly: true,
})
@IsOptional()
SSL_KEY_PATH: string;
@ -899,6 +911,7 @@ export class ConfigVariables {
description:
'Path to the SSL certificate for enabling HTTPS in local development',
type: ConfigVariableType.STRING,
isEnvOnly: true,
})
@IsOptional()
SSL_CERT_PATH: string;
@ -934,6 +947,7 @@ export class ConfigVariables {
description: 'Driver for the LLM chat model',
type: ConfigVariableType.ENUM,
options: Object.values(LLMChatModelDriver),
isEnvOnly: true,
})
LLM_CHAT_MODEL_DRIVER: LLMChatModelDriver;
@ -965,6 +979,7 @@ export class ConfigVariables {
description: 'Driver for LLM tracing',
type: ConfigVariableType.ENUM,
options: Object.values(LLMTracingDriver),
isEnvOnly: true,
})
LLM_TRACING_DRIVER: LLMTracingDriver = LLMTracingDriver.Console;
@ -972,6 +987,7 @@ export class ConfigVariables {
group: ConfigVariablesGroup.ServerConfig,
description: 'Enable or disable multi-workspace support',
type: ConfigVariableType.BOOLEAN,
isEnvOnly: true,
})
@IsOptional()
IS_MULTIWORKSPACE_ENABLED = false;
@ -1040,6 +1056,7 @@ export class ConfigVariables {
description: 'Driver for captcha integration',
type: ConfigVariableType.ENUM,
options: Object.values(CaptchaDriverType),
isEnvOnly: true,
})
@IsOptional()
CAPTCHA_DRIVER?: CaptchaDriverType;

View File

@ -78,7 +78,7 @@ describe('ConfigValueConverterService', () => {
});
const mockConfigVariables = {
NODE_PORT: 3000,
CACHE_STORAGE_TTL: 3600 * 24 * 7,
};
const module: TestingModule = await Test.createTestingModule({
@ -122,25 +122,25 @@ describe('ConfigValueConverterService', () => {
it('should use number transformer for number type', () => {
jest.spyOn(TypedReflect, 'getMetadata').mockReturnValueOnce({
NODE_PORT: {
CACHE_STORAGE_TTL: {
type: ConfigVariableType.NUMBER,
group: ConfigVariablesGroup.ServerConfig,
description: 'Port for the node server',
description: 'Time-to-live for cache storage in seconds',
},
});
typeTransformers.number.toApp.mockReturnValueOnce(3000);
typeTransformers.number.toApp.mockReturnValueOnce(3600 * 24 * 7);
const result = service.convertDbValueToAppValue(
'3000',
'NODE_PORT' as keyof ConfigVariables,
'604800',
'CACHE_STORAGE_TTL' as keyof ConfigVariables,
);
expect(typeTransformers.number.toApp).toHaveBeenCalledWith(
'3000',
'604800',
undefined,
);
expect(result).toBe(3000);
expect(result).toBe(3600 * 24 * 7);
});
it('should use string transformer for string type', () => {
@ -222,28 +222,28 @@ describe('ConfigValueConverterService', () => {
it('should infer type from default value when no metadata is available', () => {
jest.spyOn(TypedReflect, 'getMetadata').mockReturnValueOnce(undefined);
typeTransformers.number.toApp.mockReturnValueOnce(3000);
typeTransformers.number.toApp.mockReturnValueOnce(3600 * 24 * 7);
const result = service.convertDbValueToAppValue(
'3000',
'NODE_PORT' as keyof ConfigVariables,
'604800',
'CACHE_STORAGE_TTL' as keyof ConfigVariables,
);
expect(typeTransformers.number.toApp).toHaveBeenCalledWith(
'3000',
'604800',
undefined,
);
expect(result).toBe(3000);
expect(result).toBe(3600 * 24 * 7);
});
it('should return undefined for null or undefined input', () => {
const result1 = service.convertDbValueToAppValue(
null,
'NODE_PORT' as keyof ConfigVariables,
'CACHE_STORAGE_TTL' as keyof ConfigVariables,
);
const result2 = service.convertDbValueToAppValue(
undefined,
'NODE_PORT' as keyof ConfigVariables,
'CACHE_STORAGE_TTL' as keyof ConfigVariables,
);
expect(result1).toBeUndefined();
@ -252,10 +252,10 @@ describe('ConfigValueConverterService', () => {
it('should propagate errors from transformers with context', () => {
jest.spyOn(TypedReflect, 'getMetadata').mockReturnValueOnce({
NODE_PORT: {
CACHE_STORAGE_TTL: {
type: ConfigVariableType.NUMBER,
group: ConfigVariablesGroup.ServerConfig,
description: 'Port for the node server',
description: 'Time-to-live for cache storage in seconds',
},
});
@ -268,9 +268,11 @@ describe('ConfigValueConverterService', () => {
expect(() => {
service.convertDbValueToAppValue(
'not-a-number',
'NODE_PORT' as keyof ConfigVariables,
'CACHE_STORAGE_TTL' as keyof ConfigVariables,
);
}).toThrow(
`Failed to convert CACHE_STORAGE_TTL to app value: Test error`,
);
}).toThrow(`Failed to convert NODE_PORT to app value: Test error`);
});
});
@ -300,25 +302,25 @@ describe('ConfigValueConverterService', () => {
it('should use number transformer for number type', () => {
jest.spyOn(TypedReflect, 'getMetadata').mockReturnValueOnce({
NODE_PORT: {
CACHE_STORAGE_TTL: {
type: ConfigVariableType.NUMBER,
group: ConfigVariablesGroup.ServerConfig,
description: 'Port for the node server',
description: 'Time-to-live for cache storage in seconds',
},
});
typeTransformers.number.toStorage.mockReturnValueOnce(3000);
typeTransformers.number.toStorage.mockReturnValueOnce(3600 * 24 * 7);
const result = service.convertAppValueToDbValue(
3000,
'NODE_PORT' as keyof ConfigVariables,
3600 * 24 * 7,
'CACHE_STORAGE_TTL' as keyof ConfigVariables,
);
expect(typeTransformers.number.toStorage).toHaveBeenCalledWith(
3000,
3600 * 24 * 7,
undefined,
);
expect(result).toBe(3000);
expect(result).toBe(3600 * 24 * 7);
});
it('should use string transformer for string type', () => {
@ -400,28 +402,28 @@ describe('ConfigValueConverterService', () => {
it('should infer type from default value when no metadata is available', () => {
jest.spyOn(TypedReflect, 'getMetadata').mockReturnValueOnce(undefined);
typeTransformers.number.toStorage.mockReturnValueOnce(3000);
typeTransformers.number.toStorage.mockReturnValueOnce(3600 * 24 * 7);
const result = service.convertAppValueToDbValue(
3000,
'NODE_PORT' as keyof ConfigVariables,
3600 * 24 * 7,
'CACHE_STORAGE_TTL' as keyof ConfigVariables,
);
expect(typeTransformers.number.toStorage).toHaveBeenCalledWith(
3000,
3600 * 24 * 7,
undefined,
);
expect(result).toBe(3000);
expect(result).toBe(3600 * 24 * 7);
});
it('should return null for null or undefined input', () => {
const result1 = service.convertAppValueToDbValue(
null,
'NODE_PORT' as keyof ConfigVariables,
'CACHE_STORAGE_TTL' as keyof ConfigVariables,
);
const result2 = service.convertAppValueToDbValue(
undefined,
'NODE_PORT' as keyof ConfigVariables,
'CACHE_STORAGE_TTL' as keyof ConfigVariables,
);
expect(result1).toBeNull();
@ -449,10 +451,10 @@ describe('ConfigValueConverterService', () => {
it('should propagate errors from transformers with context', () => {
jest.spyOn(TypedReflect, 'getMetadata').mockReturnValueOnce({
NODE_PORT: {
CACHE_STORAGE_TTL: {
type: ConfigVariableType.NUMBER,
group: ConfigVariablesGroup.ServerConfig,
description: 'Port for the node server',
description: 'Time-to-live for cache storage in seconds',
},
});
@ -465,9 +467,9 @@ describe('ConfigValueConverterService', () => {
expect(() => {
service.convertAppValueToDbValue(
'not-a-number' as any,
'NODE_PORT' as keyof ConfigVariables,
'CACHE_STORAGE_TTL' as keyof ConfigVariables,
);
}).toThrow(`Failed to convert NODE_PORT to DB value: Test error`);
}).toThrow(`Failed to convert CACHE_STORAGE_TTL to DB value: Test error`);
});
});
@ -497,10 +499,10 @@ describe('ConfigValueConverterService', () => {
it('should validate number values when converting to storage', () => {
jest.spyOn(TypedReflect, 'getMetadata').mockReturnValueOnce({
NODE_PORT: {
CACHE_STORAGE_TTL: {
type: ConfigVariableType.NUMBER,
group: ConfigVariablesGroup.ServerConfig,
description: 'Port for the node server',
description: 'Time-to-live for cache storage in seconds',
},
});
@ -510,11 +512,11 @@ describe('ConfigValueConverterService', () => {
expect(() => {
service.convertAppValueToDbValue(
'invalid-port' as any,
'NODE_PORT' as keyof ConfigVariables,
'invalid-ttl' as any,
'CACHE_STORAGE_TTL' as keyof ConfigVariables,
);
}).toThrow(
'Failed to convert NODE_PORT to DB value: Expected number, got string',
'Failed to convert CACHE_STORAGE_TTL to DB value: Expected number, got string',
);
});

View File

@ -0,0 +1,73 @@
import { createHash } from 'crypto';
import { ConfigVariables } from 'src/engine/core-modules/twenty-config/config-variables';
import { ConfigVariablesGroup } from 'src/engine/core-modules/twenty-config/enums/config-variables-group.enum';
import { TwentyConfigService } from 'src/engine/core-modules/twenty-config/twenty-config.service';
import { TypedReflect } from 'src/utils/typed-reflect';
export abstract class DriverFactoryBase<TDriver> {
private currentDriver: TDriver | null = null;
private currentConfigKey: string | null = null;
constructor(protected readonly twentyConfigService: TwentyConfigService) {}
getCurrentDriver(): TDriver {
let configKey: string;
try {
configKey = this.buildConfigKey();
} catch (error) {
throw new Error(
`Failed to build config key for ${this.constructor.name}. Original error: ${error instanceof Error ? error.message : String(error)}`,
);
}
if (this.currentConfigKey !== configKey) {
try {
this.currentDriver = this.createDriver();
} catch (error) {
throw new Error(
`Failed to create driver for ${this.constructor.name} with config key: ${configKey}. Original error: ${error instanceof Error ? error.message : String(error)}`,
);
}
this.currentConfigKey = configKey;
}
if (!this.currentDriver) {
throw new Error(
`Failed to create driver for ${this.constructor.name} with config key: ${configKey}`,
);
}
return this.currentDriver;
}
protected getConfigGroupHash(group: ConfigVariablesGroup): string {
const groupVariables = this.getConfigVariablesByGroup(group);
const configValues = groupVariables
.map((key) => `${key}=${this.twentyConfigService.get(key)}`)
.sort()
.join('|');
return createHash('sha256')
.update(configValues)
.digest('hex')
.substring(0, 16);
}
private getConfigVariablesByGroup(
group: ConfigVariablesGroup,
): Array<keyof ConfigVariables> {
const metadata =
TypedReflect.getMetadata('config-variables', ConfigVariables) ?? {};
return Object.keys(metadata)
.filter((key) => metadata[key]?.group === group)
.map((key) => key as keyof ConfigVariables);
}
protected abstract buildConfigKey(): string;
protected abstract createDriver(): TDriver;
}

View File

@ -14,6 +14,10 @@ import { EnvironmentConfigDriver } from 'src/engine/core-modules/twenty-config/d
import { ConfigVariableType } from 'src/engine/core-modules/twenty-config/enums/config-variable-type.enum';
import { ConfigVariablesGroup } from 'src/engine/core-modules/twenty-config/enums/config-variables-group.enum';
import { ConfigStorageService } from 'src/engine/core-modules/twenty-config/storage/config-storage.service';
import {
ConfigVariableException,
ConfigVariableExceptionCode,
} from 'src/engine/core-modules/twenty-config/twenty-config.exception';
import { User } from 'src/engine/core-modules/user/user.entity';
import { Workspace } from 'src/engine/core-modules/workspace/workspace.entity';
import { TypedReflect } from 'src/utils/typed-reflect';
@ -154,7 +158,10 @@ describe('ConfigStorageService', () => {
throw error;
});
await expect(service.get(key)).rejects.toThrow('Conversion error');
await expect(service.get(key)).rejects.toThrow(ConfigVariableException);
await expect(service.get(key)).rejects.toMatchObject({
code: ConfigVariableExceptionCode.VALIDATION_FAILED,
});
});
it('should decrypt sensitive string values', async () => {
@ -250,93 +257,26 @@ describe('ConfigStorageService', () => {
const result = await service.get(key);
expect(result).toBe(encryptedValue); // Should fall back to encrypted value
expect(environmentConfigDriver.get).toHaveBeenCalledWith('APP_SECRET');
});
it('should skip decryption for non-string sensitive values', async () => {
const key = 'SENSITIVE_CONFIG' as keyof ConfigVariables;
const value = { someKey: 'someValue' };
const mockRecord = createMockKeyValuePair(
key as string,
JSON.stringify(value),
expect(result).toBe(encryptedValue);
expect(authUtils.decryptText).toHaveBeenCalledWith(
encryptedValue,
'test-secret',
);
jest
.spyOn(keyValuePairRepository, 'findOne')
.mockResolvedValue(mockRecord);
jest.spyOn(TypedReflect, 'getMetadata').mockReturnValue({
[key]: {
isSensitive: true,
type: ConfigVariableType.ARRAY,
group: ConfigVariablesGroup.ServerConfig,
description: 'Test sensitive config',
},
});
(
configValueConverter.convertDbValueToAppValue as jest.Mock
).mockReturnValue(value);
it('should handle findOne errors', async () => {
const key = 'AUTH_PASSWORD_ENABLED' as keyof ConfigVariables;
const error = new Error('Database error');
const result = await service.get(key);
jest.spyOn(keyValuePairRepository, 'findOne').mockRejectedValue(error);
expect(result).toBe(value);
expect(authUtils.decryptText).not.toHaveBeenCalled();
});
it('should handle decryption failure in loadAll() by skipping failed values', async () => {
const configVars: KeyValuePair[] = [
createMockKeyValuePair('SENSITIVE_CONFIG_1', 'encrypted:value1'),
createMockKeyValuePair('SENSITIVE_CONFIG_2', 'encrypted:value2'),
createMockKeyValuePair('NORMAL_CONFIG', 'normal-value'),
];
jest.spyOn(keyValuePairRepository, 'find').mockResolvedValue(configVars);
jest.spyOn(TypedReflect, 'getMetadata').mockReturnValue({
SENSITIVE_CONFIG_1: {
isSensitive: true,
type: ConfigVariableType.STRING,
group: ConfigVariablesGroup.ServerConfig,
description: 'Test sensitive config 1',
},
SENSITIVE_CONFIG_2: {
isSensitive: true,
type: ConfigVariableType.STRING,
group: ConfigVariablesGroup.ServerConfig,
description: 'Test sensitive config 2',
},
NORMAL_CONFIG: {
type: ConfigVariableType.STRING,
group: ConfigVariablesGroup.ServerConfig,
description: 'Test normal config',
},
});
(
configValueConverter.convertDbValueToAppValue as jest.Mock
).mockImplementation((value) => value);
// Mock decryption to fail for the second sensitive value
(authUtils.decryptText as jest.Mock)
.mockImplementationOnce((text) => text.replace('encrypted:', ''))
.mockImplementationOnce(() => {
throw new Error('Decryption failed');
});
const result = await service.loadAll();
expect(result.size).toBe(3);
expect(result.get('SENSITIVE_CONFIG_1' as keyof ConfigVariables)).toBe(
'value1',
);
expect(result.get('SENSITIVE_CONFIG_2' as keyof ConfigVariables)).toBe(
'encrypted:value2',
); // Original encrypted value
expect(result.get('NORMAL_CONFIG' as keyof ConfigVariables)).toBe(
'normal-value',
await expect(service.get(key)).rejects.toThrow(ConfigVariableException);
await expect(service.get(key)).rejects.toThrow(
`Failed to retrieve config variable ${key}: Database error`,
);
await expect(service.get(key)).rejects.toMatchObject({
code: ConfigVariableExceptionCode.INTERNAL_ERROR,
});
});
});
@ -344,7 +284,7 @@ describe('ConfigStorageService', () => {
it('should update existing record', async () => {
const key = 'AUTH_PASSWORD_ENABLED' as keyof ConfigVariables;
const value = true;
const storedValue = 'true';
const convertedValue = 'true';
const mockRecord = createMockKeyValuePair(key as string, 'false');
@ -354,32 +294,32 @@ describe('ConfigStorageService', () => {
(
configValueConverter.convertAppValueToDbValue as jest.Mock
).mockReturnValue(storedValue);
).mockReturnValue(convertedValue);
await service.set(key, value);
expect(keyValuePairRepository.update).toHaveBeenCalledWith(
{ id: '1' },
{ value: storedValue },
{ id: mockRecord.id },
{ value: convertedValue },
);
});
it('should insert new record', async () => {
it('should insert new record when not found', async () => {
const key = 'AUTH_PASSWORD_ENABLED' as keyof ConfigVariables;
const value = true;
const storedValue = 'true';
const convertedValue = 'true';
jest.spyOn(keyValuePairRepository, 'findOne').mockResolvedValue(null);
(
configValueConverter.convertAppValueToDbValue as jest.Mock
).mockReturnValue(storedValue);
).mockReturnValue(convertedValue);
await service.set(key, value);
expect(keyValuePairRepository.insert).toHaveBeenCalledWith({
key: key as string,
value: storedValue,
value: convertedValue,
userId: null,
workspaceId: null,
type: KeyValuePairType.CONFIG_VARIABLE,
@ -388,8 +328,8 @@ describe('ConfigStorageService', () => {
it('should handle conversion errors', async () => {
const key = 'AUTH_PASSWORD_ENABLED' as keyof ConfigVariables;
const value = true;
const error = new Error('Conversion error');
const value = 'not-a-boolean';
const error = new Error('Invalid boolean value');
(
configValueConverter.convertAppValueToDbValue as jest.Mock
@ -397,7 +337,68 @@ describe('ConfigStorageService', () => {
throw error;
});
await expect(service.set(key, value)).rejects.toThrow('Conversion error');
await expect(
service.set(key, value as unknown as boolean),
).rejects.toThrow(ConfigVariableException);
await expect(
service.set(key, value as unknown as boolean),
).rejects.toMatchObject({
code: ConfigVariableExceptionCode.VALIDATION_FAILED,
});
});
it('should handle update errors', async () => {
const key = 'AUTH_PASSWORD_ENABLED' as keyof ConfigVariables;
const value = true;
const convertedValue = 'true';
const error = new Error('Update error');
const mockRecord = createMockKeyValuePair(key as string, 'false');
jest
.spyOn(keyValuePairRepository, 'findOne')
.mockResolvedValue(mockRecord);
(
configValueConverter.convertAppValueToDbValue as jest.Mock
).mockReturnValue(convertedValue);
jest.spyOn(keyValuePairRepository, 'update').mockRejectedValue(error);
await expect(service.set(key, value)).rejects.toThrow(
ConfigVariableException,
);
await expect(service.set(key, value)).rejects.toThrow(
`Failed to save config variable ${key}: Update error`,
);
await expect(service.set(key, value)).rejects.toMatchObject({
code: ConfigVariableExceptionCode.INTERNAL_ERROR,
});
});
it('should handle insert errors', async () => {
const key = 'AUTH_PASSWORD_ENABLED' as keyof ConfigVariables;
const value = true;
const convertedValue = 'true';
const error = new Error('Insert error');
jest.spyOn(keyValuePairRepository, 'findOne').mockResolvedValue(null);
(
configValueConverter.convertAppValueToDbValue as jest.Mock
).mockReturnValue(convertedValue);
jest.spyOn(keyValuePairRepository, 'insert').mockRejectedValue(error);
await expect(service.set(key, value)).rejects.toThrow(
ConfigVariableException,
);
await expect(service.set(key, value)).rejects.toThrow(
`Failed to save config variable ${key}: Insert error`,
);
await expect(service.set(key, value)).rejects.toMatchObject({
code: ConfigVariableExceptionCode.INTERNAL_ERROR,
});
});
it('should encrypt sensitive string values', async () => {
@ -407,6 +408,7 @@ describe('ConfigStorageService', () => {
const encryptedValue = 'encrypted:sensitive-value';
jest.spyOn(keyValuePairRepository, 'findOne').mockResolvedValue(null);
jest.spyOn(TypedReflect, 'getMetadata').mockReturnValue({
[key]: {
isSensitive: true,
@ -436,12 +438,13 @@ describe('ConfigStorageService', () => {
);
});
it('should handle encryption errors gracefully', async () => {
it('should handle encryption failure in set() by using unconverted value', async () => {
const key = 'SENSITIVE_CONFIG' as keyof ConfigVariables;
const value = 'sensitive-value';
const convertedValue = 'sensitive-value';
const convertedValue = 'converted-value';
jest.spyOn(keyValuePairRepository, 'findOne').mockResolvedValue(null);
jest.spyOn(TypedReflect, 'getMetadata').mockReturnValue({
[key]: {
isSensitive: true,
@ -492,7 +495,15 @@ describe('ConfigStorageService', () => {
jest.spyOn(keyValuePairRepository, 'delete').mockRejectedValue(error);
await expect(service.delete(key)).rejects.toThrow('Delete error');
await expect(service.delete(key)).rejects.toThrow(
ConfigVariableException,
);
await expect(service.delete(key)).rejects.toThrow(
`Failed to delete config variable ${key}: Delete error`,
);
await expect(service.delete(key)).rejects.toMatchObject({
code: ConfigVariableExceptionCode.INTERNAL_ERROR,
});
});
});
@ -552,7 +563,13 @@ describe('ConfigStorageService', () => {
jest.spyOn(keyValuePairRepository, 'find').mockRejectedValue(error);
await expect(service.loadAll()).rejects.toThrow('Find error');
await expect(service.loadAll()).rejects.toThrow(ConfigVariableException);
await expect(service.loadAll()).rejects.toThrow(
'Failed to load all config variables: Find error',
);
await expect(service.loadAll()).rejects.toMatchObject({
code: ConfigVariableExceptionCode.INTERNAL_ERROR,
});
});
describe('Null Value Handling', () => {
@ -649,7 +666,10 @@ describe('ConfigStorageService', () => {
throw new Error('Invalid boolean value');
});
await expect(service.get(key)).rejects.toThrow('Invalid boolean value');
await expect(service.get(key)).rejects.toThrow(ConfigVariableException);
await expect(service.get(key)).rejects.toMatchObject({
code: ConfigVariableExceptionCode.VALIDATION_FAILED,
});
});
it('should enforce correct types for string config variables', async () => {
@ -668,7 +688,10 @@ describe('ConfigStorageService', () => {
throw new Error('Invalid string value');
});
await expect(service.get(key)).rejects.toThrow('Invalid string value');
await expect(service.get(key)).rejects.toThrow(ConfigVariableException);
await expect(service.get(key)).rejects.toMatchObject({
code: ConfigVariableExceptionCode.VALIDATION_FAILED,
});
});
});
@ -725,78 +748,5 @@ describe('ConfigStorageService', () => {
expect(keyValuePairRepository.delete).toHaveBeenCalledTimes(2);
});
});
describe('Database Connection Issues', () => {
it('should handle database connection failures in get', async () => {
const key = 'AUTH_PASSWORD_ENABLED' as keyof ConfigVariables;
const error = new Error('Database connection failed');
jest.spyOn(keyValuePairRepository, 'findOne').mockRejectedValue(error);
await expect(service.get(key)).rejects.toThrow(
'Database connection failed',
);
});
it('should handle database connection failures in set', async () => {
const key = 'AUTH_PASSWORD_ENABLED' as keyof ConfigVariables;
const value = true;
const error = new Error('Database connection failed');
(
configValueConverter.convertAppValueToDbValue as jest.Mock
).mockReturnValue('true');
jest.spyOn(keyValuePairRepository, 'findOne').mockRejectedValue(error);
await expect(service.set(key, value)).rejects.toThrow(
'Database connection failed',
);
});
it('should handle database connection failures in delete', async () => {
const key = 'AUTH_PASSWORD_ENABLED' as keyof ConfigVariables;
const error = new Error('Database connection failed');
jest.spyOn(keyValuePairRepository, 'delete').mockRejectedValue(error);
await expect(service.delete(key)).rejects.toThrow(
'Database connection failed',
);
});
it('should handle database connection failures in loadAll', async () => {
const error = new Error('Database connection failed');
jest.spyOn(keyValuePairRepository, 'find').mockRejectedValue(error);
await expect(service.loadAll()).rejects.toThrow(
'Database connection failed',
);
});
it('should handle database operation timeouts', async () => {
const key = 'AUTH_PASSWORD_ENABLED' as keyof ConfigVariables;
const error = new Error('Database operation timed out');
let rejectPromise: ((error: Error) => void) | undefined;
const timeoutPromise = new Promise<KeyValuePair | null>((_, reject) => {
rejectPromise = reject;
});
jest
.spyOn(keyValuePairRepository, 'findOne')
.mockReturnValue(timeoutPromise);
const promise = service.get(key);
// Simulate timeout by rejecting the promise
if (!rejectPromise) {
throw new Error('Reject function not assigned');
}
rejectPromise(error);
await expect(promise).rejects.toThrow('Database operation timed out');
});
});
});
});

View File

@ -15,6 +15,10 @@ import { ConfigVariables } from 'src/engine/core-modules/twenty-config/config-va
import { ConfigValueConverterService } from 'src/engine/core-modules/twenty-config/conversion/config-value-converter.service';
import { EnvironmentConfigDriver } from 'src/engine/core-modules/twenty-config/drivers/environment-config.driver';
import { ConfigVariableType } from 'src/engine/core-modules/twenty-config/enums/config-variable-type.enum';
import {
ConfigVariableException,
ConfigVariableExceptionCode,
} from 'src/engine/core-modules/twenty-config/twenty-config.exception';
import { TypedReflect } from 'src/utils/typed-reflect';
import { ConfigStorageInterface } from './interfaces/config-storage.interface';
@ -51,12 +55,6 @@ export class ConfigStorageService implements ConfigStorageInterface {
];
}
// eslint-disable-next-line @typescript-eslint/no-explicit-any
private logAndRethrow(message: string, error: any): never {
this.logger.error(message, error);
throw error;
}
private async convertAndSecureValue<T extends keyof ConfigVariables>(
// eslint-disable-next-line @typescript-eslint/no-explicit-any
value: any,
@ -86,21 +84,18 @@ export class ConfigStorageService implements ConfigStorageInterface {
? decryptText(convertedValue, appSecret)
: encryptText(convertedValue, appSecret);
} catch (error) {
this.logger.error(
`Failed to ${isDecrypt ? 'decrypt' : 'encrypt'} value for key ${
this.logger.debug(
`${isDecrypt ? 'Decryption' : 'Encryption'} failed for key ${
key as string
}`,
error,
}: ${error.message}. Using original value.`,
);
return convertedValue;
}
} catch (error) {
this.logAndRethrow(
`Failed to convert value ${
isDecrypt ? 'from DB' : 'to DB'
} for key ${key as string}`,
error,
throw new ConfigVariableException(
`Failed to convert value for key ${key as string}: ${error.message}`,
ConfigVariableExceptionCode.VALIDATION_FAILED,
);
}
}
@ -123,7 +118,14 @@ export class ConfigStorageService implements ConfigStorageInterface {
return await this.convertAndSecureValue(result.value, key, true);
} catch (error) {
this.logAndRethrow(`Failed to get config for ${key as string}`, error);
if (error instanceof ConfigVariableException) {
throw error;
}
throw new ConfigVariableException(
`Failed to retrieve config variable ${key as string}: ${error instanceof Error ? error.message : String(error)}`,
ConfigVariableExceptionCode.INTERNAL_ERROR,
);
}
}
@ -153,7 +155,14 @@ export class ConfigStorageService implements ConfigStorageInterface {
});
}
} catch (error) {
this.logAndRethrow(`Failed to set config for ${key as string}`, error);
if (error instanceof ConfigVariableException) {
throw error;
}
throw new ConfigVariableException(
`Failed to save config variable ${key as string}: ${error instanceof Error ? error.message : String(error)}`,
ConfigVariableExceptionCode.INTERNAL_ERROR,
);
}
}
@ -163,7 +172,10 @@ export class ConfigStorageService implements ConfigStorageInterface {
this.getConfigVariableWhereClause(key as string),
);
} catch (error) {
this.logAndRethrow(`Failed to delete config for ${key as string}`, error);
throw new ConfigVariableException(
`Failed to delete config variable ${key as string}: ${error instanceof Error ? error.message : String(error)}`,
ConfigVariableExceptionCode.INTERNAL_ERROR,
);
}
}
@ -195,10 +207,10 @@ export class ConfigStorageService implements ConfigStorageInterface {
result.set(key, value);
}
} catch (error) {
this.logger.error(
`Failed to process config value for key ${key as string}`,
error,
this.logger.debug(
`Skipping invalid config value for key ${key as string}: ${error instanceof Error ? error.message : String(error)}`,
);
continue;
}
}
@ -206,7 +218,10 @@ export class ConfigStorageService implements ConfigStorageInterface {
return result;
} catch (error) {
this.logAndRethrow('Failed to load all config variables', error);
throw new ConfigVariableException(
`Failed to load all config variables: ${error instanceof Error ? error.message : String(error)}`,
ConfigVariableExceptionCode.INTERNAL_ERROR,
);
}
}
}

View File

@ -13,4 +13,4 @@ export const TEST_KEY_METRICS: ConfigKey =
'HEALTH_METRICS_TIME_WINDOW_IN_MINUTES';
export const TEST_KEY_ENV_ONLY: ConfigKey = 'PG_DATABASE_URL';
export const TEST_KEY_NONEXISTENT = 'NONEXISTENT_CONFIG_KEY';
export const TEST_KEY_STRING_VALUE = 'EMAIL_FROM_NAME';
export const TEST_KEY_STRING_VALUE: ConfigKey = 'EMAIL_FROM_NAME';

View File

@ -257,24 +257,24 @@ describe('TwentyConfig Integration', () => {
it('should reject updating config variables with invalid types', async () => {
await createConfigVariable({
input: {
key: 'NODE_PORT',
value: 3000,
key: TEST_KEY_DEFAULT,
value: true,
},
});
const updateResult = await updateConfigVariable({
input: {
key: 'NODE_PORT',
value: 'not-a-number',
key: TEST_KEY_DEFAULT,
value: 'not-a-boolean',
},
expectToFail: true,
});
expect(updateResult.errors).toBeDefined();
expect(updateResult.errors[0].message).toContain('Expected number');
expect(updateResult.errors[0].message).toContain('Expected boolean');
await deleteConfigVariable({
input: { key: 'NODE_PORT' },
input: { key: TEST_KEY_DEFAULT },
});
});
});

View File

@ -18,16 +18,15 @@ if (packageChanged && !lockfileChanged) {
warn(`${message} - <i>${idea}</i>`);
}
// Check if .env.example was changed, but not config variable documentation
const envChanged =
danger.git.modified_files.find((x) => x.includes('.env.example')) ||
danger.git.modified_files.find((x) => x.includes('twenty-config.service.ts'));
const envDocsChanged = danger.git.modified_files.includes('self-hosting.mdx');
if (envChanged && !envDocsChanged) {
const message =
'Changes were made to the config variables, but not to the documentation';
const idea =
'Please review your changes and check if a change needs to be documented!';
// Check environment configuration changes
const envExampleChanged = danger.git.modified_files.find((x) =>
x.includes('.env.example')
);
// Check if .env.example was changed
if (envExampleChanged) {
const message = 'Changes were made to .env.example';
const idea = 'Please make sure any new environment variables are properly documented with metadata in config-variables.ts';
warn(`${message} - <i>${idea}</i>`);
}