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
|
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_DEFAULT = f"""
|
||||||
SELECT {_SELECT_COLS}
|
SELECT {_SELECT_COLS}
|
||||||
FROM user_weight_profiles
|
FROM user_weight_profiles
|
||||||
WHERE user_id = :user_id
|
WHERE user_id = :user_id
|
||||||
AND is_default = TRUE
|
AND is_default = TRUE
|
||||||
|
ORDER BY id ASC
|
||||||
LIMIT 1
|
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_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_dedup_statement_matches_the_key
|
||||||
tests/sql/test_2464_land_reservation_dedup.py::test_migration_body_runs_on_a_prod_shaped_replica
|
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