-
Notifications
You must be signed in to change notification settings - Fork 398
Use a stricter date-time attribute formatter(rfc3339) #235
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
Conversation
thanks for the patch set! I'm happy to drop compatibility for 1.6 but hesitant to drop 1.7 without info on %-age of people are using 1.7 vs 1.8. Would it be hard to make this compatible with joda or some other lib? |
Should not be that hard. I'll look it to it. Thanks for the feedback. |
Tried using joda-time. It looks straight-forward except for the Offset. RFC3339 strictly requires ":" between hours and minutes (HH:mm). Eventhough the javadoc of org.joda.time.format.DateTimeFormatterBuilder#appendTimeZoneOffset(String, boolean, init, int) clearly states if the boolean is set to true, it will include the ":" * @param showSeparators if true, prints ':' separator before minute and
* second field and prints '.' separator before fraction field. The formatter still eventually considers this valid: 2012-12-02T13:05:00+0100 This is the only failing test case left. Will have to take another look. |
Used joda-time lib instead of java.time to make it java 7 compatible. |
sorry for the delay! |
I'm super happy about this patchset as it brings the library closer to conformance. The thing that makes me sad is that we may have current users who are depending on the old behavior and I'm torn on breaking them with no path forward other than freezing their version. Still debating the right move in my head. |
The current behavior is probably actually more disruptive than the fixed behavior for clients. If a user needs the old behavior they should come up with a way to inject a Dictionary so they can use the old behavior. |
Thanks for accepting the PR.
Tuan
…On Mon, Jul 10, 2017 at 10:04 AM, John Huffaker ***@***.***> wrote:
Merged #235
<#235>.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#235 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AJKOsIa6AzCtrdfqJJH-a5BQVitqlkdNks5sMWqMgaJpZM4N2Y3N>
.
--
Dinh, Van Tuan
Phone: +61 3 9214 5417
<http://facebook.com/DvTuan87>
Skype: tuan.dinh.87
http://caia.swin.edu.au/cv/tdinh
|
@tvdinh I'm gonna have to change the default back to the old DateTimeAttribute as it is breaking adoption by other users that were relying on the bad spec. At some point I'll do a major version bump and let them fix their stuff but I'm not there at the moment. You should be able to use your more specific validator by following Example 8 and configuring your Attribute (still in the JAR) for the Library in the ValidationCfg. |
Current date-time attribute formatter does not exactly comply with RFC3339 (required by Swagger specs). This PR provides a stricter one.
Note: using java.time lib, available in Java 8 only.