From 71c30551162a522d9f0789b4d5c802503586bb80 Mon Sep 17 00:00:00 2001 From: sanyue <443341200@qq.com> Date: Mon, 2 Nov 2026 21:21:31 +0800 Subject: [PATCH 1/2] fix: isolate LongestMatchColumnWidthStyleStrategy width cache per sheet holder Move the per-column maximum width cache from the strategy instance onto WriteSheetHolder so that a single LongestMatchColumnWidthStyleStrategy instance can be reused across writers without leaking widths from one workbook or sheet into another. The strategy is now stateless with respect to width tracking, which also removes the shared mutable state discussed in #1305 (thread-safety and reference strength are no longer concerns). This follows the same per-holder isolation pattern used for converter state (e.g. #740). fixes #1305 --- .../metadata/holder/WriteSheetHolder.java | 4 +- .../LongestMatchColumnWidthStyleStrategy.java | 10 +- ...gestMatchColumnWidthStyleStrategyTest.java | 92 +++++++++++++++++++ 3 files changed, 100 insertions(+), 6 deletions(-) create mode 100644 fesod-sheet/src/test/java/org/apache/fesod/sheet/write/style/column/LongestMatchColumnWidthStyleStrategyTest.java diff --git a/fesod-sheet/src/main/java/org/apache/fesod/sheet/write/metadata/holder/WriteSheetHolder.java b/fesod-sheet/src/main/java/org/apache/fesod/sheet/write/metadata/holder/WriteSheetHolder.java index c863544001..2b790b91d2 100644 --- a/fesod-sheet/src/main/java/org/apache/fesod/sheet/write/metadata/holder/WriteSheetHolder.java +++ b/fesod-sheet/src/main/java/org/apache/fesod/sheet/write/metadata/holder/WriteSheetHolder.java @@ -47,7 +47,7 @@ */ @Getter @Setter -@EqualsAndHashCode(exclude = {"parentWriteWorkbookHolder", "hasBeenInitializedTable"}) +@EqualsAndHashCode(exclude = {"parentWriteWorkbookHolder", "hasBeenInitializedTable", "columnWidthCache"}) @NoArgsConstructor public class WriteSheetHolder extends AbstractWriteHolder { /** @@ -88,6 +88,8 @@ public class WriteSheetHolder extends AbstractWriteHolder { */ private Map hasBeenInitializedTable; + private Map columnWidthCache; + /** * last column type * diff --git a/fesod-sheet/src/main/java/org/apache/fesod/sheet/write/style/column/LongestMatchColumnWidthStyleStrategy.java b/fesod-sheet/src/main/java/org/apache/fesod/sheet/write/style/column/LongestMatchColumnWidthStyleStrategy.java index a9d4191071..c0eac8b231 100644 --- a/fesod-sheet/src/main/java/org/apache/fesod/sheet/write/style/column/LongestMatchColumnWidthStyleStrategy.java +++ b/fesod-sheet/src/main/java/org/apache/fesod/sheet/write/style/column/LongestMatchColumnWidthStyleStrategy.java @@ -29,7 +29,6 @@ import java.util.List; import java.util.Map; import org.apache.commons.collections4.CollectionUtils; -import org.apache.fesod.common.util.MapUtils; import org.apache.fesod.sheet.enums.CellDataTypeEnum; import org.apache.fesod.sheet.metadata.Head; import org.apache.fesod.sheet.metadata.data.WriteCellData; @@ -48,8 +47,6 @@ public class LongestMatchColumnWidthStyleStrategy extends AbstractColumnWidthSty private static final int MAX_COLUMN_WIDTH = 255; - private final Map> cache = MapUtils.newHashMapWithExpectedSize(8); - @Override protected void setColumnWidth( WriteSheetHolder writeSheetHolder, @@ -62,8 +59,11 @@ protected void setColumnWidth( if (!needSetWidth) { return; } - Map maxColumnWidthMap = - cache.computeIfAbsent(writeSheetHolder.getSheetNo(), key -> new HashMap<>(16)); + Map maxColumnWidthMap = writeSheetHolder.getColumnWidthCache(); + if (maxColumnWidthMap == null) { + maxColumnWidthMap = new HashMap<>(16); + writeSheetHolder.setColumnWidthCache(maxColumnWidthMap); + } Integer columnWidth = dataLength(cellDataList, cell, isHead); if (columnWidth < 0) { return; diff --git a/fesod-sheet/src/test/java/org/apache/fesod/sheet/write/style/column/LongestMatchColumnWidthStyleStrategyTest.java b/fesod-sheet/src/test/java/org/apache/fesod/sheet/write/style/column/LongestMatchColumnWidthStyleStrategyTest.java new file mode 100644 index 0000000000..4e272c390b --- /dev/null +++ b/fesod-sheet/src/test/java/org/apache/fesod/sheet/write/style/column/LongestMatchColumnWidthStyleStrategyTest.java @@ -0,0 +1,92 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.fesod.sheet.write.style.column; + +import java.io.File; +import java.nio.file.Path; +import java.util.Collections; +import org.apache.fesod.sheet.FesodSheet; +import org.apache.fesod.sheet.testkit.Tags; +import org.apache.poi.ss.usermodel.Sheet; +import org.apache.poi.ss.usermodel.Workbook; +import org.apache.poi.ss.usermodel.WorkbookFactory; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Tag; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +/** + * Regression tests for the per-instance width cache of {@link LongestMatchColumnWidthStyleStrategy}. + */ +@Tag(Tags.WRITE) +class LongestMatchColumnWidthStyleStrategyTest { + + private static final String LONG_VALUE = "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"; + + /** + * Control: when every writer gets its own strategy instance, both files get their own width. + */ + @Test + void separateStrategyInstancesComputeEachWorkbookWidth(@TempDir Path dir) throws Exception { + File first = dir.resolve("separate-first.xlsx").toFile(); + File second = dir.resolve("separate-second.xlsx").toFile(); + + write(first, LONG_VALUE, new LongestMatchColumnWidthStyleStrategy()); + write(second, "A", new LongestMatchColumnWidthStyleStrategy()); + + Assertions.assertEquals(LONG_VALUE.length() * 256, columnWidth(first, 0)); + Assertions.assertEquals(1 * 256, columnWidth(second, 0)); + } + + /** + * The width cache is keyed only by sheet number and is never cleared when a workbook finishes, so a strategy + * instance reused for a second writer keeps the first workbook's maximum. As a result the second workbook's + * column width is never set and stays at POI's default instead of the computed 1 * 256. + */ + @Test + void sharedStrategyInstanceDoesNotLeakWidthAcrossWorkbooks(@TempDir Path dir) throws Exception { + File first = dir.resolve("shared-first.xlsx").toFile(); + File second = dir.resolve("shared-second.xlsx").toFile(); + + LongestMatchColumnWidthStyleStrategy shared = new LongestMatchColumnWidthStyleStrategy(); + write(first, LONG_VALUE, shared); + write(second, "A", shared); + + Assertions.assertEquals(LONG_VALUE.length() * 256, columnWidth(first, 0)); + Assertions.assertEquals( + 1 * 256, + columnWidth(second, 0), + "column width leaked from the previous workbook when the strategy instance was reused"); + } + + private static void write(File file, String value, LongestMatchColumnWidthStyleStrategy strategy) { + FesodSheet.write(file) + .registerWriteHandler(strategy) + .sheet() + .doWrite(Collections.singletonList(Collections.singletonList(value))); + } + + private static int columnWidth(File file, int column) throws Exception { + try (Workbook workbook = WorkbookFactory.create(file)) { + Sheet sheet = workbook.getSheetAt(0); + return sheet.getColumnWidth(column); + } + } +} From 044acf293a1eaf6b85b866a14d22c131994de677 Mon Sep 17 00:00:00 2001 From: sanyue <443341200@qq.com> Date: Thu, 5 Nov 2026 19:30:58 +0800 Subject: [PATCH 2/2] docs: document columnWidthCache and fix stale test javadoc --- .../fesod/sheet/write/metadata/holder/WriteSheetHolder.java | 3 +++ .../style/column/LongestMatchColumnWidthStyleStrategyTest.java | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/fesod-sheet/src/main/java/org/apache/fesod/sheet/write/metadata/holder/WriteSheetHolder.java b/fesod-sheet/src/main/java/org/apache/fesod/sheet/write/metadata/holder/WriteSheetHolder.java index 2b790b91d2..5ce2f104be 100644 --- a/fesod-sheet/src/main/java/org/apache/fesod/sheet/write/metadata/holder/WriteSheetHolder.java +++ b/fesod-sheet/src/main/java/org/apache/fesod/sheet/write/metadata/holder/WriteSheetHolder.java @@ -88,6 +88,9 @@ public class WriteSheetHolder extends AbstractWriteHolder { */ private Map hasBeenInitializedTable; + /*** + * Per-sheet-holder cache of maximum column widths, keyed by column index. + */ private Map columnWidthCache; /** diff --git a/fesod-sheet/src/test/java/org/apache/fesod/sheet/write/style/column/LongestMatchColumnWidthStyleStrategyTest.java b/fesod-sheet/src/test/java/org/apache/fesod/sheet/write/style/column/LongestMatchColumnWidthStyleStrategyTest.java index 4e272c390b..7fdd17ccd8 100644 --- a/fesod-sheet/src/test/java/org/apache/fesod/sheet/write/style/column/LongestMatchColumnWidthStyleStrategyTest.java +++ b/fesod-sheet/src/test/java/org/apache/fesod/sheet/write/style/column/LongestMatchColumnWidthStyleStrategyTest.java @@ -33,7 +33,7 @@ import org.junit.jupiter.api.io.TempDir; /** - * Regression tests for the per-instance width cache of {@link LongestMatchColumnWidthStyleStrategy}. + * Regression tests for the per-sheet-holder width cache of {@link LongestMatchColumnWidthStyleStrategy}. */ @Tag(Tags.WRITE) class LongestMatchColumnWidthStyleStrategyTest {