addresses comments

elastic · Jun 20, 2024 · 65faf40 · 65faf40
1 parent c7fc99c
commit 65faf40
Show file tree

Hide file tree

Showing 5 changed files with 86 additions and 35 deletions.
diff --git a/...n/api/detection_engine/prebuilt_rules/model/diff/three_way_diff/three_way_diff_outcome.ts b/...n/api/detection_engine/prebuilt_rules/model/diff/three_way_diff/three_way_diff_outcome.ts
@@ -5,7 +5,7 @@
  * 2.0.
  */
 
-import { isEqual } from 'lodash';
+import { isEqual, sortBy } from 'lodash';
 import { MissingVersion } from './three_way_diff';
 
 /**
@@ -38,7 +38,55 @@ export const determineDiffOutcome = <TValue>(
   const baseEqlTarget = isEqual(baseVersion, targetVersion);
   const currentEqlTarget = isEqual(currentVersion, targetVersion);
 
-  if (baseVersion === MissingVersion) {
+  return getThreeWayDiffOutcome({
+    baseEqlCurrent,
+    baseEqlTarget,
+    currentEqlTarget,
+    hasBaseVersion: baseVersion !== MissingVersion,
+  });
+};
+
+/**
+ * Determines diff outcomes of array fields that do not care about order (e.g. `[1, 2 , 3] === [3, 2, 1]`)
+ */
+export const determineOrderAgnosticDiffOutcome = <TValue>(
+  baseVersion: TValue[] | MissingVersion,
+  currentVersion: TValue[],
+  targetVersion: TValue[]
+): ThreeWayDiffOutcome => {
+  let baseEqlCurrent: boolean;
+  let baseEqlTarget: boolean;
+  if (baseVersion !== MissingVersion) {
+    baseEqlCurrent = isEqual(sortBy(baseVersion), sortBy(currentVersion));
+    baseEqlTarget = isEqual(sortBy(baseVersion), sortBy(targetVersion));
+  } else {
+    baseEqlCurrent = false;
+    baseEqlTarget = false;
+  }
+  const currentEqlTarget = isEqual(sortBy(currentVersion), sortBy(targetVersion));
+
+  return getThreeWayDiffOutcome({
+    baseEqlCurrent,
+    baseEqlTarget,
+    currentEqlTarget,
+    hasBaseVersion: baseVersion !== MissingVersion,
+  });
+};
+
+interface DetermineDiffOutcomeProps {
+  hasBaseVersion: boolean;
+  baseEqlCurrent: boolean;
+  baseEqlTarget: boolean;
+  currentEqlTarget: boolean;
+}
+
+const getThreeWayDiffOutcome = ({
+  hasBaseVersion,
+  baseEqlCurrent,
+  baseEqlTarget,
+  currentEqlTarget,
+}: DetermineDiffOutcomeProps): ThreeWayDiffOutcome => {
+  if (hasBaseVersion) {
     /**
      * We couldn't find the base version of the rule in the package so further
      * version comparison is not possible. We assume that the rule is not

diff --git a/...ion_engine/prebuilt_rules/logic/diff/calculation/algorithms/number_diff_algorithm.test.ts b/...ion_engine/prebuilt_rules/logic/diff/calculation/algorithms/number_diff_algorithm.test.ts
@@ -14,7 +14,7 @@ import {
 import { numberDiffAlgorithm } from './number_diff_algorithm';
 
 describe('numberDiffAlgorithm', () => {
-  it('returns current_version as merged output if there is no update', () => {
+  it('returns current_version as merged output if there is no update - scenario AAA', () => {
     const mockVersions: ThreeVersionsOf<number> = {
       base_version: 1,
       current_version: 1,
@@ -33,7 +33,7 @@ describe('numberDiffAlgorithm', () => {
     );
   });
 
-  it('returns current_version as merged output if current_version is different and there is no update', () => {
+  it('returns current_version as merged output if current_version is different and there is no update - scenario ABA', () => {
     const mockVersions: ThreeVersionsOf<number> = {
       base_version: 1,
       current_version: 2,
@@ -52,7 +52,7 @@ describe('numberDiffAlgorithm', () => {
     );
   });
 
-  it('returns target_version as merged output if current_version is the same and there is an update', () => {
+  it('returns target_version as merged output if current_version is the same and there is an update - scenario AAB', () => {
     const mockVersions: ThreeVersionsOf<number> = {
       base_version: 1,
       current_version: 1,
@@ -71,7 +71,7 @@ describe('numberDiffAlgorithm', () => {
     );
   });
 
-  it('returns current_version as merged output if current version is different but it matches the update', () => {
+  it('returns current_version as merged output if current version is different but it matches the update - scenario ABB', () => {
     const mockVersions: ThreeVersionsOf<number> = {
       base_version: 1,
       current_version: 2,
@@ -90,7 +90,7 @@ describe('numberDiffAlgorithm', () => {
     );
   });
 
-  it('returns current_version as merged output if all three versions are different', () => {
+  it('returns current_version as merged output if all three versions are different - scenario ABC', () => {
     const mockVersions: ThreeVersionsOf<number> = {
       base_version: 1,
       current_version: 2,
@@ -110,7 +110,7 @@ describe('numberDiffAlgorithm', () => {
   });
 
   describe('if base_version is missing', () => {
-    it('returns current_version as merged output if current_version and target_version are the same', () => {
+    it('returns current_version as merged output if current_version and target_version are the same - scenario -AA', () => {
       const mockVersions: ThreeVersionsOf<number> = {
         base_version: MissingVersion,
         current_version: 1,
@@ -129,7 +129,7 @@ describe('numberDiffAlgorithm', () => {
       );
     });
 
-    it('returns target_version as merged output if current_version and target_version are different', () => {
+    it('returns target_version as merged output if current_version and target_version are different - scenario -AB', () => {
       const mockVersions: ThreeVersionsOf<number> = {
         base_version: MissingVersion,
         current_version: 1,

diff --git a/...gine/prebuilt_rules/logic/diff/calculation/algorithms/scalar_array_diff_algorithm.test.ts b/...gine/prebuilt_rules/logic/diff/calculation/algorithms/scalar_array_diff_algorithm.test.ts
@@ -14,11 +14,11 @@ import {
 import { scalarArrayDiffAlgorithm } from './scalar_array_diff_algorithm';
 
 describe('scalarArrayDiffAlgorithm', () => {
-  it('returns current_version as merged output if there is no update', () => {
+  it('returns current_version as merged output if there is no update - scenario AAA', () => {
     const mockVersions: ThreeVersionsOf<string[]> = {
-      base_version: ['A'],
-      current_version: ['A'],
-      target_version: ['A'],
+      base_version: ['A', 'B'],
+      current_version: ['B', 'A'],
+      target_version: ['A', 'B'],
     };
 
     const result = scalarArrayDiffAlgorithm(mockVersions);
@@ -33,7 +33,7 @@ describe('scalarArrayDiffAlgorithm', () => {
     );
   });
 
-  it('returns current_version as merged output if current_version is different and there is no update', () => {
+  it('returns current_version as merged output if current_version is different and there is no update - scenario ABA', () => {
     const mockVersions: ThreeVersionsOf<string[]> = {
       base_version: ['A'],
       current_version: ['B'],
@@ -52,7 +52,7 @@ describe('scalarArrayDiffAlgorithm', () => {
     );
   });
 
-  it('returns target_version as merged output if current_version is the same and there is an update', () => {
+  it('returns target_version as merged output if current_version is the same and there is an update - scenario AAB', () => {
     const mockVersions: ThreeVersionsOf<string[]> = {
       base_version: ['A'],
       current_version: ['A'],
@@ -71,7 +71,7 @@ describe('scalarArrayDiffAlgorithm', () => {
     );
   });
 
-  it('returns current_version as merged output if current version is different but it matches the update', () => {
+  it('returns current_version as merged output if current version is different but it matches the update - scenario ABB', () => {
     const mockVersions: ThreeVersionsOf<string[]> = {
       base_version: ['A'],
       current_version: ['B'],
@@ -90,7 +90,7 @@ describe('scalarArrayDiffAlgorithm', () => {
     );
   });
 
-  it('returns custom merged version as merged output if all three versions are different', () => {
+  it('returns custom merged version as merged output if all three versions are different - scenario ABC', () => {
     const mockVersions: ThreeVersionsOf<string[]> = {
       base_version: ['A'],
       current_version: ['B', 'C'],
@@ -111,7 +111,7 @@ describe('scalarArrayDiffAlgorithm', () => {
   });
 
   describe('if base_version is missing', () => {
-    it('returns current_version as merged output if current_version and target_version are the same', () => {
+    it('returns current_version as merged output if current_version and target_version are the same - scenario -AA', () => {
       const mockVersions: ThreeVersionsOf<string[]> = {
         base_version: MissingVersion,
         current_version: ['A'],
@@ -130,7 +130,7 @@ describe('scalarArrayDiffAlgorithm', () => {
       );
     });
 
-    it('returns target_version as merged output if current_version and target_version are different', () => {
+    it('returns target_version as merged output if current_version and target_version are different - scenario -AB', () => {
       const mockVersions: ThreeVersionsOf<string[]> = {
         base_version: MissingVersion,
         current_version: ['A'],

diff --git a/...on_engine/prebuilt_rules/logic/diff/calculation/algorithms/scalar_array_diff_algorithm.ts b/...on_engine/prebuilt_rules/logic/diff/calculation/algorithms/scalar_array_diff_algorithm.ts
@@ -12,25 +12,28 @@ import type {
   ThreeWayDiff,
 } from '../../../../../../../../common/api/detection_engine/prebuilt_rules';
 import {
-  determineDiffOutcome,
+  determineOrderAgnosticDiffOutcome,
   determineIfValueCanUpdate,
   ThreeWayDiffOutcome,
   ThreeWayMergeOutcome,
   MissingVersion,
 } from '../../../../../../../../common/api/detection_engine/prebuilt_rules';
 
-export type ScalarArrayDiffAlgorithmType = string[]; // We currently don't have any fields that use any other types than strings
-
-export const scalarArrayDiffAlgorithm = (
-  versions: ThreeVersionsOf<ScalarArrayDiffAlgorithmType>
-): ThreeWayDiff<ScalarArrayDiffAlgorithmType> => {
+/**
+ * Diff algorithm used for arrays of scalar values (eg. numbers, strings, booleans, etc.)
+ *
+ * NOTE: Diffing logic will be agnostic to array order
+ */
+export const scalarArrayDiffAlgorithm = <TValue>(
+  versions: ThreeVersionsOf<TValue[]>
+): ThreeWayDiff<TValue[]> => {
   const {
     base_version: baseVersion,
     current_version: currentVersion,
     target_version: targetVersion,
   } = versions;
 
-  const diffOutcome = determineDiffOutcome(baseVersion, currentVersion, targetVersion);
+  const diffOutcome = determineOrderAgnosticDiffOutcome(baseVersion, currentVersion, targetVersion);
   const valueCanUpdate = determineIfValueCanUpdate(diffOutcome);
 
   const { mergeOutcome, mergedVersion } = mergeVersions({
@@ -65,12 +68,12 @@ interface MergeArgs<TValue> {
   diffOutcome: ThreeWayDiffOutcome;
 }
 
-const mergeVersions = ({
+const mergeVersions = <TValue>({
   baseVersion,
   currentVersion,
   targetVersion,
   diffOutcome,
-}: MergeArgs<ScalarArrayDiffAlgorithmType>): MergeResult<ScalarArrayDiffAlgorithmType> => {
+}: MergeArgs<TValue[]>): MergeResult<TValue[]> => {
   switch (diffOutcome) {
     case ThreeWayDiffOutcome.StockValueNoUpdate:
     case ThreeWayDiffOutcome.CustomizedValueNoUpdate:

diff --git a/...rebuilt_rules/logic/diff/calculation/algorithms/single_line_string_diff_algorithm.test.ts b/...rebuilt_rules/logic/diff/calculation/algorithms/single_line_string_diff_algorithm.test.ts
@@ -14,7 +14,7 @@ import {
 import { singleLineStringDiffAlgorithm } from './single_line_string_diff_algorithm';
 
 describe('singleLineStringDiffAlgorithm', () => {
-  it('returns current_version as merged output if there is no update', () => {
+  it('returns current_version as merged output if there is no update - scenario AAA', () => {
     const mockVersions: ThreeVersionsOf<string> = {
       base_version: 'A',
       current_version: 'A',
@@ -33,7 +33,7 @@ describe('singleLineStringDiffAlgorithm', () => {
     );
   });
 
-  it('returns current_version as merged output if current_version is different and there is no update', () => {
+  it('returns current_version as merged output if current_version is different and there is no update - scenario ABA', () => {
     const mockVersions: ThreeVersionsOf<string> = {
       base_version: 'A',
       current_version: 'B',
@@ -52,7 +52,7 @@ describe('singleLineStringDiffAlgorithm', () => {
     );
   });
 
-  it('returns target_version as merged output if current_version is the same and there is an update', () => {
+  it('returns target_version as merged output if current_version is the same and there is an update - scenario AAB', () => {
     const mockVersions: ThreeVersionsOf<string> = {
       base_version: 'A',
       current_version: 'A',
@@ -71,7 +71,7 @@ describe('singleLineStringDiffAlgorithm', () => {
     );
   });
 
-  it('returns current_version as merged output if current version is different but it matches the update', () => {
+  it('returns current_version as merged output if current version is different but it matches the update - scenario ABB', () => {
     const mockVersions: ThreeVersionsOf<string> = {
       base_version: 'A',
       current_version: 'B',
@@ -90,7 +90,7 @@ describe('singleLineStringDiffAlgorithm', () => {
     );
   });
 
-  it('returns current_version as merged output if all three versions are different', () => {
+  it('returns current_version as merged output if all three versions are different - scenario ABC', () => {
     const mockVersions: ThreeVersionsOf<string> = {
       base_version: 'A',
       current_version: 'B',
@@ -110,7 +110,7 @@ describe('singleLineStringDiffAlgorithm', () => {
   });
 
   describe('if base_version is missing', () => {
-    it('returns current_version as merged output if current_version and target_version are the same', () => {
+    it('returns current_version as merged output if current_version and target_version are the same - scenario -AA', () => {
       const mockVersions: ThreeVersionsOf<string> = {
         base_version: MissingVersion,
         current_version: 'A',
@@ -129,7 +129,7 @@ describe('singleLineStringDiffAlgorithm', () => {
       );
     });
 
-    it('returns target_version as merged output if current_version and target_version are different', () => {
+    it('returns target_version as merged output if current_version and target_version are different - scenario -AB', () => {
       const mockVersions: ThreeVersionsOf<string> = {
         base_version: MissingVersion,
         current_version: 'A',