Feature/asynchronous authorization rules #3956
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ion support Updated the `ISessionManager` interface and `SessionManager` class to include a `TimeSpan` parameter in the `RetrieveSession` and `SendSession` methods for timeout specification. Overloads of these methods have been added to accept a `CancellationToken` parameter for operation cancellation. The `StateManager` class's `GetState` and `SaveState` methods have also been updated to accept a `TimeSpan` parameter for timeout. Added a new project `Csla.Blazor.WebAssembly.Tests` for unit tests of the `SessionManager` class, including tests for timeout and cancellation scenarios. The solution file and a new project settings file have been updated accordingly. The `SessionManager` class now handles `OperationCanceledException` by rethrowing the exception.
Refactored `SessionManagerTests.cs` and `SessionManager.cs` to improve session handling. The `SessionMessage` object has been changed from a local variable to a class-level variable, allowing it to be accessed across different methods. The `RetrieveSession` and `SendSession` methods have been updated to return the session and assert that the returned session is equal to `SessionValue.Session`. The exception type expected in these methods with zero timeout and cancelled cancellation token has been changed from `TaskCanceledException` to `TimeoutException`. The `GetCancellationToken` method has been simplified to create a `CancellationTokenSource` with a timeout directly. In `StateManager.cs`, the `GetState` method has been updated to not return a `Session` object, instead, it just retrieves the session without assigning it to a variable.
This commit includes a significant refactoring of the `SessionManagerTests.cs` file. The `GetHttpClient` method has been simplified by removing the creation of a new `MemoryStream` instance and the use of `CslaBinaryWriter`. The stream's position is no longer reset to 0, and the array is directly obtained from the stream. Several test methods have been removed, including `RetrieveSession_WithTimeoutValue_ShouldNotThrowException`, `RetrieveSession_WithValidCancellationToken_ShouldNotThrowException`, and `SendSession_WithZeroTimeout_ShouldThrowTimeoutException`. These tests were checking for specific exceptions or session values. The `SendSession_WithTimeoutValue_ShouldNotThrowException` test method has been modified by removing the assertion that was previously checking if the operation is successful. Lastly, the `RetrieveCachedSessionSession` method has been modified by removing the call to `GetCachedSession` method of `_sessionManager`.
Updated `SessionManager.cs` methods `RetrieveSession` and `SendSession` to handle `TaskCanceledException` internally and rethrow as `TimeoutException`. Simplified `SendSession` by removing exception handling and refactored `RetrieveSession` to move `stateResult` handling outside of try-catch block. Renamed test methods in `SessionManagerTests.cs` to reflect these changes and updated expected exception type.
` ` `The SaveState method in StateManager.cs has been converted from a synchronous method to an asynchronous one. This is indicated by the addition of the async keyword and the change in return type from void to Task. Additionally, the call to _sessionManager.SendSession(timeout) within the SaveState method has been updated to use the await keyword, making this method call awaited, in line with the change to make SaveState an asynchronous method.`
Updated `Grpc.Net.Client` and `Grpc.Tools` packages in `Csla.Channels.Grpc.csproj`. Refactored `InitializeUser` method in `ApplicationContextManagerBlazor.cs` and `ApplicationContextManagerInMemory.cs` to be asynchronous. Removed `System.Transactions` reference from `Csla.Tests.csproj` and deleted `DataPortalTestDatabaseDataSet.Designer.cs` and related files. Added new test classes `HttpProxyExtensionsTests.cs` and `HttpProxyOptionsExtensionsTests.cs`. Removed `packages.config.old` file. Refactored `HttpProxy.cs` and `HttpProxyExtensions.cs` and added new property and methods in `HttpProxyOptions.cs`. Removed "Csla.Generators.CSharp" project from the solution "csla.test.sln" and its dependencies.
…error handling In this commit, several updates were made to the `SessionManager.cs` and `SessionManagerTests.cs` files. The variable `_sessionManager` was renamed to `_SessionManager` in `SessionManagerTests.cs`. The `Initialize` method was converted to an asynchronous method, and the `RetrieveSession` method call in it was updated to use `await`. XML comments were added to the `RetrieveSession`, `SendSession`, and `GetSession` methods in `SessionManager.cs` for better code documentation. The `RetrieveSession` and `SendSession` methods were updated to handle `TaskCanceledException` and throw a `TimeoutException` with a custom message. The `GetSession` method was updated to handle the case where `_session` is `null`, creating and returning a new `Session` object in this case. The `SendSession` method was updated to serialize the `_session` object and send it to the server if `SyncContextWithServer` is `true`. Finally, the `RetrieveSession` method was updated to retrieve the session from the server if `SyncContextWithServer` is `true`, deserializing and storing the retrieved session in `_session` or calling `GetSession` to get or create a new session if the retrieval is unsuccessful.
Refactored the `Initialize` method in `SessionManagerTests.cs` to be synchronous and removed the `RetrieveSession` call. The `RetrieveSession` call has been added to four test methods to ensure session retrieval before each test. Renamed and converted `RetrieveCAchedSessionSession` to an asynchronous method, adding a `RetrieveSession` call and an assertion for non-null cached sessions. Added a new test method `RetrieveNullCachedSessionSession` to assert null cached sessions.
Updated variable names `_SessionManager` and `SessionValue` to `_sessionManager` and `_sessionValue` respectively, to adhere to the common C# naming convention for private fields. All instances of these variables in the code, including in the `Initialize()`, `Deserialize()`, `RetrieveSession()`, `SendSession()`, and `GetCachedSession()` methods, as well as in test assertions, have been updated accordingly.
This commit represents a significant shift in the mocking framework used for unit testing in the `Csla.Blazor.WebAssembly.Tests.csproj` project. The `Moq` package has been replaced with `NSubstitute` in the project file and throughout the `SessionManagerTests.cs` file. This includes changes in the way mocks are created, set up, and how return values are specified for mocked methods and properties. Additionally, a new `TestHttpMessageHandler` class has been added to `SessionManagerTests.cs` to mock the behavior of an `HttpClient`. The `GetHttpClient` method has been updated to use this new class, aligning with the switch from `Moq` to `NSubstitute`.
Updated various methods across multiple files to support asynchronous operations. This includes updating `HandleRequirementAsync`, `Authorize`, `DoCreateAsync`, `DoFetchAsync`, `DoExecuteAsync`, `DoUpdateAsync`, `DoDeleteAsync`, and `HasPermission` methods to their asynchronous versions. Also, the `IAuthorizationRuleAsync` interface and `IAuthorizeDataPortal` interface have been updated to support async operations. The `cancellationToken` parameter has been renamed to `ct` in `IAuthorizationRuleAsync`. Refactor authorization rule system and improve code quality Refactored the authorization rule system by replacing `IAuthorizationRule` with `IAuthorizationRuleBase` in several files. Simplified the setter of `CacheResult` property in `AuthorizationRule.cs` and updated `IAuthorizationRule.Element` and `IAuthorizationRule.Action` properties to `IAuthorizationRuleBase.Element` and `IAuthorizationRuleBase.Action` respectively. Updated `BusinessRules.cs` to change the execution of authorization rules and improve the formatting of LINQ queries and conditional statements. Added `IAuthorizationRuleBase.cs` with a new interface and two new interfaces inheriting from it, `IAuthorizationRule` and `IAuthorizationRuleAsync`, with synchronous and asynchronous `Execute` methods respectively.
|
@luizbicalhoagl, I don't have any immediate concerns about the changes, as long as everything still works 😄 And by "works" what I mean is that pages that use the API synchronously today keep working, as do any consumers using the API in an async manner. Or maybe the Blazor pages already were using it "async" even though the implementation was sync? That may be the case, and so the changes should be transparent. The biggest risk I see is with changes to the core |
Updated the `Element` property in the `AuthorizationRule.cs` file from `protected` to `public`, allowing access from any class. Also, removed the `IAuthorizationRule.Element` property implementation from the `IAuthorizationRule.Execute(IAuthorizationContext context)` method, making the `Element` property no longer accessible through the `IAuthorizationRule` interface.
The import order in BusyStatusTests.cs has been rearranged for better code organization and readability. A potential syntax error due to the removal of a closing bracket in the ListTestSaveWhileBusyNetOnly() method has been noted. An event handler for items[0].ValidationComplete has been added in the ListTestSaveWhileNotBusyNetOnly() method to assert that items is not busy and is savable when the validation is complete. Whitespace changes have been made in the WaitForIdle_WhenBusyWillWaitUntilNotBusyAnymore() and WaitForIdle_WhenReachingTheTimeoutATimeoutExceptionIsThrown() methods for improved readability. No visible changes were made in the ListTestSaveWhileNotBusyNoActiveRuleNetOnly() method.
Thanks for the feedback I did my best to create the async methods without impacting on the previous ones If it's ok for you I can create the tests for some async authorization rules. @StefanOssendorf do you have something to add? |
|
The original request came from @mtavares628, so perhaps he has a good example for testing? I think the real scenario is an authn rule that needs to do some async data portal command - like read from a database or other backend service. You can simulate this by creating a rule that does a |
|
@luizfbicalho, @rockfordlhotka hit the nail on the head with my use case. I want to create an authorization rule that is able to perform a FetchAsync or ExecuteAsync to retrieve a value back from the database to determine whether the user is authorized or not. The criteria for that fetch would be passed in to the HasPermissionAsync method as the criteria object. You could probably extend the tests from PR #1226 to simulate the fetch. While it just checks the type of the criteria object in those tests, you could instead use the value from the criteria to perform the fetch. |
|
@luizfbicalho let me know when you have added tests for this |
I'll finish them thiis week |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I just added the basic idea so we could talk about the whole spec.
Didn't add the tests because of that
#2801