Some feedback on Pagination

[boryn]

Hi!

It was wonderful to discover that you already had thought about automatic pagination :)

I had a few observations while implementing OffsetPaginator in our project.

• Maybe we could as well bind pagination directly to the request not generally to a connector? I now connect to an API which for some endpoints, it expects limit/skip parameters passed in GET, and for some by POST. And some endpoints don't accept limit/skip parameters at all.
• Some API don't return total count, maybe with the option $noTotal, you could make a isFinished() check based on the count of currently fetched items?
• Due to some bad configuration, pagination may hit an endless loop. Maybe it would be good to introduce an "emergency" maximum iterations limit? So that if we already do 100 calls, isFinished() will return true / throw an exception?
• Having some params at defaultQuery() in the connector, it seems these values are not merged in at applyPagination(). Only values defined at defaultQuery() in the request are merged.
• Already mentioned that $paginator->setCurrentOffset() is not implemented.

I solved some of the above issues using a custom paginator:

use Saloon\Contracts\Request;
use Saloon\Enums\Method;
use Saloon\Http\Paginators\OffsetPaginator;

class CustomOffsetPaginator extends OffsetPaginator
{
    protected int $iterationsCnt = 0;
    protected int $iterationsMax = 100;

    protected function isFinished(): bool
    {
        $this->iterationsCnt++;

        if ($this->iterationsCnt > $this->iterationsMax) {
            return true;
        }

        return count($this->currentResponse->json('data')) < $this->limit();
    }

    protected function applyPagination(Request $request): void
    {
        if ($this->originalRequest->getMethod() === Method::POST) {
            $request->body()->merge([
                $this->getLimitKeyName() => $this->limit(),
                $this->getOffsetKeyName() => $this->currentOffset(),
            ]);
        } else {
            $request->query()->merge([
                $this->getLimitKeyName() => $this->limit(),
                $this->getOffsetKeyName() => $this->currentOffset(),
            ]);
        }
    }
}

IMHO it would be great to have the support for for different request methods (GET/POST, etc.) and stopping the loop without the "total" information out of the box in the library.

[sunscreem]

Just a quick +1 for the option to have $noTotal or similar. The API I'm accessing doesn't have a total annoyingly.

[Sammyjo20]

Hey @boryn many thanks for leaving this issue for me to look at - I’ve actually been working on how pagination will work for v3 of Saloon and I’d love for you to take a look.

I’ve simplified quite a lot of it, and you now are required to define your own “isFinished” so it doesn’t matter if your pagination doesn’t contain a total, you can determine it in any other way (for example one API I used recently would return exactly 1,000 records if there is another page)

https://github.com/Sammyjo20/saloon-pagination-v2

I’m still considering how the loop protection will work - but with the new paginator there won’t be an interface requiring you to put it on the connector - so you can either use it on the connector or against a request.

[boryn]

Hi @Sammyjo20!

I'll be away for some days now, but please in the meantime tell me how I should combine this new version of pagination with existing project using Saloon v2?

[Sammyjo20]

Hey @boryn - you can start using it now - but I won't be making a separate Packagist package for it, so for now just to test it - add the custom repository to your "repositories" array in composer.json and then install it via Composer

"repositories": [
    {
        "type": "vcs",
        "url": "git@github.com:Sammyjo20/saloon-pagination-v2.git"
    }
],

Then:

composer require sammyjo20/saloon-pagination

[boryn]

Is this version backwards compatible or I may expect some breaking changes in the existing implementation?

[Sammyjo20]

@boryn It works with Saloon v2 - but be careful as it's not fully tested yet.

[boryn]

Sure, I'll give some feedback (but not this week)

[MatthewLoffredo]

Just commenting that v2 is working great for me, where my API has slightly irregular pagination parameters and I had to add custom logic!

[Sammyjo20]

Hey @MatthewLoffredo when you say v2 do you mean the pagination v2 or just Saloon v2? 😀

[MatthewLoffredo]

Hey @MatthewLoffredo when you say v2 do you mean the pagination v2 or just Saloon v2? 😀

