From 92b19de0072316c2977b8b73b22e2d1e70e60e8e Mon Sep 17 00:00:00 2001 From: Art4 Date: Wed, 5 Jan 2022 00:13:42 +0100 Subject: [PATCH 01/15] Add retrieveData method, deprecate retrieveAll --- src/Redmine/Api/AbstractApi.php | 28 ++++++++++++++++++++++++---- tests/Unit/Api/CustomFieldTest.php | 25 +++++++++++++++---------- 2 files changed, 39 insertions(+), 14 deletions(-) diff --git a/src/Redmine/Api/AbstractApi.php b/src/Redmine/Api/AbstractApi.php index 8e85176e..2b4c0252 100644 --- a/src/Redmine/Api/AbstractApi.php +++ b/src/Redmine/Api/AbstractApi.php @@ -51,7 +51,7 @@ public function lastCallFailed() */ protected function get($path, $decodeIfJson = true) { - $this->client->requestGet($path); + $this->client->requestGet(strval($path)); $body = $this->client->getLastResponseBody(); $contentType = $this->client->getLastResponseContentType(); @@ -65,6 +65,7 @@ protected function get($path, $decodeIfJson = true) try { return json_decode($body, true, 512, \JSON_THROW_ON_ERROR); } catch (JsonException $e) { + // TODO: Throw Exception instead of returning string return 'Error decoding body as JSON: '.$e->getMessage(); } } @@ -161,23 +162,42 @@ protected function sanitizeParams(array $defaults, array $params) * Retrieves all the elements of a given endpoint (even if the * total number of elements is greater than 100). * + * @deprecated the `retrieveAll()` method is deprecated, use `retrieveData()` instead. + * * @param string $endpoint API end point * @param array $params optional parameters to be passed to the api (offset, limit, ...) * * @return array elements found */ protected function retrieveAll($endpoint, array $params = []) + { + @trigger_error('The '.__METHOD__.' method is deprecated, use `retrieveData()` instead.', E_USER_DEPRECATED); + + return $this->retrieveData(strval($endpoint), $params); + } + + /** + * Retrieves all the elements of a given endpoint (even if the + * total number of elements is greater than 100). + * + * @param string $endpoint API end point + * @param array $params optional parameters to be passed to the api (offset, limit, ...) + * + * @return array elements found + */ + protected function retrieveData(string $endpoint, array $params = [])/*: array*/ // TODO: check if return type could be array { if (empty($params)) { return $this->get($endpoint); } + $defaults = [ 'limit' => 25, 'offset' => 0, ]; $params = $this->sanitizeParams($defaults, $params); - $ret = []; + $data = []; $limit = $params['limit']; $offset = $params['offset']; @@ -194,7 +214,7 @@ protected function retrieveAll($endpoint, array $params = []) $params['offset'] = $offset; $newDataSet = (array) $this->get($endpoint.'?'.preg_replace('/%5B[0-9]+%5D/simU', '%5B%5D', http_build_query($params))); - $ret = array_merge_recursive($ret, $newDataSet); + $data = array_merge_recursive($data, $newDataSet); $offset += $_limit; if (empty($newDataSet) || !isset($newDataSet['limit']) || ( @@ -207,7 +227,7 @@ protected function retrieveAll($endpoint, array $params = []) } } - return $ret; + return $data; } /** diff --git a/tests/Unit/Api/CustomFieldTest.php b/tests/Unit/Api/CustomFieldTest.php index 31b49e11..a3481100 100644 --- a/tests/Unit/Api/CustomFieldTest.php +++ b/tests/Unit/Api/CustomFieldTest.php @@ -25,7 +25,8 @@ class CustomFieldTest extends TestCase public function testAllReturnsClientGetResponse() { // Test values - $response = 'API Response'; + $response = '["API Response"]'; + $expectedResponse = ['API Response']; // Create the used mock objects $client = $this->createMock(Client::class); @@ -38,12 +39,15 @@ public function testAllReturnsClientGetResponse() $client->expects($this->exactly(1)) ->method('getLastResponseBody') ->willReturn($response); + $client->expects($this->exactly(1)) + ->method('getLastResponseContentType') + ->willReturn('application/json'); // Create the object under test $api = new CustomField($client); // Perform the tests - $this->assertSame($response, $api->all()); + $this->assertSame($expectedResponse, $api->all()); } /** @@ -59,7 +63,8 @@ public function testAllReturnsClientGetResponseWithParameters() { // Test values $allParameters = ['not-used']; - $response = 'API Response'; + $response = '["API Response"]'; + $expectedResponse = ['API Response']; // Create the used mock objects $client = $this->createMock(Client::class); @@ -72,12 +77,15 @@ public function testAllReturnsClientGetResponseWithParameters() $client->expects($this->exactly(1)) ->method('getLastResponseBody') ->willReturn($response); + $client->expects($this->exactly(1)) + ->method('getLastResponseContentType') + ->willReturn('application/json'); // Create the object under test $api = new CustomField($client); // Perform the tests - $this->assertSame([$response], $api->all($allParameters)); + $this->assertSame($expectedResponse, $api->all($allParameters)); } /** @@ -94,8 +102,8 @@ public function testAllReturnsClientGetResponseWithHighLimit() // Test values $response = '{"limit":"100","items":[]}'; $allParameters = ['limit' => 250]; - $returnDataSet = [ - 'limit' => '100', + $expectedResponse = [ + 'limit' => ['100', '100', '100'], // TODO: Check response created by array_merge_recursive() 'items' => [], ]; @@ -118,10 +126,7 @@ public function testAllReturnsClientGetResponseWithHighLimit() $api = new CustomField($client); // Perform the tests - $retrievedDataSet = $api->all($allParameters); - $this->assertTrue(is_array($retrievedDataSet)); - $this->assertArrayHasKey('limit', $retrievedDataSet); - $this->assertArrayHasKey('items', $retrievedDataSet); + $this->assertSame($expectedResponse, $api->all($allParameters)); } /** From 0713b4e3e184d8508a79e277f7452650b3dd4cd4 Mon Sep 17 00:00:00 2001 From: Art4 Date: Thu, 24 Feb 2022 15:03:52 +0100 Subject: [PATCH 02/15] Refactore retrieveData so it will allways return array --- src/Redmine/Api/AbstractApi.php | 100 ++++++++++++++++++++--- tests/Unit/Api/GroupTest.php | 40 ++++++--- tests/Unit/Api/IssueCategoryTest.php | 8 +- tests/Unit/Api/IssuePriorityTest.php | 16 +++- tests/Unit/Api/IssueRelationTest.php | 67 +++++++++------ tests/Unit/Api/IssueStatusTest.php | 16 +++- tests/Unit/Api/IssueTest.php | 60 ++++++++++---- tests/Unit/Api/MembershipTest.php | 16 +++- tests/Unit/Api/NewsTest.php | 24 ++++-- tests/Unit/Api/ProjectTest.php | 32 ++++++-- tests/Unit/Api/QueryTest.php | 16 +++- tests/Unit/Api/RoleTest.php | 19 +++-- tests/Unit/Api/TimeEntryActivityTest.php | 16 +++- tests/Unit/Api/TimeEntryTest.php | 24 ++++-- tests/Unit/Api/TrackerTest.php | 16 +++- tests/Unit/Api/UserTest.php | 43 +++++++--- tests/Unit/Api/VersionTest.php | 32 ++++++-- tests/Unit/Api/WikiTest.php | 8 +- 18 files changed, 420 insertions(+), 133 deletions(-) diff --git a/src/Redmine/Api/AbstractApi.php b/src/Redmine/Api/AbstractApi.php index 2b4c0252..8cc89d3c 100644 --- a/src/Redmine/Api/AbstractApi.php +++ b/src/Redmine/Api/AbstractApi.php @@ -5,6 +5,7 @@ use JsonException; use Redmine\Api; use Redmine\Client\Client; +use Redmine\Exception\ClientException; use SimpleXMLElement; /** @@ -173,7 +174,13 @@ protected function retrieveAll($endpoint, array $params = []) { @trigger_error('The '.__METHOD__.' method is deprecated, use `retrieveData()` instead.', E_USER_DEPRECATED); - return $this->retrieveData(strval($endpoint), $params); + $data = $this->retrieveData(strval($endpoint), $params); + + if (! array_key_exists('response', $data)) { + return $data; + } + + return ('' === $data['response']) ? false : $data['response']; } /** @@ -181,23 +188,27 @@ protected function retrieveAll($endpoint, array $params = []) * total number of elements is greater than 100). * * @param string $endpoint API end point - * @param array $params optional parameters to be passed to the api (offset, limit, ...) + * @param array $params optional query parameters to be passed to the api (offset, limit, ...) * * @return array elements found */ - protected function retrieveData(string $endpoint, array $params = [])/*: array*/ // TODO: check if return type could be array + protected function retrieveData(string $endpoint, array $params = []): array { if (empty($params)) { - return $this->get($endpoint); + $this->client->requestGet($endpoint); + + return $this->getLastResponseAsArray(); } - $defaults = [ - 'limit' => 25, - 'offset' => 0, - ]; - $params = $this->sanitizeParams($defaults, $params); + $params = $this->sanitizeParams( + [ + 'limit' => 25, + 'offset' => 0, + ], + $params + ); - $data = []; + $returnData = []; $limit = $params['limit']; $offset = $params['offset']; @@ -213,10 +224,18 @@ protected function retrieveData(string $endpoint, array $params = [])/*: array*/ $params['limit'] = $_limit; $params['offset'] = $offset; - $newDataSet = (array) $this->get($endpoint.'?'.preg_replace('/%5B[0-9]+%5D/simU', '%5B%5D', http_build_query($params))); - $data = array_merge_recursive($data, $newDataSet); + $queryString = http_build_query($params); + // replace every encoded array (`foo[0]=`, `foo[1]=`, `foo[2]=`, etc => `foo[]=`) + $queryString = preg_replace('/%5B[0-9]+%5D/simU', '%5B%5D', $queryString); + + $this->client->requestGet($endpoint.'?'.$queryString); + + $newDataSet = $this->getLastResponseAsArray(); + + $returnData = array_merge_recursive($returnData, $newDataSet); $offset += $_limit; + if (empty($newDataSet) || !isset($newDataSet['limit']) || ( isset($newDataSet['offset']) && isset($newDataSet['total_count']) && @@ -227,7 +246,7 @@ protected function retrieveData(string $endpoint, array $params = [])/*: array*/ } } - return $data; + return $returnData; } /** @@ -274,4 +293,59 @@ protected function attachCustomFieldXML(SimpleXMLElement $xml, array $fields) return $xml; } + + private function getLastResponseAsArray(): array + { + $body = $this->client->getLastResponseBody(); + + // if body is empty + if ($body === '') { + return [ + 'response' => '', + ]; + } + + $contentType = $this->client->getLastResponseContentType(); + + // parse XML + if (0 === strpos($contentType, 'application/xml')) { + $returnData = new SimpleXMLElement($body); + + try { + $returnData = json_decode( + json_encode($returnData, \JSON_THROW_ON_ERROR), + true, + 512, + \JSON_THROW_ON_ERROR + ); + } catch (JsonException $e) { + throw new ClientException( + 'Error decoding body as JSON: ' . $e->getMessage(), + $e->getCode(), + $e + ); + } + } else if (0 === strpos($contentType, 'application/json')) { + try { + $returnData = json_decode( + $body, + true, + 512, + \JSON_THROW_ON_ERROR + ); + } catch (JsonException $e) { + throw new ClientException( + 'Error decoding body as JSON: ' . $e->getMessage(), + $e->getCode(), + $e + ); + } + } else { + $returnData = [ + 'response' => $body, + ]; + } + + return $returnData; + } } diff --git a/tests/Unit/Api/GroupTest.php b/tests/Unit/Api/GroupTest.php index dd3e2e6b..980ce148 100644 --- a/tests/Unit/Api/GroupTest.php +++ b/tests/Unit/Api/GroupTest.php @@ -24,7 +24,8 @@ class GroupTest extends TestCase public function testAllReturnsClientGetResponse() { // Test values - $response = 'API Response'; + $response = '["API Response"]'; + $expectedReturn = ['API Response']; // Create the used mock objects $client = $this->createMock(Client::class); @@ -37,12 +38,15 @@ public function testAllReturnsClientGetResponse() $client->expects($this->exactly(1)) ->method('getLastResponseBody') ->willReturn($response); + $client->expects($this->exactly(1)) + ->method('getLastResponseContentType') + ->willReturn('application/json'); // Create the object under test $api = new Group($client); // Perform the tests - $this->assertSame($response, $api->all()); + $this->assertSame($expectedReturn, $api->all()); } /** @@ -55,7 +59,8 @@ public function testAllReturnsClientGetResponseWithParameters() { // Test values $parameters = ['not-used']; - $response = 'API Response'; + $response = '["API Response"]'; + $expectedReturn = ['API Response']; // Create the used mock objects $client = $this->createMock(Client::class); @@ -71,12 +76,15 @@ public function testAllReturnsClientGetResponseWithParameters() $client->expects($this->exactly(1)) ->method('getLastResponseBody') ->willReturn($response); + $client->expects($this->exactly(1)) + ->method('getLastResponseContentType') + ->willReturn('application/json'); // Create the object under test $api = new Group($client); // Perform the tests - $this->assertSame([$response], $api->all($parameters)); + $this->assertSame($expectedReturn, $api->all($parameters)); } /** @@ -202,7 +210,8 @@ public function testListingCallsGetEveryTimeWithForceUpdate() public function testShowReturnsClientGetResponse() { // Test values - $response = 'API Response'; + $response = '["API Response"]'; + $expectedReturn = ['API Response']; // Create the used mock objects $client = $this->createMock(Client::class); @@ -213,12 +222,15 @@ public function testShowReturnsClientGetResponse() $client->expects($this->exactly(1)) ->method('getLastResponseBody') ->willReturn($response); + $client->expects($this->exactly(1)) + ->method('getLastResponseContentType') + ->willReturn('application/json'); // Create the object under test $api = new Group($client); // Perform the tests - $this->assertSame($response, $api->show(5)); + $this->assertSame($expectedReturn, $api->show(5)); } /** @@ -231,8 +243,9 @@ public function testShowReturnsClientGetResponse() public function testShowCallsGetUrlWithParameters() { // Test values - $response = 'API Response'; $allParameters = ['not-used']; + $response = '["API Response"]'; + $expectedReturn = ['API Response']; // Create the used mock objects $client = $this->createMock(Client::class); @@ -248,12 +261,15 @@ public function testShowCallsGetUrlWithParameters() $client->expects($this->exactly(1)) ->method('getLastResponseBody') ->willReturn($response); + $client->expects($this->exactly(1)) + ->method('getLastResponseContentType') + ->willReturn('application/json'); // Create the object under test $api = new Group($client); // Perform the tests - $this->assertSame($response, $api->show(5, $allParameters)); + $this->assertSame($expectedReturn, $api->show(5, $allParameters)); } /** @@ -266,7 +282,8 @@ public function testShowCallsGetUrlWithParameters() public function testRemoveCallsDelete() { // Test values - $response = 'API Response'; + $response = '["API Response"]'; + $expectedReturn = $response; // Create the used mock objects $client = $this->createMock(Client::class); @@ -285,12 +302,15 @@ public function testRemoveCallsDelete() $client->expects($this->exactly(1)) ->method('getLastResponseBody') ->willReturn($response); + $client->expects($this->exactly(0)) + ->method('getLastResponseContentType') + ->willReturn('application/json'); // Create the object under test $api = new Group($client); // Perform the tests - $this->assertSame($response, $api->remove(5)); + $this->assertSame($expectedReturn, $api->remove(5)); } /** diff --git a/tests/Unit/Api/IssueCategoryTest.php b/tests/Unit/Api/IssueCategoryTest.php index c7b6ff8c..d71de11d 100644 --- a/tests/Unit/Api/IssueCategoryTest.php +++ b/tests/Unit/Api/IssueCategoryTest.php @@ -57,7 +57,8 @@ public function testAllReturnsClientGetResponseWithParametersAndProject() // Test values $projectId = 5; $parameters = ['not-used']; - $response = 'API Response'; + $response = '["API Response"]'; + $expectedReturn = ['API Response']; // Create the used mock objects $client = $this->createMock(Client::class); @@ -73,12 +74,15 @@ public function testAllReturnsClientGetResponseWithParametersAndProject() $client->expects($this->exactly(1)) ->method('getLastResponseBody') ->willReturn($response); + $client->expects($this->exactly(1)) + ->method('getLastResponseContentType') + ->willReturn('application/json'); // Create the object under test $api = new IssueCategory($client); // Perform the tests - $this->assertSame([$response], $api->all($projectId, $parameters)); + $this->assertSame($expectedReturn, $api->all($projectId, $parameters)); } /** diff --git a/tests/Unit/Api/IssuePriorityTest.php b/tests/Unit/Api/IssuePriorityTest.php index e970b131..6f947a77 100644 --- a/tests/Unit/Api/IssuePriorityTest.php +++ b/tests/Unit/Api/IssuePriorityTest.php @@ -22,7 +22,8 @@ class IssuePriorityTest extends TestCase public function testAllReturnsClientGetResponse() { // Test values - $response = 'API Response'; + $response = '["API Response"]'; + $expectedReturn = ['API Response']; // Create the used mock objects $client = $this->createMock(Client::class); @@ -35,12 +36,15 @@ public function testAllReturnsClientGetResponse() $client->expects($this->exactly(1)) ->method('getLastResponseBody') ->willReturn($response); + $client->expects($this->exactly(1)) + ->method('getLastResponseContentType') + ->willReturn('application/json'); // Create the object under test $api = new IssuePriority($client); // Perform the tests - $this->assertSame($response, $api->all()); + $this->assertSame($expectedReturn, $api->all()); } /** @@ -53,7 +57,8 @@ public function testAllReturnsClientGetResponseWithParameters() { // Test values $allParameters = ['not-used']; - $response = 'API Response'; + $response = '["API Response"]'; + $expectedReturn = ['API Response']; // Create the used mock objects $client = $this->createMock(Client::class); @@ -66,11 +71,14 @@ public function testAllReturnsClientGetResponseWithParameters() $client->expects($this->exactly(1)) ->method('getLastResponseBody') ->willReturn($response); + $client->expects($this->exactly(1)) + ->method('getLastResponseContentType') + ->willReturn('application/json'); // Create the object under test $api = new IssuePriority($client); // Perform the tests - $this->assertSame([$response], $api->all($allParameters)); + $this->assertSame($expectedReturn, $api->all($allParameters)); } } diff --git a/tests/Unit/Api/IssueRelationTest.php b/tests/Unit/Api/IssueRelationTest.php index f08475da..a79e3816 100644 --- a/tests/Unit/Api/IssueRelationTest.php +++ b/tests/Unit/Api/IssueRelationTest.php @@ -22,7 +22,8 @@ class IssueRelationTest extends TestCase public function testAllReturnsClientGetResponseWithProject() { // Test values - $response = 'API Response'; + $response = '["API Response"]'; + $expectedReturn = ['API Response']; // Create the used mock objects $client = $this->createMock(Client::class); @@ -35,12 +36,15 @@ public function testAllReturnsClientGetResponseWithProject() $client->expects($this->exactly(1)) ->method('getLastResponseBody') ->willReturn($response); + $client->expects($this->exactly(1)) + ->method('getLastResponseContentType') + ->willReturn('application/json'); // Create the object under test $api = new IssueRelation($client); // Perform the tests - $this->assertSame($response, $api->all(5)); + $this->assertSame($expectedReturn, $api->all(5)); } /** @@ -53,7 +57,8 @@ public function testAllReturnsClientGetResponseWithParametersAndProject() { // Test values $parameters = ['not-used']; - $response = 'API Response'; + $response = '["API Response"]'; + $expectedReturn = ['API Response']; // Create the used mock objects $client = $this->createMock(Client::class); @@ -69,12 +74,15 @@ public function testAllReturnsClientGetResponseWithParametersAndProject() $client->expects($this->exactly(1)) ->method('getLastResponseBody') ->willReturn($response); + $client->expects($this->exactly(1)) + ->method('getLastResponseContentType') + ->willReturn('application/json'); // Create the object under test $api = new IssueRelation($client); // Perform the tests - $this->assertSame([$response], $api->all(5, $parameters)); + $this->assertSame($expectedReturn, $api->all(5, $parameters)); } /** @@ -121,6 +129,9 @@ public function testShowReturnsClientGetResponse() */ public function testShowReturnsArrayIfNull() { + $response = ''; + $expectedReturn = []; + // Create the used mock objects $client = $this->createMock(Client::class); $client->expects($this->once()) @@ -129,13 +140,16 @@ public function testShowReturnsArrayIfNull() ->willReturn(false); $client->expects($this->exactly(1)) ->method('getLastResponseBody') - ->willReturn(''); + ->willReturn($response); + $client->expects($this->exactly(1)) + ->method('getLastResponseContentType') + ->willReturn('application/json'); // Create the object under test $api = new IssueRelation($client); // Perform the tests - $this->assertSame([], $api->show(5)); + $this->assertSame($expectedReturn, $api->show(5)); } /** @@ -148,7 +162,8 @@ public function testShowReturnsArrayIfNull() public function testRemoveCallsDelete() { // Test values - $response = 'API Response'; + $response = '["API Response"]'; + $expectedReturn = '["API Response"]'; // Create the used mock objects $client = $this->createMock(Client::class); @@ -167,12 +182,15 @@ public function testRemoveCallsDelete() $client->expects($this->exactly(1)) ->method('getLastResponseBody') ->willReturn($response); + $client->expects($this->exactly(0)) + ->method('getLastResponseContentType') + ->willReturn('application/json'); // Create the object under test $api = new IssueRelation($client); // Perform the tests - $this->assertSame($response, $api->remove(5)); + $this->assertSame($expectedReturn, $api->remove(5)); } /** @@ -185,31 +203,34 @@ public function testRemoveCallsDelete() public function testCreateCallsPost() { // Test values - $response = '{"test":"response"}'; - $responseArray = ['test' => 'response']; $parameters = []; + $response = '{"test":"response"}'; + $expectedReturn = ['test' => 'response']; // Create the used mock objects $client = $this->createMock(Client::class); $client->expects($this->once()) - ->method('requestPost') - ->with( - '/issues/1/relations.json', - json_encode([ - 'relation' => [ - 'relation_type' => 'relates', - ], - ]) - ) - ->willReturn(true); + ->method('requestPost') + ->with( + '/issues/1/relations.json', + json_encode([ + 'relation' => [ + 'relation_type' => 'relates', + ], + ]) + ) + ->willReturn(true); + $client->expects($this->exactly(1)) + ->method('getLastResponseBody') + ->willReturn($response); $client->expects($this->exactly(1)) - ->method('getLastResponseBody') - ->willReturn($response); + ->method('getLastResponseContentType') + ->willReturn('application/json'); // Create the object under test $api = new IssueRelation($client); // Perform the tests - $this->assertSame($responseArray, $api->create(1, $parameters)); + $this->assertSame($expectedReturn, $api->create(1, $parameters)); } } diff --git a/tests/Unit/Api/IssueStatusTest.php b/tests/Unit/Api/IssueStatusTest.php index 0686b00e..ac812ac0 100644 --- a/tests/Unit/Api/IssueStatusTest.php +++ b/tests/Unit/Api/IssueStatusTest.php @@ -22,7 +22,8 @@ class IssueStatusTest extends TestCase public function testAllReturnsClientGetResponse() { // Test values - $response = 'API Response'; + $response = '["API Response"]'; + $expectedReturn = ['API Response']; // Create the used mock objects $client = $this->createMock(Client::class); @@ -35,12 +36,15 @@ public function testAllReturnsClientGetResponse() $client->expects($this->exactly(1)) ->method('getLastResponseBody') ->willReturn($response); + $client->expects($this->exactly(1)) + ->method('getLastResponseContentType') + ->willReturn('application/json'); // Create the object under test $api = new IssueStatus($client); // Perform the tests - $this->assertSame($response, $api->all()); + $this->assertSame($expectedReturn, $api->all()); } /** @@ -53,7 +57,8 @@ public function testAllReturnsClientGetResponseWithParametersAndProject() { // Test values $parameters = ['not-used']; - $response = 'API Response'; + $response = '["API Response"]'; + $expectedReturn = ['API Response']; // Create the used mock objects $client = $this->createMock(Client::class); @@ -69,12 +74,15 @@ public function testAllReturnsClientGetResponseWithParametersAndProject() $client->expects($this->exactly(1)) ->method('getLastResponseBody') ->willReturn($response); + $client->expects($this->exactly(1)) + ->method('getLastResponseContentType') + ->willReturn('application/json'); // Create the object under test $api = new IssueStatus($client); // Perform the tests - $this->assertSame([$response], $api->all($parameters)); + $this->assertSame($expectedReturn, $api->all($parameters)); } /** diff --git a/tests/Unit/Api/IssueTest.php b/tests/Unit/Api/IssueTest.php index 2c6a89de..6bce877b 100644 --- a/tests/Unit/Api/IssueTest.php +++ b/tests/Unit/Api/IssueTest.php @@ -13,18 +13,26 @@ */ class IssueTest extends TestCase { + public function getPriorityConstantsData() + { + return [ + [1, Issue::PRIO_LOW], + [2, Issue::PRIO_NORMAL], + [3, Issue::PRIO_HIGH], + [4, Issue::PRIO_URGENT], + [5, Issue::PRIO_IMMEDIATE], + ]; + } /** * Test the constants. * + * @dataProvider getPriorityConstantsData + * * @test */ - public function testPriorityConstants() + public function testPriorityConstants($expected, $value) { - $this->assertSame(1, Issue::PRIO_LOW); - $this->assertSame(2, Issue::PRIO_NORMAL); - $this->assertSame(3, Issue::PRIO_HIGH); - $this->assertSame(4, Issue::PRIO_URGENT); - $this->assertSame(5, Issue::PRIO_IMMEDIATE); + $this->assertSame($expected, $value); } /** @@ -36,7 +44,8 @@ public function testPriorityConstants() public function testAllReturnsClientGetResponse() { // Test values - $response = 'API Response'; + $response = '["API Response"]'; + $expectedReturn = ['API Response']; // Create the used mock objects $client = $this->createMock(Client::class); @@ -49,12 +58,15 @@ public function testAllReturnsClientGetResponse() $client->expects($this->exactly(1)) ->method('getLastResponseBody') ->willReturn($response); + $client->expects($this->exactly(1)) + ->method('getLastResponseContentType') + ->willReturn('application/json'); // Create the object under test $api = new Issue($client); // Perform the tests - $this->assertSame($response, $api->all()); + $this->assertSame($expectedReturn, $api->all()); } /** @@ -67,7 +79,8 @@ public function testAllReturnsClientGetResponseWithParameters() { // Test values $parameters = ['not-used']; - $response = 'API Response'; + $response = '["API Response"]'; + $expectedReturn = ['API Response']; // Create the used mock objects $client = $this->createMock(Client::class); @@ -83,12 +96,15 @@ public function testAllReturnsClientGetResponseWithParameters() $client->expects($this->exactly(1)) ->method('getLastResponseBody') ->willReturn($response); + $client->expects($this->exactly(1)) + ->method('getLastResponseContentType') + ->willReturn('application/json'); // Create the object under test $api = new Issue($client); // Perform the tests - $this->assertSame([$response], $api->all($parameters)); + $this->assertSame($expectedReturn, $api->all($parameters)); } /** @@ -101,7 +117,8 @@ public function testAllReturnsClientGetResponseWithParameters() public function testShowReturnsClientGetResponse() { // Test values - $response = 'API Response'; + $response = '["API Response"]'; + $expectedReturn = ['API Response']; // Create the used mock objects $client = $this->createMock(Client::class); @@ -112,12 +129,15 @@ public function testShowReturnsClientGetResponse() $client->expects($this->exactly(1)) ->method('getLastResponseBody') ->willReturn($response); + $client->expects($this->exactly(1)) + ->method('getLastResponseContentType') + ->willReturn('application/json'); // Create the object under test $api = new Issue($client); // Perform the tests - $this->assertSame($response, $api->show(5)); + $this->assertSame($expectedReturn, $api->show(5)); } /** @@ -130,8 +150,9 @@ public function testShowReturnsClientGetResponse() public function testShowCallsGetUrlWithParameters() { // Test values - $response = 'API Response'; $allParameters = ['not-used']; + $response = '["API Response"]'; + $expectedReturn = ['API Response']; // Create the used mock objects $client = $this->createMock(Client::class); @@ -147,12 +168,15 @@ public function testShowCallsGetUrlWithParameters() $client->expects($this->exactly(1)) ->method('getLastResponseBody') ->willReturn($response); + $client->expects($this->exactly(1)) + ->method('getLastResponseContentType') + ->willReturn('application/json'); // Create the object under test $api = new Issue($client); // Perform the tests - $this->assertSame($response, $api->show(5, $allParameters)); + $this->assertSame($expectedReturn, $api->show(5, $allParameters)); } /** @@ -165,7 +189,8 @@ public function testShowImplodesIncludeParametersCorrectly() { // Test values $parameters = ['include' => ['parameter1', 'parameter2']]; - $response = 'API Response'; + $response = '["API Response"]'; + $expectedReturn = ['API Response']; // Create the used mock objects $client = $this->createMock(Client::class); @@ -181,12 +206,15 @@ public function testShowImplodesIncludeParametersCorrectly() $client->expects($this->exactly(1)) ->method('getLastResponseBody') ->willReturn($response); + $client->expects($this->exactly(1)) + ->method('getLastResponseContentType') + ->willReturn('application/json'); // Create the object under test $api = new Issue($client); // Perform the tests - $this->assertSame($response, $api->show(5, $parameters)); + $this->assertSame($expectedReturn, $api->show(5, $parameters)); } /** diff --git a/tests/Unit/Api/MembershipTest.php b/tests/Unit/Api/MembershipTest.php index 9fb952c1..40c46478 100644 --- a/tests/Unit/Api/MembershipTest.php +++ b/tests/Unit/Api/MembershipTest.php @@ -24,7 +24,8 @@ class MembershipTest extends TestCase public function testAllReturnsClientGetResponseWithProject() { // Test values - $response = 'API Response'; + $response = '["API Response"]'; + $expectedReturn = ['API Response']; // Create the used mock objects $client = $this->createMock(Client::class); @@ -37,12 +38,15 @@ public function testAllReturnsClientGetResponseWithProject() $client->expects($this->exactly(1)) ->method('getLastResponseBody') ->willReturn($response); + $client->expects($this->exactly(1)) + ->method('getLastResponseContentType') + ->willReturn('application/json'); // Create the object under test $api = new Membership($client); // Perform the tests - $this->assertSame($response, $api->all(5)); + $this->assertSame($expectedReturn, $api->all(5)); } /** @@ -55,7 +59,8 @@ public function testAllReturnsClientGetResponseWithParametersAndProject() { // Test values $parameters = ['not-used']; - $response = 'API Response'; + $response = '["API Response"]'; + $expectedReturn = ['API Response']; // Create the used mock objects $client = $this->createMock(Client::class); @@ -71,12 +76,15 @@ public function testAllReturnsClientGetResponseWithParametersAndProject() $client->expects($this->exactly(1)) ->method('getLastResponseBody') ->willReturn($response); + $client->expects($this->exactly(1)) + ->method('getLastResponseContentType') + ->willReturn('application/json'); // Create the object under test $api = new Membership($client); // Perform the tests - $this->assertSame([$response], $api->all(5, $parameters)); + $this->assertSame($expectedReturn, $api->all(5, $parameters)); } /** diff --git a/tests/Unit/Api/NewsTest.php b/tests/Unit/Api/NewsTest.php index 9291bb54..b60cdcef 100644 --- a/tests/Unit/Api/NewsTest.php +++ b/tests/Unit/Api/NewsTest.php @@ -22,7 +22,8 @@ class NewsTest extends TestCase public function testAllReturnsClientGetResponse() { // Test values - $response = 'API Response'; + $response = '["API Response"]'; + $expectedReturn = ['API Response']; // Create the used mock objects $client = $this->createMock(Client::class); @@ -35,12 +36,15 @@ public function testAllReturnsClientGetResponse() $client->expects($this->exactly(1)) ->method('getLastResponseBody') ->willReturn($response); + $client->expects($this->exactly(1)) + ->method('getLastResponseContentType') + ->willReturn('application/json'); // Create the object under test $api = new News($client); // Perform the tests - $this->assertSame($response, $api->all()); + $this->assertSame($expectedReturn, $api->all()); } /** @@ -53,7 +57,8 @@ public function testAllReturnsClientGetResponseWithProject() { // Test values $projectId = 5; - $response = 'API Response'; + $response = '["API Response"]'; + $expectedReturn = ['API Response']; // Create the used mock objects $client = $this->createMock(Client::class); @@ -66,12 +71,15 @@ public function testAllReturnsClientGetResponseWithProject() $client->expects($this->exactly(1)) ->method('getLastResponseBody') ->willReturn($response); + $client->expects($this->exactly(1)) + ->method('getLastResponseContentType') + ->willReturn('application/json'); // Create the object under test $api = new News($client); // Perform the tests - $this->assertSame($response, $api->all($projectId)); + $this->assertSame($expectedReturn, $api->all($projectId)); } /** @@ -85,7 +93,8 @@ public function testAllReturnsClientGetResponseWithParametersAndProject() // Test values $projectId = 5; $parameters = ['not-used']; - $response = 'API Response'; + $response = '["API Response"]'; + $expectedReturn = ['API Response']; // Create the used mock objects $client = $this->createMock(Client::class); @@ -98,11 +107,14 @@ public function testAllReturnsClientGetResponseWithParametersAndProject() $client->expects($this->exactly(1)) ->method('getLastResponseBody') ->willReturn($response); + $client->expects($this->exactly(1)) + ->method('getLastResponseContentType') + ->willReturn('application/json'); // Create the object under test $api = new News($client); // Perform the tests - $this->assertSame([$response], $api->all($projectId, $parameters)); + $this->assertSame($expectedReturn, $api->all($projectId, $parameters)); } } diff --git a/tests/Unit/Api/ProjectTest.php b/tests/Unit/Api/ProjectTest.php index 27f757da..84d86c7d 100644 --- a/tests/Unit/Api/ProjectTest.php +++ b/tests/Unit/Api/ProjectTest.php @@ -24,7 +24,8 @@ class ProjectTest extends TestCase public function testAllReturnsClientGetResponse() { // Test values - $response = 'API Response'; + $response = '["API Response"]'; + $expectedReturn = ['API Response']; // Create the used mock objects $client = $this->createMock(Client::class); @@ -35,12 +36,15 @@ public function testAllReturnsClientGetResponse() $client->expects($this->exactly(1)) ->method('getLastResponseBody') ->willReturn($response); + $client->expects($this->exactly(1)) + ->method('getLastResponseContentType') + ->willReturn('application/json'); // Create the object under test $api = new Project($client); // Perform the tests - $this->assertSame($response, $api->all()); + $this->assertSame($expectedReturn, $api->all()); } /** @@ -53,7 +57,8 @@ public function testAllReturnsClientGetResponseWithParameters() { // Test values $parameters = ['not-used']; - $response = 'API Response'; + $response = '["API Response"]'; + $expectedReturn = ['API Response']; // Create the used mock objects $client = $this->createMock(Client::class); @@ -69,12 +74,15 @@ public function testAllReturnsClientGetResponseWithParameters() $client->expects($this->exactly(1)) ->method('getLastResponseBody') ->willReturn($response); + $client->expects($this->exactly(1)) + ->method('getLastResponseContentType') + ->willReturn('application/json'); // Create the object under test $api = new Project($client); // Perform the tests - $this->assertSame([$response], $api->all($parameters)); + $this->assertSame($expectedReturn, $api->all($parameters)); } /** @@ -87,7 +95,8 @@ public function testAllReturnsClientGetResponseWithParameters() public function testShowReturnsClientGetResponse() { // Test values - $response = 'API Response'; + $response = '["API Response"]'; + $expectedReturn = ['API Response']; // Create the used mock objects $client = $this->createMock(Client::class); @@ -101,12 +110,15 @@ public function testShowReturnsClientGetResponse() $client->expects($this->exactly(1)) ->method('getLastResponseBody') ->willReturn($response); + $client->expects($this->exactly(1)) + ->method('getLastResponseContentType') + ->willReturn('application/json'); // Create the object under test $api = new Project($client); // Perform the tests - $this->assertSame($response, $api->show(5)); + $this->assertSame($expectedReturn, $api->show(5)); } /** @@ -120,7 +132,8 @@ public function testShowReturnsClientGetResponseWithUniqueParameters() { // Test values $parameters = ['include' => ['parameter1', 'parameter2', 'enabled_modules']]; - $response = 'API Response'; + $response = '["API Response"]'; + $expectedReturn = ['API Response']; // Create the used mock objects $client = $this->createMock(Client::class); @@ -136,12 +149,15 @@ public function testShowReturnsClientGetResponseWithUniqueParameters() $client->expects($this->exactly(1)) ->method('getLastResponseBody') ->willReturn($response); + $client->expects($this->exactly(1)) + ->method('getLastResponseContentType') + ->willReturn('application/json'); // Create the object under test $api = new Project($client); // Perform the tests - $this->assertSame($response, $api->show(5, $parameters)); + $this->assertSame($expectedReturn, $api->show(5, $parameters)); } /** diff --git a/tests/Unit/Api/QueryTest.php b/tests/Unit/Api/QueryTest.php index b8299fba..d11c2ae7 100644 --- a/tests/Unit/Api/QueryTest.php +++ b/tests/Unit/Api/QueryTest.php @@ -22,7 +22,8 @@ class QueryTest extends TestCase public function testAllReturnsClientGetResponse() { // Test values - $response = 'API Response'; + $response = '["API Response"]'; + $expectedReturn = ['API Response']; // Create the used mock objects $client = $this->createMock(Client::class); @@ -35,12 +36,15 @@ public function testAllReturnsClientGetResponse() $client->expects($this->exactly(1)) ->method('getLastResponseBody') ->willReturn($response); + $client->expects($this->exactly(1)) + ->method('getLastResponseContentType') + ->willReturn('application/json'); // Create the object under test $api = new Query($client); // Perform the tests - $this->assertSame($response, $api->all()); + $this->assertSame($expectedReturn, $api->all()); } /** @@ -53,7 +57,8 @@ public function testAllReturnsClientGetResponseWithParameters() { // Test values $parameters = ['not-used']; - $response = 'API Response'; + $response = '["API Response"]'; + $expectedReturn = ['API Response']; // Create the used mock objects $client = $this->createMock(Client::class); @@ -69,11 +74,14 @@ public function testAllReturnsClientGetResponseWithParameters() $client->expects($this->exactly(1)) ->method('getLastResponseBody') ->willReturn($response); + $client->expects($this->exactly(1)) + ->method('getLastResponseContentType') + ->willReturn('application/json'); // Create the object under test $api = new Query($client); // Perform the tests - $this->assertSame([$response], $api->all($parameters)); + $this->assertSame($expectedReturn, $api->all($parameters)); } } diff --git a/tests/Unit/Api/RoleTest.php b/tests/Unit/Api/RoleTest.php index 99be59e1..9d4ed9c0 100644 --- a/tests/Unit/Api/RoleTest.php +++ b/tests/Unit/Api/RoleTest.php @@ -22,7 +22,8 @@ class RoleTest extends TestCase public function testAllReturnsClientGetResponse() { // Test values - $response = 'API Response'; + $response = '["API Response"]'; + $expectedReturn = ['API Response']; // Create the used mock objects $client = $this->createMock(Client::class); @@ -35,12 +36,15 @@ public function testAllReturnsClientGetResponse() $client->expects($this->exactly(1)) ->method('getLastResponseBody') ->willReturn($response); + $client->expects($this->exactly(1)) + ->method('getLastResponseContentType') + ->willReturn('application/json'); // Create the object under test $api = new Role($client); // Perform the tests - $this->assertSame($response, $api->all()); + $this->assertSame($expectedReturn, $api->all()); } /** @@ -53,7 +57,8 @@ public function testAllReturnsClientGetResponseWithParametersAndProject() { // Test values $parameters = ['not-used']; - $response = 'API Response'; + $response = '["API Response"]'; + $expectedReturn = ['API Response']; // Create the used mock objects $client = $this->createMock(Client::class); @@ -69,12 +74,15 @@ public function testAllReturnsClientGetResponseWithParametersAndProject() $client->expects($this->exactly(1)) ->method('getLastResponseBody') ->willReturn($response); + $client->expects($this->exactly(1)) + ->method('getLastResponseContentType') + ->willReturn('application/json'); // Create the object under test $api = new Role($client); // Perform the tests - $this->assertSame([$response], $api->all($parameters)); + $this->assertSame($expectedReturn, $api->all($parameters)); } /** @@ -200,7 +208,8 @@ public function testListingCallsGetEveryTimeWithForceUpdate() public function testShowReturnsClientGetResponse() { // Test values - $response = 'API Response'; + $response = '["API Response"]'; + $expectedReturn = ['API Response']; // Create the used mock objects $client = $this->createMock(Client::class); diff --git a/tests/Unit/Api/TimeEntryActivityTest.php b/tests/Unit/Api/TimeEntryActivityTest.php index df35e55b..50ec89eb 100644 --- a/tests/Unit/Api/TimeEntryActivityTest.php +++ b/tests/Unit/Api/TimeEntryActivityTest.php @@ -22,7 +22,8 @@ class TimeEntryActivityTest extends TestCase public function testAllReturnsClientGetResponse() { // Test values - $response = 'API Response'; + $response = '["API Response"]'; + $expectedReturn = ['API Response']; // Create the used mock objects $client = $this->createMock(Client::class); @@ -35,12 +36,15 @@ public function testAllReturnsClientGetResponse() $client->expects($this->exactly(1)) ->method('getLastResponseBody') ->willReturn($response); + $client->expects($this->exactly(1)) + ->method('getLastResponseContentType') + ->willReturn('application/json'); // Create the object under test $api = new TimeEntryActivity($client); // Perform the tests - $this->assertSame($response, $api->all()); + $this->assertSame($expectedReturn, $api->all()); } /** @@ -53,7 +57,8 @@ public function testAllReturnsClientGetResponseWithParameters() { // Test values $parameters = ['not-used']; - $response = 'API Response'; + $response = '["API Response"]'; + $expectedReturn = ['API Response']; // Create the used mock objects $client = $this->createMock(Client::class); @@ -69,12 +74,15 @@ public function testAllReturnsClientGetResponseWithParameters() $client->expects($this->exactly(1)) ->method('getLastResponseBody') ->willReturn($response); + $client->expects($this->exactly(1)) + ->method('getLastResponseContentType') + ->willReturn('application/json'); // Create the object under test $api = new TimeEntryActivity($client); // Perform the tests - $this->assertSame([$response], $api->all($parameters)); + $this->assertSame($expectedReturn, $api->all($parameters)); } public function testListingReturnsNameIdArray() diff --git a/tests/Unit/Api/TimeEntryTest.php b/tests/Unit/Api/TimeEntryTest.php index fd21f986..d1f3477a 100644 --- a/tests/Unit/Api/TimeEntryTest.php +++ b/tests/Unit/Api/TimeEntryTest.php @@ -24,7 +24,8 @@ class TimeEntryTest extends TestCase public function testAllReturnsClientGetResponse() { // Test values - $response = 'API Response'; + $response = '["API Response"]'; + $expectedReturn = ['API Response']; // Create the used mock objects $client = $this->createMock(Client::class); @@ -35,12 +36,15 @@ public function testAllReturnsClientGetResponse() $client->expects($this->exactly(1)) ->method('getLastResponseBody') ->willReturn($response); + $client->expects($this->exactly(1)) + ->method('getLastResponseContentType') + ->willReturn('application/json'); // Create the object under test $api = new TimeEntry($client); // Perform the tests - $this->assertSame($response, $api->all()); + $this->assertSame($expectedReturn, $api->all()); } /** @@ -57,7 +61,8 @@ public function testAllReturnsClientGetResponseWithParameters() 'user_id' => 10, 'limit' => 2, ]; - $response = 'API Response'; + $response = '["API Response"]'; + $expectedReturn = ['API Response']; // Create the used mock objects $client = $this->createMock(Client::class); @@ -75,12 +80,15 @@ public function testAllReturnsClientGetResponseWithParameters() $client->expects($this->exactly(1)) ->method('getLastResponseBody') ->willReturn($response); + $client->expects($this->exactly(1)) + ->method('getLastResponseContentType') + ->willReturn('application/json'); // Create the object under test $api = new TimeEntry($client); // Perform the tests - $this->assertSame([$response], $api->all($parameters)); + $this->assertSame($expectedReturn, $api->all($parameters)); } /** @@ -93,7 +101,8 @@ public function testAllReturnsClientGetResponseWithParameters() public function testShowReturnsClientGetResponse() { // Test values - $response = 'API Response'; + $response = '["API Response"]'; + $expectedReturn = ['API Response']; // Create the used mock objects $client = $this->createMock(Client::class); @@ -104,12 +113,15 @@ public function testShowReturnsClientGetResponse() $client->expects($this->exactly(1)) ->method('getLastResponseBody') ->willReturn($response); + $client->expects($this->exactly(1)) + ->method('getLastResponseContentType') + ->willReturn('application/json'); // Create the object under test $api = new TimeEntry($client); // Perform the tests - $this->assertSame($response, $api->show(5)); + $this->assertSame($expectedReturn, $api->show(5)); } /** diff --git a/tests/Unit/Api/TrackerTest.php b/tests/Unit/Api/TrackerTest.php index 9b2ccf30..7cb018f5 100644 --- a/tests/Unit/Api/TrackerTest.php +++ b/tests/Unit/Api/TrackerTest.php @@ -22,7 +22,8 @@ class TrackerTest extends TestCase public function testAllReturnsClientGetResponse() { // Test values - $response = 'API Response'; + $response = '["API Response"]'; + $expectedReturn = ['API Response']; // Create the used mock objects $client = $this->createMock(Client::class); @@ -35,12 +36,15 @@ public function testAllReturnsClientGetResponse() $client->expects($this->exactly(1)) ->method('getLastResponseBody') ->willReturn($response); + $client->expects($this->exactly(1)) + ->method('getLastResponseContentType') + ->willReturn('application/json'); // Create the object under test $api = new Tracker($client); // Perform the tests - $this->assertSame($response, $api->all()); + $this->assertSame($expectedReturn, $api->all()); } /** @@ -53,7 +57,8 @@ public function testAllReturnsClientGetResponseWithParametersAndProject() { // Test values $parameters = ['not-used']; - $response = 'API Response'; + $response = '["API Response"]'; + $expectedReturn = ['API Response']; // Create the used mock objects $client = $this->createMock(Client::class); @@ -69,12 +74,15 @@ public function testAllReturnsClientGetResponseWithParametersAndProject() $client->expects($this->exactly(1)) ->method('getLastResponseBody') ->willReturn($response); + $client->expects($this->exactly(1)) + ->method('getLastResponseContentType') + ->willReturn('application/json'); // Create the object under test $api = new Tracker($client); // Perform the tests - $this->assertSame([$response], $api->all($parameters)); + $this->assertSame($expectedReturn, $api->all($parameters)); } /** diff --git a/tests/Unit/Api/UserTest.php b/tests/Unit/Api/UserTest.php index b1b7c96a..c6a3a5d8 100644 --- a/tests/Unit/Api/UserTest.php +++ b/tests/Unit/Api/UserTest.php @@ -24,7 +24,8 @@ class UserTest extends TestCase public function testGetCurrentUserReturnsClientGetResponse() { // Test values - $response = 'API Response'; + $response = '["API Response"]'; + $expectedReturn = ['API Response']; // Create the used mock objects $client = $this->createMock(Client::class); @@ -40,12 +41,15 @@ public function testGetCurrentUserReturnsClientGetResponse() $client->expects($this->exactly(1)) ->method('getLastResponseBody') ->willReturn($response); + $client->expects($this->exactly(1)) + ->method('getLastResponseContentType') + ->willReturn('application/json'); // Create the object under test $api = new User($client); // Perform the tests - $this->assertSame($response, $api->getCurrentUser()); + $this->assertSame($expectedReturn, $api->getCurrentUser()); } /** @@ -73,6 +77,9 @@ public function testGetIdByUsernameMakesGetRequest() $client->expects($this->exactly(1)) ->method('getLastResponseContentType') ->willReturn('application/json'); + $client->expects($this->exactly(1)) + ->method('getLastResponseContentType') + ->willReturn('application/json'); // Create the object under test $api = new User($client); @@ -91,7 +98,8 @@ public function testGetIdByUsernameMakesGetRequest() public function testAllReturnsClientGetResponse() { // Test values - $response = 'API Response'; + $response = '["API Response"]'; + $expectedReturn = ['API Response']; // Create the used mock objects $client = $this->createMock(Client::class); @@ -102,12 +110,15 @@ public function testAllReturnsClientGetResponse() $client->expects($this->exactly(1)) ->method('getLastResponseBody') ->willReturn($response); + $client->expects($this->exactly(1)) + ->method('getLastResponseContentType') + ->willReturn('application/json'); // Create the object under test $api = new User($client); // Perform the tests - $this->assertSame($response, $api->all()); + $this->assertSame($expectedReturn, $api->all()); } /** @@ -123,7 +134,8 @@ public function testAllReturnsClientGetResponseWithParameters() 'offset' => 10, 'limit' => 2, ]; - $response = 'API Response'; + $response = '["API Response"]'; + $expectedReturn = ['API Response']; // Create the used mock objects $client = $this->createMock(Client::class); @@ -140,12 +152,15 @@ public function testAllReturnsClientGetResponseWithParameters() $client->expects($this->exactly(1)) ->method('getLastResponseBody') ->willReturn($response); + $client->expects($this->exactly(1)) + ->method('getLastResponseContentType') + ->willReturn('application/json'); // Create the object under test $api = new User($client); // Perform the tests - $this->assertSame([$response], $api->all($parameters)); + $this->assertSame($expectedReturn, $api->all($parameters)); } /** @@ -158,7 +173,8 @@ public function testAllReturnsClientGetResponseWithParameters() public function testShowReturnsClientGetResponse() { // Test values - $response = 'API Response'; + $response = '["API Response"]'; + $expectedReturn = ['API Response']; // Create the used mock objects $client = $this->createMock(Client::class); @@ -174,12 +190,15 @@ public function testShowReturnsClientGetResponse() $client->expects($this->exactly(1)) ->method('getLastResponseBody') ->willReturn($response); + $client->expects($this->exactly(1)) + ->method('getLastResponseContentType') + ->willReturn('application/json'); // Create the object under test $api = new User($client); // Perform the tests - $this->assertSame($response, $api->show(5)); + $this->assertSame($expectedReturn, $api->show(5)); } /** @@ -193,7 +212,8 @@ public function testShowReturnsClientGetResponseWithUniqueParameters() { // Test values $parameters = ['include' => ['parameter1', 'parameter2', 'memberships']]; - $response = 'API Response'; + $response = '["API Response"]'; + $expectedReturn = ['API Response']; // Create the used mock objects $client = $this->createMock(Client::class); @@ -209,12 +229,15 @@ public function testShowReturnsClientGetResponseWithUniqueParameters() $client->expects($this->exactly(1)) ->method('getLastResponseBody') ->willReturn($response); + $client->expects($this->exactly(1)) + ->method('getLastResponseContentType') + ->willReturn('application/json'); // Create the object under test $api = new User($client); // Perform the tests - $this->assertSame($response, $api->show(5, $parameters)); + $this->assertSame($expectedReturn, $api->show(5, $parameters)); } /** diff --git a/tests/Unit/Api/VersionTest.php b/tests/Unit/Api/VersionTest.php index dfe9c1ed..227bb5a2 100644 --- a/tests/Unit/Api/VersionTest.php +++ b/tests/Unit/Api/VersionTest.php @@ -25,7 +25,8 @@ class VersionTest extends TestCase public function testAllReturnsClientGetResponse() { // Test values - $response = 'API Response'; + $response = '["API Response"]'; + $expectedReturn = ['API Response']; // Create the used mock objects $client = $this->createMock(Client::class); @@ -36,12 +37,15 @@ public function testAllReturnsClientGetResponse() $client->expects($this->once()) ->method('getLastResponseBody') ->willReturn($response); + $client->expects($this->exactly(1)) + ->method('getLastResponseContentType') + ->willReturn('application/json'); // Create the object under test $api = new Version($client); // Perform the tests - $this->assertSame($response, $api->all(5)); + $this->assertSame($expectedReturn, $api->all(5)); } /** @@ -57,7 +61,8 @@ public function testAllReturnsClientGetResponseWithParameters() 'offset' => 10, 'limit' => 2, ]; - $response = 'API Response'; + $response = '["API Response"]'; + $expectedReturn = ['API Response']; // Create the used mock objects $client = $this->createMock(Client::class); @@ -74,12 +79,15 @@ public function testAllReturnsClientGetResponseWithParameters() $client->expects($this->once()) ->method('getLastResponseBody') ->willReturn($response); + $client->expects($this->exactly(1)) + ->method('getLastResponseContentType') + ->willReturn('application/json'); // Create the object under test $api = new Version($client); // Perform the tests - $this->assertSame([$response], $api->all(5, $parameters)); + $this->assertSame($expectedReturn, $api->all(5, $parameters)); } /** @@ -92,7 +100,8 @@ public function testAllReturnsClientGetResponseWithParameters() public function testShowWithNumericIdReturnsClientGetResponse() { // Test values - $response = 'API Response'; + $response = '["API Response"]'; + $expectedReturn = ['API Response']; // Create the used mock objects $client = $this->createMock(Client::class); @@ -103,12 +112,15 @@ public function testShowWithNumericIdReturnsClientGetResponse() $client->expects($this->once()) ->method('getLastResponseBody') ->willReturn($response); + $client->expects($this->exactly(1)) + ->method('getLastResponseContentType') + ->willReturn('application/json'); // Create the object under test $api = new Version($client); // Perform the tests - $this->assertSame($response, $api->show(5)); + $this->assertSame($expectedReturn, $api->show(5)); } /** @@ -121,7 +133,8 @@ public function testShowWithNumericIdReturnsClientGetResponse() public function testShowWithStringReturnsClientGetResponse() { // Test values - $response = 'API Response'; + $response = '["API Response"]'; + $expectedReturn = ['API Response']; // Create the used mock objects $client = $this->createMock(Client::class); @@ -132,12 +145,15 @@ public function testShowWithStringReturnsClientGetResponse() $client->expects($this->once()) ->method('getLastResponseBody') ->willReturn($response); + $client->expects($this->exactly(1)) + ->method('getLastResponseContentType') + ->willReturn('application/json'); // Create the object under test $api = new Version($client); // Perform the tests - $this->assertSame($response, $api->show('test')); + $this->assertSame($expectedReturn, $api->show('test')); } /** diff --git a/tests/Unit/Api/WikiTest.php b/tests/Unit/Api/WikiTest.php index 2ce87c6e..99060ef8 100644 --- a/tests/Unit/Api/WikiTest.php +++ b/tests/Unit/Api/WikiTest.php @@ -54,7 +54,8 @@ public function testAllReturnsClientGetResponseWithParameters() 'offset' => 10, 'limit' => 2, ]; - $response = 'API Response'; + $response = '["API Response"]'; + $expectedReturn = ['API Response']; // Create the used mock objects $client = $this->createMock(Client::class); @@ -71,12 +72,15 @@ public function testAllReturnsClientGetResponseWithParameters() $client->expects($this->once()) ->method('getLastResponseBody') ->willReturn($response); + $client->expects($this->exactly(1)) + ->method('getLastResponseContentType') + ->willReturn('application/json'); // Create the object under test $api = new Wiki($client); // Perform the tests - $this->assertSame([$response], $api->all(5, $parameters)); + $this->assertSame($expectedReturn, $api->all(5, $parameters)); } /** From b3cca74b956d1b4013f68323e27d063d95b774d2 Mon Sep 17 00:00:00 2001 From: Art4 Date: Thu, 24 Feb 2022 16:25:30 +0100 Subject: [PATCH 03/15] Throw new ConversionException if conversion went wrong --- src/Redmine/Api/AbstractApi.php | 50 +++++++++++++------ src/Redmine/Exception/ConversionException.php | 10 ++++ tests/Fixtures/MockClient.php | 2 +- tests/Integration/UrlTest.php | 33 ------------ tests/Unit/Api/IssueCategoryTest.php | 8 ++- tests/Unit/Api/WikiTest.php | 8 ++- 6 files changed, 57 insertions(+), 54 deletions(-) create mode 100644 src/Redmine/Exception/ConversionException.php diff --git a/src/Redmine/Api/AbstractApi.php b/src/Redmine/Api/AbstractApi.php index 8cc89d3c..2cac6365 100644 --- a/src/Redmine/Api/AbstractApi.php +++ b/src/Redmine/Api/AbstractApi.php @@ -5,7 +5,7 @@ use JsonException; use Redmine\Api; use Redmine\Client\Client; -use Redmine\Exception\ClientException; +use Redmine\Exception\ConversionException; use SimpleXMLElement; /** @@ -190,6 +190,8 @@ protected function retrieveAll($endpoint, array $params = []) * @param string $endpoint API end point * @param array $params optional query parameters to be passed to the api (offset, limit, ...) * + * @throws ConversionException if response body could not be converted into array + * * @return array elements found */ protected function retrieveData(string $endpoint, array $params = []): array @@ -197,7 +199,7 @@ protected function retrieveData(string $endpoint, array $params = []): array if (empty($params)) { $this->client->requestGet($endpoint); - return $this->getLastResponseAsArray(); + return $this->getLastResponseBodyAsArray(); } $params = $this->sanitizeParams( @@ -230,7 +232,7 @@ protected function retrieveData(string $endpoint, array $params = []): array $this->client->requestGet($endpoint.'?'.$queryString); - $newDataSet = $this->getLastResponseAsArray(); + $newDataSet = $this->getLastResponseBodyAsArray(); $returnData = array_merge_recursive($returnData, $newDataSet); @@ -294,22 +296,36 @@ protected function attachCustomFieldXML(SimpleXMLElement $xml, array $fields) return $xml; } - private function getLastResponseAsArray(): array + /** + * returns the last response body as array + * + * @throws ConversionException if response body could not be converted into array + */ + private function getLastResponseBodyAsArray(): array { $body = $this->client->getLastResponseBody(); + $returnData = null; // if body is empty if ($body === '') { - return [ - 'response' => '', - ]; + throw new ConversionException( + 'Could not convert empty response body into array' + ); } $contentType = $this->client->getLastResponseContentType(); // parse XML if (0 === strpos($contentType, 'application/xml')) { - $returnData = new SimpleXMLElement($body); + try { + $returnData = new SimpleXMLElement($body); + } catch (\Exception $e) { + throw new ConversionException( + 'Catched error "' . $e->getMessage() . '" while decoding body as XML: ' . $body, + $e->getCode(), + $e + ); + } try { $returnData = json_decode( @@ -319,8 +335,8 @@ private function getLastResponseAsArray(): array \JSON_THROW_ON_ERROR ); } catch (JsonException $e) { - throw new ClientException( - 'Error decoding body as JSON: ' . $e->getMessage(), + throw new ConversionException( + 'Catched error "' . $e->getMessage() . '" while en- and decoding body as JSON: ' . $body, $e->getCode(), $e ); @@ -334,16 +350,18 @@ private function getLastResponseAsArray(): array \JSON_THROW_ON_ERROR ); } catch (JsonException $e) { - throw new ClientException( - 'Error decoding body as JSON: ' . $e->getMessage(), + throw new ConversionException( + 'Catched error "' . $e->getMessage() . '" while decoding body as JSON: ' . $body, $e->getCode(), $e ); } - } else { - $returnData = [ - 'response' => $body, - ]; + } + + if (! is_array($returnData)) { + throw new ConversionException( + 'Could not convert response body into array: ' . $body + ); } return $returnData; diff --git a/src/Redmine/Exception/ConversionException.php b/src/Redmine/Exception/ConversionException.php new file mode 100644 index 00000000..5f83c3f3 --- /dev/null +++ b/src/Redmine/Exception/ConversionException.php @@ -0,0 +1,10 @@ +responseBodyMock = json_encode($return); $this->responseCodeMock = 200; - $this->responseContentType = 'application/json'; + $this->responseContentTypeMock = 'application/json'; return true; } diff --git a/tests/Integration/UrlTest.php b/tests/Integration/UrlTest.php index 1c862006..9552d9ad 100644 --- a/tests/Integration/UrlTest.php +++ b/tests/Integration/UrlTest.php @@ -21,7 +21,6 @@ public function testAttachment() { $api = $this->client->getApi('attachment'); $res = $api->show(1); - $res = json_decode($res, true); $this->assertEquals('/attachments/1.json', $res['path']); $this->assertEquals('GET', $res['method']); @@ -37,7 +36,6 @@ public function testCustomFields() { $api = $this->client->getApi('custom_fields'); $res = $api->all(); - $res = json_decode($res, true); $this->assertEquals('/custom_fields.json', $res['path']); $this->assertEquals('GET', $res['method']); @@ -55,13 +53,11 @@ public function testGroup() $this->assertEquals('POST', $res['method']); $res = $api->all(); - $res = json_decode($res, true); $this->assertEquals('/groups.json', $res['path']); $this->assertEquals('GET', $res['method']); $res = $api->show(1); - $res = json_decode($res, true); $this->assertEquals('/groups/1.json?', $res['path']); $this->assertEquals('GET', $res['method']); @@ -105,13 +101,11 @@ public function testIssue() $this->assertEquals('PUT', $res['method']); $res = $api->all(); - $res = json_decode($res, true); $this->assertEquals('/issues.json', $res['path']); $this->assertEquals('GET', $res['method']); $res = $api->show(1); - $res = json_decode($res, true); $this->assertEquals('/issues/1.json?', $res['path']); $this->assertEquals('GET', $res['method']); @@ -158,13 +152,11 @@ public function testIssueCategory() $this->assertEquals('PUT', $res['method']); $res = $api->all('testProject'); - $res = json_decode($res, true); $this->assertEquals('/projects/testProject/issue_categories.json', $res['path']); $this->assertEquals('GET', $res['method']); $res = $api->show(1); - $res = json_decode($res, true); $this->assertEquals('/issue_categories/1.json', $res['path']); $this->assertEquals('GET', $res['method']); @@ -180,7 +172,6 @@ public function testIssuePriority() { $api = $this->client->getApi('issue_priority'); $res = $api->all(); - $res = json_decode($res, true); $this->assertEquals('/enumerations/issue_priorities.json', $res['path']); $this->assertEquals('GET', $res['method']); @@ -190,7 +181,6 @@ public function testIssueRelation() { $api = $this->client->getApi('issue_relation'); $res = $api->all(1); - $res = json_decode($res, true); $this->assertEquals('/issues/1/relations.json', $res['path']); $this->assertEquals('GET', $res['method']); @@ -209,7 +199,6 @@ public function testIssueStatus() { $api = $this->client->getApi('issue_status'); $res = $api->all(); - $res = json_decode($res, true); $this->assertEquals('/issue_statuses.json', $res['path']); $this->assertEquals('GET', $res['method']); @@ -237,7 +226,6 @@ public function testMembership() $this->assertEquals('PUT', $res['method']); $res = $api->all('testProject'); - $res = json_decode($res, true); $this->assertEquals('/projects/testProject/memberships.json', $res['path']); $this->assertEquals('GET', $res['method']); @@ -253,13 +241,11 @@ public function testNews() { $api = $this->client->getApi('news'); $res = $api->all(); - $res = json_decode($res, true); $this->assertEquals('/news.json', $res['path']); $this->assertEquals('GET', $res['method']); $res = $api->all('testProject'); - $res = json_decode($res, true); $this->assertEquals('/projects/testProject/news.json', $res['path']); $this->assertEquals('GET', $res['method']); @@ -286,13 +272,11 @@ public function testProject() $this->assertEquals('PUT', $res['method']); $res = $api->all(); - $res = json_decode($res, true); $this->assertEquals('/projects.json', $res['path']); $this->assertEquals('GET', $res['method']); $res = $api->show(1); - $res = json_decode($res, true); $this->assertEquals($res['path'], '/projects/1.json?include='.urlencode('trackers,issue_categories,attachments,relations')); $this->assertEquals('GET', $res['method']); @@ -308,7 +292,6 @@ public function testQuery() { $api = $this->client->getApi('query'); $res = $api->all(); - $res = json_decode($res, true); $this->assertEquals('/queries.json', $res['path']); $this->assertEquals('GET', $res['method']); @@ -318,13 +301,11 @@ public function testRole() { $api = $this->client->getApi('role'); $res = $api->all(); - $res = json_decode($res, true); $this->assertEquals('/roles.json', $res['path']); $this->assertEquals('GET', $res['method']); $res = $api->show(1); - $res = json_decode($res, true); $this->assertEquals('/roles/1.json', $res['path']); $this->assertEquals('GET', $res['method']); @@ -349,13 +330,11 @@ public function testTimeEntry() $this->assertEquals('PUT', $res['method']); $res = $api->all(); - $res = json_decode($res, true); $this->assertEquals('/time_entries.json', $res['path']); $this->assertEquals('GET', $res['method']); $res = $api->show(1); - $res = json_decode($res, true); $this->assertEquals('/time_entries/1.json', $res['path']); $this->assertEquals('GET', $res['method']); @@ -371,7 +350,6 @@ public function testTimeEntryActivity() { $api = $this->client->getApi('time_entry_activity'); $res = $api->all(); - $res = json_decode($res, true); $this->assertEquals('/enumerations/time_entry_activities.json', $res['path']); $this->assertEquals('GET', $res['method']); @@ -381,7 +359,6 @@ public function testTracker() { $api = $this->client->getApi('tracker'); $res = $api->all(); - $res = json_decode($res, true); $this->assertEquals('/trackers.json', $res['path']); $this->assertEquals('GET', $res['method']); @@ -408,31 +385,26 @@ public function testUser() $this->assertEquals('PUT', $res['method']); $res = $api->all(); - $res = json_decode($res, true); $this->assertEquals('/users.json', $res['path']); $this->assertEquals('GET', $res['method']); $res = $api->show(1); - $res = json_decode($res, true); $this->assertEquals($res['path'], '/users/1.json?include='.urlencode('memberships,groups')); $this->assertEquals('GET', $res['method']); $res = $api->show(1, ['include' => ['memberships', 'groups']]); - $res = json_decode($res, true); $this->assertEquals($res['path'], '/users/1.json?include='.urlencode('memberships,groups')); $this->assertEquals('GET', $res['method']); $res = $api->show(1, ['include' => ['memberships', 'groups', 'parameter1']]); - $res = json_decode($res, true); $this->assertEquals($res['path'], '/users/1.json?include='.urlencode('memberships,groups,parameter1')); $this->assertEquals('GET', $res['method']); $res = $api->show(1, ['include' => ['parameter1', 'memberships', 'groups']]); - $res = json_decode($res, true); $this->assertEquals($res['path'], '/users/1.json?include='.urlencode('parameter1,memberships,groups')); $this->assertEquals('GET', $res['method']); @@ -462,13 +434,11 @@ public function testVersion() $this->assertEquals('PUT', $res['method']); $res = $api->all('testProject'); - $res = json_decode($res, true); $this->assertEquals('/projects/testProject/versions.json', $res['path']); $this->assertEquals('GET', $res['method']); $res = $api->show(1); - $res = json_decode($res, true); $this->assertEquals('/versions/1.json', $res['path']); $this->assertEquals('GET', $res['method']); @@ -504,19 +474,16 @@ public function testWiki() $this->assertEquals('PUT', $res['method']); $res = $api->all('testProject'); - $res = json_decode($res, true); $this->assertEquals('/projects/testProject/wiki/index.json', $res['path']); $this->assertEquals('GET', $res['method']); $res = $api->show('testProject', 'about'); - $res = json_decode($res, true); $this->assertEquals('/projects/testProject/wiki/about.json?include=attachments', $res['path']); $this->assertEquals('GET', $res['method']); $res = $api->show('testProject', 'about', 'v1'); - $res = json_decode($res, true); $this->assertEquals('/projects/testProject/wiki/about/v1.json?include=attachments', $res['path']); $this->assertEquals('GET', $res['method']); diff --git a/tests/Unit/Api/IssueCategoryTest.php b/tests/Unit/Api/IssueCategoryTest.php index d71de11d..5017025a 100644 --- a/tests/Unit/Api/IssueCategoryTest.php +++ b/tests/Unit/Api/IssueCategoryTest.php @@ -25,7 +25,8 @@ public function testAllReturnsClientGetResponseWithProject() { // Test values $projectId = 5; - $response = 'API Response'; + $response = '["API Response"]'; + $expectedReturn = ['API Response']; // Create the used mock objects $client = $this->createMock(Client::class); @@ -38,12 +39,15 @@ public function testAllReturnsClientGetResponseWithProject() $client->expects($this->exactly(1)) ->method('getLastResponseBody') ->willReturn($response); + $client->expects($this->exactly(1)) + ->method('getLastResponseContentType') + ->willReturn('application/json'); // Create the object under test $api = new IssueCategory($client); // Perform the tests - $this->assertSame($response, $api->all($projectId)); + $this->assertSame($expectedReturn, $api->all($projectId)); } /** diff --git a/tests/Unit/Api/WikiTest.php b/tests/Unit/Api/WikiTest.php index 99060ef8..a2fee2ed 100644 --- a/tests/Unit/Api/WikiTest.php +++ b/tests/Unit/Api/WikiTest.php @@ -22,7 +22,8 @@ class WikiTest extends TestCase public function testAllReturnsClientGetResponse() { // Test values - $response = 'API Response'; + $response = '["API Response"]'; + $expectedReturn = ['API Response']; // Create the used mock objects $client = $this->createMock(Client::class); @@ -33,12 +34,15 @@ public function testAllReturnsClientGetResponse() $client->expects($this->once()) ->method('getLastResponseBody') ->willReturn($response); + $client->expects($this->exactly(1)) + ->method('getLastResponseContentType') + ->willReturn('application/json'); // Create the object under test $api = new Wiki($client); // Perform the tests - $this->assertSame($response, $api->all(5)); + $this->assertSame($expectedReturn, $api->all(5)); } /** From e6884e59cac64f47ff812813d55d2bd69a52afe6 Mon Sep 17 00:00:00 2001 From: Art4 Date: Fri, 25 Feb 2022 09:00:29 +0100 Subject: [PATCH 04/15] Create PathSerializer --- src/Redmine/Serializer/PathSerializer.php | 38 ++++++++++++++++ tests/Unit/Serializer/PathSerializerTest.php | 46 ++++++++++++++++++++ 2 files changed, 84 insertions(+) create mode 100644 src/Redmine/Serializer/PathSerializer.php create mode 100644 tests/Unit/Serializer/PathSerializerTest.php diff --git a/src/Redmine/Serializer/PathSerializer.php b/src/Redmine/Serializer/PathSerializer.php new file mode 100644 index 00000000..fc945933 --- /dev/null +++ b/src/Redmine/Serializer/PathSerializer.php @@ -0,0 +1,38 @@ +path = $path; + $serializer->queryParams = $queryParams; + + return $serializer; + } + + private string $path; + + private array $queryParams; + + private function __construct() + { + // use factory method instead + } + + public function getPath(): string + { + $queryString = ''; + + if (! empty($this->queryParams)) { + $queryString = '?' . \http_build_query($this->queryParams); + } + + return $this->path . $queryString; + } +} diff --git a/tests/Unit/Serializer/PathSerializerTest.php b/tests/Unit/Serializer/PathSerializerTest.php new file mode 100644 index 00000000..aa50e31e --- /dev/null +++ b/tests/Unit/Serializer/PathSerializerTest.php @@ -0,0 +1,46 @@ + 'xml', + ], + 'foo.xml?format=xml' + ], + ]; + } + + /** + * @test + * + * @dataProvider getPathData + */ + public function getPathShouldReturn(string $path, array $params, string $expected) + { + $serializer = PathSerializer::create($path, $params); + + $this->assertSame($expected, $serializer->getPath()); + } +} From db1e5dafc829b24b56744663ca19d71e22251bb5 Mon Sep 17 00:00:00 2001 From: Art4 Date: Fri, 25 Feb 2022 09:21:26 +0100 Subject: [PATCH 05/15] Let PathSerializer handle values wit numeric key correctly Add more URL tests --- src/Redmine/Api/AbstractApi.php | 9 ++++---- src/Redmine/Serializer/PathSerializer.php | 3 +++ tests/Integration/UrlTest.php | 19 ++++++++++++++++ tests/Unit/Serializer/PathSerializerTest.php | 23 ++++++++++++++++---- 4 files changed, 45 insertions(+), 9 deletions(-) diff --git a/src/Redmine/Api/AbstractApi.php b/src/Redmine/Api/AbstractApi.php index 2cac6365..9f136213 100644 --- a/src/Redmine/Api/AbstractApi.php +++ b/src/Redmine/Api/AbstractApi.php @@ -6,6 +6,7 @@ use Redmine\Api; use Redmine\Client\Client; use Redmine\Exception\ConversionException; +use Redmine\Serializer\PathSerializer; use SimpleXMLElement; /** @@ -226,11 +227,9 @@ protected function retrieveData(string $endpoint, array $params = []): array $params['limit'] = $_limit; $params['offset'] = $offset; - $queryString = http_build_query($params); - // replace every encoded array (`foo[0]=`, `foo[1]=`, `foo[2]=`, etc => `foo[]=`) - $queryString = preg_replace('/%5B[0-9]+%5D/simU', '%5B%5D', $queryString); - - $this->client->requestGet($endpoint.'?'.$queryString); + $this->client->requestGet( + PathSerializer::create($endpoint, $params)->getPath() + ); $newDataSet = $this->getLastResponseBodyAsArray(); diff --git a/src/Redmine/Serializer/PathSerializer.php b/src/Redmine/Serializer/PathSerializer.php index fc945933..b7736f08 100644 --- a/src/Redmine/Serializer/PathSerializer.php +++ b/src/Redmine/Serializer/PathSerializer.php @@ -31,6 +31,9 @@ public function getPath(): string if (! empty($this->queryParams)) { $queryString = '?' . \http_build_query($this->queryParams); + + // @see #154: replace every encoded array (`foo[0]=`, `foo[1]=`, etc with `foo[]=`) + $queryString = preg_replace('/%5B[0-9]+%5D/simU', '%5B%5D', $queryString); } return $this->path . $queryString; diff --git a/tests/Integration/UrlTest.php b/tests/Integration/UrlTest.php index 9552d9ad..a7fc0c39 100644 --- a/tests/Integration/UrlTest.php +++ b/tests/Integration/UrlTest.php @@ -105,6 +105,11 @@ public function testIssue() $this->assertEquals('/issues.json', $res['path']); $this->assertEquals('GET', $res['method']); + $res = $api->all(['limit' => 250]); + + $this->assertEquals('/issues.json?limit=100&offset=0', $res['path']); + $this->assertEquals('GET', $res['method']); + $res = $api->show(1); $this->assertEquals('/issues/1.json?', $res['path']); @@ -334,6 +339,20 @@ public function testTimeEntry() $this->assertEquals('/time_entries.json', $res['path']); $this->assertEquals('GET', $res['method']); + // Test for #154: fix http_build_query encoding array values with numeric keys + $res = $api->all([ + "f" => ["spent_on"], + "op" => ["spent_on" => "><"], + "v" => [ + "spent_on" => [ + "2016-01-18", + "2016-01-22" + ] + ], + ]); + $this->assertEquals('/time_entries.json?limit=25&offset=0&f%5B%5D=spent_on&op%5Bspent_on%5D=%3E%3C&v%5Bspent_on%5D%5B%5D=2016-01-18&v%5Bspent_on%5D%5B%5D=2016-01-22', $res['path']); + $this->assertEquals('GET', $res['method']); + $res = $api->show(1); $this->assertEquals('/time_entries/1.json', $res['path']); diff --git a/tests/Unit/Serializer/PathSerializerTest.php b/tests/Unit/Serializer/PathSerializerTest.php index aa50e31e..f7fe8bb5 100644 --- a/tests/Unit/Serializer/PathSerializerTest.php +++ b/tests/Unit/Serializer/PathSerializerTest.php @@ -15,19 +15,34 @@ public function getPathData() [ 'foo.json', [], - 'foo.json' + 'foo.json', ], [ 'foo.xml?format=xml', [], - 'foo.xml?format=xml' + 'foo.xml?format=xml', ], [ 'foo.xml', [ 'format' => 'xml', ], - 'foo.xml?format=xml' + 'foo.xml?format=xml', + ], + // Test for #154: fix http_build_query encoding array values with numeric keys + [ + '/time_entries.json', + [ + 'f' => ['spent_on'], + 'op' => ['spent_on' => '><'], + 'v' => [ + 'spent_on' => [ + '2016-01-18', + '2016-01-22' + ], + ], + ], + '/time_entries.json?f%5B%5D=spent_on&op%5Bspent_on%5D=%3E%3C&v%5Bspent_on%5D%5B%5D=2016-01-18&v%5Bspent_on%5D%5B%5D=2016-01-22', ], ]; } @@ -37,7 +52,7 @@ public function getPathData() * * @dataProvider getPathData */ - public function getPathShouldReturn(string $path, array $params, string $expected) + public function getPathShouldReturnExpectedString(string $path, array $params, string $expected) { $serializer = PathSerializer::create($path, $params); From 68a515e0a4ddb20a22cedf60d137f55b52137b42 Mon Sep 17 00:00:00 2001 From: Art4 Date: Fri, 25 Feb 2022 09:30:36 +0100 Subject: [PATCH 06/15] use PathSerializer in IssueCategory api --- src/Redmine/Api/IssueCategory.php | 5 ++++- tests/Integration/UrlTest.php | 8 +++++++- tests/Unit/Api/IssueCategoryTest.php | 9 +++------ 3 files changed, 14 insertions(+), 8 deletions(-) diff --git a/src/Redmine/Api/IssueCategory.php b/src/Redmine/Api/IssueCategory.php index 66eaed62..7612f8c2 100644 --- a/src/Redmine/Api/IssueCategory.php +++ b/src/Redmine/Api/IssueCategory.php @@ -3,6 +3,7 @@ namespace Redmine\Api; use Redmine\Exception\MissingParameterException; +use Redmine\Serializer\PathSerializer; /** * Listing issue categories, creating, editing. @@ -158,6 +159,8 @@ public function update($id, array $params) */ public function remove($id, array $params = []) { - return $this->delete('/issue_categories/'.$id.'.xml?'.http_build_query($params)); + return $this->delete( + PathSerializer::create('/issue_categories/'.$id.'.xml', $params)->getPath() + ); } } diff --git a/tests/Integration/UrlTest.php b/tests/Integration/UrlTest.php index a7fc0c39..6b1a8225 100644 --- a/tests/Integration/UrlTest.php +++ b/tests/Integration/UrlTest.php @@ -169,7 +169,13 @@ public function testIssueCategory() $res = $api->remove(1); $res = json_decode($res, true); - $this->assertEquals('/issue_categories/1.xml?', $res['path']); + $this->assertEquals('/issue_categories/1.xml', $res['path']); + $this->assertEquals('DELETE', $res['method']); + + $res = $api->remove(1, ['reassign_to_id' => 16]); + $res = json_decode($res, true); + + $this->assertEquals('/issue_categories/1.xml?reassign_to_id=16', $res['path']); $this->assertEquals('DELETE', $res['method']); } diff --git a/tests/Unit/Api/IssueCategoryTest.php b/tests/Unit/Api/IssueCategoryTest.php index 5017025a..0670408b 100644 --- a/tests/Unit/Api/IssueCategoryTest.php +++ b/tests/Unit/Api/IssueCategoryTest.php @@ -248,12 +248,9 @@ public function testRemoveCallsDelete() $client->expects($this->once()) ->method('requestDelete') ->with( - $this->logicalAnd( - $this->stringStartsWith('/issue_categories/5'), - $this->logicalXor( - $this->stringContains('.json?'), - $this->stringContains('.xml?') - ) + $this->logicalXor( + $this->stringStartsWith('/issue_categories/5.json'), + $this->stringStartsWith('/issue_categories/5.xml') ) ) ->willReturn(true); From 4c914bec4d0e5e427b3373f8010c41f53f382ad6 Mon Sep 17 00:00:00 2001 From: Art4 Date: Fri, 25 Feb 2022 09:51:55 +0100 Subject: [PATCH 07/15] Replace all calls of http_build_query with PathSerializer --- src/Redmine/Api/Attachment.php | 7 ++++++- src/Redmine/Api/Group.php | 5 ++++- src/Redmine/Api/Issue.php | 6 +++++- src/Redmine/Api/Project.php | 5 ++++- src/Redmine/Api/User.php | 9 ++++----- src/Redmine/Api/Wiki.php | 18 ++++++++++++++---- tests/Integration/UrlTest.php | 6 +++--- 7 files changed, 40 insertions(+), 16 deletions(-) diff --git a/src/Redmine/Api/Attachment.php b/src/Redmine/Api/Attachment.php index d81c74d0..0c88f250 100644 --- a/src/Redmine/Api/Attachment.php +++ b/src/Redmine/Api/Attachment.php @@ -2,6 +2,8 @@ namespace Redmine\Api; +use Redmine\Serializer\PathSerializer; + /** * Attachment details. * @@ -51,7 +53,10 @@ public function download($id) */ public function upload($attachment, $params = []) { - return $this->post('/uploads.json?'.http_build_query($params), $attachment); + return $this->post( + PathSerializer::create('/uploads.json', $params)->getPath(), + $attachment + ); } /** diff --git a/src/Redmine/Api/Group.php b/src/Redmine/Api/Group.php index 3b364af9..31716973 100644 --- a/src/Redmine/Api/Group.php +++ b/src/Redmine/Api/Group.php @@ -4,6 +4,7 @@ use Exception; use Redmine\Exception\MissingParameterException; +use Redmine\Serializer\PathSerializer; /** * Handling of groups. @@ -110,7 +111,9 @@ public function update($id, array $params = []) */ public function show($id, array $params = []) { - return $this->get('/groups/'.urlencode($id).'.json?'.http_build_query($params)); + return $this->get( + PathSerializer::create('/groups/'.urlencode($id).'.json', $params)->getPath() + ); } /** diff --git a/src/Redmine/Api/Issue.php b/src/Redmine/Api/Issue.php index 78418786..9956f4ef 100644 --- a/src/Redmine/Api/Issue.php +++ b/src/Redmine/Api/Issue.php @@ -2,6 +2,8 @@ namespace Redmine\Api; +use Redmine\Serializer\PathSerializer; + /** * Listing issues, searching, editing and closing your projects issues. * @@ -59,7 +61,9 @@ public function show($id, array $params = []) $params['include'] = implode(',', $params['include']); } - return $this->get('/issues/'.urlencode($id).'.json?'.http_build_query($params)); + return $this->get( + PathSerializer::create('/issues/'.urlencode($id).'.json', $params)->getPath() + ); } /** diff --git a/src/Redmine/Api/Project.php b/src/Redmine/Api/Project.php index d12f5d1a..7ae50cbe 100755 --- a/src/Redmine/Api/Project.php +++ b/src/Redmine/Api/Project.php @@ -3,6 +3,7 @@ namespace Redmine\Api; use Redmine\Exception\MissingParameterException; +use Redmine\Serializer\PathSerializer; /** * Listing projects, creating, editing. @@ -90,7 +91,9 @@ public function show($id, array $params = []) $params['include'] = 'trackers,issue_categories,attachments,relations'; } - return $this->get('/projects/'.urlencode($id).'.json?'.http_build_query($params)); + return $this->get( + PathSerializer::create('/projects/'.urlencode($id).'.json', $params)->getPath() + ); } /** diff --git a/src/Redmine/Api/User.php b/src/Redmine/Api/User.php index d1f70b82..42082f9b 100644 --- a/src/Redmine/Api/User.php +++ b/src/Redmine/Api/User.php @@ -3,6 +3,7 @@ namespace Redmine\Api; use Redmine\Exception\MissingParameterException; +use Redmine\Serializer\PathSerializer; /** * Listing users, creating, editing. @@ -117,11 +118,9 @@ public function show($id, array $params = []) ); $params['include'] = implode(',', $params['include']); - return $this->get(sprintf( - '/users/%s.json?%s', - urlencode($id), - http_build_query($params) - )); + return $this->get( + PathSerializer::create('/users/'.urlencode($id).'.json', $params)->getPath() + ); } /** diff --git a/src/Redmine/Api/Wiki.php b/src/Redmine/Api/Wiki.php index 7157513a..39bb76ea 100644 --- a/src/Redmine/Api/Wiki.php +++ b/src/Redmine/Api/Wiki.php @@ -2,6 +2,8 @@ namespace Redmine\Api; +use Redmine\Serializer\PathSerializer; + /** * Listing Wiki pages. * @@ -44,11 +46,19 @@ public function all($project, array $params = []) */ public function show($project, $page, $version = null) { - $path = null === $version - ? '/projects/'.$project.'/wiki/'.$page.'.json?include=attachments' - : '/projects/'.$project.'/wiki/'.$page.'/'.$version.'.json?include=attachments'; + $params = [ + 'include' => 'attachments', + ]; + + if ($version === null) { + $path = '/projects/'.$project.'/wiki/'.$page.'.json'; + } else { + $path = '/projects/'.$project.'/wiki/'.$page.'/'.$version.'.json'; + } - return $this->get($path); + return $this->get( + PathSerializer::create($path, $params)->getPath() + ); } /** diff --git a/tests/Integration/UrlTest.php b/tests/Integration/UrlTest.php index 6b1a8225..543af9d4 100644 --- a/tests/Integration/UrlTest.php +++ b/tests/Integration/UrlTest.php @@ -28,7 +28,7 @@ public function testAttachment() $res = $api->upload('asdf'); $res = json_decode($res, true); - $this->assertEquals('/uploads.json?', $res['path']); + $this->assertEquals('/uploads.json', $res['path']); $this->assertEquals('POST', $res['method']); } @@ -59,7 +59,7 @@ public function testGroup() $res = $api->show(1); - $this->assertEquals('/groups/1.json?', $res['path']); + $this->assertEquals('/groups/1.json', $res['path']); $this->assertEquals('GET', $res['method']); $res = $api->remove(1); @@ -112,7 +112,7 @@ public function testIssue() $res = $api->show(1); - $this->assertEquals('/issues/1.json?', $res['path']); + $this->assertEquals('/issues/1.json', $res['path']); $this->assertEquals('GET', $res['method']); $res = $api->remove(1); From 621704f8bb6ea93487115fb5c8646d1dc5910491 Mon Sep 17 00:00:00 2001 From: Art4 Date: Fri, 25 Feb 2022 11:10:33 +0100 Subject: [PATCH 08/15] Rename ConversionException to SerializerException --- src/Redmine/Api/AbstractApi.php | 16 ++-- ...nException.php => SerializerException.php} | 2 +- src/Redmine/Serializer/JsonSerializer.php | 58 ++++++++++++ tests/Unit/Serializer/JsonSerializerTest.php | 90 +++++++++++++++++++ 4 files changed, 157 insertions(+), 9 deletions(-) rename src/Redmine/Exception/{ConversionException.php => SerializerException.php} (65%) create mode 100644 src/Redmine/Serializer/JsonSerializer.php create mode 100644 tests/Unit/Serializer/JsonSerializerTest.php diff --git a/src/Redmine/Api/AbstractApi.php b/src/Redmine/Api/AbstractApi.php index 9f136213..e4182a80 100644 --- a/src/Redmine/Api/AbstractApi.php +++ b/src/Redmine/Api/AbstractApi.php @@ -5,7 +5,7 @@ use JsonException; use Redmine\Api; use Redmine\Client\Client; -use Redmine\Exception\ConversionException; +use Redmine\Exception\SerializerException; use Redmine\Serializer\PathSerializer; use SimpleXMLElement; @@ -191,7 +191,7 @@ protected function retrieveAll($endpoint, array $params = []) * @param string $endpoint API end point * @param array $params optional query parameters to be passed to the api (offset, limit, ...) * - * @throws ConversionException if response body could not be converted into array + * @throws SerializerException if response body could not be converted into array * * @return array elements found */ @@ -298,7 +298,7 @@ protected function attachCustomFieldXML(SimpleXMLElement $xml, array $fields) /** * returns the last response body as array * - * @throws ConversionException if response body could not be converted into array + * @throws SerializerException if response body could not be converted into array */ private function getLastResponseBodyAsArray(): array { @@ -307,7 +307,7 @@ private function getLastResponseBodyAsArray(): array // if body is empty if ($body === '') { - throw new ConversionException( + throw new SerializerException( 'Could not convert empty response body into array' ); } @@ -319,7 +319,7 @@ private function getLastResponseBodyAsArray(): array try { $returnData = new SimpleXMLElement($body); } catch (\Exception $e) { - throw new ConversionException( + throw new SerializerException( 'Catched error "' . $e->getMessage() . '" while decoding body as XML: ' . $body, $e->getCode(), $e @@ -334,7 +334,7 @@ private function getLastResponseBodyAsArray(): array \JSON_THROW_ON_ERROR ); } catch (JsonException $e) { - throw new ConversionException( + throw new SerializerException( 'Catched error "' . $e->getMessage() . '" while en- and decoding body as JSON: ' . $body, $e->getCode(), $e @@ -349,7 +349,7 @@ private function getLastResponseBodyAsArray(): array \JSON_THROW_ON_ERROR ); } catch (JsonException $e) { - throw new ConversionException( + throw new SerializerException( 'Catched error "' . $e->getMessage() . '" while decoding body as JSON: ' . $body, $e->getCode(), $e @@ -358,7 +358,7 @@ private function getLastResponseBodyAsArray(): array } if (! is_array($returnData)) { - throw new ConversionException( + throw new SerializerException( 'Could not convert response body into array: ' . $body ); } diff --git a/src/Redmine/Exception/ConversionException.php b/src/Redmine/Exception/SerializerException.php similarity index 65% rename from src/Redmine/Exception/ConversionException.php rename to src/Redmine/Exception/SerializerException.php index 5f83c3f3..cfdafd34 100644 --- a/src/Redmine/Exception/ConversionException.php +++ b/src/Redmine/Exception/SerializerException.php @@ -5,6 +5,6 @@ use RuntimeException; use Redmine\Exception as RedmineException; -class ConversionException extends RuntimeException implements RedmineException +class SerializerException extends RuntimeException implements RedmineException { } diff --git a/src/Redmine/Serializer/JsonSerializer.php b/src/Redmine/Serializer/JsonSerializer.php new file mode 100644 index 00000000..c293b50d --- /dev/null +++ b/src/Redmine/Serializer/JsonSerializer.php @@ -0,0 +1,58 @@ +decode($data); + + return $serializer; + } + + private string $encoded; + + /** @var mixed $normalized */ + private $normalized; + + private function __construct() + { + // use factory method instead + } + + /** + * @return mixed + */ + public function getNormalized() + { + return $this->normalized; + } + + private function decode(string $encoded) + { + $this->encoded = $encoded; + + try { + $this->normalized = json_decode( + $encoded, + true, + 512, + \JSON_THROW_ON_ERROR + ); + } catch (JsonException $e) { + throw new SerializerException( + 'Catched error "' . $e->getMessage() . '" while decoding JSON: ' . $encoded, + $e->getCode(), + $e + ); + } + } +} diff --git a/tests/Unit/Serializer/JsonSerializerTest.php b/tests/Unit/Serializer/JsonSerializerTest.php new file mode 100644 index 00000000..51b199fa --- /dev/null +++ b/tests/Unit/Serializer/JsonSerializerTest.php @@ -0,0 +1,90 @@ + 'bar'], + ], + ]; + } + + /** + * @test + * + * @dataProvider getNormalizedAndEncodedData + */ + public function createFromStringDecodesToExpectedNormalizedData(string $data, $expected) + { + $serializer = JsonSerializer::createFromString($data); + + $this->assertSame($expected, $serializer->getNormalized()); + } + + public function getInvalidEncodedData() + { + return [ + [''], + ['["foo":"bar"]'], + ]; + } + + /** + * @test + * + * @dataProvider getInvalidEncodedData + */ + public function createFromStringWithInvalidStringThrowsException(string $data) + { + $this->expectException(SerializerException::class); + + $serializer = JsonSerializer::createFromString($data); + } +} From 50a3a1f658af26532fee1422972d294ce768c1fd Mon Sep 17 00:00:00 2001 From: Art4 Date: Fri, 25 Feb 2022 11:22:30 +0100 Subject: [PATCH 09/15] User JsonSerializer instead of json_decode --- src/Redmine/Api/AbstractApi.php | 38 +++++-------------------------- src/Redmine/Api/IssueRelation.php | 6 ++++- 2 files changed, 11 insertions(+), 33 deletions(-) diff --git a/src/Redmine/Api/AbstractApi.php b/src/Redmine/Api/AbstractApi.php index e4182a80..959db036 100644 --- a/src/Redmine/Api/AbstractApi.php +++ b/src/Redmine/Api/AbstractApi.php @@ -6,6 +6,7 @@ use Redmine\Api; use Redmine\Client\Client; use Redmine\Exception\SerializerException; +use Redmine\Serializer\JsonSerializer; use Redmine\Serializer\PathSerializer; use SimpleXMLElement; @@ -65,10 +66,9 @@ protected function get($path, $decodeIfJson = true) if (true === $decodeIfJson && '' !== $body && 0 === strpos($contentType, 'application/json')) { try { - return json_decode($body, true, 512, \JSON_THROW_ON_ERROR); - } catch (JsonException $e) { - // TODO: Throw Exception instead of returning string - return 'Error decoding body as JSON: '.$e->getMessage(); + return JsonSerializer::createFromString($body)->getNormalized(); + } catch (SerializerException $e) { + return 'Error decoding body as JSON: '.$e->getPrevious()->getMessage(); } } @@ -326,35 +326,9 @@ private function getLastResponseBodyAsArray(): array ); } - try { - $returnData = json_decode( - json_encode($returnData, \JSON_THROW_ON_ERROR), - true, - 512, - \JSON_THROW_ON_ERROR - ); - } catch (JsonException $e) { - throw new SerializerException( - 'Catched error "' . $e->getMessage() . '" while en- and decoding body as JSON: ' . $body, - $e->getCode(), - $e - ); - } + $returnData = JsonSerializer::createFromString(json_encode($returnData, \JSON_THROW_ON_ERROR))->getNormalized(); } else if (0 === strpos($contentType, 'application/json')) { - try { - $returnData = json_decode( - $body, - true, - 512, - \JSON_THROW_ON_ERROR - ); - } catch (JsonException $e) { - throw new SerializerException( - 'Catched error "' . $e->getMessage() . '" while decoding body as JSON: ' . $body, - $e->getCode(), - $e - ); - } + $returnData = JsonSerializer::createFromString($body)->getNormalized(); } if (! is_array($returnData)) { diff --git a/src/Redmine/Api/IssueRelation.php b/src/Redmine/Api/IssueRelation.php index bb6aa31f..a4b0910f 100644 --- a/src/Redmine/Api/IssueRelation.php +++ b/src/Redmine/Api/IssueRelation.php @@ -2,6 +2,8 @@ namespace Redmine\Api; +use Redmine\Serializer\JsonSerializer; + /** * Handling issue relations. * @@ -85,6 +87,8 @@ public function create($issueId, array $params = []) $params = json_encode(['relation' => $params]); - return json_decode($this->post('/issues/'.urlencode($issueId).'/relations.json', $params), true); + $response = $this->post('/issues/'.urlencode($issueId).'/relations.json', $params); + + return JsonSerializer::createFromString($response)->getNormalized(); } } From f6131aa3f88cb4dfb38e202858c171902eb5454b Mon Sep 17 00:00:00 2001 From: Art4 Date: Fri, 25 Feb 2022 12:37:58 +0100 Subject: [PATCH 10/15] Create XmlSerializer --- src/Redmine/Serializer/JsonSerializer.php | 3 + src/Redmine/Serializer/XmlSerializer.php | 76 ++++++++++++++++ tests/Unit/Serializer/XmlSerializerTest.php | 96 +++++++++++++++++++++ 3 files changed, 175 insertions(+) create mode 100644 src/Redmine/Serializer/XmlSerializer.php create mode 100644 tests/Unit/Serializer/XmlSerializerTest.php diff --git a/src/Redmine/Serializer/JsonSerializer.php b/src/Redmine/Serializer/JsonSerializer.php index c293b50d..62a58228 100644 --- a/src/Redmine/Serializer/JsonSerializer.php +++ b/src/Redmine/Serializer/JsonSerializer.php @@ -10,6 +10,9 @@ */ final class JsonSerializer { + /** + * @throws SerializerException if $data is not valid JSON + */ public static function createFromString(string $data): self { $serializer = new self(); diff --git a/src/Redmine/Serializer/XmlSerializer.php b/src/Redmine/Serializer/XmlSerializer.php new file mode 100644 index 00000000..632cc6b8 --- /dev/null +++ b/src/Redmine/Serializer/XmlSerializer.php @@ -0,0 +1,76 @@ +deserialize($data); + + return $serializer; + } + + private string $encoded; + + /** @var mixed $normalized */ + private $normalized; + + private SimpleXMLElement $deserialized; + + private function __construct() + { + // use factory method instead + } + + /** + * @return mixed + */ + public function getNormalized() + { + return $this->normalized; + } + + private function deserialize(string $encoded) + { + $this->encoded = $encoded; + + try { + $this->deserialized = new SimpleXMLElement($encoded); + } catch (Exception $e) { + throw new SerializerException( + 'Catched error "' . $e->getMessage() . '" while decoding XML: ' . $encoded, + $e->getCode(), + $e + ); + } + + $this->normalize($this->deserialized); + } + + private function normalize(SimpleXMLElement $deserialized) + { + try { + $serialized = json_encode($deserialized, \JSON_THROW_ON_ERROR); + } catch (JsonException $e) { + throw new SerializerException( + 'Catched error "' . $e->getMessage() . '" while encoding SimpleXMLElement', + $e->getCode(), + $e + ); + } + + $this->normalized = JsonSerializer::createFromString($serialized)->getNormalized(); + } +} diff --git a/tests/Unit/Serializer/XmlSerializerTest.php b/tests/Unit/Serializer/XmlSerializerTest.php new file mode 100644 index 00000000..944d2a09 --- /dev/null +++ b/tests/Unit/Serializer/XmlSerializerTest.php @@ -0,0 +1,96 @@ +', + [], + ], + [ + '', + [], + ], + [ + '', + [], + ], + [ + '1', + ['1'], + ], + [ + <<< END + + + + 4326 + + + 4325 + + + END, + [ + '@attributes' => [ + 'type' => 'array', + 'count' => '1640', + ], + 'issue' => [ + [ + 'id' => '4326', + ], + [ + 'id' => '4325', + ], + ], + ], + ], + ]; + } + + /** + * @test + * + * @dataProvider getNormalizedAndEncodedData + */ + public function createFromStringDecodesToExpectedNormalizedData(string $data, $expected) + { + $serializer = XmlSerializer::createFromString($data); + + $this->assertSame($expected, $serializer->getNormalized()); + } + + public function getInvalidEncodedData() + { + return [ + [''], + [''], + ['<>'], + [''], + [''], + ]; + } + + /** + * @test + * + * @dataProvider getInvalidEncodedData + */ + public function createFromStringWithInvalidStringThrowsException(string $data) + { + $this->expectException(SerializerException::class); + + $serializer = XmlSerializer::createFromString($data); + } +} From 2b3faa277b0010dcc7f68bf7768ef22abbfe7d59 Mon Sep 17 00:00:00 2001 From: Art4 Date: Fri, 25 Feb 2022 13:03:58 +0100 Subject: [PATCH 11/15] Use XmlSerializer in AbstracApi --- src/Redmine/Api/AbstractApi.php | 13 ++----------- src/Redmine/Serializer/JsonSerializer.php | 2 +- src/Redmine/Serializer/XmlSerializer.php | 4 ++-- 3 files changed, 5 insertions(+), 14 deletions(-) diff --git a/src/Redmine/Api/AbstractApi.php b/src/Redmine/Api/AbstractApi.php index 959db036..093eb8ee 100644 --- a/src/Redmine/Api/AbstractApi.php +++ b/src/Redmine/Api/AbstractApi.php @@ -8,6 +8,7 @@ use Redmine\Exception\SerializerException; use Redmine\Serializer\JsonSerializer; use Redmine\Serializer\PathSerializer; +use Redmine\Serializer\XmlSerializer; use SimpleXMLElement; /** @@ -316,17 +317,7 @@ private function getLastResponseBodyAsArray(): array // parse XML if (0 === strpos($contentType, 'application/xml')) { - try { - $returnData = new SimpleXMLElement($body); - } catch (\Exception $e) { - throw new SerializerException( - 'Catched error "' . $e->getMessage() . '" while decoding body as XML: ' . $body, - $e->getCode(), - $e - ); - } - - $returnData = JsonSerializer::createFromString(json_encode($returnData, \JSON_THROW_ON_ERROR))->getNormalized(); + $returnData = XmlSerializer::createFromString($body)->getNormalized(); } else if (0 === strpos($contentType, 'application/json')) { $returnData = JsonSerializer::createFromString($body)->getNormalized(); } diff --git a/src/Redmine/Serializer/JsonSerializer.php b/src/Redmine/Serializer/JsonSerializer.php index 62a58228..b15b5c41 100644 --- a/src/Redmine/Serializer/JsonSerializer.php +++ b/src/Redmine/Serializer/JsonSerializer.php @@ -39,7 +39,7 @@ public function getNormalized() return $this->normalized; } - private function decode(string $encoded) + private function decode(string $encoded): void { $this->encoded = $encoded; diff --git a/src/Redmine/Serializer/XmlSerializer.php b/src/Redmine/Serializer/XmlSerializer.php index 632cc6b8..6bf80672 100644 --- a/src/Redmine/Serializer/XmlSerializer.php +++ b/src/Redmine/Serializer/XmlSerializer.php @@ -42,7 +42,7 @@ public function getNormalized() return $this->normalized; } - private function deserialize(string $encoded) + private function deserialize(string $encoded): void { $this->encoded = $encoded; @@ -59,7 +59,7 @@ private function deserialize(string $encoded) $this->normalize($this->deserialized); } - private function normalize(SimpleXMLElement $deserialized) + private function normalize(SimpleXMLElement $deserialized): void { try { $serialized = json_encode($deserialized, \JSON_THROW_ON_ERROR); From e54a3634b06a13e8ea591811ed168d2c48db0322 Mon Sep 17 00:00:00 2001 From: Art4 Date: Fri, 25 Feb 2022 13:07:40 +0100 Subject: [PATCH 12/15] Remove check for empty body --- src/Redmine/Api/AbstractApi.php | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/src/Redmine/Api/AbstractApi.php b/src/Redmine/Api/AbstractApi.php index 093eb8ee..c600ac98 100644 --- a/src/Redmine/Api/AbstractApi.php +++ b/src/Redmine/Api/AbstractApi.php @@ -304,16 +304,8 @@ protected function attachCustomFieldXML(SimpleXMLElement $xml, array $fields) private function getLastResponseBodyAsArray(): array { $body = $this->client->getLastResponseBody(); - $returnData = null; - - // if body is empty - if ($body === '') { - throw new SerializerException( - 'Could not convert empty response body into array' - ); - } - $contentType = $this->client->getLastResponseContentType(); + $returnData = null; // parse XML if (0 === strpos($contentType, 'application/xml')) { From d96deaf664f207fd60720986e5266b9f9ee42278 Mon Sep 17 00:00:00 2001 From: Art4 Date: Fri, 25 Feb 2022 13:21:48 +0100 Subject: [PATCH 13/15] use retrieveData instead of retrieveAll --- src/Redmine/Api/AbstractApi.php | 10 +++++----- 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, 24 insertions(+), 24 deletions(-) diff --git a/src/Redmine/Api/AbstractApi.php b/src/Redmine/Api/AbstractApi.php index c600ac98..db2bc7a6 100644 --- a/src/Redmine/Api/AbstractApi.php +++ b/src/Redmine/Api/AbstractApi.php @@ -176,13 +176,13 @@ protected function retrieveAll($endpoint, array $params = []) { @trigger_error('The '.__METHOD__.' method is deprecated, use `retrieveData()` instead.', E_USER_DEPRECATED); - $data = $this->retrieveData(strval($endpoint), $params); - - if (! array_key_exists('response', $data)) { - return $data; + try { + $data = $this->retrieveData(strval($endpoint), $params); + } catch (SerializerException $e) { + $data = false; } - return ('' === $data['response']) ? false : $data['response']; + return $data; } /** diff --git a/src/Redmine/Api/CustomField.php b/src/Redmine/Api/CustomField.php index ea3739d8..523a3726 100644 --- a/src/Redmine/Api/CustomField.php +++ b/src/Redmine/Api/CustomField.php @@ -24,7 +24,7 @@ class CustomField extends AbstractApi */ public function all(array $params = []) { - $this->customFields = $this->retrieveAll('/custom_fields.json', $params); + $this->customFields = $this->retrieveData('/custom_fields.json', $params); return $this->customFields; } diff --git a/src/Redmine/Api/Group.php b/src/Redmine/Api/Group.php index 31716973..ef8169c4 100644 --- a/src/Redmine/Api/Group.php +++ b/src/Redmine/Api/Group.php @@ -28,7 +28,7 @@ class Group extends AbstractApi */ public function all(array $params = []) { - $this->groups = $this->retrieveAll('/groups.json', $params); + $this->groups = $this->retrieveData('/groups.json', $params); return $this->groups; } diff --git a/src/Redmine/Api/Issue.php b/src/Redmine/Api/Issue.php index 9956f4ef..ecb9880c 100644 --- a/src/Redmine/Api/Issue.php +++ b/src/Redmine/Api/Issue.php @@ -40,7 +40,7 @@ class Issue extends AbstractApi */ public function all(array $params = []) { - return $this->retrieveAll('/issues.json', $params); + return $this->retrieveData('/issues.json', $params); } /** diff --git a/src/Redmine/Api/IssueCategory.php b/src/Redmine/Api/IssueCategory.php index 7612f8c2..2a2a8698 100644 --- a/src/Redmine/Api/IssueCategory.php +++ b/src/Redmine/Api/IssueCategory.php @@ -28,7 +28,7 @@ class IssueCategory extends AbstractApi */ public function all($project, array $params = []) { - $this->issueCategories = $this->retrieveAll('/projects/'.$project.'/issue_categories.json', $params); + $this->issueCategories = $this->retrieveData('/projects/'.$project.'/issue_categories.json', $params); return $this->issueCategories; } diff --git a/src/Redmine/Api/IssuePriority.php b/src/Redmine/Api/IssuePriority.php index f80bb1e9..6d6b7e46 100644 --- a/src/Redmine/Api/IssuePriority.php +++ b/src/Redmine/Api/IssuePriority.php @@ -24,7 +24,7 @@ class IssuePriority extends AbstractApi */ public function all(array $params = []) { - $this->issuePriorities = $this->retrieveAll('/enumerations/issue_priorities.json', $params); + $this->issuePriorities = $this->retrieveData('/enumerations/issue_priorities.json', $params); return $this->issuePriorities; } diff --git a/src/Redmine/Api/IssueRelation.php b/src/Redmine/Api/IssueRelation.php index a4b0910f..1574b9ba 100644 --- a/src/Redmine/Api/IssueRelation.php +++ b/src/Redmine/Api/IssueRelation.php @@ -27,7 +27,7 @@ class IssueRelation extends AbstractApi */ public function all($issueId, array $params = []) { - $this->relations = $this->retrieveAll('/issues/'.urlencode($issueId).'/relations.json', $params); + $this->relations = $this->retrieveData('/issues/'.urlencode($issueId).'/relations.json', $params); return $this->relations; } diff --git a/src/Redmine/Api/IssueStatus.php b/src/Redmine/Api/IssueStatus.php index 945dab0b..6c6f776a 100644 --- a/src/Redmine/Api/IssueStatus.php +++ b/src/Redmine/Api/IssueStatus.php @@ -24,7 +24,7 @@ class IssueStatus extends AbstractApi */ public function all(array $params = []) { - $this->issueStatuses = $this->retrieveAll('/issue_statuses.json', $params); + $this->issueStatuses = $this->retrieveData('/issue_statuses.json', $params); return $this->issueStatuses; } diff --git a/src/Redmine/Api/Membership.php b/src/Redmine/Api/Membership.php index 99695f5f..ed5cbd33 100644 --- a/src/Redmine/Api/Membership.php +++ b/src/Redmine/Api/Membership.php @@ -27,7 +27,7 @@ class Membership extends AbstractApi */ public function all($project, array $params = []) { - $this->memberships = $this->retrieveAll('/projects/'.$project.'/memberships.json', $params); + $this->memberships = $this->retrieveData('/projects/'.$project.'/memberships.json', $params); return $this->memberships; } diff --git a/src/Redmine/Api/News.php b/src/Redmine/Api/News.php index 7380e56a..451ffc18 100644 --- a/src/Redmine/Api/News.php +++ b/src/Redmine/Api/News.php @@ -24,7 +24,7 @@ class News extends AbstractApi public function all($project = null, array $params = []) { $path = null === $project ? '/news.json' : '/projects/'.$project.'/news.json'; - $this->news = $this->retrieveAll($path, $params); + $this->news = $this->retrieveData($path, $params); return $this->news; } diff --git a/src/Redmine/Api/Project.php b/src/Redmine/Api/Project.php index 7ae50cbe..90fb25d2 100755 --- a/src/Redmine/Api/Project.php +++ b/src/Redmine/Api/Project.php @@ -27,7 +27,7 @@ class Project extends AbstractApi */ public function all(array $params = []) { - $this->projects = $this->retrieveAll('/projects.json', $params); + $this->projects = $this->retrieveData('/projects.json', $params); return $this->projects; } diff --git a/src/Redmine/Api/Query.php b/src/Redmine/Api/Query.php index 17720151..3bcb2e21 100644 --- a/src/Redmine/Api/Query.php +++ b/src/Redmine/Api/Query.php @@ -24,7 +24,7 @@ class Query extends AbstractApi */ public function all(array $params = []) { - $this->query = $this->retrieveAll('/queries.json', $params); + $this->query = $this->retrieveData('/queries.json', $params); return $this->query; } diff --git a/src/Redmine/Api/Role.php b/src/Redmine/Api/Role.php index 5994e481..0250a9cf 100644 --- a/src/Redmine/Api/Role.php +++ b/src/Redmine/Api/Role.php @@ -24,7 +24,7 @@ class Role extends AbstractApi */ public function all(array $params = []) { - $this->roles = $this->retrieveAll('/roles.json', $params); + $this->roles = $this->retrieveData('/roles.json', $params); return $this->roles; } diff --git a/src/Redmine/Api/Search.php b/src/Redmine/Api/Search.php index 3b47d6ab..f8326b50 100644 --- a/src/Redmine/Api/Search.php +++ b/src/Redmine/Api/Search.php @@ -22,7 +22,7 @@ class Search extends AbstractApi public function search($query, array $params = []) { $params['q'] = $query; - $this->results = $this->retrieveAll('/search.json', $params); + $this->results = $this->retrieveData('/search.json', $params); return $this->results; } diff --git a/src/Redmine/Api/TimeEntry.php b/src/Redmine/Api/TimeEntry.php index fa999003..7f8304b5 100644 --- a/src/Redmine/Api/TimeEntry.php +++ b/src/Redmine/Api/TimeEntry.php @@ -26,7 +26,7 @@ class TimeEntry extends AbstractApi */ public function all(array $params = []) { - $this->timeEntries = $this->retrieveAll('/time_entries.json', $params); + $this->timeEntries = $this->retrieveData('/time_entries.json', $params); return $this->timeEntries; } diff --git a/src/Redmine/Api/TimeEntryActivity.php b/src/Redmine/Api/TimeEntryActivity.php index 46fccc27..a0a10f77 100644 --- a/src/Redmine/Api/TimeEntryActivity.php +++ b/src/Redmine/Api/TimeEntryActivity.php @@ -22,7 +22,7 @@ class TimeEntryActivity extends AbstractApi */ public function all(array $params = []) { - $this->timeEntryActivities = $this->retrieveAll('/enumerations/time_entry_activities.json', $params); + $this->timeEntryActivities = $this->retrieveData('/enumerations/time_entry_activities.json', $params); return $this->timeEntryActivities; } diff --git a/src/Redmine/Api/Tracker.php b/src/Redmine/Api/Tracker.php index accaf672..7c3a6365 100644 --- a/src/Redmine/Api/Tracker.php +++ b/src/Redmine/Api/Tracker.php @@ -24,7 +24,7 @@ class Tracker extends AbstractApi */ public function all(array $params = []) { - $this->trackers = $this->retrieveAll('/trackers.json', $params); + $this->trackers = $this->retrieveData('/trackers.json', $params); return $this->trackers; } diff --git a/src/Redmine/Api/User.php b/src/Redmine/Api/User.php index 42082f9b..b813fdc8 100644 --- a/src/Redmine/Api/User.php +++ b/src/Redmine/Api/User.php @@ -27,7 +27,7 @@ class User extends AbstractApi */ public function all(array $params = []) { - $this->users = $this->retrieveAll('/users.json', $params); + $this->users = $this->retrieveData('/users.json', $params); return $this->users; } diff --git a/src/Redmine/Api/Version.php b/src/Redmine/Api/Version.php index 121dad71..9c90fcd3 100644 --- a/src/Redmine/Api/Version.php +++ b/src/Redmine/Api/Version.php @@ -28,7 +28,7 @@ class Version extends AbstractApi */ public function all($project, array $params = []) { - $this->versions = $this->retrieveAll('/projects/'.$project.'/versions.json', $params); + $this->versions = $this->retrieveData('/projects/'.$project.'/versions.json', $params); return $this->versions; } diff --git a/src/Redmine/Api/Wiki.php b/src/Redmine/Api/Wiki.php index 39bb76ea..6f4a178e 100644 --- a/src/Redmine/Api/Wiki.php +++ b/src/Redmine/Api/Wiki.php @@ -27,7 +27,7 @@ class Wiki extends AbstractApi */ public function all($project, array $params = []) { - $this->wikiPages = $this->retrieveAll('/projects/'.$project.'/wiki/index.json', $params); + $this->wikiPages = $this->retrieveData('/projects/'.$project.'/wiki/index.json', $params); return $this->wikiPages; } From 93b4379f96741adb560353dce128b3b6faf7e55b Mon Sep 17 00:00:00 2001 From: Art4 Date: Fri, 25 Feb 2022 21:34:06 +0100 Subject: [PATCH 14/15] declare serializer as internal --- src/Redmine/Api/AbstractApi.php | 2 +- src/Redmine/Serializer/JsonSerializer.php | 2 ++ src/Redmine/Serializer/PathSerializer.php | 2 ++ src/Redmine/Serializer/XmlSerializer.php | 2 ++ 4 files changed, 7 insertions(+), 1 deletion(-) diff --git a/src/Redmine/Api/AbstractApi.php b/src/Redmine/Api/AbstractApi.php index db2bc7a6..9a178f20 100644 --- a/src/Redmine/Api/AbstractApi.php +++ b/src/Redmine/Api/AbstractApi.php @@ -170,7 +170,7 @@ protected function sanitizeParams(array $defaults, array $params) * @param string $endpoint API end point * @param array $params optional parameters to be passed to the api (offset, limit, ...) * - * @return array elements found + * @return array|false elements found */ protected function retrieveAll($endpoint, array $params = []) { diff --git a/src/Redmine/Serializer/JsonSerializer.php b/src/Redmine/Serializer/JsonSerializer.php index b15b5c41..4da4d2ba 100644 --- a/src/Redmine/Serializer/JsonSerializer.php +++ b/src/Redmine/Serializer/JsonSerializer.php @@ -7,6 +7,8 @@ /** * JsonSerializer + * + * @internal */ final class JsonSerializer { diff --git a/src/Redmine/Serializer/PathSerializer.php b/src/Redmine/Serializer/PathSerializer.php index b7736f08..6f758912 100644 --- a/src/Redmine/Serializer/PathSerializer.php +++ b/src/Redmine/Serializer/PathSerializer.php @@ -4,6 +4,8 @@ /** * PathSerializer + * + * @internal */ final class PathSerializer { diff --git a/src/Redmine/Serializer/XmlSerializer.php b/src/Redmine/Serializer/XmlSerializer.php index 6bf80672..1fc37a3d 100644 --- a/src/Redmine/Serializer/XmlSerializer.php +++ b/src/Redmine/Serializer/XmlSerializer.php @@ -8,6 +8,8 @@ /** * XmlSerializer + * + * @internal */ final class XmlSerializer { From 183f02c60f79deea3f77c988bda9534d44de96c0 Mon Sep 17 00:00:00 2001 From: Art4 Date: Fri, 25 Feb 2022 21:43:24 +0100 Subject: [PATCH 15/15] Update CHANGELOG.md --- CHANGELOG.md | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index e27bb54c..b16609c6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased](https://github.com/kbsali/php-redmine-api/compare/v2.1.0...v2.x) +### Added + +- New method `Redmine\Client\AbstractApi::retrieveData()` to retrieve as many elements as you want as array (even if the total number of elements is greater than 100). +- New exception `Redmine\Client\SerializerException` for JSON/XML serializer related exceptions + +### Deprecated + +- `Redmine\Api\AbstractApi::retrieveAll()` is deprecated, use `Redmine\Api\AbstractApi::retrieveData()` instead + ## [v2.1.1](https://github.com/kbsali/php-redmine-api/compare/v2.1.0...v2.1.1) - 2022-01-15 ### Fixed