Conversation
…Console in library code Library code must emit diagnostics through an injected ILogger, never System.Console, so hosts can filter/redirect/redact/suppress them. Adds a NetArchTest rule over the DKNet.EfCore.Extensions assembly with a KnownViolations allow-list containing exactly today's single offender (EfCoreExceptionHandler, EfCoreExceptionHandler.cs:61). The rule fails on any NEW Console usage; the allow-list must only shrink. A companion self-check asserts the rule can still detect the known offender, so it can never silently go blind. Test-only, no production code touched. Part of monthly architecture review DRK-73. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Co-authored-by: multica-agent <github@multica.ai>
IsKeyProcessedAsync now atomically reserves a composite key (HTTP 102 placeholder) before returning, so concurrent requests with the same idempotency key can no longer all pass the check before any of them completes. The SQL store enforces this via UX_CompositeKey; the distributed-cache store narrows (documented, not eliminated) the same race since IDistributedCache has no compare-and-set primitive. Closes architecture finding IDEM-CONCURRENCY-001 (DRK-75). Co-authored-by: multica-agent <github@multica.ai>
- HmacHashing and ShaHashing _disposed fields are now volatile so a Dispose() on one thread is deterministically visible to a Compute/ Verify/Dispose read on another - IHmacHashing/IShaHashing VerifySha256/VerifySha512 ignoreCase param docs corrected: the flag has no effect on the comparison result Co-authored-by: multica-agent <github@multica.ai>
…creds S3BlobServiceFixture now reads connection string/access key/secret from the MinioContainer it already starts, instead of hardcoding live-looking Cloudflare R2 credentials. Adds ForcePathStyle (required for Minio's IP:port endpoint) and disables payload signing over plain HTTP. That surfaced two real S3BlobService bugs only exercised now that a path-style endpoint is in play: - GetBlobLocation()'s leading slash collided with path-style addressing (bucket//key), breaking SigV4 signatures on every request. Trimmed it at each call site; GetBlobLocation() itself is unchanged (locked contract test covers Azure/Local too). - Batch DeleteObjects requires a Content-MD5 the SDK's checksum pipeline won't auto-compute and Minio won't accept a substitute for; switched folder delete to per-key DeleteObjectAsync calls. Also replaces the same leaked R2 endpoint/key/secret literals (inert, pre-network-validation only) in BlobServiceSaveAsyncTests.cs with fake placeholders, and rewires its one test that actually saves data through the Minio-backed fixture instead of the now-fake endpoint. DisablePayloadSigning left as false is a deliberate deviation from the original plan (true fails immediately over Minio's plain-HTTP endpoint). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Co-authored-by: multica-agent <github@multica.ai>
…ing and ShaHashing Co-authored-by: multica-agent <github@multica.ai>
[DRK-114] Harden DKNet.Svc.Encryption hashing: _disposed visibility + ignoreCase no-op docs
…older delete - Fixture now exposes its S3Options so tests can construct their own S3BlobService instance (needed to exercise Dispose without going through the DI-registered IBlobService). - Adds DisposeReleasesUnderlyingClientAndIsIdempotent — the only gap behind S3BlobService.cs's 52.94% class-level coverage was Dispose()/Dispose(bool) never being invoked. - Adds DeleteAsyncDeletesDirectoryWithMultipleKeys and DeleteAsyncDeletesEmptyDirectoryWithoutThrowing to sanity-check the per-key DeleteFolderAsync rewrite (D135-2 checklist item). S3BlobService.cs line coverage: 52.94% -> 94.83%. Full suite: 120/120 green. Co-authored-by: multica-agent <github@multica.ai>
Sibling test projects already reference it; this project was missing it, so coverage % couldn't be measured for touched non-SQL-Server classes. Co-authored-by: multica-agent <github@multica.ai>
[DRK-135] Remove hardcoded R2 credentials from S3 blob-storage test fixture
Co-authored-by: multica-agent <github@multica.ai>
CreateItem_ConcurrentRequestsWithSameKey_OnlyOneProcessed (the only test that ever proved the atomic reservation under real concurrency) is permanently [Skip]d since bf50729 retired the SQL Server TestContainers path. Add IdempotencySqlServerStoreConcurrencyTests, exercising IdempotencySqlServerStore.IsKeyProcessedAsync directly against a file-based SQLite IdempotencyDbContext (not InMemory - needs real unique index enforcement across concurrent connections) using the same IdempotencyKeyConfiguration the SQL Server store ships with. Fires 5 concurrent reservation attempts for an identical composite key and asserts exactly one wins - mirroring the retired HTTP-level test, but at the store layer so it needs no Docker/SQL Server. Two Sqlite-provider incompatibilities in the shared configuration needed a test-local workaround (no production code touched): - Body's raw HasColumnType("nvarchar(max)") isn't valid SQLite syntax. - The Sqlite provider can't translate > /< on a DateTimeOffset column (only equality), which IsKeyProcessedAsync's expiry check relies on. Both are patched via a test-only IModelCustomizer that strips the column-type override and stores ExpiresAt as UTC ticks instead. Verified: reverting IsKeyProcessedAsync to the pre-fix check-then-act shape (commit b85e224's parent) makes this test fail (5/5 callers see (false, null) instead of 1/5) - confirming it would have caught DRK-75's original race. Restored, it passes cleanly, 10/10 repeated runs, and dotnet build/format come back clean. Refs DRK-176, DRK-174, DRK-75 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Co-authored-by: multica-agent <github@multica.ai>
- IsKeyProcessedAsync now atomically reserves the composite key by inserting a StatusCode=102 placeholder row, relying on the existing UX_CompositeKey unique index to serialize concurrent callers (only the winner proceeds to run the protected handler). - On insert collision, the blocking row is re-queried: an unexpired completed row replays its cached response, an unexpired reservation returns the existing 409/conflict path, and an expired row (stale reservation or completed entry nothing purges) lets the request proceed as new instead of permanently blocking that key. - MarkKeyAsProcessedAsync now completes the tracked reservation row in place via the new IdempotencyKeyEntity.Complete(...) instead of blindly inserting a second row. - Add IdempotencyOptions.InFlightReservationTimeout (default 30s) controlling how long a reservation is honoured before being treated as abandoned (R1). - Tighten the concurrency integration test to assert the handler ran exactly once (identical Id across every 201), and add a new test proving an expired in-flight reservation does not permanently block retries. DRK-175 / IDEM-CONCURRENCY-001 Co-authored-by: multica-agent <github@multica.ai>
… an expired row Adds CreateItem_ConcurrentRequestsAgainstExpiredReservation_OnlyOneProcessed. Seeds an already-expired StatusCode=102 reservation, then fires 5 concurrent requests for the same key. All 5 requests miss the unexpired-row filter and collide on the same stale row's unique-index INSERT; ReserveKeyAsync's collision branch returns (false, null) for every one of them when the blocking row is expired, so all 5 proceed to run the handler. FAILS on 4802a1b: 5 distinct handler executions observed instead of 1. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Co-authored-by: multica-agent <github@multica.ai>
…e residual race ReserveKeyAsync's expired-collision branch returned (false, null) to every concurrent caller that collided with the same expired row, so none of them actually won the reservation - reopening the exact double-execution race DRK-175 closed for the fresh-key path. Replace the unconditional return with a conditional UPDATE (ExecuteUpdateAsync) that only matches while the row is still expired, giving the same single-winner guarantee the unique index gives the fresh-insert path. A caller whose UPDATE affects zero rows lost the race and re-reads the row to branch like the unexpired collision path. Fixes DRK-182. Co-authored-by: multica-agent <github@multica.ai>
[DRK-175] Atomic idempotency key reservation (PostgreSQL store)
…r ≥90% diff coverage - SQL store: full lifecycle (reserve→complete→replay), in-flight recheck, collision re-query returning a completed response, expired-reservation collision allowing a fresh reservation, and the defensive entity-is-null fallback in MarkKeyAsProcessedAsync - Distributed cache store: reservation placeholder write/in-flight recheck, reservation→complete replay, and InFlightReservationTimeout expiry - IdempotencyOptions: default and custom InFlightReservationTimeout Addresses DRK-183 (PR #340 review, coverage gap 53.65% -> local run shows IdempotencySqlServerStore.cs and IdempotencyDistributedCacheStore.cs fully exercised) Co-authored-by: multica-agent <github@multica.ai>
Merging origin/dev (which now carries DRK-175's independently-added identical property) produced no textual conflict but a duplicate declaration (CS0102) since both branches added the same TimeSpan property with the same default. Kept dev's canonical declaration, removed this branch's earlier duplicate. Co-authored-by: multica-agent <github@multica.ai>
…-check [DRK-75] Atomic idempotency key reservation
[DRK-73] test(efcore-extensions): Tier-2 arch rule — no System.Console in library code
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Enhancement