I meant pagination v2! Also, I think that an example of how to use pooled async requests with pagination would be helpful. The docs don't quite show you how to implement a method to return a collection of many async paginated results at once. Here's a method I implemented in my paginator that's working for me:

public function collectAsync(): Collection
  {
    $allResults = collect();
    $pool = $this->pool(
      concurrency: 5,
      responseHandler: function (Response $response) use (&$allResults): void {
        $allResults = $allResults->concat($this->getPageItems($response));
      }
    )->send();
    $pool->wait();
    return $allResults;
  }

Then, after instantiating my paginator, I can return the results easily and cleanly from the controller with one line:

return $paginator->collectAsync();

This is assuming that you have implemented all the other methods correctly. Love this package btw, multiple pages of results in a fraction of the time is a game-changer, especially since my API has a hard limit on the per_page parameter!

[Sammyjo20]

Hey @MatthewLoffredo Thanks for the awesome feedback, I'm glad you love it! 🔥 I'd love to swap out and use this paginator way right away but I will be releasing this as part of Saloon v3 which I am currently scoping out.

Regarding your collectAsync() method this is a great idea thank you - my only question is with the $pool->wait() and it's more on my lack of understanding about pools/async but does this force it to send all the requests synchronously or will it still send the requests in parallel / concurrently?

[MatthewLoffredo]

My understanding is that $pool->wait() will just wait for all the promises from the pool to finish, they will still be in parallel though. So for example, if I use $paginator->setMaxPages(4); and then $paginator->collectAsync();, it will pull pages 1, 2, 3, and 4 concurrently, but I still want to return the results to my frontend, so I'm waiting for them all to finish first and then aggregating them. This seems to be the behavior I'm seeing.

[Sammyjo20]

This is super cool. Grabbing multiple pages and returning a single collection 🕺

I'll put the finishing touches on this and then I'll get to work on Saloon v3 :)

[boryn]

Hi @Sammyjo20!

Here is some of my feedback about Pagination v2. I gave it a try with the PagedPaginator type.

• Actually, at the API I'm connecting to, the 'data' key changes from endpoint to endpoint. Previously I had defined $apiDataKey at the request level and got it by $this->originalRequest->getApiDataKey(). Now I can see that $this->originalRequest is not accessible and I think it would be good to have it available as well. This I would use in the getPageItems(): return $response->json($this->originalRequest->getApiDataKey()) ?? [];. For now, I "hardcoded" the individual data key.
• It's not clear where we should specify the per_page parameter? I have put it into defaultQuery() at the request class. How do we customize the name of it, e.g. perpage?
• Now $paginator->collect() always returns a LazyCollection? What if we need a regular Collection?
• I saw a suggestion of converting items into DTOs at the getPageItems() level. But again, each endpoint requires to have a different DTO definition and finally we would come up with different paginator classes for different endpoints which would not be convenient. I'd rather implement converting to DTOs more at the same level as ->collect(), e.g.: $paginator->dto(), but still this can be easily done "manually" with ->map().
• I saw there is ->setMaxPages() and protected ?int $maxPages which is great. I still think about some iteration counter that would act as an "emergency exit" when we misconfigure paginator or API changes. This would stop running the script after X iterations, instead of running an endless loop trying to open the same or wrong URL.
• Now the only way to init the paginator is through $paginator = new SuperheroPaginator($connector, $request);? I liked the previous version of starting the paginator through the connector. We have many integrations with different APIs and it was very simple to have very similar code and either send a request through a regular connector or through a connector using paginator. So I would not abandon the paginate() method in the connector. To illustrate:

  if (!$this->usePagination) {
  $response = $this->integrationApi->connector()->send($this->request);
  } else {
  $response = $this->integrationApi->connector()->paginate($this->request);
  }

[ju5t]

New to Saloon (unfortunately, it could have saved us a LOT of time). Amazing work @Sammyjo20!

I'm particularly interested in pagination with DTO's. But the current DX feels off. Having 2 classes for a request that can be paginated is a bit much when you need simple pagination, and the current implementation doesn't seem to support DTO's per request type, as @boryn mentioned too.

Ideally, I would prefer the call to look something like this:

$connector->send($request)->paginate()

And define the mapping in the request, similar to Laravel Excel. This is where you map your DTO's (if you want).

