This is an automated email from the ASF dual-hosted git repository.
bengbengbalabalabeng pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/fesod.git
The following commit(s) were added to refs/heads/main by this push:
new 893abd3e fix: copy default read converter map to isolate per-reader
custom converters (#972)
893abd3e is described below
commit 893abd3e1954e381b15f298e426a17391fb1dc45
Author: aias00 <[email protected]>
AuthorDate: Thu Jul 30 20:02:21 2026 -0700
fix: copy default read converter map to isolate per-reader custom
converters (#972)
* fix: copy default read converter map to isolate per-reader custom
converters (#971)
The workbook-level read holder aliased DefaultConverterLoader's shared
static
allConverter map instead of copying it, so custom converters registered via
registerConverter() were put() into the global map and leaked into every
later,
unrelated read on the same JVM. Mirror the write side
(AbstractWriteHolder:271),
which already copies, and add a regression test.
* fix: use canonical ASF license header for ReadConverterIsolationTest
* fix: separate license header from package with a blank line
* fix: make default converter maps immutable
* style: format default converter loader test
---------
Co-authored-by: liuhy <[email protected]>
Co-authored-by: Bengbengbalabalabeng
<[email protected]>
Co-authored-by: Shuxin Pan <[email protected]>
---
.../sheet/converters/DefaultConverterLoader.java | 31 ++++++
.../read/metadata/holder/AbstractReadHolder.java | 2 +-
.../write/metadata/holder/AbstractWriteHolder.java | 2 +-
.../converters/DefaultConverterLoaderTest.java | 59 ++++++++++++
.../sheet/read/ReadConverterIsolationTest.java | 107 +++++++++++++++++++++
5 files changed, 199 insertions(+), 2 deletions(-)
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 6200008b..88b90d95 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 @@ public class DefaultConverterLoader {
putAllConverter(new StringNumberConverter());
putAllConverter(new StringStringConverter());
putAllConverter(new StringErrorConverter());
+ allConverter = Collections.unmodifiableMap(allConverter);
}
private static void initDefaultWriteConverter() {
@@ -176,6 +179,7 @@ public class DefaultConverterLoader {
putWriteStringConverter(new LongStringConverter());
putWriteStringConverter(new ShortStringConverter());
putWriteStringConverter(new StringStringConverter());
+ defaultWriteConverter =
Collections.unmodifiableMap(defaultWriteConverter);
}
/**
@@ -187,6 +191,15 @@ public class DefaultConverterLoader {
return defaultWriteConverter;
}
+ /**
+ * Copy default write converter
+ *
+ * @return
+ */
+ public static Map<ConverterKey, Converter<?>> copyDefaultWriteConverter() {
+ return new HashMap<>(loadDefaultWriteConverter());
+ }
+
private static void putWriteConverter(Converter<?> converter) {
defaultWriteConverter.put(ConverterKeyBuild.buildKey(converter.supportJavaTypeKey()),
converter);
}
@@ -205,6 +218,15 @@ public class DefaultConverterLoader {
return loadAllConverter();
}
+ /**
+ * Copy default read converter
+ *
+ * @return
+ */
+ public static Map<ConverterKey, Converter<?>> copyDefaultReadConverter() {
+ return new HashMap<>(loadDefaultReadConverter());
+ }
+
/**
* Load all converter
*
@@ -214,6 +236,15 @@ public class DefaultConverterLoader {
return allConverter;
}
+ /**
+ * Copy all converter
+ *
+ * @return
+ */
+ public static Map<ConverterKey, Converter<?>> 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 d85c1d20..f7b58be8 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 abstract class AbstractReadHolder extends
AbstractHolder implements ReadH
}
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 293f18ad..e658b5d8 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 abstract class AbstractWriteHolder extends
AbstractHolder implements Writ
// 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 00000000..e05837b8
--- /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<ConverterKey, Converter<?>> loaded, Map<ConverterKey,
Converter<?>> copy) {
+ Map.Entry<ConverterKey, Converter<?>> 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 00000000..6a7f4101
--- /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<String> {
+ @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<StringRow> first = new ArrayList<>();
+ FesodSheet.read(file, StringRow.class, new
PageReadListener<StringRow>(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<StringRow> second = new ArrayList<>();
+ FesodSheet.read(file, StringRow.class, new
PageReadListener<StringRow>(second::addAll))
+ .sheet()
+ .doRead();
+ Assertions.assertEquals(Collections.singletonList("hello"),
values(second));
+ }
+
+ private static List<String> values(List<StringRow> rows) {
+ List<String> out = new ArrayList<>();
+ for (StringRow r : rows) {
+ out.add(r.getValue());
+ }
+ return out;
+ }
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
