diff --git a/CHANGELOG.md b/CHANGELOG.md index 8a0c386..dde9f45 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,7 +6,12 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). ## [Unreleased] +### Fixed +- Empty env vars (e.g. `SECRET_KEY=`) now fall back to defaults instead of silently using `""` — fixes 500 on every request when `.env` has blank values + ### Added +- Comprehensive migration test suite (`tests/test_migrations.py` — 20 tests) covering fresh DB, existing DB, up-to-date DB, idempotent migration, version discovery, `_is_fresh_db`, migration 0001 correctness, and ordering +- Expanded `migrate.py` module docstring documenting the 8-step algorithm, protocol for adding migrations, and design decisions - Sequential migration system (`migrations/migrate.py`) — tracks applied versions in `_migrations` table, auto-detects fresh vs existing DBs, runs pending migrations in order - `migrations/versions/0001_rename_ls_to_paddle.py` — first versioned migration (absorbed from `scripts/migrate_to_paddle.py`) - Server-side financial calculator (`planner/calculator.py`) — ported JS `calc()`, `pmt()`, `calcIRR()` to Python so the full financial model is no longer exposed in client-side JavaScript diff --git a/padelnomics/src/padelnomics/core.py b/padelnomics/src/padelnomics/core.py index 9a02984..303ddb4 100644 --- a/padelnomics/src/padelnomics/core.py +++ b/padelnomics/src/padelnomics/core.py @@ -17,14 +17,20 @@ from quart import g, request, session load_dotenv() + +def _env(key: str, default: str) -> str: + """Get env var, treating empty string same as unset.""" + return os.getenv(key, "") or default + + # ============================================================================= # Configuration # ============================================================================= class Config: - APP_NAME: str = os.getenv("APP_NAME", "Padelnomics") - SECRET_KEY: str = os.getenv("SECRET_KEY", "change-me-in-production") - BASE_URL: str = os.getenv("BASE_URL", "http://localhost:5000") + APP_NAME: str = _env("APP_NAME", "Padelnomics") + SECRET_KEY: str = _env("SECRET_KEY", "change-me-in-production") + BASE_URL: str = _env("BASE_URL", "http://localhost:5000") DEBUG: bool = os.getenv("DEBUG", "false").lower() == "true" DATABASE_PATH: str = os.getenv("DATABASE_PATH", "data/app.db") @@ -42,8 +48,8 @@ class Config: } RESEND_API_KEY: str = os.getenv("RESEND_API_KEY", "") - EMAIL_FROM: str = os.getenv("EMAIL_FROM", "hello@padelnomics.io") - ADMIN_EMAIL: str = os.getenv("ADMIN_EMAIL", "leads@padelnomics.io") + EMAIL_FROM: str = _env("EMAIL_FROM", "hello@padelnomics.io") + ADMIN_EMAIL: str = _env("ADMIN_EMAIL", "leads@padelnomics.io") RATE_LIMIT_REQUESTS: int = int(os.getenv("RATE_LIMIT_REQUESTS", "100")) RATE_LIMIT_WINDOW: int = int(os.getenv("RATE_LIMIT_WINDOW", "60")) diff --git a/padelnomics/src/padelnomics/migrations/migrate.py b/padelnomics/src/padelnomics/migrations/migrate.py index fc72629..c064306 100644 --- a/padelnomics/src/padelnomics/migrations/migrate.py +++ b/padelnomics/src/padelnomics/migrations/migrate.py @@ -1,10 +1,47 @@ """ -Sequential migration runner. +Sequential migration runner for Padelnomics. -- Runs schema.sql (idempotent CREATE IF NOT EXISTS for fresh DBs) -- Scans versions/ for NNNN_*.py files and runs unapplied ones in order -- Fresh DBs: marks all versions as applied without running them - (schema.sql already contains the final schema) +Manages SQLite schema evolution with two paths: fresh databases get the +full schema from schema.sql in one shot; existing databases get incremental +migrations applied in order. + +Algorithm +--------- +1. Connect to the SQLite database (create file if missing). +2. Set WAL journal mode and enable foreign keys. +3. Check whether the DB is fresh (no application tables at all). +4. Execute schema.sql — all statements use CREATE IF NOT EXISTS, so this + is a no-op on existing databases that already have the tables. +5. Discover version files in versions/ matching NNNN_*.py. +6. Diff discovered versions against the _migrations tracking table. +7. Choose a path: + - **Fresh DB**: record every version as applied *without* executing it, + because schema.sql already contains the final schema state. + - **Existing DB with pending versions**: import each pending module and + call its ``up(conn)`` function, then record it in _migrations. + - **Up-to-date DB**: no-op, print confirmation. +8. Commit the transaction and print a summary with table names. + +Adding a new migration +---------------------- +1. Create ``versions/NNNN_description.py`` with a single ``up(conn)`` + function that receives an *uncommitted* ``sqlite3.Connection``. + The runner commits after all pending migrations succeed (batch + atomicity), so do NOT call ``conn.commit()`` inside ``up()``. +2. Update ``schema.sql`` so it reflects the final state *after* the + migration. Fresh databases must end up identical to migrated ones. +3. Keep both in sync — schema.sql is the single source of truth for + what a brand-new database looks like. + +Design decisions +---------------- +- **schema.sql as source of truth**: Fresh deploys get the full schema + instantly without replaying every historical migration. +- **Sync sqlite3, not aiosqlite**: Migrations run at startup *before* + the async event loop, so we use the stdlib sqlite3 module directly. +- **up(conn) receives an uncommitted connection**: All pending migrations + share a single transaction. If any migration fails, the entire batch + rolls back, leaving the DB in its previous consistent state. """ import importlib diff --git a/padelnomics/tests/test_migrations.py b/padelnomics/tests/test_migrations.py new file mode 100644 index 0000000..eff1e63 --- /dev/null +++ b/padelnomics/tests/test_migrations.py @@ -0,0 +1,376 @@ +""" +Tests for the sequential migration runner. + +Synchronous tests — migrate.py uses stdlib sqlite3, not aiosqlite. +Uses tmp_path for isolated DB files and monkeypatch for DATABASE_PATH. +""" + +import importlib +import re +import sqlite3 +from pathlib import Path +from unittest.mock import patch + +import pytest + +from padelnomics.migrations.migrate import _discover_versions, _is_fresh_db, migrate + +SCHEMA_PATH = ( + Path(__file__).parent.parent / "src" / "padelnomics" / "migrations" / "schema.sql" +) +VERSIONS_DIR = ( + Path(__file__).parent.parent / "src" / "padelnomics" / "migrations" / "versions" +) + +# ── Helpers ─────────────────────────────────────────────────── + + +def _old_schema_sql(): + """Return schema.sql with paddle columns swapped back to lemonsqueezy.""" + schema = SCHEMA_PATH.read_text() + schema = schema.replace("paddle_customer_id", "lemonsqueezy_customer_id") + schema = schema.replace("paddle_subscription_id", "lemonsqueezy_subscription_id") + return schema + + +def _table_names(conn): + """Return sorted list of user-visible table names.""" + rows = conn.execute( + "SELECT name FROM sqlite_master WHERE type='table'" + " AND name NOT LIKE 'sqlite_%' ORDER BY name" + ).fetchall() + return [r[0] for r in rows] + + +def _column_names(conn, table): + return [r[1] for r in conn.execute(f"PRAGMA table_info({table})").fetchall()] + + +# ── Fixtures ────────────────────────────────────────────────── + + +@pytest.fixture +def schema_sql(): + return SCHEMA_PATH.read_text() + + +@pytest.fixture +def fresh_db_path(tmp_path): + """Path to a non-existent DB file.""" + return str(tmp_path / "fresh.db") + + +@pytest.fixture +def existing_db(tmp_path): + """DB with old lemonsqueezy column names and no _migrations table.""" + db_path = str(tmp_path / "existing.db") + schema = _old_schema_sql() + # Remove the _migrations table DDL so this DB has no tracking + schema = re.sub( + r"CREATE TABLE IF NOT EXISTS _migrations\s*\([^)]+\);", + "", + schema, + ) + conn = sqlite3.connect(db_path) + conn.executescript(schema) + conn.commit() + conn.close() + return db_path + + +@pytest.fixture +def production_db(tmp_path, schema_sql): + """DB with current paddle columns but no _migrations records.""" + db_path = str(tmp_path / "production.db") + # Remove the _migrations DDL so it simulates manual migration + schema = re.sub( + r"CREATE TABLE IF NOT EXISTS _migrations\s*\([^)]+\);", + "", + schema_sql, + ) + conn = sqlite3.connect(db_path) + conn.executescript(schema) + conn.commit() + conn.close() + return db_path + + +@pytest.fixture +def up_to_date_db(tmp_path, schema_sql): + """DB with final schema and all migrations recorded.""" + db_path = str(tmp_path / "uptodate.db") + conn = sqlite3.connect(db_path) + conn.executescript(schema_sql) + # Record all discovered versions as applied + for f in sorted(VERSIONS_DIR.iterdir()): + if re.match(r"^\d{4}_.+\.py$", f.name): + conn.execute( + "INSERT INTO _migrations (name) VALUES (?)", (f.stem,) + ) + conn.commit() + conn.close() + return db_path + + +@pytest.fixture +def mock_versions_dir(tmp_path): + """Empty temp directory for version discovery tests.""" + d = tmp_path / "versions" + d.mkdir() + return d + + +# ── TestFreshDatabase ───────────────────────────────────────── + + +class TestFreshDatabase: + def test_creates_all_tables(self, fresh_db_path, monkeypatch): + monkeypatch.setenv("DATABASE_PATH", fresh_db_path) + migrate() + conn = sqlite3.connect(fresh_db_path) + tables = _table_names(conn) + conn.close() + assert "_migrations" in tables + assert "users" in tables + assert "subscriptions" in tables + assert "scenarios" in tables + + def test_records_all_versions_as_applied(self, fresh_db_path, monkeypatch): + monkeypatch.setenv("DATABASE_PATH", fresh_db_path) + migrate() + conn = sqlite3.connect(fresh_db_path) + applied = { + r[0] for r in conn.execute("SELECT name FROM _migrations").fetchall() + } + conn.close() + versions = _discover_versions() + assert applied == set(versions) + + def test_does_not_call_import_module(self, fresh_db_path, monkeypatch): + monkeypatch.setenv("DATABASE_PATH", fresh_db_path) + with patch("padelnomics.migrations.migrate.importlib.import_module") as mock_imp: + migrate() + mock_imp.assert_not_called() + + def test_uses_paddle_column_names(self, fresh_db_path, monkeypatch): + monkeypatch.setenv("DATABASE_PATH", fresh_db_path) + migrate() + conn = sqlite3.connect(fresh_db_path) + cols = _column_names(conn, "subscriptions") + conn.close() + assert "paddle_customer_id" in cols + assert "paddle_subscription_id" in cols + assert "lemonsqueezy_customer_id" not in cols + + +# ── TestExistingDatabase ────────────────────────────────────── + + +class TestExistingDatabase: + def test_applies_pending_migration(self, existing_db, monkeypatch): + monkeypatch.setenv("DATABASE_PATH", existing_db) + migrate() + conn = sqlite3.connect(existing_db) + cols = _column_names(conn, "subscriptions") + conn.close() + assert "paddle_customer_id" in cols + assert "paddle_subscription_id" in cols + assert "lemonsqueezy_customer_id" not in cols + + def test_records_migration_with_timestamp(self, existing_db, monkeypatch): + monkeypatch.setenv("DATABASE_PATH", existing_db) + migrate() + conn = sqlite3.connect(existing_db) + row = conn.execute( + "SELECT name, applied_at FROM _migrations WHERE name LIKE '0001%'" + ).fetchone() + conn.close() + assert row is not None + assert row[0] == "0001_rename_ls_to_paddle" + assert row[1] is not None # timestamp populated + + +# ── TestUpToDateDatabase ────────────────────────────────────── + + +class TestUpToDateDatabase: + def test_noop_when_all_applied(self, up_to_date_db, monkeypatch): + monkeypatch.setenv("DATABASE_PATH", up_to_date_db) + with patch("padelnomics.migrations.migrate.importlib.import_module") as mock_imp: + migrate() + mock_imp.assert_not_called() + + def test_no_duplicate_entries_on_rerun(self, up_to_date_db, monkeypatch): + monkeypatch.setenv("DATABASE_PATH", up_to_date_db) + migrate() + migrate() + conn = sqlite3.connect(up_to_date_db) + count = conn.execute("SELECT COUNT(*) FROM _migrations").fetchone()[0] + conn.close() + assert count == len(_discover_versions()) + + +# ── TestIdempotentMigration ─────────────────────────────────── + + +class TestIdempotentMigration: + def test_production_db_paddle_cols_already_exist( + self, production_db, monkeypatch + ): + """Production scenario: paddle columns exist, no _migrations table. + 0001 runs without error and gets recorded.""" + monkeypatch.setenv("DATABASE_PATH", production_db) + migrate() + conn = sqlite3.connect(production_db) + cols = _column_names(conn, "subscriptions") + applied = { + r[0] for r in conn.execute("SELECT name FROM _migrations").fetchall() + } + conn.close() + assert "paddle_customer_id" in cols + assert "0001_rename_ls_to_paddle" in applied + + +# ── TestDiscoverVersions ───────────────────────────────────── + + +class TestDiscoverVersions: + def test_finds_and_sorts_version_files(self): + versions = _discover_versions() + assert len(versions) >= 1 + assert versions[0] == "0001_rename_ls_to_paddle" + + def test_ignores_non_matching_files(self, mock_versions_dir, monkeypatch): + (mock_versions_dir / "__init__.py").write_text("") + (mock_versions_dir / "readme.txt").write_text("") + (mock_versions_dir / "0001_real.py").write_text("") + monkeypatch.setattr( + "padelnomics.migrations.migrate.VERSIONS_DIR", mock_versions_dir + ) + versions = _discover_versions() + assert versions == ["0001_real"] + + def test_returns_empty_for_missing_directory(self, tmp_path, monkeypatch): + monkeypatch.setattr( + "padelnomics.migrations.migrate.VERSIONS_DIR", + tmp_path / "nonexistent", + ) + assert _discover_versions() == [] + + +# ── TestIsFreshDb ───────────────────────────────────────────── + + +class TestIsFreshDb: + def test_empty_db_is_fresh(self, tmp_path): + conn = sqlite3.connect(str(tmp_path / "empty.db")) + assert _is_fresh_db(conn) is True + conn.close() + + def test_db_with_schema_is_not_fresh(self, tmp_path, schema_sql): + conn = sqlite3.connect(str(tmp_path / "full.db")) + conn.executescript(schema_sql) + assert _is_fresh_db(conn) is False + conn.close() + + def test_db_with_single_table_is_not_fresh(self, tmp_path): + conn = sqlite3.connect(str(tmp_path / "one.db")) + conn.execute("CREATE TABLE foo (id INTEGER PRIMARY KEY)") + assert _is_fresh_db(conn) is False + conn.close() + + +# ── TestMigration0001 ───────────────────────────────────────── + + +class TestMigration0001: + @pytest.fixture + def mod_0001(self): + return importlib.import_module( + "padelnomics.migrations.versions.0001_rename_ls_to_paddle" + ) + + def test_renames_columns(self, tmp_path, mod_0001): + conn = sqlite3.connect(str(tmp_path / "rename.db")) + conn.executescript(_old_schema_sql()) + mod_0001.up(conn) + cols = _column_names(conn, "subscriptions") + conn.close() + assert "paddle_customer_id" in cols + assert "paddle_subscription_id" in cols + assert "lemonsqueezy_customer_id" not in cols + + def test_idempotent_when_already_renamed(self, tmp_path, schema_sql, mod_0001): + conn = sqlite3.connect(str(tmp_path / "idem.db")) + conn.executescript(schema_sql) + # Should not raise even though columns are already paddle_* + mod_0001.up(conn) + cols = _column_names(conn, "subscriptions") + conn.close() + assert "paddle_customer_id" in cols + + def test_recreates_index(self, tmp_path, mod_0001): + conn = sqlite3.connect(str(tmp_path / "idx.db")) + conn.executescript(_old_schema_sql()) + mod_0001.up(conn) + indexes = conn.execute( + "SELECT name FROM sqlite_master WHERE type='index'" + " AND name='idx_subscriptions_provider'" + ).fetchall() + conn.close() + assert len(indexes) == 1 + + +# ── TestMigrationOrdering ───────────────────────────────────── + + +class TestMigrationOrdering: + def test_multiple_pending_run_in_order(self, tmp_path, monkeypatch): + """Mock two version files and verify they run in sorted order.""" + db_path = str(tmp_path / "order.db") + # Create a DB with one arbitrary table so it's not "fresh" + conn = sqlite3.connect(db_path) + conn.execute("CREATE TABLE dummy (id INTEGER PRIMARY KEY)") + conn.close() + + monkeypatch.setenv("DATABASE_PATH", db_path) + + # Create fake version files in a temp versions dir + vdir = tmp_path / "vdir" + vdir.mkdir() + (vdir / "0001_first.py").write_text("") + (vdir / "0002_second.py").write_text("") + monkeypatch.setattr( + "padelnomics.migrations.migrate.VERSIONS_DIR", vdir + ) + + call_order = [] + + def fake_import(name): + class FakeMod: + @staticmethod + def up(conn): + call_order.append(name) + return FakeMod() + + with patch( + "padelnomics.migrations.migrate.importlib.import_module", + side_effect=fake_import, + ): + migrate() + + assert call_order == [ + "padelnomics.migrations.versions.0001_first", + "padelnomics.migrations.versions.0002_second", + ] + + def test_migrations_table_created_on_existing_db( + self, existing_db, monkeypatch + ): + """An existing DB without _migrations gets the table after migrate().""" + monkeypatch.setenv("DATABASE_PATH", existing_db) + migrate() + conn = sqlite3.connect(existing_db) + tables = _table_names(conn) + conn.close() + assert "_migrations" in tables