There could still be support for custom paginator classes. You could configure them on the connector or the request level, where the request takes precedence over the connector if both are configured.

[Sammyjo20]

Hey @ju5t many thanks for your input and feedback - just to clarify are you talking about Saloon's new pagination?

Yeah this is quite a good idea - do you might providing a bit more of an example of what you were thinking in regards to the paginator? Do you still think from within the paginator we would have a getPageItems but inside of each class you could define a method like mapPaginatedRow?

Something like...

Paginator

use Sammyjo20\SaloonPagination\Paginators\PagedPaginator;
use Saloon\Contracts\Response;

class SuperheroPaginator extends PagedPaginator
{
    protected function isLastPage(Response $response): bool
    {
        return empty($response->json('next_page_url'));
    }

    protected function getPageItems(Response $response): array
    {
        return $response->json('data') ?? [];
    }
}

Request

class SuperheroRequest extends Request implements MapPaginatedResults
{
     public function mapPaginatedRow(array $row): Superhero
     {
           return Superhero::fromArray($row);
     }
}

[ju5t]

This is exactly what I meant!

With the addition of specifying the paginator on either the connector or request. For example:

class ForgeConnector extends Connector implements WithAdjustedPagination
{
    protected function getPaginator(): PagedPaginator
    {
        return SuperheroPaginator::class;
    }
}

class RetrieveServers extends Request implements MapPaginatedResults, WithAdjustedPagination
{
    protected function getPaginator(): PagedPaginator
    {
        return RetrieveServersPaginator::class;
    }

    protected function mapPaginatedRow()
    {
    }
}

$forge = new ForgeConnector;
$forge->send(new RetrieveServers)->paginate();

• In the example it will use the RetrieveServersPaginator class for pagination.
• If the request didn't specify a paginator, it would use SuperheroPaginator
• If the connector doesn't specify one either, it should use a sensible default.

I think paginator classes should only be used for edge cases. I think it's good idea to have methods like getPageItems in a custom paginator. As some API's are weird.

[Gummibeer]

Maybe we could as well bind pagination directly to the request not generally to a connector? I now connect to an API which for some endpoints, it expects limit/skip parameters passed in GET, and for some by POST. And some endpoints don't accept limit/skip parameters at all. — @boryn

I had a short chat with @Sammyjo20 yesterday about exactly that. I would like to have 4 interfaces being part of the package:

• Paginatable (base Interface for all 3)
• PagedPaginatable
• OffsetPaginatable
• CorsorPaginatable

These four interfaces would be applied to the requests that are paginatable. That way the user and/or package can check if the request passed to the paginate() function is really paginatable at all. It would also allow to implement different paginators within one connector.

The second thing I would like is the paginate() method on the request class - as the request is paginated and not the connector and also the request defines how it's paginated. For SDKs with the same paginator everywhere that could be easily solved with a custom trait you can pull in on every paginatable request.

All of that could be done in a none breaking way: the interfaces can be added and verified/used in userland code only. And the move to request instead of the connector could be done with the connector as a fallback. Both can be cleaned up with v3.

[ju5t]

The second thing I would like is the paginate() method on the request class - as the request is paginated and not the connector and also the request defines how it's paginated.

@Gummibeer for my understanding, how would this look code-wise? I don't fully grasp what you mean by specifying paginate() on the request class. Is this similar to the example code I shared earlier or is it a different concept?

[Gummibeer]

@ju5t right now the paginate() method lives in the connector - it can be moved to the request with nearly the exact same signature. Beside that it wouldn't need the request as an argument as it's available as $this and would need the connector instead. So no big change except where the method is defined.

[ju5t]

Don't you mean the response? Because right now send returns a response. This is why you've lost me a bit.

