Skip to content

fix: make DataFormatter cache configuration-aware #1068

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

Merged
delei merged 5 commits into from
Sep 6, 2026
Merged

fix: make DataFormatter cache configuration-aware #1068

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
971a589
fix: make DataFormatter cache configuration-aware
skytin1004 Sep 5, 2026
1627009
test: isolate locale-sensitive formatter test
skytin1004 Sep 5, 2026
a393674
style: apply Spotless formatting
skytin1004 Sep 5, 2026
4ecd854
Merge branch 'main' into fix/number-data-formatter-cache-config
skytin1004 Sep 5, 2026
5e258f8
Merge branch 'main' into fix/number-data-formatter-cache-config
skytin1004 Sep 5, 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
36 changes: 30 additions & 6 deletions fesod-sheet/src/main/java/org/apache/fesod/sheet/util/NumberDataFormatterUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,28 @@ public class NumberDataFormatterUtils {
/**
* Cache DataFormatter.
*/
private static final ThreadLocal<DataFormatter> DATA_FORMATTER_THREAD_LOCAL = new ThreadLocal<DataFormatter>();
private static final ThreadLocal<DataFormatterCache> DATA_FORMATTER_THREAD_LOCAL =
new ThreadLocal<DataFormatterCache>();

private static final class DataFormatterCache {
private final boolean use1904windowing;
private final Locale locale;
private final boolean useScientificFormat;
private final DataFormatter dataFormatter;

private DataFormatterCache(boolean use1904windowing, Locale locale, boolean useScientificFormat) {
this.use1904windowing = use1904windowing;
this.locale = locale;
this.useScientificFormat = useScientificFormat;
this.dataFormatter = new DataFormatter(use1904windowing, locale, useScientificFormat);
}

@pjfanning pjfanning Sep 8, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@delei I would recommend setting this on your DataFormatter.

https://www.javadocs.dev/org.apache.poi/poi/5.5.1/org/apache/poi/ss/usermodel/DataFormatter.html#setUseCachedValuesForFormulaCells-boolean-

@skytin1004 skytin1004 Sep 9, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks, @pjfanning. Fesod uses its own formatter and already reads cached formula results. While checking this, I found that XLS formula errors are always returned as #VALUE!, regardless of the cached error. I'll follow up with a separate fix.


private boolean matches(boolean use1904windowing, Locale locale, boolean useScientificFormat) {
return this.use1904windowing == use1904windowing
&& this.locale.equals(locale)
&& this.useScientificFormat == useScientificFormat;
}
}

/**
* Format number data.
Expand Down Expand Up @@ -83,12 +104,15 @@ public static String format(
Boolean use1904windowing,
Locale locale,
Boolean useScientificFormat) {
DataFormatter dataFormatter = DATA_FORMATTER_THREAD_LOCAL.get();
if (dataFormatter == null) {
dataFormatter = new DataFormatter(use1904windowing, locale, useScientificFormat);
DATA_FORMATTER_THREAD_LOCAL.set(dataFormatter);
boolean resolvedUse1904windowing = Boolean.TRUE.equals(use1904windowing);
Locale resolvedLocale = locale == null ? Locale.getDefault() : locale;
boolean resolvedUseScientificFormat = Boolean.TRUE.equals(useScientificFormat);
DataFormatterCache cache = DATA_FORMATTER_THREAD_LOCAL.get();
if (cache == null || !cache.matches(resolvedUse1904windowing, resolvedLocale, resolvedUseScientificFormat)) {
cache = new DataFormatterCache(resolvedUse1904windowing, resolvedLocale, resolvedUseScientificFormat);
DATA_FORMATTER_THREAD_LOCAL.set(cache);
}
return dataFormatter.format(data, dataFormat, dataFormatString);
return cache.dataFormatter.format(data, dataFormat, dataFormatString);
}

public static void removeThreadLocalCache() {
Expand Down
47 changes: 43 additions & 4 deletions fesod-sheet/src/test/java/org/apache/fesod/sheet/util/NumberDataFormatterUtilsTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,14 @@
import java.math.BigDecimal;
import java.util.Locale;
import org.apache.fesod.sheet.metadata.GlobalConfiguration;
import org.apache.fesod.sheet.metadata.format.DataFormatter;
import org.apache.fesod.sheet.testkit.Tags;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Tag;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.junit.jupiter.api.parallel.ResourceLock;
import org.junit.jupiter.api.parallel.Resources;
import org.mockito.Mock;
import org.mockito.Mockito;
import org.mockito.junit.jupiter.MockitoExtension;
Expand Down Expand Up @@ -88,19 +89,57 @@ void test_format_scientific() {
Assertions.assertEquals("12300", result);
}

@Test
@ResourceLock(Resources.LOCALE)
void test_format_replacesCachedFormatterWhenLocaleChanges() {
Locale defaultLocale = Locale.getDefault();
try {
Locale.setDefault(Locale.KOREA);
String korean = NumberDataFormatterUtils.format(
new BigDecimal("0.75"), (short) 18, "h:mm AM/PM", false, null, false);
String chinese = NumberDataFormatterUtils.format(
new BigDecimal("0.75"), (short) 18, "h:mm AM/PM", false, Locale.CHINA, false);

Assertions.assertTrue(korean.endsWith("오후"));
Assertions.assertTrue(chinese.endsWith("下午"));
} finally {
Locale.setDefault(defaultLocale);
}
}

@Test
void test_format_replacesCachedFormatterWhenUse1904WindowingChanges() {
String date1900 =
NumberDataFormatterUtils.format(BigDecimal.ONE, (short) 14, "yyyy-mm-dd", false, Locale.US, false);
String date1904 =
NumberDataFormatterUtils.format(BigDecimal.ONE, (short) 14, "yyyy-mm-dd", true, Locale.US, false);

Assertions.assertEquals("1900-01-01", date1900);
Assertions.assertEquals("1904-01-02", date1904);
}

@Test
void test_format_replacesCachedFormatterWhenScientificFormatChanges() {
BigDecimal data = new BigDecimal("100000000000");
String plain = NumberDataFormatterUtils.format(data, null, "General", false, Locale.US, false);
String scientific = NumberDataFormatterUtils.format(data, null, "General", false, Locale.US, true);

Assertions.assertEquals("100000000000", plain);
Assertions.assertEquals("1E+11", scientific);
}

@Test
void test_ThreadLocal_Cache_And_Remove() throws NoSuchFieldException, IllegalAccessException {
Field field = NumberDataFormatterUtils.class.getDeclaredField("DATA_FORMATTER_THREAD_LOCAL");
field.setAccessible(true);

@SuppressWarnings("unchecked")
ThreadLocal<DataFormatter> threadLocal = (ThreadLocal<DataFormatter>) field.get(null);
ThreadLocal<?> threadLocal = (ThreadLocal<?>) field.get(null);

Assertions.assertNull(threadLocal.get());

NumberDataFormatterUtils.format(new BigDecimal("1"), null, "0", false, Locale.US, false);

DataFormatter cachedFormatter = threadLocal.get();
Object cachedFormatter = threadLocal.get();
Assertions.assertNotNull(cachedFormatter);

NumberDataFormatterUtils.format(new BigDecimal("2"), null, "0", false, Locale.US, false);
Expand Down
Loading