arnaudroger / SimpleFlatMapper

Fast and Easy mapping from database and csv to POJO. A java micro ORM, lightweight alternative to iBatis and Hibernate. Fast Csv Parser and Csv Mapper
http://simpleflatmapper.org
MIT License
440 stars 76 forks source link

CSV parsing uses incorrect DateTimeFormatter for LocalDate fields #511

Open clamothe opened 6 years ago

clamothe commented 6 years ago
class Bean {
    private LocalDate approved;

    public LocalDate getApproved() {
        return approved;
    }

    public void setApproved(LocalDate approved) {
        this.approved = approved;
    }

}

static {
        CsvParser
                .mapTo(Bean.class)
                .stream(reader).count();
}

This results in the following exception:

java.time.format.DateTimeParseException: Text '1995-01-01' could not be parsed at index 10

    at java.time.format.DateTimeFormatter.parseResolved0(DateTimeFormatter.java:1949)
    at java.time.format.DateTimeFormatter.parse(DateTimeFormatter.java:1851)
    at java.time.LocalDate.parse(LocalDate.java:400)
    at org.simpleflatmapper.converter.impl.time.CharSequenceToLocalDateConverter.convert(CharSequenceToLocalDateConverter.java:21)
    at org.simpleflatmapper.converter.impl.time.CharSequenceToLocalDateConverter.convert(CharSequenceToLocalDateConverter.java:10)
    at org.simpleflatmapper.csv.impl.cellreader.CharSequenceConverterCellValueReader.read(CharSequenceConverterCellValueReader.java:18)
    at org.simpleflatmapper.csv.impl.cellreader.CellSetterImpl.set(CellSetterImpl.java:21)
    at org.simpleflatmapper.csv.generated.com.concentricsky.c4o.curriculumservice.datascrubbing.server.coci.AsmCsvMapperCellHandlerToCociProgramS10_I1._cellValue(Unknown Source)
    at org.simpleflatmapper.csv.generated.com.concentricsky.c4o.curriculumservice.datascrubbing.server.coci.AsmCsvMapperCellHandlerToCociProgramS10_I1.cellValue(Unknown Source)
    at org.simpleflatmapper.csv.mapper.CsvMapperCellConsumer.newCell(CsvMapperCellConsumer.java:102)
    at org.simpleflatmapper.csv.mapper.CsvMapperCellConsumer.newCell(CsvMapperCellConsumer.java:94)
    at org.simpleflatmapper.csv.parser.UnescapeCellPreProcessor.newCell(UnescapeCellPreProcessor.java:16)
    at org.simpleflatmapper.csv.parser.Asm_S_2c_Q_22_E_22_CharConsumer.consumeAllBuffer(ConfigurableCharConsumer.java:94)
    at org.simpleflatmapper.csv.CsvReader._parseAll(CsvReader.java:57)
    at org.simpleflatmapper.csv.CsvReader.parseAll(CsvReader.java:50)
    at org.simpleflatmapper.csv.impl.CsvMapperImpl$CsvSpliterator.forEachRemaining(CsvMapperImpl.java:167)
    at java.util.stream.ReferencePipeline$Head.forEach(ReferencePipeline.java:580)

By default, a date format of ""yyyy-MM-dd HH:mm:ss" is used, even for parsing LocalDates. This is defined in CsvMapperBuilder::defaultDateFormat and does not differ per Date field type.

The tests written for CharSequenceToLocalDateConverter test it with a "yyyy-MM-dd" format. CSV parse should expect this format by default for the LocalDate field.

I was able to work around this using, however overriding the default date format isn't a solution if I also want to parse LocalDateTime fields.

        CsvMapperFactory
                .newInstance()
                .defaultDateFormat("yyyy-MM-dd")
                .newMapper(CociProgram.class)
                .stream(reader);

Does this look like a bug? Is there a better workaround? Thanks! Cool library

arnaudroger commented 6 years ago

Thanks for the bug report. I’ll need to look deeper into that one. It must be possible to have different default formatter by type.

arnaudroger commented 6 years ago

You can also specify date formatter per column. I’ll find the code later

clamothe commented 6 years ago

Great, I appreciate your responsiveness.

arnaudroger commented 6 years ago

so thinking about it, also that would be great to have a different default pattern per type i'm afraid that change my have some backward compatibility issue.

so otherwise to have the datetimeformat at the column level

  CsvMapperFactory
                .newInstance()
                .addColumnProperty("date_column", new DateFormatProperty("yyyy-MM-dd"))
                .newMapper(CociProgram.class)
                .stream(reader);

the first a Predicate if you want to apply the date format on multiple column.

  CsvMapperFactory
                .newInstance()
                .addColumnProperty(k -> k.getName().startsWith("date"), new DateFormatProperty("yyyy-MM-dd"))
                .newMapper(CociProgram.class)
                .stream(reader);

I'll keep the ticket open to update the documentation.

clamothe commented 6 years ago

Thanks! I'll use the field-specific solution for now. It is true that someone may expect different behavior than I, or existing code depends on this functionality. One could desire to parse a Date+time string into a LocalDate, though ideally that would be not be the primary supported case.

I wonder if the default date time format could be modified to optionally expect time. That could still could have backwards compatibility issues in some edge cases.

arnaudroger commented 6 years ago

updated doc

arnaudroger commented 6 years ago

just read the last comment and yes could have something that check if have time