Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes a critical thread safety issue in the CustomDateSerializer class by removing static SimpleDateFormat instances and creating new instances per serialization call instead.
Key Changes
- Removed static
DATE_FORMATandCALENDAR_FORMATfields that caused thread safety issues - Refactored
serialize()method to instantiateSimpleDateFormatlocally for each call
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| SimpleDateFormat formatter; | ||
| if (value instanceof java.sql.Date) { | ||
| gen.writeString(CALENDAR_FORMAT.format(value)); | ||
| formatter = new SimpleDateFormat("yyyy-MM-dd"); | ||
| } else { | ||
| gen.writeString(DATE_FORMAT.format(value)); | ||
| formatter = new SimpleDateFormat("yyyy-MM-dd HH:mm:ss"); | ||
| } | ||
| gen.writeString(formatter.format(value)); |
There was a problem hiding this comment.
While creating a new SimpleDateFormat instance per call fixes the thread safety issue, this approach has a performance cost. Consider using DateTimeFormatter instead, which is thread-safe and can be stored as a static final field (similar to CustomLocalDateTimeSerializer).
For example:
private static final DateTimeFormatter DATE_FORMATTER = DateTimeFormatter.ofPattern("yyyy-MM-dd");
private static final DateTimeFormatter DATETIME_FORMATTER = DateTimeFormatter.ofPattern("yyyy-MM-dd HH:mm:ss");
@Override
public void serialize(Date value, JsonGenerator gen, SerializerProvider provider) throws IOException {
if (value instanceof java.sql.Date) {
gen.writeString(DATE_FORMATTER.format(value.toInstant().atZone(ZoneId.systemDefault()).toLocalDate()));
} else {
gen.writeString(DATETIME_FORMATTER.format(value.toInstant().atZone(ZoneId.systemDefault()).toLocalDateTime()));
}
}Alternatively, if you need to stick with SimpleDateFormat, use ThreadLocal<SimpleDateFormat> to avoid creating new instances on every call while maintaining thread safety.
No description provided.