skt-shinyruo commented on code in PR #16325:
URL: https://github.com/apache/dubbo/pull/16325#discussion_r3481215379
##########
dubbo-rpc/dubbo-rpc-triple/src/main/java/org/apache/dubbo/rpc/protocol/tri/DescriptorUtils.java:
##########
@@ -99,49 +101,133 @@ public static MethodDescriptor
findReflectionMethodDescriptor(
.get(0);
} else {
List<MethodDescriptor> methodDescriptors =
serviceDescriptor.getMethods(methodName);
- if (CollectionUtils.isEmpty(methodDescriptors)) {
- return null;
- }
- // In most cases there is only one method
- if (methodDescriptors.size() == 1) {
- methodDescriptor = methodDescriptors.get(0);
- }
- // generated unary method ,use unary type
- // Response foo(Request)
- // void foo(Request,StreamObserver<Response>)
- if (methodDescriptors.size() == 2) {
- if (methodDescriptors.get(1).getRpcType() ==
MethodDescriptor.RpcType.SERVER_STREAM) {
- methodDescriptor = methodDescriptors.get(0);
- } else if (methodDescriptors.get(0).getRpcType() ==
MethodDescriptor.RpcType.SERVER_STREAM) {
- methodDescriptor = methodDescriptors.get(1);
- }
- }
+ methodDescriptor =
findSingleOrGeneratedUnaryMethodDescriptor(methodDescriptors);
}
return methodDescriptor;
}
public static MethodDescriptor findTripleMethodDescriptor(
ServiceDescriptor serviceDescriptor, String methodName,
InputStream rawMessage) throws IOException {
- MethodDescriptor methodDescriptor =
findReflectionMethodDescriptor(serviceDescriptor, methodName);
- if (methodDescriptor == null) {
- rawMessage.mark(Integer.MAX_VALUE);
- List<MethodDescriptor> methodDescriptors =
serviceDescriptor.getMethods(methodName);
- TripleRequestWrapper request =
TripleRequestWrapper.parseFrom(rawMessage);
- String[] paramTypes = request.getArgTypes().toArray(new String[0]);
- // wrapper mode the method can overload so maybe list
- for (MethodDescriptor descriptor : methodDescriptors) {
- // params type is array
- if (Arrays.equals(descriptor.getCompatibleParamSignatures(),
paramTypes)) {
- methodDescriptor = descriptor;
- break;
- }
+ if (isGeneric(methodName)) {
+ return ServiceDescriptorInternalCache.genericService()
+ .getMethods(methodName)
+ .get(0);
+ }
+ if (isEcho(methodName)) {
+ return ServiceDescriptorInternalCache.echoService()
+ .getMethods(methodName)
+ .get(0);
+ }
+
+ List<MethodDescriptor> methodDescriptors =
serviceDescriptor.getMethods(methodName);
+ if (CollectionUtils.isEmpty(methodDescriptors)) {
+ throw new UnimplementedException("method:" + methodName);
+ }
+ if (methodDescriptors.size() == 1) {
+ return methodDescriptors.get(0);
+ }
+
+ TripleRequestWrapper request = parseRequestWrapper(rawMessage);
+ List<String> argTypes = request == null ? null : request.getArgTypes();
+ if (argTypes != null) {
+ MethodDescriptor methodDescriptor =
+ findMethodDescriptorByParamTypes(methodDescriptors,
argTypes.toArray(new String[0]));
+ if (methodDescriptor != null) {
+ return methodDescriptor;
}
- if (methodDescriptor == null) {
+ if (CollectionUtils.isNotEmpty(argTypes)) {
throw new UnimplementedException("method:" + methodName);
}
+ }
+
+ MethodDescriptor methodDescriptor =
findGeneratedUnaryMethodDescriptor(methodDescriptors);
+ if (methodDescriptor != null) {
+ return methodDescriptor;
+ }
+ throw new UnimplementedException("method:" + methodName);
+ }
+
+ private static MethodDescriptor findSingleOrGeneratedUnaryMethodDescriptor(
+ List<MethodDescriptor> methodDescriptors) {
+ if (CollectionUtils.isEmpty(methodDescriptors)) {
+ return null;
+ }
+ // In most cases there is only one method
+ if (methodDescriptors.size() == 1) {
+ return methodDescriptors.get(0);
+ }
+ return findGeneratedUnaryMethodDescriptor(methodDescriptors);
+ }
+
+ private static TripleRequestWrapper parseRequestWrapper(InputStream
rawMessage) throws IOException {
+ rawMessage.mark(Integer.MAX_VALUE);
+ try {
+ return TripleRequestWrapper.parseFrom(rawMessage);
+ } catch (IOException | RuntimeException ignored) {
+ return null;
+ } finally {
rawMessage.reset();
}
- return methodDescriptor;
+ }
+
+ private static MethodDescriptor findMethodDescriptorByParamTypes(
+ List<MethodDescriptor> methodDescriptors, String[] paramTypes) {
+ for (MethodDescriptor descriptor : methodDescriptors) {
+ // wrapper mode the method can overload so maybe list
+ if (Arrays.equals(descriptor.getCompatibleParamSignatures(),
paramTypes)) {
+ return descriptor;
+ }
+ }
+ return null;
+ }
+
+ private static MethodDescriptor
findGeneratedUnaryMethodDescriptor(List<MethodDescriptor> methodDescriptors) {
+ // Generated unary methods may expose two Java methods for the same
RPC:
+ // Response foo(Request)
+ // void foo(Request, StreamObserver<Response>)
+ if (methodDescriptors.size() != 2) {
+ return null;
+ }
+
+ MethodDescriptor unaryMethodDescriptor = null;
+ MethodDescriptor serverStreamMethodDescriptor = null;
+ for (MethodDescriptor descriptor : methodDescriptors) {
+ if (descriptor.getRpcType() == MethodDescriptor.RpcType.UNARY) {
+ unaryMethodDescriptor = descriptor;
+ } else if (descriptor.getRpcType() ==
MethodDescriptor.RpcType.SERVER_STREAM) {
+ serverStreamMethodDescriptor = descriptor;
+ }
+ }
+ if (unaryMethodDescriptor == null || serverStreamMethodDescriptor ==
null) {
+ return null;
+ }
+ if (!Arrays.equals(
+ unaryMethodDescriptor.getParameterClasses(),
+ getServerStreamRequestTypes(serverStreamMethodDescriptor))) {
+ return null;
+ }
+ if (!isSameResponseType(unaryMethodDescriptor,
serverStreamMethodDescriptor)) {
+ return null;
+ }
+ return unaryMethodDescriptor;
+ }
+
+ private static Class<?>[] getServerStreamRequestTypes(MethodDescriptor
serverStreamMethodDescriptor) {
+ Class<?>[] parameterClasses =
serverStreamMethodDescriptor.getParameterClasses();
+ if (parameterClasses.length == 0
+ ||
!StreamObserver.class.isAssignableFrom(parameterClasses[parameterClasses.length
- 1])) {
+ return null;
+ }
+ return Arrays.copyOf(parameterClasses, parameterClasses.length - 1);
+ }
+
+ private static boolean isSameResponseType(
+ MethodDescriptor unaryMethodDescriptor, MethodDescriptor
serverStreamMethodDescriptor) {
+ Type[] returnTypes = unaryMethodDescriptor.getReturnTypes();
+ if (returnTypes.length == 0 || !(returnTypes[0] instanceof Class)) {
+ return false;
+ }
+ return returnTypes[0] ==
serverStreamMethodDescriptor.getActualResponseType();
Review Comment:
Thanks for the review. I updated the PR.
- The reflection path now throws `UnimplementedException` for ambiguous
2-method overloads instead of returning `null` and letting the caller NPE.
- `parseRequestWrapper` no longer swallows parse failures, so corrupt
wrappers now surface the decode error instead of falling back silently.
- I also extracted the duplicated generic/echo lookup into a shared helper.
Added regression tests for the ambiguous reflection-overload case and the
corrupt-wrapper case, and re-ran `DescriptorUtilsTest` and
`ReflectionPackableMethodTest`.
--
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]
