修复(#140): 去掉 cancelWithdrawalLogic 在新流程下的重复退款

HIF-22 把申请提现扣 commission 的逻辑移到审批时执行，但 cancel 路径漏改，撤销 pending 提现仍调 UpdateCommission(+amount) 凭空给用户加佣金，并写出一条 338 退佣金日志污染对账。参照 rejectWithdrawal 的处理方式（withdrawalCommon.go:80-87），事务体只保留 withdrawal.status -> Cancelled 的更新，删除 UpdateCommission 与误导性的 WriteCommissionLog 调用。补单测覆盖 happy path、非 pending 状态短路、跨用户撤销拒绝、与审批并发时 FOR UPDATE 行锁让 cancel 输掉 race 这四类场景。 Co-authored-by: multica-agent <github@multica.ai>
2026-06-01 22:26:25 -07:00
parent 750b7be424
commit 86896cd6b1
2 changed files with 232 additions and 8 deletions
@@ -4,7 +4,6 @@ import (
 	"context"

 	logicCommon "github.com/perfect-panel/server/internal/logic/common"
-	"github.com/perfect-panel/server/internal/model/log"
 	"github.com/perfect-panel/server/internal/model/user"
 	"github.com/perfect-panel/server/internal/svc"
 	"github.com/perfect-panel/server/internal/types"
@@ -57,13 +56,10 @@ func (l *CancelWithdrawalLogic) CancelWithdrawal(req *types.CancelWithdrawalRequ
 			return errors.Wrapf(xerr.NewErrCode(xerr.DatabaseUpdateError), "cancel withdrawal failed: %v", txErr)
 		}

-		if txErr = l.svcCtx.UserModel.UpdateCommission(l.ctx, withdrawal.UserId, withdrawal.Amount, tx); txErr != nil {
-			return errors.Wrapf(xerr.NewErrCode(xerr.DatabaseUpdateError), "refund commission failed: %v", txErr)
-		}
-
-		if txErr = logicCommon.WriteCommissionLog(tx, withdrawal.UserId, log.CommissionTypeWithdrawCancel, withdrawal.Amount, ""); txErr != nil {
-			return errors.Wrapf(xerr.NewErrCode(xerr.DatabaseInsertError), "write commission log failed: %v", txErr)
-		}
+		// Commission was NOT deducted at application time under the HIF-22
+		// approval flow, so cancellation requires no refund — only a status
+		// update. Refunding here would mint phantom commission and pollute
+		// reconciliation (mirrors rejectWithdrawal in admin/user/withdrawalCommon.go).

 		return nil
 	})
