This is an automated email from the ASF dual-hosted git repository. ggregory pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/commons-csv.git
The following commit(s) were added to refs/heads/master by this push: new 0596491 CSV-247: CSVParser to check an empty header before checking duplicates. (#47) 0596491 is described below commit 059649167c5045dfd40ebf0f97def33a9235c3d1 Author: Alex Herbert <aherb...@apache.org> AuthorDate: Mon Oct 7 18:06:11 2019 +0100 CSV-247: CSVParser to check an empty header before checking duplicates. (#47) This updates the issues test for CSV-247 and adds tests to the CSVParserTest. --- .../java/org/apache/commons/csv/CSVParser.java | 23 ++++++++------- .../checkstyle/checkstyle-suppressions.xml | 2 +- .../java/org/apache/commons/csv/CSVParserTest.java | 14 ++++++--- .../apache/commons/csv/issues/JiraCsv247Test.java | 33 ++++++++++++++++------ 4 files changed, 47 insertions(+), 25 deletions(-) diff --git a/src/main/java/org/apache/commons/csv/CSVParser.java b/src/main/java/org/apache/commons/csv/CSVParser.java index 3b3b3e7..3803d96 100644 --- a/src/main/java/org/apache/commons/csv/CSVParser.java +++ b/src/main/java/org/apache/commons/csv/CSVParser.java @@ -495,19 +495,18 @@ public final class CSVParser implements Iterable<CSVRecord>, Closeable { if (headerRecord != null) { for (int i = 0; i < headerRecord.length; i++) { final String header = headerRecord[i]; - final boolean containsHeader = header != null && hdrMap.containsKey(header); final boolean emptyHeader = header == null || header.trim().isEmpty(); - if (containsHeader) { - if (!emptyHeader && !this.format.getAllowDuplicateHeaderNames()) { - throw new IllegalArgumentException( - String.format( - "The header contains a duplicate name: \"%s\" in %s. If this is valid then use CSVFormat.withAllowDuplicateHeaderNames().", - header, Arrays.toString(headerRecord))); - } - if (emptyHeader && !this.format.getAllowMissingColumnNames()) { - throw new IllegalArgumentException( - "A header name is missing in " + Arrays.toString(headerRecord)); - } + if (emptyHeader && !this.format.getAllowMissingColumnNames()) { + throw new IllegalArgumentException( + "A header name is missing in " + Arrays.toString(headerRecord)); + } + // Note: This will always allow a duplicate header if the header is empty + final boolean containsHeader = header != null && hdrMap.containsKey(header); + if (containsHeader && !emptyHeader && !this.format.getAllowDuplicateHeaderNames()) { + throw new IllegalArgumentException( + String.format( + "The header contains a duplicate name: \"%s\" in %s. If this is valid then use CSVFormat.withAllowDuplicateHeaderNames().", + header, Arrays.toString(headerRecord))); } if (header != null) { hdrMap.put(header, Integer.valueOf(i)); diff --git a/src/main/resources/checkstyle/checkstyle-suppressions.xml b/src/main/resources/checkstyle/checkstyle-suppressions.xml index edc6783..d6b6e9c 100644 --- a/src/main/resources/checkstyle/checkstyle-suppressions.xml +++ b/src/main/resources/checkstyle/checkstyle-suppressions.xml @@ -19,5 +19,5 @@ "-//Checkstyle//DTD SuppressionFilter Configuration 1.2//EN" "https://checkstyle.org/dtds/suppressions_1_2.dtd"> <suppressions> - <suppress checks="LineLength" files="[\\/]CSVParser\.java$" lines="504"/> + <suppress checks="LineLength" files="[\\/]CSVParser\.java$" lines="508"/> </suppressions> diff --git a/src/test/java/org/apache/commons/csv/CSVParserTest.java b/src/test/java/org/apache/commons/csv/CSVParserTest.java index eeb3526..310574a 100644 --- a/src/test/java/org/apache/commons/csv/CSVParserTest.java +++ b/src/test/java/org/apache/commons/csv/CSVParserTest.java @@ -688,7 +688,7 @@ public class CSVParserTest { public void testHeaderMissing() throws Exception { final Reader in = new StringReader("a,,c\n1,2,3\nx,y,z"); - final Iterator<CSVRecord> records = CSVFormat.DEFAULT.withHeader().parse(in).iterator(); + final Iterator<CSVRecord> records = CSVFormat.DEFAULT.withHeader().withAllowMissingColumnNames().parse(in).iterator(); for (int i = 0; i < 2; i++) { assertTrue(records.hasNext()); @@ -702,23 +702,29 @@ public class CSVParserTest { @Test public void testHeaderMissingWithNull() throws Exception { - final Reader in = new StringReader("a,,c,,d\n1,2,3,4\nx,y,z,zz"); + final Reader in = new StringReader("a,,c,,e\n1,2,3,4,5\nv,w,x,y,z"); CSVFormat.DEFAULT.withHeader().withNullString("").withAllowMissingColumnNames().parse(in).iterator(); } @Test public void testHeadersMissing() throws Exception { - final Reader in = new StringReader("a,,c,,d\n1,2,3,4\nx,y,z,zz"); + final Reader in = new StringReader("a,,c,,e\n1,2,3,4,5\nv,w,x,y,z"); CSVFormat.DEFAULT.withHeader().withAllowMissingColumnNames().parse(in).iterator(); } @Test public void testHeadersMissingException() { - final Reader in = new StringReader("a,,c,,d\n1,2,3,4\nx,y,z,zz"); + final Reader in = new StringReader("a,,c,,e\n1,2,3,4,5\nv,w,x,y,z"); assertThrows(IllegalArgumentException.class, () -> CSVFormat.DEFAULT.withHeader().parse(in).iterator()); } @Test + public void testHeadersMissingOneColumnException() throws Exception { + final Reader in = new StringReader("a,,c,d,e\n1,2,3,4,5\nv,w,x,y,z"); + assertThrows(IllegalArgumentException.class, () -> CSVFormat.DEFAULT.withHeader().parse(in).iterator()); + } + + @Test public void testIgnoreCaseHeaderMapping() throws Exception { final Reader reader = new StringReader("1,2,3"); final Iterator<CSVRecord> records = CSVFormat.DEFAULT.withHeader("One", "TWO", "three").withIgnoreHeaderCase() diff --git a/src/test/java/org/apache/commons/csv/issues/JiraCsv247Test.java b/src/test/java/org/apache/commons/csv/issues/JiraCsv247Test.java index 129c3a4..daa66be 100644 --- a/src/test/java/org/apache/commons/csv/issues/JiraCsv247Test.java +++ b/src/test/java/org/apache/commons/csv/issues/JiraCsv247Test.java @@ -19,6 +19,9 @@ package org.apache.commons.csv.issues; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + import java.io.Reader; import java.io.StringReader; import java.util.Arrays; @@ -32,9 +35,14 @@ import org.junit.jupiter.api.Test; public class JiraCsv247Test { @Test - public void testHeadersMissingOneColumn() throws Exception { + public void testHeadersMissingOneColumnWhenAllowingMissingColumnNames() throws Exception { + final CSVFormat format = CSVFormat.DEFAULT.withHeader().withAllowMissingColumnNames(true); + + assertTrue(format.getAllowMissingColumnNames(), + "We should allow missing column names"); + final Reader in = new StringReader("a,,c,d,e\n1,2,3,4,5\nv,w,x,y,z"); - try (final CSVParser parser = CSVFormat.DEFAULT.withHeader().parse(in)) { + try (final CSVParser parser = format.parse(in)) { assertEquals(Arrays.asList("a", "", "c", "d", "e"), parser.getHeaderNames()); final Iterator<CSVRecord> iterator = parser.iterator(); CSVRecord record = iterator.next(); @@ -54,11 +62,20 @@ public class JiraCsv247Test { } @Test - public void testJiraDescription() throws Exception { - final Reader in = new StringReader("a,,c,d\n1,2,3,4\nx,y,z,zz"); - try (final CSVParser parser = CSVFormat.DEFAULT.withHeader().parse(in)) { - parser.iterator(); - } - } + public void testHeadersMissingThrowsWhenNotAllowingMissingColumnNames() throws Exception { + final CSVFormat format = CSVFormat.DEFAULT.withHeader(); + + assertFalse(format.getAllowMissingColumnNames(), + "By default we should not allow missing column names"); + assertThrows(IllegalArgumentException.class, () -> { + final Reader in = new StringReader("a,,c,d,e\n1,2,3,4,5\nv,w,x,y,z"); + format.parse(in); + }, "1 missing column header is not allowed"); + + assertThrows(IllegalArgumentException.class, () -> { + final Reader in = new StringReader("a,,c,d,\n1,2,3,4,5\nv,w,x,y,z"); + format.parse(in); + }, "2+ missing column headers is not allowed!"); + } } \ No newline at end of file