Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Object cell reader should return null on empty string #84

Closed
eharik opened this issue Jan 27, 2015 · 20 comments
Closed

Object cell reader should return null on empty string #84

eharik opened this issue Jan 27, 2015 · 20 comments
Milestone

Comments

@eharik
Copy link

eharik commented Jan 27, 2015

When parsing rows that have adjacent commas, indicating a lack of value, the parser throws an error.

Note that I'm running this in Scala so not sure if that's impacting anything.

test.csv
my_field,second_field
,,
class TestClass(var myField: String, var secondField)

val it = CsvParser.mapTo(classOf[TestClass]).iterate(reader)
while(it.hasNext) { println(it.next.myField) }
java.lang.Exception: org.sfm.map.MappingException: Error mapping field org.sfm.csv.CsvColumnKey@3978a417 : null
  at io.oseberg.service.bpoocc.CompletionArcgateMasterFileV1ServiceSfm.test(CompletionArcgateMasterFileV1ServiceSfm.scala:41)
  ... 43 elided
Caused by: org.sfm.map.MappingException: Error mapping field org.sfm.csv.CsvColumnKey@3978a417 : null
  at org.sfm.map.impl.RethrowFieldMapperErrorHandler.errorMappingField(RethrowFieldMapperErrorHandler.java:11)
  at org.sfm.csv.impl.CsvMapperCellConsumer.newCellForSetter(CsvMapperCellConsumer.java:162)
  at org.sfm.csv.impl.CsvMapperCellConsumer.newCell(CsvMapperCellConsumer.java:191)
  at org.sfm.csv.impl.CsvMapperCellConsumer.newCell(CsvMapperCellConsumer.java:177)
  at org.sfm.csv.parser.AbstractCsvCharConsumer.newCell(AbstractCsvCharConsumer.java:101)
  at org.sfm.csv.parser.AbstractCsvCharConsumer.newCellIfNotInQuote(AbstractCsvCharConsumer.java:45)
  at org.sfm.csv.parser.StandardCsvCharConsumer.nextRow(StandardCsvCharConsumer.java:41)
  at org.sfm.csv.CsvReader.parseRow(CsvReader.java:70)
  at org.sfm.csv.impl.CsvMapperIterator.fetch(CsvMapperIterator.java:39)
  at org.sfm.csv.impl.CsvMapperIterator.hasNext(CsvMapperIterator.java:31)
@arnaudroger
Copy link
Owner

Thanks for the report, I'll check that out. I have not test it for scala so it might be link to the types.

@arnaudroger
Copy link
Owner

did a quick test there and cannot reproduce on a simple java object, would you have the full stack trace with the line at which the npe was thrown?
Also do you know what type is secondField resolved too?
I'll try to get a scala test setup but it will take some time.

@arnaudroger
Copy link
Owner

I try the following code

import java.io.StringReader

import org.sfm.csv.CsvParser

object TestIssue84 {

  class TestClass(var myField: String, var secondField: String) 

  def main(args: Array[String]) {


    val it = CsvParser.mapTo(classOf[TestClass]).iterate(new StringReader("my_field,second_field\n,,"))
    while(it.hasNext) { println(it.next.myField); println("X") }

  }
}

and it works fine. I'm not an expert in scala it seems you I needed to specify the type for second field. If I put Any I get a error as expected. I used the intellij plugin with the last scala jars

@arnaudroger
Copy link
Owner

@arnaudroger
Copy link
Owner

just to check which version did you use?

@arnaudroger
Copy link
Owner

I think I reproduced it in 1.2.0 if that's the case the it's fixed in 1.3, can you confirm the version you are using?

@eharik
Copy link
Author

eharik commented Jan 28, 2015

Thanks for looking into this. The example above was typed free hand and the stack trace was taken from the actual error I was having in my application. In the application the type was defined for the second field in the class.

