Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
27 changes: 17 additions & 10 deletions docs/design/nuget-caching/nuget-cache.md
Original file line number Diff line number Diff line change
Expand Up @@ -47,10 +47,16 @@ library cooperative in concurrent or UI-driven environments.
#### NuGet Settings Integration

Rather than accepting source URIs directly, `NuGetCache` reads from the default
NuGet settings on the local machine via `Settings.LoadDefaultSettings(null)`. This
ensures the library respects the same `nuget.config` hierarchy (machine-wide, user,
and project-level) as the `dotnet` CLI and Visual Studio, including authenticated
feeds, proxy settings, and package source mapping.
NuGet settings via `Settings.LoadDefaultSettings(root)`, where `root` is a caller-supplied
directory (typically the directory containing the caller's project or `packages.config`
file) defaulting to the current working directory when omitted. Rooting the lookup this
way - rather than passing a literal `null` root - ensures the library respects the same
`nuget.config` hierarchy (machine-wide, user, and project-level, discovered by walking up
from `root`) as the `dotnet` CLI and Visual Studio, including authenticated feeds, proxy
settings, and package source mapping. A literal `null` root skips the walk-up entirely and
silently loses any repo-local package sources - this was the root cause of GitHub issue
#37, where `nuget-installer` could not see sources defined only in a repo-local
`nuget.config`.

#### Early-Exit on Cache Hit

Expand Down Expand Up @@ -127,7 +133,7 @@ misconfiguration, without requiring additional logging infrastructure.
#### Testability via Injected Settings

The public `EnsureCachedAsync` method is a thin wrapper that calls the internal overload
with `Settings.LoadDefaultSettings(null)`. The internal overload accepts an `ISettings`
with `Settings.LoadDefaultSettings(root)`. The internal overload accepts an `ISettings`
parameter that replaces the call to `LoadDefaultSettings`. This design gives tests full
control over the NuGet source and global packages folder without touching the developer's
real machine configuration:
Expand All @@ -149,13 +155,14 @@ without depending on shared, process-wide static state.

### Method Descriptions

#### `EnsureCachedAsync(string packageId, string version, CancellationToken)` (public)
#### `EnsureCachedAsync(string packageId, string version, string? root, CancellationToken)` (public)

Thin wrapper that delegates immediately to the internal `EnsureCachedAsync` overload,
passing `Settings.LoadDefaultSettings(null)` as the `settings` argument. This ensures
the public API continues to behave identically to the pre-testability implementation
while the full logic lives in one place. See the internal overload description below
for the complete processing steps.
passing `Settings.LoadDefaultSettings(root ?? Directory.GetCurrentDirectory())` as the
`settings` argument. Callers should pass the directory containing their project or
`packages.config` file as `root` so that a repo-local `nuget.config` is discovered the
same way `dotnet restore` discovers it; omitting `root` falls back to the current working
directory. See the internal overload description below for the complete processing steps.

Returns the absolute path to the cached package folder.

