[Java][okhttp-gson] Fix LocalDateTime Serialization - #24643
Conversation
There was a problem hiding this comment.
4 issues found across 17 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="samples/client/petstore/java/okhttp-gson-3.1/src/main/java/org/openapitools/client/JSON.java">
<violation number="1" location="samples/client/petstore/java/okhttp-gson-3.1/src/main/java/org/openapitools/client/JSON.java:129">
P2: LocalDateTime request parameters still bypass this adapter, so generated query/path/header/form values do not use the configured formatter and can omit required seconds; include `LocalDateTime` in `ApiClient.parameterToString`’s JSON-serialization branch.</violation>
</file>
<file name="samples/client/echo_api/java/okhttp-gson-user-defined-templates/src/main/java/org/openapitools/client/JSON.java">
<violation number="1" location="samples/client/echo_api/java/okhttp-gson-user-defined-templates/src/main/java/org/openapitools/client/JSON.java:361">
P2: Generated clients cannot configure the new LocalDateTime formatter through `ApiClient`, unlike the existing date and offset-date-time format settings. Adding the matching `ApiClient` forwarding method (and regenerating this sample) would make the new public JSON setting usable through the standard client entry point.</violation>
</file>
<file name="samples/client/petstore/java/okhttp-gson-group-parameter/src/main/java/org/openapitools/client/JSON.java">
<violation number="1" location="samples/client/petstore/java/okhttp-gson-group-parameter/src/main/java/org/openapitools/client/JSON.java:361">
P2: The new LocalDateTime format hook is not available through the generated `ApiClient`, unlike the existing OffsetDateTime and LocalDate hooks, so normal client configuration cannot customize this type. Adding the corresponding `ApiClient` delegate would keep the generated formatting API consistent.</violation>
</file>
<file name="samples/client/petstore/java/okhttp-gson/src/main/java/org/openapitools/client/JSON.java">
<violation number="1" location="samples/client/petstore/java/okhttp-gson/src/main/java/org/openapitools/client/JSON.java:592">
P2: The new `LocalDateTimeTypeAdapter.read` unconditionally rewrites a space at character index 10 to 'T' before calling `LocalDateTime.parse(date, formatter)`, no matter which `DateTimeFormatter` is configured. Since `setLocalDateTimeFormat(...)` lets callers supply a custom space-separated pattern (e.g. `yyyy-MM-dd HH:mm:ss`), this conversion actually destroys the very format the PR is meant to support: an incoming `2013-10-20 15:34:22` becomes `2013-10-20T15:34:22` and then fails to parse against a space-based pattern, throwing `DateTimeParseException`. Consider gating the space-to-'T' normalization on the default `ISO_LOCAL_DATE_TIME` formatter (or only performing it when the configured formatter cannot already handle the space separator), and apply the fix in the generator template so all regenerated samples stay consistent.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| localDateTypeAdapter.setFormat(dateFormat); | ||
| } | ||
|
|
||
| public static void setLocalDateTimeFormat(DateTimeFormatter dateFormat) { |
There was a problem hiding this comment.
P2: The new LocalDateTime format hook is not available through the generated ApiClient, unlike the existing OffsetDateTime and LocalDate hooks, so normal client configuration cannot customize this type. Adding the corresponding ApiClient delegate would keep the generated formatting API consistent.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At samples/client/petstore/java/okhttp-gson-group-parameter/src/main/java/org/openapitools/client/JSON.java, line 361:
<comment>The new LocalDateTime format hook is not available through the generated `ApiClient`, unlike the existing OffsetDateTime and LocalDate hooks, so normal client configuration cannot customize this type. Adding the corresponding `ApiClient` delegate would keep the generated formatting API consistent.</comment>
<file context>
@@ -310,6 +358,10 @@ public static void setLocalDateFormat(DateTimeFormatter dateFormat) {
localDateTypeAdapter.setFormat(dateFormat);
}
+ public static void setLocalDateTimeFormat(DateTimeFormatter dateFormat) {
+ localDateTimeTypeAdapter.setFormat(dateFormat);
+ }
</file context>
| default: | ||
| String date = in.nextString(); | ||
| // RFC 3339 section 5.6 permits a space in place of the ISO 8601 'T' separator | ||
| if (date.length() > 10 && date.charAt(10) == ' ') { |
There was a problem hiding this comment.
P2: The new LocalDateTimeTypeAdapter.read unconditionally rewrites a space at character index 10 to 'T' before calling LocalDateTime.parse(date, formatter), no matter which DateTimeFormatter is configured. Since setLocalDateTimeFormat(...) lets callers supply a custom space-separated pattern (e.g. yyyy-MM-dd HH:mm:ss), this conversion actually destroys the very format the PR is meant to support: an incoming 2013-10-20 15:34:22 becomes 2013-10-20T15:34:22 and then fails to parse against a space-based pattern, throwing DateTimeParseException. Consider gating the space-to-'T' normalization on the default ISO_LOCAL_DATE_TIME formatter (or only performing it when the configured formatter cannot already handle the space separator), and apply the fix in the generator template so all regenerated samples stay consistent.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At samples/client/petstore/java/okhttp-gson/src/main/java/org/openapitools/client/JSON.java, line 592:
<comment>The new `LocalDateTimeTypeAdapter.read` unconditionally rewrites a space at character index 10 to 'T' before calling `LocalDateTime.parse(date, formatter)`, no matter which `DateTimeFormatter` is configured. Since `setLocalDateTimeFormat(...)` lets callers supply a custom space-separated pattern (e.g. `yyyy-MM-dd HH:mm:ss`), this conversion actually destroys the very format the PR is meant to support: an incoming `2013-10-20 15:34:22` becomes `2013-10-20T15:34:22` and then fails to parse against a space-based pattern, throwing `DateTimeParseException`. Consider gating the space-to-'T' normalization on the default `ISO_LOCAL_DATE_TIME` formatter (or only performing it when the configured formatter cannot already handle the space separator), and apply the fix in the generator template so all regenerated samples stay consistent.</comment>
<file context>
@@ -549,6 +552,51 @@ public LocalDate read(JsonReader in) throws IOException {
+ default:
+ String date = in.nextString();
+ // RFC 3339 section 5.6 permits a space in place of the ISO 8601 'T' separator
+ if (date.length() > 10 && date.charAt(10) == ' ') {
+ date = date.substring(0, 10) + 'T' + date.substring(11);
+ }
</file context>
There was a problem hiding this comment.
1 issue found across 34 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="modules/openapi-generator/src/main/resources/Java/libraries/okhttp-gson/JSON.mustache">
<violation number="1" location="modules/openapi-generator/src/main/resources/Java/libraries/okhttp-gson/JSON.mustache:474">
P3: For the space-separated format this PR is adding, `read()` now treats a parse failure as the expected path: every space-separated `LocalDateTime` must first throw and catch a `DateTimeParseException`, then reparse after rewriting the space to 'T'. A server that consistently emits RFC 3339 space-separated datetimes (e.g. `2016-09-09 08:02:03`) will pay the cost of constructing and unwinding an exception for each and every field, rather than on rare error cases. The typical 'T'-separated path is the fast path, so impact is limited, but it may be worth normalizing the separator up front (e.g. checking `charAt(10)` before the first parse) while still only doing so when the formatter can accept it, to avoid exception-based control flow on the supported input format.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| return null; | ||
| default: | ||
| String date = in.nextString(); | ||
| try { |
There was a problem hiding this comment.
P3: For the space-separated format this PR is adding, read() now treats a parse failure as the expected path: every space-separated LocalDateTime must first throw and catch a DateTimeParseException, then reparse after rewriting the space to 'T'. A server that consistently emits RFC 3339 space-separated datetimes (e.g. 2016-09-09 08:02:03) will pay the cost of constructing and unwinding an exception for each and every field, rather than on rare error cases. The typical 'T'-separated path is the fast path, so impact is limited, but it may be worth normalizing the separator up front (e.g. checking charAt(10) before the first parse) while still only doing so when the formatter can accept it, to avoid exception-based control flow on the supported input format.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At modules/openapi-generator/src/main/resources/Java/libraries/okhttp-gson/JSON.mustache, line 474:
<comment>For the space-separated format this PR is adding, `read()` now treats a parse failure as the expected path: every space-separated `LocalDateTime` must first throw and catch a `DateTimeParseException`, then reparse after rewriting the space to 'T'. A server that consistently emits RFC 3339 space-separated datetimes (e.g. `2016-09-09 08:02:03`) will pay the cost of constructing and unwinding an exception for each and every field, rather than on rare error cases. The typical 'T'-separated path is the fast path, so impact is limited, but it may be worth normalizing the separator up front (e.g. checking `charAt(10)` before the first parse) while still only doing so when the formatter can accept it, to avoid exception-based control flow on the supported input format.</comment>
<file context>
@@ -470,11 +471,15 @@ public class JSON {
- // RFC 3339 section 5.6 permits a space in place of the ISO 8601 'T' separator
- if (date.length() > 10 && date.charAt(10) == ' ') {
- date = date.substring(0, 10) + 'T' + date.substring(11);
+ try {
+ return LocalDateTime.parse(date, formatter);
+ } catch (DateTimeParseException e) {
</file context>
[Java] [okhttp-gson] Add `anyOf` Discriminator Support (OpenAPITools#24632) [Java][okhttp-gson] Fix `LocalDateTime` Serialization (OpenAPITools#24643)
Fixes #24642
I added logic to support both
YYYY-MM-DDTHH:mm:ssandYYYY-MM-DD HH:mm:sssince RFC 3339 5.6 has a note about supporting a space instead of theT.PR checklist
Commit all changed files.
This is important, as CI jobs will verify all generator outputs of your HEAD commit as it would merge with master.
These must match the expectations made by your contribution.
You may regenerate an individual generator by passing the relevant config(s) as an argument to the script, for example
./bin/generate-samples.sh bin/configs/java*.IMPORTANT: Do NOT purge/delete any folders/files (e.g. tests) when regenerating the samples as manually written tests may be removed.
@bbdouglas (2017/07) @sreeshas (2017/08) @jfiala (2017/08) @lukoyanov (2017/09) @cbornet (2017/09) @jeff9finger (2018/01) @karismann (2019/03) @Zomzog (2019/04) @lwlee2608 (2019/10) @martin-mfg (2023/08) @KannaKim (2026/07)
Summary by cubic
Fixes
LocalDateTimehandling in Javaokhttp-gsonclients by adding a Gson TypeAdapter and exposing a formatter setter on the generatedApiClient. Inputs with either "T" or a space are accepted and serialize using the configured format.Bug Fixes
LocalDateTimeTypeAdapter(JSR310) and handle RFC 3339 §5.6 space separator on read.ApiClient#setLocalDateTimeFormat(DateTimeFormatter)delegating toJSON.setLocalDateTimeFormat(...).okhttp-gsonsamples.Dependencies
spring-webto 7.0.5 inresttemplate-springBoot4-jackson3sample.Written for commit 7c49e51. Summary will update on new commits.