From 86c3edb021608e711c19211f15cbec39f107420e Mon Sep 17 00:00:00 2001 From: Wimpie Vermaak Date: Wed, 9 Sep 2026 09:14:08 +0200 Subject: [PATCH] fix(migration): strip the file extension when resolving a v2 description Python v2 recorded `description` straight from os.listdir, so the value carries ".sql"/".py". v3 compares against `mig_file.stem`, which never does. _resolve_migration_name tried three matches and none stripped the extension - strategy 3 tests `description in stem`, but here the stem is a substring of the description, not the reverse - so it fell through to the verbatim description and nothing ever matched. Every already-applied migration therefore looked unapplied and the whole history replayed on a v2 -> v3 upgrade. This hit Hertex app-portal on 2026-09-09: a 0.2.206 -> 3.13.52 upgrade replayed 90 applied migrations and crashlooped the app on the first non-idempotent one with a duplicate key. Each match strategy now runs against the description and its extension-stripped form. A description with no file on disk is still kept verbatim, so non-matching rows are unaffected. The existing suite missed this because every fixture seeded a bare stem ("000001_create_users"), which is not what v2 ever wrote. Four tests added, including a replay of the production failure against a non-idempotent migration so a regression fails loudly rather than passing on CREATE TABLE IF NOT EXISTS. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01YPVsva4pe4rXmQaCM1bfj5 --- tests/test_issue_115_v2_upgrade.py | 84 ++++++++++++++++++++++++++++++ tina4_python/migration/runner.py | 32 ++++++++---- 2 files changed, 107 insertions(+), 9 deletions(-) diff --git a/tests/test_issue_115_v2_upgrade.py b/tests/test_issue_115_v2_upgrade.py index f92d68f7..6fa99be9 100644 --- a/tests/test_issue_115_v2_upgrade.py +++ b/tests/test_issue_115_v2_upgrade.py @@ -389,3 +389,87 @@ def test_issue_93_fresh_v3_table_still_inserts_only_canonical_columns(db, mig_di cols = _column_names(db) assert "migration_id" not in cols, "a fresh v3 table must never grow the legacy column" assert db.fetch_one("SELECT migration_name FROM tina4_migration")["migration_name"] == "000001_smoke" + + +# ── v2 stored FILENAMES, not stems ────────────────────────────────── +# +# Python v2 (tina4_python/Migration.py, 0.2.x) recorded `description` straight +# from os.listdir, so the value carried the ".sql" extension: +# +# dba.execute("insert into tina4_migration (... description ...)", [next_id, file, ...]) +# +# Every test above seeds a bare stem ("000001_create_users"), which is not what +# v2 ever wrote. That gap is why the extension case survived this suite. +# +# _resolve_migration_name matched a stem against the description three ways and +# none of them strip the extension — strategy 3 tests `description in stem`, but +# here the stem is a substring of the description, not the reverse. So it fell +# through to the verbatim description, keeping ".sql", while migrate() compares +# against mig_file.stem without it. Nothing matched and the whole history +# replayed. +# +# This hit Hertex app-portal on 2026-09-09: 90 applied migrations replayed on a +# 0.2.206 -> 3.13.52 upgrade and the app crashlooped on the first non-idempotent +# one with a duplicate key. + +def test_v2_description_with_sql_extension_resolves_to_the_stem(db, mig_dir): + """A v2 row written as ".sql" must resolve to the on-disk stem.""" + (mig_dir / "0000002_data_migration_for_show_room.sql").write_text( + "CREATE TABLE never_created (id INTEGER);" + ) + _create_v2_table(db) + _insert_v2_row(db, "0000002_data_migration_for_show_room.sql") + + Migration(db, str(mig_dir)) + + name = db.fetch_one("SELECT migration_name FROM tina4_migration")["migration_name"] + assert name == "0000002_data_migration_for_show_room", ( + "the .sql extension must be stripped so migrate() can match mig_file.stem" + ) + + +def test_v2_filenames_do_not_replay_on_upgrade(db, mig_dir): + """The production case: an applied .sql-suffixed history must not re-run. + + Uses a non-idempotent migration, so a replay fails loudly instead of + silently succeeding the way a CREATE TABLE IF NOT EXISTS would. + """ + db.execute("CREATE TABLE checklist_item_group (group_name TEXT UNIQUE)") + db.execute("INSERT INTO checklist_item_group (group_name) VALUES ('window_&_entrance')") + db.commit() + + (mig_dir / "0000002_data_migration_for_show_room.sql").write_text( + "INSERT INTO checklist_item_group (group_name) VALUES ('window_&_entrance');" + ) + _create_v2_table(db) + _insert_v2_row(db, "0000002_data_migration_for_show_room.sql") + + ran = Migration(db, str(mig_dir)).migrate() + + assert ran == [], f"already-applied v2 migrations must not replay, got {ran}" + + +def test_v2_python_migration_filename_also_resolves(db, mig_dir): + """.py migrations were recorded the same way and need the same strip.""" + (mig_dir / "000003_seed_data.py").write_text("def up(db):\n pass\n") + _create_v2_table(db) + _insert_v2_row(db, "000003_seed_data.py") + + Migration(db, str(mig_dir)) + + name = db.fetch_one("SELECT migration_name FROM tina4_migration")["migration_name"] + assert name == "000003_seed_data" + + +def test_unrelated_description_still_falls_back_verbatim(db, mig_dir): + """Control: the strip must not invent a match where no file exists.""" + (mig_dir / "000001_real.sql").write_text("CREATE TABLE t_real (id INTEGER);") + _create_v2_table(db) + _insert_v2_row(db, "999_not_on_disk.sql") + + Migration(db, str(mig_dir)) + + name = db.fetch_one( + "SELECT migration_name FROM tina4_migration WHERE description = '999_not_on_disk.sql'" + )["migration_name"] + assert name == "999_not_on_disk.sql", "no file match means the description is kept verbatim" diff --git a/tina4_python/migration/runner.py b/tina4_python/migration/runner.py index dfb467c0..a991bf39 100644 --- a/tina4_python/migration/runner.py +++ b/tina4_python/migration/runner.py @@ -156,7 +156,13 @@ def _build_stem_map(migration_folder: str | None) -> list[str]: def _resolve_migration_name(description: str, stems: list[str]) -> str: """Find the file stem that corresponds to a v2 description, or fall back. - Match strategy, in order: + Python v2 recorded `description` straight from os.listdir, so the value + normally carries the file extension (".sql"/".py") while a stem never does. + Match on the extension-stripped form as well, or a whole v2 history looks + unapplied and replays — see tests/test_issue_115_v2_upgrade.py. + + Match strategy, in order (each tried against the description and against + its extension-stripped form): 1. Exact stem match. 2. A stem ending in "_" + description (e.g. "000001_create_users" resolves a v2 description of "create_users"). @@ -165,14 +171,22 @@ def _resolve_migration_name(description: str, stems: list[str]) -> str: getAppliedMigrations() but migrate() may re-run if a real file exists with a different name. """ - if description in stems: - return description - for stem in stems: - if stem.endswith("_" + description): - return stem - for stem in stems: - if description in stem: - return stem + candidates = [description] + bare = re.sub(r"\.(sql|py)$", "", description, flags=re.IGNORECASE) + if bare != description: + candidates.append(bare) + + for candidate in candidates: + if candidate in stems: + return candidate + for candidate in candidates: + for stem in stems: + if stem.endswith("_" + candidate): + return stem + for candidate in candidates: + for stem in stems: + if candidate in stem: + return stem return description