Skip to content
Draft
45 changes: 37 additions & 8 deletions src/NuGetGallery/ViewModels/DependencySetsViewModel.cs
Original file line number Diff line number Diff line change
Expand Up @@ -24,21 +24,50 @@ public DependencySetsViewModel(IEnumerable<PackageDependency> packageDependencie

OnlyHasAllFrameworks = dependencySets.Count() == 1 && dependencySets.First().Key == null;

// Create a list to hold TFM string, parsed framework, and dependencies for proper sorting
var frameworkGroups = new List<(string tfmString, NuGetFramework framework, string friendlyName, IEnumerable<DependencyViewModel> dependencies)>();

foreach (var dependencySet in dependencySets)
{
var targetFramework = dependencySet.Key == null
? "All Frameworks"
: NuGetFramework.Parse(dependencySet.Key).ToFriendlyName();
string tfmString = dependencySet.Key;
string friendlyName;
NuGetFramework framework;

if (!DependencySets.ContainsKey(targetFramework))
if (tfmString == null)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

string.IsNullOrEmpty(tfmString )

{
friendlyName = "All Frameworks";
framework = null;
}
else
{
DependencySets.Add(targetFramework,
dependencySet.OrderBy(x => x.Id).Select(d => d.Id == null ? null : new DependencyViewModel(d.Id, d.VersionSpec)));
framework = NuGetFramework.Parse(tfmString);
friendlyName = framework.ToFriendlyName();
}

var dependencies = dependencySet.OrderBy(x => x.Id).Select(d => d.Id == null ? null : new DependencyViewModel(d.Id, d.VersionSpec));
Comment thread
joelverhagen marked this conversation as resolved.
Outdated
frameworkGroups.Add((tfmString, framework, friendlyName, dependencies));
}

// Order the top level frameworks by their resulting friendly name
DependencySets = DependencySets.OrderBy(x => x.Key).ToDictionary(x => x.Key, x => x.Value);
// Sort by framework using NuGetFrameworkSorter, with null frameworks (All Frameworks) first
var sortedGroups = frameworkGroups.OrderBy(g => g.framework,
Comparer<NuGetFramework>.Create((a, b) =>
{
// Put "All Frameworks" (null) first
if (a == null && b == null) return 0;
if (a == null) return -1;
if (b == null) return 1;
// Use NuGetFrameworkSorter for actual frameworks
return NuGetFrameworkSorter.Instance.Compare(a, b);
Comment thread
joelverhagen marked this conversation as resolved.
Outdated
}));

// Build the final dictionary with friendly names
foreach (var group in sortedGroups)
{
if (!DependencySets.ContainsKey(group.friendlyName))
{
DependencySets.Add(group.friendlyName, group.dependencies);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm very confused how this works. Why does DependencySets.Keys return the keys ordered how you expect them to?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think you need to address (at)copilot to get a response.
I missed this, I had assumed this was a collection that supported the order as we'd expect. I think this is working because of a Dictionary implementation detail -- not good.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@copilot - can you check this comment thread please?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

You're correct that relying on Dictionary insertion order was problematic, especially for .NET Framework 4.7.2 where it's not guaranteed. Changed DependencySets from IDictionary<string, IEnumerable<DependencyViewModel>> to IReadOnlyList<KeyValuePair<string, IEnumerable<DependencyViewModel>>> to explicitly preserve the sorted order. The sorted frameworks are now stored in a List which guarantees insertion order, making the ordering contract explicit. (7a1b3c0)

}
}
}
catch (Exception e)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,29 @@ public void GivenAListOfDependenciesPackageIdsWillBeOrdered()
Assert.Equal("cde", dependencyViewModels[2].Id);
Assert.Equal("def", dependencyViewModels[3].Id);
}

[Fact]
public void GivenAListOfDependenciesNet10WillBeOrderedAfterNet9()
{
// Arrange
var dependencies = new[] {
Comment thread
joelverhagen marked this conversation as resolved.
Outdated
new PackageDependency { TargetFramework = "net10.0" },
new PackageDependency { TargetFramework = "net8.0" },
new PackageDependency { TargetFramework = "net9.0" }
};

// Act
var viewModel = new DependencySetsViewModel(dependencies);

// Assert
Assert.Equal(3, viewModel.DependencySets.Count);

var dependencySetsList = viewModel.DependencySets.Keys.ToList();

Assert.Equal(".NET 8.0", dependencySetsList[0]);
Assert.Equal(".NET 9.0", dependencySetsList[1]);
Assert.Equal(".NET 10.0", dependencySetsList[2]);
}
}
}
}