[Maps][File upload] Fix geojson upload diacritic handling (#83122)

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

package.json

@@ -140,6 +140,8 @@
     "@kbn/ui-framework": "link:packages/kbn-ui-framework",
     "@kbn/ui-shared-deps": "link:packages/kbn-ui-shared-deps",
     "@kbn/utils": "link:packages/kbn-utils",
+    "@loaders.gl/core": "^2.3.1",
+    "@loaders.gl/json": "^2.3.1",
     "@slack/webhook": "^5.0.0",
     "@storybook/addons": "^6.0.16",
     "@turf/circle": "6.0.1",
@@ -375,6 +377,7 @@
     "@kbn/test": "link:packages/kbn-test",
     "@kbn/test-subj-selector": "link:packages/kbn-test-subj-selector",
     "@kbn/utility-types": "link:packages/kbn-utility-types",
+    "@loaders.gl/polyfills": "^2.3.5",
     "@mapbox/geojson-rewind": "^0.5.0",
     "@mapbox/mapbox-gl-draw": "^1.2.0",
     "@mapbox/mapbox-gl-rtl-text": "^0.2.3",

x-pack/plugins/file_upload/public/components/json_index_file_picker.js

@@ -10,7 +10,6 @@ import { FormattedMessage } from '@kbn/i18n/react';
 import { i18n } from '@kbn/i18n';
 import { parseFile } from '../util/file_parser';
 import { MAX_FILE_SIZE } from '../../common/constants/file_import';
-import _ from 'lodash';
 
 const ACCEPTABLE_FILETYPES = ['json', 'geojson'];
 const acceptedFileTypeString = ACCEPTABLE_FILETYPES.map((type) => `.${type}`).join(',');
@@ -134,14 +133,12 @@ export class JsonIndexFilePicker extends Component {
     return fileNameOnly.toLowerCase();
   }
 
-  // It's necessary to throttle progress. Updates that are too frequent cause
-  // issues (update failure) in the nested progress component
-  setFileProgress = _.debounce(({ featuresProcessed, bytesProcessed, totalBytes }) => {
+  setFileProgress = ({ featuresProcessed, bytesProcessed, totalBytes }) => {
     const percentageProcessed = parseInt((100 * bytesProcessed) / totalBytes);
     if (this.getFileParseActive()) {
       this.setState({ featuresProcessed, percentageProcessed });
     }
-  }, 150);
+  };
 
   async _parseFile(file) {
     const { currentFileTracker } = this.state;

x-pack/plugins/file_upload/public/util/file_parser.js

@@ -7,111 +7,106 @@
 import _ from 'lodash';
 import { geoJsonCleanAndValidate } from './geo_json_clean_and_validate';
 import { i18n } from '@kbn/i18n';
-import { PatternReader } from './pattern_reader';
+import { JSONLoader } from '@loaders.gl/json';
+import { loadInBatches } from '@loaders.gl/core';
 
-// In local testing, performance improvements leveled off around this size
-export const FILE_BUFFER = 1024000;
-
-const readSlice = (fileReader, file, start, stop) => {
-  const blob = file.slice(start, stop);
-  fileReader.readAsBinaryString(blob);
-};
-
-let prevFileReader;
-let prevPatternReader;
 export const fileHandler = async ({
   file,
   setFileProgress,
   cleanAndValidate,
   getFileParseActive,
-  fileReader = new FileReader(),
 }) => {
-  if (!file) {
-    return Promise.reject(
-      new Error(
-        i18n.translate('xpack.fileUpload.fileParser.noFileProvided', {
-          defaultMessage: 'Error, no file provided',
-        })
-      )
-    );
-  }
-
-  // Halt any previous file reading & pattern checking activity
-  if (prevFileReader) {
-    prevFileReader.abort();
-  }
-  if (prevPatternReader) {
-    prevPatternReader.abortStream();
-  }
-
-  // Set up feature tracking
-  let featuresProcessed = 0;
-  const onFeatureRead = (feature) => {
-    // TODO: Add handling and tracking for cleanAndValidate fails
-    featuresProcessed++;
-    return cleanAndValidate(feature);
-  };
-
-  let start;
-  let stop = FILE_BUFFER;
-
-  prevFileReader = fileReader;
+  const filePromise = new Promise(async (resolve, reject) => {
+    if (!file) {
+      reject(
+        new Error(
+          i18n.translate('xpack.fileUpload.fileParser.noFileProvided', {
+            defaultMessage: 'Error, no file provided',
+          })
+        )
+      );
+      return;
+    }
 
-  const filePromise = new Promise((resolve, reject) => {
-    const onStreamComplete = (fileResults) => {
-      if (!featuresProcessed) {
-        reject(
-          new Error(
-            i18n.translate('xpack.fileUpload.fileParser.noFeaturesDetected', {
-              defaultMessage: 'Error, no features detected',
-            })
-          )
-        );
-      } else {
-        resolve(fileResults);
-      }
-    };
-    const patternReader = new PatternReader({ onFeatureDetect: onFeatureRead, onStreamComplete });
-    prevPatternReader = patternReader;
+    const batches = await loadInBatches(file, JSONLoader, {
+      json: {
+        jsonpaths: ['$.features'],
+        _rootObjectBatches: true,
+      },
+    });
 
-    fileReader.onloadend = ({ target: { readyState, result } }) => {
-      if (readyState === FileReader.DONE) {
-        if (!getFileParseActive() || !result) {
-          fileReader.abort();
-          patternReader.abortStream();
-          resolve(null);
-          return;
+    let featuresProcessed = 0;
+    const features = [];
+    const errors = [];
+    let boolGeometryErrs = false;
+    let parsedGeojson;
+    for await (const batch of batches) {
+      if (getFileParseActive()) {
+        switch (batch.batchType) {
+          case 'root-object-batch-complete':
+            if (!getFileParseActive()) {
+              resolve(null);
+              return;
+            }
+            if (featuresProcessed) {
+              parsedGeojson = { ...batch.container, features };
+            } else {
+              // Handle single feature geoJson
+              const cleanedSingleFeature = cleanAndValidate(batch.container);
+              if (cleanedSingleFeature.geometry && cleanedSingleFeature.geometry.type) {
+                parsedGeojson = cleanedSingleFeature;
+                featuresProcessed++;
+              }
+            }
+            break;
+          default:
+            for (const feature of batch.data) {
+              if (!feature.geometry || !feature.geometry.type) {
+                if (!boolGeometryErrs) {
+                  boolGeometryErrs = true;
+                  errors.push(
+                    new Error(
+                      i18n.translate('xpack.fileUpload.fileParser.featuresOmitted', {
+                        defaultMessage: 'Some features without geometry omitted',
+                      })
+                    )
+                  );
+                }
+              } else {
+                const cleanFeature = cleanAndValidate(feature);
+                features.push(cleanFeature);
+                featuresProcessed++;
+              }
+            }
         }
         setFileProgress({
           featuresProcessed,
-          bytesProcessed: stop || file.size,
+          bytesProcessed: batch.bytesUsed,
           totalBytes: file.size,
         });
-        patternReader.writeDataToPatternStream(result);
-        if (!stop) {
-          return;
-        }
-
-        start = stop;
-        const newStop = stop + FILE_BUFFER;
-        // Check EOF
-        stop = newStop > file.size ? undefined : newStop;
-        readSlice(fileReader, file, start, stop);
+      } else {
+        break;
       }
-    };
-    fileReader.onerror = () => {
-      fileReader.abort();
-      patternReader.abortStream();
+    }
+
+    if (!featuresProcessed && getFileParseActive()) {
       reject(
         new Error(
-          i18n.translate('xpack.fileUpload.fileParser.errorReadingFile', {
-            defaultMessage: 'Error reading file',
+          i18n.translate('xpack.fileUpload.fileParser.noFeaturesDetected', {
+            defaultMessage: 'Error, no features detected',
           })
         )
       );
-    };
+    } else if (!getFileParseActive()) {
+      resolve(null);
+    } else {
+      resolve({
+        errors,
+        parsedGeojson,
+      });
+    }
   });
-  readSlice(fileReader, file, start, stop);
+
   return filePromise;
 };
 

x-pack/plugins/file_upload/public/util/file_parser.test.js

@@ -5,32 +5,11 @@
  */
 
 import { fileHandler } from './file_parser';
-jest.mock('./pattern_reader', () => ({}));
+import '@loaders.gl/polyfills';
 
 const cleanAndValidate = jest.fn((a) => a);
 const setFileProgress = jest.fn((a) => a);
 
-const getFileReader = () => {
-  const fileReader = {
-    abort: jest.fn(),
-  };
-  fileReader.readAsBinaryString = jest.fn((binaryString = '123') =>
-    fileReader.onloadend({ target: { readyState: FileReader.DONE, result: binaryString } })
-  );
-  return fileReader;
-};
-const getPatternReader = () => {
-  const patternReader = {
-    writeDataToPatternStream: jest.fn(),
-    abortStream: jest.fn(),
-  };
-  require('./pattern_reader').PatternReader = function () {
-    this.writeDataToPatternStream = () => patternReader.writeDataToPatternStream();
-    this.abortStream = () => patternReader.abortStream();
-  };
-  return patternReader;
-};
-
 const testJson = {
   type: 'Feature',
   geometry: {
@@ -63,13 +42,13 @@ describe('parse file', () => {
   });
 
   it('should reject and throw error if no file provided', async () => {
-    expect(fileHandler(null)).rejects.toThrow();
+    await fileHandler({ file: null }).catch((e) => {
+      expect(e.message).toMatch('Error, no file provided');
+    });
   });
 
   it('should abort and resolve to null if file parse cancelled', async () => {
     const fileRef = getFileRef();
-    const cancelledActionFileReader = getFileReader();
-    const cancelledActionPatternReader = getPatternReader();
 
     // Cancel file parse
     const getFileParseActive = getFileParseActiveFactory(false);
@@ -79,53 +58,61 @@ describe('parse file', () => {
       setFileProgress,
       cleanAndValidate,
       getFileParseActive,
-      fileReader: cancelledActionFileReader,
     });
 
     expect(fileHandlerResult).toBeNull();
-    expect(cancelledActionFileReader.abort.mock.calls.length).toEqual(1);
-    expect(cancelledActionPatternReader.abortStream.mock.calls.length).toEqual(1);
   });
 
-  it('should abort on file reader error', () => {
+  it('should normally read single feature valid data', async () => {
     const fileRef = getFileRef();
+    const getFileParseActive = getFileParseActiveFactory();
+    const { errors } = await fileHandler({
+      file: fileRef,
+      setFileProgress,
+      cleanAndValidate: (x) => x,
+      getFileParseActive,
+    });
 
-    const fileReaderWithErrorCall = getFileReader();
-    const patternReaderWithErrorCall = getPatternReader();
+    expect(setFileProgress.mock.calls.length).toEqual(1);
+    expect(errors.length).toEqual(0);
+  });
 
-    // Trigger on error on read
-    fileReaderWithErrorCall.readAsBinaryString = () => fileReaderWithErrorCall.onerror();
+  it('should normally read a valid single feature file', async () => {
+    const testSinglePointJson = {
+      type: 'Feature',
+      geometry: {
+        type: 'Point',
+        coordinates: [30, 10],
+      },
+      properties: {
+        name: 'Point island',
+      },
+    };
+
+    const fileRef = getFileRef(testSinglePointJson);
     const getFileParseActive = getFileParseActiveFactory();
-    expect(
-      fileHandler({
-        file: fileRef,
-        setFileProgress,
-        cleanAndValidate,
-        getFileParseActive,
-        fileReader: fileReaderWithErrorCall,
-      })
-    ).rejects.toThrow();
-
-    expect(fileReaderWithErrorCall.abort.mock.calls.length).toEqual(1);
-    expect(patternReaderWithErrorCall.abortStream.mock.calls.length).toEqual(1);
+    const { errors } = await fileHandler({
+      file: fileRef,
+      setFileProgress,
+      cleanAndValidate: (x) => x,
+      getFileParseActive,
+    });
+
+    expect(setFileProgress.mock.calls.length).toEqual(1);
+    expect(errors.length).toEqual(0);
   });
 
-  // Expect 2 calls, one reads file, next is 'undefined' to
-  // both fileReader and patternReader
-  it('should normally read binary and emit to patternReader for valid data', async () => {
+  it('should throw if no valid features', async () => {
     const fileRef = getFileRef();
-    const fileReaderForValidFile = getFileReader();
-    const patternReaderForValidFile = getPatternReader();
     const getFileParseActive = getFileParseActiveFactory();
-    fileHandler({
+
+    await fileHandler({
       file: fileRef,
       setFileProgress,
-      cleanAndValidate,
+      cleanAndValidate: () => ({ not: 'the correct content' }), // Simulate clean and validate fail
       getFileParseActive,
-      fileReader: fileReaderForValidFile,
+    }).catch((e) => {
+      expect(e.message).toMatch('Error, no features detected');
     });
-
-    expect(fileReaderForValidFile.readAsBinaryString.mock.calls.length).toEqual(2);
-    expect(patternReaderForValidFile.writeDataToPatternStream.mock.calls.length).toEqual(2);
   });
 });

x-pack/plugins/file_upload/public/util/geo_json_clean_and_validate.js

@@ -11,14 +11,14 @@ const geoJSONReader = new jsts.io.GeoJSONReader();
 const geoJSONWriter = new jsts.io.GeoJSONWriter();
 
 export function geoJsonCleanAndValidate(feature) {
-  const geometryReadResult = geoJSONReader.read(feature);
-
-  const cleanedGeometry = cleanGeometry(geometryReadResult);
-
-  // For now, return the feature unmodified
-  // TODO: Consider more robust UI feedback and general handling
-  // for features that fail cleaning and/or validation
-  if (!cleanedGeometry) {
+  let cleanedGeometry;
+  // Attempts to clean geometry. If this fails, don't generate errors at this
+  // point, these will be handled more accurately on write to ES with feedback
+  // given to user
+  try {
+    const geometryReadResult = geoJSONReader.read(feature);
+    cleanedGeometry = cleanGeometry(geometryReadResult);
+  } catch (e) {
     return feature;
   }