From 5aee8b77dab01d0696e1e08132e4626a5a143571 Mon Sep 17 00:00:00 2001 From: Pranav K Date: Sun, 8 Jul 2012 15:50:19 -0700 Subject: [PATCH] Modifying feed service to use Lucene search --- Facts/Services/FeedServiceFacts.cs | 59 ------------ Website/DataServices/CountInterceptor.cs | 26 +++++ .../DataServices/DisregardODataInterceptor.cs | 23 +++++ Website/DataServices/FeedServiceBase.cs | 96 ++++++++++++++++++- Website/DataServices/V1Feed.svc.cs | 14 +-- Website/DataServices/V2Feed.svc.cs | 12 +-- .../Lucene/LuceneSearchService.cs | 6 +- Website/Services/SearchFilter.cs | 5 + Website/Website.csproj | 4 + 9 files changed, 164 insertions(+), 81 deletions(-) create mode 100644 Website/DataServices/CountInterceptor.cs create mode 100644 Website/DataServices/DisregardODataInterceptor.cs diff --git a/Facts/Services/FeedServiceFacts.cs b/Facts/Services/FeedServiceFacts.cs index 3457853ab9..064155d04a 100644 --- a/Facts/Services/FeedServiceFacts.cs +++ b/Facts/Services/FeedServiceFacts.cs @@ -36,65 +36,6 @@ public void V1FeedSearchDoesNotReturnPrereleasePackages() Assert.Equal("https://localhost:8081/packages/Foo/1.0.0", result.First().GalleryDetailsUrl); } - [Fact] - public void V1FeedSearchDoesNotReturnUnlistedPackages() - { - // Arrange - var packageRegistration = new PackageRegistration { Id = "Foo" }; - var repo = new Mock>(MockBehavior.Strict); - repo.Setup(r => r.GetAll()).Returns(new[] { - new Package { PackageRegistration = packageRegistration, Version = "1.0.0", IsPrerelease = false, Listed = true, DownloadStatistics = new List() }, - new Package { PackageRegistration = packageRegistration, Version = "1.0.1-a", IsPrerelease = true, Listed = true, DownloadStatistics = new List() }, - new Package { PackageRegistration = new PackageRegistration { Id ="baz" }, Version = "2.0", Listed = false, DownloadStatistics = new List() }, - }.AsQueryable()); - var searchService = new Mock(MockBehavior.Strict); - int total; - searchService.Setup(s => s.Search(It.IsAny>(), It.IsAny(), out total)).Returns, string>((_, __) => _); - var configuration = new Mock(MockBehavior.Strict); - configuration.Setup(c => c.GetSiteRoot(It.IsAny())).Returns("http://test.nuget.org/"); - var v1Service = new TestableV1Feed(repo.Object, configuration.Object, searchService.Object); - - // Act - var result = v1Service.Search(null, null); - - // Assert - Assert.Equal(1, result.Count()); - var package = result.First(); - Assert.Equal("Foo", package.Id); - Assert.Equal("1.0.0", package.Version); - Assert.Equal("http://test.nuget.org/packages/Foo/1.0.0", package.GalleryDetailsUrl); - Assert.Equal("http://test.nuget.org/package/ReportAbuse/Foo/1.0.0", package.ReportAbuseUrl); - } - - [Fact] - public void V2FeedSearchDoesNotReturnPrereleasePackagesIfFlagIsFalse() - { - // Arrange - var packageRegistration = new PackageRegistration { Id = "Foo" }; - var repo = new Mock>(MockBehavior.Strict); - repo.Setup(r => r.GetAll()).Returns(new[] { - new Package { PackageRegistration = packageRegistration, Version = "1.0.0", IsPrerelease = false, Listed = true, DownloadStatistics = new List() }, - new Package { PackageRegistration = packageRegistration, Version = "1.0.1-a", IsPrerelease = true, Listed = true, DownloadStatistics = new List() }, - }.AsQueryable()); - var searchService = new Mock(MockBehavior.Strict); - int total; - searchService.Setup(s => s.Search(It.IsAny>(), It.IsAny(), out total)).Returns, string>((_, __) => _); - var configuration = new Mock(MockBehavior.Strict); - configuration.Setup(c => c.GetSiteRoot(It.IsAny())).Returns("https://staged.nuget.org/"); - var v2Service = new TestableV2Feed(repo.Object, configuration.Object, searchService.Object); - - // Act - var result = v2Service.Search(null, null, includePrerelease: false); - - // Assert - Assert.Equal(1, result.Count()); - var package = result.First(); - Assert.Equal("Foo", package.Id); - Assert.Equal("1.0.0", package.Version); - Assert.Equal("https://staged.nuget.org/packages/Foo/1.0.0", package.GalleryDetailsUrl); - Assert.Equal("https://staged.nuget.org/package/ReportAbuse/Foo/1.0.0", package.ReportAbuseUrl); - } - [Fact] public void V1FeedFindPackagesByIdReturnsUnlistedPackagesButNotPrereleasePackages() { diff --git a/Website/DataServices/CountInterceptor.cs b/Website/DataServices/CountInterceptor.cs new file mode 100644 index 0000000000..363e6b99a0 --- /dev/null +++ b/Website/DataServices/CountInterceptor.cs @@ -0,0 +1,26 @@ +using System; +using System.Linq; +using System.Linq.Expressions; + +namespace NuGetGallery +{ + public class CountInterceptor : ExpressionVisitor + { + private readonly long count; + public CountInterceptor(long count) + { + this.count = count; + } + + protected override Expression VisitMethodCall(MethodCallExpression node) + { + var method = node.Method; + if ((method.DeclaringType == typeof(Queryable)) && method.Name.Equals("LongCount", StringComparison.Ordinal)) + { + return Expression.Constant(count); + } + + return base.VisitMethodCall(node); + } + } +} \ No newline at end of file diff --git a/Website/DataServices/DisregardODataInterceptor.cs b/Website/DataServices/DisregardODataInterceptor.cs new file mode 100644 index 0000000000..671b90b3a1 --- /dev/null +++ b/Website/DataServices/DisregardODataInterceptor.cs @@ -0,0 +1,23 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using System.Linq.Expressions; + +namespace NuGetGallery +{ + public class DisregardODataInterceptor : ExpressionVisitor + { + protected override Expression VisitMethodCall(MethodCallExpression node) + { + var methodsToIgnore = new HashSet(new[] { "Take", "Skip", "OrderBy", "ThenBy" }, StringComparer.Ordinal); + var method = node.Method; + if ((method.DeclaringType == typeof(Queryable)) && methodsToIgnore.Contains(method.Name)) + { + // The expression is of the format Queryable.OrderBy(, ). To avoid performing the + // method, we ignore it, traversing the passed in expression instead. + return Visit(node.Arguments[0]); + } + return base.VisitMethodCall(node); + } + } +} diff --git a/Website/DataServices/FeedServiceBase.cs b/Website/DataServices/FeedServiceBase.cs index 33e183e11e..58a9a138a5 100644 --- a/Website/DataServices/FeedServiceBase.cs +++ b/Website/DataServices/FeedServiceBase.cs @@ -1,11 +1,14 @@ using System; +using System.Data.Entity; using System.Data.Services; using System.Data.Services.Common; using System.Data.Services.Providers; using System.IO; +using System.Linq; using System.ServiceModel; using System.Web; using System.Web.Mvc; +using QueryInterceptor; namespace NuGetGallery { @@ -28,8 +31,8 @@ public FeedServiceBase() protected FeedServiceBase( IEntitiesContext entities, - IEntityRepository packageRepo, - IConfiguration configuration, + IEntityRepository packageRepo, + IConfiguration configuration, ISearchService searchService) { this.entities = entities; @@ -42,7 +45,7 @@ protected IEntitiesContext Entities { get { return entities; } } - + protected IEntityRepository PackageRepo { get { return packageRepo; } @@ -137,6 +140,93 @@ public object GetService(Type serviceType) return null; } + protected virtual IQueryable SearchCore(string searchTerm, string targetFramework, bool includePrerelease) + { + // Filter out unlisted packages when searching. We will return it when a generic "GetPackages" request comes and filter it on the client. + var packages = PackageRepo.GetAll() + .Include(p => p.PackageRegistration) + .Include(x => x.Authors) + .Include(x => x.PackageRegistration.Owners) + .Where(p => p.Listed); + + if (String.IsNullOrEmpty(searchTerm)) + { + return packages; + } + + var request = new HttpRequestWrapper(HttpContext.Current.Request); + SearchFilter searchFilter; + + // We can only use Lucene if the client queries for the latest versions (IsLatest \ IsLatestStable) versions of a package. + if (TryReadSearchFilter(request, out searchFilter)) + { + searchFilter.SearchTerm = searchTerm; + searchFilter.IncludePrerelease = includePrerelease; + + return GetResultsFromSearchService(packages, searchFilter); + } + return packages.Search(searchTerm); + } + + private IQueryable GetResultsFromSearchService(IQueryable packages, SearchFilter searchFilter) + { + int totalHits = 0; + var result = SearchService.Search(packages, searchFilter, out totalHits); + + // For count queries, we can ask the SearchService to not filter the source results. This would avoid hitting the database and consequently make + // it very fast. + if (searchFilter.CountOnly) + { + // At this point, we already know what the total count is. We can have it return this value very quickly without doing any SQL. + return result.InterceptWith(new CountInterceptor(totalHits)); + } + + // For relevance search, Lucene returns us a paged\sorted list. OData tries to apply default ordering and Take \ Skip on top of this. + // We avoid it by yanking these expressions out of out the tree. + return result.InterceptWith(new DisregardODataInterceptor()); + } + + private bool TryReadSearchFilter(HttpRequestBase request, out SearchFilter searchFilter) + { + searchFilter = new SearchFilter + { + Take = ReadInt(request["$top"], 30), + Skip = ReadInt(request["$skip"], 0), + CountOnly = request.Path.TrimEnd('/').EndsWith("$count") + }; + + switch(request["$orderby"]) + { + case "DownloadCount desc,Id": + searchFilter.SortProperty = SortProperty.DownloadCount; + break; + case "Published desc,Id": + searchFilter.SortProperty = SortProperty.Recent; + break; + case "concat(Title,Id),Id": + searchFilter.SortProperty = SortProperty.DisplayName; + searchFilter.SortDirection = SortDirection.Ascending; + break; + case "concat(Title,Id) desc,Id": + searchFilter.SortProperty = SortProperty.DisplayName; + searchFilter.SortDirection = SortDirection.Descending; + break; + default: + searchFilter.SortProperty = SortProperty.Relevance; + break; + } + + string filterValue = request["$filter"]; + return (filterValue.IndexOf("IsLatestVersion", StringComparison.Ordinal) != -1) || + (filterValue.IndexOf("IsAbsoluteLatestVersion", StringComparison.Ordinal) != -1); + } + + private int ReadInt(string requestValue, int defaultValue) + { + int result; + return Int32.TryParse(requestValue, out result) ? result : defaultValue; + } + protected virtual bool UseHttps() { return HttpContext.Current.Request.IsSecureConnection; diff --git a/Website/DataServices/V1Feed.svc.cs b/Website/DataServices/V1Feed.svc.cs index 9457cce36f..7a6da8c3a6 100644 --- a/Website/DataServices/V1Feed.svc.cs +++ b/Website/DataServices/V1Feed.svc.cs @@ -63,15 +63,15 @@ public IQueryable FindPackagesById(string id) [WebGet] public IQueryable Search(string searchTerm, string targetFramework) { - // Only allow listed stable releases to be returned when searching the v1 feed. - var packages = PackageRepo.GetAll().Where(p => !p.IsPrerelease && p.Listed); - - if (String.IsNullOrEmpty(searchTerm)) + var packages = PackageRepo.GetAll() + .Include(p => p.PackageRegistration) + .Where(p => !p.IsPrerelease); + if (!String.IsNullOrEmpty(searchTerm)) { - return packages.ToV1FeedPackageQuery(Configuration.GetSiteRoot(UseHttps())); + // For v1 feed, only allow stable package versions. + packages = SearchCore(searchTerm, targetFramework, includePrerelease: false); } - return packages.Search(searchTerm) - .ToV1FeedPackageQuery(Configuration.GetSiteRoot(UseHttps())); + return packages.ToV1FeedPackageQuery(Configuration.GetSiteRoot(UseHttps())); } } } diff --git a/Website/DataServices/V2Feed.svc.cs b/Website/DataServices/V2Feed.svc.cs index ce7d2f259a..e425cb6091 100644 --- a/Website/DataServices/V2Feed.svc.cs +++ b/Website/DataServices/V2Feed.svc.cs @@ -45,14 +45,8 @@ public static void InitializeService(DataServiceConfiguration config) [WebGet] public IQueryable Search(string searchTerm, string targetFramework, bool includePrerelease) { - // Filter out unlisted packages when searching. We will return it when a generic "GetPackages" request comes and filter it on the client. - var packages = PackageRepo.GetAll().Where(p => p.Listed); - if (!includePrerelease) - { - packages = packages.Where(p => !p.IsPrerelease); - } - return packages.Search(searchTerm) - .ToV2FeedPackageQuery(GetSiteRoot()); + var packages = SearchCore(searchTerm, targetFramework, includePrerelease); + return packages.ToV2FeedPackageQuery(GetSiteRoot()); } [WebGet] @@ -88,7 +82,7 @@ public IQueryable GetUpdates(string packageIds, string versions, var id = idValues[i]; SemanticVersion version; SemanticVersion currentVersion; - + if (SemanticVersion.TryParse(versionValues[i], out currentVersion) && (!versionLookup.TryGetValue(id, out version) || (currentVersion > version))) { diff --git a/Website/Infrastructure/Lucene/LuceneSearchService.cs b/Website/Infrastructure/Lucene/LuceneSearchService.cs index b556f037e0..ab712332e6 100644 --- a/Website/Infrastructure/Lucene/LuceneSearchService.cs +++ b/Website/Infrastructure/Lucene/LuceneSearchService.cs @@ -44,7 +44,7 @@ public IQueryable Search(IQueryable packages, SearchFilter sea // For the given search term, find the keys that match. var keys = SearchCore(searchFilter); totalHits = keys.Count; - if (keys.Count == 0) + if (keys.Count == 0 || searchFilter.CountOnly) { return Enumerable.Empty().AsQueryable(); } @@ -75,7 +75,7 @@ private static IList SearchCore(SearchFilter searchFilter) return new int[0]; } - SortField sortField = GetSortProperties(searchFilter); + SortField sortField = GetSortField(searchFilter); int numRecords = Math.Min((1 + searchFilter.Skip) * searchFilter.Take, MaximumRecordsToReturn); using (var directory = new LuceneFileSystem(LuceneCommon.IndexDirectory)) @@ -157,7 +157,7 @@ private static IEnumerable GetSearchTerms(string searchTerm) .Select(Escape); } - private static SortField GetSortProperties(SearchFilter searchFilter) + private static SortField GetSortField(SearchFilter searchFilter) { switch (searchFilter.SortProperty) { diff --git a/Website/Services/SearchFilter.cs b/Website/Services/SearchFilter.cs index f182227ca6..60b08ec334 100644 --- a/Website/Services/SearchFilter.cs +++ b/Website/Services/SearchFilter.cs @@ -14,6 +14,11 @@ public class SearchFilter public SortProperty SortProperty { get; set; } public SortDirection SortDirection { get; set; } + + /// + /// Determines if only this is a count only query and does not process the source queryable. + /// + public bool CountOnly { get; set; } } public enum SortProperty diff --git a/Website/Website.csproj b/Website/Website.csproj index cc6dca18e2..315cd70693 100644 --- a/Website/Website.csproj +++ b/Website/Website.csproj @@ -215,6 +215,10 @@ T4MVC.tt + + Code + + V2CuratedFeed.svc