Skip to content

Conversation

@ArgoZhang
Copy link
Member

@ArgoZhang ArgoZhang commented Jul 16, 2025

Link issues

fixes #6421

Summary By Copilot

Regression?

  • Yes
  • No

Risk

  • High
  • Medium
  • Low

Verification

  • Manual (required)
  • Automated

Packaging changes reviewed?

  • Yes
  • No
  • N/A

☑️ Self Check before Merge

⚠️ Please check all items below before review. ⚠️

  • Doc is updated/provided or not needed
  • Demo is updated/provided or not needed
  • Merge the latest code from the main branch

Summary by Sourcery

Introduce a new socket data conversion framework by defining ISocketDataPropertyConverter and ISocketDataConverter interfaces, attribute-based property mapping, and default converters for common data types. Integrate the new converters into IDataPackageAdapter and ITcpSocketClientExtensions, and update tests to cover the new functionality.

New Features:

  • Add ISocketDataPropertyConverter interface with default implementations for various primitive and enum types
  • Introduce SocketDataPropertyAttribute and SocketDataConverterAttribute for attribute-based socket data mapping and conversion
  • Define ISocketDataConverter interface and SocketDataConverterBase abstract class to convert raw socket data into entities
  • Provide new overloads of SetDataPackageAdapter in ITcpSocketClientExtensions to accept an ISocketDataConverter and apply attribute-driven conversion

Enhancements:

  • Update IDataPackageAdapter and DataPackageAdapter to use a generic TryConvertTo method with ISocketDataConverter
  • Revise ITcpSocketClientExtensions to automatically resolve and apply converters based on SocketDataConverterAttribute on entity types

Tests:

  • Add unit tests for endian-aware converters for integer, floating point, boolean, byte array, string, and enum types
  • Update existing TcpSocketFactoryTest to validate the new converter and adapter integration

@bb-auto bb-auto bot added the enhancement New feature or request label Jul 16, 2025
@sourcery-ai
Copy link
Contributor

sourcery-ai bot commented Jul 16, 2025

Reviewer's Guide

This PR introduces a comprehensive socket data conversion framework based on attributes and pluggable converters, extends the existing data package adapter and client extensions to support generic entity conversion via ISocketDataConverter, and adds extensive unit tests for both the converter implementations and the integration with the TCP socket client.

Class diagram for the new socket data conversion framework

