feat(APIM-468): change how integer config values are parsed

UK-Export-Finance · Jul 26, 2023 · 94dffdd · 94dffdd
1 parent 2e5a948
commit 94dffdd
Show file tree

Hide file tree

Showing 5 changed files with 192 additions and 71 deletions.
diff --git a/src/config/app.config.test.ts b/src/config/app.config.test.ts
@@ -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(() => {
@@ -79,57 +77,37 @@ 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);
-    });
-  });
-
   const replaceEnvironmentVariables = (newEnvVariables: Record<string, string>): void => {
     process.env = newEnvVariables;
   };
+
+  const configParsedBooleanFromEnvironmentVariablesWithDefault: {
+    configPropertyName: keyof AppConfig;
+    environmentVariableName: string;
+    defaultConfigValue: boolean;
+  }[] = [
+    {
+      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(),
+  });
 });
diff --git a/src/config/app.config.ts b/src/config/app.config.ts
@@ -1,9 +1,25 @@
 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;
+}
+
 export default registerAs('app', (): Record<string, any> => {
   const logLevel = process.env.LOG_LEVEL || 'info';
   if (!validLogLevels.includes(logLevel)) {
@@ -20,7 +36,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',

diff --git a/src/config/informatica.config.ts b/src/config/informatica.config.ts
@@ -1,4 +1,5 @@
 import { registerAs } from '@nestjs/config';
+import { getIntConfig } from '@ukef/helpers/get-int-config';
 
 export const KEY = 'informatica';
 
@@ -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),
   }),
 );