Expand Down
34 changes: 25 additions & 9 deletions src/DemaConsulting.NuGet.Caching/NuGetCache.cs
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,9 @@ namespace DemaConsulting.NuGet.Caching;
/// </summary>
/// <remarks>
/// This class reads NuGet configuration (sources and global packages folder) from
/// the default NuGet settings on the local machine, mirroring the behavior of
/// the <c>dotnet</c> CLI and Visual Studio package restore.
/// the default NuGet settings, rooted at a caller-supplied (or current working) directory,
/// mirroring the behavior of the <c>dotnet</c> CLI and Visual Studio package restore -
/// including discovery of a project/repo-local <c>nuget.config</c>.
/// This class is stateless — all state is local to each <c>EnsureCachedAsync</c> call — and is
/// safe for concurrent use.
/// </remarks>
Expand All @@ -43,13 +44,25 @@ public static class NuGetCache
/// Ensures a specific NuGet package version is available in the local global packages cache.
/// </summary>
/// <remarks>
/// Reads NuGet configuration from the default machine settings via
/// <c>Settings.LoadDefaultSettings</c>, mirroring the behavior of the <c>dotnet</c>
/// CLI and Visual Studio package restore. All caching logic is delegated to the internal
/// overload that accepts an explicit <see cref="ISettings"/> instance.
/// Reads NuGet configuration from the default settings via <c>Settings.LoadDefaultSettings</c>,
/// rooted at <paramref name="root"/> (or the current working directory when
/// <paramref name="root"/> is <see langword="null"/>). Rooting the settings lookup this way
/// mirrors the behavior of the <c>dotnet</c> CLI and Visual Studio package restore, which
/// both discover a project/repo-local <c>nuget.config</c> by walking up from an ambient
/// working directory - passing a literal <see langword="null"/> root (as earlier versions of
/// this method did) skips that walk entirely and silently loses any repo-local package
/// sources. All caching logic is delegated to the internal overload that accepts an explicit
/// <see cref="ISettings"/> instance.
/// </remarks>
/// <param name="packageId">The NuGet package identifier (e.g. <c>Newtonsoft.Json</c>).</param>
/// <param name="version">The exact version string (e.g. <c>13.0.3</c>).</param>
/// <param name="root">
/// The directory from which to begin discovering <c>nuget.config</c> files (walking up
/// through ancestor directories, then falling back to machine/user-wide settings), matching
/// the <c>dotnet</c> CLI's behavior. Typically the directory containing the caller's project
/// or <c>packages.config</c> file. When <see langword="null"/>, defaults to
/// <see cref="Directory.GetCurrentDirectory"/>.
/// </param>
/// <param name="cancellationToken">Optional cancellation token for the async operation.</param>
/// <returns>
/// The absolute path to the cached package folder inside the global packages folder.
Expand All @@ -66,11 +79,14 @@ public static class NuGetCache
public static async Task<string> EnsureCachedAsync(
string packageId,
string version,
string? root = null,
CancellationToken cancellationToken = default)
{
// Delegate to the overload that accepts an explicit ISettings instance, using the
// default machine / user NuGet configuration as the settings source
return await EnsureCachedAsync(packageId, version, Settings.LoadDefaultSettings(null), cancellationToken);
// Delegate to the overload that accepts an explicit ISettings instance, rooting the
// default settings lookup at the provided directory (or the current working directory)
// so that a project/repo-local nuget.config is discovered the same way `dotnet restore` does
return await EnsureCachedAsync(
packageId, version, Settings.LoadDefaultSettings(root ?? Directory.GetCurrentDirectory()), cancellationToken);
}
Comment thread
Malcolmnixon marked this conversation as resolved.
Comment thread
Malcolmnixon marked this conversation as resolved.

/// <summary>
Expand Down
80 changes: 80 additions & 0 deletions test/DemaConsulting.NuGet.Caching.Tests/NuGetCacheServerTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -849,6 +849,86 @@ public async Task NuGetCache_EnsureCachedAsync_DefaultRegistrar_RegistersRealCre
}
}

