From 38ca2ce18265e0034f8d5cc7e20e0a3fc18f84e8 Mon Sep 17 00:00:00 2001 From: Jan Domanski Date: Sun, 13 Jan 2019 13:06:10 +0000 Subject: [PATCH 1/3] Implement an ErrorTransport (#1529) When using AwsAuthV4 transport, it is possible that the user provides incorrect credentials. This means that the Guzzle client will get a 403 from AWS. By default Guzzle silently ignores all exceptions and stores them in `$response->setTransferInfo()` This feature allows a downstream client/library to handle them however. Currently, FOSElastica does nothing to process 403 or other errors that come back from ruflin/Elastica (transferInfo is never used). This causes a problem because 403 are silently ignored and the progress bar appears to be moving unhindered ;-) This PR is a pre-requisite for unit tests that will accompany this fix on FOSElastica # Conflicts: # test/Elastica/Transport/NullTransportTest.php --- CHANGELOG.md | 3 + lib/Elastica/Transport/Guzzle.php | 4 +- lib/Elastica/Transport/NullTransport.php | 60 +++++++++++++++++-- test/Elastica/Transport/NullTransportTest.php | 36 +++++++++++ 4 files changed, 97 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c2f4f279e8..2496ff6e2e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,9 @@ All notable changes to this project will be documented in this file based on the ### Added +* Added a transport class for mocking a HTTP 403 error codes, useful for testing response failures in inheriting clients +* [Field](https://www.elastic.co/guide/en/elasticsearch/reference/current/query-dsl-function-score-query.html#function-random) param for `Elastica\Query\FunctionScore::addRandomScoreFunction` + ### Improvements - [Backported] Reduced memory footprint of response by not keeping the raw JSON data when JSON after JSON has been parsed. [#1588](https://github.com/ruflin/Elastica/pull/1588) diff --git a/lib/Elastica/Transport/Guzzle.php b/lib/Elastica/Transport/Guzzle.php index ae22d7115e..bee603e25a 100644 --- a/lib/Elastica/Transport/Guzzle.php +++ b/lib/Elastica/Transport/Guzzle.php @@ -83,7 +83,8 @@ public function exec(Request $request, array $params) throw new GuzzleException($ex, $request, new Response($ex->getMessage())); } - $response = new Response((string) $res->getBody(), $res->getStatusCode()); + $responseBody = (string) $res->getBody(); + $response = new Response($responseBody, $res->getStatusCode()); $response->setQueryTime($end - $start); if ($connection->hasConfig('bigintConversion')) { $response->setJsonBigintConversion($connection->getConfig('bigintConversion')); @@ -93,6 +94,7 @@ public function exec(Request $request, array $params) [ 'request_header' => $request->getMethod(), 'http_code' => $res->getStatusCode(), + 'body' => $responseBody, ] ); diff --git a/lib/Elastica/Transport/NullTransport.php b/lib/Elastica/Transport/NullTransport.php index d94be7ed5d..932b5b505f 100644 --- a/lib/Elastica/Transport/NullTransport.php +++ b/lib/Elastica/Transport/NullTransport.php @@ -12,18 +12,50 @@ * host but still returns a valid response object * * @author James Boehmer + * @author Jan Domanski */ class NullTransport extends AbstractTransport { /** - * Null transport. + * Response you want to get from the transport + * + * @var Response Response + */ + protected $_response = null; + + /** + * Set response object the transport returns + * + * @param \Elastica\Response $response + * + * @return $this + */ + public function getResponse() + { + return $this->_response; + } + + /** + * Set response object the transport returns + * + * @param \Elastica\Response $response + * + * @return $this + */ + public function setResponse(Response $response) + { + $this->_response = $response; + return $this->_response; + } + + /** + * Generate an example response object * - * @param \Elastica\Request $request * @param array $params Hostname, port, path, ... * - * @return \Elastica\Response Response empty object + * @return \Elastica\Response $response */ - public function exec(Request $request, array $params) + public function generateDefaultResponse(array $params) { $response = [ 'took' => 0, @@ -40,7 +72,25 @@ public function exec(Request $request, array $params) ], 'params' => $params, ]; - return new Response(JSON::stringify($response)); } + + /** + * Null transport. + * + * @param \Elastica\Request $request + * @param array $params Hostname, port, path, ... + * + * @return \Elastica\Response Response empty object + */ + public function exec(Request $request, array $params) + { + $response = $this->getResponse(); + + if (!$response) { + $response = $this->generateDefaultResponse($params); + } + + return $response; + } } diff --git a/test/Elastica/Transport/NullTransportTest.php b/test/Elastica/Transport/NullTransportTest.php index a62e55bd19..0a212c38ef 100644 --- a/test/Elastica/Transport/NullTransportTest.php +++ b/test/Elastica/Transport/NullTransportTest.php @@ -12,9 +12,20 @@ * Elastica Null Transport Test. * * @author James Boehmer + * @author Jan Domanski */ class NullTransportTest extends BaseTest { + + /** @var NullTransport NullTransport */ + protected $transport; + + public function setUp() + { + parent::setUp(); + $this->transport = new NullTransport(); + } + /** * @group functional */ @@ -96,4 +107,29 @@ public function testOldObject() $data = $response->getData(); $this->assertEquals($params, $data['params']); } + + /** + * @group unit + */ + public function testResponse() + { + $resposeString = ''; + $response = new Response($resposeString); + $this->transport->setResponse($response); + $this->assertEquals($response, $this->transport->getResponse()); + } + + /** + * @group unit + */ + public function testGenerateDefaultResponse() + { + $params = [ 'blah' => 123 ]; + $response = $this->transport->generateDefaultResponse($params); + $this->assertEquals([], $response->getTransferInfo()); + + $responseData = $response->getData(); + $this->assertContains('params', $responseData); + $this->assertEquals($params, $responseData['params']); + } } From 45b69aef0658ddeb62e2bc742e0dc4247922ed0f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Doma=C5=84ski?= Date: Mon, 14 Jan 2019 23:01:00 +0000 Subject: [PATCH 2/3] Update CHANGELOG.md --- CHANGELOG.md | 1 - 1 file changed, 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2496ff6e2e..ab422ef6b9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,7 +11,6 @@ All notable changes to this project will be documented in this file based on the ### Added * Added a transport class for mocking a HTTP 403 error codes, useful for testing response failures in inheriting clients -* [Field](https://www.elastic.co/guide/en/elasticsearch/reference/current/query-dsl-function-score-query.html#function-random) param for `Elastica\Query\FunctionScore::addRandomScoreFunction` ### Improvements From 7aa541e09a7c93d4dbb8bf5e5a4d00141883cfc7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Doma=C5=84ski?= Date: Mon, 14 Jan 2019 23:03:15 +0000 Subject: [PATCH 3/3] Update CHANGELOG.md --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index ab422ef6b9..c2543875ab 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,9 @@ All notable changes to this project will be documented in this file based on the ### Added * Added a transport class for mocking a HTTP 403 error codes, useful for testing response failures in inheriting clients +[#1529](https://github.com/ruflin/Elastica/pull/1529) +* [Backported] Added a transport class for mocking a HTTP 403 error codes, useful for testing response failures in inheriting clients +[#1592](https://github.com/ruflin/Elastica/pull/1592) ### Improvements