Skip to content

Commit

Permalink
Small refactor of PackageSourceProvider (#5516)
Browse files Browse the repository at this point in the history
  • Loading branch information
@zivkan
zivkan authored Dec 6, 2023
1 parent a59e645 commit 1942c17
Show file tree
Hide file tree
Showing 4 changed files with 96 additions and 68 deletions.
2 changes: 0 additions & 2 deletions src/NuGet.Core/NuGet.Configuration/GlobalSuppressions.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,8 @@

using System.Diagnostics.CodeAnalysis;

[assembly: SuppressMessage("Build", "CA1062:In externally visible method 'PackageSourceProvider.PackageSourceProvider(ISettings settings, IEnumerable<PackageSource> configurationDefaultSources, bool enablePackageSourcesChangedEvent)', validate parameter 'configurationDefaultSources' is non-null before using it. If appropriate, throw an ArgumentNullException when the argument is null or add a Code Contract precondition asserting non-null argument.", Justification = "<Pending>", Scope = "member", Target = "~M:NuGet.Configuration.PackageSourceProvider.#ctor(NuGet.Configuration.ISettings,System.Collections.Generic.IEnumerable{NuGet.Configuration.PackageSource},System.Boolean)")]
[assembly: SuppressMessage("Build", "CA1031:Modify 'AddDisabledSource' to catch a more specific allowed exception type, or rethrow the exception.", Justification = "<Pending>", Scope = "member", Target = "~M:NuGet.Configuration.PackageSourceProvider.AddDisabledSource(System.String,System.Boolean,System.Boolean@)")]
[assembly: SuppressMessage("Build", "CA1031:Modify 'RemovePackageSource' to catch a more specific allowed exception type, or rethrow the exception.", Justification = "<Pending>", Scope = "member", Target = "~M:NuGet.Configuration.PackageSourceProvider.RemovePackageSource(System.String,System.Boolean,System.Boolean@)")]
[assembly: SuppressMessage("Build", "CA1062:In externally visible method 'void PackageSourceProvider.SaveActivePackageSource(PackageSource source)', validate parameter 'source' is non-null before using it. If appropriate, throw an ArgumentNullException when the argument is null or add a Code Contract precondition asserting non-null argument.", Justification = "<Pending>", Scope = "member", Target = "~M:NuGet.Configuration.PackageSourceProvider.SaveActivePackageSource(NuGet.Configuration.PackageSource)")]
[assembly: SuppressMessage("Build", "CA1031:Modify 'SaveActivePackageSource' to catch a more specific allowed exception type, or rethrow the exception.", Justification = "<Pending>", Scope = "member", Target = "~M:NuGet.Configuration.PackageSourceProvider.SaveActivePackageSource(NuGet.Configuration.PackageSource)")]
[assembly: SuppressMessage("Build", "CA1062:In externally visible method 'IWebProxy ProxyCache.GetProxy(Uri sourceUri)', validate parameter 'sourceUri' is non-null before using it. If appropriate, throw an ArgumentNullException when the argument is null or add a Code Contract precondition asserting non-null argument.", Justification = "<Pending>", Scope = "member", Target = "~M:NuGet.Configuration.ProxyCache.GetProxy(System.Uri)~System.Net.IWebProxy")]
[assembly: SuppressMessage("Build", "CA1062:In externally visible method 'ISettings Settings.LoadImmutableSettingsGivenConfigPaths(IList<string> configFilePaths, SettingsLoadingContext settingsLoadingContext)', validate parameter 'settingsLoadingContext' is non-null before using it. If appropriate, throw an ArgumentNullException when the argument is null or add a Code Contract precondition asserting non-null argument.", Justification = "<Pending>", Scope = "member", Target = "~M:NuGet.Configuration.Settings.LoadImmutableSettingsGivenConfigPaths(System.Collections.Generic.IList{System.String},NuGet.Configuration.SettingsLoadingContext)~NuGet.Configuration.ISettings")]
Expand Down
1 change: 1 addition & 0 deletions src/NuGet.Core/NuGet.Configuration/PackageSource/IPackageSourceProvider.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ public interface IPackageSourceProvider
/// <param name="source">Url of source to be searched for</param>
/// <returns>PackageSource that matches the given source. Null if none was found</returns>
/// <throws>ArgumentException when <paramref name="source"/> is null or empty.</throws>
/// <remarks>There may be multiple sources that match a given url. This method will return the first.</remarks>
PackageSource GetPackageSourceBySource(string source);

/// <summary>
Expand Down
133 changes: 67 additions & 66 deletions src/NuGet.Core/NuGet.Configuration/PackageSource/PackageSourceProvider.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
using System.Globalization;
using System.Linq;
using System.Text.RegularExpressions;
using NuGet.Common;

namespace NuGet.Configuration
{
Expand All @@ -15,7 +14,7 @@ public class PackageSourceProvider : IPackageSourceProvider
public ISettings Settings { get; private set; }

internal const int MaxSupportedProtocolVersion = 3;
private readonly IEnumerable<PackageSource> _configurationDefaultSources;
private readonly IReadOnlyList<PackageSource> _configurationDefaultSources;

public PackageSourceProvider(
ISettings settings)
Expand Down Expand Up @@ -53,48 +52,57 @@ public PackageSourceProvider(
{
Settings.SettingsChanged += (_, __) => { OnPackageSourcesChanged(); };
}
if (configurationDefaultSources is null)
{
throw new ArgumentNullException(nameof(configurationDefaultSources));
}
_configurationDefaultSources = LoadConfigurationDefaultSources(configurationDefaultSources);
}

private static IEnumerable<PackageSource> LoadConfigurationDefaultSources(IEnumerable<PackageSource> configurationDefaultSources)
private static IReadOnlyList<PackageSource> LoadConfigurationDefaultSources(IEnumerable<PackageSource> configurationDefaultSources)
{
#if !IS_CORECLR
// Global default NuGet source doesn't make sense on Mono
if (RuntimeEnvironmentHelper.IsMono)
if (Common.RuntimeEnvironmentHelper.IsMono)
{
return Enumerable.Empty<PackageSource>();
return Array.Empty<PackageSource>();
}
#endif
var packageSourceLookup = new Dictionary<string, IndexedPackageSource>(StringComparer.OrdinalIgnoreCase);
var packageIndex = 0;

foreach (var packageSource in configurationDefaultSources)
{
packageIndex = AddOrUpdateIndexedSource(packageSourceLookup, packageIndex, packageSource, packageSource.Name);
AddOrUpdateIndexedSource(packageSourceLookup, packageSource, ref packageIndex);
}

return packageSourceLookup.Values
List<PackageSource> defaultSources = new(packageSourceLookup.Count);
defaultSources.AddRange(packageSourceLookup.Values
.OrderBy(source => source.Index)
.Select(source => source.PackageSource);
.Select(source => source.PackageSource));

return defaultSources.AsReadOnly();
}

private static Dictionary<string, IndexedPackageSource> LoadPackageSourceLookup(bool byName, ISettings settings)
private static List<PackageSource> GetPackageSourceFromSettings(ISettings settings)
{
var packageSourcesSection = settings.GetSection(ConfigurationConstants.PackageSources);
var sourcesItems = packageSourcesSection?.Items.OfType<SourceItem>();

// Order the list so that the closer to the user appear first
IList<string> configFilePaths = settings.GetConfigFilePaths();
var sources = sourcesItems?.OrderBy(i => configFilePaths.IndexOf(i.Origin?.ConfigFilePath)); //lower index => higher priority => closer to user.
// get list of disabled packages
var disabledSourcesSection = settings.GetSection(ConfigurationConstants.DisabledPackageSources);
var disabledSourcesSettings = disabledSourcesSection?.Items.OfType<AddItem>();

var disabledSources = new HashSet<string>(disabledSourcesSettings?.GroupBy(setting => setting.Key).Select(group => group.First().Key) ?? Enumerable.Empty<string>());
var packageSourceLookup = new Dictionary<string, IndexedPackageSource>(StringComparer.OrdinalIgnoreCase);
List<PackageSource> packageSources;

if (sources != null)
{
// get list of disabled packages
var disabledSourcesSection = settings.GetSection(ConfigurationConstants.DisabledPackageSources);
var disabledSourcesSettings = disabledSourcesSection?.Items.OfType<AddItem>();
var disabledSources = new HashSet<string>(disabledSourcesSettings?.GroupBy(setting => setting.Key).Select(group => group.First().Key) ?? Enumerable.Empty<string>());

var packageSourceLookup = new Dictionary<string, IndexedPackageSource>(StringComparer.OrdinalIgnoreCase);
var packageIndex = 0;

foreach (var setting in sources)
Expand All @@ -103,20 +111,20 @@ private static Dictionary<string, IndexedPackageSource> LoadPackageSourceLookup(
var isEnabled = !disabledSources.Contains(name);
var packageSource = ReadPackageSource(setting, isEnabled, settings);

packageIndex = AddOrUpdateIndexedSource(packageSourceLookup, packageIndex, packageSource, byName ? packageSource.Name : packageSource.Source);
AddOrUpdateIndexedSource(packageSourceLookup, packageSource, ref packageIndex);
}
}
return packageSourceLookup;
}

private static Dictionary<string, IndexedPackageSource> LoadPackageSourceLookupByName(ISettings settings)
{
return LoadPackageSourceLookup(byName: true, settings);
}
packageSources = new(capacity: packageSourceLookup.Count);
packageSources.AddRange(packageSourceLookup.Values
.OrderBy(psi => psi.Index).
Select(psi => psi.PackageSource));
}
else
{
packageSources = new List<PackageSource>();
}

private Dictionary<string, IndexedPackageSource> LoadPackageSourceLookupBySource()
{
return LoadPackageSourceLookup(byName: false, Settings);
return packageSources;
}

/// <summary>
Expand All @@ -138,10 +146,7 @@ public static IEnumerable<PackageSource> LoadPackageSources(ISettings settings)

private static List<PackageSource> LoadPackageSources(ISettings settings, IEnumerable<PackageSource> defaultPackageSources)
{
var loadedPackageSources = LoadPackageSourceLookupByName(settings).Values
.OrderBy(source => source.Index)
.Select(source => source.PackageSource)
.ToList();
List<PackageSource> loadedPackageSources = GetPackageSourceFromSettings(settings);

if (defaultPackageSources != null && defaultPackageSources.Any())
{
Expand All @@ -151,11 +156,14 @@ private static List<PackageSource> LoadPackageSources(ISettings settings, IEnume
return loadedPackageSources;
}

// This adds package sources defined in the machine-wide NuGetDefaults.config
// which as per our docs specifies, always get added, even if a repo nuget.config
// uses a <clear />
private static void AddDefaultPackageSources(List<PackageSource> loadedPackageSources, IEnumerable<PackageSource> defaultPackageSources)
{
var defaultPackageSourcesToBeAdded = new List<PackageSource>();

foreach (var packageSource in defaultPackageSources)
foreach (var packageSource in defaultPackageSources.NoAllocEnumerate())
{
var sourceMatching = loadedPackageSources.Any(p => p.Source.Equals(packageSource.Source, StringComparison.OrdinalIgnoreCase));
var feedNameMatching = loadedPackageSources.Any(p => p.Name.Equals(packageSource.Name, StringComparison.OrdinalIgnoreCase));
Expand Down Expand Up @@ -224,15 +232,14 @@ private static bool ReadAllowInsecureConnections(SourceItem setting)
return PackageSource.DefaultAllowInsecureConnections;
}

private static int AddOrUpdateIndexedSource(
private static void AddOrUpdateIndexedSource(
Dictionary<string, IndexedPackageSource> packageSourceLookup,
int packageIndex,
PackageSource packageSource,
string lookupKey)
ref int packageIndex)
{
if (!packageSourceLookup.TryGetValue(lookupKey, out var previouslyAddedSource))
if (!packageSourceLookup.TryGetValue(packageSource.Name, out var previouslyAddedSource))
{
packageSourceLookup[lookupKey] = new IndexedPackageSource
packageSourceLookup[packageSource.Name] = new IndexedPackageSource
{
PackageSource = packageSource,
Index = packageIndex++
Expand All @@ -245,8 +252,6 @@ private static int AddOrUpdateIndexedSource(
// Pick the package source with the highest supported protocol version
previouslyAddedSource.PackageSource = packageSource;
}

return packageIndex;
}

private static PackageSourceCredential ReadCredential(string sourceName, ISettings settings)
Expand Down Expand Up @@ -296,50 +301,31 @@ private static PackageSourceCredential ReadCredentialFromEnvironment(string sour
validAuthenticationTypesText: match.Groups["authTypes"].Value);
}

private PackageSource GetPackageSource(string key, Dictionary<string, IndexedPackageSource> sourcesLookup)
public PackageSource GetPackageSourceByName(string name)
{
if (sourcesLookup.TryGetValue(key, out var indexedPackageSource))
if (string.IsNullOrEmpty(name))
{
return indexedPackageSource.PackageSource;
throw new ArgumentException(Resources.Argument_Cannot_Be_Null_Or_Empty, nameof(name));
}

if (_configurationDefaultSources != null && _configurationDefaultSources.Any())
{
var loadedPackageSources = sourcesLookup.Values
.OrderBy(source => source.Index)
.Select(source => source.PackageSource)
.ToList();
List<PackageSource> packageSources = LoadPackageSources(Settings, _configurationDefaultSources);

foreach (var packageSource in _configurationDefaultSources)
foreach (var packageSource in packageSources)
{
if (string.Equals(name, packageSource.Name, StringComparison.OrdinalIgnoreCase))
{
var isSourceMatch = loadedPackageSources.Any(p => p.Source.Equals(packageSource.Source, StringComparison.OrdinalIgnoreCase));
var isFeedNameMatch = loadedPackageSources.Any(p => p.Name.Equals(packageSource.Name, StringComparison.OrdinalIgnoreCase));

if (isSourceMatch || isFeedNameMatch)
{
return packageSource;
}
return packageSource;
}
}

return null;
}

public PackageSource GetPackageSourceByName(string name)
{
if (string.IsNullOrEmpty(name))
{
throw new ArgumentException(Resources.Argument_Cannot_Be_Null_Or_Empty, nameof(name));
}

return GetPackageSource(name, LoadPackageSourceLookupByName(Settings));
}

public HashSet<string> GetPackageSourceNamesMatchingNamePrefix(string namePrefix)
{
var names = new HashSet<string>();

IEnumerable<PackageSource> packageSources = LoadPackageSources();
List<PackageSource> packageSources = LoadPackageSources(Settings, _configurationDefaultSources);
foreach (PackageSource packageSource in packageSources)
{
if (packageSource.Name.StartsWith(namePrefix, StringComparison.OrdinalIgnoreCase))
Expand All @@ -358,7 +344,17 @@ public PackageSource GetPackageSourceBySource(string source)
throw new ArgumentException(Resources.Argument_Cannot_Be_Null_Or_Empty, nameof(source));
}

return GetPackageSource(source, LoadPackageSourceLookupBySource());
List<PackageSource> packageSources = LoadPackageSources(Settings, _configurationDefaultSources);

foreach (var packageSource in packageSources)
{
if (string.Equals(source, packageSource.Source, StringComparison.OrdinalIgnoreCase))
{
return packageSource;
}
}

return null;
}

public void RemovePackageSource(string name)
Expand Down Expand Up @@ -859,6 +855,11 @@ public string ActivePackageSourceName
/// <param name="source"></param>
public void SaveActivePackageSource(PackageSource source)
{
if (source is null)
{
throw new ArgumentNullException(nameof(source));
}

try
{
var activePackageSourceSection = Settings.GetSection(ConfigurationConstants.ActivePackageSourceSectionName);
Expand Down
28 changes: 28 additions & 0 deletions test/NuGet.Core.Tests/NuGet.Configuration.Test/PackageSourceProviderTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2271,6 +2271,34 @@ public void PackageSourcesChanged_EventRunsSubscriptions(bool subscribeToEvent)
Assert.Equal(subscribeToEvent, eventRun);
}

[Fact]
public void GetPackageSourceBySource_TwoSourcesWithSameUrl_ReturnsFirstSource()
{
// Arrange
using TestDirectory testDirectory = TestDirectory.Create();

const string sourceUrl = "https://contoso.test/nuget/index.json";
const string contents = $@"<configuration>
<packageSources>
<add key=""s1"" value=""{sourceUrl}"" />
<add key=""s2"" value=""{sourceUrl}"" />
</packageSources>
</configuration>
";
var path = Path.Combine(testDirectory.Path, Settings.DefaultSettingsFileName);
File.WriteAllText(path, contents);

Settings settings = new Settings(testDirectory.Path);
var machineDefaultSources = Array.Empty<PackageSource>();

// Act
PackageSourceProvider psp = new PackageSourceProvider(settings, machineDefaultSources);
PackageSource source = psp.GetPackageSourceBySource(sourceUrl);

// Assert
source.Name.Should().Be("s1");
}

private string CreateNuGetConfigContent(string enabledReplacement = "", string disabledReplacement = "", string activeSourceReplacement = "")
{
var nugetConfigBaseString = new StringBuilder();
Expand Down

0 comments on commit 1942c17

Please sign in to comment.