Skip to content

fix: honor GlobalConfiguration.locale for java.util.Date string conversion - #1311

Open
yebuguntang wants to merge 1 commit into
apache:mainfrom
yebuguntang:fix/date-locale-1040
Open

yebuguntang wants to merge 1 commit into
apache:mainfrom
yebuguntang:fix/date-locale-1040

Conversation

@yebuguntang

Copy link
Copy Markdown
Contributor

Purpose of the pull request

Closed: #1040

java.util.Date string conversion ignored GlobalConfiguration.locale: DateStringConverter called DateUtils.format(Date, ...) / parseDate(...) without a locale, so SimpleDateFormat always used the JVM default locale. Every java.time converter already passes the configured locale, so a POJO with both a Date and a LocalDateTime field got localized text for only one of them. This PR makes java.util.Date follow the same rule. Happy to adjust if maintainers consider the old split intentional.

What's changed?

  • Add DateUtils.format(Date, String, Locale) and DateUtils.parseDate(String, String, Locale) overloads. The existing signatures delegate with null, which still resolves to Locale.getDefault(Locale.Category.FORMAT), so behavior is unchanged for all current callers.
  • Key the SimpleDateFormat thread-local cache by locale first, then pattern, reusing the bounded FIFO maps already used by the DateTimeFormatter cache. Without this, two locales sharing a pattern would reuse the formatter built for the first locale.
  • Pass globalConfiguration.getLocale() from DateStringConverter in both read and write directions.
  • Add regression tests for localized write/read, cache isolation between locales, bounded cache size, and the default-locale fallback. Expected strings are generated from SimpleDateFormat with the target locale, so the tests hold on JDK 8 (JRE locale data) and JDK 9+ (CLDR).

No website documentation change: GlobalConfiguration.locale is already documented as affecting date/number formatting; this fix makes that statement true for java.util.Date.

Verification

  • ./mvnw -o clean package -pl fesod-common,fesod-shaded,fesod-sheet: BUILD SUCCESS, 1807 tests, 0 failures/errors (JDK 21).
  • DateStringConverterTest + DateUtilsTest: 101 tests pass on JDK 21 and on JDK 8.
  • spotless:check and git diff --check are clean.

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.

…rsion

DateStringConverter used SimpleDateFormat with the JVM default locale, while all java.time converters honor the configured locale. Pass the locale through new DateUtils format/parseDate overloads and key the SimpleDateFormat cache by locale as well as pattern. Old signatures keep their behavior by falling back to the JVM default format locale.

Fixes apache#1040

@Mikkey-f Mikkey-f 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.

Thanks @yebuguntang — reproduced and verified locally (Windows 11, JDK 17). This closes the remaining java.util.Date gap from #1040.

Root cause: DateStringConverter never read GlobalConfiguration.locale; and DateUtils's SimpleDateFormat cache was keyed by pattern only, so wiring a locale through without re-keying would silently reuse a formatter built for another locale on the same thread.

Verification

Green (head 07645b31):

  • DateStringConverterTest 7/7, DateUtilsTest 94/94 (101 total)
  • full suite: fesod-sheet 1,807 + fesod-common 60 = 1,867 tests, 0 failures/errors
  • spotless:check clean

Red side:

  • Reverting only DateStringConverter → convertToExcelDataHonorsConfiguredLocale fails with expected: <星期一 16 十一月 2026> but was: <Monday 16 November 2026>, and the read test errors — this pins the converter wiring.
  • Keeping the new signatures but resolving the locale from Locale.getDefault(Locale.Category.FORMAT) inside getCacheDateFormat → test_formatDate_respectsLocale fails, test_parseDate_respectsLocale and test_dateFormatCache_isBounded error, and both converter tests fail — this shows the locale-first cache key is required, not cosmetic.

Compatibility: the two-arg overloads delegate with null → Locale.getDefault(Locale.Category.FORMAT), and the only production callers are DateStringConverter and the DateUtils tests, so users who never call .locale(...) see no behavior change. The java.util.Date path now matches the java.time converters.

Non-blocking

  1. StringNumberConverter:62 still formats a numeric date serial into a String field through DateUtils.format(BigDecimal, ...), which takes no locale; with a pattern like dd MMMM yyyy the month name still follows the JVM default even when .locale() is set. Different converter/target type from #1040, so probably fine to leave out of this PR — but a follow-up issue may be worth it.
  2. The tests are unit-level; a small round-trip test (write and read a bean with both Date and LocalDateTime fields under .locale(Locale.CHINESE)) would cover the user-facing scenario from #1040 through the real WriteContext/AnalysisContext wiring.
  3. Optional: a converter-level test with globalConfiguration.setLocale(null) would pin the fallback path end-to-end.

@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 @yebuguntang, I verified this locally and didn't find any issues. LGTM.

This branch has not been deployed

No deployments
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] Should java.util.Date string conversion honor GlobalConfiguration.locale like java.time?

3 participants