Skip to content
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

[SemVer2] Filter SemVer v2.0.0 package versions by default on v1 and v2 OData endpoints #3694

Merged
merged 7 commits into from
Mar 28, 2017
10 changes: 6 additions & 4 deletions src/NuGetGallery/Controllers/ODataV1FeedController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
using System.Web.Http;
using System.Web.Http.OData;
using System.Web.Http.OData.Query;
using System.Web.Http.Results;
using NuGetGallery.Configuration;
using NuGetGallery.OData;
using NuGetGallery.OData.QueryFilter;
Expand Down Expand Up @@ -50,7 +49,7 @@ public IHttpActionResult Get(ODataQueryOptions<V1FeedPackage> options)
return BadRequest(ODataQueryVerifier.GetValidationFailedMessage(options));
}
var queryable = _packagesRepository.GetAll()
.Where(p => !p.IsPrerelease && !p.Deleted)
.Where(p => !p.IsPrerelease && !p.Deleted && p.SemVerLevelKey == SemVerLevelKey.Unknown)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wish there was an easier way to add conditions here rather than having to change every place where we query the packages repository.

Do you think we could extract these checks out to a function?

public static bool ShouldShowPackage(V1FeedPackage package, bool showUnlisted)
{
    return !package.IsPrerelease && !package.Deleted && package.SemVerLevelKey == SemVerLevelKey.Unknown && (showUnlisted || package.IsListed);
}

You could add this as a method in the base class (NuGetODataController) and use it in ODataV2FeedController as well.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking about consolidating this somehow too, however, the logic is not the same for v1 and v2. E.g. v1 also filters out pre-release packages from the feed.

