refactor: enforce strict architecture rules and add Pydantic response models
- Update architecture rules to be stricter (API-003 now blocks ALL exception raising in endpoints, not just HTTPException) - Update get_current_vendor_api dependency to guarantee token_vendor_id presence - Remove redundant _get_vendor_from_token helpers from all vendor API files - Move vendor access validation to service layer methods - Add Pydantic response models for media, notification, and payment endpoints - Add get_active_vendor_by_code service method for public vendor lookup - Add get_import_job_for_vendor service method with vendor validation - Update validation script to detect exception raising patterns in endpoints 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
@@ -517,39 +517,72 @@ class ArchitectureValidator:
|
||||
def _check_endpoint_exception_handling(
|
||||
self, file_path: Path, content: str, lines: list[str]
|
||||
):
|
||||
"""API-003: Check that endpoints do NOT raise HTTPException directly.
|
||||
"""API-003: Check that endpoints do NOT raise exceptions directly.
|
||||
|
||||
The architecture uses a global exception handler that catches domain
|
||||
exceptions (WizamartException subclasses) and converts them to HTTP
|
||||
responses. Endpoints should let exceptions bubble up, not catch and
|
||||
convert them manually.
|
||||
The architecture uses:
|
||||
- Dependencies (deps.py) for authentication/authorization validation
|
||||
- Services for business logic validation
|
||||
- Global exception handler that catches WizamartException subclasses
|
||||
|
||||
Endpoints should be a thin orchestration layer that trusts dependencies
|
||||
and services to handle all validation. They should NOT raise exceptions.
|
||||
"""
|
||||
rule = self._get_rule("API-003")
|
||||
if not rule:
|
||||
return
|
||||
|
||||
# Skip exception handler file - it's allowed to use HTTPException
|
||||
if "exceptions/handler.py" in str(file_path):
|
||||
# Skip exception handler file and deps.py - they're allowed to raise exceptions
|
||||
file_path_str = str(file_path)
|
||||
if "exceptions/handler.py" in file_path_str or file_path_str.endswith("deps.py"):
|
||||
return
|
||||
|
||||
for i, line in enumerate(lines, 1):
|
||||
# Check for raise HTTPException
|
||||
if "raise HTTPException" in line:
|
||||
# Skip if it's a comment
|
||||
stripped = line.strip()
|
||||
if stripped.startswith("#"):
|
||||
continue
|
||||
# Patterns that indicate endpoints are raising exceptions (BAD)
|
||||
exception_patterns = [
|
||||
("raise HTTPException", "Endpoint raises HTTPException directly"),
|
||||
("raise InvalidTokenException", "Endpoint raises InvalidTokenException - move to dependency"),
|
||||
("raise InsufficientPermissionsException", "Endpoint raises permission exception - move to dependency"),
|
||||
("raise UnauthorizedVendorAccessException", "Endpoint raises auth exception - move to dependency or service"),
|
||||
]
|
||||
|
||||
self._add_violation(
|
||||
rule_id="API-003",
|
||||
rule_name=rule["name"],
|
||||
severity=Severity.ERROR,
|
||||
file_path=file_path,
|
||||
line_number=i,
|
||||
message="Endpoint raises HTTPException directly",
|
||||
context=line.strip()[:80],
|
||||
suggestion="Use domain exceptions (e.g., VendorNotFoundException) and let global handler convert",
|
||||
)
|
||||
# Pattern that indicates redundant validation (BAD)
|
||||
redundant_patterns = [
|
||||
(r"if not hasattr\(current_user.*token_vendor", "Redundant token_vendor check - get_current_vendor_api guarantees this"),
|
||||
(r"if not hasattr\(current_user.*token_vendor_id", "Redundant token_vendor_id check - dependency guarantees this"),
|
||||
]
|
||||
|
||||
for i, line in enumerate(lines, 1):
|
||||
# Skip comments
|
||||
stripped = line.strip()
|
||||
if stripped.startswith("#"):
|
||||
continue
|
||||
|
||||
# Check for direct exception raising
|
||||
for pattern, message in exception_patterns:
|
||||
if pattern in line:
|
||||
self._add_violation(
|
||||
rule_id="API-003",
|
||||
rule_name=rule["name"],
|
||||
severity=Severity.ERROR,
|
||||
file_path=file_path,
|
||||
line_number=i,
|
||||
message=message,
|
||||
context=stripped[:80],
|
||||
suggestion="Let dependencies or services handle validation and raise exceptions",
|
||||
)
|
||||
|
||||
# Check for redundant validation patterns
|
||||
for pattern, message in redundant_patterns:
|
||||
if re.search(pattern, line):
|
||||
self._add_violation(
|
||||
rule_id="API-003",
|
||||
rule_name=rule["name"],
|
||||
severity=Severity.ERROR,
|
||||
file_path=file_path,
|
||||
line_number=i,
|
||||
message=message,
|
||||
context=stripped[:80],
|
||||
suggestion="Remove redundant check - auth dependency guarantees this attribute is present",
|
||||
)
|
||||
|
||||
def _check_endpoint_authentication(
|
||||
self, file_path: Path, content: str, lines: list[str]
|
||||
@@ -570,7 +603,21 @@ class ArchitectureValidator:
|
||||
return
|
||||
|
||||
# This is a warning-level check
|
||||
# Look for endpoints without Depends(get_current_*)
|
||||
# Look for endpoints without proper authentication
|
||||
# Valid auth patterns:
|
||||
# - Depends(get_current_*) - direct user authentication
|
||||
# - Depends(require_vendor_*) - vendor permission dependencies
|
||||
# - Depends(require_any_vendor_*) - any permission check
|
||||
# - Depends(require_all_vendor*) - all permissions check
|
||||
# - Depends(get_user_permissions) - permission fetching
|
||||
auth_patterns = [
|
||||
"Depends(get_current_",
|
||||
"Depends(require_vendor_",
|
||||
"Depends(require_any_vendor_",
|
||||
"Depends(require_all_vendor",
|
||||
"Depends(get_user_permissions",
|
||||
]
|
||||
|
||||
for i, line in enumerate(lines, 1):
|
||||
if "@router." in line and (
|
||||
"post" in line or "put" in line or "delete" in line
|
||||
@@ -582,7 +629,8 @@ class ArchitectureValidator:
|
||||
context_lines = lines[i - 1 : i + 15] # Include line before decorator
|
||||
|
||||
for ctx_line in context_lines:
|
||||
if "Depends(get_current_" in ctx_line:
|
||||
# Check for any valid auth pattern
|
||||
if any(pattern in ctx_line for pattern in auth_patterns):
|
||||
has_auth = True
|
||||
break
|
||||
# Check for public endpoint markers
|
||||
@@ -593,6 +641,17 @@ class ArchitectureValidator:
|
||||
if not has_auth and not is_public and "include_in_schema=False" not in " ".join(
|
||||
lines[i : i + 15]
|
||||
):
|
||||
# Determine appropriate suggestion based on file path
|
||||
file_path_str = str(file_path)
|
||||
if "/vendor/" in file_path_str:
|
||||
suggestion = "Add Depends(get_current_vendor_api) or permission dependency, or mark as '# public'"
|
||||
elif "/admin/" in file_path_str:
|
||||
suggestion = "Add Depends(get_current_admin_api), or mark as '# public'"
|
||||
elif "/shop/" in file_path_str:
|
||||
suggestion = "Add Depends(get_current_customer_api), or mark as '# public'"
|
||||
else:
|
||||
suggestion = "Add authentication dependency or mark as '# public' if intentionally unauthenticated"
|
||||
|
||||
self._add_violation(
|
||||
rule_id="API-004",
|
||||
rule_name=rule["name"],
|
||||
@@ -601,7 +660,7 @@ class ArchitectureValidator:
|
||||
line_number=i,
|
||||
message="Endpoint may be missing authentication",
|
||||
context=line.strip(),
|
||||
suggestion="Add Depends(get_current_user) or mark as '# public' if intentionally unauthenticated",
|
||||
suggestion=suggestion,
|
||||
)
|
||||
|
||||
def _validate_service_layer(self, target_path: Path):
|
||||
|
||||
Reference in New Issue
Block a user