Move <warnings> and <query-continue> to result's top and optimize
authorYuri Astrakhan <yuriastrakhan@gmail.com>
Tue, 19 Feb 2013 16:40:07 +0000 (11:40 -0500)
committerBrad Jorsch <bjorsch@wikimedia.org>
Tue, 19 Feb 2013 16:45:44 +0000 (11:45 -0500)
* Both the warnings and query-continue elements will now be shown
at the top of the result, making it easier for debugging and learning.
* Added int $flags parameter instead of bool $overwrite for addValue()
and setElement(). Supported flags are OVERRIDE - overrides existing value,
same meaning as true (which will continue to work), and ADD_ON_TOP which
ensures that if the value being added does not exist, it will be placed
as the first element in the parent array.
* Optimized ApiBase::setWarning to no longer use regex (faster)
* Optimized ApiResult::addValue() for a bit more efficiency
* Added ApiResult::addAsFirstElement() that inserts a named value into
the array at the top

Change-Id: I0fa2697e1eaa4947d01527bb3ad555e1051f6be4

RELEASE-NOTES-1.21
includes/api/ApiBase.php
includes/api/ApiQueryBase.php
includes/api/ApiResult.php

index 05dc4b7..184812c 100644 (file)
@@ -225,6 +225,7 @@ production.
 * BREAKING CHANGE: ApiQuery::newGenerator() and executeGeneratorModule() were deleted.
 * ApiQueryGeneratorBase::setGeneratorMode() now requires a pageset param.
 * $wgAPIGeneratorModules is now obsolete and will be ignored.
+* Added flags ApiResult::OVERRIDE and ADD_ON_TOP to setElement() and addValue()
 
 === Languages updated in 1.21 ===
 
