From 4b07c89c9e2f9c51e60af053748b77a24c2a714a Mon Sep 17 00:00:00 2001 From: Matthew DeVenny Date: Wed, 1 Jul 2026 14:17:06 -0700 Subject: [PATCH 1/2] #42 validate options Signed-off-by: Matthew DeVenny --- src/NatsDistributedCache/NatsCache.cs | 8 +-- src/NatsDistributedCache/NatsCacheOptions.cs | 4 ++ .../NatsDistributedCacheExtensions.cs | 6 ++- .../NatsDistributedCacheExtensionsTests.cs | 49 +++++++++++++++++++ 4 files changed, 61 insertions(+), 6 deletions(-) diff --git a/src/NatsDistributedCache/NatsCache.cs b/src/NatsDistributedCache/NatsCache.cs index beb86d1..6314daf 100644 --- a/src/NatsDistributedCache/NatsCache.cs +++ b/src/NatsDistributedCache/NatsCache.cs @@ -45,7 +45,7 @@ public NatsCache( var options = optionsAccessor.Value; _bucketName = !string.IsNullOrWhiteSpace(options.BucketName) ? options.BucketName - : throw new NullReferenceException("BucketName must be set"); + : throw new ArgumentException(NatsCacheOptions.BucketNameRequiredMessage, nameof(optionsAccessor)); _keyPrefix = string.IsNullOrEmpty(options.CacheKeyPrefix) ? string.Empty : options.CacheKeyPrefix.TrimEnd('.'); @@ -328,9 +328,9 @@ await kvStore.UpdateWithTtlAsync( } private async Task RemoveAsync( - string key, - NatsKVDeleteOpts? natsKvDeleteOpts = null, - CancellationToken token = default) + string key, + NatsKVDeleteOpts? natsKvDeleteOpts = null, + CancellationToken token = default) { var kvStore = await GetKvStore().ConfigureAwait(false); await kvStore.DeleteAsync(GetEncodedKey(key), natsKvDeleteOpts, cancellationToken: token) diff --git a/src/NatsDistributedCache/NatsCacheOptions.cs b/src/NatsDistributedCache/NatsCacheOptions.cs index 0f608ad..1fd79f6 100644 --- a/src/NatsDistributedCache/NatsCacheOptions.cs +++ b/src/NatsDistributedCache/NatsCacheOptions.cs @@ -7,6 +7,10 @@ namespace CodeCargo.Nats.DistributedCache /// public class NatsCacheOptions : IOptions { + // Shared by the startup options validator (AddNatsDistributedCache) and the NatsCache + // constructor guard so both validation paths report an identical message. + internal const string BucketNameRequiredMessage = "BucketName must be set"; + /// /// The NATS bucket name to use for the distributed cache. /// diff --git a/src/NatsDistributedCache/NatsDistributedCacheExtensions.cs b/src/NatsDistributedCache/NatsDistributedCacheExtensions.cs index 293d2be..f4196c9 100644 --- a/src/NatsDistributedCache/NatsDistributedCacheExtensions.cs +++ b/src/NatsDistributedCache/NatsDistributedCacheExtensions.cs @@ -25,8 +25,10 @@ public static IServiceCollection AddNatsDistributedCache( Action configureOptions, object? connectionServiceKey = null) { - services.AddOptions(); - services.Configure(configureOptions); + services.AddOptions() + .Configure(configureOptions) + .Validate(o => !string.IsNullOrWhiteSpace(o.BucketName), NatsCacheOptions.BucketNameRequiredMessage) + .ValidateOnStart(); services.AddSingleton(sp => { var optionsAccessor = sp.GetRequiredService>(); diff --git a/test/UnitTests/Extensions/NatsDistributedCacheExtensionsTests.cs b/test/UnitTests/Extensions/NatsDistributedCacheExtensionsTests.cs index abcb730..86187e3 100644 --- a/test/UnitTests/Extensions/NatsDistributedCacheExtensionsTests.cs +++ b/test/UnitTests/Extensions/NatsDistributedCacheExtensionsTests.cs @@ -186,6 +186,55 @@ public void AddNatsCache_ReturnsServiceCollection() Assert.Same(services, result); } + [Theory] + [InlineData(null)] + [InlineData("")] + [InlineData(" ")] + public void AddNatsCache_MissingBucketName_FailsValidationOnStart(string? bucketName) + { + // Arrange + var services = new ServiceCollection(); + services.AddSingleton(_mockNatsConnection.Object); + services.AddNatsDistributedCache(options => options.BucketName = bucketName); + var validator = services.BuildServiceProvider().GetRequiredService(); + + // Act - ValidateOnStart runs this at host startup; invoke it directly to prove it fails fast + var exception = Assert.Throws(() => validator.Validate()); + + // Assert - descriptive failure, not a NullReferenceException + Assert.Contains("BucketName must be set", exception.Message); + } + + [Fact] + public void AddNatsCache_ValidBucketName_PassesValidationOnStart() + { + // Arrange + var services = new ServiceCollection(); + services.AddSingleton(_mockNatsConnection.Object); + services.AddNatsDistributedCache(options => options.BucketName = "cache"); + var validator = services.BuildServiceProvider().GetRequiredService(); + + // Act + Assert - a valid bucket name passes startup validation without throwing + validator.Validate(); + } + + [Theory] + [InlineData(null)] + [InlineData("")] + [InlineData(" ")] + public void NatsCache_DirectConstruction_MissingBucketName_ThrowsArgumentException(string? bucketName) + { + // Arrange - direct construction bypasses the DI options validation pipeline + var options = Options.Create(new NatsCacheOptions { BucketName = bucketName }); + + // Act - the constructor guard fails fast with a clear ArgumentException, not a NullReferenceException + var exception = Assert.Throws( + () => new NatsCache(options, _mockNatsConnection.Object)); + + // Assert + Assert.Contains("BucketName must be set", exception.Message); + } + [Fact] public void ToHybridCacheSerializerFactory_CreatesWorkingFactory() { From 332abbcab22d82af5caf022730a15cab76bc042e Mon Sep 17 00:00:00 2001 From: Matthew DeVenny Date: Wed, 1 Jul 2026 14:21:42 -0700 Subject: [PATCH 2/2] Point BucketName ArgumentException paramName at the config value Use nameof(NatsCacheOptions.BucketName) instead of nameof(optionsAccessor) so the exception reads "(Parameter 'BucketName')" and points at the actual misconfigured value rather than the accessor argument. Pin the paramName in the direct-construction test. Co-Authored-By: Claude Opus 4.8 (1M context) Signed-off-by: Matthew DeVenny --- src/NatsDistributedCache/NatsCache.cs | 2 +- .../Extensions/NatsDistributedCacheExtensionsTests.cs | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/NatsDistributedCache/NatsCache.cs b/src/NatsDistributedCache/NatsCache.cs index 6314daf..1ac9a60 100644 --- a/src/NatsDistributedCache/NatsCache.cs +++ b/src/NatsDistributedCache/NatsCache.cs @@ -45,7 +45,7 @@ public NatsCache( var options = optionsAccessor.Value; _bucketName = !string.IsNullOrWhiteSpace(options.BucketName) ? options.BucketName - : throw new ArgumentException(NatsCacheOptions.BucketNameRequiredMessage, nameof(optionsAccessor)); + : throw new ArgumentException(NatsCacheOptions.BucketNameRequiredMessage, nameof(NatsCacheOptions.BucketName)); _keyPrefix = string.IsNullOrEmpty(options.CacheKeyPrefix) ? string.Empty : options.CacheKeyPrefix.TrimEnd('.'); diff --git a/test/UnitTests/Extensions/NatsDistributedCacheExtensionsTests.cs b/test/UnitTests/Extensions/NatsDistributedCacheExtensionsTests.cs index 86187e3..2af5542 100644 --- a/test/UnitTests/Extensions/NatsDistributedCacheExtensionsTests.cs +++ b/test/UnitTests/Extensions/NatsDistributedCacheExtensionsTests.cs @@ -231,8 +231,9 @@ public void NatsCache_DirectConstruction_MissingBucketName_ThrowsArgumentExcepti var exception = Assert.Throws( () => new NatsCache(options, _mockNatsConnection.Object)); - // Assert + // Assert - message and paramName point at the offending config value, not the accessor Assert.Contains("BucketName must be set", exception.Message); + Assert.Equal(nameof(NatsCacheOptions.BucketName), exception.ParamName); } [Fact]