I pushed some test code up to my github with the settings I am using. After looking into this a bit further I believe the issue was that I was using java.sql.Date rather than java.util.Date. (I would love if you supported joda time and I see you already have that listed as an improvement.)

Now that I've switched to java.util.Date I'm having an issue parsing empty dates. Specifically:

[error] (run-main-9) org.sfm.map.MappingException: Error mapping field org.sfm.csv.CsvColumnKey@4d0e8491 : Unparseable date: ""
org.sfm.map.MappingException: Error mapping field org.sfm.csv.CsvColumnKey@4d0e8491 : Unparseable date: ""
    at org.sfm.map.impl.RethrowFieldMapperErrorHandler.errorMappingField(RethrowFieldMapperErrorHandler.java:11)
    at org.sfm.csv.impl.CsvMapperCellConsumer.newCellForSetter(CsvMapperCellConsumer.java:162)
    at org.sfm.csv.impl.CsvMapperCellConsumer.newCell(CsvMapperCellConsumer.java:191)
    at org.sfm.csv.impl.CsvMapperCellConsumer.newCell(CsvMapperCellConsumer.java:177)
    at org.sfm.csv.parser.AbstractCsvCharConsumer.newCell(AbstractCsvCharConsumer.java:101)
    at org.sfm.csv.parser.AbstractCsvCharConsumer.newCellIfNotInQuote(AbstractCsvCharConsumer.java:45)
    at org.sfm.csv.parser.StandardCsvCharConsumer.nextRow(StandardCsvCharConsumer.java:41)
    at org.sfm.csv.CsvReader.parseRow(CsvReader.java:70)
    at org.sfm.csv.impl.CsvMapperIterator.fetch(CsvMapperIterator.java:39)
    at org.sfm.csv.impl.CsvMapperIterator.hasNext(CsvMapperIterator.java:31)
    at TestIssue84$.testDate(ScalaMain.scala:59)
    at TestIssue84$.main(ScalaMain.scala:31)
    at TestIssue84.main(ScalaMain.scala)
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.lang.reflect.Method.invoke(Method.java:606)
    at sbt.Run.invokeMain(Run.scala:67)
    at sbt.Run.run0(Run.scala:61)
    at sbt.Run.sbt$Run$$execute$1(Run.scala:51)
    at sbt.Run$$anonfun$run$1.apply$mcV$sp(Run.scala:55)
    at sbt.Run$$anonfun$run$1.apply(Run.scala:55)
    at sbt.Run$$anonfun$run$1.apply(Run.scala:55)
    at sbt.Logger$$anon$4.apply(Logger.scala:85)
    at sbt.TrapExit$App.run(TrapExit.scala:248)
    at java.lang.Thread.run(Thread.java:745)
Caused by: org.sfm.csv.impl.ParsingException: Unparseable date: ""
    at org.sfm.csv.impl.cellreader.DateCellValueReader.read(DateCellValueReader.java:26)
    at org.sfm.csv.impl.cellreader.DateCellValueReader.read(DateCellValueReader.java:11)
    at org.sfm.csv.impl.cellreader.CellSetterImpl.set(CellSetterImpl.java:21)
    at org.sfm.csv.impl.CsvMapperCellConsumer.newCellForSetter(CsvMapperCellConsumer.java:159)
    at org.sfm.csv.impl.CsvMapperCellConsumer.newCell(CsvMapperCellConsumer.java:191)
    at org.sfm.csv.impl.CsvMapperCellConsumer.newCell(CsvMapperCellConsumer.java:177)
    at org.sfm.csv.parser.AbstractCsvCharConsumer.newCell(AbstractCsvCharConsumer.java:101)
    at org.sfm.csv.parser.AbstractCsvCharConsumer.newCellIfNotInQuote(AbstractCsvCharConsumer.java:45)
    at org.sfm.csv.parser.StandardCsvCharConsumer.nextRow(StandardCsvCharConsumer.java:41)
    at org.sfm.csv.CsvReader.parseRow(CsvReader.java:70)
    at org.sfm.csv.impl.CsvMapperIterator.fetch(CsvMapperIterator.java:39)
    at org.sfm.csv.impl.CsvMapperIterator.hasNext(CsvMapperIterator.java:31)
    at TestIssue84$.testDate(ScalaMain.scala:59)
    at TestIssue84$.main(ScalaMain.scala:31)
    at TestIssue84.main(ScalaMain.scala)
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.lang.reflect.Method.invoke(Method.java:606)
    at sbt.Run.invokeMain(Run.scala:67)
    at sbt.Run.run0(Run.scala:61)
    at sbt.Run.sbt$Run$$execute$1(Run.scala:51)
    at sbt.Run$$anonfun$run$1.apply$mcV$sp(Run.scala:55)
    at sbt.Run$$anonfun$run$1.apply(Run.scala:55)
    at sbt.Run$$anonfun$run$1.apply(Run.scala:55)
    at sbt.Logger$$anon$4.apply(Logger.scala:85)
    at sbt.TrapExit$App.run(TrapExit.scala:248)
    at java.lang.Thread.run(Thread.java:745)

