Skip to content

Commit ee24d08

Browse files
audrius-vaitonisaudrius-vaitonis
authored
feat(APIM-468): change how integer config values are parsed (#357)
### Introduction Current way to parse environment variables and set default value doesn't allow using 0. 0 can be useful for APIM_INFORMATICA_MAX_REDIRECTS, APIM_INFORMATICA_TIMEOUT and similar integer environment variables. ### Resolution This is copy of similar implementation in TFS API. Created helper getIntConfig. It parses input string and returns integer from input or optional default value. Helper throws exception for these edge cases: - input is set, but not string - input and default values are undefined - input is not integer in string format. For example it is float or has some non digital characters - input number is not using decimal base ### Misc Moved boolean environment variable test to shared test `withEnvironmentVariableParsingUnitTests`
2 parents 67b5925 + bbbd32f commit ee24d08

File tree

7 files changed

+246
-122
lines changed

7 files changed

+246
-122
lines changed

‎src/config/app.config.test.ts‎

Lines changed: 37 additions & 104 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,9 @@
1-
import { RandomValueGenerator } from '@ukef-test/support/generator/random-value-generator';
1+
import { withEnvironmentVariableParsingUnitTests } from '@ukef-test/common-tests/environment-variable-parsing-unit-tests';
22

3-
import appConfig from './app.config';
3+
import appConfig, { AppConfig } from './app.config';
44
import { InvalidConfigException } from './invalid-config.exception';
55

66
describe('appConfig', () => {
7-
const valueGenerator = new RandomValueGenerator();
8-
97
let originalProcessEnv: NodeJS.ProcessEnv;
108

119
beforeEach(() => {
@@ -79,107 +77,42 @@ describe('appConfig', () => {
7977
});
8078
});
8179

82-
describe('parsing REDACT_LOGS', () => {
83-
it('sets redactLogs to true if REDACT_LOGS is true', () => {
84-
replaceEnvironmentVariables({
85-
REDACT_LOGS: 'true',
86-
});
87-
88-
const config = appConfig();
89-
90-
expect(config.redactLogs).toBe(true);
91-
});
92-
93-
it('sets redactLogs to false if REDACT_LOGS is false', () => {
94-
replaceEnvironmentVariables({
95-
REDACT_LOGS: 'false',
96-
});
97-
98-
const config = appConfig();
99-
100-
expect(config.redactLogs).toBe(false);
101-
});
102-
103-
it('sets redactLogs to true if REDACT_LOGS is not specified', () => {
104-
replaceEnvironmentVariables({});
105-
106-
const config = appConfig();
107-
108-
expect(config.redactLogs).toBe(true);
109-
});
110-
111-
it('sets redactLogs to true if REDACT_LOGS is the empty string', () => {
112-
replaceEnvironmentVariables({
113-
REDACT_LOGS: '',
114-
});
115-
116-
const config = appConfig();
117-
118-
expect(config.redactLogs).toBe(true);
119-
});
120-
121-
it('sets redactLogs to true if REDACT_LOGS is any string other than true or false', () => {
122-
replaceEnvironmentVariables({
123-
REDACT_LOGS: valueGenerator.string(),
124-
});
125-
126-
const config = appConfig();
127-
128-
expect(config.redactLogs).toBe(true);
129-
});
130-
});
131-
132-
describe('parsing SINGLE_LINE_LOG_FORMAT', () => {
133-
it('sets singleLineLogFormat to true if SINGLE_LINE_LOG_FORMAT is true', () => {
134-
replaceEnvironmentVariables({
135-
SINGLE_LINE_LOG_FORMAT: 'true',
136-
});
137-
138-
const config = appConfig();
139-
140-
expect(config.singleLineLogFormat).toBe(true);
141-
});
142-
143-
it('sets singleLineLogFormat to false if SINGLE_LINE_LOG_FORMAT is false', () => {
144-
replaceEnvironmentVariables({
145-
SINGLE_LINE_LOG_FORMAT: 'false',
146-
});
147-
148-
const config = appConfig();
149-
150-
expect(config.singleLineLogFormat).toBe(false);
151-
});
152-
153-
it('sets singleLineLogFormat to true if SINGLE_LINE_LOG_FORMAT is not specified', () => {
154-
replaceEnvironmentVariables({});
155-
156-
const config = appConfig();
157-
158-
expect(config.singleLineLogFormat).toBe(true);
159-
});
160-
161-
it('sets singleLineLogFormat to true if SINGLE_LINE_LOG_FORMAT is the empty string', () => {
162-
replaceEnvironmentVariables({
163-
SINGLE_LINE_LOG_FORMAT: '',
164-
});
165-
166-
const config = appConfig();
167-
168-
expect(config.singleLineLogFormat).toBe(true);
169-
});
170-
171-
it('sets singleLineLogFormat to true if SINGLE_LINE_LOG_FORMAT is any string other than true or false', () => {
172-
replaceEnvironmentVariables({
173-
SINGLE_LINE_LOG_FORMAT: valueGenerator.string(),
174-
});
175-
176-
const config = appConfig();
177-
178-
expect(config.singleLineLogFormat).toBe(true);
179-
});
180-
});
181-
18280
const replaceEnvironmentVariables = (newEnvVariables: Record<string, string>): void => {
18381
process.env = newEnvVariables;
18482
};
83+
84+
const configParsedBooleanFromEnvironmentVariablesWithDefault: {
85+
configPropertyName: keyof AppConfig;
86+
environmentVariableName: string;
87+
defaultConfigValue: boolean;
88+
}[] = [
89+
{
90+
configPropertyName: 'singleLineLogFormat',
91+
environmentVariableName: 'SINGLE_LINE_LOG_FORMAT',
92+
defaultConfigValue: true,
93+
},
94+
{
95+
configPropertyName: 'redactLogs',
96+
environmentVariableName: 'REDACT_LOGS',
97+
defaultConfigValue: true,
98+
},
99+
];
100+
101+
const configParsedAsIntFromEnvironmentVariablesWithDefault: {
102+
configPropertyName: keyof AppConfig;
103+
environmentVariableName: string;
104+
defaultConfigValue: number;
105+
}[] = [
106+
{
107+
configPropertyName: 'port',
108+
environmentVariableName: 'HTTP_PORT',
109+
defaultConfigValue: 3003,
110+
},
111+
];
112+
113+
withEnvironmentVariableParsingUnitTests({
114+
configParsedBooleanFromEnvironmentVariablesWithDefault,
115+
configParsedAsIntFromEnvironmentVariablesWithDefault,
116+
getConfig: () => appConfig(),
117+
});
185118
});

‎src/config/app.config.ts‎

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,26 @@
11
import { registerAs } from '@nestjs/config';
2+
import { getIntConfig } from '@ukef/helpers/get-int-config';
23

34
import { InvalidConfigException } from './invalid-config.exception';
45

56
const validLogLevels = ['fatal', 'error', 'warn', 'info', 'debug', 'trace', 'silent'];
67

8+
export interface AppConfig {
9+
name: string;
10+
env: string;
11+
versioning: {
12+
enable: boolean;
13+
prefix: string;
14+
version: string;
15+
};
16+
globalPrefix: string;
17+
port: number;
18+
apiKey: string;
19+
logLevel: string;
20+
redactLogs: boolean;
21+
singleLineLogFormat: boolean;
22+
}
23+
724
export default registerAs('app', (): Record<string, any> => {
825
const logLevel = process.env.LOG_LEVEL || 'info';
926
if (!validLogLevels.includes(logLevel)) {
@@ -20,7 +37,7 @@ export default registerAs('app', (): Record<string, any> => {
2037
},
2138

2239
globalPrefix: '/api',
23-
port: process.env.HTTP_PORT ? Number.parseInt(process.env.HTTP_PORT, 10) : 3003,
40+
port: getIntConfig(process.env.HTTP_PORT, 3003),
2441
apiKey: process.env.API_KEY,
2542
logLevel: process.env.LOG_LEVEL || 'info',
2643
redactLogs: process.env.REDACT_LOGS !== 'false',

‎src/config/informatica.config.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { registerAs } from '@nestjs/config';
2+
import { getIntConfig } from '@ukef/helpers/get-int-config';
23

34
export const KEY = 'informatica';
45

@@ -16,7 +17,7 @@ export default registerAs(
1617
baseUrl: process.env.APIM_INFORMATICA_URL,
1718
username: process.env.APIM_INFORMATICA_USERNAME,
1819
password: process.env.APIM_INFORMATICA_PASSWORD,
19-
maxRedirects: parseInt(process.env.APIM_INFORMATICA_MAX_REDIRECTS) || 5,
20-
timeout: parseInt(process.env.APIM_INFORMATICA_TIMEOUT) || 30000,
20+
maxRedirects: getIntConfig(process.env.APIM_INFORMATICA_MAX_REDIRECTS, 5),
21+
timeout: getIntConfig(process.env.APIM_INFORMATICA_TIMEOUT, 30000),
2122
}),
2223
);

‎src/helpers/get-int-config.helper.test.ts‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
import { InvalidConfigException } from '@ukef/config/invalid-config.exception';
2+
3+
import { getIntConfig } from './get-int-config';
4+
5+
describe('GetIntConfig helper', () => {
6+
describe('getIntConfig returns value', () => {
7+
it.each([
8+
{ value: undefined, defaultValue: 60, expectedResult: 60 },
9+
{ value: '123', defaultValue: 60, expectedResult: 123 },
10+
{ value: '123', defaultValue: undefined, expectedResult: 123 },
11+
{ value: '-123', defaultValue: 60, expectedResult: -123 },
12+
{ value: '0', defaultValue: 60, expectedResult: 0 },
13+
{ value: `${Number.MAX_SAFE_INTEGER}`, defaultValue: 60, expectedResult: Number.MAX_SAFE_INTEGER },
14+
{ value: `-${Number.MAX_SAFE_INTEGER}`, defaultValue: 60, expectedResult: -Number.MAX_SAFE_INTEGER },
15+
])('if input is "$value", returns $expectedResult', ({ value, defaultValue, expectedResult }) => {
16+
expect(getIntConfig(value, defaultValue)).toBe(expectedResult);
17+
});
18+
});
19+
20+
describe('getIntConfig throws invalid integer exception', () => {
21+
it.each(['abc', '12.5', '20th', '0xFF', '0b101'])(`throws InvalidConfigException for "%s" because it is not valid integer`, (value) => {
22+
const gettingTheConfig = () => getIntConfig(value as unknown as string);
23+
24+
expect(gettingTheConfig).toThrow(InvalidConfigException);
25+
expect(gettingTheConfig).toThrow(`Invalid integer value "${value}" for configuration property.`);
26+
});
27+
});
28+
29+
describe('getIntConfig throws InvalidConfigException because environment variable type is not string', () => {
30+
it.each([12, true, null, false, /.*/, {}, [], 0xff, 0b101])(
31+
'throws InvalidConfigException for "%s" because environment variable type is not string',
32+
(value) => {
33+
const gettingTheConfig = () => getIntConfig(value as unknown as string);
34+
35+
expect(gettingTheConfig).toThrow(InvalidConfigException);
36+
expect(gettingTheConfig).toThrow(`Input environment variable type for ${value} should be string.`);
37+
},
38+
);
39+
});
40+
41+
it('throws InvalidConfigException if environment variable and default value is missing', () => {
42+
const gettingTheConfig = () => getIntConfig(undefined);
43+
44+
expect(gettingTheConfig).toThrow(InvalidConfigException);
45+
expect(gettingTheConfig).toThrow("Environment variable is missing and doesn't have default value.");
46+
});
47+
});

‎src/helpers/get-int-config.ts‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
import { InvalidConfigException } from '@ukef/config/invalid-config.exception';
2+
3+
// This helper function is used to get integer from configuration.
4+
export const getIntConfig = (environmentVariable: string, defaultValue?: number): number => {
5+
if (typeof environmentVariable === 'undefined') {
6+
if (typeof defaultValue === 'undefined') {
7+
throw new InvalidConfigException(`Environment variable is missing and doesn't have default value.`);
8+
}
9+
return defaultValue;
10+
}
11+
if (typeof environmentVariable !== 'string') {
12+
throw new InvalidConfigException(`Input environment variable type for ${environmentVariable} should be string.`);
13+
}
14+
15+
const integer = parseInt(environmentVariable, 10);
16+
// Check if parseInt is number, decimal base integer and input didn't have anything non-numeric.
17+
if (isNaN(integer) || integer.toString() !== environmentVariable) {
18+
throw new InvalidConfigException(`Invalid integer value "${environmentVariable}" for configuration property.`);
19+
}
20+
return integer;
21+
};

‎src/helpers/regex.helper.test.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { regexToString } from './regex.helper';
22

3-
describe('Regex Helper', () => {
3+
describe('Regex helper', () => {
44
describe('regexToString', () => {
55
it('replaces the leading and trailing forward slashes with an empty string', () => {
66
const regex = /test/;

0 commit comments

Comments
 (0)