fix(ptica): у пользователя не может быть двух дефолтных профилей весов (#2464) #2976
5 changed files with 247 additions and 0 deletions
|
|
@ -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
|
||||
"""
|
||||
|
||||
|
|
|
|||
|
|
@ -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}"
|
||||
|
|
@ -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
|
||||
|
|
|
|||
164
backend/tests/sql/test_2464_default_profile_unique.py
Normal file
164
backend/tests/sql/test_2464_default_profile_unique.py
Normal 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
|
||||
33
data/sql/190_user_weight_profiles_one_default.sql
Normal file
33
data/sql/190_user_weight_profiles_one_default.sql
Normal 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;
|
||||
Loading…
Add table
Reference in a new issue