diff --git a/src/AspNet/AspCore.Idempotency.MsSqlStore.Tests/AspCore.Idempotency.MsSqlStore.Tests.csproj b/src/AspNet/AspCore.Idempotency.MsSqlStore.Tests/AspCore.Idempotency.MsSqlStore.Tests.csproj index d99b75ec..6036701c 100644 --- a/src/AspNet/AspCore.Idempotency.MsSqlStore.Tests/AspCore.Idempotency.MsSqlStore.Tests.csproj +++ b/src/AspNet/AspCore.Idempotency.MsSqlStore.Tests/AspCore.Idempotency.MsSqlStore.Tests.csproj @@ -20,6 +20,9 @@ runtime; build; native; contentfiles; analyzers; buildtransitive + + + diff --git a/src/AspNet/AspCore.Idempotency.MsSqlStore.Tests/Integration/IdempotencyIntegrationTests.cs b/src/AspNet/AspCore.Idempotency.MsSqlStore.Tests/Integration/IdempotencyIntegrationTests.cs index 84f861d9..42ddc258 100644 --- a/src/AspNet/AspCore.Idempotency.MsSqlStore.Tests/Integration/IdempotencyIntegrationTests.cs +++ b/src/AspNet/AspCore.Idempotency.MsSqlStore.Tests/Integration/IdempotencyIntegrationTests.cs @@ -55,9 +55,9 @@ public async Task CreateItem_ConcurrentRequestsWithSameKey_OnlyOneProcessed() var idempotencyKey = Guid.NewGuid().ToString(); var request = new CreateItemRequest { Name = "Concurrent Item" }; - // Act - Send 5 concurrent requests with the same idempotency key - // multiple requests may process simultaneously when they all arrive before any is cached. - // This is a known limitation of stateless filters without distributed locking. + // Act - Send 5 concurrent requests with the same idempotency key. + // The atomic reserve-then-check in IsKeyProcessedAsync guarantees exactly one request reaches the + // handler; the other four are turned away as conflicts before they ever call next(context). var tasks = Enumerable.Range(0, 5).Select(_ => { var httpRequest = new HttpRequestMessage(HttpMethod.Post, "/api/items") @@ -70,19 +70,12 @@ public async Task CreateItem_ConcurrentRequestsWithSameKey_OnlyOneProcessed() var responses = await Task.WhenAll(tasks); - // Assert - With concurrent requests, some or all may succeed (201 or 409 Conflict) - // The first request wins, subsequent ones get 409 Conflict (if ConflictHandling = ConflictResponse) - // or the cached response (if ConflictHandling = CachedResult) + // Assert - Exactly one request succeeds, the remaining four are conflicts. var successCount = responses.Count(r => r.StatusCode == HttpStatusCode.Created); var conflictCount = responses.Count(r => r.StatusCode == HttpStatusCode.Conflict); - // At least one should succeed (201) - successCount.ShouldBeGreaterThanOrEqualTo(1); - - // The rest should be conflicts or cached responses - var otherCount = responses.Count(r => r.StatusCode != HttpStatusCode.Created && - r.StatusCode != HttpStatusCode.Conflict); - (successCount + conflictCount + otherCount).ShouldBe(5); + successCount.ShouldBe(1); + conflictCount.ShouldBe(4); // Verify only ONE entry in database (unique constraint ensures this) await using var dbContext = fixture.GetDbContext(); diff --git a/src/AspNet/AspCore.Idempotency.MsSqlStore.Tests/Unit/IdempotencySqlServerStoreConcurrencyTests.cs b/src/AspNet/AspCore.Idempotency.MsSqlStore.Tests/Unit/IdempotencySqlServerStoreConcurrencyTests.cs new file mode 100644 index 00000000..a2deb5b6 --- /dev/null +++ b/src/AspNet/AspCore.Idempotency.MsSqlStore.Tests/Unit/IdempotencySqlServerStoreConcurrencyTests.cs @@ -0,0 +1,121 @@ +// +// Copyright (c) 2025 Steven Hoang. All rights reserved. +// Licensed under the MIT License. See LICENSE in the project root for license information. +// + +using DKNet.AspCore.Idempotency; +using DKNet.AspCore.Idempotency.MsSqlStore.Store; +using Microsoft.EntityFrameworkCore.Infrastructure; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging.Abstractions; +using Microsoft.Extensions.Options; + +namespace AspCore.Idempotency.MsSqlStore.Tests.Unit; + +/// +/// Proves the atomic reservation in under real +/// concurrent connections, without Docker/SQL Server. Points the store at a file-based SQLite database — not +/// the EF Core InMemory provider, which does not enforce UX_CompositeKey across concurrent contexts the +/// same way a real relational database does — using the exact same +/// the SQL Server store ships with. Mirrors the retired HTTP-level +/// CreateItem_ConcurrentRequestsWithSameKey_OnlyOneProcessed test (see DRK-174), but directly at the +/// store layer so it doesn't depend on SQL Server at all. +/// +public sealed class IdempotencySqlServerStoreConcurrencyTests : IAsyncLifetime +{ + #region Fields + + private readonly string _dbFilePath = + Path.Combine(Path.GetTempPath(), $"idempotency-concurrency-{Guid.NewGuid():N}.db"); + + private ServiceProvider _serviceProvider = null!; + private IdempotencySqlServerStore _store = null!; + + #endregion + + #region Methods + + public async Task DisposeAsync() + { + await _store.DisposeAsync(); + await _serviceProvider.DisposeAsync(); + File.Delete(_dbFilePath); + } + + public async Task InitializeAsync() + { + var services = new ServiceCollection(); + services.AddDbContextFactory(o => o.UseSqlite( + $"Data Source={_dbFilePath}", + // The real "Initial" migration is authored for SQL Server, so point migrations at this test + // assembly - which has none - instead. That makes the store's own migration-check path + // (EnsureDatabaseCreatedAsync) see nothing pending and skip straight past it; EnsureCreatedAsync + // below builds the schema from the live model instead. + sqlite => sqlite.MigrationsAssembly( + typeof(IdempotencySqlServerStoreConcurrencyTests).Assembly.GetName().Name)) + // IdempotencyKeyConfiguration hardcodes the Body column's raw type as "nvarchar(max)" (SQL Server's + // way of saying "unbounded"); that literal string isn't valid SQLite syntax. Strip just that one + // provider-specific override so the rest of the same configuration - crucially UX_CompositeKey - + // builds unmodified. + .ReplaceService()); + _serviceProvider = services.BuildServiceProvider(); + + var factory = _serviceProvider.GetRequiredService>(); + await using var setupContext = await factory.CreateDbContextAsync(); + await setupContext.Database.EnsureCreatedAsync(); + + _store = new IdempotencySqlServerStore( + _serviceProvider, + Options.Create(new IdempotencyOptions()), + NullLogger.Instance); + } + + [Fact] + public async Task IsKeyProcessedAsync_ConcurrentRequestsWithSameKey_OnlyOneReservationWins() + { + // Arrange + var keyInfo = new IdempotentKeyInfo + { + Endpoint = "/api/items", + Method = "POST", + IdempotentKey = Guid.NewGuid().ToString() + }; + + // Act - fire 5 concurrent reservation attempts for the identical composite key against the same + // SQLite file, exactly as 5 concurrent HTTP requests would hit the same SQL Server row. + var tasks = Enumerable.Range(0, 5) + .Select(_ => _store.IsKeyProcessedAsync(keyInfo).AsTask()) + .ToArray(); + var results = await Task.WhenAll(tasks); + + // Assert - exactly one caller wins the reservation (false, null); the unique index forces every + // other concurrent caller onto the collision path, which reports back as already-processed/in-flight. + results.Count(r => !r.processed).ShouldBe(1); + results.Count(r => r.processed).ShouldBe(4); + } + + #endregion +} + +/// +/// Applies as-is, then drops the one column-type override in it +/// that is SQL-Server-specific raw SQL, so EnsureCreatedAsync can generate valid SQLite DDL. +/// +internal sealed class SqliteCompatibleModelCustomizer(ModelCustomizerDependencies dependencies) + : ModelCustomizer(dependencies) +{ + public override void Customize(ModelBuilder modelBuilder, DbContext context) + { + base.Customize(modelBuilder, context); + + var key = modelBuilder.Entity(); + key.Property(e => e.Body).HasColumnType(null); + + // The Sqlite provider cannot translate "> "/"<" comparisons on a DateTimeOffset column (only equality) - + // IsKeyProcessedAsync's expiry check relies on exactly that. Store ExpiresAt as UTC ticks instead so the + // comparison becomes an ordinary numeric one; the property's C# type/behaviour is unaffected. + key.Property(e => e.ExpiresAt).HasConversion( + v => v.HasValue ? v.Value.UtcTicks : (long?)null, + v => v.HasValue ? new DateTimeOffset(v.Value, TimeSpan.Zero) : null); + } +} diff --git a/src/AspNet/AspCore.Idempotency.MsSqlStore.Tests/Unit/IdempotencySqlServerStoreLifecycleTests.cs b/src/AspNet/AspCore.Idempotency.MsSqlStore.Tests/Unit/IdempotencySqlServerStoreLifecycleTests.cs new file mode 100644 index 00000000..cf84b5c1 --- /dev/null +++ b/src/AspNet/AspCore.Idempotency.MsSqlStore.Tests/Unit/IdempotencySqlServerStoreLifecycleTests.cs @@ -0,0 +1,203 @@ +// +// Copyright (c) 2025 Steven Hoang. All rights reserved. +// Licensed under the MIT License. See LICENSE in the project root for license information. +// + +using DKNet.AspCore.Idempotency; +using DKNet.AspCore.Idempotency.MsSqlStore.Store; +using Microsoft.EntityFrameworkCore.Infrastructure; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging.Abstractions; +using Microsoft.Extensions.Options; + +namespace AspCore.Idempotency.MsSqlStore.Tests.Unit; + +/// +/// Covers the reserve/complete lifecycle and the collision-handler branches of +/// that +/// doesn't exercise. Uses the same file-based SQLite setup so it runs without Docker/SQL Server. +/// +public sealed class IdempotencySqlServerStoreLifecycleTests : IAsyncLifetime +{ + #region Fields + + private readonly string _dbFilePath = + Path.Combine(Path.GetTempPath(), $"idempotency-lifecycle-{Guid.NewGuid():N}.db"); + + private ServiceProvider _serviceProvider = null!; + private IdempotencySqlServerStore _store = null!; + + #endregion + + #region Methods + + public async Task DisposeAsync() + { + await _store.DisposeAsync(); + await _serviceProvider.DisposeAsync(); + File.Delete(_dbFilePath); + } + + public async Task InitializeAsync() + { + var services = new ServiceCollection(); + services.AddDbContextFactory(o => o.UseSqlite( + $"Data Source={_dbFilePath}", + sqlite => sqlite.MigrationsAssembly( + typeof(IdempotencySqlServerStoreConcurrencyTests).Assembly.GetName().Name)) + .ReplaceService()); + _serviceProvider = services.BuildServiceProvider(); + + var factory = _serviceProvider.GetRequiredService>(); + await using var setupContext = await factory.CreateDbContextAsync(); + await setupContext.Database.EnsureCreatedAsync(); + + _store = CreateStore(new IdempotencyOptions()); + } + + private IdempotencySqlServerStore CreateStore(IdempotencyOptions options) => + new(_serviceProvider, Options.Create(options), NullLogger.Instance); + + private static CachedResponse CreateResponse(int statusCode, string? body, DateTimeOffset? expiresAt) => + new() + { + StatusCode = statusCode, + Body = body, + ContentType = "application/json", + CreatedAt = DateTimeOffset.UtcNow, + ExpiresAt = expiresAt + }; + + [Fact] + public async Task IsKeyProcessedAsync_FullLifecycle_ReserveThenCompleteReturnsCachedResponse() + { + // Arrange + var keyInfo = new IdempotentKeyInfo + { + Endpoint = "/api/orders", + Method = "POST", + IdempotentKey = Guid.NewGuid().ToString() + }; + + // Act - first call finds nothing and reserves the key + var reserved = await _store.IsKeyProcessedAsync(keyInfo); + + var response = CreateResponse(201, "{\"id\":1}", DateTimeOffset.UtcNow.AddHours(1)); + await _store.MarkKeyAsProcessedAsync(keyInfo, response); + + var completed = await _store.IsKeyProcessedAsync(keyInfo); + + // Assert - reservation reports not-yet-processed, then the completed row replays the cached response + reserved.processed.ShouldBeFalse(); + reserved.response.ShouldBeNull(); + + completed.processed.ShouldBeTrue(); + completed.response.ShouldNotBeNull(); + completed.response!.StatusCode.ShouldBe(201); + completed.response.Body.ShouldBe("{\"id\":1}"); + } + + [Fact] + public async Task IsKeyProcessedAsync_WhileReservationInFlight_ReturnsTrueWithNullResponse() + { + // Arrange + var keyInfo = new IdempotentKeyInfo + { + Endpoint = "/api/orders", + Method = "POST", + IdempotentKey = Guid.NewGuid().ToString() + }; + + // Act - reserve, then re-check before anyone completes it + await _store.IsKeyProcessedAsync(keyInfo); + var inFlight = await _store.IsKeyProcessedAsync(keyInfo); + + // Assert - the caller is told the key is already being processed (409 path), with no cached body yet + inFlight.processed.ShouldBeTrue(); + inFlight.response.ShouldBeNull(); + } + + [Fact] + public async Task IsKeyProcessedAsync_CollisionOnInsert_ReQueryReturnsCompletedResponse() + { + // Arrange - seed a completed row directly. ExpiresAt is left null so the initial + // "ExpiresAt > now" lookup in IsKeyProcessedAsync misses it (null compares false), forcing the + // store down the reserve-insert path, which then collides with this row's unique CompositeKey. + var keyInfo = new IdempotentKeyInfo + { + Endpoint = "/api/orders", + Method = "POST", + IdempotentKey = Guid.NewGuid().ToString() + }; + var response = CreateResponse(200, "{\"id\":42}", null); + + var factory = _serviceProvider.GetRequiredService>(); + await using (var seedContext = await factory.CreateDbContextAsync()) + { + seedContext.IdempotencyKeys.Add(new IdempotencyKeyEntity(keyInfo, response)); + await seedContext.SaveChangesAsync(); + } + + // Act + var result = await _store.IsKeyProcessedAsync(keyInfo); + + // Assert - the collision handler re-queries and returns the already-completed response + result.processed.ShouldBeTrue(); + result.response.ShouldNotBeNull(); + result.response!.StatusCode.ShouldBe(200); + result.response.Body.ShouldBe("{\"id\":42}"); + } + + [Fact] + public async Task IsKeyProcessedAsync_ExpiredReservationCollision_ReturnsFalseForFreshReservation() + { + // Arrange - a very short InFlightReservationTimeout so the reservation row is expired by the + // time the second call collides with it, without actually waiting out the 30s default. + var shortTimeoutStore = CreateStore(new IdempotencyOptions + { + InFlightReservationTimeout = TimeSpan.FromMilliseconds(1) + }); + var keyInfo = new IdempotentKeyInfo + { + Endpoint = "/api/orders", + Method = "POST", + IdempotentKey = Guid.NewGuid().ToString() + }; + + // Act + await shortTimeoutStore.IsKeyProcessedAsync(keyInfo); // reserves, expiring almost immediately + await Task.Delay(50); + var result = await shortTimeoutStore.IsKeyProcessedAsync(keyInfo); // collides, re-query finds it expired + + // Assert - Rule R1: an abandoned/expired reservation must not permanently block retries + result.processed.ShouldBeFalse(); + result.response.ShouldBeNull(); + + await shortTimeoutStore.DisposeAsync(); + } + + [Fact] + public async Task MarkKeyAsProcessedAsync_WithoutPriorReservation_CreatesEntityDefensively() + { + // Arrange - call MarkKeyAsProcessedAsync directly, skipping IsKeyProcessedAsync's reservation. + // Exercises the defensive "entity is null" branch that should not happen in the normal flow. + var keyInfo = new IdempotentKeyInfo + { + Endpoint = "/api/orders", + Method = "POST", + IdempotentKey = Guid.NewGuid().ToString() + }; + var response = CreateResponse(204, null, DateTimeOffset.UtcNow.AddHours(1)); + + // Act + await _store.MarkKeyAsProcessedAsync(keyInfo, response); + var result = await _store.IsKeyProcessedAsync(keyInfo); + + // Assert + result.processed.ShouldBeTrue(); + result.response.ShouldNotBeNull(); + result.response!.StatusCode.ShouldBe(204); + } + + #endregion +} diff --git a/src/AspNet/AspCore.Idempotency.NpgsqlStore.Tests/Integration/IdempotencyIntegrationTests.cs b/src/AspNet/AspCore.Idempotency.NpgsqlStore.Tests/Integration/IdempotencyIntegrationTests.cs index 2b71931b..86f30ae8 100644 --- a/src/AspNet/AspCore.Idempotency.NpgsqlStore.Tests/Integration/IdempotencyIntegrationTests.cs +++ b/src/AspNet/AspCore.Idempotency.NpgsqlStore.Tests/Integration/IdempotencyIntegrationTests.cs @@ -54,9 +54,8 @@ public async Task CreateItem_ConcurrentRequestsWithSameKey_OnlyOneProcessed() var idempotencyKey = Guid.NewGuid().ToString(); var request = new CreateItemRequest { Name = "Concurrent Item" }; - // Act - Send 5 concurrent requests with the same idempotency key - // multiple requests may process simultaneously when they all arrive before any is cached. - // This is a known limitation of stateless filters without distributed locking. + // Act - Send 5 concurrent requests with the same idempotency key. + // The store's atomic reservation ensures at most one of them ever reaches the handler. var tasks = Enumerable.Range(0, 5).Select(_ => { var httpRequest = new HttpRequestMessage(HttpMethod.Post, "/api/items") @@ -69,19 +68,20 @@ public async Task CreateItem_ConcurrentRequestsWithSameKey_OnlyOneProcessed() var responses = await Task.WhenAll(tasks); - // Assert - With concurrent requests, some or all may succeed (201 or 409 Conflict) - // The first request wins, subsequent ones get 409 Conflict (if ConflictHandling = ConflictResponse) - // or the cached response (if ConflictHandling = CachedResult) - var successCount = responses.Count(r => r.StatusCode == HttpStatusCode.Created); - var conflictCount = responses.Count(r => r.StatusCode == HttpStatusCode.Conflict); - - // At least one should succeed (201) - successCount.ShouldBeGreaterThanOrEqualTo(1); + // Assert - the fixture uses ConflictHandling = CachedResult, so a duplicate arriving after the + // winner completes legitimately gets 201 with the replayed body, not 409; we don't assert an + // exact 201/409 split. Instead: the handler generates a fresh Guid per call, so every 201 + // response carrying the SAME Id proves the handler ran exactly once. + var createdIds = new List(); + foreach (var response in responses) + { + if (response.StatusCode != HttpStatusCode.Created) continue; + var item = await response.Content.ReadFromJsonAsync(); + createdIds.Add(item!.Id); + } - // The rest should be conflicts or cached responses - var otherCount = responses.Count(r => r.StatusCode != HttpStatusCode.Created && - r.StatusCode != HttpStatusCode.Conflict); - (successCount + conflictCount + otherCount).ShouldBe(5); + createdIds.ShouldNotBeEmpty("At least one request should have succeeded"); + createdIds.Distinct().Count().ShouldBe(1, "The handler must have executed exactly once"); // Verify only ONE entry in database (unique constraint ensures this) await using var dbContext = fixture.GetDbContext(); @@ -326,6 +326,109 @@ public async Task CreateItem_WithExpiredIdempotencyKey_ProcessesAsNewRequest() item.Id.ShouldNotBe(Guid.Empty); } + [Fact] + public async Task CreateItem_WithExpiredInFlightReservation_ProcessesAsNewRequest() + { + // Arrange - seed an already-expired StatusCode=102 reservation placeholder directly, simulating + // a prior request whose handler never completed (crashed, timed out) within the reservation window. + var idempotencyKey = Guid.NewGuid().ToString(); + var request = new CreateItemRequest { Name = "Expired Reservation Item" }; + + await using (var seedDbContext = fixture.GetDbContext()) + { + var expiredReservation = new IdempotencyKeyEntity( + new IdempotentKeyInfo + { + IdempotentKey = idempotencyKey, + Endpoint = "/api/items", + Method = "POST" + }, + new CachedResponse + { + StatusCode = 102, + Body = null, + ContentType = "application/json", + CreatedAt = DateTimeOffset.UtcNow.AddMinutes(-2), + ExpiresAt = DateTimeOffset.UtcNow.AddMinutes(-1) + }); + + seedDbContext.IdempotencyKeys.Add(expiredReservation); + await seedDbContext.SaveChangesAsync(); + } + + // Act - a request with the same key must not be permanently blocked by the stale reservation + var httpRequest = new HttpRequestMessage(HttpMethod.Post, "/api/items") + { + Headers = { { "X-Idempotency-Key", idempotencyKey } }, + Content = JsonContent.Create(request) + }; + var response = await fixture.HttpClient!.SendAsync(httpRequest); + + // Assert + response.StatusCode.ShouldBe(HttpStatusCode.Created); + var item = await response.Content.ReadFromJsonAsync(); + item!.Name.ShouldBe("Expired Reservation Item"); + item.Id.ShouldNotBe(Guid.Empty); + } + + [Fact] + public async Task CreateItem_ConcurrentRequestsAgainstExpiredReservation_OnlyOneProcessed() + { + // Arrange - seed an already-expired StatusCode=102 reservation so every concurrent request's + // initial unexpired-row query misses it, and every request's own reservation INSERT collides + // with this same stale row (unique index doesn't care that it's expired). + var idempotencyKey = Guid.NewGuid().ToString(); + var request = new CreateItemRequest { Name = "Expired Reservation Race Item" }; + + await using (var seedDbContext = fixture.GetDbContext()) + { + var expiredReservation = new IdempotencyKeyEntity( + new IdempotentKeyInfo + { + IdempotentKey = idempotencyKey, + Endpoint = "/api/items", + Method = "POST" + }, + new CachedResponse + { + StatusCode = 102, + Body = null, + ContentType = "application/json", + CreatedAt = DateTimeOffset.UtcNow.AddMinutes(-2), + ExpiresAt = DateTimeOffset.UtcNow.AddMinutes(-1) + }); + + seedDbContext.IdempotencyKeys.Add(expiredReservation); + await seedDbContext.SaveChangesAsync(); + } + + // Act - fire 5 concurrent requests against the same key, all racing the stale expired row + var tasks = Enumerable.Range(0, 5).Select(_ => + { + var httpRequest = new HttpRequestMessage(HttpMethod.Post, "/api/items") + { + Headers = { { "X-Idempotency-Key", idempotencyKey } }, + Content = JsonContent.Create(request) + }; + return fixture.HttpClient!.SendAsync(httpRequest); + }).ToArray(); + + var responses = await Task.WhenAll(tasks); + + // Assert - only one distinct handler execution must be observable, same as the fresh-key case + var createdIds = new List(); + foreach (var response in responses) + { + if (response.StatusCode != HttpStatusCode.Created) continue; + var item = await response.Content.ReadFromJsonAsync(); + createdIds.Add(item!.Id); + } + + createdIds.ShouldNotBeEmpty("At least one request should have succeeded"); + createdIds.Distinct().Count().ShouldBe(1, + "The handler must have executed exactly once, even when racing an already-expired reservation row"); + } + public Task DisposeAsync() => Task.CompletedTask; public Task InitializeAsync() => Task.CompletedTask; diff --git a/src/AspNet/AspCore.Idempotency.Tests/AspCore.Idempotency.Tests.csproj b/src/AspNet/AspCore.Idempotency.Tests/AspCore.Idempotency.Tests.csproj index 9e48486d..4255de58 100644 --- a/src/AspNet/AspCore.Idempotency.Tests/AspCore.Idempotency.Tests.csproj +++ b/src/AspNet/AspCore.Idempotency.Tests/AspCore.Idempotency.Tests.csproj @@ -19,6 +19,7 @@ + diff --git a/src/AspNet/AspCore.Idempotency.Tests/Unit/IdempotencyDistributedCacheReservationTests.cs b/src/AspNet/AspCore.Idempotency.Tests/Unit/IdempotencyDistributedCacheReservationTests.cs new file mode 100644 index 00000000..34e9f1b1 --- /dev/null +++ b/src/AspNet/AspCore.Idempotency.Tests/Unit/IdempotencyDistributedCacheReservationTests.cs @@ -0,0 +1,115 @@ +// +// Copyright (c) 2025 Steven Hoang. All rights reserved. +// Licensed under the MIT License. See LICENSE in the project root for license information. +// + +using DKNet.AspCore.Idempotency; +using DKNet.AspCore.Idempotency.Store; +using Microsoft.Extensions.Caching.Distributed; +using Microsoft.Extensions.Caching.Memory; +using Microsoft.Extensions.Logging; + +namespace AspCore.Idempotency.Tests.Unit; + +/// +/// Covers the in-flight reservation placeholder introduced into +/// , which the pre-existing +/// mark-then-check tests never exercise because they always mark a key as processed before checking it. +/// +public sealed class IdempotencyDistributedCacheReservationTests +{ + #region Fields + + private readonly IDistributedCache _cache; + private readonly ILogger _logger; + + #endregion + + #region Constructors + + public IdempotencyDistributedCacheReservationTests() + { + _cache = new MemoryDistributedCache(Options.Create(new MemoryDistributedCacheOptions())); + _logger = LoggerFactory.Create(b => b.AddConsole()).CreateLogger(); + } + + #endregion + + #region Methods + + private IdempotencyDistributedCacheStore CreateStore(IdempotencyOptions? options = null) => + new(_cache, Options.Create(options ?? new IdempotencyOptions()), _logger); + + private static IdempotentKeyInfo MakeKey(string key) => + new() { IdempotentKey = key, Endpoint = "/api/test", Method = "POST" }; + + [Fact] + public async Task IsKeyProcessedAsync_FreshKey_WritesReservationSeenAsInFlightByNextCall() + { + // Arrange + var store = CreateStore(); + var keyInfo = MakeKey(Guid.NewGuid().ToString()); + + // Act - first call is a genuine miss and must write the in-flight placeholder itself + var first = await store.IsKeyProcessedAsync(keyInfo); + var second = await store.IsKeyProcessedAsync(keyInfo); + + // Assert - the miss reports "not processed yet"; the placeholder it wrote makes the very next + // call for the same key see an in-flight reservation (409 path) rather than another miss. + first.processed.ShouldBeFalse(); + first.response.ShouldBeNull(); + + second.processed.ShouldBeTrue(); + second.response.ShouldBeNull(); + } + + [Fact] + public async Task IsKeyProcessedAsync_AfterReservationCompletes_ReturnsCachedResponse() + { + // Arrange + var store = CreateStore(); + var keyInfo = MakeKey(Guid.NewGuid().ToString()); + var response = new CachedResponse + { + StatusCode = 201, + Body = "{\"id\":1}", + ContentType = "application/json", + CreatedAt = DateTimeOffset.UtcNow, + ExpiresAt = DateTimeOffset.UtcNow.AddHours(1) + }; + + // Act - reserve, complete, then replay + await store.IsKeyProcessedAsync(keyInfo); + await store.MarkKeyAsProcessedAsync(keyInfo, response); + var completed = await store.IsKeyProcessedAsync(keyInfo); + + // Assert - the completed response overwrites the reservation placeholder + completed.processed.ShouldBeTrue(); + completed.response.ShouldNotBeNull(); + completed.response!.StatusCode.ShouldBe(201); + completed.response.Body.ShouldBe("{\"id\":1}"); + } + + [Fact] + public async Task IsKeyProcessedAsync_AfterReservationTimeoutElapses_TreatsKeyAsFreshMiss() + { + // Arrange - a very short reservation timeout so the placeholder expires almost immediately, + // exercising InFlightReservationTimeout's actual expiry behavior rather than just its default value. + var store = CreateStore(new IdempotencyOptions + { + InFlightReservationTimeout = TimeSpan.FromMilliseconds(1) + }); + var keyInfo = MakeKey(Guid.NewGuid().ToString()); + + // Act + await store.IsKeyProcessedAsync(keyInfo); // reserves, expiring almost immediately + await Task.Delay(50); + var result = await store.IsKeyProcessedAsync(keyInfo); + + // Assert - an abandoned reservation must not permanently block retries + result.processed.ShouldBeFalse(); + result.response.ShouldBeNull(); + } + + #endregion +} diff --git a/src/AspNet/AspCore.Idempotency.Tests/Unit/IdempotencyOptionsTests.cs b/src/AspNet/AspCore.Idempotency.Tests/Unit/IdempotencyOptionsTests.cs index 5ef003ca..1c95215a 100644 --- a/src/AspNet/AspCore.Idempotency.Tests/Unit/IdempotencyOptionsTests.cs +++ b/src/AspNet/AspCore.Idempotency.Tests/Unit/IdempotencyOptionsTests.cs @@ -82,6 +82,20 @@ public void IdempotencyOptions_HasDefaultValues() options.ConflictHandling.ShouldBe(IdempotentConflictHandling.ConflictResponse); options.JsonSerializerOptions.ShouldNotBeNull(); options.JsonSerializerOptions.PropertyNamingPolicy.ShouldBe(JsonNamingPolicy.CamelCase); + options.InFlightReservationTimeout.ShouldBe(TimeSpan.FromSeconds(30)); + } + + [Fact] + public void IdempotencyOptions_CanSetCustomInFlightReservationTimeout() + { + // Arrange + var customTimeout = TimeSpan.FromSeconds(5); + + // Act + var options = new IdempotencyOptions { InFlightReservationTimeout = customTimeout }; + + // Assert + options.InFlightReservationTimeout.ShouldBe(customTimeout); } [Fact] diff --git a/src/AspNet/DKNet.AspCore.Idempotency.MsSqlStore/Data/IdempotencyKeyEntity.cs b/src/AspNet/DKNet.AspCore.Idempotency.MsSqlStore/Data/IdempotencyKeyEntity.cs index cfa65606..1363ce95 100644 --- a/src/AspNet/DKNet.AspCore.Idempotency.MsSqlStore/Data/IdempotencyKeyEntity.cs +++ b/src/AspNet/DKNet.AspCore.Idempotency.MsSqlStore/Data/IdempotencyKeyEntity.cs @@ -101,6 +101,20 @@ private IdempotencyKeyEntity() #region Methods + /// + /// Overwrites this reservation placeholder with the final, completed response. + /// Used to turn an in-flight reservation row into the durable cached result once the + /// protected handler has finished, mirroring the constructor's field assignments. + /// + /// The completed response to store. + internal void Complete(CachedResponse response) + { + StatusCode = response.StatusCode; + Body = response.Body; + ContentType = response.ContentType; + ExpiresAt = response.ExpiresAt; + } + /// /// Sanitizes an idempotency key for use as a database key by hashing it. /// Hashing (rather than stripping characters) guarantees structurally distinct diff --git a/src/AspNet/DKNet.AspCore.Idempotency.MsSqlStore/Store/IdempotencySqlServerStore.cs b/src/AspNet/DKNet.AspCore.Idempotency.MsSqlStore/Store/IdempotencySqlServerStore.cs index 739e3921..fbcd4f68 100644 --- a/src/AspNet/DKNet.AspCore.Idempotency.MsSqlStore/Store/IdempotencySqlServerStore.cs +++ b/src/AspNet/DKNet.AspCore.Idempotency.MsSqlStore/Store/IdempotencySqlServerStore.cs @@ -8,6 +8,7 @@ using Microsoft.EntityFrameworkCore; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Options; namespace DKNet.AspCore.Idempotency.MsSqlStore.Store; @@ -17,13 +18,22 @@ namespace DKNet.AspCore.Idempotency.MsSqlStore.Store; /// internal sealed class IdempotencySqlServerStore( IServiceProvider serviceProvider, + IOptions options, ILogger logger) : IIdempotencyKeyStore, IAsyncDisposable { #region Fields + /// + /// HTTP 102 (Processing) is used as the sentinel status code for an in-flight reservation row — + /// legal under CK_StatusCode_Valid (100-599) and outside the range any real completed + /// response would use. + /// + private const int ReservationStatusCode = 102; + private static int _dbMigrationsEnsured; private static readonly SemaphoreSlim MigrationLock = new(1, 1); + private readonly IdempotencyOptions _options = options.Value; private readonly AsyncServiceScope _scope = serviceProvider.CreateAsyncScope(); #endregion @@ -56,6 +66,19 @@ private static async ValueTask EnsureDatabaseCreatedAsync(DbContext dbContext, } } + /// + /// Maps a stored entity to its representation for replay. + /// + private static CachedResponse ToCachedResponse(IdempotencyKeyEntity entity) => + new() + { + StatusCode = entity.StatusCode, + Body = entity.Body, + ContentType = entity.ContentType ?? "application/json", + CreatedAt = entity.CreatedAt, + ExpiresAt = entity.ExpiresAt + }; + /// public async ValueTask<(bool processed, CachedResponse? response)> IsKeyProcessedAsync(IdempotentKeyInfo keyInfo) { @@ -73,33 +96,64 @@ private static async ValueTask EnsureDatabaseCreatedAsync(DbContext dbContext, .FirstOrDefaultAsync(k => k.CompositeKey == sanitizedKey && k.ExpiresAt > DateTime.UtcNow) .ConfigureAwait(false); - if (existing == null) + if (existing != null && !existing.IsExpired) { - logger.LogDebug("Idempotency key not found or expired: {Key}", sanitizedKey); - return (false, null); + if (existing.StatusCode == ReservationStatusCode) + { + logger.LogDebug("Idempotency key reservation still in-flight: {Key}", sanitizedKey); + return (true, null); + } + + logger.LogInformation( + "Idempotency key found with status code {StatusCode}: {Key}", + existing.StatusCode, + sanitizedKey); + + return (true, ToCachedResponse(existing)); } - if (existing.IsExpired) + // No completed row (and no live reservation) found — reserve this key for the current caller so a + // concurrent request for the identical key can never also observe an empty slot. + logger.LogDebug("Idempotency key not found or expired, reserving: {Key}", sanitizedKey); + + var reservation = new IdempotencyKeyEntity(keyInfo, new CachedResponse + { + StatusCode = ReservationStatusCode, + Body = null, + ContentType = string.Empty, + CreatedAt = DateTimeOffset.UtcNow, + ExpiresAt = DateTimeOffset.UtcNow.Add(_options.InFlightReservationTimeout) + }); + + try { - logger.LogDebug("Idempotency key has expired: {Key}", sanitizedKey); + dbContext.IdempotencyKeys.Add(reservation); + await dbContext.SaveChangesAsync().ConfigureAwait(false); return (false, null); } + catch (DbUpdateException ex) when ((ex.InnerException?.Message ?? ex.Message).Contains("UNIQUE", StringComparison.OrdinalIgnoreCase)) + { + // Handle race condition: another concurrent request already reserved or completed this key. + logger.LogInformation( + "Idempotency key reservation collided with a concurrent request: {Key}. Re-checking status.", + sanitizedKey); - logger.LogInformation( - "Idempotency key found with status code {StatusCode}: {Key}", - existing.StatusCode, - sanitizedKey); + var concurrent = await dbContext.IdempotencyKeys + .AsNoTracking() + .FirstOrDefaultAsync(k => k.CompositeKey == sanitizedKey) + .ConfigureAwait(false); - var cachedResponse = new CachedResponse - { - StatusCode = existing.StatusCode, - Body = existing.Body, - ContentType = existing.ContentType ?? "application/json", - CreatedAt = existing.CreatedAt, - ExpiresAt = existing.ExpiresAt - }; + if (concurrent is null || concurrent.IsExpired) + { + // The competing row is gone or has since expired — treat the key as not found, matching + // Rule R1: an abandoned reservation must not permanently block retries. + return (false, null); + } + + if (concurrent.StatusCode == ReservationStatusCode) return (true, null); - return (true, cachedResponse); + return (true, ToCachedResponse(concurrent)); + } } /// @@ -117,11 +171,23 @@ public async ValueTask MarkKeyAsProcessedAsync(IdempotentKeyInfo keyInfo, Cached var factory = _scope.ServiceProvider.GetRequiredService>(); await using var dbContext = await factory.CreateDbContextAsync(); - - var entity = new IdempotencyKeyEntity(keyInfo, cachedResponse); await EnsureDatabaseCreatedAsync(dbContext); - dbContext.IdempotencyKeys.Add(entity); + var entity = await dbContext.IdempotencyKeys + .FirstOrDefaultAsync(k => k.CompositeKey == sanitizedKey) + .ConfigureAwait(false); + + if (entity is null) + { + // Defensive only: should not happen once IsKeyProcessedAsync always reserves first. + entity = new IdempotencyKeyEntity(keyInfo, cachedResponse); + dbContext.IdempotencyKeys.Add(entity); + } + else + { + entity.Complete(cachedResponse); + } + await dbContext.SaveChangesAsync().ConfigureAwait(false); logger.LogInformation( @@ -131,7 +197,7 @@ public async ValueTask MarkKeyAsProcessedAsync(IdempotentKeyInfo keyInfo, Cached } catch (DbUpdateException ex) when ((ex.InnerException?.Message??ex.Message).Contains("UNIQUE",StringComparison.OrdinalIgnoreCase)) { - // Handle race condition: Another concurrent request already inserted this key + // Handle race condition: another concurrent request already inserted this key. logger.LogInformation( "Idempotency key already processed by concurrent request: {Key}. Continuing without duplicate insert.", sanitizedKey); diff --git a/src/AspNet/DKNet.AspCore.Idempotency.NpgsqlStore/Data/IdempotencyKeyEntity.cs b/src/AspNet/DKNet.AspCore.Idempotency.NpgsqlStore/Data/IdempotencyKeyEntity.cs index 5dc508cc..a96d47fc 100644 --- a/src/AspNet/DKNet.AspCore.Idempotency.NpgsqlStore/Data/IdempotencyKeyEntity.cs +++ b/src/AspNet/DKNet.AspCore.Idempotency.NpgsqlStore/Data/IdempotencyKeyEntity.cs @@ -101,6 +101,21 @@ private IdempotencyKeyEntity() #region Methods + /// + /// Completes an in-flight reservation placeholder in place, overwriting it with the actual + /// processed response. Used when this row was inserted as a StatusCode == 102 reservation + /// and the protected handler has now finished, so the row transitions from placeholder to + /// completed cache entry without a second insert. + /// + /// The cached response to store. + internal void Complete(CachedResponse response) + { + StatusCode = response.StatusCode; + Body = response.Body; + ContentType = response.ContentType; + ExpiresAt = response.ExpiresAt; + } + /// /// Sanitizes an idempotency key for use as a database key by hashing it. /// Hashing (rather than stripping characters) guarantees structurally distinct diff --git a/src/AspNet/DKNet.AspCore.Idempotency.NpgsqlStore/Store/IdempotencyPostgresStore.cs b/src/AspNet/DKNet.AspCore.Idempotency.NpgsqlStore/Store/IdempotencyPostgresStore.cs index e45a1665..df530370 100644 --- a/src/AspNet/DKNet.AspCore.Idempotency.NpgsqlStore/Store/IdempotencyPostgresStore.cs +++ b/src/AspNet/DKNet.AspCore.Idempotency.NpgsqlStore/Store/IdempotencyPostgresStore.cs @@ -8,6 +8,7 @@ using Microsoft.EntityFrameworkCore; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Options; using Npgsql; namespace DKNet.AspCore.Idempotency.NpgsqlStore.Store; @@ -18,7 +19,8 @@ namespace DKNet.AspCore.Idempotency.NpgsqlStore.Store; /// internal sealed class IdempotencyPostgresStore( IServiceProvider serviceProvider, - ILogger logger) : IIdempotencyKeyStore, IAsyncDisposable + ILogger logger, + IOptions options) : IIdempotencyKeyStore, IAsyncDisposable { #region Fields @@ -84,13 +86,13 @@ private static bool IsUniqueViolation(DbUpdateException ex) => if (existing == null) { logger.LogDebug("Idempotency key not found or expired: {Key}", sanitizedKey); - return (false, null); + return await ReserveKeyAsync(dbContext, keyInfo, sanitizedKey).ConfigureAwait(false); } if (existing.IsExpired) { logger.LogDebug("Idempotency key has expired: {Key}", sanitizedKey); - return (false, null); + return await ReserveKeyAsync(dbContext, keyInfo, sanitizedKey).ConfigureAwait(false); } logger.LogInformation( @@ -98,18 +100,106 @@ private static bool IsUniqueViolation(DbUpdateException ex) => existing.StatusCode, sanitizedKey); - var cachedResponse = new CachedResponse + return (true, ToCachedResponse(existing)); + } + + /// + /// Attempts to atomically reserve by inserting a + /// StatusCode == 102 placeholder row, relying on the UX_CompositeKey unique index to + /// serialize concurrent callers — only the caller whose insert succeeds proceeds to run the + /// protected handler. + /// + /// The open database context to reserve the key on. + /// The idempotency key information for the reservation row. + /// The pre-computed, sanitized composite key. + /// + /// (false, null) if this call reserved the key and the caller should proceed with + /// processing; otherwise (true, response) with the completed duplicate's cached response, + /// or (true, null) if another caller currently holds an unexpired in-flight reservation. + /// + private async ValueTask<(bool processed, CachedResponse? response)> ReserveKeyAsync( + IdempotencyDbContext dbContext, IdempotentKeyInfo keyInfo, string sanitizedKey) + { + try { - StatusCode = existing.StatusCode, - Body = existing.Body, - ContentType = existing.ContentType ?? "application/json", - CreatedAt = existing.CreatedAt, - ExpiresAt = existing.ExpiresAt - }; + var reservation = new IdempotencyKeyEntity(keyInfo, new CachedResponse + { + StatusCode = 102, + Body = null, + ContentType = "application/json", + CreatedAt = DateTimeOffset.UtcNow, + ExpiresAt = DateTimeOffset.UtcNow + options.Value.InFlightReservationTimeout + }); + + dbContext.IdempotencyKeys.Add(reservation); + await dbContext.SaveChangesAsync().ConfigureAwait(false); + + return (false, null); + } + catch (DbUpdateException ex) when (IsUniqueViolation(ex)) + { + // A concurrent request already holds this composite key - find out what state it's in. + var blocking = await dbContext.IdempotencyKeys + .AsNoTracking() + .FirstOrDefaultAsync(k => k.CompositeKey == sanitizedKey) + .ConfigureAwait(false); + + if (blocking is { IsExpired: false }) + { + logger.LogInformation( + "Idempotency key already reserved or processed by a concurrent request: {Key}", + sanitizedKey); + return (true, blocking.StatusCode == 102 ? null : ToCachedResponse(blocking)); + } + + // The row blocking our insert is itself expired (a stale reservation or completed entry + // nothing ever purged). Reclaiming it with a plain read-then-write would reopen the same + // race this method exists to close, so reclaim it atomically: a conditional UPDATE that + // only matches while the row is still expired. Its affected-row count gives the same + // single-winner guarantee the unique index gives the fresh-insert path - only the caller + // whose UPDATE actually flips the row wins; every other concurrent racer's UPDATE affects + // zero rows once the winner has moved ExpiresAt into the future. + var now = DateTime.UtcNow; + var reclaimed = await dbContext.IdempotencyKeys + .Where(k => k.CompositeKey == sanitizedKey && k.ExpiresAt != null && k.ExpiresAt <= now) + .ExecuteUpdateAsync(s => s + .SetProperty(k => k.StatusCode, 102) + .SetProperty(k => k.Body, (string?)null) + .SetProperty(k => k.ContentType, "application/json") + .SetProperty(k => k.CreatedAt, now) + .SetProperty(k => k.ExpiresAt, now + options.Value.InFlightReservationTimeout)) + .ConfigureAwait(false); + + if (reclaimed == 1) + { + logger.LogDebug("Reclaimed expired idempotency key row, proceeding as new: {Key}", sanitizedKey); + return (false, null); + } + + // Another caller reclaimed (or completed) the row between our insert collision and this + // reclaim attempt - re-read its current state and branch exactly like the unexpired path. + var current = await dbContext.IdempotencyKeys + .AsNoTracking() + .FirstOrDefaultAsync(k => k.CompositeKey == sanitizedKey) + .ConfigureAwait(false); - return (true, cachedResponse); + logger.LogInformation( + "Idempotency key already reserved or processed by a concurrent request: {Key}", + sanitizedKey); + return (true, current is null || current.StatusCode == 102 ? null : ToCachedResponse(current)); + } } + private static CachedResponse ToCachedResponse(IdempotencyKeyEntity entity) => + new() + { + StatusCode = entity.StatusCode, + Body = entity.Body, + ContentType = entity.ContentType ?? "application/json", + CreatedAt = entity.CreatedAt, + ExpiresAt = entity.ExpiresAt + }; + /// public async ValueTask MarkKeyAsProcessedAsync(IdempotentKeyInfo keyInfo, CachedResponse cachedResponse) { @@ -125,11 +215,19 @@ public async ValueTask MarkKeyAsProcessedAsync(IdempotentKeyInfo keyInfo, Cached var factory = _scope.ServiceProvider.GetRequiredService>(); await using var dbContext = await factory.CreateDbContextAsync(); - - var entity = new IdempotencyKeyEntity(keyInfo, cachedResponse); await EnsureDatabaseCreatedAsync(dbContext); - dbContext.IdempotencyKeys.Add(entity); + var reservation = await dbContext.IdempotencyKeys + .FirstOrDefaultAsync(k => k.CompositeKey == sanitizedKey) + .ConfigureAwait(false); + + if (reservation != null) + reservation.Complete(cachedResponse); + else + // Defensive fallback only - should not happen now that IsKeyProcessedAsync always + // reserves the row before the handler runs. + dbContext.IdempotencyKeys.Add(new IdempotencyKeyEntity(keyInfo, cachedResponse)); + await dbContext.SaveChangesAsync().ConfigureAwait(false); logger.LogInformation( @@ -139,7 +237,8 @@ public async ValueTask MarkKeyAsProcessedAsync(IdempotentKeyInfo keyInfo, Cached } catch (DbUpdateException ex) when (IsUniqueViolation(ex)) { - // Handle race condition: Another concurrent request already inserted this key + // Handle race condition: Another concurrent request already inserted this key. + // Unreachable in the common path now, kept as a defensive guard around the fallback Add() above. logger.LogInformation( "Idempotency key already processed by concurrent request: {Key}. Continuing without duplicate insert.", sanitizedKey); diff --git a/src/AspNet/DKNet.AspCore.Idempotency/IdempotencyOptions.cs b/src/AspNet/DKNet.AspCore.Idempotency/IdempotencyOptions.cs index 7b97188b..52c56563 100644 --- a/src/AspNet/DKNet.AspCore.Idempotency/IdempotencyOptions.cs +++ b/src/AspNet/DKNet.AspCore.Idempotency/IdempotencyOptions.cs @@ -70,6 +70,16 @@ public sealed class IdempotencyOptions /// public string IdempotencyKeyPattern { get; set; } = @"^[a-zA-Z0-9\-_]+$"; + /// + /// Gets or sets how long an in-flight reservation placeholder is honoured before being treated as + /// expired and abandoned. A store that makes its check-and-reserve step atomic inserts such a + /// placeholder while the protected handler is running; once this timeout elapses without the + /// reservation being completed (e.g. the handler crashed), a fresh request for the same key is + /// allowed to proceed instead of being permanently blocked. + /// Default is 30 seconds. + /// + public TimeSpan InFlightReservationTimeout { get; set; } = TimeSpan.FromSeconds(30); + /// /// Gets or sets the JSON serializer options used to serialize response bodies before caching them. /// This is used when the conflict handling strategy is set to return cached results. diff --git a/src/AspNet/DKNet.AspCore.Idempotency/Store/IdempotencyDistributedCacheStore.cs b/src/AspNet/DKNet.AspCore.Idempotency/Store/IdempotencyDistributedCacheStore.cs index bd22010f..fd96a91b 100644 --- a/src/AspNet/DKNet.AspCore.Idempotency/Store/IdempotencyDistributedCacheStore.cs +++ b/src/AspNet/DKNet.AspCore.Idempotency/Store/IdempotencyDistributedCacheStore.cs @@ -11,6 +11,12 @@ namespace DKNet.AspCore.Idempotency.Store; /// Implementation of using a distributed cache. /// This store provides idempotency support by caching processed keys and their responses. /// +/// +/// has no atomic compare-and-set primitive, so this store narrows the +/// check-then-act race window to the gap between GetStringAsync and SetStringAsync in +/// rather than eliminating it the way the SQL store's unique index does. +/// Do not treat this store as fully atomic under concurrency. +/// internal sealed class IdempotencyDistributedCacheStore( IDistributedCache cache, IOptions options, @@ -18,6 +24,11 @@ internal sealed class IdempotencyDistributedCacheStore( { #region Fields + /// + /// HTTP 102 (Processing) is used as the sentinel status code for an in-flight reservation entry. + /// + private const int ReservationStatusCode = 102; + /// /// Gets the idempotency options used for cache configuration and JSON serialization. /// @@ -44,25 +55,50 @@ internal sealed class IdempotencyDistributedCacheStore( var cachedJson = await cache.GetStringAsync(cacheKey).ConfigureAwait(false); - if (string.IsNullOrWhiteSpace(cachedJson)) + if (!string.IsNullOrWhiteSpace(cachedJson)) { - logger.LogDebug("No cached response found for key: {CacheKey}", cacheKey); - return (false, null); + var cachedResponse = JsonSerializer.Deserialize(cachedJson, _options.JsonSerializerOptions); + + if (cachedResponse?.IsExpired == true) + { + logger.LogDebug("Cached response has expired for key: {CacheKey}", cacheKey); + await cache.RemoveAsync(cacheKey).ConfigureAwait(false); + } + else if (cachedResponse?.StatusCode == ReservationStatusCode) + { + logger.LogDebug("Reservation still in-flight for key: {CacheKey}", cacheKey); + return (true, null); + } + else + { + logger.LogDebug("Cached response found for key: {CacheKey} with status code: {StatusCode}", + cacheKey, cachedResponse?.StatusCode); + return (true, cachedResponse); + } } - var cachedResponse = JsonSerializer.Deserialize(cachedJson, _options.JsonSerializerOptions); + // No live entry found — reserve this key so a concurrent request for the identical key sees the + // in-flight placeholder instead of also observing a miss (see class remarks for the residual race). + logger.LogDebug("No cached response found for key: {CacheKey}. Reserving.", cacheKey); - // Check if the cached response has expired - if (cachedResponse?.IsExpired == true) + var reservation = new CachedResponse { - logger.LogDebug("Cached response has expired for key: {CacheKey}", cacheKey); - await cache.RemoveAsync(cacheKey).ConfigureAwait(false); - return (false, null); - } + StatusCode = ReservationStatusCode, + Body = null, + ContentType = string.Empty, + CreatedAt = DateTimeOffset.UtcNow, + ExpiresAt = DateTimeOffset.UtcNow.Add(_options.InFlightReservationTimeout) + }; + + await cache.SetStringAsync( + cacheKey, + JsonSerializer.Serialize(reservation, _options.JsonSerializerOptions), + new DistributedCacheEntryOptions + { + AbsoluteExpirationRelativeToNow = _options.InFlightReservationTimeout + }).ConfigureAwait(false); - logger.LogDebug("Cached response found for key: {CacheKey} with status code: {StatusCode}", - cacheKey, cachedResponse?.StatusCode); - return (true, cachedResponse); + return (false, null); } /// diff --git a/src/AspNet/DKNet.AspCore.Idempotency/Store/IdempotencyKeyStore.cs b/src/AspNet/DKNet.AspCore.Idempotency/Store/IdempotencyKeyStore.cs index 47756e6b..f6da57d3 100644 --- a/src/AspNet/DKNet.AspCore.Idempotency/Store/IdempotencyKeyStore.cs +++ b/src/AspNet/DKNet.AspCore.Idempotency/Store/IdempotencyKeyStore.cs @@ -7,14 +7,17 @@ namespace DKNet.AspCore.Idempotency.Store; public interface IIdempotencyKeyStore { /// - /// Checks if the key has been processed and retrieves the cached response if available. - /// Returns the cached response including HTTP status code, body, and content type. + /// Atomically checks whether the key has been processed and, if not, reserves it for the caller. + /// A call that returns (false, null) MUST have already durably recorded that this composite key + /// is now in-flight, such that a concurrent call for the identical key can never also observe + /// (false, null) — exactly one caller per key is granted the right to proceed. /// /// The idempotency key to check for prior processing. /// /// A tuple containing: - /// - A boolean indicating whether the key has been processed - /// - The CachedResponse if available, or null if no cached response exists + /// - A boolean indicating whether the key has been processed or is currently reserved by another caller + /// - The CachedResponse if the key was already completed, or null if no cached response exists yet + /// (either the key is new and now reserved by this call, or another caller's reservation is still in-flight) /// ValueTask<(bool processed, CachedResponse? response)> IsKeyProcessedAsync(IdempotentKeyInfo keyInfo); diff --git a/src/EfCore/EfCore.Extensions.Tests/Architecture/ConsoleUsageArchitectureTests.cs b/src/EfCore/EfCore.Extensions.Tests/Architecture/ConsoleUsageArchitectureTests.cs new file mode 100644 index 00000000..a47e78c8 --- /dev/null +++ b/src/EfCore/EfCore.Extensions.Tests/Architecture/ConsoleUsageArchitectureTests.cs @@ -0,0 +1,66 @@ +using DKNet.EfCore.Extensions.Extensions; +using NetArchTest.Rules; + +namespace EfCore.Extensions.Tests.ArchRules; + +/// +/// Enforces that library code in DKNet.EfCore.Extensions never writes diagnostics through +/// . A framework primitive that writes to Console bypasses the host's +/// logging configuration entirely — it cannot be filtered, redirected, redacted, or suppressed — and pollutes +/// stdout in production. Diagnostics must go through an injected ILogger instead. +/// +/// This is a Tier-2 baseline rule (architecture-review DRK-73): there is exactly one known offender today, +/// (EfCoreExceptionHandler.cs:61), which is on the allow-list +/// below. The allow-list must only ever SHRINK — when the offender is migrated to ILogger, delete its +/// entry so the rule covers the whole assembly. Do not add new names to it. +/// +/// +public sealed class ConsoleUsageArchitectureTests +{ + #region Fields + + /// + /// Today's known offenders. Must only shrink — never add a name here to silence a new violation. + /// + private static readonly string[] KnownViolations = [nameof(EfCoreExceptionHandler)]; + + #endregion + + #region Methods + + [Fact] + public void ProductionTypes_ExceptKnownOffenders_MustNotDependOnSystemConsole() + { + var result = Types.InAssembly(typeof(EfCoreExceptionHandler).Assembly) + .That() + .DoNotHaveName(KnownViolations) + .Should() + .NotHaveDependencyOn("System.Console") + .GetResult(); + + var offenders = result.FailingTypeNames ?? []; + result.IsSuccessful.ShouldBeTrue( + "Library code must emit diagnostics through an injected ILogger, never System.Console, so hosts can " + + "filter/redirect/redact/suppress them. New offenders: " + string.Join(", ", offenders) + + ". Fix by injecting ILogger instead of calling Console.*; do not add the type to the KnownViolations allow-list."); + } + + [Fact] + public void Rule_CanDetectConsoleUsage_OnTheKnownOffender() + { + // Self-check: the allow-listed offender genuinely uses System.Console. If this ever passes, the rule above + // has gone blind (NetArchTest can no longer see the dependency) and would silently stop enforcing anything. + var result = Types.InAssembly(typeof(EfCoreExceptionHandler).Assembly) + .That() + .HaveName(nameof(EfCoreExceptionHandler)) + .Should() + .NotHaveDependencyOn("System.Console") + .GetResult(); + + result.IsSuccessful.ShouldBeFalse( + "The known offender EfCoreExceptionHandler must still be detected as depending on System.Console; " + + "if this assertion fails the enforcement rule can no longer see Console usage and is worthless."); + } + + #endregion +} diff --git a/src/EfCore/EfCore.Extensions.Tests/EfCore.Extensions.Tests.csproj b/src/EfCore/EfCore.Extensions.Tests/EfCore.Extensions.Tests.csproj index 4a4005c3..26d7e970 100644 --- a/src/EfCore/EfCore.Extensions.Tests/EfCore.Extensions.Tests.csproj +++ b/src/EfCore/EfCore.Extensions.Tests/EfCore.Extensions.Tests.csproj @@ -30,6 +30,7 @@ + runtime; build; native; contentfiles; analyzers; buildtransitive diff --git a/src/Services/DKNet.Svc.BlobStorage.AwsS3/S3BlobService.cs b/src/Services/DKNet.Svc.BlobStorage.AwsS3/S3BlobService.cs index d08708ee..4b0fff72 100644 --- a/src/Services/DKNet.Svc.BlobStorage.AwsS3/S3BlobService.cs +++ b/src/Services/DKNet.Svc.BlobStorage.AwsS3/S3BlobService.cs @@ -42,7 +42,9 @@ public sealed class S3BlobService(IOptions options, ILoggertrue when a matching object exists; otherwise false. public override async Task CheckExistsAsync(BlobRequest blob, CancellationToken cancellationToken = default) { - var location = GetBlobLocation(blob); + // S3 keys are not path-segments: a leading slash here would double up with the bucket + // segment under ForcePathStyle addressing (e.g. Minio) and break SigV4 signing. + var location = GetBlobLocation(blob).TrimStart('/'); var client = await GetS3ClientAsync(cancellationToken); try @@ -78,7 +80,7 @@ public override async Task CheckExistsAsync(BlobRequest blob, Cancellation /// true when deletion completed (or object did not exist); otherwise false. public override Task DeleteAsync(BlobRequest blob, CancellationToken cancellationToken = default) { - var location = GetBlobLocation(blob); + var location = GetBlobLocation(blob).TrimStart('/'); return blob.Type == BlobTypes.File ? DeleteFileAsync(location, cancellationToken) : DeleteFolderAsync(location, cancellationToken); @@ -122,16 +124,15 @@ private async Task DeleteFolderAsync(string folderLocation, CancellationTo cancellationToken.ThrowIfCancellationRequested(); - // Delete the whole page in a single request. A ListObjectsV2 page holds at most - // 1000 keys, which is exactly the S3 batch-delete limit, so one call per page suffices - // instead of one round-trip per object. - await client.DeleteObjectsAsync( - new DeleteObjectsRequest - { - BucketName = _options.BucketName, - Objects = info.S3Objects.Select(o => new KeyVersion { Key = o.Key }).ToList() - }, - cancellationToken); + // ponytail: one DeleteObjectAsync call per key instead of a single batch + // DeleteObjectsAsync for the page. The batch API requires a Content-MD5 request-body + // checksum, which the SDK's flexible-checksum pipeline won't auto-compute (MD5 is + // blocked as "unsupported") and Minio won't accept a substitute algorithm for — so + // batch delete cannot work against Minio on this SDK version. Upgrade path: switch + // back to DeleteObjectsAsync once the SDK supports precalculated Content-MD5 or Minio + // accepts a modern checksum trailer for Multi-Object Delete. + await Task.WhenAll(info.S3Objects.Select(o => + client.DeleteObjectAsync(_options.BucketName, o.Key, cancellationToken))); } while (true); await client.DeleteObjectAsync(_options.BucketName, folderLocation, cancellationToken); @@ -165,7 +166,7 @@ private void Dispose(bool disposing) BlobRequest blob, CancellationToken cancellationToken = default) { - var location = GetBlobLocation(blob); + var location = GetBlobLocation(blob).TrimStart('/'); var client = await GetS3ClientAsync(cancellationToken); try { @@ -204,7 +205,7 @@ public override async Task GetPublicAccessUrl( TimeSpan? expiresFromNow = null, CancellationToken cancellationToken = default) { - var location = GetBlobLocation(blob); + var location = GetBlobLocation(blob).TrimStart('/'); var client = await GetS3ClientAsync(cancellationToken); var request = new GetPreSignedUrlRequest { @@ -274,7 +275,7 @@ await _client.PutBucketAsync( BlobRequest blob, [EnumeratorCancellation] CancellationToken cancellationToken = default) { - var location = GetBlobLocation(blob); + var location = GetBlobLocation(blob).TrimStart('/'); var client = await GetS3ClientAsync(cancellationToken); var info = await client.ListObjectsV2Async( @@ -321,7 +322,7 @@ public override async Task SaveAsync(BlobDetails.BlobData blob, if (existed && !blob.Overwrite) throw new InvalidOperationException($"File {blob.Name} is not allowed to override."); - var location = GetBlobLocation(blob); + var location = GetBlobLocation(blob).TrimStart('/'); var client = await GetS3ClientAsync(cancellationToken); var uploadRequest = new PutObjectRequest diff --git a/src/Services/DKNet.Svc.Encryption/HmacHashing.cs b/src/Services/DKNet.Svc.Encryption/HmacHashing.cs index befe363d..b437fc1c 100644 --- a/src/Services/DKNet.Svc.Encryption/HmacHashing.cs +++ b/src/Services/DKNet.Svc.Encryption/HmacHashing.cs @@ -41,7 +41,7 @@ public interface IHmacHashing : IDisposable /// The secret key to use for hashing. /// The expected hash signature to compare against. /// If true, the signature is base64-encoded; otherwise, hexadecimal. - /// If true, ignores case when comparing signatures. + /// Has no effect on the result: the comparison always uses on the decoded signature bytes, so signatures are compared exactly regardless of this flag. /// true if the computed hash matches the expected signature; otherwise, false. bool VerifySha256( string message, @@ -57,7 +57,7 @@ bool VerifySha256( /// The secret key to use for hashing. /// The expected hash signature to compare against. /// If true, the signature is base64-encoded; otherwise, hexadecimal. - /// If true, ignores case when comparing signatures. + /// Has no effect on the result: the comparison always uses on the decoded signature bytes, so signatures are compared exactly regardless of this flag. /// true if the computed hash matches the expected signature; otherwise, false. bool VerifySha512( string message, @@ -78,7 +78,7 @@ public sealed class HmacHashing : IHmacHashing private readonly Dictionary<(HmacAlgorithm alg, string key), HMAC> _cache = []; private readonly Lock _sync = new(); - private bool _disposed; + private volatile bool _disposed; #endregion diff --git a/src/Services/DKNet.Svc.Encryption/ShaHashing.cs b/src/Services/DKNet.Svc.Encryption/ShaHashing.cs index 2223b990..5b8ad88a 100644 --- a/src/Services/DKNet.Svc.Encryption/ShaHashing.cs +++ b/src/Services/DKNet.Svc.Encryption/ShaHashing.cs @@ -44,7 +44,7 @@ public interface IShaHashing : IDisposable // now disposable so we can release c /// /// The input text to hash and compare. /// The expected hexadecimal hash string. - /// If true performs a case-insensitive comparison. + /// Has no effect on the result: hex decoding is case-insensitive, so the comparison result is unaffected by this flag. /// true if the computed hash equals ; otherwise false. bool VerifySha256(string input, string expectedHex, bool ignoreCase = true); @@ -53,7 +53,7 @@ public interface IShaHashing : IDisposable // now disposable so we can release c /// /// The input text to hash and compare. /// The expected hexadecimal hash string. - /// If true performs a case-insensitive comparison. + /// Has no effect on the result: hex decoding is case-insensitive, so the comparison result is unaffected by this flag. /// true if the computed hash equals ; otherwise false. bool VerifySha512(string input, string expectedHex, bool ignoreCase = true); @@ -69,7 +69,7 @@ public sealed class ShaHashing : IShaHashing private readonly Dictionary _algorithms = []; private readonly object _sync = new(); // changed to object for locking - private bool _disposed; + private volatile bool _disposed; #endregion diff --git a/src/Services/Svc.BlobStorage.Tests/BlobServiceSaveAsyncTests.cs b/src/Services/Svc.BlobStorage.Tests/BlobServiceSaveAsyncTests.cs index 2d61d62d..e0b24bc1 100644 --- a/src/Services/Svc.BlobStorage.Tests/BlobServiceSaveAsyncTests.cs +++ b/src/Services/Svc.BlobStorage.Tests/BlobServiceSaveAsyncTests.cs @@ -77,9 +77,9 @@ public async Task S3_SaveAsync_DisallowedExtension_ShouldThrowFileLoadException( using var fixture = new S3BlobServiceFixture(); var options = new S3Options { - ConnectionString = "https://c4bf6253a59daf70a445861c23b45778.r2.cloudflarestorage.com", - AccessKey = "c5240e9de9fb8f2b24d67315eed90737", - Secret = "df8dc0fe841d98c8c8429e3fbe5a6e0e784865e835860b4ffeb65913d7e7346b", + ConnectionString = "https://fake-test-account.example.com", + AccessKey = "FAKEACCESSKEYFORTESTS00", + Secret = "FAKESECRETKEYFORTESTSONLY0000000000000000000000", BucketName = "dev", DisablePayloadSigning = true, IncludedExtensions = [".txt"] @@ -97,9 +97,9 @@ public async Task S3_SaveAsync_OversizedFile_ShouldThrowFileLoadException() using var fixture = new S3BlobServiceFixture(); var options = new S3Options { - ConnectionString = "https://c4bf6253a59daf70a445861c23b45778.r2.cloudflarestorage.com", - AccessKey = "c5240e9de9fb8f2b24d67315eed90737", - Secret = "df8dc0fe841d98c8c8429e3fbe5a6e0e784865e835860b4ffeb65913d7e7346b", + ConnectionString = "https://fake-test-account.example.com", + AccessKey = "FAKEACCESSKEYFORTESTS00", + Secret = "FAKESECRETKEYFORTESTSONLY0000000000000000000000", BucketName = "dev", DisablePayloadSigning = true, MaxFileSizeInMb = 1 @@ -118,9 +118,9 @@ public async Task S3_SaveAsync_FileNameTooLong_ShouldThrowFileLoadException() using var fixture = new S3BlobServiceFixture(); var options = new S3Options { - ConnectionString = "https://c4bf6253a59daf70a445861c23b45778.r2.cloudflarestorage.com", - AccessKey = "c5240e9de9fb8f2b24d67315eed90737", - Secret = "df8dc0fe841d98c8c8429e3fbe5a6e0e784865e835860b4ffeb65913d7e7346b", + ConnectionString = "https://fake-test-account.example.com", + AccessKey = "FAKEACCESSKEYFORTESTS00", + Secret = "FAKESECRETKEYFORTESTSONLY0000000000000000000000", BucketName = "dev", DisablePayloadSigning = true, MaxFileNameLength = 5 @@ -135,16 +135,10 @@ public async Task S3_SaveAsync_FileNameTooLong_ShouldThrowFileLoadException() [Fact] public async Task S3_SaveAsync_DefaultOptions_ShouldSucceed() { + // Unlike the validation-rejection tests above, this one actually saves — it needs a live + // backend, so it goes through the Minio-backed fixture instead of a placeholder S3Options. using var fixture = new S3BlobServiceFixture(); - var options = new S3Options - { - ConnectionString = "https://c4bf6253a59daf70a445861c23b45778.r2.cloudflarestorage.com", - AccessKey = "c5240e9de9fb8f2b24d67315eed90737", - Secret = "df8dc0fe841d98c8c8429e3fbe5a6e0e784865e835860b4ffeb65913d7e7346b", - BucketName = "dev", - DisablePayloadSigning = true - }; - var service = new S3BlobService(Options.Create(options), NullLogger.Instance); + var service = fixture.Service; var blobData = new BlobDetails.BlobData("test.txt", BinaryData.FromString("test")) { Overwrite = true }; var result = await service.SaveAsync(blobData); @@ -217,10 +211,10 @@ public async Task S3_GetItemAsync_ShouldWork() // Save await service.SaveAsync(blobData); - // GetItem + // GetItem — S3 keys carry no leading slash (see S3BlobService's GetBlobLocation trim) var item = await service.GetItemAsync(new BlobRequest(blobName)); item.ShouldNotBeNull(); - item!.Name.ShouldBe($"/{blobName}"); + item!.Name.ShouldBe(blobName); // Cleanup await service.DeleteAsync(new BlobRequest(blobName)); diff --git a/src/Services/Svc.BlobStorage.Tests/Fixtures/S3BlobServiceFixture.cs b/src/Services/Svc.BlobStorage.Tests/Fixtures/S3BlobServiceFixture.cs index 8148afba..b58e00e7 100644 --- a/src/Services/Svc.BlobStorage.Tests/Fixtures/S3BlobServiceFixture.cs +++ b/src/Services/Svc.BlobStorage.Tests/Fixtures/S3BlobServiceFixture.cs @@ -20,18 +20,26 @@ public S3BlobServiceFixture() _minioContainer.StartAsync().GetAwaiter().GetResult(); + Options = new S3Options + { + ConnectionString = _minioContainer.GetConnectionString(), + AccessKey = _minioContainer.GetAccessKey(), + Secret = _minioContainer.GetSecretKey(), + BucketName = "dev", + DisablePayloadSigning = false, + ForcePathStyle = true + }; + var config = new ConfigurationBuilder() .AddInMemoryCollection( new Dictionary(StringComparer.OrdinalIgnoreCase) { - { - "BlobService:S3:ConnectionString", - "https://c4bf6253a59daf70a445861c23b45778.r2.cloudflarestorage.com" - }, - { "BlobService:S3:AccessKey", "c5240e9de9fb8f2b24d67315eed90737" }, - { "BlobService:S3:Secret", "df8dc0fe841d98c8c8429e3fbe5a6e0e784865e835860b4ffeb65913d7e7346b" }, - { "BlobService:S3:BucketName", "dev" }, - { "BlobService:S3:DisablePayloadSigning", "true" } + { "BlobService:S3:ConnectionString", Options.ConnectionString }, + { "BlobService:S3:AccessKey", Options.AccessKey }, + { "BlobService:S3:Secret", Options.Secret }, + { "BlobService:S3:BucketName", Options.BucketName }, + { "BlobService:S3:DisablePayloadSigning", "false" }, + { "BlobService:S3:ForcePathStyle", "true" } }) .Build(); @@ -49,6 +57,12 @@ public S3BlobServiceFixture() public IBlobService Service { get; } + /// + /// The options this fixture's Minio container was configured with — exposed so tests can construct + /// their own instance directly (e.g. to exercise Dispose()). + /// + public S3Options Options { get; } + #endregion #region Methods diff --git a/src/Services/Svc.BlobStorage.Tests/S3BlobServiceTest.cs b/src/Services/Svc.BlobStorage.Tests/S3BlobServiceTest.cs index a811f1b0..c7adf5c4 100644 --- a/src/Services/Svc.BlobStorage.Tests/S3BlobServiceTest.cs +++ b/src/Services/Svc.BlobStorage.Tests/S3BlobServiceTest.cs @@ -1,4 +1,7 @@ using System.Text; +using Microsoft.Extensions.Logging.Abstractions; +using Microsoft.Extensions.Options; +using DKNet.Svc.BlobStorage.AwsS3; using Svc.BlobStorage.Tests.Fixtures; namespace Svc.BlobStorage.Tests; @@ -168,5 +171,49 @@ public async Task SavesFileAndList() items.Count.ShouldBeGreaterThanOrEqualTo(1); } + [Fact] + public async Task DeleteAsyncDeletesDirectoryWithMultipleKeys() + { + // Regression test for the per-key DeleteFolderAsync rewrite (one DeleteObjectAsync call per + // key instead of a single batch DeleteObjectsAsync) — confirms it drops none of several keys. + var dir = $"delete-dir-multi-{Guid.NewGuid()}"; + for (var i = 0; i < 5; i++) + { + var blob = new BlobDetails.BlobData($"{dir}/file{i}.txt", new BinaryData("bye"u8.ToArray())) + { Overwrite = true, Type = BlobTypes.File }; + await _service.SaveAsync(blob); + } + + var deleted = await _service.DeleteAsync(new BlobRequest(dir) { Type = BlobTypes.Directory }); + deleted.ShouldBeTrue(); + + var items = new List(); + await foreach (var item in _service.ListItemsAsync(new BlobRequest(dir) { Type = BlobTypes.Directory })) + items.Add(item); + + items.ShouldBeEmpty(); + } + + [Fact] + public async Task DeleteAsyncDeletesEmptyDirectoryWithoutThrowing() + { + var dir = $"delete-dir-empty-{Guid.NewGuid()}"; + + var deleted = await _service.DeleteAsync(new BlobRequest(dir) { Type = BlobTypes.Directory }); + deleted.ShouldBeTrue(); + } + + [Fact] + public async Task DisposeReleasesUnderlyingClientAndIsIdempotent() + { + var service = new S3BlobService(Options.Create(fixture.Options), NullLogger.Instance); + + // Force the lazy AmazonS3Client to be created before disposing it. + await service.CheckExistsAsync(new BlobRequest($"dispose-check-{Guid.NewGuid()}.txt")); + + Should.NotThrow(service.Dispose); + Should.NotThrow(service.Dispose); // second call must be a no-op, not re-dispose a null client + } + #endregion } \ No newline at end of file diff --git a/src/Services/Svc.Encryption.Tests/HmacHashingDisposeTests.cs b/src/Services/Svc.Encryption.Tests/HmacHashingDisposeTests.cs new file mode 100644 index 00000000..f1d9ec4b --- /dev/null +++ b/src/Services/Svc.Encryption.Tests/HmacHashingDisposeTests.cs @@ -0,0 +1,45 @@ +using System; +using DKNet.Svc.Encryption; +using Shouldly; +using Xunit; + +namespace Svc.Encryption.Tests; + +public class HmacHashingDisposeTests +{ + #region Methods + + [Fact] + public void HmacHashing_AfterDispose_ThrowsObjectDisposedException_OnComputeSha256() + { + var hmac = new HmacHashing(); + hmac.Dispose(); + Should.Throw(() => hmac.ComputeSha256("message", "key")); + } + + [Fact] + public void HmacHashing_AfterDispose_ThrowsObjectDisposedException_OnComputeSha512() + { + var hmac = new HmacHashing(); + hmac.Dispose(); + Should.Throw(() => hmac.ComputeSha512("message", "key")); + } + + [Fact] + public void HmacHashing_AfterDispose_ThrowsObjectDisposedException_OnVerifySha256() + { + var hmac = new HmacHashing(); + hmac.Dispose(); + Should.Throw(() => hmac.VerifySha256("message", "key", "sig")); + } + + [Fact] + public void HmacHashing_AfterDispose_ThrowsObjectDisposedException_OnVerifySha512() + { + var hmac = new HmacHashing(); + hmac.Dispose(); + Should.Throw(() => hmac.VerifySha512("message", "key", "sig")); + } + + #endregion +} diff --git a/src/Services/Svc.Encryption.Tests/HmacHashingIgnoreCaseTests.cs b/src/Services/Svc.Encryption.Tests/HmacHashingIgnoreCaseTests.cs new file mode 100644 index 00000000..42f32b08 --- /dev/null +++ b/src/Services/Svc.Encryption.Tests/HmacHashingIgnoreCaseTests.cs @@ -0,0 +1,48 @@ +using System; +using System.Linq; +using DKNet.Svc.Encryption; +using Shouldly; +using Xunit; + +namespace Svc.Encryption.Tests; + +public class HmacHashingIgnoreCaseTests +{ + #region Methods + + [Fact] + public void HmacHashing_VerifySha256_IgnoreCaseTrueOrFalse_ReturnsSameResult() + { + using var hmac = new HmacHashing(); + var message = "test message"; + var secretKey = "secret"; + var sig = hmac.ComputeSha256(message, secretKey, false); // hex signature + + var mixedCaseSig = new string([.. sig.Select((c, i) => i % 2 == 0 ? char.ToUpper(c) : char.ToLower(c))]); + + var rTrue = hmac.VerifySha256(message, secretKey, mixedCaseSig, false, true); + var rFalse = hmac.VerifySha256(message, secretKey, mixedCaseSig, false, false); + + rTrue.ShouldBe(rFalse); + rTrue.ShouldBeTrue(); + } + + [Fact] + public void HmacHashing_VerifySha512_IgnoreCaseTrueOrFalse_ReturnsSameResult() + { + using var hmac = new HmacHashing(); + var message = "test message"; + var secretKey = "secret"; + var sig = hmac.ComputeSha512(message, secretKey, false); // hex signature + + var mixedCaseSig = new string([.. sig.Select((c, i) => i % 2 == 0 ? char.ToUpper(c) : char.ToLower(c))]); + + var rTrue = hmac.VerifySha512(message, secretKey, mixedCaseSig, false, true); + var rFalse = hmac.VerifySha512(message, secretKey, mixedCaseSig, false, false); + + rTrue.ShouldBe(rFalse); + rTrue.ShouldBeTrue(); + } + + #endregion +} diff --git a/src/Services/Svc.Encryption.Tests/ShaHashingDisposeTests.cs b/src/Services/Svc.Encryption.Tests/ShaHashingDisposeTests.cs new file mode 100644 index 00000000..8b38734d --- /dev/null +++ b/src/Services/Svc.Encryption.Tests/ShaHashingDisposeTests.cs @@ -0,0 +1,45 @@ +using System; +using DKNet.Svc.Encryption; +using Shouldly; +using Xunit; + +namespace Svc.Encryption.Tests; + +public class ShaHashingDisposeTests +{ + #region Methods + + [Fact] + public void ShaHashing_AfterDispose_ThrowsObjectDisposedException_OnComputeSha256() + { + var sha = new ShaHashing(); + sha.Dispose(); + Should.Throw(() => sha.ComputeSha256("input")); + } + + [Fact] + public void ShaHashing_AfterDispose_ThrowsObjectDisposedException_OnComputeSha512() + { + var sha = new ShaHashing(); + sha.Dispose(); + Should.Throw(() => sha.ComputeSha512("input")); + } + + [Fact] + public void ShaHashing_AfterDispose_ThrowsObjectDisposedException_OnVerifySha256() + { + var sha = new ShaHashing(); + sha.Dispose(); + Should.Throw(() => sha.VerifySha256("input", "expected")); + } + + [Fact] + public void ShaHashing_AfterDispose_ThrowsObjectDisposedException_OnVerifySha512() + { + var sha = new ShaHashing(); + sha.Dispose(); + Should.Throw(() => sha.VerifySha512("input", "expected")); + } + + #endregion +} diff --git a/src/Services/Svc.Encryption.Tests/ShaHashingIgnoreCaseTests.cs b/src/Services/Svc.Encryption.Tests/ShaHashingIgnoreCaseTests.cs new file mode 100644 index 00000000..2623b288 --- /dev/null +++ b/src/Services/Svc.Encryption.Tests/ShaHashingIgnoreCaseTests.cs @@ -0,0 +1,46 @@ +using System; +using System.Linq; +using DKNet.Svc.Encryption; +using Shouldly; +using Xunit; + +namespace Svc.Encryption.Tests; + +public class ShaHashingIgnoreCaseTests +{ + #region Methods + + [Fact] + public void ShaHashing_VerifySha256_IgnoreCaseTrueOrFalse_ReturnsSameResult() + { + using var sha = new ShaHashing(); + var message = "test message"; + var hex = sha.ComputeSha256(message); + + var mixedCaseHex = new string([.. hex.Select((c, i) => i % 2 == 0 ? char.ToUpper(c) : char.ToLower(c))]); + + var rTrue = sha.VerifySha256(message, mixedCaseHex, true); + var rFalse = sha.VerifySha256(message, mixedCaseHex, false); + + rTrue.ShouldBe(rFalse); + rTrue.ShouldBeTrue(); + } + + [Fact] + public void ShaHashing_VerifySha512_IgnoreCaseTrueOrFalse_ReturnsSameResult() + { + using var sha = new ShaHashing(); + var message = "test message"; + var hex = sha.ComputeSha512(message); + + var mixedCaseHex = new string([.. hex.Select((c, i) => i % 2 == 0 ? char.ToUpper(c) : char.ToLower(c))]); + + var rTrue = sha.VerifySha512(message, mixedCaseHex, true); + var rFalse = sha.VerifySha512(message, mixedCaseHex, false); + + rTrue.ShouldBe(rFalse); + rTrue.ShouldBeTrue(); + } + + #endregion +}