fix(import): Fix guessFileType() for large JSON docs COMPASS-6629 (#4172)

lerouxb · web-flow · commit 8dc8ca1bf96f · 2023-03-21T19:02:51.000Z
* fix for guessFileType() for large JSON docs

* 1000 fields maybe a bit excessive

* more failsafes
diff --git a/packages/compass-import-export/src/import/guess-filetype.spec.ts b/packages/compass-import-export/src/import/guess-filetype.spec.ts
@@ -19,6 +19,63 @@ const expectedDelimiters = {
 } as const;
 
 describe('guessFileType', function () {
+  it('detects unknown for a file that is just one very long line', async function () {
+    const chars: string[] = [];
+    for (let i = 0; i < 1000001; i++) {
+      chars.push('x');
+    }
+    const input = Readable.from(chars.join(''));
+    const { type } = await guessFileType({ input });
+    expect(type).to.equal('unknown');
+  });
+
+  it('detects unknown for a file where the second line is very long', async function () {
+    const chars: string[] = ['x', '\n']; // there is one line break
+    for (let i = 0; i < 1000001; i++) {
+      chars.push('x');
+    }
+    const input = Readable.from(chars.join(''));
+    const { type } = await guessFileType({ input });
+    expect(type).to.equal('unknown');
+  });
+
+  it('detects json as long as there is one square bracket', async function () {
+    const chars: string[] = ['[', '\n']; // there is one line break
+    for (let i = 0; i < 1000001; i++) {
+      chars.push('x');
+    }
+    const input = Readable.from(chars.join(''));
+    const { type } = await guessFileType({ input });
+    expect(type).to.equal('json');
+  });
+
+  it('detects json for one square bracket even if it is otherwise valid csv', async function () {
+    const input = Readable.from('[a,b\n1,2');
+    const { type } = await guessFileType({ input });
+    expect(type).to.equal('json');
+  });
+
+  it('detects jsonl for anything starting with {', async function () {
+    const input = Readable.from('{a,b\n1,2');
+    const { type } = await guessFileType({ input });
+    expect(type).to.equal('jsonl');
+  });
+
+  it('detects json for a file with lots of fields', async function () {
+    const bigDocs: any[] = [];
+    for (let i = 0; i < 100; i++) {
+      const doc: any = {};
+      for (let j = 0; j < 100; j++) {
+        doc[`c_${j}`] = 'abcdefghijklmnop';
+      }
+      bigDocs.push(doc);
+    }
+    const jsonText = JSON.stringify(bigDocs);
+    const input = Readable.from(jsonText);
+    const { type } = await guessFileType({ input });
+    expect(type).to.equal('json');
+  });
+
   for (const filepath of Object.values(fixtures.json)) {
     const basename = path.basename(filepath);
     it(`detects ${basename} as json`, async function () {
diff --git a/packages/compass-import-export/src/import/guess-filetype.ts b/packages/compass-import-export/src/import/guess-filetype.ts
@@ -1,4 +1,4 @@
-import { PassThrough } from 'stream';
+import { PassThrough, Transform } from 'stream';
 import type { Readable } from 'stream';
 import Papa from 'papaparse';
 import StreamJSON from 'stream-json';
@@ -11,34 +11,50 @@ import { Utf8Validator } from '../utils/utf8-validator';
 const debug = createDebug('import-guess-filetype');
 
 function detectJSON(input: Readable): Promise<'json' | 'jsonl' | null> {
-  let jsonVariant: 'json' | 'jsonl' | null = null;
-
   return new Promise(function (resolve) {
     const parser = StreamJSON.parser();
 
+    let found = false;
+
     parser.once('data', (data) => {
       debug('detectJSON:data', data);
+
+      let jsonVariant: 'json' | 'jsonl' | null = null;
       if (data.name === 'startObject') {
         jsonVariant = 'jsonl';
       } else if (data.name === 'startArray') {
         jsonVariant = 'json';
       }
-      parser.destroy();
+
+      found = true;
+      resolve(jsonVariant);
     });
 
     parser.on('end', () => {
-      debug('detectJSON:end', jsonVariant);
-      resolve(jsonVariant);
+      debug('detectJSON:end');
+      if (!found) {
+        found = true;
+        // reached the end before a full doc
+        resolve(null);
+      }
     });
 
     parser.on('close', (err: Error) => {
-      debug('detectJSON:close', err, jsonVariant);
-      resolve(jsonVariant);
+      debug('detectJSON:close', err);
+      if (!found) {
+        found = true;
+        // stream closed before a full doc
+        resolve(null);
+      }
     });
 
     parser.on('error', (err: Error) => {
       debug('detectJSON:error', err);
-      resolve(null);
+      if (!found) {
+        found = true;
+        // got an error before a full doc
+        resolve(null);
+      }
     });
 
     input.pipe(parser);
@@ -73,40 +89,71 @@ function redetectDelimiter({ data }: { data: string[] }): string {
   return ',';
 }
 
-function detectCSV(input: Readable): Promise<Delimiter | null> {
+function detectCSV(
+  input: Readable,
+  jsonPromise: Promise<'json' | 'jsonl' | null>
+): Promise<Delimiter | null> {
   let csvDelimiter: Delimiter | null = null;
   let lines = 0;
-
-  return new Promise(function (resolve) {
-    Papa.parse(input, {
-      // NOTE: parsing without header: true otherwise the delimiter detection
-      // can't fail and will always detect ,
-      delimitersToGuess: supportedDelimiters,
-      step: function (results: Papa.ParseStepResult<string[]>, parser) {
-        ++lines;
-        debug('detectCSV:step', lines, results);
-        if (lines === 1) {
-          if (hasDelimiterError(results)) {
-            csvDelimiter = redetectDelimiter(results);
-          } else {
-            csvDelimiter = results.meta.delimiter;
-          }
+  let found = false;
+
+  // stop processing CSV as soon as we detect JSON
+  const jsonDetected = new Promise<null>(function (resolve) {
+    jsonPromise
+      .then((jsonType) => {
+        if (jsonType) {
+          resolve(null);
         }
-        // must be at least two lines for header row and data
-        if (lines === 2) {
-          parser.abort();
-        }
-      },
-      complete: function () {
-        debug('detectCSV:complete');
-        resolve(lines === 2 ? csvDelimiter : null);
-      },
-      error: function () {
-        debug('detectCSV:error');
-        resolve(null);
-      },
-    });
+      })
+      .catch(() => {
+        // if the file was not valid JSON, then ignore this because either CSV
+        // detection will eventually succeed FileSizeEnforcer will error
+      });
   });
+
+  return Promise.race([
+    jsonDetected,
+    new Promise<Delimiter | null>(function (resolve) {
+      Papa.parse(input, {
+        // NOTE: parsing without header: true otherwise the delimiter detection
+        // can't fail and will always detect ,
+        delimitersToGuess: supportedDelimiters,
+        step: function (results: Papa.ParseStepResult<string[]>) {
+          ++lines;
+          debug('detectCSV:step', lines, results);
+          if (lines === 1) {
+            if (hasDelimiterError(results)) {
+              csvDelimiter = redetectDelimiter(results);
+            } else {
+              csvDelimiter = results.meta.delimiter;
+            }
+          }
+          // must be at least two lines for header row and data
+          if (lines === 2) {
+            found = true;
+            debug('detectCSV:complete');
+            resolve(lines === 2 ? csvDelimiter : null);
+          }
+        },
+        complete: function () {
+          debug('detectCSV:complete');
+          if (!found) {
+            found = true;
+            // we reached the end before two lines
+            resolve(null);
+          }
+        },
+        error: function (err) {
+          debug('detectCSV:error', err);
+          if (!found) {
+            found = true;
+            // something failed before we got to the end of two lines
+            resolve(null);
+          }
+        },
+      });
+    }),
+  ]);
 }
 
 type GuessFileTypeOptions = {
@@ -122,6 +169,25 @@ type GuessFileTypeResult =
       csvDelimiter: Delimiter;
     };
 
+const MAX_LENGTH = 1000000;
+
+class FileSizeEnforcer extends Transform {
+  length = 0;
+
+  _transform(
+    chunk: Buffer,
+    enc: unknown,
+    cb: (err: null | Error, chunk?: Buffer) => void
+  ) {
+    this.length += chunk.length;
+    if (this.length > MAX_LENGTH) {
+      cb(new Error(`CSV still not detected after ${MAX_LENGTH} bytes`));
+    } else {
+      cb(null, chunk);
+    }
+  }
+}
+
 export function guessFileType({
   input,
 }: GuessFileTypeOptions): Promise<GuessFileTypeResult> {
@@ -134,13 +200,21 @@ export function guessFileType({
       });
 
       const jsStream = input.pipe(new PassThrough());
-      const csvStream = input.pipe(new PassThrough());
+      const csvStream = input
+        .pipe(new PassThrough())
+        .pipe(new FileSizeEnforcer());
+
+      const jsonPromise = detectJSON(jsStream);
 
       const [jsonVariant, csvDelimiter] = await Promise.all([
-        detectJSON(jsStream),
-        detectCSV(csvStream),
+        jsonPromise,
+        detectCSV(csvStream, jsonPromise),
       ]);
 
+      // keep streaming until both promises resolved, then destroy the input
+      // stream to stop further processing
+      input.destroy();
+
       debug('guessFileType', jsonVariant, csvDelimiter);
 
       // check JSON first because practically anything will parse as CSV
diff --git a/packages/compass-import-export/src/modules/import.ts b/packages/compass-import-export/src/modules/import.ts
@@ -519,6 +519,7 @@ const loadCSVPreviewDocs = (): ThunkAction<
   void,
   AnyAction
 > => {
+  console.log('loading preview docs');
   return async (
     dispatch: ThunkDispatch<RootImportState, void, AnyAction>,
     getState: () => RootImportState
