Merge pull request #357 from Art4/add-httpclient

[poc] new low-level API client

Author: Art4

CHANGELOG.md

@@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased](https://github.com/kbsali/php-redmine-api/compare/v2.4.0...v2.x)
 
+### Changed
+
+- The last response is saved in `Redmine\Api\AbstractApi` to prevent race conditions with `Redmine\Client\Client` implementations.
+
 ## [v2.4.0](https://github.com/kbsali/php-redmine-api/compare/v2.3.0...v2.4.0) - 2024-01-04
 
 ### Added

composer.json

@@ -49,6 +49,11 @@
         "codestyle": "php-cs-fixer fix",
         "coverage": "phpunit --coverage-html=\".phpunit.cache/code-coverage\"",
         "phpstan": "phpstan analyze --memory-limit 512M --configuration .phpstan.neon",
-        "test": "phpunit"
+        "phpunit": "phpunit",
+        "test": [
+            "@phpstan",
+            "@phpunit",
+            "@codestyle --dry-run --diff"
+        ]
     }
 }

src/Redmine/Api/AbstractApi.php

@@ -2,10 +2,14 @@
 
 namespace Redmine\Api;
 
+use Closure;
+use InvalidArgumentException;
 use Redmine\Api;
 use Redmine\Client\Client;
 use Redmine\Exception;
 use Redmine\Exception\SerializerException;
+use Redmine\Http\HttpClient;
+use Redmine\Http\Response;
 use Redmine\Serializer\JsonSerializer;
 use Redmine\Serializer\PathSerializer;
 use Redmine\Serializer\XmlSerializer;
@@ -23,9 +27,52 @@ abstract class AbstractApi implements Api
      */
     protected $client;
 
