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

Fix #1670 by improving WebMatrix curation #1747

Merged
merged 1 commit into from
Dec 9, 2013
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 10 additions & 3 deletions src/NuGetGallery/Controllers/ApiController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ public partial class ApiController : AppController
public IStatisticsService StatisticsService { get; set; }
public IContentService ContentService { get; set; }
public IIndexingService IndexingService { get; set; }
public IAutomaticallyCuratePackageCommand AutoCuratePackage { get; set; }

protected ApiController() { }

Expand All @@ -38,7 +39,8 @@ public ApiController(
IUserService userService,
INuGetExeDownloaderService nugetExeDownloaderService,
IContentService contentService,
IIndexingService indexingService)
IIndexingService indexingService,
IAutomaticallyCuratePackageCommand autoCuratePackage)
{
EntitiesContext = entitiesContext;
PackageService = packageService;
Expand All @@ -48,6 +50,7 @@ public ApiController(
ContentService = contentService;
StatisticsService = null;
IndexingService = indexingService;
AutoCuratePackage = autoCuratePackage;
}

public ApiController(
Expand All @@ -58,8 +61,9 @@ public ApiController(
INuGetExeDownloaderService nugetExeDownloaderService,
IContentService contentService,
IIndexingService indexingService,
IAutomaticallyCuratePackageCommand autoCuratePackage,
IStatisticsService statisticsService)
: this(entitiesContext, packageService, packageFileService, userService, nugetExeDownloaderService, contentService, indexingService)
: this(entitiesContext, packageService, packageFileService, userService, nugetExeDownloaderService, contentService, indexingService, autoCuratePackage)
{
StatisticsService = statisticsService;
}
Expand Down Expand Up @@ -239,7 +243,10 @@ private async Task<ActionResult> CreatePackageInternal()
}
}

var package = PackageService.CreatePackage(packageToPush, user, commitChanges: true);
var package = PackageService.CreatePackage(packageToPush, user, commitChanges: false);
AutoCuratePackage.Execute(package, packageToPush, commitChanges: false);
EntitiesContext.SaveChanges();

using (Stream uploadStream = packageToPush.GetStream())
{
await PackageFileService.SavePackageFileAsync(package, uploadStream);
Expand Down
68 changes: 47 additions & 21 deletions src/NuGetGallery/PackageCurators/WebMatrixPackageCurator.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
using System.IO;
using System;
using System.IO;
using System.Linq;
using System.Runtime.Versioning;
using NuGet;
using NuGetGallery.Packaging;

Expand All @@ -24,6 +26,7 @@ public override void Curate(
GetService<ICuratedFeedService>().CreatedCuratedPackage(
curatedFeed,
galleryPackage.PackageRegistration,
included: true,
automaticallyCurated: true,
commitChanges: commitChanges);
}
Expand All @@ -34,33 +37,56 @@ internal static bool ShouldCuratePackage(
Package galleryPackage,
INupkg nugetPackage)
{
if (!galleryPackage.IsLatestStable)
{
return false;
}
return
// Must have min client version of null or <= 2.2
(nugetPackage.Metadata.MinClientVersion == null || nugetPackage.Metadata.MinClientVersion <= new Version(2, 2)) &&

// Must be latest stable
galleryPackage.IsLatestStable &&

// Must support net40
SupportsNet40(galleryPackage) &&

// Dependencies on the gallery must be curated
DependenciesAreCurated(galleryPackage, curatedFeed) &&

bool shouldBeIncluded = galleryPackage.Tags != null &&
(
// Must have AspNetWebPages tag
ContainsAspNetWebPagesTag(galleryPackage) ||

// OR: Must not contain powershell or T4
DoesNotContainUnsupportedFiles(nugetPackage)
);
}

private static bool ContainsAspNetWebPagesTag(Package galleryPackage)
{
return galleryPackage.Tags != null &&
galleryPackage.Tags.ToLowerInvariant().Contains("aspnetwebpages");
}

if (!shouldBeIncluded)
private static bool SupportsNet40(Package galleryPackage)
{
var net40fx = new FrameworkName(".NETFramework", new Version(4, 0));
return (galleryPackage.SupportedFrameworks.Count == 0) ||
(from fx in galleryPackage.SupportedFrameworks
let fxName = VersionUtility.ParseFrameworkName(fx.TargetFramework)
where fxName == net40fx
select fx)
.Any();
}

private static bool DoesNotContainUnsupportedFiles(INupkg nugetPackage)
{
foreach (var filePath in nugetPackage.GetFiles())
{
shouldBeIncluded = true;
foreach (var filePath in nugetPackage.GetFiles())
var fi = new FileInfo(filePath);
if (fi.Extension == ".ps1" || fi.Extension == ".t4")
{
var fi = new FileInfo(filePath);
if (fi.Extension == ".ps1" || fi.Extension == ".t4")
{
return false;
}
return false;
}
}

if (!shouldBeIncluded)
{
return false;
}

return DependenciesAreCurated(galleryPackage, curatedFeed);
return true;
}
}
}
31 changes: 27 additions & 4 deletions tests/NuGetGallery.Facts/Controllers/ApiControllerFacts.cs
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ class TestableApiController : ApiController
public Mock<IContentService> MockContentService { get; private set; }
public Mock<IStatisticsService> MockStatisticsService { get; private set; }
public Mock<IIndexingService> MockIndexingService { get; private set; }
public Mock<IAutomaticallyCuratePackageCommand> MockAutoCuratePackage { get; private set; }

