015_api_key_hash idempotente: DB da create_all e 015 a metà su MySQL (#71)

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

src/backend/migrations/versions/015_api_key_hash.py

@@ -22,24 +22,44 @@ depends_on: Union[str, Sequence[str], None] = None
 
 
 def upgrade() -> None:
-    op.create_table(
-        'api_keys',
-        sa.Column('id', sa.Integer(), primary_key=True, autoincrement=True),
-        sa.Column('user_id', sa.Integer(), nullable=False),
-        sa.Column('key_hash', sa.String(64), nullable=False),
-        sa.Column('created_at', sa.DateTime(), nullable=False),
-        sa.Column('expires_at', sa.DateTime(), nullable=False),
-        sa.Column('last_used_at', sa.DateTime(), nullable=True),
-        sa.ForeignKeyConstraint(
-            ['user_id'], ['users.id'], name='fk_api_keys_user_id', ondelete='CASCADE'
-        ),
-    )
-    op.create_index('ix_api_keys_key_hash', 'api_keys', ['key_hash'], unique=True)
-    op.create_index('ix_api_keys_user_id', 'api_keys', ['user_id'])
+    # Idempotent on purpose (#71): a database born from create_all has no
+    # `uq_users_api_key` (only the unique index `ix_users_api_key`), and on
+    # MySQL a run that dies half way (no transactional DDL) leaves `api_keys`
+    # created and `alembic_version` still at 014. Every step checks first.
+    insp = sa.inspect(op.get_bind())
 
+    if 'api_keys' not in insp.get_table_names():
+        op.create_table(
+            'api_keys',
+            sa.Column('id', sa.Integer(), primary_key=True, autoincrement=True),
+            sa.Column('user_id', sa.Integer(), nullable=False),
+            sa.Column('key_hash', sa.String(64), nullable=False),
+            sa.Column('created_at', sa.DateTime(), nullable=False),
+            sa.Column('expires_at', sa.DateTime(), nullable=False),
+            sa.Column('last_used_at', sa.DateTime(), nullable=True),
+            sa.ForeignKeyConstraint(
+                ['user_id'], ['users.id'], name='fk_api_keys_user_id', ondelete='CASCADE'
+            ),
+        )
+        existing_idx: set[str] = set()
+    else:
+        existing_idx = {i['name'] for i in insp.get_indexes('api_keys')}
+
+    if 'ix_api_keys_key_hash' not in existing_idx:
+        op.create_index('ix_api_keys_key_hash', 'api_keys', ['key_hash'], unique=True)
+    if 'ix_api_keys_user_id' not in existing_idx:
+        op.create_index('ix_api_keys_user_id', 'api_keys', ['user_id'])
+
+    if 'api_key' not in {c['name'] for c in insp.get_columns('users')}:
+        return  # column already gone: nothing else to do
+
+    user_idx = {i['name'] for i in insp.get_indexes('users')}
+    user_uq = {u['name'] for u in insp.get_unique_constraints('users')}
     with op.batch_alter_table('users') as batch:
-        batch.drop_index('ix_users_api_key')
-        batch.drop_constraint('uq_users_api_key', type_='unique')
+        if 'ix_users_api_key' in user_idx:
+            batch.drop_index('ix_users_api_key')
+        if 'uq_users_api_key' in user_uq:
+            batch.drop_constraint('uq_users_api_key', type_='unique')
         batch.drop_column('api_key')
 
 

src/backend/tests/test_migrations.py

@@ -13,7 +13,10 @@ from pathlib import Path
 
 import pytest
 from alembic import command
+from alembic.operations import Operations
+from alembic.runtime.migration import MigrationContext
 from alembic.config import Config
+import sqlalchemy as sa
 from sqlalchemy import create_engine, inspect as sa_inspect, text
 
 from src.backend.config import Settings
@@ -132,6 +135,70 @@ class TestApiKeyHashMigration:
         command.upgrade(cfg, "head")  # and up again
         assert "api_keys" in _inspect(db_path).get_table_names()
 
+    def test_015_on_create_all_schema(self, sqlite_database_url):
+        """DB nato da create_all: users.api_key ha solo l'indice unique
+        ix_users_api_key, nessun vincolo uq_users_api_key (#71)."""
+        db_path, _ = sqlite_database_url
+        cfg = _alembic_config()
+        command.upgrade(cfg, "014_vision_fk_ondelete")
+
+        engine = create_engine(f"sqlite:///{db_path}")
+        with engine.begin() as conn:
+            ctx = MigrationContext.configure(conn)
+            ops = Operations(ctx)
+            with ops.batch_alter_table("users") as batch:
+                batch.drop_index("ix_users_api_key")
+                batch.drop_constraint("uq_users_api_key", type_="unique")
+            ops.create_index("ix_users_api_key", "users", ["api_key"], unique=True)
+        engine.dispose()
+
+        insp = _inspect(db_path)
+        assert not insp.get_unique_constraints("users") or "uq_users_api_key" not in {
+            u["name"] for u in insp.get_unique_constraints("users")
+        }
+
+        command.upgrade(cfg, "head")  # non deve sollevare
+
+        insp = _inspect(db_path)
+        assert "api_keys" in insp.get_table_names()
+        assert "api_key" not in {c["name"] for c in insp.get_columns("users")}
+
+    def test_015_resumes_after_partial_run(self, sqlite_database_url):
+        """015 a meta' (MySQL, DDL non transazionale): api_keys creata e
+        ix_users_api_key tolto, alembic_version ancora 014 (#71)."""
+        db_path, _ = sqlite_database_url
+        cfg = _alembic_config()
+        command.upgrade(cfg, "014_vision_fk_ondelete")
+
+        engine = create_engine(f"sqlite:///{db_path}")
+        with engine.begin() as conn:
+            ops = Operations(MigrationContext.configure(conn))
+            ops.create_table(
+                "api_keys",
+                sa.Column("id", sa.Integer(), primary_key=True, autoincrement=True),
+                sa.Column("user_id", sa.Integer(), nullable=False),
+                sa.Column("key_hash", sa.String(64), nullable=False),
+                sa.Column("created_at", sa.DateTime(), nullable=False),
+                sa.Column("expires_at", sa.DateTime(), nullable=False),
+                sa.Column("last_used_at", sa.DateTime(), nullable=True),
+                sa.ForeignKeyConstraint(
+                    ["user_id"], ["users.id"],
+                    name="fk_api_keys_user_id", ondelete="CASCADE",
+                ),
+            )
+            ops.create_index("ix_api_keys_key_hash", "api_keys", ["key_hash"], unique=True)
+            ops.drop_index("ix_users_api_key", table_name="users")
+        engine.dispose()
+
+        command.upgrade(cfg, "head")  # non deve sollevare
+
+        insp = _inspect(db_path)
+        assert "api_keys" in insp.get_table_names()
+        assert {i["name"] for i in insp.get_indexes("api_keys")} >= {
+            "ix_api_keys_key_hash", "ix_api_keys_user_id",
+        }
+        assert "api_key" not in {c["name"] for c in insp.get_columns("users")}
+
 
 class TestVisionRecipeDirMigration:
     """016_vision_recipe_dir: recipe_tasks.vision_recipe_dir, su e giu (#46)."""