commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From sebb <seb...@gmail.com>
Subject Re: svn commit: r1602206 - in /commons/proper/csv/trunk/src: changes/changes.xml main/java/org/apache/commons/csv/CSVFormat.java main/java/org/apache/commons/csv/CSVParser.java test/java/org/apache/commons/csv/CSVParserTest.java
Date Thu, 12 Jun 2014 15:58:21 GMT
On 12 June 2014 16:38,  <ggregory@apache.org> wrote:
> Author: ggregory
> Date: Thu Jun 12 15:38:24 2014
> New Revision: 1602206
>
> URL: http://svn.apache.org/r1602206
> Log:
> [CSV-121] Exception that the header contains duplicate names when the column names are
empty. Added the setting ignoreEmptyHeaders, defaults to false to keep the IAE as the default
behavior.
>
> Modified:
>     commons/proper/csv/trunk/src/changes/changes.xml
>     commons/proper/csv/trunk/src/main/java/org/apache/commons/csv/CSVFormat.java
>     commons/proper/csv/trunk/src/main/java/org/apache/commons/csv/CSVParser.java
>     commons/proper/csv/trunk/src/test/java/org/apache/commons/csv/CSVParserTest.java
>
> Modified: commons/proper/csv/trunk/src/changes/changes.xml
> URL: http://svn.apache.org/viewvc/commons/proper/csv/trunk/src/changes/changes.xml?rev=1602206&r1=1602205&r2=1602206&view=diff
> ==============================================================================
> --- commons/proper/csv/trunk/src/changes/changes.xml (original)
> +++ commons/proper/csv/trunk/src/changes/changes.xml Thu Jun 12 15:38:24 2014
> @@ -40,6 +40,7 @@
>    <body>
>
>      <release version="1.0" date="TBD" description="First release">
> +      <action issue="CSV-121" type="add" dev="ggregory" due-to="Sebastian Hardt">IllegalArgumentException
thrown when the header contains duplicate names when the column names are empty.</action>
>        <action issue="CSV-120" type="add" dev="ggregory" due-to="Sergei Lebedev">CSVFormat#withHeader
doesn't work with CSVPrinter</action>
>        <action issue="CSV-119" type="add" dev="ggregory" due-to="Sergei Lebedev">CSVFormat
is missing a print(...) method</action>
>        <action issue="CSV-118" type="fix" dev="ggregory" due-to="Enrique Lara">CSVRecord.toMap()
throws NPE on formats with no
>
> Modified: commons/proper/csv/trunk/src/main/java/org/apache/commons/csv/CSVFormat.java
> URL: http://svn.apache.org/viewvc/commons/proper/csv/trunk/src/main/java/org/apache/commons/csv/CSVFormat.java?rev=1602206&r1=1602205&r2=1602206&view=diff
> ==============================================================================
> --- commons/proper/csv/trunk/src/main/java/org/apache/commons/csv/CSVFormat.java (original)
> +++ commons/proper/csv/trunk/src/main/java/org/apache/commons/csv/CSVFormat.java Thu
Jun 12 15:38:24 2014
> @@ -152,6 +152,7 @@ public final class CSVFormat implements
>      private final Character commentStart; // null if commenting is disabled
>      private final Character escape; // null if escaping is disabled
>      private final boolean ignoreSurroundingSpaces; // Should leading/trailing spaces
be ignored around values?
> +    private boolean ignoreEmptyHeaders;

Why is this not final?

