[dart] Do not convert format: date to UTC before formatting - #24706
Open
eirikb wants to merge 2 commits into
Open
[dart] Do not convert format: date to UTC before formatting#24706eirikb wants to merge 2 commits into
format: date to UTC before formatting#24706eirikb wants to merge 2 commits into
Conversation
_dateFormatter is DateFormat('yyyy-MM-dd'), which formats the y/m/d the
DateTime already carries and performs no timezone conversion. Calling
toUtc() first therefore does nothing except roll the clock back past
midnight in UTC+X zones, so the formatter prints the previous day.
Combined with mapDateTime parsing the bare wire value "2026-09-12" as
local midnight, the round trip in Europe/Oslo is "2026-09-12" ->
"2026-09-11". It is stable in UTC, which is why this went unnoticed.
Removing toUtc() is safe in both directions: toUtc() returns this when
the DateTime is already UTC, so UTC callers are unaffected, and local
callers stop being shifted. The neighbouring isDateTime branch keeps
toUtc() - an instant needs a zone, a calendar date does not. dart-dio
already models this correctly with its own Date class.
fix OpenAPITools#24703
The first test only exercised the plain, no-pattern branch, so three of the
four changed template lines were unguarded. Adding a patterned date to the
fixture plus a second test that generates with useOptional=true pins all
four:
plain / no pattern requiredDate, optionalDate
plain / pattern patternedDate
Optional / no pattern optionalDate (useOptional=true)
Optional / pattern patternedDate (useOptional=true)
Replace the blanket assertFileNotContains(".toUtc()") with assertions on the
specific emitted lines. The blanket one would break if an unrelated
date-time property were ever added to the shared fixture.
Also correct the comment: toUtc() shifts the date in both directions, back a
day east of UTC and forward a day west of it, not only backwards.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
fix #24703
The dart generator emits this for a
format: dateproperty:_dateFormatterisDateFormat('yyyy-MM-dd'), which formats the y/m/d the DateTime already carries. It does no timezone conversion, so that.toUtc()does nothing except move the value across a day boundary before the date is read.Coming the other way,
mapDateTimeparses the bare wire value"2026-09-12"withDateTime.tryParse, which gives you local midnight. Round trip in Oslo:It is stable in UTC, which I bet is why this has been around so long. We only noticed because a payment deadline got printed one day early on a physical parking fine.
Removing
.toUtc()should be safe in both directions.toUtc()returnsthiswhen the DateTime is already UTC, so UTC callers are unaffected, and local callers stop being shifted. TheisDateTimebranch right next to it keeps its.toUtc(), an instant needs a zone, a calendar date does not.FWIW
dart-dioalready handles this correctly with its ownDateclass (plain year/month/day ints), so the two dart generators disagree about this today.Tests
Added a
DateOnlyModelfixture and two tests that between them pin all four changed template lines:requiredDate,optionalDatepatternedDateOptional<T>, no patternoptionalDatewithuseOptional=trueOptional<T>, with patternpatternedDatewithuseOptional=trueBoth tests fail against the old template and pass with the fix. Samples regenerated, it comes out as two lines.
Not fixed here
While I was in there I noticed
format: dateis also wrong in a few other places: query, header and form parameters go throughparameterToString, which is type-erased and cannot telldatefromdate-time, path parameters use plain.toString(), and arrays of dates do not compile at all (DateTime.listFromJsondoes not exist). There is also an epoch-marker branch that still normalizes to UTC, and_dateFormatterhas no explicit locale so it can emit non-ASCII digits ifIntl.defaultLocaleis set.Those are separate defects with separate fixes and much bigger diffs, so I left them alone to keep this one small. Happy to look at them after if you want.
PR checklist
dart-petstore-client-lib.yamlanddart-petstore-client-lib-fake.yaml, since the template only affects dart native.)@jaumard @amondnet @sbu-WBT @kuhnroyal @agilob @ahmednfwela
Summary by cubic
Stop converting Dart
format: datefields to UTC before formatting. Previously we emitted_dateFormatter.format(value.toUtc()), which shifted local-midnight dates across day boundaries and serialized the wrong day; now we call_dateFormatter.format(value). Theformat: date-timepath is unchanged and still uses.toUtc().Review notes
modules/openapi-generator/src/main/resources/dart2/serialization/native/native_class.mustacheto remove.toUtc()in allisDatebranches (plain and pattern).DartClientCodegenTestplus aDateOnlyModelfixture to cover plain/pattern anduseOptional=truepaths; assertions check the exact emitted lines.format_test.dartandnullable_class.dart.Impact
DateTime.Written for commit e27bf96. Summary will update on new commits.