/// <summary>
/// Tests that the public <c>NuGetCache.EnsureCachedAsync(packageId, version, root, ...)</c>
/// overload discovers a project/repo-local <c>nuget.config</c> located at its <c>root</c>
/// argument, rather than only reading machine/user-wide settings.
/// </summary>
/// <remarks>
/// This is a regression test for the bug reported in GitHub issue #37: the public overload
/// previously called <c>Settings.LoadDefaultSettings(null)</c>, which never scans the
/// working directory (or any ancestor) for a repo-local <c>nuget.config</c> - identical
/// package sources placed in such a file were silently invisible. Here the WireMock source
/// is defined *only* in a real <c>nuget.config</c> file written to an isolated temp
/// directory that is not an ancestor of the process's actual working directory, and is
/// passed in via <c>root</c>: had the settings load ignored <c>root</c> (as before the fix),
/// no source would resolve the package and the call would throw
/// <see cref="InvalidOperationException"/> instead of succeeding.
/// </remarks>
[Fact]
[Trait("Category", "LocalIntegration")]
public async Task NuGetCache_EnsureCachedAsync_RootPointsToDirectoryWithRepoLocalNugetConfig_UsesThatConfigSources()
{
// Arrange - isolated global packages folder and a separate "repo root" directory that
// holds only a project-local nuget.config (simulating a repo checkout)
var globalPackagesFolder = Path.Combine(Path.GetTempPath(), Guid.NewGuid().ToString("N"));
var repoRoot = Path.Combine(Path.GetTempPath(), Guid.NewGuid().ToString("N"));
try
{
Directory.CreateDirectory(globalPackagesFolder);
Directory.CreateDirectory(repoRoot);

const string packageId = "TestPackage.RepoLocalConfig";
const string version = "1.0.0";

await using var server = new NuGetTestServer();
var nupkgBytes = NuGetPackageBuilder.CreateMinimalPackage(packageId, version);
server.RegisterV3Package(packageId, version, nupkgBytes);

// Write a real nuget.config directly into the repo root, mirroring how a repository
// scopes its own package sources independently of machine/user-wide settings
await File.WriteAllTextAsync(
Path.Combine(repoRoot, "nuget.config"),
$"""
Comment thread
Malcolmnixon marked this conversation as resolved.
<?xml version="1.0" encoding="utf-8"?>
<configuration>
<config>
<add key="globalPackagesFolder" value="{globalPackagesFolder}" />
</config>
<packageSources>
<clear />
<add key="repo-local-source" value="{server.IndexUrl}" />
Comment thread
Malcolmnixon marked this conversation as resolved.
</packageSources>
</configuration>
""",
TestContext.Current.CancellationToken);
Comment thread
Malcolmnixon marked this conversation as resolved.

// Act - call the public overload, passing the repo root explicitly so the
// repo-local nuget.config is discovered exactly as `dotnet restore` would
var packagePath = await NuGetCache.EnsureCachedAsync(
packageId, version, root: repoRoot, cancellationToken: TestContext.Current.CancellationToken);

// Assert - the package was found and cached via the repo-local source, proving
// the root parameter was honored when loading default settings
Assert.NotNull(packagePath);
Assert.True(
File.Exists(Path.Combine(packagePath, ".nupkg.metadata")),
$"Expected .nupkg.metadata in: {packagePath}");
}
finally
{
if (Directory.Exists(globalPackagesFolder))
{
Directory.Delete(globalPackagesFolder, recursive: true);
}

if (Directory.Exists(repoRoot))
{
Directory.Delete(repoRoot, recursive: true);
}
}
}

/// <summary>
/// A test double implementing <see cref="ICredentialServiceRegistrar"/> that
/// simply counts invocations of <see cref="EnsureRegistered"/>, letting tests assert that
Expand Down
16 changes: 8 additions & 8 deletions test/DemaConsulting.NuGet.Caching.Tests/NuGetCacheTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ public async Task NuGetCache_EnsureCachedAsync_ValidPackageId_ReturnsPackageFold
const string version = "1.5.0";

// Act - ensure the package is present in the local NuGet global packages cache
var packageFolder = await NuGetCache.EnsureCachedAsync(packageId, version, CancellationToken.None);
var packageFolder = await NuGetCache.EnsureCachedAsync(packageId, version, cancellationToken: CancellationToken.None);

// Assert - the returned path must point to a real directory on disk
Assert.NotNull(packageFolder);
Expand Down Expand Up @@ -77,7 +77,7 @@ public async Task NuGetCache_EnsureCachedAsync_PackageAbsentFromAllSources_Throw

// Act & Assert - calling with a non-existent package must throw InvalidOperationException
_ = await Assert.ThrowsAsync<InvalidOperationException>(
async () => await NuGetCache.EnsureCachedAsync(packageId, version, CancellationToken.None));
async () => await NuGetCache.EnsureCachedAsync(packageId, version, cancellationToken: CancellationToken.None));
}

/// <summary>
Expand All @@ -96,7 +96,7 @@ public async Task NuGetCache_EnsureCachedAsync_NullPackageId_ThrowsArgumentNullE

// Act & Assert - calling with null packageId must throw ArgumentNullException
_ = await Assert.ThrowsAsync<ArgumentNullException>(
async () => await NuGetCache.EnsureCachedAsync(null!, version, CancellationToken.None));
async () => await NuGetCache.EnsureCachedAsync(null!, version, cancellationToken: CancellationToken.None));
}

/// <summary>
Expand All @@ -115,7 +115,7 @@ public async Task NuGetCache_EnsureCachedAsync_NullVersion_ThrowsArgumentNullExc

// Act & Assert - calling with null version must throw ArgumentNullException
_ = await Assert.ThrowsAsync<ArgumentNullException>(
async () => await NuGetCache.EnsureCachedAsync(packageId, null!, CancellationToken.None));
async () => await NuGetCache.EnsureCachedAsync(packageId, null!, cancellationToken: CancellationToken.None));
}

