Skip to content

fix: key the date format cache by format index and string #1304

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
hanzhe-one wants to merge 3 commits into
base: main
Choose a base branch
from
Open

fix: key the date format cache by format index and string #1304

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e98274b
fix: key the date format cache by format index and string
hanzhe-one Nov 2, 2026
dd250c1
test: assert the exact date format cache key
hanzhe-one Nov 5, 2026
4f3d6fd
Merge branch 'main' into fix/date-format-cache-key
alaahong Nov 7, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 10 additions & 4 deletions fesod-sheet/src/main/java/org/apache/fesod/sheet/util/ExcelDateFormatDetector.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ class ExcelDateFormatDetector {
/**
* Is a cache of dates
*/
private static final ThreadLocal<Map<Short, Boolean>> DATE_THREAD_LOCAL = new ThreadLocal<>();
private static final ThreadLocal<Map<String, Boolean>> DATE_THREAD_LOCAL = new ThreadLocal<>();

/**
* The following patterns are used in {@link #isADateFormat(Short, String)}
Expand Down Expand Up @@ -67,21 +67,27 @@ static boolean isADateFormat(Short formatIndex, String formatString) {
if (formatIndex == null) {
return false;
}
Map<Short, Boolean> isDateCache = DATE_THREAD_LOCAL.get();
// Format indexes are defined per workbook, so the same index can map to different format strings.
final String cacheKey = cacheKey(formatIndex, formatString);
Map<String, Boolean> isDateCache = DATE_THREAD_LOCAL.get();
if (isDateCache == null) {
isDateCache = MapUtils.newHashMap();
DATE_THREAD_LOCAL.set(isDateCache);
} else {
Boolean isDatecachedDataList = isDateCache.get(formatIndex);
Boolean isDatecachedDataList = isDateCache.get(cacheKey);
if (isDatecachedDataList != null) {
return isDatecachedDataList;
}
}
boolean isDate = isADateFormatUncached(formatIndex, formatString);
isDateCache.put(formatIndex, isDate);
isDateCache.put(cacheKey, isDate);
return isDate;
}

private static String cacheKey(Short formatIndex, String formatString) {
return formatIndex + "\u0000" + (formatString == null ? "" : formatString);
}

/**
* Determine if it is a date format.
*
Expand Down
43 changes: 39 additions & 4 deletions fesod-sheet/src/test/java/org/apache/fesod/sheet/util/DateUtilsTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -528,6 +528,7 @@ void test_isADateFormat_false(String formatString) {
Assertions.assertFalse(DateUtils.isADateFormat(null, formatString));
}

@SuppressWarnings("unchecked")
@Test
void test_isADateFormat_Cache() throws NoSuchFieldException, IllegalAccessException {
short formatId = 200;
Expand All @@ -538,15 +539,49 @@ void test_isADateFormat_Cache() throws NoSuchFieldException, IllegalAccessExcept

Field threadLocalField = ExcelDateFormatDetector.class.getDeclaredField("DATE_THREAD_LOCAL");
threadLocalField.setAccessible(true);
ThreadLocal<Map<Short, Boolean>> tl = (ThreadLocal<Map<Short, Boolean>>) threadLocalField.get(null);
ThreadLocal<Map<String, Boolean>> tl = (ThreadLocal<Map<String, Boolean>>) threadLocalField.get(null);

Map<Short, Boolean> cache = tl.get();
Map<String, Boolean> cache = tl.get();
Assertions.assertNotNull(cache);
Assertions.assertTrue(cache.containsKey(formatId));
Assertions.assertTrue(cache.get(formatId));
Assertions.assertEquals(1, cache.size());
Assertions.assertEquals(Boolean.TRUE, cache.get(formatId + "\u0000" + formatStr));

boolean res2 = DateUtils.isADateFormat(formatId, formatStr);
Assertions.assertTrue(res2);
Assertions.assertEquals(1, cache.size());
}

@Test
void test_isADateFormat_cacheDistinguishesFormatStrings() {
short formatId = 201;

Assertions.assertTrue(DateUtils.isADateFormat(formatId, "yyyy-mm-dd"));
Assertions.assertFalse(DateUtils.isADateFormat(formatId, "0.00"));
Assertions.assertTrue(DateUtils.isADateFormat(formatId, "yyyy-mm-dd"));
}

@Test
void test_isADateFormat_cacheDistinguishesFormatStringsInReverseOrder() {
short formatId = 202;

Assertions.assertFalse(DateUtils.isADateFormat(formatId, "0.00"));
Assertions.assertTrue(DateUtils.isADateFormat(formatId, "yyyy-mm-dd"));
}

@Test
void test_isADateFormat_cacheDistinguishesNullFormatString() {
short formatId = 203;

Assertions.assertTrue(DateUtils.isADateFormat(formatId, "yyyy-mm-dd"));
Assertions.assertFalse(DateUtils.isADateFormat(formatId, null));
}

@Test
void test_isADateFormat_internalIndexStillOverridesFormatString() {
short internalFormatId = 0x0e;

Assertions.assertTrue(DateUtils.isADateFormat(internalFormatId, "0.00"));
Assertions.assertFalse(DateUtils.isADateFormat((short) (0x0e + 100), "0.00"));
}

@Test
Expand Down