From 74fe5665b57c771139d6dfd44a79aae2919c191b Mon Sep 17 00:00:00 2001 From: AdrianoDev Date: Thu, 15 Oct 2026 09:25:07 +0200 Subject: [PATCH] =?UTF-8?q?015=5Fapi=5Fkey=5Fhash=20idempotente:=20DB=20da?= =?UTF-8?q?=20create=5Fall=20e=20015=20a=20met=C3=A0=20su=20MySQL=20(#71)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Sonnet 5.5 --- .../migrations/versions/015_api_key_hash.py | 52 +++++++++----- src/backend/tests/test_migrations.py | 67 +++++++++++++++++++ 2 files changed, 103 insertions(+), 16 deletions(-) diff --git a/src/backend/migrations/versions/015_api_key_hash.py b/src/backend/migrations/versions/015_api_key_hash.py index 3ea52df..94a0b62 100644 --- a/src/backend/migrations/versions/015_api_key_hash.py +++ b/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') diff --git a/src/backend/tests/test_migrations.py b/src/backend/tests/test_migrations.py index 08a91ff..613e426 100644 --- a/src/backend/tests/test_migrations.py +++ b/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)."""