Skip to content
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

Open
wants to merge 61 commits into
base: main
Choose a base branch
from

Conversation

StefanOssendorf
Copy link
Contributor

@StefanOssendorf StefanOssendorf commented Feb 17, 2025

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.

@StefanOssendorf StefanOssendorf changed the title Add NRT to our Csla-Core library Add NRT to Csla-Core library Feb 17, 2025
@StefanOssendorf StefanOssendorf marked this pull request as ready for review February 22, 2025 14:03
@StefanOssendorf
Copy link
Contributor Author

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 😅😆

@rockfordlhotka
Copy link
Member

I'm on holiday until March 4,and will start to look then.

@StefanOssendorf
Copy link
Contributor Author

I'm on holiday until March 4,and will start to look then.

Great. Have nice holidays :)

@rockfordlhotka rockfordlhotka requested a review from Copilot March 3, 2025 17:27

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)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants