From 7c70cd283f13ee41875446a330041fa489e88e20 Mon Sep 17 00:00:00 2001 From: zwsong Date: Fri, 4 Sep 2026 23:04:49 +0800 Subject: [PATCH 1/4] fix(core): keep TypeDef root kind consistent for factory custom serializers ClassResolver.getTypeDefRootTypeId treated unregistered classes handled by a configured SerializerFactory as struct-owned (NAMED_COMPATIBLE_STRUCT=30) when no TypeInfo existed yet, while the writer encoded NAMED_EXT=32. Probe the SerializerFactory (cached, re-entrancy guarded, conservative on error): when it would supply a custom serializer, normalize the root kind to NAMED_EXT so reader and writer agree. Regression test: two identical instances sharing meta contexts, neither pre-registering the factory-serialized class, round-trip a struct that nests the non-collection subclass; the pre-registration variant is also verified. --- .../apache/fory/resolver/ClassResolver.java | 51 +++++- .../TypeDefRootKindSymmetricReproTest.java | 167 ++++++++++++++++++ 2 files changed, 217 insertions(+), 1 deletion(-) create mode 100644 java/fory-core/src/test/java/org/apache/fory/meta/TypeDefRootKindSymmetricReproTest.java 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 b347eb3865..630fce8a61 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 @@ -969,6 +969,44 @@ && checkType(cls.getName()) return typeId; } + // Cache of classes that a configured SerializerFactory would custom-serialize (i.e. non-struct). + private final java.util.Set> factoryCustomTrue = new java.util.HashSet<>(); + private final java.util.Set> factoryCustomFalse = new java.util.HashSet<>(); + private final java.util.Set> factoryCustomProbing = new java.util.HashSet<>(); + + /** + * Whether {@link #createSerializerFromFactory} would supply a custom (non-struct) serializer for + * the class. Keeps the TypeDef root kind consistent with what the writer encodes, without forcing + * serializer construction during class-metadata building. On any error or re-entrancy the class is + * treated conservatively as struct-owned. + */ + private boolean isCustomSerializedByFactory(Class cls) { + if (factoryCustomTrue.contains(cls)) { + return true; + } + if (factoryCustomFalse.contains(cls)) { + return false; + } + if (!factoryCustomProbing.add(cls)) { + // re-entrant probe for the same class: bail out conservatively + return false; + } + try { + if (createSerializerFromFactory(cls) != null) { + factoryCustomTrue.add(cls); + return true; + } + factoryCustomFalse.add(cls); + return false; + } catch (Throwable t) { + // Conservative: treat as struct-owned if the factory cannot answer safely here. + factoryCustomFalse.add(cls); + return false; + } finally { + factoryCustomProbing.remove(cls); + } + } + public int getTypeDefRootTypeId(Class cls, boolean hasFieldMetadata) { if (hasFieldMetadata) { // Preserve the normal TypeInfo/name cache so locally generated or dynamically registered @@ -980,6 +1018,11 @@ public int getTypeDefRootTypeId(Class cls, boolean hasFieldMetadata) { // must still resolve to the non-struct serializer family on the reader. return normalizeTypeDefRootTypeId(cls, typeId); } + if (isCustomSerializedByFactory(cls)) { + // A factory custom serializer is non-struct on the writer (NAMED_EXT); the reader must not + // downgrade it to a struct/compatible root kind just because it has no TypeInfo yet. + return Types.NAMED_EXT; + } return getFieldMetadataTypeIdForTypeDef(cls); } TypeInfo typeInfo = classInfoMap.get(cls); @@ -994,7 +1037,13 @@ public int getTypeDefRootTypeId(Class cls, boolean hasFieldMetadata) { } return normalizeTypeDefRootTypeId(cls, typeInfo.typeId); } - return usesNonStructTypeDef(cls) ? Types.NAMED_EXT : buildUnregisteredTypeId(cls, null); + if (usesNonStructTypeDef(cls)) { + return Types.NAMED_EXT; + } + if (isCustomSerializedByFactory(cls)) { + return Types.NAMED_EXT; + } + return buildUnregisteredTypeId(cls, null); } private int getFieldMetadataTypeIdForTypeDef(Class cls) { diff --git a/java/fory-core/src/test/java/org/apache/fory/meta/TypeDefRootKindSymmetricReproTest.java b/java/fory-core/src/test/java/org/apache/fory/meta/TypeDefRootKindSymmetricReproTest.java new file mode 100644 index 0000000000..19f8f03291 --- /dev/null +++ b/java/fory-core/src/test/java/org/apache/fory/meta/TypeDefRootKindSymmetricReproTest.java @@ -0,0 +1,167 @@ +/* + * 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.fory.meta; + +import org.apache.fory.Fory; +import org.apache.fory.config.CompatibleMode; +import org.apache.fory.context.MetaReadContext; +import org.apache.fory.context.MetaWriteContext; +import org.apache.fory.context.ReadContext; +import org.apache.fory.context.WriteContext; +import org.apache.fory.exception.DeserializationException; +import org.apache.fory.serializer.Serializer; +import org.apache.fory.serializer.SerializerFactory; +import org.testng.Assert; +import org.testng.annotations.Test; + +/** + * Regression test: two identical {@link Fory} instances (same code, shared meta contexts, neither + * pre-registering a {@code SerializerFactory}-handled class) must round-trip a struct that nests a + * NON-collection class routed through a factory custom serializer. + * + *

