Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

V2 pagination with getValue unclear #1491

Open
uncaught opened this issue Mar 26, 2024 · 3 comments
Open

V2 pagination with getValue unclear #1491

uncaught opened this issue Mar 26, 2024 · 3 comments
Labels
Needs: Attention 👋 question Customer is asking for a clarification, use case or an information. Use it for Issues of type Questi

Comments

@uncaught
Copy link

uncaught commented Mar 26, 2024

The documentation Get a collection of items mentions paging, but the example and code are not entirely clear here.

The call $messages->getValue() from the example would return a native php array type.

Is that array already filled with all items from all pages? Is that guaranteed? Or do I only get a single page here?


On a side note, it might have been better to return a \Generator that can be iterated over, independent on the page size, that will simply call the next page when it's its turn.

@SilasKenneth SilasKenneth added the question Customer is asking for a clarification, use case or an information. Use it for Issues of type Questi label Apr 3, 2024
@SilasKenneth
Copy link
Member

SilasKenneth commented Apr 3, 2024

Hi @uncaught , thanks for trying out the SDK.

Is that array already filled with all items from all pages?

No, it only contains a single page.

To get the items on all pages, use the PageIterator, which is described in the next section titled Paging through a collection

Thanks for the feedback on using the \Generator.

@uncaught
Copy link
Author

uncaught commented Apr 4, 2024

Thank you for your reply!

But so, basically, for every collection, using getValue() is potentially risky because I am in no way forced to deal with pagination. Should you not deprecate this then in favor of a safer method?

My solution is a wrapper around the PageIterator for now:

  public static function iterateCollection(GraphServiceClient $client, Promise $promise): \Generator {
    $pageIterator = new PageIterator($promise->wait(), $client->getRequestAdapter());
    while ($pageIterator->hasNext()) {
      $pageItems = [];
      $pageIterator->iterate(function ($item) use (&$pageItems) {
        $pageItems[] = $item;
      });
      foreach ($pageItems as $pageItem) {
        yield $pageItem;
      }
    }
  }

Obviously this could be implemented cleaner inside the PageIterator without using a callback, but I didn't want to create a fork for this ;)

Example call:

/** @var \Iterator<int, Event> $collection */
$collection = GraphFactory::iterateCollection($graph, $graph->me()->calendarView()->get($config));
foreach ($collection as $event) {
  //...
}

@ferizdacic
Copy link

ferizdacic commented Oct 25, 2024

When using iterateCollection posted by @uncaught and on bigger account (+10 users) after a while a am getting an error invalid_request anybody else with same issue? Any solution for it.

It works perfecty with previus version (V1) of library.

Working sample (V1)

`public function getUsers($filter = null)
{
$users = [];
$this->graph->setApiVersion('v1.0');
$userIterator = $this->graph->createCollectionRequest('GET', '/users' . (is_null($filter) ? '' : $filter))
->setReturnType(\Microsoft\Graph\Model\User::class)
->setPageSize(100);

while (!$userIterator->isEnd()) {
    $users = array_merge($users, $userIterator->getPage());
}
return $users;

}`

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs: Attention 👋 question Customer is asking for a clarification, use case or an information. Use it for Issues of type Questi
Projects
None yet
Development

No branches or pull requests

3 participants