feat(ITcpSocketClient): add TouchSocket extensions #6268
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
Reviewer's GuideThis PR introduces a string-based SendAsync extension and a raw data ReceivedCallBack API for ITcpSocketClient and data handlers, updates sample docs to demonstrate the new callback and handler options, and refreshes unit tests (including a new TouchSocketTest suite) to validate these enhancements. Sequence diagram for ITcpSocketClient.SendAsync(string) extension methodsequenceDiagram
participant User as actor User
participant Client as ITcpSocketClient
participant Extensions as ITcpSocketClientExtensions
User->>Extensions: SendAsync(client, content, encoding, token)
Extensions->>Extensions: Convert content to byte[]
Extensions->>Client: SendAsync(buffer, token)
Client-->>Extensions: ValueTask<bool>
Extensions-->>User: ValueTask<bool>
Sequence diagram for ReceivedCallBack invocation on data receptionsequenceDiagram
participant Socket as DefaultTcpSocketClient
participant Callback as ReceivedCallBack
participant Handler as DataPackageHandlerBase
Socket->>Socket: ReceiveAsync()
Socket->>Callback: ReceivedCallBack(buffer)
Callback-->>Socket: ValueTask
Socket->>Handler: _dataPackageHandler.ReceiveAsync(buffer)
Handler-->>Socket: ValueTask
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey @ArgoZhang - I've reviewed your changes - here's some feedback:
- The tests use invalid C# array literals like
[1, 2, 3]; please replace them with propernew byte[] { 1, 2, 3 }(or equivalent) so they compile. - The
ReceivedCallBackproperty name is inconsistent with .NET naming conventions (should beReceivedCallback); please rename it across interface, implementation, and docs for clarity. - The
ITcpSocketClientExtensionsclass lives under theComponentsnamespace, but it extendsITcpSocketClientinServices.TcpSocket; consider moving it to the same namespace or assembly to improve discoverability.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The tests use invalid C# array literals like `[1, 2, 3]`; please replace them with proper `new byte[] { 1, 2, 3 }` (or equivalent) so they compile.
- The `ReceivedCallBack` property name is inconsistent with .NET naming conventions (should be `ReceivedCallback`); please rename it across interface, implementation, and docs for clarity.
- The `ITcpSocketClientExtensions` class lives under the `Components` namespace, but it extends `ITcpSocketClient` in `Services.TcpSocket`; consider moving it to the same namespace or assembly to improve discoverability.
## Individual Comments
### Comment 1
<location> `src/BootstrapBlazor/Services/TcpSocket/DefaultTcpSocketClient.cs:141` </location>
<code_context>
{
buffer = buffer[..len];
+ if (ReceivedCallBack != null)
+ {
+ await ReceivedCallBack(buffer);
+ }
+
</code_context>
<issue_to_address>
Consider exception handling for ReceivedCallBack invocation.
If ReceivedCallBack throws, it may break the receive loop or cause side effects. Wrap the invocation in try-catch and handle or log exceptions to improve robustness.
</issue_to_address>
<suggested_fix>
<<<<<<< SEARCH
if (ReceivedCallBack != null)
{
await ReceivedCallBack(buffer);
}
=======
if (ReceivedCallBack != null)
{
try
{
await ReceivedCallBack(buffer);
}
catch (Exception ex)
{
// TODO: Replace with your logging framework or error handling as appropriate
Console.Error.WriteLine($"Exception in ReceivedCallBack: {ex}");
}
}
>>>>>>> REPLACE
</suggested_fix>
### Comment 2
<location> `src/BootstrapBlazor/Services/TcpSocket/DefaultTcpSocketClient.cs:31` </location>
<code_context>
+ /// <remarks>The callback function should be designed to handle the received data efficiently and
+ /// asynchronously. Ensure that the implementation does not block or perform long-running operations, as this may
+ /// impact performance.</remarks>
public Func<ReadOnlyMemory<byte>, ValueTask>? ReceivedCallBack { get; set; }
/// <summary>
</code_context>
<issue_to_address>
Potential naming inconsistency: 'ReceivedCallBack' vs. 'ReceivedCallback'.
Consider renaming to 'ReceivedCallback' to align with standard C# naming conventions.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
ArgoZhang
changed the title
13 tasks
将 `Close` 方法替换为异步的 `CloseAsync` 方法，以支持异步关闭操作并释放资源。`CloseAsync` 方法接受一个消息和一个可选的取消令牌作为参数，并返回一个 `ValueTask<bool>`，指示关闭操作的结果。原有的 `Close` 方法被移除，相关的资源释放逻辑被整合到新的异步方法中。
在 TcpSocketFactoryTest.cs 和 TouchSocketTest.cs 中，将连接关闭逻辑从 client.Close() 更改为 await client.CloseAsync(string.Empty)，以支持异步关闭连接并提高性能
# Conflicts: # src/BootstrapBlazor/Extensions/ITcpSocketClientExtensions.cs
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #6268 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 712 712
Lines 31397 31397
Branches 4437 4437
=========================================
Hits 31397 31397
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Co-Authored-By: 若汝棋茗 <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Link issues
fixes #6270
Summary By Copilot
Regression?
Risk
Verification
Packaging changes reviewed?
☑️ Self Check before Merge
Summary by Sourcery
Add a string-based SendAsync extension to ITcpSocketClient and introduce an asynchronous ReceivedCallBack mechanism on both the client and data handler layers, update DefaultTcpSocketClient implementation, refresh samples, and extend tests to cover the new functionality.
New Features:
Enhancements:
Documentation:
Tests: