Chris Schmitz
Chris Schmitz

Reputation: 20950

firstOrFail returning the wrong record from Eloquent Query Builder

I have a table in my databases called tallies that keeps track of the count for particular entities. The two key fields in the table are:

Right now in the table I have two records, the first record has a type of hardDrives and the second has a type of monitors.

I have a method in my repository used for incrementing the count for a particular record in a tallies table:

public function decreaseCountBy($type, $number){

    $countCollection = $this->tally->where('type', '=', $type )->firstOrFail();

    $record = $countCollection->first();

    // Troubleshooting output
    die(var_dump([$type, $record->toArray()]));

    // The rest of the method
    $record->count -= $number;
    $result = $record->save();
    if(!$result){
        throw new \Exception('Error saving decrementation');
    }
    return $record->count;
}

When I send the request to increment monitors and look at the output of the troubleshooting die and dump in this method and get the following output:

array (size=2)
  0 => string 'monitors' (length=8)
  1 => 
    array (size=5)
      'id' => int 4
      'type' => string 'hardDrives' (length=10)
      'count' => int 15
      'created_at' => string '2014-12-21 03:50:04' (length=19)
      'updated_at' => string '2014-12-21 14:35:28' (length=19)

Even though I'm using monitors as the value for $type in the query, I get the record for hardDrives.

After this, I tried changing the method to fire the query:

$countCollection = $this->tally->where('type', $type )->get();

And then I get the correct result:

array (size=2)
  0 => string 'monitors' (length=8)
  1 => 
    array (size=5)
      'id' => int 5
      'type' => string 'monitors' (length=8)
      'count' => int 3
      'created_at' => string '2014-12-21 03:50:04' (length=19)
      'updated_at' => string '2014-12-21 03:50:04' (length=19)

I could stop here and add in my own exception throw if there is an error finding the record, but when I read the API Documentation for the Builder class' method firstOrFail() (sorry I can't link directly to it), the method is described as:

Execute the query and get the first result or throw an exception.

I'd like to use the built in Laravel Exception that get's thrown when the records is not found rather than using my own.

Is there something I'm missing here? When I look for other examples in the laravel Eloquent documentation it looks like I'm building the query correctly.

More than anything else I'd like to know why it's failing rather than just a work around.

Resolution

Here's the final version of the method just to show everyone how it ended up:

public function decreaseCountBy($type, $number){
    $record = $this->tally->where('type', '=', $type )->firstOrFail();
    $record->count -= $number;
    $result = $record->save();
    if(!$result){
        throw new \Exception('Error saving decrementation');
    }
    return $record->count;
}

Normally when you perform a ->get(); to retrieve the data, the result is an eloquent Collection instance holding multiple records. From there if you wanted to retrieve only the first record you could use the ->first() method of the Collection class to get an eloquent Model class instance with that record's information.

In the case of firstOrFail(), what you're telling the query builder is that you only want the first record if found. Because you will only ever receive one record's worth of data, eloquent skips the collection and returns a model instance.

In my code above I've removed the line that "grabs the model of the first record", i.e. $record = $countCollection->first();, and renamed the variables to fit the expected results better, i.e. $record in place of $countCollection.

Upvotes: 1

Views: 2290

Answers (1)

lukasgeiter
lukasgeiter

Reputation: 152900

There's no need to call first() after you already have called firstOrFail(). firstOrFail() already returns a single model and not a collection and calling first() on a model triggers a completely new select statement (this time without the where)

As @Jarek Tkaczyk pointed out below, with your code, two queries would run against the DB

  1. select * from tallies where type = ?
  2. select * from tallies

This means in your case the result of the first query gets overwritten by the second one.

Some background information

firstOrFail() does nothing else than calling first() and then throwing an exception if first() returns null

public function firstOrFail($columns = array('*'))
{
    if ( ! is_null($model = $this->first($columns))) return $model;

    throw (new ModelNotFoundException)->setModel(get_class($this->model));
}

Upvotes: 1

Related Questions