Fix @QueryValue of LocalDateTime type for 3.10.x #9970
Conversation
|
|
|
@sdelamo I tried to backport code from 4.x but it's really painfull because of Java 8 requirement |
rlconst
force-pushed
the
3.10.x
branch
2 times, most recently
from
dc9c4ad
to
dae72a7
Compare
|
Removed irrelevan code formatting |
| try { | ||
| DateTimeFormatter formatter = resolveFormatter(context); | ||
| // Treat missing zone as UTC | ||
| return Optional.of(formatter.format(object.atOffset(ZoneOffset.UTC))); |
There was a problem hiding this comment.
Why not ZoneOffset.systemDefault?
There was a problem hiding this comment.
im against any implicit time zone at all, be it UTC or systemDefault. Why does this converter need to support RFC 1123? RFC 1123 makes no sense for LDT, we should not support it just like the JDK doesnt support it.
There was a problem hiding this comment.
@dstepanov because systemDefault offset on client and server could be different
There was a problem hiding this comment.
@yawkat I tried to minimize changes without changing the current semantic, especialy it was properly handled in 4.x
|
I added a test to the TCK. However, I don't reproduce the issue #10027 |
|
@sdelamo what the difference between TCK and DateTimeConversionSpec? Because if I rollback changes in converter, DateTimeConversionSpec reproducibly fails |
|
ok, ive looked at the code some more. the reason why it works on 4 but not 3 is that we changed away from using RFC 1123 datetime by default, to ISO 8601 timestamps. I am against fixing this by adding a timezone out of thin air. However I think an alternative solution is possible and I will update this PR. |
Fix for #9969 with test case