Cache error with custom pagination identifier

[andrey-helldar]

Issue

If I use the pageName parameter by default, that is, page, then there are no errors:

return CarsClass::query()
    ->orderBy('title')
    ->paginate($this->per_page);

BUT if you pass the custom key name, the result of caching is the first open page. For example, the table contains 30 entries and we output the result with a pagination of 10 entries and a page. When you open the reference /?page-all-models=2, we get the elements for the first page. Next, go to the second page, but the paginator along with the data will still contain information about the first page.

return CarsClass::query()
    ->orderBy('title')
    ->paginate($this->per_page, ['*'], 'page-all-models');

Environment

Laravel Version: 5.6.23 Laravel Model Caching Package Version: 0.2.58 PHP Version: 7.2.0 Homestead Version: 7.6.0 Operating System & Version: Windows 10 Pro

[mikebronner]

@andrey-helldar Thanks for submitting this! I'll look into it ASAP.

[mikebronner]

I have not been able to reproduce this issue. Can you retest? I noticed in your example above you are using two different page identifiers, could that be the problem? In you screenshot you have page-all-models and in your code you have page-other-key. The following test passes. If you could provide more information, that would help. Thanks!

    public function testCustomPageNamePaginationFetchesCorrectPages()
    {
        $authors1 = (new Author)
            ->paginate(3, ["*"], "custom-page", 1);
        $authors2 = (new Author)
            ->paginate(3, ["*"], "custom-page", 2);

        $this->assertNotEquals($authors1->pluck("id"), $authors2->pluck("id"));
    }

[andrey-helldar]

I thought it's obvious that when using any key other than page, this problem occurs. In his comment, he brought all the keys to the same form, as I have in the code.

[mikebronner]

@andrey-helldar Could you be more specific, as my test above shows that it does work with other values as well.

[andrey-helldar]

@mikebronner Okay, tomorrow morning I'll record a video from a desktop computer. The code above is copied from the working project.

[lksnmnn]

We do experience the same issue, as we use https://github.com/cloudcreativity/laravel-json-api which has custom pagination keys like page[size] and page[number].

This crashes due to an Array to String conversion in your pagination handler. The $page variable does not have to be a String/Number, but can be an Array as well. Maybe the quickest way to solve this is by making it configurable.

[mikebronner]

@lksnmnn Please submit the full stack trace and eloquent query. That will help me troubleshoot it further. Thanks!

[lksnmnn]

Here you go: https://gist.github.com/lksnmnn/8b4e1dab69dd4a382afcb98ab55beb57

It crashes in vendor/genealabs/laravel-model-caching/src/CachedBuilder.php L130 with the following arguments:

array:4 [▼
  "perPage" => 1
  "columns" => array:1 [▼
    0 => "*"
  ]
  "pageName" => "number"
  "page" => array:1 [▼
    "size" => "1"
  ]
]

[mikebronner]

@lksnmnn Thanks for the stacktrace. Please also provide the complete eloquent query you are running that triggers this issue, so that i can recreate it.

[lksnmnn]

Had to dig into the framework... For the request /api/v1/magazines?page[size]=1 the used eloquent query is:

public function query(EncodingParametersInterface $parameters) 
{
    $filters = $this->extractFilters($parameters);
    $this->with($query = $this->newQuery(), $this->extractIncludePaths($parameters));

    if ($this->isFindMany($filters)) {
        return $this->findMany($query, $filters);
    }

    $this->filter($query, $filters);
    $this->sort($query, (array) $parameters->getSortParameters());

    if ($this->isSearchOne($filters)) {
        return $this->first($query);
    }

    $pagination = $this->extractPagination($parameters);

    if (!$pagination->isEmpty() && !$this->hasPaging()) {
        throw new RuntimeException('Paging parameters exist but paging is not supported.');
    }

    return $pagination->isEmpty() ?
        $this->all($query) :
        $this->paginate($query, $this->normalizeParameters($parameters, $pagination));
}

which results in the following MySQL query:

SELECT * FROM `magazines` LIMIT 1 OFFSET 0

[mikebronner]

Thanks for this and request URL, hopefully I will be able to recreate it with this information. Thanks!

[mikebronner]

@andrey-helldar @lksnmnn This should now be fixed in release 0.2.61. Please let me know if it works. :)

[andrey-helldar]

@mikebronner I'm sorry that I did not write in more detail about how the package behaved with the problem - there were a lot of cases.

@lksnmnn Thank you for helping with the explanation of the problem.

[lksnmnn]

I can confirm this fix made it work together with cloudcreativity/laravel-json-api. But there is a leftover from debugging, which should be removed.

+        dump($perPage, $columns, $pageName, $page);