Merge pull request apache#2 from terrestris/data-structure-refactoring

bfs: datastructure refactoring
JakobMiksch · Jan 5, 2023 · 0acb66f · 0acb66f
2 parents 18cb5c0 + adcdb4e
commit 0acb66f
Show file tree

Hide file tree

Showing 9 changed files with 88 additions and 66 deletions.
diff --git a/superset-frontend/package-lock.json b/superset-frontend/package-lock.json
diff --git a/superset-frontend/plugins/plugin-chart-ol/package.json b/superset-frontend/plugins/plugin-chart-ol/package.json
@@ -30,6 +30,8 @@
     "access": "public"
   },
   "dependencies": {
+    "@types/geojson": "^7946.0.10",
+    "geojson": "^0.5.0",
     "geostyler": "^11.0.0",
     "geostyler-data": "^1.0.0",
     "geostyler-openlayers-parser": "^4.0.0",

diff --git a/superset-frontend/plugins/plugin-chart-ol/src/components/ChartLayer.tsx b/superset-frontend/plugins/plugin-chart-ol/src/components/ChartLayer.tsx
@@ -10,7 +10,7 @@ import {
   SupportedVizTypes,
 } from '../types';
 import { createChartComponent } from '../util/chartUtil';
-import { getCoordinateFromLocation } from '../util/geometryUtil';
+import { getCoordinateFromGeometry } from '../util/geometryUtil';
 
 /**
  * Custom OpenLayers layer that displays a number of charts
@@ -19,7 +19,10 @@ import { getCoordinateFromLocation } from '../util/geometryUtil';
 export class ChartLayer extends Layer {
   chartsPerZoom: ChartsPerZoom = {};
 
-  chartConfigs: ChartConfig = {};
+  chartConfigs: ChartConfig = {
+    type: 'FeatureCollection',
+    features: [],
+  };
 
   chartSizeValues: ChartSizeValues = {};
 
@@ -187,8 +190,7 @@ export class ChartLayer extends Layer {
    * @param zoom The zoom level.
    */
   createChartsPerZoom(zoom: number) {
-    const locations = Object.keys(this.chartConfigs);
-    const charts = locations.map(loc => {
+    const charts = this.chartConfigs.features.map(feature => {
       const container = document.createElement('div');
 
       let chartWidth = 0;
@@ -200,17 +202,16 @@ export class ChartLayer extends Layer {
 
       // add location to container
       const chartComponent = createChartComponent(
-        loc,
         this.chartVizType,
-        this.chartConfigs,
+        feature,
         chartWidth,
         chartHeight,
       );
       ReactDOM.render(chartComponent, container);
 
       return {
         htmlElement: container,
-        coordinate: getCoordinateFromLocation(loc),
+        coordinate: getCoordinateFromGeometry(feature.geometry),
         width: chartWidth,
         height: chartHeight,
       };

diff --git a/superset-frontend/plugins/plugin-chart-ol/src/types.ts b/superset-frontend/plugins/plugin-chart-ol/src/types.ts
@@ -26,6 +26,7 @@ import Source from 'ol/source/Source';
 import { Coordinate } from 'ol/coordinate';
 import { DataNode, TreeProps } from 'antd/lib/tree';
 import { Map } from 'ol';
+import { Feature, FeatureCollection, Point } from 'geojson';
 
 export interface SupersetOlPluginStylesProps {
   height: number;
@@ -42,11 +43,26 @@ export type SupportedVizTypes =
   | 'echarts_timeseries_scatter'
   | 'echarts_timeseries_step';
 
-// TODO check if we can import an existing typing, instead
-export type ChartConfig = {
-  [key: string]: any;
+// TODO find a way to reference props from plugin-chart-echarts
+export type ChartConfigProperties = {
+  setDataMask: any;
+  labelMap: any;
+  labelMapB: any;
+  groupby: any;
+  selectedValues: any;
+  formData: any;
+  groupbyB: any;
+  seriesBreakdown: any;
+  legendData: any;
+  echartOptions: any;
 };
 
+export type ChartConfigFeature = Feature<Point, ChartConfigProperties>;
+export type ChartConfig = FeatureCollection<
+  ChartConfigFeature['geometry'],
+  ChartConfigFeature['properties']
+>;
+
 interface SupersetOlPluginCustomizeProps {
   headerText: string;
   geomColumn: string;

diff --git a/superset-frontend/plugins/plugin-chart-ol/src/util/chartUtil.tsx b/superset-frontend/plugins/plugin-chart-ol/src/util/chartUtil.tsx
@@ -23,7 +23,7 @@ import {
   EchartsPie,
   EchartsTimeseries,
 } from '@superset-ui/plugin-chart-echarts';
-import { ChartConfig } from '../types';
+import { ChartConfigFeature } from '../types';
 
 /**
  * Create a chart component for a location.
@@ -36,24 +36,25 @@ import { ChartConfig } from '../types';
  * @returns The chart as React component
  */
 export const createChartComponent = (
-  loc: string,
   chartVizType: string,
-  chartConfigs: ChartConfig,
+  chartConfig: ChartConfigFeature,
   chartWidth: number,
   chartHeight: number,
 ) => {
   let chartComponent;
 
-  const config = chartConfigs[loc].echartOptions;
-  const { setDataMask } = chartConfigs[loc];
-  const { labelMap } = chartConfigs[loc];
-  const { labelMapB } = chartConfigs[loc];
-  const { groupby } = chartConfigs[loc];
-  const { selectedValues } = chartConfigs[loc];
-  const { formData } = chartConfigs[loc];
-  const { groupbyB } = chartConfigs[loc];
-  const { seriesBreakdown } = chartConfigs[loc];
-  const { legendData } = chartConfigs[loc];
+  const {
+    setDataMask,
+    labelMap,
+    labelMapB,
+    groupby,
+    selectedValues,
+    formData,
+    groupbyB,
+    seriesBreakdown,
+    legendData,
+    echartOptions,
+  } = chartConfig.properties;
 
   switch (chartVizType) {
     case 'pie':
@@ -63,7 +64,7 @@ export const createChartComponent = (
         <EchartsPie
           height={chartHeight}
           width={chartWidth}
-          echartOptions={config}
+          echartOptions={echartOptions}
           setDataMask={setDataMask}
           labelMap={labelMap}
           // TODO probably remove geom grouping
@@ -80,7 +81,7 @@ export const createChartComponent = (
         <EchartsMixedTimeseries
           height={chartHeight}
           width={chartWidth}
-          echartOptions={config}
+          echartOptions={echartOptions}
           setDataMask={setDataMask}
           labelMap={labelMap}
           labelMapB={labelMapB}
@@ -101,7 +102,7 @@ export const createChartComponent = (
         <EchartsTimeseries
           height={chartHeight}
           width={chartWidth}
-          echartOptions={config}
+          echartOptions={echartOptions}
           setDataMask={setDataMask}
           labelMap={labelMap}
           // TODO probably remove geom grouping

diff --git a/superset-frontend/plugins/plugin-chart-ol/src/util/controlPanelUtil.tsx b/superset-frontend/plugins/plugin-chart-ol/src/util/controlPanelUtil.tsx
@@ -20,7 +20,6 @@ import React from 'react';
 import { t } from '@superset-ui/core';
 import { SelectValue } from 'antd/lib/select';
 import { ControlPanelConfig } from '@superset-ui/chart-controls';
-import { ChartConfig } from '../types';
 
 /**
  * Get the layer configuration object from the control panel.
@@ -77,7 +76,7 @@ export const selectedChartMutator = (
   if (value && typeof value === 'string') {
     const parsedValue = JSON.parse(value);
     let itemFound = false;
-    response.result.forEach((config: ChartConfig) => {
+    response.result.forEach((config: any) => {
       const configString = JSON.stringify(config);
       const sameId = config.id === parsedValue.id;
       const isUpdated = configString !== value;
@@ -115,7 +114,7 @@ export const selectedChartMutator = (
       });
     }
   } else {
-    response.result.forEach((config: ChartConfig) => {
+    response.result.forEach((config: any) => {
       const configString = JSON.stringify(config);
       const label = config.slice_name;
 

diff --git a/superset-frontend/plugins/plugin-chart-ol/src/util/geometryUtil.ts b/superset-frontend/plugins/plugin-chart-ol/src/util/geometryUtil.ts
@@ -25,28 +25,7 @@ import GeoJSON from 'ol/format/GeoJSON';
 import Feature from 'ol/Feature';
 import { Point } from 'ol/geom';
 import VectorSource from 'ol/source/Vector';
-
-/**
- * Converts locations to OpenLayers features.
- *
- * @param locations The incoming data
- *
- * @returns An array of OpenLayers features
- */
-export const convertLocationsToFeatures = (locations: Array<String>) => {
-  const features = locations.map((geoJsonString: String) => {
-    const geom3857 = new GeoJSON().readGeometry(geoJsonString, {
-      // TODO: adapt to map projection
-      featureProjection: 'EPSG:3857',
-    });
-
-    return new Feature({
-      geometry: geom3857,
-    });
-  });
-
-  return features;
-};
+import { Geometry } from 'geojson';
 
 /**
  * Converts a location string to a coordinate.
@@ -55,8 +34,8 @@ export const convertLocationsToFeatures = (locations: Array<String>) => {
  *
  * @returns The coordinate
  */
-export const getCoordinateFromLocation = (location: string) => {
-  const geom: Point = new GeoJSON().readGeometry(location, {
+export const getCoordinateFromGeometry = (geometry: Geometry) => {
+  const geom: Point = new GeoJSON().readGeometry(geometry, {
     // TODO: adapt to map projection
     featureProjection: 'EPSG:3857',
   }) as Point;
@@ -69,7 +48,7 @@ export const getCoordinateFromLocation = (location: string) => {
  * @param features An Array of OpenLayers features
  * @returns The OpenLayers extent or undefined
  */
-export const getExtendFromFeatures = (features: Feature[]) => {
+export const getExtentFromFeatures = (features: Feature[]) => {
   if (features.length === 0) {
     return undefined;
   }

diff --git a/superset-frontend/plugins/plugin-chart-ol/src/util/mapUtil.tsx b/superset-frontend/plugins/plugin-chart-ol/src/util/mapUtil.tsx
@@ -18,11 +18,9 @@
  */
 
 import { Map } from 'ol';
+import GeoJSON from 'ol/format/GeoJSON';
 import { ChartConfig } from '../types';
-import {
-  convertLocationsToFeatures,
-  getExtendFromFeatures,
-} from './geometryUtil';
+import { getExtentFromFeatures } from './geometryUtil';
 
 // default map extent of world if no features are found
 // TODO: move to generic config file or plugin configuration
@@ -35,10 +33,12 @@ const defaultExtent = [-16000000, -7279000, 20500000, 11000000];
 
 export const fitMapToCharts = (olMap: Map, chartConfigs: ChartConfig) => {
   const view = olMap.getView();
+  const features = new GeoJSON().readFeatures(chartConfigs, {
+    // TODO: adapt to map projection
+    featureProjection: 'EPSG:3857',
+  });
 
-  const locations = Object.keys(chartConfigs).map(c => JSON.parse(c));
-  const features = convertLocationsToFeatures(locations);
-  const extent = getExtendFromFeatures(features) || defaultExtent;
+  const extent = getExtentFromFeatures(features) || defaultExtent;
 
   view.fit(extent, {
     // tested for a desktop size monitor

diff --git a/superset-frontend/plugins/plugin-chart-ol/src/util/transformPropsUtil.ts b/superset-frontend/plugins/plugin-chart-ol/src/util/transformPropsUtil.ts
@@ -26,6 +26,7 @@ import {
   LocationConfigMapping,
   SelectedChartConfig,
   ChartConfig,
+  ChartConfigFeature,
 } from '../types';
 
 /**
@@ -167,7 +168,10 @@ export const getEchartConfigs = (
       break;
   }
 
-  const chartConfigs: ChartConfig = {};
+  const chartConfigs: ChartConfig = {
+    type: 'FeatureCollection',
+    features: [],
+  };
 
   Object.keys(dataByLocation).forEach(location => {
     const { queriesData } = chartProps;
@@ -192,10 +196,13 @@ export const getEchartConfigs = (
     // TODO create proper clone of argument
     const transformedProps = chartTransformer(config);
 
-    if (!Object.keys(chartConfigs).includes(location)) {
-      chartConfigs[location] = {};
-    }
-    chartConfigs[location] = transformedProps;
+    const feature: ChartConfigFeature = {
+      type: 'Feature',
+      geometry: JSON.parse(location),
+      properties: transformedProps,
+    };
+
+    chartConfigs.features.push(feature);
   });
   return chartConfigs;
 };