Without the fix, the writer lazily resolves the nested subclass to its factory custom + * serializer and encodes root kind {@code NAMED_EXT} (32), while the reader decoding the field + * through field metadata expects {@code NAMED_COMPATIBLE_STRUCT} (30) — the class is neither + * registered nor a recognized non-struct family — producing {@code "TypeDef root kind does not + * match ... expected=30, actual=32"}. + * + *

{@link #withRegistration_roundTripOk()} documents that pre-registering the subclass also keeps + * the root kind consistent. + */ +public class TypeDefRootKindSymmetricReproTest { + + /** Base interface of a class that is serialized through the factory custom serializer. */ + public interface Restriction {} + + /** Non-collection subclass routed through the factory custom serializer. */ + public static class MyRestriction implements Restriction { + private String code; + + public MyRestriction() {} + + public MyRestriction(String code) { + this.code = code; + } + + public String getCode() { + return code; + } + + public void setCode(String code) { + this.code = code; + } + } + + /** Struct whose field is typed with the base interface and holds the subclass at runtime. */ + public static class Container { + private Restriction restriction; + + public Container() {} + + public Container(Restriction restriction) { + this.restriction = restriction; + } + + public Restriction getRestriction() { + return restriction; + } + + public void setRestriction(Restriction restriction) { + this.restriction = restriction; + } + } + + /** Custom serializer supplied by the factory for {@link MyRestriction}. */ + public static class RestrictionSerializer extends Serializer { + @Override + public void write(WriteContext ctx, MyRestriction value) { + ctx.writeRef(value.getCode()); + } + + @Override + public MyRestriction read(ReadContext ctx) { + MyRestriction r = new MyRestriction((String) ctx.readRef()); + ctx.reference(r); + return r; + } + } + + private static SerializerFactory restrictionFactory() { + return (typeResolver, cls) -> Restriction.class.isAssignableFrom(cls) ? new RestrictionSerializer() : null; + } + + private static Fory buildFory(boolean preRegister) { + Fory fory = + Fory.builder() + .withXlang(false) + .withMetaShare(true) + .requireClassRegistration(false) + .withRefTracking(true) + .withCompatibleMode(CompatibleMode.COMPATIBLE) + .withIntCompressed(true) + .withLongCompressed(org.apache.fory.config.Int64Encoding.VARINT) + .withCodegen(true) + .withAsyncCompilation(false) + .withSerializerFactory(restrictionFactory()) + .build(); + if (preRegister) { + fory.register(MyRestriction.class); + } + return fory; + } + + @Test + public void noRegistration_symmetric_roundTripOk() { + Fory writer = buildFory(false); + Fory reader = buildFory(false); + MetaWriteContext wc = new MetaWriteContext(); + MetaReadContext rc = new MetaReadContext(); + writer.setMetaWriteContext(wc); + writer.setMetaReadContext(rc); + reader.setMetaWriteContext(wc); + reader.setMetaReadContext(rc); + + byte[] bytes = writer.serialize(new Container(new MyRestriction("A"))); + // Regression guard: before the ClassResolver root-kind fix this threw + // DeserializationException "TypeDef root kind does not match ... expected=30, actual=32". + Container read = reader.deserialize(bytes, Container.class); + Assert.assertNotNull(read); + Assert.assertNotNull(read.getRestriction()); + Assert.assertEquals(((MyRestriction) read.getRestriction()).getCode(), "A"); + } + + @Test + public void withRegistration_roundTripOk() { + Fory writer = buildFory(true); + Fory reader = buildFory(true); + MetaWriteContext wc = new MetaWriteContext(); + MetaReadContext rc = new MetaReadContext(); + writer.setMetaWriteContext(wc); + writer.setMetaReadContext(rc); + reader.setMetaWriteContext(wc); + reader.setMetaReadContext(rc); + + byte[] bytes = writer.serialize(new Container(new MyRestriction("A"))); + Container read = reader.deserialize(bytes, Container.class); + Assert.assertNotNull(read); + Assert.assertNotNull(read.getRestriction()); + Assert.assertEquals(((MyRestriction) read.getRestriction()).getCode(), "A"); + } +} From e80df8941bed2d41fa2f3eb9e00adce19f9b7c2b Mon Sep 17 00:00:00 2001 From: zwsong Date: Sat, 5 Sep 2026 16:40:25 +0800 Subject: [PATCH 2/4] refactor: simplify isCustomSerializedByFactory to ConcurrentHashMap.computeIfAbsent Replace three HashSet caches + manual re-entrancy guard with a single ConcurrentHashMap, Boolean> and computeIfAbsent. Thread-safe, simpler, same semantics. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../apache/fory/resolver/ClassResolver.java | 45 +++++-------------- 1 file changed, 11 insertions(+), 34 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 630fce8a61..7960cee96c 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 @@ -247,6 +247,11 @@ public class ClassResolver extends TypeResolver { private final ObjectMap compositeNameBytes2TypeInfo = new ObjectMap<>(16, foryMapLoadFactor); private final ShimDispatcher shimDispatcher; + // Caches whether a configured SerializerFactory would supply a custom serializer for each class. + // Used by isCustomSerializedByFactory() to keep TypeDef root kind consistent between writer and + // reader without repeatedly invoking the factory. Thread-safe for cross-instance scenarios. + private final ConcurrentHashMap, Boolean> extSerializerFlagCache = + new ConcurrentHashMap<>(); public ClassResolver( Config config, @@ -969,42 +974,14 @@ && checkType(cls.getName()) return typeId; } - // Cache of classes that a configured SerializerFactory would custom-serialize (i.e. non-struct). - private final java.util.Set> factoryCustomTrue = new java.util.HashSet<>(); - private final java.util.Set> factoryCustomFalse = new java.util.HashSet<>(); - private final java.util.Set> factoryCustomProbing = new java.util.HashSet<>(); - - /** - * Whether {@link #createSerializerFromFactory} would supply a custom (non-struct) serializer for - * the class. Keeps the TypeDef root kind consistent with what the writer encodes, without forcing - * serializer construction during class-metadata building. On any error or re-entrancy the class is - * treated conservatively as struct-owned. - */ private boolean isCustomSerializedByFactory(Class cls) { - if (factoryCustomTrue.contains(cls)) { - return true; - } - if (factoryCustomFalse.contains(cls)) { - return false; - } - if (!factoryCustomProbing.add(cls)) { - // re-entrant probe for the same class: bail out conservatively - return false; - } - try { - if (createSerializerFromFactory(cls) != null) { - factoryCustomTrue.add(cls); - return true; + return extSerializerFlagCache.computeIfAbsent(cls, c -> { + try { + return createSerializerFromFactory(c) != null; + } catch (Throwable t) { + return false; } - factoryCustomFalse.add(cls); - return false; - } catch (Throwable t) { - // Conservative: treat as struct-owned if the factory cannot answer safely here. - factoryCustomFalse.add(cls); - return false; - } finally { - factoryCustomProbing.remove(cls); - } + }); } public int getTypeDefRootTypeId(Class cls, boolean hasFieldMetadata) { From ca1e78c475fdbaa5ab9d43c3bdde1c973bd5113c Mon Sep 17 00:00:00 2001 From: chaokunyang Date: Sat, 5 Sep 2026 19:21:16 +0800 Subject: [PATCH 3/4] fix(java): resolve factory TypeDef roots through existing serializers --- .../fory/meta/NativeTypeDefDecoder.java | 6 + .../apache/fory/resolver/ClassResolver.java | 28 +-- .../apache/fory/resolver/TypeResolver.java | 18 +- .../TypeDefRootKindSymmetricReproTest.java | 167 ----------------- .../fory/resolver/ClassResolverTest.java | 34 ++++ .../serializer/SerializerFactoryTest.java | 170 ++++++++++++++++++ 6 files changed, 227 insertions(+), 196 deletions(-) delete mode 100644 java/fory-core/src/test/java/org/apache/fory/meta/TypeDefRootKindSymmetricReproTest.java diff --git a/java/fory-core/src/main/java/org/apache/fory/meta/NativeTypeDefDecoder.java b/java/fory-core/src/main/java/org/apache/fory/meta/NativeTypeDefDecoder.java index b69c42af25..8e6abf52cf 100644 --- a/java/fory-core/src/main/java/org/apache/fory/meta/NativeTypeDefDecoder.java +++ b/java/fory-core/src/main/java/org/apache/fory/meta/NativeTypeDefDecoder.java @@ -230,6 +230,12 @@ static TypeDef decodeTypeDef( // Native TypeDef can carry class-layer fields even when the root wire type is an enum, // map, or other non-struct wrapper. Validate the resolved root class kind instead. if (rootClass != null) { + if (!hasFieldMetadata && Types.isExtType(rootTypeId)) { + // Extension roots need their actual serializer: a metadata-only TypeInfo may still have + // a provisional struct kind. Resolve and retain it through the normal owner here, not + // while building field metadata, which can recurse into serializer construction. + resolver.getTypeInfo(rootClass); + } int expectedRootTypeId = resolver.getTypeDefRootTypeId(rootClass, hasFieldMetadata); if (!isCompatibleRootKind(expectedRootTypeId, rootTypeId, !rootClassLayerRegistered)) { throw new DeserializationException( 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 7960cee96c..b347eb3865 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 @@ -247,11 +247,6 @@ public class ClassResolver extends TypeResolver { private final ObjectMap compositeNameBytes2TypeInfo = new ObjectMap<>(16, foryMapLoadFactor); private final ShimDispatcher shimDispatcher; - // Caches whether a configured SerializerFactory would supply a custom serializer for each class. - // Used by isCustomSerializedByFactory() to keep TypeDef root kind consistent between writer and - // reader without repeatedly invoking the factory. Thread-safe for cross-instance scenarios. - private final ConcurrentHashMap, Boolean> extSerializerFlagCache = - new ConcurrentHashMap<>(); public ClassResolver( Config config, @@ -974,16 +969,6 @@ && checkType(cls.getName()) return typeId; } - private boolean isCustomSerializedByFactory(Class cls) { - return extSerializerFlagCache.computeIfAbsent(cls, c -> { - try { - return createSerializerFromFactory(c) != null; - } catch (Throwable t) { - return false; - } - }); - } - public int getTypeDefRootTypeId(Class cls, boolean hasFieldMetadata) { if (hasFieldMetadata) { // Preserve the normal TypeInfo/name cache so locally generated or dynamically registered @@ -995,11 +980,6 @@ public int getTypeDefRootTypeId(Class cls, boolean hasFieldMetadata) { // must still resolve to the non-struct serializer family on the reader. return normalizeTypeDefRootTypeId(cls, typeId); } - if (isCustomSerializedByFactory(cls)) { - // A factory custom serializer is non-struct on the writer (NAMED_EXT); the reader must not - // downgrade it to a struct/compatible root kind just because it has no TypeInfo yet. - return Types.NAMED_EXT; - } return getFieldMetadataTypeIdForTypeDef(cls); } TypeInfo typeInfo = classInfoMap.get(cls); @@ -1014,13 +994,7 @@ public int getTypeDefRootTypeId(Class cls, boolean hasFieldMetadata) { } return normalizeTypeDefRootTypeId(cls, typeInfo.typeId); } - if (usesNonStructTypeDef(cls)) { - return Types.NAMED_EXT; - } - if (isCustomSerializedByFactory(cls)) { - return Types.NAMED_EXT; - } - return buildUnregisteredTypeId(cls, null); + return usesNonStructTypeDef(cls) ? Types.NAMED_EXT : buildUnregisteredTypeId(cls, null); } private int getFieldMetadataTypeIdForTypeDef(Class cls) { diff --git a/java/fory-core/src/main/java/org/apache/fory/resolver/TypeResolver.java b/java/fory-core/src/main/java/org/apache/fory/resolver/TypeResolver.java index 1feb1bee7f..f39e79fb7d 100644 --- a/java/fory-core/src/main/java/org/apache/fory/resolver/TypeResolver.java +++ b/java/fory-core/src/main/java/org/apache/fory/resolver/TypeResolver.java @@ -1226,10 +1226,24 @@ private TypeDef matchingLocalTypeDef(long headerHash, Class cls) { } // A declared polymorphic target can be an unregistered interface or abstract class. It has no // concrete local metadata owner, so do not materialize a TypeDef merely to probe for a hit. - if (getTypeInfo(cls, false) == null) { + TypeInfo typeInfo = getTypeInfo(cls, false); + if (typeInfo == null) { return null; } - TypeDef localTypeDef = getTypeDef(cls, true); + TypeDef localTypeDef = typeInfo.typeDef; + if (localTypeDef == null) { + if (typeInfo.serializer == null) { + if (!cls.isEnum() && ReflectionUtils.isAbstract(cls)) { + return null; + } + // Resolve this concrete read root before choosing its metadata shape. A provisional + // TypeInfo cannot distinguish a struct from a factory extension serializer. + typeInfo = getTypeInfo(cls); + } + // The serializer owns its TypeDef: guessing field metadata for an extension can cache a + // struct definition and corrupt a later write. Cold local structs must still match locally. + localTypeDef = buildTypeDef(typeInfo); + } return TypeDef.headerHash(localTypeDef.getId()) == headerHash ? localTypeDef : null; } diff --git a/java/fory-core/src/test/java/org/apache/fory/meta/TypeDefRootKindSymmetricReproTest.java b/java/fory-core/src/test/java/org/apache/fory/meta/TypeDefRootKindSymmetricReproTest.java deleted file mode 100644 index 19f8f03291..0000000000 --- a/java/fory-core/src/test/java/org/apache/fory/meta/TypeDefRootKindSymmetricReproTest.java +++ /dev/null @@ -1,167 +0,0 @@ -/* - * 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.fory.meta; - -import org.apache.fory.Fory; -import org.apache.fory.config.CompatibleMode; -import org.apache.fory.context.MetaReadContext; -import org.apache.fory.context.MetaWriteContext; -import org.apache.fory.context.ReadContext; -import org.apache.fory.context.WriteContext; -import org.apache.fory.exception.DeserializationException; -import org.apache.fory.serializer.Serializer; -import org.apache.fory.serializer.SerializerFactory; -import org.testng.Assert; -import org.testng.annotations.Test; - -/** - * Regression test: two identical {@link Fory} instances (same code, shared meta contexts, neither - * pre-registering a {@code SerializerFactory}-handled class) must round-trip a struct that nests a - * NON-collection class routed through a factory custom serializer. - * - *

Without the fix, the writer lazily resolves the nested subclass to its factory custom - * serializer and encodes root kind {@code NAMED_EXT} (32), while the reader decoding the field - * through field metadata expects {@code NAMED_COMPATIBLE_STRUCT} (30) — the class is neither - * registered nor a recognized non-struct family — producing {@code "TypeDef root kind does not - * match ... expected=30, actual=32"}. - * - *

{@link #withRegistration_roundTripOk()} documents that pre-registering the subclass also keeps - * the root kind consistent. - */ -public class TypeDefRootKindSymmetricReproTest { - - /** Base interface of a class that is serialized through the factory custom serializer. */ - public interface Restriction {} - - /** Non-collection subclass routed through the factory custom serializer. */ - public static class MyRestriction implements Restriction { - private String code; - - public MyRestriction() {} - - public MyRestriction(String code) { - this.code = code; - } - - public String getCode() { - return code; - } - - public void setCode(String code) { - this.code = code; - } - } - - /** Struct whose field is typed with the base interface and holds the subclass at runtime. */ - public static class Container { - private Restriction restriction; - - public Container() {} - - public Container(Restriction restriction) { - this.restriction = restriction; - } - - public Restriction getRestriction() { - return restriction; - } - - public void setRestriction(Restriction restriction) { - this.restriction = restriction; - } - } - - /** Custom serializer supplied by the factory for {@link MyRestriction}. */ - public static class RestrictionSerializer extends Serializer { - @Override - public void write(WriteContext ctx, MyRestriction value) { - ctx.writeRef(value.getCode()); - } - - @Override - public MyRestriction read(ReadContext ctx) { - MyRestriction r = new MyRestriction((String) ctx.readRef()); - ctx.reference(r); - return r; - } - } - - private static SerializerFactory restrictionFactory() { - return (typeResolver, cls) -> Restriction.class.isAssignableFrom(cls) ? new RestrictionSerializer() : null; - } - - private static Fory buildFory(boolean preRegister) { - Fory fory = - Fory.builder() - .withXlang(false) - .withMetaShare(true) - .requireClassRegistration(false) - .withRefTracking(true) - .withCompatibleMode(CompatibleMode.COMPATIBLE) - .withIntCompressed(true) - .withLongCompressed(org.apache.fory.config.Int64Encoding.VARINT) - .withCodegen(true) - .withAsyncCompilation(false) - .withSerializerFactory(restrictionFactory()) - .build(); - if (preRegister) { - fory.register(MyRestriction.class); - } - return fory; - } - - @Test - public void noRegistration_symmetric_roundTripOk() { - Fory writer = buildFory(false); - Fory reader = buildFory(false); - MetaWriteContext wc = new MetaWriteContext(); - MetaReadContext rc = new MetaReadContext(); - writer.setMetaWriteContext(wc); - writer.setMetaReadContext(rc); - reader.setMetaWriteContext(wc); - reader.setMetaReadContext(rc); - - byte[] bytes = writer.serialize(new Container(new MyRestriction("A"))); - // Regression guard: before the ClassResolver root-kind fix this threw - // DeserializationException "TypeDef root kind does not match ... expected=30, actual=32". - Container read = reader.deserialize(bytes, Container.class); - Assert.assertNotNull(read); - Assert.assertNotNull(read.getRestriction()); - Assert.assertEquals(((MyRestriction) read.getRestriction()).getCode(), "A"); - } - - @Test - public void withRegistration_roundTripOk() { - Fory writer = buildFory(true); - Fory reader = buildFory(true); - MetaWriteContext wc = new MetaWriteContext(); - MetaReadContext rc = new MetaReadContext(); - writer.setMetaWriteContext(wc); - writer.setMetaReadContext(rc); - reader.setMetaWriteContext(wc); - reader.setMetaReadContext(rc); - - byte[] bytes = writer.serialize(new Container(new MyRestriction("A"))); - Container read = reader.deserialize(bytes, Container.class); - Assert.assertNotNull(read); - Assert.assertNotNull(read.getRestriction()); - Assert.assertEquals(((MyRestriction) read.getRestriction()).getCode(), "A"); - } -} diff --git a/java/fory-core/src/test/java/org/apache/fory/resolver/ClassResolverTest.java b/java/fory-core/src/test/java/org/apache/fory/resolver/ClassResolverTest.java index ac48a604d9..2e855a5a61 100644 --- a/java/fory-core/src/test/java/org/apache/fory/resolver/ClassResolverTest.java +++ b/java/fory-core/src/test/java/org/apache/fory/resolver/ClassResolverTest.java @@ -719,6 +719,40 @@ public void testIdEnumDoesNotUseTypeDefMetaLimits() { serDeCheck(fory, TestNeedToWriteReferenceClass.A); } + @Test(dataProvider = "enableCodegen") + public void testColdLocalTypeDef(boolean codegen) { + for (boolean named : new boolean[] {false, true}) { + for (boolean typed : new boolean[] {false, true}) { + ForyBuilder builder = + Fory.builder() + .withXlang(false) + .withCompatible(true) + .withScopedMetaShare(true) + .withMaxSchemaVersionsPerType(1) + .withCodegen(codegen) + .withAsyncCompilation(false); + Fory writer = builder.build(); + Fory reader = builder.build(); + if (named) { + writer.register(Foo.class, "test.Foo"); + reader.register(Foo.class, "test.Foo"); + } else { + writer.register(Foo.class, 101); + reader.register(Foo.class, 101); + } + TypeResolver resolver = reader.getTypeResolver(); + assertNull(resolver.getTypeInfo(Foo.class, false).getSerializer()); + Foo value = new Foo(); + byte[] bytes = writer.serialize(value); + Foo copy = typed ? reader.deserialize(bytes, Foo.class) : (Foo) reader.deserialize(bytes); + assertEquals(copy, value); + TypeDef localTypeDef = writer.getTypeResolver().getTypeInfo(Foo.class).getTypeDef(); + // An exact local schema must not consume the allowance for remote schema versions. + assertNull(resolver.getCheckedRemoteTypeDef(TypeDef.headerHash(localTypeDef.getId()))); + } + } + } + @Test public void testIdExtDoesNotUseTypeDefMetaLimits() { Fory fory = diff --git a/java/fory-core/src/test/java/org/apache/fory/serializer/SerializerFactoryTest.java b/java/fory-core/src/test/java/org/apache/fory/serializer/SerializerFactoryTest.java index e9bcd503fc..3dff969f9a 100644 --- a/java/fory-core/src/test/java/org/apache/fory/serializer/SerializerFactoryTest.java +++ b/java/fory-core/src/test/java/org/apache/fory/serializer/SerializerFactoryTest.java @@ -34,8 +34,10 @@ import org.apache.fory.context.ReadContext; import org.apache.fory.context.WriteContext; import org.apache.fory.memory.MemoryBuffer; +import org.apache.fory.resolver.ClassResolver; import org.apache.fory.resolver.TypeResolver; import org.testng.Assert; +import org.testng.annotations.DataProvider; import org.testng.annotations.Test; public class SerializerFactoryTest { @@ -181,4 +183,172 @@ private static void assertKryoSerializer(Fory fory) { Object a2 = fory.deserialize(fory.serialize(a)); Assert.assertEquals(a, a2); } + + public interface Restriction {} + + public static class RestrictionValue implements Restriction { + public String code; + } + + public static class RestrictionHolder { + public Restriction restriction; + } + + public static class FactoryBean { + public int value; + public FactoryBean next; + } + + public enum FactoryEnum { + VALUE + } + + private static class RestrictionSerializer extends Serializer { + RestrictionSerializer(TypeResolver resolver) { + super(resolver.getConfig(), RestrictionValue.class); + } + + @Override + public void write(WriteContext ctx, RestrictionValue value) { + ctx.writeRef(value.code); + } + + @Override + public RestrictionValue read(ReadContext ctx) { + RestrictionValue value = new RestrictionValue(); + ctx.reference(value); + value.code = (String) ctx.readRef(); + return value; + } + } + + @DataProvider + public Object[][] factoryModes() { + return new Object[][] {{false}, {true}}; + } + + @DataProvider + public Object[][] metaFactoryModes() { + List modes = new ArrayList<>(); + for (boolean codegen : new boolean[] {false, true}) { + for (boolean registered : new boolean[] {false, true}) { + for (boolean metadataFirst : new boolean[] {false, true}) { + modes.add(new Object[] {codegen, registered, metadataFirst}); + } + } + } + return modes.toArray(new Object[0][]); + } + + private static Fory metaShareFory(boolean codegen, SerializerFactory factory) { + return Fory.builder() + .withXlang(false) + .withCompatible(true) + .withScopedMetaShare(true) + .withRefTracking(true) + .requireClassRegistration(false) + .withCodegen(codegen) + .withAsyncCompilation(false) + .withSerializerFactory(factory) + .build(); + } + + private static SerializerFactory restrictionFactory(AtomicInteger creations) { + return (resolver, cls) -> { + if (cls == RestrictionValue.class) { + creations.incrementAndGet(); + return new RestrictionSerializer(resolver); + } + return null; + }; + } + + @Test(dataProvider = "metaFactoryModes") + public void testMetaShareFactory(boolean codegen, boolean registered, boolean metadataFirst) { + AtomicInteger writerCreations = new AtomicInteger(); + AtomicInteger readerCreations = new AtomicInteger(); + Fory writer = metaShareFory(codegen, restrictionFactory(writerCreations)); + Fory reader = metaShareFory(codegen, restrictionFactory(readerCreations)); + if (registered) { + writer.register(RestrictionValue.class); + reader.register(RestrictionValue.class); + } + if (metadataFirst) { + ClassResolver resolver = (ClassResolver) reader.getTypeResolver(); + resolver.getTypeIdForTypeDef(RestrictionValue.class); + Assert.assertNull(resolver.getSerializer(RestrictionValue.class, false)); + } + RestrictionValue value = new RestrictionValue(); + value.code = "A"; + if (metadataFirst) { + RestrictionValue copy = reader.deserialize(writer.serialize(value), RestrictionValue.class); + Assert.assertEquals(copy.code, value.code); + } + RestrictionHolder holder = new RestrictionHolder(); + holder.restriction = value; + for (int i = 0; i < 2; i++) { + RestrictionHolder copy = + reader.deserialize(writer.serialize(holder), RestrictionHolder.class); + Assert.assertEquals(((RestrictionValue) copy.restriction).code, "A"); + } + RestrictionHolder restored = + writer.deserialize(reader.serialize(holder), RestrictionHolder.class); + Assert.assertEquals(((RestrictionValue) restored.restriction).code, "A"); + Assert.assertEquals(writerCreations.get(), 1); + Assert.assertEquals(readerCreations.get(), 1); + } + + @Test(dataProvider = "factoryModes") + @SuppressWarnings({"rawtypes", "unchecked"}) + public void testMetaShareEnumFactory(boolean codegen) { + SerializerFactory factory = + (resolver, cls) -> + cls == FactoryEnum.class ? new EnumSerializer(resolver.getConfig(), (Class) cls) : null; + Fory writer = metaShareFory(codegen, factory); + Fory reader = metaShareFory(codegen, factory); + Assert.assertSame(reader.deserialize(writer.serialize(FactoryEnum.VALUE)), FactoryEnum.VALUE); + } + + @Test(dataProvider = "factoryModes") + public void testMetaShareStructFactory(boolean codegen) { + SerializerFactory factory = + (resolver, cls) -> cls == FactoryBean.class ? new ObjectSerializer<>(resolver, cls) : null; + Fory writer = metaShareFory(codegen, factory); + Fory reader = metaShareFory(codegen, factory); + FactoryBean value = new FactoryBean(); + value.value = 42; + value.next = value; + FactoryBean copy = (FactoryBean) reader.deserialize(writer.serialize(value)); + Assert.assertEquals(copy.value, value.value); + Assert.assertSame(copy.next, copy); + // Reading metadata must not leave a partially constructed serializer for a later write. + FactoryBean restored = (FactoryBean) writer.deserialize(reader.serialize(copy)); + Assert.assertEquals(restored.value, value.value); + Assert.assertSame(restored.next, restored); + } + + @Test(dataProvider = "factoryModes") + public void testMetaShareFactoryFailure(boolean codegen) { + Fory writer = metaShareFory(codegen, restrictionFactory(new AtomicInteger())); + AtomicInteger attempts = new AtomicInteger(); + Fory reader = + metaShareFory( + codegen, + (resolver, cls) -> { + if (cls != RestrictionValue.class) { + return null; + } + if (attempts.incrementAndGet() == 1) { + throw new IllegalStateException("Serializer construction failed"); + } + return new RestrictionSerializer(resolver); + }); + RestrictionValue value = new RestrictionValue(); + value.code = "A"; + byte[] bytes = writer.serialize(value); + Assert.expectThrows(RuntimeException.class, () -> reader.deserialize(bytes)); + RestrictionValue copy = (RestrictionValue) reader.deserialize(bytes); + Assert.assertEquals(copy.code, value.code); + Assert.assertEquals(attempts.get(), 2); + } } From dc921a303f9f59c6a095a182e23975eb1c887f56 Mon Sep 17 00:00:00 2001 From: chaokunyang Date: Sat, 5 Sep 2026 19:39:41 +0800 Subject: [PATCH 4/4] fix(java): keep root and field TypeDef ownership distinct --- .../apache/fory/resolver/ClassResolver.java | 14 +++++++------- .../org/apache/fory/resolver/TypeResolver.java | 18 +++++++----------- 2 files changed, 14 insertions(+), 18 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 b347eb3865..bbe742a82a 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 @@ -1981,18 +1981,18 @@ private TypeDef buildTypeDef(TypeInfo typeInfo, Class seri TypeDef typeDef; Preconditions.checkArgument( serializerClass != UnknownClassSerializers.UnknownStructSerializer.class); - if (needToWriteTypeDef(serializerClass) || needsCollectionFieldTypeDef(serializerClass)) { + // Meta sharing needs the struct field schema even when compatible mode is disabled. + if (isStructSerializerClass(serializerClass) || needsCollectionFieldTypeDef(serializerClass)) { // Default collection/map serializers remain non-struct roots, but their wrapper fields still // need TypeDef metadata so remote compatible readers can evolve those fields. typeDef = typeDefMap.computeIfAbsent(typeInfo.type, cls -> TypeDef.buildTypeDef(this, cls)); } else { - // Some type will use other serializers such MapSerializer and so on. + // Field schemas may already exist for class layers or metadata probes. An empty serializer + // root has a different shape and belongs to TypeInfo, not the class's field-schema map. typeDef = - typeDefMap.computeIfAbsent( - typeInfo.type, - cls -> - NativeTypeDefEncoder.buildTypeDefWithFieldInfos( - this, cls, Collections.emptyList())); + cacheTypeDef( + NativeTypeDefEncoder.buildTypeDefWithFieldInfos( + this, typeInfo.type, Collections.emptyList())); } typeInfo.typeDef = typeDef; return typeDef; diff --git a/java/fory-core/src/main/java/org/apache/fory/resolver/TypeResolver.java b/java/fory-core/src/main/java/org/apache/fory/resolver/TypeResolver.java index f39e79fb7d..bd201b667a 100644 --- a/java/fory-core/src/main/java/org/apache/fory/resolver/TypeResolver.java +++ b/java/fory-core/src/main/java/org/apache/fory/resolver/TypeResolver.java @@ -1231,19 +1231,15 @@ private TypeDef matchingLocalTypeDef(long headerHash, Class cls) { return null; } TypeDef localTypeDef = typeInfo.typeDef; - if (localTypeDef == null) { - if (typeInfo.serializer == null) { - if (!cls.isEnum() && ReflectionUtils.isAbstract(cls)) { - return null; - } - // Resolve this concrete read root before choosing its metadata shape. A provisional - // TypeInfo cannot distinguish a struct from a factory extension serializer. - typeInfo = getTypeInfo(cls); - } - // The serializer owns its TypeDef: guessing field metadata for an extension can cache a - // struct definition and corrupt a later write. Cold local structs must still match locally. + if (localTypeDef == null && typeInfo.serializer != null) { localTypeDef = buildTypeDef(typeInfo); } + if (localTypeDef != null && TypeDef.headerHash(localTypeDef.getId()) == headerHash) { + return localTypeDef; + } + // Class layers can use a field schema independently of the serializer's root definition. + // Keep this probe metadata-only: creating serializers here changes registration/codegen order. + localTypeDef = getTypeDef(cls, true); return TypeDef.headerHash(localTypeDef.getId()) == headerHash ? localTypeDef : null; }