Compare commits
No commits in common. "4c8c02cce53313ca8a007fd0662a997389ad7c21" and "dfe7910bb77322477a762218d8debf7a9a42901f" have entirely different histories.
4c8c02cce5
...
dfe7910bb7
5 changed files with 7 additions and 485 deletions
|
|
@ -62,7 +62,6 @@ import hashlib
|
||||||
import logging
|
import logging
|
||||||
import re
|
import re
|
||||||
import secrets
|
import secrets
|
||||||
import time
|
|
||||||
from datetime import UTC, datetime
|
from datetime import UTC, datetime
|
||||||
from typing import Annotated, Literal
|
from typing import Annotated, Literal
|
||||||
|
|
||||||
|
|
@ -171,14 +170,6 @@ _SEND_UNAVAILABLE_DETAIL = "Telegram сейчас недоступен. Попр
|
||||||
# треде отдавало бы ВЕСЬ лог переписки. См. `web_support_storage.list_messages`.
|
# треде отдавало бы ВЕСЬ лог переписки. См. `web_support_storage.list_messages`.
|
||||||
_LIST_MESSAGES_LIMIT = 200
|
_LIST_MESSAGES_LIMIT = 200
|
||||||
|
|
||||||
# Идемпотентность отправки (#3471 retry-storm) — см. `_resolve_idempotency_key`.
|
|
||||||
_IDEMPOTENCY_HEADER = "idempotency-key"
|
|
||||||
# Форма клиентского ключа — как у anon-токена (`_ANON_TOKEN_RE`): произвольная
|
|
||||||
# opaque-строка клиента, без пробелов/спецсимволов, которые попали бы в SQL-параметр
|
|
||||||
# как есть. Не матчится — считаем заголовок отсутствующим и уходим на fallback,
|
|
||||||
# а не пытаемся его "починить" (тот же принцип, что у `_read_anon_token`).
|
|
||||||
_CLIENT_IDEMPOTENCY_KEY_RE = re.compile(r"^[A-Za-z0-9_.-]{8,128}\Z")
|
|
||||||
|
|
||||||
|
|
||||||
def _require_username(request: Request) -> str:
|
def _require_username(request: Request) -> str:
|
||||||
"""Достаёт X-Authenticated-User. rbac_guard (app/main.py) уже гарантирует его
|
"""Достаёт X-Authenticated-User. rbac_guard (app/main.py) уже гарантирует его
|
||||||
|
|
@ -354,51 +345,8 @@ def _rollback_quietly(db: Session) -> None:
|
||||||
logger.warning("web support: rollback после сбоя БД тоже не удался", exc_info=True)
|
logger.warning("web support: rollback после сбоя БД тоже не удался", exc_info=True)
|
||||||
|
|
||||||
|
|
||||||
def _resolve_idempotency_key(request: Request, *, identity_key: str, text: str) -> str:
|
|
||||||
"""Ключ идемпотентности inbound-отправки (#3471, миграция 301).
|
|
||||||
|
|
||||||
Почему заголовок + fallback, а не что-то одно. Явный `Idempotency-Key` —
|
|
||||||
ЕДИНСТВЕННЫЙ надёжный путь: клиент генерирует ключ ОДИН раз на "намерение
|
|
||||||
отправить" и переиспользует его на любом ретрае (fetch retry / переотправка
|
|
||||||
после таймаута) независимо от того, что именно менялось в UI между
|
|
||||||
попытками (см. `useSendSupportMessage` во фронтенде — реальный трафик
|
|
||||||
обязан идти этим путём). Fallback без заголовка — детерминированный
|
|
||||||
отпечаток sha256(identity, текст, минутное окно) — существует ТОЛЬКО для
|
|
||||||
клиентов, которые заголовок не прислали (п.5 требования — старое поведение
|
|
||||||
не должно сломаться), и у него два честных изъяна, оба снимаются самим
|
|
||||||
фактом использования заголовка:
|
|
||||||
1. два РАЗНЫХ по смыслу сообщения с одинаковым текстом от одного и того
|
|
||||||
же человека в течение одной минуты ("да" и ещё раз "да", "+", "ок")
|
|
||||||
СХЛОПНУТСЯ в одно — второе будет молча проглочено, клиент получит id
|
|
||||||
первого, оператор не увидит второе сообщение вообще;
|
|
||||||
2. окно — не скользящие 60 секунд, а округление `time.time() // 60` вниз:
|
|
||||||
фактический срок дедупликации случаен от 0 до 60с в зависимости от
|
|
||||||
момента внутри минуты, и часть настоящих повторов (ретрай ровно на
|
|
||||||
границе окна) эту защиту не получит.
|
|
||||||
Оба пункта перестают быть важны, когда клиент реально шлёт заголовок
|
|
||||||
(см. `useSendSupportMessage`).
|
|
||||||
|
|
||||||
Хранит и потенциально логирует (см. вызовы в этом файле) только САМ ключ —
|
|
||||||
он либо непрозрачный клиентский токен, либо хэш. Текст сообщения сюда
|
|
||||||
попадает ТОЛЬКО как вход в sha256, в открытом виде не сохраняется и не
|
|
||||||
возвращается — жёсткое правило проекта "текст не в логах" (ПДн) при этом
|
|
||||||
не нарушается, даже если бы этот ключ где-то залогировали.
|
|
||||||
|
|
||||||
Префиксы `client:`/`auto:` разводят два namespace'а ключей (защита от
|
|
||||||
случайного совпадения клиентского токена с fallback-хэшем) — дёшево и не
|
|
||||||
требует отдельной колонки.
|
|
||||||
"""
|
|
||||||
header = (request.headers.get(_IDEMPOTENCY_HEADER) or "").strip()
|
|
||||||
if header and _CLIENT_IDEMPOTENCY_KEY_RE.match(header):
|
|
||||||
return f"client:{header}"
|
|
||||||
window = int(time.time() // 60)
|
|
||||||
fingerprint = f"{identity_key}|{text}|{window}"
|
|
||||||
return "auto:" + hashlib.sha256(fingerprint.encode("utf-8")).hexdigest()
|
|
||||||
|
|
||||||
|
|
||||||
@router.post("/support/messages", response_model=SupportMessageOut)
|
@router.post("/support/messages", response_model=SupportMessageOut)
|
||||||
async def send_support_message(
|
async def send_support_message(
|
||||||
request: Request,
|
|
||||||
payload: SupportMessageInput,
|
payload: SupportMessageInput,
|
||||||
username: Annotated[str, Depends(_require_username)],
|
username: Annotated[str, Depends(_require_username)],
|
||||||
db: Annotated[Session, Depends(get_db)],
|
db: Annotated[Session, Depends(get_db)],
|
||||||
|
|
@ -432,37 +380,6 @@ async def send_support_message(
|
||||||
if cooldown is not None:
|
if cooldown is not None:
|
||||||
raise _too_many_failures_error(cooldown)
|
raise _too_many_failures_error(cooldown)
|
||||||
|
|
||||||
# Идемпотентность (#3471). Что именно закрывает этот pre-check — важно не
|
|
||||||
# переоценить: строка в БД (и её idempotency_key) появляется ТОЛЬКО ПОСЛЕ
|
|
||||||
# успешной доставки в Telegram (H1 ниже), поэтому потерю самого запроса на
|
|
||||||
# плече Selectel -> api.telegram.org этот механизм НЕ дедуплицирует — если
|
|
||||||
# `send_message` не удался, ключ нигде не записан, и повтор клиента после
|
|
||||||
# неудачи это законная первая попытка. Реально закрывается другой, тоже
|
|
||||||
# частый случай: доставка УЖЕ состоялась (Telegram принял, строка
|
|
||||||
# закоммичена), но ответ до клиента не дошёл (обрыв на обратном пути,
|
|
||||||
# клиентский таймаут) или пользователь кликнул "отправить" второй раз по
|
|
||||||
# той же ещё не отрисовавшейся отправке — тогда pre-check находит уже
|
|
||||||
# записанное сообщение по ключу и отдаёт его же, не уходя в Telegram снова.
|
|
||||||
# Тред может ещё не существовать (это первая попытка этого ключа) — тогда
|
|
||||||
# сравнивать не с чем, идём в Telegram как обычно; сам факт "нет треда"
|
|
||||||
# здесь безопасен, т.к. тред создаётся ТОЛЬКО в этой же ручке (см. H1) —
|
|
||||||
# если бы сообщение под этим ключом уже было записано, тред уже был бы.
|
|
||||||
idempotency_key = _resolve_idempotency_key(request, identity_key=username, text=payload.text)
|
|
||||||
existing_thread_id = storage.find_thread_id(db, username)
|
|
||||||
if existing_thread_id is not None:
|
|
||||||
existing = storage.find_inbound_by_idempotency_key(
|
|
||||||
db, thread_id=existing_thread_id, idempotency_key=idempotency_key
|
|
||||||
)
|
|
||||||
if existing is not None:
|
|
||||||
logger.info(
|
|
||||||
"web support: repeat send (idempotency key already recorded) "
|
|
||||||
"username=%s thread_id=%d message_id=%s",
|
|
||||||
username,
|
|
||||||
existing_thread_id,
|
|
||||||
existing["id"],
|
|
||||||
)
|
|
||||||
return SupportMessageOut(**existing)
|
|
||||||
|
|
||||||
# Общий клиент приложения (#tg-connection-resilience): на каждый запрос
|
# Общий клиент приложения (#tg-connection-resilience): на каждый запрос
|
||||||
# свой создавать нельзя — это ноль keep-alive и полный TCP+TLS-хендшейк
|
# свой создавать нельзя — это ноль keep-alive и полный TCP+TLS-хендшейк
|
||||||
# до api.telegram.org перед каждой отправкой. Живёт в lifespan.
|
# до api.telegram.org перед каждой отправкой. Живёт в lifespan.
|
||||||
|
|
@ -521,7 +438,6 @@ async def send_support_message(
|
||||||
text_body=payload.text,
|
text_body=payload.text,
|
||||||
topic_message_id=topic_message_id,
|
topic_message_id=topic_message_id,
|
||||||
support_chat_id=settings.telegram_support_chat_id,
|
support_chat_id=settings.telegram_support_chat_id,
|
||||||
idempotency_key=idempotency_key,
|
|
||||||
)
|
)
|
||||||
db.commit()
|
db.commit()
|
||||||
except SQLAlchemyError:
|
except SQLAlchemyError:
|
||||||
|
|
@ -692,31 +608,6 @@ async def send_anon_support_message(
|
||||||
raise _too_many_failures_error(cooldown)
|
raise _too_many_failures_error(cooldown)
|
||||||
|
|
||||||
display_id = _anon_display_id(token)
|
display_id = _anon_display_id(token)
|
||||||
|
|
||||||
# Идемпотентность (#3471) — см. развёрнутый комментарий в авторизованной
|
|
||||||
# ветке. Ограничение специфично для анонимной ветки: если предыдущая
|
|
||||||
# попытка сама провалилась ДО выдачи куки (`is_new_token=True` тогда и
|
|
||||||
# сейчас), identity_key каждый раз новый (случайный токен) и pre-check
|
|
||||||
# структурно не может найти прошлую попытку — это тот же класс проблемы,
|
|
||||||
# что и потеря куки в сети, вне scope этой правки.
|
|
||||||
idempotency_key = _resolve_idempotency_key(request, identity_key=thread_key, text=payload.text)
|
|
||||||
existing_thread_id = storage.find_thread_id(db, thread_key)
|
|
||||||
if existing_thread_id is not None:
|
|
||||||
existing = storage.find_inbound_by_idempotency_key(
|
|
||||||
db, thread_id=existing_thread_id, idempotency_key=idempotency_key
|
|
||||||
)
|
|
||||||
if existing is not None:
|
|
||||||
logger.info(
|
|
||||||
"web support (anon): repeat send (idempotency key already recorded) "
|
|
||||||
"%s thread_id=%d message_id=%s",
|
|
||||||
display_id,
|
|
||||||
existing_thread_id,
|
|
||||||
existing["id"],
|
|
||||||
)
|
|
||||||
if is_new_token:
|
|
||||||
_set_anon_cookie(response, token)
|
|
||||||
return SupportMessageOut(**existing)
|
|
||||||
|
|
||||||
# Общий клиент приложения (#tg-connection-resilience): на каждый запрос
|
# Общий клиент приложения (#tg-connection-resilience): на каждый запрос
|
||||||
# свой создавать нельзя — это ноль keep-alive и полный TCP+TLS-хендшейк
|
# свой создавать нельзя — это ноль keep-alive и полный TCP+TLS-хендшейк
|
||||||
# до api.telegram.org перед каждой отправкой. Живёт в lifespan.
|
# до api.telegram.org перед каждой отправкой. Живёт в lifespan.
|
||||||
|
|
@ -766,7 +657,6 @@ async def send_anon_support_message(
|
||||||
text_body=payload.text,
|
text_body=payload.text,
|
||||||
topic_message_id=topic_message_id,
|
topic_message_id=topic_message_id,
|
||||||
support_chat_id=settings.telegram_support_chat_id,
|
support_chat_id=settings.telegram_support_chat_id,
|
||||||
idempotency_key=idempotency_key,
|
|
||||||
)
|
)
|
||||||
db.commit()
|
db.commit()
|
||||||
except SQLAlchemyError:
|
except SQLAlchemyError:
|
||||||
|
|
|
||||||
|
|
@ -61,36 +61,6 @@ def get_or_create_thread(db: Session, username: str) -> int:
|
||||||
return int(row[0])
|
return int(row[0])
|
||||||
|
|
||||||
|
|
||||||
def find_inbound_by_idempotency_key(
|
|
||||||
db: Session, *, thread_id: int, idempotency_key: str
|
|
||||||
) -> dict[str, Any] | None:
|
|
||||||
"""Уже записанное inbound-сообщение с этим ключом идемпотентности в треде,
|
|
||||||
если есть (#3471). Вызывается ИЗ `app.api.v1.support` ДО похода в Telegram
|
|
||||||
(`send_support_message` / `send_anon_support_message`) — повтор с тем же
|
|
||||||
ключом не должен создавать второе зеркало в топике, а не только вторую
|
|
||||||
строку в БД. `thread_id`, а не username/anon-token: таблица не хранит
|
|
||||||
identity напрямую, а тред уже гарантированно существует к моменту, когда
|
|
||||||
этот ключ мог быть записан (тред создаётся ДО `record_inbound`, см. H1 в
|
|
||||||
докстринге `app.api.v1.support`)."""
|
|
||||||
row = (
|
|
||||||
db.execute(
|
|
||||||
text(
|
|
||||||
"""
|
|
||||||
SELECT id, direction, text_body, operator_tg_id, created_at
|
|
||||||
FROM web_support_messages
|
|
||||||
WHERE thread_id = CAST(:thread_id AS bigint)
|
|
||||||
AND direction = 'in'
|
|
||||||
AND idempotency_key = :idempotency_key
|
|
||||||
"""
|
|
||||||
),
|
|
||||||
{"thread_id": thread_id, "idempotency_key": idempotency_key},
|
|
||||||
)
|
|
||||||
.mappings()
|
|
||||||
.one_or_none()
|
|
||||||
)
|
|
||||||
return dict(row) if row is not None else None
|
|
||||||
|
|
||||||
|
|
||||||
def record_inbound(
|
def record_inbound(
|
||||||
db: Session,
|
db: Session,
|
||||||
*,
|
*,
|
||||||
|
|
@ -98,7 +68,6 @@ def record_inbound(
|
||||||
text_body: str,
|
text_body: str,
|
||||||
topic_message_id: int | None,
|
topic_message_id: int | None,
|
||||||
support_chat_id: int | None,
|
support_chat_id: int | None,
|
||||||
idempotency_key: str | None = None,
|
|
||||||
) -> dict[str, Any]:
|
) -> dict[str, Any]:
|
||||||
"""Записывает сообщение пользователя сайта (direction='in'). `topic_message_id` —
|
"""Записывает сообщение пользователя сайта (direction='in'). `topic_message_id` —
|
||||||
id зеркала (sendMessage) в support-топике, ключ маршрутизации ответа оператора.
|
id зеркала (sendMessage) в support-топике, ключ маршрутизации ответа оператора.
|
||||||
|
|
@ -106,103 +75,18 @@ def record_inbound(
|
||||||
review M1): скоупит будущий резолв `find_thread_by_topic_message` к ТЕКУЩЕЙ
|
review M1): скоупит будущий резолв `find_thread_by_topic_message` к ТЕКУЩЕЙ
|
||||||
support-группе — если группу когда-нибудь сменят/пересоздадут, Telegram
|
support-группе — если группу когда-нибудь сменят/пересоздадут, Telegram
|
||||||
message_id стартует заново с 1 в новом чате и может совпасть с числом из
|
message_id стартует заново с 1 в новом чате и может совпасть с числом из
|
||||||
старого — без этого поля коллизия была бы ТИХОЙ (см. миграцию 187/188).
|
старого — без этого поля коллизия была бы ТИХОЙ (см. миграцию 187/188)."""
|
||||||
|
|
||||||
`idempotency_key` (#3471, миграция 301) — вызывающая сторона (`app.api.v1.support`)
|
|
||||||
делает SELECT-затем-действие pre-check ДО Telegram-похода (см.
|
|
||||||
`find_inbound_by_idempotency_key`), но этот pre-check САМ ПО СЕБЕ гонку не
|
|
||||||
закрывает (TOCTOU): два запроса с одним ключом могут пройти его одновременно
|
|
||||||
и оба уйти в Telegram. Последняя линия защиты — здесь: `INSERT ... ON
|
|
||||||
CONFLICT (thread_id, idempotency_key) DO NOTHING` на partial unique индексе
|
|
||||||
(мигр. 301, тот же predicate). Если конфликт всё же случился, проигравший
|
|
||||||
НЕ создаёт вторую строку — читает уже вставленную и возвращает её, так что
|
|
||||||
оба запроса-конкурента получают ОДИН и тот же id. `idempotency_key=None`
|
|
||||||
(дефолт) ведёт себя как раньше: NULL никогда не конфликтует сам с собой в
|
|
||||||
partial-индексе (WHERE idempotency_key IS NOT NULL), INSERT всегда проходит."""
|
|
||||||
row = (
|
row = (
|
||||||
db.execute(
|
db.execute(
|
||||||
text(
|
text(
|
||||||
"""
|
"""
|
||||||
INSERT INTO web_support_messages
|
INSERT INTO web_support_messages
|
||||||
(thread_id, direction, text_body, topic_message_id,
|
(thread_id, direction, text_body, topic_message_id,
|
||||||
support_chat_id, operator_tg_id, idempotency_key, created_at)
|
support_chat_id, operator_tg_id, created_at)
|
||||||
VALUES
|
VALUES
|
||||||
(CAST(:thread_id AS bigint), 'in', :text_body,
|
(CAST(:thread_id AS bigint), 'in', :text_body,
|
||||||
CAST(:topic_message_id AS bigint),
|
CAST(:topic_message_id AS bigint),
|
||||||
CAST(:support_chat_id AS bigint), NULL, :idempotency_key, NOW())
|
CAST(:support_chat_id AS bigint), NULL, NOW())
|
||||||
ON CONFLICT (thread_id, idempotency_key)
|
|
||||||
WHERE idempotency_key IS NOT NULL AND direction = 'in'
|
|
||||||
DO NOTHING
|
|
||||||
RETURNING id, direction, text_body, operator_tg_id, created_at
|
|
||||||
"""
|
|
||||||
),
|
|
||||||
{
|
|
||||||
"thread_id": thread_id,
|
|
||||||
"text_body": text_body,
|
|
||||||
"topic_message_id": topic_message_id,
|
|
||||||
"support_chat_id": support_chat_id,
|
|
||||||
"idempotency_key": idempotency_key,
|
|
||||||
},
|
|
||||||
)
|
|
||||||
.mappings()
|
|
||||||
.one_or_none()
|
|
||||||
)
|
|
||||||
if row is not None:
|
|
||||||
return dict(row)
|
|
||||||
|
|
||||||
# Конфликт пойман индексом — idempotency_key почти наверняка не NULL здесь
|
|
||||||
# (при NULL partial-индекс в конфликт не участвует), но НЕ `assert`: это
|
|
||||||
# прод-путь ПОСЛЕ уже доставленного в Telegram сообщения (см. H1 в
|
|
||||||
# app.api.v1.support), а `except SQLAlchemyError` в вызывающей стороне
|
|
||||||
# `AssertionError` не ловит — под `python -O` assert вдобавок исчезает
|
|
||||||
# молча и `existing` осталось бы `None`, что развалилось бы чуть ниже при
|
|
||||||
# сборке ответа. Вместо падения — явная ветка с логом и WORKING откатом.
|
|
||||||
existing = (
|
|
||||||
find_inbound_by_idempotency_key(db, thread_id=thread_id, idempotency_key=idempotency_key)
|
|
||||||
if idempotency_key is not None
|
|
||||||
else None
|
|
||||||
)
|
|
||||||
if existing is not None:
|
|
||||||
# Ожидаемый случай гонки (#3471): проигравший запрос уже отправил своё
|
|
||||||
# СОБСТВЕННОЕ зеркало в Telegram (свой topic_message_id) до того, как
|
|
||||||
# обнаружил конфликт здесь — эта копия зеркала теперь осиротела
|
|
||||||
# (реплай оператора на неё никуда не смаршрутизируется, т.к. строки
|
|
||||||
# для неё в БД нет). Осознанно не чиним это в этом PR (см. коммент
|
|
||||||
# выше по коду), но фиксируем в логе, а не молчим — ровно то же самое,
|
|
||||||
# для чего уже есть `_warn_operator_message_not_recorded` в другом месте.
|
|
||||||
logger.warning(
|
|
||||||
"web support: гонка по idempotency_key — зеркало topic_message_id=%s "
|
|
||||||
"(thread_id=%d) отправлено, но НЕ записано, победила строка id=%d",
|
|
||||||
topic_message_id,
|
|
||||||
thread_id,
|
|
||||||
existing["id"],
|
|
||||||
)
|
|
||||||
return existing
|
|
||||||
|
|
||||||
# Крайний случай (в норме недостижим при READ COMMITTED, которую использует
|
|
||||||
# этот сервис): индекс сообщил о конфликте, но повторное чтение строку не
|
|
||||||
# нашло. Логируем и пишем БЕЗ идемпотентности — NULL-ключ никогда не
|
|
||||||
# конфликтует сам с собой (partial-индекс его не видит), INSERT гарантированно
|
|
||||||
# пройдёт; для этого одного сообщения дедупликация отключается, но клиент
|
|
||||||
# получает корректный ответ вместо 500 после уже состоявшейся доставки.
|
|
||||||
logger.warning(
|
|
||||||
"web support: ON CONFLICT сообщил о конфликте (thread_id=%d, "
|
|
||||||
"idempotency_key=%s), но повторное чтение строки её не нашло — "
|
|
||||||
"записываем без идемпотентности",
|
|
||||||
thread_id,
|
|
||||||
"<set>" if idempotency_key is not None else None,
|
|
||||||
)
|
|
||||||
row = (
|
|
||||||
db.execute(
|
|
||||||
text(
|
|
||||||
"""
|
|
||||||
INSERT INTO web_support_messages
|
|
||||||
(thread_id, direction, text_body, topic_message_id,
|
|
||||||
support_chat_id, operator_tg_id, idempotency_key, created_at)
|
|
||||||
VALUES
|
|
||||||
(CAST(:thread_id AS bigint), 'in', :text_body,
|
|
||||||
CAST(:topic_message_id AS bigint),
|
|
||||||
CAST(:support_chat_id AS bigint), NULL, NULL, NOW())
|
|
||||||
RETURNING id, direction, text_body, operator_tg_id, created_at
|
RETURNING id, direction, text_body, operator_tg_id, created_at
|
||||||
"""
|
"""
|
||||||
),
|
),
|
||||||
|
|
|
||||||
|
|
@ -1,59 +0,0 @@
|
||||||
-- 301_web_support_message_idempotency_key.sql
|
|
||||||
-- Идемпотентность отправки веб-сообщения в поддержку (#3471 retry-storm):
|
|
||||||
-- канал Selectel -> api.telegram.org теряет заметную долю коротких запросов
|
|
||||||
-- (см. app/api/v1/support.py, _INTERACTIVE_SEND_TIMEOUT_S), поэтому повтор
|
|
||||||
-- клиента (fetch-ретрай/двойной клик/переотправка по таймауту) — обычное
|
|
||||||
-- дело. Раньше повтор создавал ВТОРУЮ строку в web_support_messages и ВТОРОЕ
|
|
||||||
-- зеркало в support-топике.
|
|
||||||
--
|
|
||||||
-- idempotency_key — client-provided (заголовок Idempotency-Key) ИЛИ
|
|
||||||
-- детерминированный fallback-отпечаток sha256(identity|текст|минутное окно)
|
|
||||||
-- для клиентов без заголовка (app/api/v1/support.py::_resolve_idempotency_key).
|
|
||||||
-- Всегда opaque-строка (клиентский токен или hex-хэш) — ТЕЛО СООБЩЕНИЯ сюда
|
|
||||||
-- никогда не попадает в открытом виде, только его отпечаток.
|
|
||||||
--
|
|
||||||
-- Scoped к (thread_id, direction='in'): идемпотентность осмысленна только для
|
|
||||||
-- inbound (клиентских) сообщений — direction='out' (ответ оператора) её не
|
|
||||||
-- требует, там уже есть своя partial-уникальность на topic_message_id (187).
|
|
||||||
-- thread_id, а не username/anon-token: таблица не хранит identity напрямую,
|
|
||||||
-- а тред уже гарантированно создан (`get_or_create_thread`) к моменту INSERT
|
|
||||||
-- (см. H1 в app/api/v1/support.py — тред создаётся ДО record_inbound).
|
|
||||||
--
|
|
||||||
-- Гонку двух одновременных запросов с одним ключом закрывает САМ уникальный
|
|
||||||
-- индекс + `INSERT ... ON CONFLICT DO NOTHING` в
|
|
||||||
-- web_support_storage.record_inbound (не read-then-write) — на конфликте
|
|
||||||
-- вызывающая сторона дочитывает уже вставленную строку и возвращает её тем
|
|
||||||
-- же ответом (тот же id), а не создаёт вторую строку.
|
|
||||||
--
|
|
||||||
-- IDEMPOTENCY: ADD COLUMN/CREATE INDEX IF NOT EXISTS — безопасный re-run.
|
|
||||||
-- Зависимости: 187_web_support_chat.sql (таблица web_support_messages).
|
|
||||||
|
|
||||||
BEGIN;
|
|
||||||
|
|
||||||
-- Блокирующий DDL не должен ждать чужую сессию бесконечно: без этого
|
|
||||||
-- ALTER встаёт в очередь за долгим запросом и уводит за собой ВСЕ
|
|
||||||
-- последующие обращения к таблице (#2752). Пять секунд — не успел взять
|
|
||||||
-- лок, деплой падает честно, а прод продолжает работать.
|
|
||||||
SET LOCAL lock_timeout = '5s';
|
|
||||||
|
|
||||||
ALTER TABLE web_support_messages
|
|
||||||
ADD COLUMN IF NOT EXISTS idempotency_key text;
|
|
||||||
|
|
||||||
DO $$
|
|
||||||
BEGIN
|
|
||||||
IF NOT EXISTS (
|
|
||||||
SELECT 1 FROM pg_constraint WHERE conname = 'web_support_messages_idempotency_key_len_chk'
|
|
||||||
) THEN
|
|
||||||
ALTER TABLE web_support_messages
|
|
||||||
ADD CONSTRAINT web_support_messages_idempotency_key_len_chk
|
|
||||||
CHECK (idempotency_key IS NULL OR char_length(idempotency_key) BETWEEN 1 AND 160);
|
|
||||||
END IF;
|
|
||||||
END $$;
|
|
||||||
|
|
||||||
COMMENT ON COLUMN web_support_messages.idempotency_key IS 'Ключ идемпотентности inbound-отправки (#3471) — client-provided заголовок Idempotency-Key ("client:...") ИЛИ sha256-отпечаток thread+текст+минутное окно ("auto:...") для клиентов без заголовка. NULL для direction=''out'' и для строк до этой миграции.';
|
|
||||||
|
|
||||||
CREATE UNIQUE INDEX IF NOT EXISTS web_support_messages_thread_idempotency_uq
|
|
||||||
ON web_support_messages (thread_id, idempotency_key)
|
|
||||||
WHERE idempotency_key IS NOT NULL AND direction = 'in';
|
|
||||||
|
|
||||||
COMMIT;
|
|
||||||
|
|
@ -76,20 +76,6 @@ class _FakeTelegramClient:
|
||||||
return _FakeTelegramClient._response
|
return _FakeTelegramClient._response
|
||||||
|
|
||||||
|
|
||||||
@pytest.fixture(autouse=True)
|
|
||||||
def _no_existing_idempotent_message(monkeypatch: pytest.MonkeyPatch) -> None:
|
|
||||||
"""Дефолт для тестов, которые не проверяют идемпотентность (#3471) напрямую:
|
|
||||||
без этого автостаба pre-check в `support.py` дёргал бы РЕАЛЬНУЮ
|
|
||||||
storage-функцию против MagicMock `db` в каждом тесте, где замокан
|
|
||||||
`find_thread_id` для своих целей (rate-limit / DB-failure / anon-сценарии
|
|
||||||
и т.д.) — падало бы на MagicMock-результате, никак не связанное с тем, что
|
|
||||||
тест на самом деле проверяет. Тесты идемпотентности переопределяют это
|
|
||||||
через `_install_fake_storage` ниже."""
|
|
||||||
monkeypatch.setattr(
|
|
||||||
support_module.storage, "find_inbound_by_idempotency_key", lambda *a, **kw: None
|
|
||||||
)
|
|
||||||
|
|
||||||
|
|
||||||
@pytest.fixture(autouse=True)
|
@pytest.fixture(autouse=True)
|
||||||
def _fake_telegram_client(monkeypatch: pytest.MonkeyPatch) -> Any:
|
def _fake_telegram_client(monkeypatch: pytest.MonkeyPatch) -> Any:
|
||||||
_FakeTelegramClient.calls = []
|
_FakeTelegramClient.calls = []
|
||||||
|
|
@ -252,9 +238,7 @@ def test_send_message_happy_path_mirrors_with_website_marker(
|
||||||
monkeypatch.setattr(support_module.storage, "get_or_create_thread", fake_get_or_create_thread)
|
monkeypatch.setattr(support_module.storage, "get_or_create_thread", fake_get_or_create_thread)
|
||||||
recorded = {}
|
recorded = {}
|
||||||
|
|
||||||
def fake_record_inbound(
|
def fake_record_inbound(db, *, thread_id, text_body, topic_message_id, support_chat_id):
|
||||||
db, *, thread_id, text_body, topic_message_id, support_chat_id, idempotency_key=None
|
|
||||||
):
|
|
||||||
recorded.update(
|
recorded.update(
|
||||||
thread_id=thread_id,
|
thread_id=thread_id,
|
||||||
text_body=text_body,
|
text_body=text_body,
|
||||||
|
|
@ -1068,156 +1052,3 @@ def test_anon_send_failures_trigger_cooldown_per_ip(
|
||||||
blocked = client.post("/api/v1/trade-in/support/anon/messages", json={"text": "2"})
|
blocked = client.post("/api/v1/trade-in/support/anon/messages", json={"text": "2"})
|
||||||
assert blocked.status_code == 429
|
assert blocked.status_code == 429
|
||||||
assert len(_fake_telegram_client.calls) == calls_before # до Telegram не дошло
|
assert len(_fake_telegram_client.calls) == calls_before # до Telegram не дошло
|
||||||
# ── idempotency (#3471) ──────────────────────────────────────────────────────
|
|
||||||
|
|
||||||
|
|
||||||
class _FakeIdempotentStorage:
|
|
||||||
"""In-memory stand-in для `web_support_storage`, достаточный чтобы точно
|
|
||||||
воспроизвести идемпотентный контракт (thread_id, idempotency_key) БЕЗ
|
|
||||||
настоящей БД: unique-конфликт на повторном ключе внутри `record_inbound`
|
|
||||||
(см. миграцию 301 и storage.record_inbound docstring)."""
|
|
||||||
|
|
||||||
def __init__(self) -> None:
|
|
||||||
self.threads: dict[str, int] = {}
|
|
||||||
self.messages: list[dict[str, Any]] = []
|
|
||||||
self._next_id = 1
|
|
||||||
|
|
||||||
def find_thread_id(self, db: Any, username: str) -> int | None:
|
|
||||||
return self.threads.get(username)
|
|
||||||
|
|
||||||
def get_or_create_thread(self, db: Any, username: str) -> int:
|
|
||||||
if username not in self.threads:
|
|
||||||
self.threads[username] = len(self.threads) + 1
|
|
||||||
return self.threads[username]
|
|
||||||
|
|
||||||
def find_inbound_by_idempotency_key(
|
|
||||||
self, db: Any, *, thread_id: int, idempotency_key: str
|
|
||||||
) -> dict[str, Any] | None:
|
|
||||||
for m in self.messages:
|
|
||||||
if m["_thread_id"] == thread_id and m["_idempotency_key"] == idempotency_key:
|
|
||||||
return {k: v for k, v in m.items() if not k.startswith("_")}
|
|
||||||
return None
|
|
||||||
|
|
||||||
def record_inbound(
|
|
||||||
self,
|
|
||||||
db: Any,
|
|
||||||
*,
|
|
||||||
thread_id: int,
|
|
||||||
text_body: str,
|
|
||||||
topic_message_id: int | None,
|
|
||||||
support_chat_id: int | None,
|
|
||||||
idempotency_key: str | None = None,
|
|
||||||
) -> dict[str, Any]:
|
|
||||||
# Тот же контракт, что и настоящий `INSERT ... ON CONFLICT DO NOTHING`:
|
|
||||||
# конфликт по (thread_id, idempotency_key) отдаёт УЖЕ существующую строку.
|
|
||||||
if idempotency_key is not None:
|
|
||||||
existing = self.find_inbound_by_idempotency_key(
|
|
||||||
db, thread_id=thread_id, idempotency_key=idempotency_key
|
|
||||||
)
|
|
||||||
if existing is not None:
|
|
||||||
return existing
|
|
||||||
row = {
|
|
||||||
"id": self._next_id,
|
|
||||||
"direction": "in",
|
|
||||||
"text_body": text_body,
|
|
||||||
"operator_tg_id": None,
|
|
||||||
"created_at": "2026-09-12T00:00:00+00:00",
|
|
||||||
"_thread_id": thread_id,
|
|
||||||
"_idempotency_key": idempotency_key,
|
|
||||||
}
|
|
||||||
self._next_id += 1
|
|
||||||
self.messages.append(row)
|
|
||||||
return {k: v for k, v in row.items() if not k.startswith("_")}
|
|
||||||
|
|
||||||
|
|
||||||
def _install_fake_storage(monkeypatch: pytest.MonkeyPatch) -> _FakeIdempotentStorage:
|
|
||||||
fake = _FakeIdempotentStorage()
|
|
||||||
monkeypatch.setattr(support_module.storage, "find_thread_id", fake.find_thread_id)
|
|
||||||
monkeypatch.setattr(support_module.storage, "get_or_create_thread", fake.get_or_create_thread)
|
|
||||||
monkeypatch.setattr(
|
|
||||||
support_module.storage,
|
|
||||||
"find_inbound_by_idempotency_key",
|
|
||||||
fake.find_inbound_by_idempotency_key,
|
|
||||||
)
|
|
||||||
monkeypatch.setattr(support_module.storage, "record_inbound", fake.record_inbound)
|
|
||||||
return fake
|
|
||||||
|
|
||||||
|
|
||||||
def test_repeat_send_with_same_idempotency_header_returns_same_id_single_mirror(
|
|
||||||
client: TestClient, monkeypatch: pytest.MonkeyPatch, _fake_telegram_client: Any
|
|
||||||
) -> None:
|
|
||||||
"""Повтор с тем же `Idempotency-Key` — тот же id, ОДНО зеркало в Telegram
|
|
||||||
(не только одна строка в БД, см. #3471 требование п.3)."""
|
|
||||||
_install_fake_storage(monkeypatch)
|
|
||||||
headers = {**_auth("kopylov"), "Idempotency-Key": "retry-abc123"}
|
|
||||||
|
|
||||||
r1 = client.post(
|
|
||||||
"/api/v1/trade-in/support/messages", json={"text": "первое сообщение"}, headers=headers
|
|
||||||
)
|
|
||||||
r2 = client.post(
|
|
||||||
"/api/v1/trade-in/support/messages", json={"text": "первое сообщение"}, headers=headers
|
|
||||||
)
|
|
||||||
|
|
||||||
assert r1.status_code == 200, r1.text
|
|
||||||
assert r2.status_code == 200, r2.text
|
|
||||||
assert r1.json()["id"] == r2.json()["id"]
|
|
||||||
# Ключевая проверка: повтор НЕ ушёл в Telegram второй раз.
|
|
||||||
assert len(_fake_telegram_client.calls) == 1
|
|
||||||
|
|
||||||
|
|
||||||
def test_send_with_different_idempotency_keys_creates_different_messages(
|
|
||||||
client: TestClient, monkeypatch: pytest.MonkeyPatch, _fake_telegram_client: Any
|
|
||||||
) -> None:
|
|
||||||
_install_fake_storage(monkeypatch)
|
|
||||||
r1 = client.post(
|
|
||||||
"/api/v1/trade-in/support/messages",
|
|
||||||
json={"text": "вопрос один"},
|
|
||||||
headers={**_auth("kopylov"), "Idempotency-Key": "key-one-111"},
|
|
||||||
)
|
|
||||||
r2 = client.post(
|
|
||||||
"/api/v1/trade-in/support/messages",
|
|
||||||
json={"text": "вопрос два"},
|
|
||||||
headers={**_auth("kopylov"), "Idempotency-Key": "key-two-222"},
|
|
||||||
)
|
|
||||||
|
|
||||||
assert r1.status_code == 200, r1.text
|
|
||||||
assert r2.status_code == 200, r2.text
|
|
||||||
assert r1.json()["id"] != r2.json()["id"]
|
|
||||||
assert len(_fake_telegram_client.calls) == 2
|
|
||||||
|
|
||||||
|
|
||||||
def test_repeat_send_without_idempotency_header_still_dedupes_via_fallback_window(
|
|
||||||
client: TestClient, monkeypatch: pytest.MonkeyPatch, _fake_telegram_client: Any
|
|
||||||
) -> None:
|
|
||||||
"""Требование п.5: отсутствие явного ключа не должно ломать старых клиентов —
|
|
||||||
сервер сам считает детерминированный fallback-отпечаток (thread+текст+минутное
|
|
||||||
окно), так что тот же повтор ТЕМ ЖЕ клиентом без заголовка тоже не создаёт
|
|
||||||
дубль. Время фиксируем, чтобы не зависеть от границы минутного окна в CI."""
|
|
||||||
_install_fake_storage(monkeypatch)
|
|
||||||
monkeypatch.setattr(support_module.time, "time", lambda: 1_800_000_000.0)
|
|
||||||
|
|
||||||
r1 = client.post(
|
|
||||||
"/api/v1/trade-in/support/messages", json={"text": "не могу войти"}, headers=_auth("bob")
|
|
||||||
)
|
|
||||||
r2 = client.post(
|
|
||||||
"/api/v1/trade-in/support/messages", json={"text": "не могу войти"}, headers=_auth("bob")
|
|
||||||
)
|
|
||||||
|
|
||||||
assert r1.status_code == 200, r1.text
|
|
||||||
assert r2.status_code == 200, r2.text
|
|
||||||
assert r1.json()["id"] == r2.json()["id"]
|
|
||||||
assert len(_fake_telegram_client.calls) == 1
|
|
||||||
|
|
||||||
|
|
||||||
def test_send_without_idempotency_header_first_call_still_succeeds(
|
|
||||||
client: TestClient, monkeypatch: pytest.MonkeyPatch, _fake_telegram_client: Any
|
|
||||||
) -> None:
|
|
||||||
"""Требование п.5 (не ломает старых клиентов): один запрос без заголовка
|
|
||||||
отрабатывает как раньше — 200, тред создан, зеркало отправлено ровно раз."""
|
|
||||||
_install_fake_storage(monkeypatch)
|
|
||||||
r = client.post(
|
|
||||||
"/api/v1/trade-in/support/messages", json={"text": "просто вопрос"}, headers=_auth("carol")
|
|
||||||
)
|
|
||||||
assert r.status_code == 200, r.text
|
|
||||||
assert r.json()["persisted"] is True
|
|
||||||
assert len(_fake_telegram_client.calls) == 1
|
|
||||||
|
|
|
||||||
|
|
@ -42,7 +42,6 @@
|
||||||
* setQueryData-merge path that would be pure incidental complexity here.
|
* setQueryData-merge path that would be pure incidental complexity here.
|
||||||
*/
|
*/
|
||||||
|
|
||||||
import { useRef } from "react";
|
|
||||||
import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query";
|
import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query";
|
||||||
|
|
||||||
import { apiFetch, HTTPError } from "@/lib/api";
|
import { apiFetch, HTTPError } from "@/lib/api";
|
||||||
|
|
@ -130,36 +129,13 @@ export function useSupportUnread(enabled: boolean, scope: SupportScope = "auth")
|
||||||
|
|
||||||
export function useSendSupportMessage(scope: SupportScope = "auth") {
|
export function useSendSupportMessage(scope: SupportScope = "auth") {
|
||||||
const queryClient = useQueryClient();
|
const queryClient = useQueryClient();
|
||||||
// Идемпотентность (#3471): ОДИН `Idempotency-Key` на все повторы ОДНОГО
|
|
||||||
// намерения отправить. Бэкенд пишет строку/ключ ТОЛЬКО ПОСЛЕ успешной
|
|
||||||
// доставки в Telegram, поэтому это не про потери на плече Selectel ->
|
|
||||||
// api.telegram.org (неудачный send там — законная первая попытка) — это
|
|
||||||
// про то, что доставка УЖЕ состоялась, а ответ до клиента не дошёл (обрыв
|
|
||||||
// на обратном пути / клиентский таймаут) или пользователь дважды нажал
|
|
||||||
// "отправить" по одной и той же ещё не отрисовавшейся отправке. Без
|
|
||||||
// заголовка единственная защита — backend-fallback (минутный
|
|
||||||
// hash-отпечаток, `_resolve_idempotency_key` в app/api/v1/support.py), а
|
|
||||||
// он ненадёжен для осмысленно повторяющихся коротких сообщений ("+", "ок",
|
|
||||||
// два "да" подряд). Намерение отождествляем с текстом: тот же текст подряд —
|
|
||||||
// это ретрай ЭТОГО намерения, переиспользуем ключ; новый текст — новое
|
|
||||||
// намерение, новый UUID.
|
|
||||||
const intentRef = useRef<{ text: string; key: string } | null>(null);
|
|
||||||
|
|
||||||
return useMutation<SupportMessage, Error, string>({
|
return useMutation<SupportMessage, Error, string>({
|
||||||
mutationFn: (text) => {
|
mutationFn: (text) =>
|
||||||
if (!intentRef.current || intentRef.current.text !== text) {
|
apiFetch<SupportMessage>(`${scopeBase(scope)}/messages`, {
|
||||||
intentRef.current = { text, key: crypto.randomUUID() };
|
|
||||||
}
|
|
||||||
return apiFetch<SupportMessage>(`${scopeBase(scope)}/messages`, {
|
|
||||||
method: "POST",
|
method: "POST",
|
||||||
headers: { "Idempotency-Key": intentRef.current.key },
|
|
||||||
body: JSON.stringify({ text }),
|
body: JSON.stringify({ text }),
|
||||||
});
|
}),
|
||||||
},
|
|
||||||
onSuccess: () => {
|
onSuccess: () => {
|
||||||
// Успех закрывает намерение — следующая отправка (даже с тем же текстом)
|
|
||||||
// обязана получить НОВЫЙ ключ, иначе она молча схлопнется с этой.
|
|
||||||
intentRef.current = null;
|
|
||||||
queryClient.invalidateQueries({ queryKey: messagesKey(scope) });
|
queryClient.invalidateQueries({ queryKey: messagesKey(scope) });
|
||||||
},
|
},
|
||||||
});
|
});
|
||||||
|
|
|
||||||
Loading…
Add table
Reference in a new issue