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

Support automatic forward-paging of entity collections #3

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

deviantintegral
Copy link
Contributor

@deviantintegral deviantintegral commented Jan 26, 2021

As a developer using the drupal.org API, it's common to have to load multiple pages of results. This PR adds a new class that transparently loads new pages as you iterate over a collection.

Highlights

Catching errors when decoding

json_decode() relies on manually checking error codes via json_last_error(). Guzzle used have a function to do this, but it was removed in Guzzle 6. 28929cf pulls in symfony/serializer so we can use it's JSON decoder. I know, it would be nice if it was it's own package, but I think the benefits of familiarity to Drupal developers outweighs the (small) cost of additional code in vendor.

A trait for rel link handling

44e6685 lets us share methods between the existing collection class and the new one.

A new interface for collections

e938b8c adds a new interface to the existing collection class. Technically there's a small BC break here because I added return typehinting to the \Iterator methods. I personally don't think it's worth rolling a new major release just for this given that code is likely broken if it breaks this contract anyways. If there's disagreement there, I'd be open to removing it or doing a little more refactoring that does break BC more directly.

Remaining Tasks

  • Add a new client method to support using the paging collection 10b59d8
  • Deprecate \Hussainweb\DrupalApi\Request\Request::getEntityClass() and move the responsibility to the request object. As is, it's brittle to extend this since it relies on specific class naming and FQCNs. I decided against this - I think being explicit about class mappings is more robust, but it isn't strictly required for this feature.

@deviantintegral deviantintegral changed the title Support automatic forward-paging of entity collections [WIP] Support automatic forward-paging of entity collections Jan 26, 2021
@deviantintegral deviantintegral changed the title [WIP] Support automatic forward-paging of entity collections Support automatic forward-paging of entity collections Jan 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant