Skip to content

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

Description

@BigDataDZ

Search before asking

  • 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:

public static class BooleanYesNoConverter implements Converter<Boolean> {

    @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 fix: honor wildcard-key custom converters when writing CSV (#1045) (#1056) #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?

  • 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)

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions