refactoring tasks
This commit is contained in:
238
Docs/Refactoring.md
Normal file
238
Docs/Refactoring.md
Normal file
@@ -0,0 +1,238 @@
|
|||||||
|
# BanGUI — Refactoring Instructions for AI Agents
|
||||||
|
|
||||||
|
This document is the single source of truth for any AI agent performing a refactoring task on the BanGUI codebase.
|
||||||
|
Read it in full before writing a single line of code.
|
||||||
|
The authoritative description of every module, its responsibilities, and the allowed dependency direction is in [Architekture.md](Architekture.md). Always cross-reference it.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## 0. Golden Rules
|
||||||
|
|
||||||
|
1. **Architecture first.** Every change must comply with the layered architecture defined in [Architekture.md §2](Architekture.md). Dependencies flow inward: `routers → services → repositories`. Never add an import that reverses this direction.
|
||||||
|
2. **One concern per file.** Each module has an explicitly stated purpose in [Architekture.md](Architekture.md). Do not add responsibilities to a module that do not belong there.
|
||||||
|
3. **No behaviour change.** Refactoring must preserve all existing behaviour. If a function's public signature, return value, or side-effects must change, that is a feature — create a separate task for it.
|
||||||
|
4. **Tests stay green.** Run the full test suite (`pytest backend/`) before and after every change. Do not submit work that introduces new failures.
|
||||||
|
5. **Smallest diff wins.** Prefer targeted edits. Do not rewrite a file when a few lines suffice.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## 1. Before You Start
|
||||||
|
|
||||||
|
### 1.1 Understand the project
|
||||||
|
|
||||||
|
Read the following documents in order:
|
||||||
|
|
||||||
|
1. [Architekture.md](Architekture.md) — full system overview, component map, module purposes, dependency rules.
|
||||||
|
2. [Docs/Backend-Development.md](Backend-Development.md) — coding conventions, testing strategy, environment setup.
|
||||||
|
3. [Docs/Tasks.md](Tasks.md) — open issues and planned work; avoid touching areas that have pending conflicting changes.
|
||||||
|
|
||||||
|
### 1.2 Map the code to the architecture
|
||||||
|
|
||||||
|
Before editing, locate every file that is in scope:
|
||||||
|
|
||||||
|
```
|
||||||
|
backend/app/
|
||||||
|
routers/ HTTP layer — zero business logic
|
||||||
|
services/ Business logic — orchestrates repositories + clients
|
||||||
|
repositories/ Data access — raw SQL only
|
||||||
|
models/ Pydantic schemas
|
||||||
|
tasks/ APScheduler jobs
|
||||||
|
utils/ Pure helpers, no framework deps
|
||||||
|
main.py App factory, lifespan, middleware
|
||||||
|
config.py Pydantic settings
|
||||||
|
dependencies.py FastAPI Depends() wiring
|
||||||
|
|
||||||
|
frontend/src/
|
||||||
|
api/ Typed fetch wrappers + endpoint constants
|
||||||
|
components/ Presentational UI, no API calls
|
||||||
|
hooks/ All state, side-effects, API calls
|
||||||
|
pages/ Route components — orchestration only
|
||||||
|
providers/ React context
|
||||||
|
types/ TypeScript interfaces
|
||||||
|
utils/ Pure helpers
|
||||||
|
```
|
||||||
|
|
||||||
|
Confirm which layer every file you intend to touch belongs to. If unsure, consult [Architekture.md §2.2](Architekture.md) (backend) or [Architekture.md §3.2](Architekture.md) (frontend).
|
||||||
|
|
||||||
|
### 1.3 Run the baseline
|
||||||
|
|
||||||
|
```bash
|
||||||
|
# Backend
|
||||||
|
pytest backend/ -x --tb=short
|
||||||
|
|
||||||
|
# Frontend
|
||||||
|
cd frontend && npm run test
|
||||||
|
```
|
||||||
|
|
||||||
|
Record the number of passing tests. After refactoring, that number must be equal or higher.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## 2. Backend Refactoring
|
||||||
|
|
||||||
|
### 2.1 Routers (`app/routers/`)
|
||||||
|
|
||||||
|
**Allowed content:** request parsing, response serialisation, dependency injection via `Depends()`, delegation to a service, HTTP error mapping.
|
||||||
|
**Forbidden content:** SQL queries, business logic, direct use of `fail2ban_client`, any logic that would also make sense in a unit test without an HTTP request.
|
||||||
|
|
||||||
|
Checklist:
|
||||||
|
- [ ] Every handler calls exactly one service method per logical operation.
|
||||||
|
- [ ] No `if`/`elif` chains that implement business rules — move these to the service.
|
||||||
|
- [ ] No raw SQL or repository imports.
|
||||||
|
- [ ] All response models are Pydantic schemas from `app/models/`.
|
||||||
|
- [ ] HTTP status codes are consistent with API conventions (200 OK, 201 Created, 204 No Content, 400/422 for client errors, 404 for missing resources, 500 only for unexpected failures).
|
||||||
|
|
||||||
|
### 2.2 Services (`app/services/`)
|
||||||
|
|
||||||
|
**Allowed content:** business rules, coordination between repositories and external clients, validation that goes beyond Pydantic, fail2ban command orchestration.
|
||||||
|
**Forbidden content:** raw SQL, direct aiosqlite calls, FastAPI `HTTPException` (raise domain exceptions instead and let the router or exception handler convert them).
|
||||||
|
|
||||||
|
Checklist:
|
||||||
|
- [ ] Service classes / functions accept plain Python types or domain models — not `Request` or `Response` objects.
|
||||||
|
- [ ] No direct `aiosqlite` usage — go through a repository.
|
||||||
|
- [ ] No `HTTPException` — raise a custom domain exception or a plain `ValueError`/`RuntimeError` with a clear message.
|
||||||
|
- [ ] No circular imports between services — if two services need each other's logic, extract the shared logic to a utility or a third service.
|
||||||
|
|
||||||
|
### 2.3 Repositories (`app/repositories/`)
|
||||||
|
|
||||||
|
**Allowed content:** SQL queries, result mapping to domain models, transaction management.
|
||||||
|
**Forbidden content:** business logic, fail2ban calls, HTTP concerns, logging beyond debug-level traces.
|
||||||
|
|
||||||
|
Checklist:
|
||||||
|
- [ ] Every public method accepts a `db: aiosqlite.Connection` parameter — sessions are not managed internally.
|
||||||
|
- [ ] Methods return typed domain models or plain Python primitives, never raw `aiosqlite.Row` objects exposed to callers.
|
||||||
|
- [ ] No business rules (e.g., no "if this setting is missing, create a default" logic — that belongs in the service).
|
||||||
|
|
||||||
|
### 2.4 Models (`app/models/`)
|
||||||
|
|
||||||
|
- Keep **Request**, **Response**, and **Domain** model types clearly separated (see [Architekture.md §2.2](Architekture.md)).
|
||||||
|
- Do not use response models as function arguments inside service or repository code.
|
||||||
|
- Validators (`@field_validator`, `@model_validator`) belong in models only when they concern data shape, not business rules.
|
||||||
|
|
||||||
|
### 2.5 Tasks (`app/tasks/`)
|
||||||
|
|
||||||
|
- Tasks must be thin: fetch inputs → call one service method → log result.
|
||||||
|
- Error handling must be inside the task (APScheduler swallows unhandled exceptions — log them explicitly).
|
||||||
|
- No direct repository or `fail2ban_client` use; go through a service.
|
||||||
|
|
||||||
|
### 2.6 Utils (`app/utils/`)
|
||||||
|
|
||||||
|
- Must have zero framework dependencies (no FastAPI, no aiosqlite imports).
|
||||||
|
- Must be pure or near-pure functions.
|
||||||
|
- `fail2ban_client.py` is the single exception — it wraps the socket protocol but still has no service-layer logic.
|
||||||
|
|
||||||
|
### 2.7 Dependencies (`app/dependencies.py`)
|
||||||
|
|
||||||
|
- This file is the **only** place where service constructors are called and injected.
|
||||||
|
- Do not construct services inside router handlers; always receive them via `Depends()`.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## 3. Frontend Refactoring
|
||||||
|
|
||||||
|
### 3.1 Pages (`src/pages/`)
|
||||||
|
|
||||||
|
**Allowed content:** composing components and hooks, layout decisions, routing.
|
||||||
|
**Forbidden content:** direct `fetch`/`axios` calls, inline business logic, state management beyond what is needed to coordinate child components.
|
||||||
|
|
||||||
|
Checklist:
|
||||||
|
- [ ] All data fetching goes through a hook from `src/hooks/`.
|
||||||
|
- [ ] No API function from `src/api/` is called directly inside a page component.
|
||||||
|
|
||||||
|
### 3.2 Components (`src/components/`)
|
||||||
|
|
||||||
|
**Allowed content:** rendering, styling, event handlers that call prop callbacks.
|
||||||
|
**Forbidden content:** API calls, hook-level state (prefer lifting state to the page or a dedicated hook), direct use of `src/api/`.
|
||||||
|
|
||||||
|
Checklist:
|
||||||
|
- [ ] Components receive all data via props.
|
||||||
|
- [ ] Components emit changes via callback props (`onXxx`).
|
||||||
|
- [ ] No `useEffect` that calls an API function — that belongs in a hook.
|
||||||
|
|
||||||
|
### 3.3 Hooks (`src/hooks/`)
|
||||||
|
|
||||||
|
**Allowed content:** `useState`, `useEffect`, `useCallback`, `useRef`; calls to `src/api/`; local state derivation.
|
||||||
|
**Forbidden content:** JSX rendering, Fluent UI components.
|
||||||
|
|
||||||
|
Checklist:
|
||||||
|
- [ ] Each hook has a single, focused concern matching its name (e.g., `useBans` only manages ban data).
|
||||||
|
- [ ] Hooks return a stable interface: `{ data, loading, error, refetch }` or equivalent.
|
||||||
|
- [ ] Shared logic between hooks is extracted to `src/utils/` (pure) or a parent hook (stateful).
|
||||||
|
|
||||||
|
### 3.4 API layer (`src/api/`)
|
||||||
|
|
||||||
|
- `client.ts` is the only place that calls `fetch`. All other api files call `client.ts`.
|
||||||
|
- `endpoints.ts` is the single source of truth for URL strings.
|
||||||
|
- API functions must be typed: explicit request and response TypeScript interfaces from `src/types/`.
|
||||||
|
|
||||||
|
### 3.5 Types (`src/types/`)
|
||||||
|
|
||||||
|
- Interfaces must match the backend Pydantic response schemas exactly (field names, optionality).
|
||||||
|
- Do not use `any`. Use `unknown` and narrow with type guards when the shape is genuinely unknown.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## 4. General Code Quality Rules
|
||||||
|
|
||||||
|
### Naming
|
||||||
|
- Python: `snake_case` for variables/functions, `PascalCase` for classes.
|
||||||
|
- TypeScript: `camelCase` for variables/functions, `PascalCase` for components and types.
|
||||||
|
- File names must match the primary export they contain.
|
||||||
|
|
||||||
|
### Error handling
|
||||||
|
- Backend: raise typed exceptions; map them to HTTP status codes in `main.py` exception handlers or in the router — nowhere else.
|
||||||
|
- Frontend: all API call error states are represented in hook return values; never swallow errors silently.
|
||||||
|
|
||||||
|
### Logging (backend)
|
||||||
|
- Use `structlog` with bound context loggers — never bare `print()`.
|
||||||
|
- Log at `debug` in repositories, `info` in services for meaningful events, `warning`/`error` in tasks and exception handlers.
|
||||||
|
- Never log sensitive data (passwords, session tokens, raw IP lists larger than a handful of entries).
|
||||||
|
|
||||||
|
### Async correctness (backend)
|
||||||
|
- Every function that touches I/O (database, fail2ban socket, HTTP) must be `async def`.
|
||||||
|
- Never call `asyncio.run()` inside a running event loop.
|
||||||
|
- Do not use `time.sleep()` — use `await asyncio.sleep()`.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## 5. Refactoring Workflow
|
||||||
|
|
||||||
|
Follow this sequence for every refactoring task:
|
||||||
|
|
||||||
|
1. **Read** the relevant section of [Architekture.md](Architekture.md) for the files you will touch.
|
||||||
|
2. **Run** the full test suite to confirm the baseline.
|
||||||
|
3. **Identify** the violation or smell: which rule from this document does it break?
|
||||||
|
4. **Plan** the minimal change: what is the smallest edit that fixes the violation?
|
||||||
|
5. **Edit** the code. One logical change per commit.
|
||||||
|
6. **Verify** imports: nothing new violates the dependency direction.
|
||||||
|
7. **Run** the full test suite. All previously passing tests must still pass.
|
||||||
|
8. **Update** any affected docstrings or inline comments to reflect the new structure.
|
||||||
|
9. **Do not** update `Architekture.md` unless the refactor changes the documented structure — that requires a separate review.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## 6. Common Violations to Look For
|
||||||
|
|
||||||
|
| Violation | Where it typically appears | Fix |
|
||||||
|
|---|---|---|
|
||||||
|
| Business logic in a router handler | `app/routers/*.py` | Extract logic to the corresponding service |
|
||||||
|
| Direct `aiosqlite` calls in a service | `app/services/*.py` | Move the query into the matching repository |
|
||||||
|
| `HTTPException` raised inside a service | `app/services/*.py` | Raise a domain exception; catch and convert it in the router or exception handler |
|
||||||
|
| API call inside a React component | `src/components/*.tsx` | Move to a hook; pass data via props |
|
||||||
|
| Hardcoded URL string in a hook or component | `src/hooks/*.ts`, `src/components/*.tsx` | Use the constant from `src/api/endpoints.ts` |
|
||||||
|
| `any` type in TypeScript | anywhere in `src/` | Replace with a concrete interface from `src/types/` |
|
||||||
|
| `print()` statements in production code | `backend/app/**/*.py` | Replace with `structlog` logger |
|
||||||
|
| Synchronous I/O in an async function | `backend/app/**/*.py` | Use the async equivalent |
|
||||||
|
| A repository method that contains an `if` with a business rule | `app/repositories/*.py` | Move the rule to the service layer |
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## 7. Out of Scope
|
||||||
|
|
||||||
|
Do not make the following changes unless explicitly instructed in a separate task:
|
||||||
|
|
||||||
|
- Adding new API endpoints or pages.
|
||||||
|
- Changing database schema or migration files.
|
||||||
|
- Upgrading dependencies.
|
||||||
|
- Altering Docker or CI configuration.
|
||||||
|
- Modifying `Architekture.md` or `Tasks.md`.
|
||||||
526
Docs/Tasks.md
526
Docs/Tasks.md
@@ -6,54 +6,502 @@ This document breaks the entire BanGUI project into development stages, ordered
|
|||||||
|
|
||||||
## Open Issues
|
## Open Issues
|
||||||
|
|
||||||
### ~~1. Dashboard — Version Tag Mismatch~~ ✅ Done
|
> **Architectural Review — 2026-03-16**
|
||||||
|
> The findings below were identified by auditing every backend and frontend module against the rules in [Refactoring.md](Refactoring.md) and [Architekture.md](Architekture.md).
|
||||||
**Implemented:**
|
> Tasks are grouped by layer and ordered so that lower-level fixes (repositories, services) are done before the layers that depend on them.
|
||||||
- `frontend/vite.config.ts`: reads `package.json#version` at build time and injects it as the global `__APP_VERSION__` via Vite `define`.
|
|
||||||
- `frontend/src/vite-env.d.ts`: adds `declare const __APP_VERSION__: string` so TypeScript knows about the global.
|
|
||||||
- `frontend/src/layouts/MainLayout.tsx`: renders `BanGUI v{__APP_VERSION__}` in the sidebar footer when expanded (hidden when collapsed).
|
|
||||||
- `frontend/src/components/ServerStatusBar.tsx`: tooltip changed from `"fail2ban version"` to `"fail2ban daemon version"`.
|
|
||||||
- `Docker/release.sh`: after bumping `VERSION`, also updates `frontend/package.json#version` via `sed` to keep them in sync.
|
|
||||||
- `frontend/package.json`: version bumped from `0.9.0` to `0.9.3` to match `Docker/VERSION`.
|
|
||||||
- Tests added: `src/components/__tests__/ServerStatusBar.test.tsx`, `src/layouts/__tests__/MainLayout.test.tsx`.
|
|
||||||
|
|
||||||
**Problem:** The `ServerStatusBar` component on the Dashboard displays `v{status.version}`, which is the **fail2ban daemon version** (e.g. `v1.1.0`). The BanGUI application version lives in `Docker/VERSION` (e.g. `v0.9.3`) and is unrelated to the fail2ban version. Users see a version number they don't recognise and assume it reflects the BanGUI release.
|
|
||||||
|
|
||||||
**Goal:** Make the distinction clear and expose the BanGUI application version.
|
|
||||||
|
|
||||||
**Suggested approach:**
|
|
||||||
1. Inject the BanGUI app version at build time — add a `define` entry in `frontend/vite.config.ts` that reads the `version` field from `frontend/package.json` (e.g. `__APP_VERSION__`). Keep `frontend/package.json` and `Docker/VERSION` in sync (update the release script `Docker/release.sh` or `Makefile` to write `package.json#version` from `VERSION`).
|
|
||||||
2. Show the BanGUI version in the sidebar footer inside `MainLayout.tsx` (collapsed view: show only when expanded, or via tooltip). This is the natural place for an "about" version tag.
|
|
||||||
3. Update the fail2ban version tooltip in `ServerStatusBar.tsx` from the generic `"fail2ban version"` to something like `"fail2ban daemon version"` so the two are no longer visually indistinguishable.
|
|
||||||
|
|
||||||
**Files:** `frontend/vite.config.ts`, `frontend/package.json`, `Docker/VERSION`, `Docker/release.sh`, `frontend/src/layouts/MainLayout.tsx`, `frontend/src/components/ServerStatusBar.tsx`.
|
|
||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
### ~~2. Dashboard — Improve "Failures" Tooltip~~ ✅ Done
|
### BACKEND
|
||||||
|
|
||||||
**Implemented:** In `frontend/src/components/ServerStatusBar.tsx`, changed the `Failures:` label to `Failed Attempts:` and updated the tooltip from `"Currently failing IPs"` to `"Total failed authentication attempts currently tracked by fail2ban across all active jails"`. Updated `ServerStatusBar.test.tsx` to assert the new label text.
|
|
||||||
|
|
||||||
**Problem:** The `ServerStatusBar` shows a "Failures: 42" counter with the tooltip `"Currently failing IPs"`. In fail2ban terminology *failures* are individual **failed authentication attempts** tracked in the fail2ban DB, not the number of unique IPs that failed. The current wording is ambiguous and misleading — users may think it means broken connections or error states.
|
|
||||||
|
|
||||||
**Goal:** Replace the tooltip with accurate, self-explanatory wording.
|
|
||||||
|
|
||||||
**Suggested fix:** Change the `Tooltip` content for the Failures stat in `ServerStatusBar.tsx` from `"Currently failing IPs"` to something like `"Total failed authentication attempts currently tracked by fail2ban across all active jails"`. Additionally, consider renaming the label from `"Failures:"` to `"Failed Attempts:"` to match the tooltip language.
|
|
||||||
|
|
||||||
**Files:** `frontend/src/components/ServerStatusBar.tsx`.
|
|
||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
### ~~3. Config → Server Tab — Move "Service Health" to Top~~ ✅ Done
|
#### TASK B-1 — Create a `fail2ban_db` repository for direct fail2ban database queries
|
||||||
|
|
||||||
**Implemented:** In `frontend/src/components/config/ServerTab.tsx`, moved `<ServerHealthSection />` from the end of the JSX return to be the first element rendered inside the tab container, before all settings fields.
|
**Violated rule:** Refactoring.md §2.2 — Services must not perform direct `aiosqlite` calls; go through a repository.
|
||||||
|
|
||||||
**Problem:** In the Config page → Server tab, the `Service Health` panel (`ServerHealthSection`) is rendered at the bottom of the tab, after all settings sections (log level, log target, DB purge settings, map thresholds, reload/restart buttons). This means users must scroll past all editable fields to check service connectivity status, even though the health status is the most critical piece of context — it indicates whether the server is reachable at all.
|
**Files affected:**
|
||||||
|
- `backend/app/services/ban_service.py` — lines 247, 398, 568, 646: four separate `aiosqlite.connect(f"file:{db_path}?mode=ro", uri=True)` blocks that execute raw SQL against the fail2ban SQLite database.
|
||||||
|
- `backend/app/services/history_service.py` — lines 118, 208: two more direct `aiosqlite.connect()` blocks against the fail2ban database.
|
||||||
|
|
||||||
**Goal:** Move the `<ServerHealthSection />` block to the **top** of the `ServerTab` render output, before any settings fields.
|
**What to do:**
|
||||||
|
|
||||||
**Suggested fix:** In `frontend/src/components/config/ServerTab.tsx`, move the `{/* Service Health & Log Viewer section */}` block (currently at the end of the JSX return around line 415) to be the first section rendered inside the tab container.
|
1. Create `backend/app/repositories/fail2ban_db_repo.py`.
|
||||||
|
2. Move all SQL that touches the fail2ban database into clearly named async functions in that module. Each function must accept the fail2ban database path (`db_path: str`) as a parameter (connection management stays inside the repository function, since the fail2ban database is an external, read-only resource not managed by BanGUI's own connection pool).
|
||||||
**Files:** `frontend/src/components/config/ServerTab.tsx`.
|
- `get_currently_banned(db_path, jail_filter, since) -> list[BanRecord]`
|
||||||
|
- `get_ban_counts_by_bucket(db_path, ...) -> list[int]`
|
||||||
|
- `check_db_nonempty(db_path) -> bool`
|
||||||
|
- `get_history_for_ip(db_path, ip) -> list[HistoryRecord]`
|
||||||
|
- `get_history_page(db_path, ...) -> tuple[list[HistoryRecord], int]`
|
||||||
|
— Adjust signatures as needed to cover all query sites.
|
||||||
|
3. Replace the inline `aiosqlite.connect` blocks in `ban_service.py` and `history_service.py` with calls to the new repository functions.
|
||||||
|
4. Add the new repository to `backend/tests/test_repositories/` with unit tests that mock the SQLite file.
|
||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
|
#### TASK B-2 — Remove direct SQL query from `routers/geo.py`
|
||||||
|
|
||||||
|
**Violated rule:** Refactoring.md §2.1 — Routers must contain zero business logic; no SQL or repository imports.
|
||||||
|
|
||||||
|
**Files affected:**
|
||||||
|
- `backend/app/routers/geo.py` — lines 157–165: the `re_resolve_geo` handler runs `db.execute("SELECT ip FROM geo_cache WHERE country_code IS NULL")` directly.
|
||||||
|
|
||||||
|
**What to do:**
|
||||||
|
|
||||||
|
1. Add a function `get_unresolved_ips(db: aiosqlite.Connection) -> list[str]` to the appropriate repository (`geo_cache_repo.py` — create it if it does not yet exist, or add it to `settings_repo.py` if the table belongs there).
|
||||||
|
2. In the router handler, replace the inline SQL block with a single call to the new repository function via `geo_service` (preferred) or directly if the service layer already handles this path.
|
||||||
|
3. The final handler body must contain no `db.execute` calls.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
#### TASK B-3 — Remove repository import from `routers/blocklist.py`
|
||||||
|
|
||||||
|
**Violated rule:** Refactoring.md §2.1 — Routers must not import from repositories; all data access must go through services.
|
||||||
|
|
||||||
|
**Files affected:**
|
||||||
|
- `backend/app/routers/blocklist.py` — line 45: `from app.repositories import import_log_repo`; the `get_import_log` handler (around line 220) calls `import_log_repo.list_logs()` directly.
|
||||||
|
|
||||||
|
**What to do:**
|
||||||
|
|
||||||
|
1. Add a `list_import_logs(db, source_id, page, page_size) -> tuple[list[ImportRunResult], int]` method to `blocklist_service.py` (it can be a thin wrapper that calls `import_log_repo.list_logs` internally).
|
||||||
|
2. In the router, replace the direct `import_log_repo.list_logs(...)` call with `await blocklist_service.list_import_logs(...)`.
|
||||||
|
3. Remove the `import_log_repo` import from the router.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
#### TASK B-4 — Move `conffile_parser.py` from `services/` to `utils/`
|
||||||
|
|
||||||
|
**Violated rule:** Refactoring.md §2.2 and Architecture §2.1 — `services/` is for business logic. `conffile_parser.py` is a pure, stateless parsing library with no framework dependencies (no FastAPI, no aiosqlite). It belongs in `utils/`.
|
||||||
|
|
||||||
|
**Files affected:**
|
||||||
|
- `backend/app/services/conffile_parser.py` — all callers that import from `app.services.conffile_parser`.
|
||||||
|
|
||||||
|
**What to do:**
|
||||||
|
|
||||||
|
1. Move the file: `backend/app/services/conffile_parser.py` → `backend/app/utils/conffile_parser.py`.
|
||||||
|
2. Update every import in the codebase from `from app.services.conffile_parser import ...` to `from app.utils.conffile_parser import ...`.
|
||||||
|
3. Run the full test suite to confirm nothing is broken.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
#### TASK B-5 — Create a `geo_cache_repo` and remove direct SQL from `geo_service.py`
|
||||||
|
|
||||||
|
**Violated rule:** Refactoring.md §2.2 — Services must not execute raw SQL; go through a repository.
|
||||||
|
|
||||||
|
**Files affected:**
|
||||||
|
- `backend/app/services/geo_service.py` — multiple direct `db.execute` / `db.executemany` calls in `cache_stats()` (line 187), `load_cache_from_db()` (line 271), `_persist_entry()` (lines 304–316), `_persist_neg_entry()` (lines 329–338), `flush_dirty()` (lines 795+), and geo-data batch persist blocks (lines 588–612).
|
||||||
|
|
||||||
|
**What to do:**
|
||||||
|
|
||||||
|
1. Create `backend/app/repositories/geo_cache_repo.py` with typed async functions for every SQL operation currently inline in `geo_service.py`:
|
||||||
|
- `load_all(db) -> list[GeoCacheRow]`
|
||||||
|
- `upsert_entry(db, geo_row) -> None`
|
||||||
|
- `upsert_neg_entry(db, ip) -> None`
|
||||||
|
- `flush_dirty(db, entries) -> int`
|
||||||
|
- `get_stats(db) -> dict[str, int]`
|
||||||
|
- `get_unresolved_ips(db) -> list[str]` (also needed by B-2)
|
||||||
|
2. Replace every `db.execute` / `db.executemany` call in `geo_service.py` with calls to the new repository.
|
||||||
|
3. Add tests in `backend/tests/test_repositories/test_geo_cache_repo.py`.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
#### TASK B-6 — Remove direct SQL from `tasks/geo_re_resolve.py`
|
||||||
|
|
||||||
|
**Violated rule:** Refactoring.md §2.5 — Tasks must not use repositories directly; they must call a service method.
|
||||||
|
|
||||||
|
**Files affected:**
|
||||||
|
- `backend/app/tasks/geo_re_resolve.py` — line 53: `async with db.execute("SELECT ip FROM geo_cache WHERE country_code IS NULL")`.
|
||||||
|
|
||||||
|
**What to do:**
|
||||||
|
|
||||||
|
After completing TASK B-5, a `geo_service` method (or via `geo_cache_repo` through `geo_service`) that returns unresolved IPs will exist.
|
||||||
|
|
||||||
|
1. Replace the inline SQL block in `_run_re_resolve` with a call to that service method (e.g., `unresolved = await geo_service.get_unresolved_ips(db)`).
|
||||||
|
2. The task function must contain no `db.execute` calls of its own.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
#### TASK B-7 — Replace `Any` type annotations in `ban_service.py`
|
||||||
|
|
||||||
|
**Violated rule:** Backend-Development.md §1 — Never use `Any`; all functions must have explicit type annotations.
|
||||||
|
|
||||||
|
**Files affected:**
|
||||||
|
- `backend/app/services/ban_service.py` — lines 192, 271, 346, 434, 455: uses of `Any` for `geo_enricher` parameter and `geo_map` dict value type.
|
||||||
|
|
||||||
|
**What to do:**
|
||||||
|
|
||||||
|
1. Define a precise callable type alias for the geo enricher, e.g.:
|
||||||
|
```python
|
||||||
|
from collections.abc import Awaitable, Callable
|
||||||
|
GeoEnricher: TypeAlias = Callable[[str], Awaitable[GeoInfo | None]]
|
||||||
|
```
|
||||||
|
2. Replace `geo_enricher: Any | None` with `geo_enricher: GeoEnricher | None` (both occurrences).
|
||||||
|
3. Replace `geo_map: dict[str, Any]` with `geo_map: dict[str, GeoInfo]` (both occurrences).
|
||||||
|
4. Replace the inner `_safe_lookup` return type `tuple[str, Any]` with `tuple[str, GeoInfo | None]`.
|
||||||
|
5. Run `mypy --strict` or `pyright` to confirm zero remaining type errors in this file.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
#### TASK B-8 — Remove `print()` from `geo_service.py` docstring example
|
||||||
|
|
||||||
|
**Violated rule:** Refactoring.md §4 / Backend-Development.md §2 — Never use `print()` in production code; use `structlog`.
|
||||||
|
|
||||||
|
**Files affected:**
|
||||||
|
- `backend/app/services/geo_service.py` — line 33: `print(info.country_code) # "DE"` appears inside a module-level docstring usage example.
|
||||||
|
|
||||||
|
**What to do:**
|
||||||
|
|
||||||
|
Remove or rewrite the docstring snippet so it does not contain a bare `print()` call. If the example is kept, annotate it clearly as a documentation-only code block that should not be copied into production code, or replace with a comment like `# info.country_code == "DE"`.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
#### TASK B-9 — Remove direct SQL from `main.py` lifespan into `geo_service`
|
||||||
|
|
||||||
|
**Violated rule:** Refactoring.md §2 — Application startup code must not execute raw SQL; data-access logic belongs in a repository (or, when count semantics belong to a domain concern, a service method).
|
||||||
|
|
||||||
|
**Files affected:**
|
||||||
|
- `backend/app/main.py` — lines 164–168: the lifespan handler runs `db.execute("SELECT COUNT(*) FROM geo_cache WHERE country_code IS NULL")` directly to log a startup warning about unresolved geo entries.
|
||||||
|
|
||||||
|
**What to do:**
|
||||||
|
|
||||||
|
1. After TASK B-5 is complete, `geo_cache_repo` will expose a `get_stats(db) -> dict[str, int]` function (or a dedicated `count_unresolved(db) -> int`). Use that.
|
||||||
|
2. If B-5 is not yet merged, add an interim function `count_unresolved(db: aiosqlite.Connection) -> int` to `geo_cache_repo.py` now and call it from `geo_service` as `geo_service.count_unresolved_cached(db) -> Awaitable[int]`.
|
||||||
|
3. Replace the inline `async with db.execute(...)` block in `main.py` with a single `await geo_service.count_unresolved_cached(db)` call.
|
||||||
|
4. The `main.py` lifespan function must contain no `db.execute` calls of its own.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
#### TASK B-10 — Replace `Any` type usage in `history_service.py`
|
||||||
|
|
||||||
|
**Violated rule:** Backend-Development.md §1 — Never use `Any`; all functions must have explicit type annotations.
|
||||||
|
|
||||||
|
**Files affected:**
|
||||||
|
- `backend/app/services/history_service.py` — uses `Any` for `geo_enricher` and query parameter lists.
|
||||||
|
|
||||||
|
**What to do:**
|
||||||
|
|
||||||
|
1. Define a shared `GeoEnricher` type alias (e.g., in `app/services/geo_service.py` or a new `app/models/geo.py`) similar to TASK B-7.
|
||||||
|
2. Update `history_service.py` to use `GeoEnricher | None` for the `geo_enricher` parameter.
|
||||||
|
3. Replace `list[Any]` for SQL parameters with a more precise type (e.g., `list[object]` or a custom `SqlParam` alias).
|
||||||
|
4. Run `mypy --strict` or `pyright` to confirm there are no remaining `Any` usages in `history_service.py`.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
#### TASK B-11 — Reduce `Any` usage in `server_service.py`
|
||||||
|
|
||||||
|
**Violated rule:** Backend-Development.md §1 — Never use `Any`; all functions must have explicit type annotations.
|
||||||
|
|
||||||
|
**Files affected:**
|
||||||
|
- `backend/app/services/server_service.py` — uses `Any` for raw socket response values and command parameters.
|
||||||
|
|
||||||
|
**What to do:**
|
||||||
|
|
||||||
|
1. Define typed aliases for the expected response and command shapes used by `Fail2BanClient` (e.g., `Fail2BanResponse = tuple[int, object]`, `Fail2BanCommand = list[str | int | None]`).
|
||||||
|
2. Replace `Any` with those aliases in `_ok`, `_safe_get`, and other helper functions.
|
||||||
|
3. Ensure the public API functions (`get_settings`, etc.) have explicit return types and avoid propagating `Any` to callers.
|
||||||
|
4. Run `mypy --strict` or `pyright` to confirm no remaining `Any` usages in `server_service.py`.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
### FRONTEND
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
#### TASK F-1 — Wrap `SetupPage` API calls in a dedicated hook
|
||||||
|
|
||||||
|
**Violated rule:** Refactoring.md §3.1 — Pages must not call API functions from `src/api/` directly; all data fetching goes through hooks.
|
||||||
|
|
||||||
|
**Files affected:**
|
||||||
|
- `frontend/src/pages/SetupPage.tsx` — lines 24, 114, 179: imports `getSetupStatus` and `submitSetup` from `../api/setup` and calls them directly inside the component.
|
||||||
|
|
||||||
|
**What to do:**
|
||||||
|
|
||||||
|
1. Create `frontend/src/hooks/useSetup.ts` that encapsulates:
|
||||||
|
- Fetching setup status on mount (`{ isSetupComplete, loading, error }`).
|
||||||
|
- A `submitSetup(payload)` mutation that returns `{ submitting, submitError, submit }`.
|
||||||
|
2. Update `SetupPage.tsx` to use `useSetup` exclusively; remove all direct `api/setup` imports from the page.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
#### TASK F-2 — Wrap `JailDetailPage` jail-control API calls in a hook
|
||||||
|
|
||||||
|
**Violated rule:** Refactoring.md §3.1 — Pages must not call API functions directly.
|
||||||
|
|
||||||
|
**Files affected:**
|
||||||
|
- `frontend/src/pages/JailDetailPage.tsx` — lines 37–44, 262, 272, 285, 295: imports and directly calls `startJail`, `stopJail`, `setJailIdle`, `reloadJail` from `../api/jails`.
|
||||||
|
|
||||||
|
**What to do:**
|
||||||
|
|
||||||
|
1. Check whether `useJailDetail` or `useJails` already expose these control actions. If so, use those hook-provided callbacks instead of calling the API directly.
|
||||||
|
2. If they do not, add `start()`, `stop()`, `reload()`, `setIdle(idle: boolean)` actions to the appropriate hook (e.g., `useJailDetail`).
|
||||||
|
3. Remove all direct `startJail` / `stopJail` / `setJailIdle` / `reloadJail` API imports from the page.
|
||||||
|
4. The `ApiError` import may remain if it is used only for `instanceof` type-narrowing in error handlers, but prefer exposing an `error: ApiError | null` from the hook instead.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
#### TASK F-3 — Wrap `MapPage` config API call in a hook
|
||||||
|
|
||||||
|
**Violated rule:** Refactoring.md §3.1 — Pages must not call API functions directly.
|
||||||
|
|
||||||
|
**Files affected:**
|
||||||
|
- `frontend/src/pages/MapPage.tsx` — line 34: imports `fetchMapColorThresholds` from `../api/config` and calls it in a `useEffect`.
|
||||||
|
|
||||||
|
**What to do:**
|
||||||
|
|
||||||
|
1. Create `frontend/src/hooks/useMapColorThresholds.ts` (or add the fetch to the existing `useMapData` hook if it is cohesive).
|
||||||
|
2. Replace the inline `useEffect` + `fetchMapColorThresholds` pattern in `MapPage` with the new hook call.
|
||||||
|
3. Remove the direct `api/config` import from the page.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
#### TASK F-4 — Wrap `BlocklistsPage` preview API call in a hook
|
||||||
|
|
||||||
|
**Violated rule:** Refactoring.md §3.1 — Pages must not call API functions directly.
|
||||||
|
|
||||||
|
**Files affected:**
|
||||||
|
- `frontend/src/pages/BlocklistsPage.tsx` — line 54: imports `previewBlocklist` from `../api/blocklist`.
|
||||||
|
|
||||||
|
**What to do:**
|
||||||
|
|
||||||
|
1. Add a `previewBlocklist(url)` action to the existing `useBlocklists` hook (or create a `useBlocklistPreview` hook), returning `{ preview, previewing, previewError, runPreview }`.
|
||||||
|
2. Update `BlocklistsPage` to call the hook action instead of the raw API function.
|
||||||
|
3. Remove the direct `api/blocklist` import for `previewBlocklist` from the page.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
#### TASK F-5 — Move all API calls out of `BannedIpsSection` into a hook
|
||||||
|
|
||||||
|
**Violated rule:** Refactoring.md §3.2 — Components must not call API functions; all data must come via props or hooks invoked in the parent.
|
||||||
|
|
||||||
|
**Files affected:**
|
||||||
|
- `frontend/src/components/jail/BannedIpsSection.tsx` — imports and directly calls `fetchJailBannedIps` and `unbanIp` from `../../api/jails`.
|
||||||
|
|
||||||
|
**What to do:**
|
||||||
|
|
||||||
|
1. Create `frontend/src/hooks/useJailBannedIps.ts` with state `{ bannedIps, loading, error, page, totalPages, refetch }` and an `unban(ip)` action.
|
||||||
|
2. Invoke this hook in the parent page (`JailDetailPage`) and pass `bannedIps`, `loading`, `error`, `onUnban`, and pagination props down to `BannedIpsSection`.
|
||||||
|
3. Remove all `api/` imports from `BannedIpsSection.tsx`; the component receives everything through props.
|
||||||
|
4. Update `BannedIpsSection` tests to use props instead of mocking API calls directly.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
#### TASK F-6 — Move all API calls out of config tab and dialog components into hooks
|
||||||
|
|
||||||
|
**Violated rule:** Refactoring.md §3.2 — Components must not call API functions.
|
||||||
|
|
||||||
|
**Files affected (all in `frontend/src/components/config/`):**
|
||||||
|
- `FiltersTab.tsx` — calls `fetchFilters`, `fetchFilterFile`, `updateFilterFile` from `../../api/config` directly.
|
||||||
|
- `JailsTab.tsx` — calls multiple config API functions directly.
|
||||||
|
- `ActionsTab.tsx` — calls config API functions directly.
|
||||||
|
- `ExportTab.tsx` — calls multiple file-management API functions directly.
|
||||||
|
- `JailFilesTab.tsx` — calls API functions for jail file management.
|
||||||
|
- `ServerHealthSection.tsx` — calls `fetchFail2BanLog`, `fetchServiceStatus` from `../../api/config`.
|
||||||
|
- `CreateFilterDialog.tsx` — calls `createFilter` from `../../api/config`.
|
||||||
|
- `CreateJailDialog.tsx` — calls `createJailConfigFile` from `../../api/config`.
|
||||||
|
- `CreateActionDialog.tsx` — calls `createAction` from `../../api/config`.
|
||||||
|
- `ActivateJailDialog.tsx` — calls `activateJail`, `validateJailConfig` from `../../api/config`.
|
||||||
|
- `AssignFilterDialog.tsx` — calls `assignFilterToJail` from `../../api/config` and `fetchJails` from `../../api/jails`.
|
||||||
|
- `AssignActionDialog.tsx` — calls `assignActionToJail` from `../../api/config` and `fetchJails` from `../../api/jails`.
|
||||||
|
|
||||||
|
**What to do:**
|
||||||
|
|
||||||
|
For each component listed:
|
||||||
|
|
||||||
|
1. Identify or create the appropriate hook in `frontend/src/hooks/`. Group related concerns — for example, a single `useFiltersConfig` hook can cover fetch, update, and create actions for filters.
|
||||||
|
2. Move all `useEffect` + API call patterns from the component into the hook. The hook must return `{ data, loading, error, refetch, ...actions }`.
|
||||||
|
3. The component must receive data and action callbacks exclusively through props or a hook called in its closest page ancestor.
|
||||||
|
4. Remove all `../../api/` imports from the component files listed above.
|
||||||
|
5. Update or add unit tests for any new hooks created.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
#### TASK F-7 — Move `SetupGuard` API call into a hook
|
||||||
|
|
||||||
|
**Violated rule:** Refactoring.md §3.2 — Components must not contain a `useEffect` that calls an API function.
|
||||||
|
|
||||||
|
**Files affected:**
|
||||||
|
- `frontend/src/components/SetupGuard.tsx` — line 12: imports `getSetupStatus` from `../api/setup`; lines 28–36: calls it directly inside a `useEffect`.
|
||||||
|
|
||||||
|
**What to do:**
|
||||||
|
|
||||||
|
1. The `useSetup` hook created for TASK F-1 exposes setup-status fetching. Reuse it here, or extract the status-only slice into a `useSetupStatus()` hook that `SetupGuard` and `SetupPage` can both consume.
|
||||||
|
2. Replace the inline `useEffect` + `getSetupStatus` pattern in `SetupGuard` with a call to the hook.
|
||||||
|
3. Remove the direct `../api/setup` import from `SetupGuard.tsx`.
|
||||||
|
4. Update `SetupGuard` tests — they currently mock `../../api/setup` directly; update them to mock the hook instead.
|
||||||
|
|
||||||
|
**Dependency:** Can share hook infrastructure with TASK F-1.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
#### TASK F-8 — Move `ServerTab` direct API calls into hooks
|
||||||
|
|
||||||
|
**Violated rule:** Refactoring.md §3.2 — Components must not call API functions.
|
||||||
|
|
||||||
|
**Files affected:**
|
||||||
|
- `frontend/src/components/config/ServerTab.tsx`:
|
||||||
|
- lines 36-41: imports `fetchMapColorThresholds`, `updateMapColorThresholds`, `reloadConfig`, `restartFail2Ban` from `../../api/config` and calls each directly inside `useCallback`/`useEffect` handlers.
|
||||||
|
|
||||||
|
*Note: This component was inadvertently omitted from the TASK F-6 file list despite belonging to the same `components/config/` family.*
|
||||||
|
|
||||||
|
**What to do:**
|
||||||
|
|
||||||
|
1. The `fetchMapColorThresholds` / `updateMapColorThresholds` concern overlaps with TASK F-3 (`useMapColorThresholds` hook). Extend that hook or create a dedicated `useMapColorThresholdsConfig` hook that also exposes an `update(payload)` action.
|
||||||
|
2. Add `reload()` and `restart()` actions to a suitable config hook (e.g., a `useServerActions` hook or extend `useServerSettings` in `src/hooks/useConfig.ts`).
|
||||||
|
3. Replace all direct `reloadConfig()`, `restartFail2Ban()`, `fetchMapColorThresholds()`, and `updateMapColorThresholds()` calls in `ServerTab` with the hook-provided actions.
|
||||||
|
4. Remove all `../../api/config` imports for these four functions from `ServerTab.tsx`.
|
||||||
|
|
||||||
|
**Dependency:** Coordinate with TASK F-3 to avoid creating duplicate `useMapColorThresholds` hook logic.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
#### TASK F-9 — Move `TimezoneProvider` API call into a hook
|
||||||
|
|
||||||
|
**Violated rule:** Refactoring.md §3.2 — A component (including a provider component) must not contain a `useEffect` that calls an API function directly; API calls belong in `src/hooks/`.
|
||||||
|
|
||||||
|
**Files affected:**
|
||||||
|
- `frontend/src/providers/TimezoneProvider.tsx` — line 20: imports `fetchTimezone` from `../api/setup`; lines 57–62: calls it directly inside a `useCallback` that is invoked from `useEffect`.
|
||||||
|
|
||||||
|
**What to do:**
|
||||||
|
|
||||||
|
1. Create `frontend/src/hooks/useTimezoneData.ts` (or add to an existing setup-related hook) that fetches the timezone and returns `{ timezone, loading, error }`.
|
||||||
|
2. Call this hook inside `TimezoneProvider` and drive the context value from the hook's `timezone` output — removing the inline `fetchTimezone()` call.
|
||||||
|
3. Remove the direct `../api/setup` import from `TimezoneProvider.tsx`.
|
||||||
|
4. The hook may be reused in any future component that needs the configured timezone without going through the context.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
#### TASK B-12 — Remove `Any` type annotations in `config_service.py`
|
||||||
|
|
||||||
|
**Violated rule:** Backend-Development.md §1 — Never use `Any`; all functions must have explicit type annotations.
|
||||||
|
|
||||||
|
**Files affected:**
|
||||||
|
- `backend/app/services/config_service.py` — several helper functions (`_ok`, `_to_dict`, `_ensure_list`, `_safe_get`, `_set`, `_set_global`) use `Any` for inputs/outputs.
|
||||||
|
|
||||||
|
**What to do:**
|
||||||
|
|
||||||
|
1. Define typed aliases for the fail2ban client response and command shapes (e.g., `Fail2BanResponse = tuple[int, object | None]`, `Fail2BanCommand = list[str | int | None]`).
|
||||||
|
2. Replace `Any` in helper signatures with the new aliases (and use `object`/`str`/`int` where appropriate).
|
||||||
|
3. Run `mypy --strict` or `pyright` to confirm no remaining `Any` usages in this file.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
#### TASK B-13 — Remove `Any` type annotations in `jail_service.py`
|
||||||
|
|
||||||
|
**Violated rule:** Backend-Development.md §1 — Never use `Any`; all functions must have explicit type annotations.
|
||||||
|
|
||||||
|
**Files affected:**
|
||||||
|
- `backend/app/services/jail_service.py` — helper utilities (`_ok`, `_to_dict`, `_ensure_list`, `_safe_get`, etc.) use `Any` for raw fail2ban responses and command parameters.
|
||||||
|
|
||||||
|
**What to do:**
|
||||||
|
|
||||||
|
1. Define typed aliases for fail2ban response and command shapes (e.g., `Fail2BanResponse`, `Fail2BanCommand`).
|
||||||
|
2. Update helper function signatures to use the new types instead of `Any`.
|
||||||
|
3. Run `mypy --strict` or `pyright` to confirm no remaining `Any` usages in this file.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
#### TASK B-14 — Remove `Any` type annotations in `health_service.py`
|
||||||
|
|
||||||
|
**Violated rule:** Backend-Development.md §1 — Never use `Any`; all functions must have explicit type annotations.
|
||||||
|
|
||||||
|
**Files affected:**
|
||||||
|
- `backend/app/services/health_service.py` — helper functions `_ok` and `_to_dict` and their callers currently use `Any`.
|
||||||
|
|
||||||
|
**What to do:**
|
||||||
|
|
||||||
|
1. Define typed aliases for fail2ban responses (e.g. `Fail2BanResponse = tuple[int, object | None]`).
|
||||||
|
2. Update `_ok`, `_to_dict`, and any helper usage sites to use concrete types instead of `Any`.
|
||||||
|
3. Run `mypy --strict` or `pyright` to confirm no remaining `Any` usages in this file.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
#### TASK B-15 — Remove `Any` type annotations in `blocklist_service.py`
|
||||||
|
|
||||||
|
**Violated rule:** Backend-Development.md §1 — Never use `Any`; all functions must have explicit type annotations.
|
||||||
|
|
||||||
|
**Files affected:**
|
||||||
|
- `backend/app/services/blocklist_service.py` — helper `_row_to_source()` and other internal functions currently use `Any`.
|
||||||
|
|
||||||
|
**What to do:**
|
||||||
|
|
||||||
|
1. Replace `Any` with precise types for repository row dictionaries (e.g. `dict[str, object]` or a dedicated `BlocklistSourceRow` TypedDict).
|
||||||
|
2. Update helper signatures and any call sites accordingly.
|
||||||
|
3. Run `mypy --strict` or `pyright` to confirm no remaining `Any` usages in this file.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
#### TASK B-16 — Remove `Any` type annotations in `import_log_repo.py`
|
||||||
|
|
||||||
|
**Violated rule:** Backend-Development.md §1 — Never use `Any`; all functions must have explicit type annotations.
|
||||||
|
|
||||||
|
**Files affected:**
|
||||||
|
- `backend/app/repositories/import_log_repo.py` — returns `dict[str, Any]` and accepts `list[Any]` parameters.
|
||||||
|
|
||||||
|
**What to do:**
|
||||||
|
|
||||||
|
1. Define a typed row model (e.g. `ImportLogRow = TypedDict[...]`) or a Pydantic model for import log entries.
|
||||||
|
2. Update public function signatures to return typed structures instead of `dict[str, Any]` and to accept properly typed query parameters.
|
||||||
|
3. Update callers (e.g. `routers/blocklist.py` and `services/blocklist_service.py`) to work with the new types.
|
||||||
|
4. Run `mypy --strict` or `pyright` to confirm no remaining `Any` usages in this file.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
#### TASK B-17 — Remove `Any` type annotations in `config_file_service.py`
|
||||||
|
|
||||||
|
**Violated rule:** Backend-Development.md §1 — Never use `Any`; all functions must have explicit type annotations.
|
||||||
|
|
||||||
|
**Files affected:**
|
||||||
|
- `backend/app/services/config_file_service.py` — internal helpers (`_to_dict_inner`, `_ok`, etc.) use `Any` for fail2ban response objects.
|
||||||
|
|
||||||
|
**What to do:**
|
||||||
|
|
||||||
|
1. Introduce typed aliases for fail2ban command/response shapes (e.g. `Fail2BanResponse`, `Fail2BanCommand`).
|
||||||
|
2. Replace `Any` in helper function signatures and return types with these aliases.
|
||||||
|
3. Run `mypy --strict` or `pyright` to confirm no remaining `Any` usages in this file.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
#### TASK B-18 — Remove `Any` type annotations in `fail2ban_client.py`
|
||||||
|
|
||||||
|
**Violated rule:** Backend-Development.md §1 — Never use `Any`; all functions must have explicit type annotations.
|
||||||
|
|
||||||
|
**Files affected:**
|
||||||
|
- `backend/app/utils/fail2ban_client.py` — the public client interface uses `Any` for command and response types.
|
||||||
|
|
||||||
|
**What to do:**
|
||||||
|
|
||||||
|
1. Define clear type aliases such as `Fail2BanCommand = list[str | int | bool | None]` and `Fail2BanResponse = object` (or a more specific union of expected response shapes).
|
||||||
|
2. Update `_send_command_sync`, `_coerce_command_token`, and `Fail2BanClient.send` signatures to use these aliases.
|
||||||
|
3. Run `mypy --strict` or `pyright` to confirm no remaining `Any` usages in this file.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
#### TASK B-19 — Remove `Any` annotations from background tasks
|
||||||
|
|
||||||
|
**Violated rule:** Backend-Development.md §1 — Never use `Any`; all functions must have explicit type annotations.
|
||||||
|
|
||||||
|
**Files affected:**
|
||||||
|
- `backend/app/tasks/health_check.py` — uses `app: Any` and `last_activation: dict[str, Any] | None`.
|
||||||
|
- `backend/app/tasks/geo_re_resolve.py` — uses `app: Any`.
|
||||||
|
|
||||||
|
**What to do:**
|
||||||
|
|
||||||
|
1. Define a typed model for the shared application state (e.g., a `TypedDict` or `Protocol`) that includes the expected properties on `app.state` (e.g., `settings`, `db`, `server_status`, `last_activation`, `pending_recovery`).
|
||||||
|
2. Change task callbacks to accept `FastAPI` (or the typed app) instead of `Any`.
|
||||||
|
3. Replace `dict[str, Any]` with a lean typed record (e.g., a `TypedDict` or a small `@dataclass`) for `last_activation`.
|
||||||
|
4. Run `mypy --strict` or `pyright` to confirm no remaining `Any` usages in these files.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
#### TASK B-20 — Remove `type: ignore` in `dependencies.get_settings`
|
||||||
|
|
||||||
|
**Violated rule:** Backend-Development.md §1 — Avoid `Any` and ignored type errors.
|
||||||
|
|
||||||
|
**Files affected:**
|
||||||
|
- `backend/app/dependencies.py` — `get_settings` currently uses `# type: ignore[no-any-return]`.
|
||||||
|
|
||||||
|
**What to do:**
|
||||||
|
|
||||||
|
1. Introduce a typed model (e.g., `TypedDict` or `Protocol`) for `app.state` to declare `settings: Settings` and other shared state properties.
|
||||||
|
2. Update `get_settings` (and any other helpers that read from `app.state`) so the return type is inferred as `Settings` without needing a `type: ignore` comment.
|
||||||
|
3. Run `mypy --strict` or `pyright` to confirm the type ignore is no longer needed.
|
||||||
|
|||||||
Reference in New Issue
Block a user