classDiagram
    class ISocketDataPropertyConverter {
        +object? Convert(ReadOnlyMemory<byte> data)
    }
    class ISocketDataConverter~TEntity~ {
        +bool TryConvertTo(ReadOnlyMemory<byte> data, out TEntity? entity)
    }
    class SocketDataConverterBase~TEntity~ {
        +bool TryConvertTo(ReadOnlyMemory<byte> data, out TEntity? entity)
        #bool Parse(ReadOnlyMemory<byte> data, TEntity entity)
    }
    class SocketDataConverterAttribute {
        +Type? Type
    }
    class SocketDataPropertyAttribute {
        +Type? Type
        +int Offset
        +int Length
        +string? EncodingName
        +Type? ConverterType
        +object?[]? ConverterParameters
    }
    class SocketDataPropertyExtensions {
        +ISocketDataPropertyConverter? GetConverter(SocketDataPropertyAttribute)
        +object? ConvertTo(SocketDataPropertyAttribute, ReadOnlyMemory<byte> data)
    }
    ISocketDataConverter~TEntity~ <|-- SocketDataConverterBase~TEntity~
    SocketDataPropertyAttribute <.. SocketDataPropertyExtensions
    SocketDataPropertyAttribute <.. ISocketDataPropertyConverter
    SocketDataConverterAttribute <.. ISocketDataConverter~TEntity~

    %% Concrete converters
    class SocketDataByteArrayConverter {
        +object? Convert(ReadOnlyMemory<byte> data)
    }
    class SocketDataStringConverter {
        +object? Convert(ReadOnlyMemory<byte> data)
    }
    class SocketDataEnumConverter {
        +object? Convert(ReadOnlyMemory<byte> data)
    }
    class SocketDataBoolConverter {
        +object? Convert(ReadOnlyMemory<byte> data)
    }
    class SocketDataInt16BigEndianConverter {
        +object? Convert(ReadOnlyMemory<byte> data)
    }
    class SocketDataInt16LittleEndianConverter {
        +object? Convert(ReadOnlyMemory<byte> data)
    }
    class SocketDataInt32BigEndianConverter {
        +object? Convert(ReadOnlyMemory<byte> data)
    }
    class SocketDataInt32LittleEndianConverter {
        +object? Convert(ReadOnlyMemory<byte> data)
    }
    class SocketDataInt64BigEndianConverter {
        +object? Convert(ReadOnlyMemory<byte> data)
    }
    class SocketDataInt64LittleEndianConverter {
        +object? Convert(ReadOnlyMemory<byte> data)
    }
    class SocketDataSingleBigEndianConverter {
        +object? Convert(ReadOnlyMemory<byte> data)
    }
    class SocketDataSingleLittleEndianConverter {
        +object? Convert(ReadOnlyMemory<byte> data)
    }
    class SocketDataDoubleBigEndianConverter {
        +object? Convert(ReadOnlyMemory<byte> data)
    }
    class SocketDataDoubleLittleEndianConverter {
        +object? Convert(ReadOnlyMemory<byte> data)
    }
    class SocketDataUInt16BigEndianConverter {
        +object? Convert(ReadOnlyMemory<byte> data)
    }
    class SocketDataUInt16LittleEndianConverter {
        +object? Convert(ReadOnlyMemory<byte> data)
    }
    class SocketDataUInt32BigEndianConverter {
        +object? Convert(ReadOnlyMemory<byte> data)
    }
    class SocketDataUInt32LittleEndianConverter {
        +object? Convert(ReadOnlyMemory<byte> data)
    }
    class SocketDataUInt64BigEndianConverter {
        +object? Convert(ReadOnlyMemory<byte> data)
    }
    class SocketDataUInt64LittleEndianConverter {
        +object? Convert(ReadOnlyMemory<byte> data)
    }
    ISocketDataPropertyConverter <|.. SocketDataByteArrayConverter
    ISocketDataPropertyConverter <|.. SocketDataStringConverter
    ISocketDataPropertyConverter <|.. SocketDataEnumConverter
    ISocketDataPropertyConverter <|.. SocketDataBoolConverter
    ISocketDataPropertyConverter <|.. SocketDataInt16BigEndianConverter
    ISocketDataPropertyConverter <|.. SocketDataInt16LittleEndianConverter
    ISocketDataPropertyConverter <|.. SocketDataInt32BigEndianConverter
    ISocketDataPropertyConverter <|.. SocketDataInt32LittleEndianConverter
    ISocketDataPropertyConverter <|.. SocketDataInt64BigEndianConverter
    ISocketDataPropertyConverter <|.. SocketDataInt64LittleEndianConverter
    ISocketDataPropertyConverter <|.. SocketDataSingleBigEndianConverter
    ISocketDataPropertyConverter <|.. SocketDataSingleLittleEndianConverter
    ISocketDataPropertyConverter <|.. SocketDataDoubleBigEndianConverter
    ISocketDataPropertyConverter <|.. SocketDataDoubleLittleEndianConverter
    ISocketDataPropertyConverter <|.. SocketDataUInt16BigEndianConverter
    ISocketDataPropertyConverter <|.. SocketDataUInt16LittleEndianConverter
    ISocketDataPropertyConverter <|.. SocketDataUInt32BigEndianConverter
    ISocketDataPropertyConverter <|.. SocketDataUInt32LittleEndianConverter
    ISocketDataPropertyConverter <|.. SocketDataUInt64BigEndianConverter
    ISocketDataPropertyConverter <|.. SocketDataUInt64LittleEndianConverter
