Skip to content

Commit

Permalink
feat(APIM-468): change how integer config values are parsed (#357)
Browse files Browse the repository at this point in the history
### 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`
  • Loading branch information
avaitonis authored Jul 28, 2023
2 parents 67b5925 + bbbd32f commit ee24d08
Show file tree
Hide file tree
Showing 7 changed files with 246 additions and 122 deletions.
141 changes: 37 additions & 104 deletions src/config/app.config.test.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,9 @@
import { RandomValueGenerator } from '@ukef-test/support/generator/random-value-generator';
import { withEnvironmentVariableParsingUnitTests } from '@ukef-test/common-tests/environment-variable-parsing-unit-tests';

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

describe('appConfig', () => {
const valueGenerator = new RandomValueGenerator();

let originalProcessEnv: NodeJS.ProcessEnv;

beforeEach(() => {
Expand Down Expand Up @@ -79,107 +77,42 @@ describe('appConfig', () => {
});
});

describe('parsing REDACT_LOGS', () => {
it('sets redactLogs to true if REDACT_LOGS is true', () => {
replaceEnvironmentVariables({
REDACT_LOGS: 'true',
});

const config = appConfig();

expect(config.redactLogs).toBe(true);
});

it('sets redactLogs to false if REDACT_LOGS is false', () => {
replaceEnvironmentVariables({
REDACT_LOGS: 'false',
});

const config = appConfig();

expect(config.redactLogs).toBe(false);
});

it('sets redactLogs to true if REDACT_LOGS is not specified', () => {
replaceEnvironmentVariables({});

const config = appConfig();

expect(config.redactLogs).toBe(true);
});

it('sets redactLogs to true if REDACT_LOGS is the empty string', () => {
replaceEnvironmentVariables({
REDACT_LOGS: '',
});

const config = appConfig();

expect(config.redactLogs).toBe(true);
});

it('sets redactLogs to true if REDACT_LOGS is any string other than true or false', () => {
replaceEnvironmentVariables({
REDACT_LOGS: valueGenerator.string(),
});

const config = appConfig();

expect(config.redactLogs).toBe(true);
});
});

describe('parsing SINGLE_LINE_LOG_FORMAT', () => {
it('sets singleLineLogFormat to true if SINGLE_LINE_LOG_FORMAT is true', () => {
replaceEnvironmentVariables({
SINGLE_LINE_LOG_FORMAT: 'true',
});

const config = appConfig();

expect(config.singleLineLogFormat).toBe(true);
});

it('sets singleLineLogFormat to false if SINGLE_LINE_LOG_FORMAT is false', () => {
replaceEnvironmentVariables({
SINGLE_LINE_LOG_FORMAT: 'false',
});

const config = appConfig();

expect(config.singleLineLogFormat).toBe(false);
});

it('sets singleLineLogFormat to true if SINGLE_LINE_LOG_FORMAT is not specified', () => {
replaceEnvironmentVariables({});

const config = appConfig();

expect(config.singleLineLogFormat).toBe(true);
});

it('sets singleLineLogFormat to true if SINGLE_LINE_LOG_FORMAT is the empty string', () => {
replaceEnvironmentVariables({
SINGLE_LINE_LOG_FORMAT: '',
});

const config = appConfig();

expect(config.singleLineLogFormat).toBe(true);
});

it('sets singleLineLogFormat to true if SINGLE_LINE_LOG_FORMAT is any string other than true or false', () => {
replaceEnvironmentVariables({
SINGLE_LINE_LOG_FORMAT: valueGenerator.string(),
});

const config = appConfig();

expect(config.singleLineLogFormat).toBe(true);
});
});

const replaceEnvironmentVariables = (newEnvVariables: Record<string, string>): void => {
process.env = newEnvVariables;
};

const configParsedBooleanFromEnvironmentVariablesWithDefault: {
configPropertyName: keyof AppConfig;
environmentVariableName: string;
defaultConfigValue: boolean;
}[] = [
{
configPropertyName: 'singleLineLogFormat',
environmentVariableName: 'SINGLE_LINE_LOG_FORMAT',
defaultConfigValue: true,
},
{
configPropertyName: 'redactLogs',
environmentVariableName: 'REDACT_LOGS',
defaultConfigValue: true,
},
];

const configParsedAsIntFromEnvironmentVariablesWithDefault: {
configPropertyName: keyof AppConfig;
environmentVariableName: string;
defaultConfigValue: number;
}[] = [
{
configPropertyName: 'port',
environmentVariableName: 'HTTP_PORT',
defaultConfigValue: 3003,
},
];

withEnvironmentVariableParsingUnitTests({
configParsedBooleanFromEnvironmentVariablesWithDefault,
configParsedAsIntFromEnvironmentVariablesWithDefault,
getConfig: () => appConfig(),
});
});
19 changes: 18 additions & 1 deletion src/config/app.config.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,26 @@
import { registerAs } from '@nestjs/config';
import { getIntConfig } from '@ukef/helpers/get-int-config';

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

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

export interface AppConfig {
name: string;
env: string;
versioning: {
enable: boolean;
prefix: string;
version: string;
};
globalPrefix: string;
port: number;
apiKey: string;
logLevel: string;
redactLogs: boolean;
singleLineLogFormat: boolean;
}

export default registerAs('app', (): Record<string, any> => {
const logLevel = process.env.LOG_LEVEL || 'info';
if (!validLogLevels.includes(logLevel)) {
Expand All @@ -20,7 +37,7 @@ export default registerAs('app', (): Record<string, any> => {
},

globalPrefix: '/api',
port: process.env.HTTP_PORT ? Number.parseInt(process.env.HTTP_PORT, 10) : 3003,
port: getIntConfig(process.env.HTTP_PORT, 3003),
apiKey: process.env.API_KEY,
logLevel: process.env.LOG_LEVEL || 'info',
redactLogs: process.env.REDACT_LOGS !== 'false',
Expand Down
5 changes: 3 additions & 2 deletions src/config/informatica.config.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { registerAs } from '@nestjs/config';
import { getIntConfig } from '@ukef/helpers/get-int-config';

export const KEY = 'informatica';

Expand All @@ -16,7 +17,7 @@ export default registerAs(
baseUrl: process.env.APIM_INFORMATICA_URL,
username: process.env.APIM_INFORMATICA_USERNAME,
password: process.env.APIM_INFORMATICA_PASSWORD,
maxRedirects: parseInt(process.env.APIM_INFORMATICA_MAX_REDIRECTS) || 5,
timeout: parseInt(process.env.APIM_INFORMATICA_TIMEOUT) || 30000,
maxRedirects: getIntConfig(process.env.APIM_INFORMATICA_MAX_REDIRECTS, 5),
timeout: getIntConfig(process.env.APIM_INFORMATICA_TIMEOUT, 30000),
}),
);
47 changes: 47 additions & 0 deletions src/helpers/get-int-config.helper.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
import { InvalidConfigException } from '@ukef/config/invalid-config.exception';

import { getIntConfig } from './get-int-config';

describe('GetIntConfig helper', () => {
describe('getIntConfig returns value', () => {
it.each([
{ value: undefined, defaultValue: 60, expectedResult: 60 },
{ value: '123', defaultValue: 60, expectedResult: 123 },
{ value: '123', defaultValue: undefined, expectedResult: 123 },
{ value: '-123', defaultValue: 60, expectedResult: -123 },
{ value: '0', defaultValue: 60, expectedResult: 0 },
{ value: `${Number.MAX_SAFE_INTEGER}`, defaultValue: 60, expectedResult: Number.MAX_SAFE_INTEGER },
{ value: `-${Number.MAX_SAFE_INTEGER}`, defaultValue: 60, expectedResult: -Number.MAX_SAFE_INTEGER },
])('if input is "$value", returns $expectedResult', ({ value, defaultValue, expectedResult }) => {
expect(getIntConfig(value, defaultValue)).toBe(expectedResult);
});
});

describe('getIntConfig throws invalid integer exception', () => {
it.each(['abc', '12.5', '20th', '0xFF', '0b101'])(`throws InvalidConfigException for "%s" because it is not valid integer`, (value) => {
const gettingTheConfig = () => getIntConfig(value as unknown as string);

expect(gettingTheConfig).toThrow(InvalidConfigException);
expect(gettingTheConfig).toThrow(`Invalid integer value "${value}" for configuration property.`);
});
});

describe('getIntConfig throws InvalidConfigException because environment variable type is not string', () => {
it.each([12, true, null, false, /.*/, {}, [], 0xff, 0b101])(
'throws InvalidConfigException for "%s" because environment variable type is not string',
(value) => {
const gettingTheConfig = () => getIntConfig(value as unknown as string);

expect(gettingTheConfig).toThrow(InvalidConfigException);
expect(gettingTheConfig).toThrow(`Input environment variable type for ${value} should be string.`);
},
);
});

it('throws InvalidConfigException if environment variable and default value is missing', () => {
const gettingTheConfig = () => getIntConfig(undefined);

expect(gettingTheConfig).toThrow(InvalidConfigException);
expect(gettingTheConfig).toThrow("Environment variable is missing and doesn't have default value.");
});
});
21 changes: 21 additions & 0 deletions src/helpers/get-int-config.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
import { InvalidConfigException } from '@ukef/config/invalid-config.exception';

// This helper function is used to get integer from configuration.
export const getIntConfig = (environmentVariable: string, defaultValue?: number): number => {
if (typeof environmentVariable === 'undefined') {
if (typeof defaultValue === 'undefined') {
throw new InvalidConfigException(`Environment variable is missing and doesn't have default value.`);
}
return defaultValue;
}
if (typeof environmentVariable !== 'string') {
throw new InvalidConfigException(`Input environment variable type for ${environmentVariable} should be string.`);
}

const integer = parseInt(environmentVariable, 10);
// Check if parseInt is number, decimal base integer and input didn't have anything non-numeric.
if (isNaN(integer) || integer.toString() !== environmentVariable) {
throw new InvalidConfigException(`Invalid integer value "${environmentVariable}" for configuration property.`);
}
return integer;
};
2 changes: 1 addition & 1 deletion src/helpers/regex.helper.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { regexToString } from './regex.helper';

describe('Regex Helper', () => {
describe('Regex helper', () => {
describe('regexToString', () => {
it('replaces the leading and trailing forward slashes with an empty string', () => {
const regex = /test/;
Expand Down
Loading

0 comments on commit ee24d08

Please sign in to comment.