Skip to content

cleanup phpdoc and BatchResult interface #73

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Oct 23, 2015
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 0 additions & 14 deletions spec/Exception/RequestExceptionSpec.php
Original file line number Diff line number Diff line change
Expand Up @@ -27,18 +27,4 @@ function it_has_a_request(RequestInterface $request)
{
$this->getRequest()->shouldReturn($request);
}

function it_wraps_an_exception(RequestInterface $request)
{
$e = new \Exception('message');

$requestException = $this->wrapException($request, $e);

$requestException->getMessage()->shouldReturn('message');
}

function it_does_not_wrap_if_request_exception(RequestInterface $request, RequestException $requestException)
{
$this->wrapException($request, $requestException)->shouldReturn($requestException);
}
}
10 changes: 7 additions & 3 deletions spec/Exception/TransferExceptionSpec.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,14 @@

namespace spec\Http\Client\Exception;

use Http\Client\Exception\TransferException;
use PhpSpec\ObjectBehavior;
use Prophecy\Argument;

class TransferExceptionSpec extends ObjectBehavior
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

or is there a way to spec these things without instantiating the class?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can add a stub, like in case of BatchRequest trait and test that.

{
function it_is_initializable()
function let()
{
$this->shouldHaveType('Http\Client\Exception\TransferException');
$this->beAnInstanceOf('spec\Http\Client\Exception\TransferExceptionStub');
}

function it_is_a_runtime_exception()
Expand All @@ -22,3 +22,7 @@ function it_is_an_exception()
$this->shouldImplement('Http\Client\Exception');
}
}

