Skip to content

Commit

Permalink
[8.x] Handle both JSON and XJSON in processor editor (#200692) (#201910)
Browse files Browse the repository at this point in the history
# Backport

This will backport the following commits from `main` to `8.x`:
- [Handle both JSON and XJSON in processor editor
(#200692)](#200692)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Sonia Sanz
Vivas","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-11-27T06:42:30Z","message":"Handle
both JSON and XJSON in processor editor (#200692)\n\nCloses
[#175753](https://github.com/elastic/kibana/issues/175753)\r\n\r\n##
Summary\r\nWhen a user creates a pipeline with a processor that has an
JSON editor\r\nfield [Foreach, Grok, Inference, Redact and Script], our
editor convert\r\nthe input to XJSON. This can be confused to the user,
who see their\r\ninput modified without understanding the reason. For
that, this PR\r\ncreates a new editor that handles both XJSON and JSON
format and does\r\nnot changes the content that the user introduced
while editing. Once the\r\npipeline is saved, it will always retrieve
the parsed data.\r\n\r\nI've created a whole new editor instead of
modifying `XJsonEditor`\r\nbecause the original one is still in use in
the `Custom processor`. This\r\nis because I've created
[a\r\nlist](https://github.com/SoniaSanzV/kibana/blob/d7d5ecafa7dbae96fe52c2e37394520b6353bd92/x-pack/plugins/ingest_pipelines/public/application/components/pipeline_editor/utils.ts#L151-L157)\r\nof
the processor fields that needs to be serialized
in\r\n`convertProcessorInternalToProcessor`. Since we don't know the
name of\r\nthe custom processor, we can't apply this workflow. I'm not
super happy\r\nwith the idea of adding manually the names of the
processors to a list.\r\nI would like to have something more agnostic
but I haven't come up with\r\na solution that allows me to do that
without breaking other fields. So,\r\nany suggestions are super
welcome!\r\n\r\n### How to test\r\nCreate one of the following
processors: Foreach, Grok, Inference, Redact\r\nor Script. In the JSON
field, add a JSON with both with and without\r\nescaped strings. While
you are editing the processor before saving the\r\npipeline, you should
see the processor as you typed it. In the `Show\r\nRequest` flyout, you
should see the properly parsed JSON.\r\n\r\nThe pipeline should save
with the correct parsed values and, once saved,\r\nif you click update,
you only will get parsed values.\r\n\r\n###
Demo\r\n\r\n\r\nhttps://github.com/user-attachments/assets/1f9681df-2fb4-4ed5-ac30-03f2937abfe9","sha":"8839894646451144a9534157c08cde18727aa240","branchLabelMapping":{"^v9.0.0$":"main","^v8.18.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["Team:Kibana
Management","release_note:skip","v9.0.0","Feature:Ingest Node
Pipelines","backport:prev-minor"],"title":"Handle both JSON and XJSON in
processor
editor","number":200692,"url":"https://github.com/elastic/kibana/pull/200692","mergeCommit":{"message":"Handle
both JSON and XJSON in processor editor (#200692)\n\nCloses
[#175753](https://github.com/elastic/kibana/issues/175753)\r\n\r\n##
Summary\r\nWhen a user creates a pipeline with a processor that has an
JSON editor\r\nfield [Foreach, Grok, Inference, Redact and Script], our
editor convert\r\nthe input to XJSON. This can be confused to the user,
who see their\r\ninput modified without understanding the reason. For
that, this PR\r\ncreates a new editor that handles both XJSON and JSON
format and does\r\nnot changes the content that the user introduced
while editing. Once the\r\npipeline is saved, it will always retrieve
the parsed data.\r\n\r\nI've created a whole new editor instead of
modifying `XJsonEditor`\r\nbecause the original one is still in use in
the `Custom processor`. This\r\nis because I've created
[a\r\nlist](https://github.com/SoniaSanzV/kibana/blob/d7d5ecafa7dbae96fe52c2e37394520b6353bd92/x-pack/plugins/ingest_pipelines/public/application/components/pipeline_editor/utils.ts#L151-L157)\r\nof
the processor fields that needs to be serialized
in\r\n`convertProcessorInternalToProcessor`. Since we don't know the
name of\r\nthe custom processor, we can't apply this workflow. I'm not
super happy\r\nwith the idea of adding manually the names of the
processors to a list.\r\nI would like to have something more agnostic
but I haven't come up with\r\na solution that allows me to do that
without breaking other fields. So,\r\nany suggestions are super
welcome!\r\n\r\n### How to test\r\nCreate one of the following
processors: Foreach, Grok, Inference, Redact\r\nor Script. In the JSON
field, add a JSON with both with and without\r\nescaped strings. While
you are editing the processor before saving the\r\npipeline, you should
see the processor as you typed it. In the `Show\r\nRequest` flyout, you
should see the properly parsed JSON.\r\n\r\nThe pipeline should save
with the correct parsed values and, once saved,\r\nif you click update,
you only will get parsed values.\r\n\r\n###
Demo\r\n\r\n\r\nhttps://github.com/user-attachments/assets/1f9681df-2fb4-4ed5-ac30-03f2937abfe9","sha":"8839894646451144a9534157c08cde18727aa240"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/200692","number":200692,"mergeCommit":{"message":"Handle
both JSON and XJSON in processor editor (#200692)\n\nCloses
[#175753](https://github.com/elastic/kibana/issues/175753)\r\n\r\n##
Summary\r\nWhen a user creates a pipeline with a processor that has an
JSON editor\r\nfield [Foreach, Grok, Inference, Redact and Script], our
editor convert\r\nthe input to XJSON. This can be confused to the user,
who see their\r\ninput modified without understanding the reason. For
that, this PR\r\ncreates a new editor that handles both XJSON and JSON
format and does\r\nnot changes the content that the user introduced
while editing. Once the\r\npipeline is saved, it will always retrieve
the parsed data.\r\n\r\nI've created a whole new editor instead of
modifying `XJsonEditor`\r\nbecause the original one is still in use in
the `Custom processor`. This\r\nis because I've created
[a\r\nlist](https://github.com/SoniaSanzV/kibana/blob/d7d5ecafa7dbae96fe52c2e37394520b6353bd92/x-pack/plugins/ingest_pipelines/public/application/components/pipeline_editor/utils.ts#L151-L157)\r\nof
the processor fields that needs to be serialized
in\r\n`convertProcessorInternalToProcessor`. Since we don't know the
name of\r\nthe custom processor, we can't apply this workflow. I'm not
super happy\r\nwith the idea of adding manually the names of the
processors to a list.\r\nI would like to have something more agnostic
but I haven't come up with\r\na solution that allows me to do that
without breaking other fields. So,\r\nany suggestions are super
welcome!\r\n\r\n### How to test\r\nCreate one of the following
processors: Foreach, Grok, Inference, Redact\r\nor Script. In the JSON
field, add a JSON with both with and without\r\nescaped strings. While
you are editing the processor before saving the\r\npipeline, you should
see the processor as you typed it. In the `Show\r\nRequest` flyout, you
should see the properly parsed JSON.\r\n\r\nThe pipeline should save
with the correct parsed values and, once saved,\r\nif you click update,
you only will get parsed values.\r\n\r\n###
Demo\r\n\r\n\r\nhttps://github.com/user-attachments/assets/1f9681df-2fb4-4ed5-ac30-03f2937abfe9","sha":"8839894646451144a9534157c08cde18727aa240"}}]}]
BACKPORT-->

Co-authored-by: Sonia Sanz Vivas <[email protected]>
  • Loading branch information
kibanamachine and SoniaSanzV authored Nov 27, 2024
1 parent 2a508ef commit 9a76b27
Show file tree
Hide file tree
Showing 19 changed files with 675 additions and 63 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,117 @@
/*
* 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.
*/

import { act } from 'react-dom/test-utils';
import { setup, SetupResult, getProcessorValue, setupEnvironment } from './processor.helpers';

const FOREACH_TYPE = 'foreach';

describe('Processor: Foreach', () => {
let onUpdate: jest.Mock;
let testBed: SetupResult;
const { httpSetup } = setupEnvironment();

beforeAll(() => {
jest.useFakeTimers({ legacyFakeTimers: true });
// disable all react-beautiful-dnd development warnings
(window as any)['__@hello-pangea/dnd-disable-dev-warnings'] = true;
});

afterAll(() => {
jest.useRealTimers();
// enable all react-beautiful-dnd development warnings
(window as any)['__@hello-pangea/dnd-disable-dev-warnings'] = false;
});

beforeEach(async () => {
onUpdate = jest.fn();

await act(async () => {
testBed = await setup(httpSetup, {
value: {
processors: [],
},
onFlyoutOpen: jest.fn(),
onUpdate,
});
});

const { component, actions } = testBed;

component.update();

// Open flyout to add new processor
actions.addProcessor();
// Add type (the other fields are not visible until a type is selected)
await actions.addProcessorType(FOREACH_TYPE);
});

test('prevents form submission if required fields are not provided', async () => {
const {
actions: { saveNewProcessor },
form,
} = testBed;

// Click submit button with only the type defined
await saveNewProcessor();

// Expect form error as "field" is a required parameter
expect(form.getErrorsMessages()).toEqual(['A field value is required.']);
});

test('saves with default parameter values', async () => {
const {
actions: { saveNewProcessor },
form,
} = testBed;

// Add "field" value
form.setInputValue('fieldNameField.input', 'test_foreach_processor');

// Save the field
await saveNewProcessor();

const processors = getProcessorValue(onUpdate, FOREACH_TYPE);

expect(processors[0][FOREACH_TYPE]).toEqual({
field: 'test_foreach_processor',
});
});

test('accepts processor definitions that contains escaped characters', async () => {
const {
actions: { saveNewProcessor },
form,
find,
component,
} = testBed;

// Add "field" value
form.setInputValue('fieldNameField.input', 'test_foreach_processor');

await act(async () => {
find('processorField').simulate('change', {
jsonContent: '{"def_1":"""aaa"bbb""", "def_2":"aaa(bbb"}',
});

// advance timers to allow the form to validate
jest.advanceTimersByTime(0);
});
component.update();

// Save the field
await saveNewProcessor();

const processors = getProcessorValue(onUpdate, FOREACH_TYPE);

expect(processors[0][FOREACH_TYPE]).toEqual({
field: 'test_foreach_processor',
// eslint-disable-next-line prettier/prettier
processor: { def_1: 'aaa\"bbb', def_2: 'aaa(bbb' },
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -126,4 +126,41 @@ describe('Processor: Grok', () => {

expect(processors[0][GROK_TYPE].patterns).toEqual([escapedValue]);
});

test('accepts pattern definitions that contains escaped characters', async () => {
const {
actions: { saveNewProcessor },
form,
find,
component,
} = testBed;

// Add "field" value
form.setInputValue('fieldNameField.input', 'test_grok_processor');

// Add pattern 1
form.setInputValue('droppableList.input-0', 'pattern1');

await act(async () => {
find('patternDefinitionsField').simulate('change', {
jsonContent: '{"pattern_1":"""aaa"bbb""", "pattern_2":"aaa(bbb"}',
});

// advance timers to allow the form to validate
jest.advanceTimersByTime(0);
});
component.update();

// Save the field
await saveNewProcessor();

const processors = getProcessorValue(onUpdate, GROK_TYPE);

expect(processors[0][GROK_TYPE]).toEqual({
field: 'test_grok_processor',
patterns: ['pattern1'],
// eslint-disable-next-line prettier/prettier
pattern_definitions: { pattern_1: 'aaa\"bbb', pattern_2: 'aaa(bbb' },
});
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
/*
* 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.
*/

import { act } from 'react-dom/test-utils';
import { setup, SetupResult, getProcessorValue, setupEnvironment } from './processor.helpers';

const INFERENCE_TYPE = 'inference';

describe('Processor: Script', () => {
let onUpdate: jest.Mock;
let testBed: SetupResult;
const { httpSetup } = setupEnvironment();

beforeAll(() => {
jest.useFakeTimers({ legacyFakeTimers: true });
// disable all react-beautiful-dnd development warnings
(window as any)['__@hello-pangea/dnd-disable-dev-warnings'] = true;
});

afterAll(() => {
jest.useRealTimers();
// enable all react-beautiful-dnd development warnings
(window as any)['__@hello-pangea/dnd-disable-dev-warnings'] = false;
});

beforeEach(async () => {
onUpdate = jest.fn();

await act(async () => {
testBed = await setup(httpSetup, {
value: {
processors: [],
},
onFlyoutOpen: jest.fn(),
onUpdate,
});
});

const { component, actions } = testBed;

component.update();

// Open flyout to add new processor
actions.addProcessor();
// Add type (the other fields are not visible until a type is selected)
await actions.addProcessorType(INFERENCE_TYPE);
});

test('prevents form submission if required fields are not provided', async () => {
const {
actions: { saveNewProcessor },
form,
} = testBed;

// Click submit button with only the type defined
await saveNewProcessor();

// Expect form error as "field" is a required parameter
expect(form.getErrorsMessages()).toEqual([
'A deployment, an inference, or a model ID value is required.',
]);
});

test('accepts inference config and field maps that contains escaped characters', async () => {
const {
actions: { saveNewProcessor },
find,
form,
component,
} = testBed;

form.setInputValue('inferenceModelId.input', 'test_inference_processor');

await act(async () => {
find('inferenceConfig').simulate('change', {
jsonContent: '{"inf_conf_1":"""aaa"bbb""", "inf_conf_2": "aaa(bbb"}',
});

// advance timers to allow the form to validate
jest.advanceTimersByTime(0);
});
component.update();

await act(async () => {
find('fieldMap').simulate('change', {
jsonContent: '{"field_map_1":"""aaa"bbb""", "field_map_2": "aaa(bbb"}',
});

// advance timers to allow the form to validate
jest.advanceTimersByTime(0);
});
component.update();

// Save the field
await saveNewProcessor();

const processors = getProcessorValue(onUpdate, INFERENCE_TYPE);

expect(processors[0][INFERENCE_TYPE]).toEqual({
model_id: 'test_inference_processor',
// eslint-disable-next-line prettier/prettier
inference_config: { inf_conf_1: 'aaa\"bbb', inf_conf_2: 'aaa(bbb' },
// eslint-disable-next-line prettier/prettier
field_map: { field_map_1: 'aaa\"bbb', field_map_2: 'aaa(bbb' },
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -199,4 +199,10 @@ type TestSubject =
| 'ignoreMissingPipelineSwitch.input'
| 'destinationField.input'
| 'datasetField.input'
| 'namespaceField.input';
| 'namespaceField.input'
| 'processorField'
| 'paramsField'
| 'scriptSource'
| 'inferenceModelId.input'
| 'inferenceConfig'
| 'fieldMap';
Original file line number Diff line number Diff line change
Expand Up @@ -138,4 +138,40 @@ describe('Processor: Redact', () => {
pattern_definitions: { GITHUB_NAME: '@%{USERNAME}' },
});
});
test('accepts pattern definitions that contains escaped characters', async () => {
const {
actions: { saveNewProcessor },
component,
find,
form,
} = testBed;

// Add "field" value
form.setInputValue('fieldNameField.input', 'test_redact_processor');

// Add one pattern to the list
form.setInputValue('droppableList.input-0', 'pattern1');

await act(async () => {
find('patternDefinitionsField').simulate('change', {
jsonContent: '{"pattern_1":"""aaa"bbb""", "pattern_2":"aaa(bbb"}',
});

// advance timers to allow the form to validate
jest.advanceTimersByTime(0);
});
component.update();

// Save the field
await saveNewProcessor();

const processors = getProcessorValue(onUpdate, REDACT_TYPE);

expect(processors[0][REDACT_TYPE]).toEqual({
field: 'test_redact_processor',
patterns: ['pattern1'],

pattern_definitions: { pattern_1: 'aaa"bbb', pattern_2: 'aaa(bbb' },
});
});
});
Loading

0 comments on commit 9a76b27

Please sign in to comment.