From d48d61e54cf557809cb04fdf11844d44819edac3 Mon Sep 17 00:00:00 2001 From: Maxim Palenov Date: Tue, 14 Jan 2025 20:15:36 +0100 Subject: [PATCH] use rule meta to persist time unit --- .../src/time_duration/time_duration.test.ts | 122 +++++++----------- .../src/time_duration/time_duration.ts | 31 ++--- .../to_simple_rule_schedule.test.ts | 38 ++++-- .../rule_schema/to_simple_rule_schedule.ts | 23 +++- .../pages/rule_creation/helpers.test.ts | 6 +- .../pages/rule_creation/helpers.ts | 1 + ...get_field_diffs_for_grouped_fields.test.ts | 6 +- .../rule_details/rule_schedule_section.tsx | 1 + .../detection_engine/rules/helpers.test.tsx | 4 +- 9 files changed, 118 insertions(+), 114 deletions(-) diff --git a/x-pack/solutions/security/packages/kbn-securitysolution-utils/src/time_duration/time_duration.test.ts b/x-pack/solutions/security/packages/kbn-securitysolution-utils/src/time_duration/time_duration.test.ts index e30cae48f2f90..c968e3b1eed7f 100644 --- a/x-pack/solutions/security/packages/kbn-securitysolution-utils/src/time_duration/time_duration.test.ts +++ b/x-pack/solutions/security/packages/kbn-securitysolution-utils/src/time_duration/time_duration.test.ts @@ -11,13 +11,13 @@ describe('TimeDuration', () => { describe('fromMilliseconds', () => { it.each([ [5000, new TimeDuration(5, 's')], - [600000, new TimeDuration(10, 'm')], - [25200000, new TimeDuration(7, 'h')], - [777600000, new TimeDuration(9, 'd')], + [600000, new TimeDuration(600, 's')], + [25200000, new TimeDuration(25200, 's')], + [777600000, new TimeDuration(777600, 's')], [-3000, new TimeDuration(-3, 's')], - [-300000, new TimeDuration(-5, 'm')], - [-18000000, new TimeDuration(-5, 'h')], - [-604800000, new TimeDuration(-7, 'd')], + [-300000, new TimeDuration(-300, 's')], + [-18000000, new TimeDuration(-18000, 's')], + [-604800000, new TimeDuration(-604800, 's')], ])('parses "%s"', (ms, expectedTimeDuration) => { const result = TimeDuration.fromMilliseconds(ms); @@ -40,17 +40,17 @@ describe('TimeDuration', () => { ['-5h', new TimeDuration(-5, 'h')], ['-7d', new TimeDuration(-7, 'd')], ['0s', new TimeDuration(0, 's')], - ['0m', new TimeDuration(0, 's')], - ['0h', new TimeDuration(0, 's')], - ['0d', new TimeDuration(0, 's')], + ['0m', new TimeDuration(0, 'm')], + ['0h', new TimeDuration(0, 'h')], + ['0d', new TimeDuration(0, 'd')], ['+0s', new TimeDuration(0, 's')], - ['+0m', new TimeDuration(0, 's')], - ['+0h', new TimeDuration(0, 's')], - ['+0d', new TimeDuration(0, 's')], - ['-0s', new TimeDuration(0, 's')], - ['-0m', new TimeDuration(0, 's')], - ['-0h', new TimeDuration(0, 's')], - ['-0d', new TimeDuration(0, 's')], + ['+0m', new TimeDuration(0, 'm')], + ['+0h', new TimeDuration(0, 'h')], + ['+0d', new TimeDuration(0, 'd')], + ['-0s', new TimeDuration(-0, 's')], + ['-0m', new TimeDuration(-0, 'm')], + ['-0h', new TimeDuration(-0, 'h')], + ['-0d', new TimeDuration(-0, 'd')], ])('parses "%s"', (duration, expectedTimeDuration) => { const result = TimeDuration.parse(duration); @@ -160,68 +160,40 @@ describe('TimeDuration', () => { ); }); - describe('toNormalizedTimeDuration', () => { + describe('toDesiredTimeUnit', () => { it.each([ - [new TimeDuration(5, 's'), new TimeDuration(5, 's')], - [new TimeDuration(65, 's'), new TimeDuration(65, 's')], - [new TimeDuration(600, 's'), new TimeDuration(10, 'm')], - [new TimeDuration(650, 's'), new TimeDuration(650, 's')], - [new TimeDuration(90, 'm'), new TimeDuration(90, 'm')], - [new TimeDuration(25200, 's'), new TimeDuration(7, 'h')], - [new TimeDuration(120, 'm'), new TimeDuration(2, 'h')], - [new TimeDuration(36, 'h'), new TimeDuration(36, 'h')], - [new TimeDuration(777600, 's'), new TimeDuration(9, 'd')], - [new TimeDuration(5184000, 's'), new TimeDuration(60, 'd')], - [new TimeDuration(1440, 'm'), new TimeDuration(1, 'd')], - [new TimeDuration(48, 'h'), new TimeDuration(2, 'd')], - [new TimeDuration(-5, 's'), new TimeDuration(-5, 's')], - [new TimeDuration(-65, 's'), new TimeDuration(-65, 's')], - [new TimeDuration(-600, 's'), new TimeDuration(-10, 'm')], - [new TimeDuration(-650, 's'), new TimeDuration(-650, 's')], - [new TimeDuration(-90, 'm'), new TimeDuration(-90, 'm')], - [new TimeDuration(-25200, 's'), new TimeDuration(-7, 'h')], - [new TimeDuration(-120, 'm'), new TimeDuration(-2, 'h')], - [new TimeDuration(-36, 'h'), new TimeDuration(-36, 'h')], - [new TimeDuration(-777600, 's'), new TimeDuration(-9, 'd')], - [new TimeDuration(-5184000, 's'), new TimeDuration(-60, 'd')], - [new TimeDuration(-1440, 'm'), new TimeDuration(-1, 'd')], - [new TimeDuration(-48, 'h'), new TimeDuration(-2, 'd')], - ])('converts %j to normalized time duration %j', (timeDuration, expected) => { - const result = timeDuration.toNormalizedTimeDuration(); - - expect(result).toEqual(expected); - }); - - it.each([ - [new TimeDuration(0, 's')], - [new TimeDuration(0, 'm')], - [new TimeDuration(0, 'h')], - [new TimeDuration(0, 'd')], - ])('converts %j to 0s', (timeDuration) => { - const result = timeDuration.toNormalizedTimeDuration(); - - expect(result).toEqual(new TimeDuration(0, 's')); - }); + [new TimeDuration(60, 's'), 'm', new TimeDuration(1, 'm')], + [new TimeDuration(60, 'm'), 'h', new TimeDuration(1, 'h')], + [new TimeDuration(24, 'h'), 'd', new TimeDuration(1, 'd')], + [new TimeDuration(3600, 's'), 'm', new TimeDuration(60, 'm')], + [new TimeDuration(86400, 's'), 'm', new TimeDuration(1440, 'm')], + [new TimeDuration(1440, 'm'), 'h', new TimeDuration(24, 'h')], + [new TimeDuration(1, 'd'), 's', new TimeDuration(86400, 's')], + [new TimeDuration(1, 'd'), 'm', new TimeDuration(1440, 'm')], + [new TimeDuration(1, 'd'), 'h', new TimeDuration(24, 'h')], + [new TimeDuration(1, 'h'), 's', new TimeDuration(3600, 's')], + [new TimeDuration(1, 'h'), 'm', new TimeDuration(60, 'm')], + [new TimeDuration(1, 'm'), 's', new TimeDuration(60, 's')], + ] as const)( + 'transforms %j to desired time unit %s', + (timeDuration, desiredTimeUnit, expected) => { + const result = timeDuration.toDesiredTimeUnit(desiredTimeUnit); + + expect(result).toEqual(expected); + } + ); it.each([ - // @ts-expect-error testing invalid unit - [new TimeDuration(1, 'S')], - // @ts-expect-error testing invalid unit - [new TimeDuration(2, 'M')], - // @ts-expect-error testing invalid unit - [new TimeDuration(3, 'H')], - // @ts-expect-error testing invalid unit - [new TimeDuration(4, 'D')], - // @ts-expect-error testing invalid unit - [new TimeDuration(5, 'Y')], - // @ts-expect-error testing invalid unit - [new TimeDuration(7, 'nanos')], - // @ts-expect-error testing invalid unit - [new TimeDuration(8, 'ms')], - // @ts-expect-error testing invalid unit - [new TimeDuration(0, 'invalid')], - ])('returns %j unchanged', (timeDuration) => { - const result = timeDuration.toNormalizedTimeDuration(); + [new TimeDuration(61, 's'), 'm'], + [new TimeDuration(61, 'm'), 'h'], + [new TimeDuration(25, 'h'), 'd'], + [new TimeDuration(3601, 's'), 'm'], + [new TimeDuration(86401, 's'), 'm'], + [new TimeDuration(1441, 'm'), 'h'], + // @ts-expect-error testing invalid time unit + [new TimeDuration(1441, 'invalid'), 'h'], + ] as const)('returns %j as is', (timeDuration, desiredTimeUnit) => { + const result = timeDuration.toDesiredTimeUnit(desiredTimeUnit); expect(result).toEqual(timeDuration); }); diff --git a/x-pack/solutions/security/packages/kbn-securitysolution-utils/src/time_duration/time_duration.ts b/x-pack/solutions/security/packages/kbn-securitysolution-utils/src/time_duration/time_duration.ts index 540368da5067e..e0864e3ea4e69 100644 --- a/x-pack/solutions/security/packages/kbn-securitysolution-utils/src/time_duration/time_duration.ts +++ b/x-pack/solutions/security/packages/kbn-securitysolution-utils/src/time_duration/time_duration.ts @@ -18,7 +18,7 @@ export class TimeDuration { * Constructs a time duration from milliseconds. The output is normalized. */ static fromMilliseconds(ms: number): TimeDuration { - return new TimeDuration(Math.round(ms / 1000), 's').toNormalizedTimeDuration(); + return new TimeDuration(Math.round(ms / 1000), 's'); } /* @@ -41,7 +41,7 @@ export class TimeDuration { const value = parseInt(matchArray[1], 10); const unit = matchArray[2] as TimeDuration['unit']; - return new TimeDuration(value, unit).toNormalizedTimeDuration(); + return new TimeDuration(value, unit); } constructor(public value: number, public unit: TimeDurationUnits) {} @@ -70,36 +70,31 @@ export class TimeDuration { } /** - * Converts time duration to the largest possible units. E.g. - * - 60s transformed to 1m - * - 3600s transformed to 1h - * - 1440m transformed to 1d + * Tries to transform the current time duration to desired time unit. + * When it's possible returns transformed time duration instance. + * Otherwise returns the same instance. */ - toNormalizedTimeDuration(): TimeDuration { + toDesiredTimeUnit(desiredTimeUnit: TimeDurationUnits): TimeDuration { const ms = this.toMilliseconds(); if (ms === undefined) { return this; } - if (ms === 0) { - return new TimeDuration(0, 's'); + if (desiredTimeUnit === 's' && ms % 1000 === 0) { + return new TimeDuration(ms / 1000, 's'); } - if (ms % (3600000 * 24) === 0) { - return new TimeDuration(ms / (3600000 * 24), 'd'); + if (desiredTimeUnit === 'm' && ms % 60000 === 0) { + return new TimeDuration(ms / 60000, 'm'); } - if (ms % 3600000 === 0) { + if (desiredTimeUnit === 'h' && ms % 3600000 === 0) { return new TimeDuration(ms / 3600000, 'h'); } - if (ms % 60000 === 0) { - return new TimeDuration(ms / 60000, 'm'); - } - - if (ms % 1000 === 0) { - return new TimeDuration(ms / 1000, 's'); + if (desiredTimeUnit === 'd' && ms % (3600000 * 24) === 0) { + return new TimeDuration(ms / (3600000 * 24), 'd'); } return this; diff --git a/x-pack/solutions/security/plugins/security_solution/common/api/detection_engine/model/rule_schema/to_simple_rule_schedule.test.ts b/x-pack/solutions/security/plugins/security_solution/common/api/detection_engine/model/rule_schema/to_simple_rule_schedule.test.ts index c2383090a466a..1616e68020ace 100644 --- a/x-pack/solutions/security/plugins/security_solution/common/api/detection_engine/model/rule_schema/to_simple_rule_schedule.test.ts +++ b/x-pack/solutions/security/plugins/security_solution/common/api/detection_engine/model/rule_schema/to_simple_rule_schedule.test.ts @@ -23,19 +23,19 @@ describe('toSimpleRuleSchedule', () => { ], [ { interval: '60s', from: 'now-2m', to: 'now' }, - { interval: '60s', lookback: '1m' }, + { interval: '60s', lookback: '60s' }, ], [ { interval: '60s', from: 'now-2h', to: 'now' }, - { interval: '60s', lookback: '119m' }, + { interval: '60s', lookback: '7140s' }, ], [ { interval: '60m', from: 'now-3h', to: 'now' }, - { interval: '60m', lookback: '2h' }, + { interval: '60m', lookback: '120m' }, ], [ { interval: '3600s', from: 'now-5h', to: 'now' }, - { interval: '3600s', lookback: '4h' }, + { interval: '3600s', lookback: '14400s' }, ], [ { interval: '1m', from: 'now-120s', to: 'now' }, @@ -55,11 +55,11 @@ describe('toSimpleRuleSchedule', () => { ], [ { interval: '30m', from: 'now-30m', to: 'now' }, - { interval: '30m', lookback: '0s' }, + { interval: '30m', lookback: '0m' }, ], [ { interval: '1h', from: 'now-1h', to: 'now' }, - { interval: '1h', lookback: '0s' }, + { interval: '1h', lookback: '0h' }, ], [ { interval: '60s', from: 'now-1m', to: 'now' }, @@ -67,19 +67,19 @@ describe('toSimpleRuleSchedule', () => { ], [ { interval: '60m', from: 'now-1h', to: 'now' }, - { interval: '60m', lookback: '0s' }, + { interval: '60m', lookback: '0m' }, ], [ { interval: '1m', from: 'now-60s', to: 'now' }, - { interval: '1m', lookback: '0s' }, + { interval: '1m', lookback: '0m' }, ], [ { interval: '1h', from: 'now-60m', to: 'now' }, - { interval: '1h', lookback: '0s' }, + { interval: '1h', lookback: '0h' }, ], [ { interval: '1h', from: 'now-3600s', to: 'now' }, - { interval: '1h', lookback: '0s' }, + { interval: '1h', lookback: '0h' }, ], [ { interval: '0s', from: 'now', to: 'now' }, @@ -91,6 +91,24 @@ describe('toSimpleRuleSchedule', () => { expect(result).toEqual(expected); }); + it.each([ + [ + { interval: '100s', from: 'now-400s', to: 'now', meta: { from: '3m' } }, + { interval: '100s', lookback: '5m' }, + ], + [ + { interval: '100m', from: 'now-400m', to: 'now', meta: { from: '5h' } }, + { interval: '100m', lookback: '5h' }, + ], + ])( + 'transforms %j to simple rule schedule with desired time unit', + (fullRuleSchedule, expected) => { + const result = toSimpleRuleSchedule(fullRuleSchedule); + + expect(result).toEqual(expected); + } + ); + it.each([ [{ interval: 'invalid', from: 'now-11m', to: 'now' }], [{ interval: '10m', from: 'invalid', to: 'now' }], diff --git a/x-pack/solutions/security/plugins/security_solution/common/api/detection_engine/model/rule_schema/to_simple_rule_schedule.ts b/x-pack/solutions/security/plugins/security_solution/common/api/detection_engine/model/rule_schema/to_simple_rule_schedule.ts index 5227099e63111..0ae49001de70b 100644 --- a/x-pack/solutions/security/plugins/security_solution/common/api/detection_engine/model/rule_schema/to_simple_rule_schedule.ts +++ b/x-pack/solutions/security/plugins/security_solution/common/api/detection_engine/model/rule_schema/to_simple_rule_schedule.ts @@ -6,15 +6,23 @@ */ import { calcDateMathDiff } from '@kbn/securitysolution-utils/date_math'; -import { TimeDuration as TimeDurationUtil } from '@kbn/securitysolution-utils/time_duration'; +import { TimeDuration } from '@kbn/securitysolution-utils/time_duration'; import type { RuleSchedule, SimpleRuleSchedule } from './rule_schedule'; +interface RuleScheduleMeta { + meta?: { + from?: string; + }; +} + /** * Transforms RuleSchedule to SimpleRuleSchedule by replacing `from` and `to` with `lookback`. * * The transformation is only possible when `to` equals to `now` and result `lookback` is non-negative. */ -export function toSimpleRuleSchedule(ruleSchedule: RuleSchedule): SimpleRuleSchedule | undefined { +export function toSimpleRuleSchedule( + ruleSchedule: RuleSchedule & RuleScheduleMeta +): SimpleRuleSchedule | undefined { if (ruleSchedule.to !== 'now') { return undefined; } @@ -25,8 +33,17 @@ export function toSimpleRuleSchedule(ruleSchedule: RuleSchedule): SimpleRuleSche return undefined; } + // Rule meta may contain non-normalized user entered look-back value + // It's required to extract time units to restore desired time units + // in form inputs. + const rawLookBack = TimeDuration.parse(ruleSchedule.meta?.from ?? ''); + + const interval = TimeDuration.parse(ruleSchedule.interval); + return { interval: ruleSchedule.interval, - lookback: TimeDurationUtil.fromMilliseconds(lookBackMs).toString(), + lookback: TimeDuration.fromMilliseconds(lookBackMs) + .toDesiredTimeUnit(rawLookBack?.unit ?? interval?.unit ?? 's') + .toString(), }; } diff --git a/x-pack/solutions/security/plugins/security_solution/public/detection_engine/rule_creation_ui/pages/rule_creation/helpers.test.ts b/x-pack/solutions/security/plugins/security_solution/public/detection_engine/rule_creation_ui/pages/rule_creation/helpers.test.ts index 8d9de3d305ab2..d8815d7d109b2 100644 --- a/x-pack/solutions/security/plugins/security_solution/public/detection_engine/rule_creation_ui/pages/rule_creation/helpers.test.ts +++ b/x-pack/solutions/security/plugins/security_solution/public/detection_engine/rule_creation_ui/pages/rule_creation/helpers.test.ts @@ -588,7 +588,7 @@ describe('helpers', () => { test('returns formatted object as ScheduleStepRuleJson', () => { const result = formatScheduleStepData(mockData); const expected: ScheduleStepRuleJson = { - from: 'now-11m', + from: 'now-660s', to: 'now', interval: '5m', meta: { @@ -606,7 +606,7 @@ describe('helpers', () => { delete mockStepData.to; const result = formatScheduleStepData(mockStepData); const expected: ScheduleStepRuleJson = { - from: 'now-11m', + from: 'now-660s', to: 'now', interval: '5m', meta: { @@ -624,7 +624,7 @@ describe('helpers', () => { }; const result = formatScheduleStepData(mockStepData); const expected: ScheduleStepRuleJson = { - from: 'now-11m', + from: 'now-660s', to: 'now', interval: '5m', meta: { diff --git a/x-pack/solutions/security/plugins/security_solution/public/detection_engine/rule_creation_ui/pages/rule_creation/helpers.ts b/x-pack/solutions/security/plugins/security_solution/public/detection_engine/rule_creation_ui/pages/rule_creation/helpers.ts index 1788247cacce7..741e896a89d54 100644 --- a/x-pack/solutions/security/plugins/security_solution/public/detection_engine/rule_creation_ui/pages/rule_creation/helpers.ts +++ b/x-pack/solutions/security/plugins/security_solution/public/detection_engine/rule_creation_ui/pages/rule_creation/helpers.ts @@ -586,6 +586,7 @@ export const formatScheduleStepData = (scheduleData: ScheduleStepRule): Schedule return { ...formatScheduleData, meta: { + // User entered look-back value. It's used to persist time units. from: scheduleData.from, }, }; diff --git a/x-pack/solutions/security/plugins/security_solution/public/detection_engine/rule_management/components/rule_details/per_field_diff/get_field_diffs_for_grouped_fields.test.ts b/x-pack/solutions/security/plugins/security_solution/public/detection_engine/rule_management/components/rule_details/per_field_diff/get_field_diffs_for_grouped_fields.test.ts index 597878ce1f797..bc7d72692c936 100644 --- a/x-pack/solutions/security/plugins/security_solution/public/detection_engine/rule_management/components/rule_details/per_field_diff/get_field_diffs_for_grouped_fields.test.ts +++ b/x-pack/solutions/security/plugins/security_solution/public/detection_engine/rule_management/components/rule_details/per_field_diff/get_field_diffs_for_grouped_fields.test.ts @@ -9,7 +9,7 @@ import type { RuleSchedule } from '../../../../../../common/api/detection_engine import type { ThreeWayDiff } from '../../../../../../common/api/detection_engine'; import { getFieldDiffsForRuleSchedule } from './get_field_diffs_for_grouped_fields'; -describe('getFieldDiffsForRulSchedule', () => { +describe('getFieldDiffsForRuleSchedule', () => { describe('full rule schedule', () => { it('returns interval diff', () => { const result = getFieldDiffsForRuleSchedule({ @@ -308,7 +308,7 @@ describe('getFieldDiffsForRulSchedule', () => { { fieldName: 'lookback', currentVersion: '1m', - targetVersion: '0s', + targetVersion: '0m', }, ]); }); @@ -332,7 +332,7 @@ describe('getFieldDiffsForRulSchedule', () => { { fieldName: 'lookback', currentVersion: '', - targetVersion: '0s', + targetVersion: '0m', }, ]); }); diff --git a/x-pack/solutions/security/plugins/security_solution/public/detection_engine/rule_management/components/rule_details/rule_schedule_section.tsx b/x-pack/solutions/security/plugins/security_solution/public/detection_engine/rule_management/components/rule_details/rule_schedule_section.tsx index 9eba3395b36fa..3f2256a94a78a 100644 --- a/x-pack/solutions/security/plugins/security_solution/public/detection_engine/rule_management/components/rule_details/rule_schedule_section.tsx +++ b/x-pack/solutions/security/plugins/security_solution/public/detection_engine/rule_management/components/rule_details/rule_schedule_section.tsx @@ -64,6 +64,7 @@ export const RuleScheduleSection = ({ interval: rule.interval, from: rule.from, to, + meta: rule.meta, }); const ruleSectionListItems = !simpleRuleSchedule diff --git a/x-pack/solutions/security/plugins/security_solution/public/detections/pages/detection_engine/rules/helpers.test.tsx b/x-pack/solutions/security/plugins/security_solution/public/detections/pages/detection_engine/rules/helpers.test.tsx index 2152d10115b2d..d93389a427757 100644 --- a/x-pack/solutions/security/plugins/security_solution/public/detections/pages/detection_engine/rules/helpers.test.tsx +++ b/x-pack/solutions/security/plugins/security_solution/public/detections/pages/detection_engine/rules/helpers.test.tsx @@ -157,7 +157,7 @@ describe('rule helpers', () => { maxSignals: 100, setup: '# this is some setup documentation', }; - const scheduleRuleStepData = { from: '0s', interval: '5m' }; + const scheduleRuleStepData = { from: '0m', interval: '5m' }; const ruleActionsStepData = { enabled: true, actions: [], @@ -335,7 +335,7 @@ describe('rule helpers', () => { const result: ScheduleStepRule = getScheduleStepsData(mockedRule); const expected = { interval: mockedRule.interval, - from: '0s', + from: '0m', }; expect(result).toEqual(expected);