From aa72f84b22c1ee8be9891b3782a6fc69557655c9 Mon Sep 17 00:00:00 2001 From: Bengbengbalabalabeng Date: Tue, 11 Aug 2026 17:52:13 +0800 Subject: [PATCH 1/2] fix: prioritize subclass fields and ignore superclass annotations on field shadowing --- .../apache/fesod/sheet/util/ClassUtils.java | 25 +- .../apache/fesod/sheet/util/FieldUtils.java | 37 +++ .../core/ClassUtilsFieldOverrideTest.java | 289 ++++++++++++++++++ .../fesod/sheet/util/FieldUtilsTest.java | 63 ++++ 4 files changed, 392 insertions(+), 22 deletions(-) diff --git a/fesod-sheet/src/main/java/org/apache/fesod/sheet/util/ClassUtils.java b/fesod-sheet/src/main/java/org/apache/fesod/sheet/util/ClassUtils.java index 980fc8548..55aae5be8 100644 --- a/fesod-sheet/src/main/java/org/apache/fesod/sheet/util/ClassUtils.java +++ b/fesod-sheet/src/main/java/org/apache/fesod/sheet/util/ClassUtils.java @@ -29,7 +29,6 @@ import java.lang.reflect.Modifier; import java.util.ArrayList; import java.util.Collection; -import java.util.Collections; import java.util.HashMap; import java.util.HashSet; import java.util.LinkedHashSet; @@ -225,13 +224,7 @@ private static Map doDeclaredFieldContentMap(Class if (clazz == null) { return null; } - List tempFieldList = new ArrayList<>(); - Class tempClass = clazz; - while (tempClass != null) { - Collections.addAll(tempFieldList, tempClass.getDeclaredFields()); - // Get the parent class and give it to yourself - tempClass = tempClass.getSuperclass(); - } + List tempFieldList = FieldUtils.resolveFields(clazz); ContentStyle parentContentStyle = clazz.getAnnotation(ContentStyle.class); ContentFontStyle parentContentFontStyle = clazz.getAnnotation(ContentFontStyle.class); @@ -305,20 +298,8 @@ public static FieldCache declaredFields(Class clazz, ConfigurationHolder conf } private static FieldCache doDeclaredFields(Class clazz, ConfigurationHolder configurationHolder) { - List tempFieldList = new ArrayList<>(); - Map fieldNameToField = new HashMap<>(); - Class tempClass = clazz; - // Prefer subclass fields, only process the bottom-most (subclass) definition for fields with the same name - while (tempClass != null) { - for (Field field : tempClass.getDeclaredFields()) { - String fieldName = FieldUtils.resolveCglibFieldName(field); - if (!fieldNameToField.containsKey(fieldName)) { - fieldNameToField.put(fieldName, field); - tempFieldList.add(field); - } - } - tempClass = tempClass.getSuperclass(); - } + List tempFieldList = FieldUtils.resolveFields(clazz); + ExcelIgnoreUnannotated excelIgnoreUnannotated = clazz.getAnnotation(ExcelIgnoreUnannotated.class); Set ignoreSet = new HashSet<>(); // First collect all field names annotated with ExcelIgnore (including subclass overrides) diff --git a/fesod-sheet/src/main/java/org/apache/fesod/sheet/util/FieldUtils.java b/fesod-sheet/src/main/java/org/apache/fesod/sheet/util/FieldUtils.java index f80accc6d..c82a2d432 100644 --- a/fesod-sheet/src/main/java/org/apache/fesod/sheet/util/FieldUtils.java +++ b/fesod-sheet/src/main/java/org/apache/fesod/sheet/util/FieldUtils.java @@ -27,12 +27,20 @@ import java.lang.reflect.Field; import java.lang.reflect.Modifier; +import java.util.ArrayList; +import java.util.Collections; +import java.util.HashSet; +import java.util.List; import java.util.Map; +import java.util.Set; +import lombok.AccessLevel; +import lombok.NoArgsConstructor; import org.apache.fesod.common.util.MemberUtils; import org.apache.fesod.common.util.StringUtils; import org.apache.fesod.shaded.cglib.beans.BeanMap; import org.apache.fesod.sheet.metadata.NullObject; +@NoArgsConstructor(access = AccessLevel.PRIVATE) public class FieldUtils { public static Class nullObjectClass = NullObject.class; @@ -178,4 +186,33 @@ public static Field getField(final Class cls, final String fieldName, final b } return match; } + + /** + * Resolves and retrieves all declared fields from the specified class and its inheritance hierarchy. + * If a field with the same name (resolved by CGLIB) is declared in both a subclass and a superclass, + * the subclass field definition takes precedence. + * + * @param cls the target {@link Class}, must not be {@code null} + * @return a {@link List} containing all resolved fields, or an empty list if no fields are found + */ + public static List resolveFields(Class cls) { + Validate.isTrue(cls != null, "The class must not be null"); + + List result = new ArrayList<>(); + Set fieldNames = new HashSet<>(); + Class tempClass = cls; + while (tempClass != null && tempClass != Object.class) { + for (Field field : tempClass.getDeclaredFields()) { + if (Modifier.isStatic(field.getModifiers()) || field.isSynthetic()) { + continue; + } + + if (fieldNames.add(resolveCglibFieldName(field))) { + result.add(field); + } + } + tempClass = tempClass.getSuperclass(); + } + return Collections.unmodifiableList(result); + } } diff --git a/fesod-sheet/src/test/java/org/apache/fesod/sheet/core/ClassUtilsFieldOverrideTest.java b/fesod-sheet/src/test/java/org/apache/fesod/sheet/core/ClassUtilsFieldOverrideTest.java index 7b2e9012a..0f344f7e1 100644 --- a/fesod-sheet/src/test/java/org/apache/fesod/sheet/core/ClassUtilsFieldOverrideTest.java +++ b/fesod-sheet/src/test/java/org/apache/fesod/sheet/core/ClassUtilsFieldOverrideTest.java @@ -19,8 +19,10 @@ package org.apache.fesod.sheet.core; +import java.io.File; import java.nio.file.Path; import java.util.ArrayList; +import java.util.Date; import java.util.List; import java.util.function.Function; import lombok.Getter; @@ -28,12 +30,30 @@ import org.apache.fesod.sheet.FesodSheet; import org.apache.fesod.sheet.annotation.ExcelIgnore; import org.apache.fesod.sheet.annotation.ExcelProperty; +import org.apache.fesod.sheet.annotation.format.DateTimeFormat; +import org.apache.fesod.sheet.annotation.format.NumberFormat; +import org.apache.fesod.sheet.annotation.write.style.ContentFontStyle; +import org.apache.fesod.sheet.annotation.write.style.ContentStyle; +import org.apache.fesod.sheet.converters.Converter; +import org.apache.fesod.sheet.enums.CellDataTypeEnum; +import org.apache.fesod.sheet.enums.poi.HorizontalAlignmentEnum; +import org.apache.fesod.sheet.metadata.GlobalConfiguration; +import org.apache.fesod.sheet.metadata.data.WriteCellData; +import org.apache.fesod.sheet.metadata.property.ExcelContentProperty; import org.apache.fesod.sheet.testkit.Tags; +import org.apache.fesod.sheet.testkit.assertions.ExcelAssertions; import org.apache.fesod.sheet.testkit.base.AbstractExcelTest; +import org.apache.fesod.sheet.testkit.enums.ExcelFormat; +import org.apache.fesod.sheet.testkit.params.ExcelFormatSource; +import org.apache.fesod.sheet.testkit.params.FormatScope; +import org.apache.fesod.sheet.util.DateUtils; +import org.apache.poi.ss.usermodel.DataFormatter; +import org.apache.poi.ss.usermodel.HorizontalAlignment; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Tag; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.io.TempDir; +import org.junit.jupiter.params.ParameterizedTest; @Tag(Tags.ROUND_TRIP) public class ClassUtilsFieldOverrideTest extends AbstractExcelTest { @@ -233,6 +253,275 @@ public void validateHeaderWithExcelIgnore(@TempDir Path tempDir) { Assertions.assertTrue(header12.contains("field"), "Child6 should contain field"); } + public static class ParentStringConverter implements Converter { + + @Override + public CellDataTypeEnum supportExcelTypeKey() { + return CellDataTypeEnum.STRING; + } + + @Override + public WriteCellData convertToExcelData( + String value, ExcelContentProperty contentProperty, GlobalConfiguration globalConfiguration) + throws Exception { + return new WriteCellData<>("Parent: " + value); + } + } + + public static class ChildStringConverter implements Converter { + + @Override + public CellDataTypeEnum supportExcelTypeKey() { + return CellDataTypeEnum.STRING; + } + + @Override + public WriteCellData convertToExcelData( + String value, ExcelContentProperty contentProperty, GlobalConfiguration globalConfiguration) + throws Exception { + return new WriteCellData<>("Child: " + value); + } + } + + @Setter + @Getter + static class ParentWithAnnotation { + @DateTimeFormat("yyyy") + Date date; + + @NumberFormat("#.##%") + Double doubleValue; + + @DateTimeFormat("yyyy") + Date childDate; + + @NumberFormat("#.##%") + Double childDoubleValue; + + @ContentStyle(horizontalAlignment = HorizontalAlignmentEnum.CENTER) + String string1; + + @ContentFontStyle(fontHeightInPoints = 30) + String string2; + + @ExcelProperty(converter = ParentStringConverter.class) + String string3; + + @ContentStyle(horizontalAlignment = HorizontalAlignmentEnum.CENTER) + String childString1; + + @ContentFontStyle(fontHeightInPoints = 30) + String childString2; + + @ExcelProperty(converter = ParentStringConverter.class) + String childString3; + } + + @Setter + @Getter + static class Child extends ParentWithAnnotation { + Date childDate; + Double childDoubleValue; + String childString1; + String childString2; + String childString3; + + static List data() { + List data = new ArrayList<>(); + Child child = new Child(); + child.setDate(new Date()); + child.setDoubleValue(0.5D); + child.setChildDate(new Date()); + child.setChildDoubleValue(0.5D); + child.setString1("string1"); + child.setString2("string2"); + child.setString3("string3"); + child.setChildString1("childString1"); + child.setChildString2("childString2"); + child.setChildString3("childString3"); + data.add(child); + return data; + } + } + + @Setter + @Getter + static class ChildWithAnnotation extends ParentWithAnnotation { + @DateTimeFormat("yyyy-MM-dd") + Date childDate; + + @NumberFormat("#.00") + Double childDoubleValue; + + @ContentStyle(horizontalAlignment = HorizontalAlignmentEnum.LEFT) + String childString1; + + @ContentFontStyle(fontHeightInPoints = 25) + String childString2; + + @ExcelProperty(converter = ChildStringConverter.class) + String childString3; + + static List data() { + List data = new ArrayList<>(); + ChildWithAnnotation child = new ChildWithAnnotation(); + child.setDate(new Date()); + child.setDoubleValue(0.5D); + child.setChildDate(new Date()); + child.setChildDoubleValue(0.5D); + child.setString1("string1"); + child.setString2("string2"); + child.setString3("string3"); + child.setChildString1("childString1"); + child.setChildString2("childString2"); + child.setChildString3("childString3"); + data.add(child); + return data; + } + } + + @ParameterizedTest + @ExcelFormatSource(value = FormatScope.BINARY) + void test_fieldShadowing_subclassWithoutAnnotation(ExcelFormat format) throws Exception { + File file = createTempFile(format); + DataFormatter formatter = new DataFormatter(); + + // Subclass overrides property without declaring annotations. + // Subclass attributes completely shadow parent class attributes; parent class annotations should not take + // effect/be inherited. + FesodSheet.write(file).head(Child.class).sheet().doWrite(Child.data()); + + try (ExcelAssertions ea = ExcelAssertions.assertThat(file)) { + ea.sheet(0) + .row(1) + // col 0 = childDate: subclass shadows parent, no @DateTimeFormat inherited + .cell(0) + .hasDataFormatString(DateUtils.defaultDateFormat) + .and() + // col 1 = childDoubleValue: subclass shadows parent, no @NumberFormat inherited + .cell(1) + .satisfies(cell -> { + String formatCellValue = formatter.formatCellValue(cell); + Assertions.assertEquals("0.5", formatCellValue); + }) + .and() + // col 2 = childString1: subclass shadows parent, no @ContentStyle inherited + .cell(2) + .satisfies(cell -> { + Assertions.assertEquals("childString1", cell.getStringCellValue()); + Assertions.assertNotEquals( + HorizontalAlignment.CENTER, cell.getCellStyle().getAlignment()); + }) + .and() + // col 3 = childString2: subclass shadows parent, no @ContentFontStyle inherited + .cell(3) + .satisfies(cell -> Assertions.assertEquals("childString2", cell.getStringCellValue())) + .and() + // col 4 = childString3: subclass shadows parent, no converter inherited + .cell(4) + .satisfies(cell -> Assertions.assertEquals("childString3", cell.getStringCellValue())) + .and() + // col 5 = date: parent field retains @DateTimeFormat("yyyy") + .cell(5) + .hasDataFormatString("yyyy") + .and() + // col 6 = doubleValue: parent field retains @NumberFormat("#.##%") + .cell(6) + .satisfies(cell -> { + String formatCellValue = formatter.formatCellValue(cell); + Assertions.assertEquals("50%", formatCellValue); + }) + .and() + // col 7 = string1: parent field retains @ContentStyle(CENTER) + .cell(7) + .satisfies(cell -> { + Assertions.assertEquals("string1", cell.getStringCellValue()); + Assertions.assertEquals( + HorizontalAlignment.CENTER, cell.getCellStyle().getAlignment()); + }) + .and() + // col 8 = string2: parent field retains @ContentFontStyle(fontHeightInPoints = 30) + .cell(8) + .satisfies(cell -> Assertions.assertEquals("string2", cell.getStringCellValue())) + .hasFontSize((short) 30) + .and() + // col 9 = string3: parent field retains @ExcelProperty converter + .cell(9) + .satisfies(cell -> Assertions.assertEquals("Parent: string3", cell.getStringCellValue())); + } + } + + @ParameterizedTest + @ExcelFormatSource(value = FormatScope.BINARY) + void test_fieldShadowing_subclassAnnotationPrecedence(ExcelFormat format) throws Exception { + File file = createTempFile(format); + DataFormatter formatter = new DataFormatter(); + + // Subclass overrides a property and explicitly declares a subclass-specific annotations. + // Directly use the annotation format on subclass attributes + FesodSheet.write(file).head(ChildWithAnnotation.class).sheet().doWrite(ChildWithAnnotation.data()); + + try (ExcelAssertions ea = ExcelAssertions.assertThat(file)) { + ea.sheet(0) + .row(1) + // col 0 = childDate: subclass @DateTimeFormat("yyyy-MM-dd") takes precedence + .cell(0) + .hasDataFormatString("yyyy-MM-dd") + .and() + // col 1 = childDoubleValue: subclass @NumberFormat("#.00") takes precedence + .cell(1) + .satisfies(cell -> { + String formatCellValue = formatter.formatCellValue(cell); + Assertions.assertEquals(".50", formatCellValue); + }) + .and() + // col 2 = childString1: subclass @ContentStyle(LEFT) takes precedence + .cell(2) + .satisfies(cell -> { + Assertions.assertEquals("childString1", cell.getStringCellValue()); + Assertions.assertEquals( + HorizontalAlignment.LEFT, cell.getCellStyle().getAlignment()); + }) + .and() + // col 3 = childString2: subclass @ContentFontStyle(25) takes precedence + .cell(3) + .satisfies(cell -> Assertions.assertEquals("childString2", cell.getStringCellValue())) + .hasFontSize((short) 25) + .and() + // col 4 = childString3: subclass @ExcelProperty converter takes precedence + .cell(4) + .satisfies(cell -> Assertions.assertEquals("Child: childString3", cell.getStringCellValue())) + .and() + // col 5 = date: parent @DateTimeFormat("yyyy") retained + .cell(5) + .hasDataFormatString("yyyy") + .and() + // col 6 = doubleValue: parent @NumberFormat("#.##%") retained + .cell(6) + .satisfies(cell -> { + String formatCellValue = formatter.formatCellValue(cell); + Assertions.assertEquals("50%", formatCellValue); + }) + .and() + // col 7 = string1: parent field retains @ContentStyle(CENTER) + .cell(7) + .satisfies(cell -> { + Assertions.assertEquals("string1", cell.getStringCellValue()); + Assertions.assertEquals( + HorizontalAlignment.CENTER, cell.getCellStyle().getAlignment()); + }) + .and() + // col 8 = string2: parent field retains @ContentFontStyle(fontHeightInPoints = 30) + .cell(8) + .satisfies(cell -> Assertions.assertEquals("string2", cell.getStringCellValue())) + .hasFontSize((short) 30) + .and() + // col 9 = string3: parent field retains @ExcelProperty converter + .cell(9) + .satisfies(cell -> Assertions.assertEquals("Parent: string3", cell.getStringCellValue())); + } + } + private static Function> extractExcelHeader() { // Helper to read first row (header) from generated file Function> readHeader = path -> { diff --git a/fesod-sheet/src/test/java/org/apache/fesod/sheet/util/FieldUtilsTest.java b/fesod-sheet/src/test/java/org/apache/fesod/sheet/util/FieldUtilsTest.java index e72a27362..55af0eee4 100644 --- a/fesod-sheet/src/test/java/org/apache/fesod/sheet/util/FieldUtilsTest.java +++ b/fesod-sheet/src/test/java/org/apache/fesod/sheet/util/FieldUtilsTest.java @@ -21,7 +21,9 @@ import java.lang.reflect.Field; import java.util.HashMap; +import java.util.List; import java.util.Map; +import java.util.stream.Collectors; import java.util.stream.Stream; import org.apache.fesod.shaded.cglib.beans.BeanMap; import org.apache.fesod.sheet.metadata.NullObject; @@ -242,4 +244,65 @@ void test_getField_packagePrivateClass_publicField() { Assertions.assertNotNull(field); Assertions.assertTrue(field.isAccessible()); } + + static class ParentClass { + public static final String STATIC_PARENT_FIELD = "STATIC"; + private String commonField = "parentCommon"; + private Integer parentOnlyField; + } + + static class ChildClass extends ParentClass { + private String commonField = "childCommon"; + private Double childOnlyField; + } + + static class EmptyClass {} + + @Test + void test_resolveFields_nullClass_throwsException() { + IllegalArgumentException exception = + Assertions.assertThrows(IllegalArgumentException.class, () -> FieldUtils.resolveFields(null)); + Assertions.assertEquals("The class must not be null", exception.getMessage()); + } + + @Test + void test_resolveFields_emptyClass_returnsEmptyList() { + List fields = FieldUtils.resolveFields(EmptyClass.class); + Assertions.assertNotNull(fields); + Assertions.assertTrue(fields.isEmpty()); + } + + @Test + void test_resolveFields_InheritanceAndShadowing_SubclassTakesPrecedence() { + List fields = FieldUtils.resolveFields(ChildClass.class); + + Assertions.assertEquals(3, fields.size()); + + List fieldNames = fields.stream().map(Field::getName).collect(Collectors.toList()); + Assertions.assertTrue(fieldNames.contains("childOnlyField")); + Assertions.assertTrue(fieldNames.contains("parentOnlyField")); + Assertions.assertTrue(fieldNames.contains("commonField")); + + Field commonField = fields.stream() + .filter(f -> f.getName().equals("commonField")) + .findFirst() + .get(); + Assertions.assertEquals(ChildClass.class, commonField.getDeclaringClass()); + } + + @Test + void test_resolveFields_IgnoresStaticFields() { + List fields = FieldUtils.resolveFields(ChildClass.class); + + boolean hasStaticField = fields.stream().anyMatch(f -> f.getName().equals("STATIC_PARENT_FIELD")); + + Assertions.assertFalse(hasStaticField); + } + + @Test + void test_resolveFields_returnsUnmodifiableList() { + List fields = FieldUtils.resolveFields(ChildClass.class); + + Assertions.assertThrows(UnsupportedOperationException.class, fields::clear); + } } From 89fe93ff6c00c16d895b46cead0999932d0b04d3 Mon Sep 17 00:00:00 2001 From: Bengbengbalabalabeng Date: Wed, 12 Aug 2026 17:29:49 +0800 Subject: [PATCH 2/2] refactor: simplify field skip condition --- .../java/org/apache/fesod/sheet/util/FieldUtils.java | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/fesod-sheet/src/main/java/org/apache/fesod/sheet/util/FieldUtils.java b/fesod-sheet/src/main/java/org/apache/fesod/sheet/util/FieldUtils.java index c82a2d432..a7fcf74de 100644 --- a/fesod-sheet/src/main/java/org/apache/fesod/sheet/util/FieldUtils.java +++ b/fesod-sheet/src/main/java/org/apache/fesod/sheet/util/FieldUtils.java @@ -203,13 +203,13 @@ public static List resolveFields(Class cls) { Class tempClass = cls; while (tempClass != null && tempClass != Object.class) { for (Field field : tempClass.getDeclaredFields()) { - if (Modifier.isStatic(field.getModifiers()) || field.isSynthetic()) { + boolean shouldSkip = Modifier.isStatic(field.getModifiers()) + || field.isSynthetic() + || !fieldNames.add(resolveCglibFieldName(field)); + if (shouldSkip) { continue; } - - if (fieldNames.add(resolveCglibFieldName(field))) { - result.add(field); - } + result.add(field); } tempClass = tempClass.getSuperclass(); }