private INupkg PackageFromInputStream { get; set; }

Expand All @@ -44,6 +45,7 @@ public TestableApiController(MockBehavior behavior = MockBehavior.Default)
ContentService = (MockContentService = new Mock<IContentService>()).Object;
StatisticsService = (MockStatisticsService = new Mock<IStatisticsService>()).Object;
IndexingService = (MockIndexingService = new Mock<IIndexingService>()).Object;
AutoCuratePackage = (MockAutoCuratePackage = new Mock<IAutomaticallyCuratePackageCommand>()).Object;

MockPackageFileService = new Mock<IPackageFileService>(MockBehavior.Strict);
MockPackageFileService.Setup(p => p.SavePackageFileAsync(It.IsAny<Package>(), It.IsAny<Stream>())).Returns(Task.FromResult(0));
Expand Down Expand Up @@ -141,7 +143,26 @@ public void WillCreateAPackageFromTheNuGetPackage()

controller.CreatePackagePut();

controller.MockPackageService.Verify(x => x.CreatePackage(nuGetPackage.Object, It.IsAny<User>(), true));
controller.MockPackageService.Verify(x => x.CreatePackage(nuGetPackage.Object, It.IsAny<User>(), false));
controller.MockEntitiesContext.VerifyCommitted();
}

[Fact]
public void WillCurateThePackage()
{
var nuGetPackage = new Mock<INupkg>();
nuGetPackage.Setup(x => x.Metadata.Id).Returns("theId");
nuGetPackage.Setup(x => x.Metadata.Version).Returns(new SemanticVersion("1.0.42"));
var user = new User() { EmailAddress = "[email protected]" };
var controller = new TestableApiController();
var apiKey = Guid.NewGuid();
controller.SetCurrentUser(user);
controller.SetupPackageFromInputStream(nuGetPackage);

controller.CreatePackagePut();

controller.MockAutoCuratePackage.Verify(x => x.Execute(It.IsAny<Package>(), nuGetPackage.Object, false));
controller.MockEntitiesContext.VerifyCommitted();
}

[Fact]
Expand All @@ -158,7 +179,8 @@ public void WillCreateAPackageWithTheUserMatchingTheApiKey()

controller.CreatePackagePut();

controller.MockPackageService.Verify(x => x.CreatePackage(It.IsAny<INupkg>(), user, true));
controller.MockPackageService.Verify(x => x.CreatePackage(It.IsAny<INupkg>(), user, false));
controller.MockEntitiesContext.VerifyCommitted();
}

[Fact]
Expand All @@ -171,7 +193,7 @@ public void CreatePackageRefreshesNuGetExeIfCommandLinePackageIsUploaded()
var user = new User() { EmailAddress = "[email protected]" };
var controller = new TestableApiController();
var apiKey = Guid.NewGuid();
controller.MockPackageService.Setup(p => p.CreatePackage(nuGetPackage.Object, It.IsAny<User>(), true))
controller.MockPackageService.Setup(p => p.CreatePackage(nuGetPackage.Object, It.IsAny<User>(), false))
.Returns(new Package { IsLatestStable = true });
controller.MockNuGetExeDownloaderService.Setup(s => s.UpdateExecutableAsync(nuGetPackage.Object)).Verifiable();
controller.SetCurrentUser(user);
Expand All @@ -194,7 +216,7 @@ public void CreatePackageDoesNotRefreshNuGetExeIfItIsNotLatestStable()
var user = new User() { EmailAddress = "[email protected]" };
var controller = new TestableApiController();
var apiKey = Guid.NewGuid();
controller.MockPackageService.Setup(p => p.CreatePackage(nuGetPackage.Object, It.IsAny<User>(), true))
controller.MockPackageService.Setup(p => p.CreatePackage(nuGetPackage.Object, It.IsAny<User>(), false))
.Returns(new Package { IsLatest = true, IsLatestStable = false });
controller.MockNuGetExeDownloaderService.Setup(s => s.UpdateExecutableAsync(nuGetPackage.Object)).Verifiable();
controller.SetCurrentUser(user);
Expand All @@ -205,6 +227,7 @@ public void CreatePackageDoesNotRefreshNuGetExeIfItIsNotLatestStable()