My preferred DX would be (as mentioned earlier in an earlier comment https://github.com/Sammyjo20/Saloon/issues/192#issuecomment-1604800219):

$forge->send(new RetrieveServers)->paginate();

Where the response contains the paginate-method and the request defines the pagination class.

[Gummibeer]

No, as the response isn't paginatable. It's only paginated. Without the Paginator classes you would have the $page argument on the request which passes it to the defaultQuery(). So having the logic how the request is adjusted for pagination on the request is better than having a special response class that would have to know which request it relates to and how to adjust and repeat that request.

Don't get me wrong - it's only about the definition of the paginator and its configuration. Calling the $connector->paginate() like $connector->pool() to start paginating and get all the responses is still what I want as the public API. But that method shouldn't define the paginator to be used but only call $request->paginator(Request&Paginatable $request): Paginator.

[ju5t]

Got it. Sounds nearly identical to what I mentioned before, but a different approach and probably no breaking changes.

No, as the response isn't paginatable. It's only paginated.

Might have been a bit too quick with my previous reply. In my example send could be seen as a way to prepare the request, where paginate returns the result. Similar to the query builder in Laravel for example. send could be called request too.

$forge->request(new RetrieveServers)->paginate();
$forge->request(new RetrieveServers)->get();

I know it's not really necessary, but it would open up possibilities to implement things like:

$forge->withoutRateLimit()->request(new RetrieveServers)->paginate();

My biggest concern was not being able to map individual data/records into a DTO, but what you describe doesn't block that for v3. That was my biggest concern, I don't have a strong opinion on the current or future DX, so whatever it will be I will use :)

[Gummibeer]

So my idea in quick pseudo-code:

The public usage is like now:

$forge->paginate(new RetrieveServers);

But the underlying logic is: ForgeConnector

class ForgeConnector extends Connector
{
  use HasPaginatedRequests; // adds the predefined `paginate()` method
}

HasPaginatedRequests

trait HasPaginatedRequests
{
  public function paginate(Request&Paginatable $request, mixed ... $additionalData): Paginator 
  {
    return $request->paginator($this, ...$additionalData);
  }
}

RetrieveServers

class RetrieveServers extends Request implements PagedPaginatable
{
    public function paginator(Connector $connector, mixed ... $additionalData): Paginator // required by interface
    {
        return new PagedPaginator($connector, $this, ...$additionalData);
    }
}

With that example it should be clear that it's not really about changing who sends the request or who prepares it or what the paginator does. All that can remain or be changed. It's only the place where the paginator is instantiated and with the interfaces and the intersection type it's impossible to paginate a request that's not paginatable anymore. The interface requires the paginator() on the request and the HasPaginatedRequests::paginate() method only allows Request&Paginatable and just calls the paginator() method. That way it's also just a shorthand as you could also do:

$request = new RetrieveServers();
$request->paginator($forge)->collect();
// vs
$forge->paginate(new RetrieveServers())->collect();

[Sammyjo20]

Hi all!

Many thanks for this great discussion around pagination and all your contributions to help make it even better. If you are not already aware, I have a work-in-progress repo for the next version of pagination which I will be continuing to update based on feedback leading up to the release of v3 (no release date yet)

Current WIP repo: https://github.com/saloonphp/pagination-v2

I'll assume that everyone has had a read of how the new pagination may work but based on your feedback I'm proposing a newer, better way of doing it - a hybrid between what we have now and what we have in the new version.

Let's just quickly cover the positives and negatives of what we have now and what I am proposing

Saloon v2 Pagination (Current)

• Great for JSON APIs
• Great having the paginate() method on the connector
• Difficult to configure as different paginators have different arguments and the ...$additionalArguments is hard to get around
• Not very developer-friendly
• Doesn't work well for non-JSON APIs as there are hard-coded methods to resolve the next page etc.

Saloon v3 Pagination (At the time of writing)

• Should work with all types of APIs
• Really customizable
• Class-based to keep it tidy
• Not so nice because it doesn't have the paginate() method on the connector
• Another class that you have to make over and over again for each integration

Overall I think the newer pagination is much better than the old but I wanted to collate all your feedback and propose a revised version. If people like it, then I will go ahead and start making changes to the repo.

Proposed Pagination

So personally I think we shouldn't have it class-based anymore because it's just another class to maintain but I think we should actually define all of the methods on the connector or request like @ju5t suggested with interfaces like @Gummibeer has suggested. It's easier to show some examples and explain.

