[File upload] File upload additional unit tests (#40192) (#41071)

* Dice up validation and parsing

* Should call onRemove callback if there is one on reset

* Remove unneeded onRemove refs since reset is now being called instead

* Dice up json preview and parse & clean

* Unit tests for preview and parse & clean

* This isn't accurate for MultiPoint and will be refactored heavily anyway for dynamic mappings in the near future

* Move index pattern validity check to service file. Update index functions to return name arr

* Test index name/pattern validity check

* Review feedback

* Review feedback

* Review feedback

* Review feedback

x-pack/legacy/plugins/file_upload/public/components/index_settings.js

@@ -8,17 +8,40 @@ import React, { Fragment, Component } from 'react';
 import { i18n } from '@kbn/i18n';
 import { EuiFormRow, EuiFieldText, EuiSelect, EuiCallOut } from '@elastic/eui';
 import { FormattedMessage } from '@kbn/i18n/react';
-import { getExistingIndices, getExistingIndexPatterns }
-  from '../util/indexing_service';
+import {
+  getExistingIndexNames,
+  getExistingIndexPatternNames,
+  checkIndexPatternValid,
+} from '../util/indexing_service';
 
 export class IndexSettings extends Component {
 
   state = {
     indexNameError: '',
     indexDisabled: true,
-    indexPatterns: null,
-    indexNames: null,
     indexName: '',
+    indexNameList: [],
+    indexPatternList: [],
+  };
+
+  async componentDidMount() {
+    this._isMounted = true;
+    this.loadExistingIndexData();
+  }
+
+  componentWillUnmount() {
+    this._isMounted = false;
+  }
+
+  loadExistingIndexData = async () => {
+    const indexNameList = await getExistingIndexNames();
+    const indexPatternList = await getExistingIndexPatternNames();
+    if (this._isMounted) {
+      this.setState({
+        indexNameList,
+        indexPatternList
+      });
+    }
   };
 
   componentDidUpdate(prevProps, prevState) {
@@ -36,48 +59,24 @@ export class IndexSettings extends Component {
     }
   }
 
-  async _getIndexNames() {
-    if (this.state.indexNames) {
-      return this.state.indexNames;
-    }
-    const indices = await getExistingIndices();
-    const indexNames = indices
-      ? indices.map(({ name }) => name)
-      : [];
-    this.setState({ indexNames });
-    return indexNames;
-  }
-
-  async _getIndexPatterns() {
-    if (this.state.indexPatterns) {
-      return this.state.indexPatterns;
-    }
-    const patterns = await getExistingIndexPatterns();
-    const indexPatterns = patterns
-      ? patterns.map(({ name }) => name)
-      : [];
-    this.setState({ indexPatterns });
-    return indexPatterns;
-  }
-
   _setIndexName = async name => {
     const errorMessage = await this._isIndexNameAndPatternValid(name);
     return this.setState({
       indexName: name,
       indexNameError: errorMessage
     });
-  }
+  };
 
   _onIndexChange = async ({ target }) => {
     const name = target.value;
     await this._setIndexName(name);
     this.props.setIndexName(name);
-  }
+  };
 
   _isIndexNameAndPatternValid = async name => {
-    const indexNames = await this._getIndexNames();
-    const indexPatterns = await this._getIndexPatterns();
-    if (indexNames.find(i => i === name) || indexPatterns.find(i => i === name)) {
+    const { indexNameList, indexPatternList } = this.state;
+    const nameAlreadyInUse = [ ...indexNameList, ...indexPatternList ].includes(name);
+    if (nameAlreadyInUse) {
       return (
         <FormattedMessage
           id="xpack.fileUpload.indexSettings.indexNameAlreadyExistsErrorMessage"
@@ -86,13 +85,8 @@ export class IndexSettings extends Component {
       );
     }
 
-    const reg = new RegExp('[\\\\/\*\?\"\<\>\|\\s\,\#]+');
-    if (
-      (name !== name.toLowerCase()) || // name should be lowercase
-      (name === '.' || name === '..')   || // name can't be . or ..
-      name.match(/^[-_+]/) !== null  || // name can't start with these chars
-      name.match(reg) !== null // name can't contain these chars
-    ) {
+    const indexPatternValid = checkIndexPatternValid(name);
+    if (!indexPatternValid) {
       return (
         <FormattedMessage
           id="xpack.fileUpload.indexSettings.indexNameContainsIllegalCharactersErrorMessage"
@@ -101,7 +95,7 @@ export class IndexSettings extends Component {
       );
     }
     return '';
-  }
+  };
 
   render() {
     const { setSelectedIndexType, indexTypes } = this.props;

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

@@ -21,89 +21,87 @@ export class JsonIndexFilePicker extends Component {
   state = {
     fileUploadError: '',
     fileParsingProgress: '',
-    fileRef: null
   };
 
-  componentDidUpdate(prevProps, prevState) {
-    if (prevState.fileRef !== this.props.fileRef) {
-      this.setState({ fileRef: this.props.fileRef });
-    }
+  async componentDidMount() {
+    this._isMounted = true;
   }
 
-  _fileHandler = async fileList => {
-    const {
-      resetFileAndIndexSettings, setParsedFile, onFileRemove, onFileUpload,
-      transformDetails, setFileRef, setIndexName
-    } = this.props;
+  componentWillUnmount() {
+    this._isMounted = false;
+  }
 
-    const { fileRef } = this.state;
-
-    resetFileAndIndexSettings();
+  _fileHandler = fileList => {
+    const fileArr = Array.from(fileList);
+    this.props.resetFileAndIndexSettings();
     this.setState({ fileUploadError: '' });
-    if (fileList.length === 0) { // Remove
-      setParsedFile(null);
-      if (onFileRemove) {
-        onFileRemove(fileRef);
-      }
-    } else if (fileList.length === 1) { // Parse & index file
-      const file = fileList[0];
-      if (!file.name) {
-        this.setState({
-          fileUploadError: i18n.translate(
-            'xpack.fileUpload.jsonIndexFilePicker.noFileNameError',
-            { defaultMessage: 'No file name provided' })
-        });
-        return;
-      }
-
-      // Check file type, assign default index name
-      const splitNameArr = file.name.split('.');
-      const fileType = splitNameArr.pop();
-      const types = ACCEPTABLE_FILETYPES.reduce((accu, type) => {
-        accu = accu ? `${accu}, ${type}` : type;
-        return accu;
-      }, '');
-      if (!ACCEPTABLE_FILETYPES.includes(fileType)) {
-        this.setState({
-          fileUploadError: (
-            <FormattedMessage
-              id="xpack.fileUpload.jsonIndexFilePicker.acceptableTypesError"
-              defaultMessage="File is not one of acceptable types: {types}"
-              values={{ types }}
-            />
-          )
-        });
-        return;
-      }
-      const initIndexName = splitNameArr[0];
-      setIndexName(initIndexName);
-
-      // Check valid size
-      const { size } = file;
-      if (size > MAX_FILE_SIZE) {
-        this.setState({
-          fileUploadError: (
-            <FormattedMessage
-              id="xpack.fileUpload.jsonIndexFilePicker.acceptableFileSize"
-              defaultMessage="File size {fileSize} bytes exceeds max file size of {maxFileSize}"
-              values={{
-                fileSize: size,
-                maxFileSize: MAX_FILE_SIZE
-              }}
-            />
-          )
-        });
-        return;
-      }
-
-      // Parse file
-      this.setState({ fileParsingProgress: i18n.translate(
-        'xpack.fileUpload.jsonIndexFilePicker.parsingFile',
-        { defaultMessage: 'Parsing file...' })
+    if (fileArr.length === 0) { // Remove
+      return;
+    }
+    const file = fileArr[0];
+    let initIndexName;
+    try {
+      initIndexName = this._getIndexName(file);
+    } catch (error) {
+      this.setState({
+        fileUploadError: i18n.translate('xpack.fileUpload.jsonIndexFilePicker.errorGettingIndexName', {
+          defaultMessage: 'Error retrieving index name: {errorMessage}',
+          values: {
+            errorMessage: error.message
+          }
+        })
       });
-      const parsedFileResult = await parseFile(
-        file, onFileUpload, transformDetails
-      ).catch(err => {
+      return;
+    }
+
+    this.props.setIndexName(initIndexName);
+    this._parseFile(file);
+  };
+
+  _getIndexName({ name, size }) {
+    if (!name) {
+      throw new Error(i18n.translate('xpack.fileUpload.jsonIndexFilePicker.noFileNameError', {
+        defaultMessage: 'No file name provided'
+      }));
+    }
+
+    const splitNameArr = name.split('.');
+    const fileType = splitNameArr.pop();
+    if (!ACCEPTABLE_FILETYPES.includes(fileType)) {
+      throw new Error(i18n.translate('xpack.fileUpload.jsonIndexFilePicker.acceptableTypesError', {
+        defaultMessage: 'File is not one of acceptable types: {types}',
+        values: {
+          types: ACCEPTABLE_FILETYPES.join(', ')
+        }
+      }));
+    }
+
+    if (size > MAX_FILE_SIZE) {
+      throw new Error(i18n.translate('xpack.fileUpload.jsonIndexFilePicker.acceptableFileSize', {
+        defaultMessage: 'File size {fileSize} bytes exceeds max file size of {maxFileSize}',
+        values: {
+          fileSize: size,
+          maxFileSize: MAX_FILE_SIZE
+        }
+      }));
+    }
+
+    return splitNameArr[0];
+  }
+
+  async _parseFile(file) {
+    const {
+      setFileRef, setParsedFile, resetFileAndIndexSettings, onFileUpload, transformDetails
+    } = this.props;
+    // Parse file
+    this.setState({ fileParsingProgress: i18n.translate(
+      'xpack.fileUpload.jsonIndexFilePicker.parsingFile',
+      { defaultMessage: 'Parsing file...' })
+    });
+    const parsedFileResult = await parseFile(
+      file, transformDetails, onFileUpload
+    ).catch(err => {
+      if (this._isMounted) {
         this.setState({
           fileUploadError: (
             <FormattedMessage
@@ -115,23 +113,18 @@ export class JsonIndexFilePicker extends Component {
             />
           )
         });
-      });
-      this.setState({ fileParsingProgress: '' });
-      if (!parsedFileResult) {
-        if (fileRef) {
-          if (onFileRemove) {
-            onFileRemove(fileRef);
-          }
-          setFileRef(null);
-        }
-        return;
       }
-      setFileRef(file);
-      setParsedFile(parsedFileResult);
-
-    } else {
-      // No else
+    });
+    if (!this._isMounted) {
+      return;
     }
+    this.setState({ fileParsingProgress: '' });
+    if (!parsedFileResult) {
+      resetFileAndIndexSettings();
+      return;
+    }
+    setFileRef(file);
+    setParsedFile(parsedFileResult);
   }
 
   render() {

x-pack/legacy/plugins/file_upload/public/components/json_upload_and_parse.js

@@ -63,6 +63,9 @@ export class JsonUploadAndParse extends Component {
   };
 
   _resetFileAndIndexSettings = () => {
+    if (this.props.onFileRemove && this.state.fileRef) {
+      this.props.onFileRemove(this.state.fileRef);
+    }
     this.setState({
       indexTypes: [],
       selectedIndexType: '',
@@ -209,7 +212,7 @@ export class JsonUploadAndParse extends Component {
       currentIndexingStage, indexDataResp, indexPatternResp, fileRef,
       indexName, indexTypes, showImportProgress
     } = this.state;
-    const { onFileUpload, onFileRemove, transformDetails } = this.props;
+    const { onFileUpload, transformDetails } = this.props;
 
     return (
       <EuiForm>
@@ -226,7 +229,6 @@ export class JsonUploadAndParse extends Component {
               <JsonIndexFilePicker
                 {...{
                   onFileUpload,
-                  onFileRemove,
                   fileRef,
                   setIndexName: indexName => this.setState({ indexName }),
                   setFileRef: fileRef => this.setState({ fileRef }),
@@ -270,5 +272,6 @@ JsonUploadAndParse.propTypes = {
   ]),
   onIndexReadyStatusChange: PropTypes.func,
   onIndexingComplete: PropTypes.func,
-  onFileUpload: PropTypes.func
+  onFileUpload: PropTypes.func,
+  onFileRemove: PropTypes.func,
 };

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

@@ -8,9 +8,38 @@ import _ from 'lodash';
 import { geoJsonCleanAndValidate } from './geo_json_clean_and_validate';
 import { i18n } from '@kbn/i18n';
 
-export async function parseFile(file, previewCallback = null, transformDetails,
-  FileReader = window.FileReader) {
+export async function readFile(file) {
+  const readPromise = new Promise((resolve, reject) => {
+    if (!file) {
+      reject(new Error(i18n.translate(
+        'xpack.fileUpload.fileParser.noFileProvided', {
+          defaultMessage: 'Error, no file provided',
+        })));
+    }
+    const fr = new window.FileReader();
+    fr.onload = e => resolve(e.target.result);
+    fr.onerror = () => {
+      fr.abort();
+      reject(new Error(i18n.translate(
+        'xpack.fileUpload.fileParser.errorReadingFile', {
+          defaultMessage: 'Error reading file',
+        })));
+    };
+    fr.readAsText(file);
+  });
 
+  return await readPromise;
+}
+
+export function jsonPreview(json, previewFunction) {
+  // Call preview (if any)
+  if (json && previewFunction) {
+    const defaultName = _.get(json, 'name', 'Import File');
+    previewFunction(_.cloneDeep(json), defaultName);
+  }
+}
+
+export async function parseFile(file, transformDetails, previewCallback = null) {
   let cleanAndValidate;
   if (typeof transformDetails === 'object') {
     cleanAndValidate = transformDetails.cleanAndValidate;
@@ -31,25 +60,11 @@ export async function parseFile(file, previewCallback = null, transformDetails,
     }
   }
 
-  return new Promise((resolve, reject) => {
-    const fr = new FileReader();
-    fr.onload = ({ target: { result } }) => {
-      try {
-        const parsedJson = JSON.parse(result);
-        // Clean & validate
-        const cleanAndValidJson = cleanAndValidate(parsedJson);
-        if (!cleanAndValidJson) {
-          return;
-        }
-        if (previewCallback) {
-          const defaultName = _.get(cleanAndValidJson, 'name', 'Import File');
-          previewCallback(cleanAndValidJson, defaultName);
-        }
-        resolve(cleanAndValidJson);
-      } catch (e) {
-        reject(e);
-      }
-    };
-    fr.readAsText(file);
-  });
+  const rawResults = await readFile(file);
+  const parsedJson = JSON.parse(rawResults);
+  const jsonResult = cleanAndValidate(parsedJson);
+  jsonPreview(jsonResult, previewCallback);
+
+  return jsonResult;
 }
+

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

@@ -0,0 +1,86 @@
+/*
+ * 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.
+ */
+
+import { parseFile, jsonPreview } from './file_parser';
+
+describe('parse file', () => {
+  const cleanAndValidate = jest.fn(a => a);
+  const previewFunction = jest.fn();
+  const transformDetails = {
+    cleanAndValidate
+  };
+  const getFileRef = fileContent =>
+    new File([fileContent], 'test.json', { type: 'text/json' });
+
+  beforeEach(() => {
+    cleanAndValidate.mockClear();
+    previewFunction.mockClear();
+  });
+
+  it('should parse valid JSON', async () => {
+    const validJsonFileResult = JSON.stringify({
+      'type': 'Feature',
+      'geometry': {
+        'type': 'Polygon',
+        'coordinates': [[
+          [-104.05, 78.99],
+          [-87.22, 78.98],
+          [-86.58, 75.94],
+          [-104.03, 75.94],
+          [-104.05, 78.99]
+        ]]
+      },
+    });
+
+    await parseFile(getFileRef(validJsonFileResult), transformDetails);
+    // Confirm cleanAndValidate called
+    expect(cleanAndValidate.mock.calls.length).toEqual(1);
+    // Confirm preview function not called
+    expect(previewFunction.mock.calls.length).toEqual(0);
+  });
+
+  it('should call preview callback function if provided', async () => {
+    const validJsonFileResult = JSON.stringify({
+      'type': 'Feature',
+      'geometry': {
+        'type': 'Polygon',
+        'coordinates': [[
+          [-104.05, 78.99],
+          [-87.22, 78.98],
+          [-86.58, 75.94],
+          [-104.03, 75.94],
+          [-104.05, 78.99]
+        ]]
+      },
+    });
+
+    await parseFile(getFileRef(validJsonFileResult), transformDetails, previewFunction);
+    // Confirm preview function called
+    expect(previewFunction.mock.calls.length).toEqual(1);
+  });
+
+  it('should use object clone for preview function', () => {
+    const justFinalJson = {
+      'type': 'Feature',
+      'geometry': {
+        'type': 'Polygon',
+        'coordinates': [[
+          [-104.05, 78.99],
+          [-87.22,  78.98],
+          [-86.58,  75.94],
+          [-104.03, 75.94],
+          [-104.05, 78.99]
+        ]]
+      },
+    };
+
+    jsonPreview(justFinalJson, previewFunction);
+    // Confirm equal object passed
+    expect(previewFunction.mock.calls[0][0]).toEqual(justFinalJson);
+    // Confirm not the same object
+    expect(previewFunction.mock.calls[0][0]).not.toBe(justFinalJson);
+  });
+});

x-pack/legacy/plugins/file_upload/public/util/geo_processing.js

@@ -6,7 +6,6 @@
 
 import _ from 'lodash';
 import { ES_GEO_FIELD_TYPE } from '../../common/constants/file_import';
-import { i18n } from '@kbn/i18n';
 
 const DEFAULT_SETTINGS = {
   number_of_shards: 1
@@ -62,16 +61,6 @@ export function geoJsonToEs(parsedGeojson, datatype) {
   } else if (datatype === ES_GEO_FIELD_TYPE.GEO_POINT) {
     return features.reduce((accu, { geometry, properties }) => {
       const { coordinates } = geometry;
-      if (Array.isArray(coordinates[0])) {
-        throw(
-          i18n.translate(
-            'xpack.fileUpload.geoProcessing.notPointError', {
-              defaultMessage: 'Coordinates {coordinates} does not contain point datatype',
-              values: { coordinates: coordinates.toString() }
-            })
-        );
-        return accu;
-      }
       accu.push({
         coordinates,
         ...(!_.isEmpty(properties) ? { ...properties } : {})

x-pack/legacy/plugins/file_upload/public/util/indexing_service.js

@@ -67,7 +67,6 @@ export async function indexData(parsedFile, transformDetails, indexName, dataTyp
   return indexWriteResults;
 }
 
-
 function transformDataByFormatForIndexing(transform, parsedFile, dataType) {
   let indexingDetails;
   if (!transform) {
@@ -237,21 +236,40 @@ async function getIndexPatternId(name) {
   }
 }
 
-export async function getExistingIndices() {
+export const getExistingIndexNames = async () => {
   const basePath = chrome.addBasePath('/api');
-  return await http({
+  const indexes = await http({
     url: `${basePath}/index_management/indices`,
     method: 'GET',
   });
-}
+  return indexes
+    ? indexes.map(({ name }) => name)
+    : [];
+};
 
-export async function getExistingIndexPatterns() {
+export const getExistingIndexPatternNames = async () => {
   const savedObjectsClient = chrome.getSavedObjectsClient();
-  return savedObjectsClient.find({
+  const indexPatterns = await savedObjectsClient.find({
     type: 'index-pattern',
     fields: ['id', 'title', 'type', 'fields'],
     perPage: 10000
-  }).then(({ savedObjects }) =>
-    savedObjects.map(savedObject => savedObject.get('title'))
+  }).then(
+    ({ savedObjects }) => savedObjects.map(savedObject => savedObject.get('title'))
   );
+  return indexPatterns
+    ? indexPatterns.map(({ name }) => name)
+    : [];
+};
+
+export function checkIndexPatternValid(name) {
+  const byteLength = encodeURI(name)
+    .split(/%(?:u[0-9A-F]{2})?[0-9A-F]{2}|./).length - 1;
+  const reg = new RegExp('[\\\\/\*\?\"\<\>\|\\s\,\#]+');
+  const indexPatternInvalid =
+    byteLength > 255 || // name can't be greater than 255 bytes
+    name !== name.toLowerCase() || // name should be lowercase
+    (name === '.' || name === '..')   || // name can't be . or ..
+    name.match(/^[-_+]/) !== null  || // name can't start with these chars
+    name.match(reg) !== null;  // name can't contain these chars
+  return !indexPatternInvalid;
 }

x-pack/legacy/plugins/file_upload/public/util/indexing_service.test.js

@@ -0,0 +1,55 @@
+/*
+ * 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.
+ */
+
+import { checkIndexPatternValid } from './indexing_service';
+
+describe('indexing_service', () => {
+
+  const validNames = [
+    'lowercaseletters', // Lowercase only
+    '123', // Cannot include \, /, *, ?, ", <, >, |, " " (space character), , (comma), #
+    'does_not_start_with_underscores', // Cannot start with _
+    'does-not-start-with-a-dash', // Cannot start with -
+    'does+not+start+with+a+plus', // Cannot start with +
+    'is..not..just..two..periods', // name can't be ..
+    'is.not.just.one.period', // name can't be .
+    'x'.repeat(255) // Cannot be longer than 255 bytes
+  ];
+  validNames.forEach(validName => {
+    it(`Should validate index pattern: "${validName}"`, () => {
+      const isValid = checkIndexPatternValid(validName);
+      expect(isValid).toEqual(true);
+    });
+  });
+
+  const inValidNames = [
+    'someUpperCaseLetters', // Lowercase only
+    '1\\2\\3', // Cannot include \
+    '1/2/3', // Cannot include /
+    '1*2*3', // Cannot include *
+    '1?2?3', // Cannot include ?
+    '1"2"3', // Cannot include "
+    '1<2<3', // Cannot include <
+    '1>2>3', // Cannot include >
+    '1|2|3', // Cannot include |
+    '1 2 3', // Cannot include space character
+    '1,2,3', // Cannot include ,
+    '1#2#3', // Cannot include #
+    '_starts_with_underscores', // Cannot start with _
+    '-starts-with-a-dash', // Cannot start with -
+    '+starts+with+a+plus', // Cannot start with +
+    '..', // name can't be ..
+    '.', // name can't be .
+    'x'.repeat(256), // Cannot be longer than 255 bytes
+    'ü'.repeat(128) // Cannot be longer than 255 bytes (using 2 byte char)
+  ];
+  inValidNames.forEach(inValidName => {
+    it(`Should invalidate index pattern: "${inValidName}"`, () => {
+      const isValid = checkIndexPatternValid(inValidName);
+      expect(isValid).toEqual(false);
+    });
+  });
+});