Merge pull request #248 from aik099/searching-fix

Use "/rest/api/2/search/jql" instead of "/rest/api/2/search" API call internally

Author: aik099

CHANGELOG.md

@@ -10,9 +10,11 @@ This project adheres to [Semantic Versioning](https://semver.org/).
 - The `Api::downloadAttachment` method now throws an exception, when attempting to download from a non-Jira website by [@aik099] (#240).
 - The `$params` argument of the `Api::getWorklogs` method is now optional by [@aik099] (#244).
 - The `$params` argument of the `Api::getTransitions` method is now optional by [@aik099] (#244).
+- The `$start_at = 0` argument of `Api::search` method was renamed into the `$next_page_token = null` to reflect underlying Jira API changes by [@aik099] (#248).
+- Improved performance of `Result::getIssuesCount` method by [@aik099] (#248).
 
 ### Removed
-...
+- The `Api::search` method and the `Walker` class no longer return total issue count, because underlaying Jira API doesn't support this [@aik099] (#248).
 
 ### Fixed
 - The `CurlClient` in combination with cURL version < 7.33.0 was getting `426 Upgrade Required` error on any request to the Atlassian Cloud servers by [@aik099] (#239).

src/Jira/Api.php

@@ -617,28 +617,39 @@ public function createIssue($project_key, $summary, $issue_type, array $other_fi
 	/**
 	 * Query issues.
 	 *
-	 * @param string  $jql         JQL.
-	 * @param integer $start_at    Start at.
-	 * @param integer $max_results Max results.
-	 * @param string  $fields      Fields.
+	 * @param string      $jql             JQL.
+	 * @param string|null $next_page_token Next page token.
+	 * @param integer     $max_results     Max results.
+	 * @param string      $fields          Fields.
 	 *
 	 * @return Result
+	 * @throws Exception When the "$next_page_token" parameter has an invalid value.
 	 * @link   https://developer.atlassian.com/cloud/jira/platform/rest/v2/api-group-issue-search/#api-rest-api-2-search-get
 	 */
-	public function search($jql, $start_at = 0, $max_results = 20, $fields = '*navigable')
+	public function search($jql, $next_page_token = null, $max_results = 20, $fields = '*navigable')
 	{
-		$result = $this->api(
-			self::REQUEST_GET,
-			'/rest/api/2/search',
-			array(
-				'jql' => $jql,
-				'startAt' => $start_at,
-				'maxResults' => $max_results,
-				'fields' => $fields,
-			)
+		// Polyfill for users who don't paginate themselves.
+		if ( $next_page_token === 0 ) {
+			$next_page_token = null;
+		}
+
+		if ( !is_string($next_page_token) && $next_page_token !== null ) {
+			$error_msg = 'The "$next_page_token" value must be either null (for the first page) ';
+			$error_msg .= 'or come from the "nextPageToken" key of the previous search response.';
+			throw new Exception($error_msg);
+		}
+
+		$params = array(
+			'jql' => $jql,
+			'maxResults' => $max_results,
+			'fields' => $fields,
 		);
 
-		return $result;
+		if ( $next_page_token !== null ) {
+			$params['nextPageToken'] = $next_page_token;
+		}
+
+		return $this->api(self::REQUEST_GET, '/rest/api/2/search/jql', $params);
 	}
 
 	/**

src/Jira/Api/Result.php

@@ -108,7 +108,7 @@ public function getTotal()
 	 */
 	public function getIssuesCount()
 	{
-		return count($this->getIssues());
+		return isset($this->result['issues']) ? count($this->result['issues']) : 0;
 	}
 
 	/**
@@ -118,17 +118,17 @@ public function getIssuesCount()
 	 */
 	public function getIssues()
 	{
-		if ( isset($this->result['issues']) ) {
-			$result = array();
+		if ( !isset($this->result['issues']) ) {
+			return array();
+		}
 
-			foreach ( $this->result['issues'] as $issue ) {
-				$result[] = new Issue($issue);
-			}
+		$result = array();
 
-			return $result;
+		foreach ( $this->result['issues'] as $issue ) {
+			$result[] = new Issue($issue);
 		}
 
-		return array();
+		return $result;
 	}
 
 	/**

src/Jira/Issues/Walker.php

@@ -27,7 +27,7 @@
 
 use chobie\Jira\Api;
 
-class Walker implements \Iterator, \Countable
+class Walker implements \IteratorAggregate
 {
 
 	/**
@@ -44,62 +44,13 @@ class Walker implements \Iterator, \Countable
 	 */
 	protected $jql = null;
 
-	/**
-	 * Offset.
-	 *
-	 * @var integer
-	 */
-	protected $offset = 0;
-
-	/**
-	 * Current record index.
-	 *
-	 * @var integer
-	 */
-	protected $current = 0;
-
-	/**
-	 * Total issue count.
-	 *
-	 * @var integer
-	 */
-	protected $total = null;
-
-	/**
-	 * Issue count on current page.
-	 *
-	 * @var integer
-	 */
-	protected $max = 0;
-
-	/**
-	 * Index of issue in issue list (across all issue pages).
-	 *
-	 * @var integer
-	 */
-	protected $startAt = 0;
-
 	/**
 	 * Issues per page.
 	 *
 	 * @var integer
 	 */
 	protected $perPage = 50;
 
-	/**
-	 * Was JQL executed.
-	 *
-	 * @var boolean
-	 */
-	protected $executed = false;
-
-	/**
-	 * Result.
-	 *
-	 * @var array
-	 */
-	protected $issues = array();
-
 	/**
 	 * List of fields to query.
 	 *
@@ -144,143 +95,53 @@ public function push($jql, $fields = null)
 	}
 
 	/**
-	 * Return the current element.
-	 *
-	 * @return mixed Can return any type.
-	 * @link   http://php.net/manual/en/iterator.current.php
-	 */
-	public function current()
-	{
-		if ( is_callable($this->callback) ) {
-			$tmp = $this->issues[$this->offset];
-			$callback = $this->callback;
-
-			return $callback($tmp);
-		}
-
-		return $this->issues[$this->offset];
-	}
-
-	/**
-	 * Move forward to next element.
+	 * @inheritDoc
 	 *
-	 * @return void Any returned value is ignored.
-	 * @link   http://php.net/manual/en/iterator.next.php
-	 */
-	public function next()
-	{
-		$this->offset++;
-	}
-
-	/**
-	 * Return the key of the current element.
-	 *
-	 * @return mixed scalar on success, or null on failure.
-	 * @link   http://php.net/manual/en/iterator.key.php
-	 */
-	public function key()
-	{
-		if ( $this->startAt > 0 ) {
-			return $this->offset + (($this->startAt - 1) * $this->perPage);
-		}
-
-		return 0;
-	}
-
-	/**
-	 * Checks if current position is valid.
-	 *
-	 * @return boolean The return value will be casted to boolean and then evaluated.
-	 *                 Returns true on success or false on failure.
 	 * @throws \Exception When "Walker::push" method wasn't called.
 	 * @throws Api\UnauthorizedException When it happens.
-	 * @link   http://php.net/manual/en/iterator.valid.php
 	 */
-	public function valid()
+	public function getIterator()
 	{
 		if ( $this->jql === null ) {
 			throw new \Exception('you have to call Jira_Walker::push($jql, $fields) at first');
 		}
 
-		if ( !$this->executed ) {
-			try {
-				$result = $this->api->search($this->getQuery(), $this->key(), $this->perPage, $this->fields);
+		$next_page_token = null;
+		$jql = $this->getQuery();
 
-				$this->setResult($result);
-				$this->executed = true;
+		try {
+			while ( true ) {
+				$result = $this->api->search($jql, $next_page_token, $this->perPage, $this->fields);
 
-				if ( $result->getTotal() == 0 ) {
-					return false;
+				if ( $result->getIssuesCount() === 0 ) {
+					return;
 				}
 
-				return true;
-			}
-			catch ( Api\UnauthorizedException $e ) {
-				throw $e;
-			}
-			catch ( \Exception $e ) {
-				error_log($e->getMessage());
-
-				return false;
-			}
-		}
-		else {
-			if ( $this->offset >= $this->max && $this->key() < $this->total ) {
-				try {
-					$result = $this->api->search($this->getQuery(), $this->key(), $this->perPage, $this->fields);
-					$this->setResult($result);
+				foreach ( $result->getIssues() as $issue ) {
+					if ( is_callable($this->callback) ) {
+						$callback = $this->callback;
 
-					return true;
+						yield $callback($issue);
+					}
+					else {
+						yield $issue;
+					}
 				}
-				catch ( Api\UnauthorizedException $e ) {
-					throw $e;
-				}
-				catch ( \Exception $e ) {
-					error_log($e->getMessage());
 
-					return false;
-				}
-			}
-			else {
-				if ( ($this->startAt - 1) * $this->perPage + $this->offset < $this->total ) {
-					return true;
-				}
+				$raw_result = $result->getResult();
+				$next_page_token = array_key_exists('nextPageToken', $raw_result) ? $raw_result['nextPageToken'] : null;
 
-				return false;
+				if ( empty($next_page_token) ) {
+					break;
+				}
 			}
 		}
-	}
-
-	/**
-	 * Rewind the Iterator to the first element.
-	 *
-	 * @return void Any returned value is ignored.
-	 * @link   http://php.net/manual/en/iterator.rewind.php
-	 */
-	public function rewind()
-	{
-		$this->offset = 0;
-		$this->startAt = 0;
-		$this->current = 0;
-		$this->max = 0;
-		$this->total = null;
-		$this->executed = false;
-		$this->issues = array();
-	}
-
-	/**
-	 * Count elements of an object.
-	 *
-	 * @return integer The custom count as an integer.
-	 * @link   http://php.net/manual/en/countable.count.php
-	 */
-	public function count()
-	{
-		if ( $this->total === null ) {
-			$this->valid();
+		catch ( Api\UnauthorizedException $e ) {
+			throw $e;
+		}
+		catch ( \Exception $e ) {
+			error_log($e->getMessage());
 		}
-
-		return $this->total;
 	}
 
 	/**
@@ -301,22 +162,6 @@ public function setDelegate($callable)
 		}
 	}
 
-	/**
-	 * Sets result.
-	 *
-	 * @param Api\Result $result Result.
-	 *
-	 * @return void
-	 */
-	protected function setResult(Api\Result $result)
-	{
-		$this->total = $result->getTotal();
-		$this->offset = 0;
-		$this->max = $result->getIssuesCount();
-		$this->issues = $result->getIssues();
-		$this->startAt++;
-	}
-
 	/**
 	 * Returns JQL.
 	 *