>      private final boolean ignoreEmptyLines;
>      private final String recordSeparator; // for outputs
>      private final String nullString; // the string to be used for null values
> @@ -172,7 +173,7 @@ public final class CSVFormat implements
>       * </ul>
>       */
>      public static final CSVFormat DEFAULT = new CSVFormat(COMMA, DOUBLE_QUOTE_CHAR,
null, null, null,
> -                                                            false, true, CRLF, null,
null, false);
> +                                                            false, true, CRLF, null,
null, false, false);
>
>      /**
>       * Comma separated format as defined by <a href="http://tools.ietf.org/html/rfc4180">RFC
4180</a>.
> @@ -264,7 +265,7 @@ public final class CSVFormat implements
>       * @throws IllegalArgumentException if the delimiter is a line break character
>       */
>      public static CSVFormat newFormat(final char delimiter) {
> -        return new CSVFormat(delimiter, null, null, null, null, false, false, null,
null, null, false);
> +        return new CSVFormat(delimiter, null, null, null, null, false, false, null,
null, null, false, false);
>      }
>
>      /**
> @@ -291,13 +292,15 @@ public final class CSVFormat implements
>       * @param header
>       *            the header
>       * @param skipHeaderRecord TODO
> +     * @param ignoreEmptyHeaders TODO
>       * @throws IllegalArgumentException if the delimiter is a line break character
>       */
>      private CSVFormat(final char delimiter, final Character quoteChar,
>              final Quote quotePolicy, final Character commentStart,
>              final Character escape, final boolean ignoreSurroundingSpaces,
>              final boolean ignoreEmptyLines, final String recordSeparator,
> -            final String nullString, final String[] header, final boolean skipHeaderRecord)
{
> +            final String nullString, final String[] header, final boolean skipHeaderRecord,
> +            final boolean ignoreEmptyHeaders) {
>          if (isLineBreak(delimiter)) {
>              throw new IllegalArgumentException("The delimiter cannot be a line break");
>          }
> @@ -307,6 +310,7 @@ public final class CSVFormat implements
>          this.commentStart = commentStart;
>          this.escape = escape;
>          this.ignoreSurroundingSpaces = ignoreSurroundingSpaces;
> +        this.ignoreEmptyHeaders = ignoreEmptyHeaders;
>          this.ignoreEmptyLines = ignoreEmptyLines;
>          this.recordSeparator = recordSeparator;
>          this.nullString = nullString;
> @@ -448,6 +452,16 @@ public final class CSVFormat implements
>      }
>
>      /**
> +     * Specifies whether empty headers are ignored when parsing the header line.
> +     *
> +     * @return <tt>true</tt> if headers are ignored when parsing the header
line, <tt>false</tt> to throw an
> +     *         {@link IllegalArgumentException}..
> +     */
> +    public boolean getIgnoreEmptyHeaders() {
> +        return ignoreEmptyHeaders;
> +    }
> +
> +    /**
>       * Specifies whether empty lines between records are ignored when parsing input.
>       *
>       * @return <tt>true</tt> if empty lines between records are ignored,
<tt>false</tt> if they are turned into empty
> @@ -718,7 +732,8 @@ public final class CSVFormat implements
>              throw new IllegalArgumentException("The comment start character cannot be
a line break");
>          }
>          return new CSVFormat(delimiter, quoteChar, quotePolicy, commentStart, escape,
> -                ignoreSurroundingSpaces, ignoreEmptyLines, recordSeparator, nullString,
header, skipHeaderRecord);
> +                ignoreSurroundingSpaces, ignoreEmptyLines, recordSeparator, nullString,
header, skipHeaderRecord,
> +                ignoreEmptyHeaders);
>      }
>
>      /**
> @@ -735,7 +750,8 @@ public final class CSVFormat implements
>              throw new IllegalArgumentException("The delimiter cannot be a line break");
>          }
>          return new CSVFormat(delimiter, quoteChar, quotePolicy, commentStart, escape,
> -                ignoreSurroundingSpaces, ignoreEmptyLines, recordSeparator, nullString,
header, skipHeaderRecord);
> +                ignoreSurroundingSpaces, ignoreEmptyLines, recordSeparator, nullString,
header, skipHeaderRecord,
> +                ignoreEmptyHeaders);
>      }
>
>      /**
> @@ -765,7 +781,8 @@ public final class CSVFormat implements
>              throw new IllegalArgumentException("The escape character cannot be a line
break");
>          }
>          return new CSVFormat(delimiter, quoteChar, quotePolicy, commentStart, escape,
> -                ignoreSurroundingSpaces, ignoreEmptyLines, recordSeparator, nullString,
header, skipHeaderRecord);
> +                ignoreSurroundingSpaces, ignoreEmptyLines, recordSeparator, nullString,
header, skipHeaderRecord,
> +                ignoreEmptyHeaders);
>      }
>
>      /**
> @@ -787,7 +804,22 @@ public final class CSVFormat implements
>       */
>      public CSVFormat withHeader(final String... header) {
>          return new CSVFormat(delimiter, quoteChar, quotePolicy, commentStart, escape,
> -                ignoreSurroundingSpaces, ignoreEmptyLines, recordSeparator, nullString,
header, skipHeaderRecord);
> +                ignoreSurroundingSpaces, ignoreEmptyLines, recordSeparator, nullString,
header, skipHeaderRecord,
> +                ignoreEmptyHeaders);
> +    }
> +
> +    /**
> +     * Sets the empty header behavior of the format.
> +     *
> +     * @param ignoreEmptyHeaders
> +     *            the empty header behavior, <tt>true</tt> to ignore empty
headers in the header line,
> +     *            <tt>false</tt> to cause an {@link IllegalArgumentException}
to be thrown.
> +     * @return A new CSVFormat that is equal to this but with the specified empty header
behavior.
> +     */
> +    public CSVFormat withIgnoreEmptyHeaders(final boolean ignoreEmptyHeaders) {
> +        return new CSVFormat(delimiter, quoteChar, quotePolicy, commentStart, escape,
> +                ignoreSurroundingSpaces, ignoreEmptyHeaders, recordSeparator, nullString,
header, skipHeaderRecord,
> +                ignoreEmptyHeaders);
>      }
>
>      /**
> @@ -800,7 +832,8 @@ public final class CSVFormat implements
>       */
>      public CSVFormat withIgnoreEmptyLines(final boolean ignoreEmptyLines) {
>          return new CSVFormat(delimiter, quoteChar, quotePolicy, commentStart, escape,
> -                ignoreSurroundingSpaces, ignoreEmptyLines, recordSeparator, nullString,
header, skipHeaderRecord);
> +                ignoreSurroundingSpaces, ignoreEmptyLines, recordSeparator, nullString,
header, skipHeaderRecord,
> +                ignoreEmptyHeaders);
>      }
>
>      /**
> @@ -813,7 +846,8 @@ public final class CSVFormat implements
>       */
>      public CSVFormat withIgnoreSurroundingSpaces(final boolean ignoreSurroundingSpaces)
{
>          return new CSVFormat(delimiter, quoteChar, quotePolicy, commentStart, escape,
> -                ignoreSurroundingSpaces, ignoreEmptyLines, recordSeparator, nullString,
header, skipHeaderRecord);
> +                ignoreSurroundingSpaces, ignoreEmptyLines, recordSeparator, nullString,
header, skipHeaderRecord,
> +                ignoreEmptyHeaders);
>      }
>
>      /**
> @@ -833,7 +867,8 @@ public final class CSVFormat implements
>       */
>      public CSVFormat withNullString(final String nullString) {
>          return new CSVFormat(delimiter, quoteChar, quotePolicy, commentStart, escape,
> -                ignoreSurroundingSpaces, ignoreEmptyLines, recordSeparator, nullString,
header, skipHeaderRecord);
> +                ignoreSurroundingSpaces, ignoreEmptyLines, recordSeparator, nullString,
header, skipHeaderRecord,
> +                ignoreEmptyHeaders);
>      }
>
>      /**
> @@ -863,7 +898,8 @@ public final class CSVFormat implements
>              throw new IllegalArgumentException("The quoteChar cannot be a line break");
>          }
>          return new CSVFormat(delimiter, quoteChar, quotePolicy, commentStart, escape,
> -                ignoreSurroundingSpaces, ignoreEmptyLines, recordSeparator, nullString,
header, skipHeaderRecord);
> +                ignoreSurroundingSpaces, ignoreEmptyLines, recordSeparator, nullString,
header, skipHeaderRecord,
> +                ignoreEmptyHeaders);
>      }
>
>      /**
> @@ -876,7 +912,8 @@ public final class CSVFormat implements
>       */
>      public CSVFormat withQuotePolicy(final Quote quotePolicy) {
>          return new CSVFormat(delimiter, quoteChar, quotePolicy, commentStart, escape,
> -                ignoreSurroundingSpaces, ignoreEmptyLines, recordSeparator, nullString,
header, skipHeaderRecord);
> +                ignoreSurroundingSpaces, ignoreEmptyLines, recordSeparator, nullString,
header, skipHeaderRecord,
> +                ignoreEmptyHeaders);
>      }
>
>      /**
> @@ -901,7 +938,8 @@ public final class CSVFormat implements
>       */
>      public CSVFormat withRecordSeparator(final String recordSeparator) {
>          return new CSVFormat(delimiter, quoteChar, quotePolicy, commentStart, escape,
> -                ignoreSurroundingSpaces, ignoreEmptyLines, recordSeparator, nullString,
header, skipHeaderRecord);
> +                ignoreSurroundingSpaces, ignoreEmptyLines, recordSeparator, nullString,
header, skipHeaderRecord,
> +                ignoreEmptyHeaders);
>      }
>
>      /**
> @@ -915,6 +953,7 @@ public final class CSVFormat implements
>       */
>      public CSVFormat withSkipHeaderRecord(final boolean skipHeaderRecord) {
>          return new CSVFormat(delimiter, quoteChar, quotePolicy, commentStart, escape,
> -                ignoreSurroundingSpaces, ignoreEmptyLines, recordSeparator, nullString,
header, skipHeaderRecord);
> +                ignoreSurroundingSpaces, ignoreEmptyLines, recordSeparator, nullString,
header, skipHeaderRecord,
> +                ignoreEmptyHeaders);
>      }
>  }
>
> Modified: commons/proper/csv/trunk/src/main/java/org/apache/commons/csv/CSVParser.java
> URL: http://svn.apache.org/viewvc/commons/proper/csv/trunk/src/main/java/org/apache/commons/csv/CSVParser.java?rev=1602206&r1=1602205&r2=1602206&view=diff
> ==============================================================================
> --- commons/proper/csv/trunk/src/main/java/org/apache/commons/csv/CSVParser.java (original)
> +++ commons/proper/csv/trunk/src/main/java/org/apache/commons/csv/CSVParser.java Thu
Jun 12 15:38:24 2014
> @@ -360,28 +360,31 @@ public final class CSVParser implements
>          if (formatHeader != null) {
>              hdrMap = new LinkedHashMap<String, Integer>();
>
> -            String[] header = null;
> +            String[] headerRecord = null;

This is an independent change; please try to keep commits to a single change.

>              if (formatHeader.length == 0) {
>                  // read the header from the first line of the file
>                  final CSVRecord nextRecord = this.nextRecord();
>                  if (nextRecord != null) {
> -                    header = nextRecord.values();
> +                    headerRecord = nextRecord.values();
>                  }
>              } else {
>                  if (this.format.getSkipHeaderRecord()) {
>                      this.nextRecord();
>                  }
> -                header = formatHeader;
> +                headerRecord = formatHeader;
>              }
>
>              // build the name to index mappings
> -            if (header != null) {
> -                for (int i = 0; i < header.length; i++) {
> -                    if (hdrMap.containsKey(header[i])) {
> -                        throw new IllegalArgumentException("The header contains duplicate
names: " +
> -                                Arrays.toString(header));
> +            if (headerRecord != null) {
> +                for (int i = 0; i < headerRecord.length; i++) {
> +                    final String header = headerRecord[i];
> +                    final boolean containsHeader = hdrMap.containsKey(header);
> +                    final boolean emptyHeader = header.trim().isEmpty();
> +                    if (containsHeader && (!emptyHeader || (emptyHeader &&
!this.format.getIgnoreEmptyHeaders()))) {
> +                        throw new IllegalArgumentException("The header contains a duplicate
name: \"" + header
> +                                + "\" in " + Arrays.toString(headerRecord));
>                      }
> -                    hdrMap.put(header[i], Integer.valueOf(i));
> +                    hdrMap.put(header, Integer.valueOf(i));
>                  }
>              }
>          }
>
> Modified: commons/proper/csv/trunk/src/test/java/org/apache/commons/csv/CSVParserTest.java
> URL: http://svn.apache.org/viewvc/commons/proper/csv/trunk/src/test/java/org/apache/commons/csv/CSVParserTest.java?rev=1602206&r1=1602205&r2=1602206&view=diff
> ==============================================================================
> --- commons/proper/csv/trunk/src/test/java/org/apache/commons/csv/CSVParserTest.java
(original)
> +++ commons/proper/csv/trunk/src/test/java/org/apache/commons/csv/CSVParserTest.java
Thu Jun 12 15:38:24 2014
> @@ -493,7 +493,7 @@ public class CSVParserTest {
>      }
>
>      @Test(expected = IllegalArgumentException.class)
> -    public void testDuplicateHeaderEntries() throws Exception {
> +    public void testDuplicateHeaders() throws Exception {
>          CSVParser.parse("a,b,a\n1,2,3\nx,y,z", CSVFormat.DEFAULT.withHeader(new String[]{}));
>      }
>
> @@ -656,6 +656,12 @@ public class CSVParserTest {
>      }
>
>      @Test
> +    public void testHeadersMissing() throws Exception {
> +        final Reader in = new StringReader("a,,c,,d\n1,2,3,4\nx,y,z,zz");
> +        CSVFormat.DEFAULT.withHeader().withIgnoreEmptyHeaders(true).parse(in).iterator();
> +    }
> +
> +    @Test
>      public void testHeaderComment() throws Exception {
>          final Reader in = new StringReader("# comment\na,b,c\n1,2,3\nx,y,z");
>
>
>

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Mime
View raw message