From 81ff96042d54cec66fce41b329dca583378c2c1c Mon Sep 17 00:00:00 2001 From: Peter van den Hout Date: Tue, 20 Apr 2021 16:03:38 +0200 Subject: [PATCH] Fix for: error on bad search term #117 (#118) --- .../ExceptionExtensions.cs | 1 - .../Posts/Search/IPostRanker.cs | 10 +++ .../Posts/Search/PostRanker.cs | 56 +++++++++++++++ .../Posts/{ => Search}/SearchPostsQuery.cs | 64 ++++------------- .../Posts/Search/SearchQueryExtensions.cs | 22 ++++++ .../ServiceCollectionExtensions.cs | 3 + .../Areas/Blog/Pages/Index.cshtml.cs | 1 + .../Posts/Search/PostRankerTests.cs | 69 +++++++++++++++++++ .../{ => Search}/SearchPostsQueryTests.cs | 2 +- .../Search/SearchQueryExtensionsTests.cs | 51 ++++++++++++++ .../{ => Search}/SearchPostsQueryTests.cs | 11 ++- .../{ => Search}/SearchPostsQueryTests.cs | 11 ++- 12 files changed, 243 insertions(+), 58 deletions(-) create mode 100644 src/Opw.PineBlog.Core/Posts/Search/IPostRanker.cs create mode 100644 src/Opw.PineBlog.Core/Posts/Search/PostRanker.cs rename src/Opw.PineBlog.Core/Posts/{ => Search}/SearchPostsQuery.cs (75%) create mode 100644 src/Opw.PineBlog.Core/Posts/Search/SearchQueryExtensions.cs create mode 100644 tests/Opw.PineBlog.Core.Tests/Posts/Search/PostRankerTests.cs rename tests/Opw.PineBlog.Core.Tests/Posts/{ => Search}/SearchPostsQueryTests.cs (99%) create mode 100644 tests/Opw.PineBlog.Core.Tests/Posts/Search/SearchQueryExtensionsTests.cs rename tests/Opw.PineBlog.EntityFrameworkCore.Tests/Posts/{ => Search}/SearchPostsQueryTests.cs (94%) rename tests/Opw.PineBlog.MongoDb.Tests/Posts/{ => Search}/SearchPostsQueryTests.cs (94%) diff --git a/src/Opw.PineBlog.Abstractions/ExceptionExtensions.cs b/src/Opw.PineBlog.Abstractions/ExceptionExtensions.cs index 229f129..946005e 100644 --- a/src/Opw.PineBlog.Abstractions/ExceptionExtensions.cs +++ b/src/Opw.PineBlog.Abstractions/ExceptionExtensions.cs @@ -4,7 +4,6 @@ namespace Opw.PineBlog { - //TODO: move to Opw.Common public static class ExceptionExtensions { public static string GetAggregatedExceptionMessage(this ValidationErrorException ex) diff --git a/src/Opw.PineBlog.Core/Posts/Search/IPostRanker.cs b/src/Opw.PineBlog.Core/Posts/Search/IPostRanker.cs new file mode 100644 index 0000000..fc9aa51 --- /dev/null +++ b/src/Opw.PineBlog.Core/Posts/Search/IPostRanker.cs @@ -0,0 +1,10 @@ +using Opw.PineBlog.Entities; +using System.Collections.Generic; + +namespace Opw.PineBlog.Posts.Search +{ + public interface IPostRanker + { + IEnumerable Rank(IEnumerable posts, string query); + } +} \ No newline at end of file diff --git a/src/Opw.PineBlog.Core/Posts/Search/PostRanker.cs b/src/Opw.PineBlog.Core/Posts/Search/PostRanker.cs new file mode 100644 index 0000000..cbd528a --- /dev/null +++ b/src/Opw.PineBlog.Core/Posts/Search/PostRanker.cs @@ -0,0 +1,56 @@ +using Opw.PineBlog.Entities; +using System; +using System.Collections.Generic; +using System.Linq; +using System.Text.RegularExpressions; + +namespace Opw.PineBlog.Posts.Search +{ + // TODO: add more test for ranking (hits count) + public class PostRanker : IPostRanker + { + public IEnumerable Rank(IEnumerable posts, string query) + { + if (posts == null || !posts.Any()) + { + return new List(); + } + + var terms = query.ParseTerms(); + var rankedPosts = new List>(); + + foreach (var post in posts) + { + var rank = 0; + foreach (var term in terms) + { + int hits; + if (!string.IsNullOrWhiteSpace(post.Title) && post.Title.ToLower().Contains(term)) + { + hits = Regex.Matches(post.Title.ToLower(), term).Count; + rank += hits * 10; + } + if (!string.IsNullOrWhiteSpace(post.Categories) && post.Categories.ToLower().Contains(term)) + { + hits = Regex.Matches(post.Categories.ToLower(), term).Count; + rank += hits * 10; + } + if (!string.IsNullOrWhiteSpace(post.Description) && post.Description.ToLower().Contains(term)) + { + hits = Regex.Matches(post.Description.ToLower(), term).Count; + rank += hits * 3; + } + if (!string.IsNullOrWhiteSpace(post.Content) && post.Content.ToLower().Contains(term)) + { + hits = Regex.Matches(post.Content.ToLower(), term).Count; + rank += hits * 1; + } + } + + rankedPosts.Add(new Tuple(post, rank)); + } + + return rankedPosts.OrderByDescending(t => t.Item2).Select(t => t.Item1).ToList(); + } + } +} diff --git a/src/Opw.PineBlog.Core/Posts/SearchPostsQuery.cs b/src/Opw.PineBlog.Core/Posts/Search/SearchPostsQuery.cs similarity index 75% rename from src/Opw.PineBlog.Core/Posts/SearchPostsQuery.cs rename to src/Opw.PineBlog.Core/Posts/Search/SearchPostsQuery.cs index 159b7e5..883c291 100644 --- a/src/Opw.PineBlog.Core/Posts/SearchPostsQuery.cs +++ b/src/Opw.PineBlog.Core/Posts/Search/SearchPostsQuery.cs @@ -7,13 +7,12 @@ using System.Collections.Generic; using System.Linq; using System.Linq.Expressions; -using System.Text.RegularExpressions; using System.Threading; using System.Threading.Tasks; using System.Web; // TODO: improve test coverage -namespace Opw.PineBlog.Posts +namespace Opw.PineBlog.Posts.Search { /// /// Query that searches posts. @@ -42,6 +41,7 @@ public class Handler : IRequestHandler> { private readonly IOptionsSnapshot _blogOptions; private readonly IBlogUnitOfWork _uow; + private readonly IPostRanker _postRanker; private readonly PostUrlHelper _postUrlHelper; private readonly FileUrlHelper _fileUrlHelper; @@ -49,13 +49,20 @@ public class Handler : IRequestHandler> /// Implementation of SearchPostsQuery.Handler. /// /// The blog unit of work. + /// Post ranker. /// The blog options. /// Post URL helper. /// File URL helper. - public Handler(IBlogUnitOfWork uow, IOptionsSnapshot blogOptions, PostUrlHelper postUrlHelper, FileUrlHelper fileUrlHelper) + public Handler( + IBlogUnitOfWork uow, + IPostRanker postRanker, + IOptionsSnapshot blogOptions, + PostUrlHelper postUrlHelper, + FileUrlHelper fileUrlHelper) { _blogOptions = blogOptions; _uow = uow; + _postRanker = postRanker; _postUrlHelper = postUrlHelper; _fileUrlHelper = fileUrlHelper; } @@ -82,7 +89,7 @@ public async Task> Handle(SearchPostsQuery request, Cancel pagingUrlPartFormat += "&" + string.Format(_blogOptions.Value.SearchQueryUrlPartFormat, HttpUtility.UrlEncode(request.SearchQuery)); posts = await _uow.Posts.GetAsync(predicates, 0, int.MaxValue, cancellationToken); - posts = RankPosts(posts, request.SearchQuery); + posts = _postRanker.Rank(posts, request.SearchQuery); posts = await GetPagedListAsync(posts, predicates, pager, pagingUrlPartFormat, cancellationToken); } else @@ -111,19 +118,12 @@ public async Task> Handle(SearchPostsQuery request, Cancel return Result.Success(model); } - private IEnumerable ParseTerms(string query) - { - // convert multiple spaces into one space - query = Regex.Replace(query, @"\s+", " ").Trim(); - return query.ToLower().Split(' ').ToList(); - } - private Expression> BuildSearchExpression(string query) { var parameterExp = Expression.Parameter(typeof(Post), "p"); Expression exp = null; - foreach (var term in ParseTerms(query)) + foreach (var term in query.ParseTerms()) { exp = ConcatOr(exp, GetContainsExpression(nameof(Post.Title), term.Trim(), parameterExp)); exp = ConcatOr(exp, GetContainsExpression(nameof(Post.Description), term.Trim(), parameterExp)); @@ -152,46 +152,6 @@ private Expression GetContainsExpression(string propertyName, string term, Param return Expression.Call(propertyExp, method, someValue); } - // TODO: test ranking - private IEnumerable RankPosts(IEnumerable posts, string query) - { - var terms = ParseTerms(query); - var rankedPosts = new List>(); - - foreach (var post in posts) - { - var rank = 0; - foreach (var term in terms) - { - int hits; - if (post.Title.ToLower().Contains(term)) - { - hits = Regex.Matches(post.Title.ToLower(), term).Count; - rank += hits * 10; - } - if (post.Categories.ToLower().Contains(term)) - { - hits = Regex.Matches(post.Categories.ToLower(), term).Count; - rank += hits * 10; - } - if (post.Description.ToLower().Contains(term)) - { - hits = Regex.Matches(post.Description.ToLower(), term).Count; - rank += hits * 3; - } - if (post.Content.ToLower().Contains(term)) - { - hits = Regex.Matches(post.Content.ToLower(), term).Count; - rank += hits * 1; - } - } - - rankedPosts.Add(new Tuple(post, rank)); - } - - return rankedPosts.OrderByDescending(t => t.Item2).Select(t => t.Item1).ToList(); - } - private async Task> GetPagedListAsync(IEnumerable>> predicates, Pager pager, string pagingUrlPartFormat, CancellationToken cancellationToken) { var skip = (pager.CurrentPage - 1) * pager.ItemsPerPage; diff --git a/src/Opw.PineBlog.Core/Posts/Search/SearchQueryExtensions.cs b/src/Opw.PineBlog.Core/Posts/Search/SearchQueryExtensions.cs new file mode 100644 index 0000000..91b7997 --- /dev/null +++ b/src/Opw.PineBlog.Core/Posts/Search/SearchQueryExtensions.cs @@ -0,0 +1,22 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using System.Text.RegularExpressions; + +namespace Opw.PineBlog.Posts.Search +{ + public static class SearchQueryExtensions + { + public static IEnumerable ParseTerms(this string query) + { + if (string.IsNullOrEmpty(query)) + { + return Array.Empty(); + } + + // convert multiple spaces into one space + query = Regex.Replace(query, @"\s+", " ").Trim(); + return query.ToLower().Split(' ').ToList(); + } + } +} diff --git a/src/Opw.PineBlog.Core/ServiceCollectionExtensions.cs b/src/Opw.PineBlog.Core/ServiceCollectionExtensions.cs index e7b09c0..74cf8c6 100644 --- a/src/Opw.PineBlog.Core/ServiceCollectionExtensions.cs +++ b/src/Opw.PineBlog.Core/ServiceCollectionExtensions.cs @@ -12,6 +12,7 @@ using Opw.PineBlog.Files.Azure; using Opw.PineBlog.Feeds; using Opw.PineBlog.Blogs; +using Opw.PineBlog.Posts.Search; namespace Opw.PineBlog { @@ -52,6 +53,8 @@ public static IServiceCollection AddPineBlogCore(this IServiceCollection service services.AddTransient, UpdateBlogSettingsCommandValidator>(); + services.AddTransient(); + services.AddTransient(); services.AddTransient(); services.AddTransient(); diff --git a/src/Opw.PineBlog.RazorPages/Areas/Blog/Pages/Index.cshtml.cs b/src/Opw.PineBlog.RazorPages/Areas/Blog/Pages/Index.cshtml.cs index 553eb51..60c1542 100644 --- a/src/Opw.PineBlog.RazorPages/Areas/Blog/Pages/Index.cshtml.cs +++ b/src/Opw.PineBlog.RazorPages/Areas/Blog/Pages/Index.cshtml.cs @@ -5,6 +5,7 @@ using Microsoft.Extensions.Options; using Opw.PineBlog.Models; using Opw.PineBlog.Posts; +using Opw.PineBlog.Posts.Search; using System.Threading; using System.Threading.Tasks; diff --git a/tests/Opw.PineBlog.Core.Tests/Posts/Search/PostRankerTests.cs b/tests/Opw.PineBlog.Core.Tests/Posts/Search/PostRankerTests.cs new file mode 100644 index 0000000..6b14876 --- /dev/null +++ b/tests/Opw.PineBlog.Core.Tests/Posts/Search/PostRankerTests.cs @@ -0,0 +1,69 @@ +using FluentAssertions; +using Opw.PineBlog.Entities; +using System.Collections.Generic; +using System.Linq; +using Xunit; + +namespace Opw.PineBlog.Posts.Search +{ + public class PostRankerTests + { + [Fact] + public void Rank_Should_0Posts_WhenPostsNull() + { + var query = "c# dotnet pineblog"; + + var results = new PostRanker().Rank(null, query); + + results.Should().HaveCount(0); + } + + [Fact] + public void Rank_Should_2Posts_WhenQueryNull() + { + var posts = new List + { + new Post { Slug = "1", Title = "pineblog", Categories = "pineblog", Description = "pineblog", Content = "pinelog" }, + new Post { Slug = "2", Title = "pineblog", Categories = "pineblog", Description = "pineblog", Content = "pinelog" }, + }; + + var results = new PostRanker().Rank(posts, null); + + results.Should().HaveCount(2); + } + + [Fact] + public void Rank_Should_2Posts_WhenPostProperiesNull() + { + var query = "c# dotnet pineblog"; + var posts = new List + { + new Post { Slug = "1", Title = "pineblog", Categories = "pineblog", Description = "pineblog", Content = "pinelog" }, + new Post { Slug = "2", Title = null, Categories = null, Description = null, Content = null }, + }; + + var results = new PostRanker().Rank(posts, query); + + results.Should().HaveCount(2); + } + + [Fact] + public void Rank_Should_PostsRankedCorrectly_ForOneMatchingTerm() + { + var query = "c# dotnet pineblog"; + var posts = new List + { + new Post { Slug = "5", Title = "xxx", Categories = "xxx", Description = "xxx", Content = "xxx" }, + new Post { Slug = "4", Title = "pineblog", Categories = "xxx", Description = "xxx", Content = "xxx" }, + new Post { Slug = "3", Title = "pineblog", Categories = "pineblog", Description = "xxx", Content = "xxx" }, + new Post { Slug = "2", Title = "pineblog", Categories = "pineblog", Description = "pineblog", Content = "xxx" }, + new Post { Slug = "1", Title = "pineblog", Categories = "pineblog", Description = "pineblog", Content = "pinelog" }, + }; + + var results = new PostRanker().Rank(posts, query); + + results.Should().HaveCount(5); + results.Select(p => p.Slug).Should().BeEquivalentTo(new string[] { "1", "2", "3", "4", "5" }); + } + } +} diff --git a/tests/Opw.PineBlog.Core.Tests/Posts/SearchPostsQueryTests.cs b/tests/Opw.PineBlog.Core.Tests/Posts/Search/SearchPostsQueryTests.cs similarity index 99% rename from tests/Opw.PineBlog.Core.Tests/Posts/SearchPostsQueryTests.cs rename to tests/Opw.PineBlog.Core.Tests/Posts/Search/SearchPostsQueryTests.cs index 0929f59..61aa3ac 100644 --- a/tests/Opw.PineBlog.Core.Tests/Posts/SearchPostsQueryTests.cs +++ b/tests/Opw.PineBlog.Core.Tests/Posts/Search/SearchPostsQueryTests.cs @@ -10,7 +10,7 @@ using System.Threading.Tasks; using Xunit; -namespace Opw.PineBlog.Posts +namespace Opw.PineBlog.Posts.Search { public class SearchPostsQueryTests : MediatRTestsBase { diff --git a/tests/Opw.PineBlog.Core.Tests/Posts/Search/SearchQueryExtensionsTests.cs b/tests/Opw.PineBlog.Core.Tests/Posts/Search/SearchQueryExtensionsTests.cs new file mode 100644 index 0000000..6285310 --- /dev/null +++ b/tests/Opw.PineBlog.Core.Tests/Posts/Search/SearchQueryExtensionsTests.cs @@ -0,0 +1,51 @@ +using FluentAssertions; +using Xunit; + +namespace Opw.PineBlog.Posts.Search +{ + public class SearchQueryExtensionsTests + { + [Fact] + public void ParseTerms_Should_ConvertMultipleSpaceIntoOne_ForQueryNull() + { + string query = null; + + var result = query.ParseTerms(); + + result.Should().HaveCount(0); + } + + [Fact] + public void ParseTerms_Should_ConvertMultipleSpaceIntoOne_ForQuery() + { + var query = "c# dotnet pineblog "; + + var result = query.ParseTerms(); + + result.Should().HaveCount(3); + result.Should().BeEquivalentTo(new string[] { "c#", "dotnet", "pineblog" }); + } + + [Fact] + public void ParseTerms_Should_ToLower_ForQuery() + { + var query = " C# DOTNET pineblog "; + + var result = query.ParseTerms(); + + result.Should().HaveCount(3); + result.Should().BeEquivalentTo(new string[] { "c#", "dotnet", "pineblog" }); + } + + [Fact] + public void ParseTerms_Should_3Terms_ForQuery() + { + var query = " C# DOTNET pineblog "; + + var result = query.ParseTerms(); + + result.Should().HaveCount(3); + result.Should().BeEquivalentTo(new string[] { "c#", "dotnet", "pineblog" }); + } + } +} diff --git a/tests/Opw.PineBlog.EntityFrameworkCore.Tests/Posts/SearchPostsQueryTests.cs b/tests/Opw.PineBlog.EntityFrameworkCore.Tests/Posts/Search/SearchPostsQueryTests.cs similarity index 94% rename from tests/Opw.PineBlog.EntityFrameworkCore.Tests/Posts/SearchPostsQueryTests.cs rename to tests/Opw.PineBlog.EntityFrameworkCore.Tests/Posts/Search/SearchPostsQueryTests.cs index 55c86f4..4bbe71b 100644 --- a/tests/Opw.PineBlog.EntityFrameworkCore.Tests/Posts/SearchPostsQueryTests.cs +++ b/tests/Opw.PineBlog.EntityFrameworkCore.Tests/Posts/Search/SearchPostsQueryTests.cs @@ -1,15 +1,17 @@ using FluentAssertions; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Options; +using Moq; using Opw.PineBlog.Entities; using Opw.PineBlog.EntityFrameworkCore; using Opw.PineBlog.Files; using System; +using System.Collections.Generic; using System.Threading; using System.Threading.Tasks; using Xunit; -namespace Opw.PineBlog.Posts +namespace Opw.PineBlog.Posts.Search { public class SearchPostsQueryTests : EntityFrameworkCoreTestsBase { @@ -24,7 +26,12 @@ public SearchPostsQueryTests() var postUrlHelper = ServiceProvider.GetRequiredService(); var fileUrlHelper = ServiceProvider.GetRequiredService(); - searchPostsQueryHandler = new SearchPostsQuery.Handler(uow, options, postUrlHelper, fileUrlHelper); + var postRankerMock = new Mock(); + postRankerMock + .Setup(m => m.Rank(It.IsAny>(), It.IsAny())) + .Returns((IEnumerable posts, string _) => posts); + + searchPostsQueryHandler = new SearchPostsQuery.Handler(uow, postRankerMock.Object, options, postUrlHelper, fileUrlHelper); } [Fact] diff --git a/tests/Opw.PineBlog.MongoDb.Tests/Posts/SearchPostsQueryTests.cs b/tests/Opw.PineBlog.MongoDb.Tests/Posts/Search/SearchPostsQueryTests.cs similarity index 94% rename from tests/Opw.PineBlog.MongoDb.Tests/Posts/SearchPostsQueryTests.cs rename to tests/Opw.PineBlog.MongoDb.Tests/Posts/Search/SearchPostsQueryTests.cs index 718b974..8c01175 100644 --- a/tests/Opw.PineBlog.MongoDb.Tests/Posts/SearchPostsQueryTests.cs +++ b/tests/Opw.PineBlog.MongoDb.Tests/Posts/Search/SearchPostsQueryTests.cs @@ -1,15 +1,17 @@ using FluentAssertions; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Options; +using Moq; using Opw.PineBlog.Entities; using Opw.PineBlog.Files; using Opw.PineBlog.MongoDb; using System; +using System.Collections.Generic; using System.Threading; using System.Threading.Tasks; using Xunit; -namespace Opw.PineBlog.Posts +namespace Opw.PineBlog.Posts.Search { public class SearchPostsQueryTests : MongoDbTestsBase { @@ -24,7 +26,12 @@ public SearchPostsQueryTests(MongoDbDatabaseFixture fixture) : base(fixture) var postUrlHelper = ServiceProvider.GetRequiredService(); var fileUrlHelper = ServiceProvider.GetRequiredService(); - searchPostsQueryHandler = new SearchPostsQuery.Handler(uow, options, postUrlHelper, fileUrlHelper); + var postRankerMock = new Mock(); + postRankerMock + .Setup(m => m.Rank(It.IsAny>(), It.IsAny())) + .Returns((IEnumerable posts, string _) => posts); + + searchPostsQueryHandler = new SearchPostsQuery.Handler(uow, postRankerMock.Object, options, postUrlHelper, fileUrlHelper); } [Fact(Skip = Constants.SkipMongoDbTests)]