-
-
Notifications
You must be signed in to change notification settings - Fork 408
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add NRT to Csla-Core library #4528
base: main
Are you sure you want to change the base?
Conversation
NRT annotations adjusted as necessary
Please start reviewing the changes. I have some warnings to fix before merging but I think it'll take some time to review all changes 😅😆 |
I'm on holiday until March 4,and will start to look then. |
Great. Have nice holidays :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR Overview
This PR adds nullable reference annotations and updates related deserialization logic to improve type safety and error handling in the Csla-Core library. Key changes include:
- Replacing dependency injection–based instantiation of DataPortalResponse with direct constructor calls.
- Updating deserialization calls to throw when null, ensuring non-null results.
- Adjusting various helper and tag helper methods to incorporate nullable checks and improved error messages.
Reviewed Changes
File | Description |
---|---|
Source/Csla.Channels.Grpc/GrpcPortal.cs | Updated error handling and deserialization to enforce non-null returns. |
Source/Csla.AspNetCore/ConfigurationExtensions.cs | Added required using statements to support new annotations. |
Source/Csla.Channels.RabbitMq/* | Revised instantiation and nullability checks in DataPortal and RabbitMq methods. |
Source/Csla.AspNetCore/Blazor/* and other UI files | Adapted code for stricter type checks and improved error messaging. |
Source/Csla.Generators/* | Adjusted generated serialization code to correctly handle nullability. |
Copilot reviewed 327 out of 327 changed files in this pull request and generated no comments.
Comments suppressed due to low confidence (3)
Source/Csla.Blazor.WebAssembly.Tests/SessionManagerTests.cs:35
- The constructor call with an empty array may be unintended; please verify that passing an empty array to SessionMessage meets the expected behavior.
_sessionValue = new SessionMessage([])
Source/Csla.AspNetCore/Blazor/ApplicationContextManagerInMemory.cs:152
- The newly introduced check enforces that the principal must be a ClaimsPrincipal, which might affect callers passing other IPrincipal implementations; please confirm that this change is compatible with all intended usage scenarios.
if (principal is not ClaimsPrincipal claimsPrincipal)
Source/Csla.Channels.Grpc/GrpcProxy.cs:118
- Ensure that CreateOperationTag correctly handles a null routingToken, as it is now nullable, to avoid forming malformed operation tags.
private string CreateOperationTag(string operation, string? versionToken, string? routingToken)
Adding nullable reference annotations to the core csla project.
There are a lot of API changes which I tried to document in the added csla 10 upgrade document.