Let's say you wanted to have the same pagination for every request on a connector. You would first add the interface of the target pagination: WithPagedPagination, WithOffsetPagination, WithCursorPagination or WithPagination if you want to write a custom paginator.

Then, you add the trait HasPagination. This places the paginate() method on your connector, just like you currently have which I think everyone likes.

The interfaces will require you to define a few methods. Let's take the WithPagedPagination interface as an example. When you add the interface, you must define the following methods:

• isLastPage
• getPageItems

Note It may be that we rename the methods to better suit being in a connector/request like isPaginatorOnLastPage or something better.

class ForgeConnector extends Connector implements WithPagedPagination
{
     use HasPagination;

     protected function resolveBaseUrl(): string
     {
          return '...';
     }

    protected function isLastPage(Response $response): bool
    {
        return empty($response->json('next_page_url'));
    }

    protected function getPageItems(Response $response): array
    {
        return $response->json('data') ?? [];
    }
}

Next, all you have to do is call your requests using the paginate method - and Saloon will do the rest for you.

$connector->paginate(new GetServersRequest);

Now, sometimes you may need to have a separate strategy of pagination for specific requests, or let's say in a worst-case scenario every request of an API integration has different pagination. I think that you should use the same exact interfaces in the same way as the connector...

You would still use $connector->paginate($request) but instead, Saloon will check if the request is an instance of WithPagedPagination etc and would use the request's methods instead of the connector. If you don't have pagination on the request, it will fall-back to the connector for global configuration, just what the connector is for.

class GetServersRequest extends Request implements WithPagedPagination
{
    // ...

    // Same exact methods!

    protected function isLastPage(Response $response): bool
    {
        return empty($response->json('next_page_url'));
    }

    protected function getPageItems(Response $response): array
    {
        return $response->json('data') ?? [];
    }
}

This way you have the following benefits:

• No extra classes
• Everything stays within the domain of the connector/request
• Every data type will work because you get access to the $response object (@Gummibeer)
• Can be easily customised if an API needs to implement a slightly different pagination strategy.

What do you guys think?

Of course I will still collate the rest of your feedback like @boryn's request to have the $originalRequest in the paginator and things like that.

[ju5t]

Sounds good!

The interfaces will require you to define a few methods. Let's take the WithPagedPagination interface as an example. When you add the interface, you must define the following methods:

Does this mean we would have to implement this for every connector?

For repeatability, I would prefer classes over traits, but it's not something I have a strong opinion on.

[Sammyjo20]

@ju5t Yes in theory you would - but it's two methods so hopefully it's not too labourious? I could absolutely provide opinionated traits that implement the methods with some sensible defaults but it's tricky because unless you're dealing with a Laravel API it's hard to trust an exact implementation of pagination

[Sammyjo20]

I'm going to toy with the idea that internally it will still go into a "Paginator" class like pagination v2 currently works so maybe I can expose a method like createPaginator which returns a paginator class that you can build up yourself exactly how it works in the WIP branch currently...

[Sammyjo20]

Tagging @juse-less to get their opinion on things 👍👌

[juse-less]

Thanks, @Sammyjo20!

I'm honestly not sure how everything can be solved nicely. Both from an implementation perspective, but also from a developer/'user' perspective.

It's been on my mind for so long, in order to also figure out the resumable/continuable paginator feature. Dropping the idea of that feature, which is dependent on serialising and unserialising data in a safe way, would make things a bit easier overall.

I feel like my mind is all over the place with this, but hopefully some flashbacks to when we worked with- and designed it, might help.

One of the things I recall us going back and forth with is solving pagination in a generic way. But even when looking at RESTful APIs, there were still a plethora of ways to paginate. I remember us being stuck quite a bit on this. The ones we primarily tried targeting were

• page-based, where query parameters are used to select a page.
• offset-based, where query parameters are used to get N entries, with an offset of M.
• I believe we also found another case/situation similar to page and offset, but can't quite recall what.
• cursor-based, where query parameters are used to select 'next' or 'previous' page, without being able to control offsets or anything.

