r70576 MediaWiki - Code Review archive

Repository:	MediaWiki
Revision:	< r70575‎ \| r70576 \| r70577 >
Date:	16:01, 6 August 2010
Author:	peter17
Status:	resolved (Comments)
Tags:
Comment:	This will register the links to distant templates in the globaltemplatelinks table and keep the table up to date when the pages of the calling wiki are edited, moved or deleted; this will not invalidate the local cache when the distant templates are edited/moved/deleted (TODO)
Modified paths:	/branches/iwtransclusion/phase3/includes/Article.php (modified) (history) /branches/iwtransclusion/phase3/includes/LinksUpdate.php (modified) (history) /branches/iwtransclusion/phase3/includes/Title.php (modified) (history) /branches/iwtransclusion/phase3/includes/db/Database.php (modified) (history) /branches/iwtransclusion/phase3/includes/parser/Parser.php (modified) (history) /branches/iwtransclusion/phase3/includes/parser/ParserOutput.php (modified) (history)

Diff [purge]

Index: branches/iwtransclusion/phase3/includes/Article.php
—	—	@@ -3172,7 +3172,7 @@
3173	3173	* @return boolean true if successful
3174	3174	*/
3175	3175	public function doDeleteArticle( $reason, $suppress = false, $id = 0, $commit = true ) {
3176		~~- global $wgDeferredUpdateList, $wgUseTrackbacks;~~
	3176	+ global $wgDeferredUpdateList, $wgUseTrackbacks, $wgGlobalDB, $wgWikiID;
3177	3177
3178	3178	wfDebug( __METHOD__ . "\n" );
3179	3179
—	—	@@ -3270,6 +3270,14 @@
3271	3271	$dbw->delete( 'externallinks', array( 'el_from' => $id ) );
3272	3272	$dbw->delete( 'langlinks', array( 'll_from' => $id ) );
3273	3273	$dbw->delete( 'redirect', array( 'rd_from' => $id ) );
	3274	+
	3275	+ if ( $wgGlobalDB ) {
	3276	+ $dbw2 = wfGetDB( DB_MASTER, array(), $wgGlobalDB );
	3277	+ $dbw2->delete( 'globaltemplatelinks',
	3278	+ array( 'gtl_from_wiki' => $wgWikiID,
	3279	+ 'gtl_from_page' => $id )
	3280	+ );
	3281	+ }
3274	3282	}
3275	3283
3276	3284	# If using cleanup triggers, we can skip some manual deletes
Index: branches/iwtransclusion/phase3/includes/parser/Parser.php
—	—	@@ -3174,8 +3174,9 @@
3175	3175	}
3176	3176	} elseif ( $title->isTrans() ) {
3177	3177	// TODO: Work by Peter17 in progress
3178		-
	3178	+
3179	3179	$text = Interwiki::interwikiTransclude( $title );
	3180	+ $this->registerDistantTemplate( $title );
3180	3181
3181	3182	if ( $text !== false ) {
3182	3183	# Preprocess it like a template
—	—	@@ -3323,6 +3324,21 @@
3324	3325	}
3325	3326	return array( $text, $finalTitle );
3326	3327	}
	3328	+
	3329	+ /**
	3330	+ * Register a distant template as used
	3331	+ */
	3332	+ function registerDistantTemplate( $title ) {
	3333	+ $templateCb = array( 'Parser', 'distantTemplateCallback' );
	3334	+ $stuff = call_user_func( $templateCb, $title, $this );
	3335	+ $text = $stuff['text'];
	3336	+ $finalTitle = isset( $stuff['finalTitle'] ) ? $stuff['finalTitle'] : $title;
	3337	+ if ( isset( $stuff['deps'] ) ) {
	3338	+ foreach ( $stuff['deps'] as $dep ) {
	3339	+ $this->mOutput->addDistantTemplate( $dep['title'], $dep['page_id'], $dep['rev_id'] );
	3340	+ }
	3341	+ }
	3342	+ }
