Should wrap database writes in transactions

[olivernybroe]

This package has a race condition. https://github.com/rtconner/laravel-likeable/blob/d3b61686703a23a4aa61f593e55bc10b87f60621/src/Likeable.php#L75-L84

[rtconner]

Am I blind? I'm just not seeing the race condition.

[olivernybroe]

@rtconner Sorry, should prob. had written a more in depth description.

The code in question is the following

$like = $this->likes()
    ->where('user_id', '=', $userId)
    ->first();

if($like) return;

$like = new Like();
$like->user_id = $userId;
$this->likes()->save($like); 

So let's say two request comes in at the same time, trying to like the same thing as the same user.

Request 1, first runs the query checking if a like already exist, which is false, so it continues forward with creating the like. However before it has saved the like, request 2 hits this code and checks if a like already exist, which is false, because request 1 has not finished running it's create command.
Request 1 then finishing saving the like, but request 2 already passed the check for an existing like, so it also now tries to save a like, which will fail, as this like already exist.

To avoid we can simply wrap it in a DB::transaction.

[rtconner]

todo: wrap calls in DB::transaction()