diff --git a/.architecture-rules/api.yaml b/.architecture-rules/api.yaml index a141988d..19d29f59 100644 --- a/.architecture-rules/api.yaml +++ b/.architecture-rules/api.yaml @@ -204,3 +204,53 @@ api_endpoint_rules: file_pattern: "app/api/v1/shop/**/*.py" discouraged_patterns: - "db.query(.*).all()" + + - id: "API-007" + name: "API endpoints must NOT import database models directly" + severity: "error" + description: | + API endpoints must follow the layered architecture: Routes → Services → Models. + Routes should NEVER import database models directly. + + WHY THIS MATTERS: + - Layered architecture: Each layer has clear responsibilities + - Testability: Routes can be tested without database + - Flexibility: Database schema changes don't affect route signatures + - Type safety: Schemas define the API contract, not database structure + + For dependency injection (e.g., current user/customer), use schemas instead + of database models as return types. + + WRONG: + from models.database.customer import Customer + from models.database.order import Order + from app.modules.customers.models.customer import Customer + + @router.get("/orders") + def get_orders(customer: Customer = Depends(get_current_customer)): + ... + + RIGHT: + from app.modules.customers.schemas import CustomerContext + from app.modules.orders.services import order_service + + @router.get("/orders") + def get_orders(customer: CustomerContext = Depends(get_current_customer)): + return order_service.get_orders(db, customer.id, customer.vendor_id) + + ALLOWED IMPORTS IN API ROUTES: + - Schemas: from models.schema.* or from app.modules.*.schemas + - Services: from app.services.* or from app.modules.*.services + - Dependencies: from app.api.deps + - Database session: from app.core.database import get_db + + NOT ALLOWED: + - from models.database.* + - from app.modules.*.models.* + pattern: + file_pattern: "app/api/**/*.py" + anti_patterns: + - "from models\\.database\\." + - "from app\\.modules\\.[a-z_]+\\.models\\." + exclude_files: + - "app/api/deps.py" # Dependencies may need model access for queries diff --git a/alembic/env.py b/alembic/env.py index a9dbe3ed..3f40b9b3 100644 --- a/alembic/env.py +++ b/alembic/env.py @@ -145,7 +145,7 @@ except ImportError as e: # CUSTOMER MODELS # ---------------------------------------------------------------------------- try: - from models.database.customer import Customer, CustomerAddress + from app.modules.customers.models.customer import Customer, CustomerAddress print(" ✓ Customer models imported (2 models)") print(" - Customer") diff --git a/app/api/deps.py b/app/api/deps.py index 84f0577a..a06e51b6 100644 --- a/app/api/deps.py +++ b/app/api/deps.py @@ -740,7 +740,7 @@ def _validate_customer_token(token: str, request: Request, db: Session): from jose import JWTError, jwt - from models.database.customer import Customer + from app.modules.customers.models.customer import Customer # Decode and validate customer JWT token try: diff --git a/app/modules/analytics/services/stats_service.py b/app/modules/analytics/services/stats_service.py index 20287e43..1831ba7a 100644 --- a/app/modules/analytics/services/stats_service.py +++ b/app/modules/analytics/services/stats_service.py @@ -19,7 +19,7 @@ from sqlalchemy import func from sqlalchemy.orm import Session from app.exceptions import AdminOperationException, VendorNotFoundException -from models.database.customer import Customer +from app.modules.customers.models.customer import Customer from models.database.inventory import Inventory from models.database.marketplace_import_job import MarketplaceImportJob from models.database.marketplace_product import MarketplaceProduct diff --git a/app/modules/loyalty/services/card_service.py b/app/modules/loyalty/services/card_service.py index d3301f13..f929a381 100644 --- a/app/modules/loyalty/services/card_service.py +++ b/app/modules/loyalty/services/card_service.py @@ -144,7 +144,7 @@ class CardService: Returns: (cards, total_count) """ - from models.database.customer import Customer + from app.modules.customers.models.customer import Customer query = ( db.query(LoyaltyCard) diff --git a/app/modules/messaging/services/messaging_service.py b/app/modules/messaging/services/messaging_service.py index 6e128468..cc61ad02 100644 --- a/app/modules/messaging/services/messaging_service.py +++ b/app/modules/messaging/services/messaging_service.py @@ -25,7 +25,7 @@ from app.modules.messaging.models.message import ( MessageAttachment, ParticipantType, ) -from models.database.customer import Customer +from app.modules.customers.models.customer import Customer from models.database.user import User logger = logging.getLogger(__name__) diff --git a/app/services/email_service.py b/app/services/email_service.py index e291017c..0afaade7 100644 --- a/app/services/email_service.py +++ b/app/services/email_service.py @@ -1122,7 +1122,7 @@ class EmailService: # 2. Customer's preferred language if customer_id: - from models.database.customer import Customer + from app.modules.customers.models.customer import Customer customer = ( self.db.query(Customer).filter(Customer.id == customer_id).first() diff --git a/docs/proposals/PLAN_storefront-module-restructure.md b/docs/proposals/PLAN_storefront-module-restructure.md new file mode 100644 index 00000000..0a74a032 --- /dev/null +++ b/docs/proposals/PLAN_storefront-module-restructure.md @@ -0,0 +1,344 @@ +# Implementation Plan: Storefront Module Restructure + +## Overview + +Restructure the platform to properly separate **core platform concerns** (customer auth/identity) from **e-commerce concerns** (cart, checkout, catalog), and rename "shop" to "storefront" throughout. + +## Current Issues + +1. **API routes import database models directly** - violates layered architecture +2. **No architecture rule enforces this** - validator doesn't catch it +3. **"shop" naming is misleading** - not all platforms sell items +4. **Customer auth is bundled with e-commerce** - should be core/platform concern + +--- + +## Phase 1: Add Missing Architecture Rule + +### Task 1.1: Add API-007 rule to prevent direct model imports + +**File:** `.architecture-rules/api.yaml` + +Add new rule: +```yaml +- id: "API-007" + name: "API endpoints must NOT import database models directly" + severity: "error" + description: | + API endpoints should import from services, not directly from database models. + This enforces the layered architecture: Routes → Services → Models. + + The only exception is for type hints in FastAPI dependency injection, + which should use schemas or services that return the appropriate types. + + WRONG: + from models.database.customer import Customer + from models.database.order import Order + + RIGHT: + from app.modules.customers.services import customer_service + from models.schema.customer import CustomerResponse + pattern: + file_pattern: "app/api/**/*.py" + anti_patterns: + - "from models\\.database\\." + - "from app\\.modules\\..*\\.models\\." +``` + +### Task 1.2: Update validator to check this rule + +**File:** `scripts/validate_architecture.py` + +Add validation for API-007. + +--- + +## Phase 2: Rename "shop" to "storefront" + +### Task 2.1: Rename API directory +``` +app/api/v1/shop/ → app/api/v1/storefront/ +``` + +### Task 2.2: Update all imports referencing shop +- Update router registrations in `app/api/v1/__init__.py` +- Update any references in middleware +- Update documentation + +### Task 2.3: Rename routes/pages if applicable +``` +app/routes/shop_pages.py → app/routes/storefront_pages.py +``` + +--- + +## Phase 3: Create New E-commerce Modules + +### Task 3.1: Create `cart` module +``` +app/modules/cart/ +├── __init__.py +├── definition.py +├── config.py +├── exceptions.py +├── models/ +│ ├── __init__.py +│ └── cart.py # Cart, CartItem models (if persistent) +├── schemas/ +│ ├── __init__.py +│ └── cart.py # Move from models/schema/cart.py +├── services/ +│ ├── __init__.py +│ └── cart_service.py # Move from app/services/cart_service.py +└── routes/ + └── api/ + ├── __init__.py + └── storefront.py # Cart endpoints for customers +``` + +### Task 3.2: Create `checkout` module +``` +app/modules/checkout/ +├── __init__.py +├── definition.py +├── config.py +├── exceptions.py +├── schemas/ +│ ├── __init__.py +│ └── checkout.py # CheckoutRequest, CheckoutResponse +├── services/ +│ ├── __init__.py +│ └── checkout_service.py # Orchestrates cart → order conversion +└── routes/ + └── api/ + ├── __init__.py + └── storefront.py # Place order endpoint +``` + +### Task 3.3: Create `catalog` module +``` +app/modules/catalog/ +├── __init__.py +├── definition.py +├── config.py +├── schemas/ +│ ├── __init__.py +│ └── catalog.py # ProductListResponse, ProductDetailResponse +├── services/ +│ ├── __init__.py +│ └── catalog_service.py # Customer-facing product queries +└── routes/ + └── api/ + ├── __init__.py + └── storefront.py # Product browsing endpoints +``` + +--- + +## Phase 4: Move Storefront Routes to Modules + +### Task 4.1: Move customer auth routes to `customers` module + +**From:** `app/api/v1/storefront/auth.py` +**To:** `app/modules/customers/routes/api/storefront.py` + +Endpoints: +- `POST /auth/register` - Customer registration +- `POST /auth/login` - Customer login +- `POST /auth/logout` - Customer logout +- `POST /auth/password-reset/request` - Request password reset +- `POST /auth/password-reset/confirm` - Confirm password reset + +### Task 4.2: Move customer profile routes to `customers` module + +**From:** `app/api/v1/storefront/profile.py` +**To:** `app/modules/customers/routes/api/storefront.py` (append) + +Endpoints: +- `GET /profile` - Get customer profile +- `PUT /profile` - Update customer profile + +### Task 4.3: Move customer address routes to `customers` module + +**From:** `app/api/v1/storefront/addresses.py` +**To:** `app/modules/customers/routes/api/storefront.py` (append) + +Endpoints: +- `GET /addresses` - List customer addresses +- `POST /addresses` - Create address +- `PUT /addresses/{id}` - Update address +- `DELETE /addresses/{id}` - Delete address + +### Task 4.4: Move cart routes to `cart` module + +**From:** `app/api/v1/storefront/carts.py` +**To:** `app/modules/cart/routes/api/storefront.py` + +### Task 4.5: Move order placement to `checkout` module + +**From:** `app/api/v1/storefront/orders.py` (POST /orders only) +**To:** `app/modules/checkout/routes/api/storefront.py` + +### Task 4.6: Move order viewing to `orders` module + +**From:** `app/api/v1/storefront/orders.py` (GET endpoints) +**To:** `app/modules/orders/routes/api/storefront.py` + +### Task 4.7: Move product browsing to `catalog` module + +**From:** `app/api/v1/storefront/products.py` +**To:** `app/modules/catalog/routes/api/storefront.py` + +### Task 4.8: Move messaging routes to `messaging` module + +**From:** `app/api/v1/storefront/messages.py` +**To:** `app/modules/messaging/routes/api/storefront.py` + +--- + +## Phase 5: Fix Direct Model Imports + +### Task 5.1: Update dependency injection pattern + +**Current pattern (violates layered architecture):** +```python +from models.database.customer import Customer + +@router.get("/orders") +def get_orders(customer: Customer = Depends(get_current_customer_api)): + ... +``` + +**New pattern (proper layered architecture):** +```python +from app.modules.customers.schemas import CustomerContext + +@router.get("/orders") +def get_orders(customer: CustomerContext = Depends(get_current_customer_api)): + ... +``` + +### Task 5.2: Create `CustomerContext` schema + +**File:** `app/modules/customers/schemas/context.py` +```python +class CustomerContext(BaseModel): + """Customer context for dependency injection in storefront routes.""" + id: int + vendor_id: int + email: str + first_name: str | None + last_name: str | None + + model_config = ConfigDict(from_attributes=True) +``` + +### Task 5.3: Update `get_current_customer_api` dependency + +**File:** `app/api/deps.py` + +Change return type from `Customer` (database model) to `CustomerContext` (schema). + +### Task 5.4: Update all storefront routes + +Replace all `Customer` type hints with `CustomerContext`. + +--- + +## Phase 6: Delete Legacy Files + +### Task 6.1: Delete `models/database/customer.py` + +After all imports are updated to use `app.modules.customers.models.customer`. + +### Task 6.2: Delete `app/api/v1/storefront/` directory + +After all routes are moved to their respective modules. + +### Task 6.3: Delete `app/services/cart_service.py` + +After migrated to `app/modules/cart/services/cart_service.py`. + +--- + +## Phase 7: Update Documentation + +### Task 7.1: Update API documentation +- Rename all "shop" references to "storefront" +- Update endpoint paths + +### Task 7.2: Update architecture documentation +- Document the layered architecture rule +- Document module structure + +--- + +## Module Dependency Graph (Final State) + +``` + ┌─────────────┐ + │ core │ + │ (tenancy) │ + └──────┬──────┘ + │ + ┌────────────┼────────────┐ + │ │ │ + ▼ ▼ ▼ + ┌──────────┐ ┌──────────┐ ┌──────────┐ + │customers │ │ payments │ │messaging │ + │ (auth) │ │ │ │ │ + └────┬─────┘ └────┬─────┘ └──────────┘ + │ │ + │ ┌────────┴────────┐ + │ │ │ + ▼ ▼ ▼ + ┌──────────┐ ┌──────────┐ + │ cart │ │ orders │ + │ │ │ │ + └────┬─────┘ └────┬─────┘ + │ │ + └──────────┬──────────┘ + │ + ▼ + ┌──────────┐ + │ checkout │ + │ │ + └──────────┘ +``` + +--- + +## Execution Order + +1. **Phase 1** - Add architecture rule (enables detection) +2. **Phase 2** - Rename shop → storefront (terminology) +3. **Phase 3** - Create new modules (cart, checkout, catalog) +4. **Phase 4** - Move routes to modules +5. **Phase 5** - Fix direct model imports +6. **Phase 6** - Delete legacy files +7. **Phase 7** - Update documentation + +--- + +## Files Modified Summary + +### New Files +- `.architecture-rules/api.yaml` (modified - add API-007) +- `app/modules/cart/**` (new module) +- `app/modules/checkout/**` (new module) +- `app/modules/catalog/**` (new module) +- `app/modules/customers/routes/api/storefront.py` +- `app/modules/customers/schemas/context.py` +- `app/modules/orders/routes/api/storefront.py` +- `app/modules/messaging/routes/api/storefront.py` + +### Deleted Files +- `app/api/v1/shop/` (entire directory) +- `app/routes/shop_pages.py` +- `models/database/customer.py` +- `app/services/cart_service.py` + +### Modified Files +- `app/api/deps.py` (CustomerContext return type) +- `scripts/validate_architecture.py` (add API-007 check) +- All files currently importing from legacy locations diff --git a/scripts/seed_demo.py b/scripts/seed_demo.py index d44e8c7f..8614968b 100644 --- a/scripts/seed_demo.py +++ b/scripts/seed_demo.py @@ -54,7 +54,7 @@ from middleware.auth import AuthManager from app.modules.cms.models import ContentPage from models.database.admin import PlatformAlert from models.database.company import Company -from models.database.customer import Customer, CustomerAddress +from app.modules.customers.models.customer import Customer, CustomerAddress from models.database.marketplace_import_job import MarketplaceImportJob from models.database.marketplace_product import MarketplaceProduct from models.database.marketplace_product_translation import ( diff --git a/scripts/validate_architecture.py b/scripts/validate_architecture.py index a4b5a808..153763dc 100755 --- a/scripts/validate_architecture.py +++ b/scripts/validate_architecture.py @@ -1771,6 +1771,9 @@ class ArchitectureValidator: # API-005: Check vendor_id scoping for vendor/shop endpoints self._check_vendor_scoping(file_path, content, lines) + # API-007: Check for direct model imports + self._check_no_model_imports(file_path, content, lines) + def _check_pydantic_usage(self, file_path: Path, content: str, lines: list[str]): """API-001: Ensure endpoints use Pydantic models""" rule = self._get_rule("API-001") @@ -2061,6 +2064,59 @@ class ArchitectureValidator: ) return # Only report once per file + def _check_no_model_imports( + self, file_path: Path, content: str, lines: list[str] + ): + """API-007: Check that API endpoints do NOT import database models directly. + + Routes should follow layered architecture: Routes → Services → Models. + Database models should only be imported in services, not in API routes. + + Allowed: schemas, services, deps, database session + Not allowed: models.database.*, app.modules.*.models.* + """ + rule = self._get_rule("API-007") + if not rule: + return + + # Skip deps.py - it may need model access for queries + file_path_str = str(file_path) + if file_path_str.endswith("deps.py"): + return + + # Check for noqa + if "noqa: api-007" in content.lower(): + return + + # Patterns that indicate direct model imports + model_import_patterns = [ + (r"from models\.database\.", "Importing from legacy models.database location"), + (r"from app\.modules\.[a-z_]+\.models\.", "Importing from module models"), + ] + + for i, line in enumerate(lines, 1): + # Skip comments + stripped = line.strip() + if stripped.startswith("#"): + continue + + # Check for noqa on this specific line + if "noqa: api-007" in line.lower(): + continue + + for pattern, message in model_import_patterns: + if re.search(pattern, line): + self._add_violation( + rule_id="API-007", + rule_name="API endpoints must NOT import database models directly", + severity=Severity.ERROR, + file_path=file_path, + line_number=i, + message=message, + context=line.strip()[:80], + suggestion="Import from services or schemas instead. Use CustomerContext schema for dependency injection.", + ) + def _validate_service_layer(self, target_path: Path): """Validate service layer rules (SVC-001 to SVC-007)""" print("🔧 Validating service layer...") diff --git a/tests/fixtures/customer_fixtures.py b/tests/fixtures/customer_fixtures.py index cfa17f51..11515e31 100644 --- a/tests/fixtures/customer_fixtures.py +++ b/tests/fixtures/customer_fixtures.py @@ -8,7 +8,7 @@ See tests/conftest.py for details on fixture best practices. import pytest -from models.database.customer import Customer, CustomerAddress +from app.modules.customers.models.customer import Customer, CustomerAddress from models.database.order import Order diff --git a/tests/integration/api/v1/shop/test_addresses.py b/tests/integration/api/v1/shop/test_addresses.py index e406436f..7e2525d2 100644 --- a/tests/integration/api/v1/shop/test_addresses.py +++ b/tests/integration/api/v1/shop/test_addresses.py @@ -11,7 +11,7 @@ from unittest.mock import patch import pytest from jose import jwt -from models.database.customer import Customer, CustomerAddress +from app.modules.customers.models.customer import Customer, CustomerAddress @pytest.fixture diff --git a/tests/integration/api/v1/shop/test_orders.py b/tests/integration/api/v1/shop/test_orders.py index 3b680817..550fedeb 100644 --- a/tests/integration/api/v1/shop/test_orders.py +++ b/tests/integration/api/v1/shop/test_orders.py @@ -12,7 +12,7 @@ from unittest.mock import patch, MagicMock import pytest from jose import jwt -from models.database.customer import Customer +from app.modules.customers.models.customer import Customer from models.database.invoice import Invoice, InvoiceStatus, VendorInvoiceSettings from models.database.order import Order, OrderItem diff --git a/tests/integration/api/v1/shop/test_password_reset.py b/tests/integration/api/v1/shop/test_password_reset.py index a2640b83..6bbbea3f 100644 --- a/tests/integration/api/v1/shop/test_password_reset.py +++ b/tests/integration/api/v1/shop/test_password_reset.py @@ -9,7 +9,7 @@ from unittest.mock import MagicMock, patch import pytest -from models.database.customer import Customer +from app.modules.customers.models.customer import Customer from models.database.password_reset_token import PasswordResetToken diff --git a/tests/integration/api/v1/vendor/test_invoices.py b/tests/integration/api/v1/vendor/test_invoices.py index 06323045..17e6511a 100644 --- a/tests/integration/api/v1/vendor/test_invoices.py +++ b/tests/integration/api/v1/vendor/test_invoices.py @@ -9,7 +9,7 @@ from decimal import Decimal import pytest -from models.database.customer import Customer +from app.modules.customers.models.customer import Customer from models.database.invoice import Invoice, InvoiceStatus, VendorInvoiceSettings from models.database.order import Order diff --git a/tests/unit/models/database/test_customer.py b/tests/unit/models/database/test_customer.py index 05067f76..338f7145 100644 --- a/tests/unit/models/database/test_customer.py +++ b/tests/unit/models/database/test_customer.py @@ -3,7 +3,7 @@ import pytest -from models.database.customer import Customer, CustomerAddress +from app.modules.customers.models.customer import Customer, CustomerAddress @pytest.mark.unit diff --git a/tests/unit/services/test_admin_customer_service.py b/tests/unit/services/test_admin_customer_service.py index 0dd4ec76..cf9f113a 100644 --- a/tests/unit/services/test_admin_customer_service.py +++ b/tests/unit/services/test_admin_customer_service.py @@ -9,7 +9,7 @@ import pytest from app.exceptions.customer import CustomerNotFoundException from app.services.admin_customer_service import AdminCustomerService -from models.database.customer import Customer +from app.modules.customers.models.customer import Customer @pytest.fixture diff --git a/tests/unit/services/test_customer_address_service.py b/tests/unit/services/test_customer_address_service.py index 935ea1fd..7555ce6b 100644 --- a/tests/unit/services/test_customer_address_service.py +++ b/tests/unit/services/test_customer_address_service.py @@ -7,7 +7,7 @@ import pytest from app.exceptions import AddressLimitExceededException, AddressNotFoundException from app.services.customer_address_service import CustomerAddressService -from models.database.customer import CustomerAddress +from app.modules.customers.models.customer import CustomerAddress from models.schema.customer import CustomerAddressCreate, CustomerAddressUpdate diff --git a/tests/unit/services/test_order_service.py b/tests/unit/services/test_order_service.py index 781287dc..eb660b77 100644 --- a/tests/unit/services/test_order_service.py +++ b/tests/unit/services/test_order_service.py @@ -27,7 +27,7 @@ from app.services.order_service import ( OrderService, order_service, ) -from models.database.customer import Customer +from app.modules.customers.models.customer import Customer from models.database.order import Order, OrderItem