Skip to content
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -175,10 +176,10 @@ private static void initDefaultWriteConverter() {
/**
* Load default write converter
*
* @return
* @return Unmodifiable map of default write converters
*/
public static Map<ConverterKey, Converter<?>> loadDefaultWriteConverter() {
return defaultWriteConverter;
return Collections.unmodifiableMap(defaultWriteConverter);
}

private static void putWriteConverter(Converter<?> converter) {
Expand All @@ -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<ConverterKey, Converter<?>> loadDefaultReadConverter() {
return loadAllConverter();
Expand All @@ -202,10 +203,10 @@ public static Map<ConverterKey, Converter<?>> loadDefaultReadConverter() {
/**
* Load all converter
*
* @return
* @return Unmodifiable map of all converters
*/
public static Map<ConverterKey, Converter<?>> loadAllConverter() {
return allConverter;
return Collections.unmodifiableMap(allConverter);
}

private static void putAllConverter(Converter<?> converter) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()));
}
Expand Down
Original file line number Diff line number Diff line change
@@ -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<String> {

@Override
public Class<String> 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");
}
Comment on lines +77 to +93
Copy link

Copilot AI Dec 17, 2025

Choose a reason for hiding this comment

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

The test only validates converter isolation for ExcelWriter instances, but the PR description and code changes indicate that the fix also applies to ExcelReader instances. Consider adding a corresponding test case for ExcelReader to ensure complete coverage of the bug fix and prevent regression in the read path.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Added a test for ExcelReader at 1d958e9.

}