@@ -0,0 +1,228 @@
+package user
+
+import (
+	"context"
+	"fmt"
+	"strings"
+	"testing"
+
+	"github.com/DATA-DOG/go-sqlmock"
+	usermodel "github.com/perfect-panel/server/internal/model/user"
+	"github.com/perfect-panel/server/internal/svc"
+	"github.com/perfect-panel/server/internal/types"
+	"github.com/perfect-panel/server/pkg/constant"
+	"github.com/perfect-panel/server/pkg/logger"
+	"github.com/perfect-panel/server/pkg/xerr"
+	"github.com/pkg/errors"
+	"gorm.io/driver/mysql"
+	"gorm.io/gorm"
+)
+
+// TestCancelWithdrawal_DoesNotRefundCommission 验证 HIF-140 修复：
+// 用户撤销 pending 提现的事务体必须只更新 withdrawal.status，
+// 严禁触发 UpdateCommission（commission 列读 / 写）或写 333/338 commission 日志。
+//
+// sqlmock 严格匹配期望 SQL：只允许出现 BEGIN / SELECT withdrawal FOR UPDATE /
+// UPDATE withdrawal SET status / COMMIT，不允许出现 SELECT/UPDATE `user` 或
+// INSERT system_logs。
+func TestCancelWithdrawal_DoesNotRefundCommission(t *testing.T) {
+	const (
+		withdrawalID = int64(987654)
+		userID       = int64(42)
+		amount       = int64(2500)
+	)
+
+	db, mock, cleanup := newCancelWithdrawalTestDB(t)
+	defer cleanup()
+
+	mock.ExpectBegin()
+	mock.ExpectQuery("FROM `withdrawals`").
+		WithArgs(withdrawalID, 1).
+		WillReturnRows(sqlmock.NewRows([]string{"id", "user_id", "amount", "status"}).
+			AddRow(withdrawalID, userID, amount, usermodel.WithdrawalStatusPending))
+	mock.ExpectExec("UPDATE `withdrawals` SET").
+		WithArgs(usermodel.WithdrawalStatusCancelled, sqlmock.AnyArg(), withdrawalID, usermodel.WithdrawalStatusPending).
+		WillReturnResult(sqlmock.NewResult(0, 1))
+	mock.ExpectCommit()
+
+	logic := newTestCancelWithdrawalLogic(t, db, userID)
+	resp, err := logic.CancelWithdrawal(&types.CancelWithdrawalRequest{WithdrawalId: withdrawalID})
+	if err != nil {
+		t.Fatalf("CancelWithdrawal unexpected error: %v", err)
+	}
+	if resp == nil || resp.Id != withdrawalID {
+		t.Fatalf("CancelWithdrawal response = %+v, want id=%d", resp, withdrawalID)
+	}
+	if resp.Status != usermodel.WithdrawalStatusCancelled {
+		t.Fatalf("CancelWithdrawal status = %d, want %d", resp.Status, usermodel.WithdrawalStatusCancelled)
+	}
+	if err := mock.ExpectationsWereMet(); err != nil {
+		t.Fatalf("unmet sql expectations: %v", err)
+	}
+}
+
+// TestCancelWithdrawal_RejectsNonPending 覆盖：状态非 pending（已批准/拒绝/已撤销）
+// 时撤销必须短路返回 WithdrawalStatusInvalid，且不得写任何状态或佣金。
+func TestCancelWithdrawal_RejectsNonPending(t *testing.T) {
+	const (
+		withdrawalID = int64(55555)
+		userID       = int64(42)
+	)
+
+	cases := []struct {
+		name   string
+		status uint8
+	}{
+		{"already approved", usermodel.WithdrawalStatusApproved},
+		{"already rejected", usermodel.WithdrawalStatusRejected},
+		{"already cancelled", usermodel.WithdrawalStatusCancelled},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			db, mock, cleanup := newCancelWithdrawalTestDB(t)
+			defer cleanup()
+
+			mock.ExpectBegin()
+			mock.ExpectQuery("FROM `withdrawals`").
+				WithArgs(withdrawalID, 1).
+				WillReturnRows(sqlmock.NewRows([]string{"id", "user_id", "amount", "status"}).
+					AddRow(withdrawalID, userID, int64(2500), tc.status))
+			mock.ExpectRollback()
+
+			logic := newTestCancelWithdrawalLogic(t, db, userID)
+			_, err := logic.CancelWithdrawal(&types.CancelWithdrawalRequest{WithdrawalId: withdrawalID})
+			if !isCancelErrCode(err, xerr.WithdrawalStatusInvalid) {
+				t.Fatalf("CancelWithdrawal err = %v, want WithdrawalStatusInvalid", err)
+			}
+			if err := mock.ExpectationsWereMet(); err != nil {
+				t.Fatalf("unmet sql expectations: %v", err)
+			}
+		})
+	}
+}
+
+// TestCancelWithdrawal_RejectsOtherUserWithdrawal 覆盖：当前登录用户尝试撤销
+// 不属于自己的 pending 提现，必须返回 PermissionDenied 且不得写库。
+func TestCancelWithdrawal_RejectsOtherUserWithdrawal(t *testing.T) {
+	const (
+		withdrawalID = int64(33333)
+		ownerUserID  = int64(99)
+		attackerID   = int64(42)
+	)
+
+	db, mock, cleanup := newCancelWithdrawalTestDB(t)
+	defer cleanup()
+
+	mock.ExpectBegin()
+	mock.ExpectQuery("FROM `withdrawals`").
+		WithArgs(withdrawalID, 1).
+		WillReturnRows(sqlmock.NewRows([]string{"id", "user_id", "amount", "status"}).
+			AddRow(withdrawalID, ownerUserID, int64(2500), usermodel.WithdrawalStatusPending))
+	mock.ExpectRollback()
+
+	logic := newTestCancelWithdrawalLogic(t, db, attackerID)
+	_, err := logic.CancelWithdrawal(&types.CancelWithdrawalRequest{WithdrawalId: withdrawalID})
+	if !isCancelErrCode(err, xerr.PermissionDenied) {
+		t.Fatalf("CancelWithdrawal err = %v, want PermissionDenied", err)
+	}
+	if err := mock.ExpectationsWereMet(); err != nil {
+		t.Fatalf("unmet sql expectations: %v", err)
+	}
+}
+
+// TestCancelWithdrawal_ConcurrentRaceLosesViaFORUPDATE 模拟撤销与审批并发的场景：
+// 第二个 cancel 在 LoadPendingWithdrawalForUpdate 取到行时，状态已被先到的 approve
+// 改成 1（FOR UPDATE 行锁让出后看到的最新状态），cancel 应当短路返回错误，
+// 严禁继续往下写 status 或动 commission。
+func TestCancelWithdrawal_ConcurrentRaceLosesViaFORUPDATE(t *testing.T) {
+	const (
+		withdrawalID = int64(77777)
+		userID       = int64(42)
+	)
+
+	db, mock, cleanup := newCancelWithdrawalTestDB(t)
+	defer cleanup()
+
+	mock.ExpectBegin()
+	mock.ExpectQuery("FROM `withdrawals`").
+		WithArgs(withdrawalID, 1).
+		WillReturnRows(sqlmock.NewRows([]string{"id", "user_id", "amount", "status"}).
+			AddRow(withdrawalID, userID, int64(2500), usermodel.WithdrawalStatusApproved))
+	mock.ExpectRollback()
+
+	logic := newTestCancelWithdrawalLogic(t, db, userID)
+	_, err := logic.CancelWithdrawal(&types.CancelWithdrawalRequest{WithdrawalId: withdrawalID})
+	if !isCancelErrCode(err, xerr.WithdrawalStatusInvalid) {
+		t.Fatalf("CancelWithdrawal err = %v, want WithdrawalStatusInvalid (loser of race)", err)
+	}
+	if err := mock.ExpectationsWereMet(); err != nil {
+		t.Fatalf("unmet sql expectations: %v", err)
+	}
+}
+
+func newCancelWithdrawalTestDB(t *testing.T) (*gorm.DB, sqlmock.Sqlmock, func()) {
+	t.Helper()
+
+	sqlDB, mock, err := sqlmock.New(sqlmock.QueryMatcherOption(sqlmock.QueryMatcherFunc(func(expectedSQL, actualSQL string) error {
+		if strings.Contains(actualSQL, expectedSQL) {
+			return nil
+		}
+		return fmt.Errorf("actual sql %q does not contain %q", actualSQL, expectedSQL)
+	})))
+	if err != nil {
+		t.Fatalf("create sqlmock: %v", err)
+	}
+
+	db, err := gorm.Open(mysql.New(mysql.Config{Conn: sqlDB, SkipInitializeWithVersion: true}), &gorm.Config{})
+	if err != nil {
+		_ = sqlDB.Close()
+		t.Fatalf("open gorm db: %v", err)
+	}
+
+	return db, mock, func() {
+		_ = sqlDB.Close()
+	}
+}
+
+func newTestCancelWithdrawalLogic(t *testing.T, db *gorm.DB, userID int64) *CancelWithdrawalLogic {
+	t.Helper()
+	ctx := context.WithValue(context.Background(), constant.CtxKeyUser, &usermodel.User{Id: userID})
+	return &CancelWithdrawalLogic{
+		Logger: logger.WithContext(ctx),
+		ctx:    ctx,
+		svcCtx: &svc.ServiceContext{
+			DB:        db,
+			UserModel: stubUserModelForCancel{},
+		},
+	}
+}
+
+// stubUserModelForCancel 是 user.Model 的零依赖替身，仅覆盖 CancelWithdrawal 必须
+// 调用的 ClearUserCache。其它方法被调用会导致 nil-interface panic，正好可以暴露
+// 测试边界外的意外依赖。
+type stubUserModelForCancel struct {
+	usermodel.Model
+}
+
+func (stubUserModelForCancel) ClearUserCache(_ context.Context, _ ...*usermodel.User) error {
+	return nil
+}
+
+func cancelErrCodeOf(err error) uint32 {
+	if err == nil {
+		return 0
+	}
+	type coder interface {
+		GetErrCode() uint32
+	}
+	cause := errors.Cause(err)
+	if c, ok := cause.(coder); ok {
+		return c.GetErrCode()
+	}
+	return 0
+}
+
+func isCancelErrCode(err error, code uint32) bool {
+	return cancelErrCodeOf(err) == code
+}