Alanxtl commented on code in PR #3154:
URL: https://github.com/apache/dubbo-go/pull/3154#discussion_r2688745078
##########
protocol/triple/triple.go:
##########
@@ -141,6 +149,18 @@ func (tp *TripleProtocol) Destroy() {
tp.BaseProtocol.Destroy()
}
+// isGenericCall checks if the generic parameter indicates a generic call
+func isGenericCall(generic string) bool {
+ if generic == "" {
+ return false
+ }
+ lowerGeneric := strings.ToLower(generic)
+ return lowerGeneric == constant.GenericSerializationDefault ||
+ lowerGeneric == constant.GenericSerializationGson ||
+ lowerGeneric == constant.GenericSerializationProtobuf ||
+ lowerGeneric == constant.GenericSerializationProtobufJson
+}
Review Comment:
use `strings.EqualFold` instead, no need to `toLower` and `==`
##########
server/server.go:
##########
@@ -176,6 +179,61 @@ func (s *Server) genSvcOpts(handler any, info
*common.ServiceInfo, opts ...Servi
return newSvcOpts, nil
}
+// isNillable checks if a reflect.Value's kind supports nil checking.
+func isNillable(v reflect.Value) bool {
+ switch v.Kind() {
+ case reflect.Chan, reflect.Func, reflect.Interface, reflect.Map,
reflect.Pointer, reflect.Slice:
+ return true
+ default:
+ return false
+ }
+}
+
+// isReflectNil safely checks if a reflect.Value is nil.
+func isReflectNil(v reflect.Value) bool {
+ return isNillable(v) && v.IsNil()
+}
+
+// CallMethodByReflection invokes the given method via reflection and
processes its return values.
+// This is a shared helper function used by both server/server.go and
protocol/triple/server.go.
+func CallMethodByReflection(ctx context.Context, method reflect.Method,
handler any, args []any) (any, error) {
+ in := []reflect.Value{reflect.ValueOf(handler)}
+ in = append(in, reflect.ValueOf(ctx))
+ for _, arg := range args {
+ in = append(in, reflect.ValueOf(arg))
+ }
+ returnValues := method.Func.Call(in)
+
+ // Process return values
+ if len(returnValues) == 1 {
+ if isReflectNil(returnValues[0]) {
+ return nil, nil
+ }
+ if err, ok := returnValues[0].Interface().(error); ok {
+ return nil, err
+ }
+ return nil, nil
+ }
+ var result any
+ var err error
+ if !isReflectNil(returnValues[0]) {
+ result = returnValues[0].Interface()
+ }
+ if len(returnValues) > 1 && !isReflectNil(returnValues[1]) {
+ if e, ok := returnValues[1].Interface().(error); ok {
+ err = e
+ }
+ }
+ return result, err
+}
+
+// createReflectionMethodFunc creates a MethodFunc that calls the given method
via reflection.
+func createReflectionMethodFunc(method reflect.Method) func(ctx
context.Context, args []any, handler any) (any, error) {
+ return func(ctx context.Context, args []any, handler any) (any, error) {
+ return CallMethodByReflection(ctx, method, handler, args)
+ }
+}
+
// Add a method with a name of a different first-letter case
// to achieve interoperability with java
// TODO: The method name case sensitivity in Dubbo-java should be addressed.
Review Comment:
I'm wondering is this todo solved?
##########
protocol/triple/server.go:
##########
@@ -503,51 +503,101 @@ func createServiceInfoWithReflection(svc
common.RPCService) *common.ServiceInfo
if methodType.Name == "Reference" {
continue
}
- paramsNum := methodType.Type.NumIn()
- // the first param is receiver itself, the second param is ctx
- // just ignore them
- if paramsNum < 2 {
- logger.Fatalf("TRIPLE does not support %s method that
does not have any parameter", methodType.Name)
- continue
- }
- paramsTypes := make([]reflect.Type, paramsNum-2)
- for j := 2; j < paramsNum; j++ {
- paramsTypes[j-2] = methodType.Type.In(j)
- }
- methodInfo := common.MethodInfo{
- Name: methodType.Name,
- // only support Unary invocation now
- Type: constant.CallUnary,
- ReqInitFunc: func() any {
- params := make([]any, len(paramsTypes))
- for k, paramType := range paramsTypes {
- params[k] =
reflect.New(paramType).Interface()
- }
- return params
- },
+ methodInfo := buildMethodInfoWithReflection(methodType)
+ if methodInfo != nil {
+ methodInfos = append(methodInfos, *methodInfo)
}
- methodInfos = append(methodInfos, methodInfo)
}
- // only support no-idl mod call unary
- genericMethodInfo := common.MethodInfo{
- Name: "$invoke",
- Type: constant.CallUnary,
+ // Add $invoke method for generic call support
+ methodInfos = append(methodInfos, buildGenericMethodInfo())
Review Comment:
this `methodInfos` is appended inside the loop in line 508 and appended
outside the loop in line 513, is it correct?
##########
protocol/triple/triple_protocol/codec.go:
##########
@@ -227,29 +322,51 @@ func (c *protoWrapperCodec) Marshal(message any) ([]byte,
error) {
return proto.Marshal(wrapperReq)
}
+// Unmarshal handles both TripleResponseWrapper (for responses) and
TripleRequestWrapper (for requests).
+// It determines the format by checking if message is a slice (request) or not
(response).
func (c *protoWrapperCodec) Unmarshal(binary []byte, message any) error {
- var params []any
- var ok bool
- params, ok = message.([]any)
- if !ok {
- params = []any{message}
- }
+ // Check if message is a slice - if so, it's a request with multiple
args
+ params, isSlice := message.([]any)
+ if isSlice {
Review Comment:
ditto
##########
server/server.go:
##########
@@ -176,6 +179,61 @@ func (s *Server) genSvcOpts(handler any, info
*common.ServiceInfo, opts ...Servi
return newSvcOpts, nil
}
+// isNillable checks if a reflect.Value's kind supports nil checking.
+func isNillable(v reflect.Value) bool {
+ switch v.Kind() {
+ case reflect.Chan, reflect.Func, reflect.Interface, reflect.Map,
reflect.Pointer, reflect.Slice:
+ return true
+ default:
+ return false
+ }
+}
+
+// isReflectNil safely checks if a reflect.Value is nil.
+func isReflectNil(v reflect.Value) bool {
+ return isNillable(v) && v.IsNil()
+}
Review Comment:
is this method redefined in the protocol/triple/server.go:594?
##########
protocol/triple/triple_protocol/codec.go:
##########
@@ -114,11 +118,62 @@ func (c *protoBinaryCodec) Marshal(message any) ([]byte,
error) {
func (c *protoBinaryCodec) Unmarshal(data []byte, message any) error {
protoMessage, ok := message.(proto.Message)
if !ok {
- return errNotProto(message)
+ // Non-proto types indicate a generic call - try to unwrap from
wrapper format.
+ // This is used by the server when receiving Java/Go generic
calls.
+ return c.unmarshalWrappedMessage(data, message)
}
return proto.Unmarshal(data, protoMessage)
}
+// unmarshalWrappedMessage handles both TripleResponseWrapper and
TripleRequestWrapper formats.
+// It determines the format by checking if message is a slice (request) or not
(response).
+func (c *protoBinaryCodec) unmarshalWrappedMessage(data []byte, message any)
error {
+ hessianCodec := &hessian2Codec{}
+
+ // Check if message is a slice - if so, it's a request with multiple
args
+ params, isSlice := message.([]any)
+ if isSlice {
Review Comment:
```suggestion
if params, isSlice := message.([]any); isSlice {
```
--
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]
