mirror of
https://github.com/ipnet-mesh/meshcore-hub.git
synced 2026-08-07 01:13:11 +02:00
Merge pull request #211 from ipnet-mesh/fix/node-retention-foreign-key
fix: enable async SQLite FK enforcement and clean up orphaned node relations
This commit is contained in:
JingleManSweep
@@ -0,0 +1,195 @@
|
||||
# Node Association Foreign Key Fix
|
||||
|
||||
**Date:** 2026-05-15
|
||||
**Status:** Reviewed
|
||||
**Severity:** High (500 errors on `/api/v1/user/profiles`, broken Nodes/Members pages)
|
||||
|
||||
## Problem
|
||||
|
||||
The node retention policy (`NODE_CLEANUP`) deletes `Node` rows, but orphaned rows remain in dependent tables (`user_profile_nodes`, `event_observers`, `node_tags`). When the API loads user profiles with their adopted nodes via `selectinload`, the `assoc.node` relationship resolves to `None` because the referenced node no longer exists. This causes an `AttributeError` crash:
|
||||
|
||||
```
|
||||
AttributeError: 'NoneType' object has no attribute 'public_key'
|
||||
at api/routes/user_profiles.py:96 — public_key=assoc.node.public_key
|
||||
```
|
||||
|
||||
The same issue exists for any code that accesses `.node` on `EventObserver` or `NodeTag` after their referenced node has been deleted.
|
||||
|
||||
## Root Cause
|
||||
|
||||
The `cleanup_inactive_nodes()` function in `collector/cleanup.py` deletes `Node` rows directly via `DELETE FROM nodes WHERE last_seen < cutoff`. Three dependent tables define `ForeignKey("nodes.id", ondelete="CASCADE")`:
|
||||
|
||||
- `user_profile_nodes` (`user_profile_node.py:39`)
|
||||
- `event_observers` (`event_observer.py:57`)
|
||||
- `node_tags` (`node_tag.py:32`)
|
||||
|
||||
However, **the cascade never executes** because:
|
||||
|
||||
1. The **async SQLAlchemy engine** (`create_async_engine` in `database.py:115`) does **not** have the `PRAGMA foreign_keys=ON` listener that the sync engine has (`database.py:41-47`).
|
||||
2. SQLite does not enforce foreign keys or cascades by default. The `PRAGMA` must be enabled per-connection.
|
||||
3. The collector's cleanup runs via the async engine, so SQLite silently ignores the `ondelete="CASCADE"` constraints, leaving orphaned rows pointing to deleted `nodes.id` values.
|
||||
|
||||
## Affected Code
|
||||
|
||||
| File | Lines | Issue |
|
||||
|------|-------|-------|
|
||||
| `common/database.py` | 115 | Async engine created without SQLite FK pragma listener |
|
||||
| `api/routes/user_profiles.py` | 34-42, 93-101 | `assoc.node.public_key` crashes when `assoc.node is None` |
|
||||
| `collector/cleanup.py` | 166-225 | Node cleanup doesn't cascade to dependent tables |
|
||||
| `collector/cli.py` | 517-601 | CLI `cleanup` command only runs event data cleanup, not node cleanup |
|
||||
| `collector/cli.py` | 537-545 | Docstring says "Node records are never deleted" — misleading once orphan cleanup is added |
|
||||
| `collector/cli.py` | 717-724 | `truncate` cascade warning omits `user_profile_nodes` and `event_observers` |
|
||||
| `tests/test_collector/conftest.py` | 35 | Async test fixture has no FK pragma — can't verify cascade |
|
||||
| `tests/test_api/conftest.py` | 49 | Sync test engine has no FK pragma — can't verify cascade |
|
||||
| `tests/test_collector/test_cleanup.py` | — | Missing test for `cleanup_inactive_nodes` entirely |
|
||||
|
||||
## Plan
|
||||
|
||||
### Step 1: Fix async SQLite FK enforcement
|
||||
|
||||
**File:** `src/meshcore_hub/common/database.py`
|
||||
|
||||
Add the `PRAGMA foreign_keys=ON` event listener to `self.async_engine` (the `create_async_engine` call), mirroring the existing listener on the sync engine. Use the existing module-scope `event` import — do NOT re-import it.
|
||||
|
||||
```python
|
||||
# After creating self.async_engine (at database.py:115):
|
||||
if database_url.startswith("sqlite"):
|
||||
@event.listens_for(self.async_engine.sync_engine, "connect")
|
||||
def set_sqlite_pragma_async(dbapi_connection, connection_record):
|
||||
cursor = dbapi_connection.cursor()
|
||||
cursor.execute("PRAGMA foreign_keys=ON")
|
||||
cursor.close()
|
||||
```
|
||||
|
||||
This fixes the root cause for all future cleanup operations across all three dependent tables.
|
||||
|
||||
### Step 2: Add null-safety to API routes
|
||||
|
||||
**File:** `src/meshcore_hub/api/routes/user_profiles.py`
|
||||
|
||||
Make `_build_adopted_nodes()` resilient to orphaned associations:
|
||||
|
||||
1. In `_build_adopted_nodes()` (line 34): Skip associations where `assoc.node is None`, log a warning with `profile.id` and `assoc.node_id` for diagnostics.
|
||||
2. Remove the duplicate inline adoption loop in `list_profiles()` (lines 93-101); refactor it to call `_build_adopted_nodes()` instead.
|
||||
|
||||
This prevents the 500 error even if orphaned rows exist in the database (e.g., from past cleanup runs before this fix). All three callers (`get_my_profile`, `get_profile`, `list_profiles`) are protected.
|
||||
|
||||
### Step 3: Add orphan cleanup function
|
||||
|
||||
**File:** `src/meshcore_hub/collector/cleanup.py`
|
||||
|
||||
Add a new function `cleanup_orphaned_node_relations(db, dry_run=False) -> dict` that:
|
||||
|
||||
1. Finds orphaned rows in all three dependent tables (`user_profile_nodes`, `event_observers`, `node_tags`) where `node_id` does not exist in the `nodes` table (using `LEFT JOIN ... WHERE nodes.id IS NULL` pattern).
|
||||
2. Deletes those orphaned rows (or counts them in dry-run mode).
|
||||
3. Returns a dict with per-table counts, e.g. `{"user_profile_nodes": 3, "event_observers": 0, "node_tags": 5}`.
|
||||
|
||||
This repairs existing production databases that already have broken data from past cleanup runs.
|
||||
|
||||
### Step 4: Integrate orphan cleanup into retention cycle
|
||||
|
||||
**File:** `src/meshcore_hub/collector/subscriber.py`
|
||||
|
||||
In the `run_cleanup()` async function within `_start_cleanup_scheduler()` (around line 303):
|
||||
|
||||
1. Remove the inline `async def run_cleanup()` and extract it as a proper method `_run_scheduled_cleanup(db_session)` on the subscriber class.
|
||||
2. After `cleanup_inactive_nodes()`, call `cleanup_orphaned_node_relations()`.
|
||||
3. Log the orphan cleanup results.
|
||||
|
||||
This ensures orphans are cleaned up automatically in future runs as a belt-and-suspenders defense even after the PRAGMA fix.
|
||||
|
||||
### Step 5: Wire cleanup into CLI
|
||||
|
||||
**File:** `src/meshcore_hub/collector/cli.py`
|
||||
|
||||
1. Add a `--node-cleanup` option (default: `false`) to the `cleanup` CLI command. When set, also runs `cleanup_inactive_nodes()` and `cleanup_orphaned_node_relations()`.
|
||||
2. Update the docstring to remove "Node records are never deleted" and document the new option.
|
||||
3. Add a `--node-cleanup-days` option (default: `30`) for the node inactivity threshold.
|
||||
4. Display orphan cleanup results in the output.
|
||||
|
||||
### Step 6: Fix test fixtures with FK PRAGMA
|
||||
|
||||
**File:** `tests/test_collector/conftest.py`
|
||||
|
||||
Add `PRAGMA foreign_keys=ON` event listener to the async engine created at line 35.
|
||||
|
||||
**File:** `tests/test_api/conftest.py`
|
||||
|
||||
Add `PRAGMA foreign_keys=ON` event listener to the sync engine created at line 49.
|
||||
|
||||
### Step 7: Fix `truncate` CLI cascade warning
|
||||
|
||||
**File:** `src/meshcore_hub/collector/cli.py` (lines 717-724)
|
||||
|
||||
Add `user_profile_nodes` and `event_observers` to the cascade warning list.
|
||||
|
||||
### Step 8: Tests
|
||||
|
||||
- **`tests/test_collector/test_cleanup.py`**:
|
||||
- Add test for `cleanup_inactive_nodes()` — create a node with associations in all three dependent tables, delete the node, verify cascade removes all dependent rows.
|
||||
- Add test for `cleanup_orphaned_node_relations()` — create orphaned rows in all three tables (rows referencing non-existent node_ids), verify the function deletes them.
|
||||
|
||||
- **`tests/test_api/test_user_profiles.py`**:
|
||||
- Add test verifying `list_profiles` returns 200 (not 500) when orphaned `UserProfileNode` rows exist with a deleted node, and that the orphaned association is excluded from `adopted_nodes` output.
|
||||
|
||||
### Step 9: Documentation
|
||||
|
||||
- **`docs/upgrading.md`**: Add a note about this fix, the automatic orphan cleanup, and a manual repair command for existing deployments.
|
||||
- **`AGENTS.md`**: No changes needed (no new env vars or config).
|
||||
- **`SCHEMAS.md`**: No changes needed (FK relationships unchanged).
|
||||
|
||||
## Execution Order
|
||||
|
||||
1. Fix async FK pragma (Step 1) — prevents future orphans
|
||||
2. Fix test fixtures with FK pragma (Step 6) — unblocks testability
|
||||
3. Add null-safety (Step 2) — stops the 500 errors immediately
|
||||
4. Add orphan cleanup function (Step 3) — provides the repair tool
|
||||
5. Integrate into retention cycle (Step 4) — makes it automatic
|
||||
6. Wire into CLI (Step 5) — exposes manual control
|
||||
7. Fix truncate warning (Step 7)
|
||||
8. Add tests (Step 8)
|
||||
9. Update docs (Step 9)
|
||||
|
||||
## Risk Assessment
|
||||
|
||||
- **Step 1 (PRAGMA fix):** Low risk. Enables a constraint that was always intended to be active. All `ondelete` clauses use either `CASCADE` or `SET NULL`, which are safe operations.
|
||||
- **Step 2 (Null-safety):** Zero risk. Purely defensive — only affects display, changes no data.
|
||||
- **Step 3-4 (Orphan cleanup):** Low risk. Deletes rows that reference non-existent nodes, which are broken data by definition.
|
||||
- **Step 5 (CLI):** Low risk. The `--node-cleanup` flag defaults to `false` (opt-in), preserving backward compatibility.
|
||||
- **Step 6 (Test fix):** Zero risk. Only affects test infrastructure.
|
||||
|
||||
## Verification
|
||||
|
||||
After deploying:
|
||||
|
||||
1. Check the API returns 200 for `/api/v1/user/profiles`:
|
||||
```bash
|
||||
curl -s -o /dev/null -w "%{http_code}" http://localhost:8000/api/v1/user/profiles
|
||||
```
|
||||
2. Run manual cleanup to repair existing orphans:
|
||||
```bash
|
||||
meshcore-hub collector cleanup --dry-run --node-cleanup
|
||||
meshcore-hub collector cleanup --node-cleanup
|
||||
```
|
||||
3. Confirm no orphaned rows remain across all three tables:
|
||||
```sql
|
||||
SELECT 'user_profile_nodes' AS tbl, COUNT(*) FROM user_profile_nodes upn
|
||||
LEFT JOIN nodes n ON n.id = upn.node_id WHERE n.id IS NULL
|
||||
UNION ALL
|
||||
SELECT 'event_observers', COUNT(*) FROM event_observers eo
|
||||
LEFT JOIN nodes n ON n.id = eo.observer_node_id WHERE n.id IS NULL
|
||||
UNION ALL
|
||||
SELECT 'node_tags', COUNT(*) FROM node_tags nt
|
||||
LEFT JOIN nodes n ON n.id = nt.node_id WHERE n.id IS NULL;
|
||||
```
|
||||
Expected: 0 for all three.
|
||||
|
||||
## Resolved Review Issues
|
||||
|
||||
| Issue | Resolution |
|
||||
|-------|-----------|
|
||||
| Orphan cleanup scope (UserProfileNode only vs. all 3 tables) | Cover all 3 tables: `user_profile_nodes`, `event_observers`, `node_tags` |
|
||||
| Test fixtures lack FK pragma | Add `PRAGMA foreign_keys=ON` to both collector and API test fixtures |
|
||||
| CLI `cleanup` doesn't run node cleanup | Add `--node-cleanup` option (default off, opt-in) |
|
||||
| Step 1 code uses shadow-import `from sqlalchemy import event as ...` | Reuse existing module-scope `event` import |
|
||||
| Missing test for `cleanup_inactive_nodes` | Add test with cascade verification |
|
||||
@@ -0,0 +1,50 @@
|
||||
# Tasks — Node Association Foreign Key Fix
|
||||
|
||||
Reference: [plan.md](./plan.md)
|
||||
|
||||
## Phase 1: Root Cause Fix + Defensive Hardening
|
||||
|
||||
- [ ] **T1.1** — Add `PRAGMA foreign_keys=ON` event listener to `self.async_engine.sync_engine` in `src/meshcore_hub/common/database.py` (after line 115). Reuse existing module-scope `event` import. Guard with `if database_url.startswith("sqlite"):`.
|
||||
- [ ] **T1.2** — Add `PRAGMA foreign_keys=ON` event listener to async engine in `tests/test_collector/conftest.py:35`.
|
||||
- [ ] **T1.3** — Add `PRAGMA foreign_keys=ON` event listener to sync engine in `tests/test_api/conftest.py:49`.
|
||||
- [ ] **T1.4** — In `src/meshcore_hub/api/routes/user_profiles.py`, add null-guard in `_build_adopted_nodes()` (line 34): skip associations where `assoc.node is None`, log a warning with `assoc.node_id`.
|
||||
- [ ] **T1.5** — In `src/meshcore_hub/api/routes/user_profiles.py`, refactor the duplicate inline adoption loop in `list_profiles()` (lines 92–101) to call `_build_adopted_nodes()` instead.
|
||||
|
||||
## Phase 2: Orphan Cleanup Function
|
||||
|
||||
- [ ] **T2.1** — Add `cleanup_orphaned_node_relations(db: AsyncSession, dry_run: bool = False) -> dict[str, int]` to `src/meshcore_hub/collector/cleanup.py`. Must cover all three tables (`user_profile_nodes.node_id`, `event_observers.observer_node_id`, `node_tags.node_id`) using `LEFT JOIN nodes ... WHERE nodes.id IS NULL` pattern. Return per-table counts.
|
||||
|
||||
## Phase 3: Integration
|
||||
|
||||
- [ ] **T3.1** — In `src/meshcore_hub/collector/subscriber.py`, add `cleanup_orphaned_node_relations()` call inside the `run_cleanup()` inline function (after `cleanup_inactive_nodes()` at line ~323). Gate it behind the same `if self._node_cleanup_enabled:` block. Log the orphan results.
|
||||
- [ ] **T3.2** — In `src/meshcore_hub/collector/cli.py`, add `--node-cleanup` flag (default `false`) and `--node-cleanup-days` option (default `30`) to the `cleanup` CLI command. When `--node-cleanup` is set, run `cleanup_inactive_nodes()` and `cleanup_orphaned_node_relations()`. Display results.
|
||||
- [ ] **T3.3** — Update the `cleanup` command docstring (`cli.py:537–545`): remove "Node records are never deleted", document `--node-cleanup` and `--node-cleanup-days`.
|
||||
- [ ] **T3.4** — In `src/meshcore_hub/collector/cli.py`, add `user_profile_nodes` and `event_observers` to the truncate cascade warning (lines 717–724).
|
||||
|
||||
## Phase 4: Tests
|
||||
|
||||
- [ ] **T4.1** — Add test for `cleanup_inactive_nodes()` in `tests/test_collector/test_cleanup.py`: create a node with rows in all three dependent tables (`user_profile_nodes`, `event_observers`, `node_tags`), run cleanup, assert cascade deleted all dependent rows.
|
||||
- [ ] **T4.2** — Add test for `cleanup_orphaned_node_relations()` in `tests/test_collector/test_cleanup.py`: create orphaned rows in all three tables referencing non-existent `node_id`s, run function, assert all orphans deleted and per-table counts correct.
|
||||
- [ ] **T4.3** — Add test in `tests/test_api/test_user_profiles.py`: create a profile with an adopted node, delete the node (leaving an orphaned `UserProfileNode`), call `GET /profiles`, assert 200 response and orphaned node excluded from `adopted_nodes`.
|
||||
|
||||
## Phase 5: Documentation
|
||||
|
||||
- [ ] **T5.1** — Add upgrade note to `docs/upgrading.md`: describe the async FK pragma fix, automatic orphan cleanup in the retention cycle, and the manual repair command (`meshcore-hub collector cleanup --node-cleanup`).
|
||||
|
||||
## Verification
|
||||
|
||||
After all tasks complete:
|
||||
|
||||
```bash
|
||||
# Lint and typecheck
|
||||
source .venv/bin/activate
|
||||
pre-commit run --all-files
|
||||
|
||||
# Targeted tests
|
||||
pytest tests/test_collector/test_cleanup.py -v
|
||||
pytest tests/test_api/test_user_profiles.py -v
|
||||
pytest tests/test_common/ -v
|
||||
|
||||
# Full suite (only if changes span components)
|
||||
pytest
|
||||
```
|
||||
@@ -2,6 +2,33 @@
|
||||
|
||||
This guide covers upgrading from a previous MeshCore Hub release to the current version. Check the relevant version section below before upgrading.
|
||||
|
||||
## v0.11.0
|
||||
|
||||
### Async SQLite Foreign Key Fix
|
||||
|
||||
The async SQLAlchemy engine now enables `PRAGMA foreign_keys=ON` for SQLite databases, matching the behavior of the sync engine. Previously, cascade deletes (`ondelete="CASCADE"`) were silently ignored when the collector deleted inactive nodes via the async engine, leaving orphaned rows in `user_profile_nodes`, `event_observers`, and `node_tags`.
|
||||
|
||||
**This is an automatic fix** — no configuration changes are required. The orphaned rows that may have accumulated in existing databases can be cleaned up with:
|
||||
|
||||
```bash
|
||||
# Dry run to preview
|
||||
meshcore-hub collector cleanup --node-cleanup --dry-run
|
||||
|
||||
# Live cleanup
|
||||
meshcore-hub collector cleanup --node-cleanup
|
||||
```
|
||||
|
||||
The collector's scheduled cleanup cycle now also runs orphan cleanup automatically after node deletion when `NODE_CLEANUP_ENABLED=true`.
|
||||
|
||||
### CLI Changes
|
||||
|
||||
The `meshcore-hub collector cleanup` command now accepts:
|
||||
|
||||
| Flag | Default | Description |
|
||||
|------|---------|-------------|
|
||||
| `--node-cleanup` | `false` | Also delete inactive nodes and orphaned relations |
|
||||
| `--node-cleanup-days` | `30` | Inactivity threshold for node deletion |
|
||||
|
||||
## v0.10.0
|
||||
|
||||
This release introduces OIDC authentication, user profiles with node adoption, removes the Members system, replaces `role=infra` tags with adoption-based infrastructure detection, and replaces the admin tag editor with an inline editor on the node detail page.
|
||||
|
||||
@@ -32,6 +32,13 @@ def _build_adopted_nodes(profile: UserProfile) -> list[AdoptedNodeRead]:
|
||||
"""Extract adopted node list from a profile's eager-loaded associations."""
|
||||
adopted_nodes = []
|
||||
for assoc in profile.node_associations:
|
||||
if assoc.node is None:
|
||||
logger.warning(
|
||||
"Orphaned UserProfileNode detected: profile=%s, node_id=%s",
|
||||
profile.id,
|
||||
assoc.node_id,
|
||||
)
|
||||
continue
|
||||
adopted_nodes.append(
|
||||
AdoptedNodeRead(
|
||||
public_key=assoc.node.public_key,
|
||||
@@ -89,16 +96,6 @@ async def list_profiles(
|
||||
|
||||
items = []
|
||||
for profile in profiles:
|
||||
adopted_nodes = []
|
||||
for assoc in profile.node_associations:
|
||||
adopted_nodes.append(
|
||||
AdoptedNodeRead(
|
||||
public_key=assoc.node.public_key,
|
||||
name=assoc.node.name,
|
||||
adv_type=assoc.node.adv_type,
|
||||
adopted_at=assoc.adopted_at,
|
||||
)
|
||||
)
|
||||
items.append(
|
||||
UserProfileListItem(
|
||||
id=profile.id,
|
||||
@@ -108,7 +105,7 @@ async def list_profiles(
|
||||
description=profile.description,
|
||||
url=profile.url,
|
||||
node_count=len(profile.node_associations),
|
||||
adopted_nodes=adopted_nodes,
|
||||
adopted_nodes=_build_adopted_nodes(profile),
|
||||
)
|
||||
)
|
||||
|
||||
|
||||
@@ -13,11 +13,14 @@ from sqlalchemy.ext.asyncio import AsyncSession
|
||||
from meshcore_hub.common.models import (
|
||||
Advertisement,
|
||||
EventLog,
|
||||
EventObserver,
|
||||
Message,
|
||||
Node,
|
||||
NodeTag,
|
||||
Telemetry,
|
||||
TracePath,
|
||||
)
|
||||
from meshcore_hub.common.models.user_profile_node import UserProfileNode
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
@@ -223,3 +226,53 @@ async def cleanup_inactive_nodes(
|
||||
cutoff_date.isoformat(),
|
||||
)
|
||||
return count
|
||||
|
||||
|
||||
async def cleanup_orphaned_node_relations(
|
||||
db: AsyncSession,
|
||||
dry_run: bool = False,
|
||||
) -> dict[str, int]:
|
||||
"""Delete orphaned rows in node-dependent tables.
|
||||
|
||||
Finds and deletes rows in user_profile_nodes, event_observers,
|
||||
and node_tags that reference node_id values not present in the
|
||||
nodes table.
|
||||
|
||||
Args:
|
||||
db: Database session
|
||||
dry_run: If True, only count records without deleting
|
||||
|
||||
Returns:
|
||||
Dict mapping table names to deletion counts
|
||||
"""
|
||||
counts: dict[str, int] = {}
|
||||
tables = [
|
||||
("user_profile_nodes", UserProfileNode, UserProfileNode.node_id),
|
||||
("event_observers", EventObserver, EventObserver.observer_node_id),
|
||||
("node_tags", NodeTag, NodeTag.node_id),
|
||||
]
|
||||
|
||||
for table_name, model, fk_column in tables:
|
||||
subq = select(Node.id).where(Node.id == fk_column)
|
||||
if dry_run:
|
||||
count_stmt = select(func.count()).select_from(model).where(~subq.exists())
|
||||
result = await db.execute(count_stmt)
|
||||
count = result.scalar() or 0
|
||||
else:
|
||||
del_stmt = delete(model).where(~subq.exists())
|
||||
result = await db.execute(del_stmt)
|
||||
count = result.rowcount or 0 # type: ignore[attr-defined]
|
||||
|
||||
counts[table_name] = count
|
||||
if count > 0:
|
||||
logger.info(
|
||||
"%s: %s %d orphaned rows",
|
||||
table_name,
|
||||
"would delete" if dry_run else "deleted",
|
||||
count,
|
||||
)
|
||||
|
||||
if not dry_run:
|
||||
await db.commit()
|
||||
|
||||
return counts
|
||||
|
||||
@@ -522,6 +522,18 @@ def import_tags_cmd(
|
||||
envvar="DATA_RETENTION_DAYS",
|
||||
help="Number of days to retain data (default: 30)",
|
||||
)
|
||||
@click.option(
|
||||
"--node-cleanup",
|
||||
is_flag=True,
|
||||
default=False,
|
||||
help="Also delete inactive nodes and orphaned relations",
|
||||
)
|
||||
@click.option(
|
||||
"--node-cleanup-days",
|
||||
type=int,
|
||||
default=30,
|
||||
help="Delete nodes not seen for this many days (default: 30)",
|
||||
)
|
||||
@click.option(
|
||||
"--dry-run",
|
||||
is_flag=True,
|
||||
@@ -532,6 +544,8 @@ def import_tags_cmd(
|
||||
def cleanup_cmd(
|
||||
ctx: click.Context,
|
||||
retention_days: int,
|
||||
node_cleanup: bool,
|
||||
node_cleanup_days: int,
|
||||
dry_run: bool,
|
||||
) -> None:
|
||||
"""Manually run data cleanup to delete old events.
|
||||
@@ -543,7 +557,9 @@ def cleanup_cmd(
|
||||
- Trace paths
|
||||
- Event logs
|
||||
|
||||
Node records are never deleted - only event data.
|
||||
Use --node-cleanup to also delete inactive nodes (not seen for
|
||||
--node-cleanup-days) and any orphaned rows in user_profile_nodes,
|
||||
event_observers, and node_tags that reference deleted nodes.
|
||||
|
||||
Use --dry-run to preview what would be deleted without
|
||||
actually deleting anything.
|
||||
@@ -568,7 +584,11 @@ def cleanup_cmd(
|
||||
click.echo("")
|
||||
|
||||
from meshcore_hub.common.database import DatabaseManager
|
||||
from meshcore_hub.collector.cleanup import cleanup_old_data
|
||||
from meshcore_hub.collector.cleanup import (
|
||||
cleanup_old_data,
|
||||
cleanup_inactive_nodes,
|
||||
cleanup_orphaned_node_relations,
|
||||
)
|
||||
|
||||
# Initialize database
|
||||
db = DatabaseManager(ctx.obj["database_url"])
|
||||
@@ -591,6 +611,29 @@ def cleanup_cmd(
|
||||
click.echo(f" Event logs: {stats.event_logs_deleted}")
|
||||
click.echo(f" Total: {stats.total_deleted}")
|
||||
|
||||
if node_cleanup:
|
||||
click.echo("")
|
||||
nodes_deleted = await cleanup_inactive_nodes(
|
||||
session,
|
||||
node_cleanup_days,
|
||||
dry_run=dry_run,
|
||||
)
|
||||
mode = "would be" if dry_run else "were"
|
||||
click.echo(
|
||||
f" Inactive nodes {mode} deleted: {nodes_deleted}"
|
||||
f" (older than {node_cleanup_days} days)"
|
||||
)
|
||||
|
||||
orphan_counts = await cleanup_orphaned_node_relations(
|
||||
session,
|
||||
dry_run=dry_run,
|
||||
)
|
||||
if any(orphan_counts.values()):
|
||||
click.echo(" Orphaned relations:")
|
||||
for table_name, count in orphan_counts.items():
|
||||
if count > 0:
|
||||
click.echo(f" {table_name}: {count}")
|
||||
|
||||
if dry_run:
|
||||
click.echo("")
|
||||
click.echo("(Dry run - no data was actually deleted)")
|
||||
@@ -718,6 +761,8 @@ def truncate_cmd(
|
||||
"WARNING: Clearing nodes will also clear all related data due to foreign keys:"
|
||||
)
|
||||
click.echo(" - node_tags")
|
||||
click.echo(" - user_profile_nodes")
|
||||
click.echo(" - event_observers")
|
||||
click.echo(" - advertisements")
|
||||
click.echo(" - messages")
|
||||
click.echo(" - telemetry")
|
||||
|
||||
@@ -294,6 +294,7 @@ class Subscriber(LetsMeshNormalizer):
|
||||
from meshcore_hub.collector.cleanup import (
|
||||
cleanup_old_data,
|
||||
cleanup_inactive_nodes,
|
||||
cleanup_orphaned_node_relations,
|
||||
)
|
||||
|
||||
# Get async session and run cleanup
|
||||
@@ -322,6 +323,18 @@ class Subscriber(LetsMeshNormalizer):
|
||||
nodes_deleted,
|
||||
)
|
||||
|
||||
orphan_counts = (
|
||||
await cleanup_orphaned_node_relations(
|
||||
session,
|
||||
dry_run=False,
|
||||
)
|
||||
)
|
||||
if any(orphan_counts.values()):
|
||||
logger.info(
|
||||
"Orphan cleanup completed: %s",
|
||||
orphan_counts,
|
||||
)
|
||||
|
||||
loop.run_until_complete(run_cleanup())
|
||||
self._last_cleanup = now
|
||||
|
||||
|
||||
@@ -113,6 +113,18 @@ class DatabaseManager:
|
||||
# Create async engine for async operations
|
||||
async_url = database_url.replace("sqlite://", "sqlite+aiosqlite://")
|
||||
self.async_engine = create_async_engine(async_url, echo=echo)
|
||||
|
||||
# Enable foreign keys for async SQLite engine
|
||||
if database_url.startswith("sqlite"):
|
||||
|
||||
@event.listens_for(self.async_engine.sync_engine, "connect")
|
||||
def set_sqlite_pragma_async(
|
||||
dbapi_connection: object, connection_record: object
|
||||
) -> None:
|
||||
cursor = dbapi_connection.cursor() # type: ignore[attr-defined]
|
||||
cursor.execute("PRAGMA foreign_keys=ON")
|
||||
cursor.close()
|
||||
|
||||
from sqlalchemy.ext.asyncio import async_sessionmaker
|
||||
|
||||
self.async_session_factory = async_sessionmaker(
|
||||
|
||||
@@ -8,7 +8,7 @@ from unittest.mock import MagicMock, patch
|
||||
|
||||
import pytest
|
||||
from fastapi.testclient import TestClient
|
||||
from sqlalchemy import create_engine
|
||||
from sqlalchemy import create_engine, event as sa_event
|
||||
from sqlalchemy.orm import sessionmaker
|
||||
|
||||
from meshcore_hub.api.app import create_app
|
||||
@@ -50,6 +50,13 @@ def api_db_engine(test_db_path):
|
||||
db_url,
|
||||
connect_args={"check_same_thread": False},
|
||||
)
|
||||
|
||||
@sa_event.listens_for(engine, "connect")
|
||||
def set_sqlite_pragma(dbapi_connection: object, connection_record: object) -> None:
|
||||
cursor = dbapi_connection.cursor() # type: ignore[attr-defined]
|
||||
cursor.execute("PRAGMA foreign_keys=ON")
|
||||
cursor.close()
|
||||
|
||||
Base.metadata.create_all(engine)
|
||||
yield engine
|
||||
Base.metadata.drop_all(engine)
|
||||
|
||||
@@ -5,6 +5,7 @@ from unittest.mock import patch
|
||||
import pytest
|
||||
|
||||
from meshcore_hub.common.models import UserProfile
|
||||
from meshcore_hub.common.models.user_profile_node import UserProfileNode
|
||||
|
||||
TEST_USER_ID = "oidc-user-123"
|
||||
OTHER_USER_ID = "oidc-user-456"
|
||||
@@ -81,6 +82,36 @@ class TestListProfiles:
|
||||
assert len(profile["adopted_nodes"]) == 1
|
||||
assert profile["adopted_nodes"][0]["name"] == sample_node.name
|
||||
|
||||
def test_list_profiles_resilient_to_orphaned_adoption(
|
||||
self, client_no_auth, api_db_session, sample_user_profile, sample_node
|
||||
):
|
||||
"""Test that orphaned UserProfileNode rows don't cause 500 errors."""
|
||||
from sqlalchemy import text
|
||||
|
||||
adoption = UserProfileNode(
|
||||
user_profile_id=sample_user_profile.id,
|
||||
node_id=sample_node.id,
|
||||
)
|
||||
api_db_session.add(adoption)
|
||||
api_db_session.commit()
|
||||
|
||||
api_db_session.execute(text("PRAGMA foreign_keys=OFF"))
|
||||
api_db_session.execute(
|
||||
text("DELETE FROM nodes WHERE id = :id"),
|
||||
{"id": sample_node.id},
|
||||
)
|
||||
api_db_session.commit()
|
||||
api_db_session.execute(text("PRAGMA foreign_keys=ON"))
|
||||
|
||||
response = client_no_auth.get(
|
||||
"/api/v1/user/profiles",
|
||||
headers=USER_HEADERS,
|
||||
)
|
||||
assert response.status_code == 200
|
||||
data = response.json()
|
||||
profile = next(p for p in data["items"] if p["id"] == sample_user_profile.id)
|
||||
assert profile["adopted_nodes"] == []
|
||||
|
||||
|
||||
class TestGetMyProfile:
|
||||
"""Tests for GET /user/profile/me endpoint."""
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
"""Fixtures for collector component tests."""
|
||||
|
||||
import pytest
|
||||
from sqlalchemy import event as sa_event
|
||||
from sqlalchemy.ext.asyncio import AsyncSession, create_async_engine
|
||||
from sqlalchemy.ext.asyncio import async_sessionmaker
|
||||
|
||||
@@ -34,6 +35,14 @@ async def async_db_session():
|
||||
# Create async engine with in-memory database
|
||||
engine = create_async_engine("sqlite+aiosqlite:///:memory:")
|
||||
|
||||
@sa_event.listens_for(engine.sync_engine, "connect")
|
||||
def set_sqlite_pragma_async(
|
||||
dbapi_connection: object, connection_record: object
|
||||
) -> None:
|
||||
cursor = dbapi_connection.cursor() # type: ignore[attr-defined]
|
||||
cursor.execute("PRAGMA foreign_keys=ON")
|
||||
cursor.close()
|
||||
|
||||
# Create tables
|
||||
async with engine.begin() as conn:
|
||||
await conn.run_sync(Base.metadata.create_all)
|
||||
|
||||
@@ -2,17 +2,27 @@
|
||||
|
||||
import pytest
|
||||
from datetime import datetime, timedelta, timezone
|
||||
from sqlalchemy import func, select
|
||||
from sqlalchemy.ext.asyncio import AsyncSession
|
||||
|
||||
from meshcore_hub.collector.cleanup import cleanup_old_data, CleanupStats
|
||||
from meshcore_hub.collector.cleanup import (
|
||||
cleanup_inactive_nodes,
|
||||
cleanup_old_data,
|
||||
cleanup_orphaned_node_relations,
|
||||
CleanupStats,
|
||||
)
|
||||
from meshcore_hub.common.models import (
|
||||
Advertisement,
|
||||
EventLog,
|
||||
EventObserver,
|
||||
Message,
|
||||
Node,
|
||||
NodeTag,
|
||||
Telemetry,
|
||||
TracePath,
|
||||
)
|
||||
from meshcore_hub.common.models.user_profile_node import UserProfileNode
|
||||
from meshcore_hub.common.models.user_profile import UserProfile
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@@ -248,3 +258,162 @@ async def test_cleanup_stats_repr() -> None:
|
||||
assert "total=21" in repr_str
|
||||
assert "advertisements=10" in repr_str
|
||||
assert "messages=5" in repr_str
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_cleanup_inactive_nodes_cascades(async_db_session: AsyncSession) -> None:
|
||||
"""Test that deleting inactive nodes cascades to dependent tables."""
|
||||
old_date = datetime.now(timezone.utc) - timedelta(days=60)
|
||||
|
||||
node = Node(
|
||||
public_key="a" * 64,
|
||||
name="Stale Node",
|
||||
last_seen=old_date,
|
||||
)
|
||||
async_db_session.add(node)
|
||||
await async_db_session.flush()
|
||||
|
||||
profile = UserProfile(user_id="cascade-test-user", name="Test")
|
||||
async_db_session.add(profile)
|
||||
await async_db_session.flush()
|
||||
|
||||
adoption = UserProfileNode(
|
||||
user_profile_id=profile.id,
|
||||
node_id=node.id,
|
||||
)
|
||||
async_db_session.add(adoption)
|
||||
|
||||
tag = NodeTag(node_id=node.id, key="role", value="gateway")
|
||||
async_db_session.add(tag)
|
||||
|
||||
observer = EventObserver(
|
||||
event_type="message",
|
||||
event_hash="abc123def456abc123def456abc12345",
|
||||
observer_node_id=node.id,
|
||||
)
|
||||
async_db_session.add(observer)
|
||||
|
||||
await async_db_session.commit()
|
||||
|
||||
deleted = await cleanup_inactive_nodes(
|
||||
async_db_session, inactivity_days=30, dry_run=False
|
||||
)
|
||||
assert deleted == 1
|
||||
|
||||
assert await async_db_session.scalar(select(func.count()).select_from(Node)) == 0
|
||||
assert (
|
||||
await async_db_session.scalar(select(func.count()).select_from(UserProfileNode))
|
||||
== 0
|
||||
)
|
||||
assert await async_db_session.scalar(select(func.count()).select_from(NodeTag)) == 0
|
||||
assert (
|
||||
await async_db_session.scalar(select(func.count()).select_from(EventObserver))
|
||||
== 0
|
||||
)
|
||||
|
||||
assert (
|
||||
await async_db_session.scalar(select(func.count()).select_from(UserProfile))
|
||||
== 1
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_cleanup_orphaned_node_relations(
|
||||
async_db_session: AsyncSession,
|
||||
) -> None:
|
||||
"""Test orphan cleanup deletes rows referencing non-existent nodes."""
|
||||
from sqlalchemy import text
|
||||
|
||||
node = Node(
|
||||
public_key="o" * 64,
|
||||
name="Temporary Node",
|
||||
)
|
||||
async_db_session.add(node)
|
||||
await async_db_session.flush()
|
||||
|
||||
profile = UserProfile(user_id="orphan-test-user", name="Test")
|
||||
async_db_session.add(profile)
|
||||
await async_db_session.flush()
|
||||
|
||||
adoption = UserProfileNode(
|
||||
user_profile_id=profile.id,
|
||||
node_id=node.id,
|
||||
)
|
||||
async_db_session.add(adoption)
|
||||
|
||||
tag = NodeTag(node_id=node.id, key="role", value="gateway")
|
||||
async_db_session.add(tag)
|
||||
|
||||
observer = EventObserver(
|
||||
event_type="message",
|
||||
event_hash="abc123def456abc123def456abc12399",
|
||||
observer_node_id=node.id,
|
||||
)
|
||||
async_db_session.add(observer)
|
||||
|
||||
await async_db_session.commit()
|
||||
|
||||
await async_db_session.execute(text("PRAGMA foreign_keys=OFF"))
|
||||
await async_db_session.execute(
|
||||
text("DELETE FROM nodes WHERE id = :id"), {"id": node.id}
|
||||
)
|
||||
await async_db_session.commit()
|
||||
await async_db_session.execute(text("PRAGMA foreign_keys=ON"))
|
||||
|
||||
counts = await cleanup_orphaned_node_relations(async_db_session, dry_run=False)
|
||||
|
||||
assert counts["user_profile_nodes"] == 1
|
||||
assert counts["event_observers"] == 1
|
||||
assert counts["node_tags"] == 1
|
||||
|
||||
assert (
|
||||
await async_db_session.scalar(select(func.count()).select_from(UserProfileNode))
|
||||
== 0
|
||||
)
|
||||
assert await async_db_session.scalar(select(func.count()).select_from(NodeTag)) == 0
|
||||
assert (
|
||||
await async_db_session.scalar(select(func.count()).select_from(EventObserver))
|
||||
== 0
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_cleanup_orphaned_node_relations_dry_run(
|
||||
async_db_session: AsyncSession,
|
||||
) -> None:
|
||||
"""Test orphan dry-run counts but does not delete."""
|
||||
from sqlalchemy import text
|
||||
|
||||
node = Node(
|
||||
public_key="p" * 64,
|
||||
name="Dry Run Node",
|
||||
)
|
||||
async_db_session.add(node)
|
||||
await async_db_session.flush()
|
||||
|
||||
profile = UserProfile(user_id="orphan-dryrun-user", name="Test")
|
||||
async_db_session.add(profile)
|
||||
await async_db_session.flush()
|
||||
|
||||
adoption = UserProfileNode(
|
||||
user_profile_id=profile.id,
|
||||
node_id=node.id,
|
||||
)
|
||||
async_db_session.add(adoption)
|
||||
await async_db_session.commit()
|
||||
|
||||
await async_db_session.execute(text("PRAGMA foreign_keys=OFF"))
|
||||
await async_db_session.execute(
|
||||
text("DELETE FROM nodes WHERE id = :id"), {"id": node.id}
|
||||
)
|
||||
await async_db_session.commit()
|
||||
await async_db_session.execute(text("PRAGMA foreign_keys=ON"))
|
||||
|
||||
counts = await cleanup_orphaned_node_relations(async_db_session, dry_run=True)
|
||||
|
||||
assert counts["user_profile_nodes"] == 1
|
||||
|
||||
assert (
|
||||
await async_db_session.scalar(select(func.count()).select_from(UserProfileNode))
|
||||
== 1
|
||||
)
|
||||
|
||||
Reference in New Issue
Block a user