fix(ptica): у пользователя не может быть двух дефолтных профилей весов (#2464)
All checks were successful
CI Trade-In / changes (pull_request) Successful in 8s
CI / changes (pull_request) Successful in 9s
CI Trade-In / backend-tests (pull_request) Has been skipped
CI Trade-In / browser-tests (pull_request) Has been skipped
CI Trade-In / frontend-checks (pull_request) Has been skipped
CI / frontend-tests (pull_request) Has been skipped
CI / openapi-codegen-check (pull_request) Successful in 2m26s
CI / backend-tests (pull_request) Successful in 17m14s

create_profile/update_profile делают «снять is_default у всех → поставить новому»
двумя отдельными операторами. Между ними инвариант нарушен, и при одновременных
запросах у пользователя может оказаться ДВА профиля с is_default=TRUE. А читающий
_SELECT_DEFAULT брал LIMIT 1 БЕЗ ORDER BY — выбор молча перескакивал между ними от
запроса к запросу.

Два рубежа, а не один:

  миграция 190 — частичный уникальный индекс (user_id) WHERE is_default: два
                 дефолта становятся невозможными на уровне БД;
  ORDER BY id  — детерминированный выбор, если индекс когда-нибудь снимут.
                 Соседние запросы этого файла тай-брейк по id уже имеют.

Индекс не мешает штатной переустановке дефолта: порядок операторов в коде уже
правильный (сначала снять у всех, потом поставить), поэтому в момент проверки
дефолтов ноль. Это отдельно проверено тестом.

Безопасность миграции: на проде нарушений нет — у admin один дефолт, у __system__
ноль, ни одного пользователя с двумя. Таблица в 4 строки, индексируется мгновенно.
lock_timeout проставлен по #2752.

Тест проверяет ПОВЕДЕНИЕ на живом Postgres: вторая установка дефолта отвергается
базой. Плюс фальсификация — без индекса два дефолта вставляются молча; без неё
зелёный тест неотличим от «оно и так не вставлялось». Плюс два контроля:
переустановка дефолта работает, разные пользователи сохраняют свои.

Тест про ORDER BY вынесен в tests/services/site_finder, а НЕ внесён в
skip_allowlist: живой БД он не требует, и пропускаться вместе с DB-тестами ему
незачем. Против origin/main он краснеет, показывая запрос без тай-брейка.

Прогоны: без БД — 648 passed rc=0; с БД — 5 passed rc=0.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
bot-backend 2026-08-20 16:58:48 +05:00
parent 9b18c23a5a
commit afa648b21f
5 changed files with 247 additions and 0 deletions

View file

@ -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
"""

View file

@ -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}"

View file

@ -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

View file

@ -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

View file

@ -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;