Merge pull request #448 from ycombinator/fix-pagination-for-queues

Fix iteration of queue messages

Author: Jamie Hannaford

CONTRIBUTING.md

@@ -27,7 +27,7 @@ Test Instructions
 
 ### To run unit tests:
 ```bash
-phpunit
+vendor/bin/phpunit
 ```
 
 ### To run the full suite of acceptance tests:

composer.json

@@ -26,6 +26,7 @@
         "psr/log": "~1.0"
     },
     "require-dev" : {
+        "phpunit/phpunit": "4.3.*",
         "guzzle/guzzle": "~3.8",
         "satooshi/php-coveralls": "0.6.*@dev",
         "jakub-onderka/php-parallel-lint": "0.*",

lib/OpenCloud/Queues/Collection/MessageIterator.php

@@ -0,0 +1,29 @@
+<?php
+/**
+ * Copyright 2012-2014 Rackspace US, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+namespace OpenCloud\Queues\Collection;
+
+use OpenCloud\Common\Collection\PaginatedIterator;
+
+class MessageIterator extends PaginatedIterator
+{
+    protected function shouldAppend()
+    {
+        return $this->nextUrl ||
+            ($this->position % $this->getOption('limit.page') == 0);
+    }
+}

lib/OpenCloud/Queues/Resource/Queue.php

@@ -22,6 +22,7 @@
 use OpenCloud\Common\Http\Message\Formatter;
 use OpenCloud\Common\Resource\PersistentResource;
 use OpenCloud\Queues\Exception;
+use OpenCloud\Queues\Collection\MessageIterator;
 use OpenCloud\Common\Metadata;
 
 /**
@@ -250,7 +251,7 @@ public function createMessages(array $messages)
      *      messages as well as unclaimed messages. If not specified, defaults
      *      to FALSE (i.e. only unclaimed messages are returned).
      *
-     * @return Collection
+     * @return \OpenCloud\Queues\Collection\MessageIterator
      */
     public function listMessages(array $options = array())
     {
@@ -276,7 +277,7 @@ public function listMessages(array $options = array())
                 'limit.page' => 10
             );
 
-        return PaginatedIterator::factory($this, $options);
+        return MessageIterator::factory($this, $options);
     }
 
     /**

tests/OpenCloud/Smoke/Unit/AbstractUnit.php

@@ -28,7 +28,7 @@
  *
  * @link
  */
-abstract class AbstractUnit
+abstract class AbstractUnit extends \PHPUnit_Framework_TestCase
 {
     /**
      * The credentials cache filename.

tests/OpenCloud/Smoke/Unit/Queues.php

@@ -17,6 +17,8 @@
 
 namespace OpenCloud\Smoke\Unit;
 
+use OpenCloud\Queues\Resource\Queue;
+
 class Queues extends AbstractUnit implements UnitInterface
 {
     const QUEUE_NAME = 'test_queue';
@@ -71,33 +73,45 @@ public function doMessageBlock()
         $this->step('Messages');
 
         // post
+        $numMessagesCreated = 0;
         $this->stepInfo('Create messages for queue %s', $this->queue->getName());
         $this->queue->createMessage(array(
             'body' => (object) array(
                     'instructions' => 'Do it now!'
                 ),
             'ttl' => 300
         ));
+        ++$numMessagesCreated;
+
+        for ($creationBatch = 0; $creationBatch < 3; ++$creationBatch) {
+            $messages = array();
+            for ($messageIndex = 0; $messageIndex < Queue::MAX_POST_MESSAGES; ++$messageIndex) {
+                $messages[] = array(
+                    'body' => (object) array('message_number' => ($creationBatch * Queue::MAX_POST_MESSAGES) + $messageIndex + 1),
+                    'ttl'  => mt_rand(300, 600)
+                );
+                ++$numMessagesCreated;
+            }
+            $this->queue->createMessages($messages);
+        }
 
-        $this->queue->createMessages(array(
-            array(
-                'body' => (object) array('foo' => 'bar'),
-                'ttl'  => 700
-            ),
-            array(
-                'body' => (object) array('baz' => 'lol'),
-                'ttl'  => 600
-            )
-        ));
-
-        // list
+        // list ( 'echo' => true is needed to list client's own messages)
         $step = $this->stepInfo('List messages for queue %s', $this->queue->getName());
-        $messages = $this->queue->listMessages();
+        $messages = $this->queue->listMessages(array(
+            'echo' => true
+        ));
         $ids = array();
+        $numMessagesListed = 0;
+        $step->stepInfo("%-30s | %-40s", "Message ID", "Message body");
+        $step->stepInfo("%-30s | %-40s", str_repeat("-", 30), str_repeat("-", 40));
         foreach ($messages as $message) {
-            $step->stepInfo($message->getId());
+            $step->stepInfo("%-30s | %-40s", $message->getId(), json_encode($message->getBody()));
             $ids[] = $message->getId();
+            ++$numMessagesListed;
         }
+        $this->stepInfo("Number of messages to be listed:    " . $numMessagesCreated);
+        $this->stepInfo("Number of messages actually listed: " . $numMessagesListed);
+        $this->assertEquals($numMessagesCreated, $numMessagesListed);
 
         array_pop($ids);
     }

tests/OpenCloud/Tests/Queues/Resource/QueueTest.php

@@ -122,7 +122,7 @@ public function test_Get_Message()
     public function test_List_Message()
     {
         $this->assertInstanceOf(
-            self::COLLECTION_CLASS,
+            'OpenCloud\Queues\Collection\MessageIterator',
             $this->queue->setName('foo')->listMessages(array(
                 'ids' => array(100, 901, 58)
             ))