feat(TcpSocket): add data type check prevent assign exception#548
Merged
feat(TcpSocket): add data type check prevent assign exception#548
Conversation
Reviewer's GuideEnhances the TCP socket data conversion pipeline by enforcing strict type checks before property assignments, introducing a reusable reflection helper for converter instantiation, and extending unit tests to validate the new behavior. Class diagram for new ActivatorExtensions and updated data converter usageclassDiagram
class ActivatorExtesnions {
+object? CreateInstance(Type type, object?[]? args = null)
+TType? CreateInstance<TType>(Type type, object?[]? args = null)
}
class DataPropertyExtensions {
+IDataPropertyConverter? GetPropertyConverter(PropertyInfo p)
}
class ITcpSocketClientExtensions {
+void SetDataPackageAdapter<TEntity>(ITcpSocketClient client, ...)
}
class DataConverter {
+bool Parse(ReadOnlyMemory<byte> data, TEntity entity)
}
DataPropertyExtensions --> ActivatorExtesnions : uses
ITcpSocketClientExtensions --> ActivatorExtesnions : uses
DataConverter --> DataPropertyExtensions : uses
IDataPropertyConverter <|.. DataPropertyExtensions
IDataConverter <|.. ITcpSocketClientExtensions
Flow diagram for property assignment with type check in DataConverterflowchart TD
A[Start Parse method] --> B[Get property converter attribute]
B --> C[Convert data to value]
C --> D{Is value type equal to property type?}
D -- Yes --> E[Assign value to property]
D -- No --> F[Skip assignment]
E --> G[Continue parsing]
F --> G[Continue parsing]
G --> H[Return result]
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Pull Request Overview
This PR adds data type validation to prevent assignment exceptions in TcpSocket data conversion. The main improvement is adding a type check before setting property values to ensure the converted data matches the expected property type.
- Introduced type validation in data conversion to prevent assignment exceptions
- Refactored object instantiation to use a new extension method for better error handling
- Updated version numbers for the affected packages
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/extensions/BootstrapBlazor.Socket/Extensions/ActivatorExtesnions.cs | New extension methods for safe object instantiation with type checking |
| src/extensions/BootstrapBlazor.Socket/DataConverter/DataConverter.cs | Added type validation before property assignment |
| src/extensions/BootstrapBlazor.Socket/Extensions/DataPropertyExtensions.cs | Refactored to use new CreateInstance extension method |
| src/extensions/BootstrapBlazor.TcpSocket/Extensions/ITcpSocketClientExtensions.cs | Refactored to use new CreateInstance extension method |
| test/UnitTestTcpSocket/TcpSocketFactoryTest.cs | Added test case for data conversion functionality |
| test/UnitTestTcpSocket/TcpSocketPropertyConverterTest.cs | Removed unnecessary type cast in assertion |
| src/extensions/BootstrapBlazor.Socket/BootstrapBlazor.Socket.csproj | Version bump to 9.0.3 |
| src/extensions/BootstrapBlazor.TcpSocket/BootstrapBlazor.TcpSocket.csproj | Version bump to 9.0.2 |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey there - I've reviewed your changes and they look great!
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `src/extensions/BootstrapBlazor.Socket/Extensions/ActivatorExtesnions.cs:12` </location>
<code_context>
+/// <summary>
+/// Activator 扩展方法
+/// </summary>
+public static class ActivatorExtesnions
+{
+ /// <summary>
</code_context>
<issue_to_address>
Typo in class name: 'ActivatorExtesnions' should be 'ActivatorExtensions'.
Please rename both the class and file to 'ActivatorExtensions' for consistency.
</issue_to_address>
<suggested_fix>
<<<<<<< SEARCH
public static class ActivatorExtesnions
=======
public static class ActivatorExtensions
>>>>>>> REPLACE
</suggested_fix>
### Comment 2
<location> `src/extensions/BootstrapBlazor.Socket/Extensions/ActivatorExtesnions.cs:22` </location>
<code_context>
+ /// <returns></returns>
+ public static object? CreateInstance(this Type type, object?[]? args = null)
+ {
+ var bindings = BindingFlags.NonPublic | BindingFlags.Public | BindingFlags.Instance | BindingFlags.Default;
+ return Activator.CreateInstance(type, bindings, null, args, null);
+ }
</code_context>
<issue_to_address>
BindingFlags.Default is redundant when combined with other flags.
BindingFlags.Default is zero and does not change the behavior when combined with other flags. Removing it will make the code clearer.
</issue_to_address>
### Comment 3
<location> `test/UnitTestTcpSocket/TcpSocketFactoryTest.cs:942` </location>
<code_context>
+ await tcs.Task;
+
+ // 验证实体类不为空
+ Assert.NotNull(entity);
+ Assert.Equal("3.14", entity.Value1.ToString("#.##"));
+
+ Task ReceivedCallBack(MockConverterEntity? data)
</code_context>
<issue_to_address>
Test assertions could be more comprehensive.
Also assert 'Header' and 'Body' properties to fully validate the conversion logic.
</issue_to_address>
<suggested_fix>
<<<<<<< SEARCH
// 验证实体类不为空
Assert.NotNull(entity);
Assert.Equal("3.14", entity.Value1.ToString("#.##"));
=======
// 验证实体类不为空
Assert.NotNull(entity);
Assert.Equal("3.14", entity.Value1.ToString("#.##"));
// 验证 Header 和 Body 属性
Assert.NotNull(entity.Header);
Assert.NotNull(entity.Body);
// 可根据预期值进一步断言
// Assert.Equal(expectedHeader, entity.Header);
// Assert.Equal(expectedBody, entity.Body);
>>>>>>> REPLACE
</suggested_fix>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
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
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 #547
Summary By Copilot
Regression?
Risk
Verification
Packaging changes reviewed?
☑️ Self Check before Merge
Summary by Sourcery
Prevent assignment exceptions by validating converter return types and streamline converter instantiation via reflection, while extending the test suite and standardizing headers.
Bug Fixes:
Enhancements:
Tests:
Chores: