diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c7410d1e9e..24aa2078a0 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -138,6 +138,58 @@ jobs: -n auto \ --splits 4 --group ${{ matrix.shard }} + postgres-migrations: + name: PostgreSQL Migration Compatibility + runs-on: ubuntu-latest + needs: backend-lint + services: + postgres: + image: postgres:16 + env: + POSTGRES_USER: printops + POSTGRES_PASSWORD: printops + POSTGRES_DB: printops + ports: + - 5432:5432 + options: >- + --health-cmd "pg_isready -U printops -d printops" + --health-interval 5s + --health-timeout 5s + --health-retries 10 + env: + DATABASE_URL: postgresql+asyncpg://printops:printops@127.0.0.1:5432/printops + PRINTOPS_POSTGRES_TEST_URL: postgresql+asyncpg://printops:printops@127.0.0.1:5432/printops + DATA_DIR: /tmp/printops-postgres-test + LOG_TO_FILE: 'false' + TESTING: '1' + steps: + - uses: actions/checkout@v6 + + - name: Set up Python + uses: actions/setup-python@v6 + with: + python-version: ${{ env.PYTHON_VERSION }} + + - name: Cache pip + uses: actions/cache@v5 + with: + path: ~/.cache/pip + key: ${{ runner.os }}-pip-${{ hashFiles('requirements-dev.lock.txt') }} + restore-keys: | + ${{ runner.os }}-pip- + + - name: Install dependencies + run: pip install --require-hashes -r requirements-dev.lock.txt + + - name: Configure non-UTC database default + run: >- + docker exec "${{ job.services.postgres.id }}" + psql -U printops -d printops + -c "ALTER DATABASE printops SET timezone TO 'Europe/Istanbul'" + + - name: Run live PostgreSQL migration tests + run: python -m pytest backend/tests/postgres/test_issue_142_postgres_runtime.py -q + # ============================================================================ # Frontend Checks # ============================================================================ diff --git a/backend/app/api/routes/support.py b/backend/app/api/routes/support.py index 6c1bfd4434..746ef8d9bf 100644 --- a/backend/app/api/routes/support.py +++ b/backend/app/api/routes/support.py @@ -41,6 +41,7 @@ ) from backend.app.services.network_utils import get_network_interfaces from backend.app.services.printer_manager import printer_manager +from backend.app.utils.local_time import utcnow_naive router = APIRouter(prefix="/support", tags=["support"]) logger = logging.getLogger(__name__) @@ -486,9 +487,8 @@ async def _collect_queue_info(db: AsyncSession) -> dict: ) ).scalar_one_or_none() if oldest_row is not None: - # created_at is naive in this codebase (server_default=func.now()); compare - # against naive utc-now to get the actual age without TZ-conversion surprises. - age = (datetime.now() - oldest_row).total_seconds() + # created_at is naive UTC; compare it with the same clock contract. + age = (utcnow_naive() - oldest_row).total_seconds() info["oldest_pending_age_seconds"] = int(age) else: info["oldest_pending_age_seconds"] = None diff --git a/backend/app/core/database.py b/backend/app/core/database.py index 4b7191ce30..c2b8651139 100644 --- a/backend/app/core/database.py +++ b/backend/app/core/database.py @@ -9,7 +9,7 @@ from backend.app.core.active_print_migrations import migrate_active_print_spoolman from backend.app.core.archive_metadata_migration import repair_archive_plate_metadata from backend.app.core.config import settings -from backend.app.core.db_dialect import is_sqlite +from backend.app.core.db_dialect import is_already_applied as _is_already_applied, is_sqlite, postgres_connect_args from backend.app.core.library_migrations import reclassify_sliced_3mf_library_files from backend.app.core.number_sequence_migrations import migrate_number_sequence_monthly_reset_policy from backend.app.core.rfid_core_weight_migration import repair_rfid_core_weights @@ -48,9 +48,17 @@ def _resolve_pool_kwargs() -> dict: return kwargs +def _resolve_connect_args() -> dict: + """Return dialect-appropriate arguments for new DB connections.""" + return postgres_connect_args(settings.database_url, sqlite=is_sqlite()) + + def _create_engine(): """Create the async engine with dialect-appropriate settings.""" kwargs = _resolve_pool_kwargs() + connect_args = _resolve_connect_args() + if connect_args: + kwargs["connect_args"] = connect_args global _pool_config _pool_config = { "pool_size": kwargs["pool_size"], @@ -453,13 +461,9 @@ async def _migrate_encrypt_legacy_secrets() -> None: async def _safe_execute(conn, sql): """Execute a DDL migration statement, silently ignoring idempotency errors. - 'already exists', 'duplicate column name' (SQLite ADD COLUMN), 'no such column' - (SQLite RENAME COLUMN), 'duplicate key', and the compound - 'column … does not exist' (PostgreSQL RENAME COLUMN idempotency) are swallowed - so that re-running DDL migrations is safe. The compound check additionally - requires the SQL to be a RENAME COLUMN statement so that "does not exist" errors - from ADD COLUMN or CREATE INDEX (which would indicate schema corruption, not - idempotency) are never silently swallowed. + PostgreSQL uses locale-independent SQLSTATE codes; SQLite retains its stable + error-text fallback. Missing-column errors are accepted for RENAME COLUMN + only, so corrupt schemas still abort startup. Any other error is logged and re-raised — callers must not assume silent recovery, as a failure will abort the migration sequence and prevent application startup. @@ -477,14 +481,7 @@ async def _safe_execute(conn, sql): async with conn.begin_nested(): await conn.execute(text(sql)) except (OperationalError, ProgrammingError) as exc: - msg = str(exc).lower() - # Only swallow "column … does not exist" for RENAME COLUMN — not for ADD COLUMN - # or CREATE INDEX where it would indicate schema corruption, not idempotency. - column_not_exists = "rename column" in sql.lower() and "column" in msg and "does not exist" in msg - if ( - not any(k in msg for k in ("already exists", "duplicate key", "duplicate column name", "no such column")) - and not column_not_exists - ): + if not _is_already_applied(exc, sql): logger.error("Migration statement failed: %s | SQL: %.200s", exc, sql) raise @@ -2479,17 +2476,15 @@ async def run_migrations(conn): # SQLite does not support ALTER TABLE ADD CONSTRAINT — handled by __table_args__ at creation. # Runs AFTER the backfill so Fall B rows don't fail constraint validation. if not is_sqlite(): + add_constraint = ( + "ALTER TABLE oidc_providers ADD CONSTRAINT ck_auto_link_requires_verified_email_claim " + "CHECK (auto_link_existing_accounts = FALSE OR email_claim != 'email' OR require_email_verified = TRUE)" + ) try: async with conn.begin_nested(): - await conn.execute( - text( - "ALTER TABLE oidc_providers ADD CONSTRAINT ck_auto_link_requires_verified_email_claim " - "CHECK (auto_link_existing_accounts = FALSE OR email_claim != 'email' OR require_email_verified = TRUE)" - ) - ) + await conn.execute(text(add_constraint)) except (OperationalError, ProgrammingError) as exc: - msg = str(exc).lower() - if "already exists" not in msg: + if not _is_already_applied(exc, add_constraint): logger.error( "Security constraint migration FAILED — auto_link safety constraint may not be enforced: %s", exc, diff --git a/backend/app/core/db_dialect.py b/backend/app/core/db_dialect.py index 9733fc9664..1d25c262ae 100644 --- a/backend/app/core/db_dialect.py +++ b/backend/app/core/db_dialect.py @@ -6,6 +6,9 @@ from sqlalchemy import func, text +_PG_ALREADY_APPLIED = frozenset({"42701", "42P07", "42710", "23505"}) +_PG_UNDEFINED_COLUMN = "42703" + def is_postgres() -> bool: """Check if using PostgreSQL based on DATABASE_URL.""" @@ -21,6 +24,40 @@ def is_sqlite() -> bool: return settings.database_url.startswith("sqlite") +def postgres_connect_args(database_url: str, *, sqlite: bool) -> dict: + """Pin PostgreSQL sessions to UTC while leaving SQLite untouched.""" + if sqlite: + return {} + if "+asyncpg" in database_url: + return {"server_settings": {"timezone": "UTC"}} + return {"options": "-c timezone=UTC"} + + +def sqlstate(exc: BaseException) -> str | None: + """Return a SQLAlchemy-wrapped PostgreSQL SQLSTATE when available.""" + orig = getattr(exc, "orig", None) + for attr in ("sqlstate", "pgcode"): + code = getattr(orig, attr, None) + if code: + return str(code) + return None + + +def is_already_applied(exc: BaseException, sql: str) -> bool: + """Classify idempotent DDL by SQLSTATE, with SQLite's text fallback.""" + is_rename = "rename column" in sql.lower() + state = sqlstate(exc) + if state is not None: + return state in _PG_ALREADY_APPLIED or (state == _PG_UNDEFINED_COLUMN and is_rename) + + message = str(exc).lower() + if any( + marker in message for marker in ("already exists", "duplicate key", "duplicate column name", "no such column") + ): + return True + return is_rename and "column" in message and "does not exist" in message + + async def upsert_setting(db, model, key: str, value: str): """Dialect-aware INSERT ... ON CONFLICT UPDATE for the Settings table.""" if is_postgres(): diff --git a/backend/app/main.py b/backend/app/main.py index 4ff3b13720..8ac9f84d64 100644 --- a/backend/app/main.py +++ b/backend/app/main.py @@ -136,6 +136,7 @@ ) from backend.app.services.tasmota import tasmota_service from backend.app.utils.filament_types import printer_filament_type +from backend.app.utils.local_time import utcnow_naive from backend.app.utils.print_jobs import ignore_internal_printer_job @@ -5251,14 +5252,13 @@ async def record_ams_history(): _ams_cleanup_counter += 1 if _ams_cleanup_counter >= 288: _ams_cleanup_counter = 0 - # Get retention days from settings from backend.app.models.settings import Settings result = await db.execute(select(Settings).where(Settings.key == "ams_history_retention_days")) setting = result.scalar_one_or_none() retention_days = int(setting.value) if setting else AMS_HISTORY_RETENTION_DAYS - cutoff = datetime.utcnow() - timedelta(days=retention_days) + cutoff = utcnow_naive() - timedelta(days=retention_days) result = await db.execute(delete(AMSSensorHistory).where(AMSSensorHistory.recorded_at < cutoff)) await db.commit() if result.rowcount > 0: @@ -5384,7 +5384,7 @@ async def record_printer_sensor_history(): setting = result.scalar_one_or_none() retention_days = int(setting.value) if setting else PRINTER_SENSOR_HISTORY_RETENTION_DAYS - cutoff = datetime.utcnow() - timedelta(days=retention_days) + cutoff = utcnow_naive() - timedelta(days=retention_days) cleanup = await db.execute( delete(PrinterSensorHistory).where(PrinterSensorHistory.recorded_at < cutoff) ) diff --git a/backend/tests/postgres/test_issue_142_postgres_runtime.py b/backend/tests/postgres/test_issue_142_postgres_runtime.py new file mode 100644 index 0000000000..ba4a215825 --- /dev/null +++ b/backend/tests/postgres/test_issue_142_postgres_runtime.py @@ -0,0 +1,79 @@ +"""Live PostgreSQL coverage for locale-safe migrations and UTC timestamps (#142).""" + +from __future__ import annotations + +import os +from datetime import datetime, timedelta, timezone + +import pytest +from sqlalchemy import text +from sqlalchemy.exc import ProgrammingError + +POSTGRES_TEST_URL = os.environ.get("PRINTOPS_POSTGRES_TEST_URL") +pytestmark = pytest.mark.skipif(not POSTGRES_TEST_URL, reason="live PostgreSQL test URL not configured") + + +@pytest.mark.asyncio +async def test_repeated_startup_localised_errors_and_utc_defaults(): + from backend.app.core import database + + assert database.settings.database_url == POSTGRES_TEST_URL + + # Fresh installation followed by a repeated startup exercises the complete + # migration sequence against PostgreSQL, including already-applied DDL. + await database.init_db() + await database.init_db() + + async with database.engine.begin() as conn: + assert (await conn.execute(text("SHOW TimeZone"))).scalar_one() == "UTC" + + await conn.execute(text("DROP TABLE IF EXISTS issue_142_runtime_probe")) + await conn.execute( + text( + "CREATE TABLE issue_142_runtime_probe (" + "id INTEGER PRIMARY KEY, created_at TIMESTAMP WITHOUT TIME ZONE DEFAULT CURRENT_TIMESTAMP)" + ) + ) + await conn.execute(text("INSERT INTO issue_142_runtime_probe (id) VALUES (1)")) + stored = (await conn.execute(text("SELECT created_at FROM issue_142_runtime_probe WHERE id = 1"))).scalar_one() + utc_now = datetime.now(timezone.utc).replace(tzinfo=None) + assert abs(stored - utc_now) < timedelta(minutes=1) + + await database._safe_execute(conn, "ALTER TABLE issue_142_runtime_probe ADD COLUMN value INTEGER") + await database._safe_execute(conn, "ALTER TABLE issue_142_runtime_probe ADD COLUMN value INTEGER") + + # PostgreSQL emits the error and SQLSTATE; only the message is supplied + # in German to prove classification never depends on English wording. + with pytest.raises(ProgrammingError) as duplicate: + async with conn.begin_nested(): + await conn.execute( + text( + "DO $$ BEGIN RAISE EXCEPTION USING ERRCODE = '42701', " + "MESSAGE = 'Spalte ist bereits vorhanden'; END $$" + ) + ) + assert "Spalte ist bereits vorhanden" in str(duplicate.value) + assert database._is_already_applied( + duplicate.value, + "ALTER TABLE issue_142_runtime_probe ADD COLUMN value INTEGER", + ) + + with pytest.raises(ProgrammingError) as unexpected: + async with conn.begin_nested(): + await conn.execute( + text( + "DO $$ BEGIN RAISE EXCEPTION USING ERRCODE = '42P01', " + "MESSAGE = 'already exists, aber die Tabelle fehlt'; END $$" + ) + ) + assert not database._is_already_applied( + unexpected.value, + "ALTER TABLE issue_142_runtime_probe ADD COLUMN other INTEGER", + ) + with pytest.raises(ProgrammingError): + await database._safe_execute( + conn, + "ALTER TABLE issue_142_missing_table ADD COLUMN value INTEGER", + ) + + await conn.execute(text("DROP TABLE issue_142_runtime_probe")) diff --git a/backend/tests/unit/test_issue_142_postgres_migrations.py b/backend/tests/unit/test_issue_142_postgres_migrations.py new file mode 100644 index 0000000000..800dc95827 --- /dev/null +++ b/backend/tests/unit/test_issue_142_postgres_migrations.py @@ -0,0 +1,188 @@ +"""Regression coverage for locale-independent PostgreSQL migrations (#142).""" + +from __future__ import annotations + +from datetime import datetime, timedelta, timezone + +import pytest +from sqlalchemy import Column, DateTime, Integer, MetaData, Table, func, select, text +from sqlalchemy.exc import OperationalError, ProgrammingError +from sqlalchemy.ext.asyncio import create_async_engine + + +class _FakePostgresError(Exception): + def __init__(self, sqlstate: str, message: str): + super().__init__(message) + self.sqlstate = sqlstate + + +def _pg_error(sqlstate: str, message: str, sql: str) -> ProgrammingError: + return ProgrammingError(sql, {}, _FakePostgresError(sqlstate, message)) + + +ADD_COLUMN = "ALTER TABLE pipeline_runs ADD COLUMN parent_run_id INTEGER" +RENAME_COLUMN = "ALTER TABLE pipeline_runs RENAME COLUMN old_name TO new_name" +CREATE_INDEX = "CREATE INDEX ix_pipeline_runs_parent ON pipeline_runs (parent_run_id)" + + +class TestMigrationErrorClassification: + @pytest.mark.parametrize( + "sqlstate,message,sql", + [ + ("42701", 'столбец "parent_run_id" уже существует', ADD_COLUMN), + ("42P07", 'отношение "pipeline_runs" уже существует', "CREATE TABLE pipeline_runs (id INTEGER)"), + ( + "42710", + 'ограничение "ck_state" уже существует', + "ALTER TABLE pipeline_runs ADD CONSTRAINT ck_state CHECK (id > 0)", + ), + ("23505", "doppelter Schlüssel verletzt Unique-Constraint", CREATE_INDEX), + ], + ) + def test_postgres_idempotency_uses_sqlstate_not_message(self, sqlstate: str, message: str, sql: str): + from backend.app.core.database import _is_already_applied + + assert _is_already_applied(_pg_error(sqlstate, message, sql), sql) is True + + def test_undefined_column_is_idempotent_only_for_rename(self): + from backend.app.core.database import _is_already_applied + + error = _pg_error("42703", 'столбец "old_name" не существует', RENAME_COLUMN) + assert _is_already_applied(error, RENAME_COLUMN) is True + assert _is_already_applied(error, CREATE_INDEX) is False + + @pytest.mark.parametrize("sqlstate", ["42P01", "42704", "42601"]) + def test_unexpected_postgres_errors_are_not_swallowed(self, sqlstate: str): + from backend.app.core.database import _is_already_applied + + error = _pg_error(sqlstate, "already exists but this is a real failure", ADD_COLUMN) + assert _is_already_applied(error, ADD_COLUMN) is False + + def test_sqlite_keeps_its_unlocalised_message_fallback(self): + from backend.app.core.database import _is_already_applied + + duplicate = OperationalError(ADD_COLUMN, {}, Exception("duplicate column name: parent_run_id")) + missing_table = OperationalError(ADD_COLUMN, {}, Exception("no such table: pipeline_runs")) + assert _is_already_applied(duplicate, ADD_COLUMN) is True + assert _is_already_applied(missing_table, ADD_COLUMN) is False + + def test_psycopg_pgcode_is_supported(self): + from backend.app.core.db_dialect import sqlstate + + class _PsycopgError(Exception): + pgcode = "42P07" + + error = ProgrammingError("CREATE TABLE issue_142 (id INTEGER)", {}, _PsycopgError()) + assert sqlstate(error) == "42P07" + + @pytest.mark.asyncio + async def test_safe_execute_repeated_sqlite_migration_is_idempotent(self): + from backend.app.core.database import _safe_execute + + engine = create_async_engine("sqlite+aiosqlite:///:memory:") + try: + async with engine.begin() as conn: + await conn.execute(text("CREATE TABLE issue_142 (id INTEGER PRIMARY KEY)")) + await _safe_execute(conn, "ALTER TABLE issue_142 ADD COLUMN value INTEGER") + await _safe_execute(conn, "ALTER TABLE issue_142 ADD COLUMN value INTEGER") + columns = {row[1] for row in await conn.execute(text("PRAGMA table_info(issue_142)"))} + finally: + await engine.dispose() + + assert columns == {"id", "value"} + + +class TestPostgresSessionTimezone: + def test_sqlite_gets_no_connect_args(self, monkeypatch): + from backend.app.core import database + + monkeypatch.setattr(database, "is_sqlite", lambda: True) + assert database._resolve_connect_args() == {} + + def test_asyncpg_session_is_pinned_to_utc(self, monkeypatch): + from backend.app.core import database + + monkeypatch.setattr(database, "is_sqlite", lambda: False) + monkeypatch.setattr( + database.settings, + "database_url", + "postgresql+asyncpg://user:password@postgres/printops", + raising=False, + ) + assert database._resolve_connect_args() == {"server_settings": {"timezone": "UTC"}} + + def test_other_postgres_drivers_use_libpq_options(self, monkeypatch): + from backend.app.core import database + + monkeypatch.setattr(database, "is_sqlite", lambda: False) + monkeypatch.setattr( + database.settings, + "database_url", + "postgresql+psycopg://user:password@postgres/printops", + raising=False, + ) + assert database._resolve_connect_args() == {"options": "-c timezone=UTC"} + + def test_create_engine_passes_postgres_connect_args(self, monkeypatch): + from backend.app.core import database + + captured: dict = {} + + def fake_create_async_engine(url: str, **kwargs): + captured.update(kwargs) + return create_async_engine("sqlite+aiosqlite:///:memory:") + + monkeypatch.setattr(database, "is_sqlite", lambda: False) + monkeypatch.setattr( + database.settings, + "database_url", + "postgresql+asyncpg://user:password@postgres/printops", + raising=False, + ) + monkeypatch.setattr(database, "create_async_engine", fake_create_async_engine) + + database._create_engine() + + assert captured["connect_args"] == {"server_settings": {"timezone": "UTC"}} + + @pytest.mark.asyncio + async def test_sqlite_server_default_remains_naive_utc(self): + metadata = MetaData() + probe = Table( + "issue_142_timezone_probe", + metadata, + Column("id", Integer, primary_key=True), + Column("created_at", DateTime, server_default=func.now()), + ) + engine = create_async_engine("sqlite+aiosqlite:///:memory:") + try: + async with engine.begin() as conn: + await conn.run_sync(metadata.create_all) + await conn.execute(probe.insert()) + stored = (await conn.execute(select(probe.c.created_at))).scalar_one() + finally: + await engine.dispose() + + utc_now = datetime.now(timezone.utc).replace(tzinfo=None) + assert abs(stored - utc_now) < timedelta(minutes=1) + + +@pytest.mark.asyncio +async def test_support_queue_age_uses_naive_utc(db_session, monkeypatch): + from backend.app.api.routes import support + from backend.app.models.print_queue import PrintQueueItem + + utc_now = datetime(2026, 9, 7, 7, 0, 0) + monkeypatch.setattr(support, "utcnow_naive", lambda: utc_now) + db_session.add( + PrintQueueItem( + printer_id=1, + status="pending", + created_at=utc_now - timedelta(seconds=30), + ) + ) + await db_session.commit() + + info = await support._collect_queue_info(db_session) + + assert info["oldest_pending_age_seconds"] == 30 diff --git a/tools/check_source_size_budget.py b/tools/check_source_size_budget.py index 5ccad5afab..1138d3fe3d 100644 --- a/tools/check_source_size_budget.py +++ b/tools/check_source_size_budget.py @@ -84,7 +84,7 @@ "backend/app/api/routes/projects.py": 2136, "backend/app/api/routes/mfa.py": 2262, "backend/app/api/routes/spoolman_inventory.py": 2060, - "backend/app/core/database.py": 4223, + "backend/app/core/database.py": 4218, "backend/app/main.py": 6823, "backend/app/services/bambu_mqtt.py": 5928, "backend/app/services/notification_service.py": 2184, @@ -125,7 +125,7 @@ "backend/app/api/routes/spoolman.py::link_spool": 297, "backend/app/api/routes/support.py::_collect_support_info": 452, "backend/app/api/routes/updates.py::_perform_update": 303, - "backend/app/core/database.py::run_migrations": 2803, + "backend/app/core/database.py::run_migrations": 2801, "backend/app/core/database.py::seed_default_groups": 347, "backend/app/main.py::lifespan": 401, "backend/app/main.py::on_ams_change": 677,