3327	3343
3328	3344	function fetchTemplate( $title ) {
3329	3345	$rv = $this->fetchTemplateAndTitle( $title );
—	—	@@ -3391,7 +3407,22 @@
3392	3408	'deps' => $deps );
3393	3409	}
3394	3410
	3411	+ static function distantTemplateCallback( $title, $parser=false ) {
	3412	+ $text = '';
	3413	+ $rev_id = null;
	3414	+ $deps[] = array(
	3415	+ 'title' => $title,
	3416	+ 'page_id' => $title->getArticleID(),
	3417	+ 'rev_id' => $rev_id );
3395	3418
	3419	+ $finalTitle = $title;
	3420	+
	3421	+ return array(
	3422	+ 'text' => $text,
	3423	+ 'finalTitle' => $finalTitle,
	3424	+ 'deps' => $deps );
	3425	+ }
	3426	+
3396	3427	/**
3397	3428	* Triple brace replacement -- used for template arguments
3398	3429	* @private
Index: branches/iwtransclusion/phase3/includes/parser/ParserOutput.php
—	—	@@ -16,6 +16,8 @@
17	17	$mLinks = array(), # 2-D map of NS/DBK to ID for the links in the document. ID=zero for broken.
18	18	$mTemplates = array(), # 2-D map of NS/DBK to ID for the template references. ID=zero for broken.
19	19	$mTemplateIds = array(), # 2-D map of NS/DBK to rev ID for the template references. ID=zero for broken.
	20	+ $mDistantTemplates = array(), # 3-D map of WIKIID/NS/DBK to ID for the template references. ID=zero for broken.
	21	+ $mDistantTemplateIds = array(), # 3-D map of WIKIID/NS/DBK to rev ID for the template references. ID=zero for broken.
20	22	$mImages = array(), # DB keys of the images used, in the array key only
21	23	$mExternalLinks = array(), # External link URLs, in the key only
22	24	$mInterwikiLinks = array(), # 2-D map of prefix/DBK (in keys only) for the inline interwiki links in the document.
—	—	@@ -50,6 +52,8 @@
51	53	function getSections() { return $this->mSections; }
52	54	function &getLinks() { return $this->mLinks; }
53	55	function &getTemplates() { return $this->mTemplates; }
	56	+ function &getDistantTemplates() { return $this->mDistantTemplates; }
	57	+ function &getDistantTemplateIds() { return $this->mDistantTemplateIds; }
54	58	function &getImages() { return $this->mImages; }
55	59	function &getExternalLinks() { return $this->mExternalLinks; }
56	60	function getNoGallery() { return $this->mNoGallery; }
—	—	@@ -204,6 +208,25 @@
205	209	$this->mTemplateIds[$ns][$dbk] = $rev_id; // For versioning
206	210	}
207	211
	212	+ function addDistantTemplate( $title, $page_id, $rev_id ) {
	213	+ $wikiid = $title->getTransWikiID();
	214	+ if ( $wikiid !=='' ) {
	215	+ $ns = $title->getNamespace();
	216	+ $dbk = $title->getDBkey();
	217	+ if ( !isset( $this->mDistantTemplates[$wikiid] ) ) {
	218	+ $this->mDistantTemplates[$wikiid] = array();
	219	+ }
	220	+ if ( !isset( $this->mDistantTemplates[$wikiid][$ns] ) ) {
	221	+ $this->mDistantTemplates[$wikiid][$ns] = array();
	222	+ }
	223	+ $this->mDistantTemplates[$wikiid][$ns][$dbk] = $page_id;
	224	+ if ( !isset( $this->mDistantTemplateIds[$wikiid][$ns] ) ) {
	225	+ $this->mDistantTemplateIds[$wikiid][$ns] = array();
	226	+ }
	227	+ $this->mDistantTemplateIds[$wikiid][$ns][$dbk] = $rev_id; // For versioning
	228	+ }
	229	+ }
	230	+
208	231	/**
209	232	* @param $title Title object, must be an interwiki link
210	233	* @throws MWException if given invalid input
Index: branches/iwtransclusion/phase3/includes/db/Database.php
—	—	@@ -1281,7 +1281,37 @@
1282	1282	return false;
1283	1283	}
1284	1284	}
	1285	+
	1286	+ /**
	1287	+ * Build a partial where clause from a 3-d array
	1288	+ * The keys on each level may be either integers or strings.
	1289	+ *
	1290	+ * @param $data Array: organized as 3-d array(baseKeyVal => array(middleKeyVal => array(subKeyVal => <ignored>, ...), ...), ...)
	1291	+ * @param $baseKey String: field name to match the base-level keys to (eg 'gtl_to_wikiid')
	1292	+ * @param $middleKey String: field name to match the middle-level keys to (eg 'gtl_to_namespace')
	1293	+ * @param $subKey String: field name to match the sub-level keys to (eg 'gtl_to_title')
	1294	+ * @return Mixed: string SQL fragment, or false if no items in array.
	1295	+ */
	1296	+ function makeWhereFrom3d( $data, $baseKey, $middleKey, $subKey ) {
	1297	+ $conds = array();
	1298	+ foreach ( $data as $base => $subdata ) {
	1299	+ foreach ( $subdata as $middle => $sub ) {
	1300	+ if ( count( $sub ) ) {
	1301	+ $conds[] = $this->makeList(
	1302	+ array( $baseKey => $base, $middleKey => $middle, $subKey => array_keys( $sub ) ),
	1303	+ LIST_AND);
	1304	+ }
	1305	+ }
	1306	+ }
1285	1307
	1308	+ if ( $conds ) {
	1309	+ return $this->makeList( $conds, LIST_OR );
	1310	+ } else {
	1311	+ // Nothing to search for...
	1312	+ return false;
	1313	+ }
	1314	+ }
	1315	+