-    public function __construct(Client $client)
+    /**
+     * @var HttpClient
+     */
+    private $httpClient;
+
+    /**
+     * @var Response
+     */
+    private $lastResponse;
+
+    /**
+     * @param Client|HttpClient $client
+     */
+    public function __construct($client)
     {
-        $this->client = $client;
+        if (! is_object($client) || (! $client instanceof Client && ! $client instanceof HttpClient)) {
+            throw new InvalidArgumentException(sprintf(
+                '%s(): Argument #1 ($client) must be of type %s or %s, `%s` given',
+                __METHOD__,
+                Client::class,
+                HttpClient::class,
+                (is_object($client)) ? get_class($client) : gettype($client)
+            ));
+        }
+
+        if ($client instanceof Client) {
+            $this->client = $client;
+        }
+
+        $httpClient = $client;
+
+        if (! $httpClient instanceof HttpClient) {
+            $httpClient = $this->handleClient($client);
+        }
+
+        $this->httpClient = $httpClient;
+    }
+
+    final protected function getHttpClient(): HttpClient
+    {
+        return $this->httpClient;
+    }
+
+    final protected function getLastResponse(): Response
+    {
+        return $this->lastResponse !== null ? $this->lastResponse : $this->createResponse(0, '', '');
     }
 
     /**
@@ -40,7 +87,13 @@ public function lastCallFailed()
     {
         @trigger_error('`' . __METHOD__ . '()` is deprecated since v2.1.0, use \Redmine\Client\Client::getLastResponseStatusCode() instead.', E_USER_DEPRECATED);
 
-        $code = $this->client->getLastResponseStatusCode();
+        if ($this->lastResponse !== null) {
+            $code = $this->lastResponse->getStatusCode();
+        } elseif ($this->client !== null) {
+            $code = $this->client->getLastResponseStatusCode();
+        } else {
+            $code = 0;
+        }
 
         return 200 !== $code && 201 !== $code;
     }
@@ -55,10 +108,10 @@ public function lastCallFailed()
      */
     protected function get($path, $decodeIfJson = true)
     {
-        $this->client->requestGet(strval($path));
+        $this->lastResponse = $this->getHttpClient()->request('GET', strval($path));
 
-        $body = $this->client->getLastResponseBody();
-        $contentType = $this->client->getLastResponseContentType();
+        $body = $this->lastResponse->getBody();
+        $contentType = $this->lastResponse->getContentType();
 
         // if response is XML, return a SimpleXMLElement object
         if ('' !== $body && 0 === strpos($contentType, 'application/xml')) {
@@ -82,16 +135,17 @@ protected function get($path, $decodeIfJson = true)
      * @param string $path
      * @param string $data
      *
-     * @return string|false
+     * @return string|SimpleXMLElement|false
      */
     protected function post($path, $data)
     {
-        $this->client->requestPost($path, $data);
+        $this->lastResponse = $this->getHttpClient()->request('POST', strval($path), $data);
 
-        $body = $this->client->getLastResponseBody();
+        $body = $this->lastResponse->getBody();
+        $contentType = $this->lastResponse->getContentType();
 
         // if response is XML, return a SimpleXMLElement object
-        if ('' !== $body && 0 === strpos($this->client->getLastResponseContentType(), 'application/xml')) {
+        if ('' !== $body && 0 === strpos($contentType, 'application/xml')) {
             return new SimpleXMLElement($body);
         }
 
@@ -104,16 +158,17 @@ protected function post($path, $data)
      * @param string $path
      * @param string $data
      *
-     * @return string|false
+     * @return string|SimpleXMLElement
      */
     protected function put($path, $data)
     {
-        $this->client->requestPut($path, $data);
+        $this->lastResponse = $this->getHttpClient()->request('PUT', strval($path), $data);
 
-        $body = $this->client->getLastResponseBody();
+        $body = $this->lastResponse->getBody();
+        $contentType = $this->lastResponse->getContentType();
 
         // if response is XML, return a SimpleXMLElement object
-        if ('' !== $body && 0 === strpos($this->client->getLastResponseContentType(), 'application/xml')) {
+        if ('' !== $body && 0 === strpos($contentType, 'application/xml')) {
             return new SimpleXMLElement($body);
         }
 
@@ -125,13 +180,13 @@ protected function put($path, $data)
      *
      * @param string $path
      *
-     * @return false|SimpleXMLElement|string
+     * @return string
      */
     protected function delete($path)
     {
-        $this->client->requestDelete($path);
+        $this->lastResponse = $this->getHttpClient()->request('DELETE', strval($path));
 
-        return $this->client->getLastResponseBody();
+        return $this->lastResponse->getBody();
     }
 
     /**
@@ -179,7 +234,7 @@ protected function retrieveAll($endpoint, array $params = [])
         try {
             $data = $this->retrieveData(strval($endpoint), $params);
         } catch (Exception $e) {
-            if ($this->client->getLastResponseBody() === '') {
+            if ($this->getLastResponse()->getBody() === '') {
                 return false;
             }
 
@@ -203,9 +258,9 @@ protected function retrieveAll($endpoint, array $params = [])
     protected function retrieveData(string $endpoint, array $params = []): array
     {
         if (empty($params)) {
-            $this->client->requestGet($endpoint);
+            $this->lastResponse = $this->getHttpClient()->request('GET', strval($endpoint));
 
-            return $this->getLastResponseBodyAsArray();
+            return $this->getResponseAsArray($this->lastResponse);
         }
 
         $params = $this->sanitizeParams(
@@ -232,11 +287,12 @@ protected function retrieveData(string $endpoint, array $params = []): array
             $params['limit'] = $_limit;
             $params['offset'] = $offset;
 
-            $this->client->requestGet(
+            $this->lastResponse = $this->getHttpClient()->request(
+                'GET',
                 PathSerializer::create($endpoint, $params)->getPath()
             );
 
-            $newDataSet = $this->getLastResponseBodyAsArray();
+            $newDataSet = $this->getResponseAsArray($this->lastResponse);
 
             $returnData = array_merge_recursive($returnData, $newDataSet);
 
@@ -310,11 +366,10 @@ protected function attachCustomFieldXML(SimpleXMLElement $xml, array $fields)
      *
      * @throws SerializerException if response body could not be converted into array
      */
-    private function getLastResponseBodyAsArray(): array
+    private function getResponseAsArray(Response $response): array
     {
-        $body = $this->client->getLastResponseBody();
-
-        $contentType = $this->client->getLastResponseContentType();
+        $body = $response->getBody();
+        $contentType = $response->getContentType();
         $returnData = null;
 
         // parse XML
@@ -330,4 +385,70 @@ private function getLastResponseBodyAsArray(): array
 
         return $returnData;
     }
+
+    private function handleClient(Client $client): HttpClient
+    {
+        $responseFactory = Closure::fromCallable([$this, 'createResponse']);
+
+        return new class ($client, $responseFactory) implements HttpClient {
+            private $client;
+            private $responseFactory;
+
+            public function __construct(Client $client, Closure $responseFactory)
+            {
+                $this->client = $client;
+                $this->responseFactory = $responseFactory;
+            }
+
+            public function request(string $method, string $path, string $body = ''): Response
+            {
+                if ($method === 'POST') {
+                    $this->client->requestPost($path, $body);
+                } elseif ($method === 'PUT') {
+                    $this->client->requestPut($path, $body);
+                } elseif ($method === 'DELETE') {
+                    $this->client->requestDelete($path);
+                } else {
+                    $this->client->requestGet($path);
+                }
+
+                return ($this->responseFactory)(
+                    $this->client->getLastResponseStatusCode(),
+                    $this->client->getLastResponseContentType(),
+                    $this->client->getLastResponseBody()
+                );
+            }
+        };
+    }
+
+    private function createResponse(int $statusCode, string $contentType, string $body): Response
+    {
+        return new class ($statusCode, $contentType, $body) implements Response {
+            private $statusCode;
+            private $contentType;
+            private $body;
+
+            public function __construct(int $statusCode, string $contentType, string $body)
+            {
+                $this->statusCode = $statusCode;
+                $this->contentType = $contentType;
+                $this->body = $body;
+            }
+
+            public function getStatusCode(): int
+            {
+                return $this->statusCode;
+            }
+
+            public function getContentType(): string
+            {
+                return $this->contentType;
+            }
+
+            public function getBody(): string
+            {
+                return $this->body;
+            }
+        };
+    }
 }

src/Redmine/Api/CustomField.php

@@ -55,7 +55,7 @@ public function all(array $params = [])
         try {
             $this->customFields = $this->list($params);
         } catch (Exception $e) {
-            if ($this->client->getLastResponseBody() === '') {
+            if ($this->getLastResponse()->getBody() === '') {
                 return false;
             }
 

src/Redmine/Api/Group.php

@@ -58,7 +58,7 @@ public function all(array $params = [])
         try {
             $this->groups = $this->list($params);
         } catch (Exception $e) {
-            if ($this->client->getLastResponseBody() === '') {
+            if ($this->getLastResponse()->getBody() === '') {
                 return false;
             }