eed3si9n / scalaxb

scalaxb is an XML data binding tool for Scala.
http://scalaxb.org/
MIT License
337 stars 154 forks source link

Allow alternatives to XMLGregorianCalendar for xsd:dateTime #376

Open RomanIakovlev opened 8 years ago

RomanIakovlev commented 8 years ago

Currently there's no way to configure scalaxb to use anything but XMLGregorianCalendar for xsd:dateTime and other time-related types in generated classes. It would be convenient to use java.time.* classes instead, either by default or based on a config value.

tOverney commented 4 years ago

Thinking about having a go at it! @eed3si9n what do you think of adding a settingKey scalaxbUseJavaTime defaulting to false ?

I would then change places like this to use that boolean and map the xs:date and other those to some new BuiltInSimpleTypeSymbol if the setting key is true otherwise keep the current mapping.

e.g.:

object XsJavaTimeDate extends BuiltInSimpleTypeSymbol("java.time.LocalDate") {}

  ...
  case "date" if useJavaTime => XsJavaTimeDate
  case "date" => XsDate
  ...
eed3si9n commented 4 years ago

I'd say scalaxbUseJavaTime should default to hell yea.

khajavi commented 3 years ago

Hey @eed3si9n I sent this pull request https://github.com/eed3si9n/scalaxb/pull/542 This is my first try. Could you please review that pull request? I have a question on what is the corresponding data type for xs:time in java.time.* package?

tOverney commented 3 years ago

Crap! I guess I was not clear enough in my above post but I started to work on that too: https://github.com/eed3si9n/scalaxb/compare/master...tOverney:javaTime Plann-ed/-ing to finish it tomorrow!

I should have opened a draft PR...

tOverney commented 3 years ago

The thing missing from my WIP is how to make fromAny work with the two possible types!

khajavi commented 3 years ago

Hey Tristan, @tOverney Great work but here are two notes about your works:

  1. LocalTime does not correspond to the xs:time type because xs:time has timeZoneOffset information in the xmlschema spec. So if we use LocalTime it loss the timeZoneOffset.
  2. Also LocalDate is not correspond to the xs:DateTime. xs:DateTime have timeZoneOffset but LocalDate doesn't.

This is why I asked that question.

tOverney commented 3 years ago

Hey, This is a valid point, using LocalDate and LocalDateTime with the ISO_* DateTimeFormatter is fully compliant with the w3spec in terms of parsing and writing of values.

But indeed if you read a date that is not in your timezone you would write it back in your timezone (while still writting the same correct epoch instant). And as far as I'm aware/concerned all apps I've worked with tend to work with in a single timezone with LocalDate everywhere;

I would argue that adding support for java.time.* is about convenience (as written in the initial post from @RomanIakovlev) and if one really needs to keep timezone data then that's where setting useJavaTime = false comes into place and you would still get classes that do not convert parsed values into your own timezone.

For the LocalTime I do see an issue since it's basically a 24h circular value and could completely break everything... I guess OffsetTime could do the trick, right?

deenar commented 3 years ago

Hi Tristan @tOverney.

My 2 cents

using LocalDate and LocalDateTime with the ISO_* DateTimeFormatter is fully compliant with the w3spec in terms of parsing and writing of values I was under the impression that al xs:dateTime, xs:date and xs:Time all have optional timezone elements, so just using LocalDate and LocalDateTime would break the roundtrip that scalaxb and @eed3si9n triea hard to maintain.

Using java.time.OffsetDateTime should support this and there is a toLocalDateTime and toLocalDate method on OffsetDateTime, so it is not too inconvenient for those working with dates from single timezones.

Deenar

eed3si9n commented 3 years ago

Yes +1 on OffsetDateTime - https://docs.oracle.com/javase/8/docs/api/java/time/OffsetDateTime.html

This reminds me of a Twitter convo I had on this - https://twitter.com/jodastephen/status/882926344547520514

tOverney commented 3 years ago

Arguments make sense for OffsetDateTime for xs:dateTime! but is using a type including a time element really a good idea for xs:date? I find this quite misleading

eed3si9n commented 3 years ago

https://www.w3.org/TR/xmlschema-2/#date

The ·lexical space· of date consists of finite-length sequences of characters of the form: '-'? yyyy '-' mm '-' dd zzzzzz? where the date and optional timezone are represented exactly the same way as they are for dateTime. The first moment of the interval is that represented by: '-' yyyy '-' mm '-' dd 'T00:00:00' zzzzzz? and the least upper bound of the interval is the timeline point represented (noncanonically) by: '-' yyyy '-' mm '-' dd 'T24:00:00' zzzzzz?.

If we need to represent 2002-10-10+13, I'm not sure if there are any other option than using 00:00:00 of OffsetDateTime.

deenar commented 3 years ago

There isn't an OffsetDate only type. I suspect this is really to do with the timezone offset, the LocalDate might be different given timezones. It is not different from the current state where XMLGregorianCalendar is used for all types.

atais commented 3 years ago

How about super flexible parameter:

dateTypeClass where user could provide a full path to the desired class. default: javax.xml.datatype.XMLGregorianCalendar some would use org.joda.time.DateTime and others (like me) java.util.String.

You can read in the #543 why this would be important.

Do you think it could work?