diff --git a/.architecture-rules.yaml b/.architecture-rules.yaml index 248f08b9..5e85c959 100644 --- a/.architecture-rules.yaml +++ b/.architecture-rules.yaml @@ -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) # ============================================================================ diff --git a/scripts/validate_architecture.py b/scripts/validate_architecture.py index d85b8945..42941ae7 100755 --- a/scripts/validate_architecture.py +++ b/scripts/validate_architecture.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...")