Merge pull request #2850 from NuGet/issue-2849

Title of newly uploaded package should not match existing package registration id

src/NuGetGallery/App_Start/DefaultDependenciesModule.cs

@@ -128,6 +128,11 @@ protected override void Load(ContainerBuilder builder)
                 .AsSelf()
                 .As<IUserService>()
                 .InstancePerLifetimeScope();
+
+            builder.RegisterType<PackageNamingConflictValidator>()
+                .AsSelf()
+                .As<IPackageNamingConflictValidator>()
+                .InstancePerLifetimeScope();
 
             builder.RegisterType<PackageService>()
                 .AsSelf()

src/NuGetGallery/Controllers/PackagesController.cs

@@ -804,28 +804,42 @@ public virtual ActionResult Edit(string id, string version, EditPackageRequest f
             var user = GetCurrentUser();
             if (!ModelState.IsValid)
             {
-                formData.PackageId = package.PackageRegistration.Id;
-                formData.PackageTitle = package.Title;
-                formData.Version = package.Version;
-
-                var packageRegistration = _packageService.FindPackageRegistrationById(id);
-                formData.PackageVersions = packageRegistration.Packages
-                        .OrderByDescending(p => new NuGetVersion(p.Version), Comparer<NuGetVersion>.Create((a, b) => a.CompareTo(b)))
-                        .ToList();
-
-                return View(formData);
+                return EditFailed(id, formData, package);
             }
 
             // Add the edit request to a queue where it will be processed in the background.
             if (formData.Edit != null)
             {
-                _editPackageService.StartEditPackageRequest(package, formData.Edit, user);
-                _entitiesContext.SaveChanges();
+                try
+                {
+                    _editPackageService.StartEditPackageRequest(package, formData.Edit, user);
+                    _entitiesContext.SaveChanges();
+                }
+                catch (EntityException ex)
+                {
+                    ModelState.AddModelError("Edit.VersionTitle", ex.Message);
+
+                    return EditFailed(id, formData, package);
+                }
             }
 
             return SafeRedirect(returnUrl ?? Url.Package(id, version));
         }
 
+        private ActionResult EditFailed(string id, EditPackageRequest formData, Package package)
+        {
+            formData.PackageId = package.PackageRegistration.Id;
+            formData.PackageTitle = package.Title;
+            formData.Version = package.Version;
+
+            var packageRegistration = _packageService.FindPackageRegistrationById(id);
+            formData.PackageVersions = packageRegistration.Packages
+                .OrderByDescending(p => new NuGetVersion(p.Version), Comparer<NuGetVersion>.Create((a, b) => a.CompareTo(b)))
+                .ToList();
+
+            return View(formData);
+        }
+
         [Authorize]
         [RequiresAccountConfirmation("accept ownership of a package")]
         public virtual ActionResult ConfirmOwner(string id, string username, string token)
@@ -1024,8 +1038,16 @@ public virtual async Task<ActionResult> VerifyPackage(VerifyPackageRequest formD
                 };
 
                 // update relevant database tables
-                package = _packageService.CreatePackage(nugetPackage, packageStreamMetadata, currentUser, commitChanges: false);
-                Debug.Assert(package.PackageRegistration != null);
+                try
+                { 
+                    package = _packageService.CreatePackage(nugetPackage, packageStreamMetadata, currentUser, commitChanges: false);
+                    Debug.Assert(package.PackageRegistration != null);
+                }
+                catch (EntityException ex)
+                {
+                    TempData["Message"] = ex.Message;
+                    return Redirect(Url.UploadPackage());
+                }
 
                 _packageService.PublishPackage(package, commitChanges: false);
 

src/NuGetGallery/NuGetGallery.csproj

@@ -37,7 +37,7 @@
     <MvcProjectUpgradeChecked>true</MvcProjectUpgradeChecked>
     <UseGlobalApplicationHostFile />
     <MvcBuildViews Condition=" '$(MvcBuildViews)' == '' ">false</MvcBuildViews>
-</PropertyGroup>
+  </PropertyGroup>
   <PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Debug|AnyCPU' ">
     <DebugSymbols>true</DebugSymbols>
     <DebugType>full</DebugType>
@@ -1185,6 +1185,8 @@
     <Compile Include="Services\PackageDeleteService.cs" />
     <Compile Include="Services\PackageFileService.cs" />
     <Compile Include="Filters\RequireSslAttribute.cs" />
+    <Compile Include="Services\PackageIdAndTitleValidationService.cs" />
+    <Compile Include="Services\PackageNamingConflictValidator.cs" />
     <Compile Include="Services\PackageSearchResults.cs" />
     <Compile Include="Services\JsonAggregateStatsService.cs" />
     <Compile Include="Services\SearchResults.cs" />
