From 43b1eb3f7241e7f34f618c0b168fd8dae698a0e0 Mon Sep 17 00:00:00 2001 From: Yuri Astrakhan Date: Mon, 7 Jul 2014 14:07:14 -0400 Subject: [PATCH] Added ApiResult::NO_SIZE_CHECK flag for addValue() 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 | 1 + includes/api/ApiBase.php | 4 +--- includes/api/ApiFormatBase.php | 6 ++---- includes/api/ApiMain.php | 9 ++++----- includes/api/ApiQuery.php | 8 +++----- includes/api/ApiResult.php | 23 +++++++++++++++-------- 6 files changed, 26 insertions(+), 25 deletions(-) diff --git a/RELEASE-NOTES-1.24 b/RELEASE-NOTES-1.24 index 0c91012f31..50ee9494e0 100644 --- a/RELEASE-NOTES-1.24 +++ b/RELEASE-NOTES-1.24 @@ -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 diff --git a/includes/api/ApiBase.php b/includes/api/ApiBase.php index 4f4d051a8c..7ebd0c38d1 100644 --- a/includes/api/ApiBase.php +++ b/includes/api/ApiBase.php @@ -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 ); } /** diff --git a/includes/api/ApiFormatBase.php b/includes/api/ApiFormatBase.php index 03a6843a99..848d129085 100644 --- a/includes/api/ApiFormatBase.php +++ b/includes/api/ApiFormatBase.php @@ -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 ); } /** diff --git a/includes/api/ApiMain.php b/includes/api/ApiMain.php index 84db9edc17..a5873e6ef9 100644 --- a/includes/api/ApiMain.php +++ b/includes/api/ApiMain.php @@ -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']; } diff --git a/includes/api/ApiQuery.php b/includes/api/ApiQuery.php index cd6a840237..f4b28fa50a 100644 --- a/includes/api/ApiQuery.php +++ b/includes/api/ApiQuery.php @@ -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 ) { diff --git a/includes/api/ApiResult.php b/includes/api/ApiResult.php index b30d9ddd75..ac64cf01e1 100644 --- a/includes/api/ApiResult.php +++ b/includes/api/ApiResult.php @@ -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 ); } } } -- 2.20.1