Skip to content

Commit 5ebe5d2

Browse files
chaokunyangchaokunyang
authored
fix(java): fix register serializer for abstract class (#2347)
## What does this PR do? fix register serializer for abstract class ## Related issues #2345 #2311 ## Does this PR introduce any user-facing change? <!-- If any user-facing interface changes, please [open an issue](https://github.com/apache/fory/issues/new/choose) describing the need to do so and update the document if necessary. --> - [ ] Does this PR introduce any public API change? - [ ] Does this PR introduce any binary protocol compatibility change? ## Benchmark <!-- When the PR has an impact on performance (if you don't know whether the PR will have an impact on performance, you can submit the PR first, and if it will have impact on performance, the code reviewer will explain it), be sure to attach a benchmark data here. -->
1 parent ff0ced5 commit 5ebe5d2

File tree

3 files changed

+50
-82
lines changed

3 files changed

+50
-82
lines changed

‎java/fory-core/src/main/java/org/apache/fory/reflect/ReflectionUtils.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@
6161
@SuppressWarnings({"unchecked", "rawtypes"})
6262
public class ReflectionUtils {
6363
public static boolean isAbstract(Class<?> clazz) {
64-
if (clazz.isArray()) {
64+
if (clazz.isArray() || clazz.isPrimitive()) {
6565
return false;
6666
}
6767
return Modifier.isAbstract(clazz.getModifiers());

‎java/fory-core/src/main/java/org/apache/fory/resolver/ClassResolver.java‎

Lines changed: 11 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -469,7 +469,7 @@ public void register(Class<?> cls, int classId) {
469469
classInfo = new ClassInfo(this, cls, null, id, NOT_SUPPORT_XLANG);
470470
// make `extRegistry.registeredClassIdMap` and `classInfoMap` share same classInfo
471471
// instances.
472-
setClassInfo(cls, classInfo);
472+
classInfoMap.put(cls, classInfo);
473473
}
474474
// serializer will be set lazily in `addSerializer` method if it's null.
475475
registeredId2ClassInfo[id] = classInfo;
@@ -517,7 +517,7 @@ public void register(Class<?> cls, String namespace, String name) {
517517
MetaStringBytes nameBytes = metaStringResolver.getOrCreateMetaStringBytes(encodeTypeName(name));
518518
ClassInfo classInfo =
519519
new ClassInfo(cls, fullNameBytes, nsBytes, nameBytes, false, null, NO_CLASS_ID, (short) -1);
520-
setClassInfo(cls, classInfo);
520+
classInfoMap.put(cls, classInfo);
521521
compositeNameBytes2ClassInfo.put(
522522
new TypeNameBytes(nsBytes.hashCode, nameBytes.hashCode), classInfo);
523523
extRegistry.registeredClasses.put(fullname, cls);
@@ -748,6 +748,12 @@ public void registerSerializer(Class<?> type, Serializer<?> serializer) {
748748
register(type);
749749
}
750750
addSerializer(type, serializer);
751+
ClassInfo classInfo = classInfoMap.get(type);
752+
classInfoMap.put(type, classInfo);
753+
// in order to support customized serializer for abstract or interface.
754+
if (!type.isPrimitive() && (ReflectionUtils.isAbstract(type) || type.isInterface())) {
755+
extRegistry.absClassInfo.put(type, classInfo);
756+
}
751757
}
752758

753759
public void setSerializerFactory(SerializerFactory serializerFactory) {
@@ -860,7 +866,7 @@ private void addSerializer(Class<?> type, Serializer<?> serializer) {
860866

861867
if (classInfo == null || classId != classInfo.classId) {
862868
classInfo = new ClassInfo(this, type, null, classId, (short) 0);
863-
setClassInfo(type, classInfo);
869+
classInfoMap.put(type, classInfo);
864870
if (registered) {
865871
registeredId2ClassInfo[classId] = classInfo;
866872
}
@@ -1295,14 +1301,6 @@ public ClassInfo getClassInfo(Class<?> cls, boolean createClassInfoIfNotFound) {
12951301
}
12961302
}
12971303

1298-
void setClassInfo(Class<?> cls, ClassInfo classInfo) {
1299-
classInfoMap.put(cls, classInfo);
1300-
// in order to support customized serializer for abstract or interface.
1301-
if (!cls.isPrimitive() && (ReflectionUtils.isAbstract(cls) || cls.isInterface())) {
1302-
extRegistry.absClassInfo.put(cls, classInfo);
1303-
}
1304-
}
1305-
13061304
@Internal
13071305
public ClassInfo getOrUpdateClassInfo(Class<?> cls) {
13081306
ClassInfo classInfo = classInfoCache;
@@ -1796,7 +1794,7 @@ public void writeClassInternal(MemoryBuffer buffer, Class<?> cls) {
17961794
classInfo =
17971795
new ClassInfo(
17981796
this, cls, null, classId == null ? NO_CLASS_ID : classId, NOT_SUPPORT_XLANG);
1799-
setClassInfo(cls, classInfo);
1797+
classInfoMap.put(cls, classInfo);
18001798
}
18011799
writeClassInternal(buffer, classInfo);
18021800
}
@@ -1977,7 +1975,7 @@ private ClassInfo populateBytesToClassInfo(
19771975
// don't create serializer here, if the class is an interface,
19781976
// there won't be serializer since interface has no instance.
19791977
if (!classInfoMap.containsKey(cls)) {
1980-
setClassInfo(cls, classInfo);
1978+
classInfoMap.put(cls, classInfo);
19811979
}
19821980
}
19831981
compositeNameBytes2ClassInfo.put(typeNameBytes, classInfo);

‎java/fory-core/src/test/java/org/apache/fory/resolver/ClassResolverTest.java‎

Lines changed: 38 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,6 @@
4646
import lombok.ToString;
4747
import org.apache.fory.Fory;
4848
import org.apache.fory.ForyTestBase;
49-
import org.apache.fory.ThreadSafeFory;
5049
import org.apache.fory.builder.Generated;
5150
import org.apache.fory.config.Language;
5251
import org.apache.fory.logging.Logger;
@@ -402,26 +401,16 @@ public Foo read(MemoryBuffer buffer) {
402401

403402
@Test
404403
public void testFooCustomSerializer() {
405-
ThreadSafeFory threadSafeFory =
406-
Fory.builder().withLanguage(Language.JAVA).buildThreadSafeFory();
407-
Assert.assertThrows(
408-
() -> threadSafeFory.registerSerializer(Foo.class, FooCustomSerializer.class));
409-
threadSafeFory.registerSerializer(Foo.class, f -> new FooCustomSerializer(f, Foo.class));
404+
Fory fory = Fory.builder().withLanguage(Language.JAVA).build();
405+
Assert.assertThrows(() -> fory.registerSerializer(Foo.class, FooCustomSerializer.class));
406+
fory.registerSerializer(Foo.class, f -> new FooCustomSerializer(f, Foo.class));
410407
final Foo foo = new Foo();
411408
foo.setF1(100);
412409

413-
threadSafeFory.execute(
414-
fory -> {
415-
Assert.assertEquals(foo, serDe(fory, foo));
416-
return null;
417-
});
418-
threadSafeFory.execute(
419-
fory -> {
420-
Assert.assertEquals(
421-
fory.getClassResolver().getSerializer(foo.getClass()).getClass(),
422-
FooCustomSerializer.class);
423-
return null;
424-
});
410+
Assert.assertEquals(foo, serDe(fory, foo));
411+
Assert.assertEquals(
412+
fory.getClassResolver().getSerializer(foo.getClass()).getClass(),
413+
FooCustomSerializer.class);
425414
}
426415

427416
interface ITest {
@@ -467,30 +456,32 @@ public ITest read(MemoryBuffer buffer) {
467456

468457
@Test
469458
public void testInterfaceCustomSerializer() {
470-
ThreadSafeFory threadSafeFory =
471-
Fory.builder()
472-
.withLanguage(Language.JAVA)
473-
.requireClassRegistration(false)
474-
.buildThreadSafeFory();
475-
threadSafeFory.registerSerializer(
476-
ITest.class, f -> new InterfaceCustomSerializer(f, ITest.class));
459+
Fory fory = Fory.builder().withLanguage(Language.JAVA).requireClassRegistration(false).build();
460+
fory.registerSerializer(ITest.class, new InterfaceCustomSerializer(fory, ITest.class));
477461
final ITest iTest = new ImplTest();
478462
iTest.setF1(100);
479463

480-
threadSafeFory.execute(
481-
fory -> {
482-
Assert.assertEquals(iTest, serDe(fory, iTest));
483-
return null;
484-
});
485-
threadSafeFory.execute(
486-
fory -> {
487-
Assert.assertEquals(
488-
fory.getClassResolver().getSerializer(iTest.getClass()).getClass(),
489-
InterfaceCustomSerializer.class);
490-
return null;
491-
});
464+
Assert.assertEquals(iTest, serDe(fory, iTest));
465+
Assert.assertEquals(
466+
fory.getClassResolver().getSerializer(iTest.getClass()).getClass(),
467+
InterfaceCustomSerializer.class);
468+
469+
fory = Fory.builder().withLanguage(Language.JAVA).requireClassRegistration(false).build();
470+
fory.register(ITest.class);
471+
Assert.assertNotEquals(
472+
fory.getClassResolver().getSerializer(ImplTest.class).getClass(),
473+
InterfaceCustomSerializer.class);
474+
475+
fory = Fory.builder().withLanguage(Language.JAVA).requireClassRegistration(false).build();
476+
fory.registerSerializer(ITest.class, new InterfaceCustomSerializer(fory, ITest.class));
477+
Assert.assertEquals(
478+
fory.getClassResolver().getSerializer(ImplTest.class).getClass(),
479+
InterfaceCustomSerializer.class);
492480
}
493481

482+
@Test
483+
public void testRegisterAbstractClass() {}
484+
494485
@Data
495486
abstract static class AbsTest {
496487
int f1;
@@ -530,43 +521,22 @@ public AbsTest read(MemoryBuffer buffer) {
530521

531522
@Test
532523
public void testAbstractCustomSerializer() {
533-
ThreadSafeFory threadSafeFory =
534-
Fory.builder()
535-
.withLanguage(Language.JAVA)
536-
.requireClassRegistration(false)
537-
.buildThreadSafeFory();
538-
threadSafeFory.registerSerializer(
539-
AbsTest.class, f -> new AbstractCustomSerializer(f, AbsTest.class));
524+
Fory fory = Fory.builder().withLanguage(Language.JAVA).requireClassRegistration(false).build();
525+
fory.registerSerializer(AbsTest.class, new AbstractCustomSerializer(fory, AbsTest.class));
540526
final AbsTest absTest = new SubAbsTest();
541527
absTest.setF1(100);
542528

543-
threadSafeFory.execute(
544-
fory -> {
545-
Assert.assertEquals(absTest, serDe(fory, absTest));
546-
return null;
547-
});
548-
threadSafeFory.execute(
549-
fory -> {
550-
Assert.assertEquals(
551-
fory.getClassResolver().getSerializer(absTest.getClass()).getClass(),
552-
AbstractCustomSerializer.class);
553-
return null;
554-
});
529+
Assert.assertEquals(absTest, serDe(fory, absTest));
530+
Assert.assertEquals(
531+
fory.getClassResolver().getSerializer(absTest.getClass()).getClass(),
532+
AbstractCustomSerializer.class);
555533

556534
final AbsTest abs2Test = new Sub2AbsTest();
557535
abs2Test.setF1(100);
558536

559-
threadSafeFory.execute(
560-
fory -> {
561-
Assert.assertEquals(abs2Test.getF1(), serDe(fory, abs2Test).getF1());
562-
return null;
563-
});
564-
threadSafeFory.execute(
565-
fory -> {
566-
Assert.assertEquals(
567-
fory.getClassResolver().getSerializer(abs2Test.getClass()).getClass(),
568-
AbstractCustomSerializer.class);
569-
return null;
570-
});
537+
Assert.assertEquals(abs2Test.getF1(), serDe(fory, abs2Test).getF1());
538+
Assert.assertEquals(
539+
fory.getClassResolver().getSerializer(abs2Test.getClass()).getClass(),
540+
AbstractCustomSerializer.class);
571541
}
572542
}

0 commit comments

Comments
 (0)