1286	1316	/**
1287	1317	* Bitwise operations
1288	1318	*/
Index: branches/iwtransclusion/phase3/includes/LinksUpdate.php
—	—	@@ -14,6 +14,7 @@
15	15	$mLinks, //!< Map of title strings to IDs for the links in the document
16	16	$mImages, //!< DB keys of the images used, in the array key only
17	17	$mTemplates, //!< Map of title strings to IDs for the template references, including broken ones
	18	+ $mDistantTemplates,//!< Map of title strings to IDs for the distant template references, including broken ones
18	19	$mExternals, //!< URLs of external links, array key only
19	20	$mCategories, //!< Map of category names to sort keys
20	21	$mInterlangs, //!< Map of language codes to titles
—	—	@@ -51,6 +52,7 @@
52	53	$this->mLinks = $parserOutput->getLinks();
53	54	$this->mImages = $parserOutput->getImages();
54	55	$this->mTemplates = $parserOutput->getTemplates();
	56	+ $this->mDistantTemplates = $parserOutput->getDistantTemplates();
55	57	$this->mExternals = $parserOutput->getExternalLinks();
56	58	$this->mCategories = $parserOutput->getCategories();
57	59	$this->mProperties = $parserOutput->getProperties();
—	—	@@ -124,6 +126,15 @@
125	127	$existing = $this->getExistingTemplates();
126	128	$this->incrTableUpdate( 'templatelinks', 'tl', $this->getTemplateDeletions( $existing ),
127	129	$this->getTemplateInsertions( $existing ) );
	130	+
	131	+ # Distant template links
	132	+ global $wgGlobalDB;
	133	+ if ( $wgGlobalDB ) {
	134	+ $existing = $this->getDistantExistingTemplates();
	135	+ $this->incrSharedTableUpdate( 'globaltemplatelinks', 'gtl',
	136	+ $this->getDistantTemplateDeletions( $existing ),
	137	+ $this->getDistantTemplateInsertions( $existing ) );
	138	+ }
