Skip to content

Commit

Permalink
Merge pull request #148 from laminas/fix/response-code-validation-har…
Browse files Browse the repository at this point in the history
…dening

Hardening scenarios around failed Github API and GraphQL calls
  • Loading branch information
Ocramius authored Jun 4, 2021
2 parents 07da329 + 5868741 commit cf278ac
Show file tree
Hide file tree
Showing 9 changed files with 397 additions and 7 deletions.
14 changes: 11 additions & 3 deletions src/Github/Api/GraphQL/RunGraphQLQuery.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,14 @@

namespace Laminas\AutomaticReleases\Github\Api\GraphQL;

use Psl;
use Psl\Json;
use Psl\Type;
use Psr\Http\Client\ClientInterface;
use Psr\Http\Message\RequestFactoryInterface;

use function array_key_exists;

final class RunGraphQLQuery implements RunQuery
{
private const ENDPOINT = 'https://api.github.com/graphql';
Expand Down Expand Up @@ -56,8 +59,13 @@ public function __invoke(

Type\literal_scalar(200)->assert($response->getStatusCode());

return Json\typed($responseBody, Type\shape([
'data' => Type\dict(Type\string(), Type\mixed()),
]))['data'];
$response = Json\typed($responseBody, Type\shape([
'data' => Type\dict(Type\string(), Type\mixed()),
'errors' => Type\optional(Type\mixed()),
]));

Psl\invariant(! array_key_exists('errors', $response), 'GraphQL query execution failed');

return $response['data'];
}
}
2 changes: 1 addition & 1 deletion src/Github/Api/V3/CreatePullRequestThroughApiCall.php
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ public function __invoke(

$response = $this->client->sendRequest($request);

Psl\invariant($response->getStatusCode() >= 200 || $response->getStatusCode() <= 299, 'Failed to create pull request through GitHub API.');
Psl\invariant($response->getStatusCode() >= 200 && $response->getStatusCode() <= 299, 'Failed to create pull request through GitHub API.');

$responseBody = $response
->getBody()
Expand Down
2 changes: 1 addition & 1 deletion src/Github/Api/V3/SetDefaultBranchThroughApiCall.php
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ public function __invoke(

$response = $this->client->sendRequest($request);

Psl\invariant($response->getStatusCode() >= 200 || $response->getStatusCode() <= 299, 'Failed to set default branch through GitHub API.');
Psl\invariant($response->getStatusCode() >= 200 && $response->getStatusCode() <= 299, 'Failed to set default branch through GitHub API.');

$responseBody = $response
->getBody()
Expand Down
108 changes: 108 additions & 0 deletions test/unit/Github/Api/GraphQL/RunGraphQLQueryTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,14 @@
use Laminas\Diactoros\Response;
use PHPUnit\Framework\MockObject\MockObject;
use PHPUnit\Framework\TestCase;
use Psl\Exception\InvariantViolationException;
use Psl\SecureRandom;
use Psl\Type\Exception\AssertException;
use Psr\Http\Client\ClientInterface;
use Psr\Http\Message\RequestFactoryInterface;
use Psr\Http\Message\RequestInterface;

/** @covers \Laminas\AutomaticReleases\Github\Api\GraphQL\RunGraphQLQuery */
final class RunGraphQLQueryTest extends TestCase
{
/** @var ClientInterface&MockObject */
Expand Down Expand Up @@ -84,4 +87,109 @@ public function testSuccessfulRequest(): void
$this->runQuery->__invoke('the-query', ['a' => 'b'])
);
}

/**
* @psalm-param positive-int $responseCode
*
* @dataProvider exampleFailureResponseCodes
*/
public function testWillThrowIfGraphQLResponseIsNotSuccessful(int $responseCode): void
{
$validResponse = (new Response())
->withStatus($responseCode);

$validResponse->getBody()->write(<<<'JSON'
{
"data": {"foo": "bar"}
}
JSON
);

$this
->httpClient
->expects(self::once())
->method('sendRequest')
->with(self::callback(function (RequestInterface $request): bool {
self::assertSame(
[
'Host' => ['the-domain.com'],
'Content-Type' => ['application/json'],
'User-Agent' => ['Ocramius\'s minimal GraphQL client - stolen from Dunglas'],
'Authorization' => ['bearer ' . $this->apiToken],
],
$request->getHeaders()
);

self::assertSame(
'{"query":"the-query","variables":{"a":"b"}}',
$request->getBody()->__toString()
);

return true;
}))
->willReturn($validResponse);

$this->expectException(AssertException::class);

self::assertSame(
['foo' => 'bar'],
$this->runQuery->__invoke('the-query', ['a' => 'b'])
);
}

/** @psalm-return non-empty-list<array{positive-int}> */
public function exampleFailureResponseCodes(): array
{
return [
[199],
[201],
[400],
[500],
];
}

public function testWillThrowIfGraphQLResponseIncludesErrorsInResponse(): void
{
$validResponse = new Response();

$validResponse->getBody()->write(<<<'JSON'
{
"errors": ["nope"],
"data": {"foo": "bar"}
}
JSON
);

$this
->httpClient
->expects(self::once())
->method('sendRequest')
->with(self::callback(function (RequestInterface $request): bool {
self::assertSame(
[
'Host' => ['the-domain.com'],
'Content-Type' => ['application/json'],
'User-Agent' => ['Ocramius\'s minimal GraphQL client - stolen from Dunglas'],
'Authorization' => ['bearer ' . $this->apiToken],
],
$request->getHeaders()
);

self::assertSame(
'{"query":"the-query","variables":{"a":"b"}}',
$request->getBody()->__toString()
);

return true;
}))
->willReturn($validResponse);

$this->expectException(InvariantViolationException::class);
$this->expectExceptionMessage('GraphQL query execution failed');

self::assertSame(
['foo' => 'bar'],
$this->runQuery->__invoke('the-query', ['a' => 'b'])
);
}
}
1 change: 1 addition & 0 deletions test/unit/Github/Api/V3/CreateMilestoneTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
use Psr\Http\Message\RequestInterface;
use Psr\Log\LoggerInterface;

