Changed custom pattern handling to not silently ignore errors and other minor improvements (#53)

* Changed custom pattern handling to not silently ignore errors, but at least store them in a violations list

* Updated #53 to include the recommended code changes by the repository owner

Author: sandreas

src/Grok.Net.Tests/Grok.Net.Tests.csproj

@@ -27,6 +27,9 @@
     <None Update="Resources\grok-custom-patterns">
       <CopyToOutputDirectory>Always</CopyToOutputDirectory>
     </None>
+    <None Update="Resources\grok-custom-patterns-invalid">
+      <CopyToOutputDirectory>Always</CopyToOutputDirectory>
+    </None>
     <None Update="Resources\example-log-file">
       <CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
     </None>

src/Grok.Net.Tests/Resources/grok-custom-patterns

@@ -1,5 +1,3 @@
 ZIPCODE [1-9]{1}[0-9]{2}\s{0,1}[0-9]{3}
 FLOAT [+-]?([0-9]*[.,])?[0-9]+
-WRONGPATTERN1 \\\\\
-WRONGPATTERN2 \\\\\
 WRONGFLOAT .*

src/Grok.Net.Tests/Resources/grok-custom-patterns-invalid

@@ -0,0 +1,2 @@
+WRONGPATTERN1 \\\\\
+WRONGPATTERN2 \\\\\

src/Grok.Net.Tests/UnitTests.cs

@@ -1,4 +1,5 @@
 using System;
+using System.Collections.Generic;
 using System.IO;
 using GrokNet;
 using Xunit;
@@ -9,7 +10,8 @@ public class UnitTests
     {
         private static Stream ReadCustomFile() =>
             File.OpenRead($"Resources{Path.DirectorySeparatorChar}grok-custom-patterns");
-
+        private static Stream ReadCustomFileWithInvalidPatterns() =>
+            File.OpenRead($"Resources{Path.DirectorySeparatorChar}grok-custom-patterns-invalid");
         [Fact]
         public void Parse_Empty_Logs_Not_Throws()
         {
@@ -127,7 +129,7 @@ public void Parse_IPv4_Pattern()
         [InlineData("2001:db8:85a3:0:0:8a2e:370:7334")]
         [InlineData("2001:db8:85a3::8a2e:370:7334")]
         [InlineData("::1")] // Loopback
-        [InlineData("::")]  // Default route
+        [InlineData("::")] // Default route
         public void Parse_IPv6_Pattern(string ipAddress)
         {
             // Arrange
@@ -196,29 +198,34 @@ public void Load_Custom_Patterns(string zipcode)
             Assert.Equal(email, grokResult[1].Value);
         }
 
-        [Fact]
-        public void Load_Wrong_Custom_Patterns()
+        [Theory]
+        [InlineData("122001")]
+        [InlineData("122 001")]
+        [InlineData("235 012")]
+        public void Load_Custom_Patterns_From_IDictionary(string zipcode)
         {
             // Arrange
-            const string client = "192.168.1.1";
-            const string duration = "1";
+            var customPatterns = new Dictionary<string, string>
+            {
+                { "ZIPCODE", "[1-9]{1}[0-9]{2}\\s{0,1}[0-9]{3}" }, 
+                { "FLOAT", "[+-]?([0-9]*[.,]}?[0-9]+)" }
+            };
+            const string email = "[email protected]";
 
-            var sut = new Grok("%{WRONGPATTERN1:duration}:%{WRONGPATTERN2:client}", ReadCustomFile());
+            var sut = new Grok("%{ZIPCODE:zipcode}:%{EMAILADDRESS:email}", customPatterns);
 
-            try
-            {
-                // Act
-                GrokResult grokResult = sut.Parse($"{duration}:{client}");
-
-                // Assert (checks if regex is invalid)
-                Assert.Equal("", grokResult[0].Value);
-                Assert.Equal("", grokResult[1].Value);
-            }
-            catch
-            {
-                // Assert (checks if pattern is invalid)
-                Assert.Throws<FormatException>(() => sut.Parse($"{duration}:{client}"));
-            }
+            // Act
+            GrokResult grokResult = sut.Parse($"{zipcode}:{email}");
+
+            // Assert
+            Assert.Equal(zipcode, grokResult[0].Value);
+            Assert.Equal(email, grokResult[1].Value);
+        }
+
+        [Fact]
+        public void Load_Invalid_Custom_Patterns()
+        {
+            Assert.Throws<FormatException>(() => new Grok("%{WRONGPATTERN1:duration}:%{WRONGPATTERN2:client}", ReadCustomFileWithInvalidPatterns()));
         }
 
         [Fact]
@@ -247,7 +254,7 @@ public void Parse_Pattern_With_Type_Should_Parse_To_Specified_Type()
             Assert.Equal(floatValue, grokResult[2].Value);
             Assert.IsType<double>(grokResult[2].Value); // Float converts to double actually
         }
-        
+
         [Theory]
         [InlineData("INT", "2147483648", "int")]
         [InlineData("DATESTAMP", "11-31-2021 02:08:58", "datetime")]
@@ -264,5 +271,6 @@ public void Parse_With_Type_Parse_Exception_Should_Ignore_Type(string regex, str
             Assert.Equal(nameof(parse), grokResult[0].Key);
             Assert.Equal(parse, grokResult[0].Value);
         }
+        
     }
-}
+}

