diff --git a/fesod-sheet/src/main/java/org/apache/fesod/sheet/converters/DefaultConverterLoader.java b/fesod-sheet/src/main/java/org/apache/fesod/sheet/converters/DefaultConverterLoader.java index 6200008b5..88b90d955 100644 --- a/fesod-sheet/src/main/java/org/apache/fesod/sheet/converters/DefaultConverterLoader.java +++ b/fesod-sheet/src/main/java/org/apache/fesod/sheet/converters/DefaultConverterLoader.java @@ -25,6 +25,8 @@ package org.apache.fesod.sheet.converters; +import java.util.Collections; +import java.util.HashMap; import java.util.Map; import org.apache.fesod.common.util.MapUtils; import org.apache.fesod.sheet.converters.ConverterKeyBuild.ConverterKey; @@ -139,6 +141,7 @@ private static void initAllConverter() { putAllConverter(new StringNumberConverter()); putAllConverter(new StringStringConverter()); putAllConverter(new StringErrorConverter()); + allConverter = Collections.unmodifiableMap(allConverter); } private static void initDefaultWriteConverter() { @@ -176,6 +179,7 @@ private static void initDefaultWriteConverter() { putWriteStringConverter(new LongStringConverter()); putWriteStringConverter(new ShortStringConverter()); putWriteStringConverter(new StringStringConverter()); + defaultWriteConverter = Collections.unmodifiableMap(defaultWriteConverter); } /** @@ -187,6 +191,15 @@ public static Map> loadDefaultWriteConverter() { return defaultWriteConverter; } + /** + * Copy default write converter + * + * @return + */ + public static Map> copyDefaultWriteConverter() { + return new HashMap<>(loadDefaultWriteConverter()); + } + private static void putWriteConverter(Converter converter) { defaultWriteConverter.put(ConverterKeyBuild.buildKey(converter.supportJavaTypeKey()), converter); } @@ -205,6 +218,15 @@ public static Map> loadDefaultReadConverter() { return loadAllConverter(); } + /** + * Copy default read converter + * + * @return + */ + public static Map> copyDefaultReadConverter() { + return new HashMap<>(loadDefaultReadConverter()); + } + /** * Load all converter * @@ -214,6 +236,15 @@ public static Map> loadAllConverter() { return allConverter; } + /** + * Copy all converter + * + * @return + */ + public static Map> copyAllConverter() { + return new HashMap<>(loadAllConverter()); + } + private static void putAllConverter(Converter converter) { allConverter.put( ConverterKeyBuild.buildKey(converter.supportJavaTypeKey(), converter.supportExcelTypeKey()), converter); diff --git a/fesod-sheet/src/main/java/org/apache/fesod/sheet/read/metadata/holder/AbstractReadHolder.java b/fesod-sheet/src/main/java/org/apache/fesod/sheet/read/metadata/holder/AbstractReadHolder.java index d85c1d200..f7b58be88 100644 --- a/fesod-sheet/src/main/java/org/apache/fesod/sheet/read/metadata/holder/AbstractReadHolder.java +++ b/fesod-sheet/src/main/java/org/apache/fesod/sheet/read/metadata/holder/AbstractReadHolder.java @@ -120,7 +120,7 @@ public AbstractReadHolder(ReadBasicParameter readBasicParameter, AbstractReadHol } if (parentAbstractReadHolder == null) { - setConverterMap(DefaultConverterLoader.loadDefaultReadConverter()); + setConverterMap(DefaultConverterLoader.copyDefaultReadConverter()); } else { setConverterMap(new HashMap<>(parentAbstractReadHolder.getConverterMap())); } diff --git a/fesod-sheet/src/main/java/org/apache/fesod/sheet/write/metadata/holder/AbstractWriteHolder.java b/fesod-sheet/src/main/java/org/apache/fesod/sheet/write/metadata/holder/AbstractWriteHolder.java index 293f18adc..e658b5d8e 100644 --- a/fesod-sheet/src/main/java/org/apache/fesod/sheet/write/metadata/holder/AbstractWriteHolder.java +++ b/fesod-sheet/src/main/java/org/apache/fesod/sheet/write/metadata/holder/AbstractWriteHolder.java @@ -268,7 +268,7 @@ public AbstractWriteHolder(WriteBasicParameter writeBasicParameter, AbstractWrit // Set converterMap if (parentAbstractWriteHolder == null) { - setConverterMap(new HashMap<>(DefaultConverterLoader.loadDefaultWriteConverter())); + setConverterMap(DefaultConverterLoader.copyDefaultWriteConverter()); } else { setConverterMap(new HashMap<>(parentAbstractWriteHolder.getConverterMap())); if (CollectionUtils.isNotEmpty(parentAbstractWriteHolder.getCustomConverterList())) { diff --git a/fesod-sheet/src/test/java/org/apache/fesod/sheet/converters/DefaultConverterLoaderTest.java b/fesod-sheet/src/test/java/org/apache/fesod/sheet/converters/DefaultConverterLoaderTest.java new file mode 100644 index 000000000..e05837b86 --- /dev/null +++ b/fesod-sheet/src/test/java/org/apache/fesod/sheet/converters/DefaultConverterLoaderTest.java @@ -0,0 +1,59 @@ +/* + * 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.converters; + +import java.util.Map; +import org.apache.fesod.sheet.converters.ConverterKeyBuild.ConverterKey; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; + +public class DefaultConverterLoaderTest { + + @Test + void loadDefaultWriteConverterIsImmutableAndCopyIsMutable() { + assertLoadIsImmutableAndCopyIsMutable( + DefaultConverterLoader.loadDefaultWriteConverter(), DefaultConverterLoader.copyDefaultWriteConverter()); + } + + @Test + void loadDefaultReadConverterIsImmutableAndCopyIsMutable() { + assertLoadIsImmutableAndCopyIsMutable( + DefaultConverterLoader.loadDefaultReadConverter(), DefaultConverterLoader.copyDefaultReadConverter()); + } + + @Test + void loadAllConverterIsImmutableAndCopyIsMutable() { + assertLoadIsImmutableAndCopyIsMutable( + DefaultConverterLoader.loadAllConverter(), DefaultConverterLoader.copyAllConverter()); + } + + private static void assertLoadIsImmutableAndCopyIsMutable( + Map> loaded, Map> copy) { + Map.Entry> entry = + loaded.entrySet().iterator().next(); + + Assertions.assertThrows( + UnsupportedOperationException.class, () -> loaded.put(entry.getKey(), entry.getValue())); + + copy.remove(entry.getKey()); + Assertions.assertFalse(copy.containsKey(entry.getKey())); + Assertions.assertTrue(loaded.containsKey(entry.getKey())); + } +} diff --git a/fesod-sheet/src/test/java/org/apache/fesod/sheet/read/ReadConverterIsolationTest.java b/fesod-sheet/src/test/java/org/apache/fesod/sheet/read/ReadConverterIsolationTest.java new file mode 100644 index 000000000..6a7f4101a --- /dev/null +++ b/fesod-sheet/src/test/java/org/apache/fesod/sheet/read/ReadConverterIsolationTest.java @@ -0,0 +1,107 @@ +/* + * 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.read; + +import java.io.File; +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; +import lombok.Data; +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.read.listener.PageReadListener; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; + +/** + * A converter registered on one {@link org.apache.fesod.sheet.ExcelReader} must not leak into a + * later, unrelated read. + */ +public class ReadConverterIsolationTest { + + @Data + public static class StringRow { + private String value; + } + + /** Appends a marker so leakage is observable. */ + public static class MarkerConverter implements Converter { + @Override + public Class supportJavaTypeKey() { + return String.class; + } + + @Override + public CellDataTypeEnum supportExcelTypeKey() { + return CellDataTypeEnum.STRING; + } + + @Override + public String convertToJavaData( + ReadCellData cellData, + ExcelContentProperty contentProperty, + GlobalConfiguration globalConfiguration) { + return cellData.getStringValue() + " [MARKER]"; + } + + @Override + public WriteCellData convertToExcelData( + String value, ExcelContentProperty contentProperty, GlobalConfiguration globalConfiguration) { + return new WriteCellData<>(value); + } + } + + @Test + void registeredConverterDoesNotLeakIntoLaterRead() throws Exception { + File file = File.createTempFile("conv-iso", ".xlsx"); + file.deleteOnExit(); + StringRow out = new StringRow(); + out.setValue("hello"); + FesodSheet.write(file, StringRow.class).sheet().doWrite(Collections.singletonList(out)); + + // First read: register the marker converter -> values carry the marker. + List first = new ArrayList<>(); + FesodSheet.read(file, StringRow.class, new PageReadListener(first::addAll)) + .registerConverter(new MarkerConverter()) + .sheet() + .doRead(); + Assertions.assertEquals(Collections.singletonList("hello [MARKER]"), values(first)); + + // Second read: fresh reader, NO converter registered -> must NOT see the marker. + List second = new ArrayList<>(); + FesodSheet.read(file, StringRow.class, new PageReadListener(second::addAll)) + .sheet() + .doRead(); + Assertions.assertEquals(Collections.singletonList("hello"), values(second)); + } + + private static List values(List rows) { + List out = new ArrayList<>(); + for (StringRow r : rows) { + out.add(r.getValue()); + } + return out; + } +}