0

I have troubleshoot all day with eager loading/n+1 issue, researched and read and watched tutorials about this issue, but haven't solved it yet. I have set up the relationships for the models, but when I passing in the data with a helper function I got this n+1 issue. I want to grab an artist name from the url site.com/Artist/songs and get all its songs and display an url like this.

site.com/$Artist/songs/$id

My artists/index.blade.php view looks like this http://i61.tinypic.com/2nqzatk.jpg I'm not sure what I'm missing here.

Thanks in advance!

My tables

songs id, title, body, slug, hits, artist_id, created_at, updated_at

artists id, name, body, created_at, updated_at

routes.php

Event::listen('illuminate.query', function($query)
{
    var_dump($query);
});
...
Route::get('{artist}/songs', 'ArtistsController@index');
Route::get('{artist}/songs/{id}', ['as' => 'artist.songs.show', 'uses' => 'ArtistsController@show']);

Model: Song.php

class Song extends Eloquent {
    protected $guarded = ['id'];

    /**
     * Setting up relationship for the artist model for easier usage
     *
     */
    public function artist()
    {
        return $this->belongsTo('Artist');
    }

    // Override find method
    public static function find($id, $name = null)
    {
        $song = static::with('artist')->find($id);

        // If the song is found
        if ($song)
        {
            // If the song doesn't belong to that artist, throw an exception which will redirect to home, defined in global.php
            if ($name and $song->artist->name !== $name)
            {
                throw new Illuminate\Database\Eloquent\ModelNotFoundException;
            }

            return $song;
        }
        // If the song is not found, throw an exception which will redirect to home, defined in global.php
        else
        {
            throw new Illuminate\Database\Eloquent\ModelNotFoundException;
        }

    }

    // Get songs from artist
    public static function byArtist($name)
    {
        return Artist::byName($name)->songs;
    }

}

Model Artist.php

class Artist extends Eloquent {
    protected $fillable = [];

    /**
     * Setting up relationship with the song model for easier usage
     * $artist->songs;
     */
    public function songs()
    {
        return $this->hasMany('Song');
    }

    // Get artist by name
    public static function byName($name)
    {
        return static::whereName($name)->first();
    }

}

Controller: ArtistsController.php

class ArtistsController extends BaseController {

    // Set default layout for this controller
    protected $layout = 'layouts.master';

    /**
     * Display a listing of the resource.
     * GET /artists
     *
     * @return Response
     */
    public function index($name)
    {
        $this->data['songs'] = Song::byArtist($name);

        $this->layout->content = View::make('artists.index', $this->data);
    }

helpers.php

function link_to_artist_song(Song $song)
{
    return link_to_route('artist.songs.show', $song->title, [$song->artist->name, $song->id]);
}

Index view for the artists artists/index.blade.php http://i61.tinypic.com/2nqzatk.jpg

@extends('layouts.master')

@section('content')

    @if(isset($songs))
        <h1>All Songs</h1>

        <ul class="list-group">
        @foreach($songs as $song)
            <li class="list-group-item">{{ link_to_artist_song($song) }}</li>
        @endforeach
        </ul>
    @endif
@stop
Vartox
  • 189
  • 3
  • 12

3 Answers3

2

You never eager load anything, that's why you could be facing n+1 issue. If I get your right, what is a bit hard with the code you have here, you want all songs of given artist with $name from the url, right?

So here's everything you need to make it work:

// controller
public function index($name)
{
    // with('songs') is eager loading related songs for you
    $this->data['artist'] = Artist::with('songs')->whereName($name)->first();

    $this->layout->content = View::make('artists.index', $this->data);
}

// the problem of your queries is in the helper:
function link_to_artist_song(Song $song)
{
    return link_to_route('artist.songs.show', $song->title, [
       $song->artist->name, // this is calling db query for each song to retrieve its artist (despite it is always the same)
       $song->id]);
}

// so instead use this in your view
@foreach($artist->songs as $song)
   <li class="list-group-item">
     {{ link_to_route('artist.songs.show', $song->title, [$artist->name, $song->id]) }}
   </li>
@endforeach
Jarek Tkaczyk
  • 78,987
  • 25
  • 159
  • 157
  • Yes, that's right I want to get all song by a given artist with $name from url. I replaced with your line of code `$this->data['songs'] = Artist::with('songs')->whereName($name)->first();` Now I getting this error with my helper. "Argument 1 passed to link_to_artist_song() must be an instance of Song, boolean given, called in..." So these methods doesn't work at all for fetching songs? song model `public static function byArtist($name) { return Artist::byName($name)->songs; }` artist model `public static function byName($name) { return static::whereName($name)->first(); }` – Vartox Apr 14 '14 at 20:01
  • I want to be able to grab the artist name and display the link like this `domain.com/ArtistName/songs/1` That's why I want the eager loading. – Vartox Apr 14 '14 at 20:11
  • You want to list all songs of a given artist with name = $name, so all you need is one artist and all related songs. Unless you want something else. check my edit in a sec – Jarek Tkaczyk Apr 14 '14 at 21:32
0

The n+1 problem exists when you have lots of songs with lots of artists. It has to get all the songs (1 query) and then for each song, get the artists (n queries).

In this case, you already know the song, so that's one query, then you need all the artists for that song, which is just one additional query.

The n+1 problem would only come into play if you were trying to find a song of a certain genre for example, which could return many songs. Then for each song, you would then have to make an additional query to get that song's artists. This would be where eager loading is most useful.

$song = Song::find($id);
$artist = $song->artist;
user1669496
  • 32,176
  • 9
  • 73
  • 65
0

Every time you grab a song, and do $song->artist it will do a query.

You can also use Query scopes.

class Song extends Eloquent {

    public function scopeByArtist($query, $name) {
        return Artist::whereName($name)->first()->songs(); //->with('artist');
    }
}

Like this the artist is already loaded.

Query with:

$songs = Song::byArtist($name)->get();
hannesvdvreken
  • 4,858
  • 1
  • 15
  • 17