-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Triple stream support (issue#1332) #1360
Triple stream support (issue#1332) #1360
Conversation
Hi @namelessssssssssss, welcome to SOFAStack community, Please sign Contributor License Agreement! After you signed CLA, we will automatically sync the status of this pull request in 3 minutes. |
@EvenLjj PTAL :) |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1360 +/- ##
============================================
+ Coverage 72.04% 72.19% +0.15%
- Complexity 791 793 +2
============================================
Files 422 425 +3
Lines 17814 18052 +238
Branches 2768 2801 +33
============================================
+ Hits 12834 13033 +199
- Misses 3565 3590 +25
- Partials 1415 1429 +14 ☔ View full report in Codecov by Sentry. |
This is interesting 😀 |
确实是这样的, 因此方案中还需要考虑如何让 bolt 协议来支持流式处理. 我能想到两种方案:
我个人比较希望采用第一种方案,让bolt 原生支持 stream ,这能让使用bolt 的其他框架,也能享受到 stream 的优势. 回到这个 PR 上, 我建议统筹考虑 bolt/sofarpc 应该如何支持 stream ,再来合并这个PR. |
0a14a70
to
df27427
Compare
WalkthroughThe updates involve enriching RPC functionalities with added support for various streaming types (unary, client, server, and bidirectional). Changes include adjusting method signatures to accommodate streaming, introducing new utilities for handling streaming and exceptions, and expanding test cases to ensure robustness. This overhaul not only streamlines method invocations but also extends the RPC framework's capabilities to effectively handle complex streaming scenarios. Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 14
Configuration used: CodeRabbit UI
Files ignored due to path filters (3)
remoting/remoting-triple/build/generated/source/proto/main/java/triple/GenericProto.java
is excluded by:!**/generated/**
remoting/remoting-triple/build/generated/source/proto/main/java/triple/GenericServiceGrpc.java
is excluded by:!**/generated/**
remoting/remoting-triple/build/generated/source/proto/main/java/triple/SofaGenericServiceTriple.java
is excluded by:!**/generated/**
Files selected for processing (29)
- bootstrap/bootstrap-api/src/main/java/com/alipay/sofa/rpc/bootstrap/DefaultClientProxyInvoker.java (1 hunks)
- core-impl/client/src/main/java/com/alipay/sofa/rpc/client/AbstractCluster.java (1 hunks)
- core/api/src/main/java/com/alipay/sofa/rpc/common/RpcConstants.java (1 hunks)
- core/api/src/main/java/com/alipay/sofa/rpc/config/AbstractInterfaceConfig.java (3 hunks)
- core/api/src/main/java/com/alipay/sofa/rpc/config/ConsumerConfig.java (2 hunks)
- core/api/src/main/java/com/alipay/sofa/rpc/config/MethodConfig.java (3 hunks)
- core/api/src/main/java/com/alipay/sofa/rpc/config/ProviderConfig.java (1 hunks)
- core/api/src/main/java/com/alipay/sofa/rpc/message/MessageBuilder.java (1 hunks)
- core/api/src/main/java/com/alipay/sofa/rpc/transport/StreamHandler.java (1 hunks)
- core/exception/src/main/java/com/alipay/sofa/rpc/core/exception/RpcErrorType.java (1 hunks)
- remoting/remoting-bolt/src/main/java/com/alipay/sofa/rpc/filter/ConsumerGenericFilter.java (1 hunks)
- remoting/remoting-triple/src/main/java/com/alipay/sofa/rpc/message/triple/stream/ClientStreamObserverAdapter.java (1 hunks)
- remoting/remoting-triple/src/main/java/com/alipay/sofa/rpc/message/triple/stream/ResponseSerializeStreamHandler.java (1 hunks)
- remoting/remoting-triple/src/main/java/com/alipay/sofa/rpc/server/triple/GenericServiceImpl.java (6 hunks)
- remoting/remoting-triple/src/main/java/com/alipay/sofa/rpc/server/triple/TripleServer.java (8 hunks)
- remoting/remoting-triple/src/main/java/com/alipay/sofa/rpc/server/triple/UniqueIdInvoker.java (3 hunks)
- remoting/remoting-triple/src/main/java/com/alipay/sofa/rpc/transport/triple/TripleClientInvoker.java (8 hunks)
- remoting/remoting-triple/src/main/java/com/alipay/sofa/rpc/utils/SofaProtoUtils.java (2 hunks)
- remoting/remoting-triple/src/main/java/com/alipay/sofa/rpc/utils/TripleExceptionUtils.java (1 hunks)
- remoting/remoting-triple/src/main/proto/transformer.proto (1 hunks)
- remoting/remoting-triple/src/test/java/com/alipay/sofa/rpc/server/triple/GenericServiceImplTest.java (2 hunks)
- test/test-integration/src/main/proto/helloworld.proto (1 hunks)
- test/test-integration/src/test/java/com/alipay/sofa/rpc/test/triple/GreeterImpl.java (1 hunks)
- test/test-integration/src/test/java/com/alipay/sofa/rpc/test/triple/TripleServerTest.java (2 hunks)
- test/test-integration/src/test/java/com/alipay/sofa/rpc/triple/stream/ClientRequest.java (1 hunks)
- test/test-integration/src/test/java/com/alipay/sofa/rpc/triple/stream/HelloService.java (1 hunks)
- test/test-integration/src/test/java/com/alipay/sofa/rpc/triple/stream/HelloServiceImpl.java (1 hunks)
- test/test-integration/src/test/java/com/alipay/sofa/rpc/triple/stream/ServerResponse.java (1 hunks)
- test/test-integration/src/test/java/com/alipay/sofa/rpc/triple/stream/TripleGenericStreamTest.java (1 hunks)
Files not reviewed due to errors (1)
- (no review received)
Additional comments: 53
remoting/remoting-triple/src/main/proto/transformer.proto (2)
- 13-13: The addition of
genericBiStream
method supports bidirectional streaming, aligning with the PR's objectives to enhance streaming capabilities.- 15-15: The addition of
genericServerStream
method supports server streaming, which is consistent with the PR's goal to introduce various streaming models.test/test-integration/src/test/java/com/alipay/sofa/rpc/triple/stream/ServerResponse.java (1)
- 19-35: The
ServerResponse
class is well-structured with appropriate fields and methods. Good job following best practices for encapsulation.test/test-integration/src/main/proto/helloworld.proto (1)
- 29-29: The addition of
SayHelloBinary
method supports bidirectional streaming, aligning with the PR's objectives to enhance streaming capabilities.remoting/remoting-triple/src/main/java/com/alipay/sofa/rpc/utils/TripleExceptionUtils.java (1)
- 24-30: The
asStatusRuntimeException
method is well-implemented, correctly handling null cases and convertingThrowable
into aStatusRuntimeException
. Good job following best practices for error handling.test/test-integration/src/test/java/com/alipay/sofa/rpc/triple/stream/HelloService.java (1)
- 21-38: The
HelloService
interface is well-designed, covering unary, bidirectional streaming, and server streaming use cases. The constants for triggering specific stream behaviors are a thoughtful addition.core/api/src/main/java/com/alipay/sofa/rpc/transport/StreamHandler.java (1)
- 22-47: The
StreamHandler
interface provides a clear and concise contract for handling streaming messages, completion, and exceptions, aligning well with the PR's objectives and resembling gRPC'sStreamObserver
.remoting/remoting-triple/src/main/java/com/alipay/sofa/rpc/message/triple/stream/ResponseSerializeStreamHandler.java (1)
- 30-62: The
ResponseSerializeStreamHandler
class correctly implements theStreamHandler
interface, effectively handling message serialization, stream completion, and exceptions. The dynamic serializer selection based onserializeType
is a good practice.core/exception/src/main/java/com/alipay/sofa/rpc/core/exception/RpcErrorType.java (1)
- 96-99: The addition of
CLIENT_CALL_TYPE
with the value260
to represent an unsupported RPC call type exception is a good practice. It enhances error specificity and aids in debugging. However, it would be beneficial to ensure that this new error type is well-documented where it's used, to inform developers of its purpose and usage context.remoting/remoting-triple/src/main/java/com/alipay/sofa/rpc/message/triple/stream/ClientStreamObserverAdapter.java (1)
- 43-70: The implementation of
ClientStreamObserverAdapter
is well-structured and aligns with the objectives of integrating streaming capabilities. However, it's recommended to add error handling around the deserialization logic in theonNext
method to gracefully handle potential serialization issues.test/test-integration/src/test/java/com/alipay/sofa/rpc/test/triple/GreeterImpl.java (1)
- 55-73: The
sayHelloBinary
method correctly implements a binary streaming pattern. For completeness and better test coverage, consider adding basic logging or handling in theonError
andonCompleted
methods, even if it's just logging the events.remoting/remoting-triple/src/main/java/com/alipay/sofa/rpc/utils/SofaProtoUtils.java (1)
- 71-87: The implementation of
mapGrpcCallType
method is well-done, providing clear mapping between SOFA RPC call types and gRPC method types. It's recommended to add documentation explaining the mapping logic and the scenarios in which theSofaRpcException
will be thrown for unsupported invoke types.test/test-integration/src/test/java/com/alipay/sofa/rpc/triple/stream/HelloServiceImpl.java (1)
- 21-89: The implementations of streaming methods in
HelloServiceImpl
are well-done and align with the expected streaming patterns. It would be beneficial to add comments explaining the purpose and usage of special commands likeCMD_TRIGGER_STREAM_FINISH
andCMD_TRIGGER_STEAM_ERROR
within the test scenarios.remoting/remoting-triple/src/test/java/com/alipay/sofa/rpc/server/triple/GenericServiceImplTest.java (1)
- 66-66: The addition of the
providerConfig
parameter to theGenericServiceImpl
constructor and the extra argument in theTripleClientInvoker.getRequest
call are significant changes that enhance integration with the streaming capabilities. Ensure that the impact of these changes is well-documented and thoroughly tested to confirm their effectiveness and to avoid potential regressions.bootstrap/bootstrap-api/src/main/java/com/alipay/sofa/rpc/bootstrap/DefaultClientProxyInvoker.java (1)
- 95-95: The change from
getMethodInvokeType(request.getMethodName())
togetMethodInvokeType(request)
in thedecorateRequest
method likely enhances how theinvokeType
is determined by considering more information from theSofaRequest
object. Ensure that the new method signature is supported everywhere it's used and that this change integrates well with the rest of the system.remoting/remoting-triple/src/main/java/com/alipay/sofa/rpc/server/triple/UniqueIdInvoker.java (2)
- 19-19: The addition of imports for
RpcConstants
,StreamHandler
, and Java utility classes supports the new functionality introduced in this file. These are standard and necessary for the changes made.Also applies to: 31-31, 35-35, 37-37
- 194-194: Changing the access level of the
getServiceUniqueName
method to public is likely intended to make it accessible from outside the class. Ensure there are no unintended side effects of making this method public.core/api/src/main/java/com/alipay/sofa/rpc/config/MethodConfig.java (2)
- 19-20: The addition of imports for
RpcConstants
,RpcErrorType
,SofaRpcException
,SofaResponseCallback
, andStreamHandler
supports the new functionality introduced in this file. These are standard and necessary for the changes made.Also applies to: 21-21, 23-23
- 336-369: The
mapStreamType
method introduces logic to determine the stream call type for a given method, which is a crucial enhancement for supporting streaming. Consider refining the error messages thrown by this method to ensure they are clear and provide sufficient information for debugging.core/api/src/main/java/com/alipay/sofa/rpc/config/ProviderConfig.java (1)
- 211-211: The addition of the
loadMethodCallType
call in thesetRef
method likely aims to enhance automatic configuration based on the reference class. Ensure that theloadMethodCallType
method is implemented correctly and assess its impact on performance and behavior.Verification successful
The
loadMethodCallType
method is indeed implemented inAbstractInterfaceConfig.java
and used inProviderConfig.java
, confirming the initial observation. While the method's existence and purpose align with the expectations, assessing its implementation details and impact on performance and behavior directly is beyond the current scope. It's recommended to review the implementation ofloadMethodCallType
for correctness and efficiency.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify the implementation of loadMethodCallType grep -R "loadMethodCallType" . # Assess performance impact if possibleLength of output: 291
remoting/remoting-triple/src/main/java/com/alipay/sofa/rpc/server/triple/GenericServiceImpl.java (7)
- 21-26: Adding imports for
RpcConstants
,ReflectCache
,StringUtils
, andProviderConfig
indicates that these classes are now being used withinGenericServiceImpl
. Ensure that these new dependencies are justified and optimally used within the implementation to avoid unnecessary coupling and to maintain modularity.- 34-37: The addition of imports for
ResponseSerializeStreamHandler
andStreamHandler
suggests the introduction of streaming capabilities withinGenericServiceImpl
. It's crucial to ensure that these handlers are correctly implemented and integrated, particularly focusing on error handling, stream lifecycle management, and performance considerations.- 61-66: The introduction of a new private field
providerConfig
and its initialization within the constructor enhances the class's capability to access provider configuration details. This change should be carefully reviewed to ensure that it aligns with the class's responsibilities and does not introduce unnecessary dependencies or complexity.- 77-78: The method
setClassLoaderAndGetRequestMethod
is being called with specific parameters. It's important to verify that this method correctly handles class loading and method retrieval, especially considering the dynamic nature of class loading in Java and potential classloader leaks.- 84-84: The method
setUnaryOrServerRequestParams
is being utilized to set request parameters for unary or server streaming calls. Ensure that this method adequately validates its inputs and correctly handles the serialization and deserialization of request parameters to prevent issues such as data corruption or security vulnerabilities.- 198-203: The method
getBidirectionalStreamRequestMethod
retrieves the method for bidirectional streaming. It's important to verify that this method accurately identifies the correct method to be used for bidirectional streaming, considering potential overloads and ensuring compatibility with the streaming protocol used.- 325-335: The methods
getSerialization
andgetDefaultSerialization
are designed to retrieve the serialization type for the provider. It's important to ensure that these methods correctly determine the serialization type based on the provider configuration and fall back to a sensible default if necessary. Additionally, the impact of serialization choices on performance and compatibility should be considered.test/test-integration/src/test/java/com/alipay/sofa/rpc/test/triple/TripleServerTest.java (1)
- 36-43: Adding imports for
StreamObserver
,CountDownLatch
, andTimeUnit
indicates that the new test methodtestBiStream
utilizes these classes for testing bidirectional streaming functionality. Ensure that these imports are optimally used within the test to maintain clarity and focus on testing objectives.remoting/remoting-triple/src/main/java/com/alipay/sofa/rpc/server/triple/TripleServer.java (3)
- 60-62: The addition of imports for
ArrayList
andCollection
suggests new collection manipulations withinTripleServer
. Ensure that these collections are used efficiently, considering their impact on performance, especially in concurrent environments or when handling large datasets.- 117-117: The adjustment in the declaration of
invokerMap
to explicitly specify its type asConcurrentHashMap
is a good practice for clarity and ensuring thread safety in concurrent access scenarios. This change aligns with the need for thread-safe operations on theinvokerMap
.- 301-301: The addition of
providerConfig
as a parameter in the instantiation ofGenericServiceImpl
withingetServerServiceDefinition
method enhances the ability to pass configuration details directly to the service implementation. Ensure that this change is consistently applied and that theproviderConfig
is used appropriately withinGenericServiceImpl
.remoting/remoting-triple/src/main/java/com/alipay/sofa/rpc/transport/triple/TripleClientInvoker.java (12)
- 41-50: New imports related to message handling and stream processing have been introduced. Ensure that these imports are used effectively throughout the file and that there are no unused imports to maintain code cleanliness.
- 74-89: The introduction of new class fields such as
serializer
andserialization
is noted. It's important to ensure that these fields are properly encapsulated and accessed in a thread-safe manner if necessary, given the concurrent nature of network programming.- 111-125: The
getRequest
method has been modified to support serialization of method arguments. Ensure that the serialization process is secure and efficient, especially for large data payloads. Consider handling potential serialization exceptions gracefully.- 145-153: The
invoke
method now includes logic to handle different call types (unary, client streaming, server streaming, and bidirectional streaming). It's crucial to ensure that the mapping betweenSofaRequest
call types and gRPC call types is accurate and that each case is handled appropriately.- 156-167: The
mapCallType
method correctly maps SOFA RPC call types to gRPC call types. Verify that all supported call types are covered and that the default case (unary) is the intended fallback option.- 170-180: The
streamCall
method's switch statement handles different streaming call types. Ensure that each case is implemented correctly and that the default case throws an exception for unknown call types, as this aids in debugging and maintainability.- 184-200: The
unaryCall
method's implementation for unary calls looks correct. Pay attention to error handling and ensure that the response deserialization process is secure and efficient.- 203-210: The
stubCall
method uses reflection to invoke methods on the stub. Ensure that this approach is safe and consider potential performance implications of using reflection.- 213-250: The
binaryStreamCall
and related streaming call methods (clientStreamCall
,serverStreamCall
) introduce complex logic for handling streaming calls. It's important to thoroughly test these methods to ensure they handle stream lifecycle events correctly and efficiently.- 318-318: The
asyncInvoke
method has been modified to support asynchronous invocation. Ensure that the asynchronous logic is correctly implemented and that callback and future mechanisms are properly handled.- 345-355: The
rebuildTrueRequestArgSigs
method rebuilds argument signatures for stream calls. Verify that this method correctly handles null arguments and that the argument types are accurately reconstructed.- 479-494: The
getMethodDescriptor
method constructs aMethodDescriptor
for gRPC calls. Ensure that the method descriptor is correctly built, including the correct call type and marshaller for request and response types.core/api/src/main/java/com/alipay/sofa/rpc/common/RpcConstants.java (1)
- 124-139: New constants for different invocation types (unary, client streaming, server streaming, and bidirectional streaming) have been added. Ensure that these constants are used consistently throughout the codebase and that their naming convention aligns with existing constants for clarity and maintainability.
core/api/src/main/java/com/alipay/sofa/rpc/config/ConsumerConfig.java (4)
- 30-30: Importing
SofaRequest
is necessary for the changes made in this file, specifically for the methodgetMethodInvokeType
which now accepts aSofaRequest
object instead of a method name. This change aligns with the PR's objective to enhance stream transport support.- 939-952: The method
getMethodInvokeType
has been modified to accept aSofaRequest
object instead of a method name. This change is crucial for supporting stream transport, as it allows the method to determine the invocation type based on the request's details, including the method name and potentially other request-specific information in the future. This approach enhances flexibility and aligns with the PR's objectives.However, it's important to ensure that all calls to this method throughout the codebase have been updated to pass a
SofaRequest
object instead of just a method name to avoid runtime errors.Verification successful
Based on the script execution results, which did not find any instances of
getMethodInvokeType
being called with a single string argument, it appears that all calls to this method have been correctly updated to pass aSofaRequest
object. This aligns with the requirements for supporting stream transport and enhancing flexibility as mentioned in the review comment. Therefore, the changes have been successfully verified.* 959-970: The `getAndCacheCallType` method is a new addition that dynamically determines and caches the call type for a given `SofaRequest`. This method is essential for the stream transport support, as it allows for efficient handling of different types of streaming requests by caching the call type at runtime. This caching mechanism can improve performance by avoiding repeated determination of the call type for the same method.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for calls to getMethodInvokeType with a single string argument, which should now be updated. ast-grep --lang java --pattern $'getMethodInvokeType($_)' | grep -v 'SofaRequest'Length of output: 80
It's important to ensure that the caching mechanism is thread-safe and that the cache is properly invalidated or updated when necessary to prevent stale or incorrect call type information.
- 977-994: The
buildMethodConfigKey
andgetMethodSignature
methods are new additions that support the caching mechanism for method call types.buildMethodConfigKey
constructs a unique key for each method configuration, which includes the method signature.getMethodSignature
generates a string representation of a method's signature, which is used as part of the cache key.These methods are crucial for ensuring that the call type for each unique method is correctly identified and cached. It's important to verify that the method signature generation correctly handles overloads and different parameter types to ensure uniqueness and accuracy in the cache keys.
core/api/src/main/java/com/alipay/sofa/rpc/config/AbstractInterfaceConfig.java (4)
- 215-218: The introduction of the
methodCallType
field is a significant addition that supports the caching of method call types. This field is crucial for the stream transport support, as it allows for efficient retrieval of the call type for each method, which is essential for handling different types of streaming requests.It's important to ensure that access to this field is thread-safe, given that it may be accessed and modified by multiple threads concurrently.
- 258-264: The
loadMethodCallType
method is responsible for initializing themethodCallType
cache with the default call type for each method of the provided interface class. This method is crucial for the stream transport support, as it pre-populates the cache with default values, which can then be overridden based on specific configurations or request types.It's important to ensure that this method is called at the appropriate time during the initialization of the interface config to ensure that the cache is correctly populated before it's used.
- 266-268: The
getMethodCallType
method provides a way to retrieve the cached call type for a given method name. This method is essential for efficiently determining the call type for method invocations, supporting the stream transport enhancements introduced in the PR.It's important to verify that the method name used as the key in the
methodCallType
map accurately reflects the method's signature, especially in cases where method overloading is used, to ensure the correct call type is retrieved.
- 1038-1038: The
buildmkey
method is used to construct a unique key for caching method-specific configurations, including the call type. This method is crucial for ensuring that configurations are correctly associated with the appropriate methods, especially in cases where methods are overloaded.It's important to ensure that the key generation logic accurately reflects the method's signature to prevent collisions or incorrect configurations being applied to methods.
core-impl/client/src/main/java/com/alipay/sofa/rpc/client/AbstractCluster.java (1)
- 660-663: Using
Integer.MAX_VALUE
as a timeout for streaming requests effectively means "wait indefinitely." While this might be suitable for some streaming scenarios, it's important to consider if there are cases where a more finite timeout would be appropriate to avoid potential resource exhaustion or hanging connections. Additionally, the uniform handling of different streaming types (CLIENT_STREAMING
,BI_STREAMING
,SERVER_STREAMING
) without differentiation might be intentional but ensure this aligns with the design goals and consider if any specific handling is needed for each type.
remoting/remoting-triple/src/main/java/com/alipay/sofa/rpc/server/triple/UniqueIdInvoker.java
Outdated
Show resolved
Hide resolved
...ting/remoting-triple/src/main/java/com/alipay/sofa/rpc/server/triple/GenericServiceImpl.java
Outdated
Show resolved
Hide resolved
...ting/remoting-triple/src/main/java/com/alipay/sofa/rpc/server/triple/GenericServiceImpl.java
Outdated
Show resolved
Hide resolved
...ting/remoting-triple/src/main/java/com/alipay/sofa/rpc/server/triple/GenericServiceImpl.java
Outdated
Show resolved
Hide resolved
...ting/remoting-triple/src/main/java/com/alipay/sofa/rpc/server/triple/GenericServiceImpl.java
Outdated
Show resolved
Hide resolved
test/test-integration/src/test/java/com/alipay/sofa/rpc/test/triple/TripleServerTest.java
Outdated
Show resolved
Hide resolved
remoting/remoting-triple/src/main/java/com/alipay/sofa/rpc/server/triple/TripleServer.java
Show resolved
Hide resolved
test/test-integration/src/test/java/com/alipay/sofa/rpc/triple/stream/ClientRequest.java
Outdated
Show resolved
Hide resolved
core/api/src/main/java/com/alipay/sofa/rpc/message/MessageBuilder.java
Outdated
Show resolved
Hide resolved
remoting/remoting-bolt/src/main/java/com/alipay/sofa/rpc/filter/ConsumerGenericFilter.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 10
...ting/remoting-triple/src/main/java/com/alipay/sofa/rpc/server/triple/GenericServiceImpl.java
Outdated
Show resolved
Hide resolved
...ting/remoting-triple/src/main/java/com/alipay/sofa/rpc/server/triple/GenericServiceImpl.java
Outdated
Show resolved
Hide resolved
...ting/remoting-triple/src/main/java/com/alipay/sofa/rpc/server/triple/GenericServiceImpl.java
Show resolved
Hide resolved
...ting/remoting-triple/src/main/java/com/alipay/sofa/rpc/server/triple/GenericServiceImpl.java
Show resolved
Hide resolved
...ting/remoting-triple/src/main/java/com/alipay/sofa/rpc/server/triple/GenericServiceImpl.java
Outdated
Show resolved
Hide resolved
test/test-integration/src/test/java/com/alipay/sofa/rpc/test/triple/stream/ClientRequest.java
Outdated
Show resolved
Hide resolved
test/test-integration/src/test/java/com/alipay/sofa/rpc/test/triple/stream/ClientRequest.java
Outdated
Show resolved
Hide resolved
test/test-integration/src/test/java/com/alipay/sofa/rpc/test/triple/stream/HelloService.java
Outdated
Show resolved
Hide resolved
...st-integration/src/test/java/com/alipay/sofa/rpc/test/triple/stream/ExtendClientRequest.java
Outdated
Show resolved
Hide resolved
837b208
to
0103a31
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
.../remoting-triple/src/main/java/com/alipay/sofa/rpc/transport/triple/TripleClientInvoker.java
Outdated
Show resolved
Hide resolved
.../remoting-triple/src/main/java/com/alipay/sofa/rpc/transport/triple/TripleClientInvoker.java
Outdated
Show resolved
Hide resolved
...ting/remoting-triple/src/main/java/com/alipay/sofa/rpc/server/triple/GenericServiceImpl.java
Outdated
Show resolved
Hide resolved
...ting/remoting-triple/src/main/java/com/alipay/sofa/rpc/server/triple/GenericServiceImpl.java
Outdated
Show resolved
Hide resolved
...ting/remoting-triple/src/main/java/com/alipay/sofa/rpc/server/triple/GenericServiceImpl.java
Outdated
Show resolved
Hide resolved
...ting/remoting-triple/src/main/java/com/alipay/sofa/rpc/server/triple/GenericServiceImpl.java
Outdated
Show resolved
Hide resolved
...ting/remoting-triple/src/main/java/com/alipay/sofa/rpc/server/triple/GenericServiceImpl.java
Outdated
Show resolved
Hide resolved
remoting/remoting-triple/src/main/java/com/alipay/sofa/rpc/server/triple/UniqueIdInvoker.java
Outdated
Show resolved
Hide resolved
.../remoting-triple/src/main/java/com/alipay/sofa/rpc/transport/triple/TripleClientInvoker.java
Outdated
Show resolved
Hide resolved
core/api/src/main/java/com/alipay/sofa/rpc/message/MessageBuilder.java
Outdated
Show resolved
Hide resolved
remoting/remoting-triple/src/main/java/com/alipay/sofa/rpc/utils/SofaProtoUtils.java
Outdated
Show resolved
Hide resolved
remoting/remoting-triple/src/main/java/com/alipay/sofa/rpc/utils/SofaProtoUtils.java
Outdated
Show resolved
Hide resolved
bd169a3
to
266006d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
Out of diff range and nitpick comments (3)
test/test-integration/src/test/java/com/alipay/sofa/rpc/test/triple/stream/HelloService.java (2)
23-23
: Ensure consistent naming for command constants.Consider using a more descriptive name for
CMD_TRIGGER_STREAM_FINISH
to clarify its purpose, such asCMD_FINISH_STREAM
.
25-25
: Ensure consistent naming for command constants.Consider using a more descriptive name for
CMD_TRIGGER_STREAM_ERROR
to clarify its purpose, such asCMD_ERROR_STREAM
.remoting/remoting-triple/src/main/java/com/alipay/sofa/rpc/transport/triple/TripleClientInvoker.java (1)
483-498
: Review the method descriptor construction for completeness.Consider adding more detailed comments explaining each step in the
getMethodDescriptor
method. This will improve code readability and maintainability, especially for new developers.
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (11)
- remoting/remoting-triple/src/main/java/com/alipay/sofa/rpc/server/triple/GenericServiceImpl.java (5 hunks)
- remoting/remoting-triple/src/main/java/com/alipay/sofa/rpc/server/triple/TripleServer.java (5 hunks)
- remoting/remoting-triple/src/main/java/com/alipay/sofa/rpc/server/triple/UniqueIdInvoker.java (3 hunks)
- remoting/remoting-triple/src/main/java/com/alipay/sofa/rpc/transport/triple/TripleClientInvoker.java (10 hunks)
- remoting/remoting-triple/src/main/java/com/alipay/sofa/rpc/utils/SofaProtoUtils.java (2 hunks)
- remoting/remoting-triple/src/test/java/com/alipay/sofa/rpc/server/triple/GenericServiceImplTest.java (1 hunks)
- test/test-integration/src/test/java/com/alipay/sofa/rpc/test/triple/stream/ClientRequest.java (1 hunks)
- test/test-integration/src/test/java/com/alipay/sofa/rpc/test/triple/stream/ExtendClientRequest.java (1 hunks)
- test/test-integration/src/test/java/com/alipay/sofa/rpc/test/triple/stream/HelloService.java (1 hunks)
- test/test-integration/src/test/java/com/alipay/sofa/rpc/test/triple/stream/HelloServiceImpl.java (1 hunks)
- test/test-integration/src/test/java/com/alipay/sofa/rpc/test/triple/stream/TripleGenericStreamTest.java (1 hunks)
Files not reviewed due to errors (1)
- remoting/remoting-triple/src/main/java/com/alipay/sofa/rpc/server/triple/TripleServer.java (no review received)
Files skipped from review due to trivial changes (2)
- test/test-integration/src/test/java/com/alipay/sofa/rpc/test/triple/stream/ClientRequest.java
- test/test-integration/src/test/java/com/alipay/sofa/rpc/test/triple/stream/ExtendClientRequest.java
Additional comments not posted (14)
test/test-integration/src/test/java/com/alipay/sofa/rpc/test/triple/stream/HelloService.java (2)
29-29
: Define the method for bidirectional streaming.Ensure that the
StreamHandler
types for request and response are correctly used and documented.
31-31
: Define the method for server streaming.Ensure that the
StreamHandler
for the response is correctly used and documented.test/test-integration/src/test/java/com/alipay/sofa/rpc/test/triple/stream/HelloServiceImpl.java (2)
27-60
: Review the implementation of bidirectional streaming.Ensure that the method handles different command types (
finish
,error
) and processes extended client requests correctly.
64-78
: Review the implementation of server streaming.Ensure that the method handles the
error
command correctly and finishes the stream appropriately.remoting/remoting-triple/src/test/java/com/alipay/sofa/rpc/server/triple/GenericServiceImplTest.java (1)
139-139
: Ensure correct building of requests in tests.Verify that the
TripleClientInvoker.buildRequest
method is correctly used to construct requests based onSofaRequest
.remoting/remoting-triple/src/main/java/com/alipay/sofa/rpc/utils/SofaProtoUtils.java (2)
79-93
: Review the method for mapping gRPC call types.Ensure that the method correctly translates custom call types into gRPC
MethodDescriptor.MethodType
.
95-105
: Review the caching mechanism for stream call types.Verify that the method correctly caches the stream type for each method based on its signature.
remoting/remoting-triple/src/main/java/com/alipay/sofa/rpc/server/triple/UniqueIdInvoker.java (1)
181-191
: Review the method for getting declared methods based on call type.Ensure that the method correctly adjusts the argument types list based on the server streaming call type.
test/test-integration/src/test/java/com/alipay/sofa/rpc/test/triple/stream/TripleGenericStreamTest.java (2)
87-158
: Review the test cases for bidirectional streaming.Verify that the tests correctly simulate different streaming scenarios and handle exceptions as expected.
160-214
: Review the test cases for server streaming.Ensure that the tests correctly handle finish and exception scenarios in server streaming.
remoting/remoting-triple/src/main/java/com/alipay/sofa/rpc/server/triple/GenericServiceImpl.java (2)
98-167
: Review the implementation of bidirectional streaming.Ensure that the method correctly initializes streaming, handles stream messages, and manages errors.
169-196
: Review the implementation of server streaming.Verify that the method correctly initializes the stream, sets up request parameters, and handles stream completion and errors.
remoting/remoting-triple/src/main/java/com/alipay/sofa/rpc/transport/triple/TripleClientInvoker.java (2)
115-115
: EnsuremethodCallType
is properly initialized.This script checks that
methodCallType
is initialized before any usage to prevent runtime errors.Verification successful
The verification process has confirmed that
methodCallType
is initialized before its usage in theTripleClientInvoker.java
file. The variable is initially set tonull
and subsequently assigned a value fromSofaProtoUtils.cacheStreamCallType(consumerConfig.getProxyClass())
. It is then used to retrieve a value based on a method name. This sequence ensures thatmethodCallType
is properly initialized before it is accessed, aligning with the concerns raised in the review comment.Conclusion:
- The initialization and usage of
methodCallType
are correctly implemented in theTripleClientInvoker.java
.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that `methodCallType` is initialized before use. # Test: Search for the initialization and usage of `methodCallType`. Expect: Initialization before usage. rg --type java 'methodCallType' -- 'TripleClientInvoker.java'Length of output: 124
Script:
#!/bin/bash # Description: Verify that `methodCallType` is initialized before use in TripleClientInvoker.java. # Test: Search for the initialization and usage of `methodCallType` in TripleClientInvoker.java. Expect: Initialization before usage. fd 'TripleClientInvoker.java' | xargs rg 'methodCallType'Length of output: 275
199-239
: Ensure proper error handling in streaming calls.This script checks that errors in streaming calls are handled properly to ensure robustness.
Verification successful
The error handling in the streaming calls within the
TripleClientInvoker.java
file has been verified. The methodonError
is appropriately used to handle exceptions by converting them intoStatusRuntimeException
and passing them to the observer'sonError
method. This ensures that errors are properly propagated and handled in the streaming context.
- The
onError
method in theStreamHandler
interface is implemented to handle exceptions by callingobserver.onError
, which is a standard practice in handling streaming errors in gRPC (which is likely the underlying technology given the context).This confirms that the error handling mechanisms are in place as expected.
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify error handling in streaming calls. # Test: Search for error handling patterns in streaming methods. Expect: Proper error handling. rg --type java 'onError' -- 'TripleClientInvoker.java'Length of output: 117
Script:
#!/bin/bash # Description: Verify error handling in streaming calls in the correct file. # Test: Search for error handling patterns in streaming methods. Expect: Proper error handling. fd 'TripleClientInvoker.java' | xargs rg 'onError'Length of output: 258
.../remoting-triple/src/main/java/com/alipay/sofa/rpc/transport/triple/TripleClientInvoker.java
Outdated
Show resolved
Hide resolved
.../remoting-triple/src/main/java/com/alipay/sofa/rpc/transport/triple/TripleClientInvoker.java
Outdated
Show resolved
Hide resolved
437ff33
to
3d00d9d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 13
Out of diff range and nitpick comments (2)
test/test-integration/src/test/java/com/alipay/sofa/rpc/test/triple/stream/ExtendServerResponse.java (1)
27-30
: Ensure proper documentation for the constructor.Consider adding Javadoc comments to the constructor to explain its parameters and purpose, enhancing code maintainability and readability.
remoting/remoting-triple/src/main/java/com/alipay/sofa/rpc/server/triple/GenericServiceImpl.java (1)
Line range hint
68-97
: Ensure thread safety and context management.The method modifies the thread's context class loader but may not properly restore it if an exception is thrown before the finally block. Consider using a try-finally block to ensure the original class loader is always restored.
+ finally { + Thread.currentThread().setContextClassLoader(oldClassLoader); + }
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (18)
- remoting/remoting-triple/src/main/java/com/alipay/sofa/rpc/message/triple/stream/ClientStreamObserverAdapter.java (1 hunks)
- remoting/remoting-triple/src/main/java/com/alipay/sofa/rpc/message/triple/stream/ResponseSerializeStreamHandler.java (1 hunks)
- remoting/remoting-triple/src/main/java/com/alipay/sofa/rpc/server/triple/GenericServiceImpl.java (5 hunks)
- remoting/remoting-triple/src/main/java/com/alipay/sofa/rpc/server/triple/TripleServer.java (5 hunks)
- remoting/remoting-triple/src/main/java/com/alipay/sofa/rpc/server/triple/UniqueIdInvoker.java (3 hunks)
- remoting/remoting-triple/src/main/java/com/alipay/sofa/rpc/transport/triple/TripleClientInvoker.java (10 hunks)
- remoting/remoting-triple/src/main/java/com/alipay/sofa/rpc/utils/SofaProtoUtils.java (2 hunks)
- remoting/remoting-triple/src/test/java/com/alipay/sofa/rpc/server/triple/GenericServiceImplTest.java (1 hunks)
- test/test-integration/src/main/proto/helloworld.proto (1 hunks)
- test/test-integration/src/test/java/com/alipay/sofa/rpc/test/triple/GreeterImpl.java (2 hunks)
- test/test-integration/src/test/java/com/alipay/sofa/rpc/test/triple/stream/ClientRequest.java (1 hunks)
- test/test-integration/src/test/java/com/alipay/sofa/rpc/test/triple/stream/ExtendClientRequest.java (1 hunks)
- test/test-integration/src/test/java/com/alipay/sofa/rpc/test/triple/stream/ExtendServerResponse.java (1 hunks)
- test/test-integration/src/test/java/com/alipay/sofa/rpc/test/triple/stream/HelloService.java (1 hunks)
- test/test-integration/src/test/java/com/alipay/sofa/rpc/test/triple/stream/HelloServiceImpl.java (1 hunks)
- test/test-integration/src/test/java/com/alipay/sofa/rpc/test/triple/stream/ServerResponse.java (1 hunks)
- test/test-integration/src/test/java/com/alipay/sofa/rpc/test/triple/stream/TripleGenericStreamTest.java (1 hunks)
- test/test-integration/src/test/java/com/alipay/sofa/rpc/test/triple/stream/TripleStubStreamTest.java (1 hunks)
Files skipped from review due to trivial changes (1)
- test/test-integration/src/test/java/com/alipay/sofa/rpc/test/triple/stream/ServerResponse.java
Files skipped from review as they are similar to previous changes (9)
- remoting/remoting-triple/src/main/java/com/alipay/sofa/rpc/server/triple/TripleServer.java
- remoting/remoting-triple/src/main/java/com/alipay/sofa/rpc/server/triple/UniqueIdInvoker.java
- remoting/remoting-triple/src/main/java/com/alipay/sofa/rpc/utils/SofaProtoUtils.java
- remoting/remoting-triple/src/test/java/com/alipay/sofa/rpc/server/triple/GenericServiceImplTest.java
- test/test-integration/src/test/java/com/alipay/sofa/rpc/test/triple/stream/ClientRequest.java
- test/test-integration/src/test/java/com/alipay/sofa/rpc/test/triple/stream/ExtendClientRequest.java
- test/test-integration/src/test/java/com/alipay/sofa/rpc/test/triple/stream/HelloService.java
- test/test-integration/src/test/java/com/alipay/sofa/rpc/test/triple/stream/HelloServiceImpl.java
- test/test-integration/src/test/java/com/alipay/sofa/rpc/test/triple/stream/TripleGenericStreamTest.java
Additional comments not posted (15)
test/test-integration/src/test/java/com/alipay/sofa/rpc/test/triple/stream/ExtendServerResponse.java (2)
32-34
: LGTM! The getter method forextendString
is implemented correctly.
36-38
: LGTM! The setter method forextendString
is implemented correctly.test/test-integration/src/main/proto/helloworld.proto (4)
28-28
: LGTM! The unary RPC methodSayHello
is defined correctly.
29-29
: LGTM! The bidirectional streaming RPC methodSayHelloBinary
is defined correctly.
31-31
: LGTM! The client streaming RPC methodSayHelloClientStream
is defined correctly.
33-33
: LGTM! The server streaming RPC methodSayHelloServerStream
is defined correctly.remoting/remoting-triple/src/main/java/com/alipay/sofa/rpc/message/triple/stream/ResponseSerializeStreamHandler.java (2)
58-60
: LGTM! TheonFinish
method correctly completes the stream.
63-65
: LGTM! TheonException
method correctly handles exceptions by converting them and passing toonError
.remoting/remoting-triple/src/main/java/com/alipay/sofa/rpc/message/triple/stream/ClientStreamObserverAdapter.java (2)
62-65
: LGTM! TheonError
method correctly forwards the error to theStreamHandler
.
67-70
: LGTM! TheonCompleted
method correctly signals the completion of the stream.test/test-integration/src/test/java/com/alipay/sofa/rpc/test/triple/GreeterImpl.java (1)
116-123
: LGTM! The server streaming methodsayHelloServerStream
is implemented correctly.test/test-integration/src/test/java/com/alipay/sofa/rpc/test/triple/stream/TripleStubStreamTest.java (1)
156-167
: Verify correct message ordering in server streaming.The test checks messages received from server streaming but does not verify the order of messages, which is crucial for certain applications. Consider adding checks to ensure that messages are received in the correct order.
remoting/remoting-triple/src/main/java/com/alipay/sofa/rpc/transport/triple/TripleClientInvoker.java (3)
154-154
: Handle unknown streaming call types more gracefully.This change logs a warning and returns a default response instead of throwing an exception, which might be more graceful in a production environment.
187-221
: Implementation of bidirectional streaming looks robust.This method correctly sets up and handles bidirectional streaming, aligning with the PR's objectives for enhanced streaming capabilities.
475-477
: Refactor to simplify the mapping of call types.This refactoring uses
Optional
to simplify the control flow, making the code cleaner and easier to read.
.../remoting-triple/src/main/java/com/alipay/sofa/rpc/transport/triple/TripleClientInvoker.java
Show resolved
Hide resolved
.../src/main/java/com/alipay/sofa/rpc/message/triple/stream/ResponseSerializeStreamHandler.java
Outdated
Show resolved
Hide resolved
.../src/main/java/com/alipay/sofa/rpc/message/triple/stream/ResponseSerializeStreamHandler.java
Outdated
Show resolved
Hide resolved
...ple/src/main/java/com/alipay/sofa/rpc/message/triple/stream/ClientStreamObserverAdapter.java
Outdated
Show resolved
Hide resolved
...ple/src/main/java/com/alipay/sofa/rpc/message/triple/stream/ClientStreamObserverAdapter.java
Outdated
Show resolved
Hide resolved
test/test-integration/src/test/java/com/alipay/sofa/rpc/test/triple/GreeterImpl.java
Show resolved
Hide resolved
...t-integration/src/test/java/com/alipay/sofa/rpc/test/triple/stream/TripleStubStreamTest.java
Outdated
Show resolved
Hide resolved
...t-integration/src/test/java/com/alipay/sofa/rpc/test/triple/stream/TripleStubStreamTest.java
Show resolved
Hide resolved
...ting/remoting-triple/src/main/java/com/alipay/sofa/rpc/server/triple/GenericServiceImpl.java
Outdated
Show resolved
Hide resolved
...ting/remoting-triple/src/main/java/com/alipay/sofa/rpc/server/triple/GenericServiceImpl.java
Outdated
Show resolved
Hide resolved
3d00d9d
to
9a7ab52
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 8
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (19)
- remoting/remoting-triple/src/main/java/com/alipay/sofa/rpc/message/triple/stream/ClientStreamObserverAdapter.java (1 hunks)
- remoting/remoting-triple/src/main/java/com/alipay/sofa/rpc/message/triple/stream/ResponseSerializeStreamHandler.java (1 hunks)
- remoting/remoting-triple/src/main/java/com/alipay/sofa/rpc/server/triple/GenericServiceImpl.java (5 hunks)
- remoting/remoting-triple/src/main/java/com/alipay/sofa/rpc/server/triple/TripleServer.java (5 hunks)
- remoting/remoting-triple/src/main/java/com/alipay/sofa/rpc/server/triple/UniqueIdInvoker.java (3 hunks)
- remoting/remoting-triple/src/main/java/com/alipay/sofa/rpc/transport/triple/TripleClientInvoker.java (9 hunks)
- remoting/remoting-triple/src/main/java/com/alipay/sofa/rpc/utils/SofaProtoUtils.java (2 hunks)
- remoting/remoting-triple/src/test/java/com/alipay/sofa/rpc/server/triple/GenericServiceImplTest.java (2 hunks)
- test/test-integration/src/main/proto/helloworld.proto (1 hunks)
- test/test-integration/src/test/java/com/alipay/sofa/rpc/test/triple/GreeterImpl.java (2 hunks)
- test/test-integration/src/test/java/com/alipay/sofa/rpc/test/triple/NativeGrpcGreeterImpl.java (1 hunks)
- test/test-integration/src/test/java/com/alipay/sofa/rpc/test/triple/stream/ClientRequest.java (1 hunks)
- test/test-integration/src/test/java/com/alipay/sofa/rpc/test/triple/stream/ExtendClientRequest.java (1 hunks)
- test/test-integration/src/test/java/com/alipay/sofa/rpc/test/triple/stream/ExtendServerResponse.java (1 hunks)
- test/test-integration/src/test/java/com/alipay/sofa/rpc/test/triple/stream/HelloService.java (1 hunks)
- test/test-integration/src/test/java/com/alipay/sofa/rpc/test/triple/stream/HelloServiceImpl.java (1 hunks)
- test/test-integration/src/test/java/com/alipay/sofa/rpc/test/triple/stream/ServerResponse.java (1 hunks)
- test/test-integration/src/test/java/com/alipay/sofa/rpc/test/triple/stream/TripleGenericStreamTest.java (1 hunks)
- test/test-integration/src/test/java/com/alipay/sofa/rpc/test/triple/stream/TripleStubStreamTest.java (1 hunks)
Files skipped from review as they are similar to previous changes (14)
- remoting/remoting-triple/src/main/java/com/alipay/sofa/rpc/message/triple/stream/ClientStreamObserverAdapter.java
- remoting/remoting-triple/src/main/java/com/alipay/sofa/rpc/message/triple/stream/ResponseSerializeStreamHandler.java
- remoting/remoting-triple/src/main/java/com/alipay/sofa/rpc/server/triple/TripleServer.java
- remoting/remoting-triple/src/main/java/com/alipay/sofa/rpc/server/triple/UniqueIdInvoker.java
- remoting/remoting-triple/src/test/java/com/alipay/sofa/rpc/server/triple/GenericServiceImplTest.java
- test/test-integration/src/main/proto/helloworld.proto
- test/test-integration/src/test/java/com/alipay/sofa/rpc/test/triple/GreeterImpl.java
- test/test-integration/src/test/java/com/alipay/sofa/rpc/test/triple/stream/ClientRequest.java
- test/test-integration/src/test/java/com/alipay/sofa/rpc/test/triple/stream/ExtendClientRequest.java
- test/test-integration/src/test/java/com/alipay/sofa/rpc/test/triple/stream/ExtendServerResponse.java
- test/test-integration/src/test/java/com/alipay/sofa/rpc/test/triple/stream/HelloService.java
- test/test-integration/src/test/java/com/alipay/sofa/rpc/test/triple/stream/HelloServiceImpl.java
- test/test-integration/src/test/java/com/alipay/sofa/rpc/test/triple/stream/ServerResponse.java
- test/test-integration/src/test/java/com/alipay/sofa/rpc/test/triple/stream/TripleGenericStreamTest.java
Additional comments not posted (14)
test/test-integration/src/test/java/com/alipay/sofa/rpc/test/triple/NativeGrpcGreeterImpl.java (2)
73-76
: Potential duplicate responses per request.Verify if sending two responses per request in
sayHelloBinary
is intended. If not, consider sending only one response or adjusting the logic accordingly.
119-126
: Implementation of server streaming looks correct.The method
sayHelloServerStream
correctly implements the server streaming pattern by sending multiple responses.remoting/remoting-triple/src/main/java/com/alipay/sofa/rpc/utils/SofaProtoUtils.java (3)
82-94
: Proper handling of call type mapping.The method
mapGrpcCallType
correctly maps custom call types to gRPC method types and handles unsupported types with an exception.
97-107
: Efficient caching of method stream types.The method
cacheStreamCallType
efficiently determines and caches stream call types for methods, ensuring that only valid types are stored.
115-135
: Correct identification of stream call types.The method
mapStreamType
accurately identifies different streaming call types based on method signatures and handles errors appropriately.remoting/remoting-triple/src/main/java/com/alipay/sofa/rpc/server/triple/GenericServiceImpl.java (3)
68-73
: Proper handling of method invocation for unary calls.The method
generic
correctly handles unary call invocations, including class loader management and exception handling.
98-167
: Efficient handling of bidirectional streaming.The method
genericBiStream
efficiently handles bidirectional streaming calls with proper synchronization for initialization and robust error management.
169-196
: Correct implementation of server streaming.The method
genericServerStream
correctly implements server streaming functionality, including parameter setup, error handling, and class loader management.test/test-integration/src/test/java/com/alipay/sofa/rpc/test/triple/stream/TripleStubStreamTest.java (5)
124-150
: Potential mismatch in expected response count.Verify if the expected count of responses in
testTripleStubBiStream
matches the actual responses sent by the server. Adjust theCountDownLatch
or the test logic as necessary to prevent test hangs.
155-182
: Verify response message format in client streaming test.Ensure that the response message in
testTripleStubClientStream
correctly reflects the number of requests received. Adjust the test or server logic if there's a mismatch.
185-195
: Correct testing of server streaming functionality.The method
testTripleStubServerStream
correctly tests the server streaming functionality with appropriate synchronization and response validation.
199-289
: Comprehensive testing of protocol interactions.The method
testGrpcClientToTripleServer
comprehensively tests interactions between a gRPC client and a Triple server across various streaming scenarios, ensuring correct behavior and synchronization.
291-361
: Effective testing of client-server interactions across protocols.The method
testTripleClientToGrpcServer
effectively tests interactions between a Triple client and a gRPC server, covering various streaming types and ensuring synchronization and correct behavior.remoting/remoting-triple/src/main/java/com/alipay/sofa/rpc/transport/triple/TripleClientInvoker.java (1)
449-465
: ThegetMethodDescriptor
method constructs aMethodDescriptor
for gRPC calls. It's crucial to ensure that the method correctly handles all types of methods, especially new streaming types. Also, consider enhancing the method to handle more complex scenarios as streaming evolves.
.../remoting-triple/src/main/java/com/alipay/sofa/rpc/transport/triple/TripleClientInvoker.java
Show resolved
Hide resolved
.../remoting-triple/src/main/java/com/alipay/sofa/rpc/transport/triple/TripleClientInvoker.java
Show resolved
Hide resolved
.../remoting-triple/src/main/java/com/alipay/sofa/rpc/transport/triple/TripleClientInvoker.java
Outdated
Show resolved
Hide resolved
test/test-integration/src/test/java/com/alipay/sofa/rpc/test/triple/NativeGrpcGreeterImpl.java
Show resolved
Hide resolved
test/test-integration/src/test/java/com/alipay/sofa/rpc/test/triple/NativeGrpcGreeterImpl.java
Show resolved
Hide resolved
test/test-integration/src/test/java/com/alipay/sofa/rpc/test/triple/NativeGrpcGreeterImpl.java
Show resolved
Hide resolved
test/test-integration/src/test/java/com/alipay/sofa/rpc/test/triple/NativeGrpcGreeterImpl.java
Show resolved
Hide resolved
9a7ab52
to
d222c55
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (19)
- remoting/remoting-triple/src/main/java/com/alipay/sofa/rpc/message/triple/stream/ClientStreamObserverAdapter.java (1 hunks)
- remoting/remoting-triple/src/main/java/com/alipay/sofa/rpc/message/triple/stream/ResponseSerializeStreamHandler.java (1 hunks)
- remoting/remoting-triple/src/main/java/com/alipay/sofa/rpc/server/triple/GenericServiceImpl.java (5 hunks)
- remoting/remoting-triple/src/main/java/com/alipay/sofa/rpc/server/triple/TripleServer.java (5 hunks)
- remoting/remoting-triple/src/main/java/com/alipay/sofa/rpc/server/triple/UniqueIdInvoker.java (3 hunks)
- remoting/remoting-triple/src/main/java/com/alipay/sofa/rpc/transport/triple/TripleClientInvoker.java (9 hunks)
- remoting/remoting-triple/src/main/java/com/alipay/sofa/rpc/utils/SofaProtoUtils.java (2 hunks)
- remoting/remoting-triple/src/test/java/com/alipay/sofa/rpc/server/triple/GenericServiceImplTest.java (2 hunks)
- test/test-integration/src/main/proto/helloworld.proto (1 hunks)
- test/test-integration/src/test/java/com/alipay/sofa/rpc/test/triple/GreeterImpl.java (2 hunks)
- test/test-integration/src/test/java/com/alipay/sofa/rpc/test/triple/NativeGrpcGreeterImpl.java (1 hunks)
- test/test-integration/src/test/java/com/alipay/sofa/rpc/test/triple/stream/ClientRequest.java (1 hunks)
- test/test-integration/src/test/java/com/alipay/sofa/rpc/test/triple/stream/ExtendClientRequest.java (1 hunks)
- test/test-integration/src/test/java/com/alipay/sofa/rpc/test/triple/stream/ExtendServerResponse.java (1 hunks)
- test/test-integration/src/test/java/com/alipay/sofa/rpc/test/triple/stream/HelloService.java (1 hunks)
- test/test-integration/src/test/java/com/alipay/sofa/rpc/test/triple/stream/HelloServiceImpl.java (1 hunks)
- test/test-integration/src/test/java/com/alipay/sofa/rpc/test/triple/stream/ServerResponse.java (1 hunks)
- test/test-integration/src/test/java/com/alipay/sofa/rpc/test/triple/stream/TripleGenericStreamTest.java (1 hunks)
- test/test-integration/src/test/java/com/alipay/sofa/rpc/test/triple/stream/TripleStubStreamTest.java (1 hunks)
Files skipped from review as they are similar to previous changes (16)
- remoting/remoting-triple/src/main/java/com/alipay/sofa/rpc/message/triple/stream/ClientStreamObserverAdapter.java
- remoting/remoting-triple/src/main/java/com/alipay/sofa/rpc/message/triple/stream/ResponseSerializeStreamHandler.java
- remoting/remoting-triple/src/main/java/com/alipay/sofa/rpc/server/triple/TripleServer.java
- remoting/remoting-triple/src/main/java/com/alipay/sofa/rpc/server/triple/UniqueIdInvoker.java
- remoting/remoting-triple/src/main/java/com/alipay/sofa/rpc/utils/SofaProtoUtils.java
- remoting/remoting-triple/src/test/java/com/alipay/sofa/rpc/server/triple/GenericServiceImplTest.java
- test/test-integration/src/main/proto/helloworld.proto
- test/test-integration/src/test/java/com/alipay/sofa/rpc/test/triple/NativeGrpcGreeterImpl.java
- test/test-integration/src/test/java/com/alipay/sofa/rpc/test/triple/stream/ClientRequest.java
- test/test-integration/src/test/java/com/alipay/sofa/rpc/test/triple/stream/ExtendClientRequest.java
- test/test-integration/src/test/java/com/alipay/sofa/rpc/test/triple/stream/ExtendServerResponse.java
- test/test-integration/src/test/java/com/alipay/sofa/rpc/test/triple/stream/HelloService.java
- test/test-integration/src/test/java/com/alipay/sofa/rpc/test/triple/stream/HelloServiceImpl.java
- test/test-integration/src/test/java/com/alipay/sofa/rpc/test/triple/stream/ServerResponse.java
- test/test-integration/src/test/java/com/alipay/sofa/rpc/test/triple/stream/TripleGenericStreamTest.java
- test/test-integration/src/test/java/com/alipay/sofa/rpc/test/triple/stream/TripleStubStreamTest.java
Additional comments not posted (8)
test/test-integration/src/test/java/com/alipay/sofa/rpc/test/triple/GreeterImpl.java (1)
117-123
: Implementation of server streaming looks good.remoting/remoting-triple/src/main/java/com/alipay/sofa/rpc/server/triple/GenericServiceImpl.java (3)
68-73
: Proper handling of class loader and error management in unary call handling.
98-167
: Well-implemented bidirectional streaming with robust error handling and resource management.
169-196
: Server streaming implementation is robust with appropriate error handling and class loader management.remoting/remoting-triple/src/main/java/com/alipay/sofa/rpc/transport/triple/TripleClientInvoker.java (4)
142-156
: Effective management of different streaming call types with appropriate error handling.
168-185
: Proper handling of unary gRPC calls with correct serialization and response processing.
187-221
: Bidirectional streaming setup is robust with proper message handling and exception management.
227-240
: Server streaming call handling is correctly implemented with appropriate setup and response management.
test/test-integration/src/test/java/com/alipay/sofa/rpc/test/triple/GreeterImpl.java
Show resolved
Hide resolved
test/test-integration/src/test/java/com/alipay/sofa/rpc/test/triple/GreeterImpl.java
Show resolved
Hide resolved
test/test-integration/src/test/java/com/alipay/sofa/rpc/test/triple/GreeterImpl.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
d222c55
to
c7435df
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 12
Out of diff range and nitpick comments (3)
core/api/src/main/java/com/alipay/sofa/rpc/transport/SofaStreamObserver.java (2)
47-47
: Ensure proper documentation foronError
method.The documentation for the
onError
method could be improved by explaining the expected behavior in more detail, especially regarding the state of the stream after an error occurs.
47-47
: Consider enhancing the documentation foronError
.It would be beneficial to include more details about how errors should be handled and the state of the stream after an error is reported.
remoting/remoting-triple/src/main/java/com/alipay/sofa/rpc/server/triple/GenericServiceImpl.java (1)
Line range hint
73-86
: Improve exception handling ingeneric
method.The method should catch specific exceptions such as
SerializationException
and handle them appropriately, instead of throwing a genericSofaRpcRuntimeException
.+ } catch (SerializationException se) { + LOGGER.error("Serialization error", se); + throw new SofaRpcException("Error during serialization", se); + }
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (20)
- core/api/src/main/java/com/alipay/sofa/rpc/transport/SofaStreamObserver.java (1 hunks)
- remoting/remoting-triple/src/main/java/com/alipay/sofa/rpc/message/triple/stream/ClientStreamObserverAdapter.java (1 hunks)
- remoting/remoting-triple/src/main/java/com/alipay/sofa/rpc/message/triple/stream/ResponseSerializeSofaStreamObserver.java (1 hunks)
- remoting/remoting-triple/src/main/java/com/alipay/sofa/rpc/server/triple/GenericServiceImpl.java (5 hunks)
- remoting/remoting-triple/src/main/java/com/alipay/sofa/rpc/server/triple/TripleServer.java (5 hunks)
- remoting/remoting-triple/src/main/java/com/alipay/sofa/rpc/server/triple/UniqueIdInvoker.java (3 hunks)
- remoting/remoting-triple/src/main/java/com/alipay/sofa/rpc/transport/triple/TripleClientInvoker.java (9 hunks)
- remoting/remoting-triple/src/main/java/com/alipay/sofa/rpc/utils/SofaProtoUtils.java (2 hunks)
- remoting/remoting-triple/src/test/java/com/alipay/sofa/rpc/server/triple/GenericServiceImplTest.java (2 hunks)
- test/test-integration/src/main/proto/helloworld.proto (1 hunks)
- test/test-integration/src/test/java/com/alipay/sofa/rpc/test/triple/GreeterImpl.java (2 hunks)
- test/test-integration/src/test/java/com/alipay/sofa/rpc/test/triple/NativeGrpcGreeterImpl.java (1 hunks)
- test/test-integration/src/test/java/com/alipay/sofa/rpc/test/triple/stream/ClientRequest.java (1 hunks)
- test/test-integration/src/test/java/com/alipay/sofa/rpc/test/triple/stream/ExtendClientRequest.java (1 hunks)
- test/test-integration/src/test/java/com/alipay/sofa/rpc/test/triple/stream/ExtendServerResponse.java (1 hunks)
- test/test-integration/src/test/java/com/alipay/sofa/rpc/test/triple/stream/HelloService.java (1 hunks)
- test/test-integration/src/test/java/com/alipay/sofa/rpc/test/triple/stream/HelloServiceImpl.java (1 hunks)
- test/test-integration/src/test/java/com/alipay/sofa/rpc/test/triple/stream/ServerResponse.java (1 hunks)
- test/test-integration/src/test/java/com/alipay/sofa/rpc/test/triple/stream/TripleGenericStreamTest.java (1 hunks)
- test/test-integration/src/test/java/com/alipay/sofa/rpc/test/triple/stream/TripleStubStreamTest.java (1 hunks)
Files skipped from review as they are similar to previous changes (16)
- remoting/remoting-triple/src/main/java/com/alipay/sofa/rpc/message/triple/stream/ClientStreamObserverAdapter.java
- remoting/remoting-triple/src/main/java/com/alipay/sofa/rpc/server/triple/TripleServer.java
- remoting/remoting-triple/src/main/java/com/alipay/sofa/rpc/server/triple/UniqueIdInvoker.java
- remoting/remoting-triple/src/main/java/com/alipay/sofa/rpc/utils/SofaProtoUtils.java
- remoting/remoting-triple/src/test/java/com/alipay/sofa/rpc/server/triple/GenericServiceImplTest.java
- test/test-integration/src/main/proto/helloworld.proto
- test/test-integration/src/test/java/com/alipay/sofa/rpc/test/triple/GreeterImpl.java
- test/test-integration/src/test/java/com/alipay/sofa/rpc/test/triple/NativeGrpcGreeterImpl.java
- test/test-integration/src/test/java/com/alipay/sofa/rpc/test/triple/stream/ClientRequest.java
- test/test-integration/src/test/java/com/alipay/sofa/rpc/test/triple/stream/ExtendClientRequest.java
- test/test-integration/src/test/java/com/alipay/sofa/rpc/test/triple/stream/ExtendServerResponse.java
- test/test-integration/src/test/java/com/alipay/sofa/rpc/test/triple/stream/HelloService.java
- test/test-integration/src/test/java/com/alipay/sofa/rpc/test/triple/stream/HelloServiceImpl.java
- test/test-integration/src/test/java/com/alipay/sofa/rpc/test/triple/stream/ServerResponse.java
- test/test-integration/src/test/java/com/alipay/sofa/rpc/test/triple/stream/TripleGenericStreamTest.java
- test/test-integration/src/test/java/com/alipay/sofa/rpc/test/triple/stream/TripleStubStreamTest.java
Additional comments not posted (8)
core/api/src/main/java/com/alipay/sofa/rpc/transport/SofaStreamObserver.java (2)
28-28
: MethodonNext
implementation looks good.
36-36
: MethodonCompleted
implementation is correct.remoting/remoting-triple/src/main/java/com/alipay/sofa/rpc/message/triple/stream/ResponseSerializeSofaStreamObserver.java (2)
59-59
: MethodonCompleted
correctly delegates to the underlying observer.
63-65
: Verify the implementation ofTripleExceptionUtils.asStatusRuntimeException
.Verification successful
The implementation of
TripleExceptionUtils.asStatusRuntimeException
correctly handles both null and non-null exceptions, converting them intoStatusRuntimeException
with appropriate descriptions and causes. This method is well-suited for its usage in theonError
method ofResponseSerializeSofaStreamObserver
, ensuring that all types of exceptions are properly formatted for gRPC communication.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the implementation of `TripleExceptionUtils.asStatusRuntimeException` to ensure it handles all exception types correctly. # Test: Search for the method implementation. Expect: Proper exception handling logic. rg --type java 'asStatusRuntimeException' -- $(fd TripleExceptionUtils.java)Length of output: 248
Script:
#!/bin/bash # Description: Fetch the complete implementation of `asStatusRuntimeException` to verify its exception handling capabilities. # Using ast-grep to retrieve the method implementation ast-grep --lang java --pattern $'public static StatusRuntimeException asStatusRuntimeException(Throwable $_) { $$$ }'Length of output: 1150
remoting/remoting-triple/src/main/java/com/alipay/sofa/rpc/server/triple/GenericServiceImpl.java (3)
170-197
: MethodgenericServerStream
is implemented correctly.
234-234
: MethodgetAppResponse
is implemented correctly.
260-273
: MethodgetArgTypes
is implemented correctly.remoting/remoting-triple/src/main/java/com/alipay/sofa/rpc/transport/triple/TripleClientInvoker.java (1)
157-165
: MethodstubCall
is implemented correctly.
Motivation:
This PR can provides stream transport support for triple protocol, including server streaming and bidirectional/client streaming.
Modification:
Change log (Chinese) :
概述
Stream 方式是一种异步的流式处理方式,可以在数据传输过程中逐个处理数据,避免一次性传输大量数据造成的性能问题。服务端Stream 是指服务端在处理请求时,将数据分成多个部分逐个返回给客户端的过程;客户端 Stream 是指客户端在请求服务器时,将请求参数分成多个部分逐个发送给服务器的过程。Stream 方式可以让我们在处理大量数据时更高效地使用资源,提高系统的性能和响应速度。
业务场景
流式传输适配下列业务场景:
实现的流传输应具有以下语义保证:
流传输模型
对于流式传输,我们可以将Stream分为四种类型 (借鉴于gRPC):
Unary,传统请求-响应模型:客户端一次请求,服务端一次响应
ClientStream,客户端流:客户端多次请求,服务端最终返回一次响应,服务端接收请求期间可以返回已完成/取消信号来停止客户端继续发送请求,或者在客户端请求完成后返回响应。
ServerStream,服务端流:客户端单次请求,服务端返回多次响应,客户端接收请求期间可以返回已完成/取消信号来停止接受服务端的响应,或者等待服务端发送完成相应信号。
BidirectionalStream,双向流:客户端发送一次请求后,客户端-服务端均可相互多次乱序发送请求,直到任意一方发送已完成/取消信号。
流传输接口定义
我们先从RPC调用的最上层开始分析。首先,我们需要定义客户端/服务端用于接收/发送流式请求的接口,该接口定义了如何发送、接收、处理数据流,以及数据流的异常、结束操作。
考虑到目前传输层的实现,此处可以考虑直接使用 gRPC 中的 StreamObserver,这样对于 Triple 协议的传输层实现来说比较方便(TripleClientInvoker 直接依赖 gRPC 进行传输),但缺点是可能导致其它未来可能支持流传输的协议同时耦合了gRPC的API。
更好的方案是使用自定义的 StreamObserver(此处先叫做StreamHandler),将其和gRPC解耦,在 Triple 传输层通过适配器转换为gRPC StreamObserver。此处使用自定义的StreamHandler。
我们为该接口定义以下方法,实际和gRPC中的StreamObserver一致:
全局处理
**1,标记请求类型 **
首先是识别方法调用类型,并为 SofaRequest 打上流传输的标记。
对于客户端来说,SofaRequest在客户端代理中创建。其中标记传输类型的字段在DefaultClientProxyInvoker中的 decorateRequest方法中设置(对于泛型调用)。
setInvokeType方法最终调用ConsumerConfig中的getMethodInvokeType,尝试从接口配置中获取已缓存的调用方式,否则使用默认值(一元调用)。可以将判断调用类型的操作添加在其中的 getMethodInvokeType 中,这样也可以将方法调用模式缓存,防止每次调用都要进行繁琐的判断操作。
修改后的方法:
主要完成以下三个操作:
此处将调用方式缓存到了MethodConfig中,因为InterfaceConfig使用的是UnmodifiableMap,在其初始化完成后已不能再添加新的信息。由于 MethodConfig 本身没有保存 Method 引用,且不会默认创建,使得获取方法参数及返回值的具体类型有些困难,需要在实际请求时通过 SofaRequest 拿到具体的参数 Class,才可较简单的判断方法参数及返回值中StreamHandler的出现情况,判断请求类型。
2,修改AbstractCluster.doSendMsg
在该方法中通过获取SofaRequest的RequestType,判断是使用同步、异步、回调还是其它调用方法。对于流式调用,需要在其中添加新请求类型的处理逻辑。
根据流式传输方法的定义,如果客户端希望通过Stream向服务端再次发送信息,它需要通过调用RPC方法返回的StreamHandler中的onMessage方法来向服务端发送下一条信息。
消费者对流传输方法的第一次调用实际是向提供者注册传输会话,且需要依赖返回的StreamHandler向提供者发送后续信息(且需要确保StreamHandler已初始化完毕,保证消费者直接使用其发送消息不会出错),此处选择同步调用。
协议处理
Triple协议
SofaRPC 的 Triple 协议传输层实现目前直接依赖 gRPC 进行。
泛型调用
当 TripleClientInvoker 中 useGeneric 字段为 false 时,表示消费者调用的服务存在IDL及对应的Stub,可以直接通过生成的Stub进行调用。当 useGeneric 为 true 时,将在运行时动态指定调用的服务及方法,借助底层的泛型服务完成传输。
因此,Triple协议的修改分为两个方面:非泛型调用时,通过服务接口生成的stub进行流式调用;以及泛型调用时,通过预生成的GenericService stub发起流式调用。
IDL修改
对于泛型调用,Triple协议通过将已序列化完成的请求统一封装为 Request,通过预生成的gRPC stub完成传输过程。
因此需要修改transformer.proto, 添加流式调用方法的定义。 考虑客户端流和双向流使用可能使用相同的调用方法,此处没有单独定义客户端流的传输方法。
之后,服务端实现新的泛型服务方法
以上实现的主要难点在于,流调用方法传入及返回的均为StreamHandler,这意味着在实际请求到来之前无法得知具体的请求类型。因此以上的实现中,序列化及方法参数信息是在接收到第一次请求之后才被初始化的。
Triple传输层修改
*由于实际上三种流传输的底层流程差别不大,以下的修改示例均为双向流(BidirectionalStream)。
客户端
以下为对TripleClientInvoker相关的修改。
重构invoke方法,根据callType区分为三种调用流程
然后,根据具体调用方式选择具体调用逻辑
实现双向流传输
为MethodDescriptor设置新的调用类型
添加工具方法:
服务端
方法绑定问题
对于服务端,泛型调用的入口是 GenericService 的实现,即 GenericSerivceImpl。
在为泛型服务的IDL中添加流式方法的定义并重新编译后,发现一元调用时(UNARY)不能正确的选择GenericServiceImple 的 generic 方法,而选择调用了 genericBiStream 方法。
断点调试可以发现,底层 ServerMethodDefinition 和 CallHandler 的绑定错误,使得方法选择了ID错误的CallHandler,从而调用了错误的方法。ServerMethodDefinition 在 ServerImpl.runInternal() 中通过 registry.lookupMethod(methodName)获取。实际调用的是 MutableHandlerRegistry,通过其中的 services 获取 ServerServiceDefinition。而 service 中的 ServerServiceDefinition 由 TripleServer.registerProcessor 放入,自此回到了 SofaRPC 自己的实现。
**分析数据流,可以发现将客户端实际服务方法和泛型服务方法绑定起来的操作发生在TripleServer.buildSofaServiceDef方法中。**该流程实际在服务提供者启动,导出服务接口时进行。
修改后的实现:
这样 ServerMethodDefinition 就和 callHandler 正确的绑定起来了。
方法定位问题
以上方法传入的 ProviderConfig 提供了实际服务接口的配置信息,包括这些方法的调用方式。但是调用方式默认被设置为UNARY。在上层需要附加一部分根据接口方法参数判断调用类型的逻辑。
修改后:
在接口配置(AbstractInterfaceConfig)中提供了按Class匹配并缓存方法调用模式的方法:
*写在 AbstractInterfaceConfig 是考虑消费者是否能复用这部分逻辑
在 ProviderConfig 初始化时,设置服务引用时会尝试匹配服务方法的具体调用方式,并缓存:
流信息序列化问题
对于双向流和客户端流,接口声明时的入参和返回值均仅为 StreamHandler。这导致无论是客户端请求和服务端响应都无法在调用开始就得知具体的消息类型。因此获取、缓存具体消息类型、序列化器的操作需要延迟在客户端/服务端第一次得到具体请求时再进行。
客户端向服务端发送消息使用的StreamHandler适配器,TripleClientInvoker中的匿名实现,将请求序列化为Request:
客户端处理服务端响应使用的StreamHandler适配器,将Response反序列化为指定类型:
服务端发送响应时使用的StreamHandler适配器,将原始响应序列化为Response:
服务端处理客户端请求使用的StreamHandler,TripleClientInvoker 中的匿名实现,将请求反序列化为指定类型:
Summary by CodeRabbit
New Features
GenericService
.Bug Fixes
Documentation
Refactor
Tests
Chores