@@ -1622,7 +1624,6 @@
   <ItemGroup>
     <Folder Include="Areas\Admin\DynamicData\CustomPages\" />
     <Folder Include="Areas\Admin\Views\Shared\" />
-    <Folder Include="Packaging\" />
   </ItemGroup>
   <ItemGroup>
     <ProjectReference Include="..\NuGet.Services.Search.Client\NuGet.Services.Search.Client.csproj">

src/NuGetGallery/Services/EditPackageService.cs

@@ -1,22 +1,24 @@
 // 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.Data.Entity;
-using System.Diagnostics;
-using System.Globalization;
 using System.Linq;
 
 namespace NuGetGallery
 {
     public class EditPackageService
     {
         public IEntitiesContext EntitiesContext { get; set; }
+        public IPackageNamingConflictValidator PackageNamingConflictValidator { get; set; }
 
         public EditPackageService() { }
 
-        public EditPackageService(IEntitiesContext entitiesContext)
+        public EditPackageService(
+            IEntitiesContext entitiesContext,
+            IPackageNamingConflictValidator packageNamingConflictValidator)
         {
             EntitiesContext = entitiesContext;
+            PackageNamingConflictValidator = packageNamingConflictValidator;
         }
 
         /// <summary>
@@ -32,6 +34,11 @@ public virtual PackageEdit GetPendingMetadata(Package p)
 
         public virtual void StartEditPackageRequest(Package p, EditPackageVersionRequest request, User editingUser)
         {
+            if (PackageNamingConflictValidator.TitleConflictsWithExistingRegistrationId(p.PackageRegistration.Id, request.VersionTitle))
+            {
+                throw new EntityException(Strings.TitleMatchesExistingRegistration, request.VersionTitle);
+            }
+
             PackageEdit edit = new PackageEdit
             {
                 // Description

src/NuGetGallery/Services/PackageIdAndTitleValidationService.cs

@@ -0,0 +1,11 @@
+// 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.
+
+namespace NuGetGallery
+{
+    public interface IPackageNamingConflictValidator
+    {
+        bool TitleConflictsWithExistingRegistrationId(string registrationId, string packageTitle);
+        bool IdConflictsWithExistingPackageTitle(string registrationId);
+    }
+}

src/NuGetGallery/Services/PackageNamingConflictValidator.cs

@@ -0,0 +1,62 @@
+// 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.Linq;
+
+namespace NuGetGallery
+{
+    public class PackageNamingConflictValidator 
+        : IPackageNamingConflictValidator
+    {
+        private readonly IEntityRepository<PackageRegistration> _packageRegistrationRepository;
+        private readonly IEntityRepository<Package> _packageRepository;
+
+        public PackageNamingConflictValidator(
+            IEntityRepository<PackageRegistration> packageRegistrationRepository,
+            IEntityRepository<Package> packageRepository)
+        {
+            _packageRegistrationRepository = packageRegistrationRepository;
+            _packageRepository = packageRepository;
+        }
+
+        public bool TitleConflictsWithExistingRegistrationId(string registrationId, string packageTitle)
+        {
+            if (string.IsNullOrEmpty(registrationId))
+            {
+                throw new ArgumentNullException("registrationId");
+            }
+
+            if (!string.IsNullOrEmpty(packageTitle))
+            {
+                var cleanedTitle = packageTitle.Trim().ToLowerInvariant();
+                if (!string.IsNullOrEmpty(cleanedTitle))
+                {
+                    var packageRegistration = _packageRegistrationRepository.GetAll()
+                        .SingleOrDefault(pr => pr.Id.ToLower() == cleanedTitle);
+
+                    if (packageRegistration != null 
+                        && !String.Equals(packageRegistration.Id, registrationId, StringComparison.OrdinalIgnoreCase))
+                    {
+                        return true;
+                    }
+                }
+            }
+
+            return false;
+        }
+
+        public bool IdConflictsWithExistingPackageTitle(string registrationId)
+        {
+            if (string.IsNullOrEmpty(registrationId))
+            {
+                throw new ArgumentNullException("registrationId");
+            }
+
+            registrationId = registrationId.ToLowerInvariant();
+
+            return _packageRepository.GetAll()
+                .Any(p => p.Title.ToLower() == registrationId);
+        }
+    }
+}

src/NuGetGallery/Services/PackageService.cs

@@ -19,19 +19,22 @@ public class PackageService : IPackageService
         private readonly IEntityRepository<PackageRegistration> _packageRegistrationRepository;
         private readonly IEntityRepository<Package> _packageRepository;
         private readonly IEntityRepository<PackageStatistics> _packageStatsRepository;
+        private readonly IPackageNamingConflictValidator _packageNamingConflictValidator;
 
         public PackageService(
             IEntityRepository<PackageRegistration> packageRegistrationRepository,
             IEntityRepository<Package> packageRepository,
             IEntityRepository<PackageStatistics> packageStatsRepository,
             IEntityRepository<PackageOwnerRequest> packageOwnerRequestRepository,
-            IIndexingService indexingService)
+            IIndexingService indexingService,
+            IPackageNamingConflictValidator packageNamingConflictValidator)
         {
             _packageRegistrationRepository = packageRegistrationRepository;
             _packageRepository = packageRepository;
             _packageStatsRepository = packageStatsRepository;
             _packageOwnerRequestRepository = packageOwnerRequestRepository;
             _indexingService = indexingService;
+            _packageNamingConflictValidator = packageNamingConflictValidator;
         }
 
         public void EnsureValid(PackageReader packageReader)
@@ -40,6 +43,8 @@ public void EnsureValid(PackageReader packageReader)
 
             ValidateNuGetPackageMetadata(packageMetadata);
 
+            ValidatePackageTitle(packageMetadata);
+
             var supportedFrameworks = GetSupportedFrameworks(packageReader).Select(fn => fn.ToShortNameOrNull()).ToArray();
             if (!supportedFrameworks.AnySafe(sf => sf == null))
             {
@@ -53,6 +58,8 @@ public Package CreatePackage(PackageReader nugetPackage, PackageStreamMetadata p
 
             ValidateNuGetPackageMetadata(packageMetadata);
 
+            ValidatePackageTitle(packageMetadata);
+
             var packageRegistration = CreateOrGetPackageRegistration(user, packageMetadata);
 
             var package = CreatePackageFromNuGetPackage(packageRegistration, nugetPackage, packageMetadata, packageStreamMetadata, user);
@@ -393,6 +400,11 @@ private PackageRegistration CreateOrGetPackageRegistration(User currentUser, Pac
 
             if (packageRegistration == null)
             {
+                if (_packageNamingConflictValidator.IdConflictsWithExistingPackageTitle(packageMetadata.Id))
+                {
+                    throw new EntityException(Strings.NewRegistrationIdMatchesExistingPackageTitle, packageMetadata.Id);
+                }
+
                 packageRegistration = new PackageRegistration
                 {
                     Id = packageMetadata.Id
@@ -592,6 +604,14 @@ private static void ValidateSupportedFrameworks(string[] supportedFrameworks)
             }
         }
 
+        private void ValidatePackageTitle(PackageMetadata packageMetadata)
+        {
+            if (_packageNamingConflictValidator.TitleConflictsWithExistingRegistrationId(packageMetadata.Id, packageMetadata.Title))
+            {
+                throw new EntityException(Strings.TitleMatchesExistingRegistration, packageMetadata.Title);
+            }
+        }
+
         public void UpdateIsLatest(PackageRegistration packageRegistration, bool commitChanges = true)
         {
             if (!packageRegistration.Packages.Any())

src/NuGetGallery/Strings.resx

@@ -356,4 +356,10 @@ The {2} Team</value>
   <data name="No" xml:space="preserve">
     <value>No</value>
   </data>
+  <data name="TitleMatchesExistingRegistration" xml:space="preserve">
+    <value>The title of your package, '{0}', is similar to the ID of an existing package, which can cause confusion with our users. Please modify the title of your package and try uploading again.</value>
+  </data>
+  <data name="NewRegistrationIdMatchesExistingPackageTitle" xml:space="preserve">
+    <value>The ID of your package, '{0}', is similar to the title of an existing package, which can cause confusion with our users. Please modify the ID of your package and try uploading again.</value>
+  </data>
 </root>

tests/NuGetGallery.Facts/NuGetGallery.Facts.csproj

@@ -291,6 +291,7 @@
     <Compile Include="Filters\ApiAuthorizeAttributeFacts.cs" />
     <Compile Include="Filters\RequiresAccountConfirmationAttributeFacts.cs" />
     <Compile Include="Filters\RequireSslAttributeFacts.cs" />
+    <Compile Include="Services\PackageNamingConflictValidatorFacts.cs" />
     <Compile Include="TestPackage.cs" />
     <Compile Include="Framework\AssertEx.cs" />
     <Compile Include="Framework\Fakes.cs" />