From d1e8d53b42af60f3ca05e29f0d7a2af413652291 Mon Sep 17 00:00:00 2001 From: Art4 Date: Thu, 4 Jan 2024 16:29:41 +0100 Subject: [PATCH 01/29] Add new minimalistic HttpClient --- src/Redmine/Exception/ClientException.php | 5 +++ .../Exception/InvalidHttpMethodException.php | 10 ++++++ src/Redmine/Http/HttpClient.php | 33 +++++++++++++++++++ src/Redmine/Http/Response.php | 32 ++++++++++++++++++ 4 files changed, 80 insertions(+) create mode 100644 src/Redmine/Exception/InvalidHttpMethodException.php create mode 100644 src/Redmine/Http/HttpClient.php create mode 100644 src/Redmine/Http/Response.php diff --git a/src/Redmine/Exception/ClientException.php b/src/Redmine/Exception/ClientException.php index 426b227d..c81bc3f1 100644 --- a/src/Redmine/Exception/ClientException.php +++ b/src/Redmine/Exception/ClientException.php @@ -5,4 +5,9 @@ use Exception; use Redmine\Exception as RedmineException; +/** + * Client exception. + * + * Will be thrown if anything goes wrong on creating or sending a HTTP request + */ class ClientException extends Exception implements RedmineException {} diff --git a/src/Redmine/Exception/InvalidHttpMethodException.php b/src/Redmine/Exception/InvalidHttpMethodException.php new file mode 100644 index 00000000..8aa5fc74 --- /dev/null +++ b/src/Redmine/Exception/InvalidHttpMethodException.php @@ -0,0 +1,10 @@ + Date: Thu, 4 Jan 2024 16:56:44 +0100 Subject: [PATCH 02/29] Allow all HTTP methods in HttpClient --- .../Exception/InvalidHttpMethodException.php | 10 ---------- src/Redmine/Http/HttpClient.php | 19 +++++++------------ 2 files changed, 7 insertions(+), 22 deletions(-) delete mode 100644 src/Redmine/Exception/InvalidHttpMethodException.php diff --git a/src/Redmine/Exception/InvalidHttpMethodException.php b/src/Redmine/Exception/InvalidHttpMethodException.php deleted file mode 100644 index 8aa5fc74..00000000 --- a/src/Redmine/Exception/InvalidHttpMethodException.php +++ /dev/null @@ -1,10 +0,0 @@ - Date: Fri, 5 Jan 2024 15:02:21 +0100 Subject: [PATCH 03/29] Remove inappropriate covers phpdoc tags --- src/Redmine/Api/AbstractApi.php | 6 +++--- tests/Unit/Api/AttachmentTest.php | 2 -- tests/Unit/Api/CustomFieldTest.php | 12 ------------ tests/Unit/Api/GroupTest.php | 2 -- tests/Unit/Api/IssueCategoryTest.php | 1 - tests/Unit/Api/IssueRelationTest.php | 2 -- tests/Unit/Api/IssueTest.php | 2 -- tests/Unit/Api/ProjectTest.php | 2 -- tests/Unit/Api/RoleTest.php | 1 - tests/Unit/Api/TimeEntryTest.php | 1 - tests/Unit/Api/UserTest.php | 2 -- tests/Unit/Api/VersionTest.php | 2 -- tests/Unit/Api/WikiTest.php | 4 ---- 13 files changed, 3 insertions(+), 36 deletions(-) diff --git a/src/Redmine/Api/AbstractApi.php b/src/Redmine/Api/AbstractApi.php index 281a90f7..4ec1c082 100644 --- a/src/Redmine/Api/AbstractApi.php +++ b/src/Redmine/Api/AbstractApi.php @@ -82,7 +82,7 @@ protected function get($path, $decodeIfJson = true) * @param string $path * @param string $data * - * @return string|false + * @return string|SimpleXMLElement|false */ protected function post($path, $data) { @@ -104,7 +104,7 @@ protected function post($path, $data) * @param string $path * @param string $data * - * @return string|false + * @return string|SimpleXMLElement|false */ protected function put($path, $data) { @@ -125,7 +125,7 @@ protected function put($path, $data) * * @param string $path * - * @return false|SimpleXMLElement|string + * @return string */ protected function delete($path) { diff --git a/tests/Unit/Api/AttachmentTest.php b/tests/Unit/Api/AttachmentTest.php index 5bd3df46..dff94bbe 100644 --- a/tests/Unit/Api/AttachmentTest.php +++ b/tests/Unit/Api/AttachmentTest.php @@ -61,7 +61,6 @@ public static function responseCodeProvider(): array /** * Test show(). * - * @covers ::get * @covers ::show * @test */ @@ -90,7 +89,6 @@ public function testShowReturnsClientGetResponse() /** * Test download(). * - * @covers ::get * @covers ::download * @test */ diff --git a/tests/Unit/Api/CustomFieldTest.php b/tests/Unit/Api/CustomFieldTest.php index dba2bd2d..030a4915 100644 --- a/tests/Unit/Api/CustomFieldTest.php +++ b/tests/Unit/Api/CustomFieldTest.php @@ -44,9 +44,6 @@ function ($errno, $errstr): bool { * Test all(). * * @covers ::all - * @covers ::get - * @covers ::retrieveAll - * @covers ::isNotNull * @dataProvider getAllData * @test */ @@ -85,9 +82,6 @@ public static function getAllData(): array * Test all(). * * @covers ::all - * @covers ::get - * @covers ::retrieveAll - * @covers ::isNotNull * @test */ public function testAllReturnsClientGetResponseWithParameters() @@ -123,9 +117,6 @@ public function testAllReturnsClientGetResponseWithParameters() * Test all(). * * @covers ::all - * @covers ::get - * @covers ::retrieveAll - * @covers ::isNotNull * @test */ public function testAllReturnsClientGetResponseWithHighLimit() @@ -164,9 +155,6 @@ public function testAllReturnsClientGetResponseWithHighLimit() * Test all(). * * @covers ::all - * @covers ::get - * @covers ::retrieveAll - * @covers ::isNotNull * @test */ public function testAllCallsEndpointUntilOffsetIsHigherThanTotalCount() diff --git a/tests/Unit/Api/GroupTest.php b/tests/Unit/Api/GroupTest.php index d6081c6d..77f70f08 100644 --- a/tests/Unit/Api/GroupTest.php +++ b/tests/Unit/Api/GroupTest.php @@ -233,7 +233,6 @@ public function testListingCallsGetEveryTimeWithForceUpdate() /** * Test show(). * - * @covers ::get * @covers ::show * @test */ @@ -266,7 +265,6 @@ public function testShowReturnsClientGetResponse() /** * Test show(). * - * @covers ::get * @covers ::show * @test */ diff --git a/tests/Unit/Api/IssueCategoryTest.php b/tests/Unit/Api/IssueCategoryTest.php index d9cba882..43dc5ae2 100644 --- a/tests/Unit/Api/IssueCategoryTest.php +++ b/tests/Unit/Api/IssueCategoryTest.php @@ -237,7 +237,6 @@ public function testListingCallsGetEveryTimeWithForceUpdate() /** * Test show(). * - * @covers ::get * @covers ::show * @test */ diff --git a/tests/Unit/Api/IssueRelationTest.php b/tests/Unit/Api/IssueRelationTest.php index a75e3708..da548e53 100644 --- a/tests/Unit/Api/IssueRelationTest.php +++ b/tests/Unit/Api/IssueRelationTest.php @@ -119,7 +119,6 @@ public function testAllReturnsClientGetResponseWithParametersAndProject() /** * Test show(). * - * @covers ::get * @covers ::show * @test */ @@ -154,7 +153,6 @@ public function testShowReturnsClientGetResponse() /** * Test show(). * - * @covers ::get * @covers ::show * @test */ diff --git a/tests/Unit/Api/IssueTest.php b/tests/Unit/Api/IssueTest.php index 50390650..63fc9239 100644 --- a/tests/Unit/Api/IssueTest.php +++ b/tests/Unit/Api/IssueTest.php @@ -142,7 +142,6 @@ public function testAllReturnsClientGetResponseWithParameters() /** * Test show(). * - * @covers ::get * @covers ::show * @test */ @@ -175,7 +174,6 @@ public function testShowReturnsClientGetResponse() /** * Test show(). * - * @covers ::get * @covers ::show * @test */ diff --git a/tests/Unit/Api/ProjectTest.php b/tests/Unit/Api/ProjectTest.php index 1d177306..de28a6c6 100644 --- a/tests/Unit/Api/ProjectTest.php +++ b/tests/Unit/Api/ProjectTest.php @@ -122,7 +122,6 @@ public function testAllReturnsClientGetResponseWithParameters() /** * Test show(). * - * @covers ::get * @covers ::show * @test */ @@ -158,7 +157,6 @@ public function testShowReturnsClientGetResponse() /** * Test show(). * - * @covers ::get * @covers ::show * @test */ diff --git a/tests/Unit/Api/RoleTest.php b/tests/Unit/Api/RoleTest.php index 38022ceb..381042fe 100644 --- a/tests/Unit/Api/RoleTest.php +++ b/tests/Unit/Api/RoleTest.php @@ -232,7 +232,6 @@ public function testListingCallsGetEveryTimeWithForceUpdate() /** * Test show(). * - * @covers ::get * @covers ::show * @test */ diff --git a/tests/Unit/Api/TimeEntryTest.php b/tests/Unit/Api/TimeEntryTest.php index abf7c33a..8b01c32a 100644 --- a/tests/Unit/Api/TimeEntryTest.php +++ b/tests/Unit/Api/TimeEntryTest.php @@ -126,7 +126,6 @@ public function testAllReturnsClientGetResponseWithParameters() /** * Test show(). * - * @covers ::get * @covers ::show * @test */ diff --git a/tests/Unit/Api/UserTest.php b/tests/Unit/Api/UserTest.php index 98b65d84..33d0d767 100644 --- a/tests/Unit/Api/UserTest.php +++ b/tests/Unit/Api/UserTest.php @@ -198,7 +198,6 @@ public function testAllReturnsClientGetResponseWithParameters() /** * Test show(). * - * @covers ::get * @covers ::show * @test */ @@ -236,7 +235,6 @@ public function testShowReturnsClientGetResponse() /** * Test show(). * - * @covers ::get * @covers ::show * @test */ diff --git a/tests/Unit/Api/VersionTest.php b/tests/Unit/Api/VersionTest.php index e05bc9d2..4f4ffa1f 100644 --- a/tests/Unit/Api/VersionTest.php +++ b/tests/Unit/Api/VersionTest.php @@ -125,7 +125,6 @@ public function testAllReturnsClientGetResponseWithParameters() /** * Test show(). * - * @covers ::get * @covers ::show * @test */ @@ -158,7 +157,6 @@ public function testShowWithNumericIdReturnsClientGetResponse() /** * Test show(). * - * @covers ::get * @covers ::show * @test */ diff --git a/tests/Unit/Api/WikiTest.php b/tests/Unit/Api/WikiTest.php index 65b4bd95..2ac36674 100644 --- a/tests/Unit/Api/WikiTest.php +++ b/tests/Unit/Api/WikiTest.php @@ -123,7 +123,6 @@ public function testAllReturnsClientGetResponseWithParameters() /** * Test show(). * - * @covers ::get * @covers ::show * @test */ @@ -152,7 +151,6 @@ public function testShowWithNumericIdsReturnsClientGetResponse() /** * Test show(). * - * @covers ::get * @covers ::show * @test */ @@ -181,7 +179,6 @@ public function testShowWithIdentifierReturnsClientGetResponse() /** * Test show(). * - * @covers ::get * @covers ::show * @test */ @@ -210,7 +207,6 @@ public function testShowWithNumericIdsAndVersionReturnsClientGetResponse() /** * Test show(). * - * @covers ::get * @covers ::show * @test */ From 88d41fbc8ec54dfa6ef1c3e42ed401ed0d0e0e6f Mon Sep 17 00:00:00 2001 From: Art4 Date: Fri, 5 Jan 2024 15:11:03 +0100 Subject: [PATCH 04/29] Extracts tests for AbstractApi::get() into separate test file --- tests/Unit/Api/AbstractApi/GetTest.php | 83 ++++++++++++++++++++++++++ tests/Unit/Api/AbstractApiTest.php | 46 +------------- 2 files changed, 84 insertions(+), 45 deletions(-) create mode 100644 tests/Unit/Api/AbstractApi/GetTest.php diff --git a/tests/Unit/Api/AbstractApi/GetTest.php b/tests/Unit/Api/AbstractApi/GetTest.php new file mode 100644 index 00000000..a176ef66 --- /dev/null +++ b/tests/Unit/Api/AbstractApi/GetTest.php @@ -0,0 +1,83 @@ +createMock(Client::class); + $client->method('getLastResponseBody')->willReturn($response); + $client->method('getLastResponseContentType')->willReturn('application/json'); + + $api = new class ($client) extends AbstractApi {}; + + $method = new ReflectionMethod($api, 'get'); + $method->setAccessible(true); + + // Perform the tests + if (is_bool($decode)) { + $this->assertSame($expected, $method->invoke($api, 'path', $decode)); + } else { + $this->assertSame($expected, $method->invoke($api, 'path')); + } + } + + public static function getJsonDecodingFromGetMethodData(): array + { + return [ + 'test decode by default' => ['{"foo_bar": 12345}', null, ['foo_bar' => 12345]], + 'test decode by default, JSON decode: false' => ['{"foo_bar": 12345}', false, '{"foo_bar": 12345}'], + 'test decode by default, JSON decode: true' => ['{"foo_bar": 12345}', true, ['foo_bar' => 12345]], + 'Empty body, JSON decode: false' => ['', false, false], + 'Empty body, JSON decode: true' => ['', true, false], + 'test invalid JSON' => ['{"foo_bar":', true, 'Error decoding body as JSON: Syntax error'], + ]; + } + + /** + * @covers \Redmine\Api\AbstractApi + * @test + * @dataProvider getXmlDecodingFromGetMethodData + */ + public function testXmlDecodingFromRequestMethods($response, $decode, $expected) + { + $client = $this->createMock(Client::class); + $client->method('getLastResponseBody')->willReturn($response); + $client->method('getLastResponseContentType')->willReturn('application/xml'); + + $api = new class ($client) extends AbstractApi {}; + + $method = new ReflectionMethod($api, 'get'); + $method->setAccessible(true); + + // Perform the tests + $return = $method->invoke($api, 'path', $decode); + + $this->assertInstanceOf(SimpleXMLElement::class, $return); + $this->assertXmlStringEqualsXmlString($expected, $return->asXML()); + } + + public static function getXmlDecodingFromGetMethodData(): array + { + return [ + ['', null, ''], // test decode by default + ['', true, ''], + ['', false, ''], // test that xml decoding will be always happen + ]; + } +} diff --git a/tests/Unit/Api/AbstractApiTest.php b/tests/Unit/Api/AbstractApiTest.php index 42e6eb21..bba98a15 100644 --- a/tests/Unit/Api/AbstractApiTest.php +++ b/tests/Unit/Api/AbstractApiTest.php @@ -137,42 +137,6 @@ public static function getLastCallFailedData(): array ]; } - /** - * @covers \Redmine\Api\AbstractApi - * @test - * @dataProvider getJsonDecodingFromGetMethodData - */ - public function testJsonDecodingFromGetMethod($response, $decode, $expected) - { - $client = $this->createMock(Client::class); - $client->method('getLastResponseBody')->willReturn($response); - $client->method('getLastResponseContentType')->willReturn('application/json'); - - $api = new class ($client) extends AbstractApi {}; - - $method = new ReflectionMethod($api, 'get'); - $method->setAccessible(true); - - // Perform the tests - if (is_bool($decode)) { - $this->assertSame($expected, $method->invoke($api, 'path', $decode)); - } else { - $this->assertSame($expected, $method->invoke($api, 'path')); - } - } - - public static function getJsonDecodingFromGetMethodData(): array - { - return [ - 'test decode by default' => ['{"foo_bar": 12345}', null, ['foo_bar' => 12345]], - 'test decode by default, JSON decode: false' => ['{"foo_bar": 12345}', false, '{"foo_bar": 12345}'], - 'test decode by default, JSON decode: true' => ['{"foo_bar": 12345}', true, ['foo_bar' => 12345]], - 'Empty body, JSON decode: false' => ['', false, false], - 'Empty body, JSON decode: true' => ['', true, false], - 'test invalid JSON' => ['{"foo_bar":', true, 'Error decoding body as JSON: Syntax error'], - ]; - } - /** * @covers \Redmine\Api\AbstractApi * @test @@ -190,12 +154,7 @@ public function testXmlDecodingFromRequestMethods($methodName, $response, $decod $method->setAccessible(true); // Perform the tests - if ('get' === $methodName) { - $return = $method->invoke($api, 'path', $decode); - - $this->assertInstanceOf(SimpleXMLElement::class, $return); - $this->assertXmlStringEqualsXmlString($expected, $return->asXML()); - } elseif ('delete' === $methodName) { + if ('delete' === $methodName) { $return = $method->invoke($api, 'path'); $this->assertSame($expected, $return); @@ -210,9 +169,6 @@ public function testXmlDecodingFromRequestMethods($methodName, $response, $decod public static function getXmlDecodingFromGetMethodData(): array { return [ - ['get', '', null, ''], // test decode by default - ['get', '', true, ''], - ['get', '', false, ''], // test that xml decoding will be always happen ['post', '', null, ''], ['put', '', null, ''], ['delete', '', null, ''], From 7753792ae9e815405830f631be854be77bb01087 Mon Sep 17 00:00:00 2001 From: Art4 Date: Fri, 5 Jan 2024 16:07:03 +0100 Subject: [PATCH 05/29] Use HttpClient in AbstractApi::get() --- src/Redmine/Api/AbstractApi.php | 77 ++++++++++++++++++++++++-- tests/Unit/Api/AbstractApi/GetTest.php | 24 ++++++++ 2 files changed, 96 insertions(+), 5 deletions(-) diff --git a/src/Redmine/Api/AbstractApi.php b/src/Redmine/Api/AbstractApi.php index 4ec1c082..b2099d3f 100644 --- a/src/Redmine/Api/AbstractApi.php +++ b/src/Redmine/Api/AbstractApi.php @@ -6,6 +6,8 @@ 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 +25,29 @@ abstract class AbstractApi implements Api */ protected $client; - public function __construct(Client $client) + /** + * @var HttpClient + */ + private $httpClient; + + public function __construct($client) { - $this->client = $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; } /** @@ -55,10 +77,10 @@ public function lastCallFailed() */ protected function get($path, $decodeIfJson = true) { - $this->client->requestGet(strval($path)); + $response = $this->getHttpClient()->request('GET', strval($path)); - $body = $this->client->getLastResponseBody(); - $contentType = $this->client->getLastResponseContentType(); + $body = $response->getBody(); + $contentType = $response->getContentType(); // if response is XML, return a SimpleXMLElement object if ('' !== $body && 0 === strpos($contentType, 'application/xml')) { @@ -330,4 +352,49 @@ private function getLastResponseBodyAsArray(): array return $returnData; } + + private function handleClient(Client $client): HttpClient + { + return new class($client) implements HttpClient { + private $client; + + public function __construct(Client $client) + { + $this->client = $client; + } + + public function request(string $method, string $path, string $body = ''): Response + { + $this->client->requestGet($path); + + return new class($this->client->getLastResponseStatusCode(), $this->client->getLastResponseContentType(), $this->client->getLastResponseBody()) 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; + } + }; + } + }; + } } diff --git a/tests/Unit/Api/AbstractApi/GetTest.php b/tests/Unit/Api/AbstractApi/GetTest.php index a176ef66..cf2d786a 100644 --- a/tests/Unit/Api/AbstractApi/GetTest.php +++ b/tests/Unit/Api/AbstractApi/GetTest.php @@ -7,6 +7,8 @@ use PHPUnit\Framework\TestCase; use Redmine\Api\AbstractApi; use Redmine\Client\Client; +use Redmine\Http\HttpClient; +use Redmine\Http\Response; use ReflectionMethod; use SimpleXMLElement; @@ -15,6 +17,28 @@ */ class GetTest extends TestCase { + public function testGetWithHttpClient() + { + $response = $this->createMock(Response::class); + $response->expects($this->any())->method('getStatusCode')->willReturn(200); + $response->expects($this->any())->method('getContentType')->willReturn('application/json'); + $response->expects($this->any())->method('getBody')->willReturn('{"foo_bar": 12345}'); + + $client = $this->createMock(HttpClient::class); + $client->expects($this->exactly(1))->method('request')->with('GET', 'path.json')->willReturn($response); + + $api = new class ($client) extends AbstractApi {}; + + $method = new ReflectionMethod($api, 'get'); + $method->setAccessible(true); + + // Perform the tests + $this->assertSame( + ['foo_bar' => 12345], + $method->invoke($api, 'path.json') + ); + } + /** * @dataProvider getJsonDecodingFromGetMethodData */ From 5ece3f9b99cae780a780dc30caf1d27b2cfacb8d Mon Sep 17 00:00:00 2001 From: Art4 Date: Fri, 5 Jan 2024 16:31:05 +0100 Subject: [PATCH 06/29] Use HttpClient in AbstractApi::post() --- src/Redmine/Api/AbstractApi.php | 24 ++++++++++++--- tests/Unit/Api/AbstractApi/PostTest.php | 41 +++++++++++++++++++++++++ tests/Unit/Api/AbstractApiTest.php | 4 +-- 3 files changed, 62 insertions(+), 7 deletions(-) create mode 100644 tests/Unit/Api/AbstractApi/PostTest.php diff --git a/src/Redmine/Api/AbstractApi.php b/src/Redmine/Api/AbstractApi.php index b2099d3f..141fb67a 100644 --- a/src/Redmine/Api/AbstractApi.php +++ b/src/Redmine/Api/AbstractApi.php @@ -108,12 +108,13 @@ protected function get($path, $decodeIfJson = true) */ protected function post($path, $data) { - $this->client->requestPost($path, $data); + $response = $this->getHttpClient()->request('POST', strval($path), $data); - $body = $this->client->getLastResponseBody(); + $body = $response->getBody(); + $contentType = $response->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); } @@ -365,9 +366,22 @@ public function __construct(Client $client) public function request(string $method, string $path, string $body = ''): Response { - $this->client->requestGet($path); + if ($method === 'GET') { + $this->client->requestGet($path); + } else { + $this->client->requestPost($path, $body); + } - return new class($this->client->getLastResponseStatusCode(), $this->client->getLastResponseContentType(), $this->client->getLastResponseBody()) implements Response { + return $this->createResponse( + $this->client->getLastResponseStatusCode(), + $this->client->getLastResponseContentType(), + $this->client->getLastResponseBody() + ); + } + + public function createResponse(int $statusCode, string $contentType, string $body): Response + { + return new class($statusCode, $contentType, $body) implements Response { private $statusCode; private $contentType; private $body; diff --git a/tests/Unit/Api/AbstractApi/PostTest.php b/tests/Unit/Api/AbstractApi/PostTest.php new file mode 100644 index 00000000..c7dcea94 --- /dev/null +++ b/tests/Unit/Api/AbstractApi/PostTest.php @@ -0,0 +1,41 @@ +createMock(Response::class); + $response->expects($this->any())->method('getStatusCode')->willReturn(200); + $response->expects($this->any())->method('getContentType')->willReturn('application/xml'); + $response->expects($this->any())->method('getBody')->willReturn(''); + + $client = $this->createMock(HttpClient::class); + $client->expects($this->exactly(1))->method('request')->with('POST', 'path.xml', '')->willReturn($response); + + $api = new class ($client) extends AbstractApi {}; + + $method = new ReflectionMethod($api, 'post'); + $method->setAccessible(true); + + // Perform the tests + $return = $method->invoke($api, 'path.xml', ''); + + $this->assertInstanceOf(SimpleXMLElement::class, $return); + $this->assertXmlStringEqualsXmlString('', $return->asXML()); + } +} diff --git a/tests/Unit/Api/AbstractApiTest.php b/tests/Unit/Api/AbstractApiTest.php index bba98a15..8fd5948d 100644 --- a/tests/Unit/Api/AbstractApiTest.php +++ b/tests/Unit/Api/AbstractApiTest.php @@ -140,7 +140,7 @@ public static function getLastCallFailedData(): array /** * @covers \Redmine\Api\AbstractApi * @test - * @dataProvider getXmlDecodingFromGetMethodData + * @dataProvider getXmlDecodingFromRequestMethodsData */ public function testXmlDecodingFromRequestMethods($methodName, $response, $decode, $expected) { @@ -166,7 +166,7 @@ public function testXmlDecodingFromRequestMethods($methodName, $response, $decod } } - public static function getXmlDecodingFromGetMethodData(): array + public static function getXmlDecodingFromRequestMethodsData(): array { return [ ['post', '', null, ''], From d51284e670bdc55ed9bec4128fdb43c340460ada Mon Sep 17 00:00:00 2001 From: Art4 Date: Mon, 8 Jan 2024 09:49:23 +0100 Subject: [PATCH 07/29] restrict types in AbstractApi::__construct() --- src/Redmine/Api/AbstractApi.php | 14 ++++++++++++ tests/Unit/Api/AbstractApiTest.php | 34 ++++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+) diff --git a/src/Redmine/Api/AbstractApi.php b/src/Redmine/Api/AbstractApi.php index 141fb67a..c8f578a0 100644 --- a/src/Redmine/Api/AbstractApi.php +++ b/src/Redmine/Api/AbstractApi.php @@ -2,6 +2,7 @@ namespace Redmine\Api; +use InvalidArgumentException; use Redmine\Api; use Redmine\Client\Client; use Redmine\Exception; @@ -30,8 +31,21 @@ abstract class AbstractApi implements Api */ private $httpClient; + /** + * @param Client|HttpClient $client + */ public function __construct($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; } diff --git a/tests/Unit/Api/AbstractApiTest.php b/tests/Unit/Api/AbstractApiTest.php index 8fd5948d..8ee576e4 100644 --- a/tests/Unit/Api/AbstractApiTest.php +++ b/tests/Unit/Api/AbstractApiTest.php @@ -2,10 +2,12 @@ namespace Redmine\Tests\Unit\Api; +use InvalidArgumentException; use PHPUnit\Framework\TestCase; use Redmine\Api\AbstractApi; use Redmine\Client\Client; use Redmine\Exception\SerializerException; +use Redmine\Http\HttpClient; use ReflectionMethod; use SimpleXMLElement; @@ -14,6 +16,38 @@ */ class AbstractApiTest extends TestCase { + public function testCreateWithHttpClientWorks() + { + $client = $this->createMock(HttpClient::class); + + $api = new class ($client) extends AbstractApi {}; + + $method = new ReflectionMethod($api, 'getHttpClient'); + $method->setAccessible(true); + + $this->assertSame($client, $method->invoke($api)); + } + + public function testCreateWitClientWorks() + { + $client = $this->createMock(Client::class); + + $api = new class ($client) extends AbstractApi {}; + + $method = new ReflectionMethod($api, 'getHttpClient'); + $method->setAccessible(true); + + $this->assertInstanceOf(HttpClient::class, $method->invoke($api)); + } + + public function testCreateWithoutClitentOrHttpClientThrowsException() + { + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage('Redmine\Api\AbstractApi::__construct(): Argument #1 ($client) must be of type Redmine\Client\Client or Redmine\Http\HttpClient, `stdClass` given'); + + new class (new \stdClass()) extends AbstractApi {}; + } + /** * @test * @dataProvider getIsNotNullReturnsCorrectBooleanData From edfcdc6b04329bcd189921bb191ad847eecaca98 Mon Sep 17 00:00:00 2001 From: Art4 Date: Mon, 8 Jan 2024 09:58:16 +0100 Subject: [PATCH 08/29] Fix code style --- src/Redmine/Api/AbstractApi.php | 8 ++++---- tests/Unit/Api/AbstractApi/PostTest.php | 1 - 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/src/Redmine/Api/AbstractApi.php b/src/Redmine/Api/AbstractApi.php index c8f578a0..55b85519 100644 --- a/src/Redmine/Api/AbstractApi.php +++ b/src/Redmine/Api/AbstractApi.php @@ -46,13 +46,13 @@ public function __construct($client) )); } - if ($client instanceOf Client) { + if ($client instanceof Client) { $this->client = $client; } $httpClient = $client; - if (! $httpClient instanceOf HttpClient) { + if (! $httpClient instanceof HttpClient) { $httpClient = $this->handleClient($client); } @@ -370,7 +370,7 @@ private function getLastResponseBodyAsArray(): array private function handleClient(Client $client): HttpClient { - return new class($client) implements HttpClient { + return new class ($client) implements HttpClient { private $client; public function __construct(Client $client) @@ -395,7 +395,7 @@ public function request(string $method, string $path, string $body = ''): Respon public function createResponse(int $statusCode, string $contentType, string $body): Response { - return new class($statusCode, $contentType, $body) implements Response { + return new class ($statusCode, $contentType, $body) implements Response { private $statusCode; private $contentType; private $body; diff --git a/tests/Unit/Api/AbstractApi/PostTest.php b/tests/Unit/Api/AbstractApi/PostTest.php index c7dcea94..1d1abf3b 100644 --- a/tests/Unit/Api/AbstractApi/PostTest.php +++ b/tests/Unit/Api/AbstractApi/PostTest.php @@ -6,7 +6,6 @@ use PHPUnit\Framework\TestCase; use Redmine\Api\AbstractApi; -use Redmine\Client\Client; use Redmine\Http\HttpClient; use Redmine\Http\Response; use ReflectionMethod; From 10e1c9b107e0e062a17abed2918a980d1dddbc4f Mon Sep 17 00:00:00 2001 From: Art4 Date: Mon, 8 Jan 2024 10:03:28 +0100 Subject: [PATCH 09/29] Use HttpClient in AbstractApi::put() --- src/Redmine/Api/AbstractApi.php | 15 ++++++---- tests/Unit/Api/AbstractApi/PutTest.php | 40 ++++++++++++++++++++++++++ 2 files changed, 49 insertions(+), 6 deletions(-) create mode 100644 tests/Unit/Api/AbstractApi/PutTest.php diff --git a/src/Redmine/Api/AbstractApi.php b/src/Redmine/Api/AbstractApi.php index 55b85519..13ce8caf 100644 --- a/src/Redmine/Api/AbstractApi.php +++ b/src/Redmine/Api/AbstractApi.php @@ -145,12 +145,13 @@ protected function post($path, $data) */ protected function put($path, $data) { - $this->client->requestPut($path, $data); + $response = $this->getHttpClient()->request('PUT', strval($path), $data); - $body = $this->client->getLastResponseBody(); + $body = $response->getBody(); + $contentType = $response->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); } @@ -380,10 +381,12 @@ public function __construct(Client $client) public function request(string $method, string $path, string $body = ''): Response { - if ($method === 'GET') { - $this->client->requestGet($path); - } else { + if ($method === 'POST') { $this->client->requestPost($path, $body); + } else if ($method === 'PUT') { + $this->client->requestPut($path, $body); + } else { + $this->client->requestGet($path); } return $this->createResponse( diff --git a/tests/Unit/Api/AbstractApi/PutTest.php b/tests/Unit/Api/AbstractApi/PutTest.php new file mode 100644 index 00000000..87fe1703 --- /dev/null +++ b/tests/Unit/Api/AbstractApi/PutTest.php @@ -0,0 +1,40 @@ +createMock(Response::class); + $response->expects($this->any())->method('getStatusCode')->willReturn(200); + $response->expects($this->any())->method('getContentType')->willReturn('application/xml'); + $response->expects($this->any())->method('getBody')->willReturn(''); + + $client = $this->createMock(HttpClient::class); + $client->expects($this->exactly(1))->method('request')->with('PUT', 'path.xml', '')->willReturn($response); + + $api = new class ($client) extends AbstractApi {}; + + $method = new ReflectionMethod($api, 'put'); + $method->setAccessible(true); + + // Perform the tests + $return = $method->invoke($api, 'path.xml', ''); + + $this->assertInstanceOf(SimpleXMLElement::class, $return); + $this->assertXmlStringEqualsXmlString('', $return->asXML()); + } +} From 3a7f905793abfc7c8e9ca1f60d0fb5870b612e05 Mon Sep 17 00:00:00 2001 From: Art4 Date: Mon, 8 Jan 2024 10:20:19 +0100 Subject: [PATCH 10/29] move xml decoding tests into separate test files --- tests/Unit/Api/AbstractApi/GetTest.php | 10 ++++----- tests/Unit/Api/AbstractApi/PostTest.php | 29 +++++++++++++++++++++++++ tests/Unit/Api/AbstractApi/PutTest.php | 29 +++++++++++++++++++++++++ tests/Unit/Api/AbstractApiTest.php | 2 -- 4 files changed, 62 insertions(+), 8 deletions(-) diff --git a/tests/Unit/Api/AbstractApi/GetTest.php b/tests/Unit/Api/AbstractApi/GetTest.php index cf2d786a..29750b59 100644 --- a/tests/Unit/Api/AbstractApi/GetTest.php +++ b/tests/Unit/Api/AbstractApi/GetTest.php @@ -74,11 +74,9 @@ public static function getJsonDecodingFromGetMethodData(): array } /** - * @covers \Redmine\Api\AbstractApi - * @test * @dataProvider getXmlDecodingFromGetMethodData */ - public function testXmlDecodingFromRequestMethods($response, $decode, $expected) + public function testXmlDecodingFromGetMethod($response, $decode, $expected) { $client = $this->createMock(Client::class); $client->method('getLastResponseBody')->willReturn($response); @@ -99,9 +97,9 @@ public function testXmlDecodingFromRequestMethods($response, $decode, $expected) public static function getXmlDecodingFromGetMethodData(): array { return [ - ['', null, ''], // test decode by default - ['', true, ''], - ['', false, ''], // test that xml decoding will be always happen + 'decode by default' => ['', null, ''], // test decode by default + 'decode true' => ['', true, ''], + 'decode false' => ['', false, ''], // test that xml decoding will be always happen ]; } } diff --git a/tests/Unit/Api/AbstractApi/PostTest.php b/tests/Unit/Api/AbstractApi/PostTest.php index 1d1abf3b..7f08ea62 100644 --- a/tests/Unit/Api/AbstractApi/PostTest.php +++ b/tests/Unit/Api/AbstractApi/PostTest.php @@ -6,6 +6,7 @@ use PHPUnit\Framework\TestCase; use Redmine\Api\AbstractApi; +use Redmine\Client\Client; use Redmine\Http\HttpClient; use Redmine\Http\Response; use ReflectionMethod; @@ -37,4 +38,32 @@ public function testPostWithHttpClient() $this->assertInstanceOf(SimpleXMLElement::class, $return); $this->assertXmlStringEqualsXmlString('', $return->asXML()); } + + /** + * @dataProvider getXmlDecodingFromPostMethodData + */ + public function testXmlDecodingFromPostMethod($response, $expected) + { + $client = $this->createMock(Client::class); + $client->method('getLastResponseBody')->willReturn($response); + $client->method('getLastResponseContentType')->willReturn('application/xml'); + + $api = new class ($client) extends AbstractApi {}; + + $method = new ReflectionMethod($api, 'post'); + $method->setAccessible(true); + + // Perform the tests + $return = $method->invoke($api, 'path.xml', ''); + + $this->assertInstanceOf(SimpleXMLElement::class, $return); + $this->assertXmlStringEqualsXmlString($expected, $return->asXML()); + } + + public static function getXmlDecodingFromPostMethodData(): array + { + return [ + 'decode by default' => ['', ''], // test decode by default + ]; + } } diff --git a/tests/Unit/Api/AbstractApi/PutTest.php b/tests/Unit/Api/AbstractApi/PutTest.php index 87fe1703..41071ecb 100644 --- a/tests/Unit/Api/AbstractApi/PutTest.php +++ b/tests/Unit/Api/AbstractApi/PutTest.php @@ -6,6 +6,7 @@ use PHPUnit\Framework\TestCase; use Redmine\Api\AbstractApi; +use Redmine\Client\Client; use Redmine\Http\HttpClient; use Redmine\Http\Response; use ReflectionMethod; @@ -37,4 +38,32 @@ public function testPutWithHttpClient() $this->assertInstanceOf(SimpleXMLElement::class, $return); $this->assertXmlStringEqualsXmlString('', $return->asXML()); } + + /** + * @dataProvider getXmlDecodingFromPutMethodData + */ + public function testXmlDecodingFromPutMethod($response, $expected) + { + $client = $this->createMock(Client::class); + $client->method('getLastResponseBody')->willReturn($response); + $client->method('getLastResponseContentType')->willReturn('application/xml'); + + $api = new class ($client) extends AbstractApi {}; + + $method = new ReflectionMethod($api, 'put'); + $method->setAccessible(true); + + // Perform the tests + $return = $method->invoke($api, 'path.xml', ''); + + $this->assertInstanceOf(SimpleXMLElement::class, $return); + $this->assertXmlStringEqualsXmlString($expected, $return->asXML()); + } + + public static function getXmlDecodingFromPutMethodData(): array + { + return [ + 'decode by default' => ['', ''], // test decode by default + ]; + } } diff --git a/tests/Unit/Api/AbstractApiTest.php b/tests/Unit/Api/AbstractApiTest.php index 8ee576e4..4698f546 100644 --- a/tests/Unit/Api/AbstractApiTest.php +++ b/tests/Unit/Api/AbstractApiTest.php @@ -203,8 +203,6 @@ public function testXmlDecodingFromRequestMethods($methodName, $response, $decod public static function getXmlDecodingFromRequestMethodsData(): array { return [ - ['post', '', null, ''], - ['put', '', null, ''], ['delete', '', null, ''], ]; } From 71d8eb83677a7e4aa5c801aa8161fc338871876c Mon Sep 17 00:00:00 2001 From: Art4 Date: Mon, 8 Jan 2024 11:18:32 +0100 Subject: [PATCH 11/29] Use HttpClient in AbstractApi::delete() --- src/Redmine/Api/AbstractApi.php | 6 +- tests/Unit/Api/AbstractApi/DeleteTest.php | 67 +++++++++++++++++++++++ tests/Unit/Api/AbstractApiTest.php | 2 +- tests/Unit/Api/GroupTest.php | 2 +- tests/Unit/Api/IssueRelationTest.php | 2 +- tests/Unit/Api/MembershipTest.php | 2 +- 6 files changed, 75 insertions(+), 6 deletions(-) create mode 100644 tests/Unit/Api/AbstractApi/DeleteTest.php diff --git a/src/Redmine/Api/AbstractApi.php b/src/Redmine/Api/AbstractApi.php index 13ce8caf..93bb1ed9 100644 --- a/src/Redmine/Api/AbstractApi.php +++ b/src/Redmine/Api/AbstractApi.php @@ -167,9 +167,9 @@ protected function put($path, $data) */ protected function delete($path) { - $this->client->requestDelete($path); + $response = $this->getHttpClient()->request('DELETE', strval($path)); - return $this->client->getLastResponseBody(); + return $response->getBody(); } /** @@ -385,6 +385,8 @@ public function request(string $method, string $path, string $body = ''): Respon $this->client->requestPost($path, $body); } else if ($method === 'PUT') { $this->client->requestPut($path, $body); + } else if ($method === 'DELETE') { + $this->client->requestDelete($path); } else { $this->client->requestGet($path); } diff --git a/tests/Unit/Api/AbstractApi/DeleteTest.php b/tests/Unit/Api/AbstractApi/DeleteTest.php new file mode 100644 index 00000000..d53d5c0e --- /dev/null +++ b/tests/Unit/Api/AbstractApi/DeleteTest.php @@ -0,0 +1,67 @@ +createMock(Response::class); + $response->expects($this->any())->method('getStatusCode')->willReturn(200); + $response->expects($this->any())->method('getContentType')->willReturn('application/xml'); + $response->expects($this->any())->method('getBody')->willReturn(''); + + $client = $this->createMock(HttpClient::class); + $client->expects($this->exactly(1))->method('request')->with('DELETE', 'path.xml', '')->willReturn($response); + + $api = new class ($client) extends AbstractApi {}; + + $method = new ReflectionMethod($api, 'delete'); + $method->setAccessible(true); + + // Perform the tests + $return = $method->invoke($api, 'path.xml'); + + $this->assertSame('', $return); + } + + /** + * @dataProvider getXmlDecodingFromDeleteMethodData + */ + public function testXmlDecodingFromDeleteMethod($response, $expected) + { + $client = $this->createMock(Client::class); + $client->method('getLastResponseBody')->willReturn($response); + $client->method('getLastResponseContentType')->willReturn('application/xml'); + + $api = new class ($client) extends AbstractApi {}; + + $method = new ReflectionMethod($api, 'delete'); + $method->setAccessible(true); + + // Perform the tests + $return = $method->invoke($api, 'path.xml'); + + $this->assertSame($expected, $return); + } + + public static function getXmlDecodingFromDeleteMethodData(): array + { + return [ + 'no decode by default' => ['', ''], + ]; + } +} diff --git a/tests/Unit/Api/AbstractApiTest.php b/tests/Unit/Api/AbstractApiTest.php index 4698f546..bd475819 100644 --- a/tests/Unit/Api/AbstractApiTest.php +++ b/tests/Unit/Api/AbstractApiTest.php @@ -87,7 +87,7 @@ public static function getIsNotNullReturnsCorrectBooleanData(): array } /** - * @covers \Redmine\Api\AbstractApi + * @covers \Redmine\Api\AbstractApi::lastCallFailed * @test * @dataProvider getLastCallFailedData */ diff --git a/tests/Unit/Api/GroupTest.php b/tests/Unit/Api/GroupTest.php index 77f70f08..fa5fc56a 100644 --- a/tests/Unit/Api/GroupTest.php +++ b/tests/Unit/Api/GroupTest.php @@ -330,7 +330,7 @@ public function testRemoveCallsDelete() $client->expects($this->exactly(1)) ->method('getLastResponseBody') ->willReturn($response); - $client->expects($this->exactly(0)) + $client->expects($this->exactly(1)) ->method('getLastResponseContentType') ->willReturn('application/json'); diff --git a/tests/Unit/Api/IssueRelationTest.php b/tests/Unit/Api/IssueRelationTest.php index da548e53..439722fb 100644 --- a/tests/Unit/Api/IssueRelationTest.php +++ b/tests/Unit/Api/IssueRelationTest.php @@ -211,7 +211,7 @@ public function testRemoveCallsDelete() $client->expects($this->exactly(1)) ->method('getLastResponseBody') ->willReturn($response); - $client->expects($this->exactly(0)) + $client->expects($this->exactly(1)) ->method('getLastResponseContentType') ->willReturn('application/json'); diff --git a/tests/Unit/Api/MembershipTest.php b/tests/Unit/Api/MembershipTest.php index f9658864..f8522488 100644 --- a/tests/Unit/Api/MembershipTest.php +++ b/tests/Unit/Api/MembershipTest.php @@ -180,7 +180,7 @@ public function testRemoveMemberCallsDelete() ->method('requestDelete') ->with($this->stringContains('/memberships/5.xml')) ->willReturn(true); - $client->expects($this->once()) + $client->expects($this->exactly(2)) ->method('getLastResponseContentType') ->willReturn('application/json'); $matcher = $this->exactly(2); From 0394eb74f337053f93c5084ab2314ed62d1e61b0 Mon Sep 17 00:00:00 2001 From: Art4 Date: Mon, 8 Jan 2024 11:41:13 +0100 Subject: [PATCH 12/29] Use HttpClient in AbstractApi::retrieveData() --- src/Redmine/Api/AbstractApi.php | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/Redmine/Api/AbstractApi.php b/src/Redmine/Api/AbstractApi.php index 93bb1ed9..5a7a50a3 100644 --- a/src/Redmine/Api/AbstractApi.php +++ b/src/Redmine/Api/AbstractApi.php @@ -241,9 +241,9 @@ protected function retrieveAll($endpoint, array $params = []) protected function retrieveData(string $endpoint, array $params = []): array { if (empty($params)) { - $this->client->requestGet($endpoint); + $response = $this->getHttpClient()->request('GET', strval($endpoint)); - return $this->getLastResponseBodyAsArray(); + return $this->getLastResponseBodyAsArray($response); } $params = $this->sanitizeParams( @@ -270,11 +270,12 @@ protected function retrieveData(string $endpoint, array $params = []): array $params['limit'] = $_limit; $params['offset'] = $offset; - $this->client->requestGet( + $response = $this->getHttpClient()->request( + 'GET', PathSerializer::create($endpoint, $params)->getPath() ); - $newDataSet = $this->getLastResponseBodyAsArray(); + $newDataSet = $this->getLastResponseBodyAsArray($response); $returnData = array_merge_recursive($returnData, $newDataSet); @@ -348,11 +349,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 getLastResponseBodyAsArray(Response $response): array { - $body = $this->client->getLastResponseBody(); - - $contentType = $this->client->getLastResponseContentType(); + $body = $response->getBody(); + $contentType = $response->getContentType(); $returnData = null; // parse XML From ee431a64eb369ab7ccbc3d7ad3efd98262e3c5c1 Mon Sep 17 00:00:00 2001 From: Art4 Date: Mon, 8 Jan 2024 11:45:14 +0100 Subject: [PATCH 13/29] Use HttpClient in AbstractApi::retrieveAll() --- src/Redmine/Api/AbstractApi.php | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/src/Redmine/Api/AbstractApi.php b/src/Redmine/Api/AbstractApi.php index 5a7a50a3..963bd05e 100644 --- a/src/Redmine/Api/AbstractApi.php +++ b/src/Redmine/Api/AbstractApi.php @@ -31,6 +31,11 @@ abstract class AbstractApi implements Api */ private $httpClient; + /** + * @var Response + */ + private $lastResponse; + /** * @param Client|HttpClient $client */ @@ -217,7 +222,7 @@ protected function retrieveAll($endpoint, array $params = []) try { $data = $this->retrieveData(strval($endpoint), $params); } catch (Exception $e) { - if ($this->client->getLastResponseBody() === '') { + if (isset($this->lastResponse) && $this->lastResponse->getBody() === '') { return false; } @@ -241,9 +246,9 @@ protected function retrieveAll($endpoint, array $params = []) protected function retrieveData(string $endpoint, array $params = []): array { if (empty($params)) { - $response = $this->getHttpClient()->request('GET', strval($endpoint)); + $this->lastResponse = $this->getHttpClient()->request('GET', strval($endpoint)); - return $this->getLastResponseBodyAsArray($response); + return $this->getLastResponseBodyAsArray($this->lastResponse); } $params = $this->sanitizeParams( @@ -270,12 +275,12 @@ protected function retrieveData(string $endpoint, array $params = []): array $params['limit'] = $_limit; $params['offset'] = $offset; - $response = $this->getHttpClient()->request( + $this->lastResponse = $this->getHttpClient()->request( 'GET', PathSerializer::create($endpoint, $params)->getPath() ); - $newDataSet = $this->getLastResponseBodyAsArray($response); + $newDataSet = $this->getLastResponseBodyAsArray($this->lastResponse); $returnData = array_merge_recursive($returnData, $newDataSet); From a9599a6cd31404502308e23c612ca993b78b957d Mon Sep 17 00:00:00 2001 From: Art4 Date: Mon, 8 Jan 2024 13:09:15 +0100 Subject: [PATCH 14/29] Use HttpClient in AbstractApi::lastCallFailed() --- src/Redmine/Api/AbstractApi.php | 30 ++++++++++++++++++------------ tests/Unit/Api/AbstractApiTest.php | 27 ++++++++++++++++++++++++++- 2 files changed, 44 insertions(+), 13 deletions(-) diff --git a/src/Redmine/Api/AbstractApi.php b/src/Redmine/Api/AbstractApi.php index 963bd05e..a13eafa9 100644 --- a/src/Redmine/Api/AbstractApi.php +++ b/src/Redmine/Api/AbstractApi.php @@ -81,7 +81,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 (isset($this->lastResponse)) { + $code = $this->lastResponse->getStatusCode(); + } else if (isset($this->client)) { + $code = $this->client->getLastResponseStatusCode(); + } else{ + $code = 0; + } return 200 !== $code && 201 !== $code; } @@ -96,10 +102,10 @@ public function lastCallFailed() */ protected function get($path, $decodeIfJson = true) { - $response = $this->getHttpClient()->request('GET', strval($path)); + $this->lastResponse = $this->getHttpClient()->request('GET', strval($path)); - $body = $response->getBody(); - $contentType = $response->getContentType(); + $body = $this->lastResponse->getBody(); + $contentType = $this->lastResponse->getContentType(); // if response is XML, return a SimpleXMLElement object if ('' !== $body && 0 === strpos($contentType, 'application/xml')) { @@ -127,10 +133,10 @@ protected function get($path, $decodeIfJson = true) */ protected function post($path, $data) { - $response = $this->getHttpClient()->request('POST', strval($path), $data); + $this->lastResponse = $this->getHttpClient()->request('POST', strval($path), $data); - $body = $response->getBody(); - $contentType = $response->getContentType(); + $body = $this->lastResponse->getBody(); + $contentType = $this->lastResponse->getContentType(); // if response is XML, return a SimpleXMLElement object if ('' !== $body && 0 === strpos($contentType, 'application/xml')) { @@ -150,10 +156,10 @@ protected function post($path, $data) */ protected function put($path, $data) { - $response = $this->getHttpClient()->request('PUT', strval($path), $data); + $this->lastResponse = $this->getHttpClient()->request('PUT', strval($path), $data); - $body = $response->getBody(); - $contentType = $response->getContentType(); + $body = $this->lastResponse->getBody(); + $contentType = $this->lastResponse->getContentType(); // if response is XML, return a SimpleXMLElement object if ('' !== $body && 0 === strpos($contentType, 'application/xml')) { @@ -172,9 +178,9 @@ protected function put($path, $data) */ protected function delete($path) { - $response = $this->getHttpClient()->request('DELETE', strval($path)); + $this->lastResponse = $this->getHttpClient()->request('DELETE', strval($path)); - return $response->getBody(); + return $this->lastResponse->getBody(); } /** diff --git a/tests/Unit/Api/AbstractApiTest.php b/tests/Unit/Api/AbstractApiTest.php index bd475819..18d34c79 100644 --- a/tests/Unit/Api/AbstractApiTest.php +++ b/tests/Unit/Api/AbstractApiTest.php @@ -8,6 +8,7 @@ use Redmine\Client\Client; use Redmine\Exception\SerializerException; use Redmine\Http\HttpClient; +use Redmine\Http\Response; use ReflectionMethod; use SimpleXMLElement; @@ -91,7 +92,7 @@ public static function getIsNotNullReturnsCorrectBooleanData(): array * @test * @dataProvider getLastCallFailedData */ - public function testLastCallFailedReturnsCorrectBoolean($statusCode, $expectedBoolean) + public function testLastCallFailedWithClientReturnsCorrectBoolean($statusCode, $expectedBoolean) { $client = $this->createMock(Client::class); $client->method('getLastResponseStatusCode')->willReturn($statusCode); @@ -101,9 +102,33 @@ public function testLastCallFailedReturnsCorrectBoolean($statusCode, $expectedBo $this->assertSame($expectedBoolean, $api->lastCallFailed()); } + /** + * @covers \Redmine\Api\AbstractApi::lastCallFailed + * @test + * @dataProvider getLastCallFailedData + */ + public function testLastCallFailedWithHttpClientReturnsCorrectBoolean($statusCode, $expectedBoolean) + { + $response = $this->createMock(Response::class); + $response->method('getStatusCode')->willReturn($statusCode); + + $client = $this->createMock(HttpClient::class); + $client->method('request')->willReturn($response); + + $api = new class ($client) extends AbstractApi { + public function __construct($client) { + parent::__construct($client); + $this->get('', false); + } + }; + + $this->assertSame($expectedBoolean, $api->lastCallFailed()); + } + public static function getLastCallFailedData(): array { return [ + [0, true], [100, true], [101, true], [102, true], From 16c032160195572babd2ed587992c3cd8a465f70 Mon Sep 17 00:00:00 2001 From: Art4 Date: Mon, 8 Jan 2024 13:33:26 +0100 Subject: [PATCH 15/29] Add tests to prevent race conditions --- CHANGELOG.md | 4 ++++ tests/Unit/Api/AbstractApiTest.php | 35 ++++++++++++++++++++++++++++++ 2 files changed, 39 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8c845f8e..6c8e9e6c 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/tests/Unit/Api/AbstractApiTest.php b/tests/Unit/Api/AbstractApiTest.php index 18d34c79..9b731cd0 100644 --- a/tests/Unit/Api/AbstractApiTest.php +++ b/tests/Unit/Api/AbstractApiTest.php @@ -87,6 +87,41 @@ public static function getIsNotNullReturnsCorrectBooleanData(): array ]; } + /** + * @covers \Redmine\Api\AbstractApi::lastCallFailed + */ + public function testLastCallFailedPreventsRaceCondition() + { + $response1 = $this->createMock(Response::class); + $response1->method('getStatusCode')->willReturn(200); + + $response2 = $this->createMock(Response::class); + $response2->method('getStatusCode')->willReturn(500); + + $client = $this->createMock(HttpClient::class); + $client->method('request')->willReturnMap([ + ['GET', '200.json', $response1], + ['GET', '500.json', $response2], + ]); + + $api1 = new class ($client) extends AbstractApi { + public function __construct($client) { + parent::__construct($client); + $this->get('200.json', false); + } + }; + + $api2 = new class ($client) extends AbstractApi { + public function __construct($client) { + parent::__construct($client); + $this->get('500.json', false); + } + }; + + $this->assertSame(false, $api1->lastCallFailed()); + $this->assertSame(true, $api2->lastCallFailed()); + } + /** * @covers \Redmine\Api\AbstractApi::lastCallFailed * @test From dbbd7b24febaf37e21baa4b76b44be78180a0936 Mon Sep 17 00:00:00 2001 From: Art4 Date: Mon, 8 Jan 2024 14:18:24 +0100 Subject: [PATCH 16/29] do not use Client in deprecated all() methods --- src/Redmine/Api/AbstractApi.php | 74 +++++++++++++++------------ src/Redmine/Api/CustomField.php | 2 +- src/Redmine/Api/Group.php | 2 +- src/Redmine/Api/Issue.php | 2 +- src/Redmine/Api/IssueCategory.php | 2 +- src/Redmine/Api/IssuePriority.php | 2 +- src/Redmine/Api/IssueRelation.php | 2 +- src/Redmine/Api/IssueStatus.php | 2 +- src/Redmine/Api/Membership.php | 2 +- src/Redmine/Api/News.php | 2 +- src/Redmine/Api/Project.php | 2 +- src/Redmine/Api/Query.php | 2 +- src/Redmine/Api/Role.php | 2 +- src/Redmine/Api/Search.php | 2 +- src/Redmine/Api/TimeEntry.php | 2 +- src/Redmine/Api/TimeEntryActivity.php | 2 +- src/Redmine/Api/Tracker.php | 2 +- src/Redmine/Api/User.php | 2 +- src/Redmine/Api/Version.php | 2 +- src/Redmine/Api/Wiki.php | 2 +- 20 files changed, 61 insertions(+), 51 deletions(-) diff --git a/src/Redmine/Api/AbstractApi.php b/src/Redmine/Api/AbstractApi.php index a13eafa9..f6f77bc2 100644 --- a/src/Redmine/Api/AbstractApi.php +++ b/src/Redmine/Api/AbstractApi.php @@ -2,6 +2,7 @@ namespace Redmine\Api; +use Closure; use InvalidArgumentException; use Redmine\Api; use Redmine\Client\Client; @@ -69,6 +70,11 @@ final protected function getHttpClient(): HttpClient return $this->httpClient; } + final protected function getLastResonse(): Response + { + return isset($this->lastResponse) ? $this->lastResponse : $this->createResponse(0, '', ''); + } + /** * Returns whether or not the last api call failed. * @@ -228,7 +234,7 @@ protected function retrieveAll($endpoint, array $params = []) try { $data = $this->retrieveData(strval($endpoint), $params); } catch (Exception $e) { - if (isset($this->lastResponse) && $this->lastResponse->getBody() === '') { + if ($this->getLastResonse()->getBody() === '') { return false; } @@ -254,7 +260,7 @@ protected function retrieveData(string $endpoint, array $params = []): array if (empty($params)) { $this->lastResponse = $this->getHttpClient()->request('GET', strval($endpoint)); - return $this->getLastResponseBodyAsArray($this->lastResponse); + return $this->getResponseAsArray($this->lastResponse); } $params = $this->sanitizeParams( @@ -286,7 +292,7 @@ protected function retrieveData(string $endpoint, array $params = []): array PathSerializer::create($endpoint, $params)->getPath() ); - $newDataSet = $this->getLastResponseBodyAsArray($this->lastResponse); + $newDataSet = $this->getResponseAsArray($this->lastResponse); $returnData = array_merge_recursive($returnData, $newDataSet); @@ -360,7 +366,7 @@ protected function attachCustomFieldXML(SimpleXMLElement $xml, array $fields) * * @throws SerializerException if response body could not be converted into array */ - private function getLastResponseBodyAsArray(Response $response): array + private function getResponseAsArray(Response $response): array { $body = $response->getBody(); $contentType = $response->getContentType(); @@ -382,12 +388,16 @@ private function getLastResponseBodyAsArray(Response $response): array private function handleClient(Client $client): HttpClient { - return new class ($client) implements HttpClient { + $responseFactory = Closure::fromCallable([$this, 'createResponse']); + + return new class ($client, $responseFactory) implements HttpClient { private $client; + private $responseFactory; - public function __construct(Client $client) + public function __construct(Client $client, Closure $responseFactory) { $this->client = $client; + $this->responseFactory = $responseFactory; } public function request(string $method, string $path, string $body = ''): Response @@ -402,42 +412,42 @@ public function request(string $method, string $path, string $body = ''): Respon $this->client->requestGet($path); } - return $this->createResponse( + 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 createResponse(int $statusCode, string $contentType, string $body): Response + public function __construct(int $statusCode, string $contentType, string $body) { - 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; - } + $this->statusCode = $statusCode; + $this->contentType = $contentType; + $this->body = $body; + } - public function getStatusCode(): int - { - return $this->statusCode; - } + public function getStatusCode(): int + { + return $this->statusCode; + } - public function getContentType(): string - { - return $this->contentType; - } + public function getContentType(): string + { + return $this->contentType; + } - public function getBody(): string - { - return $this->body; - } - }; + public function getBody(): string + { + return $this->body; } }; } diff --git a/src/Redmine/Api/CustomField.php b/src/Redmine/Api/CustomField.php index 525fae43..fafda02c 100644 --- a/src/Redmine/Api/CustomField.php +++ b/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->getLastResonse()->getBody() === '') { return false; } diff --git a/src/Redmine/Api/Group.php b/src/Redmine/Api/Group.php index 51101862..a1c13281 100644 --- a/src/Redmine/Api/Group.php +++ b/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->getLastResonse()->getBody() === '') { return false; } diff --git a/src/Redmine/Api/Issue.php b/src/Redmine/Api/Issue.php index 2bd3a0ae..6b94d0a2 100644 --- a/src/Redmine/Api/Issue.php +++ b/src/Redmine/Api/Issue.php @@ -82,7 +82,7 @@ public function all(array $params = []) try { return $this->list($params); } catch (Exception $e) { - if ($this->client->getLastResponseBody() === '') { + if ($this->getLastResonse()->getBody() === '') { return false; } diff --git a/src/Redmine/Api/IssueCategory.php b/src/Redmine/Api/IssueCategory.php index bac5d553..fcf5d42e 100644 --- a/src/Redmine/Api/IssueCategory.php +++ b/src/Redmine/Api/IssueCategory.php @@ -69,7 +69,7 @@ public function all($project, array $params = []) try { return $this->listByProject(strval($project), $params); } catch (Exception $e) { - if ($this->client->getLastResponseBody() === '') { + if ($this->getLastResonse()->getBody() === '') { return false; } diff --git a/src/Redmine/Api/IssuePriority.php b/src/Redmine/Api/IssuePriority.php index 917a9579..2c54fdac 100644 --- a/src/Redmine/Api/IssuePriority.php +++ b/src/Redmine/Api/IssuePriority.php @@ -55,7 +55,7 @@ public function all(array $params = []) try { $this->issuePriorities = $this->list($params); } catch (Exception $e) { - if ($this->client->getLastResponseBody() === '') { + if ($this->getLastResonse()->getBody() === '') { return false; } diff --git a/src/Redmine/Api/IssueRelation.php b/src/Redmine/Api/IssueRelation.php index 26b7f19b..b564a81d 100644 --- a/src/Redmine/Api/IssueRelation.php +++ b/src/Redmine/Api/IssueRelation.php @@ -58,7 +58,7 @@ public function all($issueId, array $params = []) try { $this->relations = $this->listByIssueId($issueId, $params); } catch (Exception $e) { - if ($this->client->getLastResponseBody() === '') { + if ($this->getLastResonse()->getBody() === '') { return false; } diff --git a/src/Redmine/Api/IssueStatus.php b/src/Redmine/Api/IssueStatus.php index eec0e127..6a908594 100644 --- a/src/Redmine/Api/IssueStatus.php +++ b/src/Redmine/Api/IssueStatus.php @@ -55,7 +55,7 @@ public function all(array $params = []) try { $this->issueStatuses = $this->list($params); } catch (Exception $e) { - if ($this->client->getLastResponseBody() === '') { + if ($this->getLastResonse()->getBody() === '') { return false; } diff --git a/src/Redmine/Api/Membership.php b/src/Redmine/Api/Membership.php index 62d8f90f..007e8c03 100644 --- a/src/Redmine/Api/Membership.php +++ b/src/Redmine/Api/Membership.php @@ -68,7 +68,7 @@ public function all($project, array $params = []) try { $this->memberships = $this->listByProject(strval($project), $params); } catch (Exception $e) { - if ($this->client->getLastResponseBody() === '') { + if ($this->getLastResonse()->getBody() === '') { return false; } diff --git a/src/Redmine/Api/News.php b/src/Redmine/Api/News.php index 6999ca3e..0fbe7c87 100644 --- a/src/Redmine/Api/News.php +++ b/src/Redmine/Api/News.php @@ -88,7 +88,7 @@ public function all($project = null, array $params = []) $this->news = $this->listByProject(strval($project), $params); } } catch (Exception $e) { - if ($this->client->getLastResponseBody() === '') { + if ($this->getLastResonse()->getBody() === '') { return false; } diff --git a/src/Redmine/Api/Project.php b/src/Redmine/Api/Project.php index 259f1ae8..8617efe9 100755 --- a/src/Redmine/Api/Project.php +++ b/src/Redmine/Api/Project.php @@ -58,7 +58,7 @@ public function all(array $params = []) try { $this->projects = $this->list($params); } catch (Exception $e) { - if ($this->client->getLastResponseBody() === '') { + if ($this->getLastResonse()->getBody() === '') { return false; } diff --git a/src/Redmine/Api/Query.php b/src/Redmine/Api/Query.php index da629b87..b1e631a6 100644 --- a/src/Redmine/Api/Query.php +++ b/src/Redmine/Api/Query.php @@ -55,7 +55,7 @@ public function all(array $params = []) try { $this->query = $this->list($params); } catch (Exception $e) { - if ($this->client->getLastResponseBody() === '') { + if ($this->getLastResonse()->getBody() === '') { return false; } diff --git a/src/Redmine/Api/Role.php b/src/Redmine/Api/Role.php index b01b5c6d..1b6ab97f 100644 --- a/src/Redmine/Api/Role.php +++ b/src/Redmine/Api/Role.php @@ -55,7 +55,7 @@ public function all(array $params = []) try { $this->roles = $this->list($params); } catch (Exception $e) { - if ($this->client->getLastResponseBody() === '') { + if ($this->getLastResonse()->getBody() === '') { return false; } diff --git a/src/Redmine/Api/Search.php b/src/Redmine/Api/Search.php index d9fc1fbb..08d5737f 100644 --- a/src/Redmine/Api/Search.php +++ b/src/Redmine/Api/Search.php @@ -55,7 +55,7 @@ public function search($query, array $params = []) try { $this->results = $this->listByQuery($query, $params); } catch (Exception $e) { - if ($this->client->getLastResponseBody() === '') { + if ($this->getLastResonse()->getBody() === '') { return false; } diff --git a/src/Redmine/Api/TimeEntry.php b/src/Redmine/Api/TimeEntry.php index 74c49084..9e6fd0ed 100644 --- a/src/Redmine/Api/TimeEntry.php +++ b/src/Redmine/Api/TimeEntry.php @@ -57,7 +57,7 @@ public function all(array $params = []) try { $this->timeEntries = $this->list($params); } catch (Exception $e) { - if ($this->client->getLastResponseBody() === '') { + if ($this->getLastResonse()->getBody() === '') { return false; } diff --git a/src/Redmine/Api/TimeEntryActivity.php b/src/Redmine/Api/TimeEntryActivity.php index 50c4fa13..1704dae3 100644 --- a/src/Redmine/Api/TimeEntryActivity.php +++ b/src/Redmine/Api/TimeEntryActivity.php @@ -51,7 +51,7 @@ public function all(array $params = []) try { $this->timeEntryActivities = $this->list($params); } catch (Exception $e) { - if ($this->client->getLastResponseBody() === '') { + if ($this->getLastResonse()->getBody() === '') { return false; } diff --git a/src/Redmine/Api/Tracker.php b/src/Redmine/Api/Tracker.php index 500d7ded..9073dc53 100644 --- a/src/Redmine/Api/Tracker.php +++ b/src/Redmine/Api/Tracker.php @@ -55,7 +55,7 @@ public function all(array $params = []) try { $this->trackers = $this->list($params); } catch (Exception $e) { - if ($this->client->getLastResponseBody() === '') { + if ($this->getLastResonse()->getBody() === '') { return false; } diff --git a/src/Redmine/Api/User.php b/src/Redmine/Api/User.php index 536c8b67..1b2f80c6 100644 --- a/src/Redmine/Api/User.php +++ b/src/Redmine/Api/User.php @@ -58,7 +58,7 @@ public function all(array $params = []) try { $this->users = $this->list($params); } catch (Exception $e) { - if ($this->client->getLastResponseBody() === '') { + if ($this->getLastResonse()->getBody() === '') { return false; } diff --git a/src/Redmine/Api/Version.php b/src/Redmine/Api/Version.php index a77f6f95..04a112ef 100644 --- a/src/Redmine/Api/Version.php +++ b/src/Redmine/Api/Version.php @@ -67,7 +67,7 @@ public function all($project, array $params = []) try { $this->versions = $this->listByProject(strval($project), $params); } catch (Exception $e) { - if ($this->client->getLastResponseBody() === '') { + if ($this->getLastResonse()->getBody() === '') { return false; } diff --git a/src/Redmine/Api/Wiki.php b/src/Redmine/Api/Wiki.php index d5c4ed35..3d61832d 100644 --- a/src/Redmine/Api/Wiki.php +++ b/src/Redmine/Api/Wiki.php @@ -67,7 +67,7 @@ public function all($project, array $params = []) try { $this->wikiPages = $this->listByProject(strval($project), $params); } catch (Exception $e) { - if ($this->client->getLastResponseBody() === '') { + if ($this->getLastResonse()->getBody() === '') { return false; } From 094fd41ffe21070a32e2fafde949d259d38417fb Mon Sep 17 00:00:00 2001 From: Art4 Date: Mon, 8 Jan 2024 15:19:04 +0100 Subject: [PATCH 17/29] Fix tests --- src/Redmine/Api/AbstractApi.php | 12 ++++++------ tests/Unit/Api/AbstractApiTest.php | 13 ++++++++----- 2 files changed, 14 insertions(+), 11 deletions(-) diff --git a/src/Redmine/Api/AbstractApi.php b/src/Redmine/Api/AbstractApi.php index f6f77bc2..c21a0cdc 100644 --- a/src/Redmine/Api/AbstractApi.php +++ b/src/Redmine/Api/AbstractApi.php @@ -72,7 +72,7 @@ final protected function getHttpClient(): HttpClient final protected function getLastResonse(): Response { - return isset($this->lastResponse) ? $this->lastResponse : $this->createResponse(0, '', ''); + return $this->lastResponse !== null ? $this->lastResponse : $this->createResponse(0, '', ''); } /** @@ -87,11 +87,11 @@ public function lastCallFailed() { @trigger_error('`' . __METHOD__ . '()` is deprecated since v2.1.0, use \Redmine\Client\Client::getLastResponseStatusCode() instead.', E_USER_DEPRECATED); - if (isset($this->lastResponse)) { + if ($this->lastResponse !== null) { $code = $this->lastResponse->getStatusCode(); - } else if (isset($this->client)) { + } elseif ($this->client !== null) { $code = $this->client->getLastResponseStatusCode(); - } else{ + } else { $code = 0; } @@ -404,9 +404,9 @@ public function request(string $method, string $path, string $body = ''): Respon { if ($method === 'POST') { $this->client->requestPost($path, $body); - } else if ($method === 'PUT') { + } elseif ($method === 'PUT') { $this->client->requestPut($path, $body); - } else if ($method === 'DELETE') { + } elseif ($method === 'DELETE') { $this->client->requestDelete($path); } else { $this->client->requestGet($path); diff --git a/tests/Unit/Api/AbstractApiTest.php b/tests/Unit/Api/AbstractApiTest.php index 9b731cd0..98ba96fa 100644 --- a/tests/Unit/Api/AbstractApiTest.php +++ b/tests/Unit/Api/AbstractApiTest.php @@ -105,16 +105,18 @@ public function testLastCallFailedPreventsRaceCondition() ]); $api1 = new class ($client) extends AbstractApi { - public function __construct($client) { + public function __construct($client) + { parent::__construct($client); - $this->get('200.json', false); + parent::get('200.json', false); } }; $api2 = new class ($client) extends AbstractApi { - public function __construct($client) { + public function __construct($client) + { parent::__construct($client); - $this->get('500.json', false); + parent::get('500.json', false); } }; @@ -151,7 +153,8 @@ public function testLastCallFailedWithHttpClientReturnsCorrectBoolean($statusCod $client->method('request')->willReturn($response); $api = new class ($client) extends AbstractApi { - public function __construct($client) { + public function __construct($client) + { parent::__construct($client); $this->get('', false); } From e0f8fcbc5c1f04822914bf5ff05654592c53ef65 Mon Sep 17 00:00:00 2001 From: Art4 Date: Mon, 8 Jan 2024 15:34:06 +0100 Subject: [PATCH 18/29] fix for PHPUnit 9 --- tests/Unit/Api/AbstractApiTest.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/Unit/Api/AbstractApiTest.php b/tests/Unit/Api/AbstractApiTest.php index 98ba96fa..321d2bf5 100644 --- a/tests/Unit/Api/AbstractApiTest.php +++ b/tests/Unit/Api/AbstractApiTest.php @@ -100,8 +100,8 @@ public function testLastCallFailedPreventsRaceCondition() $client = $this->createMock(HttpClient::class); $client->method('request')->willReturnMap([ - ['GET', '200.json', $response1], - ['GET', '500.json', $response2], + ['GET', '200.json', '', $response1], + ['GET', '500.json', '', $response2], ]); $api1 = new class ($client) extends AbstractApi { From 40d0a3a9b34da3843f5cc0ef724c4a4196adf2e0 Mon Sep 17 00:00:00 2001 From: Art4 Date: Mon, 8 Jan 2024 15:57:54 +0100 Subject: [PATCH 19/29] increase code coverage in AbstractApi::lastCallFailed() --- tests/Unit/Api/AbstractApiTest.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/Unit/Api/AbstractApiTest.php b/tests/Unit/Api/AbstractApiTest.php index 321d2bf5..66894a0b 100644 --- a/tests/Unit/Api/AbstractApiTest.php +++ b/tests/Unit/Api/AbstractApiTest.php @@ -120,8 +120,11 @@ public function __construct($client) } }; + $api3 = new class ($client) extends AbstractApi {}; + $this->assertSame(false, $api1->lastCallFailed()); $this->assertSame(true, $api2->lastCallFailed()); + $this->assertSame(true, $api3->lastCallFailed()); } /** From c27b38e0375ca8e0f5736d7fd70373ccec4c9410 Mon Sep 17 00:00:00 2001 From: Art4 Date: Mon, 8 Jan 2024 16:39:15 +0100 Subject: [PATCH 20/29] Use HttpClient in Issue::setIssueStatus() --- src/Redmine/Api/AbstractApi.php | 2 +- src/Redmine/Api/Issue.php | 26 ++++++++++++++++++++----- tests/Unit/Api/IssueTest.php | 34 +++++++++++++++++++++++++++++++-- 3 files changed, 54 insertions(+), 8 deletions(-) diff --git a/src/Redmine/Api/AbstractApi.php b/src/Redmine/Api/AbstractApi.php index c21a0cdc..50045e7c 100644 --- a/src/Redmine/Api/AbstractApi.php +++ b/src/Redmine/Api/AbstractApi.php @@ -158,7 +158,7 @@ protected function post($path, $data) * @param string $path * @param string $data * - * @return string|SimpleXMLElement|false + * @return string|SimpleXMLElement */ protected function put($path, $data) { diff --git a/src/Redmine/Api/Issue.php b/src/Redmine/Api/Issue.php index 6b94d0a2..532c70df 100644 --- a/src/Redmine/Api/Issue.php +++ b/src/Redmine/Api/Issue.php @@ -2,12 +2,15 @@ namespace Redmine\Api; +use Redmine\Client\NativeCurlClient; +use Redmine\Client\Psr18Client; use Redmine\Exception; use Redmine\Exception\SerializerException; use Redmine\Exception\UnexpectedResponseException; use Redmine\Serializer\JsonSerializer; use Redmine\Serializer\PathSerializer; use Redmine\Serializer\XmlSerializer; +use SimpleXMLElement; /** * Listing issues, searching, editing and closing your projects issues. @@ -24,6 +27,11 @@ class Issue extends AbstractApi public const PRIO_URGENT = 4; public const PRIO_IMMEDIATE = 5; + /** + * @var IssueStatus + */ + private $issueStatusApi; + /** * List issues. * @@ -160,7 +168,7 @@ public function create(array $params = []) * * @param string $id the issue number * - * @return string|false + * @return string|SimpleXMLElement|false */ public function update($id, array $params) { @@ -219,15 +227,23 @@ public function removeWatcher($id, $watcherUserId) * @param int $id * @param string $status * - * @return string|false + * @return string|SimpleXMLElement|false */ public function setIssueStatus($id, $status) { - /** @var IssueStatus */ - $api = $this->client->getApi('issue_status'); + if ($this->issueStatusApi === null) { + if ($this->client !== null && ! $this->client instanceof NativeCurlClient && ! $this->client instanceof Psr18Client) { + /** @var IssueStatus */ + $issueStatusApi = $this->client->getApi('issue_status'); + } else { + $issueStatusApi = new IssueStatus($this->getHttpClient()); + } + + $this->issueStatusApi = $issueStatusApi; + } return $this->update($id, [ - 'status_id' => $api->getIdByName($status), + 'status_id' => $this->issueStatusApi->getIdByName($status), ]); } diff --git a/tests/Unit/Api/IssueTest.php b/tests/Unit/Api/IssueTest.php index 63fc9239..b2217f89 100644 --- a/tests/Unit/Api/IssueTest.php +++ b/tests/Unit/Api/IssueTest.php @@ -5,7 +5,11 @@ use PHPUnit\Framework\TestCase; use Redmine\Api\Issue; use Redmine\Client\Client; +use Redmine\Http\HttpClient; +use Redmine\Http\Response; +use Redmine\Serializer\XmlSerializer; use Redmine\Tests\Fixtures\MockClient; +use SimpleXMLElement; /** * @coversDefaultClass \Redmine\Api\Issue @@ -668,13 +672,13 @@ public function testUpdateCleansParameters() * @covers ::setIssueStatus * @test */ - public function testSetIssueStatus() + public function testSetIssueStatusWithClient() { // Test values $response = 'API Response'; // Create the used mock objects - $issueStatusApi = $this->createMock('Redmine\Api\Project'); + $issueStatusApi = $this->createMock('Redmine\Api\IssueStatus'); $issueStatusApi->expects($this->once()) ->method('getIdByName') ->willReturn(123); @@ -708,6 +712,32 @@ public function testSetIssueStatus() $this->assertSame($response, $api->setIssueStatus(5, 'Status Name')); } + /** + * Test setIssueStatus(). + * + * @covers ::setIssueStatus + * @test + */ + public function testSetIssueStatusWithHttpClient() + { + $response1 = $this->createMock(Response::class); + $response1->method('getContentType')->willReturn('application/json'); + $response1->method('getBody')->willReturn('{"issue_statuses":[{"name":"status_name","id":123}]}'); + + $response2 = $this->createMock(Response::class); + $response2->method('getContentType')->willReturn('application/xml'); + $response2->method('getBody')->willReturn(''); + + $client = $this->createMock(HttpClient::class); + $client->expects($this->exactly(2))->method('request')->willReturn($response1, $response2); + + // Create the object under test + $api = new Issue($client); + + // Perform the tests + $this->assertInstanceOf(SimpleXMLElement::class, $api->setIssueStatus(5, 'status_name')); + } + /** * Test addNoteToIssue(). * From b2cc80096700742ab4c8d3f56694a41bc38fc635 Mon Sep 17 00:00:00 2001 From: Art4 Date: Mon, 8 Jan 2024 16:49:03 +0100 Subject: [PATCH 21/29] Remove tests moved already to DeleteTest class --- tests/Unit/Api/AbstractApiTest.php | 36 ------------------------------ 1 file changed, 36 deletions(-) diff --git a/tests/Unit/Api/AbstractApiTest.php b/tests/Unit/Api/AbstractApiTest.php index 66894a0b..a63c6c57 100644 --- a/tests/Unit/Api/AbstractApiTest.php +++ b/tests/Unit/Api/AbstractApiTest.php @@ -237,42 +237,6 @@ public static function getLastCallFailedData(): array ]; } - /** - * @covers \Redmine\Api\AbstractApi - * @test - * @dataProvider getXmlDecodingFromRequestMethodsData - */ - public function testXmlDecodingFromRequestMethods($methodName, $response, $decode, $expected) - { - $client = $this->createMock(Client::class); - $client->method('getLastResponseBody')->willReturn($response); - $client->method('getLastResponseContentType')->willReturn('application/xml'); - - $api = new class ($client) extends AbstractApi {}; - - $method = new ReflectionMethod($api, $methodName); - $method->setAccessible(true); - - // Perform the tests - if ('delete' === $methodName) { - $return = $method->invoke($api, 'path'); - - $this->assertSame($expected, $return); - } else { - $return = $method->invoke($api, 'path', ''); - - $this->assertInstanceOf(SimpleXMLElement::class, $return); - $this->assertXmlStringEqualsXmlString($expected, $return->asXML()); - } - } - - public static function getXmlDecodingFromRequestMethodsData(): array - { - return [ - ['delete', '', null, ''], - ]; - } - /** * @covers \Redmine\Api\AbstractApi::retrieveData * From b649237a05faefb71f843cabd86c8cf8fe3cd787 Mon Sep 17 00:00:00 2001 From: Art4 Date: Mon, 8 Jan 2024 16:49:45 +0100 Subject: [PATCH 22/29] improve test script --- composer.json | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/composer.json b/composer.json index 59982d2f..f9a45932 100644 --- a/composer.json +++ b/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" + ] } } From d50b2bd398a1ac070f00d36c205d3273418d9dcf Mon Sep 17 00:00:00 2001 From: Art4 Date: Tue, 9 Jan 2024 13:47:36 +0100 Subject: [PATCH 23/29] Use IssueStatus Api in IssueApi --- src/Redmine/Api/Issue.php | 42 +++++++++----- tests/Unit/Api/IssueTest.php | 104 +++++++++++++++++++++++++++++++---- 2 files changed, 119 insertions(+), 27 deletions(-) diff --git a/src/Redmine/Api/Issue.php b/src/Redmine/Api/Issue.php index 532c70df..a571518e 100644 --- a/src/Redmine/Api/Issue.php +++ b/src/Redmine/Api/Issue.php @@ -133,7 +133,7 @@ public function show($id, array $params = []) * * @param array $params the new issue data * - * @return string|false + * @return string|SimpleXMLElement|false */ public function create(array $params = []) { @@ -231,19 +231,10 @@ public function removeWatcher($id, $watcherUserId) */ public function setIssueStatus($id, $status) { - if ($this->issueStatusApi === null) { - if ($this->client !== null && ! $this->client instanceof NativeCurlClient && ! $this->client instanceof Psr18Client) { - /** @var IssueStatus */ - $issueStatusApi = $this->client->getApi('issue_status'); - } else { - $issueStatusApi = new IssueStatus($this->getHttpClient()); - } - - $this->issueStatusApi = $issueStatusApi; - } + $issueStatusApi = $this->getIssueStatusApi(); return $this->update($id, [ - 'status_id' => $this->issueStatusApi->getIdByName($status), + 'status_id' => $issueStatusApi->getIdByName($status), ]); } @@ -281,12 +272,14 @@ private function cleanParams(array $params = []) unset($params['category']); } } + if (isset($params['status'])) { - /** @var IssueStatus */ - $apiIssueStatus = $this->client->getApi('issue_status'); - $params['status_id'] = $apiIssueStatus->getIdByName($params['status']); + $issueStatusApi = $this->getIssueStatusApi(); + + $params['status_id'] = $issueStatusApi->getIdByName($params['status']); unset($params['status']); } + if (isset($params['tracker'])) { /** @var Tracker */ $apiTracker = $this->client->getApi('tracker'); @@ -361,4 +354,23 @@ public function remove($id) { return $this->delete('/issues/' . $id . '.xml'); } + + /** + * @return IssueStatus + */ + private function getIssueStatusApi() + { + if ($this->issueStatusApi === null) { + if ($this->client !== null && ! $this->client instanceof NativeCurlClient && ! $this->client instanceof Psr18Client) { + /** @var IssueStatus */ + $issueStatusApi = $this->client->getApi('issue_status'); + } else { + $issueStatusApi = new IssueStatus($this->getHttpClient()); + } + + $this->issueStatusApi = $issueStatusApi; + } + + return $this->issueStatusApi; + } } diff --git a/tests/Unit/Api/IssueTest.php b/tests/Unit/Api/IssueTest.php index b2217f89..8263f7c1 100644 --- a/tests/Unit/Api/IssueTest.php +++ b/tests/Unit/Api/IssueTest.php @@ -7,7 +7,6 @@ use Redmine\Client\Client; use Redmine\Http\HttpClient; use Redmine\Http\Response; -use Redmine\Serializer\XmlSerializer; use Redmine\Tests\Fixtures\MockClient; use SimpleXMLElement; @@ -437,7 +436,7 @@ public function testCreateCallsPost() * @covers ::cleanParams * @test */ - public function testCreateCleansParameters() + public function testCreateWithClientCleansParameters() { // Test values $response = 'API Response'; @@ -504,6 +503,59 @@ public function testCreateCleansParameters() $this->assertSame($response, $api->create($parameters)); } + /** + * @covers ::create + * @covers ::cleanParams + * @test + */ + public function testCreateWithHttpClientRetrievesIssueStatusId() + { + $client = $this->createMock(HttpClient::class); + $client->expects($this->exactly(2)) + ->method('request') + ->willReturnCallback(function(string $method, string $path, string $body = '') { + if ($method === 'GET') { + $this->assertSame('/issue_statuses.json', $path); + $this->assertSame('', $body); + + return $this->createConfiguredMock( + Response::class, + [ + 'getContentType' => 'application/json', + 'getBody' => '{"issue_statuses":[{"name":"Status Name","id":123}]}', + ] + ); + } + + if ($method === 'POST') { + $this->assertSame('/issues.xml', $path); + $this->assertXmlStringEqualsXmlString('123', $body); + + return $this->createConfiguredMock( + Response::class, + [ + 'getContentType' => 'application/xml', + 'getBody' => '', + ] + ); + } + + throw new \Exception(); + }); + + // Create the object under test + $api = new Issue($client); + + $xmlElement = $api->create(['status' => 'Status Name']); + + // Perform the tests + $this->assertInstanceOf(SimpleXMLElement::class, $xmlElement); + $this->assertXmlStringEqualsXmlString( + '', + $xmlElement->asXml(), + ); + } + /** * Test create() and buildXML(). * @@ -720,22 +772,50 @@ public function testSetIssueStatusWithClient() */ public function testSetIssueStatusWithHttpClient() { - $response1 = $this->createMock(Response::class); - $response1->method('getContentType')->willReturn('application/json'); - $response1->method('getBody')->willReturn('{"issue_statuses":[{"name":"status_name","id":123}]}'); - - $response2 = $this->createMock(Response::class); - $response2->method('getContentType')->willReturn('application/xml'); - $response2->method('getBody')->willReturn(''); - $client = $this->createMock(HttpClient::class); - $client->expects($this->exactly(2))->method('request')->willReturn($response1, $response2); + $client->expects($this->exactly(2)) + ->method('request') + ->willReturnCallback(function(string $method, string $path, string $body = '') { + if ($method === 'GET') { + $this->assertSame('/issue_statuses.json', $path); + $this->assertSame('', $body); + + return $this->createConfiguredMock( + Response::class, + [ + 'getContentType' => 'application/json', + 'getBody' => '{"issue_statuses":[{"name":"Status Name","id":123}]}', + ] + ); + } + + if ($method === 'PUT') { + $this->assertSame('/issues/5.xml', $path); + $this->assertXmlStringEqualsXmlString('5123', $body); + + return $this->createConfiguredMock( + Response::class, + [ + 'getContentType' => 'application/xml', + 'getBody' => '', + ] + ); + } + + throw new \Exception(); + }); // Create the object under test $api = new Issue($client); + $xmlElement = $api->setIssueStatus(5, 'Status Name'); + // Perform the tests - $this->assertInstanceOf(SimpleXMLElement::class, $api->setIssueStatus(5, 'status_name')); + $this->assertInstanceOf(SimpleXMLElement::class, $xmlElement); + $this->assertXmlStringEqualsXmlString( + '', + $xmlElement->asXml(), + ); } /** From 8318db9d3a304d3ca8c745c6ea2033153f81f9a1 Mon Sep 17 00:00:00 2001 From: Art4 Date: Tue, 9 Jan 2024 14:09:01 +0100 Subject: [PATCH 24/29] Use Project Api in IssueApi --- src/Redmine/Api/Issue.php | 44 ++++++++++++++++++++++++------ tests/Unit/Api/IssueTest.php | 53 ++++++++++++++++++++++++++++++++++++ 2 files changed, 88 insertions(+), 9 deletions(-) diff --git a/src/Redmine/Api/Issue.php b/src/Redmine/Api/Issue.php index a571518e..ce9d7582 100644 --- a/src/Redmine/Api/Issue.php +++ b/src/Redmine/Api/Issue.php @@ -32,6 +32,11 @@ class Issue extends AbstractApi */ private $issueStatusApi; + /** + * @var Project + */ + private $projectApi; + /** * List issues. * @@ -261,16 +266,18 @@ public function addNoteToIssue($id, $note, $privateNote = false) private function cleanParams(array $params = []) { if (isset($params['project'])) { - /** @var Project */ - $apiProject = $this->client->getApi('project'); - $params['project_id'] = $apiProject->getIdByName($params['project']); + $projectApi = $this->getProjectApi(); + + $params['project_id'] = $projectApi->getIdByName($params['project']); unset($params['project']); - if (isset($params['category'])) { - /** @var IssueCategory */ - $apiIssueCategory = $this->client->getApi('issue_category'); - $params['category_id'] = $apiIssueCategory->getIdByName($params['project_id'], $params['category']); - unset($params['category']); - } + + } + + if (isset($params['category']) && isset($params['project_id'])) { + /** @var IssueCategory */ + $apiIssueCategory = $this->client->getApi('issue_category'); + $params['category_id'] = $apiIssueCategory->getIdByName($params['project_id'], $params['category']); + unset($params['category']); } if (isset($params['status'])) { @@ -373,4 +380,23 @@ private function getIssueStatusApi() return $this->issueStatusApi; } + + /** + * @return Project + */ + private function getProjectApi() + { + if ($this->projectApi === null) { + if ($this->client !== null && ! $this->client instanceof NativeCurlClient && ! $this->client instanceof Psr18Client) { + /** @var Project */ + $projectApi = $this->client->getApi('project'); + } else { + $projectApi = new Project($this->getHttpClient()); + } + + $this->projectApi = $projectApi; + } + + return $this->projectApi; + } } diff --git a/tests/Unit/Api/IssueTest.php b/tests/Unit/Api/IssueTest.php index 8263f7c1..10ab27b6 100644 --- a/tests/Unit/Api/IssueTest.php +++ b/tests/Unit/Api/IssueTest.php @@ -556,6 +556,59 @@ public function testCreateWithHttpClientRetrievesIssueStatusId() ); } + /** + * @covers ::create + * @covers ::cleanParams + * @test + */ + public function testCreateWithHttpClientRetrievesProjectId() + { + $client = $this->createMock(HttpClient::class); + $client->expects($this->exactly(2)) + ->method('request') + ->willReturnCallback(function(string $method, string $path, string $body = '') { + if ($method === 'GET') { + $this->assertSame('/projects.json', $path); + $this->assertSame('', $body); + + return $this->createConfiguredMock( + Response::class, + [ + 'getContentType' => 'application/json', + 'getBody' => '{"projects":[{"name":"Project Name","id":3}]}', + ] + ); + } + + if ($method === 'POST') { + $this->assertSame('/issues.xml', $path); + $this->assertXmlStringEqualsXmlString('3', $body); + + return $this->createConfiguredMock( + Response::class, + [ + 'getContentType' => 'application/xml', + 'getBody' => '', + ] + ); + } + + throw new \Exception(); + }); + + // Create the object under test + $api = new Issue($client); + + $xmlElement = $api->create(['project' => 'Project Name']); + + // Perform the tests + $this->assertInstanceOf(SimpleXMLElement::class, $xmlElement); + $this->assertXmlStringEqualsXmlString( + '', + $xmlElement->asXml(), + ); + } + /** * Test create() and buildXML(). * From 8bf399b0ae4a0ae9d0ed697dca91a3302ceca236 Mon Sep 17 00:00:00 2001 From: Art4 Date: Tue, 9 Jan 2024 14:50:57 +0100 Subject: [PATCH 25/29] Use IssueCategory Api in IssueApi --- src/Redmine/Api/Issue.php | 31 ++++++++++++++++--- tests/Unit/Api/IssueTest.php | 59 ++++++++++++++++++++++++++++++++++-- 2 files changed, 83 insertions(+), 7 deletions(-) diff --git a/src/Redmine/Api/Issue.php b/src/Redmine/Api/Issue.php index ce9d7582..1e345933 100644 --- a/src/Redmine/Api/Issue.php +++ b/src/Redmine/Api/Issue.php @@ -27,6 +27,11 @@ class Issue extends AbstractApi public const PRIO_URGENT = 4; public const PRIO_IMMEDIATE = 5; + /** + * @var IssueCategory + */ + private $issueCategoryApi; + /** * @var IssueStatus */ @@ -270,13 +275,12 @@ private function cleanParams(array $params = []) $params['project_id'] = $projectApi->getIdByName($params['project']); unset($params['project']); - } if (isset($params['category']) && isset($params['project_id'])) { - /** @var IssueCategory */ - $apiIssueCategory = $this->client->getApi('issue_category'); - $params['category_id'] = $apiIssueCategory->getIdByName($params['project_id'], $params['category']); + $issueCategoryApi = $this->getIssueCategoryApi(); + + $params['category_id'] = $issueCategoryApi->getIdByName($params['project_id'], $params['category']); unset($params['category']); } @@ -362,6 +366,25 @@ public function remove($id) return $this->delete('/issues/' . $id . '.xml'); } + /** + * @return IssueCategory + */ + private function getIssueCategoryApi() + { + if ($this->issueCategoryApi === null) { + if ($this->client !== null && ! $this->client instanceof NativeCurlClient && ! $this->client instanceof Psr18Client) { + /** @var IssueCategory */ + $issueCategoryApi = $this->client->getApi('issue_category'); + } else { + $issueCategoryApi = new IssueCategory($this->getHttpClient()); + } + + $this->issueCategoryApi = $issueCategoryApi; + } + + return $this->issueCategoryApi; + } + /** * @return IssueStatus */ diff --git a/tests/Unit/Api/IssueTest.php b/tests/Unit/Api/IssueTest.php index 10ab27b6..bc92aa2e 100644 --- a/tests/Unit/Api/IssueTest.php +++ b/tests/Unit/Api/IssueTest.php @@ -513,7 +513,7 @@ public function testCreateWithHttpClientRetrievesIssueStatusId() $client = $this->createMock(HttpClient::class); $client->expects($this->exactly(2)) ->method('request') - ->willReturnCallback(function(string $method, string $path, string $body = '') { + ->willReturnCallback(function (string $method, string $path, string $body = '') { if ($method === 'GET') { $this->assertSame('/issue_statuses.json', $path); $this->assertSame('', $body); @@ -566,7 +566,7 @@ public function testCreateWithHttpClientRetrievesProjectId() $client = $this->createMock(HttpClient::class); $client->expects($this->exactly(2)) ->method('request') - ->willReturnCallback(function(string $method, string $path, string $body = '') { + ->willReturnCallback(function (string $method, string $path, string $body = '') { if ($method === 'GET') { $this->assertSame('/projects.json', $path); $this->assertSame('', $body); @@ -609,6 +609,59 @@ public function testCreateWithHttpClientRetrievesProjectId() ); } + /** + * @covers ::create + * @covers ::cleanParams + * @test + */ + public function testCreateWithHttpClientRetrievesIssueCategoryId() + { + $client = $this->createMock(HttpClient::class); + $client->expects($this->exactly(2)) + ->method('request') + ->willReturnCallback(function (string $method, string $path, string $body = '') { + if ($method === 'GET') { + $this->assertSame('/projects/3/issue_categories.json', $path); + $this->assertSame('', $body); + + return $this->createConfiguredMock( + Response::class, + [ + 'getContentType' => 'application/json', + 'getBody' => '{"issue_categories":[{"name":"Category Name","id":45}]}', + ] + ); + } + + if ($method === 'POST') { + $this->assertSame('/issues.xml', $path); + $this->assertXmlStringEqualsXmlString('345', $body); + + return $this->createConfiguredMock( + Response::class, + [ + 'getContentType' => 'application/xml', + 'getBody' => '', + ] + ); + } + + throw new \Exception(); + }); + + // Create the object under test + $api = new Issue($client); + + $xmlElement = $api->create(['project_id' => 3, 'category' => 'Category Name']); + + // Perform the tests + $this->assertInstanceOf(SimpleXMLElement::class, $xmlElement); + $this->assertXmlStringEqualsXmlString( + '', + $xmlElement->asXml(), + ); + } + /** * Test create() and buildXML(). * @@ -828,7 +881,7 @@ public function testSetIssueStatusWithHttpClient() $client = $this->createMock(HttpClient::class); $client->expects($this->exactly(2)) ->method('request') - ->willReturnCallback(function(string $method, string $path, string $body = '') { + ->willReturnCallback(function (string $method, string $path, string $body = '') { if ($method === 'GET') { $this->assertSame('/issue_statuses.json', $path); $this->assertSame('', $body); From bdd04dfa544c7d10c080b800b8b6a5c4b44194c6 Mon Sep 17 00:00:00 2001 From: Art4 Date: Tue, 9 Jan 2024 15:07:06 +0100 Subject: [PATCH 26/29] Use Tracker Api in IssueApi --- src/Redmine/Api/Issue.php | 31 +++++++++++++++++-- tests/Unit/Api/IssueTest.php | 60 ++++++++++++++++++++++++++++++++++++ 2 files changed, 88 insertions(+), 3 deletions(-) diff --git a/src/Redmine/Api/Issue.php b/src/Redmine/Api/Issue.php index 1e345933..4389f71d 100644 --- a/src/Redmine/Api/Issue.php +++ b/src/Redmine/Api/Issue.php @@ -42,6 +42,11 @@ class Issue extends AbstractApi */ private $projectApi; + /** + * @var Tracker + */ + private $trackerApi; + /** * List issues. * @@ -292,11 +297,12 @@ private function cleanParams(array $params = []) } if (isset($params['tracker'])) { - /** @var Tracker */ - $apiTracker = $this->client->getApi('tracker'); - $params['tracker_id'] = $apiTracker->getIdByName($params['tracker']); + $trackerApi = $this->getTrackerApi(); + + $params['tracker_id'] = $trackerApi->getIdByName($params['tracker']); unset($params['tracker']); } + if (isset($params['assigned_to'])) { /** @var User */ $apiUser = $this->client->getApi('user'); @@ -422,4 +428,23 @@ private function getProjectApi() return $this->projectApi; } + + /** + * @return Tracker + */ + private function getTrackerApi() + { + if ($this->trackerApi === null) { + if ($this->client !== null && ! $this->client instanceof NativeCurlClient && ! $this->client instanceof Psr18Client) { + /** @var Tracker */ + $trackerApi = $this->client->getApi('tracker'); + } else { + $trackerApi = new Tracker($this->getHttpClient()); + } + + $this->trackerApi = $trackerApi; + } + + return $this->trackerApi; + } } diff --git a/tests/Unit/Api/IssueTest.php b/tests/Unit/Api/IssueTest.php index bc92aa2e..742622af 100644 --- a/tests/Unit/Api/IssueTest.php +++ b/tests/Unit/Api/IssueTest.php @@ -434,6 +434,9 @@ public function testCreateCallsPost() * * @covers ::create * @covers ::cleanParams + * @covers ::getIssueCategoryApi + * @covers ::getIssueStatusApi + * @covers ::getProjectApi * @test */ public function testCreateWithClientCleansParameters() @@ -506,6 +509,7 @@ public function testCreateWithClientCleansParameters() /** * @covers ::create * @covers ::cleanParams + * @covers ::getIssueStatusApi * @test */ public function testCreateWithHttpClientRetrievesIssueStatusId() @@ -559,6 +563,7 @@ public function testCreateWithHttpClientRetrievesIssueStatusId() /** * @covers ::create * @covers ::cleanParams + * @covers ::getProjectApi * @test */ public function testCreateWithHttpClientRetrievesProjectId() @@ -612,6 +617,7 @@ public function testCreateWithHttpClientRetrievesProjectId() /** * @covers ::create * @covers ::cleanParams + * @covers ::getIssueCategoryApi * @test */ public function testCreateWithHttpClientRetrievesIssueCategoryId() @@ -662,6 +668,60 @@ public function testCreateWithHttpClientRetrievesIssueCategoryId() ); } + /** + * @covers ::create + * @covers ::cleanParams + * @covers ::getTrackerApi + * @test + */ + public function testCreateWithHttpClientRetrievesTrackerId() + { + $client = $this->createMock(HttpClient::class); + $client->expects($this->exactly(2)) + ->method('request') + ->willReturnCallback(function (string $method, string $path, string $body = '') { + if ($method === 'GET') { + $this->assertSame('/trackers.json', $path); + $this->assertSame('', $body); + + return $this->createConfiguredMock( + Response::class, + [ + 'getContentType' => 'application/json', + 'getBody' => '{"trackers":[{"name":"Tracker Name","id":9}]}', + ] + ); + } + + if ($method === 'POST') { + $this->assertSame('/issues.xml', $path); + $this->assertXmlStringEqualsXmlString('9', $body); + + return $this->createConfiguredMock( + Response::class, + [ + 'getContentType' => 'application/xml', + 'getBody' => '', + ] + ); + } + + throw new \Exception(); + }); + + // Create the object under test + $api = new Issue($client); + + $xmlElement = $api->create(['tracker' => 'Tracker Name']); + + // Perform the tests + $this->assertInstanceOf(SimpleXMLElement::class, $xmlElement); + $this->assertXmlStringEqualsXmlString( + '', + $xmlElement->asXml(), + ); + } + /** * Test create() and buildXML(). * From 3e15c737c4631aadce1fd46d55ae70de9c576023 Mon Sep 17 00:00:00 2001 From: Art4 Date: Tue, 9 Jan 2024 15:31:49 +0100 Subject: [PATCH 27/29] Use User Api in IssueApi --- src/Redmine/Api/Issue.php | 37 ++++++++++++++++++---- tests/Unit/Api/IssueTest.php | 60 ++++++++++++++++++++++++++++++++++-- 2 files changed, 89 insertions(+), 8 deletions(-) diff --git a/src/Redmine/Api/Issue.php b/src/Redmine/Api/Issue.php index 4389f71d..f92bd8a2 100644 --- a/src/Redmine/Api/Issue.php +++ b/src/Redmine/Api/Issue.php @@ -47,6 +47,11 @@ class Issue extends AbstractApi */ private $trackerApi; + /** + * @var User + */ + private $userApi; + /** * List issues. * @@ -304,15 +309,16 @@ private function cleanParams(array $params = []) } if (isset($params['assigned_to'])) { - /** @var User */ - $apiUser = $this->client->getApi('user'); - $params['assigned_to_id'] = $apiUser->getIdByUsername($params['assigned_to']); + $userApi = $this->getUserApi(); + + $params['assigned_to_id'] = $userApi->getIdByUsername($params['assigned_to']); unset($params['assigned_to']); } + if (isset($params['author'])) { - /** @var User */ - $apiUser = $this->client->getApi('user'); - $params['author_id'] = $apiUser->getIdByUsername($params['author']); + $userApi = $this->getUserApi(); + + $params['author_id'] = $userApi->getIdByUsername($params['author']); unset($params['author']); } @@ -447,4 +453,23 @@ private function getTrackerApi() return $this->trackerApi; } + + /** + * @return User + */ + private function getUserApi() + { + if ($this->userApi === null) { + if ($this->client !== null && ! $this->client instanceof NativeCurlClient && ! $this->client instanceof Psr18Client) { + /** @var User */ + $userApi = $this->client->getApi('user'); + } else { + $userApi = new User($this->getHttpClient()); + } + + $this->userApi = $userApi; + } + + return $this->userApi; + } } diff --git a/tests/Unit/Api/IssueTest.php b/tests/Unit/Api/IssueTest.php index 742622af..582e8c38 100644 --- a/tests/Unit/Api/IssueTest.php +++ b/tests/Unit/Api/IssueTest.php @@ -437,6 +437,8 @@ public function testCreateCallsPost() * @covers ::getIssueCategoryApi * @covers ::getIssueStatusApi * @covers ::getProjectApi + * @covers ::getTrackerApi + * @covers ::getUserApi * @test */ public function testCreateWithClientCleansParameters() @@ -467,7 +469,7 @@ public function testCreateWithClientCleansParameters() ->willReturn('cleanedValue'); $client = $this->createMock(Client::class); - $client->expects($this->exactly(6)) + $client->expects($this->exactly(5)) ->method('getApi') ->willReturnMap( [ @@ -722,6 +724,60 @@ public function testCreateWithHttpClientRetrievesTrackerId() ); } + /** + * @covers ::create + * @covers ::cleanParams + * @covers ::getUserApi + * @test + */ + public function testCreateWithHttpClientRetrievesUserId() + { + $client = $this->createMock(HttpClient::class); + $client->expects($this->exactly(2)) + ->method('request') + ->willReturnCallback(function (string $method, string $path, string $body = '') { + if ($method === 'GET') { + $this->assertSame('/users.json', $path); + $this->assertSame('', $body); + + return $this->createConfiguredMock( + Response::class, + [ + 'getContentType' => 'application/json', + 'getBody' => '{"users":[{"login":"Author Name","id":5},{"login":"Assigned to User Name","id":6}]}', + ] + ); + } + + if ($method === 'POST') { + $this->assertSame('/issues.xml', $path); + $this->assertXmlStringEqualsXmlString('65', $body); + + return $this->createConfiguredMock( + Response::class, + [ + 'getContentType' => 'application/xml', + 'getBody' => '', + ] + ); + } + + throw new \Exception(); + }); + + // Create the object under test + $api = new Issue($client); + + $xmlElement = $api->create(['assigned_to' => 'Assigned to User Name', 'author' => 'Author Name']); + + // Perform the tests + $this->assertInstanceOf(SimpleXMLElement::class, $xmlElement); + $this->assertXmlStringEqualsXmlString( + '', + $xmlElement->asXml(), + ); + } + /** * Test create() and buildXML(). * @@ -844,7 +900,7 @@ public function testUpdateCleansParameters() ->willReturn('cleanedValue'); $client = $this->createMock(Client::class); - $client->expects($this->exactly(6)) + $client->expects($this->exactly(5)) ->method('getApi') ->willReturnMap( [ From 43120390a3bc1cf85b97a3ceedfbca5a3b9c3631 Mon Sep 17 00:00:00 2001 From: Art4 Date: Tue, 9 Jan 2024 15:55:54 +0100 Subject: [PATCH 28/29] Fix typo --- src/Redmine/Api/AbstractApi.php | 4 +-- src/Redmine/Api/CustomField.php | 2 +- src/Redmine/Api/Group.php | 2 +- src/Redmine/Api/Issue.php | 2 +- src/Redmine/Api/IssueCategory.php | 2 +- src/Redmine/Api/IssuePriority.php | 2 +- src/Redmine/Api/IssueRelation.php | 2 +- src/Redmine/Api/IssueStatus.php | 2 +- src/Redmine/Api/Membership.php | 2 +- src/Redmine/Api/News.php | 2 +- src/Redmine/Api/Project.php | 2 +- src/Redmine/Api/Query.php | 2 +- src/Redmine/Api/Role.php | 2 +- src/Redmine/Api/Search.php | 2 +- src/Redmine/Api/TimeEntry.php | 2 +- src/Redmine/Api/TimeEntryActivity.php | 2 +- src/Redmine/Api/Tracker.php | 2 +- src/Redmine/Api/User.php | 2 +- src/Redmine/Api/Version.php | 2 +- src/Redmine/Api/Wiki.php | 2 +- tests/Unit/Api/AbstractApiTest.php | 35 +++++++++++++++++++++------ 21 files changed, 49 insertions(+), 28 deletions(-) diff --git a/src/Redmine/Api/AbstractApi.php b/src/Redmine/Api/AbstractApi.php index 50045e7c..3b2851c2 100644 --- a/src/Redmine/Api/AbstractApi.php +++ b/src/Redmine/Api/AbstractApi.php @@ -70,7 +70,7 @@ final protected function getHttpClient(): HttpClient return $this->httpClient; } - final protected function getLastResonse(): Response + final protected function getLastResponse(): Response { return $this->lastResponse !== null ? $this->lastResponse : $this->createResponse(0, '', ''); } @@ -234,7 +234,7 @@ protected function retrieveAll($endpoint, array $params = []) try { $data = $this->retrieveData(strval($endpoint), $params); } catch (Exception $e) { - if ($this->getLastResonse()->getBody() === '') { + if ($this->getLastResponse()->getBody() === '') { return false; } diff --git a/src/Redmine/Api/CustomField.php b/src/Redmine/Api/CustomField.php index fafda02c..1b9d10c6 100644 --- a/src/Redmine/Api/CustomField.php +++ b/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->getLastResonse()->getBody() === '') { + if ($this->getLastResponse()->getBody() === '') { return false; } diff --git a/src/Redmine/Api/Group.php b/src/Redmine/Api/Group.php index a1c13281..206dcc62 100644 --- a/src/Redmine/Api/Group.php +++ b/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->getLastResonse()->getBody() === '') { + if ($this->getLastResponse()->getBody() === '') { return false; } diff --git a/src/Redmine/Api/Issue.php b/src/Redmine/Api/Issue.php index f92bd8a2..3d3264cc 100644 --- a/src/Redmine/Api/Issue.php +++ b/src/Redmine/Api/Issue.php @@ -110,7 +110,7 @@ public function all(array $params = []) try { return $this->list($params); } catch (Exception $e) { - if ($this->getLastResonse()->getBody() === '') { + if ($this->getLastResponse()->getBody() === '') { return false; } diff --git a/src/Redmine/Api/IssueCategory.php b/src/Redmine/Api/IssueCategory.php index fcf5d42e..d42900f2 100644 --- a/src/Redmine/Api/IssueCategory.php +++ b/src/Redmine/Api/IssueCategory.php @@ -69,7 +69,7 @@ public function all($project, array $params = []) try { return $this->listByProject(strval($project), $params); } catch (Exception $e) { - if ($this->getLastResonse()->getBody() === '') { + if ($this->getLastResponse()->getBody() === '') { return false; } diff --git a/src/Redmine/Api/IssuePriority.php b/src/Redmine/Api/IssuePriority.php index 2c54fdac..e3e777b3 100644 --- a/src/Redmine/Api/IssuePriority.php +++ b/src/Redmine/Api/IssuePriority.php @@ -55,7 +55,7 @@ public function all(array $params = []) try { $this->issuePriorities = $this->list($params); } catch (Exception $e) { - if ($this->getLastResonse()->getBody() === '') { + if ($this->getLastResponse()->getBody() === '') { return false; } diff --git a/src/Redmine/Api/IssueRelation.php b/src/Redmine/Api/IssueRelation.php index b564a81d..113674e4 100644 --- a/src/Redmine/Api/IssueRelation.php +++ b/src/Redmine/Api/IssueRelation.php @@ -58,7 +58,7 @@ public function all($issueId, array $params = []) try { $this->relations = $this->listByIssueId($issueId, $params); } catch (Exception $e) { - if ($this->getLastResonse()->getBody() === '') { + if ($this->getLastResponse()->getBody() === '') { return false; } diff --git a/src/Redmine/Api/IssueStatus.php b/src/Redmine/Api/IssueStatus.php index 6a908594..fa6fd577 100644 --- a/src/Redmine/Api/IssueStatus.php +++ b/src/Redmine/Api/IssueStatus.php @@ -55,7 +55,7 @@ public function all(array $params = []) try { $this->issueStatuses = $this->list($params); } catch (Exception $e) { - if ($this->getLastResonse()->getBody() === '') { + if ($this->getLastResponse()->getBody() === '') { return false; } diff --git a/src/Redmine/Api/Membership.php b/src/Redmine/Api/Membership.php index 007e8c03..7650ea22 100644 --- a/src/Redmine/Api/Membership.php +++ b/src/Redmine/Api/Membership.php @@ -68,7 +68,7 @@ public function all($project, array $params = []) try { $this->memberships = $this->listByProject(strval($project), $params); } catch (Exception $e) { - if ($this->getLastResonse()->getBody() === '') { + if ($this->getLastResponse()->getBody() === '') { return false; } diff --git a/src/Redmine/Api/News.php b/src/Redmine/Api/News.php index 0fbe7c87..e6366e17 100644 --- a/src/Redmine/Api/News.php +++ b/src/Redmine/Api/News.php @@ -88,7 +88,7 @@ public function all($project = null, array $params = []) $this->news = $this->listByProject(strval($project), $params); } } catch (Exception $e) { - if ($this->getLastResonse()->getBody() === '') { + if ($this->getLastResponse()->getBody() === '') { return false; } diff --git a/src/Redmine/Api/Project.php b/src/Redmine/Api/Project.php index 8617efe9..123086e5 100755 --- a/src/Redmine/Api/Project.php +++ b/src/Redmine/Api/Project.php @@ -58,7 +58,7 @@ public function all(array $params = []) try { $this->projects = $this->list($params); } catch (Exception $e) { - if ($this->getLastResonse()->getBody() === '') { + if ($this->getLastResponse()->getBody() === '') { return false; } diff --git a/src/Redmine/Api/Query.php b/src/Redmine/Api/Query.php index b1e631a6..475a551f 100644 --- a/src/Redmine/Api/Query.php +++ b/src/Redmine/Api/Query.php @@ -55,7 +55,7 @@ public function all(array $params = []) try { $this->query = $this->list($params); } catch (Exception $e) { - if ($this->getLastResonse()->getBody() === '') { + if ($this->getLastResponse()->getBody() === '') { return false; } diff --git a/src/Redmine/Api/Role.php b/src/Redmine/Api/Role.php index 1b6ab97f..67832c89 100644 --- a/src/Redmine/Api/Role.php +++ b/src/Redmine/Api/Role.php @@ -55,7 +55,7 @@ public function all(array $params = []) try { $this->roles = $this->list($params); } catch (Exception $e) { - if ($this->getLastResonse()->getBody() === '') { + if ($this->getLastResponse()->getBody() === '') { return false; } diff --git a/src/Redmine/Api/Search.php b/src/Redmine/Api/Search.php index 08d5737f..f286b9c6 100644 --- a/src/Redmine/Api/Search.php +++ b/src/Redmine/Api/Search.php @@ -55,7 +55,7 @@ public function search($query, array $params = []) try { $this->results = $this->listByQuery($query, $params); } catch (Exception $e) { - if ($this->getLastResonse()->getBody() === '') { + if ($this->getLastResponse()->getBody() === '') { return false; } diff --git a/src/Redmine/Api/TimeEntry.php b/src/Redmine/Api/TimeEntry.php index 9e6fd0ed..092f68e3 100644 --- a/src/Redmine/Api/TimeEntry.php +++ b/src/Redmine/Api/TimeEntry.php @@ -57,7 +57,7 @@ public function all(array $params = []) try { $this->timeEntries = $this->list($params); } catch (Exception $e) { - if ($this->getLastResonse()->getBody() === '') { + if ($this->getLastResponse()->getBody() === '') { return false; } diff --git a/src/Redmine/Api/TimeEntryActivity.php b/src/Redmine/Api/TimeEntryActivity.php index 1704dae3..871fe789 100644 --- a/src/Redmine/Api/TimeEntryActivity.php +++ b/src/Redmine/Api/TimeEntryActivity.php @@ -51,7 +51,7 @@ public function all(array $params = []) try { $this->timeEntryActivities = $this->list($params); } catch (Exception $e) { - if ($this->getLastResonse()->getBody() === '') { + if ($this->getLastResponse()->getBody() === '') { return false; } diff --git a/src/Redmine/Api/Tracker.php b/src/Redmine/Api/Tracker.php index 9073dc53..85f3507c 100644 --- a/src/Redmine/Api/Tracker.php +++ b/src/Redmine/Api/Tracker.php @@ -55,7 +55,7 @@ public function all(array $params = []) try { $this->trackers = $this->list($params); } catch (Exception $e) { - if ($this->getLastResonse()->getBody() === '') { + if ($this->getLastResponse()->getBody() === '') { return false; } diff --git a/src/Redmine/Api/User.php b/src/Redmine/Api/User.php index 1b2f80c6..ca719a70 100644 --- a/src/Redmine/Api/User.php +++ b/src/Redmine/Api/User.php @@ -58,7 +58,7 @@ public function all(array $params = []) try { $this->users = $this->list($params); } catch (Exception $e) { - if ($this->getLastResonse()->getBody() === '') { + if ($this->getLastResponse()->getBody() === '') { return false; } diff --git a/src/Redmine/Api/Version.php b/src/Redmine/Api/Version.php index 04a112ef..3252c985 100644 --- a/src/Redmine/Api/Version.php +++ b/src/Redmine/Api/Version.php @@ -67,7 +67,7 @@ public function all($project, array $params = []) try { $this->versions = $this->listByProject(strval($project), $params); } catch (Exception $e) { - if ($this->getLastResonse()->getBody() === '') { + if ($this->getLastResponse()->getBody() === '') { return false; } diff --git a/src/Redmine/Api/Wiki.php b/src/Redmine/Api/Wiki.php index 3d61832d..7a066978 100644 --- a/src/Redmine/Api/Wiki.php +++ b/src/Redmine/Api/Wiki.php @@ -67,7 +67,7 @@ public function all($project, array $params = []) try { $this->wikiPages = $this->listByProject(strval($project), $params); } catch (Exception $e) { - if ($this->getLastResonse()->getBody() === '') { + if ($this->getLastResponse()->getBody() === '') { return false; } diff --git a/tests/Unit/Api/AbstractApiTest.php b/tests/Unit/Api/AbstractApiTest.php index a63c6c57..153e7656 100644 --- a/tests/Unit/Api/AbstractApiTest.php +++ b/tests/Unit/Api/AbstractApiTest.php @@ -49,6 +49,27 @@ public function testCreateWithoutClitentOrHttpClientThrowsException() new class (new \stdClass()) extends AbstractApi {}; } + /** + * @covers ::getLastResponse + */ + public function testGetLastResponseWithHttpClientWorks() + { + $client = $this->createMock(HttpClient::class); + + $api = new class ($client) extends AbstractApi { + public function __construct($client) + { + parent::__construct($client); + parent::get('200.json', false); + } + }; + + $method = new ReflectionMethod($api, 'getLastResponse'); + $method->setAccessible(true); + + $this->assertInstanceOf(Response::class, $method->invoke($api)); + } + /** * @test * @dataProvider getIsNotNullReturnsCorrectBooleanData @@ -88,7 +109,7 @@ public static function getIsNotNullReturnsCorrectBooleanData(): array } /** - * @covers \Redmine\Api\AbstractApi::lastCallFailed + * @covers ::lastCallFailed */ public function testLastCallFailedPreventsRaceCondition() { @@ -128,7 +149,7 @@ public function __construct($client) } /** - * @covers \Redmine\Api\AbstractApi::lastCallFailed + * @covers ::lastCallFailed * @test * @dataProvider getLastCallFailedData */ @@ -143,7 +164,7 @@ public function testLastCallFailedWithClientReturnsCorrectBoolean($statusCode, $ } /** - * @covers \Redmine\Api\AbstractApi::lastCallFailed + * @covers ::lastCallFailed * @test * @dataProvider getLastCallFailedData */ @@ -238,7 +259,7 @@ public static function getLastCallFailedData(): array } /** - * @covers \Redmine\Api\AbstractApi::retrieveData + * @covers ::retrieveData * * @dataProvider retrieveDataData */ @@ -265,7 +286,7 @@ public static function retrieveDataData(): array } /** - * @covers \Redmine\Api\AbstractApi::retrieveData + * @covers ::retrieveData * * @dataProvider getRetrieveDataToExceptionData */ @@ -295,7 +316,7 @@ public static function getRetrieveDataToExceptionData(): array } /** - * @covers \Redmine\Api\AbstractApi::retrieveAll + * @covers ::retrieveAll * * @dataProvider getRetrieveAllData */ @@ -324,7 +345,7 @@ public static function getRetrieveAllData(): array } /** - * @covers \Redmine\Api\AbstractApi::attachCustomFieldXML + * @covers ::attachCustomFieldXML */ public function testDeprecatedAttachCustomFieldXML() { From 63dfce81b247487b705b13be9fae9da7905631a1 Mon Sep 17 00:00:00 2001 From: Art4 Date: Tue, 9 Jan 2024 16:19:16 +0100 Subject: [PATCH 29/29] simplify client mock --- tests/Unit/Api/AbstractApiTest.php | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/tests/Unit/Api/AbstractApiTest.php b/tests/Unit/Api/AbstractApiTest.php index 153e7656..4cb0c355 100644 --- a/tests/Unit/Api/AbstractApiTest.php +++ b/tests/Unit/Api/AbstractApiTest.php @@ -56,13 +56,7 @@ public function testGetLastResponseWithHttpClientWorks() { $client = $this->createMock(HttpClient::class); - $api = new class ($client) extends AbstractApi { - public function __construct($client) - { - parent::__construct($client); - parent::get('200.json', false); - } - }; + $api = new class ($client) extends AbstractApi {}; $method = new ReflectionMethod($api, 'getLastResponse'); $method->setAccessible(true);