Skip to content

feat: honor wildcard custom converters when reading - #1086

Open
BigDataDZ wants to merge 1 commit into
apache:mainfrom
BigDataDZ:fix/read-wildcard-converter
Open

feat: honor wildcard custom converters when reading#1086
BigDataDZ wants to merge 1 commit into
apache:mainfrom
BigDataDZ:fix/read-wildcard-converter

Conversation

@BigDataDZ

Copy link
Copy Markdown

Purpose of the pull request

Custom converters registered with supportExcelTypeKey() == null (the wildcard contract: "matches
every cell data type") never applied on the read path: the built-in concrete-key converter silently
took over, or reading failed with Converter not found. With this change, a single
registerConverter(...) call works for reading exactly as it already does for writing.

Closed: #1085

What's changed?

AbstractReadHolder now routes custom converter registration through registerCustomConverter,
which — when supportExcelTypeKey() is null — additionally registers the converter under every
concrete CellDataTypeEnum key (all except EMPTY, which is filtered before lookup).

Read lookups (ConverterUtils#convertToJavaObject, ConverterUtils#convertToStringMap) use the
concrete cell type as key, so a wildcard registration at (JavaType, null) never matched: e.g. a
wildcard Boolean converter reading a "yes" cell produced false via the built-in
BooleanStringConverter (Boolean.valueOf("yes")), with no warning. This is the read-path twin of
#1045/#1056 (write/CSV flavor, addressed by PR #1069) and mirrors that PR's registration-expansion
approach, expanded to all concrete types because read lookups can arrive with any cell type.

Lookup code is untouched. Explicit-key registrations keep their priority semantics, and users
without wildcard custom converters are unaffected.

New WildcardConverterReadTest (round-trip): the wildcard-read case fails on current main
("yes" read as false) and passes with this change; the explicit STRING-key case is a
regression guard. Full fesod-sheet suite: 920/920 green.

Checklist

  • I have read the Contributor Guide.
  • I have written the necessary doc or comment.
  • I have added the necessary unit tests and all cases have passed.

@BigDataDZ BigDataDZ changed the title fix: honor wildcard custom converters when reading feat: honor wildcard custom converters when reading Sep 10, 2026
Signed-off-by: BigDataDZ <76271875+BigDataDZ@users.noreply.github.com>

@skytin1004 skytin1004 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @BigDataDZ,
I tried registering a STRING converter first and a wildcard converter second. Before this change, the STRING converter handled string cells. With this change, the wildcard converter handles them instead.
Is this change intentional? If not, could you keep the STRING converter handling these cells and add a test for this registration order?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

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

2 participants