Skip to content

Conversation

@alexeyzimarev
Copy link
Contributor

@alexeyzimarev alexeyzimarev commented Nov 7, 2025

PR Type

Enhancement


Description

  • Add .NET 10 target framework support across all projects

  • Update package versions for .NET 10 RC2 and bump existing versions

  • Refactor test infrastructure to use HostBuilder pattern

  • Exclude System.Linq.Async for .NET 10 (built-in support)

  • Apply code modernization with target-typed new expressions


Diagram Walkthrough

flowchart LR
  A["Add net10.0 target"] --> B["Update package versions"]
  B --> C["Refactor test setup"]
  C --> D["Exclude redundant packages"]
  D --> E["Modernize code syntax"]
Loading

File Walkthrough

Relevant files
Configuration changes
6 files
Directory.Build.props
Add net10.0 to target frameworks                                                 
+1/-1     
Eventuous.EventStore.csproj
Exclude System.Linq.Async for .NET 10                                       
+3/-0     
Eventuous.Tests.EventStore.csproj
Add System.Linq.Async exclusion for .NET 10                           
+5/-2     
Eventuous.Postgresql.csproj
Exclude System.Linq.Async for .NET 10                                       
+2/-0     
Eventuous.Sql.Base.csproj
Exclude System.Linq.Async for .NET 10                                       
+1/-1     
Eventuous.SqlServer.csproj
Exclude System.Linq.Async for .NET 10                                       
+3/-1     
Dependencies
1 files
Directory.Packages.props
Add .NET 10 package versions and update existing                 
+19/-13 
Tests
2 files
RegistrationTests.cs
Refactor test setup with HostBuilder pattern                         
+24/-3   
RegistrationTests.cs
Refactor test to use HostBuilder async pattern                     
+12/-5   
Enhancement
2 files
RedisAllStreamSubscription.cs
Use target-typed new expressions and ToString()                   
+3/-3     
RedisStreamSubscription.cs
Use ToString() for MessageId parsing                                         
+1/-1     
Formatting
1 files
SqlEventStoreBase.cs
Remove extra blank line in method                                               
+0/-1     

@qodo-code-review
Copy link

qodo-code-review bot commented Nov 7, 2025

PR Compliance Guide 🔍

(Compliance updated until commit fbda68f)

Below is a summary of compliance checks for this PR:

Security Compliance
🟢
No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
🎫 No ticket provided
  • Create ticket/issue
Codebase Duplication Compliance
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
🟢
Generic: Meaningful Naming and Self-Documenting Code

Objective: Ensure all identifiers clearly express their purpose and intent, making code
self-documenting

Status: Passed

Generic: Secure Error Handling

Objective: To prevent the leakage of sensitive system information through error messages while
providing sufficient detail for internal debugging.

Status: Passed

Generic: Secure Logging Practices

Objective: To ensure logs are useful for debugging and auditing without exposing sensitive
information like PII, PHI, or cardholder data.

Status: Passed

Generic: Comprehensive Audit Trails

Objective: To create a detailed and reliable record of critical system actions for security analysis
and compliance.

Status:
No logging added: The new host/test server setup and lifecycle changes add operations without any audit or
log trail in the added lines, but as this is test code and only a partial diff, it may be
acceptable.

Referred Code
public async Task Setup() {
    _host = new HostBuilder()
        .ConfigureWebHost(webHostBuilder => webHostBuilder
            .UseTestServer()
            .UseStartup<Startup>()
        )
        .Build();
    await _host.StartAsync();

    _server = _host.GetTestServer();
}

[After(Test)]
public async Task Teardown() {
    _server.Dispose();
    await _host.StopAsync();
    _host.Dispose();
}
Generic: Robust Error Handling and Edge Case Management

Objective: Ensure comprehensive error handling that provides meaningful context and graceful
degradation

Status:
Missing null checks: Newly added parsing and indexing assume presence and format (e.g., Guid.Parse and
streamEvents[0]) without explicit error handling or bounds checks, which may be handled
elsewhere but is not visible here.

Referred Code
var streamEvents = await database.StreamRangeAsync(new(stream), streamPosition).NoContext();
var entry        = streamEvents[0];

