Implemented automated architecture validation to enforce design decisions: Architecture Validation System: - Created .architecture-rules.yaml with comprehensive rule definitions - Implemented validate_architecture.py script with AST-based validation - Added pre-commit hook configuration for automatic validation - Comprehensive documentation in docs/architecture/architecture-patterns.md Key Design Rules Enforced: - API-001 to API-004: API endpoint patterns (Pydantic models, no business logic, exception handling, auth) - SVC-001 to SVC-004: Service layer patterns (domain exceptions, db session params, no HTTP concerns) - MDL-001 to MDL-002: Model separation (SQLAlchemy vs Pydantic) - EXC-001 to EXC-002: Exception handling (custom exceptions, no bare except) - JS-001 to JS-003: JavaScript patterns (apiClient, logger, Alpine components) - TPL-001: Template patterns (extend base.html) Features: - Validates separation of concerns (routes vs services vs models) - Enforces proper exception handling (domain exceptions in services, HTTP in routes) - Checks database session patterns and Pydantic model usage - JavaScript and template validation - Detailed error reporting with suggestions - Integration with pre-commit hooks and CI/CD UI Fix: - Fixed icon names in content-pages.html (pencil→edit, trash→delete) Documentation: - Added architecture patterns guide with examples - Created scripts/README.md for validator usage - Updated mkdocs.yml with architecture documentation - Built and verified documentation successfully Usage: python scripts/validate_architecture.py # Validate all python scripts/validate_architecture.py --verbose # With details python scripts/validate_architecture.py --errors-only # Errors only 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
612 lines
18 KiB
Markdown
612 lines
18 KiB
Markdown
# Architecture Patterns & Design Decisions
|
|
|
|
This document describes the architectural patterns and design decisions that must be followed throughout the codebase.
|
|
|
|
> **Note:** These patterns are enforced automatically by `scripts/validate_architecture.py`. Run the validator before committing code.
|
|
|
|
---
|
|
|
|
## Table of Contents
|
|
|
|
1. [Core Principles](#core-principles)
|
|
2. [Layered Architecture](#layered-architecture)
|
|
3. [API Endpoint Patterns](#api-endpoint-patterns)
|
|
4. [Service Layer Patterns](#service-layer-patterns)
|
|
5. [Model Patterns](#model-patterns)
|
|
6. [Exception Handling](#exception-handling)
|
|
7. [JavaScript Patterns](#javascript-patterns)
|
|
8. [Validation](#validation)
|
|
|
|
---
|
|
|
|
## Core Principles
|
|
|
|
### 1. Separation of Concerns
|
|
|
|
**Each layer has specific responsibilities:**
|
|
|
|
- **Routes/Endpoints**: HTTP handling, validation, authentication, response formatting
|
|
- **Services**: Business logic, data processing, orchestration
|
|
- **Models**: Data structure and persistence
|
|
|
|
**❌ Bad Example - Business logic in endpoint:**
|
|
|
|
```python
|
|
@router.post("/vendors")
|
|
async def create_vendor(vendor: VendorCreate, db: Session = Depends(get_db)):
|
|
# ❌ BAD: Business logic in endpoint
|
|
if db.query(Vendor).filter(Vendor.subdomain == vendor.subdomain).first():
|
|
raise HTTPException(status_code=409, detail="Vendor exists")
|
|
|
|
db_vendor = Vendor(**vendor.dict())
|
|
db.add(db_vendor)
|
|
db.commit()
|
|
db.refresh(db_vendor)
|
|
return db_vendor
|
|
```
|
|
|
|
**✅ Good Example - Delegated to service:**
|
|
|
|
```python
|
|
@router.post("/vendors", response_model=VendorResponse)
|
|
async def create_vendor(
|
|
vendor: VendorCreate,
|
|
current_user: User = Depends(get_current_admin),
|
|
db: Session = Depends(get_db)
|
|
):
|
|
try:
|
|
# ✅ GOOD: Delegate to service
|
|
result = vendor_service.create_vendor(db, vendor)
|
|
return result
|
|
except VendorAlreadyExistsError as e:
|
|
raise HTTPException(status_code=409, detail=str(e))
|
|
except Exception as e:
|
|
logger.error(f"Failed to create vendor: {e}")
|
|
raise HTTPException(status_code=500, detail="Internal server error")
|
|
```
|
|
|
|
### 2. Type Safety
|
|
|
|
Use Pydantic for API validation, SQLAlchemy for database models.
|
|
|
|
### 3. Proper Exception Handling
|
|
|
|
Services throw domain exceptions, routes convert to HTTP responses.
|
|
|
|
---
|
|
|
|
## Layered Architecture
|
|
|
|
```
|
|
┌─────────────────────────────────────────────────────────┐
|
|
│ API Layer │
|
|
│ (app/api/v1/**/*.py, app/routes/**/*.py) │
|
|
│ │
|
|
│ Responsibilities: │
|
|
│ - HTTP request/response handling │
|
|
│ - Authentication/Authorization │
|
|
│ - Input validation (Pydantic models) │
|
|
│ - Exception handling (domain → HTTP) │
|
|
│ │
|
|
│ ❌ Should NOT: │
|
|
│ - Contain business logic │
|
|
│ - Directly access database (except via services) │
|
|
│ - Raise domain exceptions │
|
|
└─────────────────────────────────────────────────────────┘
|
|
↓
|
|
┌─────────────────────────────────────────────────────────┐
|
|
│ Service Layer │
|
|
│ (app/services/**/*.py) │
|
|
│ │
|
|
│ Responsibilities: │
|
|
│ - Business logic │
|
|
│ - Data validation (business rules) │
|
|
│ - Database operations │
|
|
│ - Orchestration of multiple operations │
|
|
│ │
|
|
│ ❌ Should NOT: │
|
|
│ - Know about HTTP (no HTTPException) │
|
|
│ - Create database sessions (accept as parameter) │
|
|
│ - Handle HTTP-specific concerns │
|
|
└─────────────────────────────────────────────────────────┘
|
|
↓
|
|
┌─────────────────────────────────────────────────────────┐
|
|
│ Data Layer │
|
|
│ (app/models/**/*.py) │
|
|
│ │
|
|
│ Responsibilities: │
|
|
│ - Database schema (SQLAlchemy models) │
|
|
│ - API schemas (Pydantic models) │
|
|
│ - Data structure definitions │
|
|
│ │
|
|
│ ❌ Should NOT: │
|
|
│ - Mix SQLAlchemy and Pydantic in same class │
|
|
│ - Contain business logic │
|
|
└─────────────────────────────────────────────────────────┘
|
|
```
|
|
|
|
---
|
|
|
|
## API Endpoint Patterns
|
|
|
|
### Rule API-001: Use Pydantic Models
|
|
|
|
**All endpoints MUST use Pydantic models for request/response.**
|
|
|
|
```python
|
|
# ✅ GOOD: Pydantic models for type safety
|
|
class VendorCreate(BaseModel):
|
|
name: str = Field(..., max_length=200)
|
|
subdomain: str = Field(..., max_length=100)
|
|
is_active: bool = True
|
|
|
|
class VendorResponse(BaseModel):
|
|
id: int
|
|
name: str
|
|
subdomain: str
|
|
created_at: datetime
|
|
|
|
class Config:
|
|
from_attributes = True # For SQLAlchemy compatibility
|
|
|
|
@router.post("/vendors", response_model=VendorResponse)
|
|
async def create_vendor(vendor: VendorCreate, db: Session = Depends(get_db)):
|
|
result = vendor_service.create_vendor(db, vendor)
|
|
return result
|
|
```
|
|
|
|
```python
|
|
# ❌ BAD: Raw dict, no validation
|
|
@router.post("/vendors")
|
|
async def create_vendor(data: dict):
|
|
return {"name": data["name"]} # No type safety!
|
|
```
|
|
|
|
### Rule API-002: No Business Logic in Endpoints
|
|
|
|
**Endpoints should only handle HTTP concerns.**
|
|
|
|
```python
|
|
# ✅ GOOD: Delegate to service
|
|
@router.post("/vendors")
|
|
async def create_vendor(vendor: VendorCreate, db: Session = Depends(get_db)):
|
|
result = vendor_service.create_vendor(db, vendor)
|
|
return result
|
|
```
|
|
|
|
```python
|
|
# ❌ BAD: Business logic in endpoint
|
|
@router.post("/vendors")
|
|
async def create_vendor(vendor: VendorCreate, db: Session = Depends(get_db)):
|
|
# ❌ Database operations belong in service!
|
|
db_vendor = Vendor(**vendor.dict())
|
|
db.add(db_vendor)
|
|
db.commit()
|
|
return db_vendor
|
|
```
|
|
|
|
### Rule API-003: Proper Exception Handling
|
|
|
|
**Catch service exceptions and convert to HTTPException.**
|
|
|
|
```python
|
|
# ✅ GOOD: Proper exception handling
|
|
@router.post("/vendors", response_model=VendorResponse)
|
|
async def create_vendor(vendor: VendorCreate, db: Session = Depends(get_db)):
|
|
try:
|
|
result = vendor_service.create_vendor(db, vendor)
|
|
return result
|
|
except VendorAlreadyExistsError as e:
|
|
raise HTTPException(status_code=409, detail=str(e))
|
|
except ValueError as e:
|
|
raise HTTPException(status_code=400, detail=str(e))
|
|
except Exception as e:
|
|
logger.error(f"Unexpected error creating vendor: {e}")
|
|
raise HTTPException(status_code=500, detail="Internal server error")
|
|
```
|
|
|
|
### Rule API-004: Authentication
|
|
|
|
**Protected endpoints must use dependency injection for auth.**
|
|
|
|
```python
|
|
# ✅ GOOD: Use Depends for auth
|
|
@router.post("/vendors")
|
|
async def create_vendor(
|
|
vendor: VendorCreate,
|
|
current_user: User = Depends(get_current_admin), # ✅ Auth required
|
|
db: Session = Depends(get_db)
|
|
):
|
|
result = vendor_service.create_vendor(db, vendor)
|
|
return result
|
|
```
|
|
|
|
---
|
|
|
|
## Service Layer Patterns
|
|
|
|
### Rule SVC-001: No HTTPException in Services
|
|
|
|
**Services should NOT know about HTTP. Raise domain exceptions instead.**
|
|
|
|
```python
|
|
# ✅ GOOD: Domain exception
|
|
class VendorAlreadyExistsError(Exception):
|
|
"""Raised when vendor with same subdomain already exists"""
|
|
pass
|
|
|
|
class VendorService:
|
|
def create_vendor(self, db: Session, vendor_data: VendorCreate):
|
|
if self._vendor_exists(db, vendor_data.subdomain):
|
|
raise VendorAlreadyExistsError(
|
|
f"Vendor with subdomain '{vendor_data.subdomain}' already exists"
|
|
)
|
|
|
|
# Business logic...
|
|
vendor = Vendor(**vendor_data.dict())
|
|
db.add(vendor)
|
|
db.commit()
|
|
return vendor
|
|
```
|
|
|
|
```python
|
|
# ❌ BAD: HTTPException in service
|
|
class VendorService:
|
|
def create_vendor(self, db: Session, vendor_data: VendorCreate):
|
|
if self._vendor_exists(db, vendor_data.subdomain):
|
|
# ❌ Service shouldn't know about HTTP!
|
|
raise HTTPException(status_code=409, detail="Vendor exists")
|
|
```
|
|
|
|
### Rule SVC-002: Create Custom Exception Classes
|
|
|
|
**Don't use generic `Exception`. Create specific domain exceptions.**
|
|
|
|
```python
|
|
# ✅ GOOD: Specific exceptions
|
|
class VendorError(Exception):
|
|
"""Base exception for vendor-related errors"""
|
|
pass
|
|
|
|
class VendorNotFoundError(VendorError):
|
|
"""Raised when vendor is not found"""
|
|
pass
|
|
|
|
class VendorAlreadyExistsError(VendorError):
|
|
"""Raised when vendor already exists"""
|
|
pass
|
|
|
|
class VendorService:
|
|
def get_vendor(self, db: Session, vendor_code: str):
|
|
vendor = db.query(Vendor).filter(Vendor.vendor_code == vendor_code).first()
|
|
if not vendor:
|
|
raise VendorNotFoundError(f"Vendor '{vendor_code}' not found")
|
|
return vendor
|
|
```
|
|
|
|
```python
|
|
# ❌ BAD: Generic Exception
|
|
class VendorService:
|
|
def get_vendor(self, db: Session, vendor_code: str):
|
|
vendor = db.query(Vendor).filter(Vendor.vendor_code == vendor_code).first()
|
|
if not vendor:
|
|
raise Exception("Vendor not found") # ❌ Too generic!
|
|
return vendor
|
|
```
|
|
|
|
### Rule SVC-003: Database Session as Parameter
|
|
|
|
**Services should receive database session as parameter, not create it internally.**
|
|
|
|
```python
|
|
# ✅ GOOD: db session as parameter
|
|
class VendorService:
|
|
def create_vendor(self, db: Session, vendor_data: VendorCreate):
|
|
vendor = Vendor(**vendor_data.dict())
|
|
db.add(vendor)
|
|
db.commit()
|
|
db.refresh(vendor)
|
|
return vendor
|
|
|
|
def _vendor_exists(self, db: Session, subdomain: str) -> bool:
|
|
return db.query(Vendor).filter(Vendor.subdomain == subdomain).first() is not None
|
|
```
|
|
|
|
```python
|
|
# ❌ BAD: Creating session internally
|
|
class VendorService:
|
|
def create_vendor(self, vendor_data: VendorCreate):
|
|
# ❌ Don't create session here - makes testing hard
|
|
db = SessionLocal()
|
|
vendor = Vendor(**vendor_data.dict())
|
|
db.add(vendor)
|
|
db.commit()
|
|
return vendor
|
|
```
|
|
|
|
**Benefits:**
|
|
|
|
- Testability (can inject mock session)
|
|
- Transaction control (caller controls commit/rollback)
|
|
- Resource management (caller handles session lifecycle)
|
|
|
|
### Rule SVC-004: Use Pydantic for Input Validation
|
|
|
|
**Service methods should accept Pydantic models for complex inputs.**
|
|
|
|
```python
|
|
# ✅ GOOD: Pydantic model ensures validation
|
|
class VendorService:
|
|
def create_vendor(self, db: Session, vendor_data: VendorCreate):
|
|
# vendor_data is already validated by Pydantic
|
|
vendor = Vendor(**vendor_data.dict())
|
|
db.add(vendor)
|
|
db.commit()
|
|
return vendor
|
|
```
|
|
|
|
---
|
|
|
|
## Model Patterns
|
|
|
|
### Rule MDL-001: SQLAlchemy for Database Models
|
|
|
|
```python
|
|
# ✅ GOOD: SQLAlchemy model
|
|
from sqlalchemy import Column, Integer, String, Boolean
|
|
from app.database import Base
|
|
|
|
class Vendor(Base):
|
|
__tablename__ = "vendors"
|
|
|
|
id = Column(Integer, primary_key=True, index=True)
|
|
name = Column(String(200), nullable=False)
|
|
subdomain = Column(String(100), unique=True, nullable=False)
|
|
is_active = Column(Boolean, default=True)
|
|
```
|
|
|
|
### Rule MDL-002: Separate Pydantic from SQLAlchemy
|
|
|
|
**NEVER mix SQLAlchemy and Pydantic in the same class.**
|
|
|
|
```python
|
|
# ❌ BAD: Mixing SQLAlchemy and Pydantic
|
|
class Vendor(Base, BaseModel): # ❌ Don't do this!
|
|
__tablename__ = "vendors"
|
|
name: str = Column(String(200))
|
|
```
|
|
|
|
```python
|
|
# ✅ GOOD: Separate models
|
|
# Database model (app/models/vendor.py)
|
|
class Vendor(Base):
|
|
__tablename__ = "vendors"
|
|
id = Column(Integer, primary_key=True)
|
|
name = Column(String(200), nullable=False)
|
|
|
|
# API model (app/api/v1/admin/vendors.py)
|
|
class VendorCreate(BaseModel):
|
|
name: str = Field(..., max_length=200)
|
|
|
|
class VendorResponse(BaseModel):
|
|
id: int
|
|
name: str
|
|
|
|
class Config:
|
|
from_attributes = True
|
|
```
|
|
|
|
---
|
|
|
|
## Exception Handling
|
|
|
|
### Rule EXC-001: Domain-Specific Exceptions
|
|
|
|
**Create exception hierarchy in `app/exceptions/`**
|
|
|
|
```python
|
|
# app/exceptions/vendor_exceptions.py
|
|
|
|
class VendorError(Exception):
|
|
"""Base exception for vendor domain"""
|
|
pass
|
|
|
|
class VendorNotFoundError(VendorError):
|
|
"""Vendor does not exist"""
|
|
pass
|
|
|
|
class VendorAlreadyExistsError(VendorError):
|
|
"""Vendor already exists"""
|
|
pass
|
|
|
|
class VendorValidationError(VendorError):
|
|
"""Vendor data validation failed"""
|
|
pass
|
|
```
|
|
|
|
### Rule EXC-002: Never Use Bare Except
|
|
|
|
```python
|
|
# ❌ BAD: Bare except
|
|
try:
|
|
result = do_something()
|
|
except: # ❌ Catches EVERYTHING including KeyboardInterrupt!
|
|
pass
|
|
```
|
|
|
|
```python
|
|
# ✅ GOOD: Specific exceptions
|
|
try:
|
|
result = do_something()
|
|
except ValueError as e:
|
|
logger.error(f"Validation error: {e}")
|
|
except DatabaseError as e:
|
|
logger.error(f"Database error: {e}")
|
|
except Exception as e:
|
|
logger.error(f"Unexpected error: {e}")
|
|
raise
|
|
```
|
|
|
|
---
|
|
|
|
## JavaScript Patterns
|
|
|
|
### Rule JS-001: Use apiClient Directly
|
|
|
|
```javascript
|
|
// ✅ GOOD
|
|
const vendors = await apiClient.get('/api/v1/vendors');
|
|
```
|
|
|
|
```javascript
|
|
// ❌ BAD
|
|
const vendors = await window.apiClient.get('/api/v1/vendors');
|
|
```
|
|
|
|
### Rule JS-002: Use Centralized Logger
|
|
|
|
```javascript
|
|
// ✅ GOOD: Centralized logger
|
|
const vendorLog = window.LogConfig.createLogger('vendors');
|
|
vendorLog.info('Loading vendors...');
|
|
vendorLog.error('Failed to load vendors:', error);
|
|
```
|
|
|
|
```javascript
|
|
// ❌ BAD: console
|
|
console.log('Loading vendors...'); // ❌ Use logger instead
|
|
```
|
|
|
|
### Rule JS-003: Alpine Components Pattern
|
|
|
|
```javascript
|
|
// ✅ GOOD: Proper Alpine component
|
|
function vendorsManager() {
|
|
return {
|
|
// ✅ Inherit base layout functionality
|
|
...data(),
|
|
|
|
// ✅ Set page identifier for sidebar
|
|
currentPage: 'vendors',
|
|
|
|
// Component state
|
|
vendors: [],
|
|
loading: false,
|
|
|
|
// ✅ Init with guard
|
|
async init() {
|
|
if (window._vendorsInitialized) {
|
|
return;
|
|
}
|
|
window._vendorsInitialized = true;
|
|
|
|
await this.loadVendors();
|
|
}
|
|
};
|
|
}
|
|
```
|
|
|
|
---
|
|
|
|
## Validation
|
|
|
|
### Running the Validator
|
|
|
|
```bash
|
|
# Validate entire codebase
|
|
python scripts/validate_architecture.py
|
|
|
|
# Validate specific directory
|
|
python scripts/validate_architecture.py app/api/
|
|
|
|
# Verbose output with context
|
|
python scripts/validate_architecture.py --verbose
|
|
|
|
# Errors only (suppress warnings)
|
|
python scripts/validate_architecture.py --errors-only
|
|
```
|
|
|
|
### Pre-commit Hook
|
|
|
|
Install pre-commit to validate automatically before commits:
|
|
|
|
```bash
|
|
# Install pre-commit
|
|
pip install pre-commit
|
|
|
|
# Setup hooks
|
|
pre-commit install
|
|
|
|
# Run manually
|
|
pre-commit run --all-files
|
|
```
|
|
|
|
### CI/CD Integration
|
|
|
|
Add to your CI pipeline:
|
|
|
|
```yaml
|
|
# .github/workflows/ci.yml
|
|
- name: Validate Architecture
|
|
run: |
|
|
python scripts/validate_architecture.py
|
|
```
|
|
|
|
---
|
|
|
|
## Quick Reference
|
|
|
|
| Layer | Responsibility | Can Use | Cannot Use |
|
|
|-------|---------------|---------|------------|
|
|
| **API Endpoints** | HTTP handling, auth, validation | Pydantic, HTTPException, Depends | Direct DB access, business logic |
|
|
| **Services** | Business logic, orchestration | DB session, domain exceptions | HTTPException, HTTP concepts |
|
|
| **Models** | Data structure | SQLAlchemy OR Pydantic | Mixing both in same class |
|
|
|
|
---
|
|
|
|
## Common Violations and Fixes
|
|
|
|
### Violation: Business logic in endpoint
|
|
|
|
```python
|
|
# Before
|
|
@router.post("/vendors")
|
|
async def create_vendor(vendor: VendorCreate, db: Session = Depends(get_db)):
|
|
db_vendor = Vendor(**vendor.dict())
|
|
db.add(db_vendor)
|
|
db.commit()
|
|
return db_vendor
|
|
```
|
|
|
|
```python
|
|
# After ✅
|
|
@router.post("/vendors")
|
|
async def create_vendor(vendor: VendorCreate, db: Session = Depends(get_db)):
|
|
try:
|
|
return vendor_service.create_vendor(db, vendor)
|
|
except VendorAlreadyExistsError as e:
|
|
raise HTTPException(status_code=409, detail=str(e))
|
|
```
|
|
|
|
### Violation: HTTPException in service
|
|
|
|
```python
|
|
# Before
|
|
class VendorService:
|
|
def create_vendor(self, db: Session, vendor_data):
|
|
if exists:
|
|
raise HTTPException(status_code=409, detail="Exists")
|
|
```
|
|
|
|
```python
|
|
# After ✅
|
|
class VendorService:
|
|
def create_vendor(self, db: Session, vendor_data):
|
|
if exists:
|
|
raise VendorAlreadyExistsError("Vendor already exists")
|
|
```
|
|
|
|
---
|
|
|
|
**Remember:** These patterns are enforced automatically. Run `python scripts/validate_architecture.py` before committing!
|