Return a UTC ZonedDateTime from CargoRoutingDTO::getArrivalDeadline #147
Conversation
This file contains 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
added 2 commits
October 3, 2024 00:06
The convention of assertJ is: assertThat(actual)...(expected) Reversing these arguments creates a misleading message. Clear messages are particularly important for AdminAcceptanceTest::adminSiteCanBookNewCargo, because the expected value is neither constant nor obvious.
By switching the admin/show template from org.thymeleaf.expression.Dates::format to org.thymeleaf.expression.Temporals::format we avoid an implicit conversion via Date::from(Instant), which was introducing coupling to user.timezone, introducing an off-by-one error in the representation of the arrivalDeadline. Temporals::format is also coupled to user.timezone, therefore instead of returning an instant we return a ZonedDateTime. To ensure that our representation of the output date is consistent with the parsing of the input date (see CargoAdminController::register), we specify ZoneOffset.UTC in both places.
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.
Internally, the arrivalDeadline assumes a UTC context; but when returning that information to the web client, the timezone information was being stripped away from the value. This caused the output representation of the arrivalDeadline to be an interpretation of the arrival instant in the user.timezone, rather than in UTC. Thus under certain conditions an off-by-one error was introduced.
The original version of the admin/show template was using org.thymeleaf.expression.Dates, which was expecting that the arrivalDate in the model would be a java.util.Date. As a consequence, deep in the call stack the arrivalDate Instant was being converted via Date.from(Instant). This conversion keeps the instant (epochMillis) constant, but the day/month/year are Date::normalized values, which is to say computed from epochMillis and TimeZone::getDefaultRef....
To eliminate the off by one error, the template is updated to use org.thymeleaf.expression.Temporals::format, which is intended for use with the java.time API. We then load a ZonedDateTime, rather than an Instant, into the model, ensuring that the output data matches the original input.