From 74daad3832be641e7e4fb85dffd88e925164bd03 Mon Sep 17 00:00:00 2001 From: sijun-yang Date: Wed, 17 Dec 2025 11:39:50 +0900 Subject: [PATCH 1/6] fix: return immutable maps from DefaultConverterLoader --- .../sheet/converters/DefaultConverterLoader.java | 11 ++++++----- .../read/metadata/holder/AbstractReadHolder.java | 2 +- .../write/metadata/holder/AbstractWriteHolder.java | 2 +- 3 files changed, 8 insertions(+), 7 deletions(-) diff --git a/fesod/src/main/java/org/apache/fesod/sheet/converters/DefaultConverterLoader.java b/fesod/src/main/java/org/apache/fesod/sheet/converters/DefaultConverterLoader.java index 3b8a144d0..76c66fe0d 100644 --- a/fesod/src/main/java/org/apache/fesod/sheet/converters/DefaultConverterLoader.java +++ b/fesod/src/main/java/org/apache/fesod/sheet/converters/DefaultConverterLoader.java @@ -19,6 +19,7 @@ package org.apache.fesod.sheet.converters; +import java.util.Collections; import java.util.Map; import org.apache.fesod.sheet.converters.ConverterKeyBuild.ConverterKey; import org.apache.fesod.sheet.converters.bigdecimal.BigDecimalBooleanConverter; @@ -175,10 +176,10 @@ private static void initDefaultWriteConverter() { /** * Load default write converter * - * @return + * @return Unmodifiable map of default write converters */ public static Map> loadDefaultWriteConverter() { - return defaultWriteConverter; + return Collections.unmodifiableMap(defaultWriteConverter); } private static void putWriteConverter(Converter converter) { @@ -193,7 +194,7 @@ private static void putWriteStringConverter(Converter converter) { /** * Load default read converter * - * @return + * @return Unmodifiable map of default read converters */ public static Map> loadDefaultReadConverter() { return loadAllConverter(); @@ -202,10 +203,10 @@ public static Map> loadDefaultReadConverter() { /** * Load all converter * - * @return + * @return Unmodifiable map of all converters */ public static Map> loadAllConverter() { - return allConverter; + return Collections.unmodifiableMap(allConverter); } private static void putAllConverter(Converter converter) { diff --git a/fesod/src/main/java/org/apache/fesod/sheet/read/metadata/holder/AbstractReadHolder.java b/fesod/src/main/java/org/apache/fesod/sheet/read/metadata/holder/AbstractReadHolder.java index 779ec0c04..0e451b311 100644 --- a/fesod/src/main/java/org/apache/fesod/sheet/read/metadata/holder/AbstractReadHolder.java +++ b/fesod/src/main/java/org/apache/fesod/sheet/read/metadata/holder/AbstractReadHolder.java @@ -114,7 +114,7 @@ public AbstractReadHolder(ReadBasicParameter readBasicParameter, AbstractReadHol } if (parentAbstractReadHolder == null) { - setConverterMap(DefaultConverterLoader.loadDefaultReadConverter()); + setConverterMap(new HashMap<>(DefaultConverterLoader.loadDefaultReadConverter())); } else { setConverterMap(new HashMap<>(parentAbstractReadHolder.getConverterMap())); } diff --git a/fesod/src/main/java/org/apache/fesod/sheet/write/metadata/holder/AbstractWriteHolder.java b/fesod/src/main/java/org/apache/fesod/sheet/write/metadata/holder/AbstractWriteHolder.java index 80122ba2f..88b2ce70d 100644 --- a/fesod/src/main/java/org/apache/fesod/sheet/write/metadata/holder/AbstractWriteHolder.java +++ b/fesod/src/main/java/org/apache/fesod/sheet/write/metadata/holder/AbstractWriteHolder.java @@ -255,7 +255,7 @@ public AbstractWriteHolder(WriteBasicParameter writeBasicParameter, AbstractWrit // Set converterMap if (parentAbstractWriteHolder == null) { - setConverterMap(DefaultConverterLoader.loadDefaultWriteConverter()); + setConverterMap(new HashMap<>(DefaultConverterLoader.loadDefaultWriteConverter())); } else { setConverterMap(new HashMap<>(parentAbstractWriteHolder.getConverterMap())); } From 595085f1bc09a05ef1692d3cd2cc4216aec691a2 Mon Sep 17 00:00:00 2001 From: sijun-yang Date: Wed, 17 Dec 2025 11:39:54 +0900 Subject: [PATCH 2/6] test: verify custom converters do not leak across ExcelWriter instances --- .../converter/ConverterIsolationTest.java | 73 +++++++++++++++++++ 1 file changed, 73 insertions(+) create mode 100644 fesod/src/test/java/org/apache/fesod/sheet/converter/ConverterIsolationTest.java diff --git a/fesod/src/test/java/org/apache/fesod/sheet/converter/ConverterIsolationTest.java b/fesod/src/test/java/org/apache/fesod/sheet/converter/ConverterIsolationTest.java new file mode 100644 index 000000000..12c1399c0 --- /dev/null +++ b/fesod/src/test/java/org/apache/fesod/sheet/converter/ConverterIsolationTest.java @@ -0,0 +1,73 @@ +/* + * 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.converter; + +import static org.junit.jupiter.api.Assertions.*; +import java.io.File; +import org.apache.fesod.sheet.ExcelWriter; +import org.apache.fesod.sheet.FesodSheet; +import org.apache.fesod.sheet.converters.Converter; +import org.apache.fesod.sheet.enums.CellDataTypeEnum; +import org.apache.fesod.sheet.metadata.GlobalConfiguration; +import org.apache.fesod.sheet.metadata.data.WriteCellData; +import org.apache.fesod.sheet.metadata.property.ExcelContentProperty; +import org.apache.fesod.sheet.util.TestFileUtil; +import org.junit.jupiter.api.Test; + +public class ConverterIsolationTest { + + public static class TestData {} + + public static class ConverterA implements Converter { + + @Override + public Class supportJavaTypeKey() { + return String.class; + } + + @Override + public CellDataTypeEnum supportExcelTypeKey() { + return CellDataTypeEnum.STRING; + } + + @Override + public WriteCellData convertToExcelData(String value, ExcelContentProperty p, GlobalConfiguration g) { + return new WriteCellData<>("A-" + value); + } + } + + @Test + public void testConverterIsolation() { + ExcelWriter writer1 = FesodSheet.write(new File(TestFileUtil.getPath() + "writer1.xlsx"), TestData.class) + .registerConverter(new ConverterA()) + .build(); + + ExcelWriter writer2 = FesodSheet.write(new File(TestFileUtil.getPath() + "writer2.xlsx"), TestData.class) + .build(); + + boolean writer2HasConverterA = writer2.writeContext().currentWriteHolder().converterMap().values().stream() + .anyMatch(c -> c instanceof ConverterA); + + writer1.finish(); + writer2.finish(); + + assertFalse(writer2HasConverterA, "Custom converter should not leak between ExcelWriter instances"); + } +} From 1d958e9f1ca967a2eaed7bb9882fe5e8999f4a58 Mon Sep 17 00:00:00 2001 From: sijun-yang Date: Wed, 17 Dec 2025 12:23:43 +0900 Subject: [PATCH 3/6] test: verify custom converters do not leak across ExcelReader instances --- .../converter/ConverterIsolationTest.java | 55 +++++++++++++++++-- 1 file changed, 51 insertions(+), 4 deletions(-) diff --git a/fesod/src/test/java/org/apache/fesod/sheet/converter/ConverterIsolationTest.java b/fesod/src/test/java/org/apache/fesod/sheet/converter/ConverterIsolationTest.java index 12c1399c0..21ede9eb0 100644 --- a/fesod/src/test/java/org/apache/fesod/sheet/converter/ConverterIsolationTest.java +++ b/fesod/src/test/java/org/apache/fesod/sheet/converter/ConverterIsolationTest.java @@ -21,11 +21,15 @@ import static org.junit.jupiter.api.Assertions.*; import java.io.File; +import java.util.ArrayList; + +import org.apache.fesod.sheet.ExcelReader; import org.apache.fesod.sheet.ExcelWriter; import org.apache.fesod.sheet.FesodSheet; import org.apache.fesod.sheet.converters.Converter; import org.apache.fesod.sheet.enums.CellDataTypeEnum; import org.apache.fesod.sheet.metadata.GlobalConfiguration; +import org.apache.fesod.sheet.metadata.data.ReadCellData; import org.apache.fesod.sheet.metadata.data.WriteCellData; import org.apache.fesod.sheet.metadata.property.ExcelContentProperty; import org.apache.fesod.sheet.util.TestFileUtil; @@ -35,7 +39,7 @@ public class ConverterIsolationTest { public static class TestData {} - public static class ConverterA implements Converter { + public static class WriteConverterA implements Converter { @Override public Class supportJavaTypeKey() { @@ -53,21 +57,64 @@ public WriteCellData convertToExcelData(String value, ExcelContentProperty p, } } + public static class ReadConverterA implements Converter { + + @Override + public Class supportJavaTypeKey() { + return String.class; + } + + @Override + public CellDataTypeEnum supportExcelTypeKey() { + return CellDataTypeEnum.STRING; + } + + @Override + public String convertToJavaData(ReadCellData cellData, ExcelContentProperty p, GlobalConfiguration g) { + return "A-" + cellData.getStringValue(); + } + } + @Test - public void testConverterIsolation() { + public void testWriterConverterIsolation() { ExcelWriter writer1 = FesodSheet.write(new File(TestFileUtil.getPath() + "writer1.xlsx"), TestData.class) - .registerConverter(new ConverterA()) + .registerConverter(new WriteConverterA()) .build(); ExcelWriter writer2 = FesodSheet.write(new File(TestFileUtil.getPath() + "writer2.xlsx"), TestData.class) .build(); boolean writer2HasConverterA = writer2.writeContext().currentWriteHolder().converterMap().values().stream() - .anyMatch(c -> c instanceof ConverterA); + .anyMatch(c -> c instanceof WriteConverterA); writer1.finish(); writer2.finish(); assertFalse(writer2HasConverterA, "Custom converter should not leak between ExcelWriter instances"); } + + @Test + public void testReaderConverterIsolation() { + File testFile = TestFileUtil.createNewFile("converter_isolation_test.xlsx"); + + FesodSheet.write(testFile, TestData.class) + .sheet() + .doWrite(new ArrayList<>()); + + ExcelReader reader1 = FesodSheet.read(testFile, TestData.class, null) + .registerConverter(new ReadConverterA()) + .build(); + + ExcelReader reader2 = FesodSheet.read(testFile, TestData.class, null) + .build(); + + boolean leaked = reader2.analysisContext().currentReadHolder() + .converterMap().values().stream() + .anyMatch(c -> c instanceof ReadConverterA); + + reader1.finish(); + reader2.finish(); + + assertFalse(leaked); + } } From bdef645e7a3a6cc80055f41ea8071f373899354f Mon Sep 17 00:00:00 2001 From: sijun-yang Date: Wed, 17 Dec 2025 12:26:05 +0900 Subject: [PATCH 4/6] style: update code style --- .../fesod/sheet/converter/ConverterIsolationTest.java | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/fesod/src/test/java/org/apache/fesod/sheet/converter/ConverterIsolationTest.java b/fesod/src/test/java/org/apache/fesod/sheet/converter/ConverterIsolationTest.java index 21ede9eb0..7473ecd98 100644 --- a/fesod/src/test/java/org/apache/fesod/sheet/converter/ConverterIsolationTest.java +++ b/fesod/src/test/java/org/apache/fesod/sheet/converter/ConverterIsolationTest.java @@ -22,7 +22,6 @@ import static org.junit.jupiter.api.Assertions.*; import java.io.File; import java.util.ArrayList; - import org.apache.fesod.sheet.ExcelReader; import org.apache.fesod.sheet.ExcelWriter; import org.apache.fesod.sheet.FesodSheet; @@ -97,19 +96,15 @@ public void testWriterConverterIsolation() { public void testReaderConverterIsolation() { File testFile = TestFileUtil.createNewFile("converter_isolation_test.xlsx"); - FesodSheet.write(testFile, TestData.class) - .sheet() - .doWrite(new ArrayList<>()); + FesodSheet.write(testFile, TestData.class).sheet().doWrite(new ArrayList<>()); ExcelReader reader1 = FesodSheet.read(testFile, TestData.class, null) .registerConverter(new ReadConverterA()) .build(); - ExcelReader reader2 = FesodSheet.read(testFile, TestData.class, null) - .build(); + ExcelReader reader2 = FesodSheet.read(testFile, TestData.class, null).build(); - boolean leaked = reader2.analysisContext().currentReadHolder() - .converterMap().values().stream() + boolean leaked = reader2.analysisContext().currentReadHolder().converterMap().values().stream() .anyMatch(c -> c instanceof ReadConverterA); reader1.finish(); From e14f40dcc5e935929b38ead7fd038afa72222fe7 Mon Sep 17 00:00:00 2001 From: sijun-yang Date: Wed, 17 Dec 2025 13:04:05 +0900 Subject: [PATCH 5/6] test: add missing @TestMethodOrder annotation --- .../apache/fesod/sheet/converter/ConverterIsolationTest.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/fesod/src/test/java/org/apache/fesod/sheet/converter/ConverterIsolationTest.java b/fesod/src/test/java/org/apache/fesod/sheet/converter/ConverterIsolationTest.java index 7473ecd98..666864ef0 100644 --- a/fesod/src/test/java/org/apache/fesod/sheet/converter/ConverterIsolationTest.java +++ b/fesod/src/test/java/org/apache/fesod/sheet/converter/ConverterIsolationTest.java @@ -32,8 +32,11 @@ import org.apache.fesod.sheet.metadata.data.WriteCellData; import org.apache.fesod.sheet.metadata.property.ExcelContentProperty; import org.apache.fesod.sheet.util.TestFileUtil; +import org.junit.jupiter.api.MethodOrderer; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.TestMethodOrder; +@TestMethodOrder(MethodOrderer.MethodName.class) public class ConverterIsolationTest { public static class TestData {} From a0f920727dc8f2d4809ca4c6b08a0dd2c817563d Mon Sep 17 00:00:00 2001 From: sijun-yang Date: Wed, 17 Dec 2025 13:05:32 +0900 Subject: [PATCH 6/6] Revert "test: add missing @TestMethodOrder annotation" This reverts commit e14f40dcc5e935929b38ead7fd038afa72222fe7. --- .../apache/fesod/sheet/converter/ConverterIsolationTest.java | 3 --- 1 file changed, 3 deletions(-) diff --git a/fesod/src/test/java/org/apache/fesod/sheet/converter/ConverterIsolationTest.java b/fesod/src/test/java/org/apache/fesod/sheet/converter/ConverterIsolationTest.java index 666864ef0..7473ecd98 100644 --- a/fesod/src/test/java/org/apache/fesod/sheet/converter/ConverterIsolationTest.java +++ b/fesod/src/test/java/org/apache/fesod/sheet/converter/ConverterIsolationTest.java @@ -32,11 +32,8 @@ import org.apache.fesod.sheet.metadata.data.WriteCellData; import org.apache.fesod.sheet.metadata.property.ExcelContentProperty; import org.apache.fesod.sheet.util.TestFileUtil; -import org.junit.jupiter.api.MethodOrderer; import org.junit.jupiter.api.Test; -import org.junit.jupiter.api.TestMethodOrder; -@TestMethodOrder(MethodOrderer.MethodName.class) public class ConverterIsolationTest { public static class TestData {}