Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[7.6] [Index template] Fix editor should support mappings types (#55804) #56275

Merged
merged 2 commits into from
Jan 29, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -13,4 +13,4 @@ export const setup = (props: any) =>
wrapComponent: false,
},
defaultProps: props,
});
})();
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ describe('<MappingsEditor />', () => {
},
},
};
const testBed = await setup({ onUpdate: mockOnUpdate, defaultValue })();
const testBed = await setup({ onUpdate: mockOnUpdate, defaultValue });
const { exists } = testBed;

expect(exists('mappingsEditor')).toBe(true);
Expand All @@ -44,7 +44,7 @@ describe('<MappingsEditor />', () => {
},
},
};
const testBed = await setup({ onUpdate: mockOnUpdate, defaultValue })();
const testBed = await setup({ onUpdate: mockOnUpdate, defaultValue });
const { exists } = testBed;

expect(exists('mappingsEditor')).toBe(true);
Expand Down
Original file line number Diff line number Diff line change
@@ -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) => (
<input
data-test-subj="mockCodeEditor"
onChange={(syntheticEvent: any) => {
props.onChange(syntheticEvent.jsonString);
}}
/>
),
}));

import { registerTestBed, nextTick, TestBed } from '../../../../../../../../../test_utils';
import { LoadMappingsProvider } from './load_mappings_provider';

const ComponentToTest = ({ onJson }: { onJson: () => void }) => (
<LoadMappingsProvider onJson={onJson}>
{openModal => (
<button onClick={openModal} data-test-subj="load-json-button">
Load JSON
</button>
)}
</LoadMappingsProvider>
);

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('<LoadMappingsProvider />', () => {
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);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -17,15 +18,15 @@ 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;

type OpenJsonModalFunc = () => void;

interface Props {
onJson(json: { [key: string]: any }): void;
children: (deleteProperty: OpenJsonModalFunc) => React.ReactNode;
children: (openModal: OpenJsonModalFunc) => React.ReactNode;
}

interface State {
Expand Down Expand Up @@ -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<State>({ isModalOpen: false });
const [totalErrorsToDisplay, setTotalErrorsToDisplay] = useState<number>(MAX_ERRORS_TO_DISPLAY);
const jsonContent = useRef<Parameters<OnJsonEditorUpdateHandler>['0'] | undefined>();
const jsonContent = useRef<Parameters<OnJsonEditorUpdateHandler>['0'] | undefined>(undefined);
const view: ModalView =
state.json !== undefined && state.errors !== undefined ? 'validationResult' : 'json';
const i18nTexts = getTexts(view, state.errors?.length);
Expand All @@ -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
Expand All @@ -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();
}
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import { documentationService } from '../../../services/documentation';
export const MultipleMappingsWarning = () => (
<EuiCallOut
title={i18n.translate('xpack.idxMgmt.mappingsEditor.mappingTypesDetectedCallOutTitle', {
defaultMessage: 'Mapping types detected',
defaultMessage: 'Multiple mapping types detected',
})}
iconType="alert"
color="warning"
Expand All @@ -23,7 +23,7 @@ export const MultipleMappingsWarning = () => (
<p>
<FormattedMessage
id="xpack.idxMgmt.mappingsEditor.mappingTypesDetectedCallOutDescription"
defaultMessage="The mappings for this template uses types, which have been removed. {docsLink}"
defaultMessage="The mappings for this template uses multiple types, which are not supported. {docsLink}"
values={{
docsLink: (
<EuiLink href={documentationService.getAlternativeToMappingTypesLink()} target="_blank">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,5 @@ export * from './mappings_editor';
export * from './components/load_mappings';

export { OnUpdateHandler, Types } from './mappings_state';

export { doMappingsHaveType } from './lib';
Loading