Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[poc] new low-level API client #357

Merged
merged 29 commits into from
Jan 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
d1e8d53
Add new minimalistic HttpClient
Art4 Jan 4, 2024
bc105b3
Allow all HTTP methods in HttpClient
Art4 Jan 4, 2024
acdd0c5
Remove inappropriate covers phpdoc tags
Art4 Jan 5, 2024
88d41fb
Extracts tests for AbstractApi::get() into separate test file
Art4 Jan 5, 2024
7753792
Use HttpClient in AbstractApi::get()
Art4 Jan 5, 2024
5ece3f9
Use HttpClient in AbstractApi::post()
Art4 Jan 5, 2024
d51284e
restrict types in AbstractApi::__construct()
Art4 Jan 8, 2024
edfcdc6
Fix code style
Art4 Jan 8, 2024
10e1c9b
Use HttpClient in AbstractApi::put()
Art4 Jan 8, 2024
3a7f905
move xml decoding tests into separate test files
Art4 Jan 8, 2024
71d8eb8
Use HttpClient in AbstractApi::delete()
Art4 Jan 8, 2024
0394eb7
Use HttpClient in AbstractApi::retrieveData()
Art4 Jan 8, 2024
ee431a6
Use HttpClient in AbstractApi::retrieveAll()
Art4 Jan 8, 2024
a9599a6
Use HttpClient in AbstractApi::lastCallFailed()
Art4 Jan 8, 2024
16c0321
Add tests to prevent race conditions
Art4 Jan 8, 2024
dbbd7b2
do not use Client in deprecated all() methods
Art4 Jan 8, 2024
094fd41
Fix tests
Art4 Jan 8, 2024
e0f8fcb
fix for PHPUnit 9
Art4 Jan 8, 2024
40d0a3a
increase code coverage in AbstractApi::lastCallFailed()
Art4 Jan 8, 2024
c27b38e
Use HttpClient in Issue::setIssueStatus()
Art4 Jan 8, 2024
b2cc800
Remove tests moved already to DeleteTest class
Art4 Jan 8, 2024
b649237
improve test script
Art4 Jan 8, 2024
d50b2bd
Use IssueStatus Api in IssueApi
Art4 Jan 9, 2024
8318db9
Use Project Api in IssueApi
Art4 Jan 9, 2024
8bf399b
Use IssueCategory Api in IssueApi
Art4 Jan 9, 2024
bdd04df
Use Tracker Api in IssueApi
Art4 Jan 9, 2024
3e15c73
Use User Api in IssueApi
Art4 Jan 9, 2024
4312039
Fix typo
Art4 Jan 9, 2024
63dfce8
simplify client mock
Art4 Jan 9, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
7 changes: 6 additions & 1 deletion composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"
]
}
}
173 changes: 147 additions & 26 deletions src/Redmine/Api/AbstractApi.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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, '', '');
}

/**
Expand All @@ -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;
}
Expand All @@ -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')) {
Expand All @@ -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);
}

Expand All @@ -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);
}

Expand All @@ -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();
}

/**
Expand Down Expand Up @@ -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;
}

Expand All @@ -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(
Expand All @@ -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);

Expand Down Expand Up @@ -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
Expand All @@ -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;
}
};
}
}
2 changes: 1 addition & 1 deletion src/Redmine/Api/CustomField.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
2 changes: 1 addition & 1 deletion src/Redmine/Api/Group.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
Loading
Loading