/// <summary>
Expand All @@ -131,7 +131,7 @@ public async Task NuGetCache_EnsureCachedAsync_InvalidVersion_ThrowsArgumentExce

// Act & Assert: calling with an invalid version must throw ArgumentException
_ = await Assert.ThrowsAsync<ArgumentException>(
async () => await NuGetCache.EnsureCachedAsync(packageId, version, CancellationToken.None));
async () => await NuGetCache.EnsureCachedAsync(packageId, version, cancellationToken: CancellationToken.None));
}

/// <summary>
Expand All @@ -155,7 +155,7 @@ public async Task NuGetCache_EnsureCachedAsync_PackageAbsentFromAllSources_Excep

// Act - calling with a non-existent package must throw InvalidOperationException
var exception = await Assert.ThrowsAsync<InvalidOperationException>(
async () => await NuGetCache.EnsureCachedAsync(packageId, version, CancellationToken.None));
async () => await NuGetCache.EnsureCachedAsync(packageId, version, cancellationToken: CancellationToken.None));

// Assert - the exception message must identify both the package ID and the version
// so that callers have enough context to diagnose the problem
Expand All @@ -180,8 +180,8 @@ public async Task NuGetCache_EnsureCachedAsync_CalledTwiceWithSamePackage_Return
const string version = "1.5.0";

// Act - call EnsureCachedAsync twice with the same package identity
var firstPath = await NuGetCache.EnsureCachedAsync(packageId, version, CancellationToken.None);
var secondPath = await NuGetCache.EnsureCachedAsync(packageId, version, CancellationToken.None);
var firstPath = await NuGetCache.EnsureCachedAsync(packageId, version, cancellationToken: CancellationToken.None);
var secondPath = await NuGetCache.EnsureCachedAsync(packageId, version, cancellationToken: CancellationToken.None);

// Assert - both calls must return identical paths, proving the method is idempotent
// and does not change the cache location on subsequent calls
Expand Down
8 changes: 4 additions & 4 deletions test/DemaConsulting.NuGet.Caching.Tests/NuGetCachingTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ public async Task NuGetCaching_EnsureCachedAsync_WhenKnownPackageAndVersionProvi
const string version = "1.5.0";

// Act: invoke the library's package caching capability
var packageFolder = await NuGetCache.EnsureCachedAsync(packageId, version, CancellationToken.None);
var packageFolder = await NuGetCache.EnsureCachedAsync(packageId, version, cancellationToken: CancellationToken.None);

// Assert: the library returned a valid path to the cached package on disk
Assert.NotNull(packageFolder);
Expand Down Expand Up @@ -80,7 +80,7 @@ public async Task NuGetCaching_EnsureCachedAsync_WhenPackageNotFound_ThrowsInval

// Act & Assert: the library should throw when the package cannot be found
var ex = await Assert.ThrowsAsync<InvalidOperationException>(
async () => await NuGetCache.EnsureCachedAsync(packageId, version, CancellationToken.None));
async () => await NuGetCache.EnsureCachedAsync(packageId, version, cancellationToken: CancellationToken.None));

// Assert: the exception message must identify the package ID and version
Assert.Contains(packageId, ex.Message);
Expand All @@ -103,7 +103,7 @@ public async Task NuGetCaching_EnsureCachedAsync_WhenPackageIdIsNull_ThrowsArgum

// Act & Assert: the library should throw ArgumentNullException for a null package ID
_ = await Assert.ThrowsAsync<ArgumentNullException>(
async () => await NuGetCache.EnsureCachedAsync(null!, version, CancellationToken.None));
async () => await NuGetCache.EnsureCachedAsync(null!, version, cancellationToken: CancellationToken.None));
}

/// <summary>
Expand All @@ -122,6 +122,6 @@ public async Task NuGetCaching_EnsureCachedAsync_WhenVersionIsNull_ThrowsArgumen

// Act & Assert: the library should throw ArgumentNullException for a null version
_ = await Assert.ThrowsAsync<ArgumentNullException>(
async () => await NuGetCache.EnsureCachedAsync(packageId, null!, CancellationToken.None));
async () => await NuGetCache.EnsureCachedAsync(packageId, null!, cancellationToken: CancellationToken.None));
}
}
Loading