Skip to content

Commit f6a834a

Browse files
SilasKennethSilasKenneth
authored
Merge pull request #103 from microsoftgraph/fix/batch-request-serialize
Batch Request Serialization bug fixes
2 parents adabb08 + da658a4 commit f6a834a

File tree

6 files changed

+126
-22
lines changed

6 files changed

+126
-22
lines changed

‎src/Requests/BatchRequestItem.php

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88

99
namespace Microsoft\Graph\Core\Requests;
1010

11+
use InvalidArgumentException;
1112
use League\Uri\Contracts\UriException;
1213
use Microsoft\Kiota\Abstractions\RequestHeaders;
1314
use Microsoft\Kiota\Abstractions\RequestInformation;
@@ -82,7 +83,7 @@ class BatchRequestItem implements Parsable
8283
public function __construct(RequestInformation $requestInformation, string $id = "", ?array $dependsOn = null)
8384
{
8485
if (!$requestInformation->httpMethod) {
85-
throw new \InvalidArgumentException("HTTP method cannot be NULL/empty");
86+
throw new InvalidArgumentException("HTTP method cannot be NULL/empty");
8687
}
8788
$this->id = ($id) ?: Uuid::uuid4();
8889
$this->method = $requestInformation->httpMethod;
@@ -129,13 +130,15 @@ public function setUrl(string $url): void
129130
// Set relative URL
130131
$urlParts = parse_url($url);
131132
if (!$urlParts || !array_key_exists('path', $urlParts)) {
132-
throw new \InvalidArgumentException("Invalid URL {$url}");
133+
throw new InvalidArgumentException("Invalid URL {$url}");
133134
}
134135
// Set relative URL
135136
// Remove API version
136-
$urlWithoutVersion = preg_replace("/\/(v1.0|beta)/", '',"{$urlParts['path']}");
137+
$urlWithoutVersion = preg_replace("/\/(v1.0|beta)/", '', "{$urlParts['path']}");
137138
if (!$urlWithoutVersion) {
138-
throw new RuntimeException("Error occurred during regex replacement of API version in URL string: $url");
139+
throw new InvalidArgumentException(
140+
"Error occurred during regex replacement of API version in URL string: $url"
141+
);
139142
}
140143
$this->url = $urlWithoutVersion;
141144
$this->url .= (array_key_exists('query', $urlParts)) ? "?{$urlParts['query']}" : '';
@@ -239,6 +242,16 @@ public function serialize(SerializationWriter $writer): void
239242
$headers[$key] = implode(", ", $val);
240243
}
241244
$writer->writeAnyValue('headers', $headers);
242-
$writer->writeAnyValue('body', ($this->getBody()) ? json_decode($this->getBody()->getContents()) : null);
245+
if ($this->getBody()) {
246+
// API expects JSON object or base 64 encoded value for the body
247+
// We JSON decode the stream contents so that the body is not written as a string
248+
$jsonObject = json_decode($this->getBody()->getContents(), true);
249+
$isJsonString = $jsonObject && (json_last_error() === JSON_ERROR_NONE);
250+
$this->getBody()->rewind();
251+
$writer->writeAnyValue(
252+
'body',
253+
$isJsonString ? $jsonObject : base64_encode($this->getBody()->getContents())
254+
);
255+
}
243256
}
244257
}

‎src/Requests/BatchResponseContent.php

Lines changed: 19 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -8,17 +8,15 @@
88

99
namespace Microsoft\Graph\Core\Requests;
1010

11+
use Exception;
1112
use GuzzleHttp\Psr7\Utils;
1213
use InvalidArgumentException;
13-
use Microsoft\Kiota\Abstractions\RequestInformation;
1414
use Microsoft\Kiota\Abstractions\Serialization\Parsable;
1515
use Microsoft\Kiota\Abstractions\Serialization\ParseNode;
1616
use Microsoft\Kiota\Abstractions\Serialization\ParseNodeFactory;
1717
use Microsoft\Kiota\Abstractions\Serialization\ParseNodeFactoryRegistry;
1818
use Microsoft\Kiota\Abstractions\Serialization\SerializationWriter;
19-
use Microsoft\Kiota\Serialization\Json\JsonParseNodeFactory;
2019
use RuntimeException;
21-
use UnexpectedValueException;
2220

