Skip to content

Commit c51b75d

Browse files
committed
merge: merge branch 'abe/Improve-perfs-of-CSV-parsing-for-Table-components-PROD-10196'
2 parents 0e1b8f5 + ba94fae commit c51b75d

File tree

4 files changed

+72
-33
lines changed

4 files changed

+72
-33
lines changed

src/FileUtils/AgGridUtils.js

Lines changed: 39 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
// Copyright (c) Cosmo Tech.
22
// Licensed under the MIT license.
3+
import DateUtils from '../DateUtils/DateUtils';
34
import { ValidationUtils } from '../ValidationUtils';
45
import { Error as PanelError } from '../models';
56
import CSV from './CSVUtils';
@@ -86,38 +87,50 @@ const _getColTypeFromTypeArray = (typeArray) => {
8687
};
8788

8889
const _validateFormat = (rows, hasHeader, cols, options) => {
89-
const colsData = cols.map((col) => ({ ...col, type: _getColTypeFromTypeArray(col.type) }));
9090
const errors = [];
91-
const knownColsCount = colsData.length;
91+
const knownColsCount = cols.length;
9292
const startIndex = hasHeader ? 1 : 0;
93+
94+
const colMeta = cols.map((col) => ({
95+
field: col.field,
96+
type: _getColTypeFromTypeArray(col.type),
97+
acceptsEmptyFields: col.acceptsEmptyFields ?? col.cellEditorParams?.acceptsEmptyFields ?? false,
98+
colOptions: {
99+
...options,
100+
enumValues: col.enumValues ?? col.cellEditorParams?.enumValues,
101+
minValue: col.minValue,
102+
maxValue: col.maxValue,
103+
},
104+
}));
105+
// For date columns, convert min & max values to do it only once
106+
colMeta.forEach((col) => {
107+
if (col.type === 'date' && options?.dateFormat) {
108+
const colOptions = col.colOptions;
109+
if (colOptions.minValue != null) colOptions.minDate = DateUtils.parse(colOptions.minValue, options?.dateFormat);
110+
if (colOptions.maxValue != null) colOptions.maxDate = DateUtils.parse(colOptions.maxValue, options?.dateFormat);
111+
}
112+
});
113+
93114
for (let rowIndex = startIndex; rowIndex < rows.length; rowIndex++) {
94115
const row = rows[rowIndex];
95116
while (row[row.length - 1] === undefined && row.length > knownColsCount) row.pop();
96-
if (row.length !== knownColsCount || row.includes(undefined))
97-
_forgeColumnsCountError(row, rowIndex + 1, colsData, errors);
98-
row.forEach((rowCell, colIndex) => {
99-
if (colIndex < knownColsCount) {
100-
const colType = colsData[colIndex].type;
101-
if (colType && rowCell !== undefined) {
102-
// use of cellEditorParams is deprecated
103-
const colOptions = {
104-
...options,
105-
enumValues: colsData[colIndex]?.enumValues ?? colsData[colIndex]?.cellEditorParams?.enumValues,
106-
minValue: colsData[colIndex]?.minValue,
107-
maxValue: colsData[colIndex]?.maxValue,
108-
};
109-
const acceptsEmptyFields =
110-
// use of cellEditorParams is deprecated
111-
colsData[colIndex].acceptsEmptyFields ?? colsData[colIndex].cellEditorParams?.acceptsEmptyFields ?? false;
112-
const validationResult = ValidationUtils.isValid(rowCell, colType, colOptions, acceptsEmptyFields);
113-
if (validationResult !== true) {
114-
const { summary: errorSummary, context: errorContext } = validationResult;
115-
const errorLoc = `Line ${rowIndex + 1}, Column ${colIndex + 1} ("${colsData[colIndex].field}")`;
116-
errors.push(new PanelError(errorSummary, errorLoc, errorContext));
117-
}
118-
}
117+
118+
if (row.length !== knownColsCount || row.includes(undefined)) {
119+
_forgeColumnsCountError(row, rowIndex + 1, cols, errors);
120+
}
121+
122+
for (let colIndex = 0; colIndex < knownColsCount; colIndex++) {
123+
const { type, colOptions, acceptsEmptyFields, field } = colMeta[colIndex];
124+
const value = row[colIndex];
125+
if (value === undefined) continue;
126+
127+
const validationResult = ValidationUtils.isValid(value, type, colOptions, acceptsEmptyFields);
128+
if (validationResult !== true) {
129+
const { summary: errorSummary, context: errorContext } = validationResult;
130+
const errorLoc = `Line ${rowIndex + 1}, Column ${colIndex + 1} ("${field}")`;
131+
errors.push(new PanelError(errorSummary, errorLoc, errorContext));
119132
}
120-
});
133+
}
121134
}
122135

123136
return errors;

src/FileUtils/__test__/AgGridUtils.spec.js

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,30 @@ describe('parse valid CSV strings', () => {
8383
});
8484
});
8585

