r107771 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r107770‎ | r107771 | r107772 >
Date:12:44, 1 January 2012
Author:ialex
Status:reverted (Comments)
Tags:
Comment:
Follow-up r107769:
* do the same with getTouched(); removed its $db param since it's pretty useless now (and nothing is using it actually)
* also set $mIsNew in loadFromRow() when there's no DB row (i.e. the page doesn't exist)

Also forgot to say that I moved both functions near other related functions.
Modified paths:
  • /trunk/phase3/includes/Title.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/Title.php
@@ -64,6 +64,7 @@
6565 var $mArticleID = -1; // /< Article ID, fetched from the link cache on demand
6666 var $mLatestID = false; // /< ID of most recent revision
6767 var $mCounter = -1; // /< Number of times this page has been viewed (-1 means "not loaded")
 68+ private $mTouched; // /< Timestamp of the last time this page was touched
6869 private $mIsNew; // /< Whether this is a "new page" (i.e. it has only one revision)
6970 private $mEstimateRevisions; // /< Estimated number of revisions; null of not loaded
7071 var $mRestrictions = array(); // /< Array of groups allowed to edit this article
@@ -234,6 +235,7 @@
235236 array(
236237 'page_namespace', 'page_title', 'page_id',
237238 'page_len', 'page_is_redirect', 'page_latest',
 239+ 'page_counter', 'page_touched', 'page_is_new',
238240 ),
239241 array( 'page_id' => $ids ),
240242 __METHOD__
@@ -277,6 +279,8 @@
278280 $this->mLatestID = (int)$row->page_latest;
279281 if ( isset( $row->page_counter ) )
280282 $this->mCounter = (int)$row->page_counter;
 283+ if ( isset( $row->page_touched ) )
 284+ $this->mTouched = $row->page_touched;
281285 if ( isset( $row->page_is_new ) )
282286 $this->mIsNew = (bool)$row->page_is_new;
283287 } else { // page not found
@@ -285,6 +289,8 @@
286290 $this->mRedirect = false;
287291 $this->mLatestID = 0;
288292 $this->mCounter = 0;
 293+ $this->mTouched = '19700101000000';
 294+ $this->mIsNew = false;
289295 }
290296 }
291297
@@ -2787,6 +2793,28 @@
27882794 }
27892795
27902796 /**
 2797+ * Get the last touched timestamp
 2798+ *
 2799+ * @return String last-touched timestamp
 2800+ */
 2801+ public function getTouched() {
 2802+ if ( $this->mTouched == null ) {
 2803+ if ( $this->exists() ) {
 2804+ $dbr = wfGetDB( DB_SLAVE );
 2805+ $this->mTouched = $dbr->selectField( 'page',
 2806+ 'page_touched',
 2807+ array( 'page_id' => $this->getArticleID() ),
 2808+ __METHOD__
 2809+ );
 2810+ } else {
 2811+ $this->mTouched = '19700101000000';
 2812+ }
 2813+ }
 2814+
 2815+ return $this->mTouched;
 2816+ }
 2817+
 2818+ /**
27912819 * Check if this is a new page (i.e. it has only one revision)
27922820 *
27932821 * @return bool
@@ -4281,18 +4309,6 @@
42824310 }
42834311
42844312 /**
4285 - * Get the last touched timestamp
4286 - *
4287 - * @param $db DatabaseBase: optional db
4288 - * @return String last-touched timestamp
4289 - */
4290 - public function getTouched( $db = null ) {
4291 - $db = isset( $db ) ? $db : wfGetDB( DB_SLAVE );
4292 - $touched = $db->selectField( 'page', 'page_touched', $this->pageCond(), __METHOD__ );
4293 - return $touched;
4294 - }
4295 -
4296 - /**
42974313 * Get the timestamp when this page was updated since the user last saw it.
42984314 *
42994315 * @param $user User

Follow-up revisions

RevisionCommit summaryAuthorDate
r107934Per Aaron, fix for r107771: Title::getTouched() should return false on non-ex...ialex20:15, 3 January 2012
r107945Revert r107769, r107771, r107825, r107840, r107927, r107934...brion21:44, 3 January 2012

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r107769Turn Title::isNewPage() into something useful by caching its result and prelo...ialex12:23, 1 January 2012

Comments

#Comment by Duplicatebug (talk | contribs)   13:24, 2 January 2012

It is better, to select the page_counter field only when needed ($wgDisableCounters is not true) and to set that field only when allowed. Unfortunately Title::getCount does not respect that setting, too.

#Comment by IAlex (talk | contribs)   13:55, 2 January 2012

That function is not called if $wgDisableCounters is true.

#Comment by Duplicatebug (talk | contribs)   16:56, 2 January 2012

Yes, for core, but extensions can use this function and get a value,. It is in the most cases 0, but can also a higher value. Giving always zero is better, when this option is disabled.

#Comment by Aaron Schulz (talk | contribs)   19:51, 3 January 2012

getTouched() used to return false if there is no page, now it gives some epoch date. Are any callers expecting this?

#Comment by IAlex (talk | contribs)   20:16, 3 January 2012

Restored previous behaviour in r107934.

Status & tagging log