From: Brad Jorsch <bjorsch@wikimedia.org>
Date: Fri, 1 Feb 2013 22:17:19 +0000 (-0500)
Subject: Clean up ApiQueryImageInfo continuation
X-Git-Tag: 1.31.0-rc.0~20764
X-Git-Url: https://git.cyclocoop.org/%7B%24www_url%7Dadmin/compta/exercices/journal.php?a=commitdiff_plain;h=2c9dfcb7d1b527e8f472d58f507eaaaade71bb2a;p=lhc%2Fweb%2Fwiklou.git

Clean up ApiQueryImageInfo continuation

Reviewing this code, I spotted a few issues:
* 'imagerepository' is added to non-images, and to images that were
  skipped this round due to iicontinue. The latter is particularly
  troublesome, as clients may wind up with an incorrect value when
  merging continued results.
* Say two images are being queried, A.jpg and B.jpg. If the query needs
  to be continued somewhere in the middle of A.jpg's old versions, but
  then A.jpg is deleted before the client sends the continuation query,
  it will start in the middle of B.jpg's old versions instead of at the
  beginning of B.jpg's revisions.
* If the query needs to be continued somewhere in the middle of A.jpg's
  old versions, but in the continuation query some other module that is
  also being continued fills the result object, iicontinue will be
  reset to the *beginning* of A.jpg's old versions instead of preserving
  the position in the middle.

Change-Id: I08e2941010c7a70ff90b6244bfddd5ed0540fc9f
---

diff --git a/RELEASE-NOTES-1.21 b/RELEASE-NOTES-1.21
index bf1a20b7fb..6ffe112e7c 100644
--- a/RELEASE-NOTES-1.21
+++ b/RELEASE-NOTES-1.21
@@ -202,6 +202,10 @@ production.
   redirects.
 * On error, any warnings generated before that error will be shown in the result.
 * action=help suports generalized submodules (modules=query+value), querymodules obsolete
+* ApiQueryImageInfo continuation is more reliable. The only major change is
+  that the imagerepository property will no longer be set on page objects not
+  processed in the current query (i.e. non-images or those skipped due to
+  iicontinue).
 
 === API internal changes in 1.21 ===
 * For debugging only, a new global $wgDebugAPI removes many API restrictions when true.
diff --git a/includes/api/ApiQueryImageInfo.php b/includes/api/ApiQueryImageInfo.php
index 4b9bc98839..351753c7a2 100644
--- a/includes/api/ApiQueryImageInfo.php
+++ b/includes/api/ApiQueryImageInfo.php
@@ -52,9 +52,8 @@ class ApiQueryImageInfo extends ApiQueryBase {
 			$titles = array_keys( $pageIds[NS_FILE] );
 			asort( $titles ); // Ensure the order is always the same
 
-			$skip = false;
+			$fromTitle = null;
 			if ( !is_null( $params['continue'] ) ) {
-				$skip = true;
 				$cont = explode( '|', $params['continue'] );
 				$this->dieContinueUsageIf( count( $cont ) != 2 );
 				$fromTitle = strval( $cont[0] );
@@ -77,12 +76,18 @@ class ApiQueryImageInfo extends ApiQueryBase {
 				$images = RepoGroup::singleton()->findFiles( $titles );
 			}
 			foreach ( $titles as $title ) {
+				$pageId = $pageIds[NS_FILE][$title];
+				$start = $title === $fromTitle ? $fromTimestamp : $params['start'];
+
 				if ( !isset( $images[$title] ) ) {
+					$result->addValue(
+						array( 'query', 'pages', intval( $pageId ) ),
+						'imagerepository', ''
+					);
+					// The above can't fail because it doesn't increase the result size
 					continue;
 				}
 
-				$start = $skip ? $fromTimestamp : $params['start'];
-				$pageId = $pageIds[NS_FILE][$title];
 				$img = $images[$title];
 
 				$fit = $result->addValue(
@@ -97,10 +102,11 @@ class ApiQueryImageInfo extends ApiQueryBase {
 						// thing again. When the violating queries have been
 						// out-continued, the result will get through
 						$this->setContinueEnumParameter( 'start',
-							wfTimestamp( TS_ISO_8601, $img->getTimestamp() ) );
+							$start !== null ? $start : wfTimestamp( TS_ISO_8601, $img->getTimestamp() )
+						);
 					} else {
 						$this->setContinueEnumParameter( 'continue',
-							$this->getContinueStr( $img ) );
+							$this->getContinueStr( $img, $start ) );
 					}
 					break;
 				}
@@ -164,18 +170,6 @@ class ApiQueryImageInfo extends ApiQueryBase {
 				if ( !$fit ) {
 					break;
 				}
-				$skip = false;
-			}
-
-			$data = $this->getResultData();
-			foreach ( $data['query']['pages'] as $pageid => $arr ) {
-				if ( is_array( $arr ) && !isset( $arr['imagerepository'] ) ) {
-					$result->addValue(
-						array( 'query', 'pages', $pageid ),
-						'imagerepository', ''
-					);
-				}
-				// The above can't fail because it doesn't increase the result size
 			}
 		}
 	}
@@ -433,9 +427,11 @@ class ApiQueryImageInfo extends ApiQueryBase {
 	 * @param $img File
 	 * @return string
 	 */
-	protected function getContinueStr( $img ) {
-		return $img->getOriginalTitle()->getText() .
-			'|' . $img->getTimestamp();
+	protected function getContinueStr( $img, $start = null ) {
+		if ( $start === null ) {
+			$start = $img->getTimestamp();
+		}
+		return $img->getOriginalTitle()->getText() . '|' . $start;
 	}
 
 	public function getAllowedParams() {