128	139
129	140	# Category links
130	141	$existing = $this->getExistingCategories();
—	—	@@ -341,7 +352,39 @@
342	353	$this->mDb->insert( $table, $insertions, __METHOD__, 'IGNORE' );
343	354	}
344	355	}
	356	+
	357	+ /**
	358	+ * Update a shared table by doing a delete query then an insert query
	359	+ * @private
	360	+ */
	361	+ function incrSharedTableUpdate( $table, $prefix, $deletions, $insertions ) {
345	362
	363	+ global $wgWikiID;
	364	+ global $wgGlobalDB;
	365	+
	366	+ if ( $wgGlobalDB ) {
	367	+ $dbw = wfGetDB( DB_MASTER, array(), $wgGlobalDB );
	368	+ $where = array( "{$prefix}_from_wiki" => $wgWikiID,
	369	+ "{$prefix}_from_page" => $this->mId
	370	+ );
	371	+ $baseKey = "{$prefix}_to_wiki";
	372	+ $middleKey = "{$prefix}_to_namespace";
	373	+
	374	+ $clause = $dbw->makeWhereFrom3d( $deletions, $baseKey, $middleKey, "{$prefix}_to_title" );
	375	+ if ( $clause ) {
	376	+ $where[] = $clause;
	377	+ } else {
	378	+ $where = false;
	379	+ }
	380	+
	381	+ if ( $where ) {
	382	+ $dbw->delete( $table, $where, __METHOD__ );
	383	+ }
	384	+ if ( count( $insertions ) ) {
	385	+ $dbw->insert( $table, $insertions, __METHOD__, 'IGNORE' );
	386	+ }
	387	+ }
	388	+ }
346	389
347	390	/**
348	391	* Get an array of pagelinks insertions for passing to the DB
—	—	@@ -383,6 +426,32 @@
384	427	}
385	428	return $arr;
386	429	}
	430	+
	431	+ /**
	432	+ * Get an array of distant template insertions. Like getLinkInsertions()
	433	+ * @private
	434	+ */
	435	+ function getDistantTemplateInsertions( $existing = array() ) {
	436	+ global $wgWikiID;
	437	+ $arr = array();
	438	+ foreach( $this->mDistantTemplates as $wikiid => $templatesToNS ) {
	439	+ foreach( $templatesToNS as $ns => $dbkeys ) {
	440	+ $diffs = isset( $existing[$wikiid] ) && isset( $existing[$wikiid][$ns] ) ? array_diff_key( $dbkeys, $existing[$wikiid][$ns] ) : $dbkeys;
	441	+ foreach ( $diffs as $dbk => $id ) {
	442	+ $arr[] = array(
	443	+ 'gtl_from_wiki' => $wgWikiID,
	444	+ 'gtl_from_page' => $this->mId,
	445	+ 'gtl_from_namespace' => $this->mTitle->getNsText(),
	446	+ 'gtl_from_title' => $this->mTitle->getText(),
	447	+ 'gtl_to_wiki' => $wikiid,
	448	+ 'gtl_to_namespace' => $ns,
	449	+ 'gtl_to_title' => $dbk
	450	+ );
	451	+ }
	452	+ }
	453	+ }
	454	+ return $arr;
	455	+ }