Loading

Class diagram for updated IDataPackageAdapter and ITcpSocketClientExtensions

classDiagram
    class IDataPackageAdapter {
        +ValueTask HandlerAsync(ReadOnlyMemory<byte> data, CancellationToken token = default)
        +bool TryConvertTo~TEntity~(ReadOnlyMemory<byte> data, ISocketDataConverter~TEntity~ socketDataConverter, out TEntity? entity)
    }
    class ITcpSocketClient {
        +Func<ReadOnlyMemory<byte>, Task> ReceivedCallBack
    }
    class ITcpSocketClientExtensions {
        +void SetDataPackageAdapter~TEntity~(ITcpSocketClient client, IDataPackageAdapter adapter, ISocketDataConverter~TEntity~ socketDataConverter, Func<TEntity?, Task> callback)
        +void SetDataPackageAdapter~TEntity~(ITcpSocketClient client, IDataPackageAdapter adapter, Func<TEntity?, Task> callback)
    }
    IDataPackageAdapter <.. ITcpSocketClientExtensions : uses
    ITcpSocketClient <.. ITcpSocketClientExtensions : configures
    ISocketDataConverter~TEntity~ <.. IDataPackageAdapter : uses
Loading

File-Level Changes

Change Details Files
Introduce socket data property conversion framework
  • Added ISocketDataPropertyConverter interface
  • Defined SocketDataPropertyAttribute for mapping entity properties
  • Implemented concrete converters (byte[], string, bool, numeric, enum)
  • Added SocketDataPropertyExtensions to apply attribute-driven conversion
src/BootstrapBlazor/Services/TcpSocket/PropertyConverter/ISocketDataPropertyConverter.cs
src/BootstrapBlazor/Services/TcpSocket/PropertyConverter/SocketDataPropertyAttribute.cs
src/BootstrapBlazor/Services/TcpSocket/PropertyConverter/SocketDataPropertyExtensions.cs
src/BootstrapBlazor/Services/TcpSocket/PropertyConverter/SocketDataByteArrayConverter.cs
src/BootstrapBlazor/Services/TcpSocket/PropertyConverter/SocketDataBoolConverter.cs
src/BootstrapBlazor/Services/TcpSocket/PropertyConverter/SocketDataEnumConverter.cs
src/BootstrapBlazor/Services/TcpSocket/PropertyConverter/SocketDataInt16BigEndianConverter.cs
src/BootstrapBlazor/Services/TcpSocket/PropertyConverter/SocketDataInt16LittleEndianConverter.cs
src/BootstrapBlazor/Services/TcpSocket/PropertyConverter/SocketDataInt32BigEndianConverter.cs
src/BootstrapBlazor/Services/TcpSocket/PropertyConverter/SocketDataInt32LittleEndianConverter.cs
src/BootstrapBlazor/Services/TcpSocket/PropertyConverter/SocketDataInt64BigEndianConverter.cs
src/BootstrapBlazor/Services/TcpSocket/PropertyConverter/SocketDataInt64LittleEndianConverter.cs
src/BootstrapBlazor/Services/TcpSocket/PropertyConverter/SocketDataSingleBigEndianConverter.cs
src/BootstrapBlazor/Services/TcpSocket/PropertyConverter/SocketDataSingleLittleEndianConverter.cs
src/BootstrapBlazor/Services/TcpSocket/PropertyConverter/SocketDataDoubleBigEndianConverter.cs
src/BootstrapBlazor/Services/TcpSocket/PropertyConverter/SocketDataDoubleLittleEndianConverter.cs
src/BootstrapBlazor/Services/TcpSocket/PropertyConverter/SocketDataUInt16BigEndianConverter.cs
src/BootstrapBlazor/Services/TcpSocket/PropertyConverter/SocketDataUInt16LittleEndianConverter.cs
src/BootstrapBlazor/Services/TcpSocket/PropertyConverter/SocketDataUInt32BigEndianConverter.cs
src/BootstrapBlazor/Services/TcpSocket/PropertyConverter/SocketDataUInt32LittleEndianConverter.cs
src/BootstrapBlazor/Services/TcpSocket/PropertyConverter/SocketDataUInt64BigEndianConverter.cs
src/BootstrapBlazor/Services/TcpSocket/PropertyConverter/SocketDataUInt64LittleEndianConverter.cs
Extend data package adapter and client extensions to use ISocketDataConverter
  • Updated IDataPackageAdapter.TryConvertTo signature to generic with converter
  • Modified DataPackageAdapter.TryConvertTo implementation
  • Added SetDataPackageAdapter overloads in ITcpSocketClientExtensions to accept ISocketDataConverter and attribute-driven defaults
