All checks were successful
CI Trade-In / changes (pull_request) Successful in 11s
CI / changes (pull_request) Successful in 12s
CI Trade-In / frontend-checks (pull_request) Has been skipped
CI / backend-tests (pull_request) Has been skipped
CI / openapi-codegen-check (pull_request) Has been skipped
CI / frontend-tests (pull_request) Has been skipped
CI Trade-In / backend-tests (pull_request) Successful in 1m11s
Три дефекта, каждый блокировал легальный публичный запуск. 1. Адрес физлица сохранялся в базу ДО любого согласия: согласие фиксировалось только на форме заявки, то есть ПОСЛЕ записи адреса. Для пилота с договором терпимо, для человека с улицы — нет. Проверка согласия поставлена первой строкой расчёта, до геокодирования и до обоих мест записи адреса. Хранение — колонками на самой оценке, 1:1 с уже работающим прецедентом для заявок (миграция 182): IP клиента, версия политики, дословный снимок текста. Отдельная таблица событий не заводилась: согласие даётся ровно на создание этой строки, и когда строка удаляется по сроку, исчезновение доказательства вместе с данными логично. Enforcement НЕ выводится из пустого created_by — первая версия так и делала и сломала 92 несвязанных теста оценщика, которые зовут расчёт без имени пользователя, проверяя ценовую логику. Вместо этого явный флаг, который выставляет единственный боевой вызывающий. B2B-поток не тронут: поле согласия опционально, иначе сломались бы пилоты, чей фронт его не шлёт. 2. Срок жизни оценки применялся только как фильтр при чтении — физического удаления не было ни в одной фоновой задаче, данные жили вечно вопреки декларированному сроку. Заведена задача удаления пачками с ограничением на прогон и коммитом после каждой пачки, идемпотентная. В расписании она ВЫКЛЮЧЕНА: это первая автоматическая задача, удаляющая персональные данные, и первый прогон должен быть под наблюдением. 3. Пути «удалите мои данные» не было. Добавлен сервис удаления и админская ручка. Ключи: имя пользователя, идентификатор оценки, телефон, чат в телеграме. Честно зафиксировано в коде: аноним без ссылки на оценку, без оставленного телефона и без обращения в поддержку неидентифицируем — удалить его данные без дополнительной идентификации нельзя. Отдельно: удаление чистит только копию в базе, зеркало переписки в телеграм-топике не удаляется ничем в кодовой базе, нужен ручной шаг. 4. Соответствие текста согласия на фронте и снимка на бэке держалось на комментарии. Теперь есть тест, который ловит расхождение. Сроки хранения вынесены в настройки. Значение для заявок предложено инженерно (типичный отраслевой диапазон), юридически обоснованный срок — за юристом, и это записано в коде. Тесты: 2775 passed.
232 lines
9.4 KiB
Python
232 lines
9.4 KiB
Python
"""ЭТАП 4 B2C launch — retention -> physical deletion task (part B).
|
|
|
|
Covers app/tasks/purge_expired_trade_in_data.py:
|
|
- batched DELETE (not a single unbounded DELETE), each batch its own commit
|
|
- stops draining a table once a batch returns fewer rows than batch_size
|
|
- safety cap (max_batches) bounds a single run even on a huge backlog
|
|
- both tables (trade_in_estimates, trade_in_leads) get drained
|
|
- failure path: rollback + mark_failed with partial counters, exception re-raised
|
|
- SQL shape: DELETE (not UPDATE/deactivate), no psycopg `::` cast trap
|
|
|
|
Style mirrors tests/test_deactivate_stale_listings.py (_FakeDB, monkeypatched
|
|
runs_mod.mark_done/mark_failed).
|
|
"""
|
|
|
|
from __future__ import annotations
|
|
|
|
import os
|
|
import re
|
|
from pathlib import Path
|
|
from typing import Any
|
|
|
|
import pytest
|
|
|
|
os.environ.setdefault("DATABASE_URL", "postgresql+psycopg://test:test@localhost:5432/test")
|
|
|
|
from app.tasks import purge_expired_trade_in_data as task_mod
|
|
|
|
_SQL_DIR = Path(__file__).resolve().parents[1] / "data" / "sql"
|
|
_MIGRATION_193 = _SQL_DIR / "193_trade_in_privacy_retention.sql"
|
|
|
|
|
|
class _FakeResult:
|
|
def __init__(self, rowcount: int) -> None:
|
|
self.rowcount = rowcount
|
|
|
|
|
|
class _FakeDB:
|
|
"""Pops rowcounts in call order -- caller supplies the exact sequence expected."""
|
|
|
|
def __init__(self, rowcounts: list[int]) -> None:
|
|
self._rowcounts = list(rowcounts)
|
|
self.executed: list[tuple[Any, Any]] = []
|
|
self.commits = 0
|
|
self.rolled_back = False
|
|
|
|
def execute(self, stmt: Any, params: dict[str, Any] | None = None) -> _FakeResult:
|
|
self.executed.append((stmt, params))
|
|
return _FakeResult(self._rowcounts.pop(0))
|
|
|
|
def commit(self) -> None:
|
|
self.commits += 1
|
|
|
|
def rollback(self) -> None:
|
|
self.rolled_back = True
|
|
|
|
|
|
def _patch_runs(monkeypatch: pytest.MonkeyPatch) -> dict[str, Any]:
|
|
marked: dict[str, Any] = {}
|
|
monkeypatch.setattr(
|
|
task_mod.runs_mod,
|
|
"mark_done",
|
|
lambda _db, run_id, counters: marked.update(
|
|
kind="done", run_id=run_id, counters=dict(counters)
|
|
),
|
|
)
|
|
monkeypatch.setattr(
|
|
task_mod.runs_mod,
|
|
"mark_failed",
|
|
lambda _db, run_id, err, counters: marked.update(
|
|
kind="failed", run_id=run_id, err=err, counters=dict(counters)
|
|
),
|
|
)
|
|
return marked
|
|
|
|
|
|
# ── batching behaviour ──────────────────────────────────────────────────────
|
|
|
|
|
|
def test_stops_when_batch_below_size(monkeypatch: pytest.MonkeyPatch) -> None:
|
|
marked = _patch_runs(monkeypatch)
|
|
# estimates: one batch of 3 (< batch_size=10) -> stop. leads: one batch of 0 -> stop.
|
|
db = _FakeDB([3, 0])
|
|
out = task_mod.purge_expired_trade_in_data(db, run_id=1, batch_size=10, max_batches=20) # type: ignore[arg-type]
|
|
assert out == {"estimates_deleted": 3, "leads_deleted": 0}
|
|
assert len(db.executed) == 2
|
|
assert db.commits == 2
|
|
assert marked["counters"] == out
|
|
|
|
|
|
def test_loops_until_below_batch_size(monkeypatch: pytest.MonkeyPatch) -> None:
|
|
_patch_runs(monkeypatch)
|
|
# estimates: 5,5,2 (batch_size=5) -> 12 total, 3 batches. leads: 5,1 -> 6 total, 2 batches.
|
|
db = _FakeDB([5, 5, 2, 5, 1])
|
|
out = task_mod.purge_expired_trade_in_data(db, run_id=2, batch_size=5, max_batches=20) # type: ignore[arg-type]
|
|
assert out == {"estimates_deleted": 12, "leads_deleted": 6}
|
|
assert len(db.executed) == 5
|
|
assert db.commits == 5, "each batch must commit independently, not one final commit"
|
|
|
|
|
|
def test_respects_max_batches_cap(monkeypatch: pytest.MonkeyPatch) -> None:
|
|
"""Every batch returns a FULL batch_size (never 'caught up') -- only the
|
|
max_batches safety cap can stop the loop. Proves the cap is enforced, not
|
|
just coincidentally matching a 'caught up' condition."""
|
|
_patch_runs(monkeypatch)
|
|
db = _FakeDB([5, 5, 5, 5, 5, 5]) # exactly max_batches=3 per table, no more
|
|
out = task_mod.purge_expired_trade_in_data(db, run_id=3, batch_size=5, max_batches=3) # type: ignore[arg-type]
|
|
assert out == {"estimates_deleted": 15, "leads_deleted": 15}
|
|
assert len(db.executed) == 6 # 3 (estimates) + 3 (leads), NOT unbounded
|
|
|
|
|
|
def test_default_batch_size_and_max_batches_from_settings(monkeypatch: pytest.MonkeyPatch) -> None:
|
|
_patch_runs(monkeypatch)
|
|
db = _FakeDB([0, 0]) # first batch already empty on both tables -> stop immediately
|
|
task_mod.purge_expired_trade_in_data(db, run_id=4) # type: ignore[arg-type]
|
|
_stmt, params = db.executed[0]
|
|
assert params is not None
|
|
assert params["batch_size"] == task_mod.settings.trade_in_purge_batch_size
|
|
|
|
|
|
# ── table coverage / SQL shape ──────────────────────────────────────────────
|
|
|
|
|
|
def test_drains_both_tables_in_order(monkeypatch: pytest.MonkeyPatch) -> None:
|
|
_patch_runs(monkeypatch)
|
|
db = _FakeDB([0, 0])
|
|
task_mod.purge_expired_trade_in_data(db, run_id=5, batch_size=100, max_batches=1) # type: ignore[arg-type]
|
|
first_sql = str(getattr(db.executed[0][0], "text", db.executed[0][0]))
|
|
second_sql = str(getattr(db.executed[1][0], "text", db.executed[1][0]))
|
|
assert "trade_in_estimates" in first_sql
|
|
assert "trade_in_leads" in second_sql
|
|
|
|
|
|
def test_estimates_sql_is_delete_not_update() -> None:
|
|
sql = task_mod._DELETE_EXPIRED_ESTIMATES_SQL.text
|
|
assert "DELETE FROM trade_in_estimates" in sql
|
|
assert "UPDATE" not in sql.upper()
|
|
assert "expires_at < NOW()" in sql
|
|
assert "ORDER BY expires_at" in sql
|
|
assert "LIMIT CAST(:batch_size AS int)" in sql
|
|
assert not re.search(r":\w+::", sql)
|
|
|
|
|
|
def test_leads_sql_is_delete_not_update() -> None:
|
|
sql = task_mod._DELETE_EXPIRED_LEADS_SQL.text
|
|
assert "DELETE FROM trade_in_leads" in sql
|
|
assert "UPDATE" not in sql.upper()
|
|
assert "expires_at < NOW()" in sql
|
|
assert "ORDER BY expires_at" in sql
|
|
assert not re.search(r":\w+::", sql)
|
|
|
|
|
|
def test_sql_does_not_delete_whole_table_unbounded() -> None:
|
|
"""Neither statement is a bare `DELETE FROM table` -- both scope via a
|
|
subselect + LIMIT batch."""
|
|
statements = (
|
|
task_mod._DELETE_EXPIRED_ESTIMATES_SQL.text,
|
|
task_mod._DELETE_EXPIRED_LEADS_SQL.text,
|
|
)
|
|
for sql in statements:
|
|
assert "WHERE id IN (" in sql
|
|
assert "LIMIT" in sql
|
|
|
|
|
|
# ── failure path ─────────────────────────────────────────────────────────────
|
|
|
|
|
|
def test_failure_path_rollback_and_mark_failed(monkeypatch: pytest.MonkeyPatch) -> None:
|
|
marked = _patch_runs(monkeypatch)
|
|
|
|
class _BoomDB(_FakeDB):
|
|
def execute(self, stmt: Any, params: dict[str, Any] | None = None) -> _FakeResult:
|
|
# First batch (estimates) succeeds and commits; second call (still
|
|
# draining estimates, or first leads call) explodes.
|
|
if len(self.executed) >= 1:
|
|
raise RuntimeError("db exploded")
|
|
return super().execute(stmt, params)
|
|
|
|
db = _BoomDB([5]) # only ONE successful batch before the boom
|
|
with pytest.raises(RuntimeError, match="db exploded"):
|
|
task_mod.purge_expired_trade_in_data(db, run_id=6, batch_size=5, max_batches=20) # type: ignore[arg-type]
|
|
|
|
assert db.rolled_back is True
|
|
assert marked["kind"] == "failed"
|
|
assert marked["run_id"] == 6
|
|
# Partial progress preserved in the counters passed to mark_failed (first
|
|
# estimates batch of 5 already committed before the boom on the 2nd call).
|
|
assert marked["counters"]["estimates_deleted"] == 5
|
|
assert marked["counters"]["leads_deleted"] == 0
|
|
|
|
|
|
def test_idempotent_zero_rowcount_is_not_an_error(monkeypatch: pytest.MonkeyPatch) -> None:
|
|
"""Re-running against an already-drained backlog just deletes 0 rows, cleanly."""
|
|
marked = _patch_runs(monkeypatch)
|
|
db = _FakeDB([0, 0])
|
|
out = task_mod.purge_expired_trade_in_data(db, run_id=7, batch_size=500, max_batches=20) # type: ignore[arg-type]
|
|
assert out == {"estimates_deleted": 0, "leads_deleted": 0}
|
|
assert marked["kind"] == "done"
|
|
|
|
|
|
# ── migration 193 ────────────────────────────────────────────────────────────
|
|
|
|
|
|
def test_migration_193_exists() -> None:
|
|
assert _MIGRATION_193.is_file(), f"missing migration: {_MIGRATION_193}"
|
|
|
|
|
|
def test_migration_193_is_transactional() -> None:
|
|
sql = _MIGRATION_193.read_text("utf-8")
|
|
assert "BEGIN;" in sql
|
|
assert "COMMIT;" in sql
|
|
|
|
|
|
def test_migration_193_backfills_and_sets_not_null() -> None:
|
|
sql = _MIGRATION_193.read_text("utf-8")
|
|
assert "ADD COLUMN IF NOT EXISTS expires_at" in sql
|
|
assert "WHERE expires_at IS NULL" in sql
|
|
assert "SET NOT NULL" in sql
|
|
assert "180 days" in sql
|
|
|
|
|
|
def test_migration_193_seeds_purge_schedule_disabled_by_default() -> None:
|
|
sql = _MIGRATION_193.read_text("utf-8")
|
|
assert "'purge_expired_trade_in_data'" in sql
|
|
assert "ON CONFLICT (source) DO NOTHING" in sql
|
|
# Seeded disabled -- first automated PII-DELETE job in trade-in deserves a
|
|
# supervised first run before the scheduler can trigger it unattended.
|
|
assert re.search(r"'purge_expired_trade_in_data',\s*\n\s*false,", sql)
|
|
|
|
|
|
def test_migration_193_no_psycopg_trap() -> None:
|
|
sql = _MIGRATION_193.read_text("utf-8")
|
|
assert not re.search(r":\w+::", sql)
|