Skip to content

Commit ef51c1e

Browse files
author
Gary Gregory
committed
[CSV-239] Cannot get headers in column order from CSVRecord.
- Redo so that the record tracks its source parser where call sites can find metadata. This avoids adding slots to the record class itself.
1 parent 9dcad06 commit ef51c1e

6 files changed

Lines changed: 121 additions & 81 deletions

File tree

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

Lines changed: 35 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -340,7 +340,7 @@ public static CSVParser parse(final URL url, final Charset charset, final CSVFor
340340
/** A mapping of column names to column indices */
341341
private final Map<String, Integer> headerMap;
342342

343-
/** Preserve the column order to avoid re-computing it. */
343+
/** The column order to avoid re-computing it. */
344344
private final List<String> headerNames;
345345

346346
private final Lexer lexer;
@@ -444,6 +444,12 @@ public void close() throws IOException {
444444
}
445445
}
446446

447+
private Map<String, Integer> createEmptyHeaderMap() {
448+
return this.format.getIgnoreHeaderCase() ?
449+
new TreeMap<>(String.CASE_INSENSITIVE_ORDER) :
450+
new TreeMap<>();
451+
}
452+
447453
/**
448454
* Creates the name to index mapping if the format defines a header.
449455
*
@@ -454,10 +460,7 @@ private Map<String, Integer> createHeaderMap() throws IOException {
454460
Map<String, Integer> hdrMap = null;
455461
final String[] formatHeader = this.format.getHeader();
456462
if (formatHeader != null) {
457-
hdrMap = this.format.getIgnoreHeaderCase() ?
458-
new TreeMap<>(String.CASE_INSENSITIVE_ORDER) :
459-
new TreeMap<>();
460-
463+
hdrMap = createEmptyHeaderMap();
461464
String[] headerRecord = null;
462465
if (formatHeader.length == 0) {
463466
// read the header from the first line of the file
@@ -523,7 +526,31 @@ public String getFirstEndOfLine() {
523526
* @return a copy of the header map.
524527
*/
525528
public Map<String, Integer> getHeaderMap() {
526-
return this.headerMap == null ? null : new LinkedHashMap<>(this.headerMap);
529+
if (this.headerMap == null) {
530+
return null;
531+
}
532+
final Map<String, Integer> map = createEmptyHeaderMap();
533+
map.putAll(this.headerMap);
534+
return map;
535+
}
536+
537+
/**
538+
* Returns the header map.
539+
*
540+
* @return the header map.
541+
*/
542+
Map<String, Integer> getHeaderMapRaw() {
543+
return this.headerMap;
544+
}
545+
546+
/**
547+
* Returns a copy of the header names that iterates in column order.
548+
*
549+
* @return a copy of the header names that iterates in column order.
550+
* @since 1.7
551+
*/
552+
public List<String> getHeaderNames() {
553+
return new ArrayList<>(headerNames);
527554
}
528555

529556
/**
@@ -633,8 +660,8 @@ CSVRecord nextRecord() throws IOException {
633660
if (!this.recordList.isEmpty()) {
634661
this.recordNumber++;
635662
final String comment = sb == null ? null : sb.toString();
636-
result = new CSVRecord(this.recordList.toArray(new String[this.recordList.size()]), this.headerMap,
637-
this.headerNames, comment, this.recordNumber, startCharPosition);
663+
result = new CSVRecord(this, this.recordList.toArray(new String[this.recordList.size()]),
664+
comment, this.recordNumber, startCharPosition);
638665
}
639666
return result;
640667
}

‎src/main/java/org/apache/commons/csv/CSVRecord.java‎

Lines changed: 28 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@
1818
package org.apache.commons.csv;
1919

2020
import java.io.Serializable;
21-
import java.util.ArrayList;
2221
import java.util.Arrays;
2322
import java.util.Iterator;
2423
import java.util.LinkedHashMap;
@@ -40,24 +39,20 @@ public final class CSVRecord implements Serializable, Iterable<String> {
4039
/** The accumulated comments (if any) */
4140
private final String comment;
4241

43-
/** The column name to index mapping. */
44-
private final Map<String, Integer> headerMap;
45-
46-
/** The column order to avoid re-computing it. */
47-
private final List<String> headerNames;
48-
4942
/** The record number. */
5043
private final long recordNumber;
5144

5245
/** The values of the record */
5346
private final String[] values;
5447

