Copilot commented on code in PR #3500:
URL: https://github.com/apache/dubbo-go/pull/3500#discussion_r3627785893
##########
internal/config.go:
##########
@@ -215,3 +215,23 @@ func ValidateRegistryIDs(ids []string, regs
map[string]*global.RegistryConfig) e
}
return nil
}
+
+// ValidateGenericType validates the generic serialization type (generic mode).
+// An empty value means the call is not generic and is allowed. Unknown values
fail
+// fast instead of silently falling back to the Map generalizer.
+//
+// Valid values: "true" (Map, default), "gson", "protobuf-json", "bean".
+// "protobuf" is kept as a legacy compatibility value and is not recommended.
+func ValidateGenericType(generic string) error {
+ switch {
+ case generic == "",
+ strings.EqualFold(generic,
constant.GenericSerializationDefault),
+ strings.EqualFold(generic, constant.GenericSerializationGson),
+ strings.EqualFold(generic,
constant.GenericSerializationProtobufJson),
+ strings.EqualFold(generic, constant.GenericSerializationBean),
+ strings.EqualFold(generic,
constant.GenericSerializationProtobuf):
+ return nil
Review Comment:
ValidateGenericType treats generic="protobuf" as valid, but the generic
filter currently does not recognize "protobuf" as generic mode
(filter/generic/util.go:isGeneric only checks true/gson/protobuf-json/bean).
That means a reference configured with WithGenericType("protobuf") may be
treated as non-generic at runtime despite passing validation.
##########
internal/config.go:
##########
@@ -215,3 +215,23 @@ func ValidateRegistryIDs(ids []string, regs
map[string]*global.RegistryConfig) e
}
return nil
}
+
+// ValidateGenericType validates the generic serialization type (generic mode).
+// An empty value means the call is not generic and is allowed. Unknown values
fail
+// fast instead of silently falling back to the Map generalizer.
+//
+// Valid values: "true" (Map, default), "gson", "protobuf-json", "bean".
+// "protobuf" is kept as a legacy compatibility value and is not recommended.
+func ValidateGenericType(generic string) error {
+ switch {
+ case generic == "",
+ strings.EqualFold(generic,
constant.GenericSerializationDefault),
+ strings.EqualFold(generic, constant.GenericSerializationGson),
+ strings.EqualFold(generic,
constant.GenericSerializationProtobufJson),
+ strings.EqualFold(generic, constant.GenericSerializationBean),
+ strings.EqualFold(generic,
constant.GenericSerializationProtobuf):
+ return nil
+ default:
+ return fmt.Errorf("invalid generic type %q, valid values: true,
gson, protobuf-json, bean", generic)
Review Comment:
The error message for invalid generic types omits the documented
legacy-accepted value ("protobuf") and also doesn’t mention that empty string
is allowed for non-generic calls. Including these in the message will make the
failure mode clearer and align with the function docstring.
##########
client/options.go:
##########
@@ -360,14 +366,35 @@ func WithGeneric() ReferenceOption {
}
}
-// WithGenericType sets the generic serialization type for generic call
-// Valid values: "true" (default), "gson", "protobuf", "protobuf-json"
+// WithGenericType sets the generic mode (generalization format), which
decides how
+// business objects are generalized into a generic structure.
+//
+// Valid values: "true" (default, Map), "gson", "protobuf-json", "bean".
+// "protobuf" is kept as a legacy compatibility value and is not recommended.
+// An unknown value is rejected when the reference is created (see init()),
rather
+// than silently falling back to the Map generalizer.
+//
+// Note: the generic mode is different from the transport serialization set via
+// WithSerialization; the latter controls the on-the-wire encoding (hessian2 /
+// protobuf / json / msgpack).
func WithGenericType(genericType string) ReferenceOption {
return func(opts *ReferenceOptions) {
opts.Reference.Generic = genericType
}
}
+// WithGenericIncludeClass controls whether the "class" field is kept in the
result
+// of Map generic calls (generic=true). It only affects the Map generalizer
and has
+// no effect on gson / bean / protobuf-json. The default is true.
+func WithGenericIncludeClass(include bool) ReferenceOption {
+ return func(opts *ReferenceOptions) {
+ if opts.Reference.Params == nil {
+ opts.Reference.Params = make(map[string]string)
+ }
+ opts.Reference.Params[constant.GenericIncludeClassKey] =
strconv.FormatBool(include)
+ }
Review Comment:
WithGenericIncludeClass writes generic.include.class into Reference.Params,
but MapGeneralizer currently reads generic.include.class only from the global
config environment (filter/generic/generalizer/map.go:getGenericIncludeClass)
and does not consult URL/reference params. As a result, this option likely has
no effect and still effectively relies on global configuration.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
