r68448 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r68447‎ | r68448 | r68449 >
Date:11:47, 23 June 2010
Author:peter17
Status:resolved (Comments)
Tags:
Comment:
Adding 2 fields in the interwiki table and upgrading my code
Modified paths:
  • /branches/iwtransclusion/phase3/includes/Interwiki.php (modified) (history)
  • /branches/iwtransclusion/phase3/includes/Title.php (modified) (history)
  • /branches/iwtransclusion/phase3/includes/parser/Parser.php (modified) (history)
  • /branches/iwtransclusion/phase3/maintenance/tables.sql (modified) (history)

Diff [purge]

Index: branches/iwtransclusion/phase3/maintenance/tables.sql
@@ -1068,6 +1068,12 @@
10691069 -- insertion.
10701070 iw_url blob NOT NULL,
10711071
 1072+ -- The URL of the file api.php
 1073+ iw_api blob NOT NULL,
 1074+
 1075+ -- The name of the database (for a connection to be established with wfGetLB( 'dbname' ))
 1076+ iw_dbname varchar(32) NOT NULL,
 1077+
10721078 -- A boolean value indicating whether the wiki is in this project
10731079 -- (used, for example, to detect redirect loops)
10741080 iw_local bool NOT NULL,
Index: branches/iwtransclusion/phase3/includes/Interwiki.php
@@ -17,10 +17,11 @@
1818
1919 protected $mPrefix, $mURL, $mAPI, $mLocal, $mTrans;
2020
21 - public function __construct( $prefix = null, $url = '', $api = '', $local = 0, $trans = 0 ) {
 21+ public function __construct( $prefix = null, $url = '', $api = '', $dbname = '', $local = 0, $trans = 0 ) {
2222 $this->mPrefix = $prefix;
2323 $this->mURL = $url;
2424 $this->mAPI = $api;
 25+ $this->mDBname = $dbname;
2526 $this->mLocal = $local;
2627 $this->mTrans = $trans;
2728 }
@@ -169,10 +170,12 @@
170171 * @return Boolean: whether everything was there
171172 */
172173 protected static function loadFromArray( $mc ) {
173 - if( isset( $mc['iw_url'] ) && isset( $mc['iw_api'] ) && isset( $mc['iw_local'] ) && isset( $mc['iw_trans'] ) ) {
 174+ if( isset( $mc['iw_url'] ) && isset( $mc['iw_api'] ) && isset( $mc['iw_dbname'] )
 175+ && isset( $mc['iw_local'] ) && isset( $mc['iw_trans'] ) ) {
174176 $iw = new Interwiki();
175177 $iw->mURL = $mc['iw_url'];
176178 $iw->mAPI = $mc['iw_api'];
 179+ $iw->mDBname = $mc['iw_dbname'];
177180 $iw->mLocal = $mc['iw_local'];
178181 $iw->mTrans = $mc['iw_trans'];
179182 return $iw;
@@ -200,9 +203,16 @@
201204 * @return String: the URL
202205 */
203206 public function getAPI( ) {
204 - $url = $this->mAPI;
 207+ return $this->mAPI;
 208+ }
205209
206 - return $url;
 210+ /**
 211+ * Get the DB name for this wiki
 212+ *
 213+ * @return String: the DB name
 214+ */
 215+ public function getDBname( ) {
 216+ return $this->mDBname;
207217 }
208218
209219 /**
Index: branches/iwtransclusion/phase3/includes/parser/Parser.php
@@ -3055,7 +3055,7 @@
30563056 // $text = $this->interwikiTransclude( $title, 'render' );
30573057 // $isHTML = true;
30583058 //} else {
3059 - $text = $this->interwikiTransclude( $title, 'raw' );
 3059+ $text = $this->interwikiTransclude( $title );
30603060 # Preprocess it like a template
30613061 $text = $this->preprocessToDom( $text, self::PTD_FOR_INCLUSION );
30623062 $isChildObj = true;
@@ -3273,15 +3273,17 @@
32743274 * Transclude an interwiki link.
32753275 * TODO: separate in interwikiTranscludeFromDB & interwikiTranscludeFromAPI according to the iw type
32763276 */
3277 - function interwikiTransclude( $title, $action ) {
 3277+ function interwikiTransclude( $title ) {
32783278
32793279 global $wgEnableScaryTranscluding;
32803280
32813281 if ( !$wgEnableScaryTranscluding ) {
32823282 return wfMsg('scarytranscludedisabled');
32833283 }
 3284+
 3285+ $fullTitle = $title->getNsText().':'.$title->getText();
32843286
3285 - $url1 = $title->getTransAPI( )."?action=query&prop=revisions&titles=$titles&rvprop=content&format=json";
 3287+ $url1 = $title->getTransAPI( )."?action=query&prop=revisions&titles=$fullTitle&rvprop=content&format=json";
32863288
32873289 if ( strlen( $url1 ) > 255 ) {
32883290 return wfMsg( 'scarytranscludetoolong' );
@@ -3289,8 +3291,7 @@
32903292
32913293 $text = $this->fetchTemplateMaybeFromCache( $url1 );
32923294
3293 - $titles = $title->getNsText().':'.$title->getText();
3294 - $url2 = $title->getTransAPI( )."?action=parse&text={{".$titles."}}&prop=templates&format=json";
 3295+ $url2 = $title->getTransAPI( )."?action=parse&text={{".$fullTitle."}}&prop=templates&format=json";
32953296
32963297 $get = Http::get( $url2 );
32973298 $myArray = FormatJson::decode($get, true);
@@ -3313,7 +3314,7 @@
33143315
33153316 }
33163317
3317 - return "<h2>$titles</h2><pre>$text</pre> List of templates: <pre>".$listSubTemplates.'</pre>' . $list2;
 3318+ return "<h2>$fullTitle</h2><pre>$text</pre> List of templates: <pre>".$listSubTemplates.'</pre>' . $list2;
33183319 }
33193320
33203321
Index: branches/iwtransclusion/phase3/includes/Title.php
@@ -528,6 +528,19 @@
529529 }
530530
531531 /**
 532+ * Returns the DB name of the distant wiki
 533+ * which owns the object.
 534+ *
 535+ * @return \type{\string} the DB name
 536+ */
 537+ public function getTransDBname() {
 538+ if ( $this->mInterwiki == '' )
 539+ return false;
 540+
 541+ return Interwiki::fetch( $this->mInterwiki )->getDBname();
 542+ }
 543+
 544+ /**
532545 * Escape a text fragment, say from a link, for a URL
533546 *
534547 * @param $fragment string containing a URL or link fragment (after the "#")

Follow-up revisions

RevisionCommit summaryAuthorDate
r68449Merging with trunk r68448peter1712:18, 23 June 2010
r69542Add iw_api and iw_wikiid fields to the interwiki table, plus rudimentary supp...catrope11:55, 19 July 2010
r87104Merge of r86170, r68448, r69480reedy00:19, 29 April 2011
r92985Merge r87104, which itself is a merge of r68170, r68448, r69480reedy17:19, 24 July 2011

Comments

#Comment by Nikerabbit (talk | contribs)   12:23, 23 June 2010
+if ( $this->mInterwiki ==  )

Could you use something more explicit like === or explicit type conversion?

#Comment by Catrope (talk | contribs)   15:57, 17 July 2010

Title.php uses weak comparison for $mInterwiki all over the place, Peter says he just copied that. I see how it's bad but it's not a regression; someone should fix all of these functions in one go.

#Comment by Catrope (talk | contribs)   15:55, 17 July 2010
+  -- The name of the database (for a connection to be established with wfGetLB( 'dbname' ))
+  iw_dbname varchar(32) NOT NULL,

32 characters is maybe a bit short for this. There are currently no wiki IDs longer than 32 characters at WMF (the longest one is readerfeedback_labswikimedia with 28 characters) but there's no reason why other setups wouldn't have longer wiki IDs or why we wouldn't want to create one in the future. 64 sounds like a sane limit to me; 255 would be overkill.

Also, dbname should probably be renamed to wikiID or some such; the wiki ID is equal to the database name on prefix-less setups like WMF's but is formatted as dbname-prefix on setups using table prefixes. This means LBFactory_Multi is actually more flexible than was alluded to on wikitech-l (and more flexible than WMF needs it to be), but because of the poor documentation I doubt anyone but the original implementers knew about this. The possibility for adding the table prefix also worsens the effects of the 32-character limit mentioned above. This rename would be nice for consistency but is not absolutely required IMO.

Status & tagging log