[DRK-75] Atomic idempotency key reservation

src/AspNet/AspCore.Idempotency.MsSqlStore.Tests/AspCore.Idempotency.MsSqlStore.Tests.csproj

@@ -20,6 +20,9 @@
           <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
         </PackageReference>
         <PackageReference Include="Microsoft.AspNetCore.Mvc.Testing"/>
+        <PackageReference Include="Microsoft.EntityFrameworkCore.Sqlite"/>
+        <!-- Force fixed SQLitePCLRaw (EF Sqlite pulls vulnerable 2.1.11, GHSA-2m69-gcr7-jv3q) -->
+        <PackageReference Include="SQLitePCLRaw.bundle_e_sqlite3"/>
         <PackageReference Include="Microsoft.NET.Test.Sdk"/>
         <PackageReference Include="Shouldly"/>
         <PackageReference Include="Testcontainers.MsSql"/>

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();

src/AspNet/AspCore.Idempotency.MsSqlStore.Tests/Unit/IdempotencySqlServerStoreConcurrencyTests.cs

@@ -0,0 +1,121 @@
+// <copyright file="IdempotencySqlServerStoreConcurrencyTests.cs" company="https://drunkcoding.net">
+// Copyright (c) 2025 Steven Hoang. All rights reserved.
+// Licensed under the MIT License. See LICENSE in the project root for license information.
+// </copyright>
+
+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;
+
+/// <summary>
+///     Proves the atomic reservation in <see cref="IdempotencySqlServerStore.IsKeyProcessedAsync" /> 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 <c>UX_CompositeKey</c> across concurrent contexts the
+///     same way a real relational database does — using the exact same <see cref="IdempotencyKeyConfiguration" />
+///     the SQL Server store ships with. Mirrors the retired HTTP-level
+///     <c>CreateItem_ConcurrentRequestsWithSameKey_OnlyOneProcessed</c> test (see DRK-174), but directly at the
+///     store layer so it doesn't depend on SQL Server at all.
+/// </summary>
+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<IdempotencyDbContext>(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<IModelCustomizer, SqliteCompatibleModelCustomizer>());
+        _serviceProvider = services.BuildServiceProvider();
+
+        var factory = _serviceProvider.GetRequiredService<IDbContextFactory<IdempotencyDbContext>>();
+        await using var setupContext = await factory.CreateDbContextAsync();
+        await setupContext.Database.EnsureCreatedAsync();
+
+        _store = new IdempotencySqlServerStore(
+            _serviceProvider,
+            Options.Create(new IdempotencyOptions()),
+            NullLogger<IdempotencySqlServerStore>.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
+}
+
+/// <summary>
+///     Applies <see cref="IdempotencyKeyConfiguration" /> as-is, then drops the one column-type override in it
+///     that is SQL-Server-specific raw SQL, so <c>EnsureCreatedAsync</c> can generate valid SQLite DDL.
+/// </summary>
+internal sealed class SqliteCompatibleModelCustomizer(ModelCustomizerDependencies dependencies)
+    : ModelCustomizer(dependencies)
+{
+    public override void Customize(ModelBuilder modelBuilder, DbContext context)
+    {
+        base.Customize(modelBuilder, context);
+
+        var key = modelBuilder.Entity<IdempotencyKeyEntity>();
+        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);
+    }
+}

src/AspNet/AspCore.Idempotency.MsSqlStore.Tests/Unit/IdempotencySqlServerStoreLifecycleTests.cs

@@ -0,0 +1,203 @@
+// <copyright file="IdempotencySqlServerStoreLifecycleTests.cs" company="https://drunkcoding.net">
+// Copyright (c) 2025 Steven Hoang. All rights reserved.
+// Licensed under the MIT License. See LICENSE in the project root for license information.
+// </copyright>
+
+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;
+
+/// <summary>
+///     Covers the reserve/complete lifecycle and the collision-handler branches of
+///     <see cref="IdempotencySqlServerStore" /> that <see cref="IdempotencySqlServerStoreConcurrencyTests" />
+///     doesn't exercise. Uses the same file-based SQLite setup so it runs without Docker/SQL Server.
+/// </summary>
+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<IdempotencyDbContext>(o => o.UseSqlite(
+                $"Data Source={_dbFilePath}",
+                sqlite => sqlite.MigrationsAssembly(
+                    typeof(IdempotencySqlServerStoreConcurrencyTests).Assembly.GetName().Name))
+            .ReplaceService<IModelCustomizer, SqliteCompatibleModelCustomizer>());
+        _serviceProvider = services.BuildServiceProvider();
+
+        var factory = _serviceProvider.GetRequiredService<IDbContextFactory<IdempotencyDbContext>>();
+        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<IdempotencySqlServerStore>.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<IDbContextFactory<IdempotencyDbContext>>();
+        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
+}

src/AspNet/AspCore.Idempotency.Tests/AspCore.Idempotency.Tests.csproj

@@ -19,6 +19,7 @@
         <PackageReference Include="xunit"/>
         <PackageReference Include="xunit.runner.visualstudio"/>
         <PackageReference Include="Shouldly"/>
+        <PackageReference Include="coverlet.collector"/>
     </ItemGroup>
 
     <ItemGroup>