-
Notifications
You must be signed in to change notification settings - Fork 122
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
Fix #279 OffsetDateTime
deserialization fail with date-time pattern
#363
base: 2.19
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,4 @@ | ||
package com.fasterxml.jackson.datatype.jsr310.tofix; | ||
package com.fasterxml.jackson.datatype.jsr310.deser; | ||
|
||
import java.time.OffsetDateTime; | ||
import java.time.ZoneId; | ||
|
@@ -9,20 +9,20 @@ | |
import com.fasterxml.jackson.annotation.JsonFormat; | ||
import com.fasterxml.jackson.databind.ObjectMapper; | ||
import com.fasterxml.jackson.datatype.jsr310.ModuleTestBase; | ||
import com.fasterxml.jackson.datatype.jsr310.testutil.failure.JacksonTestFailureExpected; | ||
|
||
import static org.junit.jupiter.api.Assertions.*; | ||
|
||
public class OffsetDateTimeDeser279Test extends ModuleTestBase | ||
// [modules-java8#279] OffsetDateTimeDeserializer fails to parse date-time string with 'Z' at the end | ||
public class OffsetDateTimeDeser279Test | ||
extends ModuleTestBase | ||
{ | ||
// For [modules-java8#279] | ||
static class Wrapper279 { | ||
OffsetDateTime date; | ||
|
||
public Wrapper279(OffsetDateTime d) { date = d; } | ||
protected Wrapper279() { } | ||
|
||
@JsonFormat(shape = JsonFormat.Shape.STRING, pattern = "yyyy-MM-dd'T'HH:mm:ss'Z'") | ||
@JsonFormat(shape = JsonFormat.Shape.STRING, pattern = "yyyy-MM-dd'T'HH:mm:ss'Z'") // | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Any suggestions on patterns to test? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I can't believe I suggest this but... my hesitation on considering quoted 'Z' as anything other than constant character to include (and NOT have any semantic effect) makes me wonder if someone should just ask ChatGPT (or LLM-based code assistants) true meaning of the pattern here, its semantics? It could be the original bug report itself might be wrong. Consider
instead? Or, if not, shouldn't affect processing of underlying date/time value in any way (just included as effectively decoration) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Answer from GPT below There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In Java's date and time formatting, single quotes (') are used to denote literal characters within a pattern string. This means that any text enclosed in single quotes is treated as-is and does not have any special meaning in terms of date or time components. Understanding the 'Z' Character: Without Single Quotes (Z): In date-time formatting patterns, an unquoted Z typically represents the RFC 822 time zone, which corresponds to the time zone offset from UTC. For example, in the pattern "yyyy-MM-dd'T'HH:mm:ssZ", the Z at the end expects a numeric time zone offset like +0800 or -0500. With Single Quotes ('Z'): When Z is enclosed in single quotes, as in "yyyy-MM-dd'T'HH:mm:ss'Z'", it is treated as a literal character. This means the formatter expects the character Z to appear exactly at that position in the date-time string, without interpreting it as a time zone indicator. Practical Implications: Parsing: If you have a date-time string like "2025-03-13T13:29:57Z", where Z indicates UTC time (also known as "Zulu" time), using the pattern "yyyy-MM-dd'T'HH:mm:ss'Z'" will treat the Z as a literal character. This means the parser will expect the Z in the string but won't interpret it as indicating the UTC time zone. As a result, the parsed date-time may not be correctly set to UTC. Formatting: When formatting a date-time object to a string, using the pattern "yyyy-MM-dd'T'HH:mm:ss'Z'" will produce a string with a literal Z at the end, regardless of the actual time zone of the date-time object. For example, it might produce "2025-03-13T13:29:57Z", even if the date-time is not in UTC. Recommendation: To correctly parse and format date-time strings that include a Z to indicate UTC time, it's advisable to use patterns that interpret the Z appropriately. For ISO 8601 date-times, the X pattern letter is suitable: Pattern: "yyyy-MM-dd'T'HH:mm:ssX" Example: java For more details on date-time formatting patterns, you can refer to the SimpleDateFormat documentation. Sources There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Re-reading the original issue makes me think that we need to organize things into a list of what input/output we expect There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ChatGPT gave seemingly extensive and also (yay!) accurate answer. |
||
public OffsetDateTime getDate() { | ||
return date; | ||
} | ||
|
@@ -31,18 +31,22 @@ public void setDate(OffsetDateTime date) { | |
} | ||
} | ||
|
||
private ObjectMapper MAPPER = newMapper(); | ||
private final ObjectMapper MAPPER = newMapper(); | ||
|
||
// For [modules-java8#279] | ||
@JacksonTestFailureExpected | ||
@Test | ||
public void testWrapperWithPattern279() throws Exception | ||
public void testWrapperWithPattern279() | ||
throws Exception | ||
{ | ||
final OffsetDateTime date = OffsetDateTime.now(ZoneId.of("UTC")) | ||
OffsetDateTime date = OffsetDateTime.now(ZoneId.of("UTC")) | ||
.withYear(2025).withMonth(3).withDayOfMonth(13).withHour(13).withMinute(29).withSecond(57).withNano(0) | ||
.truncatedTo(ChronoUnit.SECONDS); | ||
final Wrapper279 input = new Wrapper279(date); | ||
final String json = MAPPER.writeValueAsString(input); | ||
Wrapper279 input = new Wrapper279(date); | ||
|
||
// serialization first | ||
String json = MAPPER.writeValueAsString(input); | ||
assertEquals("{\"date\":\"2025-03-13T13:29:57Z\"}", json); | ||
|
||
// deserialization second | ||
Wrapper279 result = MAPPER.readValue(json, Wrapper279.class); | ||
assertEquals(input.date, result.date); | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Super single-case solution... may be improve handling along the way