Added ApiResult::NO_SIZE_CHECK flag for addValue()
authorYuri Astrakhan <yurik@wikimedia.org>
Mon, 7 Jul 2014 18:07:14 +0000 (14:07 -0400)
committerBrad Jorsch <bjorsch@wikimedia.org>
Fri, 11 Jul 2014 18:53:58 +0000 (14:53 -0400)
This way we no longer need to disable size checking just for one operation
(enable|disable)SizeCheck functions were depricated.

Overall, this is a much better practice than disabling than re-enabling
the flag, as it might lead to accidentally forgetting to re-enable it,
just like the issue with the dangling file handlers, etc.

Example:

disable, do some complex logic, re-enable.  And later, by accident,
the complex logic is changed to return a value half-way, or throws
an exception that gets handled as part of normal operations. This
results in the unsafe disabled state of the result object,
which is not good (tm).

Change-Id: I389a334d35f52f23a1847aca4aef5e96b262f589

RELEASE-NOTES-1.24
includes/api/ApiBase.php
includes/api/ApiFormatBase.php
includes/api/ApiMain.php
includes/api/ApiQuery.php
includes/api/ApiResult.php

index 0c91012..50ee949 100644 (file)
@@ -239,6 +239,7 @@ changes to languages because of Bugzilla reports.
 * Removed WikiPage::isBigDeletion(). (deprecated since 1.19)
 * Removed MWInit class which contained functions related to a now discontinued
   PHP compiler called hphpc. (deprecated since 1.22)
+* ApiResult::enableSizeCheck() and disableSizeCheck() are now obsolete.
 
 ==== Renamed classes ====
 * CLDRPluralRuleConverter_Expression to CLDRPluralRuleConverterExpression
index 4f4d051..7ebd0c3 100644 (file)
@@ -251,10 +251,8 @@ abstract class ApiBase extends ContextSource {
                }
                $msg = array();
                ApiResult::setContent( $msg, $warning );
-               $result->disableSizeCheck();
                $result->addValue( 'warnings', $moduleName,
-                       $msg, ApiResult::OVERRIDE | ApiResult::ADD_ON_TOP );
-               $result->enableSizeCheck();
+                       $msg, ApiResult::OVERRIDE | ApiResult::ADD_ON_TOP | ApiResult::NO_SIZE_CHECK );
        }
 
        /**
index 03a6843..848d129 100644 (file)
@@ -362,10 +362,8 @@ class ApiFormatFeedWrapper extends ApiFormatBase {
                // Disable size checking for this because we can't continue
                // cleanly; size checking would cause more problems than it'd
                // solve
-               $result->disableSizeCheck();
-               $result->addValue( null, '_feed', $feed );
-               $result->addValue( null, '_feeditems', $feedItems );
-               $result->enableSizeCheck();
+               $result->addValue( null, '_feed', $feed, ApiResult::NO_SIZE_CHECK );
+               $result->addValue( null, '_feeditems', $feedItems, ApiResult::NO_SIZE_CHECK );
        }
 
        /**
index 84db9ed..a5873e6 100644 (file)
@@ -699,21 +699,20 @@ class ApiMain extends ApiBase {
                $warnings = isset( $oldResult['warnings'] ) ? $oldResult['warnings'] : null;
 
                $result->reset();
-               $result->disableSizeCheck();
                // Re-add the id
                $requestid = $this->getParameter( 'requestid' );
                if ( !is_null( $requestid ) ) {
-                       $result->addValue( null, 'requestid', $requestid );
+                       $result->addValue( null, 'requestid', $requestid, ApiResult::NO_SIZE_CHECK );
                }
                if ( $config->get( 'ShowHostnames' ) ) {
                        // servedby is especially useful when debugging errors
-                       $result->addValue( null, 'servedby', wfHostName() );
+                       $result->addValue( null, 'servedby', wfHostName(), ApiResult::NO_SIZE_CHECK );
                }
                if ( $warnings !== null ) {
-                       $result->addValue( null, 'warnings', $warnings );
+                       $result->addValue( null, 'warnings', $warnings, ApiResult::NO_SIZE_CHECK );
                }
 
-               $result->addValue( null, 'error', $errMessage );
+               $result->addValue( null, 'error', $errMessage, ApiResult::NO_SIZE_CHECK );
 
                return $errMessage['code'];
        }
index cd6a840..f4b28fa 100644 (file)
@@ -485,18 +485,16 @@ class ApiQuery extends ApiBase {
                // Don't check the size of exported stuff
                // It's not continuable, so it would cause more
                // problems than it'd solve
-               $result->disableSizeCheck();
                if ( $this->mParams['exportnowrap'] ) {
                        $result->reset();
                        // Raw formatter will handle this
-                       $result->addValue( null, 'text', $exportxml );
-                       $result->addValue( null, 'mime', 'text/xml' );
+                       $result->addValue( null, 'text', $exportxml, ApiResult::NO_SIZE_CHECK );
+                       $result->addValue( null, 'mime', 'text/xml', ApiResult::NO_SIZE_CHECK );
                } else {
                        $r = array();
                        ApiResult::setContent( $r, $exportxml );
-                       $result->addValue( 'query', 'export', $r );
+                       $result->addValue( 'query', 'export', $r, ApiResult::NO_SIZE_CHECK );
                }
-               $result->enableSizeCheck();
        }
 
        public function getAllowedParams( $flags = 0 ) {
index b30d9dd..ac64cf0 100644 (file)
@@ -58,6 +58,14 @@ class ApiResult extends ApiBase {
         */
        const ADD_ON_TOP = 2;
 
