From 589f4275e43b2d961a73b57d4c251e6b194c386b Mon Sep 17 00:00:00 2001 From: anurse Date: Mon, 25 Nov 2013 15:18:23 -0800 Subject: [PATCH] Fix #1670 by improving WebMatrix curation --- src/NuGetGallery/Controllers/ApiController.cs | 13 +++- .../WebMatrixPackageCurator.cs | 68 +++++++++++++------ .../Controllers/ApiControllerFacts.cs | 31 +++++++-- .../WebMatrixPackageCuratorFacts.cs | 48 +++++++++++-- 4 files changed, 127 insertions(+), 33 deletions(-) diff --git a/src/NuGetGallery/Controllers/ApiController.cs b/src/NuGetGallery/Controllers/ApiController.cs index 993568f9c3..f5555d556b 100644 --- a/src/NuGetGallery/Controllers/ApiController.cs +++ b/src/NuGetGallery/Controllers/ApiController.cs @@ -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() { } @@ -38,7 +39,8 @@ public ApiController( IUserService userService, INuGetExeDownloaderService nugetExeDownloaderService, IContentService contentService, - IIndexingService indexingService) + IIndexingService indexingService, + IAutomaticallyCuratePackageCommand autoCuratePackage) { EntitiesContext = entitiesContext; PackageService = packageService; @@ -48,6 +50,7 @@ public ApiController( ContentService = contentService; StatisticsService = null; IndexingService = indexingService; + AutoCuratePackage = autoCuratePackage; } public ApiController( @@ -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; } @@ -239,7 +243,10 @@ private async Task 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); diff --git a/src/NuGetGallery/PackageCurators/WebMatrixPackageCurator.cs b/src/NuGetGallery/PackageCurators/WebMatrixPackageCurator.cs index 5928e70438..38a8a389bb 100644 --- a/src/NuGetGallery/PackageCurators/WebMatrixPackageCurator.cs +++ b/src/NuGetGallery/PackageCurators/WebMatrixPackageCurator.cs @@ -1,5 +1,7 @@ -using System.IO; +using System; +using System.IO; using System.Linq; +using System.Runtime.Versioning; using NuGet; using NuGetGallery.Packaging; @@ -24,6 +26,7 @@ public override void Curate( GetService().CreatedCuratedPackage( curatedFeed, galleryPackage.PackageRegistration, + included: true, automaticallyCurated: true, commitChanges: commitChanges); } @@ -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; } } } \ No newline at end of file diff --git a/tests/NuGetGallery.Facts/Controllers/ApiControllerFacts.cs b/tests/NuGetGallery.Facts/Controllers/ApiControllerFacts.cs index 22d86c0dec..4f31eac7f9 100644 --- a/tests/NuGetGallery.Facts/Controllers/ApiControllerFacts.cs +++ b/tests/NuGetGallery.Facts/Controllers/ApiControllerFacts.cs @@ -31,6 +31,7 @@ class TestableApiController : ApiController public Mock MockContentService { get; private set; } public Mock MockStatisticsService { get; private set; } public Mock MockIndexingService { get; private set; } + public Mock MockAutoCuratePackage { get; private set; } private INupkg PackageFromInputStream { get; set; } @@ -44,6 +45,7 @@ public TestableApiController(MockBehavior behavior = MockBehavior.Default) ContentService = (MockContentService = new Mock()).Object; StatisticsService = (MockStatisticsService = new Mock()).Object; IndexingService = (MockIndexingService = new Mock()).Object; + AutoCuratePackage = (MockAutoCuratePackage = new Mock()).Object; MockPackageFileService = new Mock(MockBehavior.Strict); MockPackageFileService.Setup(p => p.SavePackageFileAsync(It.IsAny(), It.IsAny())).Returns(Task.FromResult(0)); @@ -141,7 +143,26 @@ public void WillCreateAPackageFromTheNuGetPackage() controller.CreatePackagePut(); - controller.MockPackageService.Verify(x => x.CreatePackage(nuGetPackage.Object, It.IsAny(), true)); + controller.MockPackageService.Verify(x => x.CreatePackage(nuGetPackage.Object, It.IsAny(), false)); + controller.MockEntitiesContext.VerifyCommitted(); + } + + [Fact] + public void WillCurateThePackage() + { + var nuGetPackage = new Mock(); + 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 = "confirmed@email.com" }; + var controller = new TestableApiController(); + var apiKey = Guid.NewGuid(); + controller.SetCurrentUser(user); + controller.SetupPackageFromInputStream(nuGetPackage); + + controller.CreatePackagePut(); + + controller.MockAutoCuratePackage.Verify(x => x.Execute(It.IsAny(), nuGetPackage.Object, false)); + controller.MockEntitiesContext.VerifyCommitted(); } [Fact] @@ -158,7 +179,8 @@ public void WillCreateAPackageWithTheUserMatchingTheApiKey() controller.CreatePackagePut(); - controller.MockPackageService.Verify(x => x.CreatePackage(It.IsAny(), user, true)); + controller.MockPackageService.Verify(x => x.CreatePackage(It.IsAny(), user, false)); + controller.MockEntitiesContext.VerifyCommitted(); } [Fact] @@ -171,7 +193,7 @@ public void CreatePackageRefreshesNuGetExeIfCommandLinePackageIsUploaded() var user = new User() { EmailAddress = "confirmed@email.com" }; var controller = new TestableApiController(); var apiKey = Guid.NewGuid(); - controller.MockPackageService.Setup(p => p.CreatePackage(nuGetPackage.Object, It.IsAny(), true)) + controller.MockPackageService.Setup(p => p.CreatePackage(nuGetPackage.Object, It.IsAny(), false)) .Returns(new Package { IsLatestStable = true }); controller.MockNuGetExeDownloaderService.Setup(s => s.UpdateExecutableAsync(nuGetPackage.Object)).Verifiable(); controller.SetCurrentUser(user); @@ -194,7 +216,7 @@ public void CreatePackageDoesNotRefreshNuGetExeIfItIsNotLatestStable() var user = new User() { EmailAddress = "confirmed@email.com" }; var controller = new TestableApiController(); var apiKey = Guid.NewGuid(); - controller.MockPackageService.Setup(p => p.CreatePackage(nuGetPackage.Object, It.IsAny(), true)) + controller.MockPackageService.Setup(p => p.CreatePackage(nuGetPackage.Object, It.IsAny(), false)) .Returns(new Package { IsLatest = true, IsLatestStable = false }); controller.MockNuGetExeDownloaderService.Setup(s => s.UpdateExecutableAsync(nuGetPackage.Object)).Verifiable(); controller.SetCurrentUser(user); @@ -205,6 +227,7 @@ public void CreatePackageDoesNotRefreshNuGetExeIfItIsNotLatestStable() // Assert controller.MockNuGetExeDownloaderService.Verify(s => s.UpdateExecutableAsync(It.IsAny()), Times.Never()); + controller.MockEntitiesContext.VerifyCommitted(); } } diff --git a/tests/NuGetGallery.Facts/PackageCurators/WebMatrixPackageCuratorFacts.cs b/tests/NuGetGallery.Facts/PackageCurators/WebMatrixPackageCuratorFacts.cs index 9a9a088e91..39ee695d26 100644 --- a/tests/NuGetGallery.Facts/PackageCurators/WebMatrixPackageCuratorFacts.cs +++ b/tests/NuGetGallery.Facts/PackageCurators/WebMatrixPackageCuratorFacts.cs @@ -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, @@ -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()); @@ -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() { @@ -219,7 +256,7 @@ public void WillIncludeThePackageUsingTheCuratedFeedKey() It.IsAny(), It.IsAny(), It.IsAny(), - It.IsAny(), + null, It.IsAny())); } @@ -238,7 +275,7 @@ public void WillIncludeThePackageUsingThePackageRegistrationKey() stubGalleryPackage.PackageRegistration, It.IsAny(), It.IsAny(), - It.IsAny(), + null, It.IsAny())); } @@ -255,7 +292,7 @@ public void WillSetTheAutomaticBitWhenIncludingThePackage() It.IsAny(), It.IsAny(), true, - It.IsAny(), + null, It.IsAny())); } @@ -275,6 +312,7 @@ private static Mock CreateStubNuGetPackage() { var stubNuGetPackage = new Mock(); stubNuGetPackage.Setup(stub => stub.GetFiles()).Returns(new string[] { }); + stubNuGetPackage.Setup(stub => stub.Metadata.Id).Returns("test"); return stubNuGetPackage; }