feat: add architecture validation system with comprehensive pattern enforcement
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>
This commit is contained in:
611
docs/architecture/architecture-patterns.md
Normal file
611
docs/architecture/architecture-patterns.md
Normal file
@@ -0,0 +1,611 @@
|
||||
# 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!
|
||||
Reference in New Issue
Block a user