Skip to content

Commit

Permalink
Modifying feed service to use Lucene search
Browse files Browse the repository at this point in the history
  • Loading branch information
pranavkm committed Jul 19, 2012
1 parent 7a325ca commit 5aee8b7
Show file tree
Hide file tree
Showing 9 changed files with 164 additions and 81 deletions.
59 changes: 0 additions & 59 deletions Facts/Services/FeedServiceFacts.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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<IEntityRepository<Package>>(MockBehavior.Strict);
repo.Setup(r => r.GetAll()).Returns(new[] {
new Package { PackageRegistration = packageRegistration, Version = "1.0.0", IsPrerelease = false, Listed = true, DownloadStatistics = new List<PackageStatistics>() },
new Package { PackageRegistration = packageRegistration, Version = "1.0.1-a", IsPrerelease = true, Listed = true, DownloadStatistics = new List<PackageStatistics>() },
new Package { PackageRegistration = new PackageRegistration { Id ="baz" }, Version = "2.0", Listed = false, DownloadStatistics = new List<PackageStatistics>() },
}.AsQueryable());
var searchService = new Mock<ISearchService>(MockBehavior.Strict);
int total;
searchService.Setup(s => s.Search(It.IsAny<IQueryable<Package>>(), It.IsAny<SearchFilter>(), out total)).Returns<IQueryable<Package>, string>((_, __) => _);
var configuration = new Mock<IConfiguration>(MockBehavior.Strict);
configuration.Setup(c => c.GetSiteRoot(It.IsAny<bool>())).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<IEntityRepository<Package>>(MockBehavior.Strict);
repo.Setup(r => r.GetAll()).Returns(new[] {
new Package { PackageRegistration = packageRegistration, Version = "1.0.0", IsPrerelease = false, Listed = true, DownloadStatistics = new List<PackageStatistics>() },
new Package { PackageRegistration = packageRegistration, Version = "1.0.1-a", IsPrerelease = true, Listed = true, DownloadStatistics = new List<PackageStatistics>() },
}.AsQueryable());
var searchService = new Mock<ISearchService>(MockBehavior.Strict);
int total;
searchService.Setup(s => s.Search(It.IsAny<IQueryable<Package>>(), It.IsAny<SearchFilter>(), out total)).Returns<IQueryable<Package>, string>((_, __) => _);
var configuration = new Mock<IConfiguration>(MockBehavior.Strict);
configuration.Setup(c => c.GetSiteRoot(It.IsAny<bool>())).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()
{
Expand Down
26 changes: 26 additions & 0 deletions Website/DataServices/CountInterceptor.cs
Original file line number Diff line number Diff line change
@@ -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);
}
}
}
23 changes: 23 additions & 0 deletions Website/DataServices/DisregardODataInterceptor.cs
Original file line number Diff line number Diff line change
@@ -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<string>(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(<Expression>, <Order-by-params>). To avoid performing the
// method, we ignore it, traversing the passed in expression instead.
return Visit(node.Arguments[0]);
}
return base.VisitMethodCall(node);
}
}
}
96 changes: 93 additions & 3 deletions Website/DataServices/FeedServiceBase.cs
Original file line number Diff line number Diff line change
@@ -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
{
Expand All @@ -28,8 +31,8 @@ public FeedServiceBase()

protected FeedServiceBase(
IEntitiesContext entities,
IEntityRepository<Package> packageRepo,
IConfiguration configuration,
IEntityRepository<Package> packageRepo,
IConfiguration configuration,
ISearchService searchService)
{
this.entities = entities;
Expand All @@ -42,7 +45,7 @@ protected IEntitiesContext Entities
{
get { return entities; }
}

protected IEntityRepository<Package> PackageRepo
{
get { return packageRepo; }
Expand Down Expand Up @@ -137,6 +140,93 @@ public object GetService(Type serviceType)
return null;
}

protected virtual IQueryable<Package> 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<Package> GetResultsFromSearchService(IQueryable<Package> 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;
Expand Down
14 changes: 7 additions & 7 deletions Website/DataServices/V1Feed.svc.cs
Original file line number Diff line number Diff line change
Expand Up @@ -63,15 +63,15 @@ public IQueryable<V1FeedPackage> FindPackagesById(string id)
[WebGet]
public IQueryable<V1FeedPackage> 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()));
}
}
}
12 changes: 3 additions & 9 deletions Website/DataServices/V2Feed.svc.cs
Original file line number Diff line number Diff line change
Expand Up @@ -45,14 +45,8 @@ public static void InitializeService(DataServiceConfiguration config)
[WebGet]
public IQueryable<V2FeedPackage> 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]
Expand Down Expand Up @@ -88,7 +82,7 @@ public IQueryable<V2FeedPackage> 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)))
{
Expand Down
6 changes: 3 additions & 3 deletions Website/Infrastructure/Lucene/LuceneSearchService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ public IQueryable<Package> Search(IQueryable<Package> 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<Package>().AsQueryable();
}
Expand Down Expand Up @@ -75,7 +75,7 @@ private static IList<int> 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))
Expand Down Expand Up @@ -157,7 +157,7 @@ private static IEnumerable<string> GetSearchTerms(string searchTerm)
.Select(Escape);
}

private static SortField GetSortProperties(SearchFilter searchFilter)
private static SortField GetSortField(SearchFilter searchFilter)
{
switch (searchFilter.SortProperty)
{
Expand Down
5 changes: 5 additions & 0 deletions Website/Services/SearchFilter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,11 @@ public class SearchFilter
public SortProperty SortProperty { get; set; }

public SortDirection SortDirection { get; set; }

/// <summary>
/// Determines if only this is a count only query and does not process the source queryable.
/// </summary>
public bool CountOnly { get; set; }
}

public enum SortProperty
Expand Down
4 changes: 4 additions & 0 deletions Website/Website.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,10 @@
<Compile Include="CuratedPackagesController.generated.cs">
<DependentUpon>T4MVC.tt</DependentUpon>
</Compile>
<Compile Include="DataServices\CountInterceptor.cs">
<SubType>Code</SubType>
</Compile>
<Compile Include="DataServices\DisregardODataInterceptor.cs" />
<Compile Include="DataServices\V2CuratedFeed.svc.cs">
<DependentUpon>V2CuratedFeed.svc</DependentUpon>
</Compile>
Expand Down

0 comments on commit 5aee8b7

Please sign in to comment.