|
| 1 | +--- |
| 2 | +name: refactoring-toward-modular-architecture |
| 3 | +description: Use when asked to restructure a layered monolith (Core/Infrastructure/Web style) into a modular monolith, extract modules or bounded contexts, or plan a feature-based reorganization — symptoms include a god aggregate with navigation collections into many features, open-generic IRepository<T> letting any code mutate any entity, DI registration scattered across many files, and third-party SDK types (Stripe, Vimeo, etc.) leaking across layers. |
| 4 | +--- |
| 5 | + |
| 6 | +# Refactoring Toward Modular Architecture |
| 7 | + |
| 8 | +## Overview |
| 9 | + |
| 10 | +Evidence first, boundaries second, sequencing third. The deliverable is a dated recommendations **document** (see [document-template.md](document-template.md)), not code changes. Every boundary claim must cite a file; every phase must leave the app releasable. |
| 11 | + |
| 12 | +## Process |
| 13 | + |
| 14 | +### 1. Inventory the current state (before proposing anything) |
| 15 | + |
| 16 | +Collect, with file paths as evidence: |
| 17 | + |
| 18 | +- **Project + package references**: which SDKs leak where (e.g. Vimeo in Core, MediatR in Core)? |
| 19 | +- **Entity graph**: navigation collections on the largest entity — each collection pointing at another feature is a paved-over module boundary. Count lines; find business logic living on it. |
| 20 | +- **Repository surface**: open-generic `IRepository<T>` registrations = no enforcement point exists. |
| 21 | +- **DbContexts + migration histories** (an existing second context, e.g. Identity, proves the split pattern works). |
| 22 | +- **DI registration surfaces**: count them; grep for duplicate registrations at divergent lifetimes — those are latent bugs. |
| 23 | +- **Cross-layer usings**: count `using *.Infrastructure` in the web layer. |
| 24 | +- **Events + handlers**: who publishes, who subscribes. A pile of `NotifyOnX`/`DiscordLogX` handlers in Infrastructure means the transport owns the policy — inverted. |
| 25 | +- **Specs/queries with cross-aggregate Includes**: these break first under any split; list them. |
| 26 | +- **Duplicated pages/services, dead code, missing `[Authorize]`**. |
| 27 | + |
| 28 | +### 2. Fix urgent findings immediately |
| 29 | + |
| 30 | +Analysis surfaces real bugs (missing auth attributes, broken email sends). Fix them **now**, before restructuring; mark them ~~struck through~~ "Fixed <date>" in the document. Never let a security gap ride a months-long refactor. |
| 31 | + |
| 32 | +### 3. Draft candidate modules |
| 33 | + |
| 34 | +Cluster by event vocabulary + nav collections + change coupling. For each: **Owns / Interface to other modules / Why it's a context**. Rate cohesion and extraction difficulty in a table. Group small look-alike contexts; keep SharedKernel deliberately tiny (no entities, no business rules); add a supporting Notifications/Ops module (transports only, no policy). |
| 35 | + |
| 36 | +### 4. Name the keystone decision |
| 37 | + |
| 38 | +Usually: who owns the god aggregate and how others reference it. Until cross-module references are **ID-only** (records keyed by `MemberId`, ownership flipped to the referencing module), no other boundary can be enforced. Say this explicitly; sequence around it. |
| 39 | + |
| 40 | +### 5. Map cross-module interactions — then check for cycles |
| 41 | + |
| 42 | +Classify each interaction: **sync query** (small public interface returning DTOs/read models, e.g. `IMembershipQueries.GetSummary(id)`) or **integration event**. Then **list bidirectional module pairs** (A queries B; B displays A's data). Any such pair dictates the project structure below — do not skip this check. |
| 43 | + |
| 44 | +### 6. Target structure: Contracts + implementation project per module |
| 45 | + |
| 46 | +One project per module does NOT work: bidirectional pairs become MSBuild reference cycles, and "reference their Contracts *namespace* only" is unenforceable by the compiler. Per module: |
| 47 | + |
| 48 | +- `Modules.X.Contracts` — public interfaces, DTOs/read models, **the integration events X publishes**. References SharedKernel only. No EF/SDK/handler code. |
| 49 | +- `Modules.X` — domain + application + EF + UI, `internal` by default. Public surface is exactly `AddXModule(IServiceCollection, IConfiguration)` + `MapXModule(IEndpointRouteBuilder)`. References other modules' **Contracts projects only**, never implementations. |
| 50 | +- Host references implementations solely to call Add/Map. Compiler enforces the seams; arch tests (NetArchTest) cover only what it can't: SDK containment, Contracts purity, no stray public types. |
| 51 | + |
| 52 | +### 7. Phase the roadmap |
| 53 | + |
| 54 | +Order: **hygiene → composition-root discipline (Add*Module regrouping in place) → smallest/cleanest module first to prove the template → ... → god aggregate last → strict enforcement**. Every phase releasable; cheap wins build the pattern before the hard split. |
| 55 | + |
| 56 | +### 8. Record open questions |
| 57 | + |
| 58 | +Policies spanning modules (e.g. graduation = policy over Billing tenure, mutating Identity roles, sending Membership comms) get an "Open question" entry with a recommendation — not a silent decision. The team confirms before the affected phase. |
| 59 | + |
| 60 | +## Gotchas (each one bit a real project) |
| 61 | + |
| 62 | +| Gotcha | Reality | |
| 63 | +|---|---| |
| 64 | +| "Namespace-based Contracts rules are enough; single project per module" | Arch tests don't stop MSBuild cycles. Find the bidirectional pair first — it forces Contracts projects. | |
| 65 | +| Integration event types "somewhere shared" | They live in the **publisher's** Contracts project, or subscribers must reference implementations. | |
| 66 | +| "Put the shared DTO in SharedKernel for now" | That's how the kernel becomes a junk drawer and the old architecture returns with more projects. | |
| 67 | +| Rewriting EF migration history | One-way door. Freeze old history; each module context starts with a **no-op baseline** against existing tables; rename schemas opportunistically. | |
| 68 | +| Cross-aggregate specs "will still work" | `X→comments→authors→progress` eager-loads break first. Replace with module-local queries + summary read models; accept extra round-trips until measured. | |
| 69 | +| Admin/reporting pages that join everything | Give them a named, read-only exception (views/read model), or they re-couple modules via contract call chains. | |
| 70 | +| Duplicate DI lifetimes "cleaned up silently" | Behavior changes there are latent bugs found, not regressions caused. Fix as isolated, bisectable commits. | |
| 71 | +| A giant `Constants.cs` | Dismantle it per owning module; it's hidden coupling. | |
| 72 | +| "Since we have modules, microservices next" | Modules make that possible later; doing it now converts compile-time errors into network errors. | |
| 73 | + |
| 74 | +## Red Flags — stop and redo the step |
| 75 | + |
| 76 | +- Proposing a target structure before citing file-level evidence |
| 77 | +- A findings list with no file paths |
| 78 | +- No bidirectional-pair check before choosing project layout |
| 79 | +- A phase that leaves the app unreleasable |
| 80 | +- A security finding scheduled for "Phase 5" |
0 commit comments