From 86cc6352b5f84d81fd8e406f9b0762f97c665e28 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Loix?= Date: Wed, 29 Jan 2020 12:35:30 +0530 Subject: [PATCH 1/2] [Index template] Fix editor should support mappings types (#55804) --- .../helpers/mappings_editor.helpers.ts | 2 +- .../mappings_editor.test.tsx | 4 +- .../load_mappings_provider.test.tsx | 193 ++++++++++++ .../load_mappings/load_mappings_provider.tsx | 81 ++++- .../components/multiple_mappings_warning.tsx | 4 +- .../app/components/mappings_editor/index.ts | 2 + .../lib/extract_mappings_definition.test.ts | 39 ++- .../lib/extract_mappings_definition.ts | 52 ++-- .../lib/mappings_validator.test.ts | 26 +- .../mappings_editor/lib/mappings_validator.ts | 74 +++-- .../mappings_editor/mappings_editor.tsx | 16 +- .../mappings_editor/mappings_state.tsx | 284 +++++++++--------- .../public/app/services/api.ts | 9 + .../api/mapping/register_mapping_route.ts | 1 + .../api/templates/register_create_route.ts | 2 + .../api/templates/register_get_routes.ts | 9 +- .../api/templates/register_update_route.ts | 2 + .../management/index_management/mapping.js | 3 +- 18 files changed, 603 insertions(+), 200 deletions(-) create mode 100644 x-pack/legacy/plugins/index_management/public/app/components/mappings_editor/components/load_mappings/load_mappings_provider.test.tsx diff --git a/x-pack/legacy/plugins/index_management/public/app/components/mappings_editor/__jest__/client_integration/helpers/mappings_editor.helpers.ts b/x-pack/legacy/plugins/index_management/public/app/components/mappings_editor/__jest__/client_integration/helpers/mappings_editor.helpers.ts index b8b67a0f36bd2..dcee956130a29 100644 --- a/x-pack/legacy/plugins/index_management/public/app/components/mappings_editor/__jest__/client_integration/helpers/mappings_editor.helpers.ts +++ b/x-pack/legacy/plugins/index_management/public/app/components/mappings_editor/__jest__/client_integration/helpers/mappings_editor.helpers.ts @@ -13,4 +13,4 @@ export const setup = (props: any) => wrapComponent: false, }, defaultProps: props, - }); + })(); diff --git a/x-pack/legacy/plugins/index_management/public/app/components/mappings_editor/__jest__/client_integration/mappings_editor.test.tsx b/x-pack/legacy/plugins/index_management/public/app/components/mappings_editor/__jest__/client_integration/mappings_editor.test.tsx index 9e390e785c7d5..723c105d403b8 100644 --- a/x-pack/legacy/plugins/index_management/public/app/components/mappings_editor/__jest__/client_integration/mappings_editor.test.tsx +++ b/x-pack/legacy/plugins/index_management/public/app/components/mappings_editor/__jest__/client_integration/mappings_editor.test.tsx @@ -28,7 +28,7 @@ describe('', () => { }, }, }; - const testBed = await setup({ onUpdate: mockOnUpdate, defaultValue })(); + const testBed = await setup({ onUpdate: mockOnUpdate, defaultValue }); const { exists } = testBed; expect(exists('mappingsEditor')).toBe(true); @@ -44,7 +44,7 @@ describe('', () => { }, }, }; - const testBed = await setup({ onUpdate: mockOnUpdate, defaultValue })(); + const testBed = await setup({ onUpdate: mockOnUpdate, defaultValue }); const { exists } = testBed; expect(exists('mappingsEditor')).toBe(true); diff --git a/x-pack/legacy/plugins/index_management/public/app/components/mappings_editor/components/load_mappings/load_mappings_provider.test.tsx b/x-pack/legacy/plugins/index_management/public/app/components/mappings_editor/components/load_mappings/load_mappings_provider.test.tsx new file mode 100644 index 0000000000000..34647af38865d --- /dev/null +++ b/x-pack/legacy/plugins/index_management/public/app/components/mappings_editor/components/load_mappings/load_mappings_provider.test.tsx @@ -0,0 +1,193 @@ +/* + * 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 React from 'react'; +import { act } from 'react-dom/test-utils'; + +jest.mock('@elastic/eui', () => ({ + ...jest.requireActual('@elastic/eui'), + // Mocking EuiCodeEditor, which uses React Ace under the hood + EuiCodeEditor: (props: any) => ( + { + props.onChange(syntheticEvent.jsonString); + }} + /> + ), +})); + +import { registerTestBed, nextTick, TestBed } from '../../../../../../../../../test_utils'; +import { LoadMappingsProvider } from './load_mappings_provider'; + +const ComponentToTest = ({ onJson }: { onJson: () => void }) => ( + + {openModal => ( + + )} + +); + +const setup = (props: any) => + registerTestBed(ComponentToTest, { + memoryRouter: { wrapComponent: false }, + defaultProps: props, + })(); + +const openModalWithJsonContent = ({ find, component }: TestBed) => async (json: any) => { + find('load-json-button').simulate('click'); + component.update(); + + // Set the mappings to load + // @ts-ignore + await act(async () => { + find('mockCodeEditor').simulate('change', { + jsonString: JSON.stringify(json), + }); + await nextTick(300); // There is a debounce in the JsonEditor that we need to wait for + }); +}; + +describe('', () => { + test('it should forward valid mapping definition', async () => { + const mappingsToLoad = { + properties: { + title: { + type: 'text', + }, + }, + }; + + const onJson = jest.fn(); + const testBed = await setup({ onJson }); + + // Open the modal and add the JSON + await openModalWithJsonContent(testBed)(mappingsToLoad); + + // Confirm + testBed.find('confirmModalConfirmButton').simulate('click'); + + const [jsonReturned] = onJson.mock.calls[0]; + expect(jsonReturned).toEqual({ ...mappingsToLoad, dynamic_templates: [] }); + }); + + test('it should detect custom single-type mappings and return it', async () => { + const mappingsToLoadOneType = { + myCustomType: { + _source: { + enabled: true, + }, + properties: { + title: { + type: 'text', + }, + }, + dynamic_templates: [], + }, + }; + + const onJson = jest.fn(); + const testBed = await setup({ onJson }); + + await openModalWithJsonContent(testBed)(mappingsToLoadOneType); + + // Confirm + testBed.find('confirmModalConfirmButton').simulate('click'); + + const [jsonReturned] = onJson.mock.calls[0]; + expect(jsonReturned).toEqual(mappingsToLoadOneType); + }); + + test('it should detect multi-type mappings and return raw without validating', async () => { + const mappingsToLoadMultiType = { + myCustomType1: { + wrongParameter: 'wont be validated neither stripped out', + properties: { + title: { + type: 'wrongType', + }, + }, + dynamic_templates: [], + }, + myCustomType2: { + properties: { + title: { + type: 'text', + }, + }, + dynamic_templates: [], + }, + }; + + const onJson = jest.fn(); + const testBed = await setup({ onJson }); + + await openModalWithJsonContent(testBed)(mappingsToLoadMultiType); + + // Confirm + testBed.find('confirmModalConfirmButton').simulate('click'); + + const [jsonReturned] = onJson.mock.calls[0]; + expect(jsonReturned).toEqual(mappingsToLoadMultiType); + }); + + test('it should detect single-type mappings under a valid mappings definition parameter', async () => { + const mappingsToLoadOneType = { + // Custom type name _is_ a valid mappings definition parameter + _source: { + _source: { + enabled: true, + }, + properties: { + title: { + type: 'text', + }, + }, + dynamic_templates: [], + }, + }; + + const onJson = jest.fn(); + const testBed = await setup({ onJson }); + + await openModalWithJsonContent(testBed)(mappingsToLoadOneType); + + // Confirm + testBed.find('confirmModalConfirmButton').simulate('click'); + + const [jsonReturned] = onJson.mock.calls[0]; + expect(jsonReturned).toEqual(mappingsToLoadOneType); + }); + + test('should treat "properties" as properties definition and **not** as a cutom type', async () => { + const mappingsToLoadOneType = { + // Custom type name _is_ a valid mappings definition parameter + properties: { + _source: { + enabled: true, + }, + properties: { + title: { + type: 'text', + }, + }, + dynamic_templates: [], + }, + }; + + const onJson = jest.fn(); + const testBed = await setup({ onJson }); + + await openModalWithJsonContent(testBed)(mappingsToLoadOneType); + + // Confirm + testBed.find('confirmModalConfirmButton').simulate('click'); + + // Make sure our handler hasn't been called + expect(onJson.mock.calls.length).toBe(0); + }); +}); diff --git a/x-pack/legacy/plugins/index_management/public/app/components/mappings_editor/components/load_mappings/load_mappings_provider.tsx b/x-pack/legacy/plugins/index_management/public/app/components/mappings_editor/components/load_mappings/load_mappings_provider.tsx index a55bd96dce3d0..e5ebe3d811bc3 100644 --- a/x-pack/legacy/plugins/index_management/public/app/components/mappings_editor/components/load_mappings/load_mappings_provider.tsx +++ b/x-pack/legacy/plugins/index_management/public/app/components/mappings_editor/components/load_mappings/load_mappings_provider.tsx @@ -5,6 +5,7 @@ */ import React, { useState, useRef } from 'react'; +import { isPlainObject } from 'lodash'; import { i18n } from '@kbn/i18n'; import { FormattedMessage } from '@kbn/i18n/react'; import { @@ -17,7 +18,7 @@ import { } from '@elastic/eui'; import { JsonEditor, OnJsonEditorUpdateHandler } from '../../shared_imports'; -import { validateMappings, MappingsValidationError } from '../../lib'; +import { validateMappings, MappingsValidationError, VALID_MAPPINGS_PARAMETERS } from '../../lib'; const MAX_ERRORS_TO_DISPLAY = 1; @@ -25,7 +26,7 @@ type OpenJsonModalFunc = () => void; interface Props { onJson(json: { [key: string]: any }): void; - children: (deleteProperty: OpenJsonModalFunc) => React.ReactNode; + children: (openModal: OpenJsonModalFunc) => React.ReactNode; } interface State { @@ -126,10 +127,13 @@ const getErrorMessage = (error: MappingsValidationError) => { } }; +const areAllObjectKeysValidParameters = (obj: { [key: string]: any }) => + Object.keys(obj).every(key => VALID_MAPPINGS_PARAMETERS.includes(key)); + export const LoadMappingsProvider = ({ onJson, children }: Props) => { const [state, setState] = useState({ isModalOpen: false }); const [totalErrorsToDisplay, setTotalErrorsToDisplay] = useState(MAX_ERRORS_TO_DISPLAY); - const jsonContent = useRef['0'] | undefined>(); + const jsonContent = useRef['0'] | undefined>(undefined); const view: ModalView = state.json !== undefined && state.errors !== undefined ? 'validationResult' : 'json'; const i18nTexts = getTexts(view, state.errors?.length); @@ -146,6 +150,44 @@ export const LoadMappingsProvider = ({ onJson, children }: Props) => { setState({ isModalOpen: false }); }; + const getMappingsMetadata = (unparsed: { + [key: string]: any; + }): { customType?: string; isMultiTypeMappings: boolean } => { + let hasCustomType = false; + let isMultiTypeMappings = false; + let customType: string | undefined; + + /** + * We need to check if there are single or multi-types mappings declared, for that we will check for the following: + * + * - Are **all** root level keys valid parameter for the mappings definition. If not, and all keys are plain object, we assume we have multi-type mappings + * - If there are more than two types, return "as is" as the UI does not support more than 1 type and will display a warning callout + * - If there is only 1 type, validate the mappings definition and return it wrapped inside the the custom type + */ + const areAllKeysValid = areAllObjectKeysValidParameters(unparsed); + const areAllValuesPlainObjects = Object.values(unparsed).every(isPlainObject); + const areAllValuesObjKeysValidParameterName = + areAllValuesPlainObjects && Object.values(unparsed).every(areAllObjectKeysValidParameters); + + if (!areAllKeysValid && areAllValuesPlainObjects) { + hasCustomType = true; + isMultiTypeMappings = Object.keys(unparsed).length > 1; + } + // If all root level keys are *valid* parameters BUT they are all plain objects which *also* have ALL valid mappings config parameter + // we can assume that they are custom types whose name matches a mappings configuration parameter. + // This is to handle the case where a custom type would be for example "dynamic" which is a mappings configuration parameter. + else if (areAllKeysValid && areAllValuesPlainObjects && areAllValuesObjKeysValidParameterName) { + hasCustomType = true; + isMultiTypeMappings = Object.keys(unparsed).length > 1; + } + + if (hasCustomType && !isMultiTypeMappings) { + customType = Object.keys(unparsed)[0]; + } + + return { isMultiTypeMappings, customType }; + }; + const loadJson = () => { if (jsonContent.current === undefined) { // No changes have been made in the JSON, this is probably a "reset()" for the user @@ -159,14 +201,41 @@ export const LoadMappingsProvider = ({ onJson, children }: Props) => { if (isValidJson) { // Parse and validate the JSON to make sure it won't break the UI const unparsed = jsonContent.current.data.format(); - const { value: parsed, errors } = validateMappings(unparsed); + + if (Object.keys(unparsed).length === 0) { + // Empty object...exit early + onJson(unparsed); + closeModal(); + return; + } + + let mappingsToValidate = unparsed; + const { isMultiTypeMappings, customType } = getMappingsMetadata(unparsed); + + if (isMultiTypeMappings) { + // Exit early, the UI will show a warning + onJson(unparsed); + closeModal(); + return; + } + + // Custom type can't be "properties", ES will not treat it as such + // as it is reserved for fields definition + if (customType !== undefined && customType !== 'properties') { + mappingsToValidate = unparsed[customType]; + } + + const { value: parsed, errors } = validateMappings(mappingsToValidate); + + // Wrap the mappings definition with custom type if one was provided. + const parsedWithType = customType !== undefined ? { [customType]: parsed } : parsed; if (errors) { - setState({ isModalOpen: true, json: { unparsed, parsed }, errors }); + setState({ isModalOpen: true, json: { unparsed, parsed: parsedWithType }, errors }); return; } - onJson(parsed); + onJson(parsedWithType); closeModal(); } }; diff --git a/x-pack/legacy/plugins/index_management/public/app/components/mappings_editor/components/multiple_mappings_warning.tsx b/x-pack/legacy/plugins/index_management/public/app/components/mappings_editor/components/multiple_mappings_warning.tsx index 507e4dac262ff..7a6c6add008b4 100644 --- a/x-pack/legacy/plugins/index_management/public/app/components/mappings_editor/components/multiple_mappings_warning.tsx +++ b/x-pack/legacy/plugins/index_management/public/app/components/mappings_editor/components/multiple_mappings_warning.tsx @@ -14,7 +14,7 @@ import { documentationService } from '../../../services/documentation'; export const MultipleMappingsWarning = () => ( (

diff --git a/x-pack/legacy/plugins/index_management/public/app/components/mappings_editor/index.ts b/x-pack/legacy/plugins/index_management/public/app/components/mappings_editor/index.ts index 58db8af3f7c5c..a796a2d1124b4 100644 --- a/x-pack/legacy/plugins/index_management/public/app/components/mappings_editor/index.ts +++ b/x-pack/legacy/plugins/index_management/public/app/components/mappings_editor/index.ts @@ -11,3 +11,5 @@ export * from './mappings_editor'; export * from './components/load_mappings'; export { OnUpdateHandler, Types } from './mappings_state'; + +export { doMappingsHaveType } from './lib'; diff --git a/x-pack/legacy/plugins/index_management/public/app/components/mappings_editor/lib/extract_mappings_definition.test.ts b/x-pack/legacy/plugins/index_management/public/app/components/mappings_editor/lib/extract_mappings_definition.test.ts index 3e38ff5991a8c..cf399f55e660e 100644 --- a/x-pack/legacy/plugins/index_management/public/app/components/mappings_editor/lib/extract_mappings_definition.test.ts +++ b/x-pack/legacy/plugins/index_management/public/app/components/mappings_editor/lib/extract_mappings_definition.test.ts @@ -73,7 +73,10 @@ describe('extractMappingsDefinition', () => { }, }; - expect(extractMappingsDefinition(mappings)).toBe(mappings.type2); + expect(extractMappingsDefinition(mappings)).toEqual({ + type: 'type2', + mappings: mappings.type2, + }); }); test('should detect that the mappings has one type and return its mapping definition', () => { @@ -97,7 +100,37 @@ describe('extractMappingsDefinition', () => { }, }; - expect(extractMappingsDefinition(mappings)).toBe(mappings.myType); + expect(extractMappingsDefinition(mappings)).toEqual({ + type: 'myType', + mappings: mappings.myType, + }); + }); + + test('should detect that the mappings has one custom type whose name matches a mappings definition parameter', () => { + const mappings = { + dynamic: { + _source: { + excludes: [], + includes: [], + enabled: true, + }, + _meta: {}, + _routing: { + required: false, + }, + dynamic: true, + properties: { + title: { + type: 'keyword', + }, + }, + }, + }; + + expect(extractMappingsDefinition(mappings)).toEqual({ + type: 'dynamic', + mappings: mappings.dynamic, + }); }); test('should detect that the mappings has one type at root level', () => { @@ -123,6 +156,6 @@ describe('extractMappingsDefinition', () => { }, }; - expect(extractMappingsDefinition(mappings)).toBe(mappings); + expect(extractMappingsDefinition(mappings)).toEqual({ mappings }); }); }); diff --git a/x-pack/legacy/plugins/index_management/public/app/components/mappings_editor/lib/extract_mappings_definition.ts b/x-pack/legacy/plugins/index_management/public/app/components/mappings_editor/lib/extract_mappings_definition.ts index eae3c5b15759c..9f2a3226b69e0 100644 --- a/x-pack/legacy/plugins/index_management/public/app/components/mappings_editor/lib/extract_mappings_definition.ts +++ b/x-pack/legacy/plugins/index_management/public/app/components/mappings_editor/lib/extract_mappings_definition.ts @@ -6,15 +6,15 @@ import { isPlainObject } from 'lodash'; import { GenericObject } from '../types'; -import { - validateMappingsConfiguration, - mappingsConfigurationSchemaKeys, -} from './mappings_validator'; +import { validateMappingsConfiguration, VALID_MAPPINGS_PARAMETERS } from './mappings_validator'; -const ALLOWED_PARAMETERS = [...mappingsConfigurationSchemaKeys, 'dynamic_templates', 'properties']; +interface MappingsWithType { + type?: string; + mappings: GenericObject; +} const isMappingDefinition = (obj: GenericObject): boolean => { - const areAllKeysValid = Object.keys(obj).every(key => ALLOWED_PARAMETERS.includes(key)); + const areAllKeysValid = Object.keys(obj).every(key => VALID_MAPPINGS_PARAMETERS.includes(key)); if (!areAllKeysValid) { return false; @@ -32,6 +32,29 @@ const isMappingDefinition = (obj: GenericObject): boolean => { return isConfigurationValid && isPropertiesValid && isDynamicTemplatesValid; }; +const getMappingsDefinitionWithType = (mappings: GenericObject): MappingsWithType[] => { + if (isMappingDefinition(mappings)) { + // No need to go any further + return [{ mappings }]; + } + + // At this point there must be one or more type mappings + const typedMappings = Object.entries(mappings).reduce( + (acc: Array<{ type: string; mappings: GenericObject }>, [type, value]) => { + if (isMappingDefinition(value)) { + acc.push({ type, mappings: value as GenericObject }); + } + return acc; + }, + [] + ); + + return typedMappings; +}; + +export const doMappingsHaveType = (mappings: GenericObject = {}): boolean => + getMappingsDefinitionWithType(mappings).filter(({ type }) => type !== undefined).length > 0; + /** * 5.x index templates can be created with multiple types. * e.g. @@ -72,19 +95,10 @@ const isMappingDefinition = (obj: GenericObject): boolean => { * * @param mappings The mappings object to validate */ -export const extractMappingsDefinition = (mappings: GenericObject = {}): GenericObject | null => { - if (isMappingDefinition(mappings)) { - // No need to go any further - return mappings; - } - - // At this point there must be one or more type mappings - const typedMappings = Object.values(mappings).reduce((acc: GenericObject[], value) => { - if (isMappingDefinition(value)) { - acc.push(value as GenericObject); - } - return acc; - }, []); +export const extractMappingsDefinition = ( + mappings: GenericObject = {} +): MappingsWithType | null => { + const typedMappings = getMappingsDefinitionWithType(mappings); // If there are no typed mappings found this means that one of the type must did not pass // the "isMappingDefinition()" validation. diff --git a/x-pack/legacy/plugins/index_management/public/app/components/mappings_editor/lib/mappings_validator.test.ts b/x-pack/legacy/plugins/index_management/public/app/components/mappings_editor/lib/mappings_validator.test.ts index f1e6efb06c649..d67c267dda6ae 100644 --- a/x-pack/legacy/plugins/index_management/public/app/components/mappings_editor/lib/mappings_validator.test.ts +++ b/x-pack/legacy/plugins/index_management/public/app/components/mappings_editor/lib/mappings_validator.test.ts @@ -18,6 +18,24 @@ describe('Mappings configuration validator', () => { }); }); + it('should detect valid mappings configuration', () => { + const mappings = { + _source: { + includes: [], + excludes: [], + enabled: true, + }, + _meta: {}, + _routing: { + required: false, + }, + dynamic: true, + }; + + const { errors } = validateMappings(mappings); + expect(errors).toBe(undefined); + }); + it('should strip out unknown configuration', () => { const mappings = { dynamic: true, @@ -30,6 +48,7 @@ describe('Mappings configuration validator', () => { excludes: ['abc'], }, properties: { title: { type: 'text' } }, + dynamic_templates: [], unknown: 123, }; @@ -37,7 +56,7 @@ describe('Mappings configuration validator', () => { const { unknown, ...expected } = mappings; expect(value).toEqual(expected); - expect(errors).toBe(undefined); + expect(errors).toEqual([{ code: 'ERR_CONFIG', configName: 'unknown' }]); }); it('should strip out invalid configuration and returns the errors for each of them', () => { @@ -47,9 +66,8 @@ describe('Mappings configuration validator', () => { dynamic_date_formats: false, // wrong format _source: { enabled: true, - includes: 'abc', + unknownProp: 'abc', // invalid excludes: ['abc'], - wrong: 123, // parameter not allowed }, properties: 'abc', }; @@ -59,10 +77,10 @@ describe('Mappings configuration validator', () => { expect(value).toEqual({ dynamic: true, properties: {}, + dynamic_templates: [], }); expect(errors).not.toBe(undefined); - expect(errors!.length).toBe(3); expect(errors!).toEqual([ { code: 'ERR_CONFIG', configName: '_source' }, { code: 'ERR_CONFIG', configName: 'dynamic_date_formats' }, diff --git a/x-pack/legacy/plugins/index_management/public/app/components/mappings_editor/lib/mappings_validator.ts b/x-pack/legacy/plugins/index_management/public/app/components/mappings_editor/lib/mappings_validator.ts index 6ccbfeb50dcf4..78d638e398593 100644 --- a/x-pack/legacy/plugins/index_management/public/app/components/mappings_editor/lib/mappings_validator.ts +++ b/x-pack/legacy/plugins/index_management/public/app/components/mappings_editor/lib/mappings_validator.ts @@ -196,23 +196,30 @@ export const validateProperties = (properties = {}): PropertiesValidatorResponse * Single source of truth to validate the *configuration* of the mappings. * Whenever a user loads a JSON object it will be validate against this Joi schema. */ -export const mappingsConfigurationSchema = t.partial({ - dynamic: t.union([t.literal(true), t.literal(false), t.literal('strict')]), - date_detection: t.boolean, - numeric_detection: t.boolean, - dynamic_date_formats: t.array(t.string), - _source: t.partial({ - enabled: t.boolean, - includes: t.array(t.string), - excludes: t.array(t.string), - }), - _meta: t.UnknownRecord, - _routing: t.partial({ - required: t.boolean, - }), -}); - -export const mappingsConfigurationSchemaKeys = Object.keys(mappingsConfigurationSchema.props); +export const mappingsConfigurationSchema = t.exact( + t.partial({ + dynamic: t.union([t.literal(true), t.literal(false), t.literal('strict')]), + date_detection: t.boolean, + numeric_detection: t.boolean, + dynamic_date_formats: t.array(t.string), + _source: t.exact( + t.partial({ + enabled: t.boolean, + includes: t.array(t.string), + excludes: t.array(t.string), + }) + ), + _meta: t.UnknownRecord, + _routing: t.interface({ + required: t.boolean, + }), + }) +); + +const mappingsConfigurationSchemaKeys = Object.keys(mappingsConfigurationSchema.type.props); +const sourceConfigurationSchemaKeys = Object.keys( + mappingsConfigurationSchema.type.props._source.type.props +); export const validateMappingsConfiguration = ( mappingsConfiguration: any @@ -222,8 +229,20 @@ export const validateMappingsConfiguration = ( let copyOfMappingsConfig = { ...mappingsConfiguration }; const result = mappingsConfigurationSchema.decode(mappingsConfiguration); + const isSchemaInvalid = isLeft(result); - if (isLeft(result)) { + const unknownConfigurationParameters = Object.keys(mappingsConfiguration).filter( + key => mappingsConfigurationSchemaKeys.includes(key) === false + ); + + const unknownSourceConfigurationParameters = + mappingsConfiguration._source !== undefined + ? Object.keys(mappingsConfiguration._source).filter( + key => sourceConfigurationSchemaKeys.includes(key) === false + ) + : []; + + if (isSchemaInvalid) { /** * To keep the logic simple we will strip out the parameters that contain errors */ @@ -235,6 +254,15 @@ export const validateMappingsConfiguration = ( }); } + if (unknownConfigurationParameters.length > 0) { + unknownConfigurationParameters.forEach(configName => configurationRemoved.add(configName)); + } + + if (unknownSourceConfigurationParameters.length > 0) { + configurationRemoved.add('_source'); + delete copyOfMappingsConfig._source; + } + copyOfMappingsConfig = pick(copyOfMappingsConfig, mappingsConfigurationSchemaKeys); const errors: MappingsValidationError[] = toArray(ordString)(configurationRemoved) @@ -252,7 +280,7 @@ export const validateMappings = (mappings: any = {}): MappingsValidatorResponse return { value: {} }; } - const { properties, dynamic_templates, ...mappingsConfiguration } = mappings; + const { properties, dynamic_templates: dynamicTemplates, ...mappingsConfiguration } = mappings; const { value: parsedConfiguration, errors: configurationErrors } = validateMappingsConfiguration( mappingsConfiguration @@ -265,8 +293,14 @@ export const validateMappings = (mappings: any = {}): MappingsValidatorResponse value: { ...parsedConfiguration, properties: parsedProperties, - dynamic_templates, + dynamic_templates: dynamicTemplates ?? [], }, errors: errors.length ? errors : undefined, }; }; + +export const VALID_MAPPINGS_PARAMETERS = [ + ...mappingsConfigurationSchemaKeys, + 'dynamic_templates', + 'properties', +]; diff --git a/x-pack/legacy/plugins/index_management/public/app/components/mappings_editor/mappings_editor.tsx b/x-pack/legacy/plugins/index_management/public/app/components/mappings_editor/mappings_editor.tsx index 7ecf1fef16941..51854784bc764 100644 --- a/x-pack/legacy/plugins/index_management/public/app/components/mappings_editor/mappings_editor.tsx +++ b/x-pack/legacy/plugins/index_management/public/app/components/mappings_editor/mappings_editor.tsx @@ -31,7 +31,7 @@ type TabName = 'fields' | 'advanced' | 'templates'; export const MappingsEditor = React.memo(({ onUpdate, defaultValue, indexSettings }: Props) => { const [selectedTab, selectTab] = useState('fields'); - const { parsedDefaultValue, multipleMappingsDeclared } = useMemo(() => { + const { parsedDefaultValue, multipleMappingsDeclared, mappingsType } = useMemo(() => { const mappingsDefinition = extractMappingsDefinition(defaultValue); if (mappingsDefinition === null) { @@ -48,7 +48,7 @@ export const MappingsEditor = React.memo(({ onUpdate, defaultValue, indexSetting dynamic_date_formats, properties = {}, dynamic_templates, - } = mappingsDefinition; + } = mappingsDefinition.mappings; const parsed = { configuration: { @@ -66,7 +66,11 @@ export const MappingsEditor = React.memo(({ onUpdate, defaultValue, indexSetting }, }; - return { parsedDefaultValue: parsed, multipleMappingsDeclared: false }; + return { + parsedDefaultValue: parsed, + multipleMappingsDeclared: false, + mappingsType: mappingsDefinition.type, + }; }, [defaultValue]); useEffect(() => { @@ -108,7 +112,11 @@ export const MappingsEditor = React.memo(({ onUpdate, defaultValue, indexSetting ) : ( - + {({ state }) => { const tabToContentMap = { fields: , diff --git a/x-pack/legacy/plugins/index_management/public/app/components/mappings_editor/mappings_state.tsx b/x-pack/legacy/plugins/index_management/public/app/components/mappings_editor/mappings_state.tsx index 54cdea9ff8a42..634fde393c2cd 100644 --- a/x-pack/legacy/plugins/index_management/public/app/components/mappings_editor/mappings_state.tsx +++ b/x-pack/legacy/plugins/index_management/public/app/components/mappings_editor/mappings_state.tsx @@ -32,7 +32,7 @@ export interface Types { export interface OnUpdateHandlerArg { isValid?: boolean; - getData: (isValid: boolean) => Mappings; + getData: (isValid: boolean) => Mappings | { [key: string]: Mappings }; validate: () => Promise; } @@ -49,149 +49,161 @@ export interface Props { fields: { [key: string]: Field }; }; onUpdate: OnUpdateHandler; + mappingsType?: string; } -export const MappingsState = React.memo(({ children, onUpdate, defaultValue }: Props) => { - const didMountRef = useRef(false); - - const parsedFieldsDefaultValue = useMemo(() => normalize(defaultValue.fields), [ - defaultValue.fields, - ]); - - const initialState: State = { - isValid: undefined, - configuration: { - defaultValue: defaultValue.configuration, - data: { - raw: defaultValue.configuration, - format: () => defaultValue.configuration, +export const MappingsState = React.memo( + ({ children, onUpdate, defaultValue, mappingsType }: Props) => { + const didMountRef = useRef(false); + + const parsedFieldsDefaultValue = useMemo(() => normalize(defaultValue.fields), [ + defaultValue.fields, + ]); + + const initialState: State = { + isValid: undefined, + configuration: { + defaultValue: defaultValue.configuration, + data: { + raw: defaultValue.configuration, + format: () => defaultValue.configuration, + }, + validate: () => Promise.resolve(true), }, - validate: () => Promise.resolve(true), - }, - templates: { - defaultValue: defaultValue.templates, - data: { - raw: defaultValue.templates, - format: () => defaultValue.templates, + templates: { + defaultValue: defaultValue.templates, + data: { + raw: defaultValue.templates, + format: () => defaultValue.templates, + }, + validate: () => Promise.resolve(true), }, - validate: () => Promise.resolve(true), - }, - fields: parsedFieldsDefaultValue, - documentFields: { - status: 'idle', - editor: 'default', - }, - fieldsJsonEditor: { - format: () => ({}), - isValid: true, - }, - search: { - term: '', - result: [], - }, - }; - - const [state, dispatch] = useReducer(reducer, initialState); - - useEffect(() => { - // If we are creating a new field, but haven't entered any name - // it is valid and we can byPass its form validation (that requires a "name" to be defined) - const isFieldFormVisible = state.fieldForm !== undefined; - const emptyNameValue = - isFieldFormVisible && - state.fieldForm!.data.raw.name !== undefined && - state.fieldForm!.data.raw.name.trim() === ''; - - const bypassFieldFormValidation = - state.documentFields.status === 'creatingField' && emptyNameValue; - - onUpdate({ - // Output a mappings object from the user's input. - getData: (isValid: boolean) => { - let nextState = state; - - if ( - state.documentFields.status === 'creatingField' && - isValid && - !bypassFieldFormValidation - ) { - // If the form field is valid and we are creating a new field that has some data - // we automatically add the field to our state. - const fieldFormData = state.fieldForm!.data.format() as Field; - if (Object.keys(fieldFormData).length !== 0) { - nextState = addFieldToState(fieldFormData, state); - dispatch({ type: 'field.add', value: fieldFormData }); - } - } - - // Pull the mappings properties from the current editor - const fields = - nextState.documentFields.editor === 'json' - ? nextState.fieldsJsonEditor.format() - : deNormalize(nextState.fields); - - const configurationData = nextState.configuration.data.format(); - const templatesData = nextState.templates.data.format(); - - return { - ...configurationData, - ...templatesData, - properties: fields, - }; + fields: parsedFieldsDefaultValue, + documentFields: { + status: 'idle', + editor: 'default', }, - validate: async () => { - const configurationFormValidator = - state.configuration.submitForm !== undefined - ? new Promise(async resolve => { - const { isValid } = await state.configuration.submitForm!(); - resolve(isValid); - }) - : Promise.resolve(true); - - const templatesFormValidator = - state.templates.form !== undefined - ? (await state.templates.form!.submit()).isValid - : Promise.resolve(true); - - const promisesToValidate = [configurationFormValidator, templatesFormValidator]; - - if (state.fieldForm !== undefined && !bypassFieldFormValidation) { - promisesToValidate.push(state.fieldForm.validate()); - } - - return Promise.all(promisesToValidate).then( - validationArray => validationArray.every(Boolean) && state.fieldsJsonEditor.isValid - ); + fieldsJsonEditor: { + format: () => ({}), + isValid: true, }, - isValid: state.isValid, - }); - }, [state]); - - useEffect(() => { - /** - * If the defaultValue has changed that probably means that we have loaded - * new data from JSON. We need to update our state with the new mappings. - */ - if (didMountRef.current) { - dispatch({ - type: 'editor.replaceMappings', - value: { - configuration: defaultValue.configuration, - templates: defaultValue.templates, - fields: parsedFieldsDefaultValue, + search: { + term: '', + result: [], + }, + }; + + const [state, dispatch] = useReducer(reducer, initialState); + + useEffect(() => { + // If we are creating a new field, but haven't entered any name + // it is valid and we can byPass its form validation (that requires a "name" to be defined) + const isFieldFormVisible = state.fieldForm !== undefined; + const emptyNameValue = + isFieldFormVisible && + state.fieldForm!.data.raw.name !== undefined && + state.fieldForm!.data.raw.name.trim() === ''; + + const bypassFieldFormValidation = + state.documentFields.status === 'creatingField' && emptyNameValue; + + onUpdate({ + // Output a mappings object from the user's input. + getData: (isValid: boolean) => { + let nextState = state; + + if ( + state.documentFields.status === 'creatingField' && + isValid && + !bypassFieldFormValidation + ) { + // If the form field is valid and we are creating a new field that has some data + // we automatically add the field to our state. + const fieldFormData = state.fieldForm!.data.format() as Field; + if (Object.keys(fieldFormData).length !== 0) { + nextState = addFieldToState(fieldFormData, state); + dispatch({ type: 'field.add', value: fieldFormData }); + } + } + + // Pull the mappings properties from the current editor + const fields = + nextState.documentFields.editor === 'json' + ? nextState.fieldsJsonEditor.format() + : deNormalize(nextState.fields); + + const configurationData = nextState.configuration.data.format(); + const templatesData = nextState.templates.data.format(); + + const mappings = { + ...configurationData, + ...templatesData, + properties: fields, + }; + + return mappingsType === undefined + ? mappings + : { + [mappingsType]: mappings, + }; }, + validate: async () => { + const configurationFormValidator = + state.configuration.submitForm !== undefined + ? new Promise(async resolve => { + const { isValid } = await state.configuration.submitForm!(); + resolve(isValid); + }) + : Promise.resolve(true); + + const templatesFormValidator = + state.templates.submitForm !== undefined + ? new Promise(async resolve => { + const { isValid } = await state.templates.submitForm!(); + resolve(isValid); + }) + : Promise.resolve(true); + + const promisesToValidate = [configurationFormValidator, templatesFormValidator]; + + if (state.fieldForm !== undefined && !bypassFieldFormValidation) { + promisesToValidate.push(state.fieldForm.validate()); + } + + return Promise.all(promisesToValidate).then( + validationArray => validationArray.every(Boolean) && state.fieldsJsonEditor.isValid + ); + }, + isValid: state.isValid, }); - } else { - didMountRef.current = true; - } - }, [defaultValue]); - - return ( - - {children({ state })} - - ); -}); + }, [state]); + + useEffect(() => { + /** + * If the defaultValue has changed that probably means that we have loaded + * new data from JSON. We need to update our state with the new mappings. + */ + if (didMountRef.current) { + dispatch({ + type: 'editor.replaceMappings', + value: { + configuration: defaultValue.configuration, + templates: defaultValue.templates, + fields: parsedFieldsDefaultValue, + }, + }); + } else { + didMountRef.current = true; + } + }, [defaultValue]); + + return ( + + {children({ state })} + + ); + } +); export const useMappingsState = () => { const ctx = useContext(StateContext); diff --git a/x-pack/legacy/plugins/index_management/public/app/services/api.ts b/x-pack/legacy/plugins/index_management/public/app/services/api.ts index bdd5c39919646..d28047825af18 100644 --- a/x-pack/legacy/plugins/index_management/public/app/services/api.ts +++ b/x-pack/legacy/plugins/index_management/public/app/services/api.ts @@ -38,6 +38,7 @@ import { uiMetricService } from './ui_metric'; import { useRequest, sendRequest } from './use_request'; import { httpService } from './http'; import { Template } from '../../../common/types'; +import { doMappingsHaveType } from '../components/mappings_editor'; let httpClient: ng.IHttpService; @@ -229,10 +230,14 @@ export function loadIndexTemplate(name: Template['name']) { } export async function saveTemplate(template: Template, isClone?: boolean) { + const includeTypeName = doMappingsHaveType(template.mappings); const result = await sendRequest({ path: `${API_BASE_PATH}/templates`, method: 'put', body: JSON.stringify(template), + query: { + include_type_name: includeTypeName, + }, }); const uimActionType = isClone ? UIM_TEMPLATE_CLONE : UIM_TEMPLATE_CREATE; @@ -243,11 +248,15 @@ export async function saveTemplate(template: Template, isClone?: boolean) { } export async function updateTemplate(template: Template) { + const includeTypeName = doMappingsHaveType(template.mappings); const { name } = template; const result = await sendRequest({ path: `${API_BASE_PATH}/templates/${encodeURIComponent(name)}`, method: 'put', body: JSON.stringify(template), + query: { + include_type_name: includeTypeName, + }, }); uiMetricService.trackMetric('count', UIM_TEMPLATE_UPDATE); diff --git a/x-pack/legacy/plugins/index_management/server/routes/api/mapping/register_mapping_route.ts b/x-pack/legacy/plugins/index_management/server/routes/api/mapping/register_mapping_route.ts index 86600aab76580..2f6530c69d938 100644 --- a/x-pack/legacy/plugins/index_management/server/routes/api/mapping/register_mapping_route.ts +++ b/x-pack/legacy/plugins/index_management/server/routes/api/mapping/register_mapping_route.ts @@ -17,6 +17,7 @@ const handler: RouterRouteHandler = async (request, callWithRequest) => { const params = { expand_wildcards: 'none', index: indexName, + include_type_name: true, }; const hit = await callWithRequest('indices.getMapping', params); diff --git a/x-pack/legacy/plugins/index_management/server/routes/api/templates/register_create_route.ts b/x-pack/legacy/plugins/index_management/server/routes/api/templates/register_create_route.ts index e134a97dd029e..8cc9f24f78a52 100644 --- a/x-pack/legacy/plugins/index_management/server/routes/api/templates/register_create_route.ts +++ b/x-pack/legacy/plugins/index_management/server/routes/api/templates/register_create_route.ts @@ -15,6 +15,7 @@ import { serializeTemplate } from '../../../../common/lib'; const handler: RouterRouteHandler = async (req, callWithRequest) => { const template = req.payload as Template; + const { include_type_name } = req.query as any; const serializedTemplate = serializeTemplate(template) as TemplateEs; const { name, order, index_patterns, version, settings, mappings, aliases } = serializedTemplate; @@ -49,6 +50,7 @@ const handler: RouterRouteHandler = async (req, callWithRequest) => { return await callWithRequest('indices.putTemplate', { name, order, + include_type_name, body: { index_patterns, version, diff --git a/x-pack/legacy/plugins/index_management/server/routes/api/templates/register_get_routes.ts b/x-pack/legacy/plugins/index_management/server/routes/api/templates/register_get_routes.ts index b450f75d1cc53..555feafa053d1 100644 --- a/x-pack/legacy/plugins/index_management/server/routes/api/templates/register_get_routes.ts +++ b/x-pack/legacy/plugins/index_management/server/routes/api/templates/register_get_routes.ts @@ -13,7 +13,9 @@ let callWithInternalUser: any; const allHandler: RouterRouteHandler = async (_req, callWithRequest) => { const managedTemplatePrefix = await getManagedTemplatePrefix(callWithInternalUser); - const indexTemplatesByName = await callWithRequest('indices.getTemplate'); + const indexTemplatesByName = await callWithRequest('indices.getTemplate', { + include_type_name: true, + }); return deserializeTemplateList(indexTemplatesByName, managedTemplatePrefix); }; @@ -21,7 +23,10 @@ const allHandler: RouterRouteHandler = async (_req, callWithRequest) => { const oneHandler: RouterRouteHandler = async (req, callWithRequest) => { const { name } = req.params; const managedTemplatePrefix = await getManagedTemplatePrefix(callWithInternalUser); - const indexTemplateByName = await callWithRequest('indices.getTemplate', { name }); + const indexTemplateByName = await callWithRequest('indices.getTemplate', { + name, + include_type_name: true, + }); if (indexTemplateByName[name]) { return deserializeTemplate({ ...indexTemplateByName[name], name }, managedTemplatePrefix); diff --git a/x-pack/legacy/plugins/index_management/server/routes/api/templates/register_update_route.ts b/x-pack/legacy/plugins/index_management/server/routes/api/templates/register_update_route.ts index 15590e2acbe71..ccbda2dab9364 100644 --- a/x-pack/legacy/plugins/index_management/server/routes/api/templates/register_update_route.ts +++ b/x-pack/legacy/plugins/index_management/server/routes/api/templates/register_update_route.ts @@ -10,6 +10,7 @@ import { serializeTemplate } from '../../../../common/lib'; const handler: RouterRouteHandler = async (req, callWithRequest) => { const { name } = req.params; + const { include_type_name } = req.query as any; const template = req.payload as Template; const serializedTemplate = serializeTemplate(template) as TemplateEs; @@ -22,6 +23,7 @@ const handler: RouterRouteHandler = async (req, callWithRequest) => { return await callWithRequest('indices.putTemplate', { name, order, + include_type_name, body: { index_patterns, version, diff --git a/x-pack/test/api_integration/apis/management/index_management/mapping.js b/x-pack/test/api_integration/apis/management/index_management/mapping.js index fa0f6e04a7a4d..13eca5bef251a 100644 --- a/x-pack/test/api_integration/apis/management/index_management/mapping.js +++ b/x-pack/test/api_integration/apis/management/index_management/mapping.js @@ -32,7 +32,8 @@ export default function({ getService }) { const { body } = await getIndexMapping(index).expect(200); - expect(body.mapping).to.eql(mappings); + // As, on 7.x we require the mappings with type (include_type_name), the default "_doc" type is returned + expect(body.mapping).to.eql({ _doc: mappings }); }); }); } From 63eb9a95f571feb340a1c398ed3b514dd1fa0adc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Se=CC=81bastien?= Date: Wed, 29 Jan 2020 15:26:44 +0530 Subject: [PATCH 2/2] Revert merge conflict resolution --- .../app/components/mappings_editor/mappings_state.tsx | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/x-pack/legacy/plugins/index_management/public/app/components/mappings_editor/mappings_state.tsx b/x-pack/legacy/plugins/index_management/public/app/components/mappings_editor/mappings_state.tsx index 634fde393c2cd..2204f4c90c0a9 100644 --- a/x-pack/legacy/plugins/index_management/public/app/components/mappings_editor/mappings_state.tsx +++ b/x-pack/legacy/plugins/index_management/public/app/components/mappings_editor/mappings_state.tsx @@ -157,11 +157,8 @@ export const MappingsState = React.memo( : Promise.resolve(true); const templatesFormValidator = - state.templates.submitForm !== undefined - ? new Promise(async resolve => { - const { isValid } = await state.templates.submitForm!(); - resolve(isValid); - }) + state.templates.form !== undefined + ? (await state.templates.form!.submit()).isValid : Promise.resolve(true); const promisesToValidate = [configurationFormValidator, templatesFormValidator];