bavix / laravel-wallet

It's easy to work with a virtual wallet
https://bavix.github.io/laravel-wallet/
MIT License
1.15k stars 230 forks source link

Race Conditions On Wallet Creation #1009

Closed ISNewton closed 1 week ago

ISNewton commented 1 week ago

Describe your task I have 2 API endpoints which reading the user wallet transactions . The API client sends concurrent requests to the APIs. The backend perform a check for the wallet existence before calling the walletTransactions relation :

    public function getStudentTransactionsInPeriod(Student $student, Episode $episode, $from = null, $to = null, Wallet|null $wallet = null): Collection
    {
        $from = $from ? Carbon::parse($from)->startOfDay() : Carbon::today()->startOfDay();
        $to = $to ? Carbon::parse($to)->endOfDay() : Carbon::today()->endOfDay();

        $wallet = $wallet ?: $student->createStudentWallet($episode->id);

        return $wallet->walletTransactions()
            ->whereIn('reason', [
                TransactionReasonEnum::DESERVED_POINTS,
                TransactionReasonEnum::CANCEL_DESERVED_POINTS,
                TransactionReasonEnum::DIRECT_ADMIN_OPERATION,
                TransactionReasonEnum::TEACHER_TRANSFER,
                TransactionReasonEnum::SUPERVISOR_TRANSFER,
            ])
            ->whereRaw('DATE(created_at) >= DATE(?)', [$from])
            ->whereRaw('DATE(created_at) <= DATE(?)', [$to])
            ->get();
    }
    public function getStudentTransactionsQuery(Student $student, Episode $episode, Wallet|null $wallet = null): Builder
    {

        $wallet = $wallet?:  $student->createStudentWallet($episode->id);

        return $wallet->walletTransactions()
            ->whereIn('reason', [
                TransactionReasonEnum::DESERVED_POINTS,
                TransactionReasonEnum::CANCEL_DESERVED_POINTS,
                TransactionReasonEnum::DIRECT_ADMIN_OPERATION,
                TransactionReasonEnum::TEACHER_TRANSFER,
                TransactionReasonEnum::SUPERVISOR_TRANSFER,
            ])
            ;
    }

The createStudentWallet just creates a wallet if doesn't exist:

    public function createStudentWallet($episodeId)
    {
        if ($this->hasWallet(self::getStudentWalletKey($episodeId))) {
            return $this->getWallet(self::getStudentWalletKey($episodeId));
        }

        $episode = Episode::find($episodeId);

        return $this->createWallet([
            'name' => 'Student wallet',
            'slug' => self::getStudentWalletKey($episodeId),
            'meta' => ['is_active' => true, 'episode_id' => $episodeId],
            'episode_id' => $episodeId,
            'institution_id' => $episode->institution_id,

        ]);
    }

I already set the driver to redis in the config:

    /**
     * Storage of the state of the balance of wallets.
     */
    'cache' => ['driver' => 'redis'],

    /**
     * A system for dealing with race conditions.
     */
    'lock' => [
        'driver' => 'redis',
        'seconds' => 1,
    ],

I am also using custom models for wallets:

    /**
     * Base model 'transaction'.
     */
    'transaction' => [
        'table' => 'transactions',
        'model' => \App\Models\UserTransaction::class,
    ],

    /**
     * Base model 'transfer'.
     */
    'transfer' => [
        'table' => 'transfers',
        'model' => Transfer::class,
    ],

    /**
     * Base model 'wallet'.
     */
    'wallet' => [
        'table' => 'wallets',
        'model' => UserWallet::class,
        'creating' => [],
        'default' => [
            'name' => 'Default Wallet',
            'slug' => 'default',
            'meta' => [],
        ],
    ],

The Issue The Issue is when client sends 2 concurrent requests to the 2 API endpoints , I get the following error :

SQLSTATE[23000]: Integrity constraint violation: 1062 Duplicate entry 'App\Models\Student-1009-episode-wallet-1860' for key 'wallets.wallets_holder_type_holder_id_slug_unique' (Connection: mysql, SQL: insert into `wallets` (`balance`, `decimal_places`, `uuid`, `name`, `slug`, `meta`, `episode_id`, `institution_id`, `holder_type`, `holder_id`, `updated_at`, `created_at`) values (0, 2, a35bcd1e-c940-45b1-aef6-02fb96d26bfd, Student wallet, episode-wallet-1860, {"is_active":true,"episode_id":1860}, 1860, 14, App\Models\Student, 1009, 2024-11-18 16:42:54, 2024-11-18 16:42:54))

I believe the two endpoints try to create the wallet event though I already set the lock driver to redis

Server:

rez1dent3 commented 1 week ago

@ISNewton Hello. For this case, you need to do the blocking yourself, because the wallet does not do it when creating a wallet.

And in your case, the competitive request falls and this is correct. The main thing here is not to create unnecessary operations, which is what happens.