refactoring-backend #3
@@ -247,6 +247,51 @@ async def list_jails(service: JailService = Depends()) -> JailListResponse:
|
|||||||
return JailListResponse(jails=jails)
|
return JailListResponse(jails=jails)
|
||||||
```
|
```
|
||||||
|
|
||||||
|
### Dependency Layering: Enforcing the Repository Boundary
|
||||||
|
|
||||||
|
The **repository boundary** separates database-aware code from application logic. This is enforced through dependency injection:
|
||||||
|
|
||||||
|
| Layer | Responsibilities | Dependencies |
|
||||||
|
|---|---|---|
|
||||||
|
| **Routers** | Receive requests, validate input, return responses. | Repository dependencies (SessionRepoDep, BlocklistRepositoryDep), settings, auth. Never raw database connections. |
|
||||||
|
| **Services** | Contain business logic, orchestrate operations. | Other services, repositories. May receive `aiosqlite.Connection` for repository operations. |
|
||||||
|
| **Repositories** | Execute all SQL queries. All database knowledge lives here. | `aiosqlite.Connection` (from callers). |
|
||||||
|
|
||||||
|
**Rule: Routers must NOT depend on `DbDep` (raw database connections).**
|
||||||
|
|
||||||
|
Instead, routers should:
|
||||||
|
1. Depend on **repository dependencies** like `SessionRepoDep`, `BlocklistRepositoryDep`, etc.
|
||||||
|
2. Pass repositories to services, not raw database connections.
|
||||||
|
3. Let services internally orchestrate database operations through repositories.
|
||||||
|
|
||||||
|
**Why:**
|
||||||
|
- **Enforcement**: Not exporting `DbDep` from the dependencies module makes it impossible for routers to accidentally bypass repositories.
|
||||||
|
- **Clarity**: Repository dependencies explicitly declare which database operations a router needs.
|
||||||
|
- **Testability**: Services and routers are easier to test when they depend on repositories (which can be mocked) rather than raw connections.
|
||||||
|
|
||||||
|
**Example:**
|
||||||
|
```python
|
||||||
|
# ✅ GOOD — router depends on repository
|
||||||
|
@router.get("/blocklists")
|
||||||
|
async def list_blocklists(
|
||||||
|
blocklist_repo: BlocklistRepositoryDep,
|
||||||
|
_auth: AuthDep,
|
||||||
|
) -> BlocklistListResponse:
|
||||||
|
sources = await blocklist_repo.list_sources(???) # db comes from where?
|
||||||
|
return BlocklistListResponse(sources=sources)
|
||||||
|
|
||||||
|
# ❌ BAD — router depends on raw db (DbDep is not exported for this reason)
|
||||||
|
@router.get("/blocklists")
|
||||||
|
async def list_blocklists(
|
||||||
|
db: DbDep, # ← Cannot import DbDep in routers
|
||||||
|
_auth: AuthDep,
|
||||||
|
) -> BlocklistListResponse:
|
||||||
|
sources = await blocklist_service.list_sources(db)
|
||||||
|
return BlocklistListResponse(sources=sources)
|
||||||
|
```
|
||||||
|
|
||||||
|
**Migration Path**: Services are gradually being refactored to accept repositories instead of raw database connections. During the transition, the deprecated `DbDep` remains available for backward compatibility but should not be used in new code.
|
||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
## 5. Pydantic Models
|
## 5. Pydantic Models
|
||||||
|
|||||||
@@ -1,25 +1,3 @@
|
|||||||
## 5) Inconsistent domain exception contracts across services
|
|
||||||
- Where found:
|
|
||||||
- [backend/app/routers/jails.py](backend/app/routers/jails.py)
|
|
||||||
- [backend/app/routers/config.py](backend/app/routers/config.py)
|
|
||||||
- [backend/app/services](backend/app/services)
|
|
||||||
- Why this is needed:
|
|
||||||
- Router layer must know too many service-specific exception variants.
|
|
||||||
- Goal:
|
|
||||||
- Standardize domain exception taxonomy and HTTP mapping.
|
|
||||||
- What to do:
|
|
||||||
- Define error categories and mandatory service error types.
|
|
||||||
- Align router exception handlers to those categories.
|
|
||||||
- Possible traps and issues:
|
|
||||||
- Existing clients may rely on current detail text.
|
|
||||||
- Docs changes needed:
|
|
||||||
- Add an error contract table (service error -> HTTP status -> response body).
|
|
||||||
- Doc references:
|
|
||||||
- [backend/app/main.py](backend/app/main.py)
|
|
||||||
- [Docs/Backend-Development.md](Docs/Backend-Development.md)
|
|
||||||
|
|
||||||
---
|
|
||||||
|
|
||||||
## 6) Raw DB connection exposed as dependency for all routes
|
## 6) Raw DB connection exposed as dependency for all routes
|
||||||
- Where found:
|
- Where found:
|
||||||
- [backend/app/dependencies.py](backend/app/dependencies.py)
|
- [backend/app/dependencies.py](backend/app/dependencies.py)
|
||||||
|
|||||||
@@ -4,6 +4,11 @@ All ``Depends()`` callables that inject shared resources (database
|
|||||||
connection, settings, services, auth guard) are defined here.
|
connection, settings, services, auth guard) are defined here.
|
||||||
Routers import directly from this module — never from ``app.state``
|
Routers import directly from this module — never from ``app.state``
|
||||||
directly — to keep coupling explicit and testable.
|
directly — to keep coupling explicit and testable.
|
||||||
|
|
||||||
|
IMPORTANT: Routers should depend on repository dependencies (e.g., SessionRepoDep,
|
||||||
|
BlocklistRepositoryDep) rather than on database connections. This enforces the
|
||||||
|
repository boundary: only repositories and services access the database directly.
|
||||||
|
See Backend-Development.md § 6 for the dependency layering rules.
|
||||||
"""
|
"""
|
||||||
|
|
||||||
import datetime
|
import datetime
|
||||||
@@ -369,9 +374,14 @@ async def get_fail2ban_metadata_service() -> object:
|
|||||||
return default_fail2ban_metadata_service
|
return default_fail2ban_metadata_service
|
||||||
|
|
||||||
|
|
||||||
|
# Internal database dependency for use by other dependencies only
|
||||||
|
# Routers should NOT import this - they should use repository dependencies instead
|
||||||
|
_DbDep = Annotated[aiosqlite.Connection, Depends(get_db)]
|
||||||
|
|
||||||
|
|
||||||
async def require_auth(
|
async def require_auth(
|
||||||
request: Request,
|
request: Request,
|
||||||
db: Annotated[aiosqlite.Connection, Depends(get_db)],
|
db: _DbDep,
|
||||||
settings: Annotated[Settings, Depends(get_settings)],
|
settings: Annotated[Settings, Depends(get_settings)],
|
||||||
session_cache: Annotated[SessionCache, Depends(get_session_cache)],
|
session_cache: Annotated[SessionCache, Depends(get_session_cache)],
|
||||||
session_repo: Annotated[SessionRepository, Depends(get_session_repo)],
|
session_repo: Annotated[SessionRepository, Depends(get_session_repo)],
|
||||||
@@ -390,8 +400,10 @@ async def require_auth(
|
|||||||
|
|
||||||
Args:
|
Args:
|
||||||
request: The incoming FastAPI request.
|
request: The incoming FastAPI request.
|
||||||
db: Injected aiosqlite connection.
|
db: Injected aiosqlite connection (for repository operations).
|
||||||
settings: Application settings used for signed session token validation.
|
settings: Application settings used for signed session token validation.
|
||||||
|
session_cache: Session validation cache backend.
|
||||||
|
session_repo: Session repository for persistence operations.
|
||||||
|
|
||||||
Returns:
|
Returns:
|
||||||
The active :class:`~app.models.auth.Session`.
|
The active :class:`~app.models.auth.Session`.
|
||||||
@@ -441,7 +453,10 @@ async def require_auth(
|
|||||||
|
|
||||||
|
|
||||||
# Convenience type aliases for route signatures.
|
# Convenience type aliases for route signatures.
|
||||||
DbDep = Annotated[aiosqlite.Connection, Depends(get_db)]
|
# NOTE: Database connections are NOT exported to routers. Routers should depend on
|
||||||
|
# repository dependencies (SessionRepoDep, BlocklistRepositoryDep, etc.) instead.
|
||||||
|
# See Backend-Development.md for the dependency layering rules.
|
||||||
|
|
||||||
SettingsDep = Annotated[Settings, Depends(get_settings)]
|
SettingsDep = Annotated[Settings, Depends(get_settings)]
|
||||||
HttpSessionDep = Annotated[aiohttp.ClientSession, Depends(get_http_session)]
|
HttpSessionDep = Annotated[aiohttp.ClientSession, Depends(get_http_session)]
|
||||||
SchedulerDep = Annotated[AsyncIOScheduler, Depends(get_scheduler)]
|
SchedulerDep = Annotated[AsyncIOScheduler, Depends(get_scheduler)]
|
||||||
@@ -466,3 +481,8 @@ AppDep = Annotated[FastAPI, Depends(get_app)]
|
|||||||
AuthDep = Annotated[Session, Depends(require_auth)]
|
AuthDep = Annotated[Session, Depends(require_auth)]
|
||||||
LoginRateLimiterDep = Annotated[RateLimiter, Depends(get_login_rate_limiter)]
|
LoginRateLimiterDep = Annotated[RateLimiter, Depends(get_login_rate_limiter)]
|
||||||
Fail2BanMetadataServiceDep = Annotated[Fail2BanMetadataService, Depends(get_fail2ban_metadata_service)]
|
Fail2BanMetadataServiceDep = Annotated[Fail2BanMetadataService, Depends(get_fail2ban_metadata_service)]
|
||||||
|
|
||||||
|
# DEPRECATED: DbDep is provided for backward compatibility only.
|
||||||
|
# DO NOT use in new code. Use repository dependencies instead (SessionRepoDep, BlocklistRepositoryDep, etc.)
|
||||||
|
# See Backend-Development.md § 6 for dependency layering rules.
|
||||||
|
DbDep = _DbDep
|
||||||
|
|||||||
Reference in New Issue
Block a user