Fix #1670 by improving WebMatrix curation

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<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);

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<ICuratedFeedService>().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;
         }
     }
 }

tests/NuGetGallery.Facts/Controllers/ApiControllerFacts.cs

@@ -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; }
 
@@ -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));
@@ -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]
@@ -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]
@@ -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);
@@ -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);
@@ -205,6 +227,7 @@ public void CreatePackageDoesNotRefreshNuGetExeIfItIsNotLatestStable()
 
                 // Assert
                 controller.MockNuGetExeDownloaderService.Verify(s => s.UpdateExecutableAsync(It.IsAny<INupkg>()), Times.Never());
+                controller.MockEntitiesContext.VerifyCommitted();
             }
         }
 

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<PackageRegistration>(),
                         It.IsAny<bool>(),
                         It.IsAny<bool>(),
-                        It.IsAny<string>(),
+                        null,
                         It.IsAny<bool>()));
             }
 
@@ -238,7 +275,7 @@ public void WillIncludeThePackageUsingThePackageRegistrationKey()
                         stubGalleryPackage.PackageRegistration,
                         It.IsAny<bool>(),
                         It.IsAny<bool>(),
-                        It.IsAny<string>(),
+                        null,
                         It.IsAny<bool>()));
             }
 
@@ -255,7 +292,7 @@ public void WillSetTheAutomaticBitWhenIncludingThePackage()
                         It.IsAny<PackageRegistration>(),
                         It.IsAny<bool>(),
                         true,
-                        It.IsAny<string>(),
+                        null,
                         It.IsAny<bool>()));
             }
 
@@ -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;
             }