AlexStocks commented on code in PR #3284:
URL: https://github.com/apache/dubbo-go/pull/3284#discussion_r3039284705
##########
filter/generic/service_filter.go:
##########
@@ -78,44 +80,126 @@ func (f *genericServiceFilter) Invoke(ctx context.Context,
invoker base.Invoker,
`, mtdName, types, args)
Review Comment:
[P1] 测试覆盖建议：此修复涉及重要的边界情况处理，建议增加测试用例验证以下场景：1) 零变长参数（空数组）；2) 单变长参数；3)
多类型变长参数（如 ...interface{}）；4) 固定参数+变长参数混合；5) 显式nil注入。这将提高代码的可靠性。
##########
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
+}
Review Comment:
[P2] 建议级改进：在 realizeVariadicArg 函数中，每次循环都调用 reflect.ValueOf(realized) 和
slice.Index(i).Set() 可能存在性能优化空间。考虑预先验证类型兼容性，或者在批量设置值时使用更高效的方式。
--
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]
