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.

Sign up for GitHub

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

[EPM] Adding support for nested fields #64829

Merged
merged 6 commits into from
Apr 30, 2020
Merged

[EPM] Adding support for nested fields #64829

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
66d5783
Allowing nested types to be merged with group
jonathan-buttner Apr 29, 2020
ee07bd0
Adding nested to case and handling other fields
jonathan-buttner Apr 29, 2020
ffb3a34
Cleaing up if logic
jonathan-buttner Apr 29, 2020
4dd19a3
Removing unneeded if statement
jonathan-buttner Apr 29, 2020
5301a3f
Adding nested type to switch and more tests
jonathan-buttner Apr 29, 2020
22b4a69
Keeping functions immutable
jonathan-buttner Apr 30, 2020
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
88 changes: 87 additions & 1 deletion x-pack/plugins/ingest_manager/server/services/epm/elasticsearch/template/template.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -291,10 +291,12 @@ test('tests processing object field with property, reverse order', () => {
type: keyword
- name: a
type: object
dynamic: false
`;
const objectFieldWithPropertyReversedMapping = {
properties: {
a: {
dynamic: false,
properties: {
b: {
ignore_above: 1024,
Expand All @@ -307,5 +309,89 @@ test('tests processing object field with property, reverse order', () => {
const fields: Field[] = safeLoad(objectFieldWithPropertyReversedLiteralYml);
const processedFields = processFields(fields);
const mappings = generateMappings(processedFields);
expect(JSON.stringify(mappings)).toEqual(JSON.stringify(objectFieldWithPropertyReversedMapping));
expect(mappings).toEqual(objectFieldWithPropertyReversedMapping);
});

test('tests processing nested field with property', () => {
const nestedYaml = `
- name: a.b
type: keyword
- name: a
type: nested
dynamic: false
`;
const expectedMapping = {
properties: {
a: {
dynamic: false,
type: 'nested',
properties: {
b: {
ignore_above: 1024,
type: 'keyword',
},
},
},
},
};
const fields: Field[] = safeLoad(nestedYaml);
const processedFields = processFields(fields);
const mappings = generateMappings(processedFields);
expect(mappings).toEqual(expectedMapping);
});

test('tests processing nested field with property, nested field first', () => {
const nestedYaml = `
- name: a
type: nested
include_in_parent: true
- name: a.b
type: keyword
`;
const expectedMapping = {
properties: {
a: {
include_in_parent: true,
type: 'nested',
properties: {
b: {
ignore_above: 1024,
type: 'keyword',
},
},
},
},
};
const fields: Field[] = safeLoad(nestedYaml);
const processedFields = processFields(fields);
const mappings = generateMappings(processedFields);
expect(mappings).toEqual(expectedMapping);
});

test('tests processing nested leaf field with properties', () => {
const nestedYaml = `
- name: a
type: object
dynamic: false
- name: a.b
type: nested
enabled: false
`;
const expectedMapping = {
properties: {
a: {
dynamic: false,
properties: {
b: {
enabled: false,
type: 'nested',
},
},
},
},
};
const fields: Field[] = safeLoad(nestedYaml);
const processedFields = processFields(fields);
const mappings = generateMappings(processedFields);
expect(mappings).toEqual(expectedMapping);
});
37 changes: 31 additions & 6 deletions x-pack/plugins/ingest_manager/server/services/epm/elasticsearch/template/template.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,12 @@ export function generateMappings(fields: Field[]): IndexTemplateMappings {
switch (type) {
case 'group':
fieldProps = generateMappings(field.fields!);
attemptAddDynamicAndEnabled(fieldProps, field);
Copy link
Contributor

@skh skh Apr 30, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 for factoring this out into a separate helper function.

Could we not mutate the input parameter fieldProps in these calls? In other places we return a (mutated) copy, so that line would read

fieldProps = attemptAddDynamicAndEnabled(fieldProps, field);

and it is more readable when we stay consistent throughout the code base.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good point, I updated to keep it similar with the other cases.

break;
case 'group-nested':
fieldProps = generateMappings(field.fields!);
fieldProps.type = 'nested';
attemptAddNestedProps(fieldProps, field);
break;
case 'integer':
fieldProps.type = 'long';
Expand All @@ -96,12 +102,11 @@ export function generateMappings(fields: Field[]): IndexTemplateMappings {
break;
case 'object':
fieldProps.type = 'object';
if (field.hasOwnProperty('enabled')) {
fieldProps.enabled = field.enabled;
}
if (field.hasOwnProperty('dynamic')) {
fieldProps.dynamic = field.dynamic;
}
attemptAddDynamicAndEnabled(fieldProps, field);
break;
case 'nested':
fieldProps.type = 'nested';
attemptAddNestedProps(fieldProps, field);
break;
case 'array':
// this assumes array fields were validated in an earlier step
Expand All @@ -128,6 +133,26 @@ export function generateMappings(fields: Field[]): IndexTemplateMappings {
return { properties: props };
}

function attemptAddDynamicAndEnabled(props: Properties, field: Field) {
if (field.hasOwnProperty('enabled')) {
props.enabled = field.enabled;
}
if (field.hasOwnProperty('dynamic')) {
props.dynamic = field.dynamic;
}
}

function attemptAddNestedProps(props: Properties, field: Field) {
attemptAddDynamicAndEnabled(props, field);

if (field.hasOwnProperty('include_in_parent')) {
props.include_in_parent = field.include_in_parent;
}
if (field.hasOwnProperty('include_in_root')) {
props.include_in_root = field.include_in_root;
}
}

function generateMultiFields(fields: Fields): MultiFields {
const multiFields: MultiFields = {};
if (fields) {
Expand Down
162 changes: 162 additions & 0 deletions x-pack/plugins/ingest_manager/server/services/epm/fields/field.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -210,4 +210,166 @@ describe('processFields', () => {
JSON.stringify(objectFieldWithPropertyExpanded)
);
});

test('correctly handles properties of object type fields where object comes second', () => {
const nested = [
{
name: 'a.b',
type: 'keyword',
},
{
name: 'a',
type: 'object',
dynamic: true,
},
];

const nestedExpanded = [
{
name: 'a',
type: 'group',
dynamic: true,
fields: [
{
name: 'b',
type: 'keyword',
},
],
},
];
expect(processFields(nested)).toEqual(nestedExpanded);
});

test('correctly handles properties of nested type fields', () => {
const nested = [
{
name: 'a',
type: 'nested',
dynamic: true,
},
{
name: 'a.b',
type: 'keyword',
},
];

const nestedExpanded = [
{
name: 'a',
type: 'group-nested',
dynamic: true,
fields: [
{
name: 'b',
type: 'keyword',
},
],
},
];
expect(processFields(nested)).toEqual(nestedExpanded);
});

test('correctly handles properties of nested type where nested top level comes second', () => {
const nested = [
{
name: 'a.b',
type: 'keyword',
},
{
name: 'a',
type: 'nested',
dynamic: true,
},
];

const nestedExpanded = [
{
name: 'a',
type: 'group-nested',
dynamic: true,
fields: [
{
name: 'b',
type: 'keyword',
},
],
},
];
expect(processFields(nested)).toEqual(nestedExpanded);
});

test('ignores redefinitions of an object field', () => {
const object = [
{
name: 'a',
type: 'object',
dynamic: true,
},
{
name: 'a',
type: 'object',
dynamic: false,
},
];

const objectExpected = [
{
name: 'a',
type: 'object',
// should preserve the field that was parsed first which had dynamic: true
dynamic: true,
},
];
expect(processFields(object)).toEqual(objectExpected);
});

test('ignores redefinitions of a nested field', () => {
const nested = [
{
name: 'a',
type: 'nested',
dynamic: true,
},
{
name: 'a',
type: 'nested',
dynamic: false,
},
];

const nestedExpected = [
{
name: 'a',
type: 'nested',
// should preserve the field that was parsed first which had dynamic: true
dynamic: true,
},
];
expect(processFields(nested)).toEqual(nestedExpected);
});

test('ignores redefinitions of a nested and object field', () => {
const nested = [
{
name: 'a',
type: 'nested',
dynamic: true,
},
{
name: 'a',
type: 'object',
dynamic: false,
},
];

const nestedExpected = [
{
name: 'a',
type: 'nested',
// should preserve the field that was parsed first which had dynamic: true
dynamic: true,
},
];
expect(processFields(nested)).toEqual(nestedExpected);
});
});
Loading