MediaWiki r46845 - Code Review

Jump to: navigation, search
Revision:r46844‎ | r46845 (on ViewVC)‎ | r46846 >
Date:14:30, 5 February 2009
Status:deferred (Comments)
* API: BREAKING CHANGE: (bug 11430) Return fewer results than the limit in some cases to prevent running out of memory
* This means queries could possibly return fewer results than the limit and still set a query-continue
* Add iicontinue, rvcontinue, cicontinue, incontinue, amfrom to faciliate query-continue for these modules
* Implemented by blocking additions to the ApiResult object if they would make it too large
** Important things like query-continue values and warnings are exempt from this check
** RSS feeds and exported XML are also exempted (size-checking them would be too messy)
** Result size is checked against $wgAPIMaxResultSize, which defaults to 8 MB

For those who really care, per-file details follow:

* Introduced ApiResult::$mSize which keeps track of the result size.
* Introduced ApiResult::size() which calculates an array's size
(which is the sum of the strlen()s of its elements).
* ApiResult::addValue() now checks that the result size stays below
$wgAPIMaxResultSize. If the item won't fit, it won't be added and addValue()
will return false. Callers should check the return value and set a
query-continue if it's false.
* Closed the back door that is ApiResult::getData(): callers can't manipulate
the data array directly anymore so they can't bypass the result size limit.
* Added ApiResult::setIndexedTagName_internal() which will call
setIndexedTagName() on an array already in the result. This is needed for the
'new' order of adding results, which means addValue()ing one result at a time
until you hit the limit or run out, then calling this function to set the tag
* Added ApiResult::disableSizeCheck() and enableSizeCheck() which disable and
enable size checking in addValue(). This is used for stuff like query-continue
elements and warnings which shouldn't count towards the result size.
* Added ApiResult::unsetValue() which removes an element from the result and
decreases $mSize.

* Like ApiResult::getData(), ApiBase::getResultData() no longer returns a
* Use ApiResult::disableSizeCheck() in ApiBase::setWarning()

* Added ApiQueryBase::addPageSubItem(), which adds page subitems one item
at a time.
* addPageSubItem() and addPageSubItems() now return whether the subitem
fit in the result.
* Use ApiResult::disableSizeCheck() in setContinueEnumParameter()

* Use ApiResult::disableSizeCheck() in ApiMain::substituteResultWithError()
* Use getParameter() rather than $mRequest to obtain requestid

* Added $wgAPIMaxResultSize, with a default value of 8 MB

* Added results one at a time, and set a query-continue if the result is full.

ApiQueryLangLinks.php and friends:
* Migrated from addPageSubItems() to addPageSubItem(). This eliminates the
need for $lastId.

ApiQueryAllLinks.php, ApiQueryWatchlist.php, ApiQueryAllimages.php, ApiQuerySearch.php:
* Renamed $data to something more appropriate ($pageids, $ids or $titles)

* Abuse siprop as a query-continue parameter and set it to all props that
couldn't be processed.

* Doesn't do continuations, because the result is supposed to be random.
* Be smart enough to not run the second query if the results of the first
didn't fit.

ApiQueryImageInfo.php, ApiQueryRevisions.php, ApiQueryCategoryInfo.php, ApiQueryInfo.php:
* Added continue parameter which basically skips the first so many items

* Throw the result in a big array first and addValue() that one element at a time if necessary
** This is necessary because the results aren't retrieved in order
* Introduced $this->pageMap to map namespace and title to page ID
* Rewritten extractRowInfo() and extractRedirRowInfo() a little
* Declared all private member variables explicitly

* Use a pagemap just like in Backlinks
* Introduce fake page IDs and keep track of them so we know where to add what
** This doesn't change the output format, because the fake page IDs start at 0 and are consecutive

* Add amfrom to facilitate query-continue

* Rewrite: put the getOtherUsersInfo() code in execute()
Modified paths:

Diff [purge]

Loading diff…

Follow-up revisions

Rev.Commit summaryAuthorDate
r46847RELEASE-NOTES for r46845catrope15:25, 5 February 2009
r46849Fix regression from r46845 which broke ApiResult::cleanUpUTF8() and caused an...catrope15:46, 5 February 2009
r47036Fix some more regressions from r46845 reported by Brad Jorsch on the mediawik...catrope13:28, 9 February 2009
r47048Fix up r47037, which was itself a fix-up of r46845. Change suggested by Brad ...catrope19:24, 9 February 2009
r47324API: Fix yet another regression from r46845 that completely broke list=users....catrope17:47, 16 February 2009
r47470API: (bug 17561) Recommit r44231 ("Added usprop=emailable to list=users"), wh...catrope22:31, 18 February 2009
r47514API: (bug 17563) Fix regression from r46845 that changed the output for list=...catrope21:24, 19 February 2009


#Comment by Catrope (talk | contribs)   17:52, 16 February 2009

Regression fixes in r46847, r46848, r46849, r47036, r47048, r47324

#Comment by Catrope (talk | contribs)   22:32, 18 February 2009

and r47470

Status & tagging log

  • 17:05, 4 January 2012 Johnduhart (talk | contribs) changed the tags for r46845 [removed: api]
  • 00:20, 14 September 2011 Meno25 (talk | contribs) changed the status of r46845 [removed: old added: deferred]
  • 14:32, 5 February 2009 Catrope (talk | contribs) changed the tags for r46845 [added: api]