fix(core): Make boolean config value parsing backward-compatible (#10560)

This commit is contained in:
कारतोफ्फेलस्क्रिप्ट™ 2024-08-27 13:51:26 +02:00 committed by GitHub
parent 4e15007577
commit 70b410f4b0
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 75 additions and 34 deletions

View file

@ -6,13 +6,24 @@ import { Container, Service } from 'typedi';
type Class = Function; type Class = Function;
type Constructable<T = unknown> = new (rawValue: string) => T; type Constructable<T = unknown> = new (rawValue: string) => T;
type PropertyKey = string | symbol; type PropertyKey = string | symbol;
type PropertyType = number | boolean | string | Class;
interface PropertyMetadata { interface PropertyMetadata {
type: unknown; type: PropertyType;
envName?: string; envName?: string;
} }
const globalMetadata = new Map<Class, Map<PropertyKey, PropertyMetadata>>(); const globalMetadata = new Map<Class, Map<PropertyKey, PropertyMetadata>>();
const readEnv = (envName: string) => {
if (envName in process.env) return process.env[envName];
// Read the value from a file, if "_FILE" environment variable is defined
const filePath = process.env[`${envName}_FILE`];
if (filePath) return readFileSync(filePath, 'utf8');
return undefined;
};
export const Config: ClassDecorator = (ConfigClass: Class) => { export const Config: ClassDecorator = (ConfigClass: Class) => {
const factory = function () { const factory = function () {
const config = new (ConfigClass as new () => Record<PropertyKey, unknown>)(); const config = new (ConfigClass as new () => Record<PropertyKey, unknown>)();
@ -26,38 +37,28 @@ export const Config: ClassDecorator = (ConfigClass: Class) => {
if (typeof type === 'function' && globalMetadata.has(type)) { if (typeof type === 'function' && globalMetadata.has(type)) {
config[key] = Container.get(type); config[key] = Container.get(type);
} else if (envName) { } else if (envName) {
let value: unknown = process.env[envName]; const value = readEnv(envName);
if (value === undefined) continue;
// Read the value from a file, if "_FILE" environment variable is defined
const filePath = process.env[`${envName}_FILE`];
if (filePath) {
value = readFileSync(filePath, 'utf8');
}
if (type === Number) { if (type === Number) {
value = Number(value); const parsed = Number(value);
if (isNaN(value as number)) { if (isNaN(parsed)) {
// TODO: add a warning console.warn(`Invalid number value for ${envName}: ${value}`);
value = undefined; } else {
config[key] = parsed;
} }
} else if (type === Boolean) { } else if (type === Boolean) {
if (value !== 'true' && value !== 'false') { if (['true', '1'].includes(value.toLowerCase())) {
// TODO: add a warning config[key] = true;
value = undefined; } else if (['false', '0'].includes(value.toLowerCase())) {
config[key] = false;
} else { } else {
value = value === 'true'; console.warn(`Invalid boolean value for ${envName}: ${value}`);
} }
} else if (type === Object) { } else if (type === String) {
// eslint-disable-next-line n8n-local-rules/no-plain-errors
throw new Error(
`Invalid decorator metadata on key "${key as string}" on ${ConfigClass.name}\n Please use explicit typing on all config fields`,
);
} else if (type !== String && type !== Object) {
value = new (type as Constructable)(value as string);
}
if (value !== undefined) {
config[key] = value; config[key] = value;
} else {
config[key] = new (type as Constructable)(value);
} }
} }
} }
@ -70,7 +71,7 @@ export const Config: ClassDecorator = (ConfigClass: Class) => {
export const Nested: PropertyDecorator = (target: object, key: PropertyKey) => { export const Nested: PropertyDecorator = (target: object, key: PropertyKey) => {
const ConfigClass = target.constructor; const ConfigClass = target.constructor;
const classMetadata = globalMetadata.get(ConfigClass) ?? new Map<PropertyKey, PropertyMetadata>(); const classMetadata = globalMetadata.get(ConfigClass) ?? new Map<PropertyKey, PropertyMetadata>();
const type = Reflect.getMetadata('design:type', target, key) as unknown; const type = Reflect.getMetadata('design:type', target, key) as PropertyType;
classMetadata.set(key, { type }); classMetadata.set(key, { type });
globalMetadata.set(ConfigClass, classMetadata); globalMetadata.set(ConfigClass, classMetadata);
}; };
@ -81,7 +82,13 @@ export const Env =
const ConfigClass = target.constructor; const ConfigClass = target.constructor;
const classMetadata = const classMetadata =
globalMetadata.get(ConfigClass) ?? new Map<PropertyKey, PropertyMetadata>(); globalMetadata.get(ConfigClass) ?? new Map<PropertyKey, PropertyMetadata>();
const type = Reflect.getMetadata('design:type', target, key) as unknown; const type = Reflect.getMetadata('design:type', target, key) as PropertyType;
if (type === Object) {
// eslint-disable-next-line n8n-local-rules/no-plain-errors
throw new Error(
`Invalid decorator metadata on key "${key as string}" on ${ConfigClass.name}\n Please use explicit typing on all config fields`,
);
}
classMetadata.set(key, { type, envName }); classMetadata.set(key, { type, envName });
globalMetadata.set(ConfigClass, classMetadata); globalMetadata.set(ConfigClass, classMetadata);
}; };

View file

@ -17,6 +17,10 @@ describe('GlobalConfig', () => {
process.env = originalEnv; process.env = originalEnv;
}); });
// deepCopy for diff to show plain objects
// eslint-disable-next-line n8n-local-rules/no-json-parse-json-stringify
const deepCopy = <T>(obj: T): T => JSON.parse(JSON.stringify(obj));
const defaultConfig: GlobalConfig = { const defaultConfig: GlobalConfig = {
path: '/', path: '/',
host: 'localhost', host: 'localhost',
@ -218,10 +222,6 @@ describe('GlobalConfig', () => {
process.env = {}; process.env = {};
const config = Container.get(GlobalConfig); const config = Container.get(GlobalConfig);
// deepCopy for diff to show plain objects
// eslint-disable-next-line n8n-local-rules/no-json-parse-json-stringify
const deepCopy = <T>(obj: T): T => JSON.parse(JSON.stringify(obj));
expect(deepCopy(config)).toEqual(defaultConfig); expect(deepCopy(config)).toEqual(defaultConfig);
expect(mockFs.readFileSync).not.toHaveBeenCalled(); expect(mockFs.readFileSync).not.toHaveBeenCalled();
}); });
@ -233,9 +233,11 @@ describe('GlobalConfig', () => {
DB_TABLE_PREFIX: 'test_', DB_TABLE_PREFIX: 'test_',
NODES_INCLUDE: '["n8n-nodes-base.hackerNews"]', NODES_INCLUDE: '["n8n-nodes-base.hackerNews"]',
DB_LOGGING_MAX_EXECUTION_TIME: '0', DB_LOGGING_MAX_EXECUTION_TIME: '0',
N8N_METRICS: 'TRUE',
N8N_TEMPLATES_ENABLED: '0',
}; };
const config = Container.get(GlobalConfig); const config = Container.get(GlobalConfig);
expect(config).toEqual({ expect(deepCopy(config)).toEqual({
...defaultConfig, ...defaultConfig,
database: { database: {
logging: defaultConfig.database.logging, logging: defaultConfig.database.logging,
@ -249,10 +251,21 @@ describe('GlobalConfig', () => {
tablePrefix: 'test_', tablePrefix: 'test_',
type: 'sqlite', type: 'sqlite',
}, },
endpoints: {
...defaultConfig.endpoints,
metrics: {
...defaultConfig.endpoints.metrics,
enable: true,
},
},
nodes: { nodes: {
...defaultConfig.nodes, ...defaultConfig.nodes,
include: ['n8n-nodes-base.hackerNews'], include: ['n8n-nodes-base.hackerNews'],
}, },
templates: {
...defaultConfig.templates,
enabled: false,
},
}); });
expect(mockFs.readFileSync).not.toHaveBeenCalled(); expect(mockFs.readFileSync).not.toHaveBeenCalled();
}); });
@ -265,7 +278,7 @@ describe('GlobalConfig', () => {
mockFs.readFileSync.calledWith(passwordFile, 'utf8').mockReturnValueOnce('password-from-file'); mockFs.readFileSync.calledWith(passwordFile, 'utf8').mockReturnValueOnce('password-from-file');
const config = Container.get(GlobalConfig); const config = Container.get(GlobalConfig);
expect(config).toEqual({ expect(deepCopy(config)).toEqual({
...defaultConfig, ...defaultConfig,
database: { database: {
...defaultConfig.database, ...defaultConfig.database,

View file

@ -0,0 +1,21 @@
import { Container } from 'typedi';
import { Config, Env } from '../src/decorators';
describe('decorators', () => {
beforeEach(() => {
Container.reset();
});
it('should throw when explicit typing is missing', () => {
expect(() => {
@Config
class InvalidConfig {
@Env('STRING_VALUE')
value = 'string';
}
Container.get(InvalidConfig);
}).toThrowError(
'Invalid decorator metadata on key "value" on InvalidConfig\n Please use explicit typing on all config fields',
);
});
});