removing legacy code on path_rewrite_middleware

This commit is contained in:
2025-11-18 23:32:07 +01:00
parent f14686c131
commit d947fa5ca0
9 changed files with 114 additions and 302 deletions

View File

@@ -64,28 +64,9 @@ Injects: request.state.vendor = <Vendor object>
**See**: [Multi-Tenant System](multi-tenant.md) for routing modes **See**: [Multi-Tenant System](multi-tenant.md) for routing modes
### 3. Path Rewrite Middleware **Note on Path-Based Routing:** Previous implementations used a `PathRewriteMiddleware` to rewrite paths at runtime. This has been replaced with **double router mounting** in `main.py`, where shop routes are registered twice with different prefixes (`/shop` and `/vendors/{vendor_code}/shop`). This approach is simpler and uses FastAPI's native routing capabilities.
**Purpose**: Rewrite request paths for proper FastAPI routing ### 3. Context Detection Middleware
**What it does**:
- Uses the `clean_path` extracted by VendorContextMiddleware
- Rewrites `request.scope['path']` to remove vendor prefix
- Allows FastAPI routes to match correctly
**Example** (Path-Based Development Mode):
```
Original path: /vendors/WIZAMART/shop/products
Clean path: /shop/products (set by VendorContextMiddleware)
Path Rewrite Middleware changes request path to: /shop/products
FastAPI router can now match: @app.get("/shop/products")
```
**Why it's needed**: FastAPI routes don't include vendor prefix, so we strip it
### 4. Context Detection Middleware
**Purpose**: Determine the type/context of the request **Purpose**: Determine the type/context of the request
@@ -115,7 +96,7 @@ else:
**Why it's useful**: Error handlers and templates adapt based on context **Why it's useful**: Error handlers and templates adapt based on context
### 5. Theme Context Middleware ### 4. Theme Context Middleware
**Purpose**: Load vendor-specific theme settings **Purpose**: Load vendor-specific theme settings
@@ -146,13 +127,12 @@ else:
graph TD graph TD
A[Client Request] --> B[1. LoggingMiddleware] A[Client Request] --> B[1. LoggingMiddleware]
B --> C[2. VendorContextMiddleware] B --> C[2. VendorContextMiddleware]
C --> D[3. PathRewriteMiddleware] C --> D[3. ContextDetectionMiddleware]
D --> E[4. ContextDetectionMiddleware] D --> E[4. ThemeContextMiddleware]
E --> F[5. ThemeContextMiddleware] E --> F[5. FastAPI Router]
F --> G[6. FastAPI Router] F --> G[Route Handler]
G --> H[Route Handler] G --> H[Response]
H --> I[Response] H --> I[Client]
I --> J[Client]
``` ```
### Why This Order Matters ### Why This Order Matters
@@ -164,25 +144,21 @@ graph TD
- Must log errors from all other middleware - Must log errors from all other middleware
2. **VendorContextMiddleware second** 2. **VendorContextMiddleware second**
- Must run before PathRewriteMiddleware (provides clean_path) - Must run before ContextDetectionMiddleware (provides vendor and clean_path)
- Must run before ContextDetectionMiddleware (provides vendor)
- Must run before ThemeContextMiddleware (provides vendor_id) - Must run before ThemeContextMiddleware (provides vendor_id)
3. **PathRewriteMiddleware third** 3. **ContextDetectionMiddleware third**
- Depends on clean_path from VendorContextMiddleware
- Must run before ContextDetectionMiddleware (rewrites path)
4. **ContextDetectionMiddleware fourth**
- Uses clean_path from VendorContextMiddleware - Uses clean_path from VendorContextMiddleware
- Uses rewritten path from PathRewriteMiddleware
- Provides context_type for ThemeContextMiddleware - Provides context_type for ThemeContextMiddleware
5. **ThemeContextMiddleware last** 4. **ThemeContextMiddleware last**
- Depends on vendor from VendorContextMiddleware - Depends on vendor from VendorContextMiddleware
- Depends on context_type from ContextDetectionMiddleware - Depends on context_type from ContextDetectionMiddleware
**Breaking this order will break the application!** **Breaking this order will break the application!**
**Note:** Path-based routing (e.g., `/vendors/{code}/shop/*`) is handled by double router mounting in `main.py`, not by middleware.
## Request State Variables ## Request State Variables
Middleware components inject these variables into `request.state`: Middleware components inject these variables into `request.state`:
@@ -191,7 +167,7 @@ Middleware components inject these variables into `request.state`:
|----------|--------|------|---------|-------------| |----------|--------|------|---------|-------------|
| `vendor` | VendorContextMiddleware | Vendor | Theme, Templates | Current vendor object | | `vendor` | VendorContextMiddleware | Vendor | Theme, Templates | Current vendor object |
| `vendor_id` | VendorContextMiddleware | int | Queries, Theme | Current vendor ID | | `vendor_id` | VendorContextMiddleware | int | Queries, Theme | Current vendor ID |
| `clean_path` | VendorContextMiddleware | str | PathRewrite, Context | Path without vendor prefix | | `clean_path` | VendorContextMiddleware | str | Context | Path without vendor prefix (for context detection) |
| `context_type` | ContextDetectionMiddleware | RequestContext | Theme, Error handlers | Request context enum | | `context_type` | ContextDetectionMiddleware | RequestContext | Theme, Error handlers | Request context enum |
| `theme` | ThemeContextMiddleware | dict | Templates | Vendor theme config | | `theme` | ThemeContextMiddleware | dict | Templates | Vendor theme config |
@@ -260,25 +236,21 @@ async def get_products(request: Request):
↓ Sets: request.state.vendor_id = 1 ↓ Sets: request.state.vendor_id = 1
↓ Sets: request.state.clean_path = "/shop/products" ↓ Sets: request.state.clean_path = "/shop/products"
3. PathRewriteMiddleware 3. ContextDetectionMiddleware
↓ Path already clean (no rewrite needed for subdomain mode)
↓ request.scope['path'] = "/shop/products"
4. ContextDetectionMiddleware
↓ Analyzes path: "/shop/products" ↓ Analyzes path: "/shop/products"
↓ Has vendor: Yes ↓ Has vendor: Yes
↓ Not admin/api/vendor dashboard ↓ Not admin/api/vendor dashboard
↓ Sets: request.state.context_type = RequestContext.SHOP ↓ Sets: request.state.context_type = RequestContext.SHOP
5. ThemeContextMiddleware 4. ThemeContextMiddleware
↓ Loads theme for vendor_id = 1 ↓ Loads theme for vendor_id = 1
↓ Sets: request.state.theme = {...theme config...} ↓ Sets: request.state.theme = {...theme config...}
6. FastAPI Router 5. FastAPI Router
↓ Matches route: @app.get("/shop/products") ↓ Matches route: @app.get("/shop/products")
↓ Calls handler function ↓ Calls handler function
7. Route Handler 6. Route Handler
↓ Accesses: request.state.vendor_id ↓ Accesses: request.state.vendor_id
↓ Queries: products WHERE vendor_id = 1 ↓ Queries: products WHERE vendor_id = 1
↓ Renders template with vendor data ↓ Renders template with vendor data

