From 4279c458c47e4407fe9dceb95830876fdcaf11f5 Mon Sep 17 00:00:00 2001 From: Samir Boulahtit Date: Fri, 28 Nov 2025 20:03:45 +0100 Subject: [PATCH] docs: add comprehensive architecture rules reference documentation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Created detailed documentation for all 51 architectural rules: New Documentation: - docs/development/architecture-rules.md (comprehensive reference) - Added to mkdocs.yml navigation Documentation Includes: - Overview and usage instructions - Severity level explanations - All 51 rules organized by category: * Backend Rules (API, Service, Model, Exception) * Frontend Rules (JavaScript, Templates, Styling) * Naming Convention Rules * Security & Multi-Tenancy Rules * Code Quality Rules * Middleware Rules For Each Rule: - Rule ID and name - Severity level - Detailed description - Good and bad code examples - Anti-patterns detected - File patterns affected Additional Sections: - Quick reference tables - Pre-commit checklist - Common violations and fixes - Ignored patterns explanation - Summary statistics (51 rules, 35 errors, 16 warnings) This documentation complements: - .architecture-rules.yaml (rule definitions) - scripts/validate_architecture.py (enforcement) - Code Quality guide - Contributing guide Documentation builds successfully with mkdocs. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- docs/development/architecture-rules.md | 796 +++++++++++++++++++++++++ mkdocs.yml | 1 + models/database/base.py | 2 + 3 files changed, 799 insertions(+) create mode 100644 docs/development/architecture-rules.md diff --git a/docs/development/architecture-rules.md b/docs/development/architecture-rules.md new file mode 100644 index 00000000..f2cb0242 --- /dev/null +++ b/docs/development/architecture-rules.md @@ -0,0 +1,796 @@ +# Architecture Rules Reference + +This document provides a comprehensive reference for all architectural rules enforced by the `scripts/validate_architecture.py` validator. + +## Overview + +The architecture validator ensures consistent patterns, separation of concerns, and best practices across the entire codebase. Rules are organized by category and severity level. + +**Version:** 2.0 +**Total Rules:** 50+ +**Configuration File:** `.architecture-rules.yaml` + +## Running the Validator + +```bash +# Check all files +python scripts/validate_architecture.py + +# Check specific directory +python scripts/validate_architecture.py app/api/ + +# Verbose output +python scripts/validate_architecture.py --verbose + +# Auto-fix where possible +python scripts/validate_architecture.py --fix +``` + +## Severity Levels + +| Severity | Description | Exit Code | Action Required | +|----------|-------------|-----------|-----------------| +| **Error** | Critical architectural violation | 1 | Must fix before committing | +| **Warning** | Pattern deviation | 0 | Should fix, doesn't block | +| **Info** | Suggestion for improvement | 0 | Optional improvement | + +--- + +## Core Architectural Principles + +1. **Separation of Concerns** - API endpoints handle HTTP, services handle business logic +2. **Layered Architecture** - Routes → Services → Models +3. **Type Safety** - Pydantic for API, SQLAlchemy for database +4. **Proper Exception Handling** - Domain exceptions in services, HTTPException in routes +5. **Multi-Tenancy** - All queries scoped to vendor_id +6. **Consistent Naming** - API files: plural, Services: singular+service, Models: singular + +--- + +## Backend Rules + +### API Endpoint Rules (app/api/v1/**/*.py) + +#### API-001: Use Pydantic Models for Request/Response +**Severity:** Error + +All API endpoints must use Pydantic models (BaseModel) for request bodies and response models. Never use raw dicts or SQLAlchemy models directly. + +```python +# ✅ Good +@router.post("/vendors", response_model=VendorResponse) +async def create_vendor(vendor: VendorCreate): + return vendor_service.create_vendor(db, vendor) + +# ❌ Bad +@router.post("/vendors") +async def create_vendor(data: dict): + return {"name": data["name"]} # No validation! +``` + +#### API-002: No Business Logic in Endpoints +**Severity:** Error + +API endpoints should only handle HTTP concerns (validation, auth, response formatting). All business logic must be delegated to service layer. + +```python +# ✅ Good +@router.post("/vendors") +async def create_vendor(vendor: VendorCreate, db: Session = Depends(get_db)): + result = vendor_service.create_vendor(db, vendor) + return result + +# ❌ Bad +@router.post("/vendors") +async def create_vendor(vendor: VendorCreate, db: Session = Depends(get_db)): + db_vendor = Vendor(name=vendor.name) + db.add(db_vendor) # Business logic in endpoint! + db.commit() + return db_vendor +``` + +**Anti-patterns detected:** +- `db.add(` +- `db.commit()` +- `db.query(` + +#### API-003: Catch Service Exceptions +**Severity:** Error + +API endpoints must catch domain exceptions from services and convert them to appropriate HTTPException with proper status codes. + +```python +# ✅ Good +@router.post("/vendors") +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 Exception as e: + logger.error(f"Unexpected error: {e}") + raise HTTPException(status_code=500, detail="Internal server error") +``` + +#### API-004: Proper Authentication +**Severity:** Warning + +Protected endpoints must use Depends() for authentication. + +```python +# ✅ Good +@router.post("/vendors") +async def create_vendor( + vendor: VendorCreate, + current_user: User = Depends(get_current_admin), + db: Session = Depends(get_db) +): + pass +``` + +#### API-005: Multi-Tenant Scoping +**Severity:** Error + +All queries in vendor/shop contexts must filter by vendor_id. + +--- + +### Service Layer Rules (app/services/**/*.py) + +#### SVC-001: No HTTPException in Services +**Severity:** Error + +Services are business logic layer - they should NOT know about HTTP. Raise domain-specific exceptions instead. + +```python +# ✅ Good +class VendorService: + def create_vendor(self, db: Session, vendor_data): + if self._vendor_exists(db, vendor_data.subdomain): + raise VendorAlreadyExistsError(f"Vendor {vendor_data.subdomain} exists") + +# ❌ Bad +class VendorService: + def create_vendor(self, db: Session, vendor_data): + if self._vendor_exists(db, vendor_data.subdomain): + raise HTTPException(status_code=409, detail="Vendor exists") # BAD! +``` + +#### SVC-002: Use Proper Exception Handling +**Severity:** Error + +Services should raise meaningful domain exceptions, not generic Exception. + +```python +# ✅ Good +class VendorAlreadyExistsError(Exception): + pass + +def create_vendor(self, db: Session, vendor_data): + if self._vendor_exists(db, vendor_data.subdomain): + raise VendorAlreadyExistsError(f"Subdomain {vendor_data.subdomain} taken") + +# ❌ Bad +def create_vendor(self, db: Session, vendor_data): + if self._vendor_exists(db, vendor_data.subdomain): + raise Exception("Vendor exists") # Too generic! +``` + +#### SVC-003: Accept DB Session as Parameter +**Severity:** Error + +Service methods should receive database session as a parameter for testability and transaction control. + +```python +# ✅ Good +def create_vendor(self, db: Session, vendor_data: VendorCreate): + vendor = Vendor(**vendor_data.dict()) + db.add(vendor) + db.commit() + return vendor + +# ❌ Bad +def create_vendor(self, vendor_data: VendorCreate): + db = SessionLocal() # Don't create session inside! + vendor = Vendor(**vendor_data.dict()) + db.add(vendor) + db.commit() +``` + +#### SVC-004: Use Pydantic Models for Input +**Severity:** Warning + +Service methods should accept Pydantic models for complex inputs to ensure type safety. + +#### SVC-005: Scope Queries to vendor_id +**Severity:** Error + +All database queries must be scoped to vendor_id to prevent cross-tenant data access. + +--- + +### Model Rules (models/**/*.py) + +#### MDL-001: Database Models Use SQLAlchemy Base +**Severity:** Error + +All database models must inherit from SQLAlchemy Base. + +#### MDL-002: Never Mix SQLAlchemy and Pydantic +**Severity:** Error + +```python +# ❌ Bad +class Product(Base, BaseModel): # NEVER DO THIS! + pass +``` + +Keep them separate: +- `models/database/product.py` - SQLAlchemy models +- `models/schema/product.py` - Pydantic models + +#### MDL-003: Use from_attributes in Pydantic +**Severity:** Error + +Pydantic response models must enable `from_attributes` to work with SQLAlchemy models. + +```python +# ✅ Good +class ProductResponse(BaseModel): + id: int + name: str + + class Config: + from_attributes = True +``` + +#### MDL-004: Use Singular Table Names +**Severity:** Warning + +Database table names should be singular lowercase (e.g., 'vendor' not 'vendors'). + +--- + +### Exception Handling Rules + +#### EXC-001: Define Custom Exceptions +**Severity:** Warning + +Create domain-specific exceptions in `app/exceptions/` for better error handling. + +```python +# ✅ Good - app/exceptions/vendor.py +class VendorError(Exception): + """Base exception for vendor-related errors""" + pass + +class VendorNotFoundError(VendorError): + pass + +class VendorAlreadyExistsError(VendorError): + pass +``` + +#### EXC-002: Never Use Bare Except +**Severity:** Error + +```python +# ❌ Bad +try: + result = service.do_something() +except: # Too broad! + pass + +# ✅ Good +try: + result = service.do_something() +except ValueError as e: + logger.error(f"Validation error: {e}") +except Exception as e: + logger.error(f"Unexpected error: {e}") +``` + +#### EXC-003: Log Exceptions with Context +**Severity:** Warning + +When catching exceptions, log them with context and stack trace. + +```python +# ✅ Good +try: + result = service.process() +except Exception as e: + logger.error(f"Processing failed: {e}", exc_info=True) +``` + +--- + +### Naming Convention Rules + +#### NAM-001: API Files Use PLURAL Names +**Severity:** Error + +``` +✅ app/api/v1/admin/vendors.py +✅ app/api/v1/admin/products.py +❌ app/api/v1/admin/vendor.py +``` + +Exceptions: `__init__.py`, `auth.py`, `health.py` + +#### NAM-002: Service Files Use SINGULAR + service +**Severity:** Error + +``` +✅ app/services/vendor_service.py +✅ app/services/product_service.py +❌ app/services/vendors_service.py +``` + +#### NAM-003: Model Files Use SINGULAR Names +**Severity:** Error + +``` +✅ models/database/product.py +✅ models/schema/vendor.py +❌ models/database/products.py +``` + +#### NAM-004: Use 'vendor' not 'shop' +**Severity:** Warning + +Use consistent terminology: 'vendor' for shop owners, 'shop' only for customer-facing frontend. + +```python +# ✅ Good +vendor_id +vendor_service + +# ❌ Bad +shop_id +shop_service +``` + +#### NAM-005: Use 'inventory' not 'stock' +**Severity:** Warning + +```python +# ✅ Good +inventory_service +inventory_level + +# ❌ Bad +stock_service +stock_level +``` + +--- + +## Frontend Rules + +### JavaScript Rules (static/**/js/**/*.js) + +#### JS-001: Use Centralized Logger +**Severity:** Error + +Never use console.log, console.error, console.warn directly. Use window.LogConfig.createLogger(). + +```javascript +// ✅ Good +const pageLog = window.LogConfig.createLogger('dashboard'); +pageLog.info('Dashboard loaded'); +pageLog.error('Failed to load data', error); + +// ❌ Bad +console.log('Dashboard loaded'); +console.error('Failed to load data', error); +``` + +Exceptions: Bootstrap messages with `console.log('✅'` allowed. + +#### JS-002: Use Lowercase apiClient +**Severity:** Error + +```javascript +// ✅ Good +await apiClient.get('/api/v1/vendors'); +await apiClient.post('/api/v1/products', data); + +// ❌ Bad +await ApiClient.get('/api/v1/vendors'); +await API_CLIENT.post('/api/v1/products', data); +``` + +#### JS-003: Alpine Components Must Inherit ...data() +**Severity:** Error + +All Alpine.js components must inherit base layout data using spread operator. + +```javascript +// ✅ Good +function adminDashboard() { + return { + ...data(), // Inherit base layout data + currentPage: 'dashboard', + // ... component-specific data + }; +} + +// ❌ Bad +function adminDashboard() { + return { + currentPage: 'dashboard', + // Missing ...data()! + }; +} +``` + +#### JS-004: Set currentPage in Components +**Severity:** Error + +All Alpine.js page components must set a currentPage identifier. + +```javascript +// ✅ Good +return { + ...data(), + currentPage: 'dashboard', // Required! + // ... +}; +``` + +#### JS-005: Initialization Guards +**Severity:** Error + +Init methods should prevent duplicate initialization. + +```javascript +// ✅ Good +init() { + if (window._pageInitialized) return; + window._pageInitialized = true; + + // Initialization logic... +} +``` + +#### JS-006: Error Handling for Async Operations +**Severity:** Error + +All API calls and async operations must have error handling. + +```javascript +// ✅ Good +async loadData() { + try { + const response = await apiClient.get('/api/v1/data'); + this.items = response.data; + } catch (error) { + window.LogConfig.logError(error, 'Load Data'); + Utils.showToast('Failed to load data', 'error'); + } +} +``` + +#### JS-007: Set Loading State +**Severity:** Warning + +Loading state should be set before and cleared after async operations. + +```javascript +// ✅ Good +async loadData() { + this.loading = true; + try { + const response = await apiClient.get('/api/v1/data'); + this.items = response.data; + } catch (error) { + // Error handling + } finally { + this.loading = false; + } +} +``` + +--- + +### Template Rules (app/templates/**/*.html) + +#### TPL-001: Admin Templates Extend admin/base.html +**Severity:** Error + +```jinja +✅ {% extends "admin/base.html" %} +❌ No extends directive +``` + +Exceptions: `base.html` itself, files in `partials/` + +#### TPL-002: Vendor Templates Extend vendor/base.html +**Severity:** Error + +```jinja +✅ {% extends "vendor/base.html" %} +``` + +#### TPL-003: Shop Templates Extend shop/base.html +**Severity:** Error + +```jinja +✅ {% extends "shop/base.html" %} +``` + +#### TPL-004: Use x-text for Dynamic Content +**Severity:** Warning + +Use x-text directive for dynamic content to prevent XSS vulnerabilities. + +```html +✅