src/BootstrapBlazor/Services/TcpSocket/DataPackageAdapter/IDataPackageAdapter.cs
src/BootstrapBlazor/Services/TcpSocket/DataPackageAdapter/DataPackageAdapter.cs
src/BootstrapBlazor/Extensions/ITcpSocketClientExtensions.cs
Add ISocketDataConverter and base class for entity conversion
  • Introduced ISocketDataConverter interface
  • Added SocketDataConverterBase with reflection-based parsing
  • Created SocketDataConverterAttribute to annotate entity converter
src/BootstrapBlazor/Services/TcpSocket/DataConverter/ISocketDataConverter.cs
src/BootstrapBlazor/Services/TcpSocket/DataConverter/SocketDataConverterBase.cs
src/BootstrapBlazor/Services/TcpSocket/DataConverter/SocketDataConverterAttribute.cs
Update TcpSocketFactoryTest to use new converter mechanism and enhance tests
  • Replaced mock adapter with DataPackageAdapter + MockEntitySocketDataConverter
  • Extended tests for all property types via SocketDataPropertyAttribute
  • Added direct TryConvertTo and generic SetDataPackageAdapter integration tests
test/UnitTest/Services/TcpSocketFactoryTest.cs
Add unit tests for individual socket data property converters
  • Created TcpSocketPropertyConverterTest covering numeric and floating converters
  • Validated both big- and little-endian conversion behaviors
test/UnitTest/Services/TcpSocketPropertyConverterTest.cs

Assessment against linked issues

Issue Objective Addressed Explanation

Possibly linked issues


Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@bb-auto bb-auto bot added this to the 9.8.0 milestone Jul 16, 2025
@ArgoZhang ArgoZhang merged commit fdbe935 into main Jul 16, 2025
3 checks passed
@ArgoZhang ArgoZhang deleted the feat-converter branch July 16, 2025 03:19
Copy link
Contributor

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @ArgoZhang - I've reviewed your changes - here's some feedback:

  • You’ve changed IDataPackageAdapter.TryConvertTo to a generic overload which will break existing non-generic adapters—consider providing a backward-compatible shim or overload.
  • The reflection-based SetDataPackageAdapter overload does not handle converter instantiation failures or type mismatches—add error checking or logging to surface misconfigurations.
  • Your default property converters use big-endian but tests cover little-endian converters—please clarify endianness semantics or offer a configurable default byte order.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- You’ve changed IDataPackageAdapter.TryConvertTo to a generic overload which will break existing non-generic adapters—consider providing a backward-compatible shim or overload.
- The reflection-based SetDataPackageAdapter overload does not handle converter instantiation failures or type mismatches—add error checking or logging to surface misconfigurations.
- Your default property converters use big-endian but tests cover little-endian converters—please clarify endianness semantics or offer a configurable default byte order.

## Individual Comments

