[Http] Better handling of nested scope requests in CurlMulti

Requests are now prepared in the send() method rather than the add()
method when adding a request during a transfer.  The send() method now
only prepares requests in the current scope in which the send method was
called.  This allows for better handling of commands that require a
request in order to prepare themselves for sending (e.g. a request that
requires a token that requires an HTTP request).  The BatchQueuePlugin
and CommandSet no longer add requests using async as that was a hack to
support the previous implementation.

src/Guzzle/Http/Curl/CurlMulti.php

@@ -147,11 +147,13 @@ class CurlMulti extends AbstractHasDispatcher implements CurlMultiInterface
     /**
      * {@inheritdoc}
      *
-     * Adds a request to the next scope (or batch or requests to be sent).  If
-     * a request is added using async, then the request is added to the current
-     * scope.  This means that the request will be sent and polled if requests
-     * are currently being sent, or that the request will be sent in the next
-     * send operation.
+     * Adds a request to a batch of requests to be sent in parallel.
+     *
+     * Async requests adds a request to the current scope to be executed in
+     * parallel with any currently executing cURL handles.  You may only add an
+     * async request while other requests are transferring.  Attempting to add
+     * an async request while no requests are transferring will add the request
+     * normally in the next available scope (typically 0).
      *
      * @param RequestInterface $request Request to add
      * @param bool             $async   Set to TRUE to add to the current scope
@@ -160,8 +162,13 @@ class CurlMulti extends AbstractHasDispatcher implements CurlMultiInterface
      */
     public function add(RequestInterface $request, $async = false)
     {
+        if ($async && $this->state != self::STATE_SENDING) {
+            $async = false;
+        }
+
         $this->requestCache = null;
         $scope = $async ? $this->scope : $this->scope + 1;
+
         if (!isset($this->requests[$scope])) {
             $this->requests[$scope] = array();
         }
@@ -170,7 +177,9 @@ class CurlMulti extends AbstractHasDispatcher implements CurlMultiInterface
             'request' => $request
         ));
 
-        if ($this->state == self::STATE_SENDING) {
+        // If requests are currently transferring and this is async, then the
+        // request must be prepared now as the send() method is not called.
+        if ($this->state == self::STATE_SENDING && $async) {
             $this->beforeSend($request);
         }
 
@@ -262,27 +271,29 @@ class CurlMulti extends AbstractHasDispatcher implements CurlMultiInterface
     public function send()
     {
         $this->scope++;
+        $this->state = self::STATE_SENDING;
+
+        // Only prepare and send requests that are in the current recursion scope
+        // Only enter the main perform() loop if there are requests in scope
+        if (!empty($this->requests[$this->scope])) {
 
-        // Don't prepare for sending again if send() is called while sending
-        if ($this->state != self::STATE_SENDING) {
-            $requests = $this->all();
             // Any exceptions thrown from this event should break the entire
             // flow of sending requests in parallel to prevent weird errors
             $this->dispatch(self::BEFORE_SEND, array(
-                'requests' => $requests
+                'requests' => $this->requests[$this->scope]
             ));
-            $this->state = self::STATE_SENDING;
-            foreach ($requests as $request) {
+
+            foreach ($this->requests[$this->scope] as $request) {
                 if ($request->getState() != RequestInterface::STATE_TRANSFER) {
                     $this->beforeSend($request);
                 }
             }
-        }
 
-        try {
-            $this->perform();
-        } catch (\Exception $e) {
-            $this->exceptions[] = $e;
+            try {
+                $this->perform();
+            } catch (\Exception $e) {
+                $this->exceptions[] = $e;
+            }
         }
 
         $this->scope--;
@@ -391,8 +402,6 @@ class CurlMulti extends AbstractHasDispatcher implements CurlMultiInterface
 
             $active = $this->executeHandles();
 
-            $curlErrors = false;
-
             // Get messages from curl handles
             while ($done = curl_multi_info_read($this->multiHandle)) {
                 foreach ($this->all() as $request) {
@@ -402,20 +411,12 @@ class CurlMulti extends AbstractHasDispatcher implements CurlMultiInterface
                             $this->processResponse($request, $handle, $done);
                         } catch (\Exception $e) {
                             $this->removeErroredRequest($request, $e);
-                            $curlErrors = true;
                         }
                         break;
                     }
                 }
             }
 
-            // We need to check if every request has been fulfilled or has
-            // encountered an error when any curl errors are encountered to
-            // avoind an endless loop.
-            if ($curlErrors && empty($this->requestCache)) {
-                break;
-            }
-
             // Notify each request as polling and handled queued responses
             if ($this->scope <= 0) {
                 $scopedPolling = $this->all();

src/Guzzle/Http/Plugin/BatchQueuePlugin.php

@@ -115,7 +115,7 @@ class BatchQueuePlugin implements EventSubscriberInterface, \Countable
         // Prepare each request for their respective curl multi objects
         while ($request = array_shift($this->queue)) {
             $multi = $request->getClient()->getCurlMulti();
-            $multi->add($request, true);
+            $multi->add($request);
             if (!in_array($multi, $multis)) {
                 $multis[] = $multi;
             }

src/Guzzle/Service/Command/CommandSet.php

@@ -90,7 +90,7 @@ class CommandSet implements \IteratorAggregate, \Countable
                 $command->getClient()->dispatch('command.before_send', array(
                     'command' => $command
                 ));
-                $command->getClient()->getCurlMulti()->add($command->getRequest(), true);
+                $command->getClient()->getCurlMulti()->add($command->getRequest());
                 if (!in_array($command->getClient()->getCurlMulti(), $multis)) {
                     $multis[] = $command->getClient()->getCurlMulti();
                 }

tests/Guzzle/Tests/Http/Curl/CurlMultiTest.php

@@ -560,4 +560,18 @@ class CurlMultiTest extends \Guzzle\Tests\GuzzleTestCase
             $this->assertEquals('Unexpected cURL error: 255', $e->getMessage());
         }
     }
+
+    /**
+     * @covers Guzzle\Http\Curl\curlMulti::add
+     */
+    public function testAddsAsyncRequestsNormallyWhenNotSending()
+    {
+        $multi = new CurlMulti();
+        $request = new Request('GET', 'http://www.google.com/');
+        $multi->add($request, true);
+
+        // Ensure that the request was added at the correct next scope
+        $requests = $this->readAttribute($multi, 'requests');
+        $this->assertEquals(array($request), $requests[0]);
+    }
 }