optimize validate logic

nager · Jan 13, 2025 · e85cc4f · e85cc4f
1 parent e20c581
commit e85cc4f
Show file tree

Hide file tree

Showing 4 changed files with 70 additions and 15 deletions.
diff --git a/src/Nager.EmailAuthentication.UnitTest/DmarcRecordParserTest.cs b/src/Nager.EmailAuthentication.UnitTest/DmarcRecordParserTest.cs
@@ -9,6 +9,7 @@ public void TryParse_InvalidDmarcString1_ReturnsTrueAndPopulatesDmarcRecord()
             var isSuccessful = DmarcRecordParser.TryParse("v=DMARC", out var dmarcDataFragment, out var parseErrors);
             Assert.IsTrue(isSuccessful);
             Assert.IsNotNull(dmarcDataFragment);
+            Assert.IsNotNull(parseErrors, "ParseErrors is null");
         }
 
         [TestMethod]
@@ -17,7 +18,7 @@ public void TryParse_InvalidDmarcString2_ReturnsTrueAndPopulatesDmarcRecord()
             var isSuccessful = DmarcRecordParser.TryParse("v=DMARC1", out var dmarcDataFragment, out var parseErrors);
             Assert.IsTrue(isSuccessful);
             Assert.IsNotNull(dmarcDataFragment);
-            Assert.IsNull(parseErrors);
+            Assert.IsNull(parseErrors, "ParseErrors is not null");
         }
 
         [TestMethod]
@@ -49,7 +50,7 @@ public void TryParse_ValidDmarcString1_ReturnsTrueAndPopulatesDmarcRecord()
             Assert.IsTrue(isSuccessful);
             Assert.IsNotNull(dmarcDataFragment);
             Assert.AreEqual("reject", dmarcDataFragment.DomainPolicy);
-            Assert.IsNull(parseErrors);
+            Assert.IsNull(parseErrors, "ParseErrors is not null");
         }
 
         [TestMethod]
@@ -60,7 +61,7 @@ public void TryParse_ValidDmarcString2_ReturnsTrueAndPopulatesDmarcRecord()
             Assert.IsNotNull(dmarcDataFragment);
             Assert.AreEqual("reject", dmarcDataFragment.DomainPolicy);
             Assert.AreEqual("none", dmarcDataFragment.SubdomainPolicy);
-            Assert.IsNull(parseErrors);
+            Assert.IsNull(parseErrors, "ParseErrors is not null");
         }
 
         [TestMethod]
@@ -74,7 +75,7 @@ public void TryParse_ValidDmarcString3_ReturnsTrueAndPopulatesDmarcRecord()
             Assert.AreEqual("100", dmarcDataFragment.PolicyPercentage);
             Assert.AreEqual("s", dmarcDataFragment.DkimAlignmentMode);
             Assert.AreEqual("s", dmarcDataFragment.SpfAlignmentMode);
-            Assert.IsNull(parseErrors);
+            Assert.IsNull(parseErrors, "ParseErrors is not null");
         }
 
         [TestMethod]
@@ -83,7 +84,7 @@ public void TryParse_CorruptDmarcString1_ReturnsTrueAndParseErrors()
             var isSuccessful = DmarcRecordParser.TryParse("verification=123456789", out var dmarcDataFragment, out var parseErrors);
             Assert.IsTrue(isSuccessful);
             Assert.IsNotNull(dmarcDataFragment);
-            Assert.IsNotNull(parseErrors);
+            Assert.IsNotNull(parseErrors, "ParseErrors is null");
             Assert.IsTrue(parseErrors.Length == 2);
         }
 
@@ -93,7 +94,18 @@ public void TryParse_CorruptDmarcString2_ReturnsFalse()
             var isSuccessful = DmarcRecordParser.TryParse(" ", out var dmarcDataFragment, out var parseErrors);
             Assert.IsFalse(isSuccessful);
             Assert.IsNull(dmarcDataFragment);
-            Assert.IsNull(parseErrors);
+            Assert.IsNull(parseErrors, "ParseErrors is not null");
+        }
+
+        [TestMethod]
+        public void TryParse_CorruptDmarcString3_ReturnsTrueWithErrors()
+        {
+            var isSuccessful = DmarcRecordParser.TryParse("v=DMARC1; p=reject; pct=200;", out var dmarcDataFragment, out var parseErrors);
+            Assert.IsTrue(isSuccessful);
+            Assert.IsNotNull(dmarcDataFragment);
+            Assert.AreEqual("reject", dmarcDataFragment.DomainPolicy);
+            Assert.AreEqual("200", dmarcDataFragment.PolicyPercentage);
+            Assert.IsNotNull(parseErrors, "ParseErrors is null");
         }
     }
 }
