Restructure PackageService to better support endpoints requiring all versions of an ID (#6905)

Author: Scott Bommarito

src/NuGetGallery.Core/Services/CorePackageService.cs

@@ -285,15 +285,26 @@ public virtual async Task UpdatePackageSigningCertificateAsync(string packageId,
             }
         }
 
-        protected IQueryable<Package> GetPackagesByIdQueryable(string id)
+        protected IQueryable<Package> GetPackagesByIdQueryable(string id, bool withDeprecations = false)
         {
-            return _packageRepository
+            var packages = _packageRepository
                 .GetAll()
                 .Include(p => p.LicenseReports)
                 .Include(p => p.PackageRegistration)
                 .Include(p => p.User)
                 .Include(p => p.SymbolPackages)
                 .Where(p => p.PackageRegistration.Id == id);
+
+            if (withDeprecations)
+            {
+                packages = packages
+                    .Include(p => p.Deprecations.Select(d => d.AlternatePackage.PackageRegistration))
+                    .Include(p => p.Deprecations.Select(d => d.AlternatePackageRegistration))
+                    .Include(p => p.Deprecations.Select(d => d.Cves))
+                    .Include(p => p.Deprecations.Select(d => d.Cwes));
+            }
+
+            return packages;
         }
 
         private static Package FindPackage(IQueryable<Package> packages, Func<IQueryable<Package>, IQueryable<Package>> predicate = null)

src/NuGetGallery/Controllers/JsonApiController.cs

@@ -41,24 +41,22 @@ public JsonApiController(
         }
 
         [HttpGet]