That's only to handle the 'paging' itself. One issue we tried dealing with here was to resolve next/previous pages, and all that, in a generic way. One of the first things we tried with, is something that @Gummibeer raises in #258, in order to customise things without needing to create a completely new paginator. Where users could overwrite a lot of the logic by providing callables. However, part of the reason we went away from that was the resumable paginators, since non-classables are tricky to correctly serialise. But I believe we also ended up in a situation where it was just too much with what we were trying to achieve. Which is why we removed that possibility and attempted other things.

One of the issues we were also dealing with was an interface that fit all paginators, which is really what made it so incredibly hard (since all gazillion APIs out there works differently). In a hindsight, I think that what @Gummibeer outlines in one of his message in this issue is the best path, at the end of the day. Then we can define the specification for the respective type of paginator around paginating itself.

The base Paginator interface can then define the more basic methods like pool(), and async(). Possibly also something like hasMore(), or something. But not all pagination supports determining that, so I'd probably not add it on the base paginator interface.

Instead of using json(), I'd probably rename it something more generic (data()?). Otherwise it could be a bit misleading. Which also partly brings me to a semi-off topic issue around paginators and JSON, that @Gummibeer mentioned in #258. You can see my thoughts on that in the 3rd/last section of my response.

A change to these json()/data() and collect() methods would be to never return responses. See @boryn's issue in #250, where it's causing issues because we wanted to do too much in the same method. Instead, we should separate the responses to a separate method (also on the Paginator interface), which I suggest in my response in that issue. These methods should still work to define on the base Paginator.

In a hindsight, paginators are technically self-contained, as long as you can trigger them. So only defining triggering methods on the base Paginator interface should be enough. That could be pool(), data(), or collect(). But if paginators should still be iterable (which I still prefer myself), then iterating it would also trigger things. And we therefore don't need to define 'last'/'first'/'next'/'previous'/etc methods on that interface. Which is really the issue we were trying to resolve.

In terms of where paginator logic should reside, in my mind, they should still have their own classes. It just doesn't make sense to me that a connector or request itself paginates in that sense. A request defines what is supposed to be sent, and a connector really just handles the connection between the API client and the SDK. So continuing to define a single paginate() (or similar) method on the connector sounds better to me. I know we discussed adding that on the request at the time, but we felt it was wrong. Now that it's being brought up again, I'm thinking that it can actually make sense from a 'paginate this request through this connector' perspective. I.e., Connector::paginate() takes a request, and Request::paginate() takes a connector. By offering both this way, a connector can have a default paginator, and a request can have a separate one, which takes precedence like with other things in Saloon.

With all these things in mind, like separating interfaces depending on paginator type, I think we should re-iterate the decision on removing the support for callables for certain logic. I think the different paginator type interfaces should have sensible 'with'-ers, where key logic (like f.ex. resolveLimit(), etc) can be overridden using callables.

Another important note we faced along the way are APIs where you can't determine total pages in any capacity. Those were really hard to do asynchronously, since we can't determine how many requests we can actually send. I.e., we basically have to run it synchronously, and send a new request once we have a response that doesn't imply there's no more entries to be retrieved. Therefore pooling didn't work well either (which is using async requests). So that should also be kept in mind.

I hope I managed to summarise my thoughts somewhat. Let me know if I missed anything, or if I need to clarify something. 🙂

[Sammyjo20]

I totally agree @juse-less thanks for your input. I'll give myself some time to mull all these ideas over.

I think perhaps something on the lines of what we have now, but implementing @Gummibeer's ideas of customizable closures on the paginators alongside using interfaces to remove the ...$additionalArguments property on the paginate() method.

I also think making the interfaces request/connector agnostic is important too, because then you can have a custom pagination strategy for requests only.

Appreciate the patience guys!

[Gummibeer]

I think that all paginators would be reducible to two things:

• has next page - can be do e by a page check, a next url key or a simple count less than
• modify the next request - adding required things

If we would go down to that as the most fundamental paginator, driven by closures, it would be the most flexible as well. And on top the page could provide specific ones for the common scenarios.

The only thing that wouldn't support is sending all requests in an async pool. To do so we would also need the number of pages.

[Sammyjo20]

I agree with both @juse-less and @Gummibeer

I'll work on the WIP repo to allow closures on various paginators.

