Skip to content

Add BatchResult #48

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 10 commits into from
Sep 22, 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
44 changes: 44 additions & 0 deletions spec/BatchResultSpec.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
<?php

namespace spec\Http\Client;

use Psr\Http\Message\RequestInterface;
use Psr\Http\Message\ResponseInterface;
use PhpSpec\ObjectBehavior;

class BatchResultSpec extends ObjectBehavior
{
function it_is_initializable()
{
$this->shouldHaveType('Http\Client\BatchResult');
}

function it_is_immutable(RequestInterface $request, ResponseInterface $response)
{
$new = $this->addResponse($request, $response);

$this->getResponses()->shouldReturn([]);
$new->shouldHaveType('Http\Client\BatchResult');
$new->getResponses()->shouldReturn([$response]);
}

function it_has_a_responses(RequestInterface $request, ResponseInterface $response)
{
$new = $this->addResponse($request, $response);

$this->hasResponses()->shouldReturn(false);
$this->getResponses()->shouldReturn([]);
$new->hasResponses()->shouldReturn(true);
$new->getResponses()->shouldReturn([$response]);
}

function it_has_a_response_for_a_request(RequestInterface $request, ResponseInterface $response)
{
$new = $this->addResponse($request, $response);

$this->shouldThrow('Http\Client\Exception\UnexpectedValueException')->duringGetResponseFor($request);
$this->hasResponseFor($request)->shouldReturn(false);
$new->getResponseFor($request)->shouldReturn($response);
$new->hasResponseFor($request)->shouldReturn(true);
}
}
60 changes: 42 additions & 18 deletions spec/Exception/BatchExceptionSpec.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,40 +2,64 @@

namespace spec\Http\Client\Exception;

use Http\Client\Exception\TransferException;
use Psr\Http\Message\ResponseInterface;
use Http\Client\BatchResult;
use Http\Client\Exception;
use Psr\Http\Message\RequestInterface;
use PhpSpec\ObjectBehavior;

class BatchExceptionSpec extends ObjectBehavior
{
function let(TransferException $e, ResponseInterface $response)
function it_is_initializable()
{
$this->beConstructedWith([$e], [$response]);
$this->shouldHaveType('Http\Client\Exception\BatchException');
}

function it_is_initializable()
function it_is_an_exception()
{
$this->shouldHaveType('Http\Client\Exception\BatchException');
$this->shouldImplement('Http\Client\Exception');
$this->shouldHaveType('Exception');
}

function it_has_a_result()
{
$this->setResult($result = new BatchResult());
$this->getResult()->shouldReturn($result);
}

function it_throws_an_exception_if_the_result_is_already_set()
{
$this->getResult()->shouldHaveType('Http\Client\BatchResult');
$this->shouldThrow('Http\Client\Exception\InvalidArgumentException')->duringSetResult(new BatchResult());
}

function it_is_a_transfer_exception()
function it_has_an_exception_for_a_request(RequestInterface $request, Exception $exception)
{
$this->shouldHaveType('Http\Client\Exception\TransferException');
$this->shouldThrow('Http\Client\Exception\UnexpectedValueException')->duringGetExceptionFor($request);
$this->hasExceptionFor($request)->shouldReturn(false);

$this->addException($request, $exception);

$this->getExceptionFor($request)->shouldReturn($exception);
$this->hasExceptionFor($request)->shouldReturn(true);
}

function it_has_exceptions(TransferException $e, TransferException $e2)
function it_has_exceptions(RequestInterface $request, Exception $exception)
{
$this->getExceptions()->shouldReturn([$e]);
$this->hasException($e)->shouldReturn(true);
$this->hasException($e2)->shouldReturn(false);
$this->hasExceptions()->shouldReturn(true);
$this->getExceptions()->shouldReturn([]);

$this->addException($request, $exception);

$this->getExceptions()->shouldReturn([$exception]);
}

function it_has_responses(ResponseInterface $response, ResponseInterface $response2)
function it_checks_if_a_request_failed(RequestInterface $request, Exception $exception)
{
$this->getResponses()->shouldReturn([$response]);
$this->hasResponse($response)->shouldReturn(true);
$this->hasResponse($response2)->shouldReturn(false);
$this->hasResponses()->shouldReturn(true);
$this->isSuccessful($request)->shouldReturn(false);
$this->isFailed($request)->shouldReturn(false);

$this->addException($request, $exception);

$this->isSuccessful($request)->shouldReturn(false);
$this->isFailed($request)->shouldReturn(true);
}
}
23 changes: 23 additions & 0 deletions spec/Exception/UnexpectedValueExceptionSpec.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
<?php

