From a52cdbe54dd1208f3ead446f28a5987e3831bd6f Mon Sep 17 00:00:00 2001 From: mikesealey Date: Tue, 19 Nov 2024 17:18:11 +0000 Subject: [PATCH 01/11] allows additional CSV delimiters --- .../server/src/api/routes/tests/table.spec.ts | 15 +++++++++++++++ packages/server/src/utilities/csv.ts | 5 ++++- 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/packages/server/src/api/routes/tests/table.spec.ts b/packages/server/src/api/routes/tests/table.spec.ts index b9d8696714..692a3c4b91 100644 --- a/packages/server/src/api/routes/tests/table.spec.ts +++ b/packages/server/src/api/routes/tests/table.spec.ts @@ -1511,5 +1511,20 @@ datasourceDescribe( }) }) }) + describe("csvToJson", () => { + const delimiters = [",", ";", ":", "|", "~", "\t", " "] + it.only.each(delimiters)( + "can parse CSVs with delimiter; %s", + async delimiter => { + const rows = await config.api.table.csvToJson({ + csvString: [ + ["a", "b", "c", "d", "e"].join(delimiter), + ["1", "2", "3", "4", "5"].join(delimiter), + ].join("\n"), + }) + expect(rows).toEqual([{ a: "1", b: "2", c: "3", d: "4", e: "5" }]) + } + ) + }) } ) diff --git a/packages/server/src/utilities/csv.ts b/packages/server/src/utilities/csv.ts index 43d712165a..4ecdbba37e 100644 --- a/packages/server/src/utilities/csv.ts +++ b/packages/server/src/utilities/csv.ts @@ -9,7 +9,10 @@ export async function jsonFromCsvString(csvString: string) { // is causing issues on conversion. ignoreEmpty will remove the key completly // if empty, so creating this empty object will ensure we return the values // with the keys but empty values - const result = await csv({ ignoreEmpty: false }).fromString(csvString) + const result = await csv({ + ignoreEmpty: false, + delimiter: [",", ";", ":", "|", "~", "\t", " "], + }).fromString(csvString) result.forEach((r, i) => { for (const [key] of Object.entries(r).filter(([, value]) => value === "")) { if (castedWithEmptyValues[i][key] === undefined) { From d02eea577085eacb627a6693e041292c7331462b Mon Sep 17 00:00:00 2001 From: mikesealey Date: Wed, 29 Jan 2025 15:42:33 +0000 Subject: [PATCH 02/11] adds checks for column headers --- .../backend/TableNavigator/utils.js | 2 +- packages/server/src/utilities/csv.ts | 58 +++++++++++++------ 2 files changed, 41 insertions(+), 19 deletions(-) diff --git a/packages/builder/src/components/backend/TableNavigator/utils.js b/packages/builder/src/components/backend/TableNavigator/utils.js index 2889d90cc0..5f13f9df33 100644 --- a/packages/builder/src/components/backend/TableNavigator/utils.js +++ b/packages/builder/src/components/backend/TableNavigator/utils.js @@ -58,7 +58,7 @@ export const parseFile = e => { resolveRows(rows) }) .catch(() => { - reject("cannot parse csv") + reject("cannot parse csv.") }) } }) diff --git a/packages/server/src/utilities/csv.ts b/packages/server/src/utilities/csv.ts index 4ecdbba37e..8540b62941 100644 --- a/packages/server/src/utilities/csv.ts +++ b/packages/server/src/utilities/csv.ts @@ -1,25 +1,47 @@ import csv from "csvtojson" export async function jsonFromCsvString(csvString: string) { - const castedWithEmptyValues = await csv({ ignoreEmpty: true }).fromString( - csvString - ) + const possibleDelimeters = [",", ";", ":", "|", "~", "\t", " "] - // By default the csvtojson library casts empty values as empty strings. This - // is causing issues on conversion. ignoreEmpty will remove the key completly - // if empty, so creating this empty object will ensure we return the values - // with the keys but empty values - const result = await csv({ - ignoreEmpty: false, - delimiter: [",", ";", ":", "|", "~", "\t", " "], - }).fromString(csvString) - result.forEach((r, i) => { - for (const [key] of Object.entries(r).filter(([, value]) => value === "")) { - if (castedWithEmptyValues[i][key] === undefined) { - r[key] = null + for (let i = 0; i < possibleDelimeters.length; i++) { + let numOfHeaders: number | undefined = undefined + let headerMismatch = false + + const castedWithEmptyValues = await csv({ + ignoreEmpty: true, + delimiter: possibleDelimeters[i], + }).fromString(csvString) + + // By default the csvtojson library casts empty values as empty strings. This + // is causing issues on conversion. ignoreEmpty will remove the key completly + // if empty, so creating this empty object will ensure we return the values + // with the keys but empty values + const result = await csv({ + ignoreEmpty: false, + delimiter: possibleDelimeters[i], + }).fromString(csvString) + for (const [i, r] of result.entries()) { + const columns = Object.keys(r) + if (numOfHeaders == null) { + numOfHeaders = columns.length + } + if (numOfHeaders !== columns.length) { + headerMismatch = true + break + } + for (const [key] of Object.entries(r).filter( + ([, value]) => value === "" + )) { + // if (castedWithEmptyValues[i][key] === undefined) { + // r[key] = null + // } } } - }) - - return result + if (headerMismatch) { + continue + } else { + return result + } + } + throw new Error("Unable to determine delimiter") } From 631448871ebbbbea321c5612101ef2b57081ffcf Mon Sep 17 00:00:00 2001 From: mikesealey Date: Wed, 29 Jan 2025 16:39:21 +0000 Subject: [PATCH 03/11] provides more robust .csv handling with various delimiters --- .../backend/TableNavigator/utils.js | 2 +- packages/server/src/utilities/csv.ts | 71 +++++++++++-------- 2 files changed, 42 insertions(+), 31 deletions(-) diff --git a/packages/builder/src/components/backend/TableNavigator/utils.js b/packages/builder/src/components/backend/TableNavigator/utils.js index 5f13f9df33..2889d90cc0 100644 --- a/packages/builder/src/components/backend/TableNavigator/utils.js +++ b/packages/builder/src/components/backend/TableNavigator/utils.js @@ -58,7 +58,7 @@ export const parseFile = e => { resolveRows(rows) }) .catch(() => { - reject("cannot parse csv.") + reject("cannot parse csv") }) } }) diff --git a/packages/server/src/utilities/csv.ts b/packages/server/src/utilities/csv.ts index 8540b62941..1e139f36fa 100644 --- a/packages/server/src/utilities/csv.ts +++ b/packages/server/src/utilities/csv.ts @@ -7,40 +7,51 @@ export async function jsonFromCsvString(csvString: string) { let numOfHeaders: number | undefined = undefined let headerMismatch = false - const castedWithEmptyValues = await csv({ - ignoreEmpty: true, - delimiter: possibleDelimeters[i], - }).fromString(csvString) + try { + const castedWithEmptyValues = await csv({ + ignoreEmpty: true, + delimiter: possibleDelimeters[i], + }).fromString(csvString) - // By default the csvtojson library casts empty values as empty strings. This - // is causing issues on conversion. ignoreEmpty will remove the key completly - // if empty, so creating this empty object will ensure we return the values - // with the keys but empty values - const result = await csv({ - ignoreEmpty: false, - delimiter: possibleDelimeters[i], - }).fromString(csvString) - for (const [i, r] of result.entries()) { - const columns = Object.keys(r) - if (numOfHeaders == null) { - numOfHeaders = columns.length + // By default the csvtojson library casts empty values as empty strings. This + // is causing issues on conversion. ignoreEmpty will remove the key completly + // if empty, so creating this empty object will ensure we return the values + // with the keys but empty values + const result = await csv({ + ignoreEmpty: false, + delimiter: possibleDelimeters[i], + }).fromString(csvString) + for (const [i, r] of result.entries()) { + // The purpose of this is to find rows that have been split + // into the wrong number of columns - Any valid .CSV file will have + // the same number of colums in each row + // If the number of columms in each row is different to + // the number of headers, this isn't the right delimiter + const columns = Object.keys(r) + if (numOfHeaders == null) { + numOfHeaders = columns.length + } + if (numOfHeaders === 1 || numOfHeaders !== columns.length) { + headerMismatch = true + break + } + for (const [key] of Object.entries(r).filter( + ([, value]) => value === "" + )) { + if (castedWithEmptyValues[i][key] === undefined) { + r[key] = null + } + } } - if (numOfHeaders !== columns.length) { - headerMismatch = true - break + if (headerMismatch) { + continue + } else { + return result } - for (const [key] of Object.entries(r).filter( - ([, value]) => value === "" - )) { - // if (castedWithEmptyValues[i][key] === undefined) { - // r[key] = null - // } - } - } - if (headerMismatch) { + } catch (err) { + // Splitting on the wrong delimiter sometimes throws CSV parsing error + // (eg unterminated strings), which tells us we've picked the wrong delimiter continue - } else { - return result } } throw new Error("Unable to determine delimiter") From 6a94aada1932b5dc350bc2f6db0205ff161cbb8e Mon Sep 17 00:00:00 2001 From: mikesealey Date: Wed, 29 Jan 2025 17:52:14 +0000 Subject: [PATCH 04/11] adds more robust testing for complex cases --- packages/server/src/utilities/csv.ts | 22 +++++--------- .../server/src/utilities/tests/csv.spec.ts | 30 +++++++++++++++++++ 2 files changed, 38 insertions(+), 14 deletions(-) diff --git a/packages/server/src/utilities/csv.ts b/packages/server/src/utilities/csv.ts index 1e139f36fa..880a078a2a 100644 --- a/packages/server/src/utilities/csv.ts +++ b/packages/server/src/utilities/csv.ts @@ -4,15 +4,10 @@ export async function jsonFromCsvString(csvString: string) { const possibleDelimeters = [",", ";", ":", "|", "~", "\t", " "] for (let i = 0; i < possibleDelimeters.length; i++) { - let numOfHeaders: number | undefined = undefined + let headers: string[] | undefined = undefined let headerMismatch = false try { - const castedWithEmptyValues = await csv({ - ignoreEmpty: true, - delimiter: possibleDelimeters[i], - }).fromString(csvString) - // By default the csvtojson library casts empty values as empty strings. This // is causing issues on conversion. ignoreEmpty will remove the key completly // if empty, so creating this empty object will ensure we return the values @@ -28,18 +23,17 @@ export async function jsonFromCsvString(csvString: string) { // If the number of columms in each row is different to // the number of headers, this isn't the right delimiter const columns = Object.keys(r) - if (numOfHeaders == null) { - numOfHeaders = columns.length + if (headers == null) { + headers = columns } - if (numOfHeaders === 1 || numOfHeaders !== columns.length) { + if (headers.length === 1 || headers.length !== columns.length) { headerMismatch = true break } - for (const [key] of Object.entries(r).filter( - ([, value]) => value === "" - )) { - if (castedWithEmptyValues[i][key] === undefined) { - r[key] = null + + for (const header of headers) { + if (r[header] === undefined || r[header] === "") { + r[header] = null } } } diff --git a/packages/server/src/utilities/tests/csv.spec.ts b/packages/server/src/utilities/tests/csv.spec.ts index 14063d0e8e..85796307f9 100644 --- a/packages/server/src/utilities/tests/csv.spec.ts +++ b/packages/server/src/utilities/tests/csv.spec.ts @@ -1,3 +1,4 @@ +import { delimiter } from "path" import { jsonFromCsvString } from "../csv" describe("csv", () => { @@ -29,5 +30,34 @@ describe("csv", () => { expect(Object.keys(r)).toEqual(["id", "optional", "title"]) ) }) + + const possibleDelimeters = [",", ";", ":", "|", "~", "\t", " "] + + const csvArray = [ + ["id", "title"], + ["1", "aaa"], + ["2", "bbb"], + ["3", "c ccc"], + ["", ""], + [":5", "eee5:e"], + ] + + test.each(possibleDelimeters)( + "Should parse with delimiter %s", + async delimiter => { + const csvString = csvArray + .map(row => row.map(col => `"${col}"`).join(delimiter)) + .join("\n") + const result = await jsonFromCsvString(csvString) + + expect(result).toEqual([ + { id: "1", title: "aaa" }, + { id: "2", title: "bbb" }, + { id: "3", title: "c ccc" }, + { id: null, title: null }, + { id: ":5", title: "eee5:e" }, + ]) + } + ) }) }) From 690f1a6ae6ba26e04c625991326efdb4f3ebf85b Mon Sep 17 00:00:00 2001 From: mikesealey Date: Wed, 29 Jan 2025 18:28:24 +0000 Subject: [PATCH 05/11] addresses lint warnings --- packages/server/src/utilities/csv.ts | 2 +- packages/server/src/utilities/tests/csv.spec.ts | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/server/src/utilities/csv.ts b/packages/server/src/utilities/csv.ts index 880a078a2a..eb42d118ca 100644 --- a/packages/server/src/utilities/csv.ts +++ b/packages/server/src/utilities/csv.ts @@ -16,7 +16,7 @@ export async function jsonFromCsvString(csvString: string) { ignoreEmpty: false, delimiter: possibleDelimeters[i], }).fromString(csvString) - for (const [i, r] of result.entries()) { + for (const [, r] of result.entries()) { // The purpose of this is to find rows that have been split // into the wrong number of columns - Any valid .CSV file will have // the same number of colums in each row diff --git a/packages/server/src/utilities/tests/csv.spec.ts b/packages/server/src/utilities/tests/csv.spec.ts index 85796307f9..b1dc192bf0 100644 --- a/packages/server/src/utilities/tests/csv.spec.ts +++ b/packages/server/src/utilities/tests/csv.spec.ts @@ -1,4 +1,3 @@ -import { delimiter } from "path" import { jsonFromCsvString } from "../csv" describe("csv", () => { From aace287ded0e17dbb45a5af9c91b182742d74f64 Mon Sep 17 00:00:00 2001 From: mikesealey Date: Fri, 31 Jan 2025 10:40:03 +0000 Subject: [PATCH 06/11] fixes spelling of delimiters --- packages/server/src/utilities/csv.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/server/src/utilities/csv.ts b/packages/server/src/utilities/csv.ts index eb42d118ca..d87d327f7d 100644 --- a/packages/server/src/utilities/csv.ts +++ b/packages/server/src/utilities/csv.ts @@ -1,9 +1,9 @@ import csv from "csvtojson" export async function jsonFromCsvString(csvString: string) { - const possibleDelimeters = [",", ";", ":", "|", "~", "\t", " "] + const possibleDelimiters = [",", ";", ":", "|", "~", "\t", " "] - for (let i = 0; i < possibleDelimeters.length; i++) { + for (let i = 0; i < possibleDelimiters.length; i++) { let headers: string[] | undefined = undefined let headerMismatch = false @@ -14,7 +14,7 @@ export async function jsonFromCsvString(csvString: string) { // with the keys but empty values const result = await csv({ ignoreEmpty: false, - delimiter: possibleDelimeters[i], + delimiter: possibleDelimiters[i], }).fromString(csvString) for (const [, r] of result.entries()) { // The purpose of this is to find rows that have been split From f6b6ac78102c767fd437fdd615215b0ea19a9f95 Mon Sep 17 00:00:00 2001 From: mikesealey Date: Fri, 31 Jan 2025 10:51:22 +0000 Subject: [PATCH 07/11] more descriptive variable name for 'row' --- packages/server/src/utilities/csv.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/server/src/utilities/csv.ts b/packages/server/src/utilities/csv.ts index d87d327f7d..ac9304b12a 100644 --- a/packages/server/src/utilities/csv.ts +++ b/packages/server/src/utilities/csv.ts @@ -16,13 +16,13 @@ export async function jsonFromCsvString(csvString: string) { ignoreEmpty: false, delimiter: possibleDelimiters[i], }).fromString(csvString) - for (const [, r] of result.entries()) { + for (const [, row] of result.entries()) { // The purpose of this is to find rows that have been split // into the wrong number of columns - Any valid .CSV file will have // the same number of colums in each row // If the number of columms in each row is different to // the number of headers, this isn't the right delimiter - const columns = Object.keys(r) + const columns = Object.keys(row) if (headers == null) { headers = columns } @@ -32,8 +32,8 @@ export async function jsonFromCsvString(csvString: string) { } for (const header of headers) { - if (r[header] === undefined || r[header] === "") { - r[header] = null + if (row[header] === undefined || row[header] === "") { + row[header] = null } } } From 2d561f124d1fa0489f89381c693b64a70a440a4f Mon Sep 17 00:00:00 2001 From: Martin McKeaveney Date: Fri, 31 Jan 2025 16:26:39 +0000 Subject: [PATCH 08/11] clear results context on every run --- packages/server/src/jsRunner/vm/isolated-vm.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/server/src/jsRunner/vm/isolated-vm.ts b/packages/server/src/jsRunner/vm/isolated-vm.ts index 37ee048dc2..b47137bcba 100644 --- a/packages/server/src/jsRunner/vm/isolated-vm.ts +++ b/packages/server/src/jsRunner/vm/isolated-vm.ts @@ -186,6 +186,7 @@ export class IsolatedVM implements VM { code = ` try { + results = {} results['${this.runResultKey}']=${this.codeWrapper(code)} } catch (e) { results['${this.runErrorKey}']=e From 6999bdf57ee3fdef93fab5813f9567f9cb04fd9c Mon Sep 17 00:00:00 2001 From: Martin McKeaveney Date: Fri, 31 Jan 2025 16:58:29 +0000 Subject: [PATCH 09/11] add test for clearing results --- packages/server/src/jsRunner/tests/isolatedVM.spec.ts | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/packages/server/src/jsRunner/tests/isolatedVM.spec.ts b/packages/server/src/jsRunner/tests/isolatedVM.spec.ts index 5a9bc05d76..2cf97c8dd2 100644 --- a/packages/server/src/jsRunner/tests/isolatedVM.spec.ts +++ b/packages/server/src/jsRunner/tests/isolatedVM.spec.ts @@ -107,4 +107,14 @@ describe("Test isolated vm directly", () => { ) expect(result).toEqual([]) }) + + it("should ensure error results are cleared between runs", () => { + const context = {} + // throw error + const result = runJSWithIsolatedVM(`test.foo.bar = 123`, context) + expect(result).toEqual({}) + // ensure error not persisted across vms + const secondResult = runJSWithIsolatedVM(`return {}`, context) + expect(secondResult).toEqual({}) + }) }) From b0e38be8447f5a123c5faa484074a0d821a85b9f Mon Sep 17 00:00:00 2001 From: Martin McKeaveney Date: Fri, 31 Jan 2025 17:23:36 +0000 Subject: [PATCH 10/11] expect to throw --- packages/server/src/jsRunner/tests/isolatedVM.spec.ts | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/packages/server/src/jsRunner/tests/isolatedVM.spec.ts b/packages/server/src/jsRunner/tests/isolatedVM.spec.ts index 2cf97c8dd2..9bf79c6040 100644 --- a/packages/server/src/jsRunner/tests/isolatedVM.spec.ts +++ b/packages/server/src/jsRunner/tests/isolatedVM.spec.ts @@ -111,10 +111,11 @@ describe("Test isolated vm directly", () => { it("should ensure error results are cleared between runs", () => { const context = {} // throw error - const result = runJSWithIsolatedVM(`test.foo.bar = 123`, context) - expect(result).toEqual({}) - // ensure error not persisted across vms - const secondResult = runJSWithIsolatedVM(`return {}`, context) + // Ensure the first execution throws an error + expect(() => runJSWithIsolatedVM(`test.foo.bar = 123`, context)).toThrow(); + + // Ensure the error is not persisted across VMs + const secondResult = runJSWithIsolatedVM(`return {}`, context); expect(secondResult).toEqual({}) }) }) From 4736c8f5c7cf567792c31930ff06a548654ade86 Mon Sep 17 00:00:00 2001 From: Martin McKeaveney Date: Fri, 31 Jan 2025 18:02:35 +0000 Subject: [PATCH 11/11] lint --- packages/server/src/jsRunner/tests/isolatedVM.spec.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/server/src/jsRunner/tests/isolatedVM.spec.ts b/packages/server/src/jsRunner/tests/isolatedVM.spec.ts index 9bf79c6040..63a1a52b09 100644 --- a/packages/server/src/jsRunner/tests/isolatedVM.spec.ts +++ b/packages/server/src/jsRunner/tests/isolatedVM.spec.ts @@ -112,10 +112,10 @@ describe("Test isolated vm directly", () => { const context = {} // throw error // Ensure the first execution throws an error - expect(() => runJSWithIsolatedVM(`test.foo.bar = 123`, context)).toThrow(); + expect(() => runJSWithIsolatedVM(`test.foo.bar = 123`, context)).toThrow() // Ensure the error is not persisted across VMs - const secondResult = runJSWithIsolatedVM(`return {}`, context); + const secondResult = runJSWithIsolatedVM(`return {}`, context) expect(secondResult).toEqual({}) }) })