diff --git a/src/Nager.EmailAuthentication/DmarcRecordParser.cs b/src/Nager.EmailAuthentication/DmarcRecordParser.cs
@@ -94,14 +94,14 @@ public static bool TryParse(
                     "p", new MappingHandler
                     {
                         Map = value => dataFragment.DomainPolicy = value,
-                        Validate = ProcessDomainPolicy
+                        Validate = ValidateDomainPolicy
                     }
                 },
                 {
                     "sp", new MappingHandler
                     {
                         Map = value => dataFragment.SubdomainPolicy = value,
-                        Validate = ProcessDomainPolicy
+                        Validate = ValidateDomainPolicy
                     }
                 },
                 {
@@ -131,7 +131,8 @@ public static bool TryParse(
                 {
                     "pct", new MappingHandler
                     {
-                        Map = value => dataFragment.PolicyPercentage = value
+                        Map = value => dataFragment.PolicyPercentage = value,
+                        Validate = ValidatePolicyPercentage
                     }
                 },
                 {
@@ -165,7 +166,7 @@ public static bool TryParse(
                 {
                     if (handler.Validate != null)
                     {
-                        errors.AddRange([.. handler.Validate(keyValue.Value)]);
+                        errors.AddRange([.. handler.Validate(new ValidateRequest { Field = keyValue.Key, Value = keyValue.Value })]);
                     }
                     handler.Map(keyValue.Value ?? "");
 
@@ -185,29 +186,63 @@ public static bool TryParse(
             return true;
         }
 
-        private static ParseError[] ProcessDomainPolicy(string? data)
+        private static ParseError[] ValidateDomainPolicy(ValidateRequest validateRequest)
         {
-            if (string.IsNullOrEmpty(data))
+            if (string.IsNullOrEmpty(validateRequest.Value))
             {
                 return [];
             }
 
             var errors = new List<ParseError>();
 
             var domainPolicy = AllowedPolicies
-                .Where(policy => policy.Equals(data, StringComparison.OrdinalIgnoreCase))
+                .Where(policy => policy.Equals(validateRequest.Value, StringComparison.OrdinalIgnoreCase))
                 .SingleOrDefault();
 
             if (domainPolicy == null)
             {
                 errors.Add(new ParseError
                 {
                     Severity = ErrorSeverity.Error,
-                    ErrorMessage = $"Unknown policy \"{data}\""
+                    ErrorMessage = $"Unknown policy \"{validateRequest.Value}\""
                 });
             }
 
             return [.. errors];
         }
+
+        private static ParseError[] ValidatePolicyPercentage(ValidateRequest validateRequest)
+        {
+            if (string.IsNullOrEmpty(validateRequest.Value))
+            {
+                return [];
+            }
+
+            var errors = new List<ParseError>();
+
+            if (!int.TryParse(validateRequest.Value, out var percentage))
+            {
+                errors.Add(new ParseError
+                {
+                    Severity = ErrorSeverity.Error,
+                    ErrorMessage = $"{validateRequest.Field} value is not a number"
+                });
+
+                return [.. errors];
+            }
+
+            if (percentage < 0 || percentage > 100)
+            {
+                errors.Add(new ParseError
+                {
+                    Severity = ErrorSeverity.Error,
+                    ErrorMessage = $"{validateRequest.Field} value is not in allowed range"
+                });
+
+                return [.. errors];
+            }
+
+            return [];
+        }
     }
 }
diff --git a/src/Nager.EmailAuthentication/MappingHandler.cs b/src/Nager.EmailAuthentication/MappingHandler.cs
@@ -17,6 +17,6 @@ internal class MappingHandler
         /// Gets or sets the optional validation logic for the input string.
         /// Returns an array of <see cref="ParseError"/> objects indicating validation issues, or null if validation is not performed.
         /// </summary>
-        public Func<string?, ParseError[]>? Validate { get; set; }
+        public Func<ValidateRequest, ParseError[]>? Validate { get; set; }
     }
 }
diff --git a/src/Nager.EmailAuthentication/Models/ValidateRequest.cs b/src/Nager.EmailAuthentication/Models/ValidateRequest.cs
@@ -0,0 +1,8 @@
+namespace Nager.EmailAuthentication.Models
+{
+    internal class ValidateRequest
+    {
+        internal required string Field { get; set; }
+        internal string? Value { get; set; }
+    }
+}