-
-
Notifications
You must be signed in to change notification settings - Fork 213
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
GUID Pattern support in RegexMatcher #699
Conversation
Status check shows a failed unit test. The test that failed I did not touch and when I run all tests locally I am unable to recreate the failure. |
Sometimes that test fails... I did update that test just now I did rerun your CI build and it is ok Maybe you can merge latest master into your PR branch? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a lot.
1] take a look at my comments
2] I did not check all regex, but is there also a simple GUID match which allows any casing? Like AAAAa1ff-2f1e-4f44-a053-e2c34a7ccBBB
?
3] Maybe it's good to replace all existing Regex usages in WireMock with this new one.
4] I'm thinking if we need to make using this new Regex configurable via the wiremock settings maybe?
/// Token for a GUID formated with `B` format specifier with lower case | ||
/// values. | ||
/// </summary> | ||
public const string GuidBLowerToken = @"\guidb"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
An easier and (smaller code) approach on this would be to define a dictionary with the tokens and regex.
Like:
private static readonly IDictionary<string,string> _replacements = new Dictionary<string, string>
{
{ @"\guidb", @"(\{[a-z0-9]{8}-([a-z0-9]{4}-){3}[a-z0-9]{12}\})" }, // Regular expression pattern associated with the expected format for `B` format specifier with lower case.
// . . .
}
using System.Text.RegularExpressions; | ||
using System.Threading.Tasks; | ||
|
||
namespace WireMock.RegularExpressions |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
General: you can just write comments / code on 1 line, no need to break at 80 characters.
/// <param name="pattern"> | ||
/// Pattern to replace token for. | ||
/// </param> | ||
private static string ReplaceGuidPattern(string pattern) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When using the dictionary, this can be simpler.
And a null check should be added.
private static string ReplaceGuidPattern(string pattern)
{
Check.NotNull(pattern, nameof(pattern));
foreach (var replacement in _replacements)
{
pattern = pattern.Replace(replacement.Key, replacement.Value);
}
}
I thought about this, but since it is a
Are you requesting I change all references to the
I'm not sure what you mean here, would you be able to point to a sample? (Sorry, I'm still new to Wiremock and have only used JSON files for configuring the dotnet tool). |
2] 3 and 4] And maybe add an property like
And the default value is set to true, but if needed, a user can set this to false. |
@brogdogg |
Adding a new
RegexGuid
class, extending theSystem.Text.RegularExpressions.Regex
class, to support patterns for identifying a GUID with respect to the way the System.Guid.ToString() formats a new GUID.Additionally, the
RegexMatcher
was updated to use this newRegexGuid
class.Potential use:
This is a suggested change to address feedback to #685