+❌

{{ item.name }}

+``` + +#### TPL-005: Use x-html ONLY for Safe Content +**Severity:** Error + +Use x-html only for trusted content like icons, never for user-generated content. + +```html +✅ +❌
+``` + +#### TPL-006: Implement Loading State +**Severity:** Warning + +All templates that load data should show loading state. + +```html +✅
Loading...
+``` + +#### TPL-007: Implement Empty State +**Severity:** Warning + +Show empty state when lists have no items. + +```html +✅ +``` + +--- + +### Styling Rules (app/templates/**/*.html) + +#### CSS-001: Use Tailwind Utility Classes +**Severity:** Warning + +Prefer Tailwind utility classes over custom CSS. + +#### CSS-002: Support Dark Mode +**Severity:** Warning + +All color classes should include dark mode variants. + +```html +✅ class="bg-white dark:bg-gray-800 text-gray-900 dark:text-white" +❌ class="bg-white text-gray-900" +``` + +#### CSS-003: Shop Templates Use CSS Variables +**Severity:** Error + +Shop templates must use CSS variables for vendor-specific theming. + +```html +✅ +❌ +``` + +#### CSS-004: Mobile-First Responsive Design +**Severity:** Warning + +Use mobile-first responsive classes. + +```html +✅ class="grid grid-cols-1 md:grid-cols-2 lg:grid-cols-4" +❌ class="grid grid-cols-4" +``` + +--- + +## Security & Multi-Tenancy Rules + +### Multi-Tenancy Rules + +#### MT-001: Scope All Queries to vendor_id +**Severity:** Error + +In vendor/shop contexts, all database queries must filter by vendor_id. + +```python +# ✅ Good +def get_products(self, db: Session, vendor_id: int): + return db.query(Product).filter(Product.vendor_id == vendor_id).all() + +# ❌ Bad +def get_products(self, db: Session): + return db.query(Product).all() # Cross-tenant data leak! +``` + +#### MT-002: No Cross-Vendor Data Access +**Severity:** Error + +Queries must never access data from other vendors. + +--- + +### Authentication & Authorization Rules + +#### AUTH-001: Use JWT Tokens +**Severity:** Error + +Authentication must use JWT tokens in Authorization: Bearer header. + +#### AUTH-002: Role-Based Access Control +**Severity:** Error + +Use Depends(get_current_admin/vendor/customer) for role checks. + +```python +# ✅ Good +@router.get("/admin/users") +async def list_users(current_user: User = Depends(get_current_admin)): + pass +``` + +#### AUTH-003: Never Store Plain Passwords +**Severity:** Error + +Always hash passwords with bcrypt before storing. + +--- + +## Middleware Rules + +#### MDW-001: Middleware Naming +**Severity:** Warning + +Middleware files should be named with simple nouns (auth.py, not auth_middleware.py). + +``` +✅ middleware/auth.py +✅ middleware/context.py +❌ middleware/auth_middleware.py +``` + +#### MDW-002: Vendor Context Injection +**Severity:** Error + +Vendor context middleware must set `request.state.vendor_id` and `request.state.vendor`. + +--- + +## Code Quality Rules + +#### QUAL-001: Format with Ruff +**Severity:** Error + +All code must be formatted with Ruff before committing. + +```bash +make format +``` + +#### QUAL-002: Pass Ruff Linting +**Severity:** Error + +All code must pass Ruff linting before committing. + +```bash +make lint +``` + +#### QUAL-003: Use Type Hints +**Severity:** Warning + +Add type hints to function parameters and return types. + +```python +# ✅ Good +def create_product(self, db: Session, data: ProductCreate) -> Product: + pass + +# Better +from typing import Optional + +def get_product(self, product_id: int) -> Optional[Product]: + pass +``` + +--- + +## Ignored Patterns + +The validator ignores these files/patterns: + +- Test files: `**/*_test.py`, `**/test_*.py` +- Cache: `**/__pycache__/**` +- Migrations: `**/alembic/versions/**` +- Dependencies: `**/node_modules/**`, `**/.venv/**`, `**/venv/**` +- Build artifacts: `**/build/**`, `**/dist/**` + +Special exceptions: +- `app/core/exceptions.py` - Allowed to use HTTPException +- `app/exceptions/handler.py` - Converts to HTTPException + +--- + +## Quick Reference + +### Pre-Commit Checklist + +Before committing code: + +- [ ] Run `make format` (Ruff formatting) +- [ ] Run `make lint` (Ruff + mypy) +- [ ] Run `python scripts/validate_architecture.py` +- [ ] Fix all **Error** level violations +- [ ] Review **Warning** level violations +- [ ] Run relevant tests + +### Common Violations and Fixes + +| Violation | Quick Fix | +|-----------|-----------| +| HTTPException in service | Create custom exception in `app/exceptions/` | +| Business logic in endpoint | Move to service layer | +| No exception handling | Add try/except, convert to HTTPException | +| console.log in JS | Use `window.LogConfig.createLogger()` | +| Missing ...data() | Add spread operator in component return | +| Bare except clause | Specify exception type | +| Raw dict return | Create Pydantic response model | + +--- + +## Configuration + +All rules are defined in `.architecture-rules.yaml`. To modify rules: + +1. Edit `.architecture-rules.yaml` +2. Update `scripts/validate_architecture.py` if implementing new checks +3. Run validator to test changes +4. Update this documentation + +--- + +## Related Documentation + +- [Code Quality Guide](code-quality.md) +- [Contributing Guide](contributing.md) +- [Architecture Overview](../architecture/overview.md) +- [Backend Development](../backend/overview.md) +- [Frontend Development](../frontend/overview.md) + +--- + +## Summary Statistics + +| Category | Rules | Errors | Warnings | +|----------|-------|--------|----------| +| Backend | 20 | 15 | 5 | +| Frontend JS | 7 | 6 | 1 | +| Frontend Templates | 7 | 3 | 4 | +| Frontend Styling | 4 | 1 | 3 | +| Naming | 5 | 3 | 2 | +| Security | 5 | 5 | 0 | +| Quality | 3 | 2 | 1 | +| **Total** | **51** | **35** | **16** | + +--- + +**Last Updated:** 2025-11-28 +**Version:** 2.0 diff --git a/mkdocs.yml b/mkdocs.yml index 3d90b21f..aca02025 100644 --- a/mkdocs.yml +++ b/mkdocs.yml @@ -103,6 +103,7 @@ nav: - Development: - Contributing Guide: development/contributing.md - Code Quality: development/code-quality.md + - Architecture Rules: development/architecture-rules.md - Code Quality Dashboard: development/code-quality-dashboard-implementation.md - Icons Guide: development/icons-guide.md - Naming Conventions: development/naming-conventions.md diff --git a/models/database/base.py b/models/database/base.py index e48bfd1e..a9d84a1e 100644 --- a/models/database/base.py +++ b/models/database/base.py @@ -2,6 +2,8 @@ from datetime import UTC, datetime from sqlalchemy import Column, DateTime +from app.core.database import Base + class TimestampMixin: """Mixin to add created_at and updated_at timestamps to models"""