387	456
388	457	/**
389	458	* Get an array of image insertions
—	—	@@ -564,6 +633,30 @@
565	634	}
566	635	return $del;
567	636	}
	637	+
	638	+ /**
	639	+ * Given an array of existing templates, returns those templates which are not in $this
	640	+ * and thus should be deleted.
	641	+ * @private
	642	+ */
	643	+ function getDistantTemplateDeletions( $existing ) {
	644	+ $del = array();
	645	+ foreach ( $existing as $wikiid => $templatesForNS ) {
	646	+ if ( isset( $this->mDistantTemplates[$wikiid] ) ) {
	647	+ $del[$wikiid] = array_diff_key( $existing[$wikiid], $this->mDistantTemplates[$wikiid] );
	648	+ } else {
	649	+ $del[$wikiid] = $existing[$wikiid];
	650	+ }
	651	+ foreach ( $templatesForNS as $ns => $dbkeys ) {
	652	+ if ( isset( $this->mDistantTemplates[$wikiid][$ns] ) ) {
	653	+ $del[$wikiid][$ns] = array_diff_key( $existing[$wikiid][$ns], $this->mDistantTemplates[$wikiid][$ns] );
	654	+ } else {
	655	+ $del[$wikiid][$ns] = $existing[$wikiid][$ns];
	656	+ }
	657	+ }
	658	+ }
	659	+ return $del;
	660	+ }
568	661
569	662	/**
570	663	* Given an array of existing images, returns those images which are not in $this
—	—	@@ -661,6 +754,33 @@
662	755	$this->mDb->freeResult( $res );
663	756	return $arr;
664	757	}
	758	+
	759	+ /**
	760	+ * Get an array of existing distant templates, as a 3-D array
	761	+ * @private
	762	+ */
	763	+ function getDistantExistingTemplates() {
	764	+ global $wgWikiID;
	765	+ global $wgGlobalDB;
	766	+
	767	+ $arr = array();
	768	+ if ( $wgGlobalDB ) {
	769	+ $dbr = wfGetDB( DB_SLAVE, array(), $wgGlobalDB );
	770	+ $res = $dbr->select( 'globaltemplatelinks', array( 'gtl_to_wiki', 'gtl_to_namespace', 'gtl_to_title' ),
	771	+ array( 'gtl_from_wiki' => $wgWikiID, 'gtl_from_page' => $this->mId ), __METHOD__, $this->mOptions );
	772	+ while ( $row = $dbr->fetchObject( $res ) ) {
	773	+ if ( !isset( $arr[$row->gtl_to_wiki] ) ) {
	774	+ $arr[$row->gtl_to_wiki] = array();
	775	+ }
	776	+ if ( !isset( $arr[$row->gtl_to_wiki][$row->gtl_to_namespace] ) ) {
	777	+ $arr[$row->gtl_to_wiki][$row->gtl_to_namespace] = array();
	778	+ }
	779	+ $arr[$row->gtl_to_wiki][$row->gtl_to_namespace][$row->gtl_to_title] = 1;
	780	+ }
	781	+ $dbr->freeResult( $res );
	782	+ }
	783	+ return $arr;
	784	+ }
665	785
666	786	/**
667	787	* Get an array of existing images, image names in the keys
Index: branches/iwtransclusion/phase3/includes/Title.php
—	—	@@ -3057,7 +3057,7 @@
3058	3058	* @return \type{\mixed} true on success, getUserPermissionsErrors()-like array on failure
3059	3059	*/
3060	3060	public function moveTo( &$nt, $auth = true, $reason = '', $createRedirect = true ) {
3061		~~- global $wgContLang;~~
	3061	+ global $wgContLang, $wgGlobalDB, $wgWikiID;
3062	3062
3063	3063	$err = $this->isValidMoveOperation( $nt, $auth, $reason );
3064	3064	if ( is_array( $err ) ) {
—	—	@@ -3105,6 +3105,15 @@
3106	3106	'cl_timestamp=cl_timestamp' ),
3107	3107	array( 'cl_from' => $pageid ),
3108	3108	__METHOD__ );
	3109	+
	3110	+ if ( $wgGlobalDB ) {
	3111	+ $dbw2 = wfGetDB( DB_MASTER, array(), $wgGlobalDB );
	3112	+ $dbw2->update( 'globaltemplatelinks',
	3113	+ array( 'gtl_from_namespace' => $nt->getNsText(),
	3114	+ 'gtl_from_title' => $nt->getText() ),
	3115	+ array ( 'gtl_from_page' => $pageid ),
	3116	+ __METHOD__ );
	3117	+ }