What do you guys think about making it a requirement to define those closures? I'm just thinking back to the current implementation in Saloon v2 where we have "opinionated" paginators for common use cases. In my example, I've always had to overwrite it, so I'm tempted to just make someone define the functions.

Cheers!

[Gummibeer]

How about a dedicated saloon paginator repo? The only thing the basic repo has to define is the UX Handy method on the connector. But now saloon needs a v3 bump "only" for pagination and the pagination has to remain till a new major version of saloon. But somehow it feels more like a plugin and not basic Feature.

I think it's a general decision how focused and limited saloon should be. And if it's wanted to have official plugins that extend the package.

[Sammyjo20]

Absolutely agree @Gummibeer!

[Sammyjo20]

What do you guys think of this?

1. You add the HasPagination to your connector and define the paginate() method
2. You choose to either do: A. Use one of Saloon's built-in paginators, and define a couple of callbacks B. Write your own paginator class that extends Paginator

Let's assume route A (the most common) I think the best strike between DX and customizability is to force the user to define when the paginator is on the last page. You will be given access to the last response as well as the paginator which has useful methods to run calculations based on total pages / results in case an API doesn't have a "next" URL.

<?php

class MyConnector extends Connector implements HasPagination
{

    public function paginate(Request $request): PagedPaginator
    {
        // The three lines below are only needed if you want a separate request pagination

        if ($request instanceof HasRequestPagination) {
            return $request->paginate($this);
        }

        // Return the paginator and define the two closures

        return new PagedPaginator(
            connector: $this,
            request: $request,
            isLastPage: static function (Response $response, PagedPaginator $paginator): bool {
                // Write your own logic

                return $paginator->getPage() - 1 === $response->json('total') / $response->json('per_page');

                // return empty($response->json('next_page_url'));
            },
            getPageItems: static function (Response $response): array {
                return $response->json('data') ?? [];
            },
            // Only provide if you want async ⬇️
            getTotalPages: static function (Response $response): int {
                return $response->json('total') / $response->json('per_page');
            }
        );
    }

}

[Gummibeer]

The getPageItems callback because the $connector->paginate()->collect() returns a collection of all items instead of the current responses? And a dedicated ->responses() would return the responses in case someone needs them?

[Sammyjo20]

Yep that's right. I'm going to change it so the default behaviour in the foreach loop will iterate over the items:

foreach($connector->paginate($request) as $item)

But then if you want responses instead you would do:

foreach($connector->paginate($request)->responses() as $response)

What do you think?

[Sammyjo20]

Hey @Gummibeer @juse-less I've just had an idea which may be the best combination of both class-based paginators and a nice DX. What about using anonymous classes like this? The IDE picks up that you need to define the abstract methods, but you also have the ability to define it all inside of the connector, no need to make your own class

 public function paginate(Request $request): CursorPaginator
{
    if ($request instanceof HasRequestPagination) {
        return $request->paginate($this);
    }

    return new class($this, $request) extends CursorPaginator {
        //
        protected function getNextCursor(Response $response): int|string
        {
            // TODO: Implement getNextCursor() method.
        }

        protected function isLastPage(Response $response): bool
        {
            // TODO: Implement isLastPage() method.
        }

        protected function getPageItems(Response $response): array
        {
            // TODO: Implement getPageItems() method.
        }
    };
}

Just tested this locally and it works great

[Sammyjo20]

Additionally, you can add the HasAsyncPagination trait directly on the anonymous class if you want to include async() pagination!

[Sammyjo20]

I've polished it a little bit and updated the PagedPaginator with this new style. With named arguments and some comments, this looks really pretty!

The other advantage to this approach is that if people wanted to write their own classes, they could just return it here instead, but it gives us so much more flexibility and there aren't anonymous functions everywhere where you can't type-hint the arguments.

/**
 * Paginate over each page
 * 
 * @param \Saloon\Contracts\Request $request
 * @return \Sammyjo20\SaloonPagination\Paginators\PagedPaginator
 */
