From 9b03569dd7cf1bec483ec8dea1f0fcd70f408a2d Mon Sep 17 00:00:00 2001 From: Marten Deinum Date: Thu, 8 Aug 2024 15:29:29 +0200 Subject: [PATCH] Improve IsoFormattingDateDataFormatter Prior to this commit all date related fields were formatted as an ISO_LOCAL_DATE_TIME. With this commit we inspect the type it actually is date, time or datetime and use the proper ISO_LOCAL formatting for that. Another improvement is to use the cached result for the formula instead of re-evaluating the formula and use that type. --- .../excel/IsoFormattingDateDataFormatter.java | 39 +++++++---- ...s.java => PoiItemReaderXlsTypesTests.java} | 8 +-- .../poi/PoiItemReaderXlsxTypesTests.java | 65 +++++++++++++++++++ .../streaming/StreamingXlsxTypesTests.java | 7 +- 4 files changed, 98 insertions(+), 21 deletions(-) rename spring-batch-excel/src/test/java/org/springframework/batch/extensions/excel/poi/{PoiItemReaderTypesTests.java => PoiItemReaderXlsTypesTests.java} (88%) create mode 100644 spring-batch-excel/src/test/java/org/springframework/batch/extensions/excel/poi/PoiItemReaderXlsxTypesTests.java diff --git a/spring-batch-excel/src/main/java/org/springframework/batch/extensions/excel/IsoFormattingDateDataFormatter.java b/spring-batch-excel/src/main/java/org/springframework/batch/extensions/excel/IsoFormattingDateDataFormatter.java index 4cb48a00..7d3e4f62 100644 --- a/spring-batch-excel/src/main/java/org/springframework/batch/extensions/excel/IsoFormattingDateDataFormatter.java +++ b/spring-batch-excel/src/main/java/org/springframework/batch/extensions/excel/IsoFormattingDateDataFormatter.java @@ -25,14 +25,13 @@ import org.apache.poi.ss.usermodel.CellType; import org.apache.poi.ss.usermodel.DataFormatter; import org.apache.poi.ss.usermodel.DateUtil; +import org.apache.poi.ss.usermodel.ExcelNumberFormat; import org.apache.poi.ss.usermodel.FormulaEvaluator; /** * Specialized subclass for formatting the date into an ISO date/time and ignore the format as given in the Excel file. * * @author Marten Deinum - * - * @see DateTimeFormatter#ISO_OFFSET_DATE_TIME */ public class IsoFormattingDateDataFormatter extends DataFormatter { @@ -46,9 +45,10 @@ public IsoFormattingDateDataFormatter(Locale locale) { @Override public String formatRawCellContents(double value, int formatIndex, String formatString, boolean use1904Windowing) { + if (DateUtil.isADateFormat(formatIndex, formatString) && DateUtil.isValidExcelDate(value)) { - return super.formatRawCellContents(value, formatIndex, "yyyy-MM-ddTHH:mm:ss", - use1904Windowing); + String formatToUse = determineFormat(formatIndex); + return super.formatRawCellContents(value, formatIndex, formatToUse, use1904Windowing); } return super.formatRawCellContents(value, formatIndex, formatString, use1904Windowing); } @@ -60,17 +60,34 @@ public String formatCellValue(Cell cell, FormulaEvaluator evaluator, Conditional } CellType cellType = cell.getCellType(); - if (cellType == CellType.FORMULA) { - if (evaluator == null) { - return cell.getCellFormula(); - } - cellType = evaluator.evaluateFormulaCell(cell); + if (cellType == CellType.FORMULA && useCachedValuesForFormulaCells()) { + cellType = cell.getCachedFormulaResultType(); } - if (cellType == CellType.NUMERIC && DateUtil.isCellDateFormatted(cell, cfEvaluator)) { + if (cellType != CellType.STRING && DateUtil.isCellDateFormatted(cell, cfEvaluator)) { + String formatToUse = determineFormat(ExcelNumberFormat.from(cell, cfEvaluator).getIdx()); LocalDateTime value = cell.getLocalDateTimeCellValue(); - return (value != null) ? value.format(DateTimeFormatter.ISO_LOCAL_DATE_TIME) : ""; + return (value != null) ? value.format(DateTimeFormatter.ofPattern(formatToUse)) : ""; } return super.formatCellValue(cell, evaluator, cfEvaluator); } + + /** + * Determine the format to use for either date, time of datetime. Based on the internal formats used by Excel. + * 14, 15, 16, 17 are dates only + * 18, 19, 20, 21 are times only + * anything else is interpreted as a datetime, including custom formats that might be in use! + * @param formatIndex the format index from excel. + * @return the format to use, never {@code null}. + */ + + private String determineFormat(int formatIndex) { + if (formatIndex >= 14 && formatIndex < 18) { + return "yyyy-MM-dd"; + } + else if (formatIndex >= 18 && formatIndex < 22) { + return "HH:mm:ss"; + } + return "yyyy-MM-dd'T'HH:mm:ss"; + } } diff --git a/spring-batch-excel/src/test/java/org/springframework/batch/extensions/excel/poi/PoiItemReaderTypesTests.java b/spring-batch-excel/src/test/java/org/springframework/batch/extensions/excel/poi/PoiItemReaderXlsTypesTests.java similarity index 88% rename from spring-batch-excel/src/test/java/org/springframework/batch/extensions/excel/poi/PoiItemReaderTypesTests.java rename to spring-batch-excel/src/test/java/org/springframework/batch/extensions/excel/poi/PoiItemReaderXlsTypesTests.java index 60534f03..2c804c4d 100644 --- a/spring-batch-excel/src/test/java/org/springframework/batch/extensions/excel/poi/PoiItemReaderTypesTests.java +++ b/spring-batch-excel/src/test/java/org/springframework/batch/extensions/excel/poi/PoiItemReaderXlsTypesTests.java @@ -27,7 +27,7 @@ import static org.assertj.core.api.Assertions.assertThat; -class PoiItemReaderTypesTests { +class PoiItemReaderXlsTypesTests { @Test void shouldBeAbleToReadMultipleTypes() throws Exception { @@ -38,7 +38,6 @@ void shouldBeAbleToReadMultipleTypes() throws Exception { reader.setUserLocale(Locale.US); // Use a Locale to not be dependent on environment reader.afterPropertiesSet(); - reader.open(new ExecutionContext()); var row1 = reader.read(); @@ -62,8 +61,7 @@ void shouldBeAbleToReadMultipleTypesWithDatesAsIso() throws Exception { var row1 = reader.read(); var row2 = reader.read(); - assertThat(row1).containsExactly("1", "1.0", "2024-05-12T00:00:00", "1899-12-31T13:14:55", "2024-05-12T13:14:55", "hello world"); - assertThat(row2).containsExactly("2", "2.5", "2023-08-08T00:00:00", "1899-12-31T11:12:13", "2023-08-08T11:12:13", "world hello"); - + assertThat(row1).containsExactly("1", "1.0", "2024-05-12", "13:14:55", "2024-05-12T13:14:55", "hello world"); + assertThat(row2).containsExactly("2", "2.5", "2023-08-08", "11:12:13", "2023-08-08T11:12:13", "world hello"); } } diff --git a/spring-batch-excel/src/test/java/org/springframework/batch/extensions/excel/poi/PoiItemReaderXlsxTypesTests.java b/spring-batch-excel/src/test/java/org/springframework/batch/extensions/excel/poi/PoiItemReaderXlsxTypesTests.java new file mode 100644 index 00000000..5add1ffe --- /dev/null +++ b/spring-batch-excel/src/test/java/org/springframework/batch/extensions/excel/poi/PoiItemReaderXlsxTypesTests.java @@ -0,0 +1,65 @@ +/* + * Copyright 2002-2024 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.batch.extensions.excel.poi; + +import java.util.Locale; + +import org.junit.jupiter.api.Test; + +import org.springframework.batch.extensions.excel.mapping.PassThroughRowMapper; +import org.springframework.batch.item.ExecutionContext; +import org.springframework.core.io.ClassPathResource; + +import static org.assertj.core.api.Assertions.assertThat; + +class PoiItemReaderXlsxTypesTests { + + @Test + void shouldBeAbleToReadMultipleTypes() throws Exception { + var reader = new PoiItemReader(); + reader.setResource(new ClassPathResource("types.xlsx")); + reader.setRowMapper(new PassThroughRowMapper()); + reader.setLinesToSkip(1); // Skip header + reader.setUserLocale(Locale.US); // Use a Locale to not be dependent on environment + reader.afterPropertiesSet(); + + reader.open(new ExecutionContext()); + + var row1 = reader.read(); + var row2 = reader.read(); + assertThat(row1).containsExactly("1", "1.0", "5/12/24", "13:14:55", "5/12/24 13:14", "hello world"); + assertThat(row2).containsExactly("2", "2.5", "8/8/23", "11:12:13", "8/8/23 11:12", "world hello"); + } + + @Test + void shouldBeAbleToReadMultipleTypesWithDatesAsIso() throws Exception { + var reader = new PoiItemReader(); + reader.setResource(new ClassPathResource("types.xls")); + reader.setRowMapper(new PassThroughRowMapper()); + reader.setLinesToSkip(1); // Skip header + reader.setUserLocale(Locale.US); // Use a Locale to not be dependent on environment + reader.setDatesAsIso(true); + reader.afterPropertiesSet(); + + reader.open(new ExecutionContext()); + + var row1 = reader.read(); + var row2 = reader.read(); + assertThat(row1).containsExactly("1", "1.0", "2024-05-12", "13:14:55", "2024-05-12T13:14:55", "hello world"); + assertThat(row2).containsExactly("2", "2.5", "2023-08-08", "11:12:13", "2023-08-08T11:12:13", "world hello"); + } +} diff --git a/spring-batch-excel/src/test/java/org/springframework/batch/extensions/excel/streaming/StreamingXlsxTypesTests.java b/spring-batch-excel/src/test/java/org/springframework/batch/extensions/excel/streaming/StreamingXlsxTypesTests.java index 4518b0da..7489d76b 100644 --- a/spring-batch-excel/src/test/java/org/springframework/batch/extensions/excel/streaming/StreamingXlsxTypesTests.java +++ b/spring-batch-excel/src/test/java/org/springframework/batch/extensions/excel/streaming/StreamingXlsxTypesTests.java @@ -43,7 +43,6 @@ void shouldBeAbleToReadMultipleTypes() throws Exception { var row2 = reader.read(); assertThat(row1).containsExactly("1", "1.0", "5/12/24", "13:14:55", "5/12/24 13:14", "hello world"); assertThat(row2).containsExactly("2", "2.5", "8/8/23", "11:12:13", "8/8/23 11:12", "world hello"); - } @Test @@ -60,9 +59,7 @@ void shouldBeAbleToReadMultipleTypesWithDatesAsIso() throws Exception { var row1 = reader.read(); var row2 = reader.read(); - assertThat(row1).containsExactly("1", "1.0", "2024-05-12T00:00:00", "1899-12-31T13:14:55", "2024-05-12T13:14:55", "hello world"); - assertThat(row2).containsExactly("2", "2.5", "2023-08-08T00:00:00", "1899-12-31T11:12:13", "2023-08-08T11:12:13", "world hello"); - + assertThat(row1).containsExactly("1", "1.0", "2024-05-12", "13:14:55", "2024-05-12T13:14:55", "hello world"); + assertThat(row2).containsExactly("2", "2.5", "2023-08-08", "11:12:13", "2023-08-08T11:12:13", "world hello"); } - }