From 61ac832d40b68e888c60226009aa0fc82db04506 Mon Sep 17 00:00:00 2001 From: Stef Heyenrath Date: Mon, 16 Sep 2019 14:54:27 +0000 Subject: [PATCH] Fix CompareTo in RequestMatchResult #344 --- .../Matchers/Request/MatchDetail.cs | 11 ++++ .../Matchers/Request/RequestMatchResult.cs | 21 ++++---- src/WireMock.Net/Owin/MappingMatcher.cs | 5 +- .../Serialization/LogEntryMapper.cs | 8 +-- .../Matchers/RequestMatchResultTests.cs | 54 +++++++++++++++++++ .../Owin/MappingMatcherTests.cs | 42 ++++++++++++--- 6 files changed, 117 insertions(+), 24 deletions(-) create mode 100644 src/WireMock.Net/Matchers/Request/MatchDetail.cs create mode 100644 test/WireMock.Net.Tests/Matchers/RequestMatchResultTests.cs diff --git a/src/WireMock.Net/Matchers/Request/MatchDetail.cs b/src/WireMock.Net/Matchers/Request/MatchDetail.cs new file mode 100644 index 000000000..1aa39c7ab --- /dev/null +++ b/src/WireMock.Net/Matchers/Request/MatchDetail.cs @@ -0,0 +1,11 @@ +using System; + +namespace WireMock.Matchers.Request +{ + public class MatchDetail + { + public Type MatcherType { get; set; } + + public double Score { get; set; } + } +} \ No newline at end of file diff --git a/src/WireMock.Net/Matchers/Request/RequestMatchResult.cs b/src/WireMock.Net/Matchers/Request/RequestMatchResult.cs index c6a5e26ff..45254b70a 100644 --- a/src/WireMock.Net/Matchers/Request/RequestMatchResult.cs +++ b/src/WireMock.Net/Matchers/Request/RequestMatchResult.cs @@ -1,5 +1,6 @@ using System; using System.Collections.Generic; +using System.Linq; namespace WireMock.Matchers.Request { @@ -14,7 +15,7 @@ public class RequestMatchResult : IComparable /// /// The match-score. /// - public double TotalScore { get; private set; } + public double TotalScore => MatchDetails.Sum(md => md.Score); /// /// Gets or sets the total number of matches. @@ -22,7 +23,7 @@ public class RequestMatchResult : IComparable /// /// The total number of matches. /// - public int TotalNumber { get; private set; } + public int TotalNumber => MatchDetails.Count; /// /// Gets or sets a value indicating whether this instance is perfect match. @@ -43,12 +44,7 @@ public class RequestMatchResult : IComparable /// /// Gets the match details. /// - public IList> MatchDetails { get; } - - /// - /// Initializes a new instance of the class. - /// - public RequestMatchResult() => MatchDetails = new List>(); + public IList MatchDetails { get; } = new List(); /// /// Adds the score. @@ -58,9 +54,7 @@ public class RequestMatchResult : IComparable /// The score. public double AddScore(Type matcherType, double score) { - TotalScore += score; - TotalNumber++; - MatchDetails.Add(new KeyValuePair(matcherType, score)); + MatchDetails.Add(new MatchDetail { MatcherType = matcherType, Score = score }); return score; } @@ -76,7 +70,10 @@ public int CompareTo(object obj) { var compareObj = (RequestMatchResult)obj; - return compareObj.AverageTotalScore.CompareTo(AverageTotalScore); + int averageTotalScoreResult = compareObj.AverageTotalScore.CompareTo(AverageTotalScore); + + // In case the score is equal, prefer the one with the most matchers. + return averageTotalScoreResult == 0 ? compareObj.TotalNumber.CompareTo(TotalNumber) : averageTotalScoreResult; } } } \ No newline at end of file diff --git a/src/WireMock.Net/Owin/MappingMatcher.cs b/src/WireMock.Net/Owin/MappingMatcher.cs index f34b5b4a0..c5b26e80e 100644 --- a/src/WireMock.Net/Owin/MappingMatcher.cs +++ b/src/WireMock.Net/Owin/MappingMatcher.cs @@ -49,8 +49,9 @@ public MappingMatcherResult FindBestMatch(RequestMessage request) } return mappings - .OrderBy(m => m.Mapping.Priority) - .FirstOrDefault(m => m.RequestMatchResult.IsPerfectMatch); + .Where(m => m.RequestMatchResult.IsPerfectMatch) + .OrderBy(m => m.Mapping.Priority).ThenBy(m => m.RequestMatchResult) + .FirstOrDefault(); } } } \ No newline at end of file diff --git a/src/WireMock.Net/Serialization/LogEntryMapper.cs b/src/WireMock.Net/Serialization/LogEntryMapper.cs index 0cd9adf21..cda15bab1 100644 --- a/src/WireMock.Net/Serialization/LogEntryMapper.cs +++ b/src/WireMock.Net/Serialization/LogEntryMapper.cs @@ -108,14 +108,14 @@ public static LogEntryModel Map(LogEntry logEntry) Response = logResponseModel, RequestMatchResult = logEntry.RequestMatchResult != null ? new LogRequestMatchModel { + IsPerfectMatch = logEntry.RequestMatchResult.IsPerfectMatch, TotalScore = logEntry.RequestMatchResult.TotalScore, TotalNumber = logEntry.RequestMatchResult.TotalNumber, - IsPerfectMatch = logEntry.RequestMatchResult.IsPerfectMatch, AverageTotalScore = logEntry.RequestMatchResult.AverageTotalScore, - MatchDetails = logEntry.RequestMatchResult.MatchDetails.Select(x => new + MatchDetails = logEntry.RequestMatchResult.MatchDetails.Select(md => new { - Name = x.Key.Name.Replace("RequestMessage", string.Empty), - Score = x.Value + Name = md.MatcherType.Name.Replace("RequestMessage", string.Empty), + Score = md.Score } as object).ToList() } : null }; diff --git a/test/WireMock.Net.Tests/Matchers/RequestMatchResultTests.cs b/test/WireMock.Net.Tests/Matchers/RequestMatchResultTests.cs new file mode 100644 index 000000000..c48d7a32c --- /dev/null +++ b/test/WireMock.Net.Tests/Matchers/RequestMatchResultTests.cs @@ -0,0 +1,54 @@ +using System.Linq; +using FluentAssertions; +using WireMock.Matchers; +using WireMock.Matchers.Request; +using Xunit; + +namespace WireMock.Net.Tests.Matchers +{ + public class RequestMatchResultTests + { + [Fact] + public void CompareTo_WithDifferentAverageScore_ReturnsBestMatch() + { + // Arrange + var result1 = new RequestMatchResult(); + result1.AddScore(typeof(WildcardMatcher), 1); + result1.AddScore(typeof(WildcardMatcher), 0.9); + + var result2 = new RequestMatchResult(); + result2.AddScore(typeof(LinqMatcher), 1); + result2.AddScore(typeof(LinqMatcher), 1); + + var results = new[] { result1, result2 }; + + // Act + var best = results.OrderBy(x => x).First(); + + // Assert + best.Should().Be(result2); + } + + [Fact] + public void CompareTo_WithSameAverageScoreButMoreMatchers_ReturnsMatchWithMoreMatchers() + { + // Arrange + var result1 = new RequestMatchResult(); + result1.AddScore(typeof(WildcardMatcher), 1); + result1.AddScore(typeof(WildcardMatcher), 1); + + var result2 = new RequestMatchResult(); + result2.AddScore(typeof(LinqMatcher), 1); + result2.AddScore(typeof(LinqMatcher), 1); + result2.AddScore(typeof(LinqMatcher), 1); + + var results = new[] { result1, result2 }; + + // Act + var best = results.OrderBy(x => x).First(); + + // Assert + best.Should().Be(result2); + } + } +} \ No newline at end of file diff --git a/test/WireMock.Net.Tests/Owin/MappingMatcherTests.cs b/test/WireMock.Net.Tests/Owin/MappingMatcherTests.cs index a05a816bd..fa6720090 100644 --- a/test/WireMock.Net.Tests/Owin/MappingMatcherTests.cs +++ b/test/WireMock.Net.Tests/Owin/MappingMatcherTests.cs @@ -70,7 +70,10 @@ public void MappingMatcher_FindBestMatch_WhenMappingThrowsException_ShouldReturn public void MappingMatcher_FindBestMatch_WhenAllowPartialMappingIsFalse_ShouldReturnExactMatch() { // Assign - var mappings = InitMappings(new[] { (Guid.Parse("00000000-0000-0000-0000-000000000001"), 0.1), (Guid.Parse("00000000-0000-0000-0000-000000000002"), 1.0) }); + var mappings = InitMappings( + (Guid.Parse("00000000-0000-0000-0000-000000000001"), new[] { 0.1 }), + (Guid.Parse("00000000-0000-0000-0000-000000000002"), new[] { 1.0 }) + ); _optionsMock.Setup(o => o.Mappings).Returns(mappings); var request = new RequestMessage(new UrlDetails("http://localhost/foo"), "GET", "::1"); @@ -88,7 +91,10 @@ public void MappingMatcher_FindBestMatch_WhenAllowPartialMappingIsTrue_ShouldRet { // Assign _optionsMock.SetupGet(o => o.AllowPartialMapping).Returns(true); - var mappings = InitMappings(new[] { (Guid.Parse("00000000-0000-0000-0000-000000000001"), 0.1), (Guid.Parse("00000000-0000-0000-0000-000000000002"), 0.9) }); + var mappings = InitMappings( + (Guid.Parse("00000000-0000-0000-0000-000000000001"), new[] { 0.1 }), + (Guid.Parse("00000000-0000-0000-0000-000000000002"), new[] { 0.9 }) + ); _optionsMock.Setup(o => o.Mappings).Returns(mappings); var request = new RequestMessage(new UrlDetails("http://localhost/foo"), "GET", "::1"); @@ -101,7 +107,27 @@ public void MappingMatcher_FindBestMatch_WhenAllowPartialMappingIsTrue_ShouldRet Check.That(result.RequestMatchResult.AverageTotalScore).IsEqualTo(0.9); } - private ConcurrentDictionary InitMappings(params (Guid guid, double match)[] matches) + [Fact] + public void MappingMatcher_FindBestMatch_WhenAllowPartialMappingIsFalse_And_WithSameAverageScoreButMoreMatchers_ReturnsMatchWithMoreMatchers() + { + // Assign + var mappings = InitMappings( + (Guid.Parse("00000000-0000-0000-0000-000000000001"), new[] { 1.0 }), + (Guid.Parse("00000000-0000-0000-0000-000000000002"), new[] { 1.0, 1.0 }) + ); + _optionsMock.Setup(o => o.Mappings).Returns(mappings); + + var request = new RequestMessage(new UrlDetails("http://localhost/foo"), "GET", "::1"); + + // Act + var result = _sut.FindBestMatch(request); + + // Assert and Verify + Check.That(result.Mapping.Guid).IsEqualTo(Guid.Parse("00000000-0000-0000-0000-000000000002")); + Check.That(result.RequestMatchResult.AverageTotalScore).IsEqualTo(0.9); + } + + private ConcurrentDictionary InitMappings(params (Guid guid, double[] scores)[] matches) { var mappings = new ConcurrentDictionary(); @@ -110,9 +136,13 @@ private ConcurrentDictionary InitMappings(params (Guid guid, dou var mappingMock = new Mock(); mappingMock.SetupGet(m => m.Guid).Returns(match.guid); - var partialMatchResult = new RequestMatchResult(); - partialMatchResult.AddScore(typeof(object), match.match); - mappingMock.Setup(m => m.GetRequestMatchResult(It.IsAny(), It.IsAny())).Returns(partialMatchResult); + var requestMatchResult = new RequestMatchResult(); + foreach (var score in match.scores) + { + requestMatchResult.AddScore(typeof(object), score); + } + + mappingMock.Setup(m => m.GetRequestMatchResult(It.IsAny(), It.IsAny())).Returns(requestMatchResult); mappings.TryAdd(match.guid, mappingMock.Object); }