From 1fe89c11d2c51a9e753ea18851b73067d1f0315e Mon Sep 17 00:00:00 2001 From: Mikkey-f <178465103+Mikkey-f@users.noreply.github.com> Date: Sun, 11 Oct 2026 18:38:32 +0800 Subject: [PATCH 1/2] fix: resolve header cells against the built-in read converters (#1098) ConverterUtils.convertToStringMap resolved the converter for header cells through the holder's converter map, which also carries user registrations, so a user-registered Converter rewrote the header text itself: "flag" became "FLAG", @ExcelProperty name matching failed, and the field was silently left null (the data cell was not converted either). The header path now uses the built-in read converters only; the data-cell path is untouched. ConverterUtilsTest is updated to the new contract: the convertToStringMap tests now assert that a user registration under the same key is not consulted, and the missing-converter case uses DATE, which has no built-in (String, DATE) converter (BOOLEAN does). invokeHeadMap now receives the raw header map, as settled in #1098. --- .../fesod/sheet/util/ConverterUtils.java | 11 +- .../read/HeaderConverterIsolationTest.java | 167 ++++++++++++++++++ .../fesod/sheet/util/ConverterUtilsTest.java | 19 +- 3 files changed, 187 insertions(+), 10 deletions(-) create mode 100644 fesod-sheet/src/test/java/org/apache/fesod/sheet/read/HeaderConverterIsolationTest.java diff --git a/fesod-sheet/src/main/java/org/apache/fesod/sheet/util/ConverterUtils.java b/fesod-sheet/src/main/java/org/apache/fesod/sheet/util/ConverterUtils.java index 007c3e6a8..baa297f16 100644 --- a/fesod-sheet/src/main/java/org/apache/fesod/sheet/util/ConverterUtils.java +++ b/fesod-sheet/src/main/java/org/apache/fesod/sheet/util/ConverterUtils.java @@ -34,6 +34,7 @@ import org.apache.fesod.sheet.converters.Converter; import org.apache.fesod.sheet.converters.ConverterKeyBuild; import org.apache.fesod.sheet.converters.ConverterKeyBuild.ConverterKey; +import org.apache.fesod.sheet.converters.DefaultConverterLoader; import org.apache.fesod.sheet.converters.NullableObjectConverter; import org.apache.fesod.sheet.converters.ReadConverterContext; import org.apache.fesod.sheet.enums.CellDataTypeEnum; @@ -41,7 +42,6 @@ import org.apache.fesod.sheet.metadata.data.CellData; import org.apache.fesod.sheet.metadata.data.ReadCellData; import org.apache.fesod.sheet.metadata.property.ExcelContentProperty; -import org.apache.fesod.sheet.read.metadata.holder.ReadSheetHolder; /** * Converting objects @@ -63,7 +63,6 @@ private ConverterUtils() {} public static Map convertToStringMap( Map> cellDataMap, AnalysisContext context) { Map stringMap = MapUtils.newHashMapWithExpectedSize(cellDataMap.size()); - ReadSheetHolder readSheetHolder = context.readSheetHolder(); int index = 0; for (Map.Entry> entry : cellDataMap.entrySet()) { Integer key = entry.getKey(); @@ -77,8 +76,12 @@ public static Map convertToStringMap( stringMap.put(key, null); continue; } - Converter converter = - readSheetHolder.converterMap().get(ConverterKeyBuild.buildKey(String.class, cellData.getType())); + // Resolve against the built-in read converters only. The header path must keep the + // file's raw header text: user-registered converters apply to data cells, so letting + // them rewrite the header would silently break @ExcelProperty name matching and the + // map handed to invokeHeadMap. See issue #1098. + Converter converter = DefaultConverterLoader.loadDefaultReadConverter() + .get(ConverterKeyBuild.buildKey(String.class, cellData.getType())); if (converter == null) { throw new ExcelDataConvertException( context.readRowHolder().getRowIndex(), diff --git a/fesod-sheet/src/test/java/org/apache/fesod/sheet/read/HeaderConverterIsolationTest.java b/fesod-sheet/src/test/java/org/apache/fesod/sheet/read/HeaderConverterIsolationTest.java new file mode 100644 index 000000000..6849040db --- /dev/null +++ b/fesod-sheet/src/test/java/org/apache/fesod/sheet/read/HeaderConverterIsolationTest.java @@ -0,0 +1,167 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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 + * + * http://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.apache.fesod.sheet.read; + +import java.io.File; +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; +import java.util.Map; +import lombok.Data; +import org.apache.fesod.sheet.FesodSheet; +import org.apache.fesod.sheet.annotation.ExcelProperty; +import org.apache.fesod.sheet.context.AnalysisContext; +import org.apache.fesod.sheet.converters.Converter; +import org.apache.fesod.sheet.enums.CellDataTypeEnum; +import org.apache.fesod.sheet.event.AnalysisEventListener; +import org.apache.fesod.sheet.metadata.GlobalConfiguration; +import org.apache.fesod.sheet.metadata.data.ReadCellData; +import org.apache.fesod.sheet.metadata.property.ExcelContentProperty; +import org.apache.fesod.sheet.testkit.Tags; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Tag; +import org.junit.jupiter.api.Test; + +/** + * The header text used for {@code @ExcelProperty} name matching — and handed to + * {@link AnalysisEventListener#invokeHeadMap} — must be resolved against the built-in read + * converters only. A user-registered {@code Converter} applies to data cells, never to + * the header. See issue #1098. + */ +@Tag(Tags.READ) +class HeaderConverterIsolationTest { + + private static final String HEADER = "flag"; + private static final String CELL_VALUE = "abc"; + + @Data + public static class FlagRow { + @ExcelProperty(HEADER) + private String flag; + } + + /** Explicit {@code STRING}-key converter: data cells are upper-cased. */ + public static class UpperCaseStringConverter implements Converter { + @Override + public Class supportJavaTypeKey() { + return String.class; + } + + @Override + public CellDataTypeEnum supportExcelTypeKey() { + return CellDataTypeEnum.STRING; + } + + @Override + public String convertToJavaData( + ReadCellData cellData, + ExcelContentProperty contentProperty, + GlobalConfiguration globalConfiguration) { + return cellData.getStringValue().toUpperCase(); + } + } + + /** Wildcard variant: declares no excel type key ({@code supportExcelTypeKey() == null}). */ + public static class WildcardUpperCaseStringConverter extends UpperCaseStringConverter { + @Override + public CellDataTypeEnum supportExcelTypeKey() { + return null; + } + } + + /** Captures both the header map handed to the listener and the mapped rows. */ + private static class CapturingListener extends AnalysisEventListener { + private Map headMap; + private final List rows = new ArrayList<>(); + + @Override + public void invokeHeadMap(Map headMap, AnalysisContext context) { + this.headMap = headMap; + } + + @Override + public void invoke(FlagRow data, AnalysisContext context) { + rows.add(data); + } + + @Override + public void doAfterAllAnalysed(AnalysisContext context) {} + } + + @Test + void headerStaysRaw_soNameMatchingWorks_andTheDataCellIsStillConverted() throws Exception { + File file = writeSample(); + + CapturingListener listener = new CapturingListener(); + FesodSheet.read(file, FlagRow.class, listener) + .registerConverter(new UpperCaseStringConverter()) + .sheet() + .doRead(); + + Assertions.assertEquals(1, listener.rows.size()); + // Before the fix the header is upper-cased to "FLAG", name matching fails silently and + // this is null; the data cell itself is not converted either. + Assertions.assertEquals("ABC", listener.rows.get(0).getFlag()); + } + + @Test + void invokeHeadMapReceivesTheRawHeaderText() throws Exception { + File file = writeSample(); + + CapturingListener listener = new CapturingListener(); + FesodSheet.read(file, FlagRow.class, listener) + .registerConverter(new UpperCaseStringConverter()) + .sheet() + .doRead(); + + Assertions.assertNotNull(listener.headMap); + // Before the fix the listener sees "FLAG" — the header went through the user converter. + Assertions.assertEquals("flag", listener.headMap.get(0)); + } + + @Test + void wildcardStringConverter_doesNotAffectHeaderOrNameMatching() throws Exception { + File file = writeSample(); + + CapturingListener listener = new CapturingListener(); + FesodSheet.read(file, FlagRow.class, listener) + .registerConverter(new WildcardUpperCaseStringConverter()) + .sheet() + .doRead(); + + // The header path never consults user registrations, wildcard or not, so name matching + // keeps working. The cell text is deliberately not asserted: a wildcard read converter + // does not reach data cells today (it is only expanded by the read-side change tracked in + // #1086), so the value is "abc" now and would become "ABC" once that lands — the header + // must stay raw in both cases. + Assertions.assertEquals("flag", listener.headMap.get(0)); + Assertions.assertEquals(1, listener.rows.size()); + Assertions.assertNotNull(listener.rows.get(0).getFlag()); + } + + private static File writeSample() throws Exception { + File file = File.createTempFile("header-converter-isolation", ".xlsx"); + file.deleteOnExit(); + FlagRow row = new FlagRow(); + row.setFlag(CELL_VALUE); + FesodSheet.write(file, FlagRow.class).sheet().doWrite(Collections.singletonList(row)); + return file; + } +} diff --git a/fesod-sheet/src/test/java/org/apache/fesod/sheet/util/ConverterUtilsTest.java b/fesod-sheet/src/test/java/org/apache/fesod/sheet/util/ConverterUtilsTest.java index a4c30bc42..3ea0e3590 100644 --- a/fesod-sheet/src/test/java/org/apache/fesod/sheet/util/ConverterUtilsTest.java +++ b/fesod-sheet/src/test/java/org/apache/fesod/sheet/util/ConverterUtilsTest.java @@ -75,37 +75,40 @@ void setUp() { Mockito.lenient().when(context.readSheetHolder()).thenReturn(readSheetHolder); Mockito.lenient().when(context.readRowHolder()).thenReturn(readRowHolder); Mockito.lenient().when(readSheetHolder.converterMap()).thenReturn(converterMap); + Mockito.lenient().when(context.currentReadHolder()).thenReturn(readSheetHolder); Mockito.lenient().when(readRowHolder.getRowIndex()).thenReturn(1); } @Test void test_convertToStringMap_normal() throws Exception { - // {0: "A", 1: "B"} + // {0: "A", 1: "B"} — header cells resolve against the built-in converters only: even with + // a user registration under the same key, the map in the holder is not consulted (#1098). Map> cellDataMap = new TreeMap<>(); cellDataMap.put(0, new ReadCellData<>("A")); cellDataMap.put(1, new ReadCellData<>("B")); ConverterKeyBuild.ConverterKey key = ConverterKeyBuild.buildKey(String.class, CellDataTypeEnum.STRING); converterMap.put(key, stringConverter); - Mockito.when(stringConverter.convertToJavaData(Mockito.any())).thenReturn("A", "B"); + Mockito.lenient().when(stringConverter.convertToJavaData(Mockito.any())).thenReturn("MOCK"); Map result = ConverterUtils.convertToStringMap(cellDataMap, context); Assertions.assertEquals(2, result.size()); Assertions.assertEquals("A", result.get(0)); Assertions.assertEquals("B", result.get(1)); + Mockito.verify(stringConverter, Mockito.never()).convertToJavaData(Mockito.any()); } @Test void test_convertToStringMap_withGap() throws Exception { - // {0: "A", 2: "C"} + // {0: "A", 2: "C"} — gap filling is unaffected by the built-in-only resolution (#1098). Map> cellDataMap = new TreeMap<>(); cellDataMap.put(0, new ReadCellData<>("A")); cellDataMap.put(2, new ReadCellData<>("C")); ConverterKeyBuild.ConverterKey key = ConverterKeyBuild.buildKey(String.class, CellDataTypeEnum.STRING); converterMap.put(key, stringConverter); - Mockito.when(stringConverter.convertToJavaData(Mockito.any())).thenReturn("A", "C"); + Mockito.lenient().when(stringConverter.convertToJavaData(Mockito.any())).thenReturn("MOCK"); Map result = ConverterUtils.convertToStringMap(cellDataMap, context); @@ -114,6 +117,7 @@ void test_convertToStringMap_withGap() throws Exception { Assertions.assertEquals("A", result.get(0)); Assertions.assertNull(result.get(1)); Assertions.assertEquals("C", result.get(2)); + Mockito.verify(stringConverter, Mockito.never()).convertToJavaData(Mockito.any()); } @Test @@ -123,14 +127,17 @@ void test_convertToStringMap_emptyCell() throws Exception { Map result = ConverterUtils.convertToStringMap(cellDataMap, context); + // Empty cells short-circuit to null without consulting any converter. Assertions.assertNull(result.get(0)); - Mockito.verify(stringConverter, Mockito.never()).convertToJavaData(Mockito.any()); } @Test void test_convertToStringMap_noConverter() { + // DATE is not covered by a built-in (String, DATE) converter, so the header path reports + // the missing converter instead of falling back to a user registration (#1098). BOOLEAN + // would be converted by StringBooleanConverter, so it no longer exercises this path. Map> cellDataMap = new HashMap<>(); - cellDataMap.put(0, new ReadCellData<>(CellDataTypeEnum.BOOLEAN)); + cellDataMap.put(0, new ReadCellData<>(CellDataTypeEnum.DATE)); Assertions.assertThrows(ExcelDataConvertException.class, () -> { ConverterUtils.convertToStringMap(cellDataMap, context); From 0fce275fce9a65482769c75a7aba4bb16b493604 Mon Sep 17 00:00:00 2001 From: Mikkey-f <178465103+Mikkey-f@users.noreply.github.com> Date: Sun, 11 Oct 2026 18:41:25 +0800 Subject: [PATCH 2/2] docs: note that invokeHeadMap receives the raw header text (#1098) --- .../org/apache/fesod/sheet/event/AnalysisEventListener.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/fesod-sheet/src/main/java/org/apache/fesod/sheet/event/AnalysisEventListener.java b/fesod-sheet/src/main/java/org/apache/fesod/sheet/event/AnalysisEventListener.java index 15c52c168..c80d61063 100644 --- a/fesod-sheet/src/main/java/org/apache/fesod/sheet/event/AnalysisEventListener.java +++ b/fesod-sheet/src/main/java/org/apache/fesod/sheet/event/AnalysisEventListener.java @@ -45,6 +45,9 @@ public void invokeHead(Map> headMap, AnalysisContext co /** * Returns the header as a map.Override the current method to receive header data. * + *

The values are the raw header text: converters registered by the user apply to data cells + * only and are not involved in header parsing. + * * @param headMap * @param context */