From 438a02f76502823a32a1e2b9e2eccdf99ac833ae Mon Sep 17 00:00:00 2001 From: Xavier Mouligneau Date: Fri, 4 Mar 2022 11:31:15 -0500 Subject: [PATCH 1/5] allow find saved object to query on _id --- .../service/lib/filter_utils.test.ts | 32 ++++++++++++++++++ .../saved_objects/service/lib/filter_utils.ts | 33 ++++++++++++++----- 2 files changed, 57 insertions(+), 8 deletions(-) diff --git a/src/core/server/saved_objects/service/lib/filter_utils.test.ts b/src/core/server/saved_objects/service/lib/filter_utils.test.ts index e9de6e77f0edfa..25a95da43c2950 100644 --- a/src/core/server/saved_objects/service/lib/filter_utils.test.ts +++ b/src/core/server/saved_objects/service/lib/filter_utils.test.ts @@ -193,6 +193,38 @@ describe('Filter Utils', () => { ).toEqual(esKuery.fromKueryExpression('alert.params.foo:bar')); }); + test('Assemble filter with just "_id"', () => { + expect( + validateConvertFilterToKueryNode(['foo'], 'foo._id: 0123456789', mockMappings) + ).toEqual(esKuery.fromKueryExpression('type: foo and _id: 0123456789')); + }); + + test('Assemble filter with "_id" and more', () => { + expect( + validateConvertFilterToKueryNode( + ['foo'], + 'foo._id: 0123456789 and (foo.updated_at: 5678654567 or foo.attributes.bytes > 1000)', + mockMappings + ) + ).toEqual( + esKuery.fromKueryExpression( + '(type: foo and _id: 0123456789) and ((type: foo and updated_at: 5678654567) or foo.bytes > 1000)' + ) + ); + }); + + test('Lets make sure that we are throwing an exception if we are using _id when it does not belong', () => { + expect(() => { + validateConvertFilterToKueryNode( + ['foo'], + 'foo.attributes._id: 0123456789 and (foo.updated_at: 5678654567 or foo.attributes.bytes > 1000)', + mockMappings + ); + }).toThrowErrorMatchingInlineSnapshot( + `"This key 'foo.attributes._id' does NOT exist in foo saved object index patterns: Bad Request"` + ); + }); + test('Lets make sure that we are throwing an exception if we get an error', () => { expect(() => { validateConvertFilterToKueryNode( diff --git a/src/core/server/saved_objects/service/lib/filter_utils.ts b/src/core/server/saved_objects/service/lib/filter_utils.ts index 6c8aee832457a8..8e3ef45f2babd7 100644 --- a/src/core/server/saved_objects/service/lib/filter_utils.ts +++ b/src/core/server/saved_objects/service/lib/filter_utils.ts @@ -16,13 +16,23 @@ type KueryNode = any; const astFunctionType = ['is', 'range', 'nested']; +const addAttributesInMappingIndex = (indexMapping: IndexMapping) => { + if (!Object.keys(indexMapping.properties).includes('_id')) { + indexMapping.properties = { + _id: { type: 'keyword' }, + ...indexMapping.properties, + }; + } +}; + export const validateConvertFilterToKueryNode = ( allowedTypes: string[], filter: string | KueryNode, indexMapping: IndexMapping ): KueryNode | undefined => { if (filter && indexMapping) { - const filterKueryNode = + addAttributesInMappingIndex(indexMapping); + let filterKueryNode = typeof filter === 'string' ? esKuery.fromKueryExpression(filter) : cloneDeep(filter); const validationFilterKuery = validateFilterKueryNode({ @@ -57,14 +67,21 @@ export const validateConvertFilterToKueryNode = ( existingKueryNode.arguments[0].value = existingKueryNode.arguments[0].value.split('.')[1]; const itemType = allowedTypes.filter((t) => t === item.type); if (itemType.length === 1) { - set( - filterKueryNode, - path, - esKuery.nodeTypes.function.buildNode('and', [ + if (path.length > 0) { + set( + filterKueryNode, + path, + esKuery.nodeTypes.function.buildNode('and', [ + esKuery.nodeTypes.function.buildNode('is', 'type', itemType[0]), + existingKueryNode, + ]) + ); + } else { + filterKueryNode = esKuery.nodeTypes.function.buildNode('and', [ esKuery.nodeTypes.function.buildNode('is', 'type', itemType[0]), - existingKueryNode, - ]) - ); + filterKueryNode, + ]); + } } } else { existingKueryNode.arguments[0].value = existingKueryNode.arguments[0].value.replace( From fa054d6ddac5f29d346cfe2b8b56f0726d7e6b83 Mon Sep 17 00:00:00 2001 From: Xavier Mouligneau Date: Fri, 4 Mar 2022 11:38:02 -0500 Subject: [PATCH 2/5] rename --- src/core/server/saved_objects/service/lib/filter_utils.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/core/server/saved_objects/service/lib/filter_utils.ts b/src/core/server/saved_objects/service/lib/filter_utils.ts index 8e3ef45f2babd7..9b9ba2c706e337 100644 --- a/src/core/server/saved_objects/service/lib/filter_utils.ts +++ b/src/core/server/saved_objects/service/lib/filter_utils.ts @@ -16,7 +16,7 @@ type KueryNode = any; const astFunctionType = ['is', 'range', 'nested']; -const addAttributesInMappingIndex = (indexMapping: IndexMapping) => { +const addPropertiesInMappingIndex = (indexMapping: IndexMapping) => { if (!Object.keys(indexMapping.properties).includes('_id')) { indexMapping.properties = { _id: { type: 'keyword' }, @@ -31,7 +31,7 @@ export const validateConvertFilterToKueryNode = ( indexMapping: IndexMapping ): KueryNode | undefined => { if (filter && indexMapping) { - addAttributesInMappingIndex(indexMapping); + addPropertiesInMappingIndex(indexMapping); let filterKueryNode = typeof filter === 'string' ? esKuery.fromKueryExpression(filter) : cloneDeep(filter); From b8cefc66b4360b0847be1955a51c7ab8bd5f66a8 Mon Sep 17 00:00:00 2001 From: Xavier Mouligneau Date: Fri, 4 Mar 2022 11:45:30 -0500 Subject: [PATCH 3/5] more test and some cleaning --- .../service/lib/filter_utils.test.ts | 18 ++++++++++++++++-- .../saved_objects/service/lib/filter_utils.ts | 18 ++++++------------ 2 files changed, 22 insertions(+), 14 deletions(-) diff --git a/src/core/server/saved_objects/service/lib/filter_utils.test.ts b/src/core/server/saved_objects/service/lib/filter_utils.test.ts index 25a95da43c2950..1842f7f96cf82f 100644 --- a/src/core/server/saved_objects/service/lib/filter_utils.test.ts +++ b/src/core/server/saved_objects/service/lib/filter_utils.test.ts @@ -193,13 +193,13 @@ describe('Filter Utils', () => { ).toEqual(esKuery.fromKueryExpression('alert.params.foo:bar')); }); - test('Assemble filter with just "_id"', () => { + test('Assemble filter with just "_id" and one type', () => { expect( validateConvertFilterToKueryNode(['foo'], 'foo._id: 0123456789', mockMappings) ).toEqual(esKuery.fromKueryExpression('type: foo and _id: 0123456789')); }); - test('Assemble filter with "_id" and more', () => { + test('Assemble filter with "_id" and one type and more', () => { expect( validateConvertFilterToKueryNode( ['foo'], @@ -213,6 +213,20 @@ describe('Filter Utils', () => { ); }); + test('Assemble filter with "_id" and multi type and more', () => { + expect( + validateConvertFilterToKueryNode( + ['foo', 'bar'], + 'foo._id: 0123456789 and bar._id: 9876543210', + mockMappings + ) + ).toEqual( + esKuery.fromKueryExpression( + '(type: foo and _id: 0123456789) and (type: bar and _id: 9876543210)' + ) + ); + }); + test('Lets make sure that we are throwing an exception if we are using _id when it does not belong', () => { expect(() => { validateConvertFilterToKueryNode( diff --git a/src/core/server/saved_objects/service/lib/filter_utils.ts b/src/core/server/saved_objects/service/lib/filter_utils.ts index 9b9ba2c706e337..b27cc615634235 100644 --- a/src/core/server/saved_objects/service/lib/filter_utils.ts +++ b/src/core/server/saved_objects/service/lib/filter_utils.ts @@ -67,20 +67,14 @@ export const validateConvertFilterToKueryNode = ( existingKueryNode.arguments[0].value = existingKueryNode.arguments[0].value.split('.')[1]; const itemType = allowedTypes.filter((t) => t === item.type); if (itemType.length === 1) { + const kueryToAdd = esKuery.nodeTypes.function.buildNode('and', [ + esKuery.nodeTypes.function.buildNode('is', 'type', itemType[0]), + existingKueryNode, + ]); if (path.length > 0) { - set( - filterKueryNode, - path, - esKuery.nodeTypes.function.buildNode('and', [ - esKuery.nodeTypes.function.buildNode('is', 'type', itemType[0]), - existingKueryNode, - ]) - ); + set(filterKueryNode, path, kueryToAdd); } else { - filterKueryNode = esKuery.nodeTypes.function.buildNode('and', [ - esKuery.nodeTypes.function.buildNode('is', 'type', itemType[0]), - filterKueryNode, - ]); + filterKueryNode = kueryToAdd; } } } else { From 6b320a64bf77d7133250d4b1adebb30dca5a2ea1 Mon Sep 17 00:00:00 2001 From: Xavier Mouligneau Date: Tue, 8 Mar 2022 09:04:28 -0500 Subject: [PATCH 4/5] review to use id instead of _id to match saved object definition --- .../service/lib/filter_utils.test.ts | 22 +++++++++---------- .../saved_objects/service/lib/filter_utils.ts | 20 ++++++++--------- 2 files changed, 20 insertions(+), 22 deletions(-) diff --git a/src/core/server/saved_objects/service/lib/filter_utils.test.ts b/src/core/server/saved_objects/service/lib/filter_utils.test.ts index 1842f7f96cf82f..971126713c5ca4 100644 --- a/src/core/server/saved_objects/service/lib/filter_utils.test.ts +++ b/src/core/server/saved_objects/service/lib/filter_utils.test.ts @@ -193,17 +193,17 @@ describe('Filter Utils', () => { ).toEqual(esKuery.fromKueryExpression('alert.params.foo:bar')); }); - test('Assemble filter with just "_id" and one type', () => { - expect( - validateConvertFilterToKueryNode(['foo'], 'foo._id: 0123456789', mockMappings) - ).toEqual(esKuery.fromKueryExpression('type: foo and _id: 0123456789')); + test('Assemble filter with just "id" and one type', () => { + expect(validateConvertFilterToKueryNode(['foo'], 'foo.id: 0123456789', mockMappings)).toEqual( + esKuery.fromKueryExpression('type: foo and _id: 0123456789') + ); }); - test('Assemble filter with "_id" and one type and more', () => { + test('Assemble filter with saved object attribute "id" and one type and more', () => { expect( validateConvertFilterToKueryNode( ['foo'], - 'foo._id: 0123456789 and (foo.updated_at: 5678654567 or foo.attributes.bytes > 1000)', + 'foo.id: 0123456789 and (foo.updated_at: 5678654567 or foo.attributes.bytes > 1000)', mockMappings ) ).toEqual( @@ -213,11 +213,11 @@ describe('Filter Utils', () => { ); }); - test('Assemble filter with "_id" and multi type and more', () => { + test('Assemble filter with saved object attribute "id" and multi type and more', () => { expect( validateConvertFilterToKueryNode( ['foo', 'bar'], - 'foo._id: 0123456789 and bar._id: 9876543210', + 'foo.id: 0123456789 and bar.id: 9876543210', mockMappings ) ).toEqual( @@ -227,15 +227,15 @@ describe('Filter Utils', () => { ); }); - test('Lets make sure that we are throwing an exception if we are using _id when it does not belong', () => { + test('Lets make sure that we are throwing an exception if we are using id outside of saved object attribute when it does not belong', () => { expect(() => { validateConvertFilterToKueryNode( ['foo'], - 'foo.attributes._id: 0123456789 and (foo.updated_at: 5678654567 or foo.attributes.bytes > 1000)', + 'foo.attributes.id: 0123456789 and (foo.updated_at: 5678654567 or foo.attributes.bytes > 1000)', mockMappings ); }).toThrowErrorMatchingInlineSnapshot( - `"This key 'foo.attributes._id' does NOT exist in foo saved object index patterns: Bad Request"` + `"This key 'foo.attributes.id' does NOT exist in foo saved object index patterns: Bad Request"` ); }); diff --git a/src/core/server/saved_objects/service/lib/filter_utils.ts b/src/core/server/saved_objects/service/lib/filter_utils.ts index b27cc615634235..27ff1c201cbddc 100644 --- a/src/core/server/saved_objects/service/lib/filter_utils.ts +++ b/src/core/server/saved_objects/service/lib/filter_utils.ts @@ -16,22 +16,12 @@ type KueryNode = any; const astFunctionType = ['is', 'range', 'nested']; -const addPropertiesInMappingIndex = (indexMapping: IndexMapping) => { - if (!Object.keys(indexMapping.properties).includes('_id')) { - indexMapping.properties = { - _id: { type: 'keyword' }, - ...indexMapping.properties, - }; - } -}; - export const validateConvertFilterToKueryNode = ( allowedTypes: string[], filter: string | KueryNode, indexMapping: IndexMapping ): KueryNode | undefined => { if (filter && indexMapping) { - addPropertiesInMappingIndex(indexMapping); let filterKueryNode = typeof filter === 'string' ? esKuery.fromKueryExpression(filter) : cloneDeep(filter); @@ -64,7 +54,9 @@ export const validateConvertFilterToKueryNode = ( const existingKueryNode: KueryNode = path.length === 0 ? filterKueryNode : get(filterKueryNode, path); if (item.isSavedObjectAttr) { - existingKueryNode.arguments[0].value = existingKueryNode.arguments[0].value.split('.')[1]; + const keySavedObjectAttr = existingKueryNode.arguments[0].value.split('.')[1]; + existingKueryNode.arguments[0].value = + keySavedObjectAttr === 'id' ? '_id' : keySavedObjectAttr; const itemType = allowedTypes.filter((t) => t === item.type); if (itemType.length === 1) { const kueryToAdd = esKuery.nodeTypes.function.buildNode('and', [ @@ -182,6 +174,8 @@ export const isSavedObjectAttr = (key: string | null | undefined, indexMapping: const keySplit = key != null ? key.split('.') : []; if (keySplit.length === 1 && fieldDefined(indexMapping, keySplit[0])) { return true; + } else if (keySplit.length === 2 && keySplit[1] === 'id') { + return true; } else if (keySplit.length === 2 && fieldDefined(indexMapping, keySplit[1])) { return true; } else { @@ -230,6 +224,10 @@ export const fieldDefined = (indexMappings: IndexMapping, key: string): boolean return true; } + if (mappingKey === 'properties.id') { + return true; + } + // If the `mappingKey` does not match a valid path, before returning false, // we want to check and see if the intended path was for a multi-field // such as `x.attributes.field.text` where `field` is mapped to both text From e612dbb1c5404928e463700924ca91bda4e84be0 Mon Sep 17 00:00:00 2001 From: Xavier Mouligneau Date: Tue, 8 Mar 2022 10:10:35 -0500 Subject: [PATCH 5/5] add more test after talking to Rudolf --- .../service/lib/filter_utils.test.ts | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/src/core/server/saved_objects/service/lib/filter_utils.test.ts b/src/core/server/saved_objects/service/lib/filter_utils.test.ts index 971126713c5ca4..92eb0d041cc274 100644 --- a/src/core/server/saved_objects/service/lib/filter_utils.test.ts +++ b/src/core/server/saved_objects/service/lib/filter_utils.test.ts @@ -36,6 +36,9 @@ const mockMappings = { }, bar: { properties: { + _id: { + type: 'keyword', + }, foo: { type: 'text', }, @@ -227,6 +230,18 @@ describe('Filter Utils', () => { ); }); + test('Allow saved object type to defined "_id" attributes and filter on it', () => { + expect( + validateConvertFilterToKueryNode( + ['foo', 'bar'], + 'foo.id: 0123456789 and bar.attributes._id: 9876543210', + mockMappings + ) + ).toEqual( + esKuery.fromKueryExpression('(type: foo and _id: 0123456789) and (bar._id: 9876543210)') + ); + }); + test('Lets make sure that we are throwing an exception if we are using id outside of saved object attribute when it does not belong', () => { expect(() => { validateConvertFilterToKueryNode( @@ -239,6 +254,18 @@ describe('Filter Utils', () => { ); }); + test('Lets make sure that we are throwing an exception if we are using _id', () => { + expect(() => { + validateConvertFilterToKueryNode( + ['foo'], + 'foo._id: 0123456789 and (foo.updated_at: 5678654567 or foo.attributes.bytes > 1000)', + mockMappings + ); + }).toThrowErrorMatchingInlineSnapshot( + `"This key 'foo._id' does NOT exist in foo saved object index patterns: Bad Request"` + ); + }); + test('Lets make sure that we are throwing an exception if we get an error', () => { expect(() => { validateConvertFilterToKueryNode(