Should wrap database writes in transactions

This package has a race condition.

Lines 75 to 84 in d3b6168

    
           if($userId) { 
        
           	$like = $this->likes() 
        
           		->where('user_id', '=', $userId) 
        
           		->first(); 
        
           	if($like) return; 
        
           	$like = new Like(); 
        
           	$like->user_id = $userId; 
        
           	$this->likes()->save($like);

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

@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.

todo: wrap calls in DB::transaction()

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

	if($like) return;

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