From 07b71bcee4b13a66a568dc592db165cb81280e36 Mon Sep 17 00:00:00 2001 From: Aaron Caldwell Date: Fri, 25 Oct 2019 14:19:32 -0600 Subject: [PATCH 1/4] Add back removed logic copying feature properties for injected data prior to modification --- .../maps/public/layers/vector_layer.js | 20 ++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/x-pack/legacy/plugins/maps/public/layers/vector_layer.js b/x-pack/legacy/plugins/maps/public/layers/vector_layer.js index 81f4f3b388d560..ac19a46a692275 100644 --- a/x-pack/legacy/plugins/maps/public/layers/vector_layer.js +++ b/x-pack/legacy/plugins/maps/public/layers/vector_layer.js @@ -137,11 +137,21 @@ export class VectorLayer extends AbstractLayer { if (!featureCollection) { return null; } - // Set default visible property on data - featureCollection.features.forEach( - feature => _.set(feature, `properties.${FEATURE_VISIBLE_PROPERTY_NAME}`, true) - ); - return featureCollection; + const copiedPropsFeatures = featureCollection.features + .map(feature => ({ + type: 'Feature', + geometry: feature.geometry, + properties: feature.properties + ? { + ...feature.properties, + [FEATURE_VISIBLE_PROPERTY_NAME]: true + } + : {} + })); + return { + type: 'FeatureCollection', + features: copiedPropsFeatures + }; } getCustomIconAndTooltipContent() { From 9dd6c2fcbde838710b6622594ff2f866693e6947 Mon Sep 17 00:00:00 2001 From: Aaron Caldwell Date: Mon, 28 Oct 2019 15:21:28 -0600 Subject: [PATCH 2/4] Back out injected data and assign featureCollection to sourceDescriptor with __ prefix --- .../public/angular/get_initial_layers.test.js | 2 - .../plugins/maps/public/layers/layer.js | 11 ----- .../client_file_source/geojson_file_source.js | 42 ++++++++++++------- .../maps/public/layers/sources/source.js | 4 -- .../maps/public/layers/util/injected_data.js | 21 ---------- .../maps/public/layers/vector_layer.js | 39 +---------------- 6 files changed, 30 insertions(+), 89 deletions(-) delete mode 100644 x-pack/legacy/plugins/maps/public/layers/util/injected_data.js diff --git a/x-pack/legacy/plugins/maps/public/angular/get_initial_layers.test.js b/x-pack/legacy/plugins/maps/public/angular/get_initial_layers.test.js index b15b94a49cebc5..5dc08751347e41 100644 --- a/x-pack/legacy/plugins/maps/public/angular/get_initial_layers.test.js +++ b/x-pack/legacy/plugins/maps/public/angular/get_initial_layers.test.js @@ -44,7 +44,6 @@ describe('kibana.yml configured with map.tilemap.url', () => { expect(layers).toEqual([{ alpha: 1, __dataRequests: [], - __injectedData: null, id: layers[0].id, applyGlobalQuery: true, label: null, @@ -87,7 +86,6 @@ describe('EMS is enabled', () => { expect(layers).toEqual([{ alpha: 1, __dataRequests: [], - __injectedData: null, id: layers[0].id, applyGlobalQuery: true, label: null, diff --git a/x-pack/legacy/plugins/maps/public/layers/layer.js b/x-pack/legacy/plugins/maps/public/layers/layer.js index c8187fd83ee4d9..45854b7b729f58 100644 --- a/x-pack/legacy/plugins/maps/public/layers/layer.js +++ b/x-pack/legacy/plugins/maps/public/layers/layer.js @@ -9,7 +9,6 @@ import { EuiIcon, EuiLoadingSpinner } from '@elastic/eui'; import turf from 'turf'; import turfBooleanContains from '@turf/boolean-contains'; import { DataRequest } from './util/data_request'; -import { InjectedData } from './util/injected_data'; import { MB_SOURCE_ID_LAYER_ID_PREFIX_DELIMITER, SOURCE_DATA_ID_ORIGIN @@ -32,11 +31,6 @@ export class AbstractLayer { } else { this._dataRequests = []; } - if (this._descriptor.__injectedData) { - this._injectedData = new InjectedData(this._descriptor.__injectedData); - } else { - this._injectedData = null; - } } static getBoundDataForSource(mbMap, sourceId) { @@ -48,7 +42,6 @@ export class AbstractLayer { const layerDescriptor = { ...options }; layerDescriptor.__dataRequests = _.get(options, '__dataRequests', []); - layerDescriptor.__injectedData = _.get(options, '__injectedData', null); layerDescriptor.id = _.get(options, 'id', uuid()); layerDescriptor.label = options.label && options.label.length > 0 ? options.label : null; layerDescriptor.minZoom = _.get(options, 'minZoom', 0); @@ -287,10 +280,6 @@ export class AbstractLayer { return this._dataRequests.find(dataRequest => dataRequest.getDataId() === id); } - getInjectedData() { - return this._injectedData ? this._injectedData.getData() : null; - } - isLayerLoading() { return this._dataRequests.some(dataRequest => dataRequest.isLoading()); } diff --git a/x-pack/legacy/plugins/maps/public/layers/sources/client_file_source/geojson_file_source.js b/x-pack/legacy/plugins/maps/public/layers/sources/client_file_source/geojson_file_source.js index cf876a59d0be4f..87de35c0189b4f 100644 --- a/x-pack/legacy/plugins/maps/public/layers/sources/client_file_source/geojson_file_source.js +++ b/x-pack/legacy/plugins/maps/public/layers/sources/client_file_source/geojson_file_source.js @@ -35,9 +35,18 @@ export class GeojsonFileSource extends AbstractVectorSource { applyGlobalQuery: DEFAULT_APPLY_GLOBAL_QUERY } - static createDescriptor(name) { + static createDescriptor(geoJson, name) { + // Wrap feature as feature collection if needed + const featureCollection = (geoJson.type === 'Feature') + ? { + type: 'FeatureCollection', + features: [{ ...geoJson }] + } + : geoJson; + return { type: GeojsonFileSource.type, + __featureCollection: featureCollection, name }; } @@ -85,16 +94,9 @@ export class GeojsonFileSource extends AbstractVectorSource { onPreviewSource(null); return; } - const sourceDescriptor = GeojsonFileSource.createDescriptor(name); + const sourceDescriptor = GeojsonFileSource.createDescriptor(geojsonFile, name); const source = new GeojsonFileSource(sourceDescriptor, inspectorAdapters); - const featureCollection = (geojsonFile.type === 'Feature') - ? { - type: 'FeatureCollection', - features: [{ ...geojsonFile }] - } - : geojsonFile; - - onPreviewSource(source, { __injectedData: featureCollection }); + onPreviewSource(source); }; }; @@ -125,6 +127,22 @@ export class GeojsonFileSource extends AbstractVectorSource { ); } + async getGeoJsonWithMeta() { + const copiedPropsFeatures = this._descriptor.__featureCollection.features + .map(feature => ({ + type: 'Feature', + geometry: feature.geometry, + properties: feature.properties ? { ...feature.properties } : {} + })); + return { + data: { + type: 'FeatureCollection', + features: copiedPropsFeatures + }, + meta: {} + }; + } + async getDisplayName() { return this._descriptor.name; } @@ -136,8 +154,4 @@ export class GeojsonFileSource extends AbstractVectorSource { shouldBeIndexed() { return GeojsonFileSource.isIndexingSource; } - - isInjectedData() { - return true; - } } diff --git a/x-pack/legacy/plugins/maps/public/layers/sources/source.js b/x-pack/legacy/plugins/maps/public/layers/sources/source.js index f96fc42f021784..3bee49cff6d18d 100644 --- a/x-pack/legacy/plugins/maps/public/layers/sources/source.js +++ b/x-pack/legacy/plugins/maps/public/layers/sources/source.js @@ -115,10 +115,6 @@ export class AbstractSource { return AbstractSource.isIndexingSource; } - isInjectedData() { - return false; - } - supportsElasticsearchFilters() { return false; } diff --git a/x-pack/legacy/plugins/maps/public/layers/util/injected_data.js b/x-pack/legacy/plugins/maps/public/layers/util/injected_data.js deleted file mode 100644 index 8c18819e9f8b57..00000000000000 --- a/x-pack/legacy/plugins/maps/public/layers/util/injected_data.js +++ /dev/null @@ -1,21 +0,0 @@ -/* - * 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. - */ -export class InjectedData { - - constructor(data) { - this._descriptor = { data }; - } - - getData() { - return this._descriptor.data; - } - - hasData() { - return !!this._descriptor.data; - } - -} - diff --git a/x-pack/legacy/plugins/maps/public/layers/vector_layer.js b/x-pack/legacy/plugins/maps/public/layers/vector_layer.js index ac19a46a692275..7372b549f64236 100644 --- a/x-pack/legacy/plugins/maps/public/layers/vector_layer.js +++ b/x-pack/legacy/plugins/maps/public/layers/vector_layer.js @@ -132,28 +132,6 @@ export class VectorLayer extends AbstractLayer { return true; } - getInjectedData() { - const featureCollection = super.getInjectedData(); - if (!featureCollection) { - return null; - } - const copiedPropsFeatures = featureCollection.features - .map(feature => ({ - type: 'Feature', - geometry: feature.geometry, - properties: feature.properties - ? { - ...feature.properties, - [FEATURE_VISIBLE_PROPERTY_NAME]: true - } - : {} - })); - return { - type: 'FeatureCollection', - features: copiedPropsFeatures - }; - } - getCustomIconAndTooltipContent() { const featureCollection = this._getSourceFeatureCollection(); @@ -520,16 +498,7 @@ export class VectorLayer extends AbstractLayer { startLoading, stopLoading, onLoadError, registerCancelCallback, dataFilters }) { - if (this._source.isInjectedData()) { - const featureCollection = this.getInjectedData(); - return { - refreshed: false, - featureCollection - }; - } - const requestToken = Symbol(`layer-source-refresh:${ this.getId()} - source`); - const searchFilters = this._getSearchFilters(dataFilters); const canSkip = await this._canSkipSourceUpdate(this._source, SOURCE_DATA_ID_ORIGIN, searchFilters); if (canSkip) { @@ -604,12 +573,8 @@ export class VectorLayer extends AbstractLayer { } _getSourceFeatureCollection() { - if (this._source.isInjectedData()) { - return this.getInjectedData(); - } else { - const sourceDataRequest = this.getSourceDataRequest(); - return sourceDataRequest ? sourceDataRequest.getData() : null; - } + const sourceDataRequest = this.getSourceDataRequest(); + return sourceDataRequest ? sourceDataRequest.getData() : null; } _syncFeatureCollectionWithMb(mbMap) { From 1c5089eced52b18ac7e0aaf5c93b97cb19aed2f3 Mon Sep 17 00:00:00 2001 From: Aaron Caldwell Date: Tue, 29 Oct 2019 15:17:28 -0600 Subject: [PATCH 3/4] Review feedback. Ensure __featureCollection is always initialized minimally to a geojson object with an empty features array --- .../client_file_source/geojson_file_source.js | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/x-pack/legacy/plugins/maps/public/layers/sources/client_file_source/geojson_file_source.js b/x-pack/legacy/plugins/maps/public/layers/sources/client_file_source/geojson_file_source.js index 87de35c0189b4f..97f3171afbb392 100644 --- a/x-pack/legacy/plugins/maps/public/layers/sources/client_file_source/geojson_file_source.js +++ b/x-pack/legacy/plugins/maps/public/layers/sources/client_file_source/geojson_file_source.js @@ -37,12 +37,20 @@ export class GeojsonFileSource extends AbstractVectorSource { static createDescriptor(geoJson, name) { // Wrap feature as feature collection if needed - const featureCollection = (geoJson.type === 'Feature') - ? { + let featureCollection; + if (geoJson.type === 'FeatureCollection') { + featureCollection = geoJson; + } else if (geoJson.type === 'Feature') { + featureCollection = { type: 'FeatureCollection', - features: [{ ...geoJson }] - } - : geoJson; + features: [geoJson] + }; + } else { // Missing or incorrect type + featureCollection = { + type: 'FeatureCollection', + features: [] + }; + } return { type: GeojsonFileSource.type, From 3789603298213924ae39f060a5feeb16dafa989c Mon Sep 17 00:00:00 2001 From: Aaron Caldwell Date: Tue, 29 Oct 2019 17:35:12 -0600 Subject: [PATCH 4/4] Check for null/undefined --- .../sources/client_file_source/geojson_file_source.js | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/x-pack/legacy/plugins/maps/public/layers/sources/client_file_source/geojson_file_source.js b/x-pack/legacy/plugins/maps/public/layers/sources/client_file_source/geojson_file_source.js index 97f3171afbb392..59cfc7b486bdd9 100644 --- a/x-pack/legacy/plugins/maps/public/layers/sources/client_file_source/geojson_file_source.js +++ b/x-pack/legacy/plugins/maps/public/layers/sources/client_file_source/geojson_file_source.js @@ -38,7 +38,13 @@ export class GeojsonFileSource extends AbstractVectorSource { static createDescriptor(geoJson, name) { // Wrap feature as feature collection if needed let featureCollection; - if (geoJson.type === 'FeatureCollection') { + + if (!geoJson) { + featureCollection = { + type: 'FeatureCollection', + features: [] + }; + } else if (geoJson.type === 'FeatureCollection') { featureCollection = geoJson; } else if (geoJson.type === 'Feature') { featureCollection = {