Proposed fix for #1183 - multiple regex within short time might give …

…identical strings (#1187)

* Proposed fix for #1183 - creating multiple regex within short time might yield identical stringsr #1183

Co-authored-by: Nikos Baxevanis <nikos.baxevanis@gmail.com>

* Suggestions from codereview:
Added a test to FixtureTests, to verify that #1183 is fixed as suggested here: #1187 (comment)

This required making sure that the outcome space of the regex pattern is great enough. Xeger seems to favor shorter strings, and this gave collisions when generating RegularExpressionValidatedType . So minimum length of the pattern in this type was lengthened from 1 to 20.

Removed assertion roulette in RegularExpressionGeneratorTests.CreateMultipleWithRegularExpressionRequestReturnsDifferentResults(...)
according to suggestion: #1187 (comment)

Co-authored-by: Magnus Mikkelsen <mmi@kamstrup.dk>
Co-authored-by: Nikos Baxevanis <nikos.baxevanis@gmail.com>
Co-authored-by: MagnusMikkelsen <cactus1978@gmail.com>

Src/AutoFixture/RegularExpressionGenerator.cs

@@ -10,6 +10,18 @@ namespace AutoFixture
     /// </summary>
     public class RegularExpressionGenerator : ISpecimenBuilder
     {
+        private readonly Random random;
+        private readonly object syncRoot;
+
+        /// <summary>
+        /// Initializes a new instance of the <see cref="RegularExpressionGenerator"/> class.
+        /// </summary>
+        public RegularExpressionGenerator()
+        {
+            this.random = new Random();
+            this.syncRoot = new object();
+        }
+
         /// <summary>
         /// Creates a string that is guaranteed to match a RegularExpressionRequest.
         /// </summary>
@@ -28,16 +40,18 @@ public object Create(object request, ISpecimenContext context)
                 return new NoSpecimen();
             }
 
-            return GenerateRegularExpression(regularExpressionRequest);
+            return this.GenerateRegularExpression(regularExpressionRequest);
         }
 
-        private static object GenerateRegularExpression(RegularExpressionRequest request)
+        private object GenerateRegularExpression(RegularExpressionRequest request)
         {
             string pattern = request.Pattern;
 
             try
             {
-                string regex = new Xeger(pattern).Generate();
+                // Use the Xeger constructor overload that that takes an instance of Random.
+                // Otherwise identically strings can be generated, if regex are generated within short time.
+                string regex = new Xeger(pattern, new Random(this.GenerateSeed())).Generate();
                 if (Regex.IsMatch(regex, pattern))
                 {
                     return regex;
@@ -54,5 +68,13 @@ private static object GenerateRegularExpression(RegularExpressionRequest request
 
             return new NoSpecimen();
         }
+
+        private int GenerateSeed()
+        {
+            lock (this.syncRoot)
+            {
+                return this.random.Next();
+            }
+        }
     }
-}
+}

Src/AutoFixtureUnitTest/DataAnnotations/RegularExpressionValidatedType.cs

@@ -4,7 +4,7 @@ namespace AutoFixtureUnitTest.DataAnnotations
 {
     public class RegularExpressionValidatedType
     {
-        public const string Pattern = @"^[a-zA-Z''-'\s]{1,40}$";
+        public const string Pattern = @"^[a-zA-Z''-'\s]{20,40}$";
 
         [RegularExpression(Pattern)]
         public string Property { get; set; }

Src/AutoFixtureUnitTest/FixtureTest.cs

@@ -1636,6 +1636,21 @@ public void CreateAnonymousWithRegularExpressionValidatedTypeReturnsCorrectResul
             Assert.Matches(RegularExpressionValidatedType.Pattern, result.Property);
         }
 
+        [Fact]
+        public void CreateManyAnonymousWithRegularExpressionValidatedTypeReturnsDifferentResults()
+        {
+            // This test exposes an issue with Xeger/Random.
+            // Xeger(pattern) internally creates an instance of Random with the default seed.
+            // This means that the RegularExpressionGenerator might create identical strings
+            // if called multiple times within a short time.
+
+            // Arrange
+            var fixture = new Fixture();
+            var result = fixture.CreateMany<RegularExpressionValidatedType>(10).Select(x => x.Property).ToArray();
+            // Assert
+            Assert.Equal(result.Distinct(), result);
+        }
+
         [Fact]
         public void CreateAnonymousWithStringLengthValidatedTypeReturnsCorrectResult()
         {

Src/AutoFixtureUnitTest/RegularExpressionGeneratorTest.cs

@@ -1,4 +1,5 @@
-using System.Text.RegularExpressions;
+using System.Linq;
+using System.Text.RegularExpressions;
 using AutoFixture;
 using AutoFixture.Kernel;
 using AutoFixtureUnitTest.Kernel;
@@ -103,6 +104,27 @@ public void CreateWithRegularExpressionRequestReturnsCorrectResult(string patter
             Assert.True(Regex.IsMatch(result.ToString(), pattern), string.Format("result: {0}", result));
         }
 
+        [Theory]
+        [InlineData(@"^[A-Z]{27}$")]
+        public void CreateMultipleWithRegularExpressionRequestReturnsDifferentResults(string pattern)
+        {
+            // This test exposes an issue with Xeger/Random.
+            // Xeger(pattern) internally creates an instance of Random with the default seed.
+            // This means that the RegularExpressionGenerator might create identical strings
+            // if called multiple times within a short time.
+
+            // Arrange
+            var sut = new RegularExpressionGenerator();
+            var request = new RegularExpressionRequest(pattern);
+            var dummyContext = new DelegatingSpecimenContext();
+
+            // Repeat a few times to make the test more robust.
+            // Use ToArray to iterate the IEnumerable at this point.
+            var result = Enumerable.Range(0, 10).Select(_ => sut.Create(request, dummyContext)).ToArray();
+
+            Assert.Equal(result.Distinct(), result);
+        }
+
         [Theory]
         [InlineData("[")]
         [InlineData(@"(?\[Test\]|\[Foo\]|\[Bar\])?(?:-)?(?\[[()a-zA-Z0-9_\s]+\])?(?:-)?(?\[[a-zA-Z0-9_\s]+\])?(?:-)?(?\[[a-zA-Z0-9_\s]+\])?(?:-)?(?\[[a-zA-Z0-9_\s]+\])?")]