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

Fix iteration of queue messages #448

Merged

Conversation

ycombinator
Copy link
Contributor

The motivation for this PR is https://community.rackspace.com/developers/f/7/p/4494/11318#11318.

This PR introduces a custom iterator (OpenCloud\Queues\Collection\MessageIterator) for iterating over the results of $queue->listMessages(). This is necessary because the underlying HTTP API's response, unlike other paginated list API responses, does not expose the marker property in each item (message) in the list of items (messages) returned. The common PaginatedIterator class relies on this API idiom and, therefore, cannot be used as-is for this particular API.

@ycombinator ycombinator changed the title Do not consider currentMarker [WIP] Do not consider currentMarker Nov 7, 2014
@jamiehannaford
Copy link
Contributor

We can probably apply these changes to a new QueuesIterator class that would live in the OpenCloud\Queues\Collection namespace. Your new iterator would extend the base PaginatedIterator and override the shouldAppend method, in a similar way to how DnsIterator does it.

This will isolate the changes to just this service and mitigate the risk of other things breaking. If this only applies to a single operation (like ListMessages), you could restrict the scope of the new class to this operation only - and have all the other Queues collection operations go through the base class.

@ycombinator ycombinator changed the title [WIP] Do not consider currentMarker Fix iteration of queue messages Nov 11, 2014
@ycombinator
Copy link
Contributor Author

@jamiehannaford I've taken your advice and created a custom iterator. This PR is ready for review now. Thank you!

@ycombinator
Copy link
Contributor Author

What do you think about making them even more strict by throwing an exception if $numMessagesCreated != $numMessagesListed?

Yes, of course. I'll see if I can pull in phpunit's assertion framework and use it for this.

jamiehannaford pushed a commit that referenced this pull request Nov 11, 2014
@jamiehannaford jamiehannaford merged commit 96a52b7 into rackspace:working Nov 11, 2014
@ycombinator ycombinator deleted the fix-pagination-for-queues branch November 11, 2014 13:21
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.

2 participants