public function paginate(Request $request): PagedPaginator
{
    if ($request instanceof HasRequestPagination) {
        return $request->paginate($this);
    }

    return new class(connector: $this, request: $request) extends PagedPaginator {
        /**
         * Check if we are on the last page
         */
        protected function isLastPage(Response $response): bool
        {
            return empty($response->json('next_page_url'));
        }

        /**
         * Get the results from the page
         */
        protected function getPageItems(Response $response): array
        {
            return $response->json('data') ?? [];
        }
    };
}

[Sammyjo20]

Also just wanted to @boryn @ju5t @MatthewLoffredo

Here's an update of progress and new features of the future pagination as I have heard all your feedback.

New Paginator Features

• Use of anonymous classes for beautiful DX (See above, still pending but I'm pretty happy)
• Each paginator has async() capabilities by default but you must define an additional method
• collect() will iterate through individual items NOT responses by default
• items() to iterate through each item
• collect() and items() now work with async and sync (async will return promises for each)
• There is now getOriginalRequest inside of the paginator
• Because the paginator is an anonymous class you can access the methods of the class like "totalResults"
• You can now overwrite the $maxPages property on the paginator to set an upper maximum to prevent infinite loops
  • Maybe we need maxTime?
• You can add the HasRequestPagination on a request and then the following three lines to have pagination on a per-request basis

Todo

• DTOs - Do we use the existing DTO conversion logic on the request or have our own? What if we just provide the request in the method so you can use the createDtoFromResponse method?

/**
 * Get the results from the page
 */
protected function getPageItems(Response $response, Request $request): array
{
    return $request->createDtoFromResponse($response);
}

Or I can provide a cool closure you can just call? (Which just proxies to the createDtoFromResponse method)

/**
 * Get the results from the page
 */
protected function getPageItems(Response $response, \Closure $convertToDto): array
{
    return $convertToDto($response);
}

[Sammyjo20]

Oooh yeah I do think the closure is sexy. Just write this and it'll use your createDtoFromResponse method on your request just like converting into DTOs normally...

/**
 * Get the results from the page
 */
protected function getPageItems(Response $response, Closure $useDto): array
{
    return $useDto();
}

...

$dtos = $connector->paginate($request)->collect();

dd($dtos->first()); // Object

I guess the only downside is that it reduces readability because if someone approaches this line of code, they won't know what "useDto" does.

[Sammyjo20]

Hey all - just a little update on this. I have reached a final version of the pagination plugin which I think I'm going to ship with v3 support.

Here's an example of a paginator - you'll be able to overwrite the methods really easily as well as overwrite how the paginator applies the pagination behind the scenes, which is all simplified and standardised - no more complicated methods and big differences between all three paginators.

<?php

class SpotifyConnector extends Connector implements HasPagination
{
    // ...

    public function paginate(Request $request): PagedPaginator
    {
          return new class(connector: $this, request: $request) extends PagedPaginator {
              /**
              * Check if we are on the last page
              */
              protected function isLastPage(Response $response): bool
              {
                 return empty($response->json('next_page_url'));
              }

              /**
              * Get the results from the page
              */
              protected function getPageItems(Response $response, Request $request): array
              {
                 return $response->json('data') ?? [];
              }
          }
    }
}

It will still support async requests, pooling - but will now have the items() method which allows you to traverse through the items and of course the collect() now goes through the items by default.

You'll be able to access properties like the current page, total results, original request, the connector all through the paginator.

You'll also be able to apply interfaces to your requests to overwrite the getPageItems method on a per-request basis - or you can even add a HasRequestPagination interface to your request to have custom request based pagination if it's completely different.

Overall the new pagination is epic - and all thanks to your feedback - so thank you very much.

[MatthewLoffredo]

Love these updates, thanks so much @Sammyjo20 !

[Sammyjo20]

It's a pleasure! I'm going to close this issue now as I'm working on v3 and you can already see a stable alpha version of the new pagination - you can even install it into Saloon v2 if you want to try it early.

Documentation for it will come with the release of v3 - which will be in the next couple of weeks!

https://github.com/saloonphp/pagination-plugin

[Sammyjo20]

Hey folks just wanted to pop my head in and let you know that the new pagination has been released for v2 as well 🙌 If you fancy upgrading to it earlier, the full docs are here: https://docs.saloon.dev/digging-deeper/pagination-v2