commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Gary Gregory <garydgreg...@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 17:34:35 GMT
On Thu, Jun 12, 2014 at 11:58 AM, sebb <sebbaz@gmail.com> wrote:

> 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?
>

Because I forgot ;-) I was making small changes to keep it all compiling
and forgot to change it to final once I was done. In SVN.
Good catch, thank you!

Gary


>
> >      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
>
>


-- 
E-Mail: garydgregory@gmail.com | ggregory@apache.org
Java Persistence with Hibernate, Second Edition
<http://www.manning.com/bauer3/>
JUnit in Action, Second Edition <http://www.manning.com/tahchiev/>
Spring Batch in Action <http://www.manning.com/templier/>
Blog: http://garygregory.wordpress.com
Home: http://garygregory.com/
Tweet! http://twitter.com/GaryGregory

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message