-
Notifications
You must be signed in to change notification settings - Fork 26
Refactor: Reduce Cognitive Complexity in AddRolesFromClaim #595
Copy link
Copy link
Closed
Labels
C#C# related codeC# related codebugSomething isn't workingSomething isn't workingcode cleanupcleaning-up codecleaning-up codegood first issueGood for newcomersGood for newcomershelp wantedExtra attention is neededExtra attention is neededrefactoringRefactoring codeRefactoring codereliability
Description
What version of FlowSynx?
1.2.2
Describe the bug
The method currently has a Cognitive Complexity of 27, which exceeds the allowed threshold of 15. This makes the code harder to maintain, reason about, and test effectively.
File: src/FlowSynx/Security/RoleClaimsTransformation.cs
Method: AddRolesFromClaim
Current Cognitive Complexity: 27
Expected Complexity: 15
Proposed refactoring
private void AddRolesFromClaim(ClaimsIdentity identity, string claimType, HashSet<string> roles)
{
foreach (var claim in identity.FindAll(claimType))
{
if (string.IsNullOrWhiteSpace(claim.Value))
continue;
if (TryAddJsonArrayRoles(claim.Value, roles))
continue;
AddDelimitedOrSingleRole(claim.Value, roles);
}
}
private bool TryAddJsonArrayRoles(string claimValue, HashSet<string> roles)
{
if (!IsJsonArray(claimValue))
return false;
try
{
using var doc = JsonDocument.Parse(claimValue);
if (doc.RootElement.ValueKind != JsonValueKind.Array)
return false;
foreach (var element in doc.RootElement.EnumerateArray())
{
var role = element.GetString();
if (!string.IsNullOrEmpty(role))
roles.Add(role);
}
return true;
}
catch
{
// Ignore invalid JSON
return false;
}
}
private static bool IsJsonArray(string value)
{
return value.StartsWith("[") && value.EndsWith("]");
}
private static void AddDelimitedOrSingleRole(string claimValue, HashSet<string> roles)
{
if (claimValue.Contains(","))
{
foreach (var role in claimValue.Split(',', StringSplitOptions.RemoveEmptyEntries))
roles.Add(role.Trim());
}
else
{
roles.Add(claimValue);
}
}Acceptance Criteria
- Cognitive Complexity ≤ 15.
- Unit tests pass for all claim formats (if they exist, if not, it will be nice to add unit tests for them)
- Code readability and maintainability improved.
- No change in functional behavior.
Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
C#C# related codeC# related codebugSomething isn't workingSomething isn't workingcode cleanupcleaning-up codecleaning-up codegood first issueGood for newcomersGood for newcomershelp wantedExtra attention is neededExtra attention is neededrefactoringRefactoring codeRefactoring codereliability