Fix #92 org.joda.DateTime serialization result is not same as Java 8 ZonedDateTime
#169
Conversation
|
Ok, this, I think, does need to go in 2.20 not 2.19(.1) due to minor changes to internal API, behavior. |
| @@ -251,6 +266,11 @@ public DateTimeFormatter createFormatter(SerializerProvider ctxt) | |||
| formatter = formatter.withZone(DateTimeZone.forTimeZone(tz)); | |||
| } | |||
| } | |||
| if (!ctxt.isEnabled(SerializationFeature.WRITE_DATES_WITH_CONTEXT_TIME_ZONE)) { | |||
| if (valueTimeZone != null && !valueTimeZone.equals(_jdkTimezone)) { | |||
There was a problem hiding this comment.
I don't think this can ever be true as valueTimeZone is Joda's DateTimeZone and _jdkTimeZone is JDK java.util.TimeZone (IDEA flags this as likely flaw).
Should maybe be:
if (valueTimeZone != null && !valueTimeZone.toTimeZone().equals(_jdkTimezone)) {
?
Oh, also maybe null check for _jdkTimeZone to avoid need to convert:
if (valueTimeZone != null) &&
((_jdkTimeZone == null) || !valueTimeZone.toTimeZone().equals(_jdkTimezone))) {
| /** | ||
| * @deprecated since 2.20 Use {@link #createFormatter(SerializerProvider, DateTimeZone)} instead | ||
| */ | ||
| @Deprecated // since 2.20 |
There was a problem hiding this comment.
Ok, looks like there are many references to this now-deprecated method. I'll fix them.
But not 100% sure if this should be deprecated or not.
There was a problem hiding this comment.
Will remove deprecation... I should have reviewed this more carefully; will make some changes in a new PR.
Fixes #92 and #146