persistentEvents.Add(
    new(
        Guid.Parse(entry[MessageId].ToString()),
        entry[MessageType]!,
        entry.Id.ToLong(),
        entry.Id.ToLong(),
Generic: Security-First Input Validation and Data Handling

Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent
vulnerabilities

Status:
Unvalidated input: The new Guid.Parse on message ID and direct dictionary/index access assume trusted data
without validation or try-parse fallbacks, which might be acceptable if upstream
guarantees exist but not shown in this diff.

Referred Code
evt => new ReceivedEvent(
    Guid.Parse(evt[MessageId].ToString()),
    evt[MessageType]!,
    evt.Id.ToLong(),
    evt.Id.ToLong(),
    evt[JsonData]!,
Compliance status legend 🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

Previous compliance checks

Compliance check up to commit 2dbf1ca
Security Compliance
🟢
No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
🎫 No ticket provided
  • Create ticket/issue
Codebase Duplication Compliance
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
🟢
Generic: Meaningful Naming and Self-Documenting Code

Objective: Ensure all identifiers clearly express their purpose and intent, making code
self-documenting

Status: Passed

Generic: Secure Logging Practices

Objective: To ensure logs are useful for debugging and auditing without exposing sensitive
information like PII, PHI, or cardholder data.

Status: Passed

Generic: Comprehensive Audit Trails

Objective: To create a detailed and reliable record of critical system actions for security analysis
and compliance.

Status:
No audit logs: The added test setup/teardown and service registrations do not introduce or reference
audit logging for critical actions, but as this is primarily test infrastructure, lack of
audit trails may be acceptable outside production paths.

Referred Code
public async Task Setup() {
    _host = new HostBuilder()
        .ConfigureWebHost(webHostBuilder => webHostBuilder
            .UseTestServer()
            .UseStartup<Startup>()
        )
        .Build();
    await _host.StartAsync();

    _server = _host.GetTestServer();
}

[After(Test)]
public async Task Teardown() {
    _server.Dispose();
    await _host.StopAsync();
    _host.Dispose();
}
Generic: Robust Error Handling and Edge Case Management

Objective: Ensure comprehensive error handling that provides meaningful context and graceful
degradation

Status:
Missing null checks: New parsing of MessageId via Guid.Parse(...) and indexing streamEvents[0] lack explicit
null/empty checks or try/catch, which could throw on malformed data or empty ranges.

Referred Code
var streamEvents = await database.StreamRangeAsync(new(stream), streamPosition).NoContext();
var entry        = streamEvents[0];

persistentEvents.Add(
    new(
        Guid.Parse(entry[MessageId].ToString()),
        entry[MessageType]!,
        entry.Id.ToLong(),
        entry.Id.ToLong(),
Generic: Secure Error Handling

Objective: To prevent the leakage of sensitive system information through error messages while
providing sufficient detail for internal debugging.

Status:
Detailed errors: Throwing SerializationException with raw message type and error details may expose
internal information if propagated to user-facing layers; context of usage is unclear from
the diff.

Referred Code
return deserialized switch {
    SuccessfullyDeserialized success => AsStreamEvent(success.Payload),
    FailedToDeserialize failed       => throw new SerializationException($"Can't deserialize {evt.MessageType}: {failed.Error}"),
    _                                => throw new("Unknown deserialization result")
};
Generic: Security-First Input Validation and Data Handling

Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent
vulnerabilities

Status:
Unvalidated input: Direct Guid.Parse(evt[MessageId].ToString()) and indexing without verifying
presence/format of fields could lead to runtime exceptions if external data is malformed;
may be acceptable if upstream guarantees schema.

Referred Code
Guid.Parse(evt[MessageId].ToString()),
evt[MessageType]!,
evt.Id.ToLong(),
evt.Id.ToLong(),
evt[JsonData]!,

@qodo-code-review
Copy link

qodo-code-review bot commented Nov 7, 2025

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Correct .NET 8 package versions

In the property group for .NET 8, change the MicrosoftExtensionsVer from 9.0.10
to a .NET 8-compatible version like 8.0.0 to avoid referencing incorrect
assemblies.

Directory.Packages.props [13-16]

 <PropertyGroup Label="Package versions for .NET 8" Condition="'$(TargetFramework)' == 'net8.0'">
   <MicrosoftTestHostVer>8.0.21</MicrosoftTestHostVer>
-  <MicrosoftExtensionsVer>9.0.10</MicrosoftExtensionsVer>
+  <MicrosoftExtensionsVer>8.0.0</MicrosoftExtensionsVer>
 </PropertyGroup>

[Suggestion processed]

Suggestion importance[1-10]: 9

__

Why: The suggestion correctly identifies a critical version mismatch where .NET 8 projects would incorrectly reference .NET 9 packages, which could lead to runtime failures.

High
Fix inconsistent package reference condition

Correct the package reference for System.Linq.Async to be included only when the
target framework is not net10.0, and remove the ExcludeAssets="All" attribute.

src/EventStore/src/Eventuous.EventStore/Eventuous.EventStore.csproj [17-19]

-<ItemGroup Condition="'$(TargetFramework)' == 'net10.0'">
-    <PackageReference Include="System.Linq.Async" ExcludeAssets="All"/>
+<ItemGroup Condition="'$(TargetFramework)' != 'net10.0'">
+    <PackageReference Include="System.Linq.Async" />
 </ItemGroup>
  • Apply / Chat
Suggestion importance[1-10]: 7

__

Why: The suggestion correctly identifies an inconsistent and likely incorrect package reference condition for System.Linq.Async and proposes a fix that aligns with the pattern used in other projects in the PR.

Medium
  • Update

@github-actions
Copy link

github-actions bot commented Nov 7, 2025

Test Results

 51 files   51 suites   27m 39s ⏱️
236 tests 236 ✅ 0 💤 0 ❌
711 runs  711 ✅ 0 💤 0 ❌

Results for commit fbda68f.

♻️ This comment has been updated with latest results.

Comment on lines +14 to 16
<MicrosoftTestHostVer>8.0.21</MicrosoftTestHostVer>
<MicrosoftExtensionsVer>9.0.10</MicrosoftExtensionsVer>
</PropertyGroup>

Choose a reason for hiding this comment

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

Suggestion: Correct .NET 8 package versions

Suggested change
<MicrosoftTestHostVer>8.0.21</MicrosoftTestHostVer>
<MicrosoftExtensionsVer>9.0.10</MicrosoftExtensionsVer>
</PropertyGroup>
<MicrosoftTestHostVer>8.0.21</MicrosoftTestHostVer>
<MicrosoftExtensionsVer>8.0.0</MicrosoftExtensionsVer>
</PropertyGroup>

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The suggestion is not correct. Microsoft extension packages 9.0.6 support .NET 8.

Choose a reason for hiding this comment

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

Question:

The suggestion is not correct. Microsoft extension packages 9.0.6 support .NET 8.

Answer:

You can steer suggestions to respect your .NET versioning policy via configuration.

Add repo-specific guidance so the Improve tool won’t “downgrade” Microsoft.Extensions packages when targeting .NET 8:

[pr_code_suggestions]
extra_instructions="""\
- Do not suggest lowering Microsoft.Extensions.* package versions for .NET 8 projects.
- When TargetFramework is net8.0, prefer Microsoft.Extensions.* >= 9.0.6 unless the PR explicitly pins a different version.
- Treat SDK/runtime compatibility as authoritative: .NET 8 supports Microsoft.Extensions.* 9.x.
- If proposing package changes, explain compatibility and link to the justification in the suggestion.
"""

Optionally, encode this as an organizational best practice so it’s applied consistently:

  • Create best_practices.md in the repo (or use global best practices) with a rule like:
    “For net8.0 projects, Microsoft.Extensions.* packages should be 9.0.6 or newer unless constrained by transitive compatibility.”

If you still see incorrect version proposals, note this is an AI limitation; keeping the above instructions and best practices short, explicit, and close to your conventions usually resolves it.

Relevant Sources:

@alexeyzimarev alexeyzimarev merged commit dd2903f into dev Nov 8, 2025
5 checks passed
@alexeyzimarev alexeyzimarev deleted the net10-target branch November 8, 2025 18:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants