diff --git a/migrations.py b/migrations.py index 2c39507..9c38c55 100644 --- a/migrations.py +++ b/migrations.py @@ -34,33 +34,9 @@ Original migration sequence (Nov 2025): - m014: Removed legacy equity accounts (MemberEquity, RetainedEarnings) - m015: Converted entry_lines to single amount field - m016: Dropped journal_entries and entry_lines tables (Fava integration) - -IDEMPOTENCY CONTRACT: -Every statement here must be a silent no-op on re-run. The migration -version bump lands in the core LNbits DB (`dbversions`) while the DDL -lands in `ext_libra` — the two writes are not atomic. If the version -bump fails after the DDL commits, the whole migration re-runs on next -boot; a bare CREATE/ALTER/INSERT then crashes the extension until -manual dbversions surgery. """ -async def _alter_add_column_safe(db, sql: str) -> None: - """ALTER TABLE ADD COLUMN that swallows duplicate-column errors. - - Neither SQLite nor Postgres supports ADD COLUMN IF NOT EXISTS - portably, so re-runs are made no-ops by swallowing the error both - backends raise for an existing column. - """ - try: - await db.execute(sql) - except Exception as exc: - msg = str(exc).lower() - if "duplicate column" in msg or "already exists" in msg: - return - raise - - async def m001_initial(db): """ Initial Libra database schema (squashed from m001-m016). @@ -87,7 +63,7 @@ async def m001_initial(db): await db.execute( f""" - CREATE TABLE IF NOT EXISTS accounts ( + CREATE TABLE accounts ( id TEXT PRIMARY KEY, name TEXT NOT NULL UNIQUE, account_type TEXT NOT NULL, @@ -100,13 +76,13 @@ async def m001_initial(db): await db.execute( """ - CREATE INDEX IF NOT EXISTS idx_accounts_user_id ON accounts (user_id); + CREATE INDEX idx_accounts_user_id ON accounts (user_id); """ ) await db.execute( """ - CREATE INDEX IF NOT EXISTS idx_accounts_type ON accounts (account_type); + CREATE INDEX idx_accounts_type ON accounts (account_type); """ ) @@ -117,7 +93,7 @@ async def m001_initial(db): await db.execute( f""" - CREATE TABLE IF NOT EXISTS extension_settings ( + CREATE TABLE extension_settings ( id TEXT NOT NULL PRIMARY KEY, libra_wallet_id TEXT, fava_url TEXT NOT NULL DEFAULT 'http://localhost:3333', @@ -135,7 +111,7 @@ async def m001_initial(db): await db.execute( f""" - CREATE TABLE IF NOT EXISTS user_wallet_settings ( + CREATE TABLE user_wallet_settings ( id TEXT NOT NULL PRIMARY KEY, user_wallet_id TEXT, updated_at TIMESTAMP NOT NULL DEFAULT {db.timestamp_now} @@ -150,7 +126,7 @@ async def m001_initial(db): await db.execute( f""" - CREATE TABLE IF NOT EXISTS manual_payment_requests ( + CREATE TABLE manual_payment_requests ( id TEXT PRIMARY KEY, user_id TEXT NOT NULL, amount INTEGER NOT NULL, @@ -167,14 +143,14 @@ async def m001_initial(db): await db.execute( """ - CREATE INDEX IF NOT EXISTS idx_manual_payment_requests_user_id + CREATE INDEX idx_manual_payment_requests_user_id ON manual_payment_requests (user_id); """ ) await db.execute( """ - CREATE INDEX IF NOT EXISTS idx_manual_payment_requests_status + CREATE INDEX idx_manual_payment_requests_status ON manual_payment_requests (status); """ ) @@ -187,7 +163,7 @@ async def m001_initial(db): await db.execute( f""" - CREATE TABLE IF NOT EXISTS balance_assertions ( + CREATE TABLE balance_assertions ( id TEXT PRIMARY KEY, date TIMESTAMP NOT NULL, account_id TEXT NOT NULL, @@ -212,21 +188,21 @@ async def m001_initial(db): await db.execute( """ - CREATE INDEX IF NOT EXISTS idx_balance_assertions_account_id + CREATE INDEX idx_balance_assertions_account_id ON balance_assertions (account_id); """ ) await db.execute( """ - CREATE INDEX IF NOT EXISTS idx_balance_assertions_status + CREATE INDEX idx_balance_assertions_status ON balance_assertions (status); """ ) await db.execute( """ - CREATE INDEX IF NOT EXISTS idx_balance_assertions_date + CREATE INDEX idx_balance_assertions_date ON balance_assertions (date); """ ) @@ -240,7 +216,7 @@ async def m001_initial(db): await db.execute( f""" - CREATE TABLE IF NOT EXISTS user_equity_status ( + CREATE TABLE user_equity_status ( user_id TEXT PRIMARY KEY, is_equity_eligible BOOLEAN NOT NULL DEFAULT FALSE, equity_account_name TEXT, @@ -254,7 +230,7 @@ async def m001_initial(db): await db.execute( """ - CREATE INDEX IF NOT EXISTS idx_user_equity_status_eligible + CREATE INDEX idx_user_equity_status_eligible ON user_equity_status (is_equity_eligible) WHERE is_equity_eligible = TRUE; """ @@ -269,7 +245,7 @@ async def m001_initial(db): await db.execute( f""" - CREATE TABLE IF NOT EXISTS account_permissions ( + CREATE TABLE account_permissions ( id TEXT PRIMARY KEY, user_id TEXT NOT NULL, account_id TEXT NOT NULL, @@ -286,7 +262,7 @@ async def m001_initial(db): # Index for looking up permissions by user await db.execute( """ - CREATE INDEX IF NOT EXISTS idx_account_permissions_user_id + CREATE INDEX idx_account_permissions_user_id ON account_permissions (user_id); """ ) @@ -294,7 +270,7 @@ async def m001_initial(db): # Index for looking up permissions by account await db.execute( """ - CREATE INDEX IF NOT EXISTS idx_account_permissions_account_id + CREATE INDEX idx_account_permissions_account_id ON account_permissions (account_id); """ ) @@ -302,7 +278,7 @@ async def m001_initial(db): # Composite index for checking specific user+account permissions await db.execute( """ - CREATE INDEX IF NOT EXISTS idx_account_permissions_user_account + CREATE INDEX idx_account_permissions_user_account ON account_permissions (user_id, account_id); """ ) @@ -310,7 +286,7 @@ async def m001_initial(db): # Index for finding permissions by type await db.execute( """ - CREATE INDEX IF NOT EXISTS idx_account_permissions_type + CREATE INDEX idx_account_permissions_type ON account_permissions (permission_type); """ ) @@ -318,7 +294,7 @@ async def m001_initial(db): # Index for finding expired permissions await db.execute( """ - CREATE INDEX IF NOT EXISTS idx_account_permissions_expires + CREATE INDEX idx_account_permissions_expires ON account_permissions (expires_at) WHERE expires_at IS NOT NULL; """ @@ -344,7 +320,6 @@ async def m001_initial(db): f""" INSERT INTO accounts (id, name, account_type, description, created_at) VALUES (:id, :name, :type, :description, {db.timestamp_now}) - ON CONFLICT (name) DO NOTHING """, { "id": str(uuid.uuid4()), @@ -367,18 +342,17 @@ async def m002_add_account_is_active(db): Default: All existing accounts are marked as active (TRUE). """ - await _alter_add_column_safe( - db, + await db.execute( """ ALTER TABLE accounts ADD COLUMN is_active BOOLEAN NOT NULL DEFAULT TRUE - """, + """ ) # Create index for faster queries filtering by is_active await db.execute( """ - CREATE INDEX IF NOT EXISTS idx_accounts_is_active ON accounts (is_active) + CREATE INDEX idx_accounts_is_active ON accounts (is_active) """ ) @@ -398,18 +372,17 @@ async def m003_add_account_is_virtual(db): Default: All existing accounts are real (is_virtual = FALSE). """ - await _alter_add_column_safe( - db, + await db.execute( """ ALTER TABLE accounts ADD COLUMN is_virtual BOOLEAN NOT NULL DEFAULT FALSE - """, + """ ) # Create index for faster queries filtering by is_virtual await db.execute( """ - CREATE INDEX IF NOT EXISTS idx_accounts_is_virtual ON accounts (is_virtual) + CREATE INDEX idx_accounts_is_virtual ON accounts (is_virtual) """ ) @@ -429,7 +402,6 @@ async def m003_add_account_is_virtual(db): f""" INSERT INTO accounts (id, name, account_type, description, is_active, is_virtual, created_at) VALUES (:id, :name, :type, :description, TRUE, TRUE, {db.timestamp_now}) - ON CONFLICT (name) DO NOTHING """, { "id": str(uuid.uuid4()), @@ -466,7 +438,7 @@ async def m004_add_rbac_tables(db): await db.execute( f""" - CREATE TABLE IF NOT EXISTS roles ( + CREATE TABLE roles ( id TEXT PRIMARY KEY, name TEXT NOT NULL UNIQUE, description TEXT, @@ -479,13 +451,13 @@ async def m004_add_rbac_tables(db): await db.execute( """ - CREATE INDEX IF NOT EXISTS idx_roles_name ON roles (name); + CREATE INDEX idx_roles_name ON roles (name); """ ) await db.execute( """ - CREATE INDEX IF NOT EXISTS idx_roles_is_default ON roles (is_default) + CREATE INDEX idx_roles_is_default ON roles (is_default) WHERE is_default = TRUE; """ ) @@ -497,7 +469,7 @@ async def m004_add_rbac_tables(db): await db.execute( f""" - CREATE TABLE IF NOT EXISTS role_permissions ( + CREATE TABLE role_permissions ( id TEXT PRIMARY KEY, role_id TEXT NOT NULL, account_id TEXT NOT NULL, @@ -512,19 +484,19 @@ async def m004_add_rbac_tables(db): await db.execute( """ - CREATE INDEX IF NOT EXISTS idx_role_permissions_role_id ON role_permissions (role_id); + CREATE INDEX idx_role_permissions_role_id ON role_permissions (role_id); """ ) await db.execute( """ - CREATE INDEX IF NOT EXISTS idx_role_permissions_account_id ON role_permissions (account_id); + CREATE INDEX idx_role_permissions_account_id ON role_permissions (account_id); """ ) await db.execute( """ - CREATE INDEX IF NOT EXISTS idx_role_permissions_type ON role_permissions (permission_type); + CREATE INDEX idx_role_permissions_type ON role_permissions (permission_type); """ ) @@ -535,7 +507,7 @@ async def m004_add_rbac_tables(db): await db.execute( f""" - CREATE TABLE IF NOT EXISTS user_roles ( + CREATE TABLE user_roles ( id TEXT PRIMARY KEY, user_id TEXT NOT NULL, role_id TEXT NOT NULL, @@ -550,19 +522,19 @@ async def m004_add_rbac_tables(db): await db.execute( """ - CREATE INDEX IF NOT EXISTS idx_user_roles_user_id ON user_roles (user_id); + CREATE INDEX idx_user_roles_user_id ON user_roles (user_id); """ ) await db.execute( """ - CREATE INDEX IF NOT EXISTS idx_user_roles_role_id ON user_roles (role_id); + CREATE INDEX idx_user_roles_role_id ON user_roles (role_id); """ ) await db.execute( """ - CREATE INDEX IF NOT EXISTS idx_user_roles_expires ON user_roles (expires_at) + CREATE INDEX idx_user_roles_expires ON user_roles (expires_at) WHERE expires_at IS NOT NULL; """ ) @@ -570,7 +542,7 @@ async def m004_add_rbac_tables(db): # Composite index for checking specific user+role assignments await db.execute( """ - CREATE INDEX IF NOT EXISTS idx_user_roles_user_role ON user_roles (user_id, role_id); + CREATE INDEX idx_user_roles_user_role ON user_roles (user_id, role_id); """ ) @@ -614,7 +586,6 @@ async def m004_add_rbac_tables(db): f""" INSERT INTO roles (id, name, description, is_default, created_by, created_at) VALUES (:id, :name, :description, :is_default, :created_by, {db.timestamp_now}) - ON CONFLICT (name) DO NOTHING """, { "id": str(uuid.uuid4()), diff --git a/tests/test_migrations.py b/tests/test_migrations.py deleted file mode 100644 index b66b595..0000000 --- a/tests/test_migrations.py +++ /dev/null @@ -1,109 +0,0 @@ -"""Migration idempotency tests. - -The migration version bump lands in the core LNbits DB (`dbversions`) -while the DDL lands in `ext_libra` — the two writes are not atomic. If -the bump fails after the DDL commits, the whole migration re-runs on -next boot, so every statement must be a silent no-op on re-run instead -of crashing with `duplicate column` / `table exists` / UNIQUE -violations (which bricks the extension until manual dbversions -surgery). - -These tests run the full migration chain twice against a fresh SQLite -database — the second pass simulates the lost-version-bump re-run. -""" -import importlib -import re -from uuid import uuid4 - -import pytest - -from lnbits.db import Database - -pytestmark = pytest.mark.anyio - - -def _module(name: str): - """Import a libra submodule under whichever path the active LNbits layout - uses (default `lnbits.extensions.libra` or bare `libra`).""" - for prefix in ("lnbits.extensions.libra", "libra"): - try: - return importlib.import_module(f"{prefix}.{name}") - except ModuleNotFoundError: - continue - raise ModuleNotFoundError(f"libra.{name}: tried both import paths") - - -migrations = _module("migrations") - -# Same discovery as lnbits.core.helpers.run_migration: m### prefix, -# module definition order. -_MIGRATION_RE = re.compile(r"^m(\d\d\d)_") -MIGRATION_FUNCTIONS = [ - fn for name, fn in vars(migrations).items() if _MIGRATION_RE.match(name) -] - -# Tables the chain must leave behind — one probe row read per table -# proves both existence and queryability after a double run. -EXPECTED_TABLES = [ - "accounts", - "extension_settings", - "user_wallet_settings", - "manual_payment_requests", - "balance_assertions", - "user_equity_status", - "account_permissions", - "roles", - "role_permissions", - "user_roles", -] - - -async def _run_all_migrations(db: Database) -> None: - async with db.connect() as conn: - for migrate in MIGRATION_FUNCTIONS: - await migrate(conn) - - -async def _seed_counts(db: Database) -> dict: - async with db.connect() as conn: - accounts = await conn.fetchall("SELECT id, name FROM accounts") - roles = await conn.fetchall("SELECT id, name FROM roles") - return { - "account_names": sorted(r["name"] for r in accounts), - "account_ids": sorted(r["id"] for r in accounts), - "role_names": sorted(r["name"] for r in roles), - "role_ids": sorted(r["id"] for r in roles), - } - - -async def test_migrations_rerun_is_noop(): - """Full chain twice: second run must not raise and not re-seed.""" - db = Database(f"ext_libra_migtest_{uuid4().hex[:8]}") - - await _run_all_migrations(db) - first = await _seed_counts(db) - - # Simulate the lost dbversions bump: everything runs again. - await _run_all_migrations(db) - second = await _seed_counts(db) - - # Seeds must not duplicate (names) and must not be replaced (ids). - assert second == first - assert first["account_names"], "seed accounts missing after migration" - assert "Employee" in first["role_names"] - - # Every table exists and is queryable after the double run. - async with db.connect() as conn: - for table in EXPECTED_TABLES: - await conn.fetchall(f"SELECT * FROM {table} LIMIT 1") # noqa: S608 - - -async def test_single_migration_rerun_is_noop(): - """Each migration individually survives an immediate re-run (the - version bump fails right after that one migration committed).""" - db = Database(f"ext_libra_migtest_{uuid4().hex[:8]}") - - async with db.connect() as conn: - for migrate in MIGRATION_FUNCTIONS: - await migrate(conn) - await migrate(conn) # re-run before "bumping" to the next