Skip to content

[Enhancement] Pin intended behavior of BooleanUtils.valueOf("true") with a test? #953

Description

@nkuprins

Search before asking

  • I searched in the issues and found nothing similar.

Motivation

BooleanUtils.valueOf(String) in fesod-common returns TRUE only for the exact string "1" - every other input, including "true", maps to FALSE:

public static Boolean valueOf(String str) {
    if (TRUE_NUMBER.equals(str)) {  // TRUE_NUMBER = "1"
        return Boolean.TRUE;
    }
    return Boolean.FALSE;
}

Its only production caller is CellTagHandler, converting the <v> value of t="b" (boolean) cells when streaming xlsx. The strict "1" only check matches Excel's own output (Excel always writes 0/1) and Apache POI's DOM model (XSSFCell.getBooleanCellValue() does the same "1".equals(v)), so it looks intentional.

However, BooleanUtilsTest only covers "1", "", and null. Nothing in the repo documents that valueOf("true") == FALSE is by design. Notably, POI's own SAX path (XSSFSheetXMLHandler) is more lenient (value.charAt(0) == '0' ? FALSE : TRUE), so the precedent is mixed and a future contributor could plausibly "fix" this as a bug and change reading behavior.

Question for the maintainers: is the "1" only matching the intended contract? If yes, would you accept a small test-only PR pinning it?

Solution

If the current behavior is intended, add assertions to BooleanUtilsTest documenting it, e.g.:

// Intentional: xlsx boolean cells use Excel's 0/1 markers; matches POI XSSFCell
Assertions.assertFalse(BooleanUtils.valueOf("true"));
Assertions.assertFalse(BooleanUtils.valueOf("0"));

optionally plus a one-line Javadoc note on valueOf stating it targets Excel's 0/1 boolean cell markers.

Alternatives

If instead the maintainers consider the strictness a defect (files written by non-Excel producers could in principle contain <v>true</v>), the alternative is making valueOf lenient - e.g. also accepting "true" case-insensitively, similar to POI's XSSFSheetXMLHandler. That would be a behavior change to xlsx boolean reading, so I did not assume it.

Anything else?

No response

Are you willing to submit a PR?

  • I'm willing to submit a PR!

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or request

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions