Compare commits
1 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
| c91f3e754a |
@@ -0,0 +1,107 @@
|
|||||||
|
# Bad Example: God Object
|
||||||
|
|
||||||
|
> A component that violates Atelier principles. Each violation is cited.
|
||||||
|
|
||||||
|
## The Code
|
||||||
|
|
||||||
|
```typescript
|
||||||
|
// UserManager.ts — 1,200 lines
|
||||||
|
class UserManager {
|
||||||
|
private users: User[] = [];
|
||||||
|
private cache: Map<string, User> = new Map();
|
||||||
|
private db: Database;
|
||||||
|
private emailService: EmailService;
|
||||||
|
private logger: Logger;
|
||||||
|
private auditLog: AuditLog;
|
||||||
|
|
||||||
|
constructor(db: Database, email: EmailService, logger: Logger, audit: AuditLog) {
|
||||||
|
this.db = db;
|
||||||
|
this.emailService = email;
|
||||||
|
this.logger = logger;
|
||||||
|
this.auditLog = audit;
|
||||||
|
}
|
||||||
|
|
||||||
|
// CRUD
|
||||||
|
async createUser(data: UserData): Promise<User> { /* 80 lines */ }
|
||||||
|
async getUser(id: string): Promise<User> { /* 40 lines */ }
|
||||||
|
async updateUser(id: string, data: Partial<UserData>): Promise<User> { /* 60 lines */ }
|
||||||
|
async deleteUser(id: string): Promise<void> { /* 50 lines */ }
|
||||||
|
async listUsers(page: number): Promise<User[]> { /* 40 lines */ }
|
||||||
|
|
||||||
|
// Email
|
||||||
|
async sendWelcomeEmail(user: User): Promise<void> { /* 50 lines */ }
|
||||||
|
async sendPasswordReset(user: User): Promise<void> { /* 50 lines */ }
|
||||||
|
async sendDeletionNotice(user: User): Promise<void> { /* 40 lines */ }
|
||||||
|
|
||||||
|
// Auth
|
||||||
|
async authenticate(email: string, password: string): Promise<boolean> { /* 70 lines */ }
|
||||||
|
async authorize(userId: string, action: string): Promise<boolean> { /* 60 lines */ }
|
||||||
|
async hashPassword(password: string): Promise<string> { /* 20 lines */ }
|
||||||
|
|
||||||
|
// Cache
|
||||||
|
private cacheGet(id: string): User | null { /* 20 lines */ }
|
||||||
|
private cacheSet(user: User): void { /* 20 lines */ }
|
||||||
|
private cacheInvalidate(id: string): void { /* 20 lines */ }
|
||||||
|
|
||||||
|
// Audit
|
||||||
|
private logAudit(action: string, userId: string): void { /* 30 lines */ }
|
||||||
|
|
||||||
|
// Validation
|
||||||
|
private validateEmail(email: string): boolean { /* 20 lines */ }
|
||||||
|
private validatePassword(password: string): boolean { /* 20 lines */ }
|
||||||
|
|
||||||
|
// Serialization
|
||||||
|
toJSON(user: User): Record<string, unknown> { /* 30 lines */ }
|
||||||
|
fromJSON(data: Record<string, unknown>): User { /* 30 lines */ }
|
||||||
|
|
||||||
|
// ... 200 more lines of helper methods
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
## Violations
|
||||||
|
|
||||||
|
### C3 Simplicity (Core)
|
||||||
|
- A 1,200-line class doing 8 different things (CRUD, email, auth, cache, audit, validation, serialization).
|
||||||
|
- The class cannot be understood in one read. Complexity is the liability.
|
||||||
|
- **Fix:** Split into `UserRepository` (CRUD), `UserEmailService` (email), `UserAuthService` (auth), `UserCache` (cache), `UserAuditLogger` (audit), `UserValidator` (validation), `UserSerializer` (serialization).
|
||||||
|
|
||||||
|
### C6 Composability (Core)
|
||||||
|
- The class takes 4 dependencies and does 8 jobs. It is not composable; it is monolithic.
|
||||||
|
- You cannot reuse the email logic without the DB, the cache, the audit log.
|
||||||
|
- **Fix:** Each responsibility is its own class. Compose them: `UserEmailService` takes only `EmailService`.
|
||||||
|
|
||||||
|
### components.md §1 Single Responsibility (UI/UX, applies to code)
|
||||||
|
- The class name is `UserManager`. "Manager" is a smell — it manages what? Everything.
|
||||||
|
- If the name is "Manager," it has no single responsibility.
|
||||||
|
- **Fix:** Name by responsibility: `UserRepository`, `UserAuthService`. Names that cannot be "And"-ed.
|
||||||
|
|
||||||
|
### C4 Locality (Core)
|
||||||
|
- Cache logic is in the same class as email logic. A change to cache touches the email methods' neighbor.
|
||||||
|
- Related logic (cache get/set/invalidate) is grouped, but unrelated logic (email) is adjacent.
|
||||||
|
- **Fix:** `UserCache` is its own class. Cache changes are local to cache.
|
||||||
|
|
||||||
|
### C2 Clarity (Core)
|
||||||
|
- A reader cannot answer "what does `UserManager` do?" in one sentence.
|
||||||
|
- The class has 20+ methods. The reader must scan all of them to find the one they need.
|
||||||
|
- **Fix:** Smaller classes with clear names. The name is the documentation.
|
||||||
|
|
||||||
|
### Security P2 Least Privilege (Security)
|
||||||
|
- The class has `db`, `emailService`, `logger`, `auditLog` — all available to all methods.
|
||||||
|
- `sendWelcomeEmail` has access to `db.delete`. Least privilege is violated.
|
||||||
|
- **Fix:** Each service has only the dependencies it needs. `UserEmailService` has `EmailService`, not `Database`.
|
||||||
|
|
||||||
|
### Testing P2 Independence (Testing)
|
||||||
|
- To test `sendWelcomeEmail`, you must construct `UserManager` with a real/mock DB, email, logger, audit.
|
||||||
|
- The test setup is 4 mocks for one method. Independence is violated.
|
||||||
|
- **Fix:** Test `UserEmailService` with one mock (`EmailService`).
|
||||||
|
|
||||||
|
## What This Example Reveals
|
||||||
|
|
||||||
|
The "God Object" is the cardinal sin of OOP. It violates C3 (Simplicity), C6 (Composability), C4 (Locality), and C2 (Clarity) simultaneously. Every other principle suffers downstream:
|
||||||
|
|
||||||
|
- Testing is hard (T2 Independence).
|
||||||
|
- Security is loose (S2 Least Privilege).
|
||||||
|
- Evolution is brittle (a change to email risks cache).
|
||||||
|
- Review is exhausting (a 1,200-line diff).
|
||||||
|
|
||||||
|
The fix is always the same: **decompose by responsibility**. The class name is the test: if it is "Manager," "Handler," or "Helper," it has no single responsibility.
|
||||||
@@ -0,0 +1,87 @@
|
|||||||
|
# Bad Example: Leaky Abstraction
|
||||||
|
|
||||||
|
> An abstraction that leaks its implementation details, violating Atelier principles. Each violation is cited.
|
||||||
|
|
||||||
|
## The Code
|
||||||
|
|
||||||
|
```typescript
|
||||||
|
// UserRepository — "abstracts" the database
|
||||||
|
class UserRepository {
|
||||||
|
async findAll(): Promise<UserRow[]> {
|
||||||
|
// Leaks: returns the raw DB row type, not a domain User
|
||||||
|
return db.query('SELECT id, email, password_hash, created_at, deleted_at FROM users');
|
||||||
|
}
|
||||||
|
|
||||||
|
async findByEmail(email: string): Promise<UserRow | null> {
|
||||||
|
// Leaks: the caller must know to filter deleted_at
|
||||||
|
const rows = await db.query('SELECT * FROM users WHERE email = $1', [email]);
|
||||||
|
return rows[0] || null;
|
||||||
|
}
|
||||||
|
|
||||||
|
async save(user: UserRow): Promise<void> {
|
||||||
|
// Leaks: the caller must know the column names and the SQL
|
||||||
|
await db.query(
|
||||||
|
'UPDATE users SET email = $1, password_hash = $2, updated_at = now() WHERE id = $3',
|
||||||
|
[user.email, user.password_hash, user.id]
|
||||||
|
);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// Usage — the leak is visible
|
||||||
|
const repo = new UserRepository();
|
||||||
|
const user = await repo.findByEmail('jane@example.com');
|
||||||
|
if (user && !user.deleted_at) { // caller must know about soft delete
|
||||||
|
user.password_hash = await hash(newPassword); // caller must know the column
|
||||||
|
await repo.save(user); // caller must know it's an UPDATE
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
## Violations
|
||||||
|
|
||||||
|
### C6 Composability (Core)
|
||||||
|
- The abstraction is supposed to hide the database. It does not.
|
||||||
|
- The caller must know: the row type (`UserRow`), the soft-delete column (`deleted_at`), the password column (`password_hash`), the SQL operation (`UPDATE`).
|
||||||
|
- The abstraction is a thin wrapper. It composes nothing; it leaks everything.
|
||||||
|
- **Fix:** Return a domain `User` (no `password_hash`, no `deleted_at`). Hide soft delete (the repo filters it). Hide persistence (the caller calls `save`, not `UPDATE`).
|
||||||
|
|
||||||
|
### C2 Clarity (Core)
|
||||||
|
- The caller's code is unclear: `if (user && !user.deleted_at)` — what is `deleted_at`? Why does the caller check it?
|
||||||
|
- The abstraction was supposed to clarify. It muddied.
|
||||||
|
- **Fix:** `repo.findByEmail()` returns `User | null` (already filtered). The caller does not know soft delete exists.
|
||||||
|
|
||||||
|
### API P2 Clarity (API, by analogy)
|
||||||
|
- The repo's API exposes the DB schema in its return types. `UserRow` is a DB concept, not a domain concept.
|
||||||
|
- The public contract (return type) leaks the private implementation (the table).
|
||||||
|
- **Fix:** The return type is `User`, a domain type. `UserRow` is internal.
|
||||||
|
|
||||||
|
### Data P8 Lifecycle Awareness (Data)
|
||||||
|
- The soft-delete lifecycle (`deleted_at`) is the repo's concern. The caller should not manage it.
|
||||||
|
- By exposing `deleted_at`, the repo forces every caller to remember the filter. A forgotten filter is a soft-delete leak.
|
||||||
|
- **Fix:** The repo filters `deleted_at IS NULL` in every query. The caller never sees `deleted_at`.
|
||||||
|
|
||||||
|
### Security P9 Secret Hygiene (Security)
|
||||||
|
- `password_hash` is in the returned `UserRow`. The caller now has access to the password hash.
|
||||||
|
- A caller that logs `user` logs the hash. A caller that serializes `user` serializes the hash.
|
||||||
|
- **Fix:** `User` does not include `password_hash`. Only `UserRepository` and `AuthService` (internal) see it.
|
||||||
|
|
||||||
|
### C4 Locality (Core)
|
||||||
|
- The SQL is in the repo, but the column knowledge (`password_hash`, `deleted_at`) is in the caller.
|
||||||
|
- A column rename touches the repo AND every caller. Locality is violated.
|
||||||
|
- **Fix:** Column names are local to the repo. The caller knows only the domain `User`.
|
||||||
|
|
||||||
|
### C5 Reversibility (Core)
|
||||||
|
- Changing the database (e.g., from SQL to NoSQL, or renaming a column) requires touching every caller.
|
||||||
|
- The abstraction was supposed to make the change local. It does not.
|
||||||
|
- **Fix:** The repo's interface (`findByEmail`, `save`) is stable. The implementation changes; the callers do not.
|
||||||
|
|
||||||
|
## What This Example Reveals
|
||||||
|
|
||||||
|
The leaky abstraction is the false promise of encapsulation. The class is named `UserRepository` (suggesting it abstracts persistence), but it returns raw DB rows, exposes lifecycle columns, and leaks secret fields. The abstraction exists in name only.
|
||||||
|
|
||||||
|
The cost:
|
||||||
|
- Every caller must know the DB schema (C6 violated).
|
||||||
|
- A schema change touches every caller (C5 violated, C4 violated).
|
||||||
|
- Secret fields leak to callers (Security P9 violated).
|
||||||
|
- The lifecycle is the caller's burden (Data P8 violated).
|
||||||
|
|
||||||
|
The fix is always the same: **the abstraction's public type is the domain type, not the implementation type**. `UserRepository.findByEmail()` returns `User | null`, where `User` has `id`, `email`, `name` — and nothing else. `password_hash`, `deleted_at`, `UserRow` are internal. The caller knows nothing about the database.
|
||||||
@@ -0,0 +1,88 @@
|
|||||||
|
# Bad Example: Silent Error
|
||||||
|
|
||||||
|
> An error-handling pattern that violates Atelier principles. Each violation is cited.
|
||||||
|
|
||||||
|
## The Code
|
||||||
|
|
||||||
|
```typescript
|
||||||
|
async function getUser(id: string): Promise<User | null> {
|
||||||
|
try {
|
||||||
|
const user = await db.query('SELECT * FROM users WHERE id = $1', [id]);
|
||||||
|
return user;
|
||||||
|
} catch (e) {
|
||||||
|
return null;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
async function processOrder(orderId: string): Promise<void> {
|
||||||
|
const order = await getOrder(orderId);
|
||||||
|
if (!order) {
|
||||||
|
return; // silently do nothing
|
||||||
|
}
|
||||||
|
// ... process
|
||||||
|
}
|
||||||
|
|
||||||
|
// Usage in a route
|
||||||
|
router.get('/users/:id', async (req, res) => {
|
||||||
|
const user = await getUser(req.params.id);
|
||||||
|
if (!user) {
|
||||||
|
res.status(404).json({ error: 'Not found' });
|
||||||
|
} else {
|
||||||
|
res.json({ data: user });
|
||||||
|
}
|
||||||
|
});
|
||||||
|
```
|
||||||
|
|
||||||
|
## Violations
|
||||||
|
|
||||||
|
### Errors P2 Fail Loudly (Errors)
|
||||||
|
- `catch (e) { return null }` swallows the error. The caller cannot distinguish "user not found" from "database down."
|
||||||
|
- A database outage returns 404s. The operator never knows. Silent failure.
|
||||||
|
- **Fix:** Catch and re-throw with context, or return a typed error (`Result<User, Error>`). Never `null` for "an error happened."
|
||||||
|
|
||||||
|
### Errors P3 Fail Specifically (Errors)
|
||||||
|
- `return null` is the least specific response. It could mean: not found, db error, network error, permission error.
|
||||||
|
- The caller's `if (!user)` cannot distinguish these. The 404 is a lie if the real cause was a 500.
|
||||||
|
- **Fix:** Return `Result` or throw. The error type/code carries the specificity.
|
||||||
|
|
||||||
|
### Errors P1 Errors are Data (Errors)
|
||||||
|
- `null` is not data. It is the absence of data. Conflating "error" with "absence" loses information.
|
||||||
|
- The error (a database failure) was data; it was thrown away and replaced with `null`.
|
||||||
|
- **Fix:** Errors are values. Return the error value, not a sentinel absence.
|
||||||
|
|
||||||
|
### Errors P4 Preserve Context (Errors)
|
||||||
|
- The catch block discards `e`. The stack trace, the error message, the cause — all gone.
|
||||||
|
- The log has no record. The operator cannot debug.
|
||||||
|
- **Fix:** Log the error with context. Wrap and re-throw: `throw new Error('getUser failed', { cause: e })`.
|
||||||
|
|
||||||
|
### Errors P9 Errors are Logged (Errors)
|
||||||
|
- The error is not logged. The handling (return null) is the entire response. The log is missing.
|
||||||
|
- An error that is not logged is an error that cannot be investigated.
|
||||||
|
- **Fix:** `logger.error({ err: e, userId: id })` before returning/rethrowing.
|
||||||
|
|
||||||
|
### Errors P10 Errors Don't Lie (Errors)
|
||||||
|
- `return null` claims "no user" when the truth may be "database down." The function lies.
|
||||||
|
- The 404 response claims "not found" when the truth may be "internal error." The API lies.
|
||||||
|
- **Fix:** The response status must match the actual condition. 500 for server errors, 404 for not found.
|
||||||
|
|
||||||
|
### Observability P2 Correlation, P3 Context (Observability)
|
||||||
|
- No `request_id`. No correlation across services.
|
||||||
|
- No context in the (missing) log. "What was the user doing?" is unanswerable.
|
||||||
|
- **Fix:** Propagate `request_id`. Log with path, method, user_id.
|
||||||
|
|
||||||
|
### Security P8 Fail Securely (Security)
|
||||||
|
- The silent failure is fail-open in disguise. If `getUser` fails due to an authz check throwing, the catch returns `null`.
|
||||||
|
- The caller treats `null` as "not found" and may proceed, or may 404. Either way, the security failure is hidden.
|
||||||
|
- **Fix:** Distinguish "not found" (404) from "authz error" (403) from "db error" (500). Never collapse them into `null`.
|
||||||
|
|
||||||
|
## What This Example Reveals
|
||||||
|
|
||||||
|
The silent error is the most common and most damaging anti-pattern. It violates Errors P2 (Fail Loudly), P3 (Fail Specifically), P1 (Errors are Data), P4 (Preserve Context), P9 (Errors are Logged), P10 (Errors Don't Lie) — six of ten error principles in one catch block.
|
||||||
|
|
||||||
|
The downstream effects:
|
||||||
|
- Operators cannot debug (no log, no context).
|
||||||
|
- Users see wrong errors (404 for a 500).
|
||||||
|
- Security failures hide (authz error becomes "not found").
|
||||||
|
- The system appears healthy when it is not (no metrics, no logs).
|
||||||
|
|
||||||
|
The fix is always the same: **never swallow an error**. Log it, wrap it, rethrow it, or return it as a typed value. Never `return null` for "something went wrong."
|
||||||
@@ -0,0 +1,79 @@
|
|||||||
|
# Good Example: API Endpoint
|
||||||
|
|
||||||
|
> A REST endpoint that follows Atelier's API principles. Each aspect cites the principle it satisfies.
|
||||||
|
|
||||||
|
## The Endpoint
|
||||||
|
|
||||||
|
```typescript
|
||||||
|
// POST /v1/orders — create an order
|
||||||
|
router.post('/v1/orders', auth, validate(CreateOrderSchema), async (req, res) => {
|
||||||
|
const { userId, items } = req.body;
|
||||||
|
|
||||||
|
const order = await orderService.create({ userId, items });
|
||||||
|
|
||||||
|
res.status(201).location(`/v1/orders/${order.id}`).json({
|
||||||
|
data: order,
|
||||||
|
});
|
||||||
|
});
|
||||||
|
```
|
||||||
|
|
||||||
|
## What Makes It Good
|
||||||
|
|
||||||
|
### Resource Naming (API P2 Clarity, P3 Predictability)
|
||||||
|
- `/v1/orders` — noun, plural, lowercase, hyphenated.
|
||||||
|
- Versioned (`/v1`) — P5 Versioning.
|
||||||
|
- No verb in the URL; the HTTP method is the verb.
|
||||||
|
|
||||||
|
### Method Semantics (API P1 Contract Fidelity, P6 Idempotency)
|
||||||
|
- POST for creation. 201 on success (not 200). `Location` header for the new resource.
|
||||||
|
- Idempotency key supported via middleware (omitted for brevity) — P6.
|
||||||
|
|
||||||
|
### Authentication (API P8 Security, Security P1 Zero Trust)
|
||||||
|
- `auth` middleware runs on every endpoint by default. No opt-in auth.
|
||||||
|
- The endpoint does not re-implement auth; it relies on the boundary check.
|
||||||
|
|
||||||
|
### Input Validation (API P8, Security P4 Input Validation)
|
||||||
|
- `validate(CreateOrderSchema)` — schema-based validation at the boundary.
|
||||||
|
- The schema (zod, joi, etc.) defines types, ranges, required fields.
|
||||||
|
- Unknown fields rejected (`additionalProperties: false` in the schema).
|
||||||
|
|
||||||
|
### Response Shape (API P2 Clarity)
|
||||||
|
- `{ data: order }` — wrapped, not a bare object. Allows adding pagination/metadata without breaking.
|
||||||
|
- The shape is consistent across all endpoints in the API.
|
||||||
|
|
||||||
|
### Error Handling (API P9 Error Transparency, Errors P1 Errors are Data)
|
||||||
|
- Errors thrown in `orderService.create` are caught by centralized middleware.
|
||||||
|
- Errors are structured: `{ error: { code, message, request_id } }`.
|
||||||
|
- 404 → `ORDER_NOT_FOUND`, 409 → `DUPLICATE_ORDER`, 422 → `VALIDATION_FAILED`.
|
||||||
|
|
||||||
|
### Observability (Observability P2 Correlation, P3 Sufficient Context)
|
||||||
|
- `request_id` propagated via middleware. Every log in the request includes it.
|
||||||
|
- Significant events logged: "order created", "order creation failed".
|
||||||
|
|
||||||
|
### Economy (Core C8, Performance P4 Resource Bounds)
|
||||||
|
- The order creation is bounded in time (the service has a timeout).
|
||||||
|
- No unbounded query; no loading all products into memory.
|
||||||
|
|
||||||
|
## The Schema (for completeness)
|
||||||
|
|
||||||
|
```typescript
|
||||||
|
const CreateOrderSchema = z.object({
|
||||||
|
userId: z.string().uuid(),
|
||||||
|
items: z.array(z.object({
|
||||||
|
productId: z.string().uuid(),
|
||||||
|
quantity: z.number().int().positive().max(100),
|
||||||
|
})).min(1).max(50),
|
||||||
|
}).strict(); // additionalProperties: false
|
||||||
|
```
|
||||||
|
|
||||||
|
- `userId` is a UUID (Data P7 Type Fidelity).
|
||||||
|
- `quantity` is bounded (Performance P4).
|
||||||
|
- `items` is bounded (1–50) (Performance P4, Security P10 Surface Minimization).
|
||||||
|
- `.strict()` rejects unknown fields (Security P4).
|
||||||
|
|
||||||
|
## What This Example Does NOT Do (And Why That's Good)
|
||||||
|
|
||||||
|
- Does not return 200 on error — the status code is the first signal (API P9).
|
||||||
|
- Does not log the request body — may contain PII (Observability P6, Security P9).
|
||||||
|
- Does not construct SQL by string interpolation — uses a service layer (Security P5 Output Safety).
|
||||||
|
- Does not skip auth for "internal" callers — Zero Trust (Security P1).
|
||||||
@@ -0,0 +1,88 @@
|
|||||||
|
# Good Example: Database Schema
|
||||||
|
|
||||||
|
> A SQL schema that follows Atelier's Data principles. Each aspect cites the principle it satisfies.
|
||||||
|
|
||||||
|
## The Schema
|
||||||
|
|
||||||
|
```sql
|
||||||
|
-- Users table
|
||||||
|
CREATE TABLE users (
|
||||||
|
id UUID PRIMARY KEY DEFAULT gen_random_uuid(),
|
||||||
|
email VARCHAR(255) NOT NULL,
|
||||||
|
name VARCHAR(100) NOT NULL,
|
||||||
|
created_at TIMESTAMPTZ NOT NULL DEFAULT now(),
|
||||||
|
updated_at TIMESTAMPTZ NOT NULL DEFAULT now(),
|
||||||
|
deleted_at TIMESTAMPTZ,
|
||||||
|
|
||||||
|
CONSTRAINT users_email_unique UNIQUE (email),
|
||||||
|
CONSTRAINT users_email_format CHECK (email ~ '^[^@]+@[^@]+\.[^@]+$')
|
||||||
|
);
|
||||||
|
|
||||||
|
CREATE INDEX users_email_idx ON users (email) WHERE deleted_at IS NULL;
|
||||||
|
CREATE INDEX users_created_at_idx ON users (created_at DESC);
|
||||||
|
|
||||||
|
-- Orders table
|
||||||
|
CREATE TABLE orders (
|
||||||
|
id UUID PRIMARY KEY DEFAULT gen_random_uuid(),
|
||||||
|
user_id UUID NOT NULL REFERENCES users(id) ON DELETE RESTRICT,
|
||||||
|
status VARCHAR(20) NOT NULL DEFAULT 'pending',
|
||||||
|
total_cents INTEGER NOT NULL,
|
||||||
|
created_at TIMESTAMPTZ NOT NULL DEFAULT now(),
|
||||||
|
|
||||||
|
CONSTRAINT orders_status_valid CHECK (status IN ('pending', 'paid', 'shipped', 'cancelled')),
|
||||||
|
CONSTRAINT orders_total_positive CHECK (total_cents >= 0)
|
||||||
|
);
|
||||||
|
|
||||||
|
CREATE INDEX orders_user_id_idx ON orders (user_id);
|
||||||
|
CREATE INDEX orders_status_created_idx ON orders (status, created_at DESC);
|
||||||
|
```
|
||||||
|
|
||||||
|
## What Makes It Good
|
||||||
|
|
||||||
|
### Truth (Data P1)
|
||||||
|
- The schema reflects the domain: `users` have `email`, `name`, lifecycle timestamps. `orders` have `status`, `total`.
|
||||||
|
- No column named after a feature (`is_active_for_X`). No application state in the schema.
|
||||||
|
|
||||||
|
### Invariants in the Schema (Data P3)
|
||||||
|
- `NOT NULL` where required: `email`, `name`, `user_id`, `status`, `total_cents`.
|
||||||
|
- `UNIQUE (email)` — emails are unique. Enforced in the DB, defended in the app.
|
||||||
|
- `CHECK (status IN (...))` — status is a finite domain. Enforced in the DB.
|
||||||
|
- `CHECK (total_cents >= 0)` — totals are non-negative. Enforced in the DB.
|
||||||
|
- `REFERENCES users(id) ON DELETE RESTRICT` — you cannot delete a user with orders. Referential integrity (P9).
|
||||||
|
|
||||||
|
### Type Fidelity (Data P7)
|
||||||
|
- `id` is `UUID`, not `VARCHAR(36)`. The type matches the domain.
|
||||||
|
- `created_at` is `TIMESTAMPTZ`, not `VARCHAR` or `INTEGER`. Timezone-aware.
|
||||||
|
- `total_cents` is `INTEGER`, not `FLOAT`. Money in cents avoids floating point (P1 Truth).
|
||||||
|
- `status` is `VARCHAR(20)` with a CHECK, not a free `TEXT`. Bounded.
|
||||||
|
|
||||||
|
### Naming Consistency (Data P6)
|
||||||
|
- snake_case: `users`, `orders`, `user_id`, `created_at`.
|
||||||
|
- Foreign key: `user_id` (singular table + `_id`), not `uid` or `user`.
|
||||||
|
- Timestamps: `created_at`, `updated_at`, `deleted_at` — consistent suffix `_at`.
|
||||||
|
|
||||||
|
### Lifecycle Awareness (Data P8)
|
||||||
|
- `deleted_at` for soft delete. Lifecycle is first-class.
|
||||||
|
- The unique index on email is partial: `WHERE deleted_at IS NULL` — allows re-registration after soft delete.
|
||||||
|
- Every query must filter `deleted_at IS NULL` (a discipline, not a schema property).
|
||||||
|
|
||||||
|
### Indexing with Intent (Data P5, P10)
|
||||||
|
- `users_email_idx` — queries by email (login, lookup). Partial (excludes soft-deleted).
|
||||||
|
- `users_created_at_idx` — list users by recency. DESC matches the typical query.
|
||||||
|
- `orders_user_id_idx` — list a user's orders. FK index (join performance).
|
||||||
|
- `orders_status_created_idx` — composite for "open orders by recency" (`WHERE status = 'pending' ORDER BY created_at DESC`).
|
||||||
|
- No index on every column. Each index serves a query.
|
||||||
|
|
||||||
|
### Migration Safety (Data P4)
|
||||||
|
- This schema is created via a migration with an `up` and a `down`.
|
||||||
|
- The `down` drops the tables in reverse order (orders, then users) to respect FKs.
|
||||||
|
- Adding a column later uses expand-contract (nullable first, then constrained).
|
||||||
|
|
||||||
|
## What This Example Does NOT Do (And Why That's Good)
|
||||||
|
|
||||||
|
- Does not use `FLOAT` for money — floating point errors (P7, P1).
|
||||||
|
- Does not use `VARCHAR` for the UUID — wrong type (P7).
|
||||||
|
- Does not omit the FK on `orders.user_id` — unenforced relationship (P9).
|
||||||
|
- Does not index every column — write amplification (P10, C8).
|
||||||
|
- Does not use `is_deleted BOOLEAN` without a timestamp — loses the deletion time (P8).
|
||||||
|
- Does not allow `status` to be free text — would lose the finite domain (P3).
|
||||||
@@ -0,0 +1,139 @@
|
|||||||
|
# Good Example: Error Handler
|
||||||
|
|
||||||
|
> An error handler that follows Atelier's Errors principles. Each aspect cites the principle it satisfies.
|
||||||
|
|
||||||
|
## The Handler
|
||||||
|
|
||||||
|
```typescript
|
||||||
|
// Centralized error middleware
|
||||||
|
import { v4 as uuidv4 } from 'uuid';
|
||||||
|
|
||||||
|
type AppError = {
|
||||||
|
code: string;
|
||||||
|
message: string;
|
||||||
|
statusCode: number;
|
||||||
|
details?: Record<string, unknown>;
|
||||||
|
cause?: unknown;
|
||||||
|
};
|
||||||
|
|
||||||
|
function errorHandler(err: unknown, req: Request, res: Response, next: NextFunction) {
|
||||||
|
const requestId = req.headers['x-request-id'] as string || uuidv4();
|
||||||
|
|
||||||
|
const appError = normalizeError(err, requestId);
|
||||||
|
|
||||||
|
// Log with full context (server-side only)
|
||||||
|
logger.error({
|
||||||
|
request_id: requestId,
|
||||||
|
code: appError.code,
|
||||||
|
message: appError.message,
|
||||||
|
details: appError.details,
|
||||||
|
path: req.path,
|
||||||
|
method: req.method,
|
||||||
|
user_id: req.user?.id,
|
||||||
|
stack: err instanceof Error ? err.stack : undefined,
|
||||||
|
});
|
||||||
|
|
||||||
|
// Respond with safe, structured error
|
||||||
|
res.status(appError.statusCode).json({
|
||||||
|
error: {
|
||||||
|
code: appError.code,
|
||||||
|
message: appError.message,
|
||||||
|
request_id: requestId,
|
||||||
|
...(appError.details ? { details: appError.details } : {}),
|
||||||
|
},
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
|
function normalizeError(err: unknown, requestId: string): AppError {
|
||||||
|
// Known error types — preserve their code and status
|
||||||
|
if (err instanceof ValidationError) {
|
||||||
|
return {
|
||||||
|
code: err.code,
|
||||||
|
message: err.message,
|
||||||
|
statusCode: 422,
|
||||||
|
details: err.details,
|
||||||
|
};
|
||||||
|
}
|
||||||
|
if (err instanceof NotFoundError) {
|
||||||
|
return { code: err.code, message: err.message, statusCode: 404 };
|
||||||
|
}
|
||||||
|
if (err instanceof AuthError) {
|
||||||
|
return { code: 'AUTH_REQUIRED', message: 'Authentication required', statusCode: 401 };
|
||||||
|
}
|
||||||
|
|
||||||
|
// Unknown error — fail securely, do not leak internals
|
||||||
|
return {
|
||||||
|
code: 'INTERNAL_ERROR',
|
||||||
|
message: 'An internal error occurred',
|
||||||
|
statusCode: 500,
|
||||||
|
cause: err,
|
||||||
|
};
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
## What Makes It Good
|
||||||
|
|
||||||
|
### Errors are Data (Errors P1)
|
||||||
|
- Errors are normalized into a structured `AppError` type. They are values, not raw exceptions.
|
||||||
|
- The `code` is a stable, machine-consumable string (`VALIDATION_FAILED`, not a free message).
|
||||||
|
- The handler treats every error as data to be transformed, not a flow to be caught.
|
||||||
|
|
||||||
|
### Fail Loudly (Errors P2)
|
||||||
|
- No `catch (e) {}`. Every error reaches the handler.
|
||||||
|
- No silent swallowing. The error is logged and responded to.
|
||||||
|
- The middleware is centralized — every route uses it. No route can "forget" to handle errors.
|
||||||
|
|
||||||
|
### Fail Specifically (Errors P3)
|
||||||
|
- `ValidationError` → 422 with `code: err.code` (specific, e.g., `INVALID_EMAIL`).
|
||||||
|
- `NotFoundError` → 404 with `code: err.code` (e.g., `USER_NOT_FOUND`).
|
||||||
|
- Unknown → 500 with `INTERNAL_ERROR` (generic only when truly unknown).
|
||||||
|
- Never "Something went wrong" — always a specific code.
|
||||||
|
|
||||||
|
### Preserve Context (Errors P4)
|
||||||
|
- Server-side log includes: `request_id`, `code`, `message`, `details`, `path`, `method`, `user_id`, `stack`.
|
||||||
|
- Client-side response includes: `code`, `message`, `request_id`, `details`.
|
||||||
|
- The `request_id` ties the client response to the server log. Debuggable.
|
||||||
|
|
||||||
|
### Recoverable When Possible (Errors P5)
|
||||||
|
- 422 (validation) — the client can fix and retry.
|
||||||
|
- 401 (auth) — the client can re-auth and retry.
|
||||||
|
- 404 (not found) — the client can correct the ID.
|
||||||
|
- Only 500 is "the server has a bug" — not recoverable by the client.
|
||||||
|
|
||||||
|
### Unrecoverable Means Stop (Errors P6)
|
||||||
|
- A 500 does not limp on. It logs and returns.
|
||||||
|
- The server does not try to "recover" from an unknown error by guessing. It fails fast.
|
||||||
|
|
||||||
|
### Errors are Boundaries (Errors P7)
|
||||||
|
- The middleware is the boundary. Internal error types (`ValidationError`, `NotFoundError`) are translated to HTTP responses here.
|
||||||
|
- Internal code throws domain errors; the boundary translates to HTTP. No internal error type leaks to the client.
|
||||||
|
|
||||||
|
### User-Facing Errors are UX (Errors P8)
|
||||||
|
- `message` is for the developer (log + response). The client UI renders a user-facing message based on `code`.
|
||||||
|
- `INTERNAL_ERROR` → "Something went wrong. We're on it." (user-facing), not the stack.
|
||||||
|
|
||||||
|
### Errors are Logged (Errors P9)
|
||||||
|
- Every error is logged at ERROR level with full context.
|
||||||
|
- The handling is the recovery (return a response); the log is the memory (investigate later).
|
||||||
|
|
||||||
|
### Errors Don't Lie (Errors P10)
|
||||||
|
- The status code matches the error type. 422 for validation, not 200.
|
||||||
|
- `INTERNAL_ERROR` is returned only for unknown errors. Known errors get their specific code.
|
||||||
|
- Never claim success on failure. Never claim failure on success.
|
||||||
|
|
||||||
|
### Security (Security P5 Output Safety, P9 Secret Hygiene)
|
||||||
|
- Unknown errors return `INTERNAL_ERROR` with no internal details. No stack trace to the client.
|
||||||
|
- The stack is logged server-side, never sent to the client.
|
||||||
|
- The error message does not echo the input (which may contain a token).
|
||||||
|
|
||||||
|
### Observability (Observability P2 Correlation, P3 Context)
|
||||||
|
- `request_id` on every error. Correlatable across services.
|
||||||
|
- Sufficient context in the log: path, method, user_id. "What was the user doing?" is answerable.
|
||||||
|
|
||||||
|
## What This Example Does NOT Do (And Why That's Good)
|
||||||
|
|
||||||
|
- Does not `catch (e) { return null }` — silent failure (P2).
|
||||||
|
- Does not return 200 with `{ error: ... }` — the status code lies (API P9).
|
||||||
|
- Does not send the stack trace to the client — information leak (Security P5).
|
||||||
|
- Does not log the request body — may contain secrets (Security P9, Observability P6).
|
||||||
|
- Does not use a generic `Error("error")` — not specific (P3).
|
||||||
@@ -0,0 +1,108 @@
|
|||||||
|
# Good Example: React Component
|
||||||
|
|
||||||
|
> A UI component that follows Atelier's UI/UX principles. Each aspect cites the principle it satisfies.
|
||||||
|
|
||||||
|
## The Component
|
||||||
|
|
||||||
|
```tsx
|
||||||
|
import { useId, useState } from 'react';
|
||||||
|
import { Button } from './Button';
|
||||||
|
import { Spinner } from './Spinner';
|
||||||
|
|
||||||
|
type DeleteButtonProps = {
|
||||||
|
/** The resource name to display in the confirmation */
|
||||||
|
resourceName: string;
|
||||||
|
/** Called when the user confirms deletion */
|
||||||
|
onDelete: () => Promise<void>;
|
||||||
|
};
|
||||||
|
|
||||||
|
export function DeleteButton({ resourceName, onDelete }: DeleteButtonProps) {
|
||||||
|
const [isConfirming, setIsConfirming] = useState(false);
|
||||||
|
const [isDeleting, setIsDeleting] = useState(false);
|
||||||
|
const confirmId = useId();
|
||||||
|
|
||||||
|
const handleConfirm = async () => {
|
||||||
|
setIsDeleting(true);
|
||||||
|
try {
|
||||||
|
await onDelete();
|
||||||
|
} finally {
|
||||||
|
setIsDeleting(false);
|
||||||
|
setIsConfirming(false);
|
||||||
|
}
|
||||||
|
};
|
||||||
|
|
||||||
|
if (isConfirming) {
|
||||||
|
return (
|
||||||
|
<span role="group" aria-labelledby={confirmId}>
|
||||||
|
<span id={confirmId}>Delete {resourceName}? This cannot be undone.</span>
|
||||||
|
<Button variant="danger" onClick={handleConfirm} disabled={isDeleting}>
|
||||||
|
{isDeleting ? <Spinner label="Deleting" /> : 'Yes, delete'}
|
||||||
|
</Button>
|
||||||
|
<Button variant="ghost" onClick={() => setIsConfirming(false)} disabled={isDeleting}>
|
||||||
|
Cancel
|
||||||
|
</Button>
|
||||||
|
</span>
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
return (
|
||||||
|
<Button variant="danger" onClick={() => setIsConfirming(true)}>
|
||||||
|
Delete
|
||||||
|
</Button>
|
||||||
|
);
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
## What Makes It Good
|
||||||
|
|
||||||
|
### Single Responsibility (components.md §1)
|
||||||
|
- The component does one thing: confirm and trigger a deletion. No "And" in the name.
|
||||||
|
- The `onDelete` callback is the single output. The component owns the confirmation UI, not the deletion logic.
|
||||||
|
|
||||||
|
### Explicit Boundaries (components.md §3)
|
||||||
|
- Props are typed (`DeleteButtonProps`). Required props are required.
|
||||||
|
- `onDelete` returns a `Promise<void>` — the caller knows it's async.
|
||||||
|
- The component never reads global state. It receives `resourceName` and `onDelete`.
|
||||||
|
|
||||||
|
### Predictable State (components.md §4)
|
||||||
|
- `isConfirming` and `isDeleting` are owned by the component (only it cares).
|
||||||
|
- State is not duplicated. The parent does not know about confirmation.
|
||||||
|
- The component transitions: idle → confirming → deleting → idle.
|
||||||
|
|
||||||
|
### Render Purity (components.md §5)
|
||||||
|
- Given the same props and state, the component renders the same output.
|
||||||
|
- Side effects (`onDelete`) are in the event handler, not in render.
|
||||||
|
- `useId()` is deterministic per component instance (React guarantee).
|
||||||
|
|
||||||
|
### Accessible by Default (components.md §6, uiux P2)
|
||||||
|
- The confirmation group has `role="group"` and `aria-labelledby`.
|
||||||
|
- The Spinner has a `label` (screen reader announces "Deleting").
|
||||||
|
- Buttons have text labels (not icon-only).
|
||||||
|
- Focus order is logical (confirm → cancel).
|
||||||
|
- Keyboard-reachable (buttons are natively focusable).
|
||||||
|
|
||||||
|
### Forgiveness (UI/UX P5, P10 Reversibility)
|
||||||
|
- Destructive action requires confirmation (P5).
|
||||||
|
- "This cannot be undone" names the consequence (P3 Clarity).
|
||||||
|
- "Cancel" is offered and is not disabled during deletion (the user can cancel the *next* action).
|
||||||
|
- The state is reversible: `isConfirming` can be set back to `false` (P10).
|
||||||
|
|
||||||
|
### Style via Tokens (components.md §7)
|
||||||
|
- `variant="danger"` and `variant="ghost"` reference design tokens, not raw colors.
|
||||||
|
- No `style={{ color: 'red' }}` — the token system owns the visual.
|
||||||
|
|
||||||
|
### Feedback (UI/UX P4)
|
||||||
|
- The button shows a Spinner while deleting (P4, P6 Performance perception).
|
||||||
|
- The button is disabled while deleting (prevents double-click).
|
||||||
|
- The label changes: "Yes, delete" → Spinner (state is communicated).
|
||||||
|
|
||||||
|
### Clarity (UI/UX P3)
|
||||||
|
- "Delete {resourceName}? This cannot be undone." — specific, names the resource and the consequence.
|
||||||
|
- No "Are you sure?" — vague. No "Submit" — wrong verb.
|
||||||
|
|
||||||
|
## What This Example Does NOT Do (And Why That's Good)
|
||||||
|
|
||||||
|
- Does not use a `window.confirm()` dialog — not accessible, not styled, not composable.
|
||||||
|
- Does not render a modal — the inline confirmation is lighter and less disruptive (P9 Simplicity).
|
||||||
|
- Does not auto-delete on click — forgiveness (P5).
|
||||||
|
- Does not hardcode "Project" — the resource name is a prop (composability, components.md §3).
|
||||||
Reference in New Issue
Block a user