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

Fix package id case-insensitivity in different places #1

Open
wants to merge 6 commits into
base: dev
Choose a base branch
from
Open

Fix package id case-insensitivity in different places #1

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
eaefc69
Correct PackageEqualityComparer for using in hashtables
derigel23 Jun 26, 2015
070893a
Indirect test of PackageEqualityComparer.Id
derigel23 Sep 3, 2015
23138ff
GetPackageCommand return packages in case-insensitive order
derigel23 Sep 3, 2015
d90e34f
ListCommand return packages in case-insensitive order
derigel23 Sep 3, 2015
82ca675
Indirect test of PackageEqualityComparer.IdAndVersion
derigel23 Sep 3, 2015
18dad6f
Indirect test of PackageEqualityComparer.IdAndVersion
derigel23 Sep 3, 2015
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
4 changes: 2 additions & 2 deletions src/CommandLine/Commands/ListCommand.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ public IEnumerable<IPackage> GetPackages()
includeDelisted: IncludeDelisted);
if (AllVersions)
{
return packages.OrderBy(p => p.Id);
return packages.OrderBy(p => p.Id, StringComparer.OrdinalIgnoreCase);
}
else
{
Expand All @@ -60,7 +60,7 @@ public IEnumerable<IPackage> GetPackages()
}
}

var result = packages.OrderBy(p => p.Id)
var result = packages.OrderBy(p => p.Id, StringComparer.OrdinalIgnoreCase)
.AsEnumerable();

// we still need to do client side filtering of delisted & prerelease packages.
Expand Down
10 changes: 5 additions & 5 deletions src/Core/Utility/PackageEqualityComparer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,13 @@ namespace NuGet
public sealed class PackageEqualityComparer : IEqualityComparer<IPackageName>
{
[SuppressMessage("Microsoft.Security", "CA2104:DoNotDeclareReadOnlyMutableReferenceTypes", Justification = "This type isn't mutable")]
public static readonly PackageEqualityComparer IdAndVersion = new PackageEqualityComparer((x, y) => x.Id.Equals(y.Id, StringComparison.OrdinalIgnoreCase) &&
x.Version.Equals(y.Version),
x => x.Id.GetHashCode() ^ x.Version.GetHashCode());
public static readonly PackageEqualityComparer IdAndVersion = new PackageEqualityComparer(
(x, y) => StringComparer.OrdinalIgnoreCase.Equals(x.Id, y.Id) && x.Version.Equals(y.Version),
x => StringComparer.OrdinalIgnoreCase.GetHashCode(x.Id) ^ x.Version.GetHashCode());

[SuppressMessage("Microsoft.Security", "CA2104:DoNotDeclareReadOnlyMutableReferenceTypes", Justification = "This type isn't mutable")]
public static readonly PackageEqualityComparer Id = new PackageEqualityComparer((x, y) => x.Id.Equals(y.Id, StringComparison.OrdinalIgnoreCase),
x => x.Id.GetHashCode());
public static readonly PackageEqualityComparer Id = new PackageEqualityComparer(
(x, y) => StringComparer.OrdinalIgnoreCase.Equals(x.Id, y.Id), x => StringComparer.OrdinalIgnoreCase.GetHashCode(x.Id));

private readonly Func<IPackageName, IPackageName, bool> _equals;
private readonly Func<IPackageName, int> _getHashCode;
Expand Down
2 changes: 1 addition & 1 deletion src/VsConsole/PowerShellCmdlets/GetPackageCommand.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -285,7 +285,7 @@ protected virtual IQueryable<IPackage> GetPackages(IPackageRepository sourceRepo
? sourceRepository.GetPackages()
: sourceRepository.Search(Filter, effectiveIncludePrerelease);
// by default, sort packages by Id
packages = packages.OrderBy(p => p.Id);
packages = packages.OrderBy(p => p.Id, StringComparer.OrdinalIgnoreCase);
return packages;
}

Expand Down
5 changes: 3 additions & 2 deletions test/CommandLine.Test/ListCommandTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ public void GetPackageReturnsAllVersionsIfAllVersionsFlagIsSet()

// Assert
Assert.Equal(5, packages.Count());
AssertPackage(new { Id = "jQuery", Ver = "1.44" }, packages.ElementAt(0));
AssertPackage(new { Id = "JQuery", Ver = "1.44" }, packages.ElementAt(0));
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be a separate test, each test should represent one feature/behavior tested to the degree possible

AssertPackage(new { Id = "jQuery", Ver = "1.50" }, packages.ElementAt(1));
AssertPackage(new { Id = "NHibernate", Ver = "1.0" }, packages.ElementAt(2));
AssertPackage(new { Id = "NHibernate", Ver = "1.1" }, packages.ElementAt(3));
Expand Down Expand Up @@ -365,7 +365,8 @@ private static IPackageRepositoryFactory CreatePackageRepositoryFactory()
multiVersionRepo.AddPackage(PackageUtility.CreatePackage("NHibernate", "1.0"));
multiVersionRepo.AddPackage(PackageUtility.CreatePackage("NHibernate", "1.1"));
multiVersionRepo.AddPackage(PackageUtility.CreatePackage("NHibernate", "1.2"));
multiVersionRepo.AddPackage(PackageUtility.CreatePackage("jQuery", "1.44"));
// different case is intended to test PackageEqualityComparer.Id
multiVersionRepo.AddPackage(PackageUtility.CreatePackage("JQuery", "1.44"));
multiVersionRepo.AddPackage(PackageUtility.CreatePackage("jQuery", "1.50"));

//Setup Factory
Expand Down
50 changes: 40 additions & 10 deletions test/Core.Test/LocalPackageRepositoryTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -259,25 +259,25 @@ public void FindPackagesByIdReturnsEmptySequenceIfNoPackagesWithSpecifiedIdAreFo
[Fact]
public void FindPackagesByIdFindsPackagesWithSpecifiedId()
{
// Arramge
// Arrange
var fileSystem = new MockFileSystem();
fileSystem.AddFile(PathFixUtility.FixPath(@"Foo.1.0\Foo.1.0.nupkg"));
fileSystem.AddFile(PathFixUtility.FixPath(@"Foo.2.0.0\Foo.2.0.0.nupkg"));
var foo_10 = PackageUtility.CreatePackage("Foo", "1.0");
var foo_20 = PackageUtility.CreatePackage("Foo", "2.0.0");

var package_dictionary = new Dictionary<String, IPackage>
{
{ PathFixUtility.FixPath(@"Foo.1.0\Foo.1.0.nupkg"), foo_10},
{ PathFixUtility.FixPath(@"Foo.2.0.0\Foo.2.0.0.nupkg"), foo_20}
};
{
{ PathFixUtility.FixPath(@"Foo.1.0\Foo.1.0.nupkg"), foo_10},
{ PathFixUtility.FixPath(@"Foo.2.0.0\Foo.2.0.0.nupkg"), foo_20}
};

var localPackageRepository = new MockLocalRepository(fileSystem, path =>
{
IPackage retval;
package_dictionary.TryGetValue(path, out retval);
return retval;
});
{
IPackage retval;
package_dictionary.TryGetValue(path, out retval);
return retval;
});

// Act
var packages = localPackageRepository.FindPackagesById("Foo").ToList();
Expand All @@ -286,6 +286,36 @@ public void FindPackagesByIdFindsPackagesWithSpecifiedId()
Assert.Equal(new[] { foo_10, foo_20 }, packages);
}

