fix: prioritize subclass fields and ignore superclass annotations on field shadowing (#1007)
* fix: prioritize subclass fields and ignore superclass annotations on field shadowing
* refactor: simplify field skip condition
* refactor: renaming methods and modifying methods to return mutable types
---------
Co-authored-by: DeleiGuo <delei@apache.org>
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 980fc85..3116e9b 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 @@
if (clazz == null) {
return null;
}
- List<Field> 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<Field> tempFieldList = FieldUtils.resolveAllFields(clazz);
ContentStyle parentContentStyle = clazz.getAnnotation(ContentStyle.class);
ContentFontStyle parentContentFontStyle = clazz.getAnnotation(ContentFontStyle.class);
@@ -305,20 +298,8 @@
}
private static FieldCache doDeclaredFields(Class<?> clazz, ConfigurationHolder configurationHolder) {
- List<Field> tempFieldList = new ArrayList<>();
- Map<String, Field> 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<Field> tempFieldList = FieldUtils.resolveAllFields(clazz);
+
ExcelIgnoreUnannotated excelIgnoreUnannotated = clazz.getAnnotation(ExcelIgnoreUnannotated.class);
Set<String> 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 f80accc..5f78016 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,19 @@
import java.lang.reflect.Field;
import java.lang.reflect.Modifier;
+import java.util.ArrayList;
+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 +185,33 @@
}
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<Field> resolveAllFields(Class<?> cls) {
+ Validate.isTrue(cls != null, "The class must not be null");
+
+ List<Field> result = new ArrayList<>();
+ Set<String> fieldNames = new HashSet<>();
+ Class<?> tempClass = cls;
+ while (tempClass != null && tempClass != Object.class) {
+ for (Field field : tempClass.getDeclaredFields()) {
+ boolean shouldSkip = Modifier.isStatic(field.getModifiers())
+ || field.isSynthetic()
+ || !fieldNames.add(resolveCglibFieldName(field));
+ if (shouldSkip) {
+ continue;
+ }
+ result.add(field);
+ }
+ tempClass = tempClass.getSuperclass();
+ }
+ return 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 7b2e901..0f344f7 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 @@
Assertions.assertTrue(header12.contains("field"), "Child6 should contain field");
}
+ public static class ParentStringConverter implements Converter<String> {
+
+ @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<String> {
+
+ @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<Child> data() {
+ List<Child> 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<ChildWithAnnotation> data() {
+ List<ChildWithAnnotation> 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<Path, List<String>> extractExcelHeader() {
// Helper to read first row (header) from generated file
Function<Path, List<String>> 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 e72a273..ca7a00c 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,58 @@
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.resolveAllFields(null));
+ Assertions.assertEquals("The class must not be null", exception.getMessage());
+ }
+
+ @Test
+ void test_resolveFields_emptyClass_returnsEmptyList() {
+ List<Field> fields = FieldUtils.resolveAllFields(EmptyClass.class);
+ Assertions.assertNotNull(fields);
+ Assertions.assertTrue(fields.isEmpty());
+ }
+
+ @Test
+ void test_resolveFields_InheritanceAndShadowing_SubclassTakesPrecedence() {
+ List<Field> fields = FieldUtils.resolveAllFields(ChildClass.class);
+
+ Assertions.assertEquals(3, fields.size());
+
+ List<String> 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<Field> fields = FieldUtils.resolveAllFields(ChildClass.class);
+
+ boolean hasStaticField = fields.stream().anyMatch(f -> f.getName().equals("STATIC_PARENT_FIELD"));
+
+ Assertions.assertFalse(hasStaticField);
+ }
}