class TransferExceptionStub extends TransferException
{
}
57 changes: 23 additions & 34 deletions src/BatchResult.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,13 @@
*/
interface BatchResult
{
/**
* Checks if there are any successful responses at all.
*
* @return boolean
*/
public function hasResponses();

/**
* Returns all successful responses.
*
Expand All @@ -21,31 +28,24 @@ interface BatchResult
public function getResponses();

/**
* Returns the response for a successful request.
* Checks if there is a successful response for a request.
*
* @param RequestInterface $request
*
* @return ResponseInterface
*
* @throws \UnexpectedValueException If request was not part of the batch or failed.
*/
public function getResponseFor(RequestInterface $request);

/**
* Checks if there are any successful responses at all
*
* @return boolean
*/
public function hasResponses();
public function isSuccessful(RequestInterface $request);

/**
* Checks if there is a successful response for a request.
* Returns the response for a successful request.
*
* @param RequestInterface $request
*
* @return ResponseInterface
*
* @throws \UnexpectedValueException If request was not part of the batch or failed.
*/
public function hasResponseFor(RequestInterface $request);
public function getResponseFor(RequestInterface $request);

/**
* Adds a response in an immutable way.
Expand All @@ -58,22 +58,11 @@ public function hasResponseFor(RequestInterface $request);
public function addResponse(RequestInterface $request, ResponseInterface $response);

/**
* Checks if a request was successful.
*
* @param RequestInterface $request
*
* @return boolean
*/
public function isSuccessful(RequestInterface $request);

/**
* Checks if a request has failed.
*
* @param RequestInterface $request
* Checks if there are any unsuccessful requests at all.
*
* @return boolean
*/
public function isFailed(RequestInterface $request);
public function hasExceptions();

/**
* Returns all exceptions for the unsuccessful requests.
Expand All @@ -83,24 +72,24 @@ public function isFailed(RequestInterface $request);
public function getExceptions();

/**
* Returns the exception for a failed request.
* Checks if there is an exception for a request, meaning the request failed.
*
* @param RequestInterface $request
*
* @return Exception
*
* @throws \UnexpectedValueException If request was not part of the batch or was successful.
* @return boolean
*/
public function getExceptionFor(RequestInterface $request);
public function isFailed(RequestInterface $request);

/**
* Checks if there is an exception for a request.
* Returns the exception for a failed request.
*
* @param RequestInterface $request
*
* @return boolean
* @return Exception
*
* @throws \UnexpectedValueException If request was not part of the batch or was successful.
*/
public function hasExceptionFor(RequestInterface $request);
public function getExceptionFor(RequestInterface $request);

/**
* Adds an exception in an immutable way.
Expand Down
2 changes: 1 addition & 1 deletion src/Exception.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
namespace Http\Client;

/**
* Every HTTP Client related Exception should implement this interface
* Every HTTP Client related Exception must implement this interface
*
* @author Márk Sági-Kazár <[email protected]>
*/
Expand Down
6 changes: 3 additions & 3 deletions src/Exception/BatchException.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,13 @@
use Http\Client\Exception;

/**
* This exception is thrown when a batch of requests led to at least one failure.
* This exception is thrown when HttpClient::sendRequests led to at least one failure.
*
* It holds the response/exception pairs and gives access to a BatchResult with the successful responses.
* It gives access to a BatchResult with the request-exception and request-response pairs.
*
* @author Márk Sági-Kazár <[email protected]>
*/
final class BatchException extends \RuntimeException implements Exception
final class BatchException extends TransferException implements Exception
{
/**
* @var BatchResult
Expand Down
7 changes: 3 additions & 4 deletions src/Exception/HttpException.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
/**
* Thrown when a response was received but has an error status code.
*
* This exception always provides the request and response objects.
* In addition to the request, this exception always provides access to the response object.
*
* @author Márk Sági-Kazár <[email protected]>
*/
Expand All @@ -31,11 +31,10 @@ public function __construct(
ResponseInterface $response,
\Exception $previous = null
) {
parent::__construct($message, $request, $previous);

$this->response = $response;
$this->code = $response->getStatusCode();


parent::__construct($message, $request, $previous);
}

/**
Expand Down
4 changes: 3 additions & 1 deletion src/Exception/NetworkException.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@
namespace Http\Client\Exception;

/**
* Thrown when the request cannot be completed caused by network issues
* Thrown when the request cannot be completed because of network issues.
*
* There is no response object as this exception is thrown when no response has been received.
*
* @author Márk Sági-Kazár <[email protected]>
*/
Expand Down
20 changes: 4 additions & 16 deletions src/Exception/RequestException.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,10 @@
use Psr\Http\Message\RequestInterface;

/**
* Base exception for when a request failed.
* Exception for when a request failed, providing access to the failed request.
*
* This could be due to an invalid request, or one of the extending exceptions
* for network errors or HTTP error responses.
*
* @author Márk Sági-Kazár <[email protected]>
*/
Expand Down Expand Up @@ -37,19 +40,4 @@ public function getRequest()
{
return $this->request;
}

/**
* @param RequestInterface $request
* @param \Exception $e
*
* @return RequestException
*/
public static function wrapException(RequestInterface $request, \Exception $e)
{
if (!$e instanceof RequestException) {
$e = new RequestException($e->getMessage(), $request, $e);
}

return $e;
}
}
2 changes: 1 addition & 1 deletion src/Exception/TransferException.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,6 @@
*
* @author Márk Sági-Kazár <[email protected]>
*/
class TransferException extends \RuntimeException implements Exception
abstract class TransferException extends \RuntimeException implements Exception
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think we never need to directly instantiate this exception. or do we?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't think so.

{
}
15 changes: 9 additions & 6 deletions src/HttpClient.php
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
interface HttpClient
{
/**
* Sends a PSR request
* Sends a PSR-7 request.
*
* @param RequestInterface $request
*
Expand All @@ -27,17 +27,20 @@ interface HttpClient
public function sendRequest(RequestInterface $request);

/**
* Sends PSR requests
* Sends several PSR-7 requests.
*
* If one or more requests led to an exception, the BatchException is thrown.
* The BatchException also gives access to the BatchResult for the successful responses.
* If the client is able to, these requests should be sent in parallel. Otherwise they will be sent sequentially.
* Either way, the caller may not rely on them being executed in any particular order.
*
* If one or more requests led to an exception, the BatchException is thrown. The BatchException gives access to the
* BatchResult that contains responses for successful calls and exceptions for unsuccessful calls.
*
* @param RequestInterface[] $requests
*
* @return BatchResult If all requests where successful.
*
* @throws Exception
* @throws BatchException
* @throws Exception On general setup problems.
* @throws BatchException If one or more requests led to exceptions.
*/
public function sendRequests(array $requests);
}