apache / apache/fesod

[Bug] Header cells are converted with user-registered String converters, breaking @ExcelProperty(name) matching

Open
#1,098 3 comments 0 reactions 0 assignees View on GitHub
Dominant language
Java
Stars
6.2k
Forks
532
Avg merge
1d 3h
Merged PRs (30d)
42

Description

### Search before asking

- [x] I searched in the [issues](https://github.com/apache/fesod/issues) and found nothing similar.

### Fesod version

main (2.1.0-incubating). Not tied to a recent change — the explicit-key case below reproduces on released versions too.

### JDK version

17 (Temurin); not JDK-specific.

### Operating system

Any

### Steps To Reproduce

```java
public static class Bean {
@ExcelProperty("flag")
private String flag;
// getters / setters
}

public static class UpperCaseConverter implements Converter {
@Override public Class supportJavaTypeKey() { return String.class; }

@Override public CellDataTypeEnum supportExcelTypeKey() { return CellDataTypeEnum.STRING; }

@Override public String convertToJavaData(
ReadCellData cellData, ExcelContentProperty p, GlobalConfiguration g) {
return cellData.getStringValue().toUpperCase();
}
}

// file under test: header "flag", one data row with the value "abc"
FesodSheet.write(file, Bean.class).sheet().doWrite(Collections.singletonList(new Bean("abc")));

List rows = FesodSheet.read(file, Bean.class, listener)
.registerConverter(new UpperCaseConverter())
.sheet()
.doReadSync();

// rows.get(0).getFlag() is null
```

### Current Behavior

The converter is invoked with the **header** cell (`"flag"`) instead of the data cell (`"abc"`): `convertToJavaData` returns `"FLAG"`, the framework then compares `"FLAG"` with the `@ExcelProperty("flag")` name, finds no matching column, and leaves the field `null`. No exception is raised, so the failure is silent.

The same path also fills `AnalysisEventListener.invokeHeadMap(...)`, so the header map handed to listeners goes through user converters as well.

Using `@ExcelProperty(index = 0)` works, because matching then does not depend on the header text.

### Expected Behavior

When the framework converts the head row to strings for name matching, header cells should be resolved against the built-in converters — user-registered converters are meant for data cells. `invokeHeadMap` should likewise receive the raw header text.

Note that in the failing case the converter is never invoked for the data cell either, so even a user who wants header conversion loses data conversion entirely.

### Anything else?

Root cause: `ConverterUtils.convertToStringMap` resolves `ConverterKeyBuild.buildKey(String.class, cellData.getType())` in the holder's converter map, which also contains user registrations. It is called from `DefaultAnalysisEventProcessor.buildHead` (header to field matching) and from `AnalysisEventListener.invokeHead`.

Found while reviewing #1086 (read-side wildcard converter fix). That PR does not introduce this — an explicit `(String, STRING)` registration already reproduces it on `main`. It does widen the set of affected registrations, since a wildcard `Converter` gets expanded to `(String, STRING)` and would therefore be picked up by the header path too.

Proposed fix: resolve the header path against the built-in read converters (e.g. `DefaultConverterLoader.copyDefaultReadConverter()`) rather than the user-extended map. One decision point for the maintainers: whether `invokeHeadMap` should keep seeing user-converted text (a niche behaviour someone might rely on) or always see the raw header.

Happy to prepare the PR if the direction looks right — unless it is preferred to handle this within #1086.

### Are you willing to submit a PR?

- [x] I'm willing to submit a PR!

Contributor guide

Open the contributing guide

Research direction

Start with ConverterUtils.convertToStringMap and trace its callers in DefaultAnalysisEventProcessor.buildHead and AnalysisEventListener.invokeHead. Compare the holder converter map with DefaultConverterLoader.copyDefaultReadConverter(), then reproduce the explicit String/STRING registration case. Done means header matching remains based on the header text, invokeHeadMap receives the expected header value, and data-cell conversion still works.

Written by the indexing model from the issue text.

Assessment

Tech stack
java
Domain
backend, data
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
68/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.