refactor(arch): clarify transaction control pattern (API-002, SVC-006)
API-002 updated: - Remove db.commit() from anti-patterns (allowed at endpoint level) - Add db.delete() to anti-patterns (business logic) - Clarify that transaction control != business logic SVC-006 added (new rule): - Services should NOT call db.commit() - Transaction control belongs at endpoint level - Exception: log_service.py for audit log commits - Severity: warning (to allow gradual migration) This aligns with industry standard: - One request = one transaction - Services do work, endpoints control commits - Enables composing multiple service calls in single transaction 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
@@ -58,12 +58,21 @@ api_endpoint_rules:
|
||||
description: |
|
||||
API endpoints should only handle HTTP concerns (validation, auth, response formatting).
|
||||
All business logic must be delegated to service layer.
|
||||
|
||||
Transaction control (db.commit) IS allowed at endpoint level - this is the recommended
|
||||
pattern for request-scoped transactions. One request = one transaction.
|
||||
|
||||
What's NOT allowed:
|
||||
- db.add() - creating entities is business logic
|
||||
- db.query() - complex queries are business logic
|
||||
- db.delete() - deleting entities is business logic
|
||||
pattern:
|
||||
file_pattern: "app/api/v1/**/*.py"
|
||||
anti_patterns:
|
||||
- "db.add("
|
||||
- "db.commit()"
|
||||
- "db.delete("
|
||||
- "db.query("
|
||||
# NOTE: db.commit() is intentionally NOT listed - it's allowed for transaction control
|
||||
|
||||
- id: "API-003"
|
||||
name: "Endpoint must NOT raise HTTPException directly"
|
||||
@@ -166,6 +175,26 @@ service_layer_rules:
|
||||
file_pattern: "app/services/**/*.py"
|
||||
check: "vendor_scoping"
|
||||
|
||||
- id: "SVC-006"
|
||||
name: "Service must NOT call db.commit()"
|
||||
severity: "warning"
|
||||
description: |
|
||||
Services should NOT commit transactions. Transaction control belongs at the
|
||||
API endpoint level where one request = one transaction.
|
||||
|
||||
This allows:
|
||||
- Composing multiple service calls in a single transaction
|
||||
- Clean rollback on any failure
|
||||
- Easier testing of services in isolation
|
||||
|
||||
The endpoint should call db.commit() after all service operations succeed.
|
||||
pattern:
|
||||
file_pattern: "app/services/**/*.py"
|
||||
anti_patterns:
|
||||
- "db.commit()"
|
||||
exceptions:
|
||||
- "log_service.py" # Audit logs may need immediate commits
|
||||
|
||||
# ============================================================================
|
||||
# MODEL RULES (models/database/*.py, models/schema/*.py)
|
||||
# ============================================================================
|
||||
|
||||
@@ -467,9 +467,12 @@ class ArchitectureValidator:
|
||||
if not rule:
|
||||
return
|
||||
|
||||
# NOTE: db.commit() is intentionally NOT included here
|
||||
# Transaction control (commit) is allowed at endpoint level
|
||||
# Only business logic operations are flagged
|
||||
anti_patterns = [
|
||||
(r"db\.add\(", "Database operations should be in service layer"),
|
||||
(r"db\.commit\(\)", "Database commits should be in service layer"),
|
||||
(r"db\.add\(", "Creating entities should be in service layer"),
|
||||
(r"db\.delete\(", "Deleting entities should be in service layer"),
|
||||
(r"db\.query\(", "Database queries should be in service layer"),
|
||||
(r"db\.execute\(", "Database operations should be in service layer"),
|
||||
]
|
||||
@@ -565,7 +568,7 @@ class ArchitectureValidator:
|
||||
)
|
||||
|
||||
def _validate_service_layer(self, target_path: Path):
|
||||
"""Validate service layer rules (SVC-001, SVC-002, SVC-003, SVC-004)"""
|
||||
"""Validate service layer rules (SVC-001, SVC-002, SVC-003, SVC-004, SVC-006)"""
|
||||
print("🔧 Validating service layer...")
|
||||
|
||||
service_files = list(target_path.glob("app/services/**/*.py"))
|
||||
@@ -587,6 +590,9 @@ class ArchitectureValidator:
|
||||
# SVC-003: DB session as parameter
|
||||
self._check_db_session_parameter(file_path, content, lines)
|
||||
|
||||
# SVC-006: No db.commit() in services
|
||||
self._check_no_commit_in_services(file_path, content, lines)
|
||||
|
||||
def _check_no_http_exception_in_services(
|
||||
self, file_path: Path, content: str, lines: list[str]
|
||||
):
|
||||
@@ -667,6 +673,40 @@ class ArchitectureValidator:
|
||||
suggestion="Accept db: Session as method parameter instead",
|
||||
)
|
||||
|
||||
def _check_no_commit_in_services(
|
||||
self, file_path: Path, content: str, lines: list[str]
|
||||
):
|
||||
"""SVC-006: Services should NOT call db.commit()
|
||||
|
||||
Transaction control belongs at the API endpoint level.
|
||||
Exception: log_service.py may need immediate commits for audit logs.
|
||||
"""
|
||||
rule = self._get_rule("SVC-006")
|
||||
if not rule:
|
||||
return
|
||||
|
||||
# Exception: log_service.py is allowed to commit (audit logs)
|
||||
if "log_service.py" in str(file_path):
|
||||
return
|
||||
|
||||
for i, line in enumerate(lines, 1):
|
||||
if "db.commit()" in line:
|
||||
# Skip if it's a comment
|
||||
stripped = line.strip()
|
||||
if stripped.startswith("#"):
|
||||
continue
|
||||
|
||||
self._add_violation(
|
||||
rule_id="SVC-006",
|
||||
rule_name=rule["name"],
|
||||
severity=Severity.WARNING,
|
||||
file_path=file_path,
|
||||
line_number=i,
|
||||
message="Service calls db.commit() - transaction control should be at endpoint level",
|
||||
context=stripped,
|
||||
suggestion="Remove db.commit() from service; let endpoint handle transaction",
|
||||
)
|
||||
|
||||
def _validate_models(self, target_path: Path):
|
||||
"""Validate model rules"""
|
||||
print("📦 Validating models...")
|
||||
|
||||
Reference in New Issue
Block a user