Skip to content

Commit

Permalink
[Mappings editor] Bring improvements from #55804 PR to master (#56282)
Browse files Browse the repository at this point in the history
  • Loading branch information
sebelga authored Jan 30, 2020
1 parent 4084be7 commit ae9cbfc
Show file tree
Hide file tree
Showing 7 changed files with 159 additions and 35 deletions.
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',
];

0 comments on commit ae9cbfc

Please sign in to comment.