FasterXML / jackson-modules-java8

Set of support modules for Java 8 datatypes (Optionals, date/time) and features (parameter names)
Apache License 2.0
400 stars 115 forks source link

Add possibility to serialize java time classes as milliseconds #96

Open C-h-e-r-r-y opened 5 years ago

C-h-e-r-r-y commented 5 years ago

Let start from example:

public class MyDto {
    @JsonFormat(shape = JsonFormat.Shape.NUMBER_INT)
    private LocalDate dob;
}

This will serialized dob as days since epoch. I find this very confusing because all legacy code works with old java.util.Date class which instaniated from milliseconds. Could you please add ability for (de)serialization all java time classes as milliseconds (and seconds and may be other time units).

It can be like this:

 public class MyDto {
     @JsonFormat(shape = JsonFormat.Shape.NUMBER_INT, timeUnit = MILLISECOND)
     private LocalDate dob;
 }

Which internally call something like localDate.atStartOfDay(timezone).toInstant().toEpochMilli() timezone can be taken from com.fasterxml.jackson.annotation.JsonFormat.timezone.

GedMarc commented 5 years ago

Hmm, But the question is why are you using LocalDate to perform this? The entire java.time library is based on a few things, but most importantly,

Separation of Concerns: The new API separates clearly between human readable date time and machine time (unix timestamp). It defines separate classes for Date, Time, DateTime, Timestamp, Timezone etc.

The other part of course is the actual representation of an "Instant", which is defined as

'Millis since unix epoch' represents an instant.
An instant is a time value (LocalDateTime)

The java.time API is driven by these two fundamental cores, Are you sure you should not be using LocalDateTime? The milliseconds are from a specific date, 1970/01/01 and would be stored as a long, so setting the shape to int doesn't make sense here either? Because of the time frame, days is a very logical pull for an int? Why use a long value when you are wanting an int? Milliseconds from a day at some point in time will be too big for an int.

I think what you are looking for is something for LocalDateTime, which operates at a much lower level, and requesting smaller values such as minutes, seconds, milliseconds makes much more sense, also the number would be represented as a LONG instead of an INT, as the scale is simply that much smaller.

 @JsonFormat(shape = JsonFormat.Shape.NUMBER_INT)
     private LocalDate dob;
 @JsonFormat(shape = JsonFormat.Shape.NUMBER_FLOAT, timeUnit = MILLISECOND)
     private LocalDateTime dobTime;
 @JsonFormat(shape = JsonFormat.Shape.NUMBER_FLOAT, timeUnit = MILLISECOND)
         private Instant dobInstant;

For a better detailed description - https://softwareengineering.stackexchange.com/questions/225343/why-are-the-java-8-java-time-classes-missing-a-getmillis-method

C-h-e-r-r-y commented 5 years ago

