AlexStocks commented on code in PR #3284:
URL: https://github.com/apache/dubbo-go/pull/3284#discussion_r3039283718
##########
filter/generic/service_filter.go:
##########
@@ -78,44 +80,126 @@ func (f *genericServiceFilter) Invoke(ctx context.Context,
invoker base.Invoker,
`, mtdName, types, args)
// get the type of the argument
- ivkUrl := invoker.GetURL()
- svc := common.ServiceMap.GetServiceByServiceKey(ivkUrl.Protocol,
ivkUrl.ServiceKey())
+ ivkURL := invoker.GetURL()
+ svc := common.ServiceMap.GetServiceByServiceKey(ivkURL.Protocol,
ivkURL.ServiceKey())
method := svc.Method()[mtdName]
if method == nil {
return &result.RPCResult{
- Err: perrors.Errorf("\"%s\" method is not found,
service key: %s", mtdName, ivkUrl.ServiceKey()),
+ Err: perrors.Errorf("\"%s\" method is not found,
service key: %s", mtdName, ivkURL.ServiceKey()),
}
}
+
argsType := method.ArgsType()
+ if err := validateGenericArgs(method.IsVariadic(), len(argsType),
len(args), mtdName); err != nil {
+ return &result.RPCResult{Err: err}
+ }
// get generic info from attachments of invocation, the default value
is "true"
generic := inv.GetAttachmentWithDefaultValue(constant.GenericKey,
constant.GenericSerializationDefault)
// get generalizer according to value in the `generic`
- g := getGeneralizer(generic)
+ // realize
+ newArgs, err := realizeInvocationArgs(getGeneralizer(generic),
argsType, args, method.IsVariadic())
+ if err != nil {
+ return &result.RPCResult{Err: err}
+ }
- if len(args) != len(argsType) {
- return &result.RPCResult{
- Err: perrors.Errorf("the number of args(=%d) is not
matched with \"%s\" method", len(args), mtdName),
+ newIvc := invocation.NewRPCInvocation(mtdName, newArgs,
inv.Attachments())
+ newIvc.SetReply(inv.Reply())
+
+ return invoker.Invoke(ctx, newIvc)
+}
+
+// validateGenericArgs checks whether the number of generic invocation
arguments
+// matches the target method signature. Variadic methods accept argCount >=
fixedParams.
+func validateGenericArgs(isVariadic bool, argsTypeCount, argCount int,
methodName string) error {
+ if isVariadic {
+ if argCount >= argsTypeCount-1 {
+ return nil
}
+ } else if argCount == argsTypeCount {
+ return nil
}
- // realize
+ return perrors.Errorf("the number of args(=%d) is not matched with
\"%s\" method", argCount, methodName)
Review Comment:
[P1] 规范级问题：错误处理格式应保持一致性。在 "realizeFixedArgs" 函数中，错误消息格式为 "realization
failed, %v"，而其他地方的错误格式为 "realization of variadic arg[%d] failed: %v"。建议统一错误消息格式。
##########
filter/generic/service_filter.go:
##########
@@ -78,44 +80,126 @@ func (f *genericServiceFilter) Invoke(ctx context.Context,
invoker base.Invoker,
`, mtdName, types, args)
// get the type of the argument
- ivkUrl := invoker.GetURL()
- svc := common.ServiceMap.GetServiceByServiceKey(ivkUrl.Protocol,
ivkUrl.ServiceKey())
+ ivkURL := invoker.GetURL()
Review Comment:
[P1] 规范级问题：变量命名应保持一致性。代码中从 "ivkUrl" 重命名为 "ivkURL" 是正确的（符合 Go
语言规范），但建议在整个代码库中统一这种缩写词的命名风格（如 "URL", "ID", "HTTP" 等应大写）。
##########
filter/generic/service_filter.go:
##########
@@ -78,44 +80,126 @@ func (f *genericServiceFilter) Invoke(ctx context.Context,
invoker base.Invoker,
`, mtdName, types, args)
// get the type of the argument
- ivkUrl := invoker.GetURL()
- svc := common.ServiceMap.GetServiceByServiceKey(ivkUrl.Protocol,
ivkUrl.ServiceKey())
+ ivkURL := invoker.GetURL()
+ svc := common.ServiceMap.GetServiceByServiceKey(ivkURL.Protocol,
ivkURL.ServiceKey())
method := svc.Method()[mtdName]
if method == nil {
return &result.RPCResult{
- Err: perrors.Errorf("\"%s\" method is not found,
service key: %s", mtdName, ivkUrl.ServiceKey()),
+ Err: perrors.Errorf("\"%s\" method is not found,
service key: %s", mtdName, ivkURL.ServiceKey()),
}
}
+
argsType := method.ArgsType()
+ if err := validateGenericArgs(method.IsVariadic(), len(argsType),
len(args), mtdName); err != nil {
+ return &result.RPCResult{Err: err}
+ }
// get generic info from attachments of invocation, the default value
is "true"
generic := inv.GetAttachmentWithDefaultValue(constant.GenericKey,
constant.GenericSerializationDefault)
// get generalizer according to value in the `generic`
- g := getGeneralizer(generic)
+ // realize
+ newArgs, err := realizeInvocationArgs(getGeneralizer(generic),
argsType, args, method.IsVariadic())
+ if err != nil {
+ return &result.RPCResult{Err: err}
+ }
- if len(args) != len(argsType) {
- return &result.RPCResult{
- Err: perrors.Errorf("the number of args(=%d) is not
matched with \"%s\" method", len(args), mtdName),
+ newIvc := invocation.NewRPCInvocation(mtdName, newArgs,
inv.Attachments())
+ newIvc.SetReply(inv.Reply())
+
+ return invoker.Invoke(ctx, newIvc)
+}
+
+// validateGenericArgs checks whether the number of generic invocation
arguments
+// matches the target method signature. Variadic methods accept argCount >=
fixedParams.
+func validateGenericArgs(isVariadic bool, argsTypeCount, argCount int,
methodName string) error {
+ if isVariadic {
+ if argCount >= argsTypeCount-1 {
+ return nil
}
+ } else if argCount == argsTypeCount {
+ return nil
}
- // realize
+ return perrors.Errorf("the number of args(=%d) is not matched with
\"%s\" method", argCount, methodName)
+}
+
+// realizeInvocationArgs converts generic invocation arguments to concrete
types.
+// For variadic methods, fixed params are realized normally and trailing args
are
+// reshaped into a typed slice via realizeVariadicArg.
+func realizeInvocationArgs(g generalizer.Generalizer, argsType []reflect.Type,
args []hessian.Object, isVariadic bool) ([]any, error) {
+ if !isVariadic {
+ return realizeFixedArgs(g, args, argsType)
+ }
+
+ newArgs, err := realizeFixedArgs(g, args[:len(argsType)-1],
argsType[:len(argsType)-1])
+ if err != nil {
+ return nil, err
+ }
+
+ variadicArg, err := realizeVariadicArg(g, args[len(argsType)-1:],
argsType[len(argsType)-1])
+ if err != nil {
+ return nil, err
+ }
+
+ return append(newArgs, variadicArg), nil
+}
+
+// realizeFixedArgs converts each arg to its corresponding concrete type via
Generalizer.Realize.
Review Comment:
[P0] 类型安全问题：在 realizeVariadicArg
函数中，slice.Index(i).Set(reflect.ValueOf(realized)) 存在潜在类型不匹配的风险。建议在设置值之前验证
realized 的类型与目标切片元素类型是否兼容，防止运行时 panic。
--
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]
