Terrence Daniels — Full-Stack .NET Engineer

Why this project

Microsoft's own reference architecture, verified rather than trusted

.NET Aspire microservices, event-driven integration, and a real transactional outbox pattern show up constantly on resumes and rarely get built end-to-end. dotnet/eShop is Microsoft's own teaching reference for exactly this shape — which made it a genuinely useful thing to rebuild file by file rather than fork wholesale: every package version re-researched against what's actually current, every file read closely enough to catch two real bugs sitting in Microsoft's own sample code, and a handful of deliberate design departures (a Decorator split, React instead of Blazor) made and recorded as this fork's own choices, not upstream's.

.NET 10 .NET Aspire 13.4 RabbitMQ EF Core Polly OpenTelemetry MSTest / Microsoft.Testing.Platform NSubstitute GitHub Actions CodeQL

Foundation first, five projects deep

Shared → EventBus → EventBusRabbitMQ → eShop.ServiceDefaults → the transactional outbox, then Identity.API

Migration order is deliberate: shared/foundation projects first, since everything else depends on them. src/Shared/'s two linked-source files, EventBus's abstractions, EventBusRabbitMQ's RabbitMQ implementation, eShop.ServiceDefaults's Aspire telemetry/health-check/resilience defaults, and IntegrationEventLogEF — the EF Core-backed transactional outbox every event-publishing service will write through — are all done and reviewed. Identity.API (Duende IdentityServer) is now in progress. The other 15 projects don't exist on disk yet; see todo.md for the honest current state rather than assuming more is done.

Deliberate deviations, not defaults

A RabbitMQEventBus mixing transport plumbing with telemetry and Polly resilience split into a three-layer Decorator chain once upstream fidelity stopped being the constraint. WebApp is going React, not Blazor — a deliberate job-market call, not a technical necessity — which cascades into a new WebBFF project not in upstream at all. Both decisions, and the reasoning behind them, are recorded in docs/architecturedesign.md.

Verification over assumption

The Polly bug below was confirmed by reflecting the real Polly.Core 8.6.6 assembly, not read off documentation. The MTP test-runner's CLI flags were confirmed against a real scratch project before trusting them in CI. A repo-wide unused-using audit was verified by deliberately planting a known-dead import first and confirming the check actually caught it, before trusting a clean result anywhere else.

Real bugs, not just version bumps

Found by reading closely and verifying against real assemblies, not by assuming upstream is correct

Every entry below was present, verbatim, in the files as they came from Microsoft's own dotnet/eShop repo unless noted otherwise — this isn't a list of typos, it's what a genuine line-by-line review turned up in a widely-referenced Microsoft reference sample.

EventBusRabbitMQ — Polly retry pipeline never actually retried

PublishAsync ran its publish step through _pipeline.Execute(async () => {...}) — the synchronous Execute<TResult> overload with TResult inferred as Task, confirmed by reflecting the real Polly.Core 8.6.6 assembly rather than assumed. Any exception thrown after the lambda's first await — exactly where BrokerUnreachableException would occur — happened after Execute had already returned, so the retry pipeline never observed the failures it was configured to catch. Fixed by moving to the real ExecuteAsync overload as part of a full Decorator split.

high · reliability

eShop.ServiceDefaults — JWT audience validation silently disabled

TokenValidationParameters.ValidateAudience = false meant a token's aud claim was never actually checked, so a token issued for one downstream API could be replayed against any other API using this same authentication code. Removed the override — ValidateAudience defaults to true, and JwtBearerOptions.Audience already auto-populates the expected value. Flagged for re-verification against real issued tokens once Identity.API lands.

high · security

EventBusRabbitMQ — a null-conditional that made its own error path unreachable

(await _rabbitMQConnection?.CreateChannelAsync()) ?? throw
            new InvalidOperationException(...)

looks like it handles a closed connection gracefully. It doesn't — the ?. short-circuits the whole parenthesized expression to a null Task, and awaiting a null task throws NullReferenceException immediately, so the ?? throw branch was dead code. A caller during startup got an opaque NRE instead of the intended message. Fixed with an explicit null check before the await.

medium · reliability

EventBusRabbitMQ — an unchecked cast could crash the whole message-receive path

ExtractTraceContextFromBasicProperties did value as byte[] then passed the result straight to Encoding.UTF8.GetString unchecked — a trace header present under the expected key but not actually a byte[] would throw ArgumentNullException outside OnMessageReceived's own try/catch, crashing the entire receive path over one malformed header. Rewritten to pattern-match (value is byte[] bytes) and fall through to the existing empty-result path instead.