namespace spec\Http\Client\Exception;

use PhpSpec\ObjectBehavior;

class UnexpectedValueExceptionSpec extends ObjectBehavior
{
function it_is_initializable()
{
$this->shouldHaveType('Http\Client\Exception\UnexpectedValueException');
}

function it_is_invalid_argument_exception()
{
$this->shouldHaveType('UnexpectedValueException');
}

function it_is_exception()
{
$this->shouldImplement('Http\Client\Exception');
}
}
104 changes: 104 additions & 0 deletions src/BatchResult.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
<?php

namespace Http\Client;

use Http\Client\Exception\UnexpectedValueException;
use Psr\Http\Message\RequestInterface;
use Psr\Http\Message\ResponseInterface;

/**
* Successful responses returned from parallel request execution
*
* @author Márk Sági-Kazár <[email protected]>
*/
final class BatchResult
{
/**
* @var \SplObjectStorage
*/
private $responses;

public function __construct()
{
$this->responses = new \SplObjectStorage();
}

/**
* Returns all successful responses
*
* @return ResponseInterface[]
*/
public function getResponses()
{
$responses = [];

foreach ($this->responses as $request) {
$responses[] = $this->responses[$request];
}

return $responses;
}

/**
* Returns a response of a request
*
* @param RequestInterface $request
*
* @return ResponseInterface
*
* @throws UnexpectedValueException
*/
public function getResponseFor(RequestInterface $request)
{
try {
return $this->responses[$request];
} catch (\UnexpectedValueException $e) {
throw new UnexpectedValueException('Request not found', $e->getCode(), $e);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

otherwise, we should look in the exceptions to see if there is an exception carrying a response and return that. if that is not found, i would throw the ecxeption we find at $request. or an invalid argument exception if the request does not exist.

Copy link
Member Author

Choose a reason for hiding this comment

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

Currently responses are handled the following way: every response and exception is collected in the BatchResult. If there are any exceptions, The BatchException gets the result and it is thrown.

There is a getExceptionFor method to get the exception. If that exception is HttpException, you can get a response. I would not mix the two methods.

I also wouldn't throw the exception, because that way you cannot process the rest.

The reason why I put exceptions here as well is that an exception cannot be immutable. But it would possibly be better, to collect exceptions somewhere else (eg. right in the batch exception?)

Copy link
Member Author

Choose a reason for hiding this comment

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

I have an ugly idea for that. 😜

In batch exception accept two objects: batch result, exception chain

The exception chain would be an object which accepts a request, an exception and the previous/next exception. THis way you have one object in the end to be injected into the batch exception. You can always get the next exception by calling getNextException, which returns the next in the chain.

Copy link
Contributor

Choose a reason for hiding this comment

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

hm. when i think of responses, i might want to process all responses,
even unsuccessful ones. but it indeed could get very confusing. so lets
adjust phpdoc to always say "successful responses" and throw an
exception if i try to request a response that does not exist (maybe with
a hint that the request failed, telling the user he should check with
the isSuccessful / isFailed method.

Copy link
Contributor

Choose a reason for hiding this comment

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

throw the exception we find at $request

I dunno about throwing it...

Copy link
Member Author

Choose a reason for hiding this comment

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

This is what I tried first. The problem is that an Exception cannot be cloned, so making it immutable that way doesn't work.

Copy link
Contributor

@dbu dbu Sep 22, 2015 via email

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

Should we do this in case of the BatchResult as well? I think it would be weird if the Result is immutable, the exception is not.

Copy link
Contributor

@dbu dbu Sep 22, 2015 via email

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

}

/**
* Checks if there are any successful responses at all
*
* @return boolean
*/
public function hasResponses()
{
return $this->responses->count() > 0;
}

/**
* Checks if there is a response of a request
*
* @param RequestInterface $request
*
* @return ResponseInterface
*/
public function hasResponseFor(RequestInterface $request)
{
return $this->responses->contains($request);
}

/**
* Adds a response in an immutable way
*
* @param RequestInterface $request
* @param ResponseInterface $response
*
* @return BatchResult
*
* @internal
*/
public function addResponse(RequestInterface $request, ResponseInterface $response)
{
$new = clone $this;
$new->responses->attach($request, $response);

return $new;
}

public function __clone()
{
$this->responses = clone $this->responses;
}
}
Loading