Copilot commented on code in PR #2535:
URL: https://github.com/apache/groovy/pull/2535#discussion_r3245940332
##########
subprojects/groovy-toml/src/main/java/groovy/toml/TomlSlurper.java:
##########
@@ -126,12 +129,20 @@ public <T> T parseTextAs(Class<T> type, String toml) {
*/
public <T> T parseAs(Class<T> type, Reader reader) {
try {
- return new TomlMapper().readValue(reader, type);
+ return mapper().readValue(reader, type);
} catch (IOException e) {
throw new TomlRuntimeException(e);
}
}
+ static TomlMapper mapper() {
+ return TomlMapper.builder()
+ .addModule(new JavaTimeModule())
+ .disable(SerializationFeature.WRITE_DATES_AS_TIMESTAMPS)
+
.disable(DeserializationFeature.ADJUST_DATES_TO_CONTEXT_TIME_ZONE)
+ .build();
Review Comment:
`mapper()` builds a new `TomlMapper` (and `JavaTimeModule`) on every call.
Since `ObjectMapper`/`TomlMapper` is thread-safe after configuration, this
should be cached (e.g., a `private static final TomlMapper` initialized once)
to avoid repeated allocation/configuration in `parseAs` and
`TomlBuilder.toToml` hot paths.
##########
subprojects/groovy-toml/src/spec/test/groovy/toml/TomlParserTest.groovy:
##########
@@ -163,4 +163,55 @@ port = 8080
def config = new TomlSlurper().parseAs(ServerConfig, stream)
assert config.port == 3000
}
+
+ // tag::temporal_class[]
+ static class Event {
+ java.time.OffsetDateTime created
+ java.time.LocalDate effective
+ java.time.LocalTime windowStart
+ String name
Review Comment:
The user guide claims typed parsing supports `java.time.LocalDateTime`, but
the new round-trip test only covers `OffsetDateTime`, `LocalDate`, and
`LocalTime`. Add a `LocalDateTime` field to `Event` and assert it round-trips
too, so the documented behavior is pinned by tests.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
