paginate() doeesn't cache the main record

[eidng8]

The setup is phpunit & array cache driver. And I've tapped in to the cache using:

$cache = \Cache::store();

The line below:

$paginator = SomeModel::paginate($perPage)->load('relation');

After that, I've found that there are two entries in the cache: the aggregate & relation. The main model (record) is missed.

While traced a bit, I was led to the line, which traced back to query builder (not eloquent builder) getCountForPagination()

public function getCountForPagination($columns = ['*'])
{
    $this->backupFieldsForCount();

    $this->aggregate = ['function' => 'count', 'columns' => $this->clearSelectAliases($columns)];

    $results = $this->get();    // <<<<<< THIS LINE

    $this->aggregate = null;

    $this->restoreFieldsForCount();

    if (isset($this->groups)) {
        return count($results);
    }

    return isset($results[0]) ? (int) array_change_key_case((array) $results[0])['aggregate'] : 0;
}

Then the subsequent call to forPage()->get() will not hit the cache. Because the $cacheMinutes property was set to null in the closure returned by getCacheCallback(). Which ends up failing the non-null test in get().

public function paginate($perPage = 15, $columns = ['*'], $pageName = 'page', $page = null)
{
    $page = $page ?: Paginator::resolveCurrentPage($pageName);

    $total = $this->getCountForPagination($columns);

    $results = $this->forPage($page, $perPage)->get($columns);   // <<<<<<< THIS WON'T HIT CACHE

    return new LengthAwarePaginator($results, $total, $perPage, $page, [
        'path' => Paginator::resolveCurrentPath(),
        'pageName' => $pageName,
    ]);
}

[dwightwatson]

Hey, really sorry for the delay in getting to this one. Good find.

I seem to recall the cache callback being added to prevent a circular loop that popped up in an earlier version of Laravel. I will try and take a look into this soon, but also happy to review any PRs.

[eidng8]

Actually, in my project, I've change the callback to 1-liner

return function() use ($columns) {return parent::get($columns);}

It works for me. But it seems that this repo is ripped off of tests, I'm not sure if this change affects anything unwanted. So I leave it to your decision, and didn't submit a PR.