validating min and max bytes

elastic · Apr 30, 2020 · 603d6a8 · 603d6a8
1 parent dd2a462
commit 603d6a8
Show file tree

Hide file tree

Showing 8 changed files with 104 additions and 39 deletions.
diff --git a/...Settings/AgentConfigurations/AgentConfigurationCreateEdit/SettingsPage/SettingFormRow.tsx b/...Settings/AgentConfigurations/AgentConfigurationCreateEdit/SettingsPage/SettingFormRow.tsx
@@ -92,15 +92,14 @@ function FormRow({
           <EuiFlexItem grow={false}>
             <EuiFieldNumber
               placeholder={setting.placeholder}
-              value={(amount as unknown) as number}
-              // Min and max settings are string in the duration type, representing the amount and unit e.g.: '1s'.
-              // becasue of that only uses it when it's defined as number.
-              min={typeof setting.min === 'number' ? setting.min : undefined}
-              max={typeof setting.max === 'number' ? setting.max : undefined}
+              value={amount}
               onChange={e =>
                 onChange(
                   setting.key,
-                  amountAndUnitToString({ amount: e.target.value, unit })
+                  amountAndUnitToString({
+                    amount: parseInt(e.target.value, 10),
+                    unit
+                  })
                 )
               }
             />

diff --git a/x-pack/plugins/apm/common/agent_configuration/amount_and_unit.ts b/x-pack/plugins/apm/common/agent_configuration/amount_and_unit.ts
@@ -4,15 +4,15 @@
  * you may not use this file except in compliance with the Elastic License.
  */
 
-interface AmountAndUnit {
-  amount: string;
+export interface AmountAndUnit {
+  amount: number;
   unit: string;
 }
 
 export function amountAndUnitToObject(value: string): AmountAndUnit {
   // matches any postive and negative number and its unit.
   const [, amount = '', unit = ''] = value.match(/(^-?\d+)?(\w+)?/) || [];
-  return { amount, unit };
+  return { amount: parseInt(amount, 10), unit };
 }
 
 export function amountAndUnitToString({ amount, unit }: AmountAndUnit) {

diff --git a/x-pack/plugins/apm/common/agent_configuration/runtime_types/bytes_rt.test.ts b/x-pack/plugins/apm/common/agent_configuration/runtime_types/bytes_rt.test.ts
@@ -8,23 +8,66 @@ import { getBytesRt } from './bytes_rt';
 import { isRight } from 'fp-ts/lib/Either';
 
 describe('bytesRt', () => {
-  const bytesRt = getBytesRt({
-    min: 0,
-    units: ['b', 'mb', 'kb']
-  });
+  describe('must accept any amount and unit', () => {
+    const bytesRt = getBytesRt({
+      units: ['b', 'mb', 'kb']
+    });
+    describe('it should not accept', () => {
+      ['mb', 1, '1', '5gb', '6tb'].map(input => {
+        it(`${JSON.stringify(input)}`, () => {
+          expect(isRight(bytesRt.decode(input))).toBe(false);
+        });
+      });
+    });
 
-  describe('it should not accept', () => {
-    ['mb', '-1kb', '5gb', '6tb'].map(input => {
-      it(`${JSON.stringify(input)}`, () => {
-        expect(isRight(bytesRt.decode(input))).toBe(false);
+    describe('it should accept', () => {
+      ['-1b', '0mb', '1b', '2kb', '3mb', '1000mb'].map(input => {
+        it(`${JSON.stringify(input)}`, () => {
+          expect(isRight(bytesRt.decode(input))).toBe(true);
+        });
       });
     });
   });
+  describe('must be at least 0b', () => {
+    const bytesRt = getBytesRt({
+      min: '0b',
+      units: ['b', 'mb', 'kb']
+    });
+
+    describe('it should not accept', () => {
+      ['mb', '-1kb', '5gb', '6tb'].map(input => {
+        it(`${JSON.stringify(input)}`, () => {
+          expect(isRight(bytesRt.decode(input))).toBe(false);
+        });
+      });
+    });
 
-  describe('it should accept', () => {
-    ['1b', '2kb', '3mb'].map(input => {
-      it(`${JSON.stringify(input)}`, () => {
-        expect(isRight(bytesRt.decode(input))).toBe(true);
+    describe('it should accept', () => {
+      ['1b', '2kb', '3mb'].map(input => {
+        it(`${JSON.stringify(input)}`, () => {
+          expect(isRight(bytesRt.decode(input))).toBe(true);
+        });
+      });
+    });
+  });
+  describe('must be between 500b and 1mb', () => {
+    const bytesRt = getBytesRt({
+      min: '500b',
+      max: '1kb',
+      units: ['b', 'mb', 'kb']
+    });
+    describe('it should not accept', () => {
+      ['mb', '-1b', '1b', '499b', '1025b', '2kb', '1mb'].map(input => {
+        it(`${JSON.stringify(input)}`, () => {
+          expect(isRight(bytesRt.decode(input))).toBe(false);
+        });
+      });
+    });
+    describe('it should accept', () => {
+      ['500b', '1024b', '1kb'].map(input => {
+        it(`${JSON.stringify(input)}`, () => {
+          expect(isRight(bytesRt.decode(input))).toBe(true);
+        });
       });
     });
   });

diff --git a/x-pack/plugins/apm/common/agent_configuration/runtime_types/bytes_rt.ts b/x-pack/plugins/apm/common/agent_configuration/runtime_types/bytes_rt.ts
@@ -10,17 +10,35 @@ import { i18n } from '@kbn/i18n';
 import { amountAndUnitToObject } from '../amount_and_unit';
 import { getRangeType } from './get_range_type';
 
-export const BYTE_UNITS = ['b', 'kb', 'mb'];
+function toBytes(amount: number, unit: string) {
+  switch (unit) {
+    case 'kb':
+      return amount * 2 ** 10;
+    case 'mb':
+      return amount * 2 ** 20;
+    case 'b':
+    default:
+      return amount;
+  }
+}
 
 export function getBytesRt({
-  min = -Infinity,
-  max = Infinity,
+  min,
+  max,
   units
 }: {
-  min?: number;
-  max?: number;
+  min?: string;
+  max?: string;
   units: string[];
 }) {
+  const { amount: minAmount, unit: minUnit } = min
+    ? amountAndUnitToObject(min)
+    : { amount: -Infinity, unit: 'b' };
+
+  const { amount: maxAmount, unit: maxUnit } = max
+    ? amountAndUnitToObject(max)
+    : { amount: Infinity, unit: 'b' };
+
   const message = i18n.translate('xpack.apm.agentConfig.bytes.errorText', {
     defaultMessage: `{rangeType, select,
         between {Must be between {min} and {max} with unit: {units}}
@@ -32,7 +50,7 @@ export function getBytesRt({
       min,
       max,
       units: units.join(', '),
-      rangeType: getRangeType(min, max)
+      rangeType: getRangeType(minAmount, maxAmount)
     }
   });
 
@@ -42,11 +60,16 @@ export function getBytesRt({
     (input, context) => {
       return either.chain(t.string.validate(input, context), inputAsString => {
         const { amount, unit } = amountAndUnitToObject(inputAsString);
-        const amountAsInt = parseInt(amount, 10);
         const isValidUnit = units.includes(unit);
-        const isValid = amountAsInt >= min && amountAsInt <= max && isValidUnit;
 
-        return isValid
+        const inputAsBytes = toBytes(amount, unit);
+        const minAsBytes = toBytes(minAmount, minUnit);
+        const maxAsBytes = toBytes(maxAmount, maxUnit);
+
+        const isValidAmount =
+          inputAsBytes >= minAsBytes && inputAsBytes <= maxAsBytes;
+
+        return isValidUnit && isValidAmount
           ? t.success(inputAsString)
           : t.failure(input, context, message);
       });

diff --git a/x-pack/plugins/apm/common/agent_configuration/runtime_types/duration_rt.ts b/x-pack/plugins/apm/common/agent_configuration/runtime_types/duration_rt.ts
@@ -8,11 +8,11 @@ import * as t from 'io-ts';
 import { either } from 'fp-ts/lib/Either';
 import moment, { unitOfTime } from 'moment';
 import { i18n } from '@kbn/i18n';
-import { amountAndUnitToObject } from '../amount_and_unit';
+import { amountAndUnitToObject, AmountAndUnit } from '../amount_and_unit';
 import { getRangeType } from './get_range_type';
 
-function getDuration({ amount, unit }: { amount: string; unit: string }) {
-  return moment.duration(parseInt(amount, 10), unit as unitOfTime.Base);
+function getDuration({ amount, unit }: AmountAndUnit) {
+  return moment.duration(amount, unit as unitOfTime.Base);
 }
 
 export function getDurationRt({
@@ -38,8 +38,8 @@ export function getDurationRt({
       min,
       max,
       rangeType: getRangeType(
-        minAmountAndUnit ? parseInt(minAmountAndUnit.amount, 10) : undefined,
-        maxAmountAndUnit ? parseInt(maxAmountAndUnit.amount, 10) : undefined
+        minAmountAndUnit ? minAmountAndUnit.amount : undefined,
+        maxAmountAndUnit ? maxAmountAndUnit.amount : undefined
       )
     }
   });

diff --git a/...ugins/apm/common/agent_configuration/setting_definitions/__snapshots__/index.test.ts.snap b/...ugins/apm/common/agent_configuration/setting_definitions/__snapshots__/index.test.ts.snap
diff --git a/x-pack/plugins/apm/common/agent_configuration/setting_definitions/index.ts b/x-pack/plugins/apm/common/agent_configuration/setting_definitions/index.ts
@@ -50,7 +50,7 @@ function getSettingDefaults(setting: RawSettingDefinition): SettingDefinition {
 
     case 'bytes': {
       const units = setting.units ?? ['b', 'kb', 'mb'];
-      const min = setting.min ?? 0;
+      const min = setting.min ?? '0b';
       const max = setting.max;
 
       return {

diff --git a/x-pack/plugins/apm/common/agent_configuration/setting_definitions/types.d.ts b/x-pack/plugins/apm/common/agent_configuration/setting_definitions/types.d.ts
@@ -80,8 +80,8 @@ interface IntegerSetting extends BaseSetting {
 
 interface BytesSetting extends BaseSetting {
   type: 'bytes';
-  min?: number;
-  max?: number;
+  min?: string;
+  max?: string;
   units?: string[];
 }