fix(migrations): schema base 000 per DB vuoti, stamp head dopo create_all (#6)

upgrade head partiva dalla 001, che sa solo aggiungere colonne a tabelle
che assume gia' esistenti: su un DB davvero vuoto falliva subito. Nuova
000_base_schema.py (revision "000_base", down_revision None) crea lo
schema pre-001 esplicitamente (create_table, non Base.metadata.create_all,
che porterebbe gia' le colonne di 001-013 e farebbe fallire 001 su
"Duplicate column"). No-op se la tabella recipes esiste gia'
(sa.inspect(bind).has_table("recipes")). 001 ora ha down_revision =
"000_base".

setup.py: init-db e reset-db, dopo create_all/drop_all+create_all,
eseguono l'equivalente di "alembic stamp head" (nuovo _stamp_head(),
command.stamp con la config di src/backend/migrations/alembic.ini) cosi'
un DB nato da create_all non ritenta le migrazioni 001-013 al prossimo
deploy.

Test: catena upgrade head su SQLite vuoto, tabelle/colonne confrontate
con Base.metadata; DB con recipes gia' nello stato pre-001 -> upgrade
head non fallisce (000 no-op). _stamp_head si appoggia a
migrations/env.py, che costruisce una connessione propria da
settings.database_url (non modificabile dai test tramite l'engine
patchato di setup.py): nei test di setup.py e' quindi no-op via
monkeypatch, cosi' init-db/reset-db continuano a girare su SQLite senza
toccare un MySQL vero.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
2026-09-21 22:49:38 +02:00
parent 805f41c757
commit e20299548f
6 changed files with 388 additions and 2 deletions
+19 -1
View File
@@ -9,6 +9,8 @@ import secrets
from datetime import datetime, timedelta from datetime import datetime, timedelta
from pathlib import Path from pathlib import Path
from alembic import command
from alembic.config import Config
from fastapi import APIRouter, HTTPException, Request, status from fastapi import APIRouter, HTTPException, Request, status
from fastapi.responses import HTMLResponse from fastapi.responses import HTMLResponse
from fastapi.templating import Jinja2Templates from fastapi.templating import Jinja2Templates
@@ -28,10 +30,24 @@ from src.backend.services.measurement_service import calculate_pass_fail
router = APIRouter(prefix="/api/setup", tags=["setup"]) router = APIRouter(prefix="/api/setup", tags=["setup"])
_templates_dir = Path(__file__).resolve().parent.parent.parent / "templates" _backend_dir = Path(__file__).resolve().parent.parent.parent
_templates_dir = _backend_dir / "templates"
_alembic_ini = _backend_dir / "migrations" / "alembic.ini"
templates = Jinja2Templates(directory=str(_templates_dir)) templates = Jinja2Templates(directory=str(_templates_dir))
def _stamp_head() -> None:
"""Mark the schema as fully migrated - the equivalent of `alembic stamp
head` - after `Base.metadata.create_all()`.
A database that got its tables this way instead of through
`upgrade head` has no row in `alembic_version`; without this, its next
deploy would try to replay migrations 001-013 and fail on a column that
is already there (see migration 000_base_schema.py).
"""
command.stamp(Config(str(_alembic_ini)), "head")
# --------------------------------------------------------------------------- # ---------------------------------------------------------------------------
# Request schemas # Request schemas
# --------------------------------------------------------------------------- # ---------------------------------------------------------------------------
@@ -296,6 +312,7 @@ async def init_db(body: PasswordBody):
await _require_not_initialised() await _require_not_initialised()
async with engine.begin() as conn: async with engine.begin() as conn:
await conn.run_sync(Base.metadata.create_all) await conn.run_sync(Base.metadata.create_all)
_stamp_head()
return {"status": "ok", "message": "Database tables created successfully"} return {"status": "ok", "message": "Database tables created successfully"}
@@ -734,5 +751,6 @@ async def reset_db(body: ResetDbBody):
async with engine.begin() as conn: async with engine.begin() as conn:
await conn.run_sync(Base.metadata.drop_all) await conn.run_sync(Base.metadata.drop_all)
await conn.run_sync(Base.metadata.create_all) await conn.run_sync(Base.metadata.create_all)
_stamp_head()
return {"status": "ok", "message": "Database reset successfully"} return {"status": "ok", "message": "Database reset successfully"}
@@ -0,0 +1,251 @@
"""base schema: state of the database immediately before migration 001
Every database that has run this project since before Alembic was wired in
was born from `Base.metadata.create_all()` (via `/api/setup/init-db`, or
`init_db()` in dev) - never from a migration chain, because there wasn't one.
`upgrade head` on a genuinely empty database therefore had no starting point:
001 is the oldest migration, and it only knows how to ADD columns to tables
that 001-013 all assume already exist.
This migration supplies that starting point, but only when there is nothing
there yet: on a database that already has `recipes` (the table 001 was
written against), it is a no-op, because that database was already born from
`create_all()` and already has this exact schema.
Written explicitly with `create_table` rather than `Base.metadata.create_all`:
`create_all` builds the CURRENT model definitions, which already carry every
column 001-013 add, and 001 would then try to add a column that is already
there (`Duplicate column`).
Revision ID: 000_base
Revises:
Create Date: 2026-09-21
"""
from typing import Sequence, Union
from alembic import op
import sqlalchemy as sa
from sqlalchemy import inspect as sa_inspect
# revision identifiers, used by Alembic.
revision: str = '000_base'
down_revision: Union[str, None] = None
branch_labels: Union[str, Sequence[str], None] = None
depends_on: Union[str, Sequence[str], None] = None
# Table creation order matters: each one only references tables created
# before it (users -> recipes -> recipe_versions -> recipe_tasks ->
# recipe_subtasks -> measurements -> access_logs / system_settings /
# recipe_version_audit). Downgrade drops them in the opposite order.
_TABLES_NEWEST_FIRST = (
'recipe_version_audit', 'system_settings', 'access_logs', 'measurements',
'recipe_subtasks', 'recipe_tasks', 'recipe_versions', 'recipes', 'users',
)
def upgrade() -> None:
bind = op.get_bind()
if sa_inspect(bind).has_table('recipes'):
# Already created (by create_all, or by a previous run of this same
# migration) - nothing to do.
return
op.create_table(
'users',
sa.Column('id', sa.Integer, primary_key=True, autoincrement=True),
sa.Column('username', sa.String(100), nullable=False),
sa.Column('password_hash', sa.String(255), nullable=False),
sa.Column('email', sa.String(255), nullable=True),
sa.Column('display_name', sa.String(255), nullable=False),
sa.Column('roles', sa.JSON, nullable=False),
sa.Column('is_admin', sa.Boolean, nullable=False, server_default='0'),
sa.Column(
'language_pref', sa.Enum('it', 'en', name='language_enum'),
nullable=False, server_default='it',
),
sa.Column(
'theme_pref', sa.Enum('light', 'dark', name='theme_enum'),
nullable=False, server_default='light',
),
sa.Column('active', sa.Boolean, nullable=False, server_default='1'),
sa.Column('api_key', sa.String(64), nullable=True),
sa.Column('created_at', sa.DateTime, nullable=False, server_default=sa.func.now()),
sa.Column('last_login', sa.DateTime, nullable=True),
sa.UniqueConstraint('username', name='uq_users_username'),
sa.UniqueConstraint('api_key', name='uq_users_api_key'),
sa.Index('ix_users_username', 'username'),
sa.Index('ix_users_api_key', 'api_key'),
mysql_engine='InnoDB',
mysql_charset='utf8mb4',
)
op.create_table(
'recipes',
sa.Column('id', sa.Integer, primary_key=True, autoincrement=True),
sa.Column('code', sa.String(100), nullable=False),
sa.Column('name', sa.String(255), nullable=False),
sa.Column('description', sa.Text, nullable=True),
sa.Column('created_by', sa.Integer, sa.ForeignKey('users.id'), nullable=False),
sa.Column('created_at', sa.DateTime, nullable=False, server_default=sa.func.now()),
sa.Column('active', sa.Boolean, nullable=False, server_default='1'),
sa.UniqueConstraint('code', name='uq_recipes_code'),
sa.Index('ix_recipes_code', 'code'),
sa.Index('ix_recipes_active', 'active'),
mysql_engine='InnoDB',
mysql_charset='utf8mb4',
)
op.create_table(
'recipe_versions',
sa.Column('id', sa.Integer, primary_key=True, autoincrement=True),
sa.Column('recipe_id', sa.Integer, sa.ForeignKey('recipes.id'), nullable=False),
sa.Column('version_number', sa.Integer, nullable=False),
sa.Column('is_current', sa.Boolean, nullable=False, server_default='0'),
sa.Column('created_by', sa.Integer, sa.ForeignKey('users.id'), nullable=False),
sa.Column('created_at', sa.DateTime, nullable=False, server_default=sa.func.now()),
sa.Column('change_notes', sa.Text, nullable=True),
sa.UniqueConstraint('recipe_id', 'version_number', name='uq_recipe_version'),
sa.Index('ix_recipe_version_current', 'recipe_id', 'is_current'),
mysql_engine='InnoDB',
mysql_charset='utf8mb4',
)
op.create_table(
'recipe_tasks',
sa.Column('id', sa.Integer, primary_key=True, autoincrement=True),
sa.Column('version_id', sa.Integer, sa.ForeignKey('recipe_versions.id'), nullable=False),
sa.Column('order_index', sa.Integer, nullable=False, server_default='0'),
sa.Column('title', sa.String(255), nullable=False),
sa.Column('directive', sa.Text, nullable=True),
sa.Column('description', sa.Text, nullable=True),
sa.Column('file_path', sa.String(500), nullable=True),
sa.Column(
'file_type', sa.Enum('image', 'pdf', name='file_type_enum'),
nullable=True,
),
sa.Column('annotations_json', sa.JSON, nullable=True),
mysql_engine='InnoDB',
mysql_charset='utf8mb4',
)
op.create_table(
'recipe_subtasks',
sa.Column('id', sa.Integer, primary_key=True, autoincrement=True),
sa.Column(
'task_id', sa.Integer,
sa.ForeignKey('recipe_tasks.id', ondelete='CASCADE'), nullable=False,
),
sa.Column('marker_number', sa.Integer, nullable=False),
sa.Column('description', sa.String(500), nullable=False),
sa.Column('measurement_type', sa.String(100), nullable=True),
sa.Column('nominal', sa.DECIMAL(12, 6), nullable=True),
sa.Column('utl', sa.DECIMAL(12, 6), nullable=True),
sa.Column('uwl', sa.DECIMAL(12, 6), nullable=True),
sa.Column('lwl', sa.DECIMAL(12, 6), nullable=True),
sa.Column('ltl', sa.DECIMAL(12, 6), nullable=True),
sa.Column('unit', sa.String(20), nullable=False, server_default='mm'),
sa.UniqueConstraint('task_id', 'marker_number', name='uq_task_marker'),
sa.Index('ix_subtask_task_id', 'task_id'),
mysql_engine='InnoDB',
mysql_charset='utf8mb4',
)
op.create_table(
'measurements',
sa.Column('id', sa.Integer, primary_key=True, autoincrement=True),
sa.Column(
'subtask_id', sa.Integer, sa.ForeignKey('recipe_subtasks.id'),
nullable=False,
),
sa.Column(
'version_id', sa.Integer, sa.ForeignKey('recipe_versions.id'),
nullable=False,
),
sa.Column(
'measured_by', sa.Integer, sa.ForeignKey('users.id'), nullable=False,
),
sa.Column('value', sa.DECIMAL(12, 6), nullable=False),
sa.Column(
'pass_fail', sa.Enum('pass', 'warning', 'fail', name='pass_fail_enum'),
nullable=False,
),
sa.Column('deviation', sa.DECIMAL(12, 6), nullable=True),
sa.Column('lot_number', sa.String(100), nullable=True),
sa.Column('serial_number', sa.String(100), nullable=True),
sa.Column(
'input_method',
sa.Enum('usb_caliper', 'manual', name='input_method_enum'),
nullable=False, server_default='manual',
),
sa.Column(
'measured_at', sa.DateTime, nullable=False, server_default=sa.func.now(),
),
sa.Column('synced_to_csv', sa.Boolean, nullable=True, server_default='0'),
sa.Index('ix_measurements_subtask_id', 'subtask_id'),
sa.Index('ix_measurements_version_id', 'version_id'),
sa.Index('ix_measurements_measured_by', 'measured_by'),
sa.Index('ix_measurements_lot_number', 'lot_number'),
sa.Index('ix_measurements_serial_number', 'serial_number'),
sa.Index('ix_measurements_measured_at', 'measured_at'),
mysql_engine='InnoDB',
mysql_charset='utf8mb4',
)
op.create_table(
'access_logs',
sa.Column('id', sa.BigInteger, primary_key=True, autoincrement=True),
sa.Column('user_id', sa.Integer, sa.ForeignKey('users.id'), nullable=True),
sa.Column('action', sa.String(100), nullable=False),
sa.Column('details', sa.JSON, nullable=True),
sa.Column('ip_address', sa.String(45), nullable=True),
sa.Column('user_agent', sa.String(500), nullable=True),
sa.Column('created_at', sa.DateTime, nullable=False, server_default=sa.func.now()),
sa.Index('ix_access_logs_action', 'action'),
sa.Index('ix_access_log_user_time', 'user_id', 'created_at'),
mysql_engine='InnoDB',
mysql_charset='utf8mb4',
)
op.create_table(
'system_settings',
sa.Column('setting_key', sa.String(100), primary_key=True),
sa.Column('setting_value', sa.Text, nullable=False),
sa.Column(
'setting_type',
sa.Enum('string', 'number', 'boolean', 'json', name='setting_type_enum'),
nullable=False, server_default='string',
),
sa.Column('description', sa.String(500), nullable=True),
sa.Column('updated_at', sa.DateTime, nullable=False, server_default=sa.func.now()),
sa.Column('updated_by', sa.Integer, sa.ForeignKey('users.id'), nullable=True),
mysql_engine='InnoDB',
mysql_charset='utf8mb4',
)
op.create_table(
'recipe_version_audit',
sa.Column('id', sa.Integer, primary_key=True, autoincrement=True),
sa.Column('recipe_id', sa.Integer, sa.ForeignKey('recipes.id'), nullable=False),
sa.Column('old_version_id', sa.Integer, nullable=True),
sa.Column('new_version_id', sa.Integer, nullable=False),
sa.Column('changed_by', sa.Integer, sa.ForeignKey('users.id'), nullable=False),
sa.Column(
'change_type',
sa.Enum('CREATE', 'UPDATE', 'ACTIVATE', 'RETIRE', name='change_type_enum'),
nullable=False,
),
sa.Column('change_reason', sa.Text, nullable=True),
sa.Column('created_at', sa.DateTime, nullable=False, server_default=sa.func.now()),
sa.Index('ix_recipe_audit_recipe_time', 'recipe_id', 'created_at'),
mysql_engine='InnoDB',
mysql_charset='utf8mb4',
)
def downgrade() -> None:
bind = op.get_bind()
inspector = sa_inspect(bind)
for table in _TABLES_NEWEST_FIRST:
if inspector.has_table(table):
op.drop_table(table)
@@ -12,7 +12,7 @@ import sqlalchemy as sa
# revision identifiers, used by Alembic. # revision identifiers, used by Alembic.
revision: str = '001_image_path' revision: str = '001_image_path'
down_revision: Union[str, None] = None down_revision: Union[str, None] = '000_base'
branch_labels: Union[str, Sequence[str], None] = None branch_labels: Union[str, Sequence[str], None] = None
depends_on: Union[str, Sequence[str], None] = None depends_on: Union[str, Sequence[str], None] = None
+109
View File
@@ -0,0 +1,109 @@
"""Tests for the Alembic migration chain, and specifically for
000_base_schema.py (issue #6): a genuinely empty database has no starting
point before it, since 001-013 all assume tables 001 was written against
already exist.
These run the real `alembic upgrade head` (via `alembic.command`), not the
async app engine: `migrations/env.py` builds its own engine from
`settings.database_url`, so `Settings.database_url` is patched at the class
level (a plain instance monkeypatch cannot override a `@property`) to point
at a throwaway SQLite file for the duration of each test.
"""
from pathlib import Path
import pytest
from alembic import command
from alembic.config import Config
from sqlalchemy import create_engine, inspect as sa_inspect, text
from src.backend.config import Settings
from src.backend.database import Base
_ALEMBIC_INI = (
Path(__file__).resolve().parents[1] / "migrations" / "alembic.ini"
)
@pytest.fixture
def sqlite_database_url(tmp_path, monkeypatch):
"""Point settings.database_url (and therefore migrations/env.py) at a
throwaway SQLite file, and return its async URL."""
db_path = tmp_path / "migration_test.db"
async_url = f"sqlite+aiosqlite:///{db_path}"
monkeypatch.setattr(Settings, "database_url", property(lambda self: async_url))
return db_path, async_url
def _alembic_config() -> Config:
return Config(str(_ALEMBIC_INI))
def _inspect(db_path: Path):
engine = create_engine(f"sqlite:///{db_path}")
try:
return sa_inspect(engine)
finally:
engine.dispose()
class TestUpgradeHeadFromEmpty:
"""Full chain on a genuinely empty SQLite database (#6)."""
def test_upgrade_head_succeeds_on_empty_database(self, sqlite_database_url):
db_path, _ = sqlite_database_url
command.upgrade(_alembic_config(), "head") # must not raise
insp = _inspect(db_path)
assert set(Base.metadata.tables) <= set(insp.get_table_names())
def test_tables_and_columns_match_base_metadata(self, sqlite_database_url):
"""Every table and column ORM models declare must exist after
`upgrade head` - the exact assertion the analysis asks for."""
db_path, _ = sqlite_database_url
command.upgrade(_alembic_config(), "head")
insp = _inspect(db_path)
db_tables = set(insp.get_table_names())
for table_name, table in Base.metadata.tables.items():
assert table_name in db_tables, f"missing table {table_name}"
db_columns = {c["name"] for c in insp.get_columns(table_name)}
model_columns = {c.name for c in table.columns}
missing = model_columns - db_columns
assert not missing, f"{table_name} missing columns: {missing}"
class TestBaseMigrationNoopsOnExistingSchema:
"""000_base_schema.py does nothing when `recipes` already exists (#6)."""
def test_upgrade_head_noops_000_when_pre_001_schema_already_there(
self, sqlite_database_url
):
db_path, _ = sqlite_database_url
cfg = _alembic_config()
# Build the pre-001 schema and record it as migrated.
command.upgrade(cfg, "000_base")
insp = _inspect(db_path)
assert "recipes" in insp.get_table_names()
assert "image_path" not in {
c["name"] for c in insp.get_columns("recipes")
}
# Erase the tracking: alembic now believes nothing has run, exactly
# like a database that was born from Base.metadata.create_all() and
# never stamped. Running 000's upgrade() again on a database that
# already has `recipes` must be a no-op, not a "table already
# exists" failure.
sync_engine = create_engine(f"sqlite:///{db_path}")
with sync_engine.begin() as conn:
conn.execute(text("DELETE FROM alembic_version"))
sync_engine.dispose()
command.upgrade(cfg, "head") # must not raise (no duplicate table)
insp = _inspect(db_path)
assert set(Base.metadata.tables) <= set(insp.get_table_names())
assert "image_path" in {
c["name"] for c in insp.get_columns("recipes")
}
+5
View File
@@ -23,6 +23,11 @@ def enable_setup(monkeypatch):
import src.backend.api.routers.setup as setup_mod import src.backend.api.routers.setup as setup_mod
monkeypatch.setattr(setup_mod, "engine", test_engine) monkeypatch.setattr(setup_mod, "engine", test_engine)
monkeypatch.setattr(setup_mod, "async_session_factory", TestSessionFactory) monkeypatch.setattr(setup_mod, "async_session_factory", TestSessionFactory)
# _stamp_head runs `alembic stamp head`, which builds its own connection
# from settings.database_url via migrations/env.py - the real MySQL URL,
# not the patched test engine above. No-op it here so init-db/reset-db
# tests never try to reach a MySQL server that does not exist in CI/dev.
monkeypatch.setattr(setup_mod, "_stamp_head", lambda: None)
async def _seed(client: AsyncClient) -> dict: async def _seed(client: AsyncClient) -> dict:
+3
View File
@@ -34,6 +34,9 @@ def enable_setup(monkeypatch):
import src.backend.api.routers.setup as setup_mod import src.backend.api.routers.setup as setup_mod
monkeypatch.setattr(setup_mod, "engine", test_engine) monkeypatch.setattr(setup_mod, "engine", test_engine)
monkeypatch.setattr(setup_mod, "async_session_factory", TestSessionFactory) monkeypatch.setattr(setup_mod, "async_session_factory", TestSessionFactory)
# See test_setup.py: _stamp_head would otherwise try to reach a real
# MySQL server via migrations/env.py.
monkeypatch.setattr(setup_mod, "_stamp_head", lambda: None)
async def _seed(client: AsyncClient) -> dict: async def _seed(client: AsyncClient) -> dict: