[Maps][File upload] Parse geojson files in chunks to avoid thread blocking

[kindsun]

Resolves #40205. This PR leverages the capabilities of FileReader to:

• Read the file into a binary format
• Give progress indication after each chunk has been read

It leverages oboejs to parse features out of the binary data stream for cleaning and validation

A couple of things to note with this PR

• We aren't currently set up in the Maps app to add layers incrementally in quick succession. In other words, there's no easy way for us to tell if a layer has been added to mapbox-gl and then add more data to it, but it's definitely possible in the near-future. This and a few other ideas have been captured in [Maps][File upload] Tracking issue for GeoJSON Upload Optimizations #46376. In the meantime, we are able to build the parsed file incrementally before display, which does work quite a bit better then how it works currently on master
• There are a number of different sax-style parsers available. oboejs is one of the stronger options in my opinion, but we're not by any means "locked in". The actual chunking logic is performed by the FileReader instance. Any future option that might replace oboejs would just have to be able to read binary chunks and identify text patterns in a similar way.

[elasticmachine]

Pinging @elastic/kibana-gis

[elasticmachine]

💔 Build Failed

• continuous-integration/kibana-ci/pull-request

[bcamper]

Giving this a whirl locally, yay :)

In addition to having % of file processed displayed, I think it could be helpful to have some additional live-updating meta info like # of features processed, perhaps broken down by # of points, lines, and polygons. Basically, more feedback for the user that shows something is actually happening (we've all stared at our % counter waiting and hoping it will increment...) and can also be of practical value.

[elasticmachine]

💚 Build Succeeded

• continuous-integration/kibana-ci/pull-request
• Commit: 325d1f5

[elasticmachine]

💔 Build Failed

• continuous-integration/kibana-ci/pull-request
• Commit: aa4f986

[elasticmachine]

💔 Build Failed

• continuous-integration/kibana-ci/pull-request
• Commit: 9e6505d

[thomasneirynck]

Thanks for addressing feedback, some additional suggestions to improve readability

[elasticmachine]

💚 Build Succeeded

• continuous-integration/kibana-ci/pull-request
• Commit: 1b7b0fb

[elasticmachine]

💚 Build Succeeded

• continuous-integration/kibana-ci/pull-request
• Commit: 7051a73

[thomasneirynck]

I added a few more comments to tidy-up, but looking much better, thanks.

I tested this PR with following file, and Kibana crashes completely. Seems to be related to having a null geometry (?)

cpd-incidents.geojson.zip

