-
Notifications
You must be signed in to change notification settings - Fork 261
IsoFormattingDateDataFormatter does not work #121
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Comments
Dude, this is hardly related to Spring Batch. |
Ok, let me be more explicit then. There is an IsoFormattingDateDataFormatter class in the project: https://github.com/spring-projects/spring-batch-extensions/blob/main/spring-batch-excel/src/main/java/org/springframework/batch/extensions/excel/IsoFormattingDateDataFormatter.java This class does not use the Java APIs correctly. When it converts a date cell to an ISO_OFFSET_DATE_TIME, it does the following: if (cellType == CellType.NUMERIC && DateUtil.isCellDateFormatted(cell, cfEvaluator)) {
LocalDateTime value = cell.getLocalDateTimeCellValue();
return (value != null) ? value.format(DateTimeFormatter.ISO_OFFSET_DATE_TIME) : "";
} As I wrote previously, a LocalDateTime can never be formatted with Not even sure what the intention is here since Excel itself does not deal with time zones, dates are always local. So if this feature in Spring Batch is supposed to convert an Excel date to an ISO offset format, it should first convert the LocalDateTime to a ZonedDateTime by picking a time zone. That time zone should probably be configurable as well since you cannot just pick an arbitrary time zone, it really depends on the contents of the actual Excel file. If it is not configurable, then the documentation should clearly state which time zone Spring Batch is going to use. |
This was an oversight on my part and this is what you get for not writing a proper unit test for the class in question. Just to clarify this isn't part of Spring Batch but rather the Spring Batch Extensions project, the latter is community driven and isn't part of Spring Batch itself. See #100 and #98 for why this class exists and what the intent of it is. |
I also ran into this issue and was wondering whether there are any plans on fixing it? Seems to me like the solution is simply to replace these 2 lines with the following code:
This would be sufficient to fix it as the code exclusively uses LocalDateTime. Excel does not support time zones, so there is no point in using a formatter like ISO_OFFSET_DATE_TIME that expects a timezone and only works with ZonedDateTimes. ISO_LOCAL_DATE_TIME uses the exact same date-time format just without the timezone information. In my opinion this behaviour should have been the default from the start as the current default mapping does not give any indication about the formatting or locale the user set on the cell making it impossible to interpret the date string correctly in Java. |
IsoFormattingDateDataFormatter reads the value as a LocalDateTime and then attempts to format it as an ISO_OFFSET_DATE_TIME. A LocalDateTime does not have zone offset information, therefore this always fails with an exception:
java.time.temporal.UnsupportedTemporalTypeException: Unsupported field: OffsetSeconds
A simple test to test this:
Am I missing something?
The text was updated successfully, but these errors were encountered: