Skip to content

Commit

Permalink
Fix for #317
Browse files Browse the repository at this point in the history
* [FIXED] Issue where invalid rows were not accounted for when skipRows was set #317
  • Loading branch information
doug-martin committed Feb 14, 2020
1 parent 205cb22 commit 9b1953b
Show file tree
Hide file tree
Showing 4 changed files with 60 additions and 3 deletions.
4 changes: 4 additions & 0 deletions History.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
# v4.0.3

* [FIXED] Issue where invalid rows were not accounted for when skipRows was set [#317](https://github.com/C2FO/fast-csv/issues/317)

# v4.0.2

* [ADDED] `writeHeaders` to `@fast-csv/format` option to prevent writing headers.
Expand Down
15 changes: 15 additions & 0 deletions packages/parse/__tests__/CsvParsingStream.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -369,6 +369,21 @@ describe('CsvParserStream', () => {
expected,
);
});

describe('strict column handling', () => {
it('should include the invalid rows when counting rows to skip', async () => {
const expectedRows = withHeadersAndMissingColumns.parsed;
await expectParsed(
parseContentAndCollect(withHeadersAndMissingColumns, {
headers: true,
strictColumnHandling: true,
skipRows: 2,
}),
expectedRows.slice(-1),
[],
);
});
});
});

describe('without headers', () => {
Expand Down
29 changes: 29 additions & 0 deletions packages/parse/__tests__/issues/issue317.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
import { EOL } from 'os';
import { parseString, RowMap, RowArray } from '../../src';

describe('Issue #317 - https://github.com/C2FO/fast-csv/issues/317', () => {
const CSV_CONTENT = ['header1,header2', 'col1', 'col2,col2', 'col3', 'col4,col4', 'col5,col5', 'col6,col6'].join(
EOL,
);
const expectedInvalidRows = [['col3']];
const expectedRows = [
{ header1: 'col4', header2: 'col4' },
{ header1: 'col5', header2: 'col5' },
{ header1: 'col6', header2: 'col6' },
];

it('skip trailing whitespace after a quoted field', done => {
const invalid: RowArray[] = [];
const rows: RowMap[] = [];
parseString(CSV_CONTENT, { headers: true, skipRows: 2, strictColumnHandling: true, maxRows: 4 })
.on('data-invalid', (row: RowArray) => invalid.push(row))
.on('data', (r: RowMap) => rows.push(r))
.on('error', done)
.on('end', (count: number) => {
expect(rows).toEqual(expectedRows);
expect(invalid).toEqual(expectedInvalidRows);
expect(count).toBe(expectedRows.length + invalid.length);
done();
});
});
});
15 changes: 12 additions & 3 deletions packages/parse/src/CsvParserStream.ts
Original file line number Diff line number Diff line change
Expand Up @@ -163,15 +163,18 @@ export class CsvParserStream<I extends Row, O extends Row> extends Transform {
return cb(new Error('Expected result from header transform'));
}
if (!withHeaders.isValid) {
return cb(null, { isValid: false, row: (parsedRow as never) as O });
if (this.shouldEmitRows) {
return cb(null, { isValid: false, row: (parsedRow as never) as O });
}
// skipped because of skipRows option remove from total row count
return this.skipRow(cb);
}
if (withHeaders.row) {
if (this.shouldEmitRows) {
return this.rowTransformerValidator.transformAndValidate(withHeaders.row, cb);
}
// skipped because of skipRows option remove from total row count
this.rowCount -= 1;
return cb(null, { row: null, isValid: true });
return this.skipRow(cb);
}
// this is a header row dont include in the rowCount or parsedRowCount
this.rowCount -= 1;
Expand All @@ -183,6 +186,12 @@ export class CsvParserStream<I extends Row, O extends Row> extends Transform {
}
}

private skipRow(cb: RowValidatorCallback<O>): void {
// skipped because of skipRows option remove from total row count
this.rowCount -= 1;
return cb(null, { row: null, isValid: true });
}

private pushRow(row: Row, cb: (err?: Error) => void): void {
try {
if (!this.parserOptions.objectMode) {
Expand Down

0 comments on commit 9b1953b

Please sign in to comment.