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 bb98e90e7 fix(go): Complete type registration in factory-based
serializer system (#2619)
bb98e90e7 is described below
commit bb98e90e751c9099fb3cb1789fa8b7658e50b833
Author: thisingl <[email protected]>
AuthorDate: Wed Sep 17 19:33:42 2025 +0800
fix(go): Complete type registration in factory-based serializer system
(#2619)
<!--
**Thanks for contributing to Apache Fory™.**
**If this is your first time opening a PR on fory, you can refer to
[CONTRIBUTING.md](https://github.com/apache/fory/blob/main/CONTRIBUTING.md).**
Contribution Checklist
- The **Apache Fory™** community has requirements on the naming of pr
titles. You can also find instructions in
[CONTRIBUTING.md](https://github.com/apache/fory/blob/main/CONTRIBUTING.md).
- Apache Fory™ has a strong focus on performance. If the PR you submit
will have an impact on performance, please benchmark it first and
provide the benchmark result here.
-->
## Why?
My previous PR #2615 had an incomplete implementation that was exposed
by the recent metashare changes.
The factory-based registration only registered serializers to
`typeToSerializers` map but didn't create complete `TypeInfo` objects
with metadata (`PkgPathBytes`, `NameBytes`, etc.). This latent bug was
exposed when metashare code path changes made `WriteMetaStringBytes()`
calls mandatory, resulting in nil pointer dereference.
And CI **didn't run** `go test -v` in the `tests/` directory, so the
codegen tests that would have caught this issue were not executed
<!-- Describe the purpose of this PR. -->
## What does this PR do?
**1. Complete the factory-based registration**
**Before (Incomplete):**
```go
// Only registered serializers, missing TypeInfo creation
for type_, factory := range factories {
serializer := factory()
r.typeToSerializers[type_] = serializer // ❌ Only serializer, no
TypeInfo
}
```
**After (Complete):**
```go
for type_, factory := range generatedSerializerFactories.factories {
serializer := factory()
// ✅ Use complete registration (creates full TypeInfo)
pkgPath := type_.PkgPath()
typeName := type_.Name()
_, err := r.registerType(type_, int32(serializer.TypeId()),
pkgPath, typeName, serializer, true)
// ✅ Handle pointer type caching for runtime compatibility
ptrType := reflect.PtrTo(type_)
r.typeToSerializers[ptrType] = serializer
if typeInfo, exists := r.typesInfo[type_]; exists {
ptrTypeInfo := TypeInfo{
Type: ptrType,
TypeID: -typeInfo.TypeID,
Serializer: serializer,
PkgPathBytes: typeInfo.PkgPathBytes, // ✅ Reuse complete
metadata
NameBytes: typeInfo.NameBytes,
IsDynamic: true,
hashValue: calcTypeHash(ptrType),
}
r.typesInfo[ptrType] = ptrTypeInfo
}
}
```
**2. Add CI test coverage for the `tests/` directory** to ensure codegen
tests are always run
## Related issues
- #2227
<!--
Is there any related issue? If this PR closes them you say say
fix/closes:
- #xxxx0
- #xxxx1
- Fixes #xxxx2
-->
## 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.
Delete section if not applicable.
-->
- [ ] 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.
Delete section if not applicable.
-->
---
ci/run_ci.sh | 1 +
go/fory/type.go | 33 +++++++++++++++++++++++++++++----
2 files changed, 30 insertions(+), 4 deletions(-)
diff --git a/ci/run_ci.sh b/ci/run_ci.sh
index 71bf93b0a..bbc09feeb 100755
--- a/ci/run_ci.sh
+++ b/ci/run_ci.sh
@@ -360,6 +360,7 @@ case $1 in
go install ./cmd/fory
cd "$ROOT/go/fory/tests"
go generate
+ go test -v
cd "$ROOT/go/fory"
go test -v
echo "Executing fory go tests succeeds"
diff --git a/go/fory/type.go b/go/fory/type.go
index de0a65446..60587af32 100644
--- a/go/fory/type.go
+++ b/go/fory/type.go
@@ -376,9 +376,33 @@ func newTypeResolver(fory *Fory) *typeResolver {
generatedSerializerFactories.mu.RLock()
for type_, factory := range generatedSerializerFactories.factories {
serializer := factory()
- r.typeToSerializers[type_] = serializer
- if typeId := serializer.TypeId(); typeId >
NotSupportCrossLanguage {
- r.typeIdToType[typeId] = type_
+
+ // Use full registration process for generated types
+ pkgPath := type_.PkgPath()
+ typeName := type_.Name()
+
+ // Register value type
+ _, err := r.registerType(type_, int32(serializer.TypeId()),
pkgPath, typeName, serializer, true)
+ if err != nil {
+ fmt.Errorf("failed to register generated type %v: %v",
type_, err)
+ }
+
+ // Also register pointer type in serializers map (without full
registration to avoid typeId conflict)
+ ptrType := reflect.PtrTo(type_)
+ r.typeToSerializers[ptrType] = serializer
+
+ // Create TypeInfo for pointer type and add to cache
+ if typeInfo, exists := r.typesInfo[type_]; exists {
+ ptrTypeInfo := TypeInfo{
+ Type: ptrType,
+ TypeID: -typeInfo.TypeID, // Use negative
ID for pointer type
+ Serializer: serializer,
+ PkgPathBytes: typeInfo.PkgPathBytes,
+ NameBytes: typeInfo.NameBytes,
+ IsDynamic: true,
+ hashValue: calcTypeHash(ptrType),
+ }
+ r.typesInfo[ptrType] = ptrTypeInfo
}
}
generatedSerializerFactories.mu.RUnlock()
@@ -508,7 +532,7 @@ func (r *typeResolver) getTypeInfo(value reflect.Value,
create bool) (TypeInfo,
if info.Serializer == nil {
/*
Lazy initialize serializer if not created yet
- mapInStruct equals false because this path isn’t
taken when extracting field info from structs;
+ mapInStruct equals false because this path isn't
taken when extracting field info from structs;
for all other map cases, it remains false
*/
serializer, err := r.createSerializer(value.Type(),
false)
@@ -530,6 +554,7 @@ func (r *typeResolver) getTypeInfo(value reflect.Value,
create bool) (TypeInfo,
value = value.Elem()
}
type_ := value.Type()
+
// Get package path and type name for registration
var typeName string
var pkgPath string
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
