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..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 @@ -47,7 +47,7 @@ */ @Getter @Setter -@EqualsAndHashCode(exclude = {"parentWriteWorkbookHolder", "hasBeenInitializedTable"}) +@EqualsAndHashCode(exclude = {"parentWriteWorkbookHolder", "hasBeenInitializedTable", "columnWidthCache"}) @NoArgsConstructor public class WriteSheetHolder extends AbstractWriteHolder { /** @@ -88,6 +88,11 @@ public class WriteSheetHolder extends AbstractWriteHolder { */ private Map hasBeenInitializedTable; + /*** + * Per-sheet-holder cache of maximum column widths, keyed by column index. + */ + 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..7fdd17ccd8 --- /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-sheet-holder 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); + } + } +}