.WithoutSortOnColumn(Version)
.WithoutSortOnColumn(Id, ShouldIgnoreOrderById<V1FeedPackage>(options))
.ToV1FeedPackageQuery(_configurationService.GetSiteRoot(UseHttps()));
Expand Down Expand Up @@ -88,7 +87,10 @@ private async Task<IHttpActionResult> GetCore(ODataQueryOptions<V1FeedPackage> o
{
var packages = _packagesRepository.GetAll()
.Include(p => p.PackageRegistration)
.Where(p => p.PackageRegistration.Id.Equals(id, StringComparison.OrdinalIgnoreCase) && !p.IsPrerelease && !p.Deleted);
.Where(p => p.PackageRegistration.Id.Equals(id, StringComparison.OrdinalIgnoreCase)
&& !p.IsPrerelease
&& !p.Deleted
&& p.SemVerLevelKey == SemVerLevelKey.Unknown);

if (!string.IsNullOrEmpty(version))
{
Expand Down Expand Up @@ -182,7 +184,7 @@ public async Task<IHttpActionResult> Search(
var packages = _packagesRepository.GetAll()
.Include(p => p.PackageRegistration)
.Include(p => p.PackageRegistration.Owners)
.Where(p => p.Listed && !p.IsPrerelease && !p.Deleted)
.Where(p => p.Listed && !p.IsPrerelease && !p.Deleted && p.SemVerLevelKey == SemVerLevelKey.Unknown)
.OrderBy(p => p.PackageRegistration.Id).ThenBy(p => p.Version)
.AsNoTracking();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ public IHttpActionResult Get(ODataQueryOptions<V2FeedPackage> options, string cu
}

var queryable = _curatedFeedService.GetPackages(curatedFeedName)
.Where(p => p.SemVerLevelKey == SemVerLevelKey.Unknown)
.ToV2FeedPackageQuery(_configurationService.GetSiteRoot(UseHttps()), _configurationService.Features.FriendlyLicenses)
.InterceptWith(new NormalizeVersionInterceptor());

Expand Down Expand Up @@ -101,7 +102,8 @@ private async Task<IHttpActionResult> GetCore(ODataQueryOptions<V2FeedPackage> o
}

var packages = _curatedFeedService.GetPackages(curatedFeedName)
.Where(p => p.PackageRegistration.Id.Equals(id, StringComparison.OrdinalIgnoreCase));
.Where(p => p.SemVerLevelKey == SemVerLevelKey.Unknown
&& p.PackageRegistration.Id.Equals(id, StringComparison.OrdinalIgnoreCase));

if (!string.IsNullOrEmpty(version))
{
Expand Down Expand Up @@ -201,6 +203,7 @@ public async Task<IHttpActionResult> Search(
// Perform actual search
var curatedFeed = _curatedFeedService.GetFeedByName(curatedFeedName, includePackages: false);
var packages = _curatedFeedService.GetPackages(curatedFeedName)
.Where(p => p.SemVerLevelKey == SemVerLevelKey.Unknown)
.OrderBy(p => p.PackageRegistration.Id).ThenBy(p => p.Version);

// todo: search hijack should take queryOptions instead of manually parsing query options
Expand Down
36 changes: 18 additions & 18 deletions src/NuGetGallery/Controllers/ODataV2FeedController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
using System.Web.Http;
using System.Web.Http.OData;
using System.Web.Http.OData.Query;
using System.Web.Http.Results;
using NuGet.Frameworks;
using NuGet.Versioning;
using NuGetGallery.Configuration;
Expand Down Expand Up @@ -52,7 +51,7 @@ public async Task<IHttpActionResult> Get(ODataQueryOptions<V2FeedPackage> option
{
// Setup the search
var packages = _packagesRepository.GetAll()
.Where(p => !p.Deleted)
.Where(p => !p.Deleted && p.SemVerLevelKey == SemVerLevelKey.Unknown)
.WithoutSortOnColumn(Version)
.WithoutSortOnColumn(Id, ShouldIgnoreOrderById<V2FeedPackage>(options))
.InterceptWith(new NormalizeVersionInterceptor()) ;
Expand Down Expand Up @@ -140,7 +139,7 @@ private async Task<IHttpActionResult> GetCore(ODataQueryOptions<V2FeedPackage> o
{
var packages = _packagesRepository.GetAll()
.Include(p => p.PackageRegistration)
.Where(p => p.PackageRegistration.Id.Equals(id, StringComparison.OrdinalIgnoreCase) && !p.Deleted);
.Where(p => p.PackageRegistration.Id.Equals(id, StringComparison.OrdinalIgnoreCase) && !p.Deleted && p.SemVerLevelKey == SemVerLevelKey.Unknown);

if (!string.IsNullOrEmpty(version))
{
Expand Down Expand Up @@ -235,7 +234,7 @@ public async Task<IHttpActionResult> Search(
var packages = _packagesRepository.GetAll()
.Include(p => p.PackageRegistration)
.Include(p => p.PackageRegistration.Owners)
.Where(p => p.Listed && !p.Deleted)
.Where(p => p.Listed && !p.Deleted && p.SemVerLevelKey == SemVerLevelKey.Unknown)
.OrderBy(p => p.PackageRegistration.Id).ThenBy(p => p.Version)
.AsNoTracking();

Expand Down Expand Up @@ -397,20 +396,21 @@ private static IEnumerable<Package> GetUpdates(
{
var updates = from p in packages.AsEnumerable()
let version = NuGetVersion.Parse(p.Version)
where versionLookup[p.PackageRegistration.Id]
.Any(versionTuple =>
{
NuGetVersion clientVersion = versionTuple.Item1;
var supportedPackageFrameworks = p.SupportedFrameworks.Select(f => f.FrameworkName);

VersionRange versionConstraint = versionTuple.Item2;

return (version > clientVersion) &&
(targetFrameworkValues == null ||
!supportedPackageFrameworks.Any() ||
targetFrameworkValues.Any(s => supportedPackageFrameworks.Any(supported => NuGetFrameworkUtility.IsCompatibleWithFallbackCheck(s, supported)))) &&
(versionConstraint == null || versionConstraint.Satisfies(version));
})
where p.SemVerLevelKey == SemVerLevelKey.Unknown
&& versionLookup[p.PackageRegistration.Id].Any(versionTuple =>
{
NuGetVersion clientVersion = versionTuple.Item1;
var supportedPackageFrameworks = p.SupportedFrameworks.Select(f => f.FrameworkName);

VersionRange versionConstraint = versionTuple.Item2;

return version > clientVersion
&& (targetFrameworkValues == null
|| !supportedPackageFrameworks.Any()
|| targetFrameworkValues.Any(s => supportedPackageFrameworks.Any(supported => NuGetFrameworkUtility.IsCompatibleWithFallbackCheck(s, supported))))
&& (versionConstraint == null
|| versionConstraint.Satisfies(version));
})
select p;

if (!includeAllVersions)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,15 @@ public class AutoCompleteDatabasePackageIdsQuery
private const string _partialIdSqlFormat = @"SELECT TOP 30 pr.ID
FROM Packages p (NOLOCK)
JOIN PackageRegistrations pr (NOLOCK) on pr.[Key] = p.PackageRegistrationKey
WHERE pr.ID LIKE {{0}}
WHERE p.[SemVerLevelKey] IS NULL AND pr.ID LIKE {{0}}
{0}
GROUP BY pr.ID
ORDER BY pr.ID";

private const string _noPartialIdSql = @"SELECT TOP 30 pr.ID
FROM Packages p (NOLOCK)
JOIN PackageRegistrations pr (NOLOCK) on pr.[Key] = p.PackageRegistrationKey
WHERE p.[SemVerLevelKey] IS NULL
GROUP BY pr.ID
ORDER BY MAX(pr.DownloadCount) DESC";

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ public class AutoCompleteDatabasePackageVersionsQuery
private const string _sqlFormat = @"SELECT p.[Version]
FROM Packages p (NOLOCK)
JOIN PackageRegistrations pr (NOLOCK) on pr.[Key] = p.PackageRegistrationKey
WHERE pr.ID = {{0}}
WHERE p.[SemVerLevelKey] IS NULL AND pr.ID = {{0}}
{0}";

public AutoCompleteDatabasePackageVersionsQuery(IEntitiesContext entities)
Expand Down
222 changes: 222 additions & 0 deletions tests/NuGetGallery.Facts/Controllers/ODataFeedControllerFactsBase.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,222 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System;
using System.Collections.Generic;
using System.IO;
using System.Linq;
using System.Net.Http;
using System.Threading;
using System.Threading.Tasks;
using System.Web;
using System.Web.Http;
using System.Web.Http.OData;
using System.Web.Http.OData.Builder;
using System.Web.Http.OData.Query;
using Moq;
using NuGetGallery.Configuration;
using NuGetGallery.Framework;
using NuGetGallery.OData;
using NuGetGallery.WebApi;

namespace NuGetGallery.Controllers
{
public abstract class ODataFeedControllerFactsBase<TController>
where TController : NuGetODataController
{
private const string _siteRoot = "https://nuget.localtest.me";
protected const string TestPackageId = "Some.Awesome.Package";

protected readonly IReadOnlyCollection<Package> NonSemVer2Packages;
protected readonly IReadOnlyCollection<Package> SemVer2Packages;
protected readonly IEntityRepository<Package> PackagesRepository;
protected readonly IQueryable<Package> AllPackages;

protected ODataFeedControllerFactsBase()
{
// Arrange
AllPackages = CreatePackagesQueryable();
NonSemVer2Packages = AllPackages.Where(p => p.SemVerLevelKey == SemVerLevelKey.Unknown).ToList();
SemVer2Packages = AllPackages.Where(p => p.SemVerLevelKey == SemVerLevelKey.SemVer2).ToList();

var packagesRepositoryMock = new Mock<IEntityRepository<Package>>(MockBehavior.Strict);
packagesRepositoryMock.Setup(m => m.GetAll()).Returns(AllPackages).Verifiable();
PackagesRepository = packagesRepositoryMock.Object;
}

protected abstract TController CreateController(
IEntityRepository<Package> packagesRepository,
IGalleryConfigurationService configurationService,
ISearchService searchService);

protected TController CreateTestableODataFeedController(HttpRequestMessage request)
{
var searchService = new Mock<ISearchService>().Object;
var configurationService = new TestGalleryConfigurationService();
configurationService.Current.SiteRoot = _siteRoot;

var controller = CreateController(PackagesRepository, configurationService, searchService);

var httpRequest = new HttpRequest(string.Empty, request.RequestUri.AbsoluteUri, request.RequestUri.Query);
var httpResponse = new HttpResponse(new StringWriter());
var httpContext = new HttpContext(httpRequest, httpResponse);

request.Properties.Add("MS_HttpContext", httpContext);
controller.Request = request;

HttpContext.Current = httpContext;

controller.ControllerContext.Controller = controller;
controller.ControllerContext.Configuration = new HttpConfiguration();

return controller;
}

protected async Task<IReadOnlyCollection<TFeedPackage>> GetCollection<TFeedPackage>(
Func<TController, ODataQueryOptions<TFeedPackage>, IHttpActionResult> controllerAction,
string requestPath)
where TFeedPackage : class
{
var queryResult = InvokeODataFeedControllerAction(controllerAction, requestPath);

return await GetValueFromQueryResult(queryResult);
}

protected async Task<IReadOnlyCollection<TFeedPackage>> GetCollection<TFeedPackage>(
Func<TController, ODataQueryOptions<TFeedPackage>, Task<IHttpActionResult>> asyncControllerAction,
string requestPath)
where TFeedPackage : class
{
var queryResult = await InvokeODataFeedControllerActionAsync(asyncControllerAction, requestPath);

return await GetValueFromQueryResult(queryResult);
}

protected async Task<int> GetInt<TFeedPackage>(
Func<TController, ODataQueryOptions<TFeedPackage>, IHttpActionResult> controllerAction,
string requestPath)
where TFeedPackage : class
{
var queryResult = InvokeODataFeedControllerAction(controllerAction, requestPath);

return int.Parse(await GetValueFromQueryResult(queryResult));
}

protected async Task<int> GetInt<TFeedPackage>(
Func<TController, ODataQueryOptions<TFeedPackage>, Task<IHttpActionResult>> asyncControllerAction,
string requestPath)
where TFeedPackage : class
{
var queryResult = await InvokeODataFeedControllerActionAsync(asyncControllerAction, requestPath);

return int.Parse(await GetValueFromQueryResult(queryResult));
}

private static IQueryable<Package> CreatePackagesQueryable()
{
var packageRegistration = new PackageRegistration { Id = TestPackageId };

var list = new List<Package>
{
new Package
{
PackageRegistration = packageRegistration,
Version = "1.0.0.0",
NormalizedVersion = "1.0.0.0",
SemVerLevelKey = SemVerLevelKey.Unknown
},
new Package
{
PackageRegistration = packageRegistration,
Version = "1.0.0.0-alpha",
NormalizedVersion = "1.0.0.0-alpha",
SemVerLevelKey = SemVerLevelKey.Unknown
},
new Package
{
PackageRegistration = packageRegistration,
Version = "2.0.0",
NormalizedVersion = "2.0.0",
SemVerLevelKey = SemVerLevelKey.Unknown
},
new Package
{
PackageRegistration = packageRegistration,
Version = "2.0.0-alpha",
NormalizedVersion = "2.0.0-alpha",
SemVerLevelKey = SemVerLevelKey.SemVer2
},
new Package
{
PackageRegistration = packageRegistration,
Version = "2.0.0-alpha.1",
NormalizedVersion = "2.0.0-alpha.1",
SemVerLevelKey = SemVerLevelKey.SemVer2
},
new Package
{
PackageRegistration = packageRegistration,
Version = "2.0.0+metadata",
NormalizedVersion = "2.0.0",
SemVerLevelKey = SemVerLevelKey.SemVer2
}
};

return list.AsQueryable();
}

private static ODataQueryContext CreateODataQueryContext<TFeedPackage>()
where TFeedPackage : class
{
var oDataModelBuilder = new ODataConventionModelBuilder();
oDataModelBuilder.EntitySet<TFeedPackage>("Packages");

return new ODataQueryContext(oDataModelBuilder.GetEdmModel(), typeof(TFeedPackage));
}

private static async Task<dynamic> GetValueFromQueryResult<TEntity>(QueryResult<TEntity> queryResult)
{
var httpResponseMessage = await queryResult.ExecuteAsync(CancellationToken.None);

if (queryResult.FormatAsCountResult)
{
var stringContent = (StringContent)httpResponseMessage.Content;
return await stringContent.ReadAsStringAsync();
}
else if (queryResult.FormatAsSingleResult)
{
var objectContent = (ObjectContent<TEntity>)httpResponseMessage.Content;
return (TEntity)objectContent.Value;
}
else
{
var objectContent = (ObjectContent<IQueryable<TEntity>>)httpResponseMessage.Content;
return ((IQueryable<TEntity>)objectContent.Value).ToList();
}
}

private async Task<QueryResult<TFeedPackage>> InvokeODataFeedControllerActionAsync<TFeedPackage>(
Func<TController, ODataQueryOptions<TFeedPackage>, Task<IHttpActionResult>> asyncControllerAction,
string requestPath)
where TFeedPackage : class
{
var request = new HttpRequestMessage(HttpMethod.Get, $"{_siteRoot}{requestPath}");
var controller = CreateTestableODataFeedController(request);

return (QueryResult<TFeedPackage>)await asyncControllerAction(controller,
new ODataQueryOptions<TFeedPackage>(CreateODataQueryContext<TFeedPackage>(), request));
}

private QueryResult<TFeedPackage> InvokeODataFeedControllerAction<TFeedPackage>(
Func<TController, ODataQueryOptions<TFeedPackage>, IHttpActionResult> controllerAction,
string requestPath)
where TFeedPackage : class
{
var request = new HttpRequestMessage(HttpMethod.Get, $"{_siteRoot}{requestPath}");
var controller = CreateTestableODataFeedController(request);

return (QueryResult<TFeedPackage>)controllerAction(controller,
new ODataQueryOptions<TFeedPackage>(CreateODataQueryContext<TFeedPackage>(), request));
}
}
}
Loading