diff --git a/src/helpers/get-int-config.ts b/src/helpers/get-int-config.ts
@@ -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 | undefined => {
+  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;
+};
diff --git a/test/common-tests/environment-variable-parsing-unit-tests.ts b/test/common-tests/environment-variable-parsing-unit-tests.ts
@@ -1,11 +1,17 @@
+import { InvalidConfigException } from '@ukef/config/invalid-config.exception';
 import { RandomValueGenerator } from '@ukef-test/support/generator/random-value-generator';
 
 interface Options<ConfigUnderTest> {
-  configDirectlyFromEnvironmentVariables: {
+  configDirectlyFromEnvironmentVariables?: {
     configPropertyName: keyof ConfigUnderTest;
     environmentVariableName: string;
   }[];
-  configParsedAsIntFromEnvironmentVariablesWithDefault: {
+  configParsedBooleanFromEnvironmentVariablesWithDefault?: {
+    configPropertyName: keyof ConfigUnderTest;
+    environmentVariableName: string;
+    defaultConfigValue: boolean;
+  }[];
+  configParsedAsIntFromEnvironmentVariablesWithDefault?: {
     configPropertyName: keyof ConfigUnderTest;
     environmentVariableName: string;
     defaultConfigValue: number;
@@ -15,6 +21,7 @@ interface Options<ConfigUnderTest> {
 
 export const withEnvironmentVariableParsingUnitTests = <ConfigUnderTest>({
   configDirectlyFromEnvironmentVariables,
+  configParsedBooleanFromEnvironmentVariablesWithDefault,
   configParsedAsIntFromEnvironmentVariablesWithDefault,
   getConfig,
 }: Options<ConfigUnderTest>): void => {
@@ -30,18 +37,79 @@ export const withEnvironmentVariableParsingUnitTests = <ConfigUnderTest>({
     process.env = originalProcessEnv;
   });
 
-  describe.each(configDirectlyFromEnvironmentVariables)('$configPropertyName', ({ configPropertyName, environmentVariableName }) => {
-    it(`is the env variable ${environmentVariableName}`, () => {
-      const environmentVariableValue = valueGenerator.string();
-      process.env = {
-        [environmentVariableName]: environmentVariableValue,
-      };
+  if (configDirectlyFromEnvironmentVariables) {
+    describe.each(configDirectlyFromEnvironmentVariables)('$configPropertyName', ({ configPropertyName, environmentVariableName }) => {
+      it(`is the env variable ${environmentVariableName}`, () => {
+        const environmentVariableValue = valueGenerator.string();
+        process.env = {
+          [environmentVariableName]: environmentVariableValue,
+        };
 
-      const { [configPropertyName]: configPropertyValue } = getConfig();
+        const { [configPropertyName]: configPropertyValue } = getConfig();
 
-      expect(configPropertyValue).toBe(environmentVariableValue);
+        expect(configPropertyValue).toBe(environmentVariableValue);
+      });
     });
-  });
+  }
+
+  if (configParsedBooleanFromEnvironmentVariablesWithDefault) {
+    describe.each(configParsedBooleanFromEnvironmentVariablesWithDefault)(
+      '$configPropertyName',
+      ({ configPropertyName, environmentVariableName, defaultConfigValue }) => {
+        it(`is true if environment variable ${environmentVariableName} is true`, () => {
+          const expectedConfigValue = true;
+          const environmentVariableValue = expectedConfigValue.toString();
+          process.env = {
+            [environmentVariableName]: environmentVariableValue,
+          };
+
+          const { [configPropertyName]: configPropertyValue } = getConfig();
+
+          expect(configPropertyValue).toBe(expectedConfigValue);
+        });
+
+        it(`is false if environment variable ${environmentVariableName} is false`, () => {
+          const expectedConfigValue = false;
+          const environmentVariableValue = expectedConfigValue.toString();
+          process.env = {
+            [environmentVariableName]: environmentVariableValue,
+          };
+
+          const { [configPropertyName]: configPropertyValue } = getConfig();
+
+          expect(configPropertyValue).toBe(expectedConfigValue);
+        });
+
+        it(`is the default value if ${environmentVariableName} is any string other than true or false`, () => {
+          process.env = {
+            [environmentVariableName]: valueGenerator.string(),
+          };
+
+          const { [configPropertyName]: configPropertyValue } = getConfig();
+
+          expect(configPropertyValue).toBe(defaultConfigValue);
+        });
+
+        it(`is the default value if ${environmentVariableName} is not specified`, () => {
+          process.env = {};
+
+          const { [configPropertyName]: configPropertyValue } = getConfig();
+
+          expect(configPropertyValue).toBe(defaultConfigValue);
+        });
+
+        it(`is the default value if ${environmentVariableName} is empty string`, () => {
+          process.env = {
+            [environmentVariableName]: '',
+          };
+
+          const { [configPropertyName]: configPropertyValue } = getConfig();
+
+          expect(configPropertyValue).toBe(defaultConfigValue);
+        });
+      },
+    );
+  }
 
   describe.each(configParsedAsIntFromEnvironmentVariablesWithDefault)(
     '$configPropertyName',
@@ -66,15 +134,52 @@ export const withEnvironmentVariableParsingUnitTests = <ConfigUnderTest>({
         expect(configPropertyValue).toBe(defaultConfigValue);
       });
 
-      it(`is the default value ${defaultConfigValue} if ${environmentVariableName} is not parseable as an integer`, () => {
+      it(`throws InvalidConfigException if ${environmentVariableName} is not parseable as an integer`, () => {
         const environmentVariableValue = 'abc';
         process.env = {
           [environmentVariableName]: environmentVariableValue,
         };
 
-        const { [configPropertyName]: configPropertyValue } = getConfig();
+        const gettingTheConfig = () => getConfig();
 
-        expect(configPropertyValue).toBe(defaultConfigValue);
+        expect(gettingTheConfig).toThrow(InvalidConfigException);
+        expect(gettingTheConfig).toThrow(`Invalid integer value "${environmentVariableValue}" for configuration property.`);
+      });
+
+      it(`throws InvalidConfigException if ${environmentVariableName} is float number`, () => {
+        const environmentVariableValue = valueGenerator.nonnegativeFloat().toString();
+        process.env = {
+          [environmentVariableName]: environmentVariableValue,
+        };
+
+        const gettingTheConfig = () => getConfig();
+
+        expect(gettingTheConfig).toThrow(InvalidConfigException);
+        expect(gettingTheConfig).toThrow(`Invalid integer value "${environmentVariableValue}" for configuration property.`);
+      });
+
+      it(`throws InvalidConfigException if ${environmentVariableName} is hex number`, () => {
+        const environmentVariableValue = '0xFF';
+        process.env = {
+          [environmentVariableName]: environmentVariableValue,
+        };
+
+        const gettingTheConfig = () => getConfig();
+
+        expect(gettingTheConfig).toThrow(InvalidConfigException);
+        expect(gettingTheConfig).toThrow(`Invalid integer value "${environmentVariableValue}" for configuration property.`);
+      });
+
+      it(`throws InvalidConfigException if ${environmentVariableName} is binary number`, () => {
+        const environmentVariableValue = '0b101';
+        process.env = {
+          [environmentVariableName]: environmentVariableValue,
+        };
+
+        const gettingTheConfig = () => getConfig();
+
+        expect(gettingTheConfig).toThrow(InvalidConfigException);
+        expect(gettingTheConfig).toThrow(`Invalid integer value "${environmentVariableValue}" for configuration property.`);
       });
     },
   );