medium · reliability

EventBusRabbitMQ — a cast that can return null, used unchecked

DeserializeMessage's as IntegrationEvent cast can genuinely return null for a malformed message body, but its return type and ProcessEvent's use of the result didn't account for that. Return type changed to IntegrationEvent?; ProcessEvent now logs and returns early on null, matching the existing pattern for an unresolvable event type. The exact same cast-can-return-null shape recurred later in IntegrationEventLogEntry.cs — caught the second time because the first fix was still fresh.

medium · reliability

eShop.ServiceDefaults — a hidden order-dependency between two unrelated files

ClaimsPrincipalExtensions.GetUserId's "sub" claim lookup only worked because a different file, AuthenticationExtensions, happens to remove "sub" from the framework's default claim-type map elsewhere — nothing enforced that ordering or documented the dependency. Now falls back to ClaimTypes.NameIdentifier regardless of whether that removal ran, so it's correct on its own.

medium · correctness

eShop.ServiceDefaults — an unproven ordering assumption picking the "default" API version

UseDefaultOpenApi picked the default API doc via descriptions[^1] — "take the last one" — assuming IApiVersionDescriptionProvider.ApiVersionDescriptions returns versions in ascending order, which is undocumented anywhere. Switched to

descriptions.MaxBy(d =>
            d.ApiVersion)

, correct regardless of provider ordering.

medium · correctness

IntegrationEventLogEF — a lazy IEnumerable wrapping a side-effecting Select

RetrieveEventLogsPendingToPublishAsync returned Task<IEnumerable<...>>, but the real implementation's

.OrderBy().Select(e =>
            e.DeserializeJsonContent(...))

chain is lazy, and DeserializeJsonContent mutates state as a side effect — returning it as IEnumerable risked that mutation re-running, or running late, on every enumeration. Changed to Task<IReadOnlyList<...>>, forcing materialization when the implementation lands.

medium · correctness

eShop.ServiceDefaults — inconsistent logic between two adjacent branches

BuildDescription's deprecation-notice branch checked for a trailing period before appending its message; the sunset-date branch immediately after it didn't do the same check — a real logic inconsistency, not a style nit. Extracted a shared AppendSentenceSeparator helper both branches now call.

low · correctness

eShop.ServiceDefaults — a magic port number in an emulator-only code path

The #if DEBUG Android-emulator issuer hardcoded https://10.0.2.2:5243 — 10.0.2.2 is a legitimate fixed emulator host alias, but :5243 was only correct if eShop.AppHost (not built yet) happens to pin Identity.API to that exact port. Now derived from identityUrl's real Uri.Port instead of a hardcoded guess.

low · correctness

Verifying it actually works

83 tests, and a Decorator split that finally let the flagship fix get proven

MSTest.Sdk on .NET's newer Microsoft.Testing.Platform runner — a genuinely different CLI surface from legacy VSTest, confirmed against a real scratch project before trusting its coverage/TRX flags in CI. Testing isn't deferred to a batched end-of-migration phase here: every project that's done gets full test coverage in the same unit of work, before the source migration moves past it.

83/83 tests, decorators make it possible

ResilientEventBusDecoratorTests.cs verifies bug #01 end-to-end for the first time in this project's history — a fake inner IEventBus that fails once then succeeds, no real RabbitMQ broker needed, since decorators wrap any IEventBus. Closes a gap that had sat unproven since the fix landed. See Testing Strategy.

Honest gap, tracked not solved

Wanted one combined HTML report across every test project — MTP's --report-html produces one file per project instead, confirmed via a real 2-project scratch solution. The actual fix merged upstream 2026-08-09 but hasn't shipped in a NuGet release yet; Dependabot already tracks the package individually, so no new tooling is needed to know when it does.

Judgment calls

Where the interesting decisions actually happened

The review posture changed partway through: early files were kept close to upstream's architecture on principle, then that constraint was explicitly dropped — "this is going to self-hosted" — in favor of treating inconsistencies across upstream's own multi-contributor codebase as things to resolve under one deliberate design, not inherit silently. That's what produced the Decorator split above, and it's why later reviews stopped citing "matches upstream" as a reason to leave something as-is. Separately, WebApp going React instead of Blazor wasn't a technical necessity — Blazor Server can call Basket.API over gRPC natively from the browser tier, which a React SPA can't — so that decision now cascades into a new WebBFF project and a Grpc.AspNetCore.Web middleware addition to Basket.API, both decided ahead of being built and recorded in todo.md as they happened.