Repository navigation
Conversation
The standard resilience handler was registered for every HttpClient and retried the non-idempotent bank POST on 5xx/timeouts, so one payment could be charged twice. The bank client is now a typed HttpClient with its own handler, retries disabled for unsafe methods; timeouts and the circuit breaker stay. The expiry was sent as "4/2027" instead of the bank's "MM/yyyy". The mapping moves to BankHttpClient.ToBankRequest with unit tests, and the WireMock stubs now require the format so the simulator enforces the contract. Adds audit.md. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…nswer An expiry year outside DateTime's range made the validator throw, so the merchant got a 500 instead of a rejection; year and month are now compared directly, and an expiry more than 20 years ahead is rejected too. When the bank's body could not be parsed, the processor returned a response with only Status set and BankService persisted it with a null hash key. The processor now returns null and BankService answers InternalError without persisting. Both pinned "NOTE: defect" tests become assertions of the fix. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ages
Directory.Build.props sets the shared target framework, nullable and usings
settings, latest-recommended analyzers, code-style enforcement in build and
warnings-as-errors. .editorconfig encodes the conventions AGENTS.md only
described ("this." on fields, System usings first, no unused usings), and
Directory.Packages.props gives one place to bump versions (AWSSDK.DynamoDBv2
4.0.104, Moq 4.20.72). .gitattributes pins LF line endings.
Fixes what the analyzers found: logging moves to [LoggerMessage] source-
generated methods (CA1848), unused usings are removed, IPaymentsRepository.Get
becomes Find (CA1716), and the BOM and missing final newlines are gone.
The Docker build context moves to the repository root so the image sees the
shared props files.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ation Bank and DynamoDB settings bind to BankOptions / DynamoOptions, validated at startup, instead of IConfiguration lookups that fell back to an empty URI at request time; the table name is no longer hard-coded twice. The typed bank client gets its base address from the options, and its timeouts are explicit under Bank:Resilience. Local-only values move to appsettings.Development.json. The validator takes a TimeProvider and compares in UTC instead of receiving the server's local DateTime.Now from the controller; tests pin a FakeTimeProvider. Every async interface takes a CancellationToken, flowed from the action. The payment POST honours it only until the bank is called: cancelling after that would leave the bank's outcome unknown, so the bank call and the write that records its answer run to completion. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The payment was persisted only after the bank answered, so a failed write lost a charged payment and a bank failure left no record at all. PaymentService (was BankService) now stores the payment as Pending, calls the bank, then records the outcome with a conditional write that only completes a Pending payment. If that last write fails the merchant still gets the real outcome and a Critical log line carries what reconciliation needs. BankClient (was BankHttpClient + BankResponseProcessor) owns the bank contract and returns a BankOutcome instead of leaking HttpResponseMessage through two abstractions; it disposes the response and maps every failure mode: 4xx, 503 and an open circuit are Failed (nothing charged), timeouts, dropped connections, other 5xx and unreadable bodies are Unknown (may have been charged). It is now unit-tested through a stub HttpMessageHandler. The authorization code is stored. Storage gets its own shape, one attribute per field with the status by name, instead of the public response serialised as JSON; PaymentStatus becomes the payment lifecycle (Pending, Authorized, Declined, Failed, Unknown). GET goes through the service and takes a Guid. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ures
The POST answered 200 with a bare id, 400 "Declined" for a valid request the
issuer refused, and 400 "InternalError" when the bank was down, telling the
client it was at fault and should not retry. Now, under api/payments:
- Authorized and Declined are 201 Created with Location and the payment body;
- a bank failure is 502 ProblemDetails carrying paymentId and paymentStatus;
- validation failures are 400 ValidationProblemDetails listing every broken
field, the same shape model binding already used;
- GET takes {paymentId:guid} and answers 404 ProblemDetails;
- unhandled exceptions go through AddExceptionHandler + AddProblemDetails and
no longer leak the exception message; a client abort is not logged as an
error.
The validator returns all errors keyed by JSON field instead of five booleans
that stopped at the first failure. Amounts are integers in minor units,
currencies come from a configured ISO 4217 allow-list (case-insensitive,
normalised to upper case), patterns are [GeneratedRegex] with [0-9] instead
of \d. PaymentId leaves the request model, which becomes an immutable record
whose ToString masks the card number and CVV. Enums serialize as names and
JSON respects nullable annotations.
The E2E suite no longer references the api project: it owns its request
record and asserts on raw JSON, so contract drift fails it.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…JSON
LogMiddleware becomes CorrelationIdMiddleware. It reads X-Correlation-ID
(the standard name, was CorrelationId), accepts it only if it matches
^[A-Za-z0-9-]{1,64}$ so a header cannot inject or bloat log lines, echoes it on
the response and adds it to every ProblemDetails. The free-text CallerId
header is no longer logged; caller identity belongs to authentication.
Serilog moves from a second ConfigurationBuilder over logconfig.json into the
Serilog section of appsettings, so it gets environment overrides and
environment variables. The console writes compact JSON with the correlation,
trace and span ids by default (the old terse console dropped CorrelationId);
Development swaps in a readable format and keeps the z-logs files. A startup
logger and try/catch/finally with CloseAndFlush make startup failures visible
and flush the file sink; UseSerilogRequestLogging adds one summary line per
request.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… image GET api/payment answering "ok" was a business route standing in for a health check that proved nothing about DynamoDB. /health/live reports the process, /health/ready also describes the payments table (3s timeout); the E2E fixture polls the live check. Swashbuckle's generator gives way to Microsoft.AspNetCore.OpenApi (AddOpenApi / MapOpenApi), with Swashbuckle's UI kept for browsing; both are mapped in Development only instead of in every environment. The runtime image becomes aspnet:10.0-noble-chiseled running as its built-in non-root user, and publish uses --no-restore. launchSettings.json loses the unused IIS Express profile and names its profile after the project. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… POST Any caller could read any payment by guessing its id. Merchants now authenticate with an X-Api-Key header; ApiKeyAuthenticationHandler maps the key to a merchant id claim through ApiKeys:Merchants, which holds only the SHA-256 of each key. The authorization fallback policy requires an authenticated merchant everywhere except the health checks and the Development OpenAPI document, so a new endpoint is never public by accident. 401s carry WWW-Authenticate and a ProblemDetails body. Payments store the merchant that created them, and GET answers 404 for another merchant's payment, exactly as for a missing one. POST is rate limited per merchant with a fixed window (RateLimiting section), answering 429 ProblemDetails with Retry-After. Outside Development the app sends HSTS and redirects to HTTPS once an HTTPS port is configured. Log lines after authentication and the request summary carry MerchantId. Local keys: dev-key-merchant-a and dev-key-merchant-b. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Every POST created a new payment, so a merchant retrying after a network blip charged the card again. An optional Idempotency-Key header (visible ASCII, up to 255) is now stored per merchant in a PaymentIdempotencyKeys table, written in the same TransactWriteItems as the pending payment: of two concurrent requests with one key exactly one reaches the bank. A repeat with the same request replays the first outcome (201 or 502, same body) with Idempotent-Replayed: true; while the first is still Pending it is a 409 with the payment id; the same key with a different request is a 422. The request fingerprint hashes the last four digits, expiry, currency and amount, never the CVV or the full card number. Keys expire after Payments:IdempotencyKeyRetention (24h) through DynamoDB TTL, and an expired key may be taken again before the TTL deletes it. PaymentService returns a PaymentAttempt (Processed, Replayed, InProgress, KeyReused) that the controller maps to HTTP; the readiness check covers both tables and the compose init creates the new one with TTL enabled. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The layers that move money and touch I/O, BankClient and PaymentsRepository, were only exercised by an E2E suite that needs the compose stack and runs nowhere automatically. PaymentGatewayApiIntegrationTests hosts the api through WebApplicationFactory<Program>, with DynamoDB Local from Testcontainers and the production bank client and resilience pipeline in front of an in-process stub bank. It proves: - the contract (201 + Location, stored record with authorization code and no full card number, bank request in MM/yyyy, 400/404/401 problems); - the POST is never retried (503, 500, timeout: one bank call each) and each failure is on record as Failed or Unknown; - a failed outcome write still answers the real outcome and leaves Pending plus a Critical reconciliation log; - a client disconnecting mid-call does not cancel the bank call or lose its outcome; concurrent requests with one Idempotency-Key reach the bank once; - per-merchant rate limiting, merchant scoping, and that no log event carries the full card number; - invalid settings stop startup. The startup tests found that ValidateDataAnnotations never looked inside ApiKeys:Merchants; ApiKeyOptions now uses the source-generated [OptionsValidator] with [ValidateEnumeratedItems] and rejects duplicate key hashes. The host owns its Serilog logger (preserveStaticLogger), so hosts sharing a process no longer replace each other's static logger. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was no CI, so the build, the tests and the style rules were claims. The workflow restores, builds in Release with warnings as errors, checks dotnet format, fails on any vulnerable package (transitive included), runs the unit and integration tests with Microsoft.Testing.Extensions.CodeCoverage and publishes the merged ReportGenerator summary to the job summary plus an artifact. A second job brings the compose stack up with --wait and runs the end-to-end suite, dumping the stack's logs if it fails. coverage.settings.xml limits coverage to the api assembly and excludes source-generated code ([LoggerMessage], [GeneratedRegex], the options validator). Dependabot proposes NuGet (grouped), Actions and image updates weekly. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The README was five lines with "add later how to run / debug", typos, and a claim that http retries were omitted when they were not. It now covers the purpose, a quick start (including the AWS credentials dotnet run needs against DynamoDB Local), the three test suites, the API contract with an example, how to steer the bank simulator, a sequence diagram of a payment, the design decisions and trade-offs (no retry on POST, record before the bank, Failed vs Unknown, cancellation, model families, card data, DynamoDB, hashed API keys, no Luhn) and the known limitations (reconciliation, secrets, TLS behind a proxy, infrastructure as code, per-instance rate limits). AGENTS.md gets its own name as heading, both tables, the working bare dotnet test, the new folders, and loses a stale fragment about pinned defect tests. The empty deploy/aws placeholder is removed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Owner
Author
|
too much not neede stuff |
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
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.
No description provided.