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

Confusing javadoc for SerializationFeature.WRITE_DATES_AS_TIMESTAMPS #1624

Closed
hvgotcodes opened this issue May 11, 2017 · 22 comments
Closed

Comments

@hvgotcodes
Copy link

Using Jackson 2.8

The documentation for WRITE_DATES_AS_TIMESTAMPS in part reads

If textual representation is used, the actual format is
     one returned by a call to
     {@link com.fasterxml.jackson.databind.SerializationConfig#getDateFormat}:
     the default setting being {@link com.fasterxml.jackson.databind.util.StdDateFormat},
     which corresponds to format String of "yyyy-MM-dd'T'HH:mm:ss.SSSZ"
     (see {@link java.text.DateFormat} for details of format Strings).

The documentation for WRITE_DATE_KEYS_AS_TIMESTAMPS in part reads

 Default value is 'false', meaning that Date-valued Map keys are serialized as textual (ISO-8601) values.

This implies that if WRITE_DATES_AS_TIMESTAMPS is disabled Jackson should return ISO8601 compliant Strings.

The problem is that a DateFormat string of "yyyy-MM-dd'T'HH:mm:ss.SSSZ" is NOT ISO8601 compliant.

The values serialized look like

2017-05-10T07:00:00.000+0000. This is not 8601 compliant.

Compare to

2017-05-10T07:00:00.000+00:00 which IS 8601 compliant.

If I try take a string Jackson serializes with the above format and create a new Date in Safari, the date is invalid.

I get around this by doing

objectMapper.setDateFormat(new SimpleDateFormat("yyyy-MM-dd'T'HH:mm:ss.SSSXXX"));

@cowtowncoder
Copy link
Member

cowtowncoder commented May 16, 2017

@hvgotcodes As you may know, ISO-8601 is a meta-standard and allows specifying and using a wide variety of concrete format variants. It is NOT a single format string. So I am not convinced these are not ISO-8601 compatible without some more documentation -- like link to an article (or Wikipedia entry) that outlines why format is not compatible.
Compatibility with Safari may be a concern, but does not define this aspect.

But more importantly, I would be interested in improvements to wording; so PRs would be welcome.
For example, it might make sense to indicate that configured DateFormat (with certain default) will be used, whether or not ISO-8601 compliancy is mentioned.

@brenuart
Copy link
Contributor

ISO8601 allows the time offset to be represented in the form ±[hh]:[mm], ±[hh][mm], or ±[hh].
So dates produced by Jackson's StdDateFormat are therefore valid ISO8601 constructs.

The standard is quite flexible and allows for many variants - which quickly becomes a nightmare when dealing with clients on different platforms or using different libraries.

Safari and Apple iOS are indeed known to only accept an offset expressed with columns (the .SSSXXXformat). Same for the Jackson Java8 Time module...
For these reasons we decided to force that format for java.util.Date and guarantee inter-operability with applications built on Java8.

@michael-o
Copy link

michael-o commented Dec 4, 2019

@cowtowncoder @brenuart That's bullshit. ISO 8601 (no hyphen here) is not a meta standard. Is says clear and loud either all components have to be basic or extended, but not mixed. If you don't have access to the actual standard document, don't postulate things. There are no variaties, all input is telescoping. See also https://issues.apache.org/jira/browse/MSHARED-837

Using SDF is just limited. If doing extended, you must use yyyy-MM-dd'T'HH:mm:ss.SSSXXX.

95% if never really understood the standard because they don't have access to the document. Unfortunately, I cannot share the document because the copy I have is solely licensed to our enterprise. I can cite portions if you like.

@hvgotcodes, I am fully with you!

@kupci
Copy link
Member

kupci commented Dec 4, 2019

Compare to

2017-05-10T07:00:00.000+00:00 which IS 8601 compliant.

So yes, if as part of the issue, an excerpt from the standard could be included to back it up, that would be helpful.

(I could swear I was reading the full document on 8601, but now when I browse certain parts, such as the appendix, it pops up with a message stating that I need to buy it, at 158 CHF : )

But I do find this:

“:” (colon) | the “:” colon character, in extended format, separates the time scale components for “hour” and “minute”, and “minute” and “second”.

https://www.iso.org/obp/ui#iso:std:iso:8601:-1:ed-1:v1:en

@michael-o
Copy link

@kupci, strictly speaking, +00:00 should never be serialized, but only Z. But is is permissive to parse such a value. I think I need to put my hands on the classes to make then really compliant.

@cowtowncoder
Copy link
Member

@michael-o Fuck you too.

@michael-o
Copy link

michael-o commented Dec 5, 2019

@michael-o Fuck you too.

Now that we have exchanged words of love, can we get this fixed finally? I am willing to provide an SR.

@GedMarc
Copy link

GedMarc commented Dec 5, 2019

Ok let's put this to rest.

A) An ISO Standard is a global standard and cannot be IP'd by definition. If you have an IP version it is because you are reading an altered/personalized/customized version. These versions will not be entertained as official standards.
What you are looking at is 2019-2 which is an adaptation of :2004, and is isolated to only those implementers of the update.
http://www.loc.gov/standards/datetime/iso-tc154-wg5_n0038_iso_wd_8601-1_2016-02-16.pdf

