Enhancement/mod11 ssn validation #796
Conversation
📝 WalkthroughWalkthroughThis pull request updates the handling of social security numbers within the application’s tests and core helper libraries. It modifies specific test values (reserved SSN and recipient identifiers), introduces a new test class for SSN validation, adds a new helper class for Mod 11 control digit calculation, and enhances the SSN validation method in the core string extensions to incorporate the new helper logic. Changes
Possibly related PRs
Suggested labels
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used🧠 Learnings (1)Test/Altinn.Correspondence.Tests/TestingController/Correspondence/CorrespondenceRetrievalTests.cs (1)⏰ Context from checks skipped due to timeout of 90000ms (1)
🔇 Additional comments (1)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/Altinn.Correspondence.Common/Helpers/Mod11.cs (1)
1-53: New Mod11 validation helper implementationThe implementation provides a clean, efficient way to validate numbers using the Mod11 algorithm, which is commonly used for validating identification numbers including social security numbers.
However, consider the following improvements:
- Add XML documentation comments explaining the purpose and usage of the Mod11 algorithm.
- Add input validation to handle null or empty input numbers.
- Add validation to ensure the weights array is non-empty.
using System.Diagnostics.CodeAnalysis; namespace Altinn.Correspondence.Common.Helpers; +/// <summary> +/// Helper class for calculating control digits using the Mod11 algorithm. +/// </summary> internal static class Mod11 { private const int Mod11Number = 11; private const int InvalidControlDigit = 10; + /// <summary> + /// Attempts to calculate a control digit for the given number using the Mod11 algorithm. + /// </summary> + /// <param name="number">The number for which to calculate the control digit.</param> + /// <param name="weights">The weights to apply to each digit in the number.</param> + /// <param name="controlDigit">When this method returns, contains the calculated control digit, or null if the calculation failed.</param> + /// <returns>True if the control digit was successfully calculated; otherwise, false.</returns> public static bool TryCalculateControlDigit(ReadOnlySpan<char> number, int[] weights, [NotNullWhen(true)] out int? controlDigit) { + if (number.IsEmpty || weights.Length == 0) + { + controlDigit = null; + return false; + } + var digits = number.ExtractDigits(); if (digits.Length != number.Length || digits.Length > weights.Length) { controlDigit = null; return false; } var sum = 0; for (var i = 0; i < digits.Length; i++) { sum += digits[i] * weights[i]; } controlDigit = Mod11Number - (sum % Mod11Number); controlDigit = controlDigit switch { Mod11Number => 0, InvalidControlDigit => null, _ => controlDigit }; return controlDigit is not null; } private static int[] ExtractDigits(this ReadOnlySpan<char> number) { var result = new int[number.Length]; var index = 0; foreach (var character in number) { if (char.IsDigit(character)) { result[index++] = character - '0'; } } Array.Resize(ref result, index); return result; } }src/Altinn.Correspondence.Common/Helpers/StringExtensions.cs (1)
44-49: Improve method documentation.The XML documentation is incomplete. Please enhance it to describe what constitutes a valid social security number (including the Mod11 check) and complete the
<returns>tag./// <summary> -/// Checks if a social security number is valid. +/// Checks if a social security number is valid by verifying it matches the expected +/// 11-digit format and validating both control digits using the Mod11 algorithm. /// </summary> /// <param name="identifier">The string to validate.</param> -/// <returns></returns> +/// <returns>True if the string is a valid social security number with correct control digits, false otherwise.</returns>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
Test/Altinn.Correspondence.Tests/Helpers/CustomWebApplicationFactory.cs(1 hunks)Test/Altinn.Correspondence.Tests/TestingController/Correspondence/CorrespondenceInitializationTests.cs(1 hunks)Test/Altinn.Correspondence.Tests/TestingUtility/StringExtensionsTests.cs(1 hunks)src/Altinn.Correspondence.Common/Helpers/Mod11.cs(1 hunks)src/Altinn.Correspondence.Common/Helpers/StringExtensions.cs(3 hunks)
🧰 Additional context used
🧠 Learnings (1)
Test/Altinn.Correspondence.Tests/TestingController/Correspondence/CorrespondenceInitializationTests.cs (1)
Learnt from: CelineTrammi
PR: Altinn/altinn-correspondence#533
File: Test/Altinn.Correspondence.Tests/TestingController/Correspondence/CorrespondenceAttachmentTests.cs:103-103
Timestamp: 2024-11-29T09:06:46.947Z
Learning: In the test cases, plain IDs without URN prefixes represent social security numbers (SSNs) and are allowed as recipients in the `WithRecipients` method.
🔇 Additional comments (7)
Test/Altinn.Correspondence.Tests/TestingController/Correspondence/CorrespondenceInitializationTests.cs (1)
433-433: SSN value has been updated to a valid Mod11 SSNThe personRecipient value has been updated from "01234567890" to "08900499559", which is now a valid social security number that passes Mod11 validation.
Test/Altinn.Correspondence.Tests/TestingUtility/StringExtensionsTests.cs (1)
1-33: Well-structured unit tests for SSN validationThe test class provides clear validation tests for both invalid and valid synthetic social security numbers using the new Mod11 validation. The tests follow the AAA (Arrange-Act-Assert) pattern and have clear naming conventions.
Test/Altinn.Correspondence.Tests/Helpers/CustomWebApplicationFactory.cs (1)
28-28: ReservedSsn constant updated to use valid Mod11 SSNThe ReservedSsn constant has been updated from an invalid value to "08900499559", which is a valid Mod11-compliant social security number. This maintains consistency with the other test cases.
src/Altinn.Correspondence.Common/Helpers/StringExtensions.cs (4)
1-1: Appropriate addition of System.Globalization namespace.The addition of this namespace is necessary to support the use of
CultureInfo.InvariantCulturein the new validation method.
8-9: LGTM: Properly defined weight arrays for Mod11 control digit calculation.These weight arrays are correctly defined for Norwegian SSN validation using the Mod11 algorithm.
18-18: Good refactoring: Improved validation logic.The method now properly delegates to the new, more thorough validation logic while maintaining its original interface.
50-56: Well-implemented Mod11 validation for SSN.The implementation correctly:
- Verifies the basic pattern (11 digits)
- Validates both control digits using the appropriate weight arrays
- Uses efficient string manipulation with Span
- Properly handles culture-invariant number parsing
This is a significant improvement over simple regex validation, as it ensures the SSN has valid check digits.
…ion logic in two methods
…cipient in initializeCorrespondenceExt to prevent Internal Server error
…pass the new ssn validation
Description
Related Issue(s)
Verification
Documentation
Summary by CodeRabbit
New Features
Tests
Refactor