-        public virtual ActionResult GetPackageOwners(string id, string version)
+        public virtual ActionResult GetPackageOwners(string id)
         {
-            var package = _packageService.FindPackageByIdAndVersion(id, version);
-            if (package == null)
+            var registration = _packageService.FindPackageRegistrationById(id);
+            if (registration == null)
             {
                 return Json(new { message = Strings.AddOwner_PackageNotFound });
             }
 
             var currentUser = GetCurrentUser();
-            if (ActionsRequiringPermissions.ManagePackageOwnership.CheckPermissionsOnBehalfOfAnyAccount(currentUser, package) != PermissionsCheckResult.Allowed)
+            if (ActionsRequiringPermissions.ManagePackageOwnership.CheckPermissionsOnBehalfOfAnyAccount(currentUser, registration) != PermissionsCheckResult.Allowed)
             {
                 return new HttpUnauthorizedResult();
             }
 
-            var packageRegistration = package.PackageRegistration;
-            var packageRegistrationOwners = package.PackageRegistration.Owners;
-            var allMatchingNamespaceOwners = package
-                .PackageRegistration
+            var packageRegistrationOwners = registration.Owners;
+            var allMatchingNamespaceOwners = registration
                 .ReservedNamespaces
                 .SelectMany(rn => rn.Owners)
                 .Distinct();
@@ -71,7 +69,7 @@ public virtual ActionResult GetPackageOwners(string id, string version)
                 .Select(u => new PackageOwnersResultViewModel(
                     u,
                     currentUser,
-                    packageRegistration,
+                    registration,
                     Url,
                     isPending: false,
                     isNamespaceOwner: true));
@@ -81,19 +79,19 @@ public virtual ActionResult GetPackageOwners(string id, string version)
                 .Select(u => new PackageOwnersResultViewModel(
                     u,
                     currentUser,
-                    packageRegistration,
+                    registration,
                     Url,
                     isPending: false,
                     isNamespaceOwner: false));
 
             owners = owners.Union(packageOwnersOnlyResultViewModel);
 
             var pending =
-                _packageOwnershipManagementService.GetPackageOwnershipRequests(package: package.PackageRegistration)
+                _packageOwnershipManagementService.GetPackageOwnershipRequests(package: registration)
                 .Select(r => new PackageOwnersResultViewModel(
                     r.NewOwner,
                     currentUser,
-                    packageRegistration,
+                    registration,
                     Url,
                     isPending: true,
                     isNamespaceOwner: false));

src/NuGetGallery/Controllers/PackagesController.cs

@@ -650,14 +650,26 @@ public virtual async Task<ActionResult> DisplayPackage(string id, string version
                 return RedirectToActionPermanent("DisplayPackage", new { id = id, version = normalized });
             }
 
-            Package package;
-            if (version != null && version.Equals(GalleryConstants.AbsoluteLatestUrlString, StringComparison.InvariantCultureIgnoreCase))
+            Package package = null;
+            // Load all packages with the ID.
+            var packages = _packageService.FindPackagesById(id);
+            if (version != null)
             {
-                package = _packageService.FindAbsoluteLatestPackageById(id, SemVerLevelKey.SemVer2);
+                if (version.Equals(GalleryConstants.AbsoluteLatestUrlString, StringComparison.InvariantCultureIgnoreCase))
+                {
+                    // The user is looking for the absolute latest version and not an exact version.
+                    package = packages.FirstOrDefault(p => p.IsLatestSemVer2);
+                }
+                else
+                {
+                    package = packages.SingleOrDefault(p => p.NormalizedVersion == NuGetVersionFormatter.Normalize(version));
+                }
             }
-            else
+
+            if (package == null)
             {
-                package = _packageService.FindPackageByIdAndVersion(id, version, SemVerLevelKey.SemVer2);
+                // If we cannot find the exact version or no version was provided, fall back to the latest version.
+                package = _packageService.FilterLatestPackage(packages, SemVerLevelKey.SemVer2, allowPrerelease: true);
             }
 
             // Validating packages should be hidden to everyone but the owners and admins.
@@ -669,7 +681,7 @@ public virtual async Task<ActionResult> DisplayPackage(string id, string version
             {
                 return HttpNotFound();
             }
-            
+
             var model = new DisplayPackageViewModel(package, currentUser);
 
             model.ValidatingTooLong = _validationService.IsValidatingTooLong(package);
@@ -1382,17 +1394,32 @@ public virtual ActionResult ManagePackageOwners(string id)
         [RequiresAccountConfirmation("manage a package")]
         public virtual async Task<ActionResult> Manage(string id, string version = null)
         {
-            var package = _packageService.FindPackageByIdAndVersion(id, version, SemVerLevelKey.SemVer2);
+            Package package = null;
+            // Load all versions of the package.
+            var packages = _packageService.FindPackagesById(id, withDeprecations: true);
+            if (version != null)
+            {
+                // Try to find the exact version if it was specified.
+                package = packages.SingleOrDefault(p => p.NormalizedVersion == NuGetVersionFormatter.Normalize(version));
+            }
+
+            if (package == null)
+            {
+                // If the exact version was not found, fall back to the latest version.
+                package = _packageService.FilterLatestPackage(packages, SemVerLevelKey.SemVer2, allowPrerelease: true);
+            }
+
             if (package == null)
             {
+                // If the package has no versions, return not found.
                 return HttpNotFound();
             }
 
             var model = new ManagePackageViewModel(
-                package, 
-                GetCurrentUser(), 
-                ReportMyPackageReasons, 
-                Url, 
+                package,
+                GetCurrentUser(),
+                ReportMyPackageReasons,
+                Url,
                 await _readMeService.GetReadMeMdAsync(package));
 
             if (!model.CanEdit && !model.CanManageOwners && !model.CanUnlistOrRelist)

src/NuGetGallery/Services/IPackageService.cs

@@ -17,6 +17,12 @@ namespace NuGetGallery
     /// </summary>
     public interface IPackageService : ICorePackageService
     {
+        /// <summary>
+        /// Returns all packages with an <see cref="Package.Id"/> of <paramref name="id"/>.
+        /// Includes deprecation entities if <paramref name="withDeprecations"/> is true.
+        /// </summary>
+        IReadOnlyCollection<Package> FindPackagesById(string id, bool withDeprecations = false);
+
         /// <summary>
         /// Gets the package with the given ID and version when exists;
         /// otherwise gets the latest package version for the given package ID matching the provided constraints.
@@ -26,9 +32,16 @@ public interface IPackageService : ICorePackageService
         /// <param name="semVerLevelKey">The SemVer-level key that determines the SemVer filter to be applied.</param>
         /// <param name="allowPrerelease"><c>True</c> indicating pre-release packages are allowed, otherwise <c>false</c>.</param>
         /// <returns></returns>
-        Package FindPackageByIdAndVersion(string id, string version, int? semVerLevelKey = null, bool allowPrerelease = true);
-
-        Package FindAbsoluteLatestPackageById(string id, int? semVerLevelKey);
+        Package FindPackageByIdAndVersion(
+            string id,
+            string version,
+            int? semVerLevelKey = null,
+            bool allowPrerelease = true);
+
+        Package FilterLatestPackage(
+            IReadOnlyCollection<Package> packages,
+            int? semVerLevelKey = SemVerLevelKey.SemVer2,
+            bool allowPrerelease = true);
 
         IEnumerable<Package> FindPackagesByOwner(User user, bool includeUnlisted, bool includeVersions = false);
 

src/NuGetGallery/Services/PackageService.cs

@@ -135,6 +135,11 @@ public override PackageRegistration FindPackageRegistrationById(string packageId
                 .SingleOrDefault(pr => pr.Id == packageId);
         }
 
+        public virtual IReadOnlyCollection<Package> FindPackagesById(string id, bool withDeprecations = false)
+        {
+            return GetPackagesByIdQueryable(id, withDeprecations).ToList();
+        }
+
         public virtual Package FindPackageByIdAndVersion(
             string id,
             string version,
@@ -170,62 +175,68 @@ public virtual Package FindPackageByIdAndVersion(
 
                 var packageVersions = packagesQuery.ToList();
 
-                // Fallback behavior: collect the latest version.
-                // Check SemVer-level and allow-prerelease constraints.
-                if (semVerLevelKey == SemVerLevelKey.SemVer2)
-                {
-                    package = packageVersions.FirstOrDefault(p => p.IsLatestStableSemVer2);
-
-                    if (package == null && allowPrerelease)
-                    {
-                        package = packageVersions.FirstOrDefault(p => p.IsLatestSemVer2);
-                    }
-                }
-
-                // Fallback behavior: collect the latest version.
-                // If SemVer-level is not defined, 
-                // or SemVer-level = 2.0.0 and no package was marked as SemVer2-latest,
-                // then check for packages marked as non-SemVer2 latest.
-                if (semVerLevelKey == SemVerLevelKey.Unknown
-                    || (semVerLevelKey == SemVerLevelKey.SemVer2 && package == null))
-                {
-                    package = packageVersions.FirstOrDefault(p => p.IsLatestStable);
-
-                    if (package == null && allowPrerelease)
-                    {
-                        package = packageVersions.FirstOrDefault(p => p.IsLatest);
-                    }
-                }
-
-                // If we couldn't find a package marked as latest, then
-                // return the most recent one (prerelease ones were already filtered out if appropriate...)
-                if (package == null)
-                {
-                    package = packageVersions.OrderByDescending(p => p.Version).FirstOrDefault();
-                }
+                package = FilterLatestPackageHelper(packageVersions, semVerLevelKey, allowPrerelease);
             }
 
             return package;
         }
 
-        public virtual Package FindAbsoluteLatestPackageById(string id, int? semVerLevelKey)
+        public virtual Package FilterLatestPackage(
+            IReadOnlyCollection<Package> packages,
+            int? semVerLevelKey = SemVerLevelKey.SemVer2,
+            bool allowPrerelease = true)
         {
-            var packageVersions = GetPackagesByIdQueryable(id);
+            return FilterLatestPackageHelper(
+                // Filter out prereleases in the list if prereleases are not allowed.
+                packages?.Where(p => allowPrerelease || !p.IsPrerelease).ToList(),
+                semVerLevelKey,
+                allowPrerelease);
+        }
 
-            Package package;
+        private static Package FilterLatestPackageHelper(
+            IReadOnlyCollection<Package> packages,
+            int? semVerLevelKey,
+            bool allowPrerelease)
+        {
+            if (packages == null)
+            {
+                throw new ArgumentNullException(nameof(packages));
+            }
+
+            Package package = null;
+
+            // Fallback behavior: collect the latest version.
+            // Check SemVer-level and allow-prerelease constraints.
             if (semVerLevelKey == SemVerLevelKey.SemVer2)
             {
-                package = packageVersions.FirstOrDefault(p => p.IsLatestSemVer2);
+                package = packages.FirstOrDefault(p => p.IsLatestStableSemVer2);
+
+                if (package == null && allowPrerelease)
+                {
+                    package = packages.FirstOrDefault(p => p.IsLatestSemVer2);
+                }
             }
-            else
+
+            // Fallback behavior: collect the latest version.
+            // If SemVer-level is not defined, 
+            // or SemVer-level = 2.0.0 and no package was marked as SemVer2-latest,
+            // then check for packages marked as non-SemVer2 latest.
+            if (semVerLevelKey == SemVerLevelKey.Unknown
+                || (semVerLevelKey == SemVerLevelKey.SemVer2 && package == null))
             {
-                package = packageVersions.FirstOrDefault(p => p.IsLatest);
+                package = packages.FirstOrDefault(p => p.IsLatestStable);
+
+                if (package == null && allowPrerelease)
+                {
+                    package = packages.FirstOrDefault(p => p.IsLatest);
+                }
             }
 
-            // If we couldn't find a package marked as latest, then return the most recent one 
+            // If we couldn't find a package marked as latest, then return the most recent one.
+            // Prereleases were already filtered out if appropriate.
             if (package == null)
             {
-                package = packageVersions.OrderByDescending(p => p.Version).FirstOrDefault();
+                package = packages.OrderByDescending(p => p.Version).FirstOrDefault();
             }
 
             return package;
@@ -277,7 +288,7 @@ private IEnumerable<Package> GetPackagesForOwners(IEnumerable<int> ownerKeys, bo
                         .ThenByDescending(p => p.Key)
                         .FirstOrDefault());
             }
-            
+
             return packages
                 .Include(p => p.PackageRegistration)
                 .Include(p => p.PackageRegistration.Owners)
@@ -360,7 +371,7 @@ public async Task RemovePackageOwnerAsync(PackageRegistration package, User user
                 await _packageRepository.CommitChangesAsync();
             }
         }
-        
+
         public bool WillPackageBeOrphanedIfOwnerRemoved(PackageRegistration package, User ownerToRemove)
         {
             return WillPackageBeOrphanedIfOwnerRemovedHelper(package.Owners, ownerToRemove);

tests/NuGetGallery.Facts/Controllers/JsonApiControllerFacts.cs

@@ -757,7 +757,7 @@ public void ReturnsFailureIfPackageNotFound()
                     var controller = GetController<JsonApiController>();
 
                     // Act
-                    var result = controller.GetPackageOwners("fakeId", "2.0.0");
+                    var result = controller.GetPackageOwners("fakeId");
                     dynamic data = ((JsonResult)result).Data;
 
                     // Assert
@@ -775,7 +775,7 @@ public void ReturnsFailureIfUserIsNotPackageOwner(Func<Fakes, User> getCurrentUs
                     controller.SetCurrentUser(currentUser);
 
                     // Act
-                    var result = controller.GetPackageOwners(fakes.Package.Id, fakes.Package.Packages.First().Version);
+                    var result = controller.GetPackageOwners(fakes.Package.Id);
 
                     // Assert
                     Assert.IsType<HttpUnauthorizedResult>(result);
@@ -808,7 +808,7 @@ private IEnumerable<PackageOwnersResultViewModel> InvokeAsUser(User currentUser)
                     var controller = GetController<JsonApiController>();
                     controller.SetCurrentUser(currentUser);
 
-                    var result = controller.GetPackageOwners("FakePackage", "2.0");
+                    var result = controller.GetPackageOwners("FakePackage");
                     return ((JsonResult)result).Data as IEnumerable<PackageOwnersResultViewModel>;
                 }