CSV-247: CSVParser to check an empty header before checking duplicates. (apache#47)

This updates the issues test for CSV-247 and adds tests to the
CSVParserTest.

Author: aherbert

src/main/java/org/apache/commons/csv/CSVParser.java

@@ -495,19 +495,18 @@ private Headers createHeaders() throws IOException {
             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));

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>

src/test/java/org/apache/commons/csv/CSVParserTest.java

@@ -688,7 +688,7 @@ public void testHeaderComment() throws Exception {
     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,22 +702,28 @@ public void testHeaderMissing() throws Exception {
 
     @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");

src/test/java/org/apache/commons/csv/issues/JiraCsv247Test.java

@@ -19,6 +19,9 @@
 
 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 @@
 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 @@ record = iterator.next();
     }
 
     @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!");
+    }
 }