55-
CSVRecord(final String[] values, final Map<String, Integer> headerMap, List<String> headerNames, final String comment,
56-
final long recordNumber, final long characterPosition) {
48+
/** The parser that originates this record. */
49+
private final CSVParser parser;
50+
51+
CSVRecord(final CSVParser parser, final String[] values, final String comment, final long recordNumber,
52+
final long characterPosition) {
5753
this.recordNumber = recordNumber;
5854
this.values = values != null ? values : EMPTY_STRING_ARRAY;
59-
this.headerMap = headerMap;
60-
this.headerNames = headerNames;
55+
this.parser = parser;
6156
this.comment = comment;
6257
this.characterPosition = characterPosition;
6358
}
@@ -84,6 +79,10 @@ public String get(final int i) {
8479
return values[i];
8580
}
8681

82+
private Map<String, Integer> getHeaderMapRaw() {
83+
return parser.getHeaderMapRaw();
84+
}
85+
8786
/**
8887
* Returns a value by name.
8988
*
@@ -98,7 +97,12 @@ public String get(final int i) {
9897
* @see CSVFormat#withNullString(String)
9998
*/
10099
public String get(final String name) {
101-
final Integer index = getIndex(name);
100+
final Map<String, Integer> headerMap = getHeaderMapRaw();
101+
if (headerMap == null) {
102+
throw new IllegalStateException(
103+
"No header mapping was specified, the record values can't be accessed by name");
104+
}
105+
final Integer index = headerMap.get(name);
102106
if (index == null) {
103107
throw new IllegalArgumentException(String.format("Mapping for %s not found, expected one of %s", name,
104108
headerMap.keySet()));
@@ -135,29 +139,13 @@ public String getComment() {
135139
}
136140

137141
/**
138-
* Returns a copy of the header names that iterates in column order.
139-
*
140-
* @return a copy of the header names that iterates in column order.
142+
* Returns the parser.
143+
*
144+
* @return the parser.
141145
* @since 1.7
142146
*/
143-
public List<String> getHeaderNames() {
144-
return new ArrayList<>(headerNames);
145-
}
146-
147-
Integer getIndex(final String name) {
148-
if (headerMap == null) {
149-
throw new IllegalStateException(
150-
"No header mapping was specified, the record values can't be accessed by name");
151-
}
152-
return headerMap.get(name);
153-
}
154-
155-
String getName(final int index) {
156-
if (headerMap == null) {
157-
throw new IllegalStateException(
158-
"No header mapping was specified, the record values can't be accessed by name");
159-
}
160-
return headerNames.get(index);
147+
public CSVParser getCSVParser() {
148+
return parser;
161149
}
162150

163151
/**
@@ -199,6 +187,7 @@ public boolean hasComment() {
199187
* @return true of this record is valid, false if not
200188
*/
201189
public boolean isConsistent() {
190+
final Map<String, Integer> headerMap = getHeaderMapRaw();
202191
return headerMap == null || headerMap.size() == values.length;
203192
}
204193

@@ -210,6 +199,7 @@ public boolean isConsistent() {
210199
* @return whether a given column is mapped.
211200
*/
212201
public boolean isMapped(final String name) {
202+
final Map<String, Integer> headerMap = getHeaderMapRaw();
213203
return headerMap != null && headerMap.containsKey(name);
214204
}
215205

@@ -221,7 +211,7 @@ public boolean isMapped(final String name) {
221211
* @return whether a given columns is mapped and has a value
222212
*/
223213
public boolean isSet(final String name) {
224-
return isMapped(name) && headerMap.get(name).intValue() < values.length;
214+
return isMapped(name) && getHeaderMapRaw().get(name).intValue() < values.length;
225215
}
226216

227217
/**
@@ -242,10 +232,10 @@ public Iterator<String> iterator() {
242232
* @return the given map.
243233
*/
244234
<M extends Map<String, String>> M putIn(final M map) {
245-
if (headerMap == null) {
235+
if (getHeaderMapRaw() == null) {
246236
return map;
247237
}
248-
for (final Entry<String, Integer> entry : headerMap.entrySet()) {
238+
for (final Entry<String, Integer> entry : getHeaderMapRaw().entrySet()) {
249239
final int col = entry.getValue().intValue();
250240
if (col < values.length) {
251241
map.put(entry.getKey(), values[col]);
@@ -291,9 +281,8 @@ public Map<String, String> toMap() {
291281
*/
292282
@Override
293283
public String toString() {
294-
return "CSVRecord [comment=" + comment + ", mapping=" + headerMap +
295-
", recordNumber=" + recordNumber + ", values=" +
296-
Arrays.toString(values) + "]";
284+
return "CSVRecord [comment='" + comment + "', recordNumber=" + recordNumber + ", values="
285+
+ Arrays.toString(values) + "]";
297286
}
298287

299288
String[] values() {

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

Lines changed: 24 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -490,6 +490,20 @@ public void testGetHeaderMap() throws Exception {
490490
}
491491
}
492492

493+
@Test
494+
public void testGetHeaderNames() throws IOException {
495+
try (final CSVParser parser = CSVParser.parse("a,b,c\n1,2,3\nx,y,z",
496+
CSVFormat.DEFAULT.withHeader("A", "B", "C"))) {
497+
final Map<String, Integer> nameIndexMap = parser.getHeaderMap();
498+
final List<String> headerNames = parser.getHeaderNames();
499+
Assert.assertEquals(nameIndexMap.size(), headerNames.size());
500+
for (int i = 0; i < headerNames.size(); i++) {
501+
final String name = headerNames.get(i);
502+
Assert.assertEquals(i, nameIndexMap.get(name).intValue());
503+
}
504+
}
505+
}
506+
493507
@Test
494508
public void testGetLine() throws IOException {
495509
try (final CSVParser parser = CSVParser.parse(CSV_INPUT, CSVFormat.DEFAULT.withIgnoreSurroundingSpaces())) {
@@ -678,9 +692,9 @@ public void testHeadersMissingException() throws Exception {
678692

679693
@Test
680694
public void testIgnoreCaseHeaderMapping() throws Exception {
681-
final Reader in = new StringReader("1,2,3");
695+
final Reader reader = new StringReader("1,2,3");
682696
final Iterator<CSVRecord> records = CSVFormat.DEFAULT.withHeader("One", "TWO", "three").withIgnoreHeaderCase()
683-
.parse(in).iterator();
697+
.parse(reader).iterator();
684698
final CSVRecord record = records.next();
685699
assertEquals("1", record.get("one"));
686700
assertEquals("2", record.get("two"));
@@ -742,10 +756,10 @@ public void testIteratorSequenceBreaking() throws IOException {
742756
// Iterator hasNext() shouldn't break sequence
743757
CSVParser parser = CSVFormat.DEFAULT.parse(new StringReader(fiveRows));
744758
int recordNumber = 0;
745-
Iterator<CSVRecord> iter = parser.iterator();
759+
final Iterator<CSVRecord> iter = parser.iterator();
746760
recordNumber = 0;
747761
while (iter.hasNext()) {
748-
CSVRecord record = iter.next();
762+
final CSVRecord record = iter.next();
749763
recordNumber++;
750764
assertEquals(String.valueOf(recordNumber), record.get(0));
751765
if (recordNumber >= 2) {
@@ -754,38 +768,38 @@ public void testIteratorSequenceBreaking() throws IOException {
754768
}
755769
iter.hasNext();
756770
while (iter.hasNext()) {
757-
CSVRecord record = iter.next();
771+
final CSVRecord record = iter.next();
758772
recordNumber++;
759773
assertEquals(String.valueOf(recordNumber), record.get(0));
760774
}
761775

762776
// Consecutive enhanced for loops shouldn't break sequence
763777
parser = CSVFormat.DEFAULT.parse(new StringReader(fiveRows));
764778
recordNumber = 0;
765-
for (CSVRecord record : parser) {
779+
for (final CSVRecord record : parser) {
766780
recordNumber++;
767781
assertEquals(String.valueOf(recordNumber), record.get(0));
768782
if (recordNumber >= 2) {
769783
break;
770784
}
771785
}
772-
for (CSVRecord record : parser) {
786+
for (final CSVRecord record : parser) {
773787
recordNumber++;
774788
assertEquals(String.valueOf(recordNumber), record.get(0));
775789
}
776790

777791
// Consecutive enhanced for loops with hasNext() peeking shouldn't break sequence
778792
parser = CSVFormat.DEFAULT.parse(new StringReader(fiveRows));
779793
recordNumber = 0;
780-
for (CSVRecord record : parser) {
794+
for (final CSVRecord record : parser) {
781795
recordNumber++;
782796
assertEquals(String.valueOf(recordNumber), record.get(0));
783797
if (recordNumber >= 2) {
784798
break;
785799
}
786800
}
787801
parser.iterator().hasNext();
788-
for (CSVRecord record : parser) {
802+
for (final CSVRecord record : parser) {
789803
recordNumber++;
790804
assertEquals(String.valueOf(recordNumber), record.get(0));
791805
}
@@ -799,7 +813,7 @@ public void testLineFeedEndings() throws IOException {
799813
assertEquals(4, records.size());
800814
}
801815
}
802-
816+
803817
@Test
804818
public void testMappedButNotSetAsOutlook2007ContactExport() throws Exception {
805819
final Reader in = new StringReader("a,b,c\n1,2\nx,y,z");

‎src/test/java/org/apache/commons/csv/CSVPrinterTest.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ private static String printable(final String s) {
8282
}
8383

8484
private final String recordSeparator = CSVFormat.DEFAULT.getRecordSeparator();
85-
private String longText2;
85+
private String longText2;
8686

8787
private void doOneRandom(final CSVFormat format) throws Exception {
8888
final Random r = new Random();

0 commit comments

Comments
 (0)