Repository navigation
feat(bill): 对接易支付与多批次积分/订阅生命周期管理 - #954
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
|
✅ 此 PR 已关联 issue,之前的提醒已自动标记为已解决。 |
72eca2e to
e4c912b
Compare
There was a problem hiding this comment.
The billing implementation introduces several correctness and security defects across payment notification handling, gateway configuration, refunds, and expiring-credit accounting. The most urgent issues can either grant credits for unsuccessful payments, allow expired credits to be spent, or expose administrative refund functionality without authorization.
Reviewed head commit: 72eca2eefe19e1b2fdb61573e1dcd751f31c9aef
- [P1] Reject non-success payment notifications (
backend/packages/app/src/windup_app/server/bill/service.py:176-190)
When the provider sends a signed callback with a failed or pendingtrade_status, this code still marks the order asPAIDand credits the user becausenotify_res.statusis never checked. A valid signature only authenticates the notification; the handler must require the provider's successful status before changing order state or issuing credits. - [P1] Configure a real payment gateway URL (
backend/packages/app/src/windup_app/server/bill/epay.py:27-37)
Every productionEpayProviderinstance uses the hard-codedhttps://pay.example.comendpoint becauseBillSettingshas no API URL setting and the constructor default is never overridden by order creation. As a result, normal orders return payment links pointing at the placeholder domain rather than the configured payment gateway. - [P1] Implement the refund service before exposing the endpoint (
backend/packages/app/src/windup_app/web/api/admin_bill.py:40-44)
Any authenticated request to/admin/bill/refundsreaches this call, butSqlAlchemyBillServicehas noprocess_refundmethod, so the endpoint always raisesAttributeErrorinstead of creating or processing a refund. The route should call an implemented service operation or remain unavailable until the refund flow exists. - [P1] Enforce administrator authorization on refund requests (
backend/packages/app/src/windup_app/web/api/admin_bill.py:28-40)
This newly mounted admin route only relies on the general JWT middleware; the intended RBAC check is commented out, so any logged-in user can invoke the refund operation once the missing service method is implemented. Refunds must verify the current user against the existing administrator authorization mechanism before performing the operation. - [P1] Prevent expired credits from being recreated as legacy balance (
backend/packages/app/src/windup_app/server/quota/service.py:308-319)
After a subscription batch expires but before the 60-second expiration worker processes it,account.balancecan still include those credits while the batch query excludes them. The aggregate balance check therefore passes, no valid batch is found, and this fallback creates a new non-expiring legacy batch, allowing expired subscription credits to be spent permanently. Reserve logic must reconcile expired batches or reject the reservation instead of converting the remainder into legacy balance. - [P2] Reconcile legacy balances when reporting batch totals (
backend/packages/app/src/windup_app/server/quota/service.py:308-317)
The registration flow still creates the initial account and transaction directly without aCreditBatch, while the new balance and subscription APIs deriveperpetual_creditsandexpiring_creditsexclusively from batches. Users with untouched registration credits therefore see zero batch totals despite a positive account balance, and the fallback only creates a batch for a later reservation rather than for the full existing balance. - [P2] Perform provider compensation when loading an order (
backend/packages/app/src/windup_app/web/api/bill.py:180-188)
The new order-detail endpoint only reads the localOrderrow, andquery_orderitself also only performs a database lookup. If the payment provider's callback is delayed or lost, a provider-paid order remains locally pending indefinitely even though the design exposes this endpoint specifically for upstream query compensation.
d82fee9 to
d01f289
Compare
…bscription lifecycle - Reject non-success trade_status in notify (F1) - Make epay api_url configurable instead of placeholder (F2) - Remove unimplemented admin refund endpoint and its RBAC gap (F3/F4) - Lazy-expire stale batches in reserve_credit to prevent legacy conversion (F5) - Create CreditBatch on registration + backfill migration for existing accounts (F6) - Add provider.query_order compensation for lost callbacks (F7) - Rebuild close_expired_orders as upstream-verified state machine with CLOSED revival (H1) - Expire-aware release/capture to prevent reviving expired frozen credits (H2) - Lock user row in _fulfill_order to serialize concurrent subscription callbacks (H3) - Fix AB-BA deadlock in process_expired_subscriptions lock order (X1) - Rewrite pending_close worker with bump-then-process and to_thread
xiaocheny214
left a comment
There was a problem hiding this comment.
感谢详细的评审,全部 10 条已在 564176b 修复。逐条回应如下:
[P1] 拒绝非成功支付通知 (F1)
已修复。handle_notify 在 verify_notify 后、锁订单前校验 notify_res.status.upper() ∈ {TRADE_SUCCESS, SUCCESS},非成功状态拒绝并抛异常(端点回 fail),订单保持 PENDING 不发货。
[P1] 配置真实网关地址 (F2)
已修复。BillSettings 增加 epay_api_url 字段(env 前缀 WINDUP_BILL_),EpayProvider.__init__ 从 settings 读取,.env.example 补全了 WINDUP_BILL_EPAY_* 段。
[P1] 退款服务未实现即暴露端点 (F3) + 管理员鉴权缺失 (F4)
已修复。删除了 admin_bill.py 和 bootstrap 挂载,退款功能后续单独实现时须复用 admin_quota.py 的 require_admin_user 模式。openapi.json 已同步移除该路由。
[P1] 过期积分转为 legacy 余额 (F5)
已修复。reserve_credit 在余额检查前调用新增的 _expire_stale_batches:FOR UPDATE 选 remaining>0 AND expires_at<=now 的批次逐个清零、同步扣减 account.balance、写 ref=lazy:batch:{id} 的 SUBSCRIPTION_EXPIRE 流水。此后 legacy fallback 只覆盖真正无批次的账户。
[P2] 注册流程不建批次导致 perpetual_credits 恒 0 (F6)
已修复。_create_credit_account 补建 CreditBatch(REGISTER_GIFT, expires_at=None)。另新增 scripts/migrations/20261010_backfill_credit_batches.sql 回填存量账户(
[P2] 订单查询无上游补偿 (F7)
已修复。新增 EpayProvider.query_order()(GET api.php?act=order,四态 paid/unpaid/not_found/unknown),service.query_order 对 PENDING/CLOSED 订单向上游补偿查询,paid 则锁单重验后补发货,金额不符拒绝。web/api/bill.py 的 get_order 端点也改调 service.query_order。
[P1] 上游关单未确认时不应本地置 CLOSED (H1)
已修复。close_expired_orders 重构为四态状态机:先无锁向上游核实 → 锁单复核 PENDING → 上游 paid 且金额匹配则 _fulfill_order 补发货(返回 "paid");unpaid/not_found 才置 CLOSED(返回 "closed");unknown 保持 PENDING 返回 "retry",过期超 24h 仍 unknown 才强关。handle_notify 允许 CLOSED 订单收到合法成功通知时复活为 PAID 并补发货。worker 改用 bump-then-process(先 zadd score=now+600,终态才 zrem)+ asyncio.to_thread 避免阻塞事件循环。
[P1] 解冻与结算退款须按原批次有效期处理 (H2)
已修复,撤回之前的异议。此前我回复"不会存在这种情况"是错误的——60s 轮询不是原子屏障:任务冻结在 T0(批次未过期),解冻在 60s worker 清零之后(任务失败处理延迟 / MQ 重投可任意长),batch.remaining += release_amt 会把过期额度加回。修复后 release_credit 和 capture_credit 按各分配批次的 expires_at 拆分:未过期部分回 batch.remaining + account.balance,已过期部分只 batch.frozen -=(解冻不回余额),写 delta=0 的 SUBSCRIPTION_EXPIRE 标记流水。幂等标记流水的 delta 写未过期返还额,保持 Σdelta 与 balance 变化一致。
[P1] 查询订阅队列前锁定用户级稳定记录 (H3)
已修复,撤回之前的异议。此前我回复"一个用户无法同时下单两个订阅"是错误的——并发单位是"回调到达"不是"用户下单"。用户可以开两个支付页(两个合法 PENDING 订单),两个异步回调可同时到达,epay notify 端点无鉴权无串行化,两个 worker 线程各持一个订单锁互不阻塞。空订阅结果集上 FOR UPDATE 锁不住任何行,两个事务都选 ACTIVE、创建重叠 30 天账期。修复后抽出 _fulfill_order,入口先 SELECT User ... WITH FOR UPDATE 锁用户行(用户行必存在、是稳定串行化点),订阅排期查询和 credit() 全部在该锁内完成。另发现并修复了 process_expired_subscriptions(batch→account)与 reserve_credit(account→batch)的 AB-BA 死锁——锁序统一为 订单→用户→订阅→账户→批次。
概述 (Summary)
实现了商业化计费体系的核心后端功能,包含易支付(Epay)网关对接、多批次有效期的积分追踪机制、订阅生命周期管理以及后台延时任务。
windup_app.server.bill.provider):PaymentProvider抽象接口,并实现EpayProvider(MD5 / 字典序验签,支持支付宝与微信收银台跳转),便于后续扩展至 Stripe 等通道。windup_app.server.bill.service):(amount_fen + 7) // 8)及订阅(Plus / Pro / Max)。with_for_update) 和 CAS 实现回调幂等处理,审计日志存入PaymentEvent。PENDING_START) 与自动轮转生效。windup_app.server.quota):CreditBatch与CreditFreezeAlloc表,实现「订阅额度(当期有效)」与「充值积分(长期有效)」共存。CreditAccount同步作为视图保持旧接口完全向下兼容。/bill/products、/bill/orders、/bill/notify/epay(白名单免鉴权)、/bill/subscription。/quota/balance,返回到期积分与永久积分细分。bill:pending_close) 10秒关单轮询任务及 60 秒订阅到期处理任务。openapi.json与schema_sync.py。验证 (Verification)
pytest tests/test_bill_*.py tests/test_quota*.py-> 全部 94 项测试通过。ruff check .-> 全部通过。lint-imports-> 分层契约全部保持 (2 kept, 0 broken)。python -m scripts.export_openapi-> 契约零漂移。Close #947