86+
describe('regression: invalid number values should trigger validation errors', () => {
87+
const options = { dateFormat: 'dd/MM/yyyy' };
88+
89+
test('should raise error for bad integer value in age column', () => {
90+
const headerCols = AgGridUtils.getFlattenColumnsWithoutGroups(CUSTOMERS_COLS).map((col) => col.field);
91+
92+
const invalidRow = headerCols.map((field) => {
93+
if (field === 'age') return 'bad_int';
94+
if (field === 'birthday') return '03/05/1978';
95+
if (field === 'height') return '1.7';
96+
return '';
97+
});
98+
99+
const csvData = [headerCols, invalidRow];
100+
const csvStr = csvData.map((row) => row.join(',')).join('\n');
101+
102+
const result = AgGridUtils.fromCSV(csvStr, true, CUSTOMERS_COLS, options);
103+
104+
expect(result.error).toBeDefined();
105+
expect(result.error.length).toBeGreaterThan(0);
106+
expect(result.error[0].summary.toLowerCase()).toMatch(/(type|incorrect|int|empty)/);
107+
});
108+
});
109+
86110
describe('parse with invalid parameters', () => {
87111
test('missing fields definition', () => {
88112
const res = AgGridUtils.fromCSV('', false, undefined);

src/FileUtils/__test__/CustomersData.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ export const CUSTOMERS_COLS = [
7373
{
7474
headerName: 'identity',
7575
children: [
76-
{ field: 'birthday', type: ['date'], minValue: new Date('1900-01-01'), maxValue: new Date('2030-01-01') },
76+
{ field: 'birthday', type: ['date'], minValue: '01/01/1900', maxValue: '01/01/2030' },
7777
{ field: 'height', type: ['number'], minValue: 0, maxValue: 2.5 },
7878
],
7979
},
@@ -88,7 +88,7 @@ export const CUSTOMERS_COLS_DEPRECATED = [
8888
type: ['enum'],
8989
cellEditorParams: { enumValues: ['AppleJuice', 'Beer', 'OrangeJuice', 'Wine'] },
9090
},
91-
{ field: 'birthday', type: ['date'], minValue: new Date('1900-01-01'), maxValue: new Date('2030-01-01') },
91+
{ field: 'birthday', type: ['date'], minValue: '01/01/1900', maxValue: '01/01/2030' },
9292
{ field: 'height', type: ['number'], minValue: 0, maxValue: 2.5 },
9393
];
9494

src/ValidationUtils/ValidationUtils.js

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -69,9 +69,7 @@ const castToDate = (dateOrStrValue, dateFormat) => {
6969
return DateUtils.parse(dateOrStrValue, dateFormat);
7070
};
7171

72-
const isDateInRange = (value, minValue, maxValue, dateFormat) => {
73-
const minDate = castToDate(minValue, dateFormat);
74-
const maxDate = castToDate(maxValue, dateFormat);
72+
const isDateInRange = (value, minDate, maxDate, dateFormat) => {
7573
const format = DateUtils.format;
7674
if (value == null) return null;
7775
if (dateFormat == null) return forgeConfigError("Missing option dateFormat, can't perform date validation.");
@@ -95,9 +93,13 @@ const isValid = (dataStr, type, options, canBeEmpty = false) => {
9593
return isBool(dataStr) || forgeTypeError(dataStr, type, options);
9694
case 'date': {
9795
if (!options?.dateFormat) return forgeConfigError("Missing option dateFormat, can't perform date validation.");
98-
if (!isDate(dataStr, options?.dateFormat)) return forgeTypeError(dataStr, type, options);
96+
9997
const valueAsDate = DateUtils.parse(dataStr, options?.dateFormat);
100-
return isDateInRange(valueAsDate, options?.minValue, options?.maxValue, options?.dateFormat);
98+
if (isNaN(valueAsDate.getTime())) return forgeTypeError(dataStr, type, options); // Invalid date
99+
100+
const minDate = options?.minDate ?? castToDate(options?.minValue, options?.dateFormat);
101+
const maxDate = options?.maxDate ?? castToDate(options?.maxValue, options?.dateFormat);
102+
return isDateInRange(valueAsDate, minDate, maxDate, options?.dateFormat);
101103
}
102104
case 'enum':
103105
if (!options.enumValues) return forgeConfigError("Missing option enumValues, can't perform enum validation.");

0 commit comments

Comments
 (0)