Skip to content

Commit

Permalink
Implement an ErrorTransport (#1529) (#1592)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
jandom authored and ruflin committed Jan 21, 2019
1 parent 6d7c20e commit f012d97
Show file tree
Hide file tree
Showing 4 changed files with 99 additions and 6 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,11 @@ This project adheres to [Semantic Versioning](http://semver.org/).

### 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

## [5.3.4](https://github.com/ruflin/Elastica/compare/5.3.3...5.3.4)
Expand Down
4 changes: 3 additions & 1 deletion lib/Elastica/Transport/Guzzle.php
Original file line number Diff line number Diff line change
Expand Up @@ -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'));
Expand All @@ -93,6 +94,7 @@ public function exec(Request $request, array $params)
[
'request_header' => $request->getMethod(),
'http_code' => $res->getStatusCode(),
'body' => $responseBody,
]
);

Expand Down
60 changes: 55 additions & 5 deletions lib/Elastica/Transport/NullTransport.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,18 +12,50 @@
* host but still returns a valid response object
*
* @author James Boehmer <james.boehmer@jamesboehmer.com>
* @author Jan Domanski <jandom@gmail.com>
*/
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,
Expand All @@ -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;
}
}
36 changes: 36 additions & 0 deletions test/Elastica/Transport/NullTransportTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,20 @@
* Elastica Null Transport Test.
*
* @author James Boehmer <james.boehmer@jamesboehmer.com>
* @author Jan Domanski <jandom@gmail.com>
*/
class NullTransportTest extends BaseTest
{

/** @var NullTransport NullTransport */
protected $transport;

public function setUp()
{
parent::setUp();
$this->transport = new NullTransport();
}

/**
* @group functional
*/
Expand Down Expand Up @@ -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']);
}
}

0 comments on commit f012d97

Please sign in to comment.