From b8f225bf86692c5f6916a023b9e51f2e668c9e03 Mon Sep 17 00:00:00 2001 From: bot-backend Date: Thu, 6 Aug 2026 15:52:44 +0300 Subject: [PATCH] =?UTF-8?q?fix(tradein/payments):=20NULLS=20NOT=20DISTINCT?= =?UTF-8?q?=20=D0=B4=D0=B5=D0=B4=D1=83=D0=BF,=20pd=5Ferased=5Fat,=20=D1=81?= =?UTF-8?q?=D1=82=D0=B0=D1=82=D1=83=D1=81=D1=8B=20T-Bank?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit HOLD-правки ревью PR #2732 (миграция ни разу не применялась на проде, manifest не трогаем): - Блокер: UNIQUE(payment_id, kind, ref_id) и UNIQUE(tbank_payment_id, status, amount_kopecks, token) не защищали при NULL (NULL != NULL в Postgres) — на проде воспроизведено 3 одинаковых INSERT -> 3 строки. Оба ключа теперь UNIQUE NULLS NOT DISTINCT (PG15+, прод на 16.4). - payment_notifications.processed_at — контракт "выдача состоялась" для PR-D, закрывает окно at-most-once (падение между INSERT нотификации и выдачей товара при захолдированных деньгах). - payments_lead_idx — FK lead_id ON DELETE SET NULL без индекса это seq scan на каждый DELETE FROM trade_in_leads, а #2547 вводит пакетное удаление. - payment_entitlements: убраны amount/consumed — модель кредитов/пакетов явно отвергнута в mera-b2c-paid-flow-decision.md §1 (capability-URL вместо кошелька). - payments.pd_erased_at — покрытие purge-контура #2547, который не знает про ПДн в payments (customer_email/customer_phone), появляющиеся в PR-D. - payments.order_id CHECK char_length <= 50 — лимит T-Bank OrderId, падать у себя, а не на /v2/Init. - payments.status CHECK сверен с github.com/nikita-vanyasin/tinkoff/status.go (developer.tbank.ru рендерит enum клиентским JS, прямого доступа нет): убраны неподтверждённые AUTHORIZED_AND_CHARGED/RECEIPT_REGISTERED, добавлены подтверждённые 3DS_CHECKING/3DS_CHECKED. ATTEMPTS_EXPIRED/PAY_CHECKING НЕ добавлены — не подтверждены ни одним источником. - config.py: комментарий TBANK_PAY_TYPE ссылался на recon-док (выбирает "O"), переставлен на mera-b2c-paid-flow-decision.md §1 (выбирает "T", источник реального дефолта). Полный pytest: 3755 passed, 9 skipped, 0 failed. --- tradein-mvp/backend/app/core/config.py | 7 +- tradein-mvp/backend/data/sql/228_payments.sql | 173 +++++++++++++----- 2 files changed, 130 insertions(+), 50 deletions(-) diff --git a/tradein-mvp/backend/app/core/config.py b/tradein-mvp/backend/app/core/config.py index 609729e1..5d7267aa 100644 --- a/tradein-mvp/backend/app/core/config.py +++ b/tradein-mvp/backend/app/core/config.py @@ -936,8 +936,11 @@ class Settings(BaseSettings): tbank_success_url: str = Field(default="", validation_alias="TBANK_SUCCESS_URL") tbank_fail_url: str = Field(default="", validation_alias="TBANK_FAIL_URL") # "O" — одностадийная (оплата сразу), "T" — двухстадийная (холд + Confirm). - # Дефолт "T": выбрана схема с холдом — оставляет возможность ручного шага - # между оплатой и выдачей (см. recon-док §2 про компромисс O vs T). + # Дефолт "T": выбрана схема с холдом (гибрид «Проба → холд → отчёт по + # ссылке», ядро — вариант B) — источник решения `mera-b2c-paid-flow- + # decision.md` §1 в корне репо, НЕ recon-док (тот сам по себе выбирает + # "O" — устарел этим решением). Не переставляй дефолт обратно на "O", не + # сверившись с decision-доком. tbank_pay_type: Literal["O", "T"] = Field(default="T", validation_alias="TBANK_PAY_TYPE") tbank_receipt_enabled: bool = Field(default=False, validation_alias="TBANK_RECEIPT_ENABLED") tbank_taxation: str = Field(default="", validation_alias="TBANK_TAXATION") diff --git a/tradein-mvp/backend/data/sql/228_payments.sql b/tradein-mvp/backend/data/sql/228_payments.sql index 97c87b77..cddc3389 100644 --- a/tradein-mvp/backend/data/sql/228_payments.sql +++ b/tradein-mvp/backend/data/sql/228_payments.sql @@ -1,42 +1,109 @@ -- 228_payments.sql -- Платёжный контур МЕРЫ (Т-Банк интернет-эквайринг) — схема БД, PR-B из серии -- A..F (см. корень репо `mera-tbank-acquiring-recon.md`, §9 «Разбивка на PR»). +-- Ни разу не применялась на проде (см. `_manifest_applied.txt`) — правится на +-- месте по итогам review (статус HOLD), без ребейза номера. -- -- ── WHY ────────────────────────────────────────────────────────────────────── -- Этот PR — ТОЛЬКО схема + конфиг + kill-switch (`PAYMENTS_ENABLED=false` в --- app/core/config.py, тот же PR). Роутера, httpx-клиента Т-Банка, подписи --- Token, статус-машины и обработчика нотификаций здесь НЕТ — они появятся в --- PR-C/D/E. До PAYMENTS_ENABLED=true эти три таблицы просто не пишутся никаким --- кодом; создание сейчас разблокирует параллельную разработку PR-C/D без --- гонки миграций. +-- app/core/config.py, тот же PR). Роутера, статус-машины и обработчика +-- нотификаций здесь НЕТ (появятся в PR-D/E; PR-C — token/tbank_client/receipt — +-- уже смержен, схемы не касается). До PAYMENTS_ENABLED=true эти три таблицы +-- просто не пишутся никаким кодом; создание сейчас разблокирует параллельную +-- разработку PR-D без гонки миграций. -- -- ── WHAT ───────────────────────────────────────────────────────────────────── -- payments — одна строка на попытку оплаты (Init → notify → -- Confirm/Cancel). order_id — наш внутренний id, --- уходит в T-Bank как OrderId (≤50 симв., см. §3 --- recon-дока); tbank_payment_id — PaymentId из --- ответа Init, известен только ПОСЛЕ вызова. +-- уходит в T-Bank как OrderId (CHECK ≤50 симв. — +-- падать у себя, а не на /v2/Init); tbank_payment_id — +-- PaymentId из ответа Init, известен только ПОСЛЕ +-- вызова. pd_erased_at — см. отдельный блок ниже. -- payment_notifications — append-only лог входящих вебхуков Т-Банка. -- Идемпотентность нотификаций — это и есть --- UNIQUE(tbank_payment_id, status, amount_kopecks, --- token): T-Bank шлёт AUTHORIZED и CONFIRMED --- одновременно, дедуп через ON CONFLICT DO NOTHING --- (сервисный код — PR-D). Осознанно БЕЗ CHECK на --- status: это сырой лог входящих данных, узкий CHECK --- здесь означал бы, что недокументированный/новый --- статус банка ломает запись самого факта нотификации. --- payment_entitlements — что выдано за платёж (кредит/доступ), чтобы --- fulfillment (PR-E) не задваивал выдачу. +-- UNIQUE NULLS NOT DISTINCT(tbank_payment_id, status, +-- amount_kopecks, token): T-Bank шлёт AUTHORIZED и +-- CONFIRMED одновременно, дедуп через ON CONFLICT DO +-- NOTHING (сервисный код — PR-D). processed_at — +-- контракт fulfillment, см. блок ниже. Осознанно БЕЗ +-- CHECK на status: это сырой лог входящих данных, +-- узкий CHECK здесь означал бы, что недокументиро- +-- ванный/новый статус банка ломает запись самого +-- факта нотификации. +-- payment_entitlements — факт «что выдано за платёж» (доступ), НЕ кошелёк. +-- См. блок про amount/consumed ниже. +-- +-- ── ИДЕМПОТЕНТНОСТЬ UNIQUE-ключей: NULLS NOT DISTINCT (найдено на проде) ──── +-- Первая версия миграции использовала обычный UNIQUE на обоих ключах +-- дедупликации. В Postgres обычный UNIQUE считает NULL уникальным относительно +-- самого себя (NULL ≠ NULL) — при ref_id IS NULL / token IS NULL несколько +-- строк с одинаковым остальным набором колонок НЕ схлопываются. Это не +-- гипотетика: три одинаковых INSERT в payment_entitlements с ref_id IS NULL +-- дали три строки вместо одной при проверке на проде (до первого реального +-- применения этой миграции — воспроизведено отдельно). PG 16.4 (прод) умеет +-- `UNIQUE NULLS NOT DISTINCT` (с PG15) — NULL трактуется как равный NULL, +-- ровно то поведение, которое ожидает сервисный слой (ON CONFLICT DO NOTHING / +-- DO UPDATE). Применено к обоим дедуп-ключам ниже. -- -- ── СТАТУСЫ T-BANK (payments.status CHECK) ────────────────────────────────── --- Список — публичный Status-enum платёжного объекта T-Bank Acquiring API --- (Init/GetState/CheckOrder). Recon §11 «Непроверенное» отдельно фиксирует: --- PARTIAL_REVERSED фигурирует в сценарии отмены, но описание enum в самой --- документации банка внутренне противоречиво — оставлен в списке нарочно --- (не блокировать легитимный переход), а не изобретён нами. --- Если прод когда-нибудь получит статус вне списка — упадёт INSERT/UPDATE в --- payments (не в payment_notifications, туда попадёт всё равно) и это будет --- сигналом расширить CHECK отдельной миграцией, а не тихим искажением данных. +-- Список сверен с публичным Status-enum T-Bank Acquiring API. Источник +-- developer.tbank.ru рендерит enum клиентским JS (правая панель схемы ответа +-- динамически подгружается) — прямого текстового доступа к разделу GetState +-- не получено; сверка выполнена по независимому активно поддерживаемому +-- Go-клиенту (github.com/nikita-vanyasin/tinkoff, файл status.go), который +-- явно комментирует каждый статус. Изменения относительно первой версии: +-- - УБРАНЫ 'AUTHORIZED_AND_CHARGED' и 'RECEIPT_REGISTERED' — не найдены ни +-- в одном сверенном источнике; RECEIPT_REGISTERED похоже на статус +-- отдельного объекта «чек» (SendClosingReceipt), не платежа. +-- - ДОБАВЛЕНЫ '3DS_CHECKING' и '3DS_CHECKED' — подтверждены сверкой. +-- - НЕ добавлены 'ATTEMPTS_EXPIRED' и 'PAY_CHECKING' (гипотеза из ревью) — +-- не нашлись ни в одном источнике, которым удалось свериться; если реально +-- существуют — расширить CHECK отдельной миграцией по факту документа. +-- - 'PARTIAL_REVERSED' оставлен: та же сверка его подтверждает (реально +-- существующий статус частичной отмены холда), а не «оставлен из +-- осторожности», как было сформулировано раньше. +-- - 'PREAUTHORIZING' оставлен как есть (не проверялся под вопрос ревью): +-- тот же Go-клиент помечает его комментарием "deprecated / removed from +-- API", но это не запрошенная часть проверки — трогать не стал, инертное +-- значение в CHECK безвредно, если банк его больше не шлёт. +-- Контракт для PR-D: если банк присылает статус вне списка ниже, обработчик +-- нотификаций обязан писать в payments.status значение 'UNKNOWN' (не поднимать +-- исключение, не терять запись) — сырое тело в любом случае лежит целиком в +-- payment_notifications.body. INSERT/UPDATE payments с любым другим +-- незнакомым значением упадёт на CHECK — это специально: тихое искажение +-- статуса хуже, чем громкий сбой одной записи. +-- +-- ── payment_entitlements: без кредитно-кошельковой семантики ──────────────── +-- Первая версия несла amount/consumed (модель «кредиты/пакеты»). Явно +-- отвергнуто в `mera-b2c-paid-flow-decision.md` (§1): «Что отвергнуто явно: +-- кредиты/пакеты, роль customer, ... — цена ошибки в guard'е выше годовой +-- выручки этой воронки». Доставка купленного выбрана через capability-URL +-- (`/r/`, волна 3 §9 того же дока), а не через списание количества с +-- баланса. Таблица остаётся фактом «что выдано за платёж» (payment → kind +-- [+ ref_id]), без количественного состояния. Если модель когда-нибудь +-- реально понадобится — восстановить amount/consumed дешевле (ADD COLUMN), +-- чем сейчас снимать с них зависимости в PR-E, которого ещё нет. +-- +-- ── payments.pd_erased_at: покрытие purge-контура #2547 ───────────────────── +-- #2547 знает про PII в trade_in_leads/trade_in_estimates, но НЕ про payments +-- — эта таблица нового хранилища ПДн (customer_email/customer_phone) появится +-- вместе с PR-D. pd_erased_at NULL = ПДн не стирались; проставляется по +-- запросу субъекта на удаление — обнуляет customer_email/customer_phone, +-- фискально значимые поля (order_id, amount_kopecks, confirmed_at, +-- terminal_key и т.д.) остаются нетронутыми (обязательны для чека/сверки с +-- банком). Сам purge-job — вне scope этого PR (схема-only); колонку дешевле +-- завести сейчас, чем добавлять отдельной миграцией после того как PR-D +-- начнёт писать ПДн в эту таблицу. +-- +-- ── payment_notifications.processed_at: контракт fulfillment (для PR-D) ───── +-- Без этой колонки обработчик получается at-most-once по ОШИБКЕ: если процесс +-- упал ПОСЛЕ INSERT нотификации, но ДО выдачи товара (payment_entitlements / +-- инкремент квоты), ретрай банка увидит уже существующую строку через +-- ON CONFLICT DO NOTHING, ответит "OK" и товар не выдастся никогда — при этом +-- деньги у клиента уже списаны/захолдированы. Контракт для PR-D: обработка +-- нотификации считается завершённой (fulfillment состоялся) ТОЛЬКО когда +-- processed_at проставлен; сам факт наличия строки в payment_notifications +-- этого не гарантирует и не должен использоваться как признак «обработано». -- -- ── IDEMPOTENCY ────────────────────────────────────────────────────────────── -- CREATE TABLE IF NOT EXISTS + DROP CONSTRAINT IF EXISTS перед ADD CONSTRAINT @@ -61,7 +128,7 @@ BEGIN; CREATE TABLE IF NOT EXISTS payments ( id uuid PRIMARY KEY DEFAULT gen_random_uuid(), - order_id text NOT NULL UNIQUE, -- наш id, -> T-Bank OrderId (<=50 симв.) + order_id text NOT NULL UNIQUE CHECK (char_length(order_id) <= 50), tbank_payment_id text UNIQUE, -- PaymentId из ответа Init (NULL до Init) terminal_key text NOT NULL, product_code text NOT NULL, -- что продали (product_code, не цена из тела запроса) @@ -75,6 +142,7 @@ CREATE TABLE IF NOT EXISTS payments ( lead_id uuid REFERENCES trade_in_leads(id) ON DELETE SET NULL, customer_email text, customer_phone text, + pd_erased_at timestamptz, -- см. блок про purge-контур #2547 в шапке файла error_code text, error_message text, @@ -100,6 +168,8 @@ ALTER TABLE payments 'AUTHORIZED', 'AUTH_FAIL', 'REJECTED', + '3DS_CHECKING', + '3DS_CHECKED', 'CONFIRMING', 'CONFIRMED', 'REVERSING', @@ -109,14 +179,17 @@ ALTER TABLE payments 'PARTIAL_REFUNDED', 'REFUNDED', 'REFUND_FAILED', - 'RECEIPT_REGISTERED', - 'AUTHORIZED_AND_CHARGED', 'UNKNOWN' )); CREATE INDEX IF NOT EXISTS payments_status_created_idx ON payments (status, created_at); CREATE INDEX IF NOT EXISTS payments_created_by_idx ON payments (created_by); CREATE INDEX IF NOT EXISTS payments_estimate_idx ON payments (estimate_id); +-- lead_id имеет FK ON DELETE SET NULL — без индекса Postgres делает seq scan +-- по payments на каждый DELETE FROM trade_in_leads (проверка "нет ли ссылок" +-- перед SET NULL). #2547 вводит пакетное физическое удаление лидов — без +-- индекса это N seq scan'ов по payments на один batch-прогон purge-джобы. +CREATE INDEX IF NOT EXISTS payments_lead_idx ON payments (lead_id); COMMENT ON TABLE payments IS 'Платёжный контур МЕРЫ (Т-Банк эквайринг). Одна строка на попытку оплаты. ' @@ -138,48 +211,52 @@ CREATE TABLE IF NOT EXISTS payment_notifications ( body jsonb NOT NULL, -- полное тело нотификации как есть received_at timestamptz NOT NULL DEFAULT now(), + -- Контракт fulfillment для PR-D — см. подробный блок в шапке файла. + -- NULL = обработка (выдача товара) ещё не завершена или не начиналась; + -- проставляется сервисным кодом ПОСЛЕ успешной выдачи, не в момент INSERT. + processed_at timestamptz, -- Дедуп-ключ идемпотентности (recon §3 п.4): T-Bank шлёт AUTHORIZED и -- CONFIRMED одновременно для одностадийной оплаты; ON CONFLICT DO NOTHING - -- в сервисном коде (PR-D) значит "уже обработано". NB: NULL в Postgres не - -- равен NULL — несколько строк с одинаковым (NULL, ...) НЕ схлопнутся этим - -- UNIQUE. На практике token де-факто заполнен всегда (иначе подпись не - -- проверить), поэтому дыра теоретическая, но сервисный слой не должен - -- полагаться на UNIQUE как единственную защиту при token IS NULL. - UNIQUE (tbank_payment_id, status, amount_kopecks, token) + -- в сервисном коде (PR-D) значит "уже обработано". NULLS NOT DISTINCT + -- (см. блок в шапке файла) — без него NULL в token/tbank_payment_id не + -- считался бы дублем самого себя, и дедуп молча переставал бы работать + -- ровно в вырожденном случае, для которого он и нужен. + UNIQUE NULLS NOT DISTINCT (tbank_payment_id, status, amount_kopecks, token) ); COMMENT ON TABLE payment_notifications IS 'Append-only лог входящих вебхуков T-Bank. Идемпотентность через UNIQUE ' - '(tbank_payment_id, status, amount_kopecks, token) + ON CONFLICT DO NOTHING.'; + 'NULLS NOT DISTINCT(tbank_payment_id, status, amount_kopecks, token) + ' + 'ON CONFLICT DO NOTHING. processed_at — контракт "выдача состоялась" для PR-D.'; -- ───────────────────────────────────────────────────────────────────────── --- payment_entitlements — что выдано за платёж +-- payment_entitlements — что выдано за платёж (факт, не кошелёк) -- ───────────────────────────────────────────────────────────────────────── CREATE TABLE IF NOT EXISTS payment_entitlements ( id uuid PRIMARY KEY DEFAULT gen_random_uuid(), payment_id uuid NOT NULL REFERENCES payments(id), subject text NOT NULL, -- username или anon-token, кому выдано - kind text NOT NULL, -- 'pdf_report' | 'estimate_pack' | ... - ref_id uuid, -- estimate_id для разового отчёта, NULL для пакетов + kind text NOT NULL, -- 'pdf_report' | 'report_link' | ... + ref_id uuid, -- estimate_id для разового отчёта, NULL если не применимо - amount int NOT NULL DEFAULT 1, - consumed int NOT NULL DEFAULT 0, expires_at timestamptz, created_at timestamptz NOT NULL DEFAULT now(), - -- Гарантия "выдали один раз" (recon §3). Та же NULL-оговорка, что и выше: - -- при ref_id IS NULL (напр. kind='estimate_pack') несколько строк с - -- одинаковым (payment_id, kind) НЕ считаются дублем этим UNIQUE — - -- сервисный слой (PR-E) обязан сам гарантировать один INSERT на платёж - -- там, где ref_id не используется как различитель. - UNIQUE (payment_id, kind, ref_id) + -- Гарантия "выдали один раз" (recon §3). NULLS NOT DISTINCT (см. блок в + -- шапке файла) — без него при ref_id IS NULL несколько строк с одинаковым + -- (payment_id, kind) НЕ считались бы дублем этим UNIQUE, что и + -- воспроизвелось на проде до первого применения миграции. + UNIQUE NULLS NOT DISTINCT (payment_id, kind, ref_id) ); COMMENT ON TABLE payment_entitlements IS - 'Что выдано за платёж (доступ/кредит). UNIQUE(payment_id, kind, ref_id) ' - 'страхует fulfillment (PR-E) от повторной выдачи по одной нотификации.'; + 'Факт "что выдано за платёж" (доступ), НЕ кредитный кошелёк — amount/' + 'consumed сознательно отсутствуют, см. mera-b2c-paid-flow-decision.md §1 ' + '(модель кредитов/пакетов отвергнута явно). UNIQUE NULLS NOT DISTINCT ' + '(payment_id, kind, ref_id) страхует fulfillment (PR-E) от повторной ' + 'выдачи по одной нотификации.'; COMMIT;