From e1d3311a2b1e74e6f0463633b743a528b76b8d15 Mon Sep 17 00:00:00 2001 From: jpdjere Date: Mon, 22 Jul 2024 16:30:00 +0200 Subject: [PATCH] Handled missing base version scenarios --- .../detection_engine/prebuilt_rules/index.ts | 1 - .../diff/three_way_diff/three_way_diff.ts | 9 --- .../three_way_diff/three_way_merge_outcome.ts | 26 -------- .../multi_line_string_diff_algorithm.test.ts | 15 +---- .../multi_line_string_diff_algorithm.ts | 63 ++++++++++--------- .../algorithms/number_diff_algorithm.test.ts | 14 +---- .../scalar_array_diff_algorithm.test.ts | 23 ++----- .../algorithms/scalar_array_diff_algorithm.ts | 61 +++++++++--------- .../algorithms/simple_diff_algorithm.ts | 47 ++++++++------ .../single_line_string_diff_algorithm.test.ts | 14 ++--- 10 files changed, 108 insertions(+), 165 deletions(-) delete mode 100644 x-pack/plugins/security_solution/common/api/detection_engine/prebuilt_rules/model/diff/three_way_diff/three_way_merge_outcome.ts diff --git a/x-pack/plugins/security_solution/common/api/detection_engine/prebuilt_rules/index.ts b/x-pack/plugins/security_solution/common/api/detection_engine/prebuilt_rules/index.ts index 08d91061f58e8..86956c001843e 100644 --- a/x-pack/plugins/security_solution/common/api/detection_engine/prebuilt_rules/index.ts +++ b/x-pack/plugins/security_solution/common/api/detection_engine/prebuilt_rules/index.ts @@ -21,4 +21,3 @@ export * from './model/diff/rule_diff/rule_diff'; export * from './model/diff/three_way_diff/three_way_diff_outcome'; export * from './model/diff/three_way_diff/three_way_diff'; export * from './model/diff/three_way_diff/three_way_diff_conflict'; -export * from './model/diff/three_way_diff/three_way_merge_outcome'; diff --git a/x-pack/plugins/security_solution/common/api/detection_engine/prebuilt_rules/model/diff/three_way_diff/three_way_diff.ts b/x-pack/plugins/security_solution/common/api/detection_engine/prebuilt_rules/model/diff/three_way_diff/three_way_diff.ts index 7c84047e15bf4..2fcb86d780f02 100644 --- a/x-pack/plugins/security_solution/common/api/detection_engine/prebuilt_rules/model/diff/three_way_diff/three_way_diff.ts +++ b/x-pack/plugins/security_solution/common/api/detection_engine/prebuilt_rules/model/diff/three_way_diff/three_way_diff.ts @@ -6,7 +6,6 @@ */ import type { ThreeWayDiffOutcome } from './three_way_diff_outcome'; -import type { ThreeWayMergeOutcome } from './three_way_merge_outcome'; import type { ThreeWayDiffConflict } from './three_way_diff_conflict'; /** @@ -111,14 +110,6 @@ export interface ThreeWayDiff { */ diff_outcome: ThreeWayDiffOutcome; - /** - * The type of result of an automatic three-way merge of three values: - * - base version - * - current version - * - target version - */ - merge_outcome: ThreeWayMergeOutcome; - /** * Boolean which determines if a base version was found and returned for the three-way-diff of the field * - true: the base version of the field was found and is either defined or undefined diff --git a/x-pack/plugins/security_solution/common/api/detection_engine/prebuilt_rules/model/diff/three_way_diff/three_way_merge_outcome.ts b/x-pack/plugins/security_solution/common/api/detection_engine/prebuilt_rules/model/diff/three_way_diff/three_way_merge_outcome.ts deleted file mode 100644 index 5c2caea0130dd..0000000000000 --- a/x-pack/plugins/security_solution/common/api/detection_engine/prebuilt_rules/model/diff/three_way_diff/three_way_merge_outcome.ts +++ /dev/null @@ -1,26 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the Elastic License - * 2.0; you may not use this file except in compliance with the Elastic License - * 2.0. - */ - -/** - * Type of result of an automatic three-way merge of three values: - * - base version - * - current version - * - target version - */ -export enum ThreeWayMergeOutcome { - /** Took current version and returned as the merged one. */ - Current = 'CURRENT', - - /** Took target version and returned as the merged one. */ - Target = 'TARGET', - - /** Merged three versions successfully into a new one. */ - Merged = 'MERGED', - - /** Couldn't merge three versions because of a conflict. */ - Conflict = 'CONFLICT', -} diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/prebuilt_rules/logic/diff/calculation/algorithms/multi_line_string_diff_algorithm.test.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/prebuilt_rules/logic/diff/calculation/algorithms/multi_line_string_diff_algorithm.test.ts index f9770f4f70a99..cc9c73d5c9a1a 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/prebuilt_rules/logic/diff/calculation/algorithms/multi_line_string_diff_algorithm.test.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/prebuilt_rules/logic/diff/calculation/algorithms/multi_line_string_diff_algorithm.test.ts @@ -8,7 +8,6 @@ import type { ThreeVersionsOf } from '../../../../../../../../common/api/detection_engine'; import { ThreeWayDiffOutcome, - ThreeWayMergeOutcome, MissingVersion, ThreeWayDiffConflict, } from '../../../../../../../../common/api/detection_engine'; @@ -28,7 +27,6 @@ describe('multiLineStringDiffAlgorithm', () => { expect.objectContaining({ merged_version: mockVersions.current_version, diff_outcome: ThreeWayDiffOutcome.StockValueNoUpdate, - merge_outcome: ThreeWayMergeOutcome.Current, conflict: ThreeWayDiffConflict.NONE, }) ); @@ -47,7 +45,6 @@ describe('multiLineStringDiffAlgorithm', () => { expect.objectContaining({ merged_version: mockVersions.current_version, diff_outcome: ThreeWayDiffOutcome.CustomizedValueNoUpdate, - merge_outcome: ThreeWayMergeOutcome.Current, conflict: ThreeWayDiffConflict.NONE, }) ); @@ -66,7 +63,6 @@ describe('multiLineStringDiffAlgorithm', () => { expect.objectContaining({ merged_version: mockVersions.target_version, diff_outcome: ThreeWayDiffOutcome.StockValueCanUpdate, - merge_outcome: ThreeWayMergeOutcome.Target, conflict: ThreeWayDiffConflict.NONE, }) ); @@ -85,7 +81,6 @@ describe('multiLineStringDiffAlgorithm', () => { expect.objectContaining({ merged_version: mockVersions.current_version, diff_outcome: ThreeWayDiffOutcome.CustomizedValueSameUpdate, - merge_outcome: ThreeWayMergeOutcome.Current, conflict: ThreeWayDiffConflict.NONE, }) ); @@ -107,7 +102,6 @@ describe('multiLineStringDiffAlgorithm', () => { expect.objectContaining({ merged_version: expectedMergedVersion, diff_outcome: ThreeWayDiffOutcome.CustomizedValueCanUpdate, - merge_outcome: ThreeWayMergeOutcome.Merged, conflict: ThreeWayDiffConflict.SOLVABLE, }) ); @@ -126,7 +120,6 @@ describe('multiLineStringDiffAlgorithm', () => { expect.objectContaining({ merged_version: mockVersions.current_version, diff_outcome: ThreeWayDiffOutcome.CustomizedValueCanUpdate, - merge_outcome: ThreeWayMergeOutcome.Conflict, conflict: ThreeWayDiffConflict.NON_SOLVABLE, }) ); @@ -146,10 +139,9 @@ describe('multiLineStringDiffAlgorithm', () => { expect(result).toEqual( expect.objectContaining({ has_base_version: false, - base_version: MissingVersion, + base_version: undefined, merged_version: mockVersions.current_version, diff_outcome: ThreeWayDiffOutcome.StockValueNoUpdate, - merge_outcome: ThreeWayMergeOutcome.Current, conflict: ThreeWayDiffConflict.NONE, }) ); @@ -167,11 +159,10 @@ describe('multiLineStringDiffAlgorithm', () => { expect(result).toEqual( expect.objectContaining({ has_base_version: false, - base_version: MissingVersion, + base_version: undefined, merged_version: mockVersions.target_version, diff_outcome: ThreeWayDiffOutcome.StockValueCanUpdate, - merge_outcome: ThreeWayMergeOutcome.Target, - conflict: ThreeWayDiffConflict.NONE, + conflict: ThreeWayDiffConflict.SOLVABLE, }) ); }); diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/prebuilt_rules/logic/diff/calculation/algorithms/multi_line_string_diff_algorithm.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/prebuilt_rules/logic/diff/calculation/algorithms/multi_line_string_diff_algorithm.ts index 040ba93d8145f..30ff8fb37ef6e 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/prebuilt_rules/logic/diff/calculation/algorithms/multi_line_string_diff_algorithm.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/prebuilt_rules/logic/diff/calculation/algorithms/multi_line_string_diff_algorithm.ts @@ -15,7 +15,6 @@ import { determineDiffOutcome, determineIfValueCanUpdate, ThreeWayDiffOutcome, - ThreeWayMergeOutcome, MissingVersion, ThreeWayDiffConflict, } from '../../../../../../../../common/api/detection_engine/prebuilt_rules'; @@ -35,13 +34,15 @@ export const multiLineStringDiffAlgorithm = ( const diffOutcome = determineDiffOutcome(baseVersion, currentVersion, targetVersion); const valueCanUpdate = determineIfValueCanUpdate(diffOutcome); - const { mergeOutcome, mergedVersion } = mergeVersions({ + const hasBaseVersion = baseVersion !== MissingVersion; + + const { conflict, mergedVersion } = mergeVersions({ + hasBaseVersion, baseVersion, currentVersion, targetVersion, diffOutcome, }); - const hasBaseVersion = baseVersion !== MissingVersion; return { has_base_version: hasBaseVersion, @@ -51,18 +52,18 @@ export const multiLineStringDiffAlgorithm = ( merged_version: mergedVersion, diff_outcome: diffOutcome, - merge_outcome: mergeOutcome, + conflict, has_update: valueCanUpdate, - conflict: determineConflictType(mergeOutcome), }; }; interface MergeResult { - mergeOutcome: ThreeWayMergeOutcome; mergedVersion: string; + conflict: ThreeWayDiffConflict; } interface MergeArgs { + hasBaseVersion: boolean; baseVersion: string | MissingVersion; currentVersion: string; targetVersion: string; @@ -70,44 +71,54 @@ interface MergeArgs { } const mergeVersions = ({ + hasBaseVersion, baseVersion, currentVersion, targetVersion, diffOutcome, }: MergeArgs): MergeResult => { switch (diffOutcome) { - case ThreeWayDiffOutcome.StockValueNoUpdate: - case ThreeWayDiffOutcome.CustomizedValueNoUpdate: - case ThreeWayDiffOutcome.CustomizedValueSameUpdate: { + case ThreeWayDiffOutcome.StockValueNoUpdate: // Scenarios AAA and -AA + case ThreeWayDiffOutcome.CustomizedValueNoUpdate: // Scenario ABA + case ThreeWayDiffOutcome.CustomizedValueSameUpdate: // Scenario ABB return { - mergeOutcome: ThreeWayMergeOutcome.Current, + conflict: ThreeWayDiffConflict.NONE, mergedVersion: currentVersion, }; - } + case ThreeWayDiffOutcome.StockValueCanUpdate: { + if (!hasBaseVersion) { + // Scenario -AB. Treated as scenario ABC, returns target + // version and marked as "SOLVABLE" conflict. + // https://github.com/elastic/kibana/pull/184889#discussion_r1636421293 + return { + mergedVersion: targetVersion, + conflict: ThreeWayDiffConflict.SOLVABLE, + }; + } + + // Scenario AAB return { - mergeOutcome: ThreeWayMergeOutcome.Target, + conflict: ThreeWayDiffConflict.NONE, mergedVersion: targetVersion, }; } + + // Scenario ABC case ThreeWayDiffOutcome.CustomizedValueCanUpdate: { - if (baseVersion === MissingVersion) { - return { - mergeOutcome: ThreeWayMergeOutcome.Conflict, - mergedVersion: currentVersion, - }; - } - const mergedVersion = merge(currentVersion, baseVersion, targetVersion, { + // TS does not realize that in ABC scenario, baseVersion cannot be missing + // Missing baseVersion scenarios were handled as -AA and -AB. + const mergedVersion = merge(currentVersion, baseVersion as string, targetVersion, { stringSeparator: /(\S+|\s+)/g, // Retains all whitespace, which we keep to preserve formatting }); return mergedVersion.conflict ? { - mergeOutcome: ThreeWayMergeOutcome.Conflict, + conflict: ThreeWayDiffConflict.NON_SOLVABLE, mergedVersion: currentVersion, } : { - mergeOutcome: ThreeWayMergeOutcome.Merged, + conflict: ThreeWayDiffConflict.SOLVABLE, mergedVersion: mergedVersion.result.join(''), }; } @@ -115,13 +126,3 @@ const mergeVersions = ({ return assertUnreachable(diffOutcome); } }; - -const determineConflictType = (mergeOutcome: ThreeWayMergeOutcome) => { - if (mergeOutcome === ThreeWayMergeOutcome.Conflict) { - return ThreeWayDiffConflict.NON_SOLVABLE; - } - if (mergeOutcome === ThreeWayMergeOutcome.Merged) { - return ThreeWayDiffConflict.SOLVABLE; - } - return ThreeWayDiffConflict.NONE; -}; diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/prebuilt_rules/logic/diff/calculation/algorithms/number_diff_algorithm.test.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/prebuilt_rules/logic/diff/calculation/algorithms/number_diff_algorithm.test.ts index af87cf59058fe..36d8875a55eed 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/prebuilt_rules/logic/diff/calculation/algorithms/number_diff_algorithm.test.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/prebuilt_rules/logic/diff/calculation/algorithms/number_diff_algorithm.test.ts @@ -8,7 +8,6 @@ import type { ThreeVersionsOf } from '../../../../../../../../common/api/detection_engine'; import { ThreeWayDiffOutcome, - ThreeWayMergeOutcome, MissingVersion, ThreeWayDiffConflict, } from '../../../../../../../../common/api/detection_engine'; @@ -28,7 +27,6 @@ describe('numberDiffAlgorithm', () => { expect.objectContaining({ merged_version: mockVersions.current_version, diff_outcome: ThreeWayDiffOutcome.StockValueNoUpdate, - merge_outcome: ThreeWayMergeOutcome.Current, conflict: ThreeWayDiffConflict.NONE, }) ); @@ -47,7 +45,6 @@ describe('numberDiffAlgorithm', () => { expect.objectContaining({ merged_version: mockVersions.current_version, diff_outcome: ThreeWayDiffOutcome.CustomizedValueNoUpdate, - merge_outcome: ThreeWayMergeOutcome.Current, conflict: ThreeWayDiffConflict.NONE, }) ); @@ -66,7 +63,6 @@ describe('numberDiffAlgorithm', () => { expect.objectContaining({ merged_version: mockVersions.target_version, diff_outcome: ThreeWayDiffOutcome.StockValueCanUpdate, - merge_outcome: ThreeWayMergeOutcome.Target, conflict: ThreeWayDiffConflict.NONE, }) ); @@ -85,7 +81,6 @@ describe('numberDiffAlgorithm', () => { expect.objectContaining({ merged_version: mockVersions.current_version, diff_outcome: ThreeWayDiffOutcome.CustomizedValueSameUpdate, - merge_outcome: ThreeWayMergeOutcome.Current, conflict: ThreeWayDiffConflict.NONE, }) ); @@ -104,7 +99,6 @@ describe('numberDiffAlgorithm', () => { expect.objectContaining({ merged_version: mockVersions.current_version, diff_outcome: ThreeWayDiffOutcome.CustomizedValueCanUpdate, - merge_outcome: ThreeWayMergeOutcome.Conflict, conflict: ThreeWayDiffConflict.NON_SOLVABLE, }) ); @@ -123,10 +117,9 @@ describe('numberDiffAlgorithm', () => { expect(result).toEqual( expect.objectContaining({ has_base_version: false, - base_version: MissingVersion, + base_version: undefined, merged_version: mockVersions.current_version, diff_outcome: ThreeWayDiffOutcome.StockValueNoUpdate, - merge_outcome: ThreeWayMergeOutcome.Current, conflict: ThreeWayDiffConflict.NONE, }) ); @@ -144,11 +137,10 @@ describe('numberDiffAlgorithm', () => { expect(result).toEqual( expect.objectContaining({ has_base_version: false, - base_version: MissingVersion, + base_version: undefined, merged_version: mockVersions.target_version, diff_outcome: ThreeWayDiffOutcome.StockValueCanUpdate, - merge_outcome: ThreeWayMergeOutcome.Target, - conflict: ThreeWayDiffConflict.NONE, + conflict: ThreeWayDiffConflict.SOLVABLE, }) ); }); diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/prebuilt_rules/logic/diff/calculation/algorithms/scalar_array_diff_algorithm.test.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/prebuilt_rules/logic/diff/calculation/algorithms/scalar_array_diff_algorithm.test.ts index ee9d684b94cae..b8aaffb6fd771 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/prebuilt_rules/logic/diff/calculation/algorithms/scalar_array_diff_algorithm.test.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/prebuilt_rules/logic/diff/calculation/algorithms/scalar_array_diff_algorithm.test.ts @@ -8,7 +8,6 @@ import type { ThreeVersionsOf } from '../../../../../../../../common/api/detection_engine'; import { ThreeWayDiffOutcome, - ThreeWayMergeOutcome, MissingVersion, ThreeWayDiffConflict, } from '../../../../../../../../common/api/detection_engine'; @@ -28,7 +27,6 @@ describe('scalarArrayDiffAlgorithm', () => { expect.objectContaining({ merged_version: mockVersions.current_version, diff_outcome: ThreeWayDiffOutcome.StockValueNoUpdate, - merge_outcome: ThreeWayMergeOutcome.Current, conflict: ThreeWayDiffConflict.NONE, }) ); @@ -47,7 +45,6 @@ describe('scalarArrayDiffAlgorithm', () => { expect.objectContaining({ merged_version: mockVersions.current_version, diff_outcome: ThreeWayDiffOutcome.CustomizedValueNoUpdate, - merge_outcome: ThreeWayMergeOutcome.Current, conflict: ThreeWayDiffConflict.NONE, }) ); @@ -66,7 +63,6 @@ describe('scalarArrayDiffAlgorithm', () => { expect.objectContaining({ merged_version: mockVersions.target_version, diff_outcome: ThreeWayDiffOutcome.StockValueCanUpdate, - merge_outcome: ThreeWayMergeOutcome.Target, conflict: ThreeWayDiffConflict.NONE, }) ); @@ -85,7 +81,6 @@ describe('scalarArrayDiffAlgorithm', () => { expect.objectContaining({ merged_version: mockVersions.current_version, diff_outcome: ThreeWayDiffOutcome.CustomizedValueSameUpdate, - merge_outcome: ThreeWayMergeOutcome.Current, conflict: ThreeWayDiffConflict.NONE, }) ); @@ -105,7 +100,6 @@ describe('scalarArrayDiffAlgorithm', () => { expect.objectContaining({ merged_version: expectedMergedVersion, diff_outcome: ThreeWayDiffOutcome.CustomizedValueCanUpdate, - merge_outcome: ThreeWayMergeOutcome.Merged, conflict: ThreeWayDiffConflict.SOLVABLE, }) ); @@ -123,9 +117,10 @@ describe('scalarArrayDiffAlgorithm', () => { expect(result).toEqual( expect.objectContaining({ + has_base_version: false, + base_version: undefined, merged_version: mockVersions.current_version, diff_outcome: ThreeWayDiffOutcome.StockValueNoUpdate, - merge_outcome: ThreeWayMergeOutcome.Current, conflict: ThreeWayDiffConflict.NONE, }) ); @@ -142,10 +137,11 @@ describe('scalarArrayDiffAlgorithm', () => { expect(result).toEqual( expect.objectContaining({ + has_base_version: false, + base_version: undefined, merged_version: mockVersions.target_version, diff_outcome: ThreeWayDiffOutcome.StockValueCanUpdate, - merge_outcome: ThreeWayMergeOutcome.Target, - conflict: ThreeWayDiffConflict.NONE, + conflict: ThreeWayDiffConflict.SOLVABLE, }) ); }); @@ -165,7 +161,6 @@ describe('scalarArrayDiffAlgorithm', () => { expect.objectContaining({ merged_version: mockVersions.current_version, diff_outcome: ThreeWayDiffOutcome.StockValueNoUpdate, - merge_outcome: ThreeWayMergeOutcome.Current, conflict: ThreeWayDiffConflict.NONE, }) ); @@ -186,7 +181,6 @@ describe('scalarArrayDiffAlgorithm', () => { expect.objectContaining({ merged_version: expectedMergedVersion, diff_outcome: ThreeWayDiffOutcome.StockValueNoUpdate, - merge_outcome: ThreeWayMergeOutcome.Current, conflict: ThreeWayDiffConflict.NONE, }) ); @@ -206,7 +200,6 @@ describe('scalarArrayDiffAlgorithm', () => { expect.objectContaining({ merged_version: expectedMergedVersion, diff_outcome: ThreeWayDiffOutcome.StockValueNoUpdate, - merge_outcome: ThreeWayMergeOutcome.Current, conflict: ThreeWayDiffConflict.NONE, }) ); @@ -226,7 +219,6 @@ describe('scalarArrayDiffAlgorithm', () => { expect.objectContaining({ merged_version: expectedMergedVersion, diff_outcome: ThreeWayDiffOutcome.StockValueNoUpdate, - merge_outcome: ThreeWayMergeOutcome.Current, conflict: ThreeWayDiffConflict.NONE, }) ); @@ -246,7 +238,6 @@ describe('scalarArrayDiffAlgorithm', () => { expect.objectContaining({ merged_version: expectedMergedVersion, diff_outcome: ThreeWayDiffOutcome.CustomizedValueCanUpdate, - merge_outcome: ThreeWayMergeOutcome.Merged, conflict: ThreeWayDiffConflict.SOLVABLE, }) ); @@ -267,7 +258,6 @@ describe('scalarArrayDiffAlgorithm', () => { expect.objectContaining({ merged_version: mockVersions.current_version, diff_outcome: ThreeWayDiffOutcome.CustomizedValueSameUpdate, - merge_outcome: ThreeWayMergeOutcome.Current, conflict: ThreeWayDiffConflict.NONE, }) ); @@ -286,7 +276,6 @@ describe('scalarArrayDiffAlgorithm', () => { expect.objectContaining({ merged_version: mockVersions.current_version, diff_outcome: ThreeWayDiffOutcome.CustomizedValueNoUpdate, - merge_outcome: ThreeWayMergeOutcome.Current, conflict: ThreeWayDiffConflict.NONE, }) ); @@ -305,7 +294,6 @@ describe('scalarArrayDiffAlgorithm', () => { expect.objectContaining({ merged_version: mockVersions.target_version, diff_outcome: ThreeWayDiffOutcome.StockValueCanUpdate, - merge_outcome: ThreeWayMergeOutcome.Target, conflict: ThreeWayDiffConflict.NONE, }) ); @@ -324,7 +312,6 @@ describe('scalarArrayDiffAlgorithm', () => { expect.objectContaining({ merged_version: [], diff_outcome: ThreeWayDiffOutcome.StockValueNoUpdate, - merge_outcome: ThreeWayMergeOutcome.Current, conflict: ThreeWayDiffConflict.NONE, }) ); diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/prebuilt_rules/logic/diff/calculation/algorithms/scalar_array_diff_algorithm.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/prebuilt_rules/logic/diff/calculation/algorithms/scalar_array_diff_algorithm.ts index f2a984dca8826..cb84ab8633b9a 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/prebuilt_rules/logic/diff/calculation/algorithms/scalar_array_diff_algorithm.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/prebuilt_rules/logic/diff/calculation/algorithms/scalar_array_diff_algorithm.ts @@ -15,7 +15,6 @@ import { determineOrderAgnosticDiffOutcome, determineIfValueCanUpdate, ThreeWayDiffOutcome, - ThreeWayMergeOutcome, MissingVersion, ThreeWayDiffConflict, } from '../../../../../../../../common/api/detection_engine/prebuilt_rules'; @@ -37,13 +36,14 @@ export const scalarArrayDiffAlgorithm = ( const diffOutcome = determineOrderAgnosticDiffOutcome(baseVersion, currentVersion, targetVersion); const valueCanUpdate = determineIfValueCanUpdate(diffOutcome); - const { mergeOutcome, mergedVersion } = mergeVersions({ - baseVersion, + const hasBaseVersion = baseVersion !== MissingVersion; + const { conflict, mergedVersion } = mergeVersions({ + hasBaseVersion, + baseVersion: hasBaseVersion ? baseVersion : undefined, currentVersion, targetVersion, diffOutcome, }); - const hasBaseVersion = baseVersion !== MissingVersion; return { has_base_version: hasBaseVersion, @@ -53,65 +53,68 @@ export const scalarArrayDiffAlgorithm = ( merged_version: mergedVersion, diff_outcome: diffOutcome, - merge_outcome: mergeOutcome, + conflict, has_update: valueCanUpdate, - conflict: - // Scalar Arrays can only result in Merged outcomes on conflict - mergeOutcome === ThreeWayMergeOutcome.Merged - ? ThreeWayDiffConflict.SOLVABLE - : ThreeWayDiffConflict.NONE, }; }; interface MergeResult { - mergeOutcome: ThreeWayMergeOutcome; mergedVersion: TValue[]; + conflict: ThreeWayDiffConflict; } interface MergeArgs { - baseVersion: TValue[] | MissingVersion; + hasBaseVersion: boolean; + baseVersion: TValue[] | undefined; currentVersion: TValue[]; targetVersion: TValue[]; diffOutcome: ThreeWayDiffOutcome; } const mergeVersions = ({ + hasBaseVersion, baseVersion, currentVersion, targetVersion, diffOutcome, }: MergeArgs): MergeResult => { - const dedupedBaseVersion = baseVersion !== MissingVersion ? uniq(baseVersion) : MissingVersion; + const dedupedBaseVersion = uniq(baseVersion); const dedupedCurrentVersion = uniq(currentVersion); const dedupedTargetVersion = uniq(targetVersion); switch (diffOutcome) { - case ThreeWayDiffOutcome.StockValueNoUpdate: - case ThreeWayDiffOutcome.CustomizedValueNoUpdate: - case ThreeWayDiffOutcome.CustomizedValueSameUpdate: { + case ThreeWayDiffOutcome.StockValueNoUpdate: // Scenarios AAA and -AA + case ThreeWayDiffOutcome.CustomizedValueNoUpdate: // Scenario ABA + case ThreeWayDiffOutcome.CustomizedValueSameUpdate: // Scenario ABB return { - mergeOutcome: ThreeWayMergeOutcome.Current, + conflict: ThreeWayDiffConflict.NONE, mergedVersion: dedupedCurrentVersion, }; - } + case ThreeWayDiffOutcome.StockValueCanUpdate: { + if (!hasBaseVersion) { + // Scenario -AB. Treated as scenario ABC, returns target + // version and marked as "SOLVABLE" conflict. + // https://github.com/elastic/kibana/pull/184889#discussion_r1636421293 + return { + mergedVersion: targetVersion, + conflict: ThreeWayDiffConflict.SOLVABLE, + }; + } + + // Scenario AAB return { - mergeOutcome: ThreeWayMergeOutcome.Target, + conflict: ThreeWayDiffConflict.NONE, mergedVersion: dedupedTargetVersion, }; } - case ThreeWayDiffOutcome.CustomizedValueCanUpdate: { - if (dedupedBaseVersion === MissingVersion) { - return { - mergeOutcome: ThreeWayMergeOutcome.Merged, - mergedVersion: union(currentVersion, targetVersion), - }; - } - const addedCurrent = difference(dedupedCurrentVersion, dedupedBaseVersion); + // Scenario ABC + case ThreeWayDiffOutcome.CustomizedValueCanUpdate: { + const addedCurrent = difference(dedupedCurrentVersion, dedupedBaseVersion as TValue[]); const removedCurrent = difference(dedupedBaseVersion, dedupedCurrentVersion); - const addedTarget = difference(dedupedTargetVersion, dedupedBaseVersion); + const addedTarget = difference(dedupedTargetVersion, dedupedBaseVersion as TValue[]); const removedTarget = difference(dedupedBaseVersion, dedupedTargetVersion); const bothAdded = union(addedCurrent, addedTarget); @@ -120,7 +123,7 @@ const mergeVersions = ({ const merged = difference(union(dedupedBaseVersion, bothAdded), bothRemoved); return { - mergeOutcome: ThreeWayMergeOutcome.Merged, + conflict: ThreeWayDiffConflict.SOLVABLE, mergedVersion: merged, }; } diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/prebuilt_rules/logic/diff/calculation/algorithms/simple_diff_algorithm.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/prebuilt_rules/logic/diff/calculation/algorithms/simple_diff_algorithm.ts index 2dd47a7364104..40e9767c08bc6 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/prebuilt_rules/logic/diff/calculation/algorithms/simple_diff_algorithm.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/prebuilt_rules/logic/diff/calculation/algorithms/simple_diff_algorithm.ts @@ -16,7 +16,6 @@ import { MissingVersion, ThreeWayDiffConflict, ThreeWayDiffOutcome, - ThreeWayMergeOutcome, } from '../../../../../../../../common/api/detection_engine/prebuilt_rules'; /** @@ -36,14 +35,15 @@ export const simpleDiffAlgorithm = ( const diffOutcome = determineDiffOutcome(baseVersion, currentVersion, targetVersion); const valueCanUpdate = determineIfValueCanUpdate(diffOutcome); - const { mergeOutcome, mergedVersion } = mergeVersions({ + const hasBaseVersion = baseVersion !== MissingVersion; + + const { conflict, mergedVersion } = mergeVersions({ + hasBaseVersion, currentVersion, targetVersion, diffOutcome, }); - const hasBaseVersion = baseVersion !== MissingVersion; - return { has_base_version: hasBaseVersion, base_version: hasBaseVersion ? baseVersion : undefined, @@ -52,52 +52,61 @@ export const simpleDiffAlgorithm = ( merged_version: mergedVersion, diff_outcome: diffOutcome, - merge_outcome: mergeOutcome, has_update: valueCanUpdate, - conflict: - // Simple Diffs algos can only results in NON_SOLVABLE conflicts - // if the diff outcome is a conflict - mergeOutcome === ThreeWayMergeOutcome.Conflict - ? ThreeWayDiffConflict.NON_SOLVABLE - : ThreeWayDiffConflict.NONE, + conflict, }; }; interface MergeResult { - mergeOutcome: ThreeWayMergeOutcome; mergedVersion: TValue; + conflict: ThreeWayDiffConflict; } interface MergeArgs { + hasBaseVersion: boolean; currentVersion: TValue; targetVersion: TValue; diffOutcome: ThreeWayDiffOutcome; } const mergeVersions = ({ + hasBaseVersion, currentVersion, targetVersion, diffOutcome, }: MergeArgs): MergeResult => { switch (diffOutcome) { - case ThreeWayDiffOutcome.StockValueNoUpdate: - case ThreeWayDiffOutcome.CustomizedValueNoUpdate: - case ThreeWayDiffOutcome.CustomizedValueSameUpdate: { + case ThreeWayDiffOutcome.StockValueNoUpdate: // Scenarios AAA and -AA + case ThreeWayDiffOutcome.CustomizedValueNoUpdate: // Scenario ABA + case ThreeWayDiffOutcome.CustomizedValueSameUpdate: // Scenario ABB return { - mergeOutcome: ThreeWayMergeOutcome.Current, mergedVersion: currentVersion, + conflict: ThreeWayDiffConflict.NONE, }; - } + case ThreeWayDiffOutcome.StockValueCanUpdate: { + if (!hasBaseVersion) { + // Scenario -AB. Treated as scenario ABC, returns target + // version and marked as "SOLVABLE" conflict. + // https://github.com/elastic/kibana/pull/184889#discussion_r1636421293 + return { + mergedVersion: targetVersion, + conflict: ThreeWayDiffConflict.SOLVABLE, + }; + } + + // Scenario AAB return { - mergeOutcome: ThreeWayMergeOutcome.Target, mergedVersion: targetVersion, + conflict: ThreeWayDiffConflict.NONE, }; } + + // Scenario ABC case ThreeWayDiffOutcome.CustomizedValueCanUpdate: { return { - mergeOutcome: ThreeWayMergeOutcome.Conflict, mergedVersion: currentVersion, + conflict: ThreeWayDiffConflict.NON_SOLVABLE, }; } default: diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/prebuilt_rules/logic/diff/calculation/algorithms/single_line_string_diff_algorithm.test.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/prebuilt_rules/logic/diff/calculation/algorithms/single_line_string_diff_algorithm.test.ts index bb3aeb66a59db..4c91f9e5f3e2a 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/prebuilt_rules/logic/diff/calculation/algorithms/single_line_string_diff_algorithm.test.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/prebuilt_rules/logic/diff/calculation/algorithms/single_line_string_diff_algorithm.test.ts @@ -8,7 +8,6 @@ import type { ThreeVersionsOf } from '../../../../../../../../common/api/detection_engine'; import { ThreeWayDiffOutcome, - ThreeWayMergeOutcome, MissingVersion, ThreeWayDiffConflict, } from '../../../../../../../../common/api/detection_engine'; @@ -28,7 +27,6 @@ describe('singleLineStringDiffAlgorithm', () => { expect.objectContaining({ merged_version: mockVersions.current_version, diff_outcome: ThreeWayDiffOutcome.StockValueNoUpdate, - merge_outcome: ThreeWayMergeOutcome.Current, conflict: ThreeWayDiffConflict.NONE, }) ); @@ -47,7 +45,6 @@ describe('singleLineStringDiffAlgorithm', () => { expect.objectContaining({ merged_version: mockVersions.current_version, diff_outcome: ThreeWayDiffOutcome.CustomizedValueNoUpdate, - merge_outcome: ThreeWayMergeOutcome.Current, conflict: ThreeWayDiffConflict.NONE, }) ); @@ -66,7 +63,6 @@ describe('singleLineStringDiffAlgorithm', () => { expect.objectContaining({ merged_version: mockVersions.target_version, diff_outcome: ThreeWayDiffOutcome.StockValueCanUpdate, - merge_outcome: ThreeWayMergeOutcome.Target, conflict: ThreeWayDiffConflict.NONE, }) ); @@ -85,7 +81,6 @@ describe('singleLineStringDiffAlgorithm', () => { expect.objectContaining({ merged_version: mockVersions.current_version, diff_outcome: ThreeWayDiffOutcome.CustomizedValueSameUpdate, - merge_outcome: ThreeWayMergeOutcome.Current, conflict: ThreeWayDiffConflict.NONE, }) ); @@ -104,7 +99,6 @@ describe('singleLineStringDiffAlgorithm', () => { expect.objectContaining({ merged_version: mockVersions.current_version, diff_outcome: ThreeWayDiffOutcome.CustomizedValueCanUpdate, - merge_outcome: ThreeWayMergeOutcome.Conflict, conflict: ThreeWayDiffConflict.NON_SOLVABLE, }) ); @@ -122,9 +116,10 @@ describe('singleLineStringDiffAlgorithm', () => { expect(result).toEqual( expect.objectContaining({ + has_base_version: false, + base_version: undefined, merged_version: mockVersions.current_version, diff_outcome: ThreeWayDiffOutcome.StockValueNoUpdate, - merge_outcome: ThreeWayMergeOutcome.Current, conflict: ThreeWayDiffConflict.NONE, }) ); @@ -141,10 +136,11 @@ describe('singleLineStringDiffAlgorithm', () => { expect(result).toEqual( expect.objectContaining({ + has_base_version: false, + base_version: undefined, merged_version: mockVersions.target_version, diff_outcome: ThreeWayDiffOutcome.StockValueCanUpdate, - merge_outcome: ThreeWayMergeOutcome.Target, - conflict: ThreeWayDiffConflict.NONE, + conflict: ThreeWayDiffConflict.SOLVABLE, }) ); });