From 267ffd1bb39ba292d793ba540479d80c94075bbd Mon Sep 17 00:00:00 2001 From: Jean-Louis Leysens Date: Tue, 10 Nov 2020 16:58:08 +0100 Subject: [PATCH 1/9] fix ilm policy deserialization --- .../sections/edit_policy/form/schema.ts | 2 +- .../sections/edit_policy/form/serializer.ts | 233 ++++++++++-------- 2 files changed, 132 insertions(+), 103 deletions(-) diff --git a/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/form/schema.ts b/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/form/schema.ts index 4d20db40187409..0ad2d923117f45 100644 --- a/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/form/schema.ts +++ b/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/form/schema.ts @@ -23,7 +23,7 @@ import { i18nTexts } from '../i18n_texts'; const { emptyField, numberGreaterThanField } = fieldValidators; const serializers = { - stringToNumber: (v: string): any => (v ? parseInt(v, 10) : undefined), + stringToNumber: (v: string): any => (v != null ? parseInt(v, 10) : undefined), }; export const schema: FormSchema = { diff --git a/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/form/serializer.ts b/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/form/serializer.ts index 2274efda426ad1..312bf1714315a7 100644 --- a/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/form/serializer.ts +++ b/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/form/serializer.ts @@ -4,10 +4,14 @@ * you may not use this file except in compliance with the Elastic License. */ -import { isEmpty, isNumber } from 'lodash'; +import { produce } from 'immer'; + +import { isEmpty, merge } from 'lodash'; import { SerializedPolicy, SerializedActionWithAllocation } from '../../../../../common/types'; +import { defaultPolicy } from '../../../constants'; + import { FormInternal, DataAllocationMetaFields } from '../types'; const serializeAllocateAction = ( @@ -16,8 +20,16 @@ const serializeAllocateAction = ( originalActions: SerializedActionWithAllocation = {} ): SerializedActionWithAllocation => { const { allocate, migrate, ...rest } = newActions; - // First copy over all non-allocate and migrate actions. - const actions: SerializedActionWithAllocation = { allocate, migrate, ...rest }; + // First copy over all non-require|include|exclude and migrate actions. + const actions: SerializedActionWithAllocation = { ...rest }; + + // We only set include, exclude and require here, so copy over all other values + if (allocate) { + const { include, exclude, require, ...restAllocate } = allocate; + if (!isEmpty(restAllocate)) { + actions.allocate = { ...restAllocate }; + } + } switch (dataTierAllocationType) { case 'node_attrs': @@ -63,123 +75,140 @@ const serializeAllocateAction = ( export const createSerializer = (originalPolicy?: SerializedPolicy) => ( data: FormInternal ): SerializedPolicy => { - const { _meta, ...policy } = data; + const { _meta, ...updatedPolicy } = data; - if (!policy.phases || !policy.phases.hot) { - policy.phases = { hot: { actions: {} } }; + if (!updatedPolicy.phases || !updatedPolicy.phases.hot) { + updatedPolicy.phases = { hot: { actions: {} } }; } - /** - * HOT PHASE SERIALIZATION - */ - if (policy.phases.hot) { - policy.phases.hot.min_age = originalPolicy?.phases.hot?.min_age ?? '0ms'; - } + return produce(originalPolicy ?? defaultPolicy, (draft) => { + // Copy over all updated fields + merge(draft, updatedPolicy); - if (policy.phases.hot?.actions) { - if (policy.phases.hot.actions?.rollover && _meta.hot.useRollover) { - if (policy.phases.hot.actions.rollover.max_age) { - policy.phases.hot.actions.rollover.max_age = `${policy.phases.hot.actions.rollover.max_age}${_meta.hot.maxAgeUnit}`; - } + // Next copy over all meta fields and delete any fields that have been removed + // by fields exposed in the form. It is very important that we do not delete + // data that the form does not control! E.g., unfollow action in hot phase. - if (policy.phases.hot.actions.rollover.max_size) { - policy.phases.hot.actions.rollover.max_size = `${policy.phases.hot.actions.rollover.max_size}${_meta.hot.maxStorageSizeUnit}`; + /** + * HOT PHASE SERIALIZATION + */ + if (draft.phases.hot) { + draft.phases.hot.min_age = draft.phases.hot.min_age ?? '0ms'; + } + + if (draft.phases.hot?.actions) { + if (draft.phases.hot.actions?.rollover && _meta.hot.useRollover) { + if (draft.phases.hot.actions.rollover.max_age) { + draft.phases.hot.actions.rollover.max_age = `${draft.phases.hot.actions.rollover.max_age}${_meta.hot.maxAgeUnit}`; + } + + if (draft.phases.hot.actions.rollover.max_size) { + draft.phases.hot.actions.rollover.max_size = `${draft.phases.hot.actions.rollover.max_size}${_meta.hot.maxStorageSizeUnit}`; + } + + if (_meta.hot.bestCompression && draft.phases.hot.actions?.forcemerge) { + draft.phases.hot.actions.forcemerge.index_codec = 'best_compression'; + } + } else { + delete draft.phases.hot.actions.rollover; + delete draft.phases.hot.actions.forcemerge; } - if (_meta.hot.bestCompression && policy.phases.hot.actions?.forcemerge) { - policy.phases.hot.actions.forcemerge.index_codec = 'best_compression'; + if ( + !updatedPolicy.phases.hot!.actions?.set_priority && + draft.phases.hot.actions.set_priority + ) { + delete draft.phases.hot.actions.set_priority; } - } else { - delete policy.phases.hot.actions?.rollover; } - } - /** - * WARM PHASE SERIALIZATION - */ - if (policy.phases.warm) { - // If warm phase on rollover is enabled, delete min age field - // An index lifecycle switches to warm phase when rollover occurs, so you cannot specify a warm phase time - // They are mutually exclusive - if (_meta.hot.useRollover && _meta.warm.warmPhaseOnRollover) { - delete policy.phases.warm.min_age; - } else if ( - (!_meta.hot.useRollover || !_meta.warm.warmPhaseOnRollover) && - policy.phases.warm.min_age - ) { - policy.phases.warm.min_age = `${policy.phases.warm.min_age}${_meta.warm.minAgeUnit}`; - } + /** + * WARM PHASE SERIALIZATION + */ + if (_meta.warm.enabled) { + const warmPhase = draft.phases.warm!; + // If warm phase on rollover is enabled, delete min age field + // An index lifecycle switches to warm phase when rollover occurs, so you cannot specify a warm phase time + // They are mutually exclusive + if ( + (!_meta.hot.useRollover || !_meta.warm.warmPhaseOnRollover) && + updatedPolicy.phases.warm!.min_age + ) { + warmPhase.min_age = `${updatedPolicy.phases.warm!.min_age}${_meta.warm.minAgeUnit}`; + } else { + delete warmPhase.min_age; + } - policy.phases.warm.actions = serializeAllocateAction( - _meta.warm, - policy.phases.warm.actions, - originalPolicy?.phases.warm?.actions - ); - - if ( - policy.phases.warm.actions.allocate && - !policy.phases.warm.actions.allocate.require && - !isNumber(policy.phases.warm.actions.allocate.number_of_replicas) && - isEmpty(policy.phases.warm.actions.allocate.include) && - isEmpty(policy.phases.warm.actions.allocate.exclude) - ) { - // remove allocate action if it does not define require or number of nodes - // and both include and exclude are empty objects (ES will fail to parse if we don't) - delete policy.phases.warm.actions.allocate; - } + warmPhase.actions = serializeAllocateAction( + _meta.warm, + updatedPolicy.phases.warm!.actions, + originalPolicy?.phases.warm?.actions + ); - if (_meta.warm.bestCompression && policy.phases.warm.actions?.forcemerge) { - policy.phases.warm.actions.forcemerge.index_codec = 'best_compression'; - } - } + if (!updatedPolicy.phases.warm!.actions?.forcemerge) { + delete warmPhase.actions.forcemerge; + } else if (_meta.warm.bestCompression) { + warmPhase.actions.forcemerge!.index_codec = 'best_compression'; + } - /** - * COLD PHASE SERIALIZATION - */ - if (policy.phases.cold) { - if (policy.phases.cold.min_age) { - policy.phases.cold.min_age = `${policy.phases.cold.min_age}${_meta.cold.minAgeUnit}`; - } + if (!updatedPolicy.phases.warm!.actions?.set_priority && warmPhase.actions.set_priority) { + delete warmPhase.actions.set_priority; + } - policy.phases.cold.actions = serializeAllocateAction( - _meta.cold, - policy.phases.cold.actions, - originalPolicy?.phases.cold?.actions - ); - - if ( - policy.phases.cold.actions.allocate && - !policy.phases.cold.actions.allocate.require && - !isNumber(policy.phases.cold.actions.allocate.number_of_replicas) && - isEmpty(policy.phases.cold.actions.allocate.include) && - isEmpty(policy.phases.cold.actions.allocate.exclude) - ) { - // remove allocate action if it does not define require or number of nodes - // and both include and exclude are empty objects (ES will fail to parse if we don't) - delete policy.phases.cold.actions.allocate; + if (!updatedPolicy.phases.warm!.actions?.shrink) { + delete warmPhase.actions.shrink; + } + } else { + delete draft.phases.warm; } - if (_meta.cold.freezeEnabled) { - policy.phases.cold.actions.freeze = {}; - } - } + /** + * COLD PHASE SERIALIZATION + */ + if (_meta.cold.enabled) { + const coldPhase = draft.phases.cold!; - /** - * DELETE PHASE SERIALIZATION - */ - if (policy.phases.delete) { - if (policy.phases.delete.min_age) { - policy.phases.delete.min_age = `${policy.phases.delete.min_age}${_meta.delete.minAgeUnit}`; - } + if (updatedPolicy.phases.cold!.min_age) { + coldPhase.min_age = `${updatedPolicy.phases.cold!.min_age}${_meta.cold.minAgeUnit}`; + } + + coldPhase.actions = serializeAllocateAction( + _meta.cold, + updatedPolicy.phases.cold!.actions, + originalPolicy?.phases.cold?.actions + ); + + if (_meta.cold.freezeEnabled) { + coldPhase.actions.freeze = {}; + } else { + delete coldPhase.actions.freeze; + } - if (originalPolicy?.phases.delete?.actions) { - const { wait_for_snapshot: __, ...rest } = originalPolicy.phases.delete.actions; - policy.phases.delete.actions = { - ...policy.phases.delete.actions, - ...rest, - }; + if (!updatedPolicy.phases.cold!.actions?.set_priority && coldPhase.actions.set_priority) { + delete coldPhase.actions.set_priority; + } + } else { + delete draft.phases.cold; } - } - return policy; + /** + * DELETE PHASE SERIALIZATION + */ + if (_meta.delete.enabled) { + const deletePhase = draft.phases.delete!; + if (updatedPolicy.phases.delete!.min_age) { + deletePhase.min_age = `${updatedPolicy.phases.delete!.min_age}${_meta.delete.minAgeUnit}`; + } + + if (originalPolicy?.phases.delete?.actions) { + const { wait_for_snapshot: __, ...rest } = originalPolicy.phases.delete.actions; + deletePhase.actions = { + ...deletePhase.actions, + ...rest, + }; + } + } else { + delete draft.phases.delete; + } + }); }; From 0991acbb7e17b7461ea184a2eb498691e358bbd0 Mon Sep 17 00:00:00 2001 From: Jean-Louis Leysens Date: Tue, 10 Nov 2020 17:01:59 +0100 Subject: [PATCH 2/9] reorder expected jest object to match actual --- .../__jest__/components/edit_policy.test.tsx | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/x-pack/plugins/index_lifecycle_management/__jest__/components/edit_policy.test.tsx b/x-pack/plugins/index_lifecycle_management/__jest__/components/edit_policy.test.tsx index 43910583ceec91..8837c35d359687 100644 --- a/x-pack/plugins/index_lifecycle_management/__jest__/components/edit_policy.test.tsx +++ b/x-pack/plugins/index_lifecycle_management/__jest__/components/edit_policy.test.tsx @@ -298,12 +298,12 @@ describe('edit policy', () => { phases: { hot: { actions: { - set_priority: { - priority: 100, - }, rollover: { - max_size: '50gb', max_age: '30d', + max_size: '50gb', + }, + set_priority: { + priority: 100, }, }, min_age: '0ms', From c40e650c6280d402a7c2ed94bc1ba5bfc7acbad3 Mon Sep 17 00:00:00 2001 From: Jean-Louis Leysens Date: Tue, 10 Nov 2020 17:05:04 +0100 Subject: [PATCH 3/9] fix removal of wait for snapshot if it is not on the form --- .../sections/edit_policy/form/serializer.ts | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/form/serializer.ts b/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/form/serializer.ts index 312bf1714315a7..20ff3582b112ec 100644 --- a/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/form/serializer.ts +++ b/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/form/serializer.ts @@ -200,12 +200,11 @@ export const createSerializer = (originalPolicy?: SerializedPolicy) => ( deletePhase.min_age = `${updatedPolicy.phases.delete!.min_age}${_meta.delete.minAgeUnit}`; } - if (originalPolicy?.phases.delete?.actions) { - const { wait_for_snapshot: __, ...rest } = originalPolicy.phases.delete.actions; - deletePhase.actions = { - ...deletePhase.actions, - ...rest, - }; + if ( + !updatedPolicy.phases.delete!.actions?.wait_for_snapshot && + deletePhase.actions.wait_for_snapshot + ) { + delete deletePhase.actions.wait_for_snapshot; } } else { delete draft.phases.delete; From e14196620ba33c43e69dae186b1400c8d3d239ae Mon Sep 17 00:00:00 2001 From: Jean-Louis Leysens Date: Tue, 10 Nov 2020 17:29:13 +0100 Subject: [PATCH 4/9] add client integration test for policy serialization of unknown fields --- .../edit_policy/constants.ts | 38 +++++++++++ .../edit_policy/edit_policy.test.ts | 65 +++++++++++++++++++ .../sections/edit_policy/form/serializer.ts | 4 +- 3 files changed, 105 insertions(+), 2 deletions(-) diff --git a/x-pack/plugins/index_lifecycle_management/__jest__/client_integration/edit_policy/constants.ts b/x-pack/plugins/index_lifecycle_management/__jest__/client_integration/edit_policy/constants.ts index 00c7d705c1f44a..68b2ac59d2a190 100644 --- a/x-pack/plugins/index_lifecycle_management/__jest__/client_integration/edit_policy/constants.ts +++ b/x-pack/plugins/index_lifecycle_management/__jest__/client_integration/edit_policy/constants.ts @@ -195,3 +195,41 @@ export const POLICY_WITH_NODE_ROLE_ALLOCATION: PolicyFromES = { }, name: POLICY_NAME, }; + +export const POLICY_WITH_KNOWN_AND_UNKNOWN_FIELDS = ({ + version: 1, + modified_date: Date.now().toString(), + policy: { + foo: 'bar', + phases: { + hot: { + min_age: '0ms', + actions: { + rollover: { + unknown_setting: 123, + max_size: '50gb', + }, + }, + }, + warm: { + actions: { + my_unfollow_action: {}, + set_priority: { + priority: 22, + unknown_setting: true, + }, + }, + }, + delete: { + wait_for_snapshot: { + policy: SNAPSHOT_POLICY_NAME, + }, + delete: { + delete_searchable_snapshot: true, + }, + }, + }, + name: POLICY_NAME, + }, + name: POLICY_NAME, +} as any) as PolicyFromES; diff --git a/x-pack/plugins/index_lifecycle_management/__jest__/client_integration/edit_policy/edit_policy.test.ts b/x-pack/plugins/index_lifecycle_management/__jest__/client_integration/edit_policy/edit_policy.test.ts index c91ee3e2a1c068..a203a434bb21a4 100644 --- a/x-pack/plugins/index_lifecycle_management/__jest__/client_integration/edit_policy/edit_policy.test.ts +++ b/x-pack/plugins/index_lifecycle_management/__jest__/client_integration/edit_policy/edit_policy.test.ts @@ -19,6 +19,7 @@ import { POLICY_WITH_INCLUDE_EXCLUDE, POLICY_WITH_NODE_ATTR_AND_OFF_ALLOCATION, POLICY_WITH_NODE_ROLE_ALLOCATION, + POLICY_WITH_KNOWN_AND_UNKNOWN_FIELDS, getDefaultHotPhasePolicy, } from './constants'; @@ -31,6 +32,70 @@ describe('', () => { server.restore(); }); + describe('serialization', () => { + /** + * We assume that policies that populate this form are loaded directly from ES and so + * are valid according to ES. There may be settings in the policy created through the ILM + * API that the UI does not cater for, like the unfollow action. We do not want to overwrite + * the configuration for these actions in the UI. + */ + it('preserves policy settings it did not configure', async () => { + httpRequestsMockHelpers.setLoadPolicies([POLICY_WITH_KNOWN_AND_UNKNOWN_FIELDS]); + httpRequestsMockHelpers.setLoadSnapshotPolicies([]); + httpRequestsMockHelpers.setListNodes({ + nodesByRoles: {}, + nodesByAttributes: { test: ['123'] }, + isUsingDeprecatedDataRoleConfig: false, + }); + + await act(async () => { + testBed = await setup(); + }); + + const { component, actions } = testBed; + component.update(); + + // Set max docs to test whether we keep the unknown fields in that object after serializing + await actions.hot.setMaxDocs('1000'); + // Remove the delete phase to ensure that we also correctly remove data + await actions.delete.enable(false); + await actions.savePolicy(); + + const latestRequest = server.requests[server.requests.length - 1]; + const entirePolicy = JSON.parse(JSON.parse(latestRequest.requestBody).body); + + expect(entirePolicy).toEqual({ + foo: 'bar', // Made up value + name: 'my_policy', + phases: { + hot: { + actions: { + rollover: { + max_docs: 1000, + max_size: '50gb', + unknown_setting: 123, // Made up setting that should stay preserved + }, + set_priority: { + priority: 100, + }, + }, + min_age: '0ms', + }, + warm: { + actions: { + my_unfollow_action: {}, // Made up action + set_priority: { + priority: 22, + unknown_setting: true, + }, + }, + min_age: '0ms', + }, + }, + }); + }); + }); + describe('hot phase', () => { describe('serialization', () => { beforeEach(async () => { diff --git a/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/form/serializer.ts b/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/form/serializer.ts index 20ff3582b112ec..2866d7fb7f25b3 100644 --- a/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/form/serializer.ts +++ b/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/form/serializer.ts @@ -141,7 +141,7 @@ export const createSerializer = (originalPolicy?: SerializedPolicy) => ( warmPhase.actions = serializeAllocateAction( _meta.warm, - updatedPolicy.phases.warm!.actions, + warmPhase.actions, originalPolicy?.phases.warm?.actions ); @@ -174,7 +174,7 @@ export const createSerializer = (originalPolicy?: SerializedPolicy) => ( coldPhase.actions = serializeAllocateAction( _meta.cold, - updatedPolicy.phases.cold!.actions, + coldPhase.actions, originalPolicy?.phases.cold?.actions ); From 57b71964f4a51af37439e2dfa0204ce001be8112 Mon Sep 17 00:00:00 2001 From: Jean-Louis Leysens Date: Tue, 10 Nov 2020 17:47:20 +0100 Subject: [PATCH 5/9] save on a few chars --- .../sections/edit_policy/form/serializer.ts | 26 +++++++++---------- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/form/serializer.ts b/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/form/serializer.ts index 2866d7fb7f25b3..b647d5aaf078dc 100644 --- a/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/form/serializer.ts +++ b/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/form/serializer.ts @@ -97,28 +97,26 @@ export const createSerializer = (originalPolicy?: SerializedPolicy) => ( } if (draft.phases.hot?.actions) { - if (draft.phases.hot.actions?.rollover && _meta.hot.useRollover) { - if (draft.phases.hot.actions.rollover.max_age) { - draft.phases.hot.actions.rollover.max_age = `${draft.phases.hot.actions.rollover.max_age}${_meta.hot.maxAgeUnit}`; + const hotPhaseActions = draft.phases.hot.actions; + if (hotPhaseActions.rollover && _meta.hot.useRollover) { + if (hotPhaseActions.rollover.max_age) { + hotPhaseActions.rollover.max_age = `${hotPhaseActions.rollover.max_age}${_meta.hot.maxAgeUnit}`; } - if (draft.phases.hot.actions.rollover.max_size) { - draft.phases.hot.actions.rollover.max_size = `${draft.phases.hot.actions.rollover.max_size}${_meta.hot.maxStorageSizeUnit}`; + if (hotPhaseActions.rollover.max_size) { + hotPhaseActions.rollover.max_size = `${hotPhaseActions.rollover.max_size}${_meta.hot.maxStorageSizeUnit}`; } - if (_meta.hot.bestCompression && draft.phases.hot.actions?.forcemerge) { - draft.phases.hot.actions.forcemerge.index_codec = 'best_compression'; + if (_meta.hot.bestCompression && hotPhaseActions.forcemerge) { + hotPhaseActions.forcemerge.index_codec = 'best_compression'; } } else { - delete draft.phases.hot.actions.rollover; - delete draft.phases.hot.actions.forcemerge; + delete hotPhaseActions.rollover; + delete hotPhaseActions.forcemerge; } - if ( - !updatedPolicy.phases.hot!.actions?.set_priority && - draft.phases.hot.actions.set_priority - ) { - delete draft.phases.hot.actions.set_priority; + if (!updatedPolicy.phases.hot!.actions?.set_priority && hotPhaseActions.set_priority) { + delete hotPhaseActions.set_priority; } } From adf967d06517d1d33594f8b2a4112289b522863a Mon Sep 17 00:00:00 2001 From: Jean-Louis Leysens Date: Wed, 11 Nov 2020 16:21:36 +0100 Subject: [PATCH 6/9] added unit test for deserializer and serializer --- .../form/deserializer_and_serializer.test.ts | 88 +++++++++++++++++++ 1 file changed, 88 insertions(+) create mode 100644 x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/form/deserializer_and_serializer.test.ts diff --git a/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/form/deserializer_and_serializer.test.ts b/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/form/deserializer_and_serializer.test.ts new file mode 100644 index 00000000000000..15837c8d612451 --- /dev/null +++ b/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/form/deserializer_and_serializer.test.ts @@ -0,0 +1,88 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ +import { cloneDeep, set } from 'lodash'; +import { SerializedPolicy } from '../../../../../common/types'; +import { deserializer } from './deserializer'; +import { createSerializer } from './serializer'; + +describe('deserializer and serializer', () => { + it('preserves unknown policy settings', () => { + const originalPolicy: SerializedPolicy = { + name: 'test', + phases: { + hot: { + actions: { + rollover: { + max_age: '1d', + max_size: '10gb', + max_docs: 1000, + }, + forcemerge: { + index_codec: 'best_compression', + max_num_segments: 22, + }, + }, + min_age: '12ms', + }, + warm: { + actions: { + shrink: { number_of_shards: 12 }, + allocate: { + number_of_replicas: 3, + }, + set_priority: { + priority: 10, + }, + }, + }, + cold: { + actions: { + allocate: { number_of_replicas: 12 }, + freeze: {}, + set_priority: { + priority: 12, + }, + }, + }, + delete: { + actions: { + delete: { + delete_searchable_snapshot: true, + }, + wait_for_snapshot: { + policy: 'test', + }, + }, + }, + }, + }; + + /** + * Next we insert unknown values at various places in the policy object + * where we know they may occur. + */ + [ + '', + 'phases.hot.actions', + 'phases.hot.actions.rollover', + 'phases.warm.actions', + 'phases.warm.actions.forcemerge', + 'phases.cold.actions', + 'phases.delete.actions', + ].forEach((path) => { + set(originalPolicy, path, { unknown: 'value' }); + }); + + const copyOfOriginalPolicy = cloneDeep(originalPolicy); + const formInternal = deserializer(copyOfOriginalPolicy); + const serializer = createSerializer(copyOfOriginalPolicy); + + expect(serializer(formInternal)).toEqual(originalPolicy); + + // Assert that the original policy is unaltered after deserialization and serialization + expect(originalPolicy).toEqual(copyOfOriginalPolicy); + }); +}); From 79c153630d7156f76661f3a69eaf04ad0c08d374 Mon Sep 17 00:00:00 2001 From: Jean-Louis Leysens Date: Wed, 11 Nov 2020 16:34:57 +0100 Subject: [PATCH 7/9] Implement feedback - move serializer function around a little bit - move serialize migrate and allocate function out of serializer file --- .../edit_policy/form/serializer/index.ts | 7 ++ .../serialize_migrate_and_allocate_actions.ts | 70 ++++++++++++++++++ .../form/{ => serializer}/serializer.ts | 72 +++---------------- 3 files changed, 85 insertions(+), 64 deletions(-) create mode 100644 x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/form/serializer/index.ts create mode 100644 x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/form/serializer/serialize_migrate_and_allocate_actions.ts rename x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/form/{ => serializer}/serializer.ts (64%) diff --git a/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/form/serializer/index.ts b/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/form/serializer/index.ts new file mode 100644 index 00000000000000..f901bfcf4d49d3 --- /dev/null +++ b/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/form/serializer/index.ts @@ -0,0 +1,7 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +export { createSerializer } from './serializer'; diff --git a/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/form/serializer/serialize_migrate_and_allocate_actions.ts b/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/form/serializer/serialize_migrate_and_allocate_actions.ts new file mode 100644 index 00000000000000..b0c807ccb270e4 --- /dev/null +++ b/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/form/serializer/serialize_migrate_and_allocate_actions.ts @@ -0,0 +1,70 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import { isEmpty } from 'lodash'; + +import { SerializedActionWithAllocation } from '../../../../../../common/types'; + +import { DataAllocationMetaFields } from '../../types'; + +export const serializeMigrateAndAllocateActions = ( + { dataTierAllocationType, allocationNodeAttribute }: DataAllocationMetaFields, + newActions: SerializedActionWithAllocation = {}, + originalActions: SerializedActionWithAllocation = {} +): SerializedActionWithAllocation => { + const { allocate, migrate, ...otherActions } = newActions; + + // First copy over all non-allocate and migrate actions. + const actions: SerializedActionWithAllocation = { ...otherActions }; + + // The UI only knows about include, exclude and require, so copy over all other values. + if (allocate) { + const { include, exclude, require, ...otherSettings } = allocate; + if (!isEmpty(otherSettings)) { + actions.allocate = { ...otherSettings }; + } + } + + switch (dataTierAllocationType) { + case 'node_attrs': + if (allocationNodeAttribute) { + const [name, value] = allocationNodeAttribute.split(':'); + actions.allocate = { + // copy over any other allocate details like "number_of_replicas" + ...actions.allocate, + require: { + [name]: value, + }, + }; + } else { + // The form has been configured to use node attribute based allocation but no node attribute + // was selected. We fall back to what was originally selected in this case. This might be + // migrate.enabled: "false" + actions.migrate = originalActions.migrate; + } + + // copy over the original include and exclude values until we can set them in the form. + if (!isEmpty(originalActions?.allocate?.include)) { + actions.allocate = { + ...actions.allocate, + include: { ...originalActions?.allocate?.include }, + }; + } + + if (!isEmpty(originalActions?.allocate?.exclude)) { + actions.allocate = { + ...actions.allocate, + exclude: { ...originalActions?.allocate?.exclude }, + }; + } + break; + case 'none': + actions.migrate = { enabled: false }; + break; + default: + } + return actions; +}; diff --git a/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/form/serializer.ts b/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/form/serializer/serializer.ts similarity index 64% rename from x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/form/serializer.ts rename to x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/form/serializer/serializer.ts index b647d5aaf078dc..1cdd0b909030af 100644 --- a/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/form/serializer.ts +++ b/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/form/serializer/serializer.ts @@ -6,71 +6,15 @@ import { produce } from 'immer'; -import { isEmpty, merge } from 'lodash'; +import { merge } from 'lodash'; -import { SerializedPolicy, SerializedActionWithAllocation } from '../../../../../common/types'; +import { SerializedPolicy } from '../../../../../../common/types'; -import { defaultPolicy } from '../../../constants'; +import { defaultPolicy } from '../../../../constants'; -import { FormInternal, DataAllocationMetaFields } from '../types'; +import { FormInternal } from '../../types'; -const serializeAllocateAction = ( - { dataTierAllocationType, allocationNodeAttribute }: DataAllocationMetaFields, - newActions: SerializedActionWithAllocation = {}, - originalActions: SerializedActionWithAllocation = {} -): SerializedActionWithAllocation => { - const { allocate, migrate, ...rest } = newActions; - // First copy over all non-require|include|exclude and migrate actions. - const actions: SerializedActionWithAllocation = { ...rest }; - - // We only set include, exclude and require here, so copy over all other values - if (allocate) { - const { include, exclude, require, ...restAllocate } = allocate; - if (!isEmpty(restAllocate)) { - actions.allocate = { ...restAllocate }; - } - } - - switch (dataTierAllocationType) { - case 'node_attrs': - if (allocationNodeAttribute) { - const [name, value] = allocationNodeAttribute.split(':'); - actions.allocate = { - // copy over any other allocate details like "number_of_replicas" - ...actions.allocate, - require: { - [name]: value, - }, - }; - } else { - // The form has been configured to use node attribute based allocation but no node attribute - // was selected. We fall back to what was originally selected in this case. This might be - // migrate.enabled: "false" - actions.migrate = originalActions.migrate; - } - - // copy over the original include and exclude values until we can set them in the form. - if (!isEmpty(originalActions?.allocate?.include)) { - actions.allocate = { - ...actions.allocate, - include: { ...originalActions?.allocate?.include }, - }; - } - - if (!isEmpty(originalActions?.allocate?.exclude)) { - actions.allocate = { - ...actions.allocate, - exclude: { ...originalActions?.allocate?.exclude }, - }; - } - break; - case 'none': - actions.migrate = { enabled: false }; - break; - default: - } - return actions; -}; +import { serializeMigrateAndAllocateActions } from './serialize_migrate_and_allocate_actions'; export const createSerializer = (originalPolicy?: SerializedPolicy) => ( data: FormInternal @@ -115,7 +59,7 @@ export const createSerializer = (originalPolicy?: SerializedPolicy) => ( delete hotPhaseActions.forcemerge; } - if (!updatedPolicy.phases.hot!.actions?.set_priority && hotPhaseActions.set_priority) { + if (!updatedPolicy.phases.hot!.actions?.set_priority) { delete hotPhaseActions.set_priority; } } @@ -137,7 +81,7 @@ export const createSerializer = (originalPolicy?: SerializedPolicy) => ( delete warmPhase.min_age; } - warmPhase.actions = serializeAllocateAction( + warmPhase.actions = serializeMigrateAndAllocateActions( _meta.warm, warmPhase.actions, originalPolicy?.phases.warm?.actions @@ -170,7 +114,7 @@ export const createSerializer = (originalPolicy?: SerializedPolicy) => ( coldPhase.min_age = `${updatedPolicy.phases.cold!.min_age}${_meta.cold.minAgeUnit}`; } - coldPhase.actions = serializeAllocateAction( + coldPhase.actions = serializeMigrateAndAllocateActions( _meta.cold, coldPhase.actions, originalPolicy?.phases.cold?.actions From 65ea7530fdbe182b59e67fbced2bb4b43ad3813b Mon Sep 17 00:00:00 2001 From: Jean-Louis Leysens Date: Thu, 12 Nov 2020 11:33:51 +0100 Subject: [PATCH 8/9] Updated serialization unit test coverage - removed the "forcemergeEnabled" meta field that was not being used - added test cases for deleting of values from policies --- .../sections/edit_policy/form/deserializer.ts | 2 - .../form/deserializer_and_serializer.test.ts | 229 +++++++++++++----- .../serialize_migrate_and_allocate_actions.ts | 5 +- .../edit_policy/form/serializer/serializer.ts | 10 +- .../application/sections/edit_policy/types.ts | 1 - 5 files changed, 174 insertions(+), 73 deletions(-) diff --git a/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/form/deserializer.ts b/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/form/deserializer.ts index 5af8807f2dec82..df5d6e2f80c15b 100644 --- a/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/form/deserializer.ts +++ b/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/form/deserializer.ts @@ -22,13 +22,11 @@ export const deserializer = (policy: SerializedPolicy): FormInternal => { const _meta: FormInternal['_meta'] = { hot: { useRollover: Boolean(hot?.actions?.rollover), - forceMergeEnabled: Boolean(hot?.actions?.forcemerge), bestCompression: hot?.actions?.forcemerge?.index_codec === 'best_compression', }, warm: { enabled: Boolean(warm), warmPhaseOnRollover: Boolean(warm?.min_age === '0ms'), - forceMergeEnabled: Boolean(warm?.actions?.forcemerge), bestCompression: warm?.actions?.forcemerge?.index_codec === 'best_compression', dataTierAllocationType: determineDataTierAllocationType(warm?.actions), }, diff --git a/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/form/deserializer_and_serializer.test.ts b/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/form/deserializer_and_serializer.test.ts index 15837c8d612451..b1e7072898566c 100644 --- a/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/form/deserializer_and_serializer.test.ts +++ b/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/form/deserializer_and_serializer.test.ts @@ -3,86 +3,181 @@ * or more contributor license agreements. Licensed under the Elastic License; * you may not use this file except in compliance with the Elastic License. */ -import { cloneDeep, set } from 'lodash'; + +import { setAutoFreeze } from 'immer'; +import { cloneDeep } from 'lodash'; import { SerializedPolicy } from '../../../../../common/types'; import { deserializer } from './deserializer'; import { createSerializer } from './serializer'; +import { FormInternal } from '../types'; -describe('deserializer and serializer', () => { - it('preserves unknown policy settings', () => { - const originalPolicy: SerializedPolicy = { - name: 'test', - phases: { - hot: { - actions: { - rollover: { - max_age: '1d', - max_size: '10gb', - max_docs: 1000, - }, - forcemerge: { - index_codec: 'best_compression', - max_num_segments: 22, - }, - }, - min_age: '12ms', +const isObject = (v: unknown): v is { [key: string]: any } => + Object.prototype.toString.call(v) === '[object Object]'; + +const unknownValue = { some: 'value' }; + +const populateWithUnknownEntries = (v: unknown) => { + if (isObject(v)) { + for (const key of Object.keys(v)) { + if (key === 'require' || key === 'include' || key === 'exclude') continue; // this will generate an invalid policy + populateWithUnknownEntries(v[key]); + } + v.unknown = unknownValue; + return; + } + if (Array.isArray(v)) { + v.forEach(populateWithUnknownEntries); + } +}; + +const originalPolicy: SerializedPolicy = { + name: 'test', + phases: { + hot: { + actions: { + rollover: { + max_age: '1d', + max_size: '10gb', + max_docs: 1000, }, - warm: { - actions: { - shrink: { number_of_shards: 12 }, - allocate: { - number_of_replicas: 3, - }, - set_priority: { - priority: 10, - }, - }, + forcemerge: { + index_codec: 'best_compression', + max_num_segments: 22, + }, + set_priority: { + priority: 1, + }, + }, + min_age: '12ms', + }, + warm: { + actions: { + shrink: { number_of_shards: 12 }, + allocate: { + number_of_replicas: 3, + }, + set_priority: { + priority: 10, + }, + migrate: { enabled: false }, + }, + }, + cold: { + actions: { + allocate: { + number_of_replicas: 12, + require: { test: 'my_value' }, + include: { test: 'my_value' }, + exclude: { test: 'my_value' }, }, - cold: { - actions: { - allocate: { number_of_replicas: 12 }, - freeze: {}, - set_priority: { - priority: 12, - }, - }, + freeze: {}, + set_priority: { + priority: 12, }, + }, + }, + delete: { + actions: { delete: { - actions: { - delete: { - delete_searchable_snapshot: true, - }, - wait_for_snapshot: { - policy: 'test', - }, - }, + delete_searchable_snapshot: true, + }, + wait_for_snapshot: { + policy: 'test', }, }, - }; - - /** - * Next we insert unknown values at various places in the policy object - * where we know they may occur. - */ - [ - '', - 'phases.hot.actions', - 'phases.hot.actions.rollover', - 'phases.warm.actions', - 'phases.warm.actions.forcemerge', - 'phases.cold.actions', - 'phases.delete.actions', - ].forEach((path) => { - set(originalPolicy, path, { unknown: 'value' }); + }, + }, +}; + +describe('deserializer and serializer', () => { + let policy: SerializedPolicy; + let serializer: ReturnType; + let formInternal: FormInternal; + + // So that we can modify produced form objects + beforeAll(() => setAutoFreeze(false)); + // This is the default in dev, so change back to true (https://github.com/immerjs/immer/blob/master/docs/freezing.md) + afterAll(() => setAutoFreeze(true)); + + beforeEach(() => { + policy = cloneDeep(originalPolicy); + serializer = createSerializer(policy); + formInternal = deserializer(policy); + }); + + it('preserves any unknown policy settings', () => { + // We populate all levels of the policy with entries our UI does not know about + populateWithUnknownEntries(policy); + + const copyOfPolicy = cloneDeep(policy); + + expect(serializer(formInternal)).toEqual(policy); + + // Assert that the policy we passed in is unaltered after deserialization and serialization + expect(policy).toEqual(copyOfPolicy); + }); + + it('removes all phases if they were disabled in the form', () => { + formInternal._meta.warm.enabled = false; + formInternal._meta.cold.enabled = false; + formInternal._meta.delete.enabled = false; + + expect(serializer(formInternal)).toEqual({ + ...policy, + phases: { + hot: policy.phases.hot, // We expect to see only the hot phase + }, }); + }); + + it('removes the forcemerge action if it is disabled in the form', () => { + delete formInternal.phases.hot!.actions.forcemerge; + delete formInternal.phases.warm!.actions.forcemerge; + + const result = serializer(formInternal); + + expect(result.phases.hot!.actions.forcemerge).toBeUndefined(); + expect(result.phases.warm!.actions.forcemerge).toBeUndefined(); + }); + + it('removes set priority if it is disabled in the form', () => { + delete formInternal.phases.hot!.actions.set_priority; + delete formInternal.phases.warm!.actions.set_priority; + delete formInternal.phases.cold!.actions.set_priority; + + const result = serializer(formInternal); + + expect(result.phases.hot!.actions.set_priority).toBeUndefined(); + expect(result.phases.warm!.actions.set_priority).toBeUndefined(); + expect(result.phases.cold!.actions.set_priority).toBeUndefined(); + }); + + it('removes freeze setting in the cold phase if it is disabled in the form', () => { + formInternal._meta.cold.freezeEnabled = false; + + const result = serializer(formInternal); + + expect(result.phases.cold!.actions.freeze).toBeUndefined(); + }); + + it('removes node attribute allocation when it is not selected in the form', () => { + // Change from 'node_attrs' to 'node_roles' + formInternal._meta.cold.dataTierAllocationType = 'node_roles'; + + const result = serializer(formInternal); + + expect(result.phases.cold!.actions.allocate!.number_of_replicas).toBe(12); + expect(result.phases.cold!.actions.allocate!.require).toBeUndefined(); + expect(result.phases.cold!.actions.allocate!.include).toBeUndefined(); + expect(result.phases.cold!.actions.allocate!.exclude).toBeUndefined(); + }); - const copyOfOriginalPolicy = cloneDeep(originalPolicy); - const formInternal = deserializer(copyOfOriginalPolicy); - const serializer = createSerializer(copyOfOriginalPolicy); + it('removes forcemerge and rollover config when rollover is disabled in hot phase', () => { + formInternal._meta.hot.useRollover = false; - expect(serializer(formInternal)).toEqual(originalPolicy); + const result = serializer(formInternal); - // Assert that the original policy is unaltered after deserialization and serialization - expect(originalPolicy).toEqual(copyOfOriginalPolicy); + expect(result.phases.hot!.actions.rollover).toBeUndefined(); + expect(result.phases.hot!.actions.forcemerge).toBeUndefined(); }); }); diff --git a/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/form/serializer/serialize_migrate_and_allocate_actions.ts b/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/form/serializer/serialize_migrate_and_allocate_actions.ts index b0c807ccb270e4..d18a63d34c101e 100644 --- a/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/form/serializer/serialize_migrate_and_allocate_actions.ts +++ b/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/form/serializer/serialize_migrate_and_allocate_actions.ts @@ -62,7 +62,10 @@ export const serializeMigrateAndAllocateActions = ( } break; case 'none': - actions.migrate = { enabled: false }; + actions.migrate = { + ...originalActions?.migrate, + enabled: false, + }; break; default: } diff --git a/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/form/serializer/serializer.ts b/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/form/serializer/serializer.ts index 1cdd0b909030af..ea2cb66dfc4de7 100644 --- a/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/form/serializer/serializer.ts +++ b/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/form/serializer/serializer.ts @@ -51,6 +51,12 @@ export const createSerializer = (originalPolicy?: SerializedPolicy) => ( hotPhaseActions.rollover.max_size = `${hotPhaseActions.rollover.max_size}${_meta.hot.maxStorageSizeUnit}`; } + if (!updatedPolicy.phases.hot!.actions?.forcemerge) { + delete hotPhaseActions.forcemerge; + } else if (_meta.hot.bestCompression) { + hotPhaseActions.forcemerge!.index_codec = 'best_compression'; + } + if (_meta.hot.bestCompression && hotPhaseActions.forcemerge) { hotPhaseActions.forcemerge.index_codec = 'best_compression'; } @@ -93,7 +99,7 @@ export const createSerializer = (originalPolicy?: SerializedPolicy) => ( warmPhase.actions.forcemerge!.index_codec = 'best_compression'; } - if (!updatedPolicy.phases.warm!.actions?.set_priority && warmPhase.actions.set_priority) { + if (!updatedPolicy.phases.warm!.actions?.set_priority) { delete warmPhase.actions.set_priority; } @@ -121,7 +127,7 @@ export const createSerializer = (originalPolicy?: SerializedPolicy) => ( ); if (_meta.cold.freezeEnabled) { - coldPhase.actions.freeze = {}; + coldPhase.actions.freeze = coldPhase.actions.freeze ?? {}; } else { delete coldPhase.actions.freeze; } diff --git a/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/types.ts b/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/types.ts index dc3d8a640e682d..7d512936290af0 100644 --- a/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/types.ts +++ b/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/types.ts @@ -18,7 +18,6 @@ export interface MinAgeField { } export interface ForcemergeFields { - forceMergeEnabled: boolean; bestCompression: boolean; } From 3e45301d684a525131b81058e4aa7f8fd9f7dc2e Mon Sep 17 00:00:00 2001 From: Jean-Louis Leysens Date: Thu, 19 Nov 2020 18:55:55 +0100 Subject: [PATCH 9/9] fixed minor issue in how serialization tests are being set up --- .../form/deserializer_and_serializer.test.ts | 32 +++++++++++++++---- .../edit_policy/form/serializer/serializer.ts | 2 +- 2 files changed, 26 insertions(+), 8 deletions(-) diff --git a/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/form/deserializer_and_serializer.test.ts b/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/form/deserializer_and_serializer.test.ts index b1e7072898566c..b379cb3956a022 100644 --- a/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/form/deserializer_and_serializer.test.ts +++ b/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/form/deserializer_and_serializer.test.ts @@ -19,7 +19,7 @@ const unknownValue = { some: 'value' }; const populateWithUnknownEntries = (v: unknown) => { if (isObject(v)) { for (const key of Object.keys(v)) { - if (key === 'require' || key === 'include' || key === 'exclude') continue; // this will generate an invalid policy + if (['require', 'include', 'exclude'].includes(key)) continue; // this will generate an invalid policy populateWithUnknownEntries(v[key]); } v.unknown = unknownValue; @@ -51,6 +51,7 @@ const originalPolicy: SerializedPolicy = { min_age: '12ms', }, warm: { + min_age: '12ms', actions: { shrink: { number_of_shards: 12 }, allocate: { @@ -63,6 +64,7 @@ const originalPolicy: SerializedPolicy = { }, }, cold: { + min_age: '30ms', actions: { allocate: { number_of_replicas: 12, @@ -77,6 +79,7 @@ const originalPolicy: SerializedPolicy = { }, }, delete: { + min_age: '33ms', actions: { delete: { delete_searchable_snapshot: true, @@ -101,20 +104,26 @@ describe('deserializer and serializer', () => { beforeEach(() => { policy = cloneDeep(originalPolicy); - serializer = createSerializer(policy); formInternal = deserializer(policy); + // Because the policy object is not deepCloned by the form lib we + // clone here so that we can mutate the policy and preserve the + // original reference in the createSerializer + serializer = createSerializer(cloneDeep(policy)); }); it('preserves any unknown policy settings', () => { + const thisTestPolicy = cloneDeep(originalPolicy); // We populate all levels of the policy with entries our UI does not know about - populateWithUnknownEntries(policy); + populateWithUnknownEntries(thisTestPolicy); + serializer = createSerializer(thisTestPolicy); - const copyOfPolicy = cloneDeep(policy); + const copyOfThisTestPolicy = cloneDeep(thisTestPolicy); - expect(serializer(formInternal)).toEqual(policy); + expect(serializer(deserializer(thisTestPolicy))).toEqual(thisTestPolicy); // Assert that the policy we passed in is unaltered after deserialization and serialization - expect(policy).toEqual(copyOfPolicy); + expect(thisTestPolicy).not.toBe(copyOfThisTestPolicy); + expect(thisTestPolicy).toEqual(copyOfThisTestPolicy); }); it('removes all phases if they were disabled in the form', () => { @@ -123,7 +132,7 @@ describe('deserializer and serializer', () => { formInternal._meta.delete.enabled = false; expect(serializer(formInternal)).toEqual({ - ...policy, + name: 'test', phases: { hot: policy.phases.hot, // We expect to see only the hot phase }, @@ -180,4 +189,13 @@ describe('deserializer and serializer', () => { expect(result.phases.hot!.actions.rollover).toBeUndefined(); expect(result.phases.hot!.actions.forcemerge).toBeUndefined(); }); + + it('removes min_age from warm when rollover is enabled', () => { + formInternal._meta.hot.useRollover = true; + formInternal._meta.warm.warmPhaseOnRollover = true; + + const result = serializer(formInternal); + + expect(result.phases.warm!.min_age).toBeUndefined(); + }); }); diff --git a/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/form/serializer/serializer.ts b/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/form/serializer/serializer.ts index ea2cb66dfc4de7..694f26abafe1d0 100644 --- a/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/form/serializer/serializer.ts +++ b/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/form/serializer/serializer.ts @@ -132,7 +132,7 @@ export const createSerializer = (originalPolicy?: SerializedPolicy) => ( delete coldPhase.actions.freeze; } - if (!updatedPolicy.phases.cold!.actions?.set_priority && coldPhase.actions.set_priority) { + if (!updatedPolicy.phases.cold!.actions?.set_priority) { delete coldPhase.actions.set_priority; } } else {