refactor(db): run Alembic at boot, retire ad-hoc _migrate_* helpers
initialize() now delegates to _apply_schema(): real boots run 'alembic upgrade head' (schema owned by the migration history); tests (DECNET_TESTING=1) keep create_all, which is faster and needs no upgrade path. MySQL wraps the upgrade in the existing GET_LOCK advisory lock so concurrent uvicorn workers don't race on DDL. Deletes the three _migrate_* crimes (attackers-table legacy drop + GeoIP backfill, TEXT->MEDIUMTEXT widening) — all now handled by the baseline migration and the _BIG_TEXT model variants. Drops the test file that only exercised the deleted helpers; adds tests pinning the alembic-vs-create_all gate and guarding that every model table is in the migration head.
This commit is contained in:
@@ -1,234 +0,0 @@
|
||||
# SPDX-License-Identifier: AGPL-3.0-or-later
|
||||
"""
|
||||
Tests for MySQLRepository._migrate_column_types().
|
||||
|
||||
No live MySQL server required — uses an in-memory SQLite engine that exposes
|
||||
the same information_schema-style query surface via a mocked connection, plus
|
||||
an integration-style test using a real async engine over aiosqlite (which
|
||||
ignores the TEXT/MEDIUMTEXT distinction but verifies the ALTER path is called
|
||||
and idempotent).
|
||||
|
||||
The ALTER TABLE branch is tested via unittest.mock: we intercept the
|
||||
information_schema query result and assert the correct MODIFY COLUMN
|
||||
statements are issued.
|
||||
"""
|
||||
from __future__ import annotations
|
||||
|
||||
import pytest
|
||||
from unittest.mock import AsyncMock, MagicMock, patch, call
|
||||
|
||||
from decnet.web.db.mysql.repository import MySQLRepository
|
||||
|
||||
|
||||
# ── helpers ──────────────────────────────────────────────────────────────────
|
||||
|
||||
def _make_repo() -> MySQLRepository:
|
||||
"""Construct a MySQLRepository without touching any real DB."""
|
||||
return MySQLRepository.__new__(MySQLRepository)
|
||||
|
||||
|
||||
# ── _migrate_column_types ─────────────────────────────────────────────────────
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_migrate_column_types_issues_alter_for_text_columns():
|
||||
"""When information_schema reports TEXT columns, ALTER TABLE is called for each."""
|
||||
repo = _make_repo()
|
||||
|
||||
# Rows returned by the information_schema query: two TEXT columns found
|
||||
fake_rows = [
|
||||
("attackers", "commands"),
|
||||
("attackers", "fingerprints"),
|
||||
("state", "value"),
|
||||
]
|
||||
|
||||
exec_results: list[str] = []
|
||||
|
||||
async def fake_execute(stmt):
|
||||
sql = str(stmt)
|
||||
if "information_schema" in sql:
|
||||
result = MagicMock()
|
||||
result.fetchall.return_value = fake_rows
|
||||
return result
|
||||
# Capture ALTER TABLE calls
|
||||
exec_results.append(sql)
|
||||
return MagicMock()
|
||||
|
||||
fake_conn = AsyncMock()
|
||||
fake_conn.execute.side_effect = fake_execute
|
||||
|
||||
fake_ctx = AsyncMock()
|
||||
fake_ctx.__aenter__ = AsyncMock(return_value=fake_conn)
|
||||
fake_ctx.__aexit__ = AsyncMock(return_value=False)
|
||||
|
||||
repo.engine = MagicMock()
|
||||
repo.engine.begin.return_value = fake_ctx
|
||||
|
||||
await repo._migrate_column_types()
|
||||
|
||||
# Three ALTER TABLE statements expected, one per TEXT column returned
|
||||
assert len(exec_results) == 3
|
||||
assert any("`commands` MEDIUMTEXT" in s for s in exec_results)
|
||||
assert any("`fingerprints` MEDIUMTEXT" in s for s in exec_results)
|
||||
assert any("`value` MEDIUMTEXT" in s for s in exec_results)
|
||||
# Verify NOT NULL is preserved
|
||||
assert all("NOT NULL" in s for s in exec_results)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_migrate_column_types_no_alter_when_already_mediumtext():
|
||||
"""When information_schema returns no TEXT rows, no ALTER is issued."""
|
||||
repo = _make_repo()
|
||||
|
||||
exec_results: list[str] = []
|
||||
|
||||
async def fake_execute(stmt):
|
||||
sql = str(stmt)
|
||||
if "information_schema" in sql:
|
||||
result = MagicMock()
|
||||
result.fetchall.return_value = [] # nothing to migrate
|
||||
return result
|
||||
exec_results.append(sql)
|
||||
return MagicMock()
|
||||
|
||||
fake_conn = AsyncMock()
|
||||
fake_conn.execute.side_effect = fake_execute
|
||||
|
||||
fake_ctx = AsyncMock()
|
||||
fake_ctx.__aenter__ = AsyncMock(return_value=fake_conn)
|
||||
fake_ctx.__aexit__ = AsyncMock(return_value=False)
|
||||
|
||||
repo.engine = MagicMock()
|
||||
repo.engine.begin.return_value = fake_ctx
|
||||
|
||||
await repo._migrate_column_types()
|
||||
|
||||
assert exec_results == [], "No ALTER TABLE should be issued when columns are already MEDIUMTEXT"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_migrate_column_types_idempotent_on_repeated_calls():
|
||||
"""Calling _migrate_column_types twice is safe: second call is a no-op."""
|
||||
repo = _make_repo()
|
||||
call_count = 0
|
||||
|
||||
async def fake_execute(stmt):
|
||||
nonlocal call_count
|
||||
sql = str(stmt)
|
||||
if "information_schema" in sql:
|
||||
result = MagicMock()
|
||||
# First call: two TEXT columns; second call: zero (already migrated)
|
||||
call_count += 1
|
||||
result.fetchall.return_value = (
|
||||
[("attackers", "commands")] if call_count == 1 else []
|
||||
)
|
||||
return result
|
||||
return MagicMock()
|
||||
|
||||
def _make_ctx():
|
||||
fake_conn = AsyncMock()
|
||||
fake_conn.execute.side_effect = fake_execute
|
||||
ctx = AsyncMock()
|
||||
ctx.__aenter__ = AsyncMock(return_value=fake_conn)
|
||||
ctx.__aexit__ = AsyncMock(return_value=False)
|
||||
return ctx
|
||||
|
||||
repo.engine = MagicMock()
|
||||
repo.engine.begin.side_effect = _make_ctx
|
||||
|
||||
await repo._migrate_column_types()
|
||||
await repo._migrate_column_types() # second call must not raise
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_migrate_column_types_default_clause_per_column():
|
||||
"""Each attacker column gets DEFAULT '[]'; state.value gets no DEFAULT."""
|
||||
repo = _make_repo()
|
||||
|
||||
all_text_rows = [
|
||||
("attackers", "commands"),
|
||||
("attackers", "fingerprints"),
|
||||
("attackers", "services"),
|
||||
("attackers", "deckies"),
|
||||
("state", "value"),
|
||||
]
|
||||
alter_stmts: list[str] = []
|
||||
|
||||
async def fake_execute(stmt):
|
||||
sql = str(stmt)
|
||||
if "information_schema" in sql:
|
||||
result = MagicMock()
|
||||
result.fetchall.return_value = all_text_rows
|
||||
return result
|
||||
alter_stmts.append(sql)
|
||||
return MagicMock()
|
||||
|
||||
fake_conn = AsyncMock()
|
||||
fake_conn.execute.side_effect = fake_execute
|
||||
|
||||
fake_ctx = AsyncMock()
|
||||
fake_ctx.__aenter__ = AsyncMock(return_value=fake_conn)
|
||||
fake_ctx.__aexit__ = AsyncMock(return_value=False)
|
||||
|
||||
repo.engine = MagicMock()
|
||||
repo.engine.begin.return_value = fake_ctx
|
||||
|
||||
await repo._migrate_column_types()
|
||||
|
||||
attacker_alters = [s for s in alter_stmts if "`attackers`" in s]
|
||||
state_alters = [s for s in alter_stmts if "`state`" in s]
|
||||
|
||||
assert len(attacker_alters) == 4
|
||||
assert len(state_alters) == 1
|
||||
|
||||
for stmt in attacker_alters:
|
||||
assert "DEFAULT '[]'" in stmt, f"Missing DEFAULT '[]' in: {stmt}"
|
||||
|
||||
# state.value has no DEFAULT in the schema
|
||||
assert "DEFAULT" not in state_alters[0], \
|
||||
f"Unexpected DEFAULT in state.value alter: {state_alters[0]}"
|
||||
|
||||
|
||||
# ── initialize override ───────────────────────────────────────────────────────
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_mysql_initialize_calls_migrate_column_types():
|
||||
"""MySQLRepository.initialize() must invoke every migration helper
|
||||
in the right order: attackers first, then column types, then seed
|
||||
the admin user.
|
||||
|
||||
The legacy ``_migrate_session_profile_table`` step (DEBT-036) was
|
||||
dropped when SessionProfile was deleted in favour of the
|
||||
``observations`` table — see DEBT-050 / BEHAVE-INTEGRATION.md."""
|
||||
repo = _make_repo()
|
||||
|
||||
call_order: list[str] = []
|
||||
|
||||
async def fake_migrate_attackers():
|
||||
call_order.append("migrate_attackers")
|
||||
|
||||
async def fake_migrate_column_types():
|
||||
call_order.append("migrate_column_types")
|
||||
|
||||
async def fake_ensure_admin():
|
||||
call_order.append("ensure_admin")
|
||||
|
||||
repo._migrate_attackers_table = fake_migrate_attackers
|
||||
repo._migrate_column_types = fake_migrate_column_types
|
||||
repo._ensure_admin_user = fake_ensure_admin
|
||||
|
||||
# Stub engine.begin() so create_all is a no-op
|
||||
fake_conn = AsyncMock()
|
||||
fake_conn.run_sync = AsyncMock()
|
||||
fake_ctx = AsyncMock()
|
||||
fake_ctx.__aenter__ = AsyncMock(return_value=fake_conn)
|
||||
fake_ctx.__aexit__ = AsyncMock(return_value=False)
|
||||
repo.engine = MagicMock()
|
||||
repo.engine.begin.return_value = fake_ctx
|
||||
|
||||
await repo.initialize()
|
||||
|
||||
assert call_order == [
|
||||
"migrate_attackers",
|
||||
"migrate_column_types",
|
||||
"ensure_admin",
|
||||
], f"Unexpected call order: {call_order}"
|
||||
79
tests/db/test_alembic_migrations.py
Normal file
79
tests/db/test_alembic_migrations.py
Normal file
@@ -0,0 +1,79 @@
|
||||
# SPDX-License-Identifier: AGPL-3.0-or-later
|
||||
"""Alembic wiring guards.
|
||||
|
||||
These pin the two halves of SQLModelRepository._apply_schema:
|
||||
* real boots run `alembic upgrade head` (schema owned by migration history),
|
||||
* tests (DECNET_TESTING=1) take the faster create_all path.
|
||||
|
||||
The first test also doubles as a drift guard: if someone adds a model table
|
||||
but forgets to autogenerate a migration, `alembic upgrade head` won't create
|
||||
it and this fails.
|
||||
"""
|
||||
from __future__ import annotations
|
||||
|
||||
import sqlite3
|
||||
|
||||
from sqlalchemy import text
|
||||
from sqlalchemy.ext.asyncio import create_async_engine
|
||||
from sqlmodel import SQLModel
|
||||
|
||||
import decnet.web.db.models # noqa: F401 (registers every table on metadata)
|
||||
from decnet.web.db.migrate import run_migrations
|
||||
from decnet.web.db.sqlite.repository import SQLiteRepository
|
||||
|
||||
|
||||
def _table_names(db_path: str) -> set[str]:
|
||||
con = sqlite3.connect(db_path)
|
||||
try:
|
||||
rows = con.execute(
|
||||
"SELECT name FROM sqlite_master WHERE type='table'"
|
||||
).fetchall()
|
||||
finally:
|
||||
con.close()
|
||||
return {r[0] for r in rows}
|
||||
|
||||
|
||||
async def test_migrations_create_every_model_table(tmp_path):
|
||||
"""`alembic upgrade head` must materialise every SQLModel table —
|
||||
catches a model added without a corresponding migration."""
|
||||
db_path = str(tmp_path / "mig.db")
|
||||
engine = create_async_engine(f"sqlite+aiosqlite:///{db_path}")
|
||||
try:
|
||||
await run_migrations(engine)
|
||||
finally:
|
||||
await engine.dispose()
|
||||
|
||||
created = _table_names(db_path)
|
||||
expected = set(SQLModel.metadata.tables)
|
||||
missing = expected - created
|
||||
assert not missing, f"migration head is missing tables: {sorted(missing)}"
|
||||
assert "alembic_version" in created
|
||||
|
||||
|
||||
async def test_real_boot_runs_alembic(tmp_path, monkeypatch):
|
||||
"""With DECNET_TESTING unset, initialize() runs migrations and stamps
|
||||
the alembic_version table."""
|
||||
monkeypatch.delenv("DECNET_TESTING", raising=False)
|
||||
repo = SQLiteRepository(db_path=str(tmp_path / "boot.db"))
|
||||
try:
|
||||
await repo._apply_schema()
|
||||
async with repo.engine.begin() as conn:
|
||||
ver = (await conn.execute(text("SELECT version_num FROM alembic_version"))).fetchall()
|
||||
finally:
|
||||
await repo.engine.dispose()
|
||||
assert ver, "alembic_version not stamped — migrations did not run"
|
||||
|
||||
|
||||
async def test_testing_mode_uses_create_all(tmp_path, monkeypatch):
|
||||
"""Under DECNET_TESTING=1 the schema comes from create_all, so there is
|
||||
no alembic_version table (Alembic was skipped)."""
|
||||
monkeypatch.setenv("DECNET_TESTING", "1")
|
||||
db_path = str(tmp_path / "test.db")
|
||||
repo = SQLiteRepository(db_path=db_path)
|
||||
try:
|
||||
await repo._apply_schema()
|
||||
finally:
|
||||
await repo.engine.dispose()
|
||||
tables = _table_names(db_path)
|
||||
assert "attackers" in tables # schema was created…
|
||||
assert "alembic_version" not in tables # …but not via Alembic
|
||||
Reference in New Issue
Block a user