This is an automated email from the ASF dual-hosted git repository.
chaokunyang pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/fory.git
The following commit(s) were added to refs/heads/main by this push:
new fdd1143eb fix(java): preserve externalizable containers in compatible
mode (#3628)
fdd1143eb is described below
commit fdd1143eb297858fba0668a1e97447bff412ce5b
Author: Sebastian Mandrean <[email protected]>
AuthorDate: Tue Apr 28 15:23:19 2026 +0200
fix(java): preserve externalizable containers in compatible mode (#3628)
## Why?
Fory can lose entries for compatible-mode map and collection classes
that implement `Externalizable` if they are handled as ordinary
containers instead of using their externalization logic.
This fixes the data loss reported in #3621.
## What does this PR do?
- Adds a regression test for an `Externalizable` `AbstractMap` stored
through a `Map<String, String>` field.
- Routes `Externalizable` map and collection implementations through the
JDK-compatible serializer path.
- Delegates those containers to `ExternalizableSerializer` so
`writeExternal` / `readExternal` preserve their state.
## Related issues
- Fixes #3621
## AI Contribution Checklist
- [x] Substantial AI assistance was used in this PR: `yes`
- [x] If `yes`, I included a completed [AI Contribution
Checklist](https://github.com/apache/fory/blob/main/AI_POLICY.md#9-contributor-checklist-for-ai-assisted-prs)
in this PR description and the required `AI Usage Disclosure`.
- [x] If `yes`, my PR description includes the required `ai_review`
summary and screenshot evidence of the final clean AI review results
from both fresh reviewers on the current PR diff or current HEAD after
the latest code changes.
AI Usage Disclosure
- substantial_ai_assistance: yes
- scope: issue investigation, regression test drafting, code change
drafting, validation command selection, PR description drafting
- affected_files_or_subsystems: Java serializer resolution, compatible
collection/map serializers, Java map serializer tests
- ai_review: line-by-line self-review completed locally; required
two-fresh-reviewer loop is pending, so this PR remains draft until final
clean review evidence can be attached
- ai_review_artifacts: pending; screenshot evidence or persisted links
for both fresh final clean AI reviews will be added before marking ready
for maintainer review
- human_verification:
- Reproduced the regression by applying the new test on `apache/main`,
where it fails with `Maps do not have the same size:0 != 1`.
- Verified the fixed branch with `mvn -pl fory-core
-Dtest=org.apache.fory.serializer.collection.MapSerializersTest#testExternalizableMapCompatibleMode
test`.
- Ran Java/style/install checks: `mvn -T10 -B --no-transfer-progress
spotless:check checkstyle:check`, `python ./ci/run_ci.py java --version
11`, `python ./ci/run_ci.py java --version 17`, `python ./ci/run_ci.py
java --version 21`, `python ./ci/run_ci.py java --version 25`, `python
./ci/run_ci.py java --install-fory`, and `mvn -T10
--no-transfer-progress clean install -DskipTests
-Dmaven.javadoc.skip=true -Dmaven.source.skip=true`.
- performance_verification: N/A; correctness fix for `Externalizable`
container serializer selection in compatible mode, no benchmark run
- provenance_license_confirmation: Apache-2.0-compatible provenance
confirmed; no incompatible third-party code introduced
## Does this PR introduce any user-facing change?
- [ ] Does this PR introduce any public API change?
- [ ] Does this PR introduce any binary protocol compatibility change?
## Benchmark
Not run. This is a correctness fix for existing compatible-mode
`Externalizable` map/collection handling and does not intentionally
change public APIs or binary protocol format.
---
.../org/apache/fory/resolver/ClassResolver.java | 8 ++-
.../collection/CollectionSerializers.java | 6 ++-
.../fory/serializer/collection/MapSerializers.java | 6 ++-
.../serializer/collection/MapSerializersTest.java | 61 ++++++++++++++++++++++
4 files changed, 77 insertions(+), 4 deletions(-)
diff --git
a/java/fory-core/src/main/java/org/apache/fory/resolver/ClassResolver.java
b/java/fory-core/src/main/java/org/apache/fory/resolver/ClassResolver.java
index e53910260..e9ed3df48 100644
--- a/java/fory-core/src/main/java/org/apache/fory/resolver/ClassResolver.java
+++ b/java/fory-core/src/main/java/org/apache/fory/resolver/ClassResolver.java
@@ -1350,7 +1350,9 @@ public class ClassResolver extends TypeResolver {
if (serializerClass != null) {
return serializerClass;
}
- if (requireJavaSerialization(cls) || useReplaceResolveSerializer(cls))
{
+ if (Externalizable.class.isAssignableFrom(cls)
+ || requireJavaSerialization(cls)
+ || useReplaceResolveSerializer(cls)) {
return CollectionSerializers.JDKCompatibleCollectionSerializer.class;
}
if (!isCrossLanguage()) {
@@ -1364,7 +1366,9 @@ public class ClassResolver extends TypeResolver {
if (serializerClass != null) {
return serializerClass;
}
- if (requireJavaSerialization(cls) || useReplaceResolveSerializer(cls))
{
+ if (Externalizable.class.isAssignableFrom(cls)
+ || requireJavaSerialization(cls)
+ || useReplaceResolveSerializer(cls)) {
return MapSerializers.JDKCompatibleMapSerializer.class;
}
if (!isCrossLanguage()) {
diff --git
a/java/fory-core/src/main/java/org/apache/fory/serializer/collection/CollectionSerializers.java
b/java/fory-core/src/main/java/org/apache/fory/serializer/collection/CollectionSerializers.java
index cc77cebe2..9d33c3ace 100644
---
a/java/fory-core/src/main/java/org/apache/fory/serializer/collection/CollectionSerializers.java
+++
b/java/fory-core/src/main/java/org/apache/fory/serializer/collection/CollectionSerializers.java
@@ -19,6 +19,7 @@
package org.apache.fory.serializer.collection;
+import java.io.Externalizable;
import java.lang.invoke.MethodHandle;
import java.lang.invoke.MethodHandles;
import java.lang.reflect.Field;
@@ -60,6 +61,7 @@ import org.apache.fory.resolver.ClassResolver;
import org.apache.fory.resolver.TypeInfo;
import org.apache.fory.resolver.TypeInfoHolder;
import org.apache.fory.resolver.TypeResolver;
+import org.apache.fory.serializer.ExternalizableSerializer;
import org.apache.fory.serializer.ReplaceResolveSerializer;
import org.apache.fory.serializer.Serializer;
import org.apache.fory.serializer.Serializers;
@@ -949,7 +951,9 @@ public class CollectionSerializers {
Class<? extends Serializer> serializerType =
ClassResolver.useReplaceResolveSerializer(cls)
? ReplaceResolveSerializer.class
- : config.getDefaultJDKStreamSerializerType();
+ : Externalizable.class.isAssignableFrom(cls)
+ ? ExternalizableSerializer.class
+ : config.getDefaultJDKStreamSerializerType();
serializer = Serializers.newSerializer(typeResolver, cls,
serializerType);
}
diff --git
a/java/fory-core/src/main/java/org/apache/fory/serializer/collection/MapSerializers.java
b/java/fory-core/src/main/java/org/apache/fory/serializer/collection/MapSerializers.java
index cb44da66c..ab8688d6a 100644
---
a/java/fory-core/src/main/java/org/apache/fory/serializer/collection/MapSerializers.java
+++
b/java/fory-core/src/main/java/org/apache/fory/serializer/collection/MapSerializers.java
@@ -19,6 +19,7 @@
package org.apache.fory.serializer.collection;
+import java.io.Externalizable;
import java.lang.invoke.MethodHandle;
import java.util.Collection;
import java.util.Collections;
@@ -43,6 +44,7 @@ import org.apache.fory.reflect.ReflectionUtils;
import org.apache.fory.resolver.ClassResolver;
import org.apache.fory.resolver.TypeInfo;
import org.apache.fory.resolver.TypeResolver;
+import org.apache.fory.serializer.ExternalizableSerializer;
import org.apache.fory.serializer.ReplaceResolveSerializer;
import org.apache.fory.serializer.Serializer;
import org.apache.fory.serializer.Serializers;
@@ -459,7 +461,9 @@ public class MapSerializers {
Class<? extends Serializer> serializerType =
ClassResolver.useReplaceResolveSerializer(cls)
? ReplaceResolveSerializer.class
- : config.getDefaultJDKStreamSerializerType();
+ : Externalizable.class.isAssignableFrom(cls)
+ ? ExternalizableSerializer.class
+ : config.getDefaultJDKStreamSerializerType();
serializer = Serializers.newSerializer(typeResolver, cls,
serializerType);
}
diff --git
a/java/fory-core/src/test/java/org/apache/fory/serializer/collection/MapSerializersTest.java
b/java/fory-core/src/test/java/org/apache/fory/serializer/collection/MapSerializersTest.java
index ca594b884..b1548c7fc 100644
---
a/java/fory-core/src/test/java/org/apache/fory/serializer/collection/MapSerializersTest.java
+++
b/java/fory-core/src/test/java/org/apache/fory/serializer/collection/MapSerializersTest.java
@@ -26,6 +26,10 @@ import static
org.apache.fory.collection.Collections.ofHashMap;
import static org.testng.Assert.assertEquals;
import com.google.common.collect.ImmutableMap;
+import java.io.Externalizable;
+import java.io.IOException;
+import java.io.ObjectInput;
+import java.io.ObjectOutput;
import java.io.Serializable;
import java.util.AbstractMap;
import java.util.ArrayList;
@@ -832,6 +836,63 @@ public class MapSerializersTest extends ForyTestBase {
}
}
+ public static class ExternalizableStringMap extends AbstractMap<String,
String>
+ implements Externalizable {
+ private transient Map<String, String> data = new LinkedHashMap<>();
+
+ public ExternalizableStringMap() {}
+
+ @Override
+ public Set<Map.Entry<String, String>> entrySet() {
+ return data.entrySet();
+ }
+
+ @Override
+ public String put(String key, String value) {
+ return data.put(key, value);
+ }
+
+ @Override
+ public void writeExternal(ObjectOutput out) throws IOException {
+ out.writeInt(size());
+ for (Map.Entry<String, String> entry : entrySet()) {
+ out.writeObject(entry.getKey());
+ out.writeObject(entry.getValue());
+ }
+ }
+
+ @Override
+ public void readExternal(ObjectInput in) throws IOException,
ClassNotFoundException {
+ data = new LinkedHashMap<>();
+ int size = in.readInt();
+ for (int i = 0; i < size; i++) {
+ put((String) in.readObject(), (String) in.readObject());
+ }
+ }
+ }
+
+ public static class ExternalizableMapHolder {
+ public Map<String, String> data = new ExternalizableStringMap();
+ }
+
+ @Test(dataProvider = "enableCodegen")
+ public void testExternalizableMapCompatibleMode(boolean enableCodegen) {
+ Fory fory =
+ builder()
+ .withLanguage(Language.JAVA)
+ .withCompatibleMode(CompatibleMode.COMPATIBLE)
+ .withCodegen(enableCodegen)
+ .requireClassRegistration(false)
+ .build();
+ ExternalizableMapHolder holder = new ExternalizableMapHolder();
+ holder.data.put("k", "v");
+
+ ExternalizableMapHolder deserialized = serDe(fory, holder);
+ Assert.assertEquals(deserialized.data.getClass(),
ExternalizableStringMap.class);
+ Assert.assertEquals(deserialized.data, holder.data);
+ Assert.assertEquals(deserialized.data.get("k"), "v");
+ }
+
@Test(dataProvider = "enableCodegen")
public void testDefaultMapSerializer(boolean enableCodegen) {
Fory fory =
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