2321
/**
2422
* Class BatchResponseContent
@@ -71,15 +69,15 @@ public function getResponse(string $requestId): BatchResponseItem
7169
}
7270

7371
/**
74-
* Deserializes a response item's body to $type. $type MUST implement Parsable
72+
* Deserializes a response item's body to $type. $type MUST implement Parsable.
73+
* Uses the ParseNodeFactory registry to get the required Parse Node implementation
7574
*
7675
* @template T of Parsable
7776
* @param string $requestId
7877
* @param class-string<T> $type Parsable class name
79-
* @param ParseNodeFactory|null $parseNodeFactory checks the ParseNodeFactoryRegistry by default
8078
* @return T|null
8179
*/
82-
public function getResponseBody(string $requestId, string $type, ?ParseNodeFactory $parseNodeFactory = null): ?Parsable
80+
public function getResponseBody(string $requestId, string $type): ?Parsable
8381
{
8482
if (!$this->responses || !array_key_exists($requestId, $this->responses)) {
8583
throw new InvalidArgumentException("No response found for id: {$requestId}");
@@ -94,17 +92,25 @@ public function getResponseBody(string $requestId, string $type, ?ParseNodeFacto
9492
throw new RuntimeException("Unable to get content-type header in response item");
9593
}
9694
$responseBody = $response->getBody() ?? Utils::streamFor(null);
97-
if ($parseNodeFactory) {
98-
$parseNode = $parseNodeFactory->getRootParseNode($contentType, $responseBody);
99-
} else {
100-
// Check the registry or default to Json deserialization
95+
try {
10196
try {
10297
$parseNode = ParseNodeFactoryRegistry::getDefaultInstance()->getRootParseNode($contentType, $responseBody);
103-
} catch (UnexpectedValueException $ex) {
104-
$parseNode = (new JsonParseNodeFactory())->getRootParseNode($contentType, $responseBody);
98+
} catch (Exception $ex) {
99+
// Responses to requests with base 64 encoded stream bodies are base 64 encoded
100+
// Tries to decode the response body and retries deserialization
101+
$responseBody->rewind();
102+
$base64DecodedBody = Utils::streamFor(base64_decode($responseBody->getContents()));
103+
$parseNode = ParseNodeFactoryRegistry::getDefaultInstance()
104+
->getRootParseNode($contentType, $base64DecodedBody);
105+
// Update response body only after we're sure decoding worked
106+
$response->setBody($base64DecodedBody);
105107
}
108+
return $parseNode->getObjectValue([$type, 'createFromDiscriminatorValue']);
109+
} catch (Exception $ex) {
110+
throw new InvalidArgumentException(
111+
"Unable to deserialize batch response for request Id: $requestId to $type"
112+
);
106113
}
107-
return $parseNode->getObjectValue([$type, 'createFromDiscriminatorValue']);
108114
}
109115

110116
public function getFieldDeserializers(): array

‎tests/Requests/BatchRequestContentTest.php

Lines changed: 32 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,23 +7,29 @@
77
use GuzzleHttp\Psr7\Utils;
88
use Microsoft\Graph\Core\Requests\BatchRequestContent;
99
use Microsoft\Graph\Core\Requests\BatchRequestItem;
10+
use Microsoft\Kiota\Abstractions\RequestAdapter;
1011
use Microsoft\Kiota\Abstractions\RequestInformation;
1112
use Microsoft\Kiota\Serialization\Json\JsonSerializationWriter;
13+
use Microsoft\Kiota\Serialization\Json\JsonSerializationWriterFactory;
1214
use PHPUnit\Framework\TestCase;
1315

1416
class BatchRequestContentTest extends TestCase
1517
{
1618
private array $requests;
1719
private RequestInformation $requestInformation;
20+
private RequestAdapter $mockRequestAdapter;
1821

1922
protected function setUp(): void
2023
{
2124
$this->requestInformation = new RequestInformation();
2225
$this->requestInformation->httpMethod = "POST";
2326
$this->requestInformation->setUri(new Uri("/v1/users"));
24-
$this->requestInformation->setStreamContent(Utils::streamFor("abcd"));
27+
$this->requestInformation->setStreamContent(Utils::streamFor("Hello world!"));
2528

2629
$this->requests = [$this->requestInformation, $this->requestInformation, $this->requestInformation];
30+
31+
$this->mockRequestAdapter = $this->createStub(RequestAdapter::class);
32+
$this->mockRequestAdapter->method('getSerializationWriterFactory')->willReturn((new JsonSerializationWriterFactory()));
2733
}
2834

2935
public function testConstructor()
@@ -77,7 +83,7 @@ public function testRemoveBatchRequestItem()
7783
$this->assertEmpty($requestContext->getRequests());
7884
}
7985

80-
public function testSerialization()
86+
public function testSerializationWithNonJsonBody()
8187
{
8288
$this->requestInformation->addHeaders(['accept' => 'application/json']);
8389
$batchRequestContent = new BatchRequestContent([$this->requestInformation]);
@@ -92,7 +98,30 @@ public function testSerialization()
9298
"method" => $this->requestInformation->httpMethod,
9399
"url" => '/v1/users',
94100
'headers' => ['content-type' => 'application/octet-stream', 'accept' => 'application/json'],
95-
"body" => urlencode("abcd")
101+
"body" => base64_encode("Hello world!")
102+
]
103+
]
104+
], JSON_UNESCAPED_SLASHES);
105+
106+
$this->assertEquals($expectedJson, $serializationWriter->getSerializedContent()->getContents());
107+
}
108+
109+
public function testSerializationWithJsonBody()
110+
{
111+
$this->requestInformation->setContentFromParsable($this->mockRequestAdapter, 'application/json', new TestUserModel('1', '1'));
112+
$batchRequestContent = new BatchRequestContent([$this->requestInformation]);
113+
114+
$serializationWriter = new JsonSerializationWriter();
115+
$serializationWriter->writeObjectValue(null, $batchRequestContent);
116+
117+
$expectedJson = json_encode([
118+
'requests' => [
119+
[
120+
"id" => $batchRequestContent->getRequests()[0]->getId(),
121+
"method" => $this->requestInformation->httpMethod,
122+
"url" => '/v1/users',
123+
'headers' => ['content-type' => 'application/octet-stream, application/json'],
124+
"body" => ["id" => "1", "name" => "1"]
96125
]
97126
]
98127
], JSON_UNESCAPED_SLASHES);

‎tests/Requests/BatchRequestItemTest.php

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,4 +78,14 @@ public function testSerialization()
7878

7979
$this->assertEquals($expectedJson, $jsonSerializationWriter->getSerializedContent()->getContents());
8080
}
81+
82+
public function testSettingInvalidUrl(): void
83+
{
84+
$this->expectException(\InvalidArgumentException::class);
85+
$item = new BatchRequestItem($this->requestInformation);
86+
$item->setId('1243');
87+
$item->setMethod('GET');
88+
$item->setUrl('');
89+
$item->setHeaders([]);
90+
}
8191
}