B) ISO 8601:2004 defines the standard as "YYYY-MM-DDThh:mm:ss.sTZD (eg 1997-07-16T19:20:30.45+01:00)"
Note the precision of milliseconds please. The specification denotes :

This profile does not specify how many digits may be used to represent the decimal fraction of a second. An adopting standard that permits fractions of a second must specify both the minimum number of digits (a number greater than or equal to one) and the maximum number of digits (the maximum may be stated to be "unlimited").

I strongly suspect your version of the ISO is an "adopted standard", that you are using the 2019-2 customization

C) +00:00 is a nullifier. parsing it is pointless, as is it's inclusion.

1994-11-05T08:15:30-05:00 corresponds to November 5, 1994, 8:15:30 am, US Eastern Standard Time.
1994-11-05T13:15:30Z corresponds to the same instant.

There is no "Z" in this ISO standard that is followed by a time zone specification. That is the
Extended Date/Time Format (EDTF) Specification standard and used for instant identification (aka, java.time not java.util)

So either - Use java.time to adopt the standard that matches with DateTimeFormatter for your format which does work 100%, or swop your ISO to match that represented by java.util.Date which is simple date format prettified.

Now can we please put this to bed,

@michael-o
Copy link

@GedMarc How does this relate to the issue here?

@GedMarc
Copy link

GedMarc commented Dec 5, 2019

@michael-o You are using java.util.Date instead of java.time. for your date format.
You are using a customized version of the ISO standard instead of the official :2004 release.
You are requesting that the altered ISO customizations be included as you are reading a customized version ISO8601 as the official release

In a nutshell -
You are using the wrong data object for your datetime pattern, Jackson is 100% ISO compliant, you are referencing an (IP) altered version of the ISO and expecting open source to cater for it.

Switch to a LocalDateTime, ZonedDateTime or similar, use @jsonformat, and then of course stop requesting IP standards to be included in open source software which by default is built on global standards and not customized IP ones.

In this case, @cowtowncoder comment is 100% valid, the library is matched perfectly to the specification, and you are requesting for IP to be included in an open source library.

In a nutshell, and to quote, I will also say - "Fuck you too"

@michael-o
Copy link

@GedMarc, I am not. I am using Instant and LocalDate. I need to investigate what is fishy with the Spring context setup here. If am wrong for javax.time I will happily accept the "fuck you too", but the wrong pattern still holds true for Date and Calendar.

About the IP stuff: no, I disagree. If you claim that your stuff is compliant w/o even having read the standard then don't claim to be compliant. It is not my fault that the standard is freely available.

@GedMarc
Copy link

GedMarc commented Dec 5, 2019

K, Have fun. Freely official available one is the open one (use my link in my first comment) and not your customized IP please buy it one.

2017-05-10T07:00:00.000+00:00 which IS 8601 compliant.

It is not ISO8601 compliant - it is custom IP buy this crap 2019-2 compliant.

