r82004 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r82003‎ | r82004 | r82005 >
Date:01:10, 12 February 2011
Author:brion
Status:ok (Comments)
Tags:
Comment:
* (bug 27333) Fix repetitive last-seen time queries on page history

Title::getNotificationTimestamp() is called for every line in HistoryPager, to compare against the revision's timestamp to determine if it's a new edit since the viewer's last visit to the page.
The function kept an internal cache within the Title object to avoid having to make the query over and over in this situation, but in the case that it returns null, the cached entry would be accidentally ignored on the next round due to use of isset() to check for the array entry. (Most of the time it's great, but if the value of the key actually *is* null, isset() returns false.) Switched to array_key_exists(), which doesn't have this false negative. Now only one query gets issued for this on a history page, even if the answer is null.
Modified paths:
  • /trunk/phase3/includes/Title.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/Title.php
@@ -3870,7 +3870,8 @@
38713871 }
38723872 // Check cache first
38733873 $uid = $user->getId();
3874 - if ( isset( $this->mNotificationTimestamp[$uid] ) ) {
 3874+ // avoid isset here, as it'll return false for null entries
 3875+ if ( array_key_exists( $uid, $this->mNotificationTimestamp ) ) {
38753876 return $this->mNotificationTimestamp[$uid];
38763877 }
38773878 if ( !$uid || !$wgShowUpdatedMarker ) {

Follow-up revisions

RevisionCommit summaryAuthorDate
r821511.17wmf1: MFT r78964, r79086, r79087, r79091, r82004, r82025, r82048, r82070,...catrope22:55, 14 February 2011
r85151MFT: r82000, r82004, r82020, r82025, r82038, r82039, r82048, r82070, r82081, ...demon20:39, 1 April 2011

Comments

#Comment by Brion VIBBER (talk | contribs)   01:18, 12 February 2011

As this decreases unnecessary queries, probably should slip it into 1.17.

Status & tagging log