From 1bc8637215aef1fed924859519e15718b49ce773 Mon Sep 17 00:00:00 2001 From: Michael Dowling Date: Sat, 20 Sep 2014 17:58:21 -0700 Subject: [PATCH] Cleaning up tests and adding more ring checks --- src/Client.php | 15 ++------------- src/Fsm.php | 2 -- src/RingBridge.php | 38 ++++++++++++++++++++++++++++++-------- tests/ClientTest.php | 28 ++-------------------------- tests/FsmTest.php | 1 + 5 files changed, 35 insertions(+), 49 deletions(-) diff --git a/src/Client.php b/src/Client.php index 070068c3..366a018e 100644 --- a/src/Client.php +++ b/src/Client.php @@ -2,7 +2,6 @@ namespace GuzzleHttp; use GuzzleHttp\Event\HasEmitterTrait; -use GuzzleHttp\Exception\RequestException; use GuzzleHttp\Message\MessageFactory; use GuzzleHttp\Message\MessageFactoryInterface; use GuzzleHttp\Message\RequestInterface; @@ -264,7 +263,7 @@ class Client implements ClientInterface return $trans->response; } - throw $this->getNoRingResponseException($trans->request); + throw RingBridge::getNoRingResponseException($trans->request); } private function createFutureResponse( @@ -285,7 +284,7 @@ class Client implements ClientInterface if ($trans->response) { return $trans->response; } - throw $this->getNoRingResponseException($trans->request); + throw RingBridge::getNoRingResponseException($trans->request); }, // Cancel function. Just proxy to the underlying future. function () use ($response) { @@ -406,16 +405,6 @@ class Client implements ClientInterface return $this->defaults['headers']; } - private function getNoRingResponseException(RequestInterface $request) - { - return new RequestException( - 'Sending the request did not return a response, exception, or ' - . 'populate the transaction with a response. This is most likely ' - . 'due to an incorrectly implemented Guzzle Ring adapter.', - $request - ); - } - /** * @deprecated Use {@see GuzzleHttp\Pool} instead. * @see GuzzleHttp\Pool diff --git a/src/Fsm.php b/src/Fsm.php index 507f4b19..39fbd07e 100644 --- a/src/Fsm.php +++ b/src/Fsm.php @@ -78,8 +78,6 @@ class Fsm if (isset($state['transition'])) { $state['transition']($trans); } - // Did not throw, so remove any exceptions on the transaction. - $trans->exception = null; // Break if the transition told us to bail, or if this is a // terminal state. if (!isset($state['success'])) { diff --git a/src/RingBridge.php b/src/RingBridge.php index 2742dfb1..bc191781 100644 --- a/src/RingBridge.php +++ b/src/RingBridge.php @@ -7,6 +7,7 @@ use GuzzleHttp\Event\ProgressEvent; use GuzzleHttp\Message\Request; use GuzzleHttp\Ring\Core; use GuzzleHttp\Stream\Stream; +use GuzzleHttp\Exception\RequestException; /** * Provides the bridge between Guzzle requests and responses and Guzzle Ring. @@ -125,7 +126,13 @@ class RingBridge MessageFactoryInterface $messageFactory, Fsm $fsm ) { + $trans->state = 'complete'; + $trans->transferInfo = isset($response['transfer_info']) + ? $response['transfer_info'] : []; + if (!empty($response['status'])) { + // Transition to "error" if an error is present. Otherwise, complete. + $trans->state = isset($response['error']) ? 'error' : 'complete'; $options = []; if (isset($response['version'])) { $options['protocol_version'] = $response['version']; @@ -142,17 +149,14 @@ class RingBridge if (isset($response['effective_url'])) { $trans->response->setEffectiveUrl($response['effective_url']); } + } elseif (empty($response['error'])) { + // When nothing was returned, then we need to add an error. + $response['error'] = self::getNoRingResponseException($trans->request); } - $trans->transferInfo = isset($response['transfer_info']) - ? $response['transfer_info'] : []; - - // Determine which state to transition to. - if (!isset($response['error'])) { - $trans->state = 'complete'; - } else { - $trans->exception = $response['error']; + if (isset($response['error'])) { $trans->state = 'error'; + $trans->exception = $response['error']; } // Complete the lifecycle of the request. @@ -186,4 +190,22 @@ class RingBridge $options ); } + + /** + * Get an exception that can be used when a ring adapter does not populate + * a response. + * + * @param RequestInterface $request + * + * @return RequestException + */ + public static function getNoRingResponseException(RequestInterface $request) + { + return new RequestException( + 'Sending the request did not return a response, exception, or ' + . 'populate the transaction with a response. This is most likely ' + . 'due to an incorrectly implemented Guzzle Ring adapter.', + $request + ); + } } diff --git a/tests/ClientTest.php b/tests/ClientTest.php index e7d5af00..a9e2e4d4 100644 --- a/tests/ClientTest.php +++ b/tests/ClientTest.php @@ -344,9 +344,9 @@ class ClientTest extends \PHPUnit_Framework_TestCase */ public function testEnsuresResponseIsPresentAfterDereferencing() { - $adapter = function () { + $adapter = new MockAdapter(function () { return new Future(function () { return []; }); - }; + }); $client = new Client(['adapter' => $adapter]); $client->get('http://httpbin.org')->deref(); } @@ -417,30 +417,6 @@ class ClientTest extends \PHPUnit_Framework_TestCase $this->assertEquals(1, $called); } - public function testCanInterceptFutureOnThenFunction() - { - $client = new Client(); - $request = $client->createRequest('GET', Server::$url, ['future' => true]); - $trans = null; - // Get the transaction. - $request->getEmitter()->on('before', function ($e) use (&$trans) { - $trans = $this->readAttribute($e, 'transaction'); - }); - // Set the response on the transaction, but set no ring response. - $adapter = new MockAdapter(function () use (&$trans) { - // Only do the transaction update when dereferenced. - return new Future(function () use (&$trans) { - $trans->response = new Response(200); - return []; - }); - }); - $client = new Client(['adapter' => $adapter]); - $future = $client->send($request); - $future->deref(); - // Did not get a ring response, but the transaction was updated. - $this->assertEquals(200, $future->getStatusCode()); - } - public function testCanReturnFutureResults() { $called = false; diff --git a/tests/FsmTest.php b/tests/FsmTest.php index 6f34aa9b..01dc2bf8 100644 --- a/tests/FsmTest.php +++ b/tests/FsmTest.php @@ -65,6 +65,7 @@ class FsmTest extends \PHPUnit_Framework_TestCase 'transition' => function (Transaction $trans) use ($t, &$c) { $c[] = 'error'; $this->assertInstanceOf('OutOfBoundsException', $t->exception); + $trans->exception = null; } ], 'end' => [