‎tests/Requests/BatchResponseContentTest.php

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,9 @@
55
use GuzzleHttp\Psr7\Utils;
66
use Microsoft\Graph\Core\Requests\BatchResponseContent;
77
use Microsoft\Graph\Core\Requests\BatchResponseItem;
8+
use Microsoft\Kiota\Abstractions\RequestInformation;
9+
use Microsoft\Kiota\Abstractions\Serialization\ParseNodeFactoryRegistry;
10+
use Microsoft\Kiota\Serialization\Json\JsonParseNodeFactory;
811
use PHPUnit\Framework\TestCase;
912

1013
class BatchResponseContentTest extends TestCase
@@ -25,6 +28,8 @@ protected function setUp(): void
2528
];
2629
$this->batchResponseContent = new BatchResponseContent();
2730
$this->batchResponseContent->setResponses($responses);
31+
ParseNodeFactoryRegistry::getDefaultInstance()
32+
->contentTypeAssociatedFactories['application/json'] = new JsonParseNodeFactory();
2833
parent::setUp();
2934
}
3035

@@ -36,4 +41,41 @@ public function testGetResponseBody(): void
3641
$this->assertEquals('123', $response->getId());
3742
$this->assertEquals('xyz', $response->getName());
3843
}
44+
45+
public function testGetResponseBodyWithInvalidIdThrowsException(): void
46+
{
47+
$this->expectException(\InvalidArgumentException::class);
48+
$this->batchResponseContent->getResponse('2');
49+
}
50+
51+
public function testGetResponseBodyThrowsExceptionGivenNonParsableClassName(): void
52+
{
53+
$this->expectException(\InvalidArgumentException::class);
54+
$this->batchResponseContent->getResponseBody('1', RequestInformation::class);
55+
}
56+
57+
public function testGetResponseBodyTriesBase64DecodingBeforeFailure(): void
58+
{
59+
$this->batchResponseContent->getResponse('1')->setBody(
60+
Utils::streamFor(base64_encode(json_encode(
61+
[
62+
'id' => '123',
63+
'name' => 'xyz'
64+
]
65+
)))
66+
);
67+
$response = $this->batchResponseContent->getResponseBody('1', TestUserModel::class);
68+
$this->assertInstanceOf(TestUserModel::class, $response);
69+
$this->assertEquals('123', $response->getId());
70+
$this->assertEquals('xyz', $response->getName());
71+
}
72+
73+
public function testGetResponseBodyTotalFailureThrowsException(): void
74+
{
75+
$this->expectException(\InvalidArgumentException::class);
76+
$this->batchResponseContent->getResponse('1')->setBody(
77+
Utils::streamFor(base64_encode("{'key':val"))
78+
);
79+
$response = $this->batchResponseContent->getResponseBody('1', TestUserModel::class);
80+
}
3981
}

‎tests/Requests/TestUserModel.php

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,11 @@ class TestUserModel implements Parsable
1919
private string $id;
2020
private string $name;
2121

22-
public function __construct() {}
22+
public function __construct(string $id = '', string $name = '')
23+
{
24+
$this->id = $id;
25+
$this->name = $name;
26+
}
2327

2428
/**
2529
* @return string

0 commit comments

Comments
 (0)