+       /**
+        * For addValue() and setElement(), do not check size while adding a value
+        * Don't use this unless you REALLY know what you're doing.
+        * Values added while the size checking was disabled will never be counted
+        * @since 1.24
+        */
+       const NO_SIZE_CHECK = 4;
+
        private $mData, $mIsRawMode, $mSize, $mCheckingSize;
 
        private $continueAllModules = array();
@@ -143,6 +151,7 @@ class ApiResult extends ApiBase {
         * Disable size checking in addValue(). Don't use this unless you
         * REALLY know what you're doing. Values added while size checking
         * was disabled will not be counted (ever)
+        * @deprecated since 1.24, use ApiResult::NO_SIZE_CHECK
         */
        public function disableSizeCheck() {
                $this->mCheckingSize = false;
@@ -150,6 +159,7 @@ class ApiResult extends ApiBase {
 
        /**
         * Re-enable size checking in addValue()
+        * @deprecated since 1.24, use ApiResult::NO_SIZE_CHECK
         */
        public function enableSizeCheck() {
                $this->mCheckingSize = true;
@@ -312,17 +322,16 @@ class ApiResult extends ApiBase {
         * @param array|string|null $path
         * @param string $name
         * @param mixed $value
-        * @param int $flags 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.
+        * @param int $flags 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, $flags = 0 ) {
                $data = &$this->mData;
-               if ( $this->mCheckingSize ) {
+               if ( $this->mCheckingSize && !( $flags & ApiResult::NO_SIZE_CHECK ) ) {
                        $newsize = $this->mSize + self::size( $value );
                        $maxResultSize = $this->getConfig()->get( 'APIMaxResultSize' );
                        if ( $newsize > $maxResultSize ) {
@@ -616,9 +625,7 @@ class ApiResult extends ApiBase {
                        }
                }
                if ( $data ) {
-                       $this->disableSizeCheck();
-                       $this->addValue( null, $key, $data, ApiResult::ADD_ON_TOP );
-                       $this->enableSizeCheck();
+                       $this->addValue( null, $key, $data, ApiResult::ADD_ON_TOP | ApiResult::NO_SIZE_CHECK );
                }
        }
 }