Add retrieveData method, deprecate retrieveAll

Author: Art4

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

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));
     }
 
     /**