index abb43e8..aff7a2e 100644 (file)
@@ -230,21 +230,27 @@ abstract class ApiBase extends ContextSource {
        public function setWarning( $warning ) {
                $result = $this->getResult();
                $data = $result->getData();
-               if ( isset( $data['warnings'][$this->getModuleName()] ) ) {
+               $moduleName = $this->getModuleName();
+               if ( isset( $data['warnings'][$moduleName] ) ) {
                        // Don't add duplicate warnings
-                       $warn_regex = preg_quote( $warning, '/' );
-                       if ( preg_match( "/{$warn_regex}(\\n|$)/", $data['warnings'][$this->getModuleName()]['*'] ) ) {
-                               return;
+                       $oldWarning = $data['warnings'][$moduleName]['*'];
+                       $warnPos = strpos( $oldWarning, $warning );
+                       // If $warning was found in $oldWarning, check if it starts at 0 or after "\n"
+                       if ( $warnPos !== false && ( $warnPos === 0 || $oldWarning[$warnPos - 1] === "\n" ) ) {
+                               // Check if $warning is followed by "\n" or the end of the $oldWarning
+                               $warnPos += strlen( $warning );
+                               if ( strlen( $oldWarning ) <= $warnPos || $oldWarning[$warnPos] === "\n" ) {
+                                       return;
+                               }
                        }
-                       $oldwarning = $data['warnings'][$this->getModuleName()]['*'];
                        // If there is a warning already, append it to the existing one
-                       $warning = "$oldwarning\n$warning";
-                       $result->unsetValue( 'warnings', $this->getModuleName() );
+                       $warning = "$oldWarning\n$warning";
                }
                $msg = array();
                ApiResult::setContent( $msg, $warning );
                $result->disableSizeCheck();
-               $result->addValue( 'warnings', $this->getModuleName(), $msg );
+               $result->addValue( 'warnings', $moduleName,
+                       $msg, ApiResult::OVERRIDE | ApiResult::ADD_ON_TOP );
                $result->enableSizeCheck();
        }
 
index 59e6652..20751e1 100644 (file)
@@ -370,7 +370,7 @@ abstract class ApiQueryBase extends ApiBase {
                $msg = array( $paramName => $paramValue );
                $result = $this->getResult();
                $result->disableSizeCheck();
-               $result->addValue( 'query-continue', $this->getModuleName(), $msg );
+               $result->addValue( 'query-continue', $this->getModuleName(), $msg, ApiResult::ADD_ON_TOP );
                $result->enableSizeCheck();
        }
 
index 5f752b3..7ecfa8e 100644 (file)
  */
 class ApiResult extends ApiBase {
 
+       /**
+        * override existing value in addValue() and setElement()
+        * @since 1.21
+        */
+       const OVERRIDE = 1;
+
+       /**
+        * For addValue() and setElement(), if the value does not exist, add it as the first element.
+        * In case the new value has no name (numerical index), all indexes will be renumbered.
+        * @since 1.21
+        */
+       const ADD_ON_TOP = 2;
+
        private $mData, $mIsRawMode, $mSize, $mCheckingSize;
 
        /**
@@ -137,15 +150,24 @@ class ApiResult extends ApiBase {
         * @param $arr array to add $value to
         * @param $name string Index of $arr to add $value at
         * @param $value mixed
-        * @param $overwrite bool Whether overwriting an existing element is allowed
+        * @param $flags int Zero or more OR-ed flags like OVERRIDE | ADD_ON_TOP. This parameter used to be
+        *        boolean, and the value of OVERRIDE=1 was specifically chosen so that it would be backwards
+        *        compatible with the new method signature.
+        *
+        * @since 1.21 int $flags replaced boolean $override
         */
-       public static function setElement( &$arr, $name, $value, $overwrite = false ) {
+       public static function setElement( &$arr, $name, $value, $flags = 0 ) {
                if ( $arr === null || $name === null || $value === null || !is_array( $arr ) || is_array( $name ) ) {
                        ApiBase::dieDebug( __METHOD__, 'Bad parameter' );
                }
 
-               if ( !isset ( $arr[$name] ) || $overwrite ) {
-                       $arr[$name] = $value;
+               $exists = isset( $arr[$name] );
+               if ( !$exists || ( $flags & ApiResult::OVERRIDE ) ) {
+                       if ( !$exists && ( $flags & ApiResult::ADD_ON_TOP ) ) {
+                               $arr = array( $name => $value ) + $arr;
+                       } else {
+                               $arr[$name] = $value;
+                       }
                } elseif ( is_array( $arr[$name] ) && is_array( $value ) ) {
                        $merged = array_intersect_key( $arr[$name], $value );
                        if ( !count( $merged ) ) {
@@ -249,11 +271,14 @@ class ApiResult extends ApiBase {
         * @param $path array|string|null
         * @param $name string
         * @param $value mixed
-        * @param $overwrite bool
-        *
+        * @param $flags int Zero or more OR-ed flags like OVERRIDE | ADD_ON_TOP. This parameter used to be
+        *        boolean, and the value of OVERRIDE=1 was specifically chosen so that it would be backwards
+        *        compatible with the new method signature.
         * @return bool True if $value fits in the result, false if not
+        *
+        * @since 1.21 int $flags replaced boolean $override
         */
-       public function addValue( $path, $name, $value, $overwrite = false ) {
+       public function addValue( $path, $name, $value, $flags = 0 ) {
                global $wgAPIMaxResultSize;
 
                $data = &$this->mData;
@@ -268,26 +293,34 @@ class ApiResult extends ApiBase {
                        $this->mSize = $newsize;
                }
 
-               if ( !is_null( $path ) ) {
-                       if ( is_array( $path ) ) {
-                               foreach ( $path as $p ) {
-                                       if ( !isset( $data[$p] ) ) {
+               $addOnTop = $flags & ApiResult::ADD_ON_TOP;
+               if ( $path !== null ) {
+                       foreach ( (array) $path as $p ) {
+                               if ( !isset( $data[$p] ) ) {
+                                       if ( $addOnTop ) {
+                                               $data = array( $p => array() ) + $data;
+                                               $addOnTop = false;
+                                       } else {
                                                $data[$p] = array();
                                        }
-                                       $data = &$data[$p];
                                }
-                       } else {
-                               if ( !isset( $data[$path] ) ) {
-                                       $data[$path] = array();
-                               }
-                               $data = &$data[$path];
+                               $data = &$data[$p];
                        }
                }
 
                if ( !$name ) {
-                       $data[] = $value; // Add list element
+                       // Add list element
+                       if ( $addOnTop ) {
+                               // This element needs to be inserted in the beginning
+                               // Numerical indexes will be renumbered
+                               array_unshift( $data, $value );
+                       } else {
+                               // Add new value at the end
+                               $data[] = $value;
+                       }
                } else {
-                       self::setElement( $data, $name, $value, $overwrite ); // Add named element
+                       // Add named element
+                       self::setElement( $data, $name, $value, $flags );
                }
                return true;
        }
@@ -300,7 +333,7 @@ class ApiResult extends ApiBase {
         */
        public function setParsedLimit( $moduleName, $limit ) {
                // Add value, allowing overwriting
-               $this->addValue( 'limits', $moduleName, $limit, true );
+               $this->addValue( 'limits', $moduleName, $limit, ApiResult::OVERRIDE );
        }
 
        /**