From c4763f15e442fca1250773cafe27b5da28438825 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?V=C3=B5=20=C4=90=E1=BA=A1i=20L=E1=BB=99c?= <88762074+VoDaiLocz@users.noreply.github.com> Date: Wed, 4 Mar 2026 13:06:03 +0700 Subject: [PATCH] Expand pattern coverage and publish prioritized content-gap audit --- CONTENT-GAP-AUDIT.md | 17 +++++ README.md | 8 +-- skills/design-patterns/SKILL.md | 4 +- .../references/anti-patterns.md | 40 +++++++++++- .../references/architecture-patterns.md | 14 ++++- .../references/code-patterns.md | 1 + .../references/framework-catalog.md | 16 +++++ .../references/migration-playbook.md | 63 +++++++++++++++++++ .../references/system-patterns.md | 46 ++++++++++++++ .../references/testing-patterns.md | 27 +++++++- 10 files changed, 223 insertions(+), 13 deletions(-) create mode 100644 CONTENT-GAP-AUDIT.md diff --git a/CONTENT-GAP-AUDIT.md b/CONTENT-GAP-AUDIT.md new file mode 100644 index 0000000..cc4d15f --- /dev/null +++ b/CONTENT-GAP-AUDIT.md @@ -0,0 +1,17 @@ +# Content Gap Audit (Design-Patterns Repo) + +## Critical +- Added missing **Transactional Outbox** system pattern and **Dual Writes** anti-pattern coverage. +- Added migration path **Direct Publish -> Transactional Outbox** to reduce data/event inconsistency risk. + +## High +- Added **Event Sourcing** as a common system pattern not previously covered. +- Added **N+1 Query Problem** anti-pattern because it is a high-frequency production issue. + +## Medium +- Added **Version Baseline** section in framework catalog to make version intent explicit and reviewable. +- Verified all references listed in `skills/design-patterns/SKILL.md` exist in repository. + +## Low +- Ran a markdown link/path scan (no actionable broken local links detected). +- Minor wording cleanup happened indirectly via added sections. diff --git a/README.md b/README.md index b98ac52..40780c6 100644 --- a/README.md +++ b/README.md @@ -119,8 +119,8 @@ Step 4: Thin routes.ts to HTTP-only controller | Capability | Reference | |------------|----------| | ๐Ÿงช **Testing Patterns** โ€” How to test every pattern | `references/testing-patterns.md` | -| ๐Ÿšซ **Anti-Patterns Catalog** โ€” 15 code smells with detection & cures | `references/anti-patterns.md` | -| ๐Ÿ”„ **Migration Playbook** โ€” 5 step-by-step refactoring guides | `references/migration-playbook.md` | +| ๐Ÿšซ **Anti-Patterns Catalog** โ€” 17 code smells with detection & cures | `references/anti-patterns.md` | +| ๐Ÿ”„ **Migration Playbook** โ€” 6 step-by-step refactoring guides | `references/migration-playbook.md` | | ๐Ÿ“„ **Output Formats** โ€” ADR, Health Check, Comparison Matrix | `references/output-format.md` | ### Framework-Specific Implementations @@ -180,8 +180,8 @@ skills/design-patterns/ โ”œโ”€โ”€ system-patterns.md # Microservices, Event-Driven, Cloud โ”œโ”€โ”€ framework-catalog.md # Next.js, FastAPI, NestJS, Django, Express, Go โ”œโ”€โ”€ testing-patterns.md # โœจ NEW โ€” Unit/Integration/E2E test code - โ”œโ”€โ”€ anti-patterns.md # โœจ NEW โ€” 15 anti-patterns with detection - โ”œโ”€โ”€ migration-playbook.md # โœจ NEW โ€” 5 migration guides + โ”œโ”€โ”€ anti-patterns.md # โœจ NEW โ€” 17 anti-patterns with detection + โ”œโ”€โ”€ migration-playbook.md # โœจ NEW โ€” 6 migration guides โ””โ”€โ”€ output-format.md # โœจ NEW โ€” ADR, Reports, Health Check RELEASE-NOTES.md diff --git a/skills/design-patterns/SKILL.md b/skills/design-patterns/SKILL.md index a9d3d74..ae7f2ce 100644 --- a/skills/design-patterns/SKILL.md +++ b/skills/design-patterns/SKILL.md @@ -244,6 +244,6 @@ Every design-patterns analysis MUST follow: - `references/system-patterns.md` โ€” Microservices, Event-Driven, Circuit Breaker, Saga, Cloud patterns - `references/framework-catalog.md` โ€” Pattern implementations for Next.js, FastAPI, NestJS, Django, Express, Go - `references/testing-patterns.md` โ€” How to test every pattern (unit, integration, e2e) with real test code -- `references/anti-patterns.md` โ€” 15 anti-patterns catalog with detection commands, severities, and cures -- `references/migration-playbook.md` โ€” 5 step-by-step migration guides with verification checkpoints +- `references/anti-patterns.md` โ€” 17 anti-patterns catalog with detection commands, severities, and cures +- `references/migration-playbook.md` โ€” 6 step-by-step migration guides with verification checkpoints - `references/output-format.md` โ€” ADR template, Refactoring Report, Health Check, Pattern Comparison diff --git a/skills/design-patterns/references/anti-patterns.md b/skills/design-patterns/references/anti-patterns.md index 3ff81eb..bf21d90 100644 --- a/skills/design-patterns/references/anti-patterns.md +++ b/skills/design-patterns/references/anti-patterns.md @@ -341,11 +341,31 @@ class Order { **Cure:** Start with Module Boundaries โ†’ Service Layer โ†’ Eventually Clean Architecture + +--- + +### 13. N+1 Query Problem + +**Severity:** ๐Ÿ”ด High + +**Detection:** Repeated query pattern inside loops or ORM lazy-loading waterfalls. + +```typescript +// ๐Ÿ”ด N+1: one query for orders + one query per order for items +const orders = await prisma.order.findMany(); +for (const order of orders) { + order.items = await prisma.orderItem.findMany({ where: { orderId: order.id } }); +} +``` + +**Cure:** Eager loading (`include`/`select`), batching, and query-level aggregation. + + --- ## System-Level Anti-Patterns -### 13. Distributed Monolith +### 14. Distributed Monolith **Severity:** ๐Ÿšจ Critical @@ -361,7 +381,7 @@ class Order { --- -### 14. Chatty Services +### 15. Chatty Services **Severity:** ๐Ÿ”ด High @@ -371,7 +391,7 @@ class Order { --- -### 15. No Timeout / No Retry +### 16. No Timeout / No Retry **Severity:** ๐Ÿ”ด High @@ -421,3 +441,17 @@ Run this checklist on any module before refactoring: - [ ] Shared database between services? - [ ] Services that must deploy together? ``` + + +### 17. Dual Writes (DB + Event Bus without Outbox) + +**Severity:** ๐Ÿšจ Critical + +**Detection:** Application writes DB and publishes message in separate non-atomic steps. + +**Symptoms:** +- Order exists in DB but `order.created` event missing +- Event published but DB transaction rolled back +- Hard-to-reconcile states across services + +**Cure:** Transactional Outbox pattern with idempotent consumers. diff --git a/skills/design-patterns/references/architecture-patterns.md b/skills/design-patterns/references/architecture-patterns.md index f98eb6f..ebe7bec 100644 --- a/skills/design-patterns/references/architecture-patterns.md +++ b/skills/design-patterns/references/architecture-patterns.md @@ -99,15 +99,20 @@ export class UserRepository { ```python # routers/user.py โ€” Controller from fastapi import APIRouter, Depends +from schemas.user import CreateUserDTO, User from services.user_service import UserService router = APIRouter() -@router.post("/users", status_code=201) +@router.post("/users", status_code=201, response_model=User) async def create_user(data: CreateUserDTO, service: UserService = Depends()): return await service.create(data) # services/user_service.py โ€” Business Logic +from fastapi import Depends, HTTPException +from repositories.user_repository import UserRepository +from utils.security import hash_password + class UserService: def __init__(self, repo: UserRepository = Depends()): self.repo = repo @@ -117,12 +122,17 @@ class UserService: if existing: raise HTTPException(409, "Email already exists") hashed = hash_password(data.password) - return await self.repo.save({**data.dict(), "password": hashed}) + return await self.repo.save({**data.model_dump(), "password": hashed}) # repositories/user_repository.py โ€” Data Access +from db.client import database + class UserRepository: async def find_by_email(self, email: str) -> User | None: return await database.users.find_one({"email": email}) + + async def save(self, data: dict) -> User: + return await database.users.insert_one(data) ``` --- diff --git a/skills/design-patterns/references/code-patterns.md b/skills/design-patterns/references/code-patterns.md index b919c68..b38fbc2 100644 --- a/skills/design-patterns/references/code-patterns.md +++ b/skills/design-patterns/references/code-patterns.md @@ -205,6 +205,7 @@ if (process.env.NODE_ENV !== "production") globalForPrisma.prisma = prisma; **Python Example:** ```python # config.py โ€” Thread-safe Singleton +import os import threading class AppConfig: diff --git a/skills/design-patterns/references/framework-catalog.md b/skills/design-patterns/references/framework-catalog.md index a5ae08c..72c5886 100644 --- a/skills/design-patterns/references/framework-catalog.md +++ b/skills/design-patterns/references/framework-catalog.md @@ -5,6 +5,22 @@ --- + +## Version Baseline (Validate Quarterly) + +> Keep examples aligned with actively maintained major versions. + +| Framework | Baseline in this catalog | +|-----------|--------------------------| +| Next.js | 15.x (App Router + Server Actions) | +| FastAPI | 0.110+ (Pydantic v2 style) | +| NestJS | 10.x+ | +| Django | 5.x | +| Express | 5.x | +| Go | 1.22+ | + +--- + ## Next.js (App Router) ### Recommended Architecture diff --git a/skills/design-patterns/references/migration-playbook.md b/skills/design-patterns/references/migration-playbook.md index 1291565..852fb36 100644 --- a/skills/design-patterns/references/migration-playbook.md +++ b/skills/design-patterns/references/migration-playbook.md @@ -351,6 +351,69 @@ Week 4: Remove old code from monolith **Step 6: Repeat** for next least-coupled module. + +--- + +## Migration 6: Direct Publish โ†’ Transactional Outbox + +**Difficulty:** โญโญโญโญ (Advanced) +**Time:** 2-5 days +**Risk:** High (touches write path + async workers) + +### Problem Signature +```typescript +await orderRepo.save(order); // DB write +await eventBus.publish(new OrderCreated); // separate write (can fail) +``` + +### Steps + +**Step 1: Add outbox table** +```sql +CREATE TABLE outbox_events ( + id UUID PRIMARY KEY, + aggregate_type TEXT NOT NULL, + aggregate_id TEXT NOT NULL, + event_type TEXT NOT NULL, + payload JSONB NOT NULL, + status TEXT NOT NULL DEFAULT 'pending', + created_at TIMESTAMPTZ NOT NULL DEFAULT now(), + sent_at TIMESTAMPTZ +); +``` + +**Step 2: Write business row + outbox row in one transaction** +```typescript +await prisma.$transaction(async (tx) => { + await tx.order.create({ data: orderData }); + await tx.outboxEvent.create({ + data: { + aggregateType: 'order', + aggregateId: orderData.id, + eventType: 'order.created', + payload: orderData, + }, + }); +}); +``` + +**Step 3: Add outbox publisher worker** +- Poll pending rows in small batches +- Publish to broker +- Mark rows as sent +- Retry with exponential backoff + +**Step 4: Make consumers idempotent** +- Deduplicate by event ID +- Store processed event IDs + +**Step 5: Verify** +```bash +# Simulate broker outage, ensure events remain in outbox pending state +npm test +``` + + --- ## Migration Verification Checklist diff --git a/skills/design-patterns/references/system-patterns.md b/skills/design-patterns/references/system-patterns.md index d1294d7..3388572 100644 --- a/skills/design-patterns/references/system-patterns.md +++ b/skills/design-patterns/references/system-patterns.md @@ -282,6 +282,50 @@ Phase 3: /api/users + /api/orders โ†’ New Phase N: 100% traffic โ†’ New, Legacy decommissioned ``` + +--- + +## 9. Event Sourcing Pattern + +### Problem +Auditability and temporal debugging are hard when you only store the latest row state. + +### Solution +Persist immutable domain events as the source of truth, and rebuild state from the event stream. + +### When to Use +- Need full audit history (fintech, compliance-heavy domains) +- Need time-travel debugging ("what did we know at T1?") +- Complex domain workflows where replay is valuable + +### Trade-offs +- Higher implementation complexity than CRUD +- Requires event versioning strategy +- Read models usually need separate projections + +--- + +## 10. Transactional Outbox Pattern + +### Problem +Dual-write risk: write DB row succeeds, publish event fails (or vice versa) causing inconsistency. + +### Solution +Write business data and outbox record in ONE local transaction, then publish asynchronously from the outbox. + +### Minimal Flow +``` +1) Begin transaction +2) Write business entity (e.g., order) +3) Write outbox event row (e.g., order.created) +4) Commit +5) Outbox worker publishes event and marks row as sent +``` + +### Why It Matters +Outbox is the default reliability companion for Event-Driven + Saga systems. + + --- ## Pattern Decision Matrix @@ -295,3 +339,5 @@ Phase N: 100% traffic โ†’ New, Legacy decommissioned | Legacy migration | Strangler Fig | Medium | Any | | Cross-cutting concerns | Sidecar | Medium | 3+ | | Monolith too big | Microservices | Very High | 8+ | +| Need full audit trail + replay | Event Sourcing | High | 5+ | +| Reliable DB + event publication | Transactional Outbox | Medium | 3+ | diff --git a/skills/design-patterns/references/testing-patterns.md b/skills/design-patterns/references/testing-patterns.md index 69c6afd..0174404 100644 --- a/skills/design-patterns/references/testing-patterns.md +++ b/skills/design-patterns/references/testing-patterns.md @@ -100,6 +100,7 @@ describe("EventBus", () => { describe("CommandHistory", () => { let history: CommandHistory; let cart: Cart; + const laptop: CartItem = { id: "1", name: "Laptop", qty: 1 }; beforeEach(() => { history = new CommandHistory(); @@ -192,9 +193,11 @@ describe("Middleware Decorators", () => { **The Key:** Mock the Repository, test the Service logic. ```typescript +import type { Mocked } from "vitest"; + describe("UserService", () => { let service: UserService; - let mockRepo: jest.Mocked; + let mockRepo: Mocked; beforeEach(() => { mockRepo = { @@ -232,6 +235,7 @@ describe("UserRepository (Integration)", () => { beforeAll(async () => { // Use test database await prisma.$connect(); + repo = new UserRepository(prisma); }); afterEach(async () => { @@ -256,15 +260,27 @@ describe("UserRepository (Integration)", () => { ```typescript // Command side โ€” test business rules describe("CreateOrderHandler", () => { + let handler: CreateOrderHandler; + const mockRepo = { save: vi.fn() }; + const mockEventBus = { publish: vi.fn() }; + + beforeEach(() => { + handler = new CreateOrderHandler(mockRepo, mockEventBus); + }); + it("validates stock before creating order", async () => { - const handler = new CreateOrderHandler(mockRepo, mockEventBus); await expect(handler.execute(new CreateOrderCommand("user-1", [ { productId: "p-1", quantity: 999 }, // Exceeds stock ]))).rejects.toThrow("Insufficient stock"); }); it("publishes OrderCreated event", async () => { + const validCommand = new CreateOrderCommand("user-1", [ + { productId: "p-1", quantity: 1 }, + ]); + await handler.execute(validCommand); + expect(mockEventBus.publish).toHaveBeenCalledWith( expect.objectContaining({ type: "OrderCreated" }) ); @@ -273,6 +289,13 @@ describe("CreateOrderHandler", () => { // Query side โ€” test data retrieval describe("GetOrderSummaryHandler", () => { + let handler: GetOrderSummaryHandler; + const readDb = { query: vi.fn() }; + + beforeEach(() => { + handler = new GetOrderSummaryHandler(readDb); + }); + it("returns denormalized view", async () => { const result = await handler.execute(new GetOrderSummaryQuery("ord-1")); expect(result).toMatchObject({