diff --git a/backend/app/services/site_finder/weight_profiles.py b/backend/app/services/site_finder/weight_profiles.py index 7639c02d..ddd5048a 100644 --- a/backend/app/services/site_finder/weight_profiles.py +++ b/backend/app/services/site_finder/weight_profiles.py @@ -96,11 +96,20 @@ _SELECT_BY_ID = f""" AND id = :profile_id """ +# ORDER BY здесь не украшение (#2464): без него LIMIT 1 брал произвольную строку, +# и при двух дефолтах у одного пользователя выбор мог молча перескакивать между +# ними от запроса к запросу. Соседние запросы этого файла тай-брейк по id уже +# имеют (см. ORDER BY is_default DESC, id ASC выше) — приводим к ним. +# +# Сам случай «два дефолта» с миграции 190 невозможен: частичный уникальный индекс +# user_weight_profiles_one_default (user_id) WHERE is_default. ORDER BY остаётся +# вторым рубежом — на случай, если индекс когда-нибудь снимут. _SELECT_DEFAULT = f""" SELECT {_SELECT_COLS} FROM user_weight_profiles WHERE user_id = :user_id AND is_default = TRUE + ORDER BY id ASC LIMIT 1 """ diff --git a/backend/tests/services/site_finder/test_2464_default_profile_order.py b/backend/tests/services/site_finder/test_2464_default_profile_order.py new file mode 100644 index 00000000..b01e3352 --- /dev/null +++ b/backend/tests/services/site_finder/test_2464_default_profile_order.py @@ -0,0 +1,32 @@ +"""Читающий запрос дефолтного профиля детерминирован по id (#2464). + +`_SELECT_DEFAULT` брал `LIMIT 1` без `ORDER BY`: при двух дефолтах у одного пользователя +выбор молча перескакивал между ними от запроса к запросу. Соседние запросы этого файла +тай-брейк по id уже имели. + +Сам случай «два дефолта» с миграции 190 невозможен — частичный уникальный индекс. Этот +тест держит ВТОРОЙ рубеж: если индекс когда-нибудь снимут, выбор не должен снова стать +произвольным. Живой БД не требует, поэтому лежит отдельно от tests/sql — чтобы +исполняться на любой машине, а не пропускаться вместе с ними. +""" + +from __future__ import annotations + +import os + +os.environ.setdefault("DATABASE_URL", "postgresql+psycopg://test:test@localhost:5432/test") + +import re + + +def test_select_default_is_ordered() -> None: + """Второй рубеж: читающий запрос детерминирован по id. + + Индекс делает два дефолта невозможными, но если его когда-нибудь снимут, + выбор не должен снова стать произвольным. + """ + from app.services.site_finder.weight_profiles import _SELECT_DEFAULT + + assert re.search( + r"ORDER BY\s+id\s+ASC", _SELECT_DEFAULT + ), f"в _SELECT_DEFAULT нет тай-брейка по id:\n{_SELECT_DEFAULT}" diff --git a/backend/tests/skip_allowlist.txt b/backend/tests/skip_allowlist.txt index 2a286c2f..3070379f 100644 --- a/backend/tests/skip_allowlist.txt +++ b/backend/tests/skip_allowlist.txt @@ -125,3 +125,12 @@ tests/sql/test_2464_land_reservation_dedup.py::test_records_with_act_number_stil tests/sql/test_2464_land_reservation_dedup.py::test_different_parcels_are_not_collapsed tests/sql/test_2464_land_reservation_dedup.py::test_migration_dedup_statement_matches_the_key tests/sql/test_2464_land_reservation_dedup.py::test_migration_body_runs_on_a_prod_shaped_replica + +# ── #2464: единственный дефолтный профиль весов (миграция 190) ──────────────── +# Нужен живой Postgres: тесты создают ВРЕМЕННУЮ user_weight_profiles, применяют +# миграцию и проверяют, что вторая установка дефолта отвергается базой. В CI ИДУТ +# (postgres-сервис, #2745); записи нужны для машины без БД и без туннеля. +tests/sql/test_2464_default_profile_unique.py::test_second_default_is_rejected_by_the_database +tests/sql/test_2464_default_profile_unique.py::test_without_migration_two_defaults_slip_through +tests/sql/test_2464_default_profile_unique.py::test_reassigning_default_still_works +tests/sql/test_2464_default_profile_unique.py::test_different_users_keep_their_own_defaults diff --git a/backend/tests/sql/test_2464_default_profile_unique.py b/backend/tests/sql/test_2464_default_profile_unique.py new file mode 100644 index 00000000..4a1599f3 --- /dev/null +++ b/backend/tests/sql/test_2464_default_profile_unique.py @@ -0,0 +1,164 @@ +"""У пользователя не может быть двух дефолтных профилей весов (#2464). + +`create_profile`/`update_profile` делают «снять is_default у всех → поставить новому» +двумя отдельными операторами. Между ними инвариант нарушен, и при одновременных запросах +у пользователя может оказаться ДВА профиля с `is_default=TRUE`. Читающий `_SELECT_DEFAULT` +брал `LIMIT 1` без `ORDER BY` — выбор молча перескакивал между ними. + +Правка из двух рубежей: + • миграция 190 — частичный уникальный индекс «не более одного дефолта на пользователя»; + • `ORDER BY id ASC` в `_SELECT_DEFAULT` — детерминированный выбор, если индекс снимут. + +Проверяется ПОВЕДЕНИЕ на живом Postgres: вторая установка дефолта обязана отвергаться +базой. Тест герметичный — таблица создаётся временной в своей же сессии. +""" + +from __future__ import annotations + +import os + +os.environ.setdefault("DATABASE_URL", "postgresql+psycopg://test:test@localhost:5432/test") + +from pathlib import Path + +import pytest +from sqlalchemy import create_engine, text +from sqlalchemy.exc import IntegrityError +from sqlalchemy.orm import sessionmaker + +_MIGRATION = ( + Path(__file__).resolve().parents[3] + / "data" + / "sql" + / "190_user_weight_profiles_one_default.sql" +) + + +def _dsn() -> str: + raw = os.environ.get("TEST_DATABASE_URL") or os.environ.get( + "DATABASE_URL", "postgresql+psycopg://gendesign@localhost:15432/gendesign" + ) + return ( + raw + if raw.startswith("postgresql+") + else raw.replace("postgresql://", "postgresql+psycopg://") + ) + + +def _db_reachable() -> tuple[bool, str]: + try: + eng = create_engine(_dsn(), connect_args={"connect_timeout": 3}) + with eng.connect() as c: + c.execute(text("SELECT 1")) + return True, "" + except Exception as exc: + return False, str(exc) + + +_DB_OK, _DB_ERR = _db_reachable() +pytestmark = pytest.mark.skipif(not _DB_OK, reason=f"Postgres недоступен: {_DB_ERR}") + +_TABLE = """ +CREATE TEMP TABLE user_weight_profiles ( + id bigserial PRIMARY KEY, + user_id text NOT NULL, + profile_name text, + weights jsonb, + is_default boolean DEFAULT false, + description text +) ON COMMIT DROP; +""" + + +@pytest.fixture +def db(): + engine = create_engine(_dsn()) + session = sessionmaker(bind=engine)() + try: + session.execute(text(_TABLE)) + n = session.execute(text("SELECT count(*) FROM user_weight_profiles")).scalar() + assert n == 0, f"запрос попал НЕ во временную таблицу ({n} строк)" + yield session + finally: + session.rollback() + session.close() + engine.dispose() + + +def _apply_migration(db) -> None: + sql = _MIGRATION.read_text() + body = sql[sql.index("BEGIN;") + len("BEGIN;") : sql.rindex("COMMIT;")] + # Комментарии снимаем ДО разбиения: «;» внутри комментария разрезала бы SQL. + code = "\n".join( + ln for ln in body.splitlines() if ln.strip() and not ln.lstrip().startswith("--") + ) + for chunk in code.split(";"): + if chunk.strip(): + db.execute(text(chunk)) + + +def _add(db, *, user: str, name: str, default: bool) -> None: + db.execute( + text( + "INSERT INTO user_weight_profiles (user_id, profile_name, is_default)" + " VALUES (:u, :n, :d)" + ), + {"u": user, "n": name, "d": default}, + ) + + +def test_second_default_is_rejected_by_the_database(db) -> None: + """Второй дефолт у того же пользователя обязан отвергаться. + + Без миграции такая вставка проходит, и у пользователя оказывается два дефолта. + """ + _apply_migration(db) + _add(db, user="u1", name="первый", default=True) + + with pytest.raises(IntegrityError): + _add(db, user="u1", name="второй", default=True) + + +def test_without_migration_two_defaults_slip_through(db) -> None: + """Фальсификация: без индекса два дефолта вставляются молча. + + Без этой проверки зелёный тест выше неотличим от «оно и так не вставлялось». + """ + _add(db, user="u1", name="первый", default=True) + _add(db, user="u1", name="второй", default=True) + n = db.execute( + text("SELECT count(*) FROM user_weight_profiles WHERE user_id='u1' AND is_default") + ).scalar() + assert n == 2, f"без индекса дефолтов {n}, ожидалось 2 — тест выше ничего не доказывает" + + +def test_reassigning_default_still_works(db) -> None: + """Контроль: штатная переустановка дефолта проходит. + + Порядок в коде — сначала снять у всех, потом поставить новому. Индекс не должен + этому мешать, иначе пользователь не сможет сменить дефолтный профиль. + """ + _apply_migration(db) + _add(db, user="u1", name="первый", default=True) + _add(db, user="u1", name="второй", default=False) + + db.execute(text("UPDATE user_weight_profiles SET is_default = FALSE WHERE user_id='u1'")) + db.execute( + text( + "UPDATE user_weight_profiles SET is_default = TRUE" + " WHERE user_id='u1' AND profile_name='второй'" + ) + ) + name = db.execute( + text("SELECT profile_name FROM user_weight_profiles WHERE user_id='u1' AND is_default") + ).scalar() + assert name == "второй" + + +def test_different_users_keep_their_own_defaults(db) -> None: + """Контроль: индекс не мешает разным пользователям иметь свой дефолт.""" + _apply_migration(db) + _add(db, user="u1", name="a", default=True) + _add(db, user="u2", name="b", default=True) + n = db.execute(text("SELECT count(*) FROM user_weight_profiles WHERE is_default")).scalar() + assert n == 2 diff --git a/data/sql/190_user_weight_profiles_one_default.sql b/data/sql/190_user_weight_profiles_one_default.sql new file mode 100644 index 00000000..f81842d2 --- /dev/null +++ b/data/sql/190_user_weight_profiles_one_default.sql @@ -0,0 +1,33 @@ +-- 190_user_weight_profiles_one_default.sql +-- #2464 — «дефолтный профиль весов» становится единственным на уровне БД. +-- +-- БАГ. create_profile/update_profile делают «снять is_default у всех → поставить +-- новому» двумя отдельными операторами. Между ними инвариант нарушен, и при +-- одновременных запросах у пользователя может оказаться ДВА профиля с +-- is_default=TRUE. Читающий запрос _SELECT_DEFAULT брал LIMIT 1 без ORDER BY, +-- то есть выбор молча перескакивал между ними от запроса к запросу. +-- +-- ЧТО ДЕЛАЕМ. Частичный уникальный индекс — «не более одного дефолта на +-- пользователя». Порядок операторов в коде уже правильный (сначала снять, потом +-- поставить), поэтому индекс не мешает штатной переустановке дефолта: после +-- UPDATE ... SET is_default=FALSE дефолтов ноль, и следующая установка проходит. +-- +-- БЕЗОПАСНОСТЬ. Проверено на проде 2026-08-20: нарушений нет — у admin один +-- дефолт, у __system__ ноль, ни одного пользователя с двумя. Таблица крошечная +-- (4 строки), создание индекса мгновенное. +-- +-- Idempotent: CREATE UNIQUE INDEX IF NOT EXISTS. +-- Apply after: 189_land_reservation_nulls_not_distinct.sql + +BEGIN; + +-- #2752: блокирующий DDL обязан иметь lock_timeout, иначе встанет в очередь за +-- чужой сессией и уведёт за собой запросы приложения. Пять секунд — про ОЖИДАНИЕ +-- блокировки, не про работу: таблица в четыре строки индексируется мгновенно. +SET LOCAL lock_timeout = '5s'; + +CREATE UNIQUE INDEX IF NOT EXISTS user_weight_profiles_one_default + ON user_weight_profiles (user_id) + WHERE is_default; + +COMMIT;