View File

@@ -452,10 +452,12 @@ Host: myplatform.com
- Sets: request.state.vendor = <Vendor> - Sets: request.state.vendor = <Vendor>
- Sets: request.state.clean_path = "/shop/products" - Sets: request.state.clean_path = "/shop/products"
2. PathRewriteMiddleware 2. FastAPI Router
- Rewrites: request.scope['path'] = "/shop/products" - Routes registered with prefix: /vendors/{vendor_code}/shop
- Matches: /vendors/WIZAMART/shop/products
- vendor_code path parameter = "WIZAMART"
3-4. Same as previous examples 3-4. Same as previous examples (Context, Theme middleware)
``` ```
## Testing Multi-Tenancy ## Testing Multi-Tenancy

View File

@@ -82,20 +82,19 @@ Custom middleware handles:
graph TB graph TB
A[Client Request] --> B[Logging Middleware] A[Client Request] --> B[Logging Middleware]
B --> C[Vendor Context Middleware] B --> C[Vendor Context Middleware]
C --> D[Path Rewrite Middleware] C --> D[Context Detection Middleware]
D --> E[Context Detection Middleware] D --> E[Theme Context Middleware]
E --> F[Theme Context Middleware] E --> F{Request Type?}
F --> G{Request Type?}
G -->|API /api/*| H[API Router] F -->|API /api/*| G[API Router]
G -->|Admin /admin/*| I[Admin Page Router] F -->|Admin /admin/*| H[Admin Page Router]
G -->|Vendor /vendor/*| J[Vendor Page Router] F -->|Vendor /vendor/*| I[Vendor Page Router]
G -->|Shop /shop/*| K[Shop Page Router] F -->|Shop /shop/*| J[Shop Page Router]
H --> L[JSON Response] G --> K[JSON Response]
I --> M[Admin HTML] H --> L[Admin HTML]
J --> N[Vendor HTML] I --> M[Vendor HTML]
K --> O[Shop HTML] J --> N[Shop HTML]
``` ```
**See:** [Request Flow](request-flow.md) for detailed journey **See:** [Request Flow](request-flow.md) for detailed journey

View File

@@ -112,25 +112,29 @@ request.state.vendor_id = 1
request.state.clean_path = "/shop/products" request.state.clean_path = "/shop/products"
``` ```
### 4. PathRewriteMiddleware ### 4. Router Matching (FastAPI Native)
**What happens**: **What happens**:
- Checks if `clean_path` is different from original path - FastAPI matches the request path against registered routes
- If different, rewrites the request path for FastAPI routing - For path-based development mode, routes are registered with two prefixes:
- `/shop/*` for subdomain/custom domain
- `/vendors/{vendor_code}/shop/*` for path-based development
**Example** (Path-Based Mode): **Example** (Path-Based Mode):
```python ```python
# Input (path-based development mode) # In main.py - Double router mounting
original_path = "/vendors/WIZAMART/shop/products" app.include_router(shop_pages.router, prefix="/shop")
clean_path = "/shop/products" # From VendorContextMiddleware app.include_router(shop_pages.router, prefix="/vendors/{vendor_code}/shop")
# Path rewrite # Request: /vendors/WIZAMART/shop/products
if clean_path != original_path: # Matches: Second router (/vendors/{vendor_code}/shop)
request.scope['path'] = clean_path # Route: @router.get("/products")
request._url = request.url.replace(path=clean_path) # vendor_code available as path parameter = "WIZAMART"
``` ```
**Note:** Previous implementations used `PathRewriteMiddleware` to rewrite paths. This has been replaced with FastAPI's native routing via double router mounting.
**Request State After**: No changes to state, but internal path updated **Request State After**: No changes to state, but internal path updated
### 5. ContextDetectionMiddleware ### 5. ContextDetectionMiddleware

View File

@@ -345,40 +345,38 @@ In Jinja2 template:
--- ---
## Potential Issue: Path-Based Development Mode ## Path-Based Routing Implementation
⚠️ **Current Implementation Gap:** **Current Solution: Double Router Mounting**
The `vendor_context_middleware` sets `clean_path` for path-based URLs, but this isn't used for FastAPI routing. The application handles path-based routing by registering shop routes **twice** with different prefixes:
**Problem:**
- Incoming: `GET http://localhost:8000/vendors/acme/shop/products`
- Routes registered: `@router.get("/shop/products")`
- FastAPI tries to match `/vendors/acme/shop/products` against `/shop/products`
- Result: ❌ 404 Not Found
**Solution (Recommended):**
Add a path rewriting middleware in `main.py`:
```python ```python
async def path_rewrite_middleware(request: Request, call_next): # In main.py
"""Rewrite path for path-based vendor routing in development mode."""
if hasattr(request.state, 'clean_path'):
# Replace request path for FastAPI routing
request._url = request._url.replace(path=request.state.clean_path)
return await call_next(request)
# In main.py, add after vendor_context_middleware:
app.middleware("http")(path_rewrite_middleware)
```
Or alternatively, mount the router twice:
```python
app.include_router(shop_pages.router, prefix="/shop") app.include_router(shop_pages.router, prefix="/shop")
app.include_router(shop_pages.router, prefix="/vendors/{vendor_code}/shop") # Path-based development mode app.include_router(shop_pages.router, prefix="/vendors/{vendor_code}/shop")
``` ```
**How This Works:**
1. **For Subdomain/Custom Domain Mode:**
- URL: `https://acme.wizamart.com/shop/products`
- Matches: First router with `/shop` prefix
- Route: `@router.get("/products")` → Full path: `/shop/products`
2. **For Path-Based Development Mode:**
- URL: `http://localhost:8000/vendors/acme/shop/products`
- Matches: Second router with `/vendors/{vendor_code}/shop` prefix
- Route: `@router.get("/products")` → Full path: `/vendors/{vendor_code}/shop/products`
- Bonus: `vendor_code` available as path parameter!
**Benefits:**
- ✅ No middleware complexity or path manipulation
- ✅ FastAPI native routing
- ✅ Explicit and maintainable
- ✅ Vendor code accessible via path parameter when needed
- ✅ Both deployment modes supported cleanly
--- ---
## Authentication in Multi-Tenant Shop ## Authentication in Multi-Tenant Shop

View File

@@ -200,23 +200,31 @@ Middleware for request/response logging and performance monitoring.
--- ---
## Path Rewriting ## Path-Based Routing Solution
### path_rewrite_middleware **Modern Approach: Double Router Mounting**
Middleware function that rewrites request paths for path-based vendor routing. Instead of using middleware to rewrite paths, the application registers shop routes **twice** with different prefixes:
**Purpose:** ```python
Allows `/vendors/VENDORCODE/shop/products` (path-based development mode) to be internally routed as `/shop/products` for proper FastAPI route matching. # In main.py
app.include_router(shop_pages.router, prefix="/shop")
app.include_router(shop_pages.router, prefix="/vendors/{vendor_code}/shop")
```
**Execution Order:** **How It Works:**
Must run AFTER VendorContextMiddleware and BEFORE ContextDetectionMiddleware. - **Subdomain/Custom Domain Mode**: Routes match `/shop/*` prefix
- **Path-Based Development Mode**: Routes match `/vendors/{vendor_code}/shop/*` prefix
- FastAPI handles routing naturally without path manipulation
- Vendor code is available as a path parameter when needed
::: middleware.path_rewrite_middleware.path_rewrite_middleware **Benefits:**
options: - ✅ No middleware complexity
show_source: true - ✅ Explicit route definitions
heading_level: 4 - ✅ FastAPI native routing
show_root_heading: false - ✅ Vendor code accessible via path parameter
**Note:** Previous implementations used `path_rewrite_middleware` to rewrite paths at runtime. This approach has been deprecated in favor of double mounting, which is simpler and more maintainable.
--- ---
@@ -228,18 +236,19 @@ The middleware stack must be configured in the correct order for proper function
graph TD graph TD
A[Request] --> B[LoggingMiddleware] A[Request] --> B[LoggingMiddleware]
B --> C[VendorContextMiddleware] B --> C[VendorContextMiddleware]
C --> D[PathRewriteMiddleware] C --> D[ContextMiddleware]
D --> E[ContextMiddleware] D --> E[ThemeContextMiddleware]
E --> F[ThemeContextMiddleware] E --> F[Application Routes]
F --> G[Application Routes] F --> G[Response]
G --> H[Response]
``` ```
**Critical Dependencies:** **Critical Dependencies:**
1. **VendorContextMiddleware** must run first to detect vendor 1. **LoggingMiddleware** runs first for request timing
2. **PathRewriteMiddleware** needs `clean_path` from VendorContext 2. **VendorContextMiddleware** detects vendor and sets clean_path
3. **ContextMiddleware** needs rewritten path 3. **ContextMiddleware** detects context type (API/Admin/Vendor/Shop)
4. **ThemeContextMiddleware** needs context type 4. **ThemeContextMiddleware** loads vendor theme based on context
**Note:** Path-based routing (e.g., `/vendors/{code}/shop/*`) is handled by double router mounting in `main.py`, not by middleware.
--- ---

View File

@@ -289,13 +289,12 @@ Understanding how a request flows through Wizamart:
graph LR graph LR
A[Client Request] --> B[LoggingMiddleware] A[Client Request] --> B[LoggingMiddleware]
B --> C[VendorContextMiddleware] B --> C[VendorContextMiddleware]
C --> D[PathRewriteMiddleware] C --> D[ContextDetectionMiddleware]
D --> E[ContextDetectionMiddleware] D --> E[ThemeContextMiddleware]
E --> F[ThemeContextMiddleware] E --> F[FastAPI Router]
F --> G[FastAPI Router] F --> G{Request Type}
G --> H{Request Type} G -->|API| H[JSON Response]
H -->|API| I[JSON Response] G -->|Page| I[HTML Template]
H -->|Page| J[HTML Template]
``` ```
**Learn more**: [Request Flow](architecture/request-flow.md) **Learn more**: [Request Flow](architecture/request-flow.md)

View File

@@ -1,63 +0,0 @@
# middleware/path_rewrite_middleware.py
"""
Path Rewrite Middleware
Rewrites request paths for path-based vendor routing.
This allows /vendor/VENDORCODE/shop/products to be routed as /shop/products
MUST run AFTER vendor_context_middleware and BEFORE context_middleware.
"""
import logging
from fastapi import Request
from starlette.datastructures import URL
logger = logging.getLogger(__name__)
async def path_rewrite_middleware(request: Request, call_next):
"""
Middleware to rewrite request paths for vendor context.
If vendor_context_middleware set request.state.clean_path, this middleware
will rewrite the request path to use the clean path instead.
This allows FastAPI route matching to work correctly with path-based routing.
Example:
Original: /vendor/WIZAMART/shop/products
Clean path: /shop/products
After rewrite: Request is routed as if path was /shop/products
MUST run after vendor_context_middleware (which sets clean_path)
MUST run before context_middleware (which needs to see the clean path)
"""
# Check if vendor_context_middleware set a clean_path
if hasattr(request.state, 'clean_path'):
clean_path = request.state.clean_path
original_path = request.url.path
# Only rewrite if clean_path is different from original path
if clean_path != original_path:
logger.debug(
f"[PATH_REWRITE] Rewriting path",
extra={
"original_path": original_path,
"clean_path": clean_path,
"vendor": getattr(request.state, 'vendor', 'NOT SET'),
}
)
# Rewrite the path by modifying the request's scope
# This affects how FastAPI's router will see the path
request.scope['path'] = clean_path
# Also update request._url to reflect the change
# This ensures request.url.path returns the rewritten path
old_url = request.url
new_url = old_url.replace(path=clean_path)
request._url = new_url
# Continue to next middleware/handler
response = await call_next(request)
return response

View File

@@ -1,17 +1,18 @@
# tests/unit/middleware/test_theme_logging_path_decorators.py # tests/unit/middleware/test_theme_logging_path_decorators.py
""" """
Comprehensive unit tests for remaining middleware components: Comprehensive unit tests for middleware components:
- ThemeContextMiddleware and ThemeContextManager - ThemeContextMiddleware and ThemeContextManager
- LoggingMiddleware - LoggingMiddleware
- path_rewrite_middleware
- rate_limit decorator - rate_limit decorator
Tests cover: Tests cover:
- Theme loading and caching - Theme loading and caching
- Request/response logging - Request/response logging
- Path rewriting for vendor routing
- Rate limit decorators - Rate limit decorators
- Edge cases and error handling - Edge cases and error handling
Note: path_rewrite_middleware has been deprecated in favor of double router mounting.
See main.py for current implementation using app.include_router() with different prefixes.
""" """
import pytest import pytest
@@ -25,7 +26,6 @@ from middleware.theme_context import (
get_current_theme, get_current_theme,
) )
from middleware.logging_middleware import LoggingMiddleware from middleware.logging_middleware import LoggingMiddleware
from middleware.path_rewrite_middleware import path_rewrite_middleware
from middleware.decorators import rate_limit from middleware.decorators import rate_limit
from app.exceptions.base import RateLimitException from app.exceptions.base import RateLimitException
@@ -351,98 +351,6 @@ class TestLoggingMiddleware:
assert process_time >= 0.1 assert process_time >= 0.1
# =============================================================================
# Path Rewrite Middleware Tests
# =============================================================================
@pytest.mark.unit
class TestPathRewriteMiddleware:
"""Test suite for path_rewrite_middleware."""
@pytest.mark.asyncio
async def test_rewrites_path_when_clean_path_different(self):
"""Test path is rewritten when clean_path differs from original."""
request = Mock(spec=Request)
request.url = Mock(path="/vendor/testvendor/shop/products")
request.state = Mock(clean_path="/shop/products")
request.scope = {"path": "/vendor/testvendor/shop/products"}
call_next = AsyncMock(return_value=Mock())
await path_rewrite_middleware(request, call_next)
# Path should be rewritten in scope
assert request.scope["path"] == "/shop/products"
call_next.assert_called_once_with(request)
@pytest.mark.asyncio
async def test_does_not_rewrite_when_paths_same(self):
"""Test path is not rewritten when clean_path same as original."""
request = Mock(spec=Request)
original_path = "/shop/products"
request.url = Mock(path=original_path)
request.state = Mock(clean_path=original_path)
request.scope = {"path": original_path}
call_next = AsyncMock(return_value=Mock())
await path_rewrite_middleware(request, call_next)
# Path should remain unchanged
assert request.scope["path"] == original_path
call_next.assert_called_once()
@pytest.mark.asyncio
async def test_does_nothing_when_no_clean_path(self):
"""Test middleware does nothing when no clean_path set."""
request = Mock(spec=Request)
request.url = Mock(path="/shop/products")
request.state = Mock(spec=[]) # No clean_path attribute
original_path = "/shop/products"
request.scope = {"path": original_path}
call_next = AsyncMock(return_value=Mock())
await path_rewrite_middleware(request, call_next)
# Path should remain unchanged
assert request.scope["path"] == original_path
call_next.assert_called_once()
@pytest.mark.asyncio
async def test_updates_request_url(self):
"""Test middleware updates request._url."""
request = Mock(spec=Request)
original_url = Mock(path="/vendor/test/shop")
request.url = original_url
request.url.replace = Mock(return_value=Mock(path="/shop"))
request.state = Mock(clean_path="/shop")
request.scope = {"path": "/vendor/test/shop"}
call_next = AsyncMock(return_value=Mock())
await path_rewrite_middleware(request, call_next)
# URL replace should have been called
request.url.replace.assert_called_once_with(path="/shop")
@pytest.mark.asyncio
async def test_preserves_vendor_context(self):
"""Test middleware preserves vendor context in request.state."""
request = Mock(spec=Request)
request.url = Mock(path="/vendor/testvendor/products")
mock_vendor = Mock()
request.state = Mock(clean_path="/products", vendor=mock_vendor)
request.scope = {"path": "/vendor/testvendor/products"}
call_next = AsyncMock(return_value=Mock())
await path_rewrite_middleware(request, call_next)
# Vendor should still be accessible
assert request.state.vendor is mock_vendor
# ============================================================================= # =============================================================================
# Rate Limit Decorator Tests # Rate Limit Decorator Tests
# ============================================================================= # =============================================================================
@@ -560,22 +468,6 @@ class TestMiddlewareEdgeCases:
# Verify database was closed # Verify database was closed
mock_db.close.assert_called_once() mock_db.close.assert_called_once()
@pytest.mark.asyncio
async def test_path_rewrite_with_query_parameters(self):
"""Test path rewrite preserves query parameters."""
request = Mock(spec=Request)
original_url = Mock(path="/vendor/test/shop?page=1")
request.url = original_url
request.url.replace = Mock(return_value=Mock(path="/shop?page=1"))
request.state = Mock(clean_path="/shop?page=1")
request.scope = {"path": "/vendor/test/shop?page=1"}
call_next = AsyncMock(return_value=Mock())
await path_rewrite_middleware(request, call_next)
request.url.replace.assert_called_once_with(path="/shop?page=1")
def test_theme_default_immutability(self): def test_theme_default_immutability(self):
"""Test that getting default theme doesn't share state.""" """Test that getting default theme doesn't share state."""
theme1 = ThemeContextManager.get_default_theme() theme1 = ThemeContextManager.get_default_theme()