3109	3118
3110	3119	if ( $protected ) {
3111	3120	# Protect the redirect title as the title used to be...
—	—	@@ -3198,7 +3207,7 @@
3199	3208	* Ignored if the user doesn't have the suppressredirect right
3200	3209	*/
3201	3210	private function moveOverExistingRedirect( &$nt, $reason = '', $createRedirect = true ) {
3202		~~- global $wgUseSquid, $wgUser, $wgContLang;~~
	3211	+ global $wgUseSquid, $wgUser, $wgContLang, $wgWikiID, $wgGlobalDB;
3203	3212
3204	3213	$comment = wfMsgForContent( '1movedto2_redir', $this->getPrefixedText(), $nt->getPrefixedText() );
3205	3214
—	—	@@ -3237,6 +3246,14 @@
3238	3247	$dbw->delete( 'externallinks', array( 'el_from' => $newid ), __METHOD__ );
3239	3248	$dbw->delete( 'langlinks', array( 'll_from' => $newid ), __METHOD__ );
3240	3249	$dbw->delete( 'redirect', array( 'rd_from' => $newid ), __METHOD__ );
	3250	+
	3251	+ if ( $wgGlobalDB ) {
	3252	+ $dbw2 = wfGetDB( DB_MASTER, array(), $wgGlobalDB );
	3253	+ $dbw2->delete( 'globaltemplatelinks',
	3254	+ array( 'gtl_from_wiki' => $wgWikiID,
	3255	+ 'gtl_from_page' => $newid ),
	3256	+ __METHOD__ );
	3257	+ }
3241	3258	}
3242	3259	// If the redirect was recently created, it may have an entry in recentchanges still
3243	3260	$dbw->delete( 'recentchanges',

Follow-up revisions

Revision	Commit summary	Author	Date
r70764	Fix remarks about r70576; display the distant templates below the edit textarea	peter17	14:46, 9 August 2010
r92992	Merge r70576...	reedy	18:03, 24 July 2011

Comments

#Comment by Catrope (talk | contribs) 13:36, 7 August 2010

-		global $wgDeferredUpdateList, $wgUseTrackbacks;
+		global $wgDeferredUpdateList, $wgUseTrackbacks, $wgGlobalDB, $wgWikiID;

If you're gonna introduce new settings like that, they have to be defined and documented in DefaultSettings.php . Also, $wgWikiID isn't needed because we have wfWikiID() for that. Moreover, $wgGlobalDB could use a more descriptive name, and the code should check that interwiki transclusion is enabled (another setting) AND global template links tracking is enabled (another setting) before messing with the globaltemplatelinks table.

+		$templateCb = array( 'Parser', 'distantTemplateCallback' );
+		$stuff = call_user_func( $templateCb, $title, $this );

This only makes sense if $templateCb is variable, which is the case in fetchTemplateAndTitle() but not here. Not sure whether it's supposed to be variable like that or not, will ask Tim.

+			if ( !isset( $this->mDistantTemplateIds[$wikiid][$ns] ) ) {
+				$this->mDistantTemplateIds[$wikiid][$ns] = array();
+			}

You need to check and set $this->mDistantTemplateIds[$wikiid] too, like you do with $this->mDistantTemplates[$wikiid].

+				if ( count( $sub ) ) {
+				$conds[] = $this->makeList(
+					array( $baseKey => $base, $middleKey => $middle, $subKey => array_keys( $sub ) ),
+					LIST_AND);

This code is misindented.

+			while ( $row = $dbr->fetchObject( $res ) ) {
[...]
+			$dbr->freeResult( $res );

You should use foreach ( $res as $row ) { and drop the freeResult() call.

Status & tagging log

09:43, 24 April 2011 Reedy (talk | contribs) changed the status of r70576 [removed: fixme added: resolved]
13:36, 7 August 2010 Catrope (talk | contribs) changed the status of r70576 [removed: new added: fixme]