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

[Mappings editor] Bring improvements from #55804 PR to master #56282

Merged
merged 3 commits into from
Jan 30, 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,77 @@
/*
* 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: [] });
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ 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
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,10 @@
import { isPlainObject } from 'lodash';

import { GenericObject } from '../types';
import {
validateMappingsConfiguration,
mappingsConfigurationSchemaKeys,
} from './mappings_validator';

const ALLOWED_PARAMETERS = [...mappingsConfigurationSchemaKeys, 'dynamic_templates', 'properties'];
import { validateMappingsConfiguration, VALID_MAPPINGS_PARAMETERS } from './mappings_validator';

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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -30,14 +48,15 @@ describe('Mappings configuration validator', () => {
excludes: ['abc'],
},
properties: { title: { type: 'text' } },
dynamic_templates: [],
unknown: 123,
};

const { value, errors } = validateMappings(mappings);

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', () => {
Expand All @@ -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',
};
Expand All @@ -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' },
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
*/
Expand All @@ -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<string>(ordString)(configurationRemoved)
Expand All @@ -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
Expand All @@ -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',
];