Skip to content

Commit

Permalink
Create a common Int Validator and use it in ingest_pipelines and Inde…
Browse files Browse the repository at this point in the history
…x_lifecycle_management (elastic#196527)

Closes [elastic#110417 ](elastic#110417)
## Summary
In the Ingest Node Pipelines section, when the users created a new
pipeline selecting de Community ID processor the users could set a
non-integer number in this field. Then, they received a server side
error when tried to create a pipeline. For fixing this, a validation
must be added in the client.

We didn't have a reusable validation for this case, but we did have a
custom validation for integer values in the Index lifecycle management
plugin. We also had the necessary translation in that plugin. So I went
forward with:

* I created a new integer validator in the `es_ui_shared` package as it
is a fairly common validation and we could take advantage of it in the
future. Also added a couple of unit test there for this validator.

* I reused in the `ingest_pipelines` plugin the strings that already
existed in `index_lifecycle_management`.

* I added the new validation in the Community ID form in the
`ingest_pipelines` plugin. Also added some test verifying that the
processor doesn't create when the seeds validation fails.

* Changed the method in the `index_lifecycle_management` validator so
now it uses the reusable one.

Now the Ingest pipeline forms shows the validation when the number is
not an integer:
![Screenshot 2024-10-16 at 12 16
47](https://github.com/user-attachments/assets/1db9ad22-b144-44a5-9012-d3ebd5a19b6f)

And the `index_lifecycle_management` still shows the validations as
expected:

<img width="756" alt="Screenshot 2024-10-16 at 11 49 53"
src="https://github.com/user-attachments/assets/680886a5-355e-4637-9da9-4b93b396e751">

(cherry picked from commit 5a42e58)
  • Loading branch information
SoniaSanzV committed Oct 24, 2024
1 parent ccc7398 commit 1c1ac89
Show file tree
Hide file tree
Showing 12 changed files with 151 additions and 28 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -20,3 +20,4 @@ export * from './lowercase_string';
export * from './is_json';
export * from './number_greater_than';
export * from './number_smaller_than';
export * from './is_integer';
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
/*
* 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", the "GNU Affero General Public License v3.0 only", and the "Server Side
* Public License v 1"; you may not use this file except in compliance with, at
* your election, the "Elastic License 2.0", the "GNU Affero General Public
* License v3.0 only", or the "Server Side Public License, v 1".
*/

import { ValidationFuncArg } from '../../hook_form_lib';
import { isInteger } from './is_integer';

describe('isInteger', () => {
const message = 'test error message';
const code = 'ERR_NOT_INT_NUMBER';

const validate = isInteger({ message });
const validator = (value: unknown) => validate({ value } as ValidationFuncArg<any, any>);

test('should return undefined if value is integer number', () => {
expect(validator(5)).toBeUndefined();
});

test('should return undefined if value string that can be parsed to integer', () => {
expect(validator('5')).toBeUndefined();
});

test('should return Validation function if value is not integer number', () => {
expect(validator(5.3)).toMatchObject({ message, code });
});

test('should return Validation function if value a string that can not be parsed to number but is not an integer', () => {
expect(validator('5.3')).toMatchObject({ message, code });
});

test('should return Validation function if value a string that can not be parsed to number', () => {
expect(validator('test')).toMatchObject({ message, code });
});

test('should return Validation function if value is boolean', () => {
expect(validator(false)).toMatchObject({ message, code });
});

test('should return undefined if value is empty', () => {
expect(validator('')).toBeUndefined();
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
/*
* 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", the "GNU Affero General Public License v3.0 only", and the "Server Side
* Public License v 1"; you may not use this file except in compliance with, at
* your election, the "Elastic License 2.0", the "GNU Affero General Public
* License v3.0 only", or the "Server Side Public License, v 1".
*/

import { ValidationFunc } from '../../hook_form_lib';
import { ERROR_CODE } from './types';

export const isInteger =
({ message }: { message: string }) =>
(...args: Parameters<ValidationFunc>): ReturnType<ValidationFunc<any, ERROR_CODE>> => {
const [{ value }] = args;

if (
value === '' ||
(typeof value === 'number' && Number.isInteger(value)) ||
(typeof value === 'string' && Number.isInteger(Number(value)))
) {
return undefined;
}

return { message, code: 'ERR_NOT_INT_NUMBER' };
};
Original file line number Diff line number Diff line change
Expand Up @@ -19,4 +19,5 @@ export type ERROR_CODE =
| 'ERR_LOWERCASE_STRING'
| 'ERR_JSON_FORMAT'
| 'ERR_SMALLER_THAN_NUMBER'
| 'ERR_GREATER_THAN_NUMBER';
| 'ERR_GREATER_THAN_NUMBER'
| 'ERR_NOT_INT_NUMBER';
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,14 @@ import { i18nTexts } from '../i18n_texts';
import {
ifExistsNumberGreaterThanZero,
ifExistsNumberNonNegative,
integerValidator,
minAgeGreaterThanPreviousPhase,
rolloverThresholdsValidator,
downsampleIntervalMultipleOfPreviousOne,
} from './validations';

const rolloverFormPaths = Object.values(ROLLOVER_FORM_PATHS);

const { emptyField, numberGreaterThanField } = fieldValidators;
const { emptyField, isInteger, numberGreaterThanField } = fieldValidators;

const serializers = {
stringToNumber: (v: string): any => (v != null ? parseInt(v, 10) : undefined),
Expand Down Expand Up @@ -150,7 +149,7 @@ const getMinAgeField = (phase: PhaseWithTiming, defaultValue?: string) => ({
validator: ifExistsNumberNonNegative,
},
{
validator: integerValidator,
validator: isInteger({ message: i18nTexts.editPolicy.errors.integerRequired }),
},
{
validator: minAgeGreaterThanPreviousPhase(phase),
Expand Down Expand Up @@ -192,7 +191,7 @@ const getDownsampleSchema = (phase: PhaseWithDownsample): FormSchema['downsample
validator: ifExistsNumberGreaterThanZero,
},
{
validator: integerValidator,
validator: isInteger({ message: i18nTexts.editPolicy.errors.integerRequired }),
},
{
validator: downsampleIntervalMultipleOfPreviousOne(phase),
Expand Down Expand Up @@ -381,7 +380,7 @@ export const getSchema = (isCloudEnabled: boolean): FormSchema => ({
validator: ifExistsNumberGreaterThanZero,
},
{
validator: integerValidator,
validator: isInteger({ message: i18nTexts.editPolicy.errors.integerRequired }),
},
],
fieldsToValidateOnChange: rolloverFormPaths,
Expand All @@ -396,7 +395,7 @@ export const getSchema = (isCloudEnabled: boolean): FormSchema => ({
validator: ifExistsNumberGreaterThanZero,
},
{
validator: integerValidator,
validator: isInteger({ message: i18nTexts.editPolicy.errors.integerRequired }),
},
],
serializer: serializers.stringToNumber,
Expand Down Expand Up @@ -424,7 +423,7 @@ export const getSchema = (isCloudEnabled: boolean): FormSchema => ({
validator: ifExistsNumberGreaterThanZero,
},
{
validator: integerValidator,
validator: isInteger({ message: i18nTexts.editPolicy.errors.integerRequired }),
},
],
serializer: serializers.stringToNumber,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,12 +101,6 @@ export const rolloverThresholdsValidator: ValidationFunc = ({ form, path }) => {
}
};

export const integerValidator: ValidationFunc<FormInternal, string, string> = (arg) => {
if (!Number.isInteger(Number(arg.value ?? ''))) {
return { message: i18nTexts.editPolicy.errors.integerRequired };
}
};

export const createPolicyNameValidations = ({
policies,
isClonedPolicy,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,19 +67,14 @@ const configurationFormSchema: FormSchema = {
formatters: [fieldFormatters.toInt],
validations: [
{
validator: ({ value }) => {
// TODO: Replace with validator added in https://github.com/elastic/kibana/pull/196527/
if (!Number.isInteger(Number(value ?? ''))) {
return {
message: i18n.translate(
'xpack.idxMgmt.dataStreamsDetailsPanel.editDataRetentionModal.dataRetentionFieldIntegerError',
{
defaultMessage: 'Only integers are allowed.',
}
),
};
}
},
validator: fieldValidators.isInteger({
message: i18n.translate(
'xpack.idxMgmt.dataStreamsDetailsPanel.editDataRetentionModal.dataRetentionFieldIntegerError',
{
defaultMessage: 'Only integers are allowed.',
}
),
}),
},
{
validator: ({ value, formData, customData }) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,4 +107,52 @@ describe('Processor: Community id', () => {
seed: 10,
});
});

test('should not add a processor if the seedField is smaller than min_value', async () => {
const {
actions: { saveNewProcessor },
form,
} = testBed;

form.setInputValue('seedField.input', '-1');

// Save the field with new changes
await saveNewProcessor();

const processors = getProcessorValue(onUpdate, COMMUNITY_ID_TYPE);

expect(processors).toHaveLength(0);
});

test('should not add a processor if the seedField is bigger than max_value', async () => {
const {
actions: { saveNewProcessor },
form,
} = testBed;

form.setInputValue('seedField.input', '65536');

// Save the field with new changes
await saveNewProcessor();

const processors = getProcessorValue(onUpdate, COMMUNITY_ID_TYPE);

expect(processors).toHaveLength(0);
});

test('should not add a processor if the seedField is not an integer', async () => {
const {
actions: { saveNewProcessor },
form,
} = testBed;

form.setInputValue('seedField.input', '10.2');

// Save the field with new changes
await saveNewProcessor();

const processors = getProcessorValue(onUpdate, COMMUNITY_ID_TYPE);

expect(processors).toHaveLength(0);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,14 @@ const seedValidator = {
values: { minValue: SEED_MIN_VALUE },
}),
}),
int: fieldValidators.isInteger({
message: i18n.translate(
'xpack.ingestPipelines.pipelineEditor.communityId.integerRequiredError',
{
defaultMessage: 'Only integers are allowed.',
}
),
}),
};

const fieldsConfig: FieldsConfig = {
Expand Down Expand Up @@ -183,7 +191,7 @@ const fieldsConfig: FieldsConfig = {
{
validator: (field) => {
if (field.value) {
return seedValidator.max(field) ?? seedValidator.min(field);
return seedValidator.max(field) ?? seedValidator.min(field) ?? seedValidator.int(field);
}
},
},
Expand Down
1 change: 1 addition & 0 deletions x-pack/plugins/translations/translations/fr-FR.json
Original file line number Diff line number Diff line change
Expand Up @@ -24131,6 +24131,7 @@
"xpack.ingestPipelines.pipelineEditor.communityId.icmpCodeLabel": "Code ICMP (facultatif)",
"xpack.ingestPipelines.pipelineEditor.communityId.icmpTypeHelpText": "Champ contenant le type ICMP de la destination. La valeur par défaut est {defaultValue}.",
"xpack.ingestPipelines.pipelineEditor.communityId.icmpTypeLabel": "Type ICMP (facultatif)",
"xpack.ingestPipelines.pipelineEditor.communityId.integerRequiredError": "Seuls les entiers sont autorisés.",
"xpack.ingestPipelines.pipelineEditor.communityId.seedHelpText": "Valeur initiale du hachage de l'ID de communauté. La valeur par défaut est {defaultValue}.",
"xpack.ingestPipelines.pipelineEditor.communityId.seedLabel": "Valeur initiale (facultatif)",
"xpack.ingestPipelines.pipelineEditor.communityId.seedMaxNumberError": "Ce nombre doit être inférieur ou égal à {maxValue}.",
Expand Down
1 change: 1 addition & 0 deletions x-pack/plugins/translations/translations/ja-JP.json
Original file line number Diff line number Diff line change
Expand Up @@ -23878,6 +23878,7 @@
"xpack.ingestPipelines.pipelineEditor.communityId.icmpCodeLabel": "ICMPコード(任意)",
"xpack.ingestPipelines.pipelineEditor.communityId.icmpTypeHelpText": "デスティネーションICMPタイプを含むフィールド。デフォルトは{defaultValue}です。",
"xpack.ingestPipelines.pipelineEditor.communityId.icmpTypeLabel": "ICMPタイプ(任意)",
"xpack.ingestPipelines.pipelineEditor.communityId.integerRequiredError": "整数のみを使用できます。",
"xpack.ingestPipelines.pipelineEditor.communityId.seedHelpText": "コミュニティIDハッシュのシード。デフォルトは{defaultValue}です。",
"xpack.ingestPipelines.pipelineEditor.communityId.seedLabel": "シード(任意)",
"xpack.ingestPipelines.pipelineEditor.communityId.seedMaxNumberError": "この数は{maxValue}以下でなければなりません。",
Expand Down
1 change: 1 addition & 0 deletions x-pack/plugins/translations/translations/zh-CN.json
Original file line number Diff line number Diff line change
Expand Up @@ -23912,6 +23912,7 @@
"xpack.ingestPipelines.pipelineEditor.communityId.icmpCodeLabel": "ICMP 代码(可选)",
"xpack.ingestPipelines.pipelineEditor.communityId.icmpTypeHelpText": "包含目标 ICMP 类型的字段。默认为 {defaultValue}。",
"xpack.ingestPipelines.pipelineEditor.communityId.icmpTypeLabel": "ICMP 类型(可选)",
"xpack.ingestPipelines.pipelineEditor.communityId.integerRequiredError": "仅允许使用整数。",
"xpack.ingestPipelines.pipelineEditor.communityId.seedHelpText": "社区 ID 哈希的种子。默认为 {defaultValue}。",
"xpack.ingestPipelines.pipelineEditor.communityId.seedLabel": "种子(可选)",
"xpack.ingestPipelines.pipelineEditor.communityId.seedMaxNumberError": "此数字必须等于或小于 {maxValue}。",
Expand Down

0 comments on commit 1c1ac89

Please sign in to comment.