swagger-api / swagger-codegen

swagger-codegen contains a template-driven engine to generate documentation, API clients and server stubs in different languages by parsing your OpenAPI / Swagger definition.
http://swagger.io
Apache License 2.0
17.04k stars 6.03k forks source link

Java Client Does Not Format date-time As Per Swagger Spec #1527

Open alexbeardsley opened 9 years ago

alexbeardsley commented 9 years ago

This bug completely breaks any service which strictly enforces RFC3339 formatting (please reference the SimpleDateFormat JavaDoc).

In ApiClient.mustache the constructor is the following:

// Use ISO 8601 format for date and datetime.
// See https://en.wikipedia.org/wiki/ISO_8601
DateFormat dateFormat = new SimpleDateFormat("yyyy-MM-dd'T'HH:mm:ss.SSSZ");

// Use UTC as the default time zone.
dateFormat.setTimeZone(TimeZone.getTimeZone("UTC"));

date-time is supposed to follow RFC3339, however this outputs 2015-11-06T06:49:57.483+0000 which is NOT RFC3339 compliant. It does not output what was likely the desired behavior: 2015-11-06T06:49:57.483Z.

The problem is that Z is a reserved constant to indicate the RFC 822 time zone. It is getting interpreted, so dateFormat.format(new Date()) always has +0000 at the end, not the literal string "Z". This could be solved by replacing Z with XXX (ISO-8601 time zone formatting as of Java 7) or by putting the Z in single quotes like the T is to indicate that the literal string "Z" should always be appended and not an interpreted time zone.

However this is made more complicated because ApiClient delegates date formatting to Jackson in the JSON class when serializing and deserializing data for POST requests. Jackson uses the same date pattern by default when WRITE_DATES_AS_TIMESTAMPS is disabled on the ObjectMapper, meaning it has the same problem where a POST request containing a date-time will always have +0000 instead of "Z". By default Jackson uses StdDateFormat which does fuzzy parsing of dates so the difference in time zone formatting does not break when deserializing.

You can manually set a DateFormat on the ObjectMapper, however that inherently disables the fuzzy parsing. Simply putting yyyy-MM-dd'T'HH:mm:ss.SSSXXX as the format will not work because in RFC3339 the fractional time (SSS) is optional, so if the response does not contain it then the formatter will throw an exception.

There is no good workaround for this bug because the ObjectMapper in the JSON class is private so there is no way to override this behavior for a specific API without modifying the generated model/client classes or swagger-codegen's mustache files.

I am not quite sure what the best solution is here. At the very least, the date formatter for ApiClient and Jackson should be the same, probably by an overridden constructor in JSON.mustache with a DateFormat/pattern parameter that ApiClient could specify.

xhh commented 9 years ago

Please have a look at PR #1534 in which the default date-time format has been updated to conform to RFC3339 and the date formatter for ApiClient and Jackson are updated to be the same.

Regarding fuzzy parsing date strings, I believe most services would use a uniform date format across the system. If so, the ApiClient#setDateFormat could be used to customize that uniform date format (e.g. the RFC3339 format without milliseconds). If it's not the case, for example, a service uses two different date formats in its endpoints, the default Java client does not supports it but the okhttp-gson Java client does support several formats, see here. If there's strong need of this on the default Java client, I'll try to make similar implementation.

alexbeardsley commented 8 years ago

This PR does not solve the problem entirely. The RFC specifically states that fractional time is optional. I agree that a developer being able to set a non-compliant date format is useful. However if there is no fractional time present, it is still valid as per the RFC. I agree that the formatting will probably be uniform across services, however services always passing dates without fractional time is still a valid RFC3339 format. If all of my services do not use fractional time, that is still valid as per the RFC, so should not require any changes by the developer.

xhh commented 8 years ago

@alexbeardsley agreed, but IMHO it would be good enough to make it work by setting the date format manually in that case. If you really want it to work out of box without any manual configuration, could you try the okhttp-gson template and let me know if it what you wants.

Maybe in the future the date handling would be changed to be "fuzzy" to support all cases of RFC3339, as I said before.

alexbeardsley commented 8 years ago

I'll give it a try. Out of the box is not necessary for my case but really it is just the principle. If fuzzy parsing is put in the backlog, having to manually set for non-fractional time should probably at least be documented to save someone some headache in the future.