Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Tilovell/403 latest stable should be default package version seen #1626

Merged
merged 3 commits into from
Oct 7, 2013
Merged

Tilovell/403 latest stable should be default package version seen #1626

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
fd20ca4
Low level part of fixing #403.
Oct 3, 2013
3593a22
UI fixes to reflect that we now take you to the latest stable package…
Oct 3, 2013
809f3b2
MINOR - test naming nit
Oct 3, 2013
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 7 additions & 7 deletions src/NuGetGallery/Services/PackageService.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,27 +106,27 @@ public virtual Package FindPackageByIdAndVersion(string id, string version, bool
.Include(p => p.LicenseReports)
.Include(p => p.PackageRegistration)
.Where(p => (p.PackageRegistration.Id == id));

if (String.IsNullOrEmpty(version) && !allowPrerelease)
{
// If there's a specific version given, don't bother filtering by prerelease. You could be asking for a prerelease package.
packagesQuery = packagesQuery.Where(p => !p.IsPrerelease);
}

var packageVersions = packagesQuery.ToList();

Package package;
if (version == null)
if (String.IsNullOrEmpty(version))
{
if (allowPrerelease)
package = packageVersions.FirstOrDefault(p => p.IsLatestStable);

if (package == null && allowPrerelease)
{
package = packageVersions.FirstOrDefault(p => p.IsLatest);
}
else
{
package = packageVersions.FirstOrDefault(p => p.IsLatestStable);
}

// If we couldn't find a package marked as latest, then
// return the most recent one.
// return the most recent one (prerelease ones were already filtered out if appropriate...)
if (package == null)
{
package = packageVersions.OrderByDescending(p => p.Version).FirstOrDefault();
Expand Down
14 changes: 0 additions & 14 deletions src/NuGetGallery/ViewModels/DisplayPackageViewModel.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,19 +45,5 @@ public void SetPendingMetadata(PackageEdit pendingMetadata)
public string Copyright { get; set; }

public bool HasPendingMetadata { get; private set; }

public bool IsLatestVersionAvailable
{
get
{
// A package can be identified as the latest available a few different ways
// First, if it's marked as the latest stable version
return LatestStableVersion
// Or if it's marked as the latest version (pre-release)
|| LatestVersion
// Or if it's the current version and no version is marked as the latest (because they're all unlisted)
|| (IsCurrent(this) && !PackageVersions.Any(p => p.LatestVersion));
}
}
}
}
11 changes: 6 additions & 5 deletions src/NuGetGallery/Views/Packages/DisplayPackage.cshtml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -169,11 +169,10 @@
This is a prerelease version of @Model.Title.
</p>
}
else if (!Model.IsLatestVersionAvailable)
else if (!Model.LatestVersion)
{
<p class="not-latest-message">
This is not the <a href="@Url.Package(Model.Id)" title="View the latest version">latest
version</a> of @Model.Title available.
There is a newer prerelease version of this package available. See the version list below for details.
</p>
}
<hgroup class="page-heading">
Expand Down Expand Up @@ -249,7 +248,8 @@
}
@if (Model.Dependencies.DependencySets == null)
{
if(Model.IsOwner(User)) {
if (Model.IsOwner(User))
{
<h3>Dependencies</h3>
<p>
An error occurred processing dependencies.
Expand All @@ -259,7 +259,8 @@
<strong>Note: This message is only visible to you and any other package owners.</strong></p>
}
}
else {
else
{
<h3>Dependencies</h3>
@Html.Partial("_PackageDependencies", Model.Dependencies)
}
Expand Down
115 changes: 62 additions & 53 deletions tests/NuGetGallery.Facts/Services/PackageServiceFacts.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
using NuGet;
using NuGetGallery.Packaging;
using Xunit;
using Xunit.Extensions;

namespace NuGetGallery
{
Expand Down Expand Up @@ -1046,108 +1047,116 @@ public void WillThrowIfThePackageDoesNotExist()
public class TheFindPackageByIdAndVersionMethod
{
[Fact]
public void WillGetTheLatestVersionWhenTheVersionArgumentIsNull()
{
var packageRegistration = new PackageRegistration { Id = "theId" };
var packages = new[]
{
new Package { Version = "1.0", PackageRegistration = packageRegistration },
new Package
{ Version = "2.0", PackageRegistration = packageRegistration, IsLatestStable = true, IsLatest = true }
}.AsQueryable();
var packageRepository = new Mock<IEntityRepository<Package>>();
packageRepository.Setup(r => r.GetAll()).Returns(packages);
var service = CreateService(packageRepository: packageRepository);

var package = service.FindPackageByIdAndVersion("theId", null);

Assert.Equal("2.0", package.Version);
}

[Fact]
public void WillGetSpecifiedVersionWhenTheVersionArgumentIsNotNull()
public void ReturnsTheRequestedPackageVersion()
{
var service = CreateService(setup:
mockPackageService =>
{
mockPackageService.Setup(x => x.FindPackageRegistrationById(It.IsAny<string>())).Throws(
new Exception("This should not be called when the version is specified."));
});
mockPackageService =>
{
mockPackageService.Setup(x => x.FindPackageRegistrationById(It.IsAny<string>())).Throws(
new Exception("This should not be called when the version is specified."));
});

Assert.DoesNotThrow(() => service.FindPackageByIdAndVersion("theId", "1.0.42"));

// Nothing to assert because it's too damn complicated to test the actual LINQ expression.
// What we're testing via the throw above is that it didn't load the registration and get the latest version.
}

[Fact]
public void WillThrowIfIdIsNull()
[Theory]
[InlineData(null)]
[InlineData("")]
public void WillThrowIfIdIsNullOrEmpty(string id)
{
var service = CreateService();
var ex = Assert.Throws<ArgumentNullException>(() => service.FindPackageByIdAndVersion(id, "1.0.42"));
Assert.Equal("id", ex.ParamName);
}

var ex = Assert.Throws<ArgumentNullException>(() => service.FindPackageByIdAndVersion(null, "1.0.42"));
[Fact]
public void ReturnsTheLatestStableVersionIfAvailable()
{
// Arrange
var repository = new Mock<IEntityRepository<Package>>(MockBehavior.Strict);
var packageRegistration = new PackageRegistration { Id = "theId" };
var package1 = new Package { Version = "1.0", PackageRegistration = packageRegistration, Listed = true, IsLatestStable = true };
var package2 = new Package { Version = "1.0.0a", PackageRegistration = packageRegistration, IsPrerelease = true, Listed = true, IsLatest = true };

Assert.Equal("id", ex.ParamName);
repository
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't know if it's in dev-start yet, but I have a helper in TestUtils\MockExtensions.cs called "HasData" which lets you rewrite these as: repository.HasData(new[] {...}). It does the Setup, Returns and AsQueryable for you :)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't look like it's there yet. CanHasData = false. :)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh I hadn't pulled dev-start looks like.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CanHasData = true!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or not. Still NoCanHasData. Unless there's another dev-start coming up?

.Setup(repo => repo.GetAll())
.Returns(new[] { package1, package2 }.AsQueryable());
var service = CreateService(packageRepository: repository);

// Act
var result = service.FindPackageByIdAndVersion("theId", version: null);

// Assert
Assert.NotNull(result);
Assert.Equal("1.0", result.Version);
}

[Fact]
public void FindPackageReturnsTheLatestVersionIfAvailable()
public void ReturnsTheLatestVersionIfNoLatestStableVersionIsAvailable()
{
// Arrange
var repository = new Mock<IEntityRepository<Package>>(MockBehavior.Strict);
var package = CreatePackage("Foo", "1.0.0");
package.IsLatest = true;
package.IsLatestStable = true;
var packageA = CreatePackage("Foo", "1.0.0a");
var packageRegistration = new PackageRegistration { Id = "theId" };
var package1 = new Package { Version = "1.0.0b", PackageRegistration = packageRegistration, IsPrerelease = true, Listed = true, IsLatest = true };
var package2 = new Package { Version = "1.0.0a", PackageRegistration = packageRegistration, IsPrerelease = true, Listed = true };

repository.Setup(repo => repo.GetAll())
.Returns(new[] { package, packageA }.AsQueryable());
repository
.Setup(repo => repo.GetAll())
.Returns(new[] { package1, package2 }.AsQueryable());
var service = CreateService(packageRepository: repository);

// Act
var result = service.FindPackageByIdAndVersion("Foo", version: null);
var result = service.FindPackageByIdAndVersion("theId", version: null);

// Assert
Assert.Equal(package, result);
Assert.NotNull(result);
Assert.Equal("1.0.0b", result.Version);
}

[Fact]
public void FindPackageReturnsTheLatestVersionIfNoLatestStableVersionIsAvailable()
public void ReturnsNullIfNoLatestStableVersionIsAvailableAndPrereleaseIsDisallowed()
{
// Arrange
var repository = new Mock<IEntityRepository<Package>>(MockBehavior.Strict);
var package = CreatePackage("Foo", "1.0.0b");
package.IsLatest = true;
var packageA = CreatePackage("Foo", "1.0.0a");
var packageRegistration = new PackageRegistration { Id = "theId" };
var package1 = new Package { Version = "1.0.0b", PackageRegistration = packageRegistration, IsPrerelease = true, Listed = true, IsLatest = true };
var package2 = new Package { Version = "1.0.0a", PackageRegistration = packageRegistration, IsPrerelease = true, Listed = true };

repository.Setup(repo => repo.GetAll())
.Returns(new[] { package, packageA }.AsQueryable());
repository
.Setup(repo => repo.GetAll())
.Returns(new[] { package1, package2 }.AsQueryable());
var service = CreateService(packageRepository: repository);

// Act
var result = service.FindPackageByIdAndVersion("Foo", null);
var result = service.FindPackageByIdAndVersion("theId", version: null, allowPrerelease: false);

// Assert
Assert.Equal(package, result);
Assert.Null(result);
}

[Fact]
public void FindPackageReturnsTheLatestVersionIfNoLatestVersionIsAvailable()
public void ReturnsTheMostRecentVersionIfNoLatestVersionIsAvailable()
{
// Arrange
var repository = new Mock<IEntityRepository<Package>>(MockBehavior.Strict);
var package = CreatePackage("Foo", "1.0.0b");
var packageA = CreatePackage("Foo", "1.0.0a");
var packageRegistration = new PackageRegistration { Id = "theId" };
var package1 = new Package { Version = "1.0.0b", PackageRegistration = packageRegistration, IsPrerelease = true, Listed = false };
var package2 = new Package { Version = "1.0.0a", PackageRegistration = packageRegistration, IsPrerelease = true, Listed = false };

repository.Setup(repo => repo.GetAll())
.Returns(new[] { package, packageA }.AsQueryable());
repository
.Setup(repo => repo.GetAll())
.Returns(new[] { package1, package2 }.AsQueryable());
var service = CreateService(packageRepository: repository);

// Act
var result = service.FindPackageByIdAndVersion("Foo", null);
var result = service.FindPackageByIdAndVersion("theId", null);

// Assert
Assert.Equal(package, result);
Assert.NotNull(result);
Assert.Equal("1.0.0b", result.Version);
}
}

Expand Down