the ISO 8601 compliant version is simply "2017-05-10T07:00:00.000Z" or without the Z

You need to stop this now.

@michael-o
Copy link

K, Have fun. Freely official available one is the open one (use my link in my first comment) and not your customized IP please buy it one.

I don't understand why you keep wrinting that? As far as I am concerned, I don't have any custom standard here. I have access to the original documents from the standards bodies.

2017-05-10T07:00:00.000+00:00 which IS 8601 compliant.

It is not ISO8601 compliant - it is custom IP buy this crap 2019-2 compliant.

the ISO 8601 compliant version is simply "2017-05-10T07:00:00.000Z" or without the Z

You need to stop this now.

Are you really certain? Chapter 4.3.13 says:

The UTC designator ["Z"] indicates that there is no time shift from UTC of day and is functionally equivalent to the expressions ‘+0000’ and ‘+00:00’. The time shift shall be expressed as positive (i.e. with the leading plus sign [“+”]) if it is ahead of or equal to UTC, and as negative (i.e. with the leading minus sign [“-”]) if it is behind UTC.

I haven't found any spots which either permits or forbids +00:00. I tend to interprete that only Z shall be used.

@michael-o
Copy link

Just verfied, disable WRITE_DATES_AS_TIMESTAMPS and you get the Instant with toString(). The documentation is misleading.

@cowtowncoder
Copy link
Member

Many confusing parts so I am not sure what is supposed to be fixed (wrt date/time types: databind itself only supports java.util.Date / java.util.Calendar)). This question is relevant regarding which specific serializers would need to be modified, and whether this issue should be moved under jackson-modules-java8 (if only relevant to Java8 date/times), or new issue created here (if both).

Further for specific question of including colon or not, my recollection is that JDK formatters themselves are problematic; this may or many not be problematic.

But what is problematic is backwards compatibility: behavior for 3.0 (master) can be changed more easily as it is major version update. Question of what kinds of changes can/should go in minor versions is the main one here. Even if addition is correct and expected from standards perspective it may break existing usage.
If change was deemed acceptable it would need to go in 2.11, at any rate; 2.10.x patches are (IMO) are risky and 2.11 should get into release candidate phase quite soon so that should be acceptable.

So:

  1. Which types are affected? -> Transfer or split this issue, indicating types involved
  2. Should default format change as default for 2.11 or not? Does 3.0 use the right format already?

@michael-o
Copy link

michael-o commented Dec 21, 2019

Further for specific question of including colon or not, my recollection is that JDK formatters themselves are problematic; this may or many not be problematic.

Correct, SimpleDateFormat has too many bugs to be seriously used.

Which types are affected? -> Transfer or split this issue, indicating types involved

Date is Instant, can be converted to and thus using Instant default format. Calendar is either OffsetDateTime or ZonedDateTime.

Should default format change as default for 2.11 or not? Does 3.0 use the right format already?

It should be corrected in 2.x towards a behavioral change in 3.0. The Java 8 module will be of no use because you probably will require Java 8 and thus, it will be the default.

@cowtowncoder
Copy link
Member

By splitting I meant division to different modules: handlers for java.util live in jackson-databind; Joda in jackson-datatype-joda, and Java 8 date/time types in jackon-datatype-jsr310.
So improvements to databind can stay here, but if (and only if) Java 8 handling also needs changes that needs separate issue for that repo.
Handling of Date and Calendar can not and does use Java 8 handlers at all at this point, nor can be changed for 2.x (it would be possible in 3.x as Java 8 can be used by databind, although likely Java 8 date/time support remains a separate module due to sheer size).

For now I assume this is about Date and Calendar serialization only.

Now: use of SimpleDateFormat is something that databind does but only for custom formats.
The main limit for specific issue of inclusion of colon or not is that X specifier was added in JDK7, and jackson-databind for longest time required only JDK6. This has been lifted in 2.8 so use would be possible now.

But actually StdDateFormat that is used for default formatting does not use format string but rather hard-coded default (since any custom variant will not be implemented by it).
And there is method