First of all I did not offer to send long as int. I thought that NUMBER_INT "is used" to make time unit representation as number without decimal points. If it refer to int as java type - another shape should be used to make it long (JsonFormat.Shape.NUMBER?). Sorry for misleading. :(

Second about java time - yes it defines clear separation for different time units. But there is a problem when new project working together with old because old code sends and recives milliseconds which often truncated to start of the day (month, year etc.) and deserializaed as Date or Calendar objects. So I have to write custom serializer to "transform" new java time unit into milliseconds which should be desirialized in old projects. I think that if jackson can do this automatically it save a lot of time.

So the main idea of this - to make interaction with old code more easier

GedMarc commented 5 years ago

Could you not use the Instant and port that across safely?

With rolling releases every 6 months, it is going to become less and less likely for old code to stay relevant. Especially with JPMS where modules have to explicitly define themselves and their packages, or face the wrath of the JVM for their library (XStream anyone?), It's pretty difficult to think that in one years time we will be on JDK 14, but in the same breathe, It is also a really great cleaning out excersize to restart the Java library pool again, it is a bit bloated.

Date date = Date.from(localDate.atStartOfDay(ZoneId.systemDefault()).toInstant());

This is correct, as there really is no actual direct correlation between java.util.Date and java.time.LocalDate. You are correct, milliseconds are not for the java.time API, and Date likes them, but they shouldn't be used to transform from one to another. I also wouldn't create a converter for them at all either, as LocalDateTime, LocalDate, ZonedDateTime, ZonedDate and Instant in JPA, Modelling, JSON, and any other domain representation all have very particular meanings. In the world of domains and domain driven design, A date for instance may be a @JsonNumber, and an @Column(length=7), and a web display @DateInputField(format="yyyy/MM/dd) and a XML field @XMLAttribute all at the same time, and they all render accordingly. You want each to manage themselves and represent themselves in a particular fashion for their output. Then think about all of these in terms of "ZonedDateTime" and none of the classifications will change, but the output for each separate domain representation changes greatly.

So, my personal, Honestly, I wouldn't try mix and match. Date and Time API are not interchangeable, and I don't think you should go that route, but yes that is personal opinion. A calendar is not a clock, and an "Instant" is denoted as two separate entities, A Calendar dictating the location of the planet around the sun, and a Clock, marking the rotation of the planet in relation to its axis. The two are joined in a Many-To-Many relationship, where one day will have many hours, and those same hours belong to many days. This is why Date is so different to the time API. The Time API does something completely different, and fulfills a very separate role than just a manager to a GregorianCalendar or otherwise Calendar.

cowtowncoder commented 5 years ago

@C-h-e-r-r-y I'd have to think about this, but I think I agree with @GedMarc on pointing out that LocalDate is not really similar to java.util.Date (or, ZonedDateTime) in that it does not actually specify a timestamp. So while I can understand it being confusing that integer number is used for different meaning across types, I don't think it would be conceptually correct to use second/milli-second counts to express LocalDate. Durations, yes, but LocalDate is meaningless without binding TimeZone. That is: although it would be possible to allow expressing it in milliseconds (just scale), it would not really match meaning of timestamp for j.u.Date or ZonedDateTime.

As to the initial choice of allowing days-since-epoch: I am not 100% sure why that was taken -- but author (Tim, @beamerblvd) is well-versed in Java 8 date/time work so I assume he had good reasons.

Finally... I try to limit addition of new datatype-specific properties in @JsonFormat. It's a balancing act so sometimes addition is relevant for wide enough range to make sense.

C-h-e-r-r-y commented 5 years ago

From theory I agree. But about practice:

  1. @JsonFormat already have timezone parameter. So it have been already designed to be used for conversion. And it already have NUMBER, NUMBER_INT variants.
  2. Is there other way to convert LocalDate to ZonedDateTime except localDate.atStartOfDay(somezone) call? If it always converted like that why not to use it?
GedMarc commented 5 years ago

Ok so number 1 I can't answer,

Number 2 You will never do that in any application ever, the logic thinking is incorrect.

Short answer, there is no such type in Java-8. The combination of a calendar date and a time zone is questionable, at least to say. Reason is that time zones are intended to operate on date AND time. Since time zones are responsible for translating between global instants/moments and local timestamps (including date AND time) a combination of just a date and a zone is simply not complete to enable such transformations.

Perhaps if I understood a little bit more of your use case, maybe I could assist a bit more? Would the builder "of"'s not work?

ZonedDateTime.of(LocalDate.of(2019, 1,23), LocalTime.of(21, 00), ZoneId.systemDefault());
        DateTimeFormatter.ofPattern("yyyy-MM-dd")
                         .parse("2019-01-23");
C-h-e-r-r-y commented 5 years ago

You will never do that in any application ever, the logic thinking is incorrect.

In theory yes. In practise no - 5+ years projects with several thousands classes definitelly have core classes (entity, dto) which hardly uses Date class. Nobody interested to spend a lot money/hours (with regression of course) to refactor this in new java time API. So here we got Date "shifted" to month, year, date and conversions like this between new code and old one.

GedMarc commented 5 years ago

Hmm, I'll leave that as is, thoughts be thoughts You are trying to convert a "local" GregorianCalendar object, into a TimeZone capable object, you have to specify a "start" as to when and which TimeZone to begin this new object. Your question of setting it without a "start of day" highlights that you want to try this conversion without actually specifying where "the sun rises", and what time zone constitutes "Midnight". This gets a little bit better when you have uses in different time zones all reading the same article from their perspective of "midnight". This is the purpose of the time API, and why it was packaged separately as "java.time". I don't think in your test case you should be targeting the time API unless you intentionally and specifically have a need to use the "Time" and "Instant" calculations, I get this from not being able to specify which time zone and which hour you actually want to start at. The time API does not work on milliseconds, the GregorianCalendar does.

I think, You can set your time zone and startOfDay appropriately to gegin your new Instant calculations, or use the Calendar which is fixed to local only and provides the same sort of functions, minus everything needed to time zone calculations,

I hope I am understanding you correctly, and hope that this is helping in any way. But you have kind of driven being able to help to a full halt, as what is needed to enter the API you don't want to do, or rather refuse, and in going to the time api, there seems to be a misunderstanding of why it is different to Calendar objects.