Detect if package only differ by metadata and show optimal user-facin…

…g error message (#3970)
NuGet · May 18, 2017 · 117a1e4 · 117a1e4
1 parent b50c6e9
commit 117a1e4
Show file tree

Hide file tree

Showing 4 changed files with 66 additions and 9 deletions.
diff --git a/src/NuGetGallery/Controllers/PackagesController.cs b/src/NuGetGallery/Controllers/PackagesController.cs
@@ -282,13 +282,35 @@ public virtual async Task<ActionResult> UploadPackage(HttpPostedFileBase uploadF
                     return View();
                 }
 
-                var package = _packageService.FindPackageByIdAndVersionStrict(nuspec.GetId(), nuspec.GetVersion().ToStringSafe());
-                if (package != null)
+                var nuspecVersion = nuspec.GetVersion();
+                var existingPackage = _packageService.FindPackageByIdAndVersionStrict(nuspec.GetId(), nuspecVersion.ToStringSafe());
+                if (existingPackage != null)
                 {
-                    ModelState.AddModelError(
-                        string.Empty,
-                        string.Format(
-                            CultureInfo.CurrentCulture, Strings.PackageExistsAndCannotBeModified, package.PackageRegistration.Id, package.Version));
+                    // Determine if the package versions only differ by metadata, 
+                    // and provide the most optimal the user-facing error message.
+                    var existingPackageVersion = new NuGetVersion(existingPackage.Version);
+                    if ((existingPackageVersion.HasMetadata || nuspecVersion.HasMetadata) 
+                        && !string.Equals(existingPackageVersion.Metadata, nuspecVersion.Metadata))
+                    {
+                        ModelState.AddModelError(
+                            string.Empty,
+                            string.Format(
+                                CultureInfo.CurrentCulture, 
+                                Strings.PackageVersionDiffersOnlyByMetadataAndCannotBeModified, 
+                                existingPackage.PackageRegistration.Id, 
+                                existingPackage.Version));
+                    }
+                    else
+                    {
+                        ModelState.AddModelError(
+                            string.Empty,
+                            string.Format(
+                                CultureInfo.CurrentCulture, 
+                                Strings.PackageExistsAndCannotBeModified, 
+                                existingPackage.PackageRegistration.Id, 
+                                existingPackage.Version));
+                    }
+
                     return View();
                 }
 

diff --git a/src/NuGetGallery/Strings.Designer.cs b/src/NuGetGallery/Strings.Designer.cs
diff --git a/src/NuGetGallery/Strings.resx b/src/NuGetGallery/Strings.resx
@@ -443,4 +443,7 @@ The {1} Team</value>
   <data name="SecurityPolicy_RequireMinClientVersionForPush" xml:space="preserve">
     <value>Your account requires client version '{0}' or higher to be able to push packages. Please contact support@nuget.org to get more details.</value>
   </data>
+  <data name="PackageVersionDiffersOnlyByMetadataAndCannotBeModified" xml:space="preserve">
+    <value>Package versions that differ only by metadata cannot be uploaded. A package with ID '{0}' and version '{1}' already exists and cannot be modified.</value>
+  </data>
 </root>
diff --git a/tests/NuGetGallery.Facts/Controllers/PackagesControllerFacts.cs b/tests/NuGetGallery.Facts/Controllers/PackagesControllerFacts.cs
@@ -1094,7 +1094,7 @@ public async Task WillShowTheViewWithErrorsWhenThePackageAlreadyExists()
                 fakeUploadedFile.Setup(x => x.InputStream).Returns(fakeFileStream);
                 var fakePackageService = new Mock<IPackageService>();
                 fakePackageService.Setup(x => x.FindPackageByIdAndVersionStrict(It.IsAny<string>(), It.IsAny<string>())).Returns(
-                    new Package { PackageRegistration = new PackageRegistration { Id = "theId" }, Version = "theVersion" });
+                    new Package { PackageRegistration = new PackageRegistration { Id = "theId" }, Version = "1.0.0" });
                 var controller = CreateController(
                     packageService: fakePackageService);
                 controller.SetCurrentUser(TestUtility.FakeUser);
@@ -1104,7 +1104,30 @@ public async Task WillShowTheViewWithErrorsWhenThePackageAlreadyExists()
                 Assert.NotNull(result);
                 Assert.False(controller.ModelState.IsValid);
                 Assert.Equal(
-                    String.Format(Strings.PackageExistsAndCannotBeModified, "theId", "theVersion"),
+                    String.Format(Strings.PackageExistsAndCannotBeModified, "theId", "1.0.0"),
+                    controller.ModelState[String.Empty].Errors[0].ErrorMessage);
+            }
+
+            [Fact]
+            public async Task WillShowTheViewWithErrorsWhenThePackageAlreadyExistsAndOnlyDiffersByMetadata()
+            {
+                var fakeUploadedFile = new Mock<HttpPostedFileBase>();
+                fakeUploadedFile.Setup(x => x.FileName).Returns("theFile.nupkg");
+                var fakeFileStream = TestPackage.CreateTestPackageStream("theId", "1.0.0+metadata2");
+                fakeUploadedFile.Setup(x => x.InputStream).Returns(fakeFileStream);
+                var fakePackageService = new Mock<IPackageService>();
+                fakePackageService.Setup(x => x.FindPackageByIdAndVersionStrict(It.IsAny<string>(), It.IsAny<string>())).Returns(
+                    new Package { PackageRegistration = new PackageRegistration { Id = "theId" }, Version = "1.0.0+metadata" });
+                var controller = CreateController(
+                    packageService: fakePackageService);
+                controller.SetCurrentUser(TestUtility.FakeUser);
+
+                var result = await controller.UploadPackage(fakeUploadedFile.Object) as ViewResult;
+
+                Assert.NotNull(result);
+                Assert.False(controller.ModelState.IsValid);
+                Assert.Equal(
+                    String.Format(Strings.PackageVersionDiffersOnlyByMetadataAndCannotBeModified, "theId", "1.0.0+metadata"),
                     controller.ModelState[String.Empty].Errors[0].ErrorMessage);
             }