@arnaudroger
Copy link
Owner

Thanks for that I'll add a ticket to better report the error, and change the date reader to return null for empty string. Hopefully I'll have that sorted by the end of the week. If you can't wait you can provide your own custom reader but you need to use the csv mapper factory directly.

@eharik
Copy link
Author

eharik commented Jan 28, 2015

Great, thanks!

@arnaudroger arnaudroger changed the title Empty fields throw MappingException Date cell reader should return null on empty string Jan 28, 2015
@arnaudroger arnaudroger added this to the 1.4.0 milestone Jan 28, 2015
@arnaudroger arnaudroger changed the title Date cell reader should return null on empty string Object cell reader should return null on empty string Jan 28, 2015
@arnaudroger
Copy link
Owner

I will be published the new version with jodatime and fixes soon, I think though that you probably will have to change the date format used. I added some doc on the wiki

https://github.com/arnaudroger/SimpleFlatMapper/wiki/CsvParser#customised-date-format

tell me if your stuck

@eharik
Copy link
Author

eharik commented Jan 29, 2015

Excellent, thanks.

Another question on the topic - with jOOQ you can use some wildcarding to specify the columns that should use a custom mapper. Is there similar support here? In other words, if I want to define that all columns matching the form *_date should use the specified column definition is that possible?

CsvMapper<MyObject> mapper = 
    CsvMapperFactory
        .newInstance()
        .addColumnDefinition("*_date", CsvColumnDefinition.dateFormatDefinition("yyyyMMdd"))
        .newMapper(MyObject.class);

@arnaudroger
Copy link
Owner

Not at the moment but it would be very difficult or had that or something similar

@arnaudroger
Copy link
Owner

The column definition is an case insensitive match right now

@lukaseder
Copy link

@arnaudroger : This looks all very interesting. If you're interested in writing a guest blog post about using jOOQ+SimpleFlatMapper on blog.jooq.org (which is published also on DZone, JavaCodeGeeks, JAXenter and voxxed), let me know. This would certainly get you some additional traction.

@arnaudroger
Copy link
Owner

Thanks very much appreciated. I need to write something I have not come around it yet just will keep you posted

@lukaseder
Copy link

Sure, any time you're ready

@arnaudroger
Copy link
Owner

@eharik on the *_date column def I added the ticket #89 you would provide a predicate on the key.
addColumnDefinition((k) -> k.getName().endsWith("_date"), CsvColumnDefinition.dateFormatDefinition("yyyyMMdd")) to do the equivalent.

@eharik
Copy link
Author

eharik commented Jan 31, 2015

very cool! I'll let you know if I have any issues.

@arnaudroger
Copy link
Owner

I will be in 1 4 1 should have it ready tomorrow

@arnaudroger
Copy link
Owner

141 is released

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants