apache / apache/fesod

[Enhancement] Custom converters registered with supportExcelTypeKey() == null never apply when reading

Open
#1,085 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 and found nothing similar.

### Fesod version

2.0.2-incubating (the code path is unchanged on current main)

### JDK version

1.8

### Operating system

Windows 10

### Minimal reproduce step

Register a custom converter whose `supportExcelTypeKey()` returns `null` (the wildcard
contract: match every cell data type), then read a file whose cells are plain strings:

```java
public static class BooleanYesNoConverter implements Converter {

@Override
public Class supportJavaTypeKey() {
return Boolean.class;
}

@Override
public CellDataTypeEnum supportExcelTypeKey() {
return null; // wildcard: match every cell type
}

@Override
public Boolean convertToJavaData(ReadCellData cellData, ExcelContentProperty contentProperty,
GlobalConfiguration globalConfiguration) {
return "yes".equalsIgnoreCase(cellData.getStringValue());
}
}

// the file contains STRING cells "yes" and "no"
FesodSheet.read(file, BooleanReadData.class, listener)
.registerConverter(new BooleanYesNoConverter())
.sheet()
.doReadSync();
```

### Current behavior

The wildcard converter is silently ignored on the read path. `ConverterUtils#convertToJavaObject`
looks the converter up with the concrete cell type key `(Boolean, STRING)`; a converter registered
under the wildcard key `(Boolean, null)` never matches, so the built-in `BooleanStringConverter`
takes over and `Boolean.valueOf("yes")` returns `false` — the row is silently read as
`flag = false`, no warning is logged.

Two more notes:

- Control case: registering the identical converter with an explicit
`CellDataTypeEnum.STRING` key works on read — only the wildcard registration is broken.
- For types/classes without any built-in concrete-key converter, the read fails with
`ExcelDataConvertException: Converter not found` instead.

The same registration works on the write path for xlsx, where the lookup key is
`(Boolean, null)` — so read and write disagree about the wildcard contract. See #1045 /
#1056 (write/CSV flavor of the same key mismatch, fix in progress in PR #1069); this report
covers the read path, which that PR does not touch.

### Expected behavior

Custom converters registered with `supportExcelTypeKey() == null` apply on read as well,
consistent with the wildcard contract and with the write path.

### Root cause

- Registration (`AbstractReadHolder#initConverterMap`): custom converters are registered only
under `(supportJavaTypeKey(), supportExcelTypeKey())`, so a wildcard registration lands on
`(Boolean, null)`.
- Lookup (`ConverterUtils#convertToJavaObject` and `ConverterUtils#convertToStringMap`): a
single exact `converterMap.get(buildKey(clazz, cellData.getType()))` with a concrete type and
no fallback.

### Suggested fix

Either option works; happy to implement either in a PR:

1. **Fallback lookup (recommended)**: in `ConverterUtils`, when the exact-key lookup misses and
the cell type is non-null, retry once with the wildcard key `(clazz, null)` before failing.
On the read side default converters are registered only under concrete keys
(`DefaultConverterLoader#putAllConverter`), so the fallback can only hit user wildcard
registrations: no shadowing, explicit registrations keep priority, and behavior only changes
where reading fails or returns wrong data today.
2. **Registration expansion** (mirrors PR #1069 on the write side): additionally register
null-key customs under every concrete cell type. Consistent with the write-side fix, but it
introduces order-dependent overwriting between wildcard and explicit registrations.

### Are you willing to submit a PR?

- [x] I'm willing to submit a PR! (fix + round-trip regression tests are already prepared and
verified: the wildcard-read case fails on current main and passes with the fix)

Contributor guide

Open the contributing guide

Research direction

Start with ConverterUtils#convertToJavaObject and #convertToStringMap, then trace registration in AbstractReadHolder#initConverterMap and defaults in DefaultConverterLoader#putAllConverter. Run the provided wildcard-read reproduction and add or run the round-trip regression tests. Done means a converter registered with supportExcelTypeKey() == null is used for concrete cell types on read, while explicit type registrations still work.

Written by the indexing model from the issue text.

Assessment

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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.