[Fact]
public void FindPackagesByIdFindsPackagesWithSpecifiedIdCaseInsensitive()
{
// Arrange
var fileSystem = new MockFileSystem();
fileSystem.AddFile(PathFixUtility.FixPath(@"FOO.2.0.0\FOO.2.0.0.nuspec"));
fileSystem.AddFile(PathFixUtility.FixPath(@"Foo.2.0.0\Foo.2.0.0.nupkg"));
var FOO_20 = PackageUtility.CreatePackage("FOO", "2.0.0");
var foo_20 = PackageUtility.CreatePackage("Foo", "2.0.0");

var package_dictionary = new Dictionary<String, IPackage>
{
{ PathFixUtility.FixPath(@"FOO.2.0.0\FOO.2.0.0.nuspec"), FOO_20},
{ PathFixUtility.FixPath(@"Foo.2.0.0\Foo.2.0.0.nupkg"), foo_20}
};

var localPackageRepository = new MockLocalRepository(fileSystem, path =>
{
IPackage retval;
package_dictionary.TryGetValue(path, out retval);
return retval;
});

// Act
var packages = localPackageRepository.FindPackagesById("FOO").ToList();

// Assert
Assert.Equal(new[] { foo_20 }, packages);
}

[Fact]
public void FindPackagesByIdIgnoresPartialIdMatches()
{
Expand Down
5 changes: 3 additions & 2 deletions test/PowerShellCmdlets.Test/GetPackageCommandTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -407,7 +407,7 @@ public void GetPackagesDoesNotCollapseVersionIfAllVersionsIsPresent()

// Assert
Assert.Equal(packages.Count(), 6);
AssertPackageResultsEqual(packages.ElementAt(0), new { Id = "jQuery", Version = new SemanticVersion("1.44") });
AssertPackageResultsEqual(packages.ElementAt(0), new { Id = "JQuery", Version = new SemanticVersion("1.44") });
AssertPackageResultsEqual(packages.ElementAt(1), new { Id = "jQuery", Version = new SemanticVersion("1.51") });
AssertPackageResultsEqual(packages.ElementAt(2), new { Id = "jQuery", Version = new SemanticVersion("1.52") });
AssertPackageResultsEqual(packages.ElementAt(3),
Expand Down Expand Up @@ -898,7 +898,8 @@ private static IPackageRepository GetRepositoryWithMultiplePackageVersions(strin
var repositoryWithMultiplePackageVersions = new Mock<IPackageRepository>();
var packages = new[]
{
PackageUtility.CreatePackage("jQuery", "1.44"),
// different case is intended to test that id is case-insensitive
PackageUtility.CreatePackage("JQuery", "1.44"),
PackageUtility.CreatePackage("jQuery", "1.51"),
PackageUtility.CreatePackage("jQuery", "1.52"),
PackageUtility.CreatePackage("NHibernate", "1.1"),
Expand Down