:5601/wzd/bundles/commons.bundle.js:239591 Uncaught (in promise) TypeError: Cannot read property 'type' of null
    at :5601/wzd/bundles/commons.bundle.js:239591
    at Array.map (<anonymous>)
    at JsonUploadAndParse._setIndexTypes (:5601/wzd/bundles/commons.bundle.js:239589)
    at JsonUploadAndParse.componentDidUpdate (:5601/wzd/bundles/commons.bundle.js:239628)
    at commitLifeCycles (webpack://%5Bname%5D/./node_modules/react-dom/cjs/react-dom.development.js?:17143)
    at commitAllLifeCycles (webpack://%5Bname%5D/./node_modules/react-dom/cjs/react-dom.development.js?:18530)
    at HTMLUnknownElement.callCallback (webpack://%5Bname%5D/./node_modules/react-dom/cjs/react-dom.development.js?:149)
    at Object.invokeGuardedCallbackDev (webpack://%5Bname%5D/./node_modules/react-dom/cjs/react-dom.development.js?:199)
    at invokeGuardedCallback (webpack://%5Bname%5D/./node_modules/react-dom/cjs/react-dom.development.js?:256)
    at commitRoot (webpack://%5Bname%5D/./node_modules/react-dom/cjs/react-dom.development.js?:18742)

Even if we do not handle error-messaging in the UX, we should avoid Kibana crashing completely when faulty inputs propagate. For that particular file, you would need to up the limit to 90mb iso 50mb.

[nreese]

looking really good. Just a few minor changes.

[kindsun]

I tested this PR with following file, and Kibana crashes completely. Seems to be related to having a null geometry (?)

cpd-incidents.geojson.zip

Interesting dataset, many geometries set to null. For now, I've added code to drop null features and display a parsing error to the user. A more robust solution would likely fall under the previously linked error-handling issue. This is the best solution for now as we're not well equipped in File Upload or Maps to handle features with null geometries. There are a lot of assumptions around features having a geometry and associated type throughout the code that we'll need to address if we want to include them in client results.

[bcamper]

It is valid GeoJSON to have a null geometry (not 100% why the spec has it but it does...), so I don't think it needs to or should be treated as an error to the user. It could be a useful warning I suppose, as some users may not be aware of these features.

…

On Tue, Oct 15, 2019 at 2:48 PM Aaron Caldwell ***@***.***> wrote: I tested this PR with following file, and Kibana crashes completely. Seems to be related to having a null geometry (?) cpd-incidents.geojson.zip Interesting dataset, many geometries set to null. For now, I've added code to drop null features and display a parsing error to the user. A more robust solution would likely fall under the previously linked error-handling issue. This is the best solution for now as we're not well equipped in File Upload or Maps to handle features with null geometries. There are a lot of assumptions around features having a geometry and associated type throughout the code that we'll need to address if we want to include them in client results. — You are receiving this because you are on a team that was mentioned. Reply to this email directly, view it on GitHub <#46710?email_source=notifications&email_token=AAAECXP5G6UAVOVXQEIKOFLQOYF7JA5CNFSM4I22SXW2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEBJ2IHI#issuecomment-542352413>, or unsubscribe <https:/notifications/unsubscribe-auth/AAAECXICBUNLTVR3S6TVQLDQOYF7JANCNFSM4I22SXWQ> .

[kindsun]

It is valid GeoJSON to have a null geometry (not 100% why the spec has it
but it does...), so I don't think it needs to or should be treated as an
error to the user. It could be a useful warning I suppose, as some users
may not be aware of these features.

Thanks @bcamper! Agreed that ultimately we need to be prepared for anything that comes through the door passing schema validation. We're not quite there yet, but I definitely think it's where we want to get to. Prior to this PR, the behavior was to show an error and not allow the user to upload a file that contained features with any "invalid" (null included) geometries. Basically an "all or nothing" approach. Since this PR is the first step in "breaking the file into chunks" and looking at it on a feature-by-feature basis, we now have the luxury of being able to handle features in a more tailored way.

So this is the first step- "Open the file in chunks and stop thread-blocking". So mostly I'm just maintaining existing functionality elsewhere (i.e.- same file limits, similar error cases, etc.). It still shows some features had "errors" as it did before, but it at least allows indexing the remaining ones. Future work, as you suggest, should definitely revisit this though and allow a diversity of warnings and errors to cover different cases!

[kindsun]

retest

[bcamper]

Makes sense, iterative improvement is good :)

…

On Tue, Oct 15, 2019 at 3:36 PM Aaron Caldwell ***@***.***> wrote: It is valid GeoJSON to have a null geometry (not 100% why the spec has it but it does...), so I don't think it needs to or should be treated as an error to the user. It could be a useful warning I suppose, as some users may not be aware of these features. Thanks @bcamper <https:/bcamper> Agreed that ultimately we need to be prepared for anything that comes through the door passing schema validation <#47472>. We're not quite there yet, but I definitely think it's where we want to get to. Prior to this PR, the behavior was to show an error and not allow the user to upload a file that contained features with any "invalid" (null included) geometries. Basically an "all or nothing" approach. Since this PR is the first step in "breaking the file into chunks" and looking at it on a feature-by-feature basis, we now have the luxury of being able to handle features in a more tailored way. So this is the first step- "Open the file in chunks and stop thread-blocking". So mostly I'm just maintaining existing functionality elsewhere (i.e.- same file limits, similar error cases, etc.), it still shows some features had "errors" as it did before, but it at least allows indexing the remaining ones. Future work, as you suggest, should definitely revisit this though and allow a diversity of warnings and errors to cover different cases! — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#46710?email_source=notifications&email_token=AAAECXJ23MM3FTBYYOV43ADQOYLURA5CNFSM4I22SXW2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEBJ6YUY#issuecomment-542370899>, or unsubscribe <https:/notifications/unsubscribe-auth/AAAECXKAJM5KRIP3TBK6VJLQOYLURANCNFSM4I22SXWQ> .

[elasticmachine]

💔 Build Failed

• continuous-integration/kibana-ci/pull-request
• Commit: 33ca43f

[nreese]

Nice change. This is great that the parsing of the file no longer blocks the browser

lgtm
code review, tested in chrome

[elasticmachine]

💚 Build Succeeded

• continuous-integration/kibana-ci/pull-request
• Commit: 95a485e