public StdDateFormat withColonInTimeZone(boolean b) { }

which may be called to change default setting, if configured directly. Looks like it was added for #1744.
It can be used like so:

    StdDateFormat dateFormat = new StdDateFormat()
        .withColonInTimeZone(true);

    ObjectMapper mapper = new ObjectMapper();
    mapper.configure(SerializationFeature.WRITE_DATES_AS_TIMESTAMPS, false);
    mapper.setDateFormat(dateFormat);

which obviously is not good user experience but allowed working around the issue.

Long story short, tho: we could change default for this setting to true for 2.11.
I will send a note on jackson-dev list to suggest change

@cowtowncoder
Copy link
Member

cowtowncoder commented Jan 22, 2020

Sent question, proposing change for 2.11. Concern is the usual backwards compatibility: some users somewhere are almost certainly counting on existing default behavior.
And although I am not too keen to add very specific SerializationFeature, I could live with that for rest of 2.x (setting to be removed from 3.0).

For what it is worth, 3.0 already includes colon by default.

@michael-o
Copy link

@cowtowncoder I agree that it should be changed for 2.11, for 3.0 most of this class (StdDateFormat) can be dropped altogether. In fact, it can be Date or Calendar for the API, shall internally convert to Instant or OffsetDateTime and use formatters there. This will allow to get rid of a lot of cruft.

@cowtowncoder cowtowncoder changed the title WRITE_DATES_AS_TIMESTAMPS documentation confusing Missing colon character in timezone part when serializing java.util.Date/Calendar (ISO-8601 compliancy) Mar 4, 2020
@cowtowncoder
Copy link
Member

Hmmh. Shoot. Was about to add a transient SerializationFeature when I realized that it is not possibly to actually apply it properly, the way things are connected: DateFormat.format() call does not allow passing context, and default format is constructed early for ObjectMapper.
With some nasty hacking it would be possible to add overloads to maybe force settings but it seems perhaps better to simply change default and hope for the best -- StdDateFormat.withColonInTimeZone() is already available since 2.10.

I think this means that there needs to be at least one pre-release/release candidate for 2.11, to let users kick tires.

@cowtowncoder cowtowncoder changed the title Missing colon character in timezone part when serializing java.util.Date/Calendar (ISO-8601 compliancy) Confusing javadoc for SerializationFeature.WRITE_DATES_AS_TIMESTAMPS Mar 5, 2020
@cowtowncoder
Copy link
Member

So, created #2643 for part about timezone offset not being ISO-8601 compliant (but earlier RFC-1123). Tried to improve javadoc for feature, but it is difficult.

@cowtowncoder cowtowncoder removed the 2.11 label Apr 12, 2020
@toolforger
Copy link

Ok let's put this to rest.

I know I'm late, just rectifying some wrong information:

A) An ISO Standard is a global standard and cannot be IP'd by definition.

Global standards are IP'd, just like everything anybody publishes.
(U.S. federal government is the one exception reported. U.S. states are not, and standards bodies are typically not U.S. federal government agencies, they are independent, so copyright law does apply.)

50 years ago, it was customary for standards bodies to charge for their standards, just as a means of financing.
It was okay at the time, as reading standards was relevant just to companies, and these preferred paying a four-digit sum whenever a new version of a standard was published over government influence, so everybody was happy.
Things are getting less okay for the public as standards are referenced by legislation, and for standards bodies as they find the contributions of knowledgeable individuals valuable (which was irrelevant before the Internet because communication was too expensive to make that approach viable).
So there's a trend towards not charging, and while ISO may jump that bandwagon some day, today they certainly haven't - they need some other source of money to finance their expenses (they aren't high compared to the relevance of their publications, but they aren't zero either).

http://www.loc.gov/standards/datetime/iso-tc154-wg5_n0038_iso_wd_8601-1_2016-02-16.pdf

That's not a standard. It's a draft. It explicitly says so on page 1:
"Warning -- This document is not an ISO International Standard. It is distributed for review and comment. It is
subject to change without notice and may not be referred to as an International Standard."

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

7 participants