r79451 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r79450‎ | r79451 | r79452 >
Date:22:35, 1 January 2011
Author:reedy
Status:resolved (Comments)
Tags:
Comment:
Followup r75334. Change to int == string, rather than int === string (not going to work)

Attempt to load/save an article object, rather than an title object from the parser cache.

Per Bryan, and per bug 26534 now logged, this really needs refactoring more cleanly. As if latestRevId == oldid set, it's doing exactly the same lookup/parse
Modified paths:
  • /trunk/phase3/includes/api/ApiParse.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/api/ApiParse.php
@@ -94,18 +94,21 @@
9595 $wgTitle = $titleObj;
9696
9797 // If for some reason the "oldid" is actually the current revision, it may be cached
98 - if ( $titleObj->getLatestRevID() === $oldid ) {
 98+ if ( $titleObj->getLatestRevID() == $oldid ) {
9999 $p_result = false;
100100 $pcache = ParserCache::singleton();
 101+
 102+ $articleObj = new Article( $titleObj );
 103+
101104 if ( $wgEnableParserCache ) {
102 - $p_result = $pcache->get( $titleObj, $popts );
 105+ $p_result = $pcache->get( $articleObj, $popts );
103106 }
104107 if ( !$p_result ) {
105108 $text = $rev->getText( Revision::FOR_THIS_USER );
106109 $p_result = $wgParser->parse( $text, $titleObj, $popts );
107110
108111 if ( $wgEnableParserCache ) {
109 - $pcache->save( $p_result, $titleObj, $popts );
 112+ $pcache->save( $p_result, $articleObj, $popts );
110113 }
111114 }
112115 } else {
@@ -157,7 +160,9 @@
158161 }
159162
160163 if ( $this->section !== false ) {
161 - $text = $this->getSectionText( $text, !is_null ( $pageid ) ? 'page id ' . $pageid : $titleObj->getText() );
 164+ $text = $this->getSectionText( $text, !is_null ( $pageid )
 165+ ? 'page id ' . $pageid : $titleObj->getText() );
 166+
162167 $p_result = $wgParser->parse( $text, $titleObj, $popts );
163168 } else {
164169 // Try the parser cache first

Follow-up revisions

RevisionCommit summaryAuthorDate
r79452Per Nikerabbit on IRC, and Bryan on CR (r79451), change back for ===, and int...reedy22:42, 1 January 2011
r79773Adding some spaces, and some else block comments (due to length or ApiParse [...reedy23:19, 6 January 2011

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r75334* If a action=parse request provides an oldid that is actually the current re...reedy19:16, 24 October 2010

Comments

#Comment by Bryan (talk | contribs)   22:41, 1 January 2011

I think doing an (int)cast instead of relying on PHP's weak comparison between strings and ints is much clearer and much less ambiguous for reviewers.

Status & tagging log