From bf75ec82ad4e28d6fd944dd76b715cf17f0bde48 Mon Sep 17 00:00:00 2001 From: Philip Gichuhi Date: Wed, 25 Aug 2021 01:45:05 +0300 Subject: [PATCH 1/7] Add a GraphServiceException and GraphError object --- src/Exception/GraphClientException.php | 3 + src/Exception/GraphServiceException.php | 120 ++++++++++++++++++++++++ src/Http/GraphError.php | 105 +++++++++++++++++++++ src/Http/GraphRequest.php | 19 ++++ 4 files changed, 247 insertions(+) create mode 100644 src/Exception/GraphServiceException.php create mode 100644 src/Http/GraphError.php diff --git a/src/Exception/GraphClientException.php b/src/Exception/GraphClientException.php index 02b09b7c..15ac3fdf 100644 --- a/src/Exception/GraphClientException.php +++ b/src/Exception/GraphClientException.php @@ -9,6 +9,9 @@ /** * Class GraphClientException + * + * Thrown when there are SDK-level errors (mostly validation errors) + * * @package Microsoft\Graph\Exception * @copyright 2021 Microsoft Corporation * @license https://opensource.org/licenses/MIT MIT License diff --git a/src/Exception/GraphServiceException.php b/src/Exception/GraphServiceException.php new file mode 100644 index 00000000..d23372ea --- /dev/null +++ b/src/Exception/GraphServiceException.php @@ -0,0 +1,120 @@ +graphRequest = $graphRequest; + $this->responseStatusCode = $responseStatusCode; + $this->responseBody = $responseBody; + $this->responseHeaders = $responseHeaders; + $message = "'".$graphRequest->getRequestType()."' request to ".$graphRequest->getRequestUri()." returned ".$responseStatusCode."\n".$responseBody; + parent::__construct($message, $responseStatusCode); + } + + /** + * Returns HTTP headers in the response from the Graph + * + * @return array + */ + public function getResponseHeaders(): array { + return $this->responseHeaders; + } + + /** + * Returns HTTP status code returned int the response from the Graph + * + * @return int + */ + public function getResponseStatusCode(): int { + return $this->responseStatusCode; + } + + /** + * Get JSON-decoded response payload from the Graph + * + * @return array + */ + public function getRawResponseBody(): array { + return $this->responseBody; + } + + /** + * Returns the error object of the payload as a model + * + * @return GraphError|null + */ + public function getError(): ?GraphError { + if (array_key_exists("error", $this->responseBody)) { + return new GraphError($this->responseBody["error"]); + } + return null; + } + + /** + * Returns the request that triggered the error response + * + * @return GraphRequest + */ + public function getRequest(): GraphRequest { + return $this->graphRequest; + } +} diff --git a/src/Http/GraphError.php b/src/Http/GraphError.php new file mode 100644 index 00000000..7068c396 --- /dev/null +++ b/src/Http/GraphError.php @@ -0,0 +1,105 @@ +propDict = $propDict; + } + + /** + * Get error code returned by the Graph + * + * @return string|null + */ + public function code(): ?string { + if (array_key_exists("code", $this->propDict)) { + return $this->propDict["code"]; + } + return null; + } + + /** + * Get error message returned by the Graph + * + * @return string|null + */ + public function message(): ?string { + if (array_key_exists("message", $this->propDict)) { + return $this->propDict["message"]; + } + return null; + } + + /** + * Get the additional error info + * + * @return GraphError|null + */ + public function innerError(): ?GraphError { + if (array_key_exists("innerError", $this->propDict)) { + return new GraphError($this->propDict["innerError"]); + } + return null; + } + + /** + * Returns the client request Id + * + * @return string|null + */ + public function clientRequestId(): ?string { + if (array_key_exists("client-request-id", $this->propDict)) { + return $this->propDict["client-request-id"]; + } + return null; + } + + /** + * Returns the request Id + * + * @return string|null + */ + public function requestId(): ?string { + if (array_key_exists("request-id", $this->propDict)) { + return $this->propDict["request-id"]; + } + return null; + } + + /** + * Returns the date of the request + * + * @return string|null + */ + public function date(): ?string { + if (array_key_exists("date", $this->propDict)) { + return $this->propDict["date"]; + } + return null; + } + + /** + * Returns all properties passed to the constructor + * + * @return array + */ + public function getProperties(): array { + return $this->propDict; + } +} diff --git a/src/Http/GraphRequest.php b/src/Http/GraphRequest.php index 289f52f4..1dcaacfd 100644 --- a/src/Http/GraphRequest.php +++ b/src/Http/GraphRequest.php @@ -105,15 +105,34 @@ public function __construct(string $requestType, string $endpoint, AbstractGraph $this->initPsr7HttpRequest(); } + /** + * Sets the request URI and updates the Psr7 request with the new URI + * + * @param UriInterface $uri + */ protected function setRequestUri(UriInterface $uri): void { $this->requestUri = $uri; $this->httpRequest = $this->httpRequest->withUri($uri); } + /** + * Returns the final request URI after resolving $endpoint to base URL + * + * @return UriInterface + */ public function getRequestUri(): UriInterface { return $this->requestUri; } + /** + * ReturnS the HTTP method used + * + * @return string + */ + public function getRequestType(): string { + return $this->requestType; + } + /** * Sets a new accessToken * From 56a4683cea27caa0f4f21793a58d4bdecd0366a7 Mon Sep 17 00:00:00 2001 From: Philip Gichuhi Date: Wed, 25 Aug 2021 02:16:25 +0300 Subject: [PATCH 2/7] Update graph request methods to throw GraphServiceException --- src/Http/GraphCollectionRequest.php | 8 +++-- src/Http/GraphRequest.php | 49 ++++++++++++++++++++++++++--- 2 files changed, 50 insertions(+), 7 deletions(-) diff --git a/src/Http/GraphCollectionRequest.php b/src/Http/GraphCollectionRequest.php index 991421d6..39c98f99 100644 --- a/src/Http/GraphCollectionRequest.php +++ b/src/Http/GraphCollectionRequest.php @@ -11,6 +11,7 @@ use Microsoft\Graph\Core\GraphConstants; use Microsoft\Graph\Exception\GraphClientException; use Microsoft\Graph\Exception\GraphException; +use Microsoft\Graph\Exception\GraphServiceException; use Microsoft\Graph\Task\PageIterator; use Psr\Http\Client\ClientExceptionInterface; @@ -85,9 +86,11 @@ public function __construct(string $requestType, string $endpoint, AbstractGraph * Gets the number of entries in the collection * * @return int the number of entries - * @throws ClientExceptionInterface|GraphClientException + * @throws ClientExceptionInterface + * @throws GraphClientException if @odata.count is not present in the response + * @throws GraphServiceException if 4xx or 5xx response is returned */ - public function count() + public function count(): int { $query = '$count=true'; $requestUri = $this->getRequestUri(); @@ -130,6 +133,7 @@ public function setPageSize(int $pageSize): self * * @return GraphResponse|array of objects of class $returnType| GraphResponse if no $returnType * @throws ClientExceptionInterface + * @throws GraphServiceException if 4xx or 5xx response is returned */ public function getPage() { diff --git a/src/Http/GraphRequest.php b/src/Http/GraphRequest.php index 1dcaacfd..448f39db 100644 --- a/src/Http/GraphRequest.php +++ b/src/Http/GraphRequest.php @@ -8,12 +8,14 @@ namespace Microsoft\Graph\Http; use GuzzleHttp\Psr7\Request; +use GuzzleHttp\Psr7\Response; use GuzzleHttp\Psr7\Utils; use Http\Client\HttpAsyncClient; use Http\Promise\Promise; use Microsoft\Graph\Core\GraphConstants; use Microsoft\Graph\Core\NationalCloud; use Microsoft\Graph\Exception\GraphClientException; +use Microsoft\Graph\Exception\GraphServiceException; use Psr\Http\Client\ClientExceptionInterface; use Psr\Http\Client\ClientInterface; use Psr\Http\Message\StreamInterface; @@ -101,7 +103,7 @@ public function __construct(string $requestType, string $endpoint, AbstractGraph $this->graphClient = $graphClient; $baseUrl = ($baseUrl) ?: $graphClient->getNationalCloud(); $this->initRequestUri($baseUrl, $endpoint); - $this->initHeaders($baseUrl); + $this->initHeaders(); $this->initPsr7HttpRequest(); } @@ -233,6 +235,7 @@ public function getBody() * @param ClientInterface|null $client (optional) When null, uses $graphClient's http client * @return array|GraphResponse|StreamInterface|object Graph Response object or response body cast to $returnType * @throws ClientExceptionInterface + * @throws GraphServiceException if 4xx or 5xx response is returned. Exception contains the error payload */ public function execute(?ClientInterface $client = null) { @@ -241,6 +244,7 @@ public function execute(?ClientInterface $client = null) } $result = $client->sendRequest($this->httpRequest); + $this->handleErrorResponse($result); // Check to see if returnType is a stream, if so return it immediately if($this->returnsStream) { @@ -269,7 +273,9 @@ public function execute(?ClientInterface $client = null) * * @param HttpAsyncClient|null $client (optional) When null, uses $graphClient's http client * @return Promise Resolves to GraphResponse object|response body cast to $returnType. Fails throwing the exception - * @throws \Exception when promise fails + * @throws GraphServiceException if 4xx or 5xx response is returned. Exception contains the error payload + * @throws ClientExceptionInterface if there are any errors while making the HTTP request + * @throws \Exception */ public function executeAsync(?HttpAsyncClient $client = null): Promise { @@ -280,6 +286,7 @@ public function executeAsync(?HttpAsyncClient $client = null): Promise return $client->sendAsyncRequest($this->httpRequest)->then( // On success, return the result/response function ($result) { + $this->handleErrorResponse($result); // Check to see if returnType is a stream, if so return it immediately if($this->returnsStream) { @@ -312,7 +319,9 @@ function ($reason) { * * @param string $path path to download the file contents to * @param ClientInterface|null $client (optional) When null, defaults to $graphClient's http client - * @throws ClientExceptionInterface|GraphClientException when unable to open $path for writing + * @throws ClientExceptionInterface + * @throws GraphClientException when unable to open $path for writing + * @throws GraphServiceException if 4xx or 5xx response is returned. Error payload is accessible from the exception */ public function download(string $path, ?ClientInterface $client = null): void { @@ -323,6 +332,7 @@ public function download(string $path, ?ClientInterface $client = null): void $resource = Utils::tryFopen($path, 'w'); $stream = Utils::streamFor($resource); $response = $client->sendRequest($this->httpRequest); + $this->handleErrorResponse($response); $stream->write($response->getBody()->getContents()); $stream->close(); } catch (\RuntimeException $ex) { @@ -336,7 +346,9 @@ public function download(string $path, ?ClientInterface $client = null): void * @param string $path path of file to be uploaded * @param ClientInterface|null $client (optional) * @return array|GraphResponse|StreamInterface|object Graph Response object or response body cast to $returnType - * @throws ClientExceptionInterface|GraphClientException if $path cannot be opened for reading + * @throws ClientExceptionInterface + * @throws GraphClientException if $path cannot be opened for reading + * @throws GraphServiceException if 4xx or 5xx response is returned. Error payload is accessible from the exception */ public function upload(string $path, ?ClientInterface $client = null) { @@ -356,7 +368,7 @@ public function upload(string $path, ?ClientInterface $client = null) /** * Sets default headers based on baseUrl being a Graph endpoint or not */ - private function initHeaders(string $baseUrl): void + private function initHeaders(): void { $coreSdkVersion = "graph-php-core/".GraphConstants::SDK_VERSION; if ($this->graphClient->getApiVersion() === GraphConstants::BETA_API_VERSION) { @@ -395,4 +407,31 @@ protected function initRequestUri(string $baseUrl, string $endpoint): void { protected function initPsr7HttpRequest(): void { $this->httpRequest = new Request($this->requestType, $this->requestUri, $this->headers, $this->requestBody); } + + /** + * Check if response status code is 4xx or 5xx + * + * @param int $httpStatusCode + * @return bool + */ + private function isErrorResponse(int $httpStatusCode): bool { + return ($httpStatusCode >=400 && $httpStatusCode <= 599); + } + + /** + * Determines whether to throw GraphServiceException or not + * + * @param Response $httpResponse + * @throws GraphServiceException + */ + private function handleErrorResponse(Response $httpResponse) { + if ($this->isErrorResponse($httpResponse->getStatusCode())) { + throw new GraphServiceException( + $this, + $httpResponse->getStatusCode(), + json_decode($httpResponse->getBody(), true), + $httpResponse->getHeaders() + ); + } + } } From 7f8e990cad2ce3bd501391537d5e58d368f49bf7 Mon Sep 17 00:00:00 2001 From: Philip Gichuhi Date: Fri, 27 Aug 2021 01:42:04 +0300 Subject: [PATCH 3/7] Update tests to expect GraphServiceException --- src/Exception/GraphServiceException.php | 3 +- src/Http/GraphCollectionRequest.php | 1 + src/Http/GraphError.php | 12 +-- tests/Exception/GraphServiceExceptionTest.php | 52 +++++++++++ tests/Http/GraphErrorTest.php | 87 +++++++++++++++++++ .../Request/GraphCollectionRequestTest.php | 19 ++++ tests/Http/Request/GraphRequestAsyncTest.php | 11 +-- tests/Http/Request/GraphRequestStreamTest.php | 20 +++++ tests/Http/Request/GraphRequestSyncTest.php | 13 +-- 9 files changed, 200 insertions(+), 18 deletions(-) create mode 100644 tests/Exception/GraphServiceExceptionTest.php create mode 100644 tests/Http/GraphErrorTest.php diff --git a/src/Exception/GraphServiceException.php b/src/Exception/GraphServiceException.php index d23372ea..11644744 100644 --- a/src/Exception/GraphServiceException.php +++ b/src/Exception/GraphServiceException.php @@ -10,6 +10,7 @@ use Microsoft\Graph\Http\GraphError; use Microsoft\Graph\Http\GraphRequest; +use Microsoft\Graph\Http\GraphRequestUtil; /** * Class GraphServiceException @@ -66,7 +67,7 @@ public function __construct( $this->responseStatusCode = $responseStatusCode; $this->responseBody = $responseBody; $this->responseHeaders = $responseHeaders; - $message = "'".$graphRequest->getRequestType()."' request to ".$graphRequest->getRequestUri()." returned ".$responseStatusCode."\n".$responseBody; + $message = "'".$graphRequest->getRequestType()."' request to ".$graphRequest->getRequestUri()." returned ".$responseStatusCode."\n".json_encode($responseBody); parent::__construct($message, $responseStatusCode); } diff --git a/src/Http/GraphCollectionRequest.php b/src/Http/GraphCollectionRequest.php index 39c98f99..0155bf9b 100644 --- a/src/Http/GraphCollectionRequest.php +++ b/src/Http/GraphCollectionRequest.php @@ -244,6 +244,7 @@ public function getPageSize(): int { * @return PageIterator call iterate() to start the iterator * @throws ClientExceptionInterface * @throws GraphClientException + * @throws GraphServiceException if 4xx or 5xx is returned when fetching the initial collection */ public function pageIterator(callable $callback): PageIterator { // temporarily disable return type in order to get first page as GraphResponse object diff --git a/src/Http/GraphError.php b/src/Http/GraphError.php index 7068c396..9125e0a0 100644 --- a/src/Http/GraphError.php +++ b/src/Http/GraphError.php @@ -27,7 +27,7 @@ public function __construct(array $propDict) { * * @return string|null */ - public function code(): ?string { + public function getCode(): ?string { if (array_key_exists("code", $this->propDict)) { return $this->propDict["code"]; } @@ -39,7 +39,7 @@ public function code(): ?string { * * @return string|null */ - public function message(): ?string { + public function getMessage(): ?string { if (array_key_exists("message", $this->propDict)) { return $this->propDict["message"]; } @@ -51,7 +51,7 @@ public function message(): ?string { * * @return GraphError|null */ - public function innerError(): ?GraphError { + public function getInnerError(): ?GraphError { if (array_key_exists("innerError", $this->propDict)) { return new GraphError($this->propDict["innerError"]); } @@ -63,7 +63,7 @@ public function innerError(): ?GraphError { * * @return string|null */ - public function clientRequestId(): ?string { + public function getClientRequestId(): ?string { if (array_key_exists("client-request-id", $this->propDict)) { return $this->propDict["client-request-id"]; } @@ -75,7 +75,7 @@ public function clientRequestId(): ?string { * * @return string|null */ - public function requestId(): ?string { + public function getRequestId(): ?string { if (array_key_exists("request-id", $this->propDict)) { return $this->propDict["request-id"]; } @@ -87,7 +87,7 @@ public function requestId(): ?string { * * @return string|null */ - public function date(): ?string { + public function getDate(): ?string { if (array_key_exists("date", $this->propDict)) { return $this->propDict["date"]; } diff --git a/tests/Exception/GraphServiceExceptionTest.php b/tests/Exception/GraphServiceExceptionTest.php new file mode 100644 index 00000000..025734d6 --- /dev/null +++ b/tests/Exception/GraphServiceExceptionTest.php @@ -0,0 +1,52 @@ +mockGraphRequest = $this->createMock(GraphRequest::class); + $this->responseStatusCode = 404; + $this->responseHeaders = [ + "Content-Type" => "application/json", + "request-id" => "2f91ee0a-e013-425b-a5bf-b1f251163969", + "client-request-id" => "2f91ee0a-e013-425b-a5bf-b1f251163969", + ]; + $this->defaultException = new GraphServiceException( + $this->mockGraphRequest, + $this->responseStatusCode, + $this->responseBody, + $this->responseHeaders + ); + } + + public function testGraphServiceExceptionIsThrowable(): void { + $this->assertInstanceOf(\Throwable::class, $this->defaultException); + } + + public function testGetters(): void { + $this->assertEquals($this->mockGraphRequest, $this->defaultException->getRequest()); + $this->assertEquals($this->responseStatusCode, $this->defaultException->getResponseStatusCode()); + $this->assertEquals($this->responseBody, $this->defaultException->getRawResponseBody()); + $this->assertEquals($this->responseHeaders, $this->defaultException->getResponseHeaders()); + $this->assertInstanceOf(GraphError::class, $this->defaultException->getError()); + } +} diff --git a/tests/Http/GraphErrorTest.php b/tests/Http/GraphErrorTest.php new file mode 100644 index 00000000..7c008bf5 --- /dev/null +++ b/tests/Http/GraphErrorTest.php @@ -0,0 +1,87 @@ +defaultGraphError = new GraphError(SampleGraphResponsePayload::ERROR_PAYLOAD["error"]); + } + + public function testGetCode(): void { + $this->assertEquals( + SampleGraphResponsePayload::ERROR_PAYLOAD["error"]["code"], + $this->defaultGraphError->getCode() + ); + + $graphError = $this->unsetProperty("code"); + $this->assertNull($graphError->getCode()); + } + + public function testGetMessage(): void { + $this->assertEquals( + SampleGraphResponsePayload::ERROR_PAYLOAD["error"]["message"], + $this->defaultGraphError->getMessage() + ); + $graphError = $this->unsetProperty("message"); + $this->assertNull($graphError->getMessage()); + } + + public function testGetInnerError(): void { + $this->assertInstanceOf(GraphError::class, $this->defaultGraphError->getInnerError()); + $graphError = $this->unsetProperty("innerError"); + $this->assertNull($graphError->getInnerError()); + } + + public function testGetClientRequestId(): void { + $this->assertNull($this->defaultGraphError->getClientRequestId()); + $this->assertEquals( + SampleGraphResponsePayload::ERROR_PAYLOAD["error"]["innerError"]["client-request-id"], + $this->defaultGraphError->getInnerError()->getClientRequestId() + ); + $graphError = $this->unsetProperty("client-request-id", SampleGraphResponsePayload::ERROR_PAYLOAD["error"]["innerError"]); + $this->assertNull($graphError->getClientRequestId()); + } + + public function testGetRequestId(): void { + $this->assertNull($this->defaultGraphError->getRequestId()); + $this->assertEquals( + SampleGraphResponsePayload::ERROR_PAYLOAD["error"]["innerError"]["request-id"], + $this->defaultGraphError->getInnerError()->getRequestId() + ); + $graphError = $this->unsetProperty("request-id", SampleGraphResponsePayload::ERROR_PAYLOAD["error"]["innerError"]); + $this->assertNull($graphError->getRequestId()); + } + + public function testGetDate(): void { + $this->assertNull($this->defaultGraphError->getDate()); + $this->assertEquals( + SampleGraphResponsePayload::ERROR_PAYLOAD["error"]["innerError"]["date"], + $this->defaultGraphError->getInnerError()->getDate() + ); + $graphError = $this->unsetProperty("date", SampleGraphResponsePayload::ERROR_PAYLOAD["error"]["innerError"]); + $this->assertNull($graphError->getDate()); + } + + public function testGetProperties(): void { + $this->assertEquals(SampleGraphResponsePayload::ERROR_PAYLOAD["error"], $this->defaultGraphError->getProperties()); + } + + private function unsetProperty(string $property, + array $payload = SampleGraphResponsePayload::ERROR_PAYLOAD["error"]): GraphError { + unset($payload[$property]); + return new GraphError($payload); + } +} diff --git a/tests/Http/Request/GraphCollectionRequestTest.php b/tests/Http/Request/GraphCollectionRequestTest.php index 9bba4d33..afe0d047 100644 --- a/tests/Http/Request/GraphCollectionRequestTest.php +++ b/tests/Http/Request/GraphCollectionRequestTest.php @@ -4,6 +4,7 @@ use Microsoft\Graph\Core\GraphConstants; use Microsoft\Graph\Exception\GraphClientException; use Microsoft\Graph\Exception\GraphException; +use Microsoft\Graph\Exception\GraphServiceException; use Microsoft\Graph\Http\GraphCollectionRequest; use Microsoft\Graph\Http\GraphRequestUtil; use Microsoft\Graph\Task\PageIterator; @@ -61,6 +62,12 @@ public function testHitEndOfCollection() $this->defaultCollectionRequest->getPage(); } + public function testGetPageThrowsExceptionOnErrorResponse(): void { + $this->expectException(GraphServiceException::class); + MockHttpClientResponseConfig::configureWithErrorPayload($this->mockHttpClient, 500); + $this->defaultCollectionRequest->getPage(); + } + public function testCount(): void { MockHttpClientResponseConfig::configureWithCollectionPayload($this->mockHttpClient); $count = $this->defaultCollectionRequest->count(); @@ -73,6 +80,12 @@ public function testCountThrowsErrorIfNoOdataCountFound(): void { $count = $this->defaultCollectionRequest->count(); } + public function testCountThrowsExceptionOnErrorResponse(): void { + $this->expectException(GraphServiceException::class); + MockHttpClientResponseConfig::configureWithErrorPayload($this->mockHttpClient, 404); + $this->defaultCollectionRequest->count(); + } + public function testPageIteratorReturnsValidPageIterator() { MockHttpClientResponseConfig::configureWithCollectionPayload($this->mockHttpClient); $numEntitiesProcessed = 0; @@ -96,4 +109,10 @@ public function testPageIteratorInitialisesUsingFirstPageOfResults() { $promise->wait(); $this->assertTrue($numEntitiesProcessed >= sizeof(SampleGraphResponsePayload::COLLECTION_PAYLOAD["value"])); } + + public function testPageIteratorThrowsExceptionIfFirstPageRequestGetsErrorResponse(): void { + $this->expectException(GraphServiceException::class); + MockHttpClientResponseConfig::configureWithErrorPayload($this->mockHttpClient, 500); + $this->defaultCollectionRequest->pageIterator(function () {}); + } } diff --git a/tests/Http/Request/GraphRequestAsyncTest.php b/tests/Http/Request/GraphRequestAsyncTest.php index 47ea2e2b..3c410142 100644 --- a/tests/Http/Request/GraphRequestAsyncTest.php +++ b/tests/Http/Request/GraphRequestAsyncTest.php @@ -9,9 +9,9 @@ namespace Microsoft\Graph\Test\Http\Request; -use GuzzleHttp\Psr7\Stream; use Http\Client\HttpAsyncClient; use Http\Promise\Promise; +use Microsoft\Graph\Exception\GraphServiceException; use Microsoft\Graph\Http\GraphResponse; use Microsoft\Graph\Test\Http\SampleGraphResponsePayload; use Microsoft\Graph\Test\Http\TestModel; @@ -62,17 +62,18 @@ public function testExecuteAsyncPromiseResolvesToGraphResponseIfNoReturnType(): $this->assertInstanceOf(GraphResponse::class, $promise->wait()); } - public function testExecuteAsyncPromiseResolvesToGraphResponseForErrorPayload(): void { + public function testExecuteAsyncPromiseThrowsExceptionForErrorResponse(): void { + $this->expectException(GraphServiceException::class); MockHttpClientAsyncResponseConfig::statusCode(400)::configureWithFulfilledPromise( $this->mockHttpClient, SampleGraphResponsePayload::ERROR_PAYLOAD ); $promise = $this->defaultGraphRequest->executeAsync(); - $this->assertInstanceOf(GraphResponse::class, $promise->wait()); + $promise->wait(); } public function testExecuteAsyncResolvesToModelForModelReturnType(): void { - MockHttpClientAsyncResponseConfig::configureWithFulfilledPromise( + MockHttpClientAsyncResponseConfig::statusCode()::configureWithFulfilledPromise( $this->mockHttpClient, SampleGraphResponsePayload::ENTITY_PAYLOAD ); @@ -81,7 +82,7 @@ public function testExecuteAsyncResolvesToModelForModelReturnType(): void { } public function testExecuteAsyncResolvesToModelArrayForCollectionRequest(): void { - MockHttpClientAsyncResponseConfig::configureWithFulfilledPromise( + MockHttpClientAsyncResponseConfig::statusCode()::configureWithFulfilledPromise( $this->mockHttpClient, SampleGraphResponsePayload::COLLECTION_PAYLOAD ); diff --git a/tests/Http/Request/GraphRequestStreamTest.php b/tests/Http/Request/GraphRequestStreamTest.php index e2952a7b..ab1c8723 100644 --- a/tests/Http/Request/GraphRequestStreamTest.php +++ b/tests/Http/Request/GraphRequestStreamTest.php @@ -3,6 +3,7 @@ namespace Microsoft\Graph\Test\Http\Request; use Microsoft\Graph\Exception\GraphClientException; +use Microsoft\Graph\Exception\GraphServiceException; use Microsoft\Graph\Test\Http\SampleGraphResponsePayload; use org\bovigo\vfs\vfsStream; use org\bovigo\vfs\vfsStreamFile; @@ -35,6 +36,16 @@ public function testInvalidUpload() $this->defaultGraphRequest->upload($file->url()); } + public function testUploadThrowsExceptionForErrorResponse() + { + $this->expectException(GraphServiceException::class); + MockHttpClientResponseConfig::configureWithErrorPayload($this->mockHttpClient); + $file = vfsStream::newFile('foo.txt') + ->withContent("content") + ->at($this->rootDir); + $this->defaultGraphRequest->upload($file->url()); + } + public function testDownload() { $file = new VfsStreamFile('foo.txt'); @@ -52,4 +63,13 @@ public function testInvalidDownload() $this->rootDir->addChild($file); $this->defaultGraphRequest->download($file->url()); } + + public function testDownloadThrowsExceptionForErrorResponse() + { + $this->expectException(GraphServiceException::class); + MockHttpClientResponseConfig::configureWithErrorPayload($this->mockHttpClient); + $file = new VfsStreamFile('foo.txt'); + $this->rootDir->addChild($file); + $this->defaultGraphRequest->download($file->url()); + } } diff --git a/tests/Http/Request/GraphRequestSyncTest.php b/tests/Http/Request/GraphRequestSyncTest.php index 9f5be8b2..a9ccbf54 100644 --- a/tests/Http/Request/GraphRequestSyncTest.php +++ b/tests/Http/Request/GraphRequestSyncTest.php @@ -10,6 +10,7 @@ use GuzzleHttp\Psr7\Stream; +use Microsoft\Graph\Exception\GraphServiceException; use Microsoft\Graph\Http\GraphResponse; use Microsoft\Graph\Test\Http\TestModel; use Microsoft\Graph\Test\TestData\Model\User; @@ -54,10 +55,10 @@ public function testExecuteWithoutReturnTypeReturnsGraphResponseForEmptyPayload( $this->assertInstanceOf(GraphResponse::class, $response); } - public function testExecuteWithoutReturnTypeReturnsGraphResponseForErrorPayload(): void { + public function testExecuteWithoutReturnTypeThrowsExceptionForErrorPayload(): void { + $this->expectException(GraphServiceException::class); MockHttpClientResponseConfig::configureWithErrorPayload($this->mockHttpClient); $response = $this->defaultGraphRequest->execute(); - $this->assertInstanceOf(GraphResponse::class, $response); } public function testExecuteWithoutReturnTypeReturnsGraphResponseForStreamPayload(): void { @@ -84,10 +85,10 @@ public function testExecuteWithModelReturnTypeReturnsModelForEmptyPayload(): voi $this->assertInstanceOf(User::class, $response); } - public function testExecuteWithModelReturnTypeReturnsModelForErrorPayload(): void { + public function testExecuteWithModelReturnTypeThrowsExceptionForErrorPayload(): void { + $this->expectException(GraphServiceException::class); MockHttpClientResponseConfig::configureWithErrorPayload($this->mockHttpClient); $response = $this->defaultGraphRequest->setReturnType(User::class)->execute(); - $this->assertInstanceOf(User::class, $response); } public function testExecuteWithModelReturnTypeReturnsModelForStreamPayload(): void { @@ -116,10 +117,10 @@ public function testExecuteWithStreamReturnTypeReturnsStreamForEmptyPayload(): v $this->assertInstanceOf(StreamInterface::class, $response); } - public function testExecuteWithStreamReturnTypeReturnsStreamForErrorPayload(): void { + public function testExecuteWithStreamReturnTypeThrowsExceptionForErrorPayload(): void { + $this->expectException(GraphServiceException::class); MockHttpClientResponseConfig::configureWithErrorPayload($this->mockHttpClient); $response = $this->defaultGraphRequest->setReturnType(StreamInterface::class)->execute(); - $this->assertInstanceOf(StreamInterface::class, $response); } public function testExecuteWithStreamReturnTypeReturnsStreamForStreamPayload(): void { From 5ce1066f5b42d2d7da3ae1e99032d4b9466853a9 Mon Sep 17 00:00:00 2001 From: Philip Gichuhi Date: Tue, 31 Aug 2021 10:23:53 +0300 Subject: [PATCH 4/7] Address PR feedback --- src/Exception/BaseError.php | 55 +++++++++ src/Exception/GraphError.php | 66 +++++++++++ src/Exception/GraphServiceException.php | 40 +++++-- src/Exception/ODataError.php | 86 ++++++++++++++ src/Http/GraphError.php | 105 ------------------ src/Http/GraphRequest.php | 2 +- tests/Exception/GraphServiceExceptionTest.php | 51 ++++++--- tests/Http/GraphErrorTest.php | 87 --------------- 8 files changed, 278 insertions(+), 214 deletions(-) create mode 100644 src/Exception/BaseError.php create mode 100644 src/Exception/GraphError.php create mode 100644 src/Exception/ODataError.php delete mode 100644 src/Http/GraphError.php delete mode 100644 tests/Http/GraphErrorTest.php diff --git a/src/Exception/BaseError.php b/src/Exception/BaseError.php new file mode 100644 index 00000000..0eddfeb9 --- /dev/null +++ b/src/Exception/BaseError.php @@ -0,0 +1,55 @@ +propDict = $propDict; + } + + /** + * Returns all properties passed to the constructor + * + * @return array + */ + public function getProperties(): array { + return $this->propDict; + } + + /** + * Returns value of $property in $propDict + * + * @param $property + * @return mixed|null + */ + protected function getProperty($property) { + if (array_key_exists($property, $this->propDict)) { + return $this->propDict[$property]; + } + return null; + } +} diff --git a/src/Exception/GraphError.php b/src/Exception/GraphError.php new file mode 100644 index 00000000..c7eee11d --- /dev/null +++ b/src/Exception/GraphError.php @@ -0,0 +1,66 @@ +getProperty("date"); + } + + /** + * Returns the client request id + * + * @return string|null + */ + public function getClientRequestId(): ?string { + return $this->getProperty("client-request-id"); + } + + /** + * Returns the request id + * + * @return string|null + */ + public function getRequestId(): ?string { + return $this->getProperty("request-id"); + } + + /** + * Returns error as a string + * + * @return string + */ + public function __toString(): string { + $errorString = ($this->getDate()) ? "Date: ".$this->getDate() : ""; + $errorString .= ($this->getClientRequestId()) ? "\nClient Request Id: ".$this->getClientRequestId() : ""; + $errorString .= ($this->getRequestId()) ? "\nRequest Id: ".$this->getRequestId() : ""; + $errorString .= ($this->getCode()) ? "\nCode: ".$this->getCode() : ""; + $errorString .= ($this->getMessage()) ? "\nMessage: ".$this->getMessage() : ""; + $errorString .= ($this->getTarget()) ? "\nTarget: ".$this->getTarget() : ""; + $errorString .= ($this->getDetails()) ? "\nDetails: ".$this->getDetails() : ""; + $errorString .= ($this->getInnerError()) ? "\nInner Error: ".$this->getInnerError() : ""; + return $errorString; + } +} diff --git a/src/Exception/GraphServiceException.php b/src/Exception/GraphServiceException.php index 11644744..78758876 100644 --- a/src/Exception/GraphServiceException.php +++ b/src/Exception/GraphServiceException.php @@ -8,9 +8,7 @@ namespace Microsoft\Graph\Exception; -use Microsoft\Graph\Http\GraphError; use Microsoft\Graph\Http\GraphRequest; -use Microsoft\Graph\Http\GraphRequestUtil; /** * Class GraphServiceException @@ -74,7 +72,7 @@ public function __construct( /** * Returns HTTP headers in the response from the Graph * - * @return array + * @return array */ public function getResponseHeaders(): array { return $this->responseHeaders; @@ -99,13 +97,13 @@ public function getRawResponseBody(): array { } /** - * Returns the error object of the payload as a model + * Returns the error object of the payload * - * @return GraphError|null + * @return ODataError|null */ - public function getError(): ?GraphError { + public function getError(): ?ODataError { if (array_key_exists("error", $this->responseBody)) { - return new GraphError($this->responseBody["error"]); + return new ODataError($this->responseBody["error"]); } return null; } @@ -118,4 +116,32 @@ public function getError(): ?GraphError { public function getRequest(): GraphRequest { return $this->graphRequest; } + + /** + * Returns the client request Id + * + * @return string|null + */ + public function getClientRequestId(): ?string { + $headerName = "client-request-id"; + if (array_key_exists($headerName, $this->responseHeaders) + && !empty($this->responseHeaders[$headerName])) { + return $this->responseHeaders[$headerName][0]; + } + return null; + } + + /** + * Returns the request Id + * + * @return string|null + */ + public function getRequestId(): ?string { + $headerName = "request-id"; + if (array_key_exists($headerName, $this->responseHeaders) + && !empty($this->responseHeaders[$headerName])) { + return $this->responseHeaders[$headerName][0]; + } + return null; + } } diff --git a/src/Exception/ODataError.php b/src/Exception/ODataError.php new file mode 100644 index 00000000..cfc85080 --- /dev/null +++ b/src/Exception/ODataError.php @@ -0,0 +1,86 @@ +getProperty("code"); + } + + /** + * Get error message returned by the Graph + * + * @return string|null + */ + public function getMessage(): ?string { + return $this->getProperty("message"); + } + + /** + * Returns the target of the error + * + * @return string|null + */ + public function getTarget(): ?string { + return $this->getProperty("target"); + } + + /** + * Returns the error details containing a code, message and target + * + * @return ODataError[]|null + */ + public function getDetails(): ?array { + $details = $this->getProperty("details"); + if ($details) { + return array_map(function ($detail) { return new ODataError($detail); }, $details); + } + return null; + } + + /** + * Get the Graph-specific error info + * + * @return GraphError|null + */ + public function getInnerError(): ?GraphError { + $innerError = $this->getProperty("innerError"); + return ($innerError) ? new GraphError($innerError) : null; + } + + /** + * Returns error as a string + * + * @return string + */ + public function __toString(): string { + $errorString = ($this->getCode()) ? "Code: ".$this->getCode() : ""; + $errorString .= ($this->getMessage()) ? "\nMessage: ".$this->getMessage() : ""; + $errorString .= ($this->getTarget()) ? "\nTarget: ".$this->getTarget() : ""; + $errorString .= ($this->getDetails()) ? "\nDetails: ".$this->getDetails() : ""; + $errorString .= ($this->getInnerError()) ? "\nInner Error: ".$this->getInnerError() : ""; + return $errorString; + } +} diff --git a/src/Http/GraphError.php b/src/Http/GraphError.php deleted file mode 100644 index 9125e0a0..00000000 --- a/src/Http/GraphError.php +++ /dev/null @@ -1,105 +0,0 @@ -propDict = $propDict; - } - - /** - * Get error code returned by the Graph - * - * @return string|null - */ - public function getCode(): ?string { - if (array_key_exists("code", $this->propDict)) { - return $this->propDict["code"]; - } - return null; - } - - /** - * Get error message returned by the Graph - * - * @return string|null - */ - public function getMessage(): ?string { - if (array_key_exists("message", $this->propDict)) { - return $this->propDict["message"]; - } - return null; - } - - /** - * Get the additional error info - * - * @return GraphError|null - */ - public function getInnerError(): ?GraphError { - if (array_key_exists("innerError", $this->propDict)) { - return new GraphError($this->propDict["innerError"]); - } - return null; - } - - /** - * Returns the client request Id - * - * @return string|null - */ - public function getClientRequestId(): ?string { - if (array_key_exists("client-request-id", $this->propDict)) { - return $this->propDict["client-request-id"]; - } - return null; - } - - /** - * Returns the request Id - * - * @return string|null - */ - public function getRequestId(): ?string { - if (array_key_exists("request-id", $this->propDict)) { - return $this->propDict["request-id"]; - } - return null; - } - - /** - * Returns the date of the request - * - * @return string|null - */ - public function getDate(): ?string { - if (array_key_exists("date", $this->propDict)) { - return $this->propDict["date"]; - } - return null; - } - - /** - * Returns all properties passed to the constructor - * - * @return array - */ - public function getProperties(): array { - return $this->propDict; - } -} diff --git a/src/Http/GraphRequest.php b/src/Http/GraphRequest.php index 448f39db..88d14c6e 100644 --- a/src/Http/GraphRequest.php +++ b/src/Http/GraphRequest.php @@ -127,7 +127,7 @@ public function getRequestUri(): UriInterface { } /** - * ReturnS the HTTP method used + * Returns the HTTP method used * * @return string */ diff --git a/tests/Exception/GraphServiceExceptionTest.php b/tests/Exception/GraphServiceExceptionTest.php index 025734d6..78c0193e 100644 --- a/tests/Exception/GraphServiceExceptionTest.php +++ b/tests/Exception/GraphServiceExceptionTest.php @@ -9,32 +9,33 @@ namespace Microsoft\Graph\Test\Exception; +use GuzzleHttp\Psr7\Response; use Microsoft\Graph\Exception\GraphServiceException; -use Microsoft\Graph\Http\GraphError; +use Microsoft\Graph\Exception\ODataError; use Microsoft\Graph\Http\GraphRequest; use Microsoft\Graph\Test\Http\SampleGraphResponsePayload; class GraphServiceExceptionTest extends \PHPUnit\Framework\TestCase { private $mockGraphRequest; - private $responseStatusCode; + private $responseStatusCode = 404; private $responseBody = SampleGraphResponsePayload::ERROR_PAYLOAD; - private $responseHeaders; + private $responseHeaders = [ + "Content-Type" => "application/json", + "request-id" => "2f91ee0a-e013-425b-a5bf-b1f251163969", + "client-request-id" => "2f91ee0a-e013-425b-a5bf-b1f251163969", + ]; private $defaultException; + private $psr7Response; protected function setUp(): void { $this->mockGraphRequest = $this->createMock(GraphRequest::class); - $this->responseStatusCode = 404; - $this->responseHeaders = [ - "Content-Type" => "application/json", - "request-id" => "2f91ee0a-e013-425b-a5bf-b1f251163969", - "client-request-id" => "2f91ee0a-e013-425b-a5bf-b1f251163969", - ]; + $this->psr7Response = new Response($this->responseStatusCode, $this->responseHeaders, json_encode($this->responseBody)); $this->defaultException = new GraphServiceException( $this->mockGraphRequest, - $this->responseStatusCode, - $this->responseBody, - $this->responseHeaders + $this->psr7Response->getStatusCode(), + json_decode($this->psr7Response->getBody(), true), + $this->psr7Response->getHeaders() ); } @@ -46,7 +47,29 @@ public function testGetters(): void { $this->assertEquals($this->mockGraphRequest, $this->defaultException->getRequest()); $this->assertEquals($this->responseStatusCode, $this->defaultException->getResponseStatusCode()); $this->assertEquals($this->responseBody, $this->defaultException->getRawResponseBody()); - $this->assertEquals($this->responseHeaders, $this->defaultException->getResponseHeaders()); - $this->assertInstanceOf(GraphError::class, $this->defaultException->getError()); + $this->assertEquals($this->psr7Response->getHeaders(), $this->defaultException->getResponseHeaders()); + $this->assertInstanceOf(ODataError::class, $this->defaultException->getError()); + } + + public function testGetClientRequestId(): void { + $headerName = "client-request-id"; + $this->assertEquals( + $this->responseHeaders[$headerName], + $this->defaultException->getClientRequestId() + ); + unset($this->responseHeaders[$headerName]); + $this->setUp(); + $this->assertNull($this->defaultException->getClientRequestId()); + } + + public function testGetRequestId(): void { + $headerName = "request-id"; + $this->assertEquals( + $this->responseHeaders[$headerName], + $this->defaultException->getRequestId() + ); + unset($this->responseHeaders[$headerName]); + $this->setUp(); + $this->assertNull($this->defaultException->getRequestId()); } } diff --git a/tests/Http/GraphErrorTest.php b/tests/Http/GraphErrorTest.php deleted file mode 100644 index 7c008bf5..00000000 --- a/tests/Http/GraphErrorTest.php +++ /dev/null @@ -1,87 +0,0 @@ -defaultGraphError = new GraphError(SampleGraphResponsePayload::ERROR_PAYLOAD["error"]); - } - - public function testGetCode(): void { - $this->assertEquals( - SampleGraphResponsePayload::ERROR_PAYLOAD["error"]["code"], - $this->defaultGraphError->getCode() - ); - - $graphError = $this->unsetProperty("code"); - $this->assertNull($graphError->getCode()); - } - - public function testGetMessage(): void { - $this->assertEquals( - SampleGraphResponsePayload::ERROR_PAYLOAD["error"]["message"], - $this->defaultGraphError->getMessage() - ); - $graphError = $this->unsetProperty("message"); - $this->assertNull($graphError->getMessage()); - } - - public function testGetInnerError(): void { - $this->assertInstanceOf(GraphError::class, $this->defaultGraphError->getInnerError()); - $graphError = $this->unsetProperty("innerError"); - $this->assertNull($graphError->getInnerError()); - } - - public function testGetClientRequestId(): void { - $this->assertNull($this->defaultGraphError->getClientRequestId()); - $this->assertEquals( - SampleGraphResponsePayload::ERROR_PAYLOAD["error"]["innerError"]["client-request-id"], - $this->defaultGraphError->getInnerError()->getClientRequestId() - ); - $graphError = $this->unsetProperty("client-request-id", SampleGraphResponsePayload::ERROR_PAYLOAD["error"]["innerError"]); - $this->assertNull($graphError->getClientRequestId()); - } - - public function testGetRequestId(): void { - $this->assertNull($this->defaultGraphError->getRequestId()); - $this->assertEquals( - SampleGraphResponsePayload::ERROR_PAYLOAD["error"]["innerError"]["request-id"], - $this->defaultGraphError->getInnerError()->getRequestId() - ); - $graphError = $this->unsetProperty("request-id", SampleGraphResponsePayload::ERROR_PAYLOAD["error"]["innerError"]); - $this->assertNull($graphError->getRequestId()); - } - - public function testGetDate(): void { - $this->assertNull($this->defaultGraphError->getDate()); - $this->assertEquals( - SampleGraphResponsePayload::ERROR_PAYLOAD["error"]["innerError"]["date"], - $this->defaultGraphError->getInnerError()->getDate() - ); - $graphError = $this->unsetProperty("date", SampleGraphResponsePayload::ERROR_PAYLOAD["error"]["innerError"]); - $this->assertNull($graphError->getDate()); - } - - public function testGetProperties(): void { - $this->assertEquals(SampleGraphResponsePayload::ERROR_PAYLOAD["error"], $this->defaultGraphError->getProperties()); - } - - private function unsetProperty(string $property, - array $payload = SampleGraphResponsePayload::ERROR_PAYLOAD["error"]): GraphError { - unset($payload[$property]); - return new GraphError($payload); - } -} From 74fdb95c3a02cb1fac8eead20f28c91977c965b0 Mon Sep 17 00:00:00 2001 From: Philip Gichuhi Date: Wed, 15 Sep 2021 20:48:51 +0300 Subject: [PATCH 5/7] Use native PHP exception types for validation errors --- src/Exception/GraphServiceException.php | 2 +- src/Http/AbstractGraphClient.php | 6 +- src/Http/GraphCollectionRequest.php | 28 +++------- src/Http/GraphRequest.php | 55 ++++++++----------- src/Http/HttpClientFactory.php | 4 +- src/Task/PageIterator.php | 10 ++-- tests/Http/AbstractGraphClientTest.php | 10 ++-- tests/Http/HttpClientFactoryTest.php | 4 +- .../Request/GraphCollectionRequestTest.php | 7 +-- tests/Http/Request/GraphRequestStreamTest.php | 4 +- tests/Http/Request/GraphRequestSyncTest.php | 2 - tests/Http/Request/GraphRequestTest.php | 12 ++-- 12 files changed, 59 insertions(+), 85 deletions(-) diff --git a/src/Exception/GraphServiceException.php b/src/Exception/GraphServiceException.php index 78758876..77d32e37 100644 --- a/src/Exception/GraphServiceException.php +++ b/src/Exception/GraphServiceException.php @@ -13,7 +13,7 @@ /** * Class GraphServiceException * - * Thrown when the Graph API returns 4xx or 5xx responses + * Thrown when the Graph API returns 5xx responses * * @package Microsoft\Graph\Exception * @copyright 2021 Microsoft Corporation diff --git a/src/Http/AbstractGraphClient.php b/src/Http/AbstractGraphClient.php index 59a34d03..b4916025 100644 --- a/src/Http/AbstractGraphClient.php +++ b/src/Http/AbstractGraphClient.php @@ -50,7 +50,7 @@ abstract class AbstractGraphClient * * @param string|null $nationalCloud if null defaults to "https://graph.microsoft.com" * @param HttpClientInterface|null $httpClient if null creates default Guzzle client - * @throws GraphClientException + * @throws \InvalidArgumentException */ public function __construct(?string $nationalCloud = NationalCloud::GLOBAL, ?HttpClientInterface $httpClient = null) @@ -105,7 +105,7 @@ public function getHttpClient(): HttpClientInterface * * @return GraphRequest The request object, which can be used to * make queries against Graph - * @throws GraphClientException + * @throws \InvalidArgumentException */ public function createRequest(string $requestType, string $endpoint): GraphRequest { @@ -125,7 +125,7 @@ public function createRequest(string $requestType, string $endpoint): GraphReque * * @return GraphCollectionRequest The request object, which can be * used to make queries against Graph - * @throws GraphClientException + * @throws \InvalidArgumentException */ public function createCollectionRequest(string $requestType, string $endpoint): GraphCollectionRequest { diff --git a/src/Http/GraphCollectionRequest.php b/src/Http/GraphCollectionRequest.php index 0155bf9b..fe5493f2 100644 --- a/src/Http/GraphCollectionRequest.php +++ b/src/Http/GraphCollectionRequest.php @@ -10,7 +10,6 @@ use GuzzleHttp\Psr7\Uri; use Microsoft\Graph\Core\GraphConstants; use Microsoft\Graph\Exception\GraphClientException; -use Microsoft\Graph\Exception\GraphException; use Microsoft\Graph\Exception\GraphServiceException; use Microsoft\Graph\Task\PageIterator; use Psr\Http\Client\ClientExceptionInterface; @@ -68,7 +67,7 @@ class GraphCollectionRequest extends GraphRequest * @param string $endpoint The URI of the endpoint to hit * @param AbstractGraphClient $graphClient * @param string $baseUrl (optional) If empty, it's set to $client's national cloud - * @throws GraphClientException + * @throws \InvalidArgumentException */ public function __construct(string $requestType, string $endpoint, AbstractGraphClient $graphClient, string $baseUrl = "") { @@ -85,12 +84,11 @@ public function __construct(string $requestType, string $endpoint, AbstractGraph /** * Gets the number of entries in the collection * - * @return int the number of entries + * @return int|null the number of entries | null if @odata.count doesn't exist for that collection * @throws ClientExceptionInterface - * @throws GraphClientException if @odata.count is not present in the response * @throws GraphServiceException if 4xx or 5xx response is returned */ - public function count(): int + public function count(): ?int { $query = '$count=true'; $requestUri = $this->getRequestUri(); @@ -99,14 +97,8 @@ public function count(): int $this->originalReturnType = $this->returnType; $this->returnType = null; $result = $this->execute(); - - if (is_a($result, GraphResponse::class) && $result->getCount()) { - return $result->getCount(); - } - - /* The $count query parameter for the Graph API - is available on several models but not all */ - throw new GraphClientException('Count unavailable for this collection'); + $this->returnType = $this->originalReturnType; + return ($result->getCount()) ?: null; } /** @@ -115,14 +107,13 @@ public function count(): int * * @param int $pageSize The page size * - * @throws GraphClientException if the requested page size exceeds - * the Graph's defined page size limit + * @throws \InvalidArgumentException if the requested page size exceeds Graph's defined page size limit * @return GraphCollectionRequest object */ public function setPageSize(int $pageSize): self { if ($pageSize > GraphConstants::MAX_PAGE_SIZE) { - throw new GraphClientException(GraphConstants::MAX_PAGE_SIZE_ERROR); + throw new \InvalidArgumentException(GraphConstants::MAX_PAGE_SIZE_ERROR); } $this->pageSize = $pageSize; return $this; @@ -146,9 +137,9 @@ public function getPage() /** * Sets the required query information to get a new page * - * @return GraphCollectionRequest + * @return void */ - private function setPageCallInfo(): self + private function setPageCallInfo(): void { // Store these to add temporary query data to request $this->originalReturnType = $this->returnType; @@ -171,7 +162,6 @@ private function setPageCallInfo(): self $this->setRequestUri(new Uri( $requestUri . GraphRequestUtil::getQueryParamConcatenator($requestUri) . $query)); } } - return $this; } /** diff --git a/src/Http/GraphRequest.php b/src/Http/GraphRequest.php index 88d14c6e..28b1c67f 100644 --- a/src/Http/GraphRequest.php +++ b/src/Http/GraphRequest.php @@ -89,15 +89,15 @@ class GraphRequest * @param string $endpoint The url path on the host to be called- * @param AbstractGraphClient $graphClient The Graph client to use * @param string $baseUrl (optional) If empty, it's set to $client's national cloud - * @throws GraphClientException + * @throws \InvalidArgumentException */ public function __construct(string $requestType, string $endpoint, AbstractGraphClient $graphClient, string $baseUrl = "") { if (!$requestType || !$endpoint || !$graphClient) { - throw new GraphClientException("Request type, endpoint and client cannot be null or empty"); + throw new \InvalidArgumentException("Request type, endpoint and client cannot be null or empty"); } if (!$graphClient->getAccessToken()) { - throw new GraphClientException(GraphConstants::NO_ACCESS_TOKEN); + throw new \InvalidArgumentException(GraphConstants::NO_ACCESS_TOKEN); } $this->requestType = $requestType; $this->graphClient = $graphClient; @@ -156,12 +156,12 @@ public function setAccessToken(string $accessToken): self * @param string $returnClass The class name to use * * @return $this object - * @throws GraphClientException when $returnClass is not an existing class + * @throws \InvalidArgumentException when $returnClass is not an existing class */ public function setReturnType(string $returnClass): self { if (!class_exists($returnClass) && !interface_exists($returnClass)) { - throw new GraphClientException("Return type specified does not match an existing class definition"); + throw new \InvalidArgumentException("Return type specified does not match an existing class definition"); } $this->returnType = $returnClass; $this->returnsStream = ($returnClass === StreamInterface::class); @@ -174,12 +174,12 @@ public function setReturnType(string $returnClass): self * @param array $headers An array of custom headers * * @return GraphRequest object - * @throws GraphClientException if attempting to overwrite SdkVersion header + * @throws \InvalidArgumentException if attempting to overwrite SdkVersion header */ public function addHeaders(array $headers): self { if (array_key_exists("SdkVersion", $headers)) { - throw new GraphClientException("Cannot overwrite SdkVersion header"); + throw new \InvalidArgumentException("Cannot overwrite SdkVersion header"); } // Recursive merge to support appending values to multi-value headers $this->headers = array_merge_recursive($this->headers, $headers); @@ -320,7 +320,7 @@ function ($reason) { * @param string $path path to download the file contents to * @param ClientInterface|null $client (optional) When null, defaults to $graphClient's http client * @throws ClientExceptionInterface - * @throws GraphClientException when unable to open $path for writing + * @throws \RuntimeException when unable to open $path for writing * @throws GraphServiceException if 4xx or 5xx response is returned. Error payload is accessible from the exception */ public function download(string $path, ?ClientInterface $client = null): void @@ -328,16 +328,13 @@ public function download(string $path, ?ClientInterface $client = null): void if (is_null($client)) { $client = $this->graphClient->getHttpClient(); } - try { - $resource = Utils::tryFopen($path, 'w'); - $stream = Utils::streamFor($resource); - $response = $client->sendRequest($this->httpRequest); - $this->handleErrorResponse($response); - $stream->write($response->getBody()->getContents()); - $stream->close(); - } catch (\RuntimeException $ex) { - throw new GraphClientException(GraphConstants::INVALID_FILE, $ex->getCode(), $ex); - } + + $resource = Utils::tryFopen($path, 'w'); + $stream = Utils::streamFor($resource); + $response = $client->sendRequest($this->httpRequest); + $this->handleErrorResponse($response); + $stream->write($response->getBody()->getContents()); + $stream->close(); } /** @@ -347,7 +344,7 @@ public function download(string $path, ?ClientInterface $client = null): void * @param ClientInterface|null $client (optional) * @return array|GraphResponse|StreamInterface|object Graph Response object or response body cast to $returnType * @throws ClientExceptionInterface - * @throws GraphClientException if $path cannot be opened for reading + * @throws \RuntimeException if $path cannot be opened for reading * @throws GraphServiceException if 4xx or 5xx response is returned. Error payload is accessible from the exception */ public function upload(string $path, ?ClientInterface $client = null) @@ -355,14 +352,10 @@ public function upload(string $path, ?ClientInterface $client = null) if (is_null($client)) { $client = $this->graphClient->getHttpClient(); } - try { - $resource = Utils::tryFopen($path, 'r'); - $stream = Utils::streamFor($resource); - $this->attachBody($stream); - return $this->execute($client); - } catch(\RuntimeException $e) { - throw new GraphClientException(GraphConstants::INVALID_FILE, $e->getCode(), $e->getPrevious()); - } + $resource = Utils::tryFopen($path, 'r'); + $stream = Utils::streamFor($resource); + $this->attachBody($stream); + return $this->execute($client); } /** @@ -394,14 +387,10 @@ private function initHeaders(): void * * @param string $baseUrl * @param string $endpoint - * @throws GraphClientException + * @throws \InvalidArgumentException */ protected function initRequestUri(string $baseUrl, string $endpoint): void { - try { - $this->requestUri = GraphRequestUtil::getRequestUri($baseUrl, $endpoint, $this->graphClient->getApiVersion()); - } catch (\InvalidArgumentException $ex) { - throw new GraphClientException($ex->getMessage(), 0, $ex); - } + $this->requestUri = GraphRequestUtil::getRequestUri($baseUrl, $endpoint, $this->graphClient->getApiVersion()); } protected function initPsr7HttpRequest(): void { diff --git a/src/Http/HttpClientFactory.php b/src/Http/HttpClientFactory.php index 37bbd81e..ee5d38b1 100644 --- a/src/Http/HttpClientFactory.php +++ b/src/Http/HttpClientFactory.php @@ -75,11 +75,11 @@ private static function getInstance(): HttpClientFactory { * * @param string $nationalCloud * @return $this - * @throws GraphClientException if $nationalCloud is empty or an invalid national cloud Host + * @throws \InvalidArgumentException if $nationalCloud is empty or an invalid national cloud Host */ public static function setNationalCloud(string $nationalCloud = NationalCloud::GLOBAL): HttpClientFactory { if (!$nationalCloud || !NationalCloud::containsNationalCloudHost($nationalCloud)) { - throw new GraphClientException("Invalid national cloud passed. See https://docs.microsoft.com/en-us/graph/deployments#microsoft-graph-and-graph-explorer-service-root-endpoints."); + throw new \InvalidArgumentException("Invalid national cloud passed. See https://docs.microsoft.com/en-us/graph/deployments#microsoft-graph-and-graph-explorer-service-root-endpoints."); } self::$nationalCloud = $nationalCloud; return self::getInstance(); diff --git a/src/Task/PageIterator.php b/src/Task/PageIterator.php index ed6609c6..16045140 100644 --- a/src/Task/PageIterator.php +++ b/src/Task/PageIterator.php @@ -11,6 +11,7 @@ use Http\Promise\FulfilledPromise; use Http\Promise\Promise; use Microsoft\Graph\Exception\GraphClientException; +use Microsoft\Graph\Exception\GraphServiceException; use Microsoft\Graph\Http\AbstractGraphClient; use Microsoft\Graph\Http\GraphResponse; use Microsoft\Graph\Http\RequestOptions; @@ -117,7 +118,7 @@ public function __construct(AbstractGraphClient $graphClient, * * @return Promise that resolves to true on completion and throws error on rejection * - * @throws \Exception if promise is rejected + * @throws ClientExceptionInterface|GraphClientException|GraphServiceException if promise is rejected */ public function iterate(): Promise { $promise = new FulfilledPromise(false); @@ -155,7 +156,7 @@ public function iterate(): Promise { * Resume iteration after $callback returning false * * @return Promise - * @throws \Exception if promise is rejected + * @throws ClientExceptionInterface|GraphClientException|GraphServiceException if promise is rejected */ public function resume(): Promise { return $this->iterate(); @@ -204,8 +205,8 @@ public function getNextLink(): ?string { /** * Fetches the next page of results * - * @throws GraphClientException * @throws ClientExceptionInterface + * @throws GraphServiceException */ private function getNextPage(): void { $nextLink = $this->getNextLink(); @@ -214,7 +215,7 @@ private function getNextPage(): void { $request = $request->addHeaders($this->requestOptions->getHeaders()); } $this->collectionResponse = $request->execute(); - if (!$this->collectionResponse || empty($this->collectionResponse->getBody())) { + if (!$this->collectionResponse || empty($this->collectionResponse->getBody()) || !array_key_exists("value", $this->collectionResponse->getBody())) { $this->entityCollection = []; return; } @@ -223,7 +224,6 @@ private function getNextPage(): void { $this->entityCollection = $this->collectionResponse->getBody()["value"]; return; } - throw new GraphClientException("No 'value' attribute found in payload after requesting ".$nextLink); } $this->entityCollection = $this->collectionResponse->getResponseAsObject($this->returnType); } diff --git a/tests/Http/AbstractGraphClientTest.php b/tests/Http/AbstractGraphClientTest.php index e7d7f0ae..c59b3b19 100644 --- a/tests/Http/AbstractGraphClientTest.php +++ b/tests/Http/AbstractGraphClientTest.php @@ -41,7 +41,7 @@ public function testConstructorWithNullParams() { } public function testConstructorWithInvalidNationalCloud() { - $this->expectException(GraphClientException::class); + $this->expectException(\InvalidArgumentException::class); $graphClient = $this->getMockBuilder(AbstractGraphClient::class) ->setConstructorArgs(["https://www.microsoft.com", null]) ->getMockForAbstractClass(); @@ -61,12 +61,12 @@ public function testCreateRequestReturnsGraphRequest() { } public function testCreateRequestWithoutSettingAccessTokenThrowsException() { - $this->expectException(GraphClientException::class); + $this->expectException(\InvalidArgumentException::class); $request = $this->defaultGraphClient->createRequest("GET", "/"); } public function testCreateRequestWithInvalidParamsThrowsException() { - $this->expectException(GraphClientException::class); + $this->expectException(\InvalidArgumentException::class); $this->defaultGraphClient->createRequest("", ""); } @@ -77,12 +77,12 @@ public function testCreateCollectionRequestReturnsGraphCollectionRequest() { } public function testCreateCollectionRequestWithoutAccessTokenThrowsException() { - $this->expectException(GraphClientException::class); + $this->expectException(\InvalidArgumentException::class); $request = $this->defaultGraphClient->createCollectionRequest("GET", "/me/users"); } public function testCreateCollectionRequestWithInvalidParamsThrowsException() { - $this->expectException(GraphClientException::class); + $this->expectException(\InvalidArgumentException::class); $this->defaultGraphClient->createCollectionRequest("", ""); } } diff --git a/tests/Http/HttpClientFactoryTest.php b/tests/Http/HttpClientFactoryTest.php index 9af63608..4803dbbb 100644 --- a/tests/Http/HttpClientFactoryTest.php +++ b/tests/Http/HttpClientFactoryTest.php @@ -12,12 +12,12 @@ class HttpClientFactoryTest extends \PHPUnit\Framework\TestCase { function testNationalCloudWithEmptyString() { - $this->expectException(GraphClientException::class); + $this->expectException(\InvalidArgumentException::class); HttpClientFactory::setNationalCloud(""); } function testNationalCloudWithInvalidUrl() { - $this->expectException(GraphClientException::class); + $this->expectException(\InvalidArgumentException::class); HttpClientFactory::setNationalCloud("https://www.microsoft.com"); } diff --git a/tests/Http/Request/GraphCollectionRequestTest.php b/tests/Http/Request/GraphCollectionRequestTest.php index afe0d047..d26d4c60 100644 --- a/tests/Http/Request/GraphCollectionRequestTest.php +++ b/tests/Http/Request/GraphCollectionRequestTest.php @@ -30,7 +30,7 @@ public function testSetPageSizeReturnsInstance(): void { } public function testSetPageSizeExceedingMaxSizeThrowsException(): void { - $this->expectException(GraphClientException::class); + $this->expectException(\InvalidArgumentException::class); $this->defaultCollectionRequest->setPageSize(GraphConstants::MAX_PAGE_SIZE + 1); } @@ -74,10 +74,9 @@ public function testCount(): void { $this->assertEquals(SampleGraphResponsePayload::COLLECTION_PAYLOAD["@odata.count"], $count); } - public function testCountThrowsErrorIfNoOdataCountFound(): void { - $this->expectException(GraphException::class); + public function testCountReturnsNullIfNoOdataCountFound(): void { MockHttpClientResponseConfig::configureWithEmptyPayload($this->mockHttpClient); - $count = $this->defaultCollectionRequest->count(); + $this->assertNull($this->defaultCollectionRequest->count()); } public function testCountThrowsExceptionOnErrorResponse(): void { diff --git a/tests/Http/Request/GraphRequestStreamTest.php b/tests/Http/Request/GraphRequestStreamTest.php index ab1c8723..bfce97ed 100644 --- a/tests/Http/Request/GraphRequestStreamTest.php +++ b/tests/Http/Request/GraphRequestStreamTest.php @@ -30,7 +30,7 @@ public function testUpload() public function testInvalidUpload() { - $this->expectException(GraphClientException::class); + $this->expectException(\RuntimeException::class); $file = new VfsStreamFile('foo.txt', 0000); $this->rootDir->addChild($file); $this->defaultGraphRequest->upload($file->url()); @@ -58,7 +58,7 @@ public function testDownload() public function testInvalidDownload() { - $this->expectException(GraphClientException::class); + $this->expectException(\RuntimeException::class); $file = new VfsStreamFile('foo.txt', 0000); $this->rootDir->addChild($file); $this->defaultGraphRequest->download($file->url()); diff --git a/tests/Http/Request/GraphRequestSyncTest.php b/tests/Http/Request/GraphRequestSyncTest.php index a9ccbf54..7a8cc611 100644 --- a/tests/Http/Request/GraphRequestSyncTest.php +++ b/tests/Http/Request/GraphRequestSyncTest.php @@ -9,10 +9,8 @@ namespace Microsoft\Graph\Test\Http\Request; -use GuzzleHttp\Psr7\Stream; use Microsoft\Graph\Exception\GraphServiceException; use Microsoft\Graph\Http\GraphResponse; -use Microsoft\Graph\Test\Http\TestModel; use Microsoft\Graph\Test\TestData\Model\User; use Psr\Http\Client\ClientExceptionInterface; use Psr\Http\Client\ClientInterface; diff --git a/tests/Http/Request/GraphRequestTest.php b/tests/Http/Request/GraphRequestTest.php index dfd91e57..7fc15fc3 100644 --- a/tests/Http/Request/GraphRequestTest.php +++ b/tests/Http/Request/GraphRequestTest.php @@ -4,10 +4,8 @@ use Microsoft\Graph\Core\GraphConstants; use Microsoft\Graph\Core\NationalCloud; -use Microsoft\Graph\Exception\GraphClientException; use Microsoft\Graph\Http\AbstractGraphClient; use Microsoft\Graph\Http\GraphRequest; -use Microsoft\Graph\Test\Http\TestModel; use Microsoft\Graph\Test\TestData\Model\User; class GraphRequestTest extends BaseGraphRequestTest @@ -18,18 +16,18 @@ public function testConstructorWithNullParametersThrowsException(): void { } public function testConstructorWithEmptyParametersThrowsException(): void { - $this->expectException(GraphClientException::class); + $this->expectException(\InvalidArgumentException::class); $request = new GraphRequest("", "", $this->mockGraphClient); } public function testConstructorWithoutAccessTokenThrowsException(): void { $graphClient = $this->getMockForAbstractClass(AbstractGraphClient::class); - $this->expectException(GraphClientException::class); + $this->expectException(\InvalidArgumentException::class); $request = new GraphRequest("GET", "/me", $graphClient); } public function testConstructorWithInvalidCustomBaseUrlThrowsException(): void { - $this->expectException(GraphClientException::class); + $this->expectException(\InvalidArgumentException::class); $baseUrl = "www.outlook.com"; # no scheme $request = new GraphRequest("GET", "/me", $this->mockGraphClient, $baseUrl); } @@ -162,7 +160,7 @@ public function testSetReturnTypeReturnsGraphRequestInstance(): void { } public function testSetReturnTypeWithInvalidClassThrowsException(): void { - $this->expectException(GraphClientException::class); + $this->expectException(\InvalidArgumentException::class); $this->defaultGraphRequest->setReturnType("Model\User"); } @@ -175,7 +173,7 @@ public function testAddHeadersReturnsGraphRequestInstance(): void { } public function testAddHeadersCannotAppendOrOverwriteSdkVersionValue(): void { - $this->expectException(GraphClientException::class); + $this->expectException(\InvalidArgumentException::class); $this->defaultGraphRequest->addHeaders([ 'SdkVersion' => 'Version1', 'Content-Encoding' => 'gzip' From 8eb7c806675bfc649df1d492ff3fa975292ded7e Mon Sep 17 00:00:00 2001 From: Philip Gichuhi Date: Thu, 16 Sep 2021 06:58:05 +0300 Subject: [PATCH 6/7] Throw GraphClientException for 4xx responses and GraphServiceException for 5xx responses --- .../{BaseError.php => BaseErrorContent.php} | 2 +- src/Exception/GraphClientException.php | 4 +- .../{GraphError.php => GraphErrorContent.php} | 2 +- src/Exception/GraphException.php | 51 ------ src/Exception/GraphResponseException.php | 147 ++++++++++++++++++ src/Exception/GraphServiceException.php | 127 +-------------- .../{ODataError.php => ODataErrorContent.php} | 12 +- src/Http/GraphCollectionRequest.php | 21 +-- src/Http/GraphRequest.php | 52 +++++-- src/Task/PageIterator.php | 17 +- tests/Exception/GraphExceptionTest.php | 21 --- ...est.php => GraphResponseExceptionTest.php} | 10 +- .../Request/GraphCollectionRequestTest.php | 25 ++- tests/Http/Request/GraphRequestAsyncTest.php | 16 +- tests/Http/Request/GraphRequestStreamTest.php | 27 +++- tests/Http/Request/GraphRequestSyncTest.php | 31 +++- tests/Task/PageIteratorTest.php | 4 +- 17 files changed, 308 insertions(+), 261 deletions(-) rename src/Exception/{BaseError.php => BaseErrorContent.php} (98%) rename src/Exception/{GraphError.php => GraphErrorContent.php} (97%) delete mode 100644 src/Exception/GraphException.php create mode 100644 src/Exception/GraphResponseException.php rename src/Exception/{ODataError.php => ODataErrorContent.php} (87%) delete mode 100644 tests/Exception/GraphExceptionTest.php rename tests/Exception/{GraphServiceExceptionTest.php => GraphResponseExceptionTest.php} (88%) diff --git a/src/Exception/BaseError.php b/src/Exception/BaseErrorContent.php similarity index 98% rename from src/Exception/BaseError.php rename to src/Exception/BaseErrorContent.php index 0eddfeb9..90b7e7ba 100644 --- a/src/Exception/BaseError.php +++ b/src/Exception/BaseErrorContent.php @@ -18,7 +18,7 @@ * @license https://opensource.org/licenses/MIT MIT License * @link https://developer.microsoft.com/graph */ -class BaseError +class BaseErrorContent { /** * Contains all properties of the error response diff --git a/src/Exception/GraphClientException.php b/src/Exception/GraphClientException.php index 15ac3fdf..a0fba943 100644 --- a/src/Exception/GraphClientException.php +++ b/src/Exception/GraphClientException.php @@ -10,14 +10,14 @@ /** * Class GraphClientException * - * Thrown when there are SDK-level errors (mostly validation errors) + * Thrown when the Graph API returns 4xx responses * * @package Microsoft\Graph\Exception * @copyright 2021 Microsoft Corporation * @license https://opensource.org/licenses/MIT MIT License * @link https://developer.microsoft.com/graph */ -class GraphClientException extends GraphException +class GraphClientException extends GraphResponseException { } diff --git a/src/Exception/GraphError.php b/src/Exception/GraphErrorContent.php similarity index 97% rename from src/Exception/GraphError.php rename to src/Exception/GraphErrorContent.php index c7eee11d..3840f813 100644 --- a/src/Exception/GraphError.php +++ b/src/Exception/GraphErrorContent.php @@ -18,7 +18,7 @@ * @license https://opensource.org/licenses/MIT MIT License * @link https://developer.microsoft.com/graph */ -class GraphError extends ODataError +class GraphErrorContent extends ODataErrorContent { /** * Returns the date of the request diff --git a/src/Exception/GraphException.php b/src/Exception/GraphException.php deleted file mode 100644 index 61e1ce49..00000000 --- a/src/Exception/GraphException.php +++ /dev/null @@ -1,51 +0,0 @@ -code}]: {$this->message}\n"; - } -} diff --git a/src/Exception/GraphResponseException.php b/src/Exception/GraphResponseException.php new file mode 100644 index 00000000..c534e0a8 --- /dev/null +++ b/src/Exception/GraphResponseException.php @@ -0,0 +1,147 @@ +graphRequest = $graphRequest; + $this->responseStatusCode = $responseStatusCode; + $this->responseBody = $responseBody; + $this->responseHeaders = $responseHeaders; + $message = "'".$graphRequest->getRequestType()."' request to ".$graphRequest->getRequestUri()." returned ".$responseStatusCode."\n".json_encode($responseBody); + parent::__construct($message, $responseStatusCode); + } + + /** + * Returns HTTP headers in the response from the Graph + * + * @return array + */ + public function getResponseHeaders(): array { + return $this->responseHeaders; + } + + /** + * Returns HTTP status code returned int the response from the Graph + * + * @return int + */ + public function getResponseStatusCode(): int { + return $this->responseStatusCode; + } + + /** + * Get JSON-decoded response payload from the Graph + * + * @return array + */ + public function getRawResponseBody(): array { + return $this->responseBody; + } + + /** + * Returns the error object of the payload + * + * @return ODataErrorContent|null + */ + public function getError(): ?ODataErrorContent { + if (array_key_exists("error", $this->responseBody)) { + return new ODataErrorContent($this->responseBody["error"]); + } + return null; + } + + /** + * Returns the request that triggered the error response + * + * @return GraphRequest + */ + public function getRequest(): GraphRequest { + return $this->graphRequest; + } + + /** + * Returns the client request Id + * + * @return string|null + */ + public function getClientRequestId(): ?string { + $headerName = "client-request-id"; + if (array_key_exists($headerName, $this->responseHeaders) + && !empty($this->responseHeaders[$headerName])) { + return $this->responseHeaders[$headerName][0]; + } + return null; + } + + /** + * Returns the request Id + * + * @return string|null + */ + public function getRequestId(): ?string { + $headerName = "request-id"; + if (array_key_exists($headerName, $this->responseHeaders) + && !empty($this->responseHeaders[$headerName])) { + return $this->responseHeaders[$headerName][0]; + } + return null; + } +} diff --git a/src/Exception/GraphServiceException.php b/src/Exception/GraphServiceException.php index 77d32e37..ea2d44b8 100644 --- a/src/Exception/GraphServiceException.php +++ b/src/Exception/GraphServiceException.php @@ -8,140 +8,17 @@ namespace Microsoft\Graph\Exception; -use Microsoft\Graph\Http\GraphRequest; - /** * Class GraphServiceException * - * Thrown when the Graph API returns 5xx responses + * Thrown when the Graph API returns a 4xx response * * @package Microsoft\Graph\Exception * @copyright 2021 Microsoft Corporation * @license https://opensource.org/licenses/MIT MIT License * @link https://developer.microsoft.com/graph */ -class GraphServiceException extends GraphException +class GraphServiceException extends GraphResponseException { - /** - * Headers returned in the response - * - * @var array - */ - private $responseHeaders; - /** - * HTTP response code - * - * @var int - */ - private $responseStatusCode; - /** - * JSON-decoded response body - * - * @var array - */ - private $responseBody; - /** - * The request that triggered the error response - * - * @var GraphRequest - */ - private $graphRequest; - - - /** - * GraphServiceException constructor. - * @param GraphRequest $graphRequest - * @param int $responseStatusCode - * @param array $responseBody - * @param array $responseHeaders - */ - public function __construct( - GraphRequest $graphRequest, - int $responseStatusCode, - array $responseBody, - array $responseHeaders - ) { - $this->graphRequest = $graphRequest; - $this->responseStatusCode = $responseStatusCode; - $this->responseBody = $responseBody; - $this->responseHeaders = $responseHeaders; - $message = "'".$graphRequest->getRequestType()."' request to ".$graphRequest->getRequestUri()." returned ".$responseStatusCode."\n".json_encode($responseBody); - parent::__construct($message, $responseStatusCode); - } - - /** - * Returns HTTP headers in the response from the Graph - * - * @return array - */ - public function getResponseHeaders(): array { - return $this->responseHeaders; - } - - /** - * Returns HTTP status code returned int the response from the Graph - * - * @return int - */ - public function getResponseStatusCode(): int { - return $this->responseStatusCode; - } - - /** - * Get JSON-decoded response payload from the Graph - * - * @return array - */ - public function getRawResponseBody(): array { - return $this->responseBody; - } - - /** - * Returns the error object of the payload - * - * @return ODataError|null - */ - public function getError(): ?ODataError { - if (array_key_exists("error", $this->responseBody)) { - return new ODataError($this->responseBody["error"]); - } - return null; - } - - /** - * Returns the request that triggered the error response - * - * @return GraphRequest - */ - public function getRequest(): GraphRequest { - return $this->graphRequest; - } - - /** - * Returns the client request Id - * - * @return string|null - */ - public function getClientRequestId(): ?string { - $headerName = "client-request-id"; - if (array_key_exists($headerName, $this->responseHeaders) - && !empty($this->responseHeaders[$headerName])) { - return $this->responseHeaders[$headerName][0]; - } - return null; - } - /** - * Returns the request Id - * - * @return string|null - */ - public function getRequestId(): ?string { - $headerName = "request-id"; - if (array_key_exists($headerName, $this->responseHeaders) - && !empty($this->responseHeaders[$headerName])) { - return $this->responseHeaders[$headerName][0]; - } - return null; - } } diff --git a/src/Exception/ODataError.php b/src/Exception/ODataErrorContent.php similarity index 87% rename from src/Exception/ODataError.php rename to src/Exception/ODataErrorContent.php index cfc85080..5e7b2f34 100644 --- a/src/Exception/ODataError.php +++ b/src/Exception/ODataErrorContent.php @@ -18,7 +18,7 @@ * @license https://opensource.org/licenses/MIT MIT License * @link https://developer.microsoft.com/graph */ -class ODataError extends BaseError +class ODataErrorContent extends BaseErrorContent { /** * Get error code returned by the Graph @@ -50,12 +50,12 @@ public function getTarget(): ?string { /** * Returns the error details containing a code, message and target * - * @return ODataError[]|null + * @return ODataErrorContent[]|null */ public function getDetails(): ?array { $details = $this->getProperty("details"); if ($details) { - return array_map(function ($detail) { return new ODataError($detail); }, $details); + return array_map(function ($detail) { return new ODataErrorContent($detail); }, $details); } return null; } @@ -63,11 +63,11 @@ public function getDetails(): ?array { /** * Get the Graph-specific error info * - * @return GraphError|null + * @return GraphErrorContent|null */ - public function getInnerError(): ?GraphError { + public function getInnerError(): ?GraphErrorContent { $innerError = $this->getProperty("innerError"); - return ($innerError) ? new GraphError($innerError) : null; + return ($innerError) ? new GraphErrorContent($innerError) : null; } /** diff --git a/src/Http/GraphCollectionRequest.php b/src/Http/GraphCollectionRequest.php index fe5493f2..4904930b 100644 --- a/src/Http/GraphCollectionRequest.php +++ b/src/Http/GraphCollectionRequest.php @@ -85,8 +85,9 @@ public function __construct(string $requestType, string $endpoint, AbstractGraph * Gets the number of entries in the collection * * @return int|null the number of entries | null if @odata.count doesn't exist for that collection - * @throws ClientExceptionInterface - * @throws GraphServiceException if 4xx or 5xx response is returned + * @throws ClientExceptionInterface if an errors occurs while making the request + * @throws GraphClientException containing error payload if 4xx response is returned. + * @throws GraphServiceException containing error payload if 5xx response is returned. */ public function count(): ?int { @@ -106,9 +107,8 @@ public function count(): ?int * to "getPage()" * * @param int $pageSize The page size - * - * @throws \InvalidArgumentException if the requested page size exceeds Graph's defined page size limit - * @return GraphCollectionRequest object + * @return GraphCollectionRequest object + * @throws \InvalidArgumentException if the requested page size exceeds Graph's defined page size limit */ public function setPageSize(int $pageSize): self { @@ -123,8 +123,9 @@ public function setPageSize(int $pageSize): self * Gets the next page of results * * @return GraphResponse|array of objects of class $returnType| GraphResponse if no $returnType - * @throws ClientExceptionInterface - * @throws GraphServiceException if 4xx or 5xx response is returned + * @throws ClientExceptionInterface if an error occurs while making the request + * @throws GraphClientException containing error payload if 4xx response is returned. + * @throws GraphServiceException containing error payload if 5xx response is returned. */ public function getPage() { @@ -232,9 +233,9 @@ public function getPageSize(): int { * * @param callable(): bool $callback function to execute against each element of $entityCollection. Must return boolean which determines if iteration should pause/proceed * @return PageIterator call iterate() to start the iterator - * @throws ClientExceptionInterface - * @throws GraphClientException - * @throws GraphServiceException if 4xx or 5xx is returned when fetching the initial collection + * @throws ClientExceptionInterface if error occurs while making the request + * @throws GraphClientException containing error payload if 4xx is returned while fetching the initial page + * @throws GraphServiceException containing error payload if 5xx is returned while fetching the initial page */ public function pageIterator(callable $callback): PageIterator { // temporarily disable return type in order to get first page as GraphResponse object diff --git a/src/Http/GraphRequest.php b/src/Http/GraphRequest.php index 28b1c67f..9fd56ab3 100644 --- a/src/Http/GraphRequest.php +++ b/src/Http/GraphRequest.php @@ -234,8 +234,9 @@ public function getBody() * * @param ClientInterface|null $client (optional) When null, uses $graphClient's http client * @return array|GraphResponse|StreamInterface|object Graph Response object or response body cast to $returnType - * @throws ClientExceptionInterface - * @throws GraphServiceException if 4xx or 5xx response is returned. Exception contains the error payload + * @throws ClientExceptionInterface if an error occurs while making the request + * @throws GraphClientException containing error payload if 4xx response is returned. + * @throws GraphServiceException containing error payload if 5xx response is returned. */ public function execute(?ClientInterface $client = null) { @@ -273,8 +274,9 @@ public function execute(?ClientInterface $client = null) * * @param HttpAsyncClient|null $client (optional) When null, uses $graphClient's http client * @return Promise Resolves to GraphResponse object|response body cast to $returnType. Fails throwing the exception - * @throws GraphServiceException if 4xx or 5xx response is returned. Exception contains the error payload * @throws ClientExceptionInterface if there are any errors while making the HTTP request + * @throws GraphClientException containing error payload if 4xx is returned + * @throws GraphServiceException containing error payload if 5xx is returned * @throws \Exception */ public function executeAsync(?HttpAsyncClient $client = null): Promise @@ -319,9 +321,10 @@ function ($reason) { * * @param string $path path to download the file contents to * @param ClientInterface|null $client (optional) When null, defaults to $graphClient's http client - * @throws ClientExceptionInterface * @throws \RuntimeException when unable to open $path for writing - * @throws GraphServiceException if 4xx or 5xx response is returned. Error payload is accessible from the exception + * @throws ClientExceptionInterface if an error occurs while making the request + * @throws GraphClientException containing error payload if 4xx is returned + * @throws GraphServiceException containing error payload if 5xx is returned */ public function download(string $path, ?ClientInterface $client = null): void { @@ -343,9 +346,10 @@ public function download(string $path, ?ClientInterface $client = null): void * @param string $path path of file to be uploaded * @param ClientInterface|null $client (optional) * @return array|GraphResponse|StreamInterface|object Graph Response object or response body cast to $returnType - * @throws ClientExceptionInterface + * @throws ClientExceptionInterface if an error occurs while making the request * @throws \RuntimeException if $path cannot be opened for reading - * @throws GraphServiceException if 4xx or 5xx response is returned. Error payload is accessible from the exception + * @throws GraphClientException containing error payload if 4xx is returned + * @throws GraphServiceException containing error payload if 5xx is returned */ public function upload(string $path, ?ClientInterface $client = null) { @@ -393,28 +397,42 @@ protected function initRequestUri(string $baseUrl, string $endpoint): void { $this->requestUri = GraphRequestUtil::getRequestUri($baseUrl, $endpoint, $this->graphClient->getApiVersion()); } + /** + * Initialises a PSR-7 Http Request object + */ protected function initPsr7HttpRequest(): void { $this->httpRequest = new Request($this->requestType, $this->requestUri, $this->headers, $this->requestBody); } /** - * Check if response status code is 4xx or 5xx + * Check if response status code is a client error * * @param int $httpStatusCode * @return bool */ - private function isErrorResponse(int $httpStatusCode): bool { - return ($httpStatusCode >=400 && $httpStatusCode <= 599); + private function is4xx(int $httpStatusCode): bool { + return ($httpStatusCode >= 400 && $httpStatusCode < 500); } /** - * Determines whether to throw GraphServiceException or not + * Check if response status code is a server error + * + * @param int $httpStatusCode + * @return bool + */ + private function is5xx(int $httpStatusCode): bool { + return ($httpStatusCode >= 500 && $httpStatusCode < 600); + } + + /** + * Throws appropriate exception type * * @param Response $httpResponse - * @throws GraphServiceException + * @throws GraphServiceException for server errors + * @throws GraphClientException for client errors */ private function handleErrorResponse(Response $httpResponse) { - if ($this->isErrorResponse($httpResponse->getStatusCode())) { + if ($this->is5xx($httpResponse->getStatusCode())) { throw new GraphServiceException( $this, $httpResponse->getStatusCode(), @@ -422,5 +440,13 @@ private function handleErrorResponse(Response $httpResponse) { $httpResponse->getHeaders() ); } + if ($this->is4xx($httpResponse->getStatusCode())) { + throw new GraphClientException( + $this, + $httpResponse->getStatusCode(), + json_decode($httpResponse->getBody(), true), + $httpResponse->getHeaders() + ); + } } } diff --git a/src/Task/PageIterator.php b/src/Task/PageIterator.php index 16045140..ac4a3e56 100644 --- a/src/Task/PageIterator.php +++ b/src/Task/PageIterator.php @@ -91,7 +91,7 @@ class PageIterator * if empty, each entity will be JSON-decoded to an array and passed to $callback * @param RequestOptions|null $requestOptions (optional) custom headers/middleware to use for subsequent calls to $entityCollection's nextLink * - * @throws GraphClientException if GraphResponse does not contain a collection of values + * @throws \InvalidArgumentException if GraphResponse does not contain a collection of values */ public function __construct(AbstractGraphClient $graphClient, GraphResponse $collectionResponse, @@ -101,7 +101,7 @@ public function __construct(AbstractGraphClient $graphClient, if (!array_key_exists("value", $collectionResponse->getBody()) || !is_array($collectionResponse->getBody()["value"])) { - throw new GraphClientException("Collection response must contain a collection of values"); + throw new \InvalidArgumentException("Collection response must contain a collection of values"); } $this->graphClient = $graphClient; @@ -118,7 +118,9 @@ public function __construct(AbstractGraphClient $graphClient, * * @return Promise that resolves to true on completion and throws error on rejection * - * @throws ClientExceptionInterface|GraphClientException|GraphServiceException if promise is rejected + * @throws ClientExceptionInterface if error occurs while making the request + * @throws GraphClientException containing error payload if 4xx is returned + * @throws GraphServiceException containing error payload if 5xx is returned */ public function iterate(): Promise { $promise = new FulfilledPromise(false); @@ -156,7 +158,9 @@ public function iterate(): Promise { * Resume iteration after $callback returning false * * @return Promise - * @throws ClientExceptionInterface|GraphClientException|GraphServiceException if promise is rejected + * @throws ClientExceptionInterface if error occurs while making the request + * @throws GraphClientException containing error payload if 4xx is returned + * @throws GraphServiceException containing error payload if 5xx is returned */ public function resume(): Promise { return $this->iterate(); @@ -205,8 +209,9 @@ public function getNextLink(): ?string { /** * Fetches the next page of results * - * @throws ClientExceptionInterface - * @throws GraphServiceException + * @throws ClientExceptionInterface if error occurs while making the request + * @throws GraphClientException containing error payload if 4xx is returned + * @throws GraphServiceException containing error payload if 5xx is returned */ private function getNextPage(): void { $nextLink = $this->getNextLink(); diff --git a/tests/Exception/GraphExceptionTest.php b/tests/Exception/GraphExceptionTest.php deleted file mode 100644 index 2d333650..00000000 --- a/tests/Exception/GraphExceptionTest.php +++ /dev/null @@ -1,21 +0,0 @@ -assertEquals("Microsoft\Graph\Exception\GraphException: [404]: bad stuff\n", $exception->__toString()); - } - - public function testChildExceptionClassToString() { - $exception = new GraphClientException("Invalid national cloud"); - $this->assertStringContainsString(get_class($exception), $exception); - } -} diff --git a/tests/Exception/GraphServiceExceptionTest.php b/tests/Exception/GraphResponseExceptionTest.php similarity index 88% rename from tests/Exception/GraphServiceExceptionTest.php rename to tests/Exception/GraphResponseExceptionTest.php index 78c0193e..de964569 100644 --- a/tests/Exception/GraphServiceExceptionTest.php +++ b/tests/Exception/GraphResponseExceptionTest.php @@ -10,12 +10,12 @@ use GuzzleHttp\Psr7\Response; -use Microsoft\Graph\Exception\GraphServiceException; -use Microsoft\Graph\Exception\ODataError; +use Microsoft\Graph\Exception\GraphResponseException; +use Microsoft\Graph\Exception\ODataErrorContent; use Microsoft\Graph\Http\GraphRequest; use Microsoft\Graph\Test\Http\SampleGraphResponsePayload; -class GraphServiceExceptionTest extends \PHPUnit\Framework\TestCase +class GraphResponseExceptionTest extends \PHPUnit\Framework\TestCase { private $mockGraphRequest; private $responseStatusCode = 404; @@ -31,7 +31,7 @@ class GraphServiceExceptionTest extends \PHPUnit\Framework\TestCase protected function setUp(): void { $this->mockGraphRequest = $this->createMock(GraphRequest::class); $this->psr7Response = new Response($this->responseStatusCode, $this->responseHeaders, json_encode($this->responseBody)); - $this->defaultException = new GraphServiceException( + $this->defaultException = new GraphResponseException( $this->mockGraphRequest, $this->psr7Response->getStatusCode(), json_decode($this->psr7Response->getBody(), true), @@ -48,7 +48,7 @@ public function testGetters(): void { $this->assertEquals($this->responseStatusCode, $this->defaultException->getResponseStatusCode()); $this->assertEquals($this->responseBody, $this->defaultException->getRawResponseBody()); $this->assertEquals($this->psr7Response->getHeaders(), $this->defaultException->getResponseHeaders()); - $this->assertInstanceOf(ODataError::class, $this->defaultException->getError()); + $this->assertInstanceOf(ODataErrorContent::class, $this->defaultException->getError()); } public function testGetClientRequestId(): void { diff --git a/tests/Http/Request/GraphCollectionRequestTest.php b/tests/Http/Request/GraphCollectionRequestTest.php index d26d4c60..381bdc37 100644 --- a/tests/Http/Request/GraphCollectionRequestTest.php +++ b/tests/Http/Request/GraphCollectionRequestTest.php @@ -3,7 +3,6 @@ use Microsoft\Graph\Core\GraphConstants; use Microsoft\Graph\Exception\GraphClientException; -use Microsoft\Graph\Exception\GraphException; use Microsoft\Graph\Exception\GraphServiceException; use Microsoft\Graph\Http\GraphCollectionRequest; use Microsoft\Graph\Http\GraphRequestUtil; @@ -62,12 +61,18 @@ public function testHitEndOfCollection() $this->defaultCollectionRequest->getPage(); } - public function testGetPageThrowsExceptionOnErrorResponse(): void { + public function testGetPageThrowsExceptionOn5xxResponse(): void { $this->expectException(GraphServiceException::class); MockHttpClientResponseConfig::configureWithErrorPayload($this->mockHttpClient, 500); $this->defaultCollectionRequest->getPage(); } + public function testGetPageThrowsExceptionOn4xxResponse(): void { + $this->expectException(GraphClientException::class); + MockHttpClientResponseConfig::configureWithErrorPayload($this->mockHttpClient, 400); + $this->defaultCollectionRequest->getPage(); + } + public function testCount(): void { MockHttpClientResponseConfig::configureWithCollectionPayload($this->mockHttpClient); $count = $this->defaultCollectionRequest->count(); @@ -79,8 +84,14 @@ public function testCountReturnsNullIfNoOdataCountFound(): void { $this->assertNull($this->defaultCollectionRequest->count()); } - public function testCountThrowsExceptionOnErrorResponse(): void { + public function testCountThrowsExceptionOn5xxResponse(): void { $this->expectException(GraphServiceException::class); + MockHttpClientResponseConfig::configureWithErrorPayload($this->mockHttpClient, 503); + $this->defaultCollectionRequest->count(); + } + + public function testCountThrowsExceptionOn4xxResponse(): void { + $this->expectException(GraphClientException::class); MockHttpClientResponseConfig::configureWithErrorPayload($this->mockHttpClient, 404); $this->defaultCollectionRequest->count(); } @@ -109,9 +120,15 @@ public function testPageIteratorInitialisesUsingFirstPageOfResults() { $this->assertTrue($numEntitiesProcessed >= sizeof(SampleGraphResponsePayload::COLLECTION_PAYLOAD["value"])); } - public function testPageIteratorThrowsExceptionIfFirstPageRequestGetsErrorResponse(): void { + public function testPageIteratorThrowsExceptionIfFirstPageRequestGets5xxResponse(): void { $this->expectException(GraphServiceException::class); MockHttpClientResponseConfig::configureWithErrorPayload($this->mockHttpClient, 500); $this->defaultCollectionRequest->pageIterator(function () {}); } + + public function testPageIteratorThrowsExceptionIfFirstPageRequestGets4xxResponse(): void { + $this->expectException(GraphClientException::class); + MockHttpClientResponseConfig::configureWithErrorPayload($this->mockHttpClient, 400); + $this->defaultCollectionRequest->pageIterator(function () {}); + } } diff --git a/tests/Http/Request/GraphRequestAsyncTest.php b/tests/Http/Request/GraphRequestAsyncTest.php index 3c410142..24e7d309 100644 --- a/tests/Http/Request/GraphRequestAsyncTest.php +++ b/tests/Http/Request/GraphRequestAsyncTest.php @@ -11,10 +11,10 @@ use Http\Client\HttpAsyncClient; use Http\Promise\Promise; +use Microsoft\Graph\Exception\GraphClientException; use Microsoft\Graph\Exception\GraphServiceException; use Microsoft\Graph\Http\GraphResponse; use Microsoft\Graph\Test\Http\SampleGraphResponsePayload; -use Microsoft\Graph\Test\Http\TestModel; use Microsoft\Graph\Test\TestData\Model\User; use Psr\Http\Client\ClientExceptionInterface; use Psr\Http\Client\NetworkExceptionInterface; @@ -62,8 +62,8 @@ public function testExecuteAsyncPromiseResolvesToGraphResponseIfNoReturnType(): $this->assertInstanceOf(GraphResponse::class, $promise->wait()); } - public function testExecuteAsyncPromiseThrowsExceptionForErrorResponse(): void { - $this->expectException(GraphServiceException::class); + public function testExecuteAsyncPromiseThrowsExceptionFor4xxResponse(): void { + $this->expectException(GraphClientException::class); MockHttpClientAsyncResponseConfig::statusCode(400)::configureWithFulfilledPromise( $this->mockHttpClient, SampleGraphResponsePayload::ERROR_PAYLOAD @@ -72,6 +72,16 @@ public function testExecuteAsyncPromiseThrowsExceptionForErrorResponse(): void { $promise->wait(); } + public function testExecuteAsyncPromiseThrowsExceptionFor5xxResponse(): void { + $this->expectException(GraphServiceException::class); + MockHttpClientAsyncResponseConfig::statusCode(500)::configureWithFulfilledPromise( + $this->mockHttpClient, + SampleGraphResponsePayload::ERROR_PAYLOAD + ); + $promise = $this->defaultGraphRequest->executeAsync(); + $promise->wait(); + } + public function testExecuteAsyncResolvesToModelForModelReturnType(): void { MockHttpClientAsyncResponseConfig::statusCode()::configureWithFulfilledPromise( $this->mockHttpClient, diff --git a/tests/Http/Request/GraphRequestStreamTest.php b/tests/Http/Request/GraphRequestStreamTest.php index bfce97ed..777db3fb 100644 --- a/tests/Http/Request/GraphRequestStreamTest.php +++ b/tests/Http/Request/GraphRequestStreamTest.php @@ -36,10 +36,20 @@ public function testInvalidUpload() $this->defaultGraphRequest->upload($file->url()); } - public function testUploadThrowsExceptionForErrorResponse() + public function testUploadThrowsExceptionFor4xxResponse() + { + $this->expectException(GraphClientException::class); + MockHttpClientResponseConfig::configureWithErrorPayload($this->mockHttpClient, 400); + $file = vfsStream::newFile('foo.txt') + ->withContent("content") + ->at($this->rootDir); + $this->defaultGraphRequest->upload($file->url()); + } + + public function testUploadThrowsExceptionFor5xxResponse() { $this->expectException(GraphServiceException::class); - MockHttpClientResponseConfig::configureWithErrorPayload($this->mockHttpClient); + MockHttpClientResponseConfig::configureWithErrorPayload($this->mockHttpClient, 500); $file = vfsStream::newFile('foo.txt') ->withContent("content") ->at($this->rootDir); @@ -64,10 +74,19 @@ public function testInvalidDownload() $this->defaultGraphRequest->download($file->url()); } - public function testDownloadThrowsExceptionForErrorResponse() + public function testDownloadThrowsExceptionFor4xxResponse() + { + $this->expectException(GraphClientException::class); + MockHttpClientResponseConfig::configureWithErrorPayload($this->mockHttpClient, 400); + $file = new VfsStreamFile('foo.txt'); + $this->rootDir->addChild($file); + $this->defaultGraphRequest->download($file->url()); + } + + public function testDownloadThrowsExceptionFor5xxResponse() { $this->expectException(GraphServiceException::class); - MockHttpClientResponseConfig::configureWithErrorPayload($this->mockHttpClient); + MockHttpClientResponseConfig::configureWithErrorPayload($this->mockHttpClient, 500); $file = new VfsStreamFile('foo.txt'); $this->rootDir->addChild($file); $this->defaultGraphRequest->download($file->url()); diff --git a/tests/Http/Request/GraphRequestSyncTest.php b/tests/Http/Request/GraphRequestSyncTest.php index 7a8cc611..556cba3e 100644 --- a/tests/Http/Request/GraphRequestSyncTest.php +++ b/tests/Http/Request/GraphRequestSyncTest.php @@ -9,6 +9,7 @@ namespace Microsoft\Graph\Test\Http\Request; +use Microsoft\Graph\Exception\GraphClientException; use Microsoft\Graph\Exception\GraphServiceException; use Microsoft\Graph\Http\GraphResponse; use Microsoft\Graph\Test\TestData\Model\User; @@ -53,9 +54,15 @@ public function testExecuteWithoutReturnTypeReturnsGraphResponseForEmptyPayload( $this->assertInstanceOf(GraphResponse::class, $response); } - public function testExecuteWithoutReturnTypeThrowsExceptionForErrorPayload(): void { + public function testExecuteWithoutReturnTypeThrowsExceptionFor4xxPayload(): void { + $this->expectException(GraphClientException::class); + MockHttpClientResponseConfig::configureWithErrorPayload($this->mockHttpClient, 400); + $response = $this->defaultGraphRequest->execute(); + } + + public function testExecuteWithoutReturnTypeThrowsExceptionFor5xxPayload(): void { $this->expectException(GraphServiceException::class); - MockHttpClientResponseConfig::configureWithErrorPayload($this->mockHttpClient); + MockHttpClientResponseConfig::configureWithErrorPayload($this->mockHttpClient, 500); $response = $this->defaultGraphRequest->execute(); } @@ -83,9 +90,15 @@ public function testExecuteWithModelReturnTypeReturnsModelForEmptyPayload(): voi $this->assertInstanceOf(User::class, $response); } - public function testExecuteWithModelReturnTypeThrowsExceptionForErrorPayload(): void { + public function testExecuteWithModelReturnTypeThrowsExceptionFor4xxPayload(): void { + $this->expectException(GraphClientException::class); + MockHttpClientResponseConfig::configureWithErrorPayload($this->mockHttpClient, 400); + $response = $this->defaultGraphRequest->setReturnType(User::class)->execute(); + } + + public function testExecuteWithModelReturnTypeThrowsExceptionFor5xxPayload(): void { $this->expectException(GraphServiceException::class); - MockHttpClientResponseConfig::configureWithErrorPayload($this->mockHttpClient); + MockHttpClientResponseConfig::configureWithErrorPayload($this->mockHttpClient, 500); $response = $this->defaultGraphRequest->setReturnType(User::class)->execute(); } @@ -115,9 +128,15 @@ public function testExecuteWithStreamReturnTypeReturnsStreamForEmptyPayload(): v $this->assertInstanceOf(StreamInterface::class, $response); } - public function testExecuteWithStreamReturnTypeThrowsExceptionForErrorPayload(): void { + public function testExecuteWithStreamReturnTypeThrowsExceptionFor4xxPayload(): void { + $this->expectException(GraphClientException::class); + MockHttpClientResponseConfig::configureWithErrorPayload($this->mockHttpClient, 400); + $response = $this->defaultGraphRequest->setReturnType(StreamInterface::class)->execute(); + } + + public function testExecuteWithStreamReturnTypeThrowsExceptionFor5xxPayload(): void { $this->expectException(GraphServiceException::class); - MockHttpClientResponseConfig::configureWithErrorPayload($this->mockHttpClient); + MockHttpClientResponseConfig::configureWithErrorPayload($this->mockHttpClient, 500); $response = $this->defaultGraphRequest->setReturnType(StreamInterface::class)->execute(); } diff --git a/tests/Task/PageIteratorTest.php b/tests/Task/PageIteratorTest.php index 4e4bd2d2..dfe5b426 100644 --- a/tests/Task/PageIteratorTest.php +++ b/tests/Task/PageIteratorTest.php @@ -10,8 +10,6 @@ use GuzzleHttp\Psr7\Utils; -use Microsoft\Graph\Exception\GraphClientException; -use Microsoft\Graph\Http\AbstractGraphClient; use Microsoft\Graph\Http\GraphCollectionRequest; use Microsoft\Graph\Http\GraphResponse; use Microsoft\Graph\Http\RequestOptions; @@ -75,7 +73,7 @@ public function testConstructorCreatesPageIterator() { } public function testConstructorThrowsExceptionIfGraphResponseIsNotACollection() { - $this->expectException(GraphClientException::class); + $this->expectException(\InvalidArgumentException::class); $pageIterator = new PageIterator( $this->mockGraphClient, $this->createCollectionResponse(SampleGraphResponsePayload::ENTITY_PAYLOAD), From 8afa6911919b6d9b81195f690f45ef45984425fd Mon Sep 17 00:00:00 2001 From: Philip Gichuhi Date: Thu, 16 Sep 2021 20:19:02 +0300 Subject: [PATCH 7/7] Add tests --- src/Core/Enum.php | 6 +-- tests/Exception/ErrorContentTest.php | 37 +++++++++++++++++++ .../Exception/GraphResponseExceptionTest.php | 2 +- 3 files changed, 40 insertions(+), 5 deletions(-) create mode 100644 tests/Exception/ErrorContentTest.php diff --git a/src/Core/Enum.php b/src/Core/Enum.php index 803d4990..6d42d7b3 100644 --- a/src/Core/Enum.php +++ b/src/Core/Enum.php @@ -16,8 +16,6 @@ */ namespace Microsoft\Graph\Core; -use Microsoft\Graph\Exception\GraphException; - /** * Class Enum * @@ -41,12 +39,12 @@ abstract class Enum * * @param string $value The value of the enum * - * @throws GraphException if enum value is invalid + * @throws \InvalidArgumentException if enum value is invalid */ public function __construct($value) { if (!self::has($value)) { - throw new GraphException("Invalid enum value $value"); + throw new \InvalidArgumentException("Invalid enum value $value"); } $this->_value = $value; } diff --git a/tests/Exception/ErrorContentTest.php b/tests/Exception/ErrorContentTest.php new file mode 100644 index 00000000..0bb6ca9b --- /dev/null +++ b/tests/Exception/ErrorContentTest.php @@ -0,0 +1,37 @@ +rawErrorContent = SampleGraphResponsePayload::ERROR_PAYLOAD["error"]; + $this->odataErrorContent = new ODataErrorContent($this->rawErrorContent); + parent::setUp(); + } + + public function testGetters(): void { + $this->assertEquals($this->rawErrorContent["code"], $this->odataErrorContent->getCode()); + $this->assertEquals($this->rawErrorContent["message"], $this->odataErrorContent->getMessage()); + $this->assertNull($this->odataErrorContent->getTarget()); + $this->assertNull($this->odataErrorContent->getDetails()); + $this->assertInstanceOf(GraphErrorContent::class, $this->odataErrorContent->getInnerError()); + $this->assertNotEmpty($this->odataErrorContent->getProperties()); + $this->assertNotEmpty(strval($this->odataErrorContent)); + } +} diff --git a/tests/Exception/GraphResponseExceptionTest.php b/tests/Exception/GraphResponseExceptionTest.php index de964569..4cc3a6ed 100644 --- a/tests/Exception/GraphResponseExceptionTest.php +++ b/tests/Exception/GraphResponseExceptionTest.php @@ -39,7 +39,7 @@ protected function setUp(): void { ); } - public function testGraphServiceExceptionIsThrowable(): void { + public function testGraphResponseExceptionIsThrowable(): void { $this->assertInstanceOf(\Throwable::class, $this->defaultException); }