fix(order): prevent duplicate refunds and redemptions

Refs #1021

app/Http/Controllers/V1/User/OrderController.php

@@ -126,6 +126,9 @@ class OrderController extends Controller
         if (!$order) {
             return $this->fail([400, __('Order does not exist or has been paid')]);
         }
+        if ($order->total_amount < 0) {
+            return $this->fail([400, __('Order amount is invalid')]);
+        }
         // free process
         if ($order->total_amount <= 0) {
             $orderService = new OrderService($order);

app/Jobs/OrderHandleJob.php

@@ -36,9 +36,7 @@ class OrderHandleJob implements ShouldQueue
      */
     public function handle()
     {
-        $order = Order::where('trade_no', $this->tradeNo)
-            ->lockForUpdate()
-            ->first();
+        $order = Order::where('trade_no', $this->tradeNo)->first();
         if (!$order) return;
         $orderService = new OrderService($order);
         switch ($order->status) {

app/Services/GiftCardService.php

@@ -15,8 +15,8 @@ use Illuminate\Support\Facades\Log;
 
 class GiftCardService
 {
-    protected readonly GiftCardCode $code;
-    protected readonly GiftCardTemplate $template;
+    protected GiftCardCode $code;
+    protected GiftCardTemplate $template;
     protected ?User $user = null;
 
     public function __construct(string $code)
@@ -106,6 +106,23 @@ class GiftCardService
         }
 
         return DB::transaction(function () use ($options) {
+            $this->code = GiftCardCode::whereKey($this->code->id)
+                ->lockForUpdate()
+                ->first()
+                ?? throw new ApiException('兑换码不存在');
+
+            $this->template = GiftCardTemplate::whereKey($this->code->template_id)
+                ->lockForUpdate()
+                ->first()
+                ?? throw new ApiException('该礼品卡类型不存在');
+
+            $this->user = User::whereKey($this->user->id)
+                ->lockForUpdate()
+                ->first()
+                ?? throw new ApiException('用户信息未提供');
+
+            $this->validate();
+
             $actualRewards = $this->template->calculateActualRewards($this->user);
 
             if ($this->template->type === GiftCardTemplate::TYPE_MYSTERY) {
@@ -198,7 +215,7 @@ class GiftCardService
             return null;
         }
 
-        $inviteUser = User::find($this->user->invite_user_id);
+        $inviteUser = User::lockForUpdate()->find($this->user->invite_user_id);
         if (!$inviteUser) {
             return null;
         }
@@ -212,7 +229,9 @@ class GiftCardService
         if (isset($rewards['balance']) && $rewards['balance'] > 0) {
             $inviteBalance = intval($rewards['balance'] * $rate);
             if ($inviteBalance > 0) {
-                $userService->addBalance($inviteUser->id, $inviteBalance);
+                if (!$userService->addBalance($inviteUser->id, $inviteBalance)) {
+                    throw new ApiException('邀请人余额发放失败');
+                }
                 $inviteRewards['balance'] = $inviteBalance;
             }
         }

app/Services/OrderService.php

@@ -33,6 +33,21 @@ class OrderService
         $this->order = $order;
     }
 
+    private function lockCurrentOrder(): ?Order
+    {
+        return Order::whereKey($this->order->id)
+            ->lockForUpdate()
+            ->first();
+    }
+
+    private function lockCurrentOrderWhenStatus(int $status): ?Order
+    {
+        return Order::whereKey($this->order->id)
+            ->where('status', $status)
+            ->lockForUpdate()
+            ->first();
+    }
+
     /**
      * Create an order from a request.
      *
@@ -56,6 +71,15 @@ class OrderService
         HookManager::call('order.create.before', [$user, $plan, $period, $couponCode]);
 
         return DB::transaction(function () use ($user, $plan, $period, $couponCode, $userService) {
+            $user = User::lockForUpdate()->find($user->id);
+            if (!$user) {
+                throw new ApiException(__('The user does not exist'));
+            }
+
+            if ($userService->isNotCompleteOrderByUserId($user->id)) {
+                throw new ApiException(__('You have an unpaid or pending order, please try again later or cancel it'));
+            }
+
             $newPeriod = PlanService::getPeriodKey($period);
 
             $order = new Order([
@@ -79,7 +103,6 @@ class OrderService
                 $orderService->handleUserBalance($user, $userService);
             }
 
-            // 余额抵扣后的 total_amount 才是用户实际支付金额，返利必须基于该金额计算。
             $orderService->setInvite(user: $user);
 
             if (!$order->save()) {
@@ -96,13 +119,19 @@ class OrderService
 
     public function open(): void
     {
-        $order = $this->order;
+        $openedOrder = DB::transaction(function () {
+            $order = $this->lockCurrentOrderWhenStatus(Order::STATUS_PROCESSING);
+            if (!$order) {
+                return null;
+            }
+
             $plan = Plan::find($order->plan_id);
+            if (!$plan) {
+                throw new \RuntimeException('订阅不存在');
+            }
 
             HookManager::call('order.open.before', $order);
 
-
-        DB::transaction(function () use ($order, $plan) {
             $this->user = User::lockForUpdate()->find($order->user_id);
 
             if ($order->surplus_credit) {
@@ -131,10 +160,19 @@ class OrderService
             if (!$order->save()) {
                 throw new \RuntimeException('订单信息保存失败');
             }
+
+            return $order;
         });
 
+        if (!$openedOrder) {
+            return;
+        }
+
+        $order = $openedOrder;
+        $this->order = $order;
+
         $eventId = match ((int) $order->type) {
-            Order::STATUS_PROCESSING => admin_setting('new_order_event_id', 0),
+            Order::TYPE_NEW_PURCHASE => admin_setting('new_order_event_id', 0),
             Order::TYPE_RENEWAL => admin_setting('renew_order_event_id', 0),
             Order::TYPE_UPGRADE => admin_setting('change_order_event_id', 0),
             default => 0,
@@ -287,16 +325,31 @@ class OrderService
 
     public function paid(string $callbackNo)
     {
-        $order = $this->order;
-        if ($order->status !== Order::STATUS_PENDING)
-            return true;
+        try {
+            [$order, $shouldDispatch] = DB::transaction(function () use ($callbackNo) {
+                $order = $this->lockCurrentOrder();
+                if (!$order) {
+                    throw new \RuntimeException('Order not found.');
+                }
+                if ((int) $order->status !== Order::STATUS_PENDING) {
+                    return [$order, false];
+                }
+
                 $order->status = Order::STATUS_PROCESSING;
                 $order->paid_at = time();
                 $order->callback_no = $callbackNo;
-        if (!$order->save())
-            return false;
-        try {
+                if (!$order->save()) {
+                    throw new \RuntimeException('Failed to save order status.');
+                }
+
+                return [$order, true];
+            });
+
+            $this->order = $order;
+
+            if ($shouldDispatch) {
                 OrderHandleJob::dispatchSync($order->trade_no);
+            }
         } catch (\Exception $e) {
             Log::error($e);
             return false;
@@ -306,25 +359,37 @@ class OrderService
 
     public function cancel(): bool
     {
-        $order = $this->order;
-        HookManager::call('order.cancel.before', $order);
         try {
-            DB::beginTransaction();
+            $cancelledOrder = DB::transaction(function () {
+                $order = $this->lockCurrentOrderWhenStatus(Order::STATUS_PENDING);
+                if (!$order) {
+                    return null;
+                }
+
+                HookManager::call('order.cancel.before', $order);
+
                 $order->status = Order::STATUS_CANCELLED;
                 if (!$order->save()) {
-                throw new \Exception('Failed to save order status.');
+                    throw new \RuntimeException('Failed to save order status.');
                 }
                 if ($order->balance_amount) {
                     $userService = new UserService();
                     if (!$userService->addBalance($order->user_id, $order->balance_amount)) {
-                    throw new \Exception('Failed to add balance.');
+                        throw new \RuntimeException('Failed to add balance.');
                     }
                 }
-            DB::commit();
-            HookManager::call('order.cancel.after', $order);
+
+                return $order;
+            });
+
+            if (!$cancelledOrder) {
+                return false;
+            }
+
+            $this->order = $cancelledOrder;
+            HookManager::call('order.cancel.after', $cancelledOrder);
             return true;
         } catch (\Exception $e) {
-            DB::rollBack();
             Log::error($e);
             return false;
         }

resources/lang/en-US.json

@@ -26,6 +26,7 @@
     "No active subscription. Unable to use our provided Apple ID": "No active subscription. Unable to use our provided Apple ID",
     "Oops, there's a problem... Please refresh the page and try again later": "Oops, there's a problem... Please refresh the page and try again later",
     "Order does not exist": "Order does not exist",
+    "Order amount is invalid": "Order amount is invalid",
     "Order does not exist or has been paid": "Order does not exist or has been paid",
     "Payment failed. Please check your credit card information": "Payment failed. Please check your credit card information",
     "Payment gateway request failed": "Payment gateway request failed",

resources/lang/ru-RU.json

@@ -26,6 +26,7 @@
     "No active subscription. Unable to use our provided Apple ID": "Нет активной подписки. Невозможно использовать предоставленный Apple ID",
     "Oops, there's a problem... Please refresh the page and try again later": "Ой, возникла проблема... Обновите страницу и попробуйте позже",
     "Order does not exist": "Заказ не существует",
+    "Order amount is invalid": "Некорректная сумма заказа",
     "Order does not exist or has been paid": "Заказ не существует или уже оплачен",
     "Payment failed. Please check your credit card information": "Ошибка оплаты. Проверьте данные карты",
     "Payment gateway request failed": "Ошибка запроса к платёжному шлюзу",

resources/lang/zh-CN.json

@@ -22,6 +22,7 @@
     "Withdrawal account": "提现账号",
     "Unsupported withdrawal": "不支持提现",
     "Order does not exist": "订单不存在",
+    "Order amount is invalid": "订单金额异常",
     "You have an unpaid or pending order, please try again later or cancel it": "您有未付款或开通中的订单，请稍后再试或将其取消",
     "This subscription has been sold out, please choose another subscription": "该订阅已售罄，请更换其它订阅",
     "This subscription cannot be renewed, please change to another subscription": "该订阅无法续费，请更换其它订阅",

resources/lang/zh-TW.json

@@ -22,6 +22,7 @@
     "Withdrawal account": "提現賬號",
     "Unsupported withdrawal": "不支持提現",
     "Order does not exist": "訂單不存在",
+    "Order amount is invalid": "訂單金額異常",
     "You have an unpaid or pending order, please try again later or cancel it": "您有未付款或開通中的訂單，請稍後再試或將其取消",
     "This subscription has been sold out, please choose another subscription": "該訂閱已售罄，請更換其它訂閱",
     "This subscription cannot be renewed, please change to another subscription": "該訂閱無法續費，請更換其它訂閱",

tests/Feature/Server/ServerHandshakeTest.php

@@ -6,7 +6,6 @@ use App\Models\Server;
 use App\Models\ServerMachine;
 use Illuminate\Foundation\Testing\RefreshDatabase;
 use Illuminate\Support\Facades\Bus;
-use Illuminate\Support\Facades\Cache;
 use Tests\TestCase;
 
 class ServerHandshakeTest extends TestCase
@@ -18,7 +17,7 @@ class ServerHandshakeTest extends TestCase
         parent::setUp();
 
         config()->set('app.key', 'base64:' . base64_encode(str_repeat('a', 32)));
-        Cache::forever('admin_settings', [
+        admin_setting([
             'server_token' => 'server-token',
             'server_ws_enable' => 0,
         ]);

tests/TestCase.php

@@ -0,0 +1,9 @@
+<?php
+
+namespace Tests;
+
+use Illuminate\Foundation\Testing\TestCase as BaseTestCase;
+
+abstract class TestCase extends BaseTestCase
+{
+}

tests/Unit/Services/OrderServiceConcurrencyTest.php

@@ -0,0 +1,193 @@
+<?php
+
+namespace Tests\Unit\Services;
+
+use App\Exceptions\ApiException;
+use App\Models\GiftCardCode;
+use App\Models\GiftCardTemplate;
+use App\Models\Order;
+use App\Models\Plan;
+use App\Models\User;
+use App\Services\GiftCardService;
+use App\Services\OrderService;
+use Illuminate\Foundation\Testing\RefreshDatabase;
+use Laravel\Sanctum\Sanctum;
+use Tests\TestCase;
+
+class OrderServiceConcurrencyTest extends TestCase
+{
+    use RefreshDatabase;
+
+    public function test_cancel_only_refunds_once_when_called_with_stale_order_models(): void
+    {
+        $user = $this->makeUser(['balance' => 0]);
+        $plan = $this->makePlan();
+        $order = $this->makeOrder($user, $plan, [
+            'status' => Order::STATUS_PENDING,
+            'balance_amount' => 100,
+            'total_amount' => 1000,
+        ]);
+
+        $firstStaleOrder = Order::findOrFail($order->id);
+        $secondStaleOrder = Order::findOrFail($order->id);
+
+        $this->assertTrue((new OrderService($firstStaleOrder))->cancel());
+        $this->assertFalse((new OrderService($secondStaleOrder))->cancel());
+        $this->assertSame(100, User::findOrFail($user->id)->balance);
+    }
+
+    public function test_open_only_applies_subscription_once_when_called_with_stale_order_models(): void
+    {
+        $user = $this->makeUser([
+            'balance' => 0,
+            'expired_at' => 0,
+            'transfer_enable' => 0,
+            'u' => 1073741824,
+            'd' => 0,
+        ]);
+        $plan = $this->makePlan();
+        $order = $this->makeOrder($user, $plan, [
+            'status' => Order::STATUS_PROCESSING,
+            'balance_amount' => 1100,
+            'total_amount' => 0,
+        ]);
+
+        $before = time();
+        $firstStaleOrder = Order::findOrFail($order->id);
+        $secondStaleOrder = Order::findOrFail($order->id);
+
+        (new OrderService($firstStaleOrder))->open();
+        (new OrderService($secondStaleOrder))->open();
+
+        $user->refresh();
+        $this->assertSame(1, $user->reset_count);
+        $this->assertSame(Order::STATUS_COMPLETED, Order::findOrFail($order->id)->status);
+        $this->assertLessThan($before + (45 * 86400), $user->expired_at);
+    }
+
+    public function test_gift_card_redeem_only_grants_rewards_once_for_stale_code_models(): void
+    {
+        $user = $this->makeUser(['balance' => 0]);
+        $template = GiftCardTemplate::create([
+            'name' => 'race-test',
+            'description' => 'race-test',
+            'type' => GiftCardTemplate::TYPE_GENERAL,
+            'status' => 1,
+            'rewards' => ['balance' => 100],
+            'admin_id' => 1,
+            'created_at' => time(),
+            'updated_at' => time(),
+        ]);
+        GiftCardCode::create([
+            'template_id' => $template->id,
+            'code' => 'RACEUNITTEST',
+            'status' => GiftCardCode::STATUS_UNUSED,
+            'usage_count' => 0,
+            'max_usage' => 1,
+            'created_at' => time(),
+            'updated_at' => time(),
+        ]);
+
+        $firstService = (new GiftCardService('RACEUNITTEST'))->setUser(User::findOrFail($user->id));
+        $secondService = (new GiftCardService('RACEUNITTEST'))->setUser(User::findOrFail($user->id));
+
+        $firstService->validate();
+        $secondService->validate();
+        $firstService->redeem();
+
+        try {
+            $secondService->redeem();
+            $this->fail('The second stale gift card redeem should fail.');
+        } catch (ApiException $exception) {
+            $this->assertNotSame('', $exception->getMessage());
+        }
+
+        $user->refresh();
+        $code = GiftCardCode::where('code', 'RACEUNITTEST')->firstOrFail();
+        $this->assertSame(100, $user->balance);
+        $this->assertSame(1, $code->usage_count);
+        $this->assertSame(1, $code->usages()->count());
+    }
+
+    public function test_checkout_rejects_negative_order_amount(): void
+    {
+        $user = $this->makeUser(['balance' => 0]);
+        Sanctum::actingAs($user);
+
+        $order = $this->makeOrder($user, $this->makePlan(), [
+            'status' => Order::STATUS_PENDING,
+            'total_amount' => -1,
+        ]);
+
+        $response = $this->postJson('/api/v1/user/order/checkout', [
+            'trade_no' => $order->trade_no,
+        ]);
+
+        $response->assertStatus(400);
+        $this->assertSame(Order::STATUS_PENDING, Order::findOrFail($order->id)->status);
+        $this->assertNull(User::findOrFail($user->id)->plan_id);
+    }
+
+    private function makeUser(array $overrides = []): User
+    {
+        return User::create(array_merge([
+            'email' => 'race-test@example.com',
+            'password' => 'password',
+            'uuid' => '00000000-0000-0000-0000-000000000001',
+            'token' => '0123456789abcdef0123456789abcdef',
+            'balance' => 0,
+            'commission_balance' => 0,
+            'transfer_enable' => 0,
+            'u' => 0,
+            'd' => 0,
+            'banned' => 0,
+            'is_admin' => 0,
+            'is_staff' => 0,
+            'expired_at' => 0,
+            'remind_expire' => 1,
+            'remind_traffic' => 1,
+            'created_at' => time(),
+            'updated_at' => time(),
+        ], $overrides));
+    }
+
+    private function makePlan(array $overrides = []): Plan
+    {
+        return Plan::create(array_merge([
+            'group_id' => null,
+            'transfer_enable' => 1111,
+            'name' => 'Race Test Plan',
+            'speed_limit' => null,
+            'show' => 1,
+            'sort' => 0,
+            'renew' => 1,
+            'prices' => [
+                Plan::PERIOD_MONTHLY => 11,
+            ],
+            'reset_traffic_method' => Plan::RESET_TRAFFIC_MONTHLY,
+            'capacity_limit' => null,
+            'sell' => 1,
+            'device_limit' => null,
+            'created_at' => time(),
+            'updated_at' => time(),
+        ], $overrides));
+    }
+
+    private function makeOrder(User $user, Plan $plan, array $overrides = []): Order
+    {
+        return Order::create(array_merge([
+            'user_id' => $user->id,
+            'plan_id' => $plan->id,
+            'type' => Order::TYPE_NEW_PURCHASE,
+            'period' => Plan::PERIOD_MONTHLY,
+            'trade_no' => uniqid('race_', true),
+            'total_amount' => 0,
+            'balance_amount' => 0,
+            'status' => Order::STATUS_PENDING,
+            'commission_status' => 0,
+            'commission_balance' => 0,
+            'created_at' => time(),
+            'updated_at' => time(),
+        ], $overrides));
+    }
+}