From cc6aff743323966eebb3a83ce46c24925facf3c2 Mon Sep 17 00:00:00 2001 From: Nathan Reese Date: Mon, 21 Oct 2019 15:19:38 -0600 Subject: [PATCH] [Maps] properly handle id collisions in Kibana index pattern (#48594) --- .../maps/public/elasticsearch_geo_utils.js | 12 ++-- .../public/elasticsearch_geo_utils.test.js | 58 ++++++++----------- .../es_search_source/es_search_source.js | 9 ++- 3 files changed, 35 insertions(+), 44 deletions(-) diff --git a/x-pack/legacy/plugins/maps/public/elasticsearch_geo_utils.js b/x-pack/legacy/plugins/maps/public/elasticsearch_geo_utils.js index 4a5715af64d0..5643bcba2681 100644 --- a/x-pack/legacy/plugins/maps/public/elasticsearch_geo_utils.js +++ b/x-pack/legacy/plugins/maps/public/elasticsearch_geo_utils.js @@ -75,17 +75,17 @@ export function hitsToGeoJson(hits, flattenHit, geoFieldName, geoFieldType) { // don't include geometry field value in properties delete properties[geoFieldName]; - // _id is unique to Elasticsearch documents. - // Move _id to FEATURE_ID_PROPERTY_NAME to standardize featureId keys across all sources - properties[FEATURE_ID_PROPERTY_NAME] = properties._id; - delete properties._id; - //create new geojson Feature for every individual geojson geometry. for (let j = 0; j < tmpGeometriesAccumulator.length; j++) { features.push({ type: 'Feature', geometry: tmpGeometriesAccumulator[j], - properties: properties + properties: { + ...properties, + // _id is not unique across Kibana index pattern. Multiple ES indices could have _id collisions + // Need to prefix with _index to guarantee uniqueness + [FEATURE_ID_PROPERTY_NAME]: `${properties._index}:${properties._id}:${j}` + } }); } } diff --git a/x-pack/legacy/plugins/maps/public/elasticsearch_geo_utils.test.js b/x-pack/legacy/plugins/maps/public/elasticsearch_geo_utils.test.js index 5f2ae9fd7b19..69f65a5cd11e 100644 --- a/x-pack/legacy/plugins/maps/public/elasticsearch_geo_utils.test.js +++ b/x-pack/legacy/plugins/maps/public/elasticsearch_geo_utils.test.js @@ -7,7 +7,6 @@ jest.mock('ui/new_platform'); jest.mock('ui/index_patterns'); -import { FEATURE_ID_PROPERTY_NAME } from '../common/constants'; import { hitsToGeoJson, geoPointToGeometry, @@ -39,35 +38,24 @@ const flattenHitMock = hit => { } } properties._id = hit._id; + properties._index = hit._index; return properties; }; describe('hitsToGeoJson', () => { - it('Should set FEATURE_ID_PROPERTY_NAME to _id', () => { - const docId = 'if3mu20BBQNX22Q14Ppm'; - const hits = [ - { - _id: docId, - fields: { - [geoFieldName]: '20,100' - } - } - ]; - const geojson = hitsToGeoJson(hits, flattenHitMock, geoFieldName, 'geo_point'); - expect(geojson.type).toBe('FeatureCollection'); - expect(geojson.features.length).toBe(1); - expect(geojson.features[0].properties[FEATURE_ID_PROPERTY_NAME]).toBe(docId); - }); - it('Should convert elasitcsearch hits to geojson', () => { const hits = [ { - _source: { + _id: 'doc1', + _index: 'index1', + fields: { [geoFieldName]: '20,100' } }, { + _id: 'doc2', + _index: 'index1', _source: { [geoFieldName]: '30,110' } @@ -81,7 +69,11 @@ describe('hitsToGeoJson', () => { coordinates: [100, 20], type: 'Point', }, - properties: {}, + properties: { + __kbn__feature_id__: 'index1:doc1:0', + _id: 'doc1', + _index: 'index1', + }, type: 'Feature', }); }); @@ -123,6 +115,8 @@ describe('hitsToGeoJson', () => { it('Should create feature per item when geometry value is an array', () => { const hits = [ { + _id: 'doc1', + _index: 'index1', _source: { [geoFieldName]: [ '20,100', @@ -141,6 +135,9 @@ describe('hitsToGeoJson', () => { type: 'Point', }, properties: { + __kbn__feature_id__: 'index1:doc1:0', + _id: 'doc1', + _index: 'index1', myField: 8 }, type: 'Feature', @@ -151,6 +148,9 @@ describe('hitsToGeoJson', () => { type: 'Point', }, properties: { + __kbn__feature_id__: 'index1:doc1:1', + _id: 'doc1', + _index: 'index1', myField: 8 }, type: 'Feature', @@ -183,13 +183,9 @@ describe('hitsToGeoJson', () => { } ]; const geojson = hitsToGeoJson(hits, indexPatternFlattenHit, 'my.location', 'geo_point'); - expect(geojson.features[0]).toEqual({ - geometry: { - coordinates: [100, 20], - type: 'Point', - }, - properties: {}, - type: 'Feature', + expect(geojson.features[0].geometry).toEqual({ + coordinates: [100, 20], + type: 'Point', }); }); @@ -202,13 +198,9 @@ describe('hitsToGeoJson', () => { } ]; const geojson = hitsToGeoJson(hits, indexPatternFlattenHit, 'my.location', 'geo_point'); - expect(geojson.features[0]).toEqual({ - geometry: { - coordinates: [100, 20], - type: 'Point', - }, - properties: {}, - type: 'Feature', + expect(geojson.features[0].geometry).toEqual({ + coordinates: [100, 20], + type: 'Point', }); }); }); diff --git a/x-pack/legacy/plugins/maps/public/layers/sources/es_search_source/es_search_source.js b/x-pack/legacy/plugins/maps/public/layers/sources/es_search_source/es_search_source.js index e5fbde681b30..7c248e332d40 100644 --- a/x-pack/legacy/plugins/maps/public/layers/sources/es_search_source/es_search_source.js +++ b/x-pack/legacy/plugins/maps/public/layers/sources/es_search_source/es_search_source.js @@ -18,7 +18,6 @@ import { ES_SEARCH, ES_GEO_FIELD_TYPE, ES_SIZE_LIMIT, - FEATURE_ID_PROPERTY_NAME, SORT_ORDER, } from '../../../../common/constants'; import { i18n } from '@kbn/i18n'; @@ -364,7 +363,7 @@ export class ESSearchSource extends AbstractESSource { return this._descriptor.tooltipProperties.length > 0; } - async _loadTooltipProperties(docId, indexPattern) { + async _loadTooltipProperties(docId, index, indexPattern) { if (this._descriptor.tooltipProperties.length === 0) { return {}; } @@ -374,7 +373,7 @@ export class ESSearchSource extends AbstractESSource { searchSource.setField('size', 1); const query = { language: 'kuery', - query: `_id:"${docId}"` + query: `_id:"${docId}" and _index:${index}` }; searchSource.setField('query', query); searchSource.setField('fields', this._descriptor.tooltipProperties); @@ -402,7 +401,7 @@ export class ESSearchSource extends AbstractESSource { async filterAndFormatPropertiesToHtml(properties) { const indexPattern = await this._getIndexPattern(); - const propertyValues = await this._loadTooltipProperties(properties[FEATURE_ID_PROPERTY_NAME], indexPattern); + const propertyValues = await this._loadTooltipProperties(properties._id, properties._index, indexPattern); return this._descriptor.tooltipProperties.map(propertyName => { return new ESTooltipProperty(propertyName, propertyName, propertyValues[propertyName], indexPattern); @@ -509,7 +508,7 @@ export class ESSearchSource extends AbstractESSource { return { index: properties._index, // Can not use index pattern title because it may reference many indices - id: properties[FEATURE_ID_PROPERTY_NAME], + id: properties._id, path: geoField.name, }; }