/** @covers \Laminas\AutomaticReleases\Github\Api\V3\CreateMilestoneThroughApiCall */
class CreateMilestoneTest extends TestCase
{
/** @var ClientInterface&MockObject */
Expand Down
1 change: 1 addition & 0 deletions test/unit/Github/Api/V3/CreatePullRequestTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
use Psr\Http\Message\RequestFactoryInterface;
use Psr\Http\Message\RequestInterface;

/** @covers \Laminas\AutomaticReleases\Github\Api\V3\CreatePullRequestThroughApiCall */
final class CreatePullRequestTest extends TestCase
{
/** @var ClientInterface&MockObject */
Expand Down
192 changes: 192 additions & 0 deletions test/unit/Github/Api/V3/CreatePullRequestThroughApiCallTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,192 @@
<?php

declare(strict_types=1);

namespace Laminas\AutomaticReleases\Test\Unit\Github\Api\V3;

use Laminas\AutomaticReleases\Git\Value\BranchName;
use Laminas\AutomaticReleases\Github\Api\V3\CreatePullRequestThroughApiCall;
use Laminas\AutomaticReleases\Github\Value\RepositoryName;
use Laminas\Diactoros\Request;
use Laminas\Diactoros\Response;
use PHPUnit\Framework\MockObject\MockObject;
use PHPUnit\Framework\TestCase;
use Psl\Exception\InvariantViolationException;
use Psl\Json\Exception\DecodeException;
use Psl\SecureRandom;
use Psr\Http\Client\ClientInterface;
use Psr\Http\Message\RequestFactoryInterface;
use Psr\Http\Message\RequestInterface;

/** @covers \Laminas\AutomaticReleases\Github\Api\V3\CreatePullRequestThroughApiCall */
final class CreatePullRequestThroughApiCallTest extends TestCase
{
/** @var ClientInterface&MockObject */
private ClientInterface $httpClient;
/** @var RequestFactoryInterface&MockObject */
private RequestFactoryInterface $messageFactory;
/** @psalm-var non-empty-string */
private string $apiToken;
private CreatePullRequestThroughApiCall $createPullRequest;

protected function setUp(): void
{
parent::setUp();

$this->httpClient = $this->createMock(ClientInterface::class);
$this->messageFactory = $this->createMock(RequestFactoryInterface::class);
$this->apiToken = 'apiToken' . SecureRandom\string(8);
$this->createPullRequest = new CreatePullRequestThroughApiCall(
$this->messageFactory,
$this->httpClient,
$this->apiToken
);
}

/**
* @psalm-param positive-int $responseCode
*
* @dataProvider exampleValidResponseCodes
*/
public function testSuccessfulRequest(int $responseCode): void
{
$this->messageFactory
->method('createRequest')
->with('POST', 'https://api.github.com/repos/foo/bar/pulls')
->willReturn(new Request('https://the-domain.com/the-path'));

$validResponse = (new Response())
->withStatus($responseCode);

$validResponse->getBody()
->write('{"url": "http://the-domain.com/pull-request"}');

$this->httpClient
->expects(self::once())
->method('sendRequest')
->with(self::callback(function (RequestInterface $request): bool {
self::assertSame(
[
'Host' => ['the-domain.com'],
'Content-Type' => ['application/json'],
'User-Agent' => ['Ocramius\'s minimal API V3 client'],
'Authorization' => ['token ' . $this->apiToken],
],
$request->getHeaders()
);

self::assertJsonStringEqualsJsonString(
<<<'JSON'
{
"base": "target-branch-name",
"body": "A description for my awesome pull request",
"draft": false,
"head": "foo-bar-baz",
"maintainer_can_modify": true,
"title": "My awesome pull request"
}
JSON
,
$request->getBody()
->__toString()
);

return true;
}))
->willReturn($validResponse);

$this->createPullRequest->__invoke(
RepositoryName::fromFullName('foo/bar'),
BranchName::fromName('foo-bar-baz'),
BranchName::fromName('target-branch-name'),
'My awesome pull request',
'A description for my awesome pull request'
);
}

/** @psalm-return non-empty-list<array{positive-int}> */
public function exampleValidResponseCodes(): array
{
return [
[200],
[201],
[204],
];
}

/**
* @psalm-param positive-int $responseCode
*
* @dataProvider exampleFailureResponseCodes
*/
public function testRequestFailedToCreatePullRequestDueToInvalidResponseCode(int $responseCode): void
{
$this->messageFactory
->method('createRequest')
->with('POST', 'https://api.github.com/repos/foo/bar/pulls')
->willReturn(new Request('https://the-domain.com/the-path'));

$invalidResponse = (new Response())
->withStatus($responseCode);

$invalidResponse->getBody()
->write('{"url": "http://the-domain.com/pull-request"}');

$this->httpClient
->expects(self::once())
->method('sendRequest')
->willReturn($invalidResponse);

$this->expectException(InvariantViolationException::class);
$this->expectExceptionMessage('Failed to create pull request through GitHub API.');

$this->createPullRequest->__invoke(
RepositoryName::fromFullName('foo/bar'),
BranchName::fromName('foo-bar-baz'),
BranchName::fromName('target-branch-name'),
'My awesome pull request',
'A description for my awesome pull request'
);
}

/** @psalm-return non-empty-list<array{positive-int}> */
public function exampleFailureResponseCodes(): array
{
return [
[199],
[400],
[401],
[500],
];
}

public function testRequestFailedToCreatePullRequestDueToInvalidResponseBody(): void
{
$this->messageFactory
->method('createRequest')
->with('POST', 'https://api.github.com/repos/foo/bar/pulls')
->willReturn(new Request('https://the-domain.com/the-path'));

$invalidResponse = (new Response())
->withStatus(200);

$invalidResponse->getBody()
->write('{"invalid": "response"}');

$this->httpClient
->expects(self::once())
->method('sendRequest')
->willReturn($invalidResponse);

$this->expectException(DecodeException::class);
$this->expectExceptionMessage('"array{\'url\': mixed}"');

$this->createPullRequest->__invoke(
RepositoryName::fromFullName('foo/bar'),
BranchName::fromName('foo-bar-baz'),
BranchName::fromName('target-branch-name'),
'My awesome pull request',
'A description for my awesome pull request'
);
}
}
1 change: 1 addition & 0 deletions test/unit/Github/Api/V3/CreateReleaseTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
use Psr\Http\Message\RequestFactoryInterface;
use Psr\Http\Message\RequestInterface;

/** @covers \Laminas\AutomaticReleases\Github\Api\V3\CreateReleaseThroughApiCall */
final class CreateReleaseTest extends TestCase
{
/** @var ClientInterface&MockObject */
Expand Down
Loading

0 comments on commit cf278ac

Please sign in to comment.