// Assert
controller.MockNuGetExeDownloaderService.Verify(s => s.UpdateExecutableAsync(It.IsAny<INupkg>()), Times.Never());
controller.MockEntitiesContext.VerifyCommitted();
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ public void WillIncludeThePackageWhenItIsTaggedWithAspNetWebPages()
var stubFeed = new CuratedFeed();
var stubGalleryPackage = CreateStubGalleryPackage();
var stubNuGetPackage = CreateStubNuGetPackage();
stubGalleryPackage.Tags = "aspnetwebpages";

bool result = WebMatrixPackageCurator.ShouldCuratePackage(
stubFeed,
Expand All @@ -91,9 +92,9 @@ public void WillNotExamineTheNuGetPackageFilesWhenTaggedWithAspNetWebPages()
{
var curator = new TestableWebMatrixPackageCurator();
var stubGalleryPackage = CreateStubGalleryPackage();
stubGalleryPackage.Tags = "aTag aspnetwebpages aThirdTag";
var stubNuGetPackage = CreateStubNuGetPackage();

stubGalleryPackage.Tags = "aTag aspnetwebpages aThirdTag";

curator.Curate(stubGalleryPackage, stubNuGetPackage.Object, commitChanges: true);

stubNuGetPackage.Verify(stub => stub.GetFiles(), Times.Never());
Expand Down Expand Up @@ -205,6 +206,42 @@ public void WillIncludeThePackageWhenThereIsNotPowerShellOrT4File()
Assert.True(result);
}

[Fact]
public void WillNotIncludeThePackageWhenMinClientVersionIsTooHigh()
{
var stubFeed = new CuratedFeed();
var stubGalleryPackage = CreateStubGalleryPackage();
var stubNuGetPackage = CreateStubNuGetPackage();
stubNuGetPackage.Setup(n => n.Metadata.MinClientVersion).Returns(new Version(3, 0));

bool result = WebMatrixPackageCurator.ShouldCuratePackage(
stubFeed,
stubGalleryPackage,
stubNuGetPackage.Object);

Assert.False(result);
}

[Fact]
public void WillNotIncludeThePackageWhenPackageDoesNotSupportNet40()
{
var stubFeed = new CuratedFeed();
var stubGalleryPackage = CreateStubGalleryPackage();
var stubNuGetPackage = CreateStubNuGetPackage();
stubGalleryPackage.Tags = "aspnetwebpages";
stubGalleryPackage.SupportedFrameworks.Add(new PackageFramework()
{
TargetFramework = "net45"
});

bool result = WebMatrixPackageCurator.ShouldCuratePackage(
stubFeed,
stubGalleryPackage,
stubNuGetPackage.Object);

Assert.False(result);
}

[Fact]
public void WillIncludeThePackageUsingTheCuratedFeedKey()
{
Expand All @@ -219,7 +256,7 @@ public void WillIncludeThePackageUsingTheCuratedFeedKey()
It.IsAny<PackageRegistration>(),
It.IsAny<bool>(),
It.IsAny<bool>(),
It.IsAny<string>(),
null,
It.IsAny<bool>()));
}

Expand All @@ -238,7 +275,7 @@ public void WillIncludeThePackageUsingThePackageRegistrationKey()
stubGalleryPackage.PackageRegistration,
It.IsAny<bool>(),
It.IsAny<bool>(),
It.IsAny<string>(),
null,
It.IsAny<bool>()));
}

Expand All @@ -255,7 +292,7 @@ public void WillSetTheAutomaticBitWhenIncludingThePackage()
It.IsAny<PackageRegistration>(),
It.IsAny<bool>(),
true,
It.IsAny<string>(),
null,
It.IsAny<bool>()));
}

Expand All @@ -275,6 +312,7 @@ private static Mock<INupkg> CreateStubNuGetPackage()
{
var stubNuGetPackage = new Mock<INupkg>();
stubNuGetPackage.Setup(stub => stub.GetFiles()).Returns(new string[] { });
stubNuGetPackage.Setup(stub => stub.Metadata.Id).Returns("test");
return stubNuGetPackage;
}

Expand Down