diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index 2329649c12..c144e7c7c1 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -1 +1 @@ -* @andrueastman @baywet @darrelmiller @zengin @MichaelMainer @ddyett @peombwa @nikithauc @ramsessanchez @calebkiage +* @andrueastman @baywet @darrelmiller @zengin @MichaelMainer @ddyett @peombwa @nikithauc @ramsessanchez @calebkiage @Ndiritu diff --git a/CHANGELOG.md b/CHANGELOG.md index 1e73ca2044..33d42cee8c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,8 +21,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Aligned mime types model generation behaviour for request bodies on response content. [#134](https://github.com/microsoft/kiota/issues/134) - Fixed an issue where some critical errors would not return a failed exit code. [#1605](https://github.com/microsoft/kiota/issues/1605) - Moved nested request configuration classes into separate files within the namespace for PHP. [#1620](https://github.com/microsoft/kiota/pull/1620) -- Fixed wrong parameter type for Request config for request executors(PHP). -- Increased indentation for errorMappings in the request executor (PHP). +- Fixed wrong parameter type for Request config for request executors(PHP). [#1629](https://github.com/microsoft/kiota/pull/1629) +- Increased indentation for errorMappings in the request executor (PHP). [#1629](https://github.com/microsoft/kiota/pull/1629) +- Fixed bugs in PHP discriminator factory methods, Guzzle request adapter send methods, stream and plain text response handling. [#1634](https://github.com/microsoft/kiota/pull/1634) ## [0.2.1] - 2022-05-30 diff --git a/abstractions/php/src/Serialization/ParseNode.php b/abstractions/php/src/Serialization/ParseNode.php index 70cae99e3e..cc93e6f2c7 100644 --- a/abstractions/php/src/Serialization/ParseNode.php +++ b/abstractions/php/src/Serialization/ParseNode.php @@ -92,6 +92,12 @@ public function getTimeValue(): ?Time; */ public function getEnumValue(string $targetEnum): ?Enum; + /** + * @param string $targetClass + * @return Enum[]|null + */ + public function getCollectionOfEnumValues(string $targetClass): ?array; + /** * Get a Stream from node. * @return StreamInterface|null diff --git a/abstractions/php/src/Serialization/SerializationWriter.php b/abstractions/php/src/Serialization/SerializationWriter.php index f61b26589a..5e99456333 100644 --- a/abstractions/php/src/Serialization/SerializationWriter.php +++ b/abstractions/php/src/Serialization/SerializationWriter.php @@ -65,13 +65,6 @@ public function writeObjectValue(?string $key, ?Parsable $value): void; */ public function getSerializedContent(): StreamInterface; - /** - * Writes the specified enum set value to the stream with an optional given key. - * @param string|null $key the key to write the value with. - * @param array|null $values the value to write to the stream. - */ - public function writeEnumSetValue(?string $key, ?array $values): void; - /** * Writes the specified enum value to the stream with an optional given key. * @param string|null $key the key to write the value with. diff --git a/http/php/guzzle/src/GuzzleRequestAdapter.php b/http/php/guzzle/src/GuzzleRequestAdapter.php index 6d43b5a392..45461875c2 100644 --- a/http/php/guzzle/src/GuzzleRequestAdapter.php +++ b/http/php/guzzle/src/GuzzleRequestAdapter.php @@ -9,12 +9,10 @@ namespace Microsoft\Kiota\Http; -use Exception; use GuzzleHttp\Client; use GuzzleHttp\Psr7\Request; use Http\Promise\FulfilledPromise; use Http\Promise\Promise; -use Http\Promise\RejectedPromise; use Microsoft\Kiota\Abstractions\ApiException; use Microsoft\Kiota\Abstractions\Authentication\AuthenticationProvider; use Microsoft\Kiota\Abstractions\RequestAdapter; @@ -31,7 +29,6 @@ use Psr\Http\Message\RequestInterface; use Psr\Http\Message\ResponseInterface; use Psr\Http\Message\StreamInterface; -use function PHPUnit\Framework\throwException; /** * Class GuzzleRequestAdapter @@ -88,23 +85,16 @@ public function sendAsync(RequestInformation $requestInfo, array $targetCallable { return $this->getHttpResponseMessage($requestInfo)->then( function (ResponseInterface $result) use ($targetCallable, $responseHandler, $errorMappings) { - try { - $this->throwFailedResponse($result, $errorMappings); + $this->throwFailedResponse($result, $errorMappings); - if ($this->shouldReturnNull($result)) { - return new FulfilledPromise(null); - } - if (!$responseHandler) { - $rootNode = $this->getRootParseNode($result); - if ($targetCallable[0] === StreamInterface::class || is_subclass_of($targetCallable[0],StreamInterface::class, $targetCallable[0])) { - return $result->getBody(); - } - return $rootNode->getObjectValue($targetCallable); - } - return $responseHandler->handleResponseAsync($result); - } catch (ApiException $exception){ - return new RejectedPromise($exception); + if ($this->is204NoContentResponse($result)) { + return null; + } + if (!$responseHandler) { + $rootNode = $this->getRootParseNode($result); + return $rootNode->getObjectValue($targetCallable); } + return $responseHandler->handleResponseAsync($result); } ); } @@ -124,19 +114,15 @@ public function sendCollectionAsync(RequestInformation $requestInfo, array $targ { return $this->getHttpResponseMessage($requestInfo)->then( function (ResponseInterface $result) use ($targetCallable, $responseHandler, $errorMappings) { - try { - $this->throwFailedResponse($result, $errorMappings); + $this->throwFailedResponse($result, $errorMappings); - if ($this->shouldReturnNull($result)) { - return new FulfilledPromise(null); - } - if (!$responseHandler) { - return $this->getRootParseNode($result)->getCollectionOfObjectValues($targetCallable); - } - return $responseHandler->handleResponseAsync($result); - } catch (ApiException $apiException) { - return new RejectedPromise($apiException); + if ($this->is204NoContentResponse($result)) { + return new FulfilledPromise(null); } + if (!$responseHandler) { + return $this->getRootParseNode($result)->getCollectionOfObjectValues($targetCallable); + } + return $responseHandler->handleResponseAsync($result); } ); } @@ -148,40 +134,38 @@ public function sendPrimitiveAsync(RequestInformation $requestInfo, string $prim { return $this->getHttpResponseMessage($requestInfo)->then( function (ResponseInterface $result) use ($primitiveType, $responseHandler, $errorMappings) { - try { - $this->throwFailedResponse($result, $errorMappings); + $this->throwFailedResponse($result, $errorMappings); - if ($this->shouldReturnNull($result)) { - return new FulfilledPromise(null); - } - if (!$responseHandler) { - $rootParseNode = $this->getRootParseNode($result); - switch (strtolower($primitiveType)) { - case 'int': - case 'long': - return $rootParseNode->getIntegerValue(); - case 'float': - return $rootParseNode->getFloatValue(); - case 'bool': - return $rootParseNode->getBooleanValue(); - case 'string': - return $rootParseNode->getStringValue(); - case \DateTime::class: - return $rootParseNode->getDateTimeValue(); - case \DateInterval::class: - return $rootParseNode->getDateIntervalValue(); - case Date::class: - return $rootParseNode->getDateValue(); - case Time::class: - return $rootParseNode->getTimeValue(); - default: - throw new \InvalidArgumentException("Unsupported primitive type $primitiveType"); - } + if ($this->is204NoContentResponse($result)) { + return null; + } + if (!$responseHandler) { + $rootParseNode = $this->getRootParseNode($result); + switch ($primitiveType) { + case 'int': + case 'long': + return $rootParseNode->getIntegerValue(); + case 'float': + return $rootParseNode->getFloatValue(); + case 'bool': + return $rootParseNode->getBooleanValue(); + case 'string': + return $rootParseNode->getStringValue(); + case \DateTime::class: + return $rootParseNode->getDateTimeValue(); + case \DateInterval::class: + return $rootParseNode->getDateIntervalValue(); + case Date::class: + return $rootParseNode->getDateValue(); + case Time::class: + return $rootParseNode->getTimeValue(); + case StreamInterface::class: + return $result->getBody(); + default: + throw new \InvalidArgumentException("Unsupported primitive type $primitiveType"); } - return $responseHandler->handleResponseAsync($result); - } catch(ApiException $apiException) { - return new RejectedPromise($apiException); } + return $responseHandler->handleResponseAsync($result); } ); } @@ -193,19 +177,15 @@ public function sendPrimitiveCollectionAsync(RequestInformation $requestInfo, st { return $this->getHttpResponseMessage($requestInfo)->then( function (ResponseInterface $result) use ($primitiveType, $responseHandler, $errorMappings) { - try { - $this->throwFailedResponse($result, $errorMappings); + $this->throwFailedResponse($result, $errorMappings); - if ($this->shouldReturnNull($result)) { - return new FulfilledPromise(null); - } - if (!$responseHandler) { - return $this->getRootParseNode($result)->getCollectionOfPrimitiveValues($primitiveType); - } - return $responseHandler->handleResponseAsync($result); - } catch (ApiException $apiException) { - return new RejectedPromise($apiException); + if ($this->is204NoContentResponse($result)) { + return null; + } + if (!$responseHandler) { + return $this->getRootParseNode($result)->getCollectionOfPrimitiveValues($primitiveType); } + return $responseHandler->handleResponseAsync($result); } ); } @@ -217,18 +197,11 @@ public function sendNoContentAsync(RequestInformation $requestInfo, ?ResponseHan { return $this->getHttpResponseMessage($requestInfo)->then( function (ResponseInterface $result) use ($responseHandler, $errorMappings) { - try { - $this->throwFailedResponse($result, $errorMappings); - if ($this->shouldReturnNull($result)) { - return new FulfilledPromise(null); - } - if ($responseHandler) { - return $responseHandler->handleResponseAsync($result); - } - return null; - } catch (ApiException $apiException) { - return new RejectedPromise($apiException); + $this->throwFailedResponse($result, $errorMappings); + if ($responseHandler) { + return $responseHandler->handleResponseAsync($result); } + return null; } ); } @@ -345,7 +318,7 @@ private function throwFailedResponse(ResponseInterface $response, ?array $errorM * @param ResponseInterface $response * @return bool */ - private function shouldReturnNull(ResponseInterface $response): bool{ + private function is204NoContentResponse(ResponseInterface $response): bool{ return $response->getStatusCode() === 204; } } diff --git a/http/php/guzzle/tests/GuzzleRequestAdapterTest.php b/http/php/guzzle/tests/GuzzleRequestAdapterTest.php index b77d6ac50b..81fcf87de2 100644 --- a/http/php/guzzle/tests/GuzzleRequestAdapterTest.php +++ b/http/php/guzzle/tests/GuzzleRequestAdapterTest.php @@ -7,6 +7,7 @@ use GuzzleHttp\Psr7\Response; use GuzzleHttp\Psr7\Utils; use Http\Promise\FulfilledPromise; +use Microsoft\Kiota\Abstractions\ApiException; use Microsoft\Kiota\Abstractions\Authentication\AuthenticationProvider; use Microsoft\Kiota\Abstractions\RequestInformation; use Microsoft\Kiota\Abstractions\ResponseHandler; @@ -172,7 +173,7 @@ public function testSendPrimitiveCollectionAsyncWithResponseHandler(): void public function testSendNoContentAsync(): void { - $requestAdapter = $this->mockRequestAdapter([new Response(200, ['Content-Type' => 'application/json'])]); + $requestAdapter = $this->mockRequestAdapter([new Response(204, ['Content-Type' => 'application/json'])]); $promise = $requestAdapter->sendNoContentAsync($this->requestInformation); $this->assertNull($promise->wait()); } @@ -186,6 +187,13 @@ public function testSendNoContentAsyncWithResponseHandler(): void $requestAdapter->sendNoContentAsync($this->requestInformation, $customResponseHandler); } + public function testExceptionThrownOnErrorResponses(): void + { + $this->expectException(ApiException::class); + $requestAdapter = $this->mockRequestAdapter([new Response(400, ['Content-Type' => 'application/json'])]); + $requestAdapter->sendAsync($this->requestInformation, array(TestUser::class, 'createFromDiscriminatorValue'))->wait(); + } + } class TestUser implements Parsable { diff --git a/serialization/php/json/src/JsonParseNode.php b/serialization/php/json/src/JsonParseNode.php index aeaa90cb0a..5f6f26cbdc 100644 --- a/serialization/php/json/src/JsonParseNode.php +++ b/serialization/php/json/src/JsonParseNode.php @@ -105,12 +105,12 @@ public function getObjectValue(array $type): ?Parsable { } /** @var Parsable $result */ $result = $callableString($this); - if($this->onBeforeAssignFieldValues !== null) { - $this->onBeforeAssignFieldValues($result); + if($this->getOnBeforeAssignFieldValues() !== null) { + $this->getOnBeforeAssignFieldValues()($result); } $this->assignFieldValues($result); - if ($this->onAfterAssignFieldValues !== null){ - $this->onAfterAssignFieldValues($result); + if ($this->getOnAfterAssignFieldValues() !== null){ + $this->getOnAfterAssignFieldValues()($result); } return $result; } diff --git a/serialization/php/json/src/JsonSerializationWriter.php b/serialization/php/json/src/JsonSerializationWriter.php index f88497e08d..3b9fb461f2 100644 --- a/serialization/php/json/src/JsonSerializationWriter.php +++ b/serialization/php/json/src/JsonSerializationWriter.php @@ -9,7 +9,6 @@ use Microsoft\Kiota\Abstractions\Enum; use Microsoft\Kiota\Abstractions\Serialization\Parsable; use Microsoft\Kiota\Abstractions\Serialization\SerializationWriter; -use Microsoft\Kiota\Abstractions\Types\Byte; use Microsoft\Kiota\Abstractions\Types\Date; use Microsoft\Kiota\Abstractions\Types\Time; use Psr\Http\Message\StreamInterface; @@ -159,19 +158,19 @@ public function writeObjectValue(?string $key, $value): void { if(!empty($key)) { $this->writePropertyName($key); } - if ($this->onBeforeObjectSerialization !== null) { - $this->onBeforeObjectSerialization($value); + if ($this->getOnBeforeObjectSerialization() !== null) { + $this->getOnBeforeObjectSerialization()($value); } $this->writer [] = '{'; - if ($this->onStartObjectSerialization !== null) { - $this->onStartObjectSerialization($value, $this); + if ($this->getOnStartObjectSerialization() !== null) { + $this->getOnStartObjectSerialization()($value, $this); } $value->serialize($this); if ($this->writer[count($this->writer) - 1] === ',') { array_pop($this->writer); } - if ($this->onAfterObjectSerialization !== null) { - $this->onAfterObjectSerialization($value); + if ($this->getOnAfterObjectSerialization() !== null) { + $this->getOnAfterObjectSerialization()($value); } $this->writer [] = '}'; } @@ -190,22 +189,6 @@ public function getSerializedContent(): StreamInterface { return Utils::streamFor(implode('', $this->writer)); } - /** - * @inheritDoc - */ - public function writeEnumSetValue(?string $key, ?array $values): void { - if ($values !== null) { - if (!empty($key)) { - $this->writePropertyName($key); - } - $valS = []; - foreach ($values as $value){ - $valS []= $value->value(); - } - $this->writeStringValue($key, implode(',', $valS)); - } - } - /** * @inheritDoc */ diff --git a/serialization/php/json/tests/JsonParseNodeTest.php b/serialization/php/json/tests/JsonParseNodeTest.php index 09698a6c17..8694007b7f 100644 --- a/serialization/php/json/tests/JsonParseNodeTest.php +++ b/serialization/php/json/tests/JsonParseNodeTest.php @@ -164,4 +164,15 @@ public function testGetChildNode(): void { $this->assertEquals('Nairobi', $address->getCity()); } + + public function testCallbacksAreCalled(): void { + $this->parseNode = (new JsonParseNodeFactory())->getRootParseNode('application/json', $this->stream); + $assigned = false; + $onAfterAssignValues = function ($result) use (&$assigned) { + $assigned = true; + }; + $this->parseNode->setOnAfterAssignFieldValues($onAfterAssignValues); + $person = $this->parseNode->getObjectValue([Person::class, 'createFromDiscriminatorValue']); + $this->assertTrue($assigned); + } } diff --git a/serialization/php/text/src/TextParseNode.php b/serialization/php/text/src/TextParseNode.php index 96dd4291ba..04b9cb920b 100644 --- a/serialization/php/text/src/TextParseNode.php +++ b/serialization/php/text/src/TextParseNode.php @@ -15,7 +15,6 @@ use Microsoft\Kiota\Abstractions\Enum; use Microsoft\Kiota\Abstractions\Serialization\Parsable; use Microsoft\Kiota\Abstractions\Serialization\ParseNode; -use Microsoft\Kiota\Abstractions\Types\Byte; use Microsoft\Kiota\Abstractions\Types\Date; use Microsoft\Kiota\Abstractions\Types\Time; use Psr\Http\Message\StreamInterface; @@ -80,7 +79,8 @@ public function getStringValue(): ?string */ public function getBooleanValue(): ?bool { - return (bool) $this->content; + $boolMap = ["true" => true, "false" => false]; + return array_key_exists($this->content, $boolMap) ? $boolMap[$this->content] : null; } /** @@ -88,7 +88,7 @@ public function getBooleanValue(): ?bool */ public function getIntegerValue(): ?int { - return (int) $this->content; + return (int) filter_var($this->content, FILTER_SANITIZE_NUMBER_INT); } /** @@ -96,13 +96,13 @@ public function getIntegerValue(): ?int */ public function getFloatValue(): ?float { - return (float) $this->content; + return (float) filter_var($this->content, FILTER_SANITIZE_NUMBER_FLOAT, ['flags' => FILTER_FLAG_ALLOW_FRACTION]); } /** * @inheritDoc */ - public function getObjectValue(string $type): ?Parsable + public function getObjectValue(array $type): ?Parsable { throw new \RuntimeException(self::NO_STRUCTURED_DATA_ERR_MSG); } @@ -110,7 +110,15 @@ public function getObjectValue(string $type): ?Parsable /** * @inheritDoc */ - public function getCollectionOfObjectValues(string $type): ?array + public function getCollectionOfObjectValues(array $type): ?array + { + throw new \RuntimeException(self::NO_STRUCTURED_DATA_ERR_MSG); + } + + /** + * @inheritDoc + */ + public function getCollectionOfEnumValues(string $targetClass): ?array { throw new \RuntimeException(self::NO_STRUCTURED_DATA_ERR_MSG); } @@ -170,14 +178,6 @@ public function getEnumValue(string $targetEnum): ?Enum return new $targetEnum($this->content); } - /** - * @inheritDoc - */ - public function getByteValue(): ?Byte - { - return new Byte((int)$this->content); - } - /** * @inheritDoc */ diff --git a/serialization/php/text/src/TextSerializationWriter.php b/serialization/php/text/src/TextSerializationWriter.php index 4bc49192a8..01e151d8ea 100644 --- a/serialization/php/text/src/TextSerializationWriter.php +++ b/serialization/php/text/src/TextSerializationWriter.php @@ -15,7 +15,6 @@ use Microsoft\Kiota\Abstractions\Enum; use Microsoft\Kiota\Abstractions\Serialization\Parsable; use Microsoft\Kiota\Abstractions\Serialization\SerializationWriter; -use Microsoft\Kiota\Abstractions\Types\Byte; use Microsoft\Kiota\Abstractions\Types\Date; use Microsoft\Kiota\Abstractions\Types\Time; use Psr\Http\Message\StreamInterface; @@ -135,7 +134,7 @@ public function getSerializedContent(): StreamInterface /** * @inheritDoc */ - public function writeEnumSetValue(?string $key, ?array $values): void + public function writeCollectionOfEnumValues(?string $key, ?array $values): void { throw new \RuntimeException(TextParseNode::NO_STRUCTURED_DATA_ERR_MSG); } @@ -266,9 +265,4 @@ public function getOnStartObjectSerialization(): ?callable { return $this->onStartObjectSerialization; } - - public function writeByteValue(?string $key, ?Byte $value): void - { - $this->writeStringValue($key, (string) $value); - } } diff --git a/serialization/php/text/tests/TextParseNodeTest.php b/serialization/php/text/tests/TextParseNodeTest.php index 85423c2224..08db65995f 100644 --- a/serialization/php/text/tests/TextParseNodeTest.php +++ b/serialization/php/text/tests/TextParseNodeTest.php @@ -28,18 +28,18 @@ function testConstructorThrowsExceptionOnEmptyContent() /** * @dataProvider invalidMethodNamesProvider */ - function testInvalidMethodsThrowException(string $methodName) + function testInvalidMethodsThrowException(string $methodName, $args = '') { $this->expectException(\Exception::class); - call_user_func([$this->parseNode, $methodName], ''); + call_user_func([$this->parseNode, $methodName], $args); } public function invalidMethodNamesProvider(): array { return [ ['getChildNode'], - ['getObjectValue'], - ['getCollectionOfObjectValues'], + ['getObjectValue', ['Message', 'createFromDiscriminatorValue']], + ['getCollectionOfObjectValues', ['Message', 'createFromDiscriminatorValue']], ['getCollectionOfPrimitiveValues'] ]; } @@ -47,7 +47,9 @@ public function invalidMethodNamesProvider(): array function testValidMethods() { $this->assertEquals('content', (new TextParseNode('content'))->getStringValue()); - $this->assertTrue((new TextParseNode('true'))->getBooleanValue()); + $this->assertFalse((new TextParseNode('false'))->getBooleanValue()); + $number = utf8_decode('123'); // some graph $count endpoints return extra Unicode chars + $this->assertEquals(123, (new TextParseNode($number))->getIntegerValue()); $this->assertEquals(123, (new TextParseNode('123'))->getIntegerValue()); $this->assertEquals(1.23, (new TextParseNode('1.23'))->getFloatValue()); $this->assertInstanceOf(\DateTime::class, (new TextParseNode('2022-05-05 13:56'))->getDateTimeValue()); @@ -57,7 +59,6 @@ function testValidMethods() $this->assertInstanceOf(TestEnum::class, (new TextParseNode('valueA'))->getEnumValue(TestEnum::class)); $binaryContent = (new TextParseNode('content'))->getBinaryContent(); $this->assertEquals('content', $binaryContent->getContents()); - $this->assertInstanceOf(Byte::class, (new TextParseNode('235'))->getByteValue()); } } diff --git a/src/Kiota.Builder/Writers/Php/CodeMethodWriter.cs b/src/Kiota.Builder/Writers/Php/CodeMethodWriter.cs index 3f8fc6b3ba..23dd3083f6 100644 --- a/src/Kiota.Builder/Writers/Php/CodeMethodWriter.cs +++ b/src/Kiota.Builder/Writers/Php/CodeMethodWriter.cs @@ -521,7 +521,7 @@ protected string GetSendRequestMethodName(bool isVoid, bool isStream, bool isCol private static void WriteFactoryMethodBody(CodeMethod codeElement, LanguageWriter writer){ var parseNodeParameter = codeElement.Parameters.OfKind(CodeParameterKind.ParseNode); if(codeElement.ShouldWriteDiscriminatorSwitch && parseNodeParameter != null) { - writer.WriteLines($"$mappingValueNode = {parseNodeParameter.Name.ToFirstCharacterUpperCase()}::getChildNode(\"{codeElement.DiscriminatorPropertyName}\");", + writer.WriteLines($"$mappingValueNode = ${parseNodeParameter.Name.ToFirstCharacterLowerCase()}->getChildNode(\"{codeElement.DiscriminatorPropertyName}\");", "if ($mappingValueNode !== null) {"); writer.IncreaseIndent(); writer.WriteLines("$mappingValue = $mappingValueNode->getStringValue();"); diff --git a/tests/Kiota.Builder.Tests/Writers/Php/CodeMethodWriterTests.cs b/tests/Kiota.Builder.Tests/Writers/Php/CodeMethodWriterTests.cs index 174ac214a1..e8cad4956d 100644 --- a/tests/Kiota.Builder.Tests/Writers/Php/CodeMethodWriterTests.cs +++ b/tests/Kiota.Builder.Tests/Writers/Php/CodeMethodWriterTests.cs @@ -811,7 +811,7 @@ public void WriteFactoryMethod() languageWriter.Write(factoryMethod); var result = stringWriter.ToString(); Assert.Contains("case 'childModel': return new ChildModel();", result); - Assert.Contains("$mappingValueNode = ParseNode::getChildNode(\"@odata.type\");", result); + Assert.Contains("$mappingValueNode = $parseNode->getChildNode(\"@odata.type\");", result); } [Fact] public void WriteApiConstructor()