### Comment 1
<location> `src/BootstrapBlazor/Services/TcpSocket/Extensions/SocketDataPropertyExtensions.cs:95` </location>
<code_context>
+        var start = attribute.Offset;
+        var length = attribute.Length;
+
+        if (data.Length >= start + length)
+        {
+            var buffer = data.Slice(start, length);
</code_context>

<issue_to_address>
Potential for out-of-bounds slice if Offset or Length are negative.

Please add checks to ensure both `start` and `length` are non-negative before slicing.
</issue_to_address>

### Comment 2
<location> `src/BootstrapBlazor/Services/TcpSocket/PropertyConverter/SocketDataEnumConverter.cs:22` </location>
<code_context>
+    public object? Convert(ReadOnlyMemory<byte> data)
+    {
+        var ret = false;
+        if (data.Length == 1)
+        {
+            ret = data.Span[0] != 0x00;
</code_context>

<issue_to_address>
Enum conversion only supports single-byte values.

This implementation assumes enums fit in a single byte. Please add support for larger underlying types or clearly document this limitation.
</issue_to_address>

### Comment 3
<location> `src/BootstrapBlazor/Services/TcpSocket/PropertyConverter/SocketDataInt32BigEndianConverter.cs:23` </location>
<code_context>
+    {
+        var ret = 0;
+        Span<byte> paddedSpan = stackalloc byte[4];
+        data.Span.CopyTo(paddedSpan[(4 - data.Length)..]);
+
+        if (BinaryPrimitives.TryReadInt32BigEndian(paddedSpan, out var v))
</code_context>

<issue_to_address>
No check for data length greater than 4 bytes.

Add a check to ensure `data.Length` is 4 or less to prevent exceptions during the slice operation.
</issue_to_address>

### Comment 4
<location> `src/BootstrapBlazor/Services/TcpSocket/PropertyConverter/SocketDataInt32LittleEndianConverter.cs:23` </location>
<code_context>
+    {
+        var ret = 0;
+        Span<byte> paddedSpan = stackalloc byte[4];
+        data.Span.CopyTo(paddedSpan[(4 - data.Length)..]);
+
+        if (BinaryPrimitives.TryReadInt32BigEndian(paddedSpan, out var v))
</code_context>

<issue_to_address>
No check for data length greater than 4 bytes.

Add a check to ensure `data.Length` is 4 or less before performing the slice to prevent exceptions.
</issue_to_address>

### Comment 5
<location> `src/BootstrapBlazor/Services/TcpSocket/PropertyConverter/SocketDataInt64BigEndianConverter.cs:23` </location>
<code_context>
+    {
+        double ret = 0;
+        Span<byte> paddedSpan = stackalloc byte[8];
+        data.Span.CopyTo(paddedSpan[(8 - data.Length)..]);
+        if (BinaryPrimitives.TryReadDoubleBigEndian(paddedSpan, out var v))
+        {
</code_context>

<issue_to_address>
No check for data length greater than 8 bytes.

Add a check to prevent `data.Length` from exceeding 8 to avoid exceptions during the slice operation.
</issue_to_address>

### Comment 6
<location> `src/BootstrapBlazor/Services/TcpSocket/PropertyConverter/SocketDataSingleBigEndianConverter.cs:23` </location>
<code_context>
+    {
+        var ret = 0;
+        Span<byte> paddedSpan = stackalloc byte[4];
+        data.Span.CopyTo(paddedSpan[(4 - data.Length)..]);
+
+        if (BinaryPrimitives.TryReadInt32BigEndian(paddedSpan, out var v))
</code_context>

<issue_to_address>
No check for data length greater than 4 bytes.

Add a check to ensure `data.Length` is not greater than 4 to prevent exceptions during the slice operation.
</issue_to_address>

### Comment 7
<location> `src/BootstrapBlazor/Services/TcpSocket/PropertyConverter/SocketDataSingleLittleEndianConverter.cs:23` </location>
<code_context>
+    {
+        var ret = 0;
+        Span<byte> paddedSpan = stackalloc byte[4];
+        data.Span.CopyTo(paddedSpan[(4 - data.Length)..]);
+
+        if (BinaryPrimitives.TryReadInt32BigEndian(paddedSpan, out var v))
</code_context>

<issue_to_address>
No check for data length greater than 4 bytes.

Add a check to ensure `data.Length` is not greater than 4 to prevent exceptions during the slice operation.
</issue_to_address>

### Comment 8
<location> `src/BootstrapBlazor/Services/TcpSocket/PropertyConverter/SocketDataUInt16BigEndianConverter.cs:23` </location>
<code_context>
+    {
+        short ret = 0;
+        Span<byte> paddedSpan = stackalloc byte[2];
+        data.Span.CopyTo(paddedSpan[(2 - data.Length)..]);
+        if (BinaryPrimitives.TryReadInt16BigEndian(paddedSpan, out var v))
+        {
</code_context>

<issue_to_address>
No check for data length greater than 2 bytes.

Add a check to ensure `data.Length` is not greater than 2 to prevent exceptions during the slice operation.
</issue_to_address>

### Comment 9
<location> `src/BootstrapBlazor/Services/TcpSocket/PropertyConverter/SocketDataUInt16LittleEndianConverter.cs:23` </location>
<code_context>
+    {
+        short ret = 0;
+        Span<byte> paddedSpan = stackalloc byte[2];
+        data.Span.CopyTo(paddedSpan[(2 - data.Length)..]);
+        if (BinaryPrimitives.TryReadInt16BigEndian(paddedSpan, out var v))
+        {
</code_context>

<issue_to_address>
No check for data length greater than 2 bytes.

Add a check to ensure `data.Length` is not greater than 2 to prevent exceptions during the slice operation.
</issue_to_address>

### Comment 10
<location> `src/BootstrapBlazor/Services/TcpSocket/PropertyConverter/SocketDataUInt32BigEndianConverter.cs:23` </location>
<code_context>
+    {
+        var ret = 0;
+        Span<byte> paddedSpan = stackalloc byte[4];
+        data.Span.CopyTo(paddedSpan[(4 - data.Length)..]);
+
+        if (BinaryPrimitives.TryReadInt32BigEndian(paddedSpan, out var v))
</code_context>

<issue_to_address>
No check for data length greater than 4 bytes.

Add a check to ensure `data.Length` is not greater than 4 to prevent exceptions during the slice operation.
</issue_to_address>

### Comment 11
<location> `src/BootstrapBlazor/Services/TcpSocket/PropertyConverter/SocketDataUInt32LittleEndianConverter.cs:23` </location>
<code_context>
+    {
+        var ret = 0;
+        Span<byte> paddedSpan = stackalloc byte[4];
+        data.Span.CopyTo(paddedSpan[(4 - data.Length)..]);
+
+        if (BinaryPrimitives.TryReadInt32BigEndian(paddedSpan, out var v))
</code_context>

<issue_to_address>
No check for data length greater than 4 bytes.

Add a check to ensure `data.Length` is not greater than 4 to prevent exceptions during the slice operation.
</issue_to_address>

### Comment 12
<location> `src/BootstrapBlazor/Services/TcpSocket/PropertyConverter/SocketDataUInt64BigEndianConverter.cs:23` </location>
<code_context>
+    {
+        double ret = 0;
+        Span<byte> paddedSpan = stackalloc byte[8];
+        data.Span.CopyTo(paddedSpan[(8 - data.Length)..]);
+        if (BinaryPrimitives.TryReadDoubleBigEndian(paddedSpan, out var v))
+        {
</code_context>

<issue_to_address>
No check for data length greater than 8 bytes.

Add a check to prevent `data.Length` from exceeding 8 to avoid exceptions during the slice operation.
</issue_to_address>

### Comment 13
<location> `src/BootstrapBlazor/Services/TcpSocket/PropertyConverter/SocketDataUInt64LittleEndianConverter.cs:23` </location>
<code_context>
+    {
+        double ret = 0;
+        Span<byte> paddedSpan = stackalloc byte[8];
+        data.Span.CopyTo(paddedSpan[(8 - data.Length)..]);
+        if (BinaryPrimitives.TryReadDoubleBigEndian(paddedSpan, out var v))
+        {
</code_context>

<issue_to_address>
No check for data length greater than 8 bytes.

Add a check to prevent `data.Length` from exceeding 8 to avoid exceptions during the slice operation.
</issue_to_address>

### Comment 14
<location> `test/UnitTest/Services/TcpSocketFactoryTest.cs:735` </location>
<code_context>

-        // 发送数据
-        var data = new ReadOnlyMemory<byte>([1, 2, 3, 4, 5]);
+        // 测试数据适配器直接调用 TryConvertTo 方法转换数据
+        var adapter2 = new DataPackageAdapter();
+        var result = adapter2.TryConvertTo(data, new MockEntitySocketDataConverter(), out var t);
+        Assert.True(result);
+        Assert.NotNull(t);
+        Assert.Equal([1, 2, 3, 4, 5], entity.Header);
+
</code_context>

<issue_to_address>
Consider adding negative test cases for TryConvertTo.

Please add tests with invalid or incomplete input to verify that TryConvertTo returns false and the output entity is null.
</issue_to_address>

<suggested_fix>
<<<<<<< SEARCH
        // 测试数据适配器直接调用 TryConvertTo 方法转换数据
        var adapter2 = new DataPackageAdapter();
        var result = adapter2.TryConvertTo(data, new MockEntitySocketDataConverter(), out var t);
        Assert.True(result);
        Assert.NotNull(t);
        Assert.Equal([1, 2, 3, 4, 5], entity.Header);

=======
        // 测试数据适配器直接调用 TryConvertTo 方法转换数据
        var adapter2 = new DataPackageAdapter();
        var result = adapter2.TryConvertTo(data, new MockEntitySocketDataConverter(), out var t);
        Assert.True(result);
        Assert.NotNull(t);
        Assert.Equal([1, 2, 3, 4, 5], entity.Header);

        // Negative test cases for TryConvertTo

        // Case 1: Empty data
        var emptyData = new ReadOnlyMemory<byte>([]);
        var negativeResult1 = adapter2.TryConvertTo(emptyData, new MockEntitySocketDataConverter(), out var negativeEntity1);
        Assert.False(negativeResult1);
        Assert.Null(negativeEntity1);

        // Case 2: Incomplete data (e.g., only 2 bytes)
        var shortData = new ReadOnlyMemory<byte>([1, 2]);
        var negativeResult2 = adapter2.TryConvertTo(shortData, new MockEntitySocketDataConverter(), out var negativeEntity2);
        Assert.False(negativeResult2);
        Assert.Null(negativeEntity2);

        // Case 3: Null data (if TryConvertTo supports nullable, otherwise skip)
        // Uncomment the following lines if TryConvertTo accepts null as input
        // ReadOnlyMemory<byte>? nullData = null;
        // var negativeResult3 = adapter2.TryConvertTo(nullData, new MockEntitySocketDataConverter(), out var negativeEntity3);
        // Assert.False(negativeResult3);
        // Assert.Null(negativeEntity3);
>>>>>>> REPLACE

</suggested_fix>

### Comment 15
<location> `test/UnitTest/Services/TcpSocketFactoryTest.cs:710` </location>
<code_context>
+        // bool
+        Assert.True(entity.Value10);
+
+        // enum
+        Assert.Equal(EnumEducation.Middle, entity.Value11);
+
+        // foo
</code_context>

<issue_to_address>
Consider adding tests for invalid enum values.

Please include a test for when the enum value is invalid to verify proper error handling.

Suggested implementation:

```csharp
        // enum
        Assert.Equal(EnumEducation.Middle, entity.Value11);

        // invalid enum value
        // Simulate deserialization or assignment of an invalid enum value
        // This assumes Value11 is settable for test purposes; adjust as needed for your codebase.
        Assert.Throws<ArgumentException>(() => entity.Value11 = (EnumEducation)999);

        // foo
        Assert.NotNull(entity.Value12);

```

If `entity.Value11` is not settable, or if the enum value is set via deserialization, you may need to simulate deserialization with an invalid value and assert the expected error handling (e.g., exception thrown, default value set, etc.). Adjust the test accordingly to fit your codebase's actual error handling for invalid enum values.
</issue_to_address>

### Comment 16
<location> `test/UnitTest/Services/TcpSocketFactoryTest.cs:689` </location>
<code_context>
+        // long
+        Assert.Equal(16, entity.Value3);
+
+        // double
+        Assert.Equal(3.14, entity.Value4);
+
+        // single
</code_context>

<issue_to_address>
Add tests for boundary and invalid floating-point values.

Please include tests for NaN, Infinity, and extreme float/double values to verify correct converter behavior.

Suggested implementation:

```csharp
        // double
        Assert.Equal(3.14, entity.Value4);

        // double boundary and invalid values
        Assert.True(double.IsNaN(entity.NaNDouble));
        Assert.True(double.IsPositiveInfinity(entity.PositiveInfinityDouble));
        Assert.True(double.IsNegativeInfinity(entity.NegativeInfinityDouble));
        Assert.Equal(double.MaxValue, entity.MaxDouble);
        Assert.Equal(double.MinValue, entity.MinDouble);

        // single
        Assert.NotEqual(0, entity.Value5);

        // single boundary and invalid values
        Assert.True(float.IsNaN(entity.NaNSingle));
        Assert.True(float.IsPositiveInfinity(entity.PositiveInfinitySingle));
        Assert.True(float.IsNegativeInfinity(entity.NegativeInfinitySingle));
        Assert.Equal(float.MaxValue, entity.MaxSingle);
        Assert.Equal(float.MinValue, entity.MinSingle);

```

You will need to ensure that the `entity` object has the following properties (or similar) for these tests to compile and pass:
- NaNDouble, PositiveInfinityDouble, NegativeInfinityDouble, MaxDouble, MinDouble (of type double)
- NaNSingle, PositiveInfinitySingle, NegativeInfinitySingle, MaxSingle, MinSingle (of type float)

If these properties do not exist, you will need to add them to the entity and ensure they are set up appropriately in your test setup.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Comment on lines +710 to +711
// enum
Assert.Equal(EnumEducation.Middle, entity.Value11);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion (testing): Consider adding tests for invalid enum values.

Please include a test for when the enum value is invalid to verify proper error handling.

Suggested implementation:

        // enum
        Assert.Equal(EnumEducation.Middle, entity.Value11);

        // invalid enum value
        // Simulate deserialization or assignment of an invalid enum value
        // This assumes Value11 is settable for test purposes; adjust as needed for your codebase.
        Assert.Throws<ArgumentException>(() => entity.Value11 = (EnumEducation)999);

        // foo
        Assert.NotNull(entity.Value12);

If entity.Value11 is not settable, or if the enum value is set via deserialization, you may need to simulate deserialization with an invalid value and assert the expected error handling (e.g., exception thrown, default value set, etc.). Adjust the test accordingly to fit your codebase's actual error handling for invalid enum values.

@codecov
Copy link

codecov bot commented Jul 16, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (4fffebd) to head (05ea295).
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##              main     #6422    +/-   ##
==========================================
  Coverage   100.00%   100.00%            
==========================================
  Files          722       746    +24     
  Lines        31844     32177   +333     
  Branches      4492      4538    +46     
==========================================
+ Hits         31844     32177   +333     
Flag Coverage Δ
BB 100.00% <100.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(ISocketDataPropertyConverter): add ISocketDataPropertyConverter interface

2 participants