yonaskolb / SwagGen

OpenAPI/Swagger 3.0 Parser and Swift code generator
MIT License
626 stars 147 forks source link

Enforce 24 hour formatting during date encoding #230

Closed floschliep closed 4 years ago

floschliep commented 4 years ago

Hi @yonaskolb,

thanks a lot for bringing this project to life and maintaining it 🙌 It's a breeze to use!

I recently encountered an issue where the backend failed to parse timestamps generated using our generated code. For example:

2020-03-24T9:56:03Z

The problematic part here is the hour component of the timestamp: 9 According to the format specified by the generated code HH (as well as the format expected by our backend), the hour component should be padded, i.e two digits: 09

After a bit of investigation, it turned out that the affected users in question had set their phones to use 12-hour time instead of 24-hour time. This has two consequences: 1) Between 1 and 9 am/pm, the hour component of the timestamp has a single digit, going against the expected format 2) Generally, even if the timestamp has the expected length (between 10 and 12 am/pm), the hour component doesn't differentiate between e.g. 11am and 11pm (whereas the former should appear as 11 and the latter should appear as 23)

It appears that Apple also officially recommends using the en_US_POSIX locale in these situations - as the formatters for decoding in Coding.swift already do.

The necessary changes to fix this I added in the first commit, e7e9563.

Additionally, while at it, I noticed the formatters don't have the Gregorian calendar set, although specified according to RFC3339, so I took the liberty to also add this in the second commit, bfa0acc. While doing this I noticed DateDay.dateFormatter has set the calendar to current purposefully, so I didn't touch it but wanted to bring it up here instead.

yonaskolb commented 4 years ago

Thanks @floschliep! Could you add a changelog entry, and also run the tests which will generate the fixtures that will need to be committed for CI

floschliep commented 4 years ago

@yonaskolb Sorry for the delay. I rebased the branch and added the changes you mentioned.