r32982 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r32981‎ | r32982 | r32983 >
Date:20:34, 8 April 2008
Author:aaron
Status:old
Tags:
Comment:
* Add redirect and size fields to title. Add accessors.
* Add newFromRow() function for Titles to optionally avoid some queries on accessor use
* Make linkbatch and some things that directly use linkcache select len/redirect fields along with the rest all in one query. This avoids query spam for link coloring.
Modified paths:
  • /trunk/phase3/includes/LinkBatch.php (modified) (history)
  • /trunk/phase3/includes/Linker.php (modified) (history)
  • /trunk/phase3/includes/Parser.php (modified) (history)
  • /trunk/phase3/includes/Parser_OldPP.php (modified) (history)
  • /trunk/phase3/includes/SpecialContributions.php (modified) (history)
  • /trunk/phase3/includes/Title.php (modified) (history)
  • /trunk/phase3/includes/WatchlistEditor.php (modified) (history)
  • /trunk/phase3/includes/filerepo/File.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/LinkBatch.php
——@@ -82,6 +82,8 @@
8383
8484 /**
8585 * Add a ResultWrapper containing IDs and titles to a LinkCache object
 86+ * Title are initialized here and they will go to the static title cache
 87+ * field of the Title class.
8688 */
8789 function addResultToCache( $cache, $res ) {
8890 if ( !$res ) {
——@@ -93,7 +95,7 @@
9496 $ids = array();
9597 $remaining = $this->data;
9698 while ( $row = $res->fetchObject() ) {
97 - $title = Title::makeTitle( $row->page_namespace, $row->page_title );
 99+ $title = Title::newFromRow( $row );
98100 $cache->addGoodLinkObj( $row->page_id, $title );
99101 $ids[$title->getPrefixedDBkey()] = $row->page_id;
100102 unset( $remaining[$row->page_namespace][$row->page_title] );
——@@ -128,7 +130,7 @@
129131 wfProfileOut( __METHOD__ );
130132 return false;
131133 }
132 - $sql = "SELECT page_id, page_namespace, page_title FROM $page WHERE $set";
 134+ $sql = "SELECT page_id, page_namespace, page_title, page_len, page_is_redirect FROM $page WHERE $set";
133135
134136 // Do query
135137 $res = new ResultWrapper( $dbr, $dbr->query( $sql, __METHOD__ ) );
Index: trunk/phase3/includes/Linker.php
——@@ -79,20 +79,16 @@
8080 /**
8181 * Return the CSS colour of a known link
8282 *
83 - * @param mixed $s
 83+ * @param Title $t
8484 * @param integer $threshold user defined threshold
8585 * @return string CSS class
8686 */
87 - function getLinkColour( $s, $threshold ) {
88 - if( $s === false ) {
89 - return '';
90 - }
91 -
 87+ function getLinkColour( $t, $threshold ) {
9288 $colour = '';
93 - if ( !empty( $s->page_is_redirect ) ) {
 89+ if ( $t->isRedirect() ) {
9490 # Page is a redirect
9591 $colour = 'mw-redirect';
96 - } elseif ( $threshold > 0 && $s->page_len < $threshold && MWNamespace::isContent( $s->page_namespace ) ) {
 92+ } elseif ( $threshold > 0 && $t->getLength() < $threshold && MWNamespace::isContent( $t->getNamespace() ) ) {
9793 # Page is a stub
9894 $colour = 'stub';
9995 }
——@@ -256,15 +252,8 @@
257253 } else {
258254 $colour = '';
259255 if ( $nt->isContentPage() ) {
260 - # FIXME: This is stupid, we should combine this query with
261 - # the Title::getArticleID() query above.
262256 $threshold = $wgUser->getOption('stubthreshold');
263 - $dbr = wfGetDB( DB_SLAVE );
264 - $s = $dbr->selectRow(
265 - array( 'page' ),
266 - array( 'page_len', 'page_is_redirect', 'page_namespace' ),
267 - array( 'page_id' => $aid ), __METHOD__ ) ;
268 - $colour = $this->getLinkColour( $s, $threshold );
 257+ $colour = $this->getLinkColour( $nt, $threshold );
269258 }
270259 $retVal = $this->makeColouredLinkObj( $nt, $colour, $text, $query, $trail, $prefix );
271260 }
Index: trunk/phase3/includes/Parser.php
——@@ -4003,10 +4003,7 @@
40044004 # Not in the link cache, add it to the query
40054005 if ( !isset( $current ) ) {
40064006 $current = $ns;
4007 - $query = "SELECT page_id, page_namespace, page_title, page_is_redirect";
4008 - if ( $threshold > 0 ) {
4009 - $query .= ', page_len';
4010 - }
 4007+ $query = "SELECT page_id, page_namespace, page_title, page_is_redirect, page_len";
40114008 $query .= " FROM $page WHERE (page_namespace=$ns AND page_title IN(";
40124009 } elseif ( $current != $ns ) {
40134010 $current = $ns;
——@@ -4029,7 +4026,7 @@
40304027 # Fetch data and form into an associative array
40314028 # non-existent = broken
40324029 while ( $s = $dbr->fetchObject($res) ) {
4033 - $title = Title::makeTitle( $s->page_namespace, $s->page_title );
 4030+ $title = Title::newFromRow( $s );
40344031 $pdbk = $title->getPrefixedDBkey();
40354032 $linkCache->addGoodLinkObj( $s->page_id, $title );
40364033 $this->mOutput->addLink( $title, $s->page_id );
——@@ -4094,10 +4091,7 @@
40954092 // construct query
40964093 $titleClause = $linkBatch->constructSet('page', $dbr);
40974094
4098 - $variantQuery = "SELECT page_id, page_namespace, page_title, page_is_redirect";
4099 - if ( $threshold > 0 ) {
4100 - $variantQuery .= ', page_len';
4101 - }
 4095+ $variantQuery = "SELECT page_id, page_namespace, page_title, page_is_redirect, page_len";
41024096
41034097 $variantQuery .= " FROM $page WHERE $titleClause";
41044098 if ( $options & RLH_FOR_UPDATE ) {
——@@ -4109,7 +4103,7 @@
41104104 // for each found variants, figure out link holders and replace
41114105 while ( $s = $dbr->fetchObject($varRes) ) {
41124106
4113 - $variantTitle = Title::makeTitle( $s->page_namespace, $s->page_title );
 4107+ $variantTitle = Title::newFromRow( $s );
41144108 $varPdbk = $variantTitle->getPrefixedDBkey();
41154109 $vardbk = $variantTitle->getDBkey();
41164110
Index: trunk/phase3/includes/SpecialContributions.php
——@@ -11,7 +11,7 @@
1212
1313 function __construct( $target, $namespace = false, $year = false, $month = false ) {
1414 parent::__construct();
15 - foreach( explode( ' ', 'uctop diff newarticle rollbacklink diff hist minoreditletter' ) as $msg ) {
 15+ foreach( explode( ' ', 'uctop diff newarticle rollbacklink diff hist newpageletter minoreditletter' ) as $msg ) {
1616 $this->messages[$msg] = wfMsgExt( $msg, array( 'escape') );
1717 }
1818 $this->target = $target;
——@@ -42,7 +42,7 @@
4343 'fields' => array(
4444 'page_namespace', 'page_title', 'page_is_new', 'page_latest', 'rev_id', 'rev_page',
4545 'rev_text_id', 'rev_timestamp', 'rev_comment', 'rev_minor_edit', 'rev_user',
46 - 'rev_user_text', 'rev_deleted'
 46+ 'rev_user_text', 'rev_parent_id', 'rev_deleted'
4747 ),
4848 'conds' => $conds,
4949 'options' => array( 'USE INDEX' => $index )
——@@ -165,6 +165,12 @@
166166 if( $rev->isDeleted( Revision::DELETED_TEXT ) ) {
167167 $d = '<span class="history-deleted">' . $d . '</span>';
168168 }
 169+
 170+ if( $rev->getParentId() === 0 ) {
 171+ $nflag = '<span class="newpage">' . $this->messages['newpageletter'] . '</span>';
 172+ } else {
 173+ $nflag = '';
 174+ }
169175
170176 if( $row->rev_minor_edit ) {
171177 $mflag = '<span class="minor">' . $this->messages['minoreditletter'] . '</span> ';
——@@ -172,7 +178,7 @@
173179 $mflag = '';
174180 }
175181
176 - $ret = "{$d} {$histlink} {$difftext} {$mflag} {$link}{$userlink}{$comment} {$topmarktext}";
 182+ $ret = "{$d} {$histlink} {$difftext} {$nflag}{$mflag} {$link}{$userlink}{$comment} {$topmarktext}";
177183 if( $rev->isDeleted( Revision::DELETED_TEXT ) ) {
178184 $ret .= ' ' . wfMsgHtml( 'deletedrev' );
179185 }
Index: trunk/phase3/includes/WatchlistEditor.php
——@@ -205,14 +205,15 @@
206206 $dbr = wfGetDB( DB_MASTER );
207207 $uid = intval( $user->getId() );
208208 list( $watchlist, $page ) = $dbr->tableNamesN( 'watchlist', 'page' );
209 - $sql = "SELECT wl_namespace, wl_title, page_id, page_is_redirect
 209+ $sql = "SELECT wl_namespace, wl_title, page_id, page_len, page_is_redirect,
 210+ page_namespace, page_title
210211 FROM {$watchlist} LEFT JOIN {$page} ON ( wl_namespace = page_namespace
211212 AND wl_title = page_title ) WHERE wl_user = {$uid}";
212213 $res = $dbr->query( $sql, __METHOD__ );
213214 if( $res && $dbr->numRows( $res ) > 0 ) {
214215 $cache = LinkCache::singleton();
215216 while( $row = $dbr->fetchObject( $res ) ) {
216 - $title = Title::makeTitleSafe( $row->wl_namespace, $row->wl_title );
 217+ $title = Title::newFromRow( $row );
217218 if( $title instanceof Title ) {
218219 // Update the link cache while we're at it
219220 if( $row->page_id ) {
Index: trunk/phase3/includes/filerepo/File.php
——@@ -844,14 +844,15 @@
845845
846846 list( $page, $imagelinks ) = $db->tableNamesN( 'page', 'imagelinks' );
847847 $encName = $db->addQuotes( $this->getName() );
848 - $sql = "SELECT page_namespace,page_title,page_id FROM $page,$imagelinks WHERE page_id=il_from AND il_to=$encName $options";
 848+ $sql = "SELECT page_namespace,page_title,page_id,page_len,page_is_redirect,
 849+ FROM $page,$imagelinks WHERE page_id=il_from AND il_to=$encName $options";
849850 $res = $db->query( $sql, __METHOD__ );
850851
851852 $retVal = array();
852853 if ( $db->numRows( $res ) ) {
853854 while ( $row = $db->fetchObject( $res ) ) {
854 - if ( $titleObj = Title::makeTitle( $row->page_namespace, $row->page_title ) ) {
855 - $linkCache->addGoodLinkObj( $row->page_id, $titleObj );
 855+ if ( $titleObj = Title::newFromRow( $row ) ) {
 856+ $linkCache->addGoodLinkObj( $row->page_id, $titleObj, $row->page_len, $row->page_is_redirect );
856857 $retVal[] = $titleObj;
857858 }
858859 }
Index: trunk/phase3/includes/Parser_OldPP.php
——@@ -4120,9 +4120,7 @@
41214121 # Not in the link cache, add it to the query
41224122 if ( !isset( $current ) ) {
41234123 $current = $ns;
4124 - $query = "SELECT page_id, page_namespace, page_title";
4125 - if ( $threshold > 0 ) {
4126 - $query .= ', page_len, page_is_redirect';
 4124+ $query = "SELECT page_id, page_namespace, page_title, page_len, page_is_redirect";
41274125 }
41284126 $query .= " FROM $page WHERE (page_namespace=$ns AND page_title IN(";
41294127 } elseif ( $current != $ns ) {
——@@ -4148,7 +4146,7 @@
41494147 # 1 = known
41504148 # 2 = stub
41514149 while ( $s = $dbr->fetchObject($res) ) {
4152 - $title = Title::makeTitle( $s->page_namespace, $s->page_title );
 4150+ $title = Title::newFromRow( $s );
41534151 $pdbk = $title->getPrefixedDBkey();
41544152 $linkCache->addGoodLinkObj( $s->page_id, $title );
41554153 $this->mOutput->addLink( $title, $s->page_id );
——@@ -4163,7 +4161,7 @@
41644162 wfProfileOut( $fname.'-check' );
41654163
41664164 # Do a second query for different language variants of links and categories
4167 - if($wgContLang->hasVariants()){
 4165+ if( $wgContLang->hasVariants() ){
41684166 $linkBatch = new LinkBatch();
41694167 $variantMap = array(); // maps $pdbkey_Variant => $keys (of link holders)
41704168 $categoryMap = array(); // maps $category_variant => $category (dbkeys)
——@@ -4210,13 +4208,11 @@
42114209 }
42124210
42134211
4214 - if(!$linkBatch->isEmpty()){
 4212+ if ( !$linkBatch->isEmpty() ){
42154213 // construct query
42164214 $titleClause = $linkBatch->constructSet('page', $dbr);
42174215
4218 - $variantQuery = "SELECT page_id, page_namespace, page_title";
4219 - if ( $threshold > 0 ) {
4220 - $variantQuery .= ', page_len, page_is_redirect';
 4216+ $variantQuery = "SELECT page_id, page_namespace, page_title, page_len, page_is_redirect";
42214217 }
42224218
42234219 $variantQuery .= " FROM $page WHERE $titleClause";
——@@ -4229,7 +4225,7 @@
42304226 // for each found variants, figure out link holders and replace
42314227 while ( $s = $dbr->fetchObject($varRes) ) {
42324228
4233 - $variantTitle = Title::makeTitle( $s->page_namespace, $s->page_title );
 4229+ $variantTitle = Title::newFromRow( $s );
42344230 $varPdbk = $variantTitle->getPrefixedDBkey();
42354231 $vardbk = $variantTitle->getDBkey();
42364232
Index: trunk/phase3/includes/Title.php
——@@ -63,6 +63,8 @@
6464 var $mDefaultNamespace; # Namespace index when there is no namespace
6565 # Zero except in {{transclusion}} tags
6666 var $mWatched; # Is $wgUser watching this page? NULL if unfilled, accessed through userIsWatching()
 67+ var $mLength; # The page length, 0 for special pages
 68+ var $mRedirect; # Is the article at this title a redirect?
6769 /**#@-*/
6870
6971
——@@ -83,6 +85,8 @@
8486 $this->mWatched = NULL;
8587 $this->mLatestID = false;
8688 $this->mOldRestrictions = false;
 89+ $this->mLength = -1;
 90+ $this->mRedirect = null;
8791 }
8892
8993 /**
——@@ -220,6 +224,20 @@
221225 }
222226 return $titles;
223227 }
 228+
 229+ /**
 230+ * Make a Title object from a DB row
 231+ * @param Row $row (needs at least page_title,page_namespace)
 232+ */
 233+ public static function newFromRow( $row ) {
 234+ $t = self::makeTitle( $row->page_namespace, $row->page_title );
 235+ $t->mArticleID = isset($row->page_id) ? $row->page_id : -1;
 236+ $t->mLength = isset($row->page_len) ? $row->page_len : -1;
 237+ $t->mRedirect = isset($row->page_is_redirect) ? $row->page_is_redirect : -1;
 238+ $t->mLatest = isset($row->page_latest) ? $row->page_latest : 0;
 239+
 240+ return $t;
 241+ }
224242
225243 /**
226244 * Create a new Title from a namespace index and a DB key.
——@@ -1463,8 +1481,49 @@
14641482 public function isTalkPage() {
14651483 return MWNamespace::isTalk( $this->getNamespace() );
14661484 }
 1485+
 1486+ /**
 1487+ * Is this an article that is a redirect page?
 1488+ * @return bool
 1489+ */
 1490+ public function isRedirect() {
 1491+ if( $this->mRedirect !== null )
 1492+ return $this->mRedirect;
 1493+ # Zero for special pages
 1494+ if( $this->mArticleID <= 0 )
 1495+ return 0;
14671496
 1497+ $dbr = wfGetDB( DB_SLAVE );
 1498+ $redir = $dbr->selectField( 'page', 'page_is_redirect',
 1499+ array( 'page_id' => $this->mArticleID ),
 1500+ __METHOD__ );
 1501+
 1502+ $this->mRedirect = $redir ? true : false;
 1503+
 1504+ return $this->mRedirect;
 1505+ }
 1506+
14681507 /**
 1508+ * What is the length of this page (-1 for special pages)?
 1509+ * @return bool
 1510+ */
 1511+ public function getLength() {
 1512+ if( $this->mLength != -1 || $this->mArticleID == 0 )
 1513+ return $this->mLength;
 1514+ # Zero for special pages
 1515+ if( $this->mArticleID <= 0 )
 1516+ return -1;
 1517+
 1518+ $dbr = wfGetDB( DB_SLAVE );
 1519+ $len = $dbr->selectField( 'page', 'page_len',
 1520+ array( 'page_id' => $this->mArticleID ),
 1521+ __METHOD__ );
 1522+ $this->mLength = intval($len);
 1523+
 1524+ return $this->mLength;
 1525+ }
 1526+
 1527+ /**
14691528 * Is this a subpage?
14701529 * @return bool
14711530 */
——@@ -2173,7 +2232,7 @@
21742233 }
21752234
21762235 $res = $db->select( array( 'page', $table ),
2177 - array( 'page_namespace', 'page_title', 'page_id' ),
 2236+ array( 'page_namespace', 'page_title', 'page_id', 'page_len', 'page_is_redirect' ),
21782237 array(
21792238 "{$prefix}_from=page_id",
21802239 "{$prefix}_namespace" => $this->getNamespace(),
——@@ -2185,7 +2244,7 @@
21862245 if ( $db->numRows( $res ) ) {
21872246 while ( $row = $db->fetchObject( $res ) ) {
21882247 if ( $titleObj = Title::makeTitle( $row->page_namespace, $row->page_title ) ) {
2189 - $linkCache->addGoodLinkObj( $row->page_id, $titleObj );
 2248+ $linkCache->addGoodLinkObj( $row->page_id, $titleObj, $row->page_len, $row->page_is_redir );
21902249 $retVal[] = $titleObj;
21912250 }
21922251 }

Follow-up revisions

RevisionCommit summaryAuthorDate
r33019Partial revert of r32982; old Title method is fine hereaaron12:15, 9 April 2008

Status & tagging log