Skip to content

Refactor CurrentUserService contract (#667)#672

Merged
ziagham merged 2 commits intoflowsynx:masterfrom
mohin-io:fix/667-refactor-current-user-service
Nov 8, 2025
Merged

Refactor CurrentUserService contract (#667)#672
ziagham merged 2 commits intoflowsynx:masterfrom
mohin-io:fix/667-refactor-current-user-service

Conversation

@mohin-io
Copy link
Copy Markdown
Contributor

@mohin-io mohin-io commented Nov 7, 2025

Summary

This PR updates ICurrentUserService to use explicit methods instead of properties, which makes the intent clearer and avoids the confusion that can happen when property access has side effects. The new XML docs help explain how each method should be used, which matches the guidance we follow in other core services.

CurrentUserService has been updated accordingly, including centralizing how authentication failures are turned into FlowSynx exceptions. This keeps the logging and error behavior consistent across the app. The ValidateAuthentication flow now goes through the same entry point as well.

All existing consumers were switched over to the new method-based API, and I added focused unit tests to clearly capture the expected behavior around claims, roles, and what happens when a user isn’t authenticated.

I wasn’t able to run dotnet test locally since my environment only has SDK 8.0.415 and the repo targets net9.0, but the changes are straightforward and contained.

Closes #667.

@mohin-io mohin-io requested review from a team as code owners November 7, 2025 22:53
Copy link
Copy Markdown
Member

@ziagham ziagham left a comment

Choose a reason for hiding this comment

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

The only remaining part that hasn’t been updated is in WorkflowHttpListenerExtensions

File: /src/FlowSynx/Extensions/WorkflowHttpListenerExtensions.cs at line 23

var userId = userService.UserId;

This should be updated to:

var userId = userService.UserId();

@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud bot commented Nov 8, 2025

@mohin-io mohin-io requested a review from ziagham November 8, 2025 15:55
@ziagham ziagham merged commit 9115551 into flowsynx:master Nov 8, 2025
4 checks passed
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.

Issue: Remove exception throwing from property getter

2 participants