src/Grok.Net/Grok.cs

@@ -19,12 +19,13 @@ public class Grok
         private static readonly Regex _grokRegex = new Regex("%{(\\w+):(\\w+)(?::\\w+)?}", RegexOptions.Compiled);
         private static readonly Regex _grokRegexWithType = new Regex("%{(\\w+):(\\w+):(\\w+)?}", RegexOptions.Compiled);
         private static readonly Regex _grokWithoutName = new Regex("%{(\\w+)}", RegexOptions.Compiled);
-
+        
         public Grok(string grokPattern)
         {
             _grokPattern = grokPattern;
             _patterns = new Dictionary<string, string>();
             _typeMaps = new Dictionary<string, string>();
+            
             LoadPatterns();
         }
 
@@ -33,6 +34,29 @@ public Grok(string grokPattern, Stream customPatterns)
         {
             LoadCustomPatterns(customPatterns);
         }
+        
+        public Grok(string grokPattern, IDictionary<string,string> customPatterns)
+            :this(grokPattern)
+        {
+            AddPatterns(customPatterns);
+        }
+
+        private void AddPatterns(IDictionary<string,string> customPatterns)
+        {
+            foreach (var pattern in customPatterns)
+            {
+                AddPatternIfNotExists(pattern.Key, pattern.Value);                    
+            }
+        }
+        
+        private void AddPatternIfNotExists(string key, string value)
+        {
+            if (!_patterns.ContainsKey(key))
+            {
+                EnsurePatternIsValid(value);
+                _patterns.Add(key, value);
+            }
+        }
 
         public GrokResult Parse(string text)
         {
@@ -149,26 +173,25 @@ private void ProcessPatternLine(string line)
             string[] strArray = line.Split(new[] { ' ' }, 2);
             if (strArray.Length != 2)
             {
-                throw new FormatException("Custom pattern was not in a correct form");
+                throw new FormatException("Pattern line was not in a correct form (two strings split by space)");
             }
 
-            if (strArray[0].Equals("#", StringComparison.OrdinalIgnoreCase))
+            if (strArray[0].StartsWith("#"))
             {
                 return;
             }
+            AddPatternIfNotExists(strArray[0], strArray[1]);
+        }
+
+        private void EnsurePatternIsValid(string pattern)
+        {
             try
             {
-                Regex.Match("", strArray[1]);
+                _ = Regex.Match("", pattern);
             }
-            catch
+            catch(Exception e)
             {
-                return;
-            }
-
-            // check before adding to avoid an exception in case the same pattern is present in the custom patterns file.
-            if (!_patterns.ContainsKey(strArray[0]))
-            {
-                _patterns.Add(strArray[0], strArray[1]);
+                throw new FormatException($"Invalid regular expression {pattern}", e);
             }
         }
 
@@ -177,12 +200,7 @@ private string ReplaceWithName(Match match)
             Group group1 = match.Groups[2];
             Group group2 = match.Groups[1];
 
-            if (_patterns.TryGetValue(group2.Value, out var str))
-            {
-                return $"(?<{group1}>{str})";
-            }
-
-            return $"(?<{group1}>)";
+            return _patterns.TryGetValue(group2.Value, out var str) ? $"(?<{group1}>{str})" : $"(?<{group1}>)";
         }
 
         private string ReplaceWithoutName(Match match)