fix: resolve header cells against the built-in read converters (#1098)

fesod-sheet/src/main/java/org/apache/fesod/sheet/event/AnalysisEventListener.java

@@ -45,6 +45,9 @@ public void invokeHead(Map<Integer, ReadCellData<?>> headMap, AnalysisContext co
     /**
      * Returns the header as a map.Override the current method to receive header data.
      *
+     * <p>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
      */

fesod-sheet/src/main/java/org/apache/fesod/sheet/util/ConverterUtils.java

@@ -34,14 +34,14 @@
 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;
 import org.apache.fesod.sheet.exception.ExcelDataConvertException;
 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<Integer, String> convertToStringMap(
             Map<Integer, ReadCellData<?>> cellDataMap, AnalysisContext context) {
         Map<Integer, String> stringMap = MapUtils.newHashMapWithExpectedSize(cellDataMap.size());
-        ReadSheetHolder readSheetHolder = context.readSheetHolder();
         int index = 0;
         for (Map.Entry<Integer, ReadCellData<?>> entry : cellDataMap.entrySet()) {
             Integer key = entry.getKey();
@@ -77,8 +76,12 @@ public static Map<Integer, String> 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(),

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<String>} 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<String> {
+        @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<FlagRow> {
+        private Map<Integer, String> headMap;
+        private final List<FlagRow> rows = new ArrayList<>();
+
+        @Override
+        public void invokeHeadMap(Map<Integer, String> 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;
+    }
+}

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<Integer, ReadCellData<?>> 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<Integer, String> 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<Integer, ReadCellData<?>> 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<Integer, String> 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<Integer, String> 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<Integer, ReadCellData<?>> 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);