Skip to content

Commit

Permalink
removes the mergeable logic in algorithms
Browse files Browse the repository at this point in the history
  • Loading branch information
@dplumlee
dplumlee committed Aug 13, 2024
1 parent 4162edd commit f34a643
Show file tree
Hide file tree
Showing 6 changed files with 27 additions and 538 deletions.
51 changes: 7 additions & 44 deletions ..._engine/prebuilt_rules/logic/diff/calculation/algorithms/eql_query_diff_algorithm.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -143,57 +143,20 @@ describe('eqlQueryDiffAlgorithm', () => {
});

describe('if all three versions are different - scenario ABC', () => {
it('returns a computated merged version with a solvable conflict if 3-way query field merge is possible', () => {
it('returns the current_version with a non-solvable conflict', () => {
const mockVersions: ThreeVersionsOf<RuleEqlQuery> = {
base_version: {
query: 'My description.\f\nThis is a second\u2001 line.\f\nThis is a third line.',
language: 'eql',
filters: [],
},
current_version: {
query: 'My GREAT description.\f\nThis is a second\u2001 line.\f\nThis is a third line.',
language: 'eql',
filters: [],
},
target_version: {
query: 'My description.\f\nThis is a second\u2001 line.\f\nThis is a GREAT line.',
language: 'eql',
filters: [],
},
};

const expectedMergedVersion: RuleEqlQuery = {
query: `My GREAT description.\f\nThis is a second\u2001 line.\f\nThis is a GREAT line.`,
language: 'eql',
filters: [],
};

const result = eqlQueryDiffAlgorithm(mockVersions);

expect(result).toEqual(
expect.objectContaining({
merged_version: expectedMergedVersion,
diff_outcome: ThreeWayDiffOutcome.CustomizedValueCanUpdate,
merge_outcome: ThreeWayMergeOutcome.Merged,
conflict: ThreeWayDiffConflict.SOLVABLE,
})
);
});

it('returns the current_version with a non-solvable conflict if 3-way query field merge is not possible', () => {
const mockVersions: ThreeVersionsOf<RuleEqlQuery> = {
base_version: {
query: 'My description.\nThis is a second line.',
query: 'query where false',
language: 'eql',
filters: [],
},
current_version: {
query: 'My GREAT description.\nThis is a third line.',
query: 'query where true',
language: 'eql',
filters: [],
},
target_version: {
query: 'My EXCELLENT description.\nThis is a fourth.',
query: 'query two where false',
language: 'eql',
filters: [],
},
Expand All @@ -211,7 +174,7 @@ describe('eqlQueryDiffAlgorithm', () => {
);
});

it('returns the current_version with a non-solvable conflict if non-mergeable fields are different', () => {
it('returns the current_version with a non-solvable conflict if one subfield has an ABA scenario and another has an AAB', () => {
const mockVersions: ThreeVersionsOf<RuleEqlQuery> = {
base_version: {
query: 'query where false',
Expand All @@ -224,9 +187,9 @@ describe('eqlQueryDiffAlgorithm', () => {
filters: [{ field: 'some query' }],
},
target_version: {
query: 'query where false',
query: 'query where true',
language: 'eql',
filters: [{ field: 'a different query' }],
filters: [],
},
};

Expand Down
136 changes: 3 additions & 133 deletions ...ction_engine/prebuilt_rules/logic/diff/calculation/algorithms/eql_query_diff_algorithm.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,144 +5,14 @@
* 2.0.
*/

import { merge } from 'node-diff3';
import { assertUnreachable } from '../../../../../../../../common/utility_types';
import type {
RuleEqlQuery,
ThreeVersionsOf,
ThreeWayDiff,
} from '../../../../../../../../common/api/detection_engine/prebuilt_rules';
import {
determineIfValueCanUpdate,
ThreeWayDiffOutcome,
ThreeWayMergeOutcome,
MissingVersion,
ThreeWayDiffConflict,
determineDiffOutcome,
} from '../../../../../../../../common/api/detection_engine/prebuilt_rules';
import { determineIfAllVersionsAreEqual } from './helpers';
import { simpleDiffAlgorithm } from './simple_diff_algorithm';

/**
* Diff algorithm for eql query types
*/
export const eqlQueryDiffAlgorithm = (
versions: ThreeVersionsOf<RuleEqlQuery>
): ThreeWayDiff<RuleEqlQuery> => {
const {
base_version: baseVersion,
current_version: currentVersion,
target_version: targetVersion,
} = versions;

const diffOutcome = determineDiffOutcome(baseVersion, currentVersion, targetVersion);

const valueCanUpdate = determineIfValueCanUpdate(diffOutcome);

const hasBaseVersion = baseVersion !== MissingVersion;

const { mergeOutcome, conflict, mergedVersion } = mergeVersions({
baseVersion: hasBaseVersion ? baseVersion : undefined,
currentVersion,
targetVersion,
diffOutcome,
});

return {
has_base_version: hasBaseVersion,
base_version: hasBaseVersion ? baseVersion : undefined,
current_version: currentVersion,
target_version: targetVersion,
merged_version: mergedVersion,
merge_outcome: mergeOutcome,

diff_outcome: diffOutcome,
conflict,
has_update: valueCanUpdate,
};
};

interface MergeResult {
mergeOutcome: ThreeWayMergeOutcome;
mergedVersion: RuleEqlQuery;
conflict: ThreeWayDiffConflict;
}

interface MergeArgs {
baseVersion: RuleEqlQuery | undefined;
currentVersion: RuleEqlQuery;
targetVersion: RuleEqlQuery;
diffOutcome: ThreeWayDiffOutcome;
}

const mergeVersions = ({
baseVersion,
currentVersion,
targetVersion,
diffOutcome,
}: MergeArgs): MergeResult => {
switch (diffOutcome) {
// Scenario -AA is treated as scenario AAA:
// https://github.com/elastic/kibana/pull/184889#discussion_r1636421293
case ThreeWayDiffOutcome.MissingBaseNoUpdate:
case ThreeWayDiffOutcome.StockValueNoUpdate:
case ThreeWayDiffOutcome.CustomizedValueNoUpdate:
case ThreeWayDiffOutcome.CustomizedValueSameUpdate:
return {
conflict: ThreeWayDiffConflict.NONE,
mergeOutcome: ThreeWayMergeOutcome.Current,
mergedVersion: currentVersion,
};

case ThreeWayDiffOutcome.StockValueCanUpdate:
return {
conflict: ThreeWayDiffConflict.NONE,
mergeOutcome: ThreeWayMergeOutcome.Target,
mergedVersion: targetVersion,
};

case ThreeWayDiffOutcome.CustomizedValueCanUpdate: {
if (baseVersion) {
// 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.query, baseVersion.query, targetVersion.query, {
stringSeparator: /(\S+|\s+)/g, // Retains all whitespace, which we keep to preserve formatting
});

// Determines if all non-mergeable fields are equal to one another
// filters are the only other variable field in the `RuleEqlQuery` type
const nonMergeableFieldsEqual = determineIfAllVersionsAreEqual(
baseVersion.filters,
currentVersion.filters,
targetVersion.filters
);

if (nonMergeableFieldsEqual && mergedVersion.conflict === false) {
return {
conflict: ThreeWayDiffConflict.SOLVABLE,
mergedVersion: { ...currentVersion, query: mergedVersion.result.join('') },
mergeOutcome: ThreeWayMergeOutcome.Merged,
};
}
}

return {
conflict: ThreeWayDiffConflict.NON_SOLVABLE,
mergeOutcome: ThreeWayMergeOutcome.Current,
mergedVersion: currentVersion,
};
}

// Scenario -AB is treated as scenario ABC, but marked as
// SOLVABLE, and returns the target version as the merged version
// https://github.com/elastic/kibana/pull/184889#discussion_r1636421293
case ThreeWayDiffOutcome.MissingBaseCanUpdate: {
return {
mergedVersion: targetVersion,
mergeOutcome: ThreeWayMergeOutcome.Target,
conflict: ThreeWayDiffConflict.SOLVABLE,
};
}
default:
return assertUnreachable(diffOutcome);
}
};
export const eqlQueryDiffAlgorithm = (versions: ThreeVersionsOf<RuleEqlQuery>) =>
simpleDiffAlgorithm<RuleEqlQuery>(versions);
41 changes: 4 additions & 37 deletions ...engine/prebuilt_rules/logic/diff/calculation/algorithms/esql_query_diff_algorithm.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -131,51 +131,18 @@ describe('esqlQueryDiffAlgorithm', () => {
});

describe('if all three versions are different - scenario ABC', () => {
it('returns a computated merged version with a solvable conflict if 3-way query field merge is possible', () => {
it('returns the current_version with a non-solvable conflict', () => {
const mockVersions: ThreeVersionsOf<RuleEsqlQuery> = {
base_version: {
query: 'My description.\f\nThis is a second\u2001 line.\f\nThis is a third line.',
language: 'esql',
},
current_version: {
query: 'My GREAT description.\f\nThis is a second\u2001 line.\f\nThis is a third line.',
language: 'esql',
},
target_version: {
query: 'My description.\f\nThis is a second\u2001 line.\f\nThis is a GREAT line.',
language: 'esql',
},
};

const expectedMergedVersion: RuleEsqlQuery = {
query: `My GREAT description.\f\nThis is a second\u2001 line.\f\nThis is a GREAT line.`,
language: 'esql',
};

const result = esqlQueryDiffAlgorithm(mockVersions);

expect(result).toEqual(
expect.objectContaining({
merged_version: expectedMergedVersion,
diff_outcome: ThreeWayDiffOutcome.CustomizedValueCanUpdate,
merge_outcome: ThreeWayMergeOutcome.Merged,
conflict: ThreeWayDiffConflict.SOLVABLE,
})
);
});

it('returns the current_version with a non-solvable conflict if 3-way query field merge is not possible', () => {
const mockVersions: ThreeVersionsOf<RuleEsqlQuery> = {
base_version: {
query: 'My description.\nThis is a second line.',
query: 'query where true',
language: 'esql',
},
current_version: {
query: 'My GREAT description.\nThis is a third line.',
query: 'query where false',
language: 'esql',
},
target_version: {
query: 'My EXCELLENT description.\nThis is a fourth.',
query: 'different query where true',
language: 'esql',
},
};
Expand Down
Loading

0 comments on commit f34a643

Please sign in to comment.