PHP - End-to-End testing bug fixes

.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

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
 

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

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<Enum>|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.

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;
     }
 }

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 {

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;
     }

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
      */

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);
+    }
 }