net10 upgraded #44
net10 upgraded #44
Conversation
Migrated all projects to target .NET 10.0, updated NuGet package versions across the solution, and replaced the legacy .sln file with a new .slnx format. CI/CD workflows and documentation were updated to reflect .NET 10.0 usage. Improved Swagger versioning configuration and fixed minor issues in Redis cache and AutoMapper registration.
Replaces Expiration.KeepTtl with the actual expiry value and sets keepTtl to false in StringSetAsync. This ensures that the cache entry uses the intended expiration time instead of keeping the previous TTL.
Upgrade to .NET 10 and update dependencies
Update global.json
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (22)
Note
|
Summary of ChangesHello @yilmaztayfun, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request performs a significant upgrade of the project's entire dependency stack to align with .NET 10.0. This comprehensive update ensures the codebase leverages the latest framework features, performance enhancements, and security improvements. The changes encompass updating the target framework for all core projects, bumping versions for a wide array of third-party NuGet packages, and adapting existing code to new API conventions introduced in the updated libraries. The modernization also includes a shift to a new solution file format, streamlining project management. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Ignored Files
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request effectively upgrades the project to .NET 10, which is a significant undertaking. The changes include updating numerous package versions, modifying target frameworks in project files, and adapting the code to breaking changes from the updated dependencies. The refactoring of the Swagger/ApiVersioning configuration to use the IConfigureOptions pattern is a notable improvement, as is the switch to the modern .slnx solution file format. I have one suggestion regarding the suppression of new warnings.
| <LangVersion>latest</LangVersion> | ||
| <Version>1.0.0</Version> | ||
| <NoWarn>$(NoWarn);CS1591;CS0436</NoWarn> | ||
| <NoWarn>$(NoWarn);CS1591;CS0436;DAPR_JOBS;DAPR_DISTRIBUTEDLOCK</NoWarn> |
There was a problem hiding this comment.
Two new warning codes, DAPR_JOBS and DAPR_DISTRIBUTEDLOCK, have been added to the suppressed warnings list. Suppressing warnings can hide potential issues and make maintenance harder. Please consider addressing these warnings instead of suppressing them. If suppression is necessary (e.g., for known issues or false positives), please add an XML comment above this line explaining the reason for future reference.
Reviewer's Guide.NET 10 upgrade across the repo, including SDK/tooling updates, migration from .sln to .slnx, adjustments for new ASP.NET/DI/Swagger APIs, safer Redis cache serialization, and updated AutoMapper registration. Sequence diagram for Swagger configuration with API versioningsequenceDiagram
participant HostBuilder
participant ServiceCollection
participant SwaggerGen
participant ConfigureSwaggerOptions
participant ApiVersionDescriptionProvider
HostBuilder->>ServiceCollection: AddAetherApiVersioning(apiTitle)
ServiceCollection->>ServiceCollection: AddApiVersioning()
ServiceCollection->>SwaggerGen: AddSwaggerGen()
ServiceCollection->>ServiceCollection: AddTransient IConfigureOptions_SwaggerGenOptions
ServiceCollection->>ConfigureSwaggerOptions: Create instance(provider, apiTitle)
HostBuilder->>SwaggerGen: Resolve SwaggerGenOptions
SwaggerGen->>ConfigureSwaggerOptions: Configure(options)
ConfigureSwaggerOptions->>ApiVersionDescriptionProvider: get ApiVersionDescriptions
loop For each ApiVersionDescription
ConfigureSwaggerOptions->>SwaggerGen: options.SwaggerDoc(groupName, OpenApiInfo)
end
Updated class diagram for Swagger configuration and DI integrationclassDiagram
class AetherApiVersioningServiceCollectionExtensions {
+IServiceCollection AddAetherApiVersioning(services, apiTitle)
}
class ConfigureSwaggerOptions {
-IApiVersionDescriptionProvider _provider
-string _apiTitle
+ConfigureSwaggerOptions(IApiVersionDescriptionProvider provider, string apiTitle)
+void Configure(SwaggerGenOptions options)
}
class IServiceCollection {
<<interface>>
+IServiceCollection AddApiVersioning()
+IServiceCollection AddSwaggerGen()
+IServiceCollection AddTransient(serviceType, implementationFactory)
}
class IApiVersionDescriptionProvider {
<<interface>>
+IReadOnlyList ApiVersionDescriptions
}
class SwaggerGenOptions {
+void SwaggerDoc(name, OpenApiInfo info)
}
class OpenApiInfo {
+string Title
+string Version
+string Description
}
AetherApiVersioningServiceCollectionExtensions ..> IServiceCollection : extends
AetherApiVersioningServiceCollectionExtensions ..> ConfigureSwaggerOptions : registers
ConfigureSwaggerOptions ..|> IConfigureOptions_SwaggerGenOptions
ConfigureSwaggerOptions --> IApiVersionDescriptionProvider : uses
ConfigureSwaggerOptions --> SwaggerGenOptions : configures
SwaggerGenOptions --> OpenApiInfo : creates
File-Level Changes
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}
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- In
CachedServiceProviderBase, castingServiceProvidertoIKeyedServiceProviderintroduces a hard dependency on a specific provider implementation and can causeInvalidCastException; consider keeping the extension-based calls or guarding the cast with a type check/fallback. - In
RedisDistributedCacheService, the explicit(string)cachedValue!cast assumes all cached values are stored as strings; if any callers store non-string data, this will now fail—consider usingRedisValue’s byte[] conversion or validating the type before casting.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `CachedServiceProviderBase`, casting `ServiceProvider` to `IKeyedServiceProvider` introduces a hard dependency on a specific provider implementation and can cause `InvalidCastException`; consider keeping the extension-based calls or guarding the cast with a type check/fallback.
- In `RedisDistributedCacheService`, the explicit `(string)cachedValue!` cast assumes all cached values are stored as strings; if any callers store non-string data, this will now fail—consider using `RedisValue`’s byte[] conversion or validating the type before casting.
## Individual Comments
### Comment 1
<location> `framework/src/BBT.Aether.Core/BBT/Aether/DependencyInjection/CachedServiceProviderBase.cs:54` </location>
<code_context>
return CachedServices.GetOrAdd(
new ServiceIdentifier(serviceKey, serviceType),
- _ => new Lazy<object?>(() => ServiceProvider.GetKeyedService(serviceType, serviceKey))
+ _ => new Lazy<object?>(() => ((IKeyedServiceProvider)ServiceProvider).GetKeyedService(serviceType, serviceKey))
).Value;
}
</code_context>
<issue_to_address>
**issue (bug_risk):** Casting ServiceProvider to IKeyedServiceProvider may cause runtime failures with non-core DI containers.
This cast assumes `ServiceProvider` implements `IKeyedServiceProvider`; if an app uses a different container or a wrapper that doesn't, you'll get an `InvalidCastException` at runtime. Consider using a safer pattern (e.g., `ServiceProvider as IKeyedServiceProvider` with a fallback) or introducing an abstraction that alternative DI providers can implement.
</issue_to_address>
### Comment 2
<location> `framework/src/BBT.Aether.Core/BBT/Aether/DependencyInjection/CachedServiceProviderBase.cs:62` </location>
<code_context>
return CachedServices.GetOrAdd(
new ServiceIdentifier(serviceKey, serviceType),
- _ => new Lazy<object?>(() => ServiceProvider.GetRequiredKeyedService(serviceType, serviceKey))
+ _ => new Lazy<object?>(() => ((IKeyedServiceProvider)ServiceProvider).GetRequiredKeyedService(serviceType, serviceKey))
).Value!;
}
</code_context>
<issue_to_address>
**issue (bug_risk):** Same casting concern for GetRequiredKeyedService can lead to unexpected InvalidCastException.
This cast will throw if `ServiceProvider` doesn’t implement `IKeyedServiceProvider`, causing an `InvalidCastException` instead of the expected “missing keyed service” error. It’d be better to resolve keyed services via the same abstraction/extension pattern used elsewhere so behavior stays consistent across different containers.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| return CachedServices.GetOrAdd( | ||
| new ServiceIdentifier(serviceKey, serviceType), | ||
| _ => new Lazy<object?>(() => ServiceProvider.GetKeyedService(serviceType, serviceKey)) | ||
| _ => new Lazy<object?>(() => ((IKeyedServiceProvider)ServiceProvider).GetKeyedService(serviceType, serviceKey)) |
There was a problem hiding this comment.
issue (bug_risk): Casting ServiceProvider to IKeyedServiceProvider may cause runtime failures with non-core DI containers.
This cast assumes ServiceProvider implements IKeyedServiceProvider; if an app uses a different container or a wrapper that doesn't, you'll get an InvalidCastException at runtime. Consider using a safer pattern (e.g., ServiceProvider as IKeyedServiceProvider with a fallback) or introducing an abstraction that alternative DI providers can implement.
| return CachedServices.GetOrAdd( | ||
| new ServiceIdentifier(serviceKey, serviceType), | ||
| _ => new Lazy<object?>(() => ServiceProvider.GetRequiredKeyedService(serviceType, serviceKey)) | ||
| _ => new Lazy<object?>(() => ((IKeyedServiceProvider)ServiceProvider).GetRequiredKeyedService(serviceType, serviceKey)) |
There was a problem hiding this comment.
issue (bug_risk): Same casting concern for GetRequiredKeyedService can lead to unexpected InvalidCastException.
This cast will throw if ServiceProvider doesn’t implement IKeyedServiceProvider, causing an InvalidCastException instead of the expected “missing keyed service” error. It’d be better to resolve keyed services via the same abstraction/extension pattern used elsewhere so behavior stays consistent across different containers.
Summary by Sourcery
Upgrade the framework, infrastructure, and tooling to target .NET 10 and align supporting configuration